From 7a8ead559b09b6be5d6997effd324c6367a7a51c Mon Sep 17 00:00:00 2001 From: Gwynne Raskind Date: Tue, 6 Oct 2020 07:54:08 -0500 Subject: [PATCH 1/3] Clean up generate_keys/main.swift quite a bit - more comments, better output to console, cleaner error reporting --- generate_keys/main.swift | 255 ++++++++++++++++++++++----------------- 1 file changed, 142 insertions(+), 113 deletions(-) diff --git a/generate_keys/main.swift b/generate_keys/main.swift index 30f4dad991..5b37d408d5 100644 --- a/generate_keys/main.swift +++ b/generate_keys/main.swift @@ -9,31 +9,79 @@ import Foundation import Security +private func commonKeychainItemAttributes() -> [String: Any] { + /// Attributes used for both adding a new item and matching an existing one. + return [ + /// The type of the item (a generic password). + kSecClass as String: kSecClassGenericPassword as String, + + /// The service string for the item (the Sparkle homepage URL). + kSecAttrService as String: "https://sparkle-project.org", + + /// The account name for the item (in this case, the key type). + kSecAttrAccount as String: "ed25519", + + /// The protocol used by the service (not actually used, so we claim SSH). + kSecAttrProtocol as String: kSecAttrProtocolSSH as String, + ] +} + +private func failure(_ message: String) -> Never { + print("") + if ProcessInfo.processInfo.environment["TERM"] != nil && isatty(STDOUT_FILENO) != 0 { + print("\u{001b}[1;91mERROR:\u{001b}[0m ", terminator: "") + } else { + print("ERROR: ", terminator: "") + } + print(message) + exit(1) +} + func findPublicKey() -> Data? { var item: CFTypeRef? - let res = SecItemCopyMatching([ - kSecClass as String: kSecClassGenericPassword, - kSecAttrService as String: "https://sparkle-project.org", - kSecAttrAccount as String: "ed25519", - kSecAttrProtocol as String: kSecAttrProtocolSSH, -// kSecAttrSynchronizableAny as String: kCFBooleanTrue, + let res = SecItemCopyMatching(commonKeychainItemAttributes().merging([ + /// Return a matched item's value as a CFData object. kSecReturnData as String: kCFBooleanTrue!, - ] as CFDictionary, &item) + ], uniquingKeysWith: { $1 }) as CFDictionary, &item) - if res == errSecSuccess, let encoded = item as? Data, let keys = Data(base64Encoded: encoded) { -// print("OK! Read the existing key saved in the Keychain.") - return keys[64...] - } else if res == errSecItemNotFound { - return nil - } else if res == errSecAuthFailed { - print("\nERROR! Access denied. Can't check existing keys in the keychain.") - print("Go to Keychain Access.app, lock the login keychain, then unlock it again.") - } else if res == errSecUserCanceled { - print("\nABORTED! You've cancelled the request to read the key from the Keychain. Please run the tool again.") - } else if res == errSecInteractionNotAllowed { - print("\nERROR! The operating system has blocked access to the Keychain.") - } else { - print("\nERROR! Unable to access existing item in the Keychain", res, "(you can look it up at osstatus.com)") + switch res { + case errSecSuccess: + if let keys = (item as? Data).flatMap({ Data(base64Encoded: $0) }) { + return keys[64...] + } else { + failure(""" + Item found, but is corrupt or has been overwritten! + + Please delete the existing item from the keychain and try again. + """) + } + case errSecItemNotFound: + return nil + case errSecAuthFailed: + failure(""" + Access denied. Can't check existing keys in the keychain. + + Go to Keychain Access.app, lock the login keychain, then unlock it again. + """) + case errSecUserCanceled: + failure(""" + User canceled the authorization request. + + To retry, run this tool again. + """) + case errSecInteractionNotAllowed: + failure(""" + The operating system has blocked access to the Keychain. + + You may be trying to run this command from a script over SSH, which is not supported. + """) + case let res: + print(""" + Unable to access an existing item in the Keychain due to an unknown error: \(res). + + You can look up this error at + """) + // Note: Don't bother percent-encoding `res`, it's always an integer value and will not need escaping. } exit(1) } @@ -44,113 +92,94 @@ func generateKeyPair(makeSyncable: Bool) -> Data { var privateEdKey = Array(repeating: 0, count: 64) guard ed25519_create_seed(&seed) == 0 else { - print("\nERROR: Unable to initialize random seed") - exit(1) + failure("Unable to initialize random seed. Try restarting your computer.") } ed25519_create_keypair(&publicEdKey, &privateEdKey, seed) - let bothKeys = Data(privateEdKey) + Data(publicEdKey); // public key can't be derived from the private one - let query = [ - // macOS doesn't support ed25519 keys, so we're forced to save the key as a "password" - // and add some made-up service data for it to prevent it clashing with other passwords. - kSecClass as String: kSecClassGenericPassword, - kSecAttrService as String: "https://sparkle-project.org", - kSecAttrAccount as String: "ed25519", - - kSecValueData as String: bothKeys.base64EncodedData() as CFData, // it's base64-encoded, because user may request to show it + let query = commonKeychainItemAttributes().merging([ + /// Mark the new item as sensitive (requires keychain password to export - e.g. a private key). kSecAttrIsSensitive as String: kCFBooleanTrue!, + + /// Mark the new item as permanent (supposedly, "stored in the keychain when created", but not actually + /// used for generic passwords - we set it anyway for good measure). kSecAttrIsPermanent as String: kCFBooleanTrue!, - kSecAttrLabel as String: "Private key for signing Sparkle updates", - kSecAttrComment as String: "Public key (SUPublicEDKey value) for this key is:\n\n\(Data(publicEdKey).base64EncodedString())", + + /// The label of the new item (shown as its name/title in Keychain Access). + kSecAttrLabel as String: "Private key for signing Sparkle updates", + + /// A comment regarding the item's content (can be viewed in Keychain Access; we give the public key here). + kSecAttrComment as String: "Public key (SUPublicEDKey value) for this key is:\n\n\(Data(publicEdKey).base64EncodedString())", + + /// A short description of the item's contents (shown as "kind" in Keychain Access"). kSecAttrDescription as String: "private key", - -// kSecAttrSynchronizable as String: (makeSyncable ? kCFBooleanTrue : kCFBooleanFalse)!, - ] as CFDictionary + + /// The actual data content of the new item. + kSecValueData as String: Data(privateEdKey + publicEdKey).base64EncodedData() as CFData - let res = SecItemAdd(query, nil) - - if res == errSecSuccess { -// print("OK! A new key has been generated and saved in the Keychain.") - return Data(publicEdKey) - } else if res == errSecDuplicateItem { - print("\nERROR: You already have a previously generated key in the Keychain") - } else if res == errSecAuthFailed { - print("\nERROR: System denied access to the Keychain. Unable to save the new key") - print("Go to Keychain Access.app, lock the login keychain, then unlock it again.") - } else { - print("\nERROR: The key could not be saved to the Keychain. error: \(res) (you can look it up at osstatus.com)") + ], uniquingKeysWith: { $1 }) as CFDictionary + + switch SecItemAdd(query, nil) { + case errSecSuccess: + return Data(publicEdKey) + case errSecDuplicateItem: + failure("You already have a conflicting key in your Keychain which was not found during lookup.") + case errSecAuthFailed: + failure(""" + System denied access to the Keychain. Unable to save the new key. + Go to Keychain Access.app, lock the login keychain, then unlock it again. + """) + case let res: + failure(""" + The key could not be saved to the Keychain due to an unknown error: \(res). + + You can look up this error at + """) } exit(1) } -//let startEsc: String, endEsc: String -//if isatty(STDOUT_FILENO) != 0 { -// startEsc = "\u{001b}[91m" -// endEsc = "\u{001b}[m" -//} else { -// startEsc = "" -// endEsc = "" -//} -/* - If you have iCloud Keychain enabled, the key may optionally be marked as - syncable so it will be available on all devices logged into your iCloud account. - - \(startEsc)WARNING: Making a signing key syncable is NOT recommended!\(endEsc) - - The syncability option is provided because it provides a backup of the signing - key should your local account data become lost or corrupted; loss of the key makes - it impossible to release updates that will be accepted by Sparkle. - -*/ -print(""" - This tool uses the macOS Keychain to store a private key for signing app updates which - will be distributed via Sparkle. The key will be associated with your user account. - - Note: You only need one signing key, no matter how many apps you embed Sparkle in. - - The keychain may ask permission for this tool to access an existing key, if one - exists, or for permission to save the new key. You must allow access in order to - successfully proceed. - - """) - -if let pubKey = findPublicKey() { - print(""" - A pre-existing signing key was found. This is how it should appear in your Info.plist: +/// Once it's safe to require Swift 5.3 and Xcode 12 for this code, rename this file to `generate_keys.swift` and +/// replace this function with a class tagged with `@main`. +func entryPoint() { - SUPublicEDKey - \(pubKey.base64EncodedString()) - - """) -} else { -// print("A new signing key will be generated.") -// -// print(""" -// Do you want to allow syncing the key to iCloud? [y/N] -// """) -// var makeSyncable: Bool? = nil -// while makeSyncable == nil { -// guard let response = readLine(strippingNewline: true) else { fatalError("EOF on stdin; can not continue") } -// switch response.lowercased() { -// case "y", "yes": makeSyncable = true -// case "n", "no", "": makeSyncable = false -// default: print("Unknown response. Allow key to sync to iCloud? [y/N]") -// } -// } -// -// print("Generating a new signing key. This may take a moment, depending on your machine.") - - let pubKey = generateKeyPair(makeSyncable: false) - print(""" - A key has been generated and saved in your keychain. Add the `SUPublicEDKey` key to - the Info.plist of each app for which you intend to use Sparkle for distributing - updates. It should appear like this: + This tool uses the macOS Keychain to store a private key for signing app updates which + will be distributed via Sparkle. The key will be associated with your user account. + + Note: You only need one signing key, no matter how many apps you embed Sparkle in. - SUPublicEDKey - \(pubKey.base64EncodedString()) + The keychain may ask permission for this tool to access an existing key, if one + exists, or for permission to save the new key. You must allow access in order to + successfully proceed. """) + + if let pubKey = findPublicKey() { + print(""" + A pre-existing signing key was found. This is how it should appear in your Info.plist: + + SUPublicEDKey + \(pubKey.base64EncodedString()) + + """) + } else { + print("Generating a new signing key. This may take a moment, depending on your machine.") + + let pubKey = generateKeyPair(makeSyncable: false) + + print(""" + A key has been generated and saved in your keychain. Add the `SUPublicEDKey` key to + the Info.plist of each app for which you intend to use Sparkle for distributing + updates. It should appear like this: + + SUPublicEDKey + \(pubKey.base64EncodedString()) + + """) + } + + print("Done.") } -print("Done.") +// Dispatch to a function because `@main` isn't stable yet at the time of this writing and top-level code is finicky. +entryPoint() From 741ec1a08b20d2dd1e6feb27e414122616805c40 Mon Sep 17 00:00:00 2001 From: Gwynne Raskind Date: Tue, 6 Oct 2020 08:14:44 -0500 Subject: [PATCH 2/3] Further cleanup generate_keys and its new "lookup-only mode". --- generate_keys/main.swift | 78 +++++++++++++++++++++++----------------- 1 file changed, 46 insertions(+), 32 deletions(-) diff --git a/generate_keys/main.swift b/generate_keys/main.swift index 5b37d408d5..cca1dca99d 100644 --- a/generate_keys/main.swift +++ b/generate_keys/main.swift @@ -27,7 +27,7 @@ private func commonKeychainItemAttributes() -> [String: Any] { } private func failure(_ message: String) -> Never { - print("") + /// Checking for both `TERM` and `isatty()` correctly detects Xcode. if ProcessInfo.processInfo.environment["TERM"] != nil && isatty(STDOUT_FILENO) != 0 { print("\u{001b}[1;91mERROR:\u{001b}[0m ", terminator: "") } else { @@ -86,7 +86,7 @@ func findPublicKey() -> Data? { exit(1) } -func generateKeyPair(makeSyncable: Bool) -> Data { +func generateKeyPair() -> Data { var seed = Array(repeating: 0, count: 32) var publicEdKey = Array(repeating: 0, count: 32) var privateEdKey = Array(repeating: 0, count: 64) @@ -141,44 +141,58 @@ func generateKeyPair(makeSyncable: Bool) -> Data { /// Once it's safe to require Swift 5.3 and Xcode 12 for this code, rename this file to `generate_keys.swift` and /// replace this function with a class tagged with `@main`. func entryPoint() { + let isLookupMode = (CommandLine.arguments.dropFirst().first.map({ $0 == "-p" }) ?? false) - print(""" - This tool uses the macOS Keychain to store a private key for signing app updates which - will be distributed via Sparkle. The key will be associated with your user account. - - Note: You only need one signing key, no matter how many apps you embed Sparkle in. - - The keychain may ask permission for this tool to access an existing key, if one - exists, or for permission to save the new key. You must allow access in order to - successfully proceed. - - """) - - if let pubKey = findPublicKey() { + /// If not in lookup-only mode, give an intro blurb. + if !isLookupMode { print(""" - A pre-existing signing key was found. This is how it should appear in your Info.plist: - - SUPublicEDKey - \(pubKey.base64EncodedString()) - + This tool uses the macOS Keychain to store a private key for signing app updates which + will be distributed via Sparkle. The key will be associated with your user account. + + Note: You only need one signing key, no matter how many apps you embed Sparkle in. + + The keychain may ask permission for this tool to access an existing key, if one + exists, or for permission to save the new key. You must allow access in order to + successfully proceed. + """) - } else { - print("Generating a new signing key. This may take a moment, depending on your machine.") + } + + switch (findPublicKey(), isLookupMode) { + /// Existing key found, lookup mode - print just the pubkey and exit + case (.some(let pubKey), true): + print(pubKey.base64EncodedString()) - let pubKey = generateKeyPair(makeSyncable: false) + /// Existing key found, normal mode - print instructions blurb and pubkey + case (.some(let pubKey), false): + print(""" + A pre-existing signing key was found. This is how it should appear in your Info.plist: + + SUPublicEDKey + \(pubKey.base64EncodedString()) + + """) - print(""" - A key has been generated and saved in your keychain. Add the `SUPublicEDKey` key to - the Info.plist of each app for which you intend to use Sparkle for distributing - updates. It should appear like this: + /// No existing key, lookup mode - error out + case (.none, true): + failure("No existing signing key found!") + + /// No existing key, normal mode - generate a new one + case (.none, false): + print("Generating a new signing key. This may take a moment, depending on your machine.") - SUPublicEDKey - \(pubKey.base64EncodedString()) + let pubKey = generateKeyPair() - """) + print(""" + A key has been generated and saved in your keychain. Add the `SUPublicEDKey` key to + the Info.plist of each app for which you intend to use Sparkle for distributing + updates. It should appear like this: + + SUPublicEDKey + \(pubKey.base64EncodedString()) + + """) } - - print("Done.") } // Dispatch to a function because `@main` isn't stable yet at the time of this writing and top-level code is finicky. From 93e4fd9424ab174d3ad9c7a4c8d789188c6d0ff9 Mon Sep 17 00:00:00 2001 From: Gwynne Raskind Date: Mon, 12 Oct 2020 12:43:43 -0500 Subject: [PATCH 3/3] Fix generate_appcast so links to localized release notes are embedded as full URLs instead of unescaped filenames. This makes them load reliably in complex contexts (such as the presence of characters like spaces in the filenames). --- generate_appcast/ArchiveItem.swift | 23 ++++++++++++++--------- generate_appcast/FeedXML.swift | 2 +- 2 files changed, 15 insertions(+), 10 deletions(-) diff --git a/generate_appcast/ArchiveItem.swift b/generate_appcast/ArchiveItem.swift index 52a8181a5c..faed72ca08 100644 --- a/generate_appcast/ArchiveItem.swift +++ b/generate_appcast/ArchiveItem.swift @@ -187,21 +187,24 @@ class ArchiveItem: CustomStringConvertible { if self.getReleaseNotesAsHTMLFragment(path) != nil { return nil } - guard let escapedFilename = path.lastPathComponent.addingPercentEncoding(withAllowedCharacters: .urlPathAllowed) else { + return self.releaseNoteURL(for: path.lastPathComponent) + } + + func releaseNoteURL(for unescapedFilename: String) -> URL? { + guard let escapedFilename = unescapedFilename.addingPercentEncoding(withAllowedCharacters: .urlPathAllowed) else { return nil } - if let releaseNotesURLPrefix = self.releaseNotesURLPrefix { - // If a URL prefix for release notes was passed on the command-line, use it + // If a URL prefix for release notes was passed on the commandline, use it return URL(string: escapedFilename, relativeTo: releaseNotesURLPrefix) - } else if let relative = self.feedURL { - return URL(string: escapedFilename, relativeTo: relative) + } else if let relativeURL = self.feedURL { + return URL(string: escapedFilename, relativeTo: relativeURL) + } else { + return URL(string: escapedFilename) } - return URL(string: escapedFilename) } func localizedReleaseNotes() -> [(String, URL)] { - let fileManager = FileManager.default var basename = archivePath.deletingPathExtension() if basename.pathExtension == "tar" { basename = basename.deletingPathExtension() @@ -211,8 +214,10 @@ class ArchiveItem: CustomStringConvertible { let localizedReleaseNoteURL = basename .appendingPathExtension(languageCode) .appendingPathExtension("html") - if fileManager.fileExists(atPath: localizedReleaseNoteURL.path) { - localizedReleaseNotes.append((languageCode, localizedReleaseNoteURL)) + if (try? localizedReleaseNoteURL.checkResourceIsReachable()) ?? false, + let localizedReleaseNoteRemoteURL = self.releaseNoteURL(for: localizedReleaseNoteURL.lastPathComponent) + { + localizedReleaseNotes.append((languageCode, localizedReleaseNoteRemoteURL)) } } return localizedReleaseNotes diff --git a/generate_appcast/FeedXML.swift b/generate_appcast/FeedXML.swift index 07040b85b8..f84f12a345 100644 --- a/generate_appcast/FeedXML.swift +++ b/generate_appcast/FeedXML.swift @@ -140,7 +140,7 @@ func writeAppcast(appcastDestPath: URL, updates: [ArchiveItem]) throws { if !languageNotesNodes.contains(where: { $0.1 == language }) { let localizedNode = XMLNode.element( withName: SUAppcastElementReleaseNotesLink, - children: [XMLNode.text(withStringValue: url.lastPathComponent) as! XMLNode], + children: [XMLNode.text(withStringValue: url.absoluteString) as! XMLNode], attributes: [XMLNode.attribute(withName: SUXMLLanguage, stringValue: language) as! XMLNode]) item.addChild(localizedNode as! XMLNode) }