Permalink
Browse files

Fixes #48: Adopt standard code signing in favor of DSA signing

Thanks to Mattt Thompson (@mattt) for tag-team-ing this with me.

With this change, if your app deploys only to 10.6+, then you can
dispense altogether with the DSA signatures on future updates to your
application: just make sure the "to" version satisfies the "from"
version's Apple code signing requirements. Most of you are probably
already doing that, and if you're not, you should be anyway.

Specifically, Sparkle validates the designated requirement of the "from"
version against the "to" version. By default, as of this writing, that
means that the bundle identifiers must be the same, and that the leaf
certificate of the signature is the same. So if you keep code signing
your app with the same cert, Sparkle will Just Work without any
additional DSA signature nonsense for you to deal with.

Traditional Sparkle DSA signatures will still be honored.

This support has only been extended to updates to the main app bundle.
If you're updating some other bundle, you will have to use DSA signatures
to secure your updates in the future.

***IMPORTANT: previously, Sparkle considered an update "safe" if both the
appcast and update were distributed over https. That is nowhere near as
strong a verification measure as code signing or the old-school DSA
signatures, so with this change, support for unsigned, https-distributed
updates has been removed. If you're targeting 10.6+, start code-signing
your apps if you haven't already, and everything will be fine. If you're
targeting earlier OS Xs, you'll need to start adding DSA signatures to
your appcasts. When you link this changed version of Sparkle into your
app, it will warn you on launch if you are not code signed and do not
have a DSA public key specified in your Info.plist.
  • Loading branch information...
1 parent 9662452 commit 21f95465da22f1d23f3adbe8ac029fd1d21e0147 @andymatuschak andymatuschak committed Jul 5, 2012
Showing with 175 additions and 30 deletions.
  1. +25 −15 SUBasicUpdateDriver.m
  2. +19 −0 SUCodeSigningVerifier.h
  3. +86 −0 SUCodeSigningVerifier.m
  4. +0 −1 SUDSAVerifier.h
  5. +1 −0 SUInstaller.h
  6. +23 −8 SUInstaller.m
  7. +13 −6 SUUpdater.m
  8. +8 −0 Sparkle.xcodeproj/project.pbxproj
