Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Swap app bundles with APFS atomic swap if team IDs match #2516

Merged
merged 3 commits into from
Mar 3, 2024
Merged

Swap app bundles with APFS atomic swap if team IDs match #2516

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
6335b15
Perform safe atomic swap when team IDs match
zorgiepoo Feb 27, 2024
534227e
Rename parameter to perform rather than skip gatekeeper scan
zorgiepoo Mar 2, 2024
2f2c58e
Perform atomic swap if new update is adhoc signed / unsigned
zorgiepoo Mar 3, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion Autoupdate/SUCodeSigningVerifier.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@

#import <Foundation/Foundation.h>

NS_ASSUME_NONNULL_BEGIN

#ifndef BUILDING_SPARKLE_TESTS
SPU_OBJC_DIRECT_MEMBERS
#endif
Expand All @@ -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
17 changes: 1 addition & 16 deletions Autoupdate/SUCodeSigningVerifier.m
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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
72 changes: 43 additions & 29 deletions Autoupdate/SUPlainInstaller.m
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ @implementation SUPlainInstaller
NSURL *_temporaryNewDirectory;

BOOL _newAndOldBundlesOnSameVolume;
BOOL _canPerformSafeAtomicSwap;
}

- (instancetype)initWithHost:(SUHost *)host bundlePath:(NSString *)bundlePath installationPath:(NSString *)installationPath
Expand All @@ -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;
Expand Down Expand Up @@ -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,
Expand All @@ -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);
}
}
}
Expand Down Expand Up @@ -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) {
Expand All @@ -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:
Expand Down Expand Up @@ -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;
Expand Down
Loading