diff --git a/Autoupdate/AppInstaller.m b/Autoupdate/AppInstaller.m index 9ecd2f239..49a76a9fd 100644 --- a/Autoupdate/AppInstaller.m +++ b/Autoupdate/AppInstaller.m @@ -236,7 +236,7 @@ - (void)extractAndInstallUpdate SPU_OBJC_DIRECT [self->_communicator handleMessageWithIdentifier:SPUExtractedArchiveWithProgress data:data]; } - }]; + } waitForCleanup:NO]; } } @@ -499,6 +499,7 @@ - (void)handleMessageWithIdentifier:(int32_t)identifier data:(NSData *)data self->_signatures = installationData.signatures; self->_updateDirectoryPath = cacheInstallationPath; self->_extractionDirectory = extractionDirectory; + self->_decryptionPassword = installationData.decryptionPassword; self->_host = [[SUHost alloc] initWithBundle:hostBundle]; self->_verifierInformation = [[SPUVerifierInformation alloc] initWithExpectedVersion:installationData.expectedVersion expectedContentLength:installationData.expectedContentLength]; diff --git a/Autoupdate/SUBinaryDeltaUnarchiver.m b/Autoupdate/SUBinaryDeltaUnarchiver.m index 79cd61fb9..cb3336d37 100644 --- a/Autoupdate/SUBinaryDeltaUnarchiver.m +++ b/Autoupdate/SUBinaryDeltaUnarchiver.m @@ -83,7 +83,7 @@ - (instancetype)initWithArchivePath:(NSString *)archivePath extractionDirectory: return self; } -- (void)unarchiveWithCompletionBlock:(void (^)(NSError * _Nullable))completionBlock progressBlock:(void (^ _Nullable)(double))progressBlock +- (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), ^{ @autoreleasepool { diff --git a/Autoupdate/SUDiskImageUnarchiver.m b/Autoupdate/SUDiskImageUnarchiver.m index c75caf1ae..f05e72b74 100644 --- a/Autoupdate/SUDiskImageUnarchiver.m +++ b/Autoupdate/SUDiskImageUnarchiver.m @@ -11,6 +11,7 @@ #import "SUDiskImageUnarchiver.h" #import "SUUnarchiverNotifier.h" #import "SULog.h" +#import "SUErrors.h" #include "AppKitPrevention.h" @@ -50,11 +51,11 @@ - (instancetype)initWithArchivePath:(NSString *)archivePath extractionDirectory: return self; } -- (void)unarchiveWithCompletionBlock:(void (^)(NSError * _Nullable))completionBlock progressBlock:(void (^ _Nullable)(double))progressBlock +- (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), ^{ SUUnarchiverNotifier *notifier = [[SUUnarchiverNotifier alloc] initWithCompletionBlock:completionBlock progressBlock:progressBlock]; - [self extractDMGWithNotifier:notifier]; + [self extractDMGWithNotifier:notifier waitForCleanup:waitForCleanup]; }); } @@ -78,7 +79,7 @@ - (BOOL)fileManager:(NSFileManager *)fileManager shouldCopyItemAtURL:(NSURL *)sr } // Called on a non-main thread. -- (void)extractDMGWithNotifier:(SUUnarchiverNotifier *)notifier SPU_OBJC_DIRECT +- (void)extractDMGWithNotifier:(SUUnarchiverNotifier *)notifier waitForCleanup:(BOOL)waitForCleanup SPU_OBJC_DIRECT { @autoreleasepool { BOOL mountedSuccessfully = NO; @@ -95,25 +96,32 @@ - (void)extractDMGWithNotifier:(SUUnarchiverNotifier *)notifier SPU_OBJC_DIRECT // Note: this check does not follow symbolic links, which is what we want while ([[NSURL fileURLWithPath:mountPoint] checkResourceIsReachableAndReturnError:NULL]); - NSData *promptData = [NSData dataWithBytes:"yes\n" length:4]; + NSMutableData *inputData = [NSMutableData data]; - // Finder doesn't verify disk images anymore beyond the code signing signature (if available) - // Opt out of the old CRC checksum checks - NSMutableArray *arguments = [@[@"attach", _archivePath, @"-mountpoint", mountPoint, @"-noverify", @"-nobrowse", @"-noautoopen"] mutableCopy]; - - if (_decryptionPassword) { - NSMutableData *passwordData = [[_decryptionPassword dataUsingEncoding:NSUTF8StringEncoding] mutableCopy]; + // Prepare stdin data for passwords and license agreements + { + // If no password is supplied, we will still be asked a password. + // In that case we respond with an empty password. + NSData *decryptionPasswordData = [_decryptionPassword dataUsingEncoding:NSUTF8StringEncoding]; + if (decryptionPasswordData != nil) { + [inputData appendData:decryptionPasswordData]; + } + // From the hdiutil docs: // read a null-terminated passphrase from standard input // - // Add the null terminator, then the newline - [passwordData appendData:[NSData dataWithBytes:"\0" length:1]]; - [passwordData appendData:promptData]; - promptData = passwordData; + // Add the null terminator + [inputData appendBytes:"\0" length:1]; - [arguments addObject:@"-stdinpass"]; + // Append prompt data for license agreements + [inputData appendBytes:"yes\n" length:4]; } + // Finder doesn't verify disk images anymore beyond the code signing signature (if available) + // Opt out of the old CRC checksum checks + // Also always pass -stdinpass so we gracefully handle password protected disk images even if we aren't expecting them + NSArray *arguments = @[@"attach", _archivePath, @"-mountpoint", mountPoint, @"-noverify", @"-nobrowse", @"-noautoopen", @"-stdinpass"]; + NSData *output = nil; NSInteger taskResult = -1; @@ -153,7 +161,7 @@ - (void)extractDMGWithNotifier:(SUUnarchiverNotifier *)notifier SPU_OBJC_DIRECT } if (@available(macOS 10.15, *)) { - if (![inputPipe.fileHandleForWriting writeData:promptData error:&error]) { + if (![inputPipe.fileHandleForWriting writeData:inputData error:&error]) { goto reportError; } } @@ -161,7 +169,7 @@ - (void)extractDMGWithNotifier:(SUUnarchiverNotifier *)notifier SPU_OBJC_DIRECT else { @try { - [inputPipe.fileHandleForWriting writeData:promptData]; + [inputPipe.fileHandleForWriting writeData:inputData]; } @catch (NSException *) { goto reportError; } @@ -175,12 +183,16 @@ - (void)extractDMGWithNotifier:(SUUnarchiverNotifier *)notifier SPU_OBJC_DIRECT taskResult = task.terminationStatus; } - + if (taskResult != 0) { NSString *resultStr = output ? [[NSString alloc] initWithData:output encoding:NSUTF8StringEncoding] : nil; SULog(SULogLevelError, @"hdiutil failed with code: %ld data: <<%@>>", (long)taskResult, resultStr); + + error = [NSError errorWithDomain:SUSparkleErrorDomain code:SUUnarchivingError userInfo:@{NSLocalizedDescriptionKey:[NSString stringWithFormat:@"Extraction failed due to hdiutil returning %ld status: %@", (long)taskResult, resultStr]}]; + goto reportError; } + mountedSuccessfully = YES; // Mounting can take some time, so increment progress @@ -271,11 +283,11 @@ - (void)extractDMGWithNotifier:(SUUnarchiverNotifier *)notifier SPU_OBJC_DIRECT [notifier notifyProgress:1.0]; - [notifier notifySuccess]; + BOOL success = YES; goto finally; reportError: - [notifier notifyFailureWithError:error]; + success = NO; finally: if (mountedSuccessfully) { @@ -289,10 +301,18 @@ - (void)extractDMGWithNotifier:(SUUnarchiverNotifier *)notifier SPU_OBJC_DIRECT if (![task launchAndReturnError:&launchCleanupError]) { SULog(SULogLevelError, @"Failed to unmount %@", mountPoint); SULog(SULogLevelError, @"Error: %@", launchCleanupError); + } else if (waitForCleanup) { + [task waitUntilExit]; } } else { SULog(SULogLevelError, @"Can't mount DMG %@", _archivePath); } + + if (success) { + [notifier notifySuccess]; + } else { + [notifier notifyFailureWithError:error]; + } } } diff --git a/Autoupdate/SUFlatPackageUnarchiver.m b/Autoupdate/SUFlatPackageUnarchiver.m index db6facf32..ba99c4ae8 100644 --- a/Autoupdate/SUFlatPackageUnarchiver.m +++ b/Autoupdate/SUFlatPackageUnarchiver.m @@ -44,7 +44,7 @@ - (instancetype)initWithFlatPackagePath:(NSString *)flatPackagePath extractionDi return self; } -- (void)unarchiveWithCompletionBlock:(void (^)(NSError * _Nullable))completionBlock progressBlock:(void (^ _Nullable)(double))progressBlock +- (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 index b7297617e..22bdec4d4 100644 --- a/Autoupdate/SUPipedUnarchiver.m +++ b/Autoupdate/SUPipedUnarchiver.m @@ -96,7 +96,7 @@ - (instancetype)initWithArchivePath:(NSString *)archivePath extractionDirectory: return self; } -- (void)unarchiveWithCompletionBlock:(void (^)(NSError * _Nullable))completionBlock progressBlock:(void (^ _Nullable)(double))progressBlock +- (void)unarchiveWithCompletionBlock:(void (^)(NSError * _Nullable))completionBlock progressBlock:(void (^ _Nullable)(double))progressBlock waitForCleanup:(BOOL)__unused waitForCleanup { NSString *command = nil; NSArray *arguments = _argumentsConformingToTypeOfPath(_archivePath, YES, &command); diff --git a/Autoupdate/SUUnarchiverProtocol.h b/Autoupdate/SUUnarchiverProtocol.h index 15e92ee06..1c3ce6c56 100644 --- a/Autoupdate/SUUnarchiverProtocol.h +++ b/Autoupdate/SUUnarchiverProtocol.h @@ -14,7 +14,7 @@ NS_ASSUME_NONNULL_BEGIN + (BOOL)mustValidateBeforeExtractionWithArchivePath:(NSString *)archivePath; -- (void)unarchiveWithCompletionBlock:(void (^)(NSError * _Nullable))completionBlock progressBlock:(void (^ _Nullable)(double))progressBlock; +- (void)unarchiveWithCompletionBlock:(void (^)(NSError * _Nullable))completionBlock progressBlock:(void (^ _Nullable)(double))progressBlock waitForCleanup:(BOOL)waitForCleanup; - (NSString *)description; diff --git a/Tests/SUUnarchiverTest.swift b/Tests/SUUnarchiverTest.swift index f337902ed..754af2030 100644 --- a/Tests/SUUnarchiverTest.swift +++ b/Tests/SUUnarchiverTest.swift @@ -50,7 +50,7 @@ class SUUnarchiverTest: XCTestCase unarchiver.unarchive(completionBlock: {(error: Error?) -> Void in XCTAssertNotNil(error) testExpectation.fulfill() - }, progressBlock: nil) + }, progressBlock: nil, waitForCleanup: true) } // swiftlint:disable function_parameter_count @@ -65,7 +65,7 @@ class SUUnarchiverTest: XCTestCase XCTAssertNotNil(error) } testExpectation.fulfill() - }, progressBlock: nil) + }, progressBlock: nil, waitForCleanup: true) } func testUnarchivingZip() @@ -132,11 +132,36 @@ class SUUnarchiverTest: XCTestCase self.unarchiveTestAppWithExtension("enc.nolicense.dmg", password: "testpass") } + func testUnarchivingEncryptedDmgWithLicenseAndWithIncorrectPassword() + { + self.unarchiveTestAppWithExtension("enc.dmg", password: "moo", expectingSuccess: false) + } + + func testUnarchivingEncryptedDmgWithLicenseAndWithoutPassword() + { + self.unarchiveTestAppWithExtension("enc.dmg", expectingSuccess: false) + } + + func testUnarchivingEncryptedDmgWithoutLicenseAndWithIncorrectPassword() + { + self.unarchiveTestAppWithExtension("enc.nolicense.dmg", password: "moo", expectingSuccess: false) + } + + func testUnarchivingEncryptedDmgWithoutLicenseAndWithoutPassword() + { + self.unarchiveTestAppWithExtension("enc.nolicense.dmg", expectingSuccess: false) + } + func testUnarchivingAPFSDMG() { self.unarchiveTestAppWithExtension("dmg", resourceName: "SparkleTestCodeSign_apfs") } + func testUnarchivingAPFSDMGWithBogusPassword() + { + self.unarchiveTestAppWithExtension("dmg", password: "moo", resourceName: "SparkleTestCodeSign_apfs") + } + func testUnarchivingAPFSAdhocSignedDMGWithAuxFiles() { self.unarchiveTestAppWithExtension("dmg", resourceName: "SparkleTestCodeSign_apfs_lzma_aux_files") diff --git a/generate_appcast/Unarchive.swift b/generate_appcast/Unarchive.swift index bf02f5796..5d2303a86 100644 --- a/generate_appcast/Unarchive.swift +++ b/generate_appcast/Unarchive.swift @@ -25,7 +25,7 @@ func unarchive(itemPath: URL, archiveDestDir: URL, callback: @escaping (Error?) } catch { callback(error) } - }, progressBlock: nil) + }, progressBlock: nil, waitForCleanup: true) } else { callback(makeError(code: .unarchivingError, "Not a supported archive format: \(itemPath.path)")) }