From 51ce7108c9b44457972840024a1e1b06cfe7459e Mon Sep 17 00:00:00 2001 From: Zorg Date: Sat, 2 Mar 2024 17:32:54 -0800 Subject: [PATCH] Swap app bundles with APFS atomic swap if team IDs match (#2516) --- Autoupdate/SUCodeSigningVerifier.h | 6 ++- Autoupdate/SUCodeSigningVerifier.m | 17 +------ Autoupdate/SUPlainInstaller.m | 72 ++++++++++++++++++------------ 3 files changed, 49 insertions(+), 46 deletions(-) diff --git a/Autoupdate/SUCodeSigningVerifier.h b/Autoupdate/SUCodeSigningVerifier.h index 82cfc2ac6..2615c0bbe 100644 --- a/Autoupdate/SUCodeSigningVerifier.h +++ b/Autoupdate/SUCodeSigningVerifier.h @@ -11,6 +11,8 @@ #import +NS_ASSUME_NONNULL_BEGIN + #ifndef BUILDING_SPARKLE_TESTS SPU_OBJC_DIRECT_MEMBERS #endif @@ -25,8 +27,10 @@ SPU_OBJC_DIRECT_MEMBERS + (BOOL)bundleAtURLIsCodeSigned:(NSURL *)bundleURL; -+ (BOOL)teamIdentifierAtURL:(NSURL *)url1 matchesTeamIdentifierAtURL:(NSURL *)url2; ++ (NSString * _Nullable)teamIdentifierAtURL:(NSURL *)url; @end +NS_ASSUME_NONNULL_END + #endif diff --git a/Autoupdate/SUCodeSigningVerifier.m b/Autoupdate/SUCodeSigningVerifier.m index 61b41a3f5..7c21eb8d5 100644 --- a/Autoupdate/SUCodeSigningVerifier.m +++ b/Autoupdate/SUCodeSigningVerifier.m @@ -258,7 +258,7 @@ + (BOOL)bundleAtURLIsCodeSigned:(NSURL *)bundleURL return (result == 0); } -static NSString * _Nullable teamIdentifierAtURL(NSURL *url) ++ (NSString * _Nullable)teamIdentifierAtURL:(NSURL *)url { SecStaticCodeRef staticCode = NULL; OSStatus staticCodeResult = SecStaticCodeCreateWithPath((__bridge CFURLRef)url, kSecCSDefaultFlags, &staticCode); @@ -282,19 +282,4 @@ + (BOOL)bundleAtURLIsCodeSigned:(NSURL *)bundleURL return signingInformation[(NSString *)kSecCodeInfoTeamIdentifier]; } -+ (BOOL)teamIdentifierAtURL:(NSURL *)url1 matchesTeamIdentifierAtURL:(NSURL *)url2 -{ - NSString *teamIdentifierForURL1 = teamIdentifierAtURL(url1); - if (teamIdentifierForURL1 == nil) { - return NO; - } - - NSString *teamIdentifierForURL2 = teamIdentifierAtURL(url2); - if (teamIdentifierForURL2 == nil) { - return NO; - } - - return [teamIdentifierForURL1 isEqualToString:teamIdentifierForURL2]; -} - @end diff --git a/Autoupdate/SUPlainInstaller.m b/Autoupdate/SUPlainInstaller.m index 7dbfbd34b..ba6cf9ce5 100644 --- a/Autoupdate/SUPlainInstaller.m +++ b/Autoupdate/SUPlainInstaller.m @@ -29,6 +29,7 @@ @implementation SUPlainInstaller NSURL *_temporaryNewDirectory; BOOL _newAndOldBundlesOnSameVolume; + BOOL _canPerformSafeAtomicSwap; } - (instancetype)initWithHost:(SUHost *)host bundlePath:(NSString *)bundlePath installationPath:(NSString *)installationPath @@ -42,7 +43,7 @@ - (instancetype)initWithHost:(SUHost *)host bundlePath:(NSString *)bundlePath in return self; } -- (void)_performInitialInstallationWithFileManager:(SUFileManager *)fileManager oldBundleURL:(NSURL *)oldBundleURL newBundleURL:(NSURL *)newBundleURL skipGatekeeperScan:(BOOL)skipGatekeeperScan progressBlock:(nullable void(^)(double))progress SPU_OBJC_DIRECT +- (void)_performInitialInstallationWithFileManager:(SUFileManager *)fileManager oldBundleURL:(NSURL *)oldBundleURL newBundleURL:(NSURL *)newBundleURL performGatekeeperScan:(BOOL)performGatekeeperScan progressBlock:(nullable void(^)(double))progress SPU_OBJC_DIRECT { // Release our new app from quarantine NSError *quarantineError = nil; @@ -90,7 +91,7 @@ - (void)_performInitialInstallationWithFileManager:(SUFileManager *)fileManager progress(8/11.0); } - if (!skipGatekeeperScan) { + if (performGatekeeperScan) { // Perform a Gatekeeper scan to pre-warm the app launch // This avoids users seeing a "Verifying..." dialog when the installed update is launched // Note the tool we use to perform the Gatekeeper scan (gktool) is technically available on macOS 14.0, @@ -100,32 +101,23 @@ - (void)_performInitialInstallationWithFileManager:(SUFileManager *)fileManager // Only perform Gatekeeper scan if we're updating an app bundle NSString *newBundlePath = newBundleURL.path; if ([newBundlePath.pathExtension caseInsensitiveCompare:@"app"] == NSOrderedSame) { - // We only invoke gktool if Autoupdate is signed with the same team identifier as the new update bundle - // Otherwise we may unfortunately run into some Privacy & Security prompt bugs in the OS (note this is *not* a security check) - // This does overall imply that for an app to test the gktool path, this path may often skipped for most common development workflows that don't - // re-sign Sparkle's Autoupdate helper - NSURL *mainExecutableURL = NSBundle.mainBundle.executableURL; - if (mainExecutableURL != nil && [SUCodeSigningVerifier teamIdentifierAtURL:mainExecutableURL matchesTeamIdentifierAtURL:newBundleURL]) { - NSURL *gktoolURL = [NSURL fileURLWithPath:@"/usr/bin/gktool" isDirectory:NO]; - if ([gktoolURL checkResourceIsReachableAndReturnError:NULL]) { - NSTask *gatekeeperScanTask = [[NSTask alloc] init]; - gatekeeperScanTask.executableURL = gktoolURL; - gatekeeperScanTask.arguments = @[@"scan", newBundlePath]; + NSURL *gktoolURL = [NSURL fileURLWithPath:@"/usr/bin/gktool" isDirectory:NO]; + if ([gktoolURL checkResourceIsReachableAndReturnError:NULL]) { + NSTask *gatekeeperScanTask = [[NSTask alloc] init]; + gatekeeperScanTask.executableURL = gktoolURL; + gatekeeperScanTask.arguments = @[@"scan", newBundlePath]; - NSError *taskError; - if (![gatekeeperScanTask launchAndReturnError:&taskError]) { - // Not a fatal error - SULog(SULogLevelError, @"Failed to perform GateKeeper scan on '%@' with error %@", newBundlePath, taskError); - } else { - [gatekeeperScanTask waitUntilExit]; - - if (gatekeeperScanTask.terminationStatus != 0) { - SULog(SULogLevelError, @"gktool failed and returned exit status %d", gatekeeperScanTask.terminationStatus); - } + NSError *taskError; + if (![gatekeeperScanTask launchAndReturnError:&taskError]) { + // Not a fatal error + SULog(SULogLevelError, @"Failed to perform GateKeeper scan on '%@' with error %@", newBundlePath, taskError); + } else { + [gatekeeperScanTask waitUntilExit]; + + if (gatekeeperScanTask.terminationStatus != 0) { + SULog(SULogLevelError, @"gktool failed and returned exit status %d", gatekeeperScanTask.terminationStatus); } } - } else { - SULog(SULogLevelDefault, @"Skipping invocation of gktool because Autoupdate is not signed with same identity as the new update %@", newBundleURL.lastPathComponent); } } } @@ -233,7 +225,7 @@ - (BOOL)startInstallationToURL:(NSURL *)installationURL fromUpdateAtURL:(NSURL * if (!_newAndOldBundlesOnSameVolume) { // If we're updating a bundle on another volume, the install process can be pretty slow. // In this case let's get out of the way and skip the Gatekeeper scan - [self _performInitialInstallationWithFileManager:fileManager oldBundleURL:oldURL newBundleURL:newFinalURL skipGatekeeperScan:YES progressBlock:progress]; + [self _performInitialInstallationWithFileManager:fileManager oldBundleURL:oldURL newBundleURL:newFinalURL performGatekeeperScan:NO progressBlock:progress]; } if (progress) { @@ -243,8 +235,8 @@ - (BOOL)startInstallationToURL:(NSURL *)installationURL fromUpdateAtURL:(NSURL * // First try swapping the application atomically NSError *swapError = nil; BOOL swappedApp; - // If the app is normalized and the installation path differs, go through the old swap path - if (SPARKLE_NORMALIZE_INSTALLED_APPLICATION_NAME && ![oldURL.path isEqual:installationURL.path]) { + // If we can not safely perform an atomic swap, or if the app is normalized and the installation path differs, go through the old swap path + if (!_canPerformSafeAtomicSwap || (SPARKLE_NORMALIZE_INSTALLED_APPLICATION_NAME && ![oldURL.path isEqual:installationURL.path])) { swappedApp = NO; } else { // We will be cleaning up the temporary directory later in -performCleanup: @@ -352,11 +344,33 @@ - (BOOL)performInitialInstallation:(NSError * __autoreleasing *)error SUFileManager *fileManager = [[SUFileManager alloc] init]; + if (@available(macOS 13.0, *)) { + NSURL *mainExecutableURL = NSBundle.mainBundle.executableURL; + if (mainExecutableURL == nil) { + // This shouldn't happen + _canPerformSafeAtomicSwap = NO; + } else { + NSString *installerTeamIdentifier = [SUCodeSigningVerifier teamIdentifierAtURL:mainExecutableURL]; + NSString *bundleTeamIdentifier = [SUCodeSigningVerifier teamIdentifierAtURL:bundle.bundleURL]; + + // If the new update is code signed and Autoupdate is not signed with the same Team ID as the new update, + // then we may run into Privacy & Security prompt issues from the OS + // To avoid these, we skip the gatekeeper scan and skip performing an atomic swap during install + _canPerformSafeAtomicSwap = (bundleTeamIdentifier == nil || (installerTeamIdentifier != nil && [installerTeamIdentifier isEqualToString:bundleTeamIdentifier])); + } + } else { + _canPerformSafeAtomicSwap = YES; + } + + if (!_canPerformSafeAtomicSwap) { + SULog(SULogLevelDefault, @"Skipping atomic rename/swap and gatekeeper scan because Autoupdate is not signed with same identity as the new update %@", bundle.bundleURL.lastPathComponent); + } + _newAndOldBundlesOnSameVolume = [fileManager itemAtURL:bundle.bundleURL isOnSameVolumeItemAsURL:_host.bundle.bundleURL]; // We can do a lot of the installation work ahead of time if the new app update does not need to be copied to another volume if (_newAndOldBundlesOnSameVolume) { - [self _performInitialInstallationWithFileManager:fileManager oldBundleURL:_host.bundle.bundleURL newBundleURL:bundle.bundleURL skipGatekeeperScan:NO progressBlock:NULL]; + [self _performInitialInstallationWithFileManager:fileManager oldBundleURL:_host.bundle.bundleURL newBundleURL:bundle.bundleURL performGatekeeperScan:_canPerformSafeAtomicSwap progressBlock:NULL]; } return YES;