diff --git a/Sparkle/SUBasicUpdateDriver.m b/Sparkle/SUBasicUpdateDriver.m index f470950d..2349f636 100644 --- a/Sparkle/SUBasicUpdateDriver.m +++ b/Sparkle/SUBasicUpdateDriver.m @@ -357,18 +357,20 @@ id updater = self.updater; id unarchiver = [SUUnarchiver unarchiverForPath:self.downloadPath updatingHostBundlePath:self.host.bundlePath decryptionPassword:updater.decryptionPassword]; - BOOL success; + BOOL success = NO; if (!unarchiver) { SULog(SULogLevelError, @"Error: No valid unarchiver for %@!", self.downloadPath); - - success = NO; } else { + self.updateValidator = [[SUUpdateValidator alloc] initWithDownloadPath:self.downloadPath signatures:self.updateItem.signatures host:self.host]; + // Currently unsafe archives are the only case where we can prevalidate before extraction, but that could change in the future - BOOL needsPrevalidation = [[unarchiver class] unsafeIfArchiveIsNotValidated]; - - self.updateValidator = [[SUUpdateValidator alloc] initWithDownloadPath:self.downloadPath signatures:self.updateItem.signatures host:self.host performingPrevalidation:needsPrevalidation]; - - success = self.updateValidator.canValidate; + BOOL needsPrevalidation = [[unarchiver class] mustValidateBeforeExtraction]; + + if (needsPrevalidation) { + success = [self.updateValidator validateDownloadPath]; + } else { + success = YES; + } } if (!success) { diff --git a/Sparkle/SUBinaryDeltaUnarchiver.m b/Sparkle/SUBinaryDeltaUnarchiver.m index 7dfce787..e9db993a 100644 --- a/Sparkle/SUBinaryDeltaUnarchiver.m +++ b/Sparkle/SUBinaryDeltaUnarchiver.m @@ -33,7 +33,7 @@ return [[path pathExtension] isEqualToString:@"delta"]; } -+ (BOOL)unsafeIfArchiveIsNotValidated ++ (BOOL)mustValidateBeforeExtraction { return YES; } diff --git a/Sparkle/SUDiskImageUnarchiver.m b/Sparkle/SUDiskImageUnarchiver.m index 4c1672c6..dbb9cf14 100644 --- a/Sparkle/SUDiskImageUnarchiver.m +++ b/Sparkle/SUDiskImageUnarchiver.m @@ -30,7 +30,7 @@ return [[path pathExtension] isEqualToString:@"dmg"]; } -+ (BOOL)unsafeIfArchiveIsNotValidated ++ (BOOL)mustValidateBeforeExtraction { return NO; } diff --git a/Sparkle/SUPipedUnarchiver.m b/Sparkle/SUPipedUnarchiver.m index 1aa17163..1849b85f 100644 --- a/Sparkle/SUPipedUnarchiver.m +++ b/Sparkle/SUPipedUnarchiver.m @@ -58,7 +58,7 @@ return ([self commandAndArgumentsConformingToTypeOfPath:path] != nil); } -+ (BOOL)unsafeIfArchiveIsNotValidated ++ (BOOL)mustValidateBeforeExtraction { return NO; } diff --git a/Sparkle/SUUnarchiverProtocol.h b/Sparkle/SUUnarchiverProtocol.h index 25a7797d..4be1ea49 100644 --- a/Sparkle/SUUnarchiverProtocol.h +++ b/Sparkle/SUUnarchiverProtocol.h @@ -14,7 +14,7 @@ NS_ASSUME_NONNULL_BEGIN + (BOOL)canUnarchivePath:(NSString *)path; -+ (BOOL)unsafeIfArchiveIsNotValidated; ++ (BOOL)mustValidateBeforeExtraction; - (void)unarchiveWithCompletionBlock:(void (^)(NSError * _Nullable))completionBlock progressBlock:(void (^ _Nullable)(double))progressBlock; diff --git a/Sparkle/SUUpdateValidator.h b/Sparkle/SUUpdateValidator.h index 040b1220..f1f0ec39 100644 --- a/Sparkle/SUUpdateValidator.h +++ b/Sparkle/SUUpdateValidator.h @@ -13,13 +13,11 @@ @interface SUUpdateValidator : NSObject -// Pass YES to performingPrevalidation if archive validation must be done immediately, before extraction -- (instancetype)initWithDownloadPath:(NSString *)downloadPath signatures:(SUSignatures *)signatures host:(SUHost *)host performingPrevalidation:(BOOL)performingPrevalidation; +- (instancetype)initWithDownloadPath:(NSString *)downloadPath signatures:(SUSignatures *)signatures host:(SUHost *)host; -// Indicates whether we can perform (post) validation later -@property (nonatomic, readonly) BOOL canValidate; +// This is "pre" validation, before the archive has been extracted +- (BOOL)validateDownloadPath; -// precondition: validation must be possible (see -canValidate) // This is "post" validation, after an archive has been extracted - (BOOL)validateWithUpdateDirectory:(NSString *)updateDirectory; diff --git a/Sparkle/SUUpdateValidator.m b/Sparkle/SUUpdateValidator.m index db240d9a..6a987678 100644 --- a/Sparkle/SUUpdateValidator.m +++ b/Sparkle/SUUpdateValidator.m @@ -19,7 +19,8 @@ @interface SUUpdateValidator () @property (nonatomic, readonly) SUHost *host; -@property (nonatomic, readonly) BOOL prevalidatedDsaSignature; +@property (nonatomic) BOOL prevalidatedSignature; +@property (nonatomic) BOOL downloadPrevalidationFailed; @property (nonatomic, readonly) SUSignatures *signatures; @property (nonatomic, readonly) NSString *downloadPath; @@ -28,41 +29,15 @@ @implementation SUUpdateValidator @synthesize host = _host; -@synthesize canValidate = _canValidate; -@synthesize prevalidatedDsaSignature = _prevalidatedDsaSignature; +@synthesize prevalidatedSignature = _prevalidatedSignature; @synthesize signatures = _signatures; +@synthesize downloadPrevalidationFailed = _downloadPrevalidationFailed; @synthesize downloadPath = _downloadPath; -- (instancetype)initWithDownloadPath:(NSString *)downloadPath signatures:(SUSignatures *)signatures host:(SUHost *)host performingPrevalidation:(BOOL)performingPrevalidation +- (instancetype)initWithDownloadPath:(NSString *)downloadPath signatures:(SUSignatures *)signatures host:(SUHost *)host { self = [super init]; if (self != nil) { - BOOL canValidate; - BOOL prevalidatedDsaSignature; - if (performingPrevalidation) { - SUPublicKeys *publicKeys = host.publicKeys; - - if (publicKeys.dsaPubKey == nil) { - prevalidatedDsaSignature = NO; - SULog(SULogLevelError, @"Failed to validate update before unarchiving because no DSA public key was found in the old app"); - } else if (signatures == nil || signatures.dsaSignature == nil) { - prevalidatedDsaSignature = NO; - SULog(SULogLevelError, @"Failed to validate update before unarchiving because no DSA signature was found"); - } else { - prevalidatedDsaSignature = [SUSignatureVerifier validatePath:downloadPath withSignatures:signatures withPublicKeys:publicKeys]; - if (!prevalidatedDsaSignature) { - SULog(SULogLevelError, @"DSA signature validation before unarchiving failed for update %@", downloadPath); - } - } - - canValidate = prevalidatedDsaSignature; - } else { - prevalidatedDsaSignature = NO; - canValidate = YES; - } - - _canValidate = canValidate; - _prevalidatedDsaSignature = prevalidatedDsaSignature; _downloadPath = [downloadPath copy]; _signatures = [signatures copy]; _host = host; @@ -70,17 +45,36 @@ return self; } +- (BOOL)validateDownloadPath { + SUPublicKeys *publicKeys = self.host.publicKeys; + SUSignatures *signatures = self.signatures; + + if (publicKeys.dsaPubKey == nil) { + SULog(SULogLevelError, @"Failed to validate update before unarchiving because no DSA public key was found in the old app"); + } else if (signatures == nil || signatures.dsaSignature == nil) { + SULog(SULogLevelError, @"Failed to validate update before unarchiving because no DSA signature was found"); + } else { + if ([SUSignatureVerifier validatePath:self.downloadPath withSignatures:signatures withPublicKeys:publicKeys]) { + self.prevalidatedSignature = YES; + return YES; + } + SULog(SULogLevelError, @"DSA signature validation before unarchiving failed for update %@", self.downloadPath); + } + self.downloadPrevalidationFailed = YES; + return NO; +} + - (BOOL)validateWithUpdateDirectory:(NSString *)updateDirectory { - assert(self.canValidate); + if (self.downloadPrevalidationFailed) { + return NO; + } SUSignatures *signatures = self.signatures; SUPublicKeys *publicKeys = self.host.publicKeys; NSString *downloadPath = self.downloadPath; SUHost *host = self.host; - BOOL prevalidatedDsaSignature = self.prevalidatedDsaSignature; - BOOL isPackage = NO; // install source could point to a new bundle or a package @@ -92,7 +86,7 @@ NSURL *installSourceURL = [NSURL fileURLWithPath:installSource]; - if (!prevalidatedDsaSignature) { + if (!self.prevalidatedSignature) { // Check to see if we have a package or bundle to validate if (isPackage) { // For package type updates, all we do is check if the DSA signature is valid