SparkleProject/Sparkle
1
0
Fork 0
mirror of https://github.com/sparkle-project/Sparkle.git synced 2025-11-01 15:34:38 +00:00
Code Issues Packages Projects Releases Wiki Activity

Avoid DSA verification if EdDSA verification passes (#1888)

Browse Source 
If EdDSA verification passes we don't need to have DSA verification pass, and don't need to require the new update to contain a DSA signature or keys.

Also reject updates if app has EdDSA but no EdDSA signature is provided
This commit is contained in:
Mayur Pawashe
2021-07-11 08:59:09 -07:00
committed by GitHub
parent 29d4b801fc
commit 5677ca3f5a
3 changed files with 25 additions and 21 deletions
Download Patch File Download Diff File Expand all files Collapse all files
Autoupdate/SUSignatureVerifier.m
+5 -5
View File
@@ -113,8 +113,8 @@
case SUSigningInputStatusPresent:
switch (signatures.ed25519SignatureStatus) {
case SUSigningInputStatusAbsent:
SULog(SULogLevelDefault, @"The update has an EdDSA signature, but it won't be used, because the old app doesn't have an EdDSA public key");
break;
SULog(SULogLevelError, @"The app has an EdDSA public key, but there is no EdDSA signature in the update, so the update will be rejected.");
return NO;
case SUSigningInputStatusInvalid:
// We will have already logged an error for this failure when the signature was read in, so just do an informational log here.
SULog(SULogLevelDefault, @"The update has an EdDSA signature, but it's invalid, so the update will automatically be rejected.");
@@ -128,10 +128,10 @@
}
if (ed25519_verify(signatures.ed25519Signature, data.bytes, data.length, self.pubKeys.ed25519PubKey)) {
SULog(SULogLevelDefault, @"OK: EdDSA signature is correct");
if (self.pubKeys.dsaPubKeyStatus == SUSigningInputStatusAbsent) {
// No need to check DSA when EdDSA verification succeeded, unless a DSA signature is provided and it's
// erroneously invalid
if (signatures.dsaSignatureStatus != SUSigningInputStatusInvalid) {
return YES;
} else {
SULog(SULogLevelDefault, @"This app has a DSA public key, so a DSA signature is required too");
}
} else {
SULog(SULogLevelError, @"EdDSA signature does not match. Data of the update file being checked is different than data that has been signed, or the public key and the private key are not from the same set.");
Tests/SUSignatureVerifierTest.m
+8 -7
View File
@@ -135,26 +135,27 @@
NSString *edSig = @"EIawm2YkDZ2gBfkEMF2+1VuuTeXnCGZOdnMdVgPPvDZioq7bvDayXqKkIIzSjKMmeFdcFJOHdnba5ZV60+gPBw==";
NSString *wrongEdSig = @"wTcpXCgWoa4NrJpsfzS61FXJIbv963//12U2ef9xstzVOLPHYK2N4/ojgpDV5N1/NGG1uWMBgK+kEWp0Z5zMDQ==";
XCTAssertTrue([v verifyFileAtPath:self.testFile
signatures:[[SUSignatures alloc] initWithDsa:dsaSig ed:nil]],
@"Allow just a DSA signature if that's all that's available");
XCTAssertFalse([v verifyFileAtPath:self.testFile
signatures:[[SUSignatures alloc] initWithDsa:dsaSig ed:nil]],
@"EdDSA signature must be present if app has EdDSA key");
XCTAssertTrue([v verifyFileAtPath:self.testFile
signatures:[[SUSignatures alloc] initWithDsa:nil ed:edSig]],
@"Require the DSA signature to match because there's a DSA public key");
@"Allow just an EdDSA signature if that's all that's available");
XCTAssertFalse([v verifyFileAtPath:self.testFile
signatures:[[SUSignatures alloc] initWithDsa:dsaSig ed:wrongEdSig]],
@"Fail on a bad Ed25519 signature regardless");
XCTAssertFalse([v verifyFileAtPath:self.testFile
XCTAssertTrue([v verifyFileAtPath:self.testFile
signatures:[[SUSignatures alloc] initWithDsa:wrongDSASig ed:edSig]],
@"Fail on a bad DSA signature if provided");
@"Allow bad DSA signature if EdDSA signature is good");
XCTAssertFalse([v verifyFileAtPath:self.testFile
signatures:[[SUSignatures alloc] initWithDsa:dsaSig ed:@"lol"]],
@"Fail if the Ed25519 signature is invalid.");
XCTAssertFalse([v verifyFileAtPath:self.testFile
signatures:[[SUSignatures alloc] initWithDsa:@"lol" ed:edSig]],
@"Fail if the DSA signature is invalid.");
@"Fail if invalid DSA signature is used even if EdDSA signature is good.");
XCTAssertTrue([v verifyFileAtPath:self.testFile
signatures:[[SUSignatures alloc] initWithDsa:dsaSig ed:edSig]],
Tests/SUUpdateValidatorTest.swift
+12 -9
View File
@@ -35,7 +35,7 @@ class SUUpdateValidatorTest: XCTestCase {
struct SignatureConfig: CaseIterable, Equatable, CustomDebugStringConvertible {
enum State: CaseIterable, Equatable {
case none, invalid, valid
case none, invalid, invalidFormat, valid
}
var dsa: State
@@ -62,7 +62,9 @@ class SUUpdateValidatorTest: XCTestCase {
let dsaSig: String?
switch config.dsa {
case .none: dsaSig = nil
case .invalid: dsaSig = "MCwCFCIHCiYYkfZavNzTitTW5tlRp/k5AhQ40poFytqcVhIYdCxQznaXeJPJDQ=="
case .invalid: dsaSig = "ABwCFCIHCIYYkfZavNzTitTW5tlRp/k5AhQ40poFytqcVhIYdCxQznaXeJPJDQ=="
// Use some invalid base64 strings
case .invalidFormat: dsaSig = "%%wCFCIHCIYYkfZavNzTitTW5tlRp/k5AhQ40poFytqcVhIYdCxQznaXeJPJDQ=="
case .valid: dsaSig = "MCwCFCIHCIYYkfZavNzTitTW5tlRp/k5AhQ40poFytqcVhIYdCxQznaXeJPJDQ=="
}
@@ -70,6 +72,8 @@ class SUUpdateValidatorTest: XCTestCase {
switch config.ed {
case .none: edSig = nil
case .invalid: edSig = "wTcpXCgWoa4NrJpsfzS61FXJIbv963//12U2ef9xstzVOLPHYK2N4/ojgpDV5N1/NGG1uWMBgK+kEWp0Z5zMDQ=="
// Use some invalid base64 strings
case .invalidFormat: edSig = "%%cpXCgWoa4NrJpsfzS61FXJIbv963//12U2ef9xstzVOLPHYK2N4/ojgpDV5N1/NGG1uWMBgK+kEWp0Z5zMDQ=="
case .valid: edSig = "EIawm2YkDZ2gBfkEMF2+1VuuTeXnCGZOdnMdVgPPvDZioq7bvDayXqKkIIzSjKMmeFdcFJOHdnba5ZV60+gPBw=="
}
@@ -84,7 +88,6 @@ class SUUpdateValidatorTest: XCTestCase {
func testPrevalidation(bundle bundleConfig: BundleConfig, signatures signatureConfig: SignatureConfig, expectedResult: Bool, line: UInt = #line) {
let host = SUHost(bundle: self.bundle(bundleConfig))
let signatures = self.signatures(signatureConfig)
let validator = SUUpdateValidator(downloadPath: self.signedTestFilePath, signatures: signatures, host: host)
let result = (try? validator.validateDownloadPath()) != nil
@@ -95,8 +98,8 @@ class SUUpdateValidatorTest: XCTestCase {
for signatureConfig in SignatureConfig.allCases {
testPrevalidation(bundle: .none, signatures: signatureConfig, expectedResult: false)
testPrevalidation(bundle: .dsaOnly, signatures: signatureConfig, expectedResult: signatureConfig.dsa == .valid)
testPrevalidation(bundle: .edOnly, signatures: signatureConfig, expectedResult: signatureConfig.ed == .valid)
testPrevalidation(bundle: .both, signatures: signatureConfig, expectedResult: signatureConfig.dsa == .valid && signatureConfig.ed != .invalid)
testPrevalidation(bundle: .edOnly, signatures: signatureConfig, expectedResult: signatureConfig.ed == .valid && signatureConfig.dsa != .invalidFormat)
testPrevalidation(bundle: .both, signatures: signatureConfig, expectedResult: signatureConfig.ed == .valid && signatureConfig.dsa != .invalidFormat)
}
}
@@ -124,15 +127,15 @@ class SUUpdateValidatorTest: XCTestCase {
for signatureConfig in SignatureConfig.allCases {
testPostValidation(bundle: .none, signatures: signatureConfig, expectedResult: false)
testPostValidation(bundle: .dsaOnly, signatures: signatureConfig, expectedResult: signatureConfig.dsa == .valid)
testPostValidation(bundle: .edOnly, signatures: signatureConfig, expectedResult: signatureConfig.ed == .valid)
testPostValidation(bundle: .both, signatures: signatureConfig, expectedResult: signatureConfig.dsa == .valid && signatureConfig.ed != .invalid)
testPostValidation(bundle: .edOnly, signatures: signatureConfig, expectedResult: signatureConfig.ed == .valid && signatureConfig.dsa != .invalidFormat)
testPostValidation(bundle: .both, signatures: signatureConfig, expectedResult: signatureConfig.ed == .valid && signatureConfig.dsa != .invalidFormat)
}
}
func testPostValidationWithCodeSigning() {
for signatureConfig in SignatureConfig.allCases {
testPostValidation(bundle: .codeSignedOnly, signatures: signatureConfig, expectedResult: true)
testPostValidation(bundle: .codeSignedBoth, signatures: signatureConfig, expectedResult: signatureConfig.dsa == .valid && signatureConfig.ed != .invalid)
testPostValidation(bundle: .codeSignedBoth, signatures: signatureConfig, expectedResult: signatureConfig.ed == .valid && signatureConfig.dsa != .invalidFormat)
testPostValidation(bundle: .codeSignedInvalidOnly, signatures: signatureConfig, expectedResult: false)
testPostValidation(bundle: .codeSignedInvalid, signatures: signatureConfig, expectedResult: false)
@@ -150,7 +153,7 @@ class SUUpdateValidatorTest: XCTestCase {
func testPostValidationWithKeyRotation() {
for signatureConfig in SignatureConfig.allCases {
let signatureIsValid = signatureConfig.dsa == .valid && (signatureConfig.ed == .valid || signatureConfig.ed == .none)
let signatureIsValid = (signatureConfig.ed == .valid && signatureConfig.dsa != .invalidFormat)
// It's okay to add DSA keys or add code signing.
testPostValidation(oldBundle: .codeSignedOnly, newBundle: .codeSignedBoth, signatures: signatureConfig, expectedResult: signatureIsValid)