From 3a64116913bcd1fe50c7cb904e2110231f783e2c Mon Sep 17 00:00:00 2001 From: Zorg Date: Sat, 4 Dec 2021 22:59:17 -0800 Subject: [PATCH] Update and review internal documentation (#2030) --- Autoupdate/AppInstaller.m | 4 ++-- Autoupdate/SUCodeSigningVerifier.m | 2 +- Documentation/Installation.md | 16 +++++++--------- Documentation/Security.md | 2 +- Sparkle/SUUpdateValidator.m | 2 +- 5 files changed, 12 insertions(+), 14 deletions(-) diff --git a/Autoupdate/AppInstaller.m b/Autoupdate/AppInstaller.m index 4f98a88a88..e6fa864868 100644 --- a/Autoupdate/AppInstaller.m +++ b/Autoupdate/AppInstaller.m @@ -203,7 +203,7 @@ - (void)extractAndInstallUpdate self.updateValidator = [[SUUpdateValidator alloc] initWithDownloadPath:archivePath signatures:self.signatures host:self.host]; // Delta & package updates will require validation before extraction - // Normal application updates are a bit more lenient allowing developers to change one of apple dev ID or DSA keys + // Normal application updates are a bit more lenient allowing developers to change one of apple dev ID or EdDSA keys BOOL needsPrevalidation = [[unarchiver class] mustValidateBeforeExtraction] || ![self.installationType isEqualToString:SPUInstallationTypeApplication]; if (needsPrevalidation) { @@ -330,7 +330,7 @@ - (void)handleMessageWithIdentifier:(int32_t)identifier data:(NSData *)data if (identifier == SPUInstallationData && self.updateDirectoryPath == nil) { dispatch_async(dispatch_get_main_queue(), ^{ // Mark that we have received the installation data - // Do not rely on eg: self.ipdateDirectoryPath != nil because we may set it to nil again if an early stage fails (i.e, archive extraction) + // Do not rely on eg: self.updateDirectoryPath != nil because we may set it to nil again if an early stage fails (i.e, archive extraction) self.receivedInstallationData = YES; SPUInstallationInputData *installationData = (SPUInstallationInputData *)SPUUnarchiveRootObjectSecurely(data, [SPUInstallationInputData class]); diff --git a/Autoupdate/SUCodeSigningVerifier.m b/Autoupdate/SUCodeSigningVerifier.m index d5ebd67142..aa1f92fe5f 100644 --- a/Autoupdate/SUCodeSigningVerifier.m +++ b/Autoupdate/SUCodeSigningVerifier.m @@ -61,7 +61,7 @@ + (BOOL)codeSignatureAtBundleURL:(NSURL *)oldBundleURL matchesSignatureAtBundleU // Note that kSecCSCheckNestedCode may not work with pre-Mavericks code signing. // See https://github.com/sparkle-project/Sparkle/issues/376#issuecomment-48824267 and https://developer.apple.com/library/mac/technotes/tn2206 - // Aditionally, there are several reasons to stay away from deep verification and to prefer DSA signing the download archive instead. + // Aditionally, there are several reasons to stay away from deep verification and to prefer EdDSA signing the download archive instead. // See https://github.com/sparkle-project/Sparkle/pull/523#commitcomment-17549302 and https://github.com/sparkle-project/Sparkle/issues/543 SecCSFlags flags = (SecCSFlags) (kSecCSDefaultFlags | kSecCSCheckAllArchitectures); result = SecStaticCodeCheckValidityWithErrors(staticCode, flags, requirement, &cfError); diff --git a/Documentation/Installation.md b/Documentation/Installation.md index c4a1ab6ae0..2396da50ef 100644 --- a/Documentation/Installation.md +++ b/Documentation/Installation.md @@ -11,21 +11,21 @@ ## After downloading the update: ### Launching the Installer -SPUCoreBasedUpdateDriver invokes SPUInstallerDriver's extraction method which invokes SUInstallerLauncher's launch installer method. The launcher can be inside a XPC service or running inside the updater/sparkle framework itself. The launcher XPC bundle includes a copy of the installer and agent application in its bundle so it can compute the path to them itself (instead of relying on the updater to pass the paths), which is for security trust reasons. +SPUCoreBasedUpdateDriver invokes SPUInstallerDriver's extraction method which invokes SUInstallerLauncher's launch installer method. An XPC Service may be used to launch the installer if necessary for Sandboxed applications. First, if the updater driver that invoked the launcher doesn't allow for interaction (in particular, the automatic/silent update driver does not allow interaction), then the launcher fails with a hint to try again with interaction enabled (in particular, with a UI based update driver). If this happens, the updater keeps a reference to the downloaded update and appcast item that is used for resuming with a UI based update driver later. The launcher in this case is telling that authorization can only be done if interaction is allowed from the driver, so the user can enter their password to start up the installer as root to install over a location that requires such privileges. -The launcher then looks for the installer and agent application first in its bundle's auxiliary directory path. Once those paths are found, the launcher copies the progress agent application to a temporary cache location. The installer is not copied because it is a plain single executable that does not rely on external relative dependencies, whereas the agent application relies on bundle resources. This is important to consider because the old application may be moved around and removed when the installation process occurs. Leaving the installer inside its bundle also could leave it in a place the user doesn't necessarily have write privileges to. +The launcher then looks for the installer and agent application. If we are inside the XPC Service, a relative path back to the framework auxiliary directory is computed otherwise the bundle's auxiliary directory path is retrieved. Once those paths are found, the launcher copies the progress agent application to a temporary cache location. The installer is not copied because it is a plain single executable that does not rely on external relative dependencies, whereas the agent application relies on bundle resources. This is important to consider because the old application may be moved around and removed when the installation process occurs. Leaving the installer inside its bundle also could leave it in a place the user doesn't necessarily have write privileges to. The installer is first submitted. If it requires root privileges, it will ask for an admin user name and password. How it determines if root privileges are necessary is based on: * If the installation type from the appcast is an interactive package based installer, then no root privileges are necessary because the package installer will ask for them. Note this type of installation is deprecated however. * If the installation type from the appcast is a guided or regular package based installer, then root privileges are necessary because the system installer utility has to run as root. -* Otherwise the installation type is a normal application update, and root privileges are only needed if write permission or changing the owner/group is currently insufficient. +* Otherwise the installation type is a normal application update, and root privileges are only needed if write permission or changing the owner is currently insufficient. The installer makes sure, after extraction, that the expected installation type from the appcast matches the type of installation found within the archive. So this hint from the appcast is not just trusted implicitly. -If the user cancels submitting the installer on authorization (if it's required), the launcher aborts the installation process silently. If everything does alright, the progress agent application is submitted next. If either of the submissions fail, the installation fails and aborts, otherwise the launcher succeeds and its job is done. Note before the submissions are done, the jobs are removed from launchd, and the jobs are submitted in a way that act as "one-time" jobs. If they fail or succeed, the jobs aren't restarted, and the jobs aren't installed in a system location for launchd to attempt launching again. +If the user cancels submitting the installer on authorization (if it's required), the launcher aborts the installation process silently. If everything goes alright, the progress agent application is submitted next. If either of the submissions fail, the installation fails and aborts, otherwise the launcher succeeds and its job is done. Note before the submissions are done, the jobs are removed from launchd, and the jobs are submitted in a way that act as "one-time" jobs. If they fail or succeed, the jobs aren't restarted, and the jobs aren't installed in a system location for launchd to attempt launching again. The most important argument passed to the installer is the host bundle identifier of the bundle to update. This bundle identifier is used so that the installer and updater know what Mach service to connect to for IPC. Mostly everything else to the installer is sent via a XPC connection. The host bundle path is passed to the progress agent, which is just used for obtaining the bundle identifier, and for UI purposes (eg: displaying app icon and name when the app needs to show progress). The type of domain the installer is running in (user vs system) is also passed to the progress agent application so it can know which domain to use to connect to the installer. @@ -37,10 +37,8 @@ If the installer doesn't receive the "hello" message from the agent in a certain When the updater attempts to set up a connection to the installer, after a certain threshold, if the installation hasn't progressed from not beginning (i.e, extraction hasn't begun), then the updater aborts the installation. -Note, we don't have a "version tag" for the installer or agent because they are either bundled inside the framework or in the XPC service, and the XPC service already has a version check of its own. - ### Installation Data -After the installer launches, the updater creates a connection to the installer and sends installation data to the installer using the `SPUInstallationData` message. This data includes the bundle application path to relaunch, the path to the bundle to update and replace, DSA signature from the appcast item, decryption password for dmg if available, the path to the downloaded directory and name of the downloaded item, and the type of expected installation. +After the installer launches, the updater creates a connection to the installer and sends installation data to the installer using the `SPUInstallationData` message. This data includes the bundle application path to relaunch, the path to the bundle to update and replace, Ed(DSA) signature from the appcast item, decryption password for dmg if available, the path to the downloaded directory and name of the downloaded item, and the type of expected installation. The installer takes note of all this information, but it moves the downloaded item into a update directory it chooses for itself. This is for two reasons: @@ -52,7 +50,7 @@ After the installer moves the downloaded item, it makes sure the item is not a s ### Update Extraction After the installer receives the input installation data, it starts extracting the update. The installer first sends a `SPUExtractionStarted` message. -If the unarchiver requires DSA validation before extraction (binary delta archives do) or if the expected installation type is a package type, then before starting the unarchiver, validation is checked to make sure the signature for the archive is valid. If the signature is not valid, then a `SPUArchiveExtractionFailed` message is sent to the updater (see below on what happens after that). We can't require validation before extracting for normal application updates because they allow changes to DSA keys. Delta updates are fine however because if those fail, then a full update can be tried. +If the unarchiver requires EdDSA validation before extraction (binary delta archives do) or if the expected installation type is a package type, then before starting the unarchiver, validation is checked to make sure the signature for the archive is valid. If the signature is not valid, then a `SPUArchiveExtractionFailed` message is sent to the updater (see below on what happens after that). We can't require validation before extracting for normal application updates because they allow changes to EdDSA keys. Delta updates are fine however because if those fail, then a full update can be tried. After extraction starts, one or more `SPUExtractedArchiveWithProgress` messages indicating the unarchival progress are sent back to the updater. On failure, the installer will send `SPUArchiveExtractionFailed`. In the updater, if the update is a delta update and extraction fails, then the full archive is downloaded and we go back to the "Sending Installation Data" section to re-send the data and begin extraction again. If the update is not a delta update and extraction fails on the other hand, then the updater aborts causing the installer to abort as well. @@ -134,4 +132,4 @@ When the progress agent exits, it cleans up by removing itself from disk. The in * When we terminate the application before doing final installation work, we send an Apple quit event to all running application instances we find under the agent's GUI login session. This does not include running instances from other logged in sessions -- that may require authenticating all the time, so trade off is not worthwhile. Also currently the installer only handles a single process identifier to watch for until termination, and certainly doesn't handle the case if more instances are launched afterwards. Note all of this only applies to instances being launched from the same bundle path, and the consequence may rarely be updating a bundle when a running instance is still left alive. -* SMJobBless() would be interesting to explore and perhaps more suitable for sparkle-cli than ordinary application updaters because using this API is less approriate for one-off installations. There may be some issues to take account of though (eg: http://www.openradar.me/20446733). Furthermore, the installer would have to change to persist rather than be a one-time install, and may have to handle mutliple connections for installing multiple bundles simutaneously. +* SMJobBless() would be interesting to explore and perhaps more suitable for sparkle-cli than ordinary application updaters because using this API is less approriate for one-off installations. Furthermore, the installer would have to change to persist rather than be a one-time install, and may have to handle mutliple connections for installing multiple bundles simutaneously. diff --git a/Documentation/Security.md b/Documentation/Security.md index 4448ad9006..faa35d1222 100644 --- a/Documentation/Security.md +++ b/Documentation/Security.md @@ -32,4 +32,4 @@ For sending Obj-C objects across the wire, we use `SPUSecureCoding` because Coco The installer handles extraction, validation, and installation of the update. This was the main reason why other sandboxed-capable forks were rejected from being secure. The point to be stressed anyway is that these all need to be done in the same process; they can't be handed off by some other process. This is also why removing references to `AuthorizationExecuteWithPrivileges` is crucial as well. -Note that the installer does *not* handle downloading. That is handled by the updater framework. This is ideal because downloading can be done without disrupting the user, allowing it to be done silently without presenting an authorization dialog. The downloading portion of code can also be stuck into a XPC service with just an entitlement for allowing incoming connections. This also means for secure installations the installer cannot know (or trust) what protocol the update was downloaded from (i.e: http vs https). A good installer will not care, which is why applying updates without a DSA signature/key is now deprecated (reminder that Apple's code signature checks are not intended for complete integrity). \ No newline at end of file +Note that the installer does *not* handle downloading. That is handled by the updater framework. This is ideal because downloading can be done without disrupting the user, allowing it to be done silently without presenting an authorization dialog. The downloading portion of code can also be stuck into a XPC service with just an entitlement for allowing incoming connections. This also means for secure installations the installer cannot know (or trust) what protocol the update was downloaded from (i.e: http vs https). A good installer will not care, which is why applying updates without a EdDSA signature/key is now deprecated (reminder that Apple's code signature checks are not intended for complete integrity). diff --git a/Sparkle/SUUpdateValidator.m b/Sparkle/SUUpdateValidator.m index ff39c74036..4d66ae6ed2 100644 --- a/Sparkle/SUUpdateValidator.m +++ b/Sparkle/SUUpdateValidator.m @@ -127,7 +127,7 @@ - (BOOL)validateWithUpdateDirectory:(NSString *)updateDirectory error:(NSError * /** * If the update is a bundle, then it must meet any one of: * - * * old and new DSA public keys are the same and valid (it allows change of Code Signing identity), or + * * old and new Ed(DSA) public keys are the same and valid (it allows change of Code Signing identity), or * * * old and new Code Signing identity are the same and valid *