Add option to verify updates before extraction (#2667)

Adds an opt-in option (SUVerifyUpdateBeforeExtraction) to enforce verifying updates before extracting them for stronger security. EdDSA signing is required to use this option. As fallback in case EdDSA keys are lost, disk image archives's code signatures are validated assuming it's Developer ID signed. Key rotation is still possible. Apple Archives (aar, yaa) now require using this option.
sparkle-project · Dec 9, 2024 · 1ca60d5 · 1ca60d5
1 parent 597825d
commit 1ca60d5
Show file tree

Hide file tree

Showing 21 changed files with 411 additions and 62 deletions.
diff --git a/Autoupdate/AppInstaller.m b/Autoupdate/AppInstaller.m
@@ -176,10 +176,10 @@ - (void)extractAndInstallUpdate SPU_OBJC_DIRECT
 
     id<SUUnarchiverProtocol> unarchiver = [SUUnarchiver unarchiverForPath:archivePath extractionDirectory:_extractionDirectory updatingHostBundlePath:_host.bundlePath decryptionPassword:_decryptionPassword expectingInstallationType:_installationType];
 
-    NSError *unarchiverError = nil;
+    NSError *prevalidationError = nil;
     BOOL success = NO;
     if (!unarchiver) {
-        unarchiverError = [NSError errorWithDomain:SUSparkleErrorDomain code:SUUnarchivingError userInfo:@{ NSLocalizedDescriptionKey: [NSString stringWithFormat:@"No valid unarchiver was found for %@", archivePath] }];
+        prevalidationError = [NSError errorWithDomain:SUSparkleErrorDomain code:SUUnarchivingError userInfo:@{ NSLocalizedDescriptionKey: [NSString stringWithFormat:@"No valid unarchiver was found for %@", archivePath] }];
 
         success = NO;
     } else {
@@ -192,20 +192,38 @@ - (void)extractAndInstallUpdate SPU_OBJC_DIRECT
         }
 
         _updateValidator = [[SUUpdateValidator alloc] initWithDownloadPath:archivePath signatures:_signatures host:_host verifierInformation:_verifierInformation];
-
-        // Delta, package updates, and .aar/.yaa archives will require validation before extraction
-        // Normal application updates are a bit more lenient allowing developers to change one of apple dev ID or EdDSA keys
-        BOOL needsPrevalidation = [[unarchiver class] mustValidateBeforeExtractionWithArchivePath:archivePath] || ![_installationType isEqualToString:SPUInstallationTypeApplication];
-
-        if (needsPrevalidation) {
-            success = [_updateValidator validateDownloadPathWithError:&unarchiverError];
+        
+        // More uncommon archives types (.aar, .yaa) need SUVerifyUpdateBeforeExtraction
+        BOOL verifyBeforeExtraction = [_host boolForInfoDictionaryKey:SUVerifyUpdateBeforeExtractionKey];
+        if (!verifyBeforeExtraction && unarchiver.needsVerifyBeforeExtractionKey) {
+            prevalidationError = [NSError errorWithDomain:SUSparkleErrorDomain code:SUValidationError userInfo:@{ NSLocalizedDescriptionKey: [NSString stringWithFormat:@"Extracting %@ archives require setting %@ to YES in the old app. Please visit https://sparkle-project.org/documentation/customization/ for more information.", archivePath.pathExtension, SUVerifyUpdateBeforeExtractionKey] }];
+            
+            success = NO;
         } else {
-            success = YES;
+            // Delta, package updates, and apps with SUVerifyUpdateBeforeExtraction will require validation before extraction
+            // Otherwise normal application updates are a bit more lenient allowing developers to change one of apple dev ID or EdDSA keys after extraction
+            BOOL archiveTypeMustValidateBeforeExtraction = [[unarchiver class] mustValidateBeforeExtraction];
+            BOOL needsPrevalidation = verifyBeforeExtraction || archiveTypeMustValidateBeforeExtraction || ![_installationType isEqualToString:SPUInstallationTypeApplication];
+
+            if (needsPrevalidation) {
+                // EdDSA signing is required, so host must have public keys
+                if (![_updateValidator validateHostHasPublicKeys:&prevalidationError]) {
+                    success = NO;
+                } else {
+                    // Falling back on code signing for prevalidation requires SUVerifyUpdateBeforeExtraction
+                    // and that update is a regular app update, and not a delta update
+                    BOOL fallbackOnCodeSigning = (verifyBeforeExtraction && !archiveTypeMustValidateBeforeExtraction && [_installationType isEqualToString:SPUInstallationTypeApplication]);
+
+                    success = [_updateValidator validateDownloadPathWithFallbackOnCodeSigning:fallbackOnCodeSigning error:&prevalidationError];
+                }
+            } else {
+                success = YES;
+            }
         }
     }
 
     if (!success) {
-        [self unarchiverDidFailWithError:unarchiverError];
+        [self unarchiverDidFailWithError:prevalidationError];
     } else {
         [unarchiver
          unarchiveWithCompletionBlock:^(NSError * _Nullable error) {

diff --git a/Autoupdate/SUBinaryDeltaUnarchiver.m b/Autoupdate/SUBinaryDeltaUnarchiver.m
@@ -28,7 +28,7 @@ + (BOOL)canUnarchivePath:(NSString *)path
     return [[path pathExtension] isEqualToString:@"delta"];
 }
 
-+ (BOOL)mustValidateBeforeExtractionWithArchivePath:(NSString *)archivePath
++ (BOOL)mustValidateBeforeExtraction
 {
     return YES;
 }
@@ -83,6 +83,11 @@ - (instancetype)initWithArchivePath:(NSString *)archivePath extractionDirectory:
     return self;
 }
 
+- (BOOL)needsVerifyBeforeExtractionKey
+{
+    return NO;
+}
+
 - (void)unarchiveWithCompletionBlock:(void (^)(NSError * _Nullable))completionBlock progressBlock:(void (^ _Nullable)(double))progressBlock waitForCleanup:(BOOL)__unused waitForCleanup
 {
     dispatch_async(dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_DEFAULT, 0), ^{

diff --git a/Autoupdate/SUCodeSigningVerifier.h b/Autoupdate/SUCodeSigningVerifier.h
@@ -29,6 +29,8 @@ SUCodeSigningVerifierDefinitionAttribute
 // Same as above except does not check for nested code. This method should be used by the framework.
 + (BOOL)codeSignatureIsValidAtBundleURL:(NSURL *)bundleURL error:(NSError *__autoreleasing *)error;
 
++ (BOOL)codeSignatureIsValidAtDownloadURL:(NSURL *)downloadURL andMatchesDeveloperIDTeamFromOldBundleURL:(NSURL *)oldBundleURL error:(NSError * __autoreleasing *)error;
+
 + (BOOL)bundleAtURLIsCodeSigned:(NSURL *)bundleURL;
 
 + (NSString * _Nullable)teamIdentifierAtURL:(NSURL *)url;

diff --git a/Autoupdate/SUCodeSigningVerifier.m b/Autoupdate/SUCodeSigningVerifier.m
@@ -26,12 +26,17 @@ + (BOOL)codeSignatureIsValidAtBundleURL:(NSURL *)newBundleURL andMatchesSignatur
     CFErrorRef cfError = NULL;
 
     result = SecStaticCodeCreateWithPath((__bridge CFURLRef)oldBundleURL, kSecCSDefaultFlags, &oldCode);
-    if (result == errSecCSUnsigned) {
+    if (result != noErr) {
         if (error != NULL) {
-            *error = [NSError errorWithDomain:SUSparkleErrorDomain code:SUInsufficientSigningError userInfo:@{ NSLocalizedDescriptionKey: [NSString stringWithFormat:@"Bundle is not code signed: %@", newBundleURL] }];
+            NSString *errorMessage =
+                (result == errSecCSUnsigned) ?
+                [NSString stringWithFormat:@"Bundle is not code signed: %@", oldBundleURL.path] :
+                [NSString stringWithFormat:@"Failed to get static code (%d): %@", result, oldBundleURL.path];
+
+            *error = [NSError errorWithDomain:SUSparkleErrorDomain code:SUInsufficientSigningError userInfo:@{ NSLocalizedDescriptionKey: errorMessage }];
         }
 
-        return NO;
+        goto finally;
     }
 
     result = SecCodeCopyDesignatedRequirement(oldCode, kSecCSDefaultFlags, &requirement);
@@ -258,15 +263,8 @@ + (BOOL)bundleAtURLIsCodeSigned:(NSURL *)bundleURL
     return (result == 0);
 }
 
-+ (NSString * _Nullable)teamIdentifierAtURL:(NSURL *)url
+static NSString * _Nullable SUTeamIdentifierFromStaticCode(SecStaticCodeRef staticCode)
 {
-    SecStaticCodeRef staticCode = NULL;
-    OSStatus staticCodeResult = SecStaticCodeCreateWithPath((__bridge CFURLRef)url, kSecCSDefaultFlags, &staticCode);
-    if (staticCodeResult != noErr) {
-        SULog(SULogLevelError, @"Failed to get static code for retrieving team identifier: %d", staticCodeResult);
-        return nil;
-    }
-
     CFDictionaryRef cfSigningInformation = NULL;
     OSStatus copySigningInfoCode = SecCodeCopySigningInformation(staticCode, kSecCSSigningInformation,
         &cfSigningInformation);
@@ -282,4 +280,145 @@ + (NSString * _Nullable)teamIdentifierAtURL:(NSURL *)url
     return signingInformation[(NSString *)kSecCodeInfoTeamIdentifier];
 }
 
++ (NSString * _Nullable)teamIdentifierAtURL:(NSURL *)url
+{
+    SecStaticCodeRef staticCode = NULL;
+    OSStatus staticCodeResult = SecStaticCodeCreateWithPath((__bridge CFURLRef)url, kSecCSDefaultFlags, &staticCode);
+    if (staticCodeResult != noErr) {
+        SULog(SULogLevelError, @"Failed to get static code for retrieving team identifier: %d", staticCodeResult);
+        return nil;
+    }
+
+    NSString *teamIdentifier = SUTeamIdentifierFromStaticCode(staticCode);
+
+    if (staticCode != NULL) {
+        CFRelease(staticCode);
+    }
+
+    return teamIdentifier;
+}
+
++ (BOOL)codeSignatureIsValidAtDownloadURL:(NSURL *)downloadURL andMatchesDeveloperIDTeamFromOldBundleURL:(NSURL *)oldBundleURL error:(NSError * __autoreleasing *)error
+{
+    NSString *teamIdentifier = nil;
+    NSString *requirementString = nil;
+    SecRequirementRef requirement = NULL;
+    SecStaticCodeRef oldStaticCode = NULL;
+    SecStaticCodeRef downloadStaticCode = NULL;
+    OSStatus result;
+
+    NSError *resultError = nil;
+
+    NSString *commonErrorMessage = @"The download archive cannot be validated with Apple Developer ID code signing as fallback (after (Ed)DSA verification has failed)";
+
+    result = SecStaticCodeCreateWithPath((__bridge CFURLRef)oldBundleURL, kSecCSDefaultFlags, &oldStaticCode);
+    if (result != errSecSuccess) {
+        NSString *errorMessage =
+            (result == errSecCSUnsigned) ?
+            [NSString stringWithFormat:@"%@. The original app is not code signed: %@", commonErrorMessage, oldBundleURL.path] :
+            [NSString stringWithFormat:@"%@. The static code could not be retrieved from the original app (%d): %@", commonErrorMessage, result, oldBundleURL.path];
+
+        resultError = [NSError errorWithDomain:SUSparkleErrorDomain code:SUInsufficientSigningError userInfo:@{ NSLocalizedDescriptionKey: errorMessage }];
+
+        goto finally;
+    }
+
+    teamIdentifier = SUTeamIdentifierFromStaticCode(oldStaticCode);
+    if (teamIdentifier == nil) {
+        resultError = [NSError errorWithDomain:SUSparkleErrorDomain code:SUInsufficientSigningError userInfo:@{ NSLocalizedDescriptionKey: [NSString stringWithFormat:@"%@. The team identifier could not be retrieved from the original app: %@", commonErrorMessage, oldBundleURL.path] }];
+
+        goto finally;
+    }
+
+    // Create a designated requirement with developer ID signing with this team ID
+    // Validate it against code signing check of this archive
+    // CertificateIssuedByApple = anchor apple generic
+    // IssuerIsDeveloperID = certificate 1[field.1.2.840.113635.100.6.2.6]
+    // LeafIsDeveloperIDApp = certificate leaf[field.1.2.840.113635.100.6.1.13]
+    // DeveloperIDTeamID = certificate leaf[subject.OU]
+    // https://developer.apple.com/documentation/technotes/tn3127-inside-code-signing-requirements#Xcode-designated-requirement-for-Developer-ID-code
+    requirementString = [NSString stringWithFormat:@"anchor apple generic and certificate 1[field.1.2.840.113635.100.6.2.6] and certificate leaf[field.1.2.840.113635.100.6.1.13] and certificate leaf[subject.OU] = %@", teamIdentifier];
+
+    result = SecRequirementCreateWithString((__bridge CFStringRef)requirementString, kSecCSDefaultFlags, &requirement);
+    if (result != errSecSuccess) {
+        resultError = [NSError errorWithDomain:SUSparkleErrorDomain code:SUInsufficientSigningError userInfo:@{ NSLocalizedDescriptionKey: [NSString stringWithFormat:@"%@. The designated requirement string with a Developer ID requirement with team identifier '%@' could not be created with error %d", commonErrorMessage, teamIdentifier, result] }];
+
+        goto finally;
+    }
+
+    result = SecStaticCodeCreateWithPath((__bridge CFURLRef)downloadURL, kSecCSDefaultFlags, &downloadStaticCode);
+    if (result != errSecSuccess) {
+        NSString *message = [NSString stringWithFormat:@"%@. The static code could not be retrieved from the download archive with error %d. The download archive may not be Apple code signed.", commonErrorMessage, result];
+
+        SULog(SULogLevelError, @"%@", message);
+
+        resultError = [NSError errorWithDomain:SUSparkleErrorDomain code:SUInsufficientSigningError userInfo:@{ NSLocalizedDescriptionKey: message }];
+
+        goto finally;
+    }
+
+    SecCSFlags flags = (SecCSFlags)kSecCSDefaultFlags;
+    CFErrorRef cfError = NULL;
+    result = SecStaticCodeCheckValidityWithErrors(downloadStaticCode, flags, requirement, &cfError);
+    if (result != errSecSuccess) {
+        NSError *underlyingError;
+        if (cfError != NULL) {
+            NSError *tmpError = CFBridgingRelease(cfError);
+            underlyingError = tmpError;
+        } else {
+            underlyingError = nil;
+        }
+
+        NSMutableDictionary *userInfo = [NSMutableDictionary dictionary];
+        if (underlyingError != nil) {
+            userInfo[NSUnderlyingErrorKey] = underlyingError;
+        }
+
+        if (result == errSecCSUnsigned) {
+            NSString *message = [NSString stringWithFormat:@"%@. The download archive is not Apple code signed.", commonErrorMessage];
+
+            SULog(SULogLevelError, @"%@", message);
+
+            userInfo[NSLocalizedDescriptionKey] = message;
+
+            resultError = [NSError errorWithDomain:SUSparkleErrorDomain code:SUInsufficientSigningError userInfo:[userInfo copy]];
+        } else if (result == errSecCSReqFailed) {
+            NSString *initialMessage = [NSString stringWithFormat:@"%@. The Apple code signature of new downloaded archive is either not Developer ID code signed, or doesn't have a Team ID that matches the old app version (%@). Please ensure that the archive and app are signed using the same Developer ID certificate.", commonErrorMessage, teamIdentifier];
+
+            NSDictionary *oldInfo = [self logSigningInfoForCode:oldStaticCode label:@"old info"];
+            NSDictionary *newInfo = [self logSigningInfoForCode:downloadStaticCode label:@"new info"];
+
+            userInfo[NSLocalizedDescriptionKey] = [NSString stringWithFormat:@"%@ old info: %@. new info: %@", initialMessage, oldInfo, newInfo];
+
+            resultError = [NSError errorWithDomain:SUSparkleErrorDomain code:SUInsufficientSigningError userInfo:[userInfo copy]];
+        } else {
+            userInfo[NSLocalizedDescriptionKey] = [NSString stringWithFormat:@"%@. The downloaded archive code signing signature failed to validate with an unknown error (%d).", commonErrorMessage, result];
+
+            resultError = [NSError errorWithDomain:SUSparkleErrorDomain code:SUInsufficientSigningError userInfo:[userInfo copy]];
+        }
+
+        goto finally;
+    }
+
+finally:
+
+    if (oldStaticCode != NULL) {
+        CFRelease(oldStaticCode);
+    }
+
+    if (requirement != NULL) {
+        CFRelease(requirement);
+    }
+
+    if (downloadStaticCode != NULL) {
+        CFRelease(downloadStaticCode);
+    }
+
+    if (resultError != nil && error != NULL) {
+        *error = resultError;
+    }
+
+    return (resultError == nil);
+}
+
 @end
diff --git a/Autoupdate/SUDiskImageUnarchiver.m b/Autoupdate/SUDiskImageUnarchiver.m
@@ -35,7 +35,7 @@ + (BOOL)canUnarchivePath:(NSString *)path
     return [[path pathExtension] isEqualToString:@"dmg"];
 }
 
-+ (BOOL)mustValidateBeforeExtractionWithArchivePath:(NSString *)archivePath
++ (BOOL)mustValidateBeforeExtraction
 {
     return NO;
 }
@@ -51,6 +51,11 @@ - (instancetype)initWithArchivePath:(NSString *)archivePath extractionDirectory:
     return self;
 }
 
+- (BOOL)needsVerifyBeforeExtractionKey
+{
+    return NO;
+}
+
 - (void)unarchiveWithCompletionBlock:(void (^)(NSError * _Nullable))completionBlock progressBlock:(void (^ _Nullable)(double))progressBlock waitForCleanup:(BOOL)waitForCleanup
 {
     dispatch_async(dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_DEFAULT, 0), ^{

diff --git a/Autoupdate/SUFlatPackageUnarchiver.m b/Autoupdate/SUFlatPackageUnarchiver.m
@@ -28,7 +28,7 @@ + (BOOL)canUnarchivePath:(NSString *)path
     return [path.pathExtension isEqualToString:@"pkg"] || [path.pathExtension isEqualToString:@"mpkg"];
 }
 
-+ (BOOL)mustValidateBeforeExtractionWithArchivePath:(NSString *)archivePath
++ (BOOL)mustValidateBeforeExtraction
 {
     return YES;
 }
@@ -44,6 +44,11 @@ - (instancetype)initWithFlatPackagePath:(NSString *)flatPackagePath extractionDi
     return self;
 }
 
+- (BOOL)needsVerifyBeforeExtractionKey
+{
+    return NO;
+}
+
 - (void)unarchiveWithCompletionBlock:(void (^)(NSError * _Nullable))completionBlock progressBlock:(void (^ _Nullable)(double))progressBlock waitForCleanup:(BOOL)__unused waitForCleanup
 {
     SUUnarchiverNotifier *notifier = [[SUUnarchiverNotifier alloc] initWithCompletionBlock:completionBlock progressBlock:progressBlock];

diff --git a/Autoupdate/SUPipedUnarchiver.m b/Autoupdate/SUPipedUnarchiver.m
@@ -81,9 +81,9 @@ + (BOOL)canUnarchivePath:(NSString *)path
     return _argumentsConformingToTypeOfPath(path, YES, NULL) != nil;
 }
 
-+ (BOOL)mustValidateBeforeExtractionWithArchivePath:(NSString *)archivePath
++ (BOOL)mustValidateBeforeExtraction
 {
-    return ([archivePath hasSuffix:@".aar"] || [archivePath hasSuffix:@".yaa"]);
+    return NO;
 }
 
 - (instancetype)initWithArchivePath:(NSString *)archivePath extractionDirectory:(NSString *)extractionDirectory
@@ -96,6 +96,11 @@ - (instancetype)initWithArchivePath:(NSString *)archivePath extractionDirectory:
     return self;
 }
 
+- (BOOL)needsVerifyBeforeExtractionKey
+{
+    return ([_archivePath hasSuffix:@".aar"] || [_archivePath hasSuffix:@".yaa"]);
+}
+
 - (void)unarchiveWithCompletionBlock:(void (^)(NSError * _Nullable))completionBlock progressBlock:(void (^ _Nullable)(double))progressBlock waitForCleanup:(BOOL)__unused waitForCleanup
 {
     NSString *command = nil;

diff --git a/Autoupdate/SUUnarchiverProtocol.h b/Autoupdate/SUUnarchiverProtocol.h
@@ -12,10 +12,12 @@ NS_ASSUME_NONNULL_BEGIN
 
 @protocol SUUnarchiverProtocol <NSObject>
 
-+ (BOOL)mustValidateBeforeExtractionWithArchivePath:(NSString *)archivePath;
++ (BOOL)mustValidateBeforeExtraction;
 
 - (void)unarchiveWithCompletionBlock:(void (^)(NSError * _Nullable))completionBlock progressBlock:(void (^ _Nullable)(double))progressBlock waitForCleanup:(BOOL)waitForCleanup;
 
+@property (nonatomic, readonly) BOOL needsVerifyBeforeExtractionKey;
+
 - (NSString *)description;
 
 @end

diff --git a/Configurations/ConfigCommon.xcconfig b/Configurations/ConfigCommon.xcconfig
@@ -104,7 +104,7 @@ SPARKLE_VERSION_PATCH = 0
 // This should be in SemVer format or empty, ie. "-beta.1"
 // These variables must have a space after the '=' too
 SPARKLE_VERSION_SUFFIX = -beta.1
-CURRENT_PROJECT_VERSION = 2041
+CURRENT_PROJECT_VERSION = 2042
 
 MARKETING_VERSION = $(SPARKLE_VERSION_MAJOR).$(SPARKLE_VERSION_MINOR).$(SPARKLE_VERSION_PATCH)$(SPARKLE_VERSION_SUFFIX)
 ALWAYS_SEARCH_USER_PATHS = NO