-
Notifications
You must be signed in to change notification settings - Fork 316
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Signing
: extract and verify intermediate key
#2715
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -55,7 +55,7 @@ enum Signing: SigningType { | |
} | ||
|
||
do { | ||
return try Curve25519.Signing.PublicKey(rawRepresentation: key) | ||
return try Self.createPublicKey(with: key) | ||
} catch { | ||
fail(Strings.signing.invalid_public_key(error.localizedDescription)) | ||
} | ||
|
@@ -76,7 +76,12 @@ enum Signing: SigningType { | |
return false | ||
} | ||
|
||
// Fixme: verify public key | ||
guard let intermediatePublicKey = Self.extractAndVerifyIntermediateKey( | ||
from: signature, | ||
publicKey: publicKey | ||
) else { | ||
return false | ||
} | ||
|
||
let salt = signature.component(.salt) | ||
let payload = signature.component(.payload) | ||
|
@@ -85,14 +90,15 @@ enum Signing: SigningType { | |
#if DEBUG | ||
Logger.verbose(Strings.signing.verifying_signature( | ||
signature: signature, | ||
publicKey: intermediatePublicKey.rawRepresentation, | ||
parameters: parameters, | ||
salt: salt, | ||
payload: payload, | ||
message: messageToVerify | ||
)) | ||
#endif | ||
|
||
let isValid = publicKey.isValidSignature(payload, for: messageToVerify) | ||
let isValid = intermediatePublicKey.isValidSignature(payload, for: messageToVerify) | ||
|
||
if !isValid { | ||
Logger.warn(Strings.signing.signature_failed_verification) | ||
|
@@ -121,7 +127,11 @@ enum Signing: SigningType { | |
|
||
// MARK: - | ||
|
||
private static let publicKey = "drCCA+6YAKOAjT7b2RosYNTrRexVWnu+dR5fw/JuKeA=" | ||
/// The actual algorithm used to verify signatures. | ||
@available(iOS 13.0, macOS 10.15, watchOS 6.0, tvOS 13.0, *) | ||
fileprivate typealias Algorithm = Curve25519.Signing.PublicKey | ||
|
||
private static let publicKey = "UC1upXWg5QVmyOSwozp755xLqquBKjjU+di6U8QhMlM=" | ||
|
||
} | ||
|
||
|
@@ -168,11 +178,12 @@ extension Signing { | |
protocol SigningPublicKey { | ||
|
||
func isValidSignature(_ signature: Data, for data: Data) -> Bool | ||
var rawRepresentation: Data { get } | ||
|
||
} | ||
|
||
@available(iOS 13.0, macOS 10.15, tvOS 13.0, watchOS 6.2, *) | ||
extension CryptoKit.Curve25519.Signing.PublicKey: SigningPublicKey {} | ||
extension Signing.Algorithm: SigningPublicKey {} | ||
|
||
// MARK: - Internal implementation (visible for tests) | ||
|
||
|
@@ -196,14 +207,6 @@ extension Signing { | |
} | ||
} | ||
|
||
static let signedPublicKeySize: Int = [Self]([ | ||
.intermediatePublicKey, | ||
.intermediateKeyExpiration, | ||
.intermediateKeySignature | ||
]) | ||
.map(\.size) | ||
.sum() | ||
|
||
static let totalSize: Int = Self.allCases.map(\.size).sum() | ||
|
||
/// Number of bytes where the component begins | ||
|
@@ -239,7 +242,65 @@ extension Signing.SignatureParameters { | |
|
||
private final class BundleToken: NSObject {} | ||
|
||
// MARK: - Data extensions | ||
private extension Signing { | ||
|
||
@available(iOS 13.0, macOS 10.15, watchOS 6.0, tvOS 13.0, *) | ||
static func createPublicKey(with data: Data) throws -> PublicKey { | ||
return try Algorithm(rawRepresentation: data) | ||
} | ||
|
||
static func extractAndVerifyIntermediateKey( | ||
from signature: Data, | ||
publicKey: Signing.PublicKey | ||
) -> Signing.PublicKey? { | ||
guard #available(iOS 13.0, macOS 10.15, watchOS 6.0, tvOS 13.0, *) else { return nil } | ||
|
||
let intermediatePublicKey = signature.component(.intermediatePublicKey) | ||
let intermediateKeyExpiration = signature.component(.intermediateKeyExpiration) | ||
let intermediateKeySignature = signature.component(.intermediateKeySignature) | ||
|
||
guard publicKey.isValidSignature(intermediateKeySignature, | ||
for: intermediateKeyExpiration + intermediatePublicKey) else { | ||
Logger.warn(Strings.signing.intermediate_key_failed_verification(signature: intermediateKeySignature)) | ||
return nil | ||
} | ||
|
||
guard let expirationDate = Self.extractAndVerifyIntermediateKeyExpiration(intermediateKeyExpiration) else { | ||
return nil | ||
} | ||
|
||
Logger.verbose(Strings.signing.intermediate_key_creating(expiration: expirationDate, | ||
data: intermediatePublicKey)) | ||
|
||
do { | ||
return try Self.createPublicKey(with: intermediatePublicKey) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We considered caching this, but I just ran a benchmark on my device and the whole process (creating this key and verifying the original signature) takes 0.17 seconds. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Sorry I meant 0.17ms! |
||
} catch { | ||
Logger.error(Strings.signing.intermediate_key_failed_creation(error)) | ||
return nil | ||
} | ||
} | ||
|
||
/// - Returns: `nil` if the key is expired or has an invalid expiration date. | ||
private static func extractAndVerifyIntermediateKeyExpiration(_ expirationData: Data) -> Date? { | ||
let daysSince1970 = UInt32(littleEndian32Bits: expirationData) | ||
|
||
guard daysSince1970 > 0 else { | ||
Logger.warn(Strings.signing.intermediate_key_invalid(expirationData)) | ||
return nil | ||
} | ||
|
||
let expirationDate = Date(daysSince1970: daysSince1970) | ||
guard expirationDate.timeIntervalSince(Date()) >= 0 else { | ||
Logger.warn(Strings.signing.intermediate_key_expired(expirationDate, expirationData)) | ||
return nil | ||
} | ||
|
||
return expirationDate | ||
} | ||
|
||
} | ||
|
||
// MARK: - Extensions | ||
|
||
private extension Data { | ||
|
||
|
@@ -252,3 +313,11 @@ private extension Data { | |
} | ||
|
||
} | ||
|
||
private extension Date { | ||
|
||
init(daysSince1970: UInt32) { | ||
self.init(timeIntervalSince1970: DispatchTimeInterval.days(Int(daysSince1970)).seconds) | ||
} | ||
|
||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So if the intermediate key is expired, we assume it's an attacker correct? As long as the backend makes sure this doesn't happen, this should be fine 👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yup /cc @bisho