View
40 SUBasicUpdateDriver.m
@@ -18,6 +18,7 @@
#import "SUPlainInstaller.h"
#import "SUPlainInstallerInternals.h"
#import "SUBinaryDeltaCommon.h"
+#import "SUCodeSigningVerifier.h"
#import "SUUpdater_Private.h"
@interface SUBasicUpdateDriver () <NSURLDownloadDelegate>; @end
@@ -203,22 +204,23 @@ - (void)download:(NSURLDownload *)d decideDestinationWithSuggestedFilename:(NSSt
[download setDestination:downloadPath allowOverwrite:YES];
}
-- (void)downloadDidFinish:(NSURLDownload *)d
+- (BOOL)validateUpdateDownloadedToPath:(NSString *)destinationPath extractedToPath:(NSString *)extractedPath DSASignature:(NSString *)DSASignature publicDSAKey:(NSString *)publicDSAKey
{
- #if !ENDANGER_USERS_WITH_INSECURE_UPDATES
- // New in Sparkle 1.5: we're now checking signatures on all non-secure downloads, where "secure" is defined as both the appcast and the download being transmitted over SSL.
- NSURL *downloadURL = [[d request] URL];
- if (!(([[downloadURL scheme] isEqualToString:@"https"] && [[appcastURL scheme] isEqualToString:@"https"]) ||
- ([downloadURL isFileURL] && [appcastURL isFileURL])))
- {
- if (![host publicDSAKey] || ![SUDSAVerifier validatePath:downloadPath withEncodedDSASignature:[updateItem DSASignature] withPublicDSAKey:[host publicDSAKey]])
- {
- [self abortUpdateWithError:[NSError errorWithDomain:SUSparkleErrorDomain code:SUSignatureError userInfo:[NSDictionary dictionaryWithObjectsAndKeys:SULocalizedString(@"An error occurred while extracting the archive. Please try again later.", nil), NSLocalizedDescriptionKey, @"The update is improperly signed.", NSLocalizedFailureReasonErrorKey, nil]]];
- return;
- }
- }
- #endif
-
+ NSString *newBundlePath = [SUInstaller appPathInUpdateFolder:extractedPath forHost:host];
+ if (!newBundlePath) return NO;
+
+ NSError *error = nil;
+ if ([SUCodeSigningVerifier codeSignatureIsValidAtPath:newBundlePath error:&error]) {
+ return YES;
+ } else {
+ SULog(@"Code signature check on update failed: %@", error);
+ }
+
+ return [SUDSAVerifier validatePath:destinationPath withEncodedDSASignature:DSASignature withPublicDSAKey:publicDSAKey];
+}
+
+- (void)downloadDidFinish:(NSURLDownload *)d
+{
[self extractUpdate];
}
@@ -280,6 +282,14 @@ - (BOOL)shouldInstallSynchronously { return NO; }
- (void)installWithToolAndRelaunch:(BOOL)relaunch
{
+#if !ENDANGER_USERS_WITH_INSECURE_UPDATES
+ if (![self validateUpdateDownloadedToPath:downloadPath extractedToPath:tempDir DSASignature:[updateItem DSASignature] publicDSAKey:[host publicDSAKey]])
+ {
+ [self abortUpdateWithError:[NSError errorWithDomain:SUSparkleErrorDomain code:SUSignatureError userInfo:[NSDictionary dictionaryWithObjectsAndKeys:SULocalizedString(@"An error occurred while extracting the archive. Please try again later.", nil), NSLocalizedDescriptionKey, @"The update is improperly signed.", NSLocalizedFailureReasonErrorKey, nil]]];
+ return;
+ }
+#endif
+
if (![updater mayUpdateAndRestart])
{
[self abortUpdate];
View
19 SUCodeSigningVerifier.h
@@ -0,0 +1,19 @@
+//
+// SUCodeSigningVerifier.h
+// Sparkle
+//
+// Created by Andy Matuschak on 7/5/12.
+//
+//
+
+#ifndef SUCODESIGNINGVERIFIER_H
+#define SUCODESIGNINGVERIFIER_H
+
+#import <Foundation/Foundation.h>
+
+@interface SUCodeSigningVerifier : NSObject
++ (BOOL)codeSignatureIsValidAtPath:(NSString *)destinationPath error:(NSError **)error;
++ (BOOL)hostApplicationIsCodeSigned;
+@end
+
+#endif
View
86 SUCodeSigningVerifier.m
@@ -0,0 +1,86 @@
+//
+// SUCodeSigningVerifier.m
+// Sparkle
+//
+// Created by Andy Matuschak on 7/5/12.
+//
+//
+
+#import <Security/CodeSigning.h>
+#import "SUCodeSigningVerifier.h"
+#import "SULog.h"
+
+@implementation SUCodeSigningVerifier
+
+extern OSStatus SecCodeCopySelf(SecCSFlags flags, SecCodeRef *self) __attribute__((weak_import));
+
+extern OSStatus SecCodeCopyDesignatedRequirement(SecStaticCodeRef code, SecCSFlags flags, SecRequirementRef *requirement) __attribute__((weak_import));
+
+extern OSStatus SecStaticCodeCreateWithPath(CFURLRef path, SecCSFlags flags, SecStaticCodeRef *staticCode) __attribute__((weak_import));
+
+extern OSStatus SecStaticCodeCheckValidityWithErrors(SecStaticCodeRef staticCode, SecCSFlags flags, SecRequirementRef requirement, CFErrorRef *errors) __attribute__((weak_import));
+
+
++ (BOOL)codeSignatureIsValidAtPath:(NSString *)destinationPath error:(NSError **)error
+{
+ // This API didn't exist prior to 10.6.
+ if (SecCodeCopySelf == NULL) return NO;
+
+ SecRequirementRef requirement = NULL;
+ OSStatus result;
+
+ SecCodeRef hostCode = NULL;
+ result = SecCodeCopySelf(kSecCSDefaultFlags, &hostCode);
+ if (result != 0) {
+ SULog(@"Failed to copy host code %d", result);
+ goto finally;
+ }
+
+ result = SecCodeCopyDesignatedRequirement(hostCode, kSecCSDefaultFlags, &requirement);
+ if (result != 0) {
+ SULog(@"Failed to copy designated requirement %d", result);
+ goto finally;
+ }
+
+ SecStaticCodeRef staticCode = NULL;
+ NSBundle *newBundle = [NSBundle bundleWithPath:destinationPath];
+ if (!newBundle) {
+ SULog(@"Failed to load NSBundle for update");
+ result = -1;
+ goto finally;
+ }
+
+ result = SecStaticCodeCreateWithPath((CFURLRef)[newBundle executableURL], kSecCSDefaultFlags, &staticCode);
+ if (result != 0) {
+ SULog(@"Failed to get static code %d", result);
+ goto finally;
+ }
+
+ result = SecStaticCodeCheckValidityWithErrors(staticCode, kSecCSDefaultFlags, requirement, (CFErrorRef *)error);
+ if (result != 0 && error) [*error autorelease];
+
+finally:
+ if (hostCode) CFRelease(hostCode);
+ if (staticCode) CFRelease(staticCode);
+ if (requirement) CFRelease(requirement);
+ return (result == 0);
+}
+
++ (BOOL)hostApplicationIsCodeSigned
+{
+ // This API didn't exist prior to 10.6.
+ if (SecCodeCopySelf == NULL) return NO;
+
+ OSStatus result;
+ SecCodeRef hostCode;
+ result = SecCodeCopySelf(kSecCSDefaultFlags, &hostCode);
+ if (result != 0) return NO;
+
+ SecRequirementRef requirement;
+ result = SecCodeCopyDesignatedRequirement(hostCode, kSecCSDefaultFlags, &requirement);
+ if (hostCode) CFRelease(hostCode);
+ if (requirement) CFRelease(requirement);
+ return (result == 0);
+}
+
+@end
View
1 SUDSAVerifier.h
@@ -11,7 +11,6 @@
#import <Cocoa/Cocoa.h>
-// For the paranoid folks!
@interface SUDSAVerifier : NSObject {}
+ (BOOL)validatePath:(NSString *)path withEncodedDSASignature:(NSString *)encodedSignature withPublicDSAKey:(NSString *)pkeyString;
@end
View
1 SUInstaller.h
@@ -14,6 +14,7 @@
@class SUHost;
@interface SUInstaller : NSObject { }
++ (NSString *)appPathInUpdateFolder:(NSString *)updateFolder forHost:(SUHost *)host;
+ (void) installFromUpdateFolder:(NSString *)updateFolder overHost:(SUHost *)host delegate:delegate synchronously:(BOOL)synchronously versionComparator:(id <SUVersionComparison>)comparator;
+ (void) finishInstallationWithResult:(BOOL)result host:(SUHost *)host error:(NSError *)error delegate:delegate;
+ (NSString*) updateFolder;
View
31 SUInstaller.m
@@ -42,14 +42,13 @@ + (BOOL)isAliasFolderAtPath:(NSString *)path
return NO;
}
-
-+ (void)installFromUpdateFolder:(NSString *)inUpdateFolder overHost:(SUHost *)host delegate:delegate synchronously:(BOOL)synchronously versionComparator:(id <SUVersionComparison>)comparator
++ (NSString *)installSourcePathInUpdateFolder:(NSString *)inUpdateFolder forHost:(SUHost *)host isPackage:(BOOL *)isPackagePtr
{
- // Search subdirectories for the application
+ // Search subdirectories for the application
NSString *currentFile,
- *newAppDownloadPath = nil,
- *bundleFileName = [[host bundlePath] lastPathComponent],
- *alternateBundleFileName = [[host name] stringByAppendingPathExtension:[[host bundlePath] pathExtension]];
+ *newAppDownloadPath = nil,
+ *bundleFileName = [[host bundlePath] lastPathComponent],
+ *alternateBundleFileName = [[host name] stringByAppendingPathExtension:[[host bundlePath] pathExtension]];
BOOL isPackage = NO;
NSString *fallbackPackagePath = nil;
NSDirectoryEnumerator *dirEnum = [[NSFileManager defaultManager] enumeratorAtPath: inUpdateFolder];
@@ -100,13 +99,29 @@ + (void)installFromUpdateFolder:(NSString *)inUpdateFolder overHost:(SUHost *)ho
}
// We don't have a valid path. Try to use the fallback package.
-
+
if (newAppDownloadPath == nil && fallbackPackagePath != nil)
{
isPackage = YES;
newAppDownloadPath = fallbackPackagePath;
}
-
+
+ if (isPackagePtr) *isPackagePtr = isPackage;
+ return newAppDownloadPath;
+}
+
++ (NSString *)appPathInUpdateFolder:(NSString *)updateFolder forHost:(SUHost *)host
+{
+ BOOL isPackage = NO;
+ NSString *path = [self installSourcePathInUpdateFolder:updateFolder forHost:host isPackage:&isPackage];
+ return isPackage ? nil : path;
+}
+
++ (void)installFromUpdateFolder:(NSString *)inUpdateFolder overHost:(SUHost *)host delegate:delegate synchronously:(BOOL)synchronously versionComparator:(id <SUVersionComparison>)comparator
+{
+ BOOL isPackage = NO;
+ NSString *newAppDownloadPath = [self installSourcePathInUpdateFolder:inUpdateFolder forHost:host isPackage:&isPackage];
+
if (newAppDownloadPath == nil)
{
[self finishInstallationWithResult:NO host:host error:[NSError errorWithDomain:SUSparkleErrorDomain code:SUMissingUpdateError userInfo:[NSDictionary dictionaryWithObject:@"Couldn't find an appropriate update in the downloaded package." forKey:NSLocalizedDescriptionKey]] delegate:delegate];
View
19 SUUpdater.m
@@ -17,6 +17,7 @@
#import "SUScheduledUpdateDriver.h"
#import "SUConstants.h"
#import "SULog.h"
+#import "SUCodeSigningVerifier.h"
#include <SystemConfiguration/SystemConfiguration.h>
@@ -83,12 +84,18 @@ - (id)initForBundle:(NSBundle *)bundle
#if !ENDANGER_USERS_WITH_INSECURE_UPDATES
// Saving-the-developer-from-a-stupid-mistake-check:
- if (![[[self feedURL] scheme] isEqualToString:@"https"] && ![host publicDSAKey])
- {
- [self notifyWillShowModalAlert];
- NSRunAlertPanel(@"Insecure update error!", @"For security reasons, you need to distribute your appcast over SSL or sign your updates. See Sparkle's documentation for more information.", @"OK", nil, nil);
- [self notifyDidShowModalAlert];
- }
+ BOOL hasPublicDSAKey = [host publicDSAKey] != nil;
+ BOOL isMainBundle = [bundle isEqualTo:[NSBundle mainBundle]];
+ BOOL hostIsCodeSigned = [SUCodeSigningVerifier hostApplicationIsCodeSigned];
+ if (!isMainBundle && !hasPublicDSAKey) {
+ [self notifyWillShowModalAlert];
+ NSRunAlertPanel(@"Insecure update error!", @"For security reasons, you need to sign your updates with a DSA key. See Sparkle's documentation for more information.", @"OK", nil, nil);
+ [self notifyDidShowModalAlert];
+ } else if (isMainBundle && !(hasPublicDSAKey || hostIsCodeSigned)) {
+ [self notifyWillShowModalAlert];
+ NSRunAlertPanel(@"Insecure update error!", @"For security reasons, you need to code sign your application or sign your updates with a DSA key. See Sparkle's documentation for more information.", @"OK", nil, nil);
+ [self notifyDidShowModalAlert];
+ }
#endif
// This runs the permission prompt if needed, but never before the app has finished launching because the runloop won't run before that
[self performSelector:@selector(startUpdateCycle) withObject:nil afterDelay:0];
View
8 Sparkle.xcodeproj/project.pbxproj
@@ -101,6 +101,8 @@
61A354550DF113C70076ECB1 /* SUUserInitiatedUpdateDriver.h in Headers */ = {isa = PBXBuildFile; fileRef = 61A354530DF113C70076ECB1 /* SUUserInitiatedUpdateDriver.h */; settings = {ATTRIBUTES = (); }; };
61A354560DF113C70076ECB1 /* SUUserInitiatedUpdateDriver.m in Sources */ = {isa = PBXBuildFile; fileRef = 61A354540DF113C70076ECB1 /* SUUserInitiatedUpdateDriver.m */; };
61AAE8280A321A7F00D8810D /* Sparkle.strings in Resources */ = {isa = PBXBuildFile; fileRef = 61AAE8220A321A7F00D8810D /* Sparkle.strings */; };
+ 61B078CE15A5FB6100600039 /* SUCodeSigningVerifier.h in Headers */ = {isa = PBXBuildFile; fileRef = 61B078CC15A5FB6100600039 /* SUCodeSigningVerifier.h */; };
+ 61B078CF15A5FB6100600039 /* SUCodeSigningVerifier.m in Sources */ = {isa = PBXBuildFile; fileRef = 61B078CD15A5FB6100600039 /* SUCodeSigningVerifier.m */; };
61B5F8ED09C4CE3C00B25A18 /* SUUpdater.h in Headers */ = {isa = PBXBuildFile; fileRef = 61B5F8E309C4CE3C00B25A18 /* SUUpdater.h */; settings = {ATTRIBUTES = (Public, ); }; };
61B5F8EE09C4CE3C00B25A18 /* SUUpdater.m in Sources */ = {isa = PBXBuildFile; fileRef = 61B5F8E409C4CE3C00B25A18 /* SUUpdater.m */; };
61B5F8EF09C4CE3C00B25A18 /* SUPlainInstallerInternals.m in Sources */ = {isa = PBXBuildFile; fileRef = 61B5F8E509C4CE3C00B25A18 /* SUPlainInstallerInternals.m */; };
@@ -340,6 +342,8 @@
61AAE84F0A321AF700D8810D /* es */ = {isa = PBXFileReference; fileEncoding = 10; lastKnownFileType = text.plist.strings; name = es; path = es.lproj/Sparkle.strings; sourceTree = "<group>"; };
61AAE8590A321B0400D8810D /* fr */ = {isa = PBXFileReference; fileEncoding = 10; lastKnownFileType = text.plist.strings; name = fr; path = fr.lproj/Sparkle.strings; sourceTree = "<group>"; };
61AAE8710A321F7700D8810D /* nl */ = {isa = PBXFileReference; fileEncoding = 10; lastKnownFileType = text.plist.strings; name = nl; path = nl.lproj/Sparkle.strings; sourceTree = "<group>"; };
+ 61B078CC15A5FB6100600039 /* SUCodeSigningVerifier.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = SUCodeSigningVerifier.h; sourceTree = "<group>"; };
+ 61B078CD15A5FB6100600039 /* SUCodeSigningVerifier.m */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.objc; path = SUCodeSigningVerifier.m; sourceTree = "<group>"; };
61B5F8E309C4CE3C00B25A18 /* SUUpdater.h */ = {isa = PBXFileReference; fileEncoding = 30; lastKnownFileType = sourcecode.c.h; path = SUUpdater.h; sourceTree = "<group>"; };
61B5F8E409C4CE3C00B25A18 /* SUUpdater.m */ = {isa = PBXFileReference; fileEncoding = 30; lastKnownFileType = sourcecode.c.objc; path = SUUpdater.m; sourceTree = "<group>"; };
61B5F8E509C4CE3C00B25A18 /* SUPlainInstallerInternals.m */ = {isa = PBXFileReference; fileEncoding = 30; lastKnownFileType = sourcecode.c.objc; path = SUPlainInstallerInternals.m; sourceTree = "<group>"; };
@@ -661,6 +665,8 @@
children = (
61299A2D09CA2DAB00B7442F /* SUDSAVerifier.h */,
61299A2E09CA2DAB00B7442F /* SUDSAVerifier.m */,
+ 61B078CC15A5FB6100600039 /* SUCodeSigningVerifier.h */,
+ 61B078CD15A5FB6100600039 /* SUCodeSigningVerifier.m */,
);
name = Support;
sourceTree = "<group>";
@@ -772,6 +778,7 @@
55C14F06136EF6DB00649790 /* SULog.h in Headers */,
55C14F0F136EF73600649790 /* finish_installation.pch in Headers */,
6158A1C5137904B300487EC1 /* SUUpdater_Private.h in Headers */,
+ 61B078CE15A5FB6100600039 /* SUCodeSigningVerifier.h in Headers */,
);
runOnlyForDeploymentPostprocessing = 0;
};
@@ -1144,6 +1151,7 @@
5D06E8ED0FD68CE4005AE3F6 /* SUBinaryDeltaCommon.m in Sources */,
5D06E93A0FD69271005AE3F6 /* SUBinaryDeltaUnarchiver.m in Sources */,
55C14F07136EF6DB00649790 /* SULog.m in Sources */,
+ 61B078CF15A5FB6100600039 /* SUCodeSigningVerifier.m in Sources */,
);
runOnlyForDeploymentPostprocessing = 0;
};

0 comments on commit 21f9546

Please sign in to comment.