From 43a9e287c34bf905461105097305e9d664d92ead Mon Sep 17 00:00:00 2001 From: Maxwell <136101+mxswd@users.noreply.github.com> Date: Sun, 12 Mar 2023 10:21:09 +1000 Subject: [PATCH] Rounded out the rest of the SmartCardStore API (#450) * Rounded out the rest of the SmartCardStore API. * Comments and shuffling around * Expose verify as public api * Verification * Tweak verify signature * Cleanup and tests --------- Co-authored-by: Max Goedjen --- .../SecretKit/Erasers/AnySecretStore.swift | 6 + .../SecretKit/OpenSSH/OpenSSHKeyWriter.swift | 7 + .../Sources/SecretKit/Types/Secret.swift | 12 ++ .../Sources/SecretKit/Types/SecretStore.swift | 20 +++ .../SecureEnclaveStore.swift | 42 +++++- .../SmartCardSecretKit/SmartCardStore.swift | 141 +++++++++++++++++- .../SecretAgentKitTests/AgentTests.swift | 13 +- .../Tests/SecretAgentKitTests/StubStore.swift | 36 +++++ .../Preview Content/PreviewStore.swift | 4 + Sources/Secretive/Views/EmptyStoreView.swift | 2 +- 10 files changed, 266 insertions(+), 17 deletions(-) diff --git a/Sources/Packages/Sources/SecretKit/Erasers/AnySecretStore.swift b/Sources/Packages/Sources/SecretKit/Erasers/AnySecretStore.swift index 0cb6c40..f70000b 100644 --- a/Sources/Packages/Sources/SecretKit/Erasers/AnySecretStore.swift +++ b/Sources/Packages/Sources/SecretKit/Erasers/AnySecretStore.swift @@ -10,6 +10,7 @@ public class AnySecretStore: SecretStore { private let _name: () -> String private let _secrets: () -> [AnySecret] private let _sign: (Data, AnySecret, SigningRequestProvenance) throws -> Data + private let _verify: (Data, Data, AnySecret) throws -> Bool private let _existingPersistedAuthenticationContext: (AnySecret) -> PersistedAuthenticationContext? private let _persistAuthentication: (AnySecret, TimeInterval) throws -> Void private let _reloadSecrets: () -> Void @@ -23,6 +24,7 @@ public class AnySecretStore: SecretStore { _id = { secretStore.id } _secrets = { secretStore.secrets.map { AnySecret($0) } } _sign = { try secretStore.sign(data: $0, with: $1.base as! SecretStoreType.SecretType, for: $2) } + _verify = { try secretStore.verify(signature: $0, for: $1, with: $2.base as! SecretStoreType.SecretType) } _existingPersistedAuthenticationContext = { secretStore.existingPersistedAuthenticationContext(secret: $0.base as! SecretStoreType.SecretType) } _persistAuthentication = { try secretStore.persistAuthentication(secret: $0.base as! SecretStoreType.SecretType, forDuration: $1) } _reloadSecrets = { secretStore.reloadSecrets() } @@ -51,6 +53,10 @@ public class AnySecretStore: SecretStore { try _sign(data, secret, provenance) } + public func verify(signature: Data, for data: Data, with secret: AnySecret) throws -> Bool { + try _verify(signature, data, secret) + } + public func existingPersistedAuthenticationContext(secret: AnySecret) -> PersistedAuthenticationContext? { _existingPersistedAuthenticationContext(secret) } diff --git a/Sources/Packages/Sources/SecretKit/OpenSSH/OpenSSHKeyWriter.swift b/Sources/Packages/Sources/SecretKit/OpenSSH/OpenSSHKeyWriter.swift index 223b935..da8c4b1 100644 --- a/Sources/Packages/Sources/SecretKit/OpenSSH/OpenSSHKeyWriter.swift +++ b/Sources/Packages/Sources/SecretKit/OpenSSH/OpenSSHKeyWriter.swift @@ -64,6 +64,10 @@ extension OpenSSHKeyWriter { switch algorithm { case .ellipticCurve: return "ecdsa-sha2-nistp" + String(describing: length) + case .rsa: + // All RSA keys use the same 512 bit hash function, per + // https://security.stackexchange.com/questions/255074/why-are-rsa-sha2-512-and-rsa-sha2-256-supported-but-not-reported-by-ssh-q-key + return "rsa-sha2-512" } } @@ -76,6 +80,9 @@ extension OpenSSHKeyWriter { switch algorithm { case .ellipticCurve: return "nistp" + String(describing: length) + case .rsa: + // All RSA keys use the same 512 bit hash function + return "rsa-sha2-512" } } diff --git a/Sources/Packages/Sources/SecretKit/Types/Secret.swift b/Sources/Packages/Sources/SecretKit/Types/Secret.swift index 6fc57a1..8f9656c 100644 --- a/Sources/Packages/Sources/SecretKit/Types/Secret.swift +++ b/Sources/Packages/Sources/SecretKit/Types/Secret.swift @@ -20,6 +20,7 @@ public protocol Secret: Identifiable, Hashable { public enum Algorithm: Hashable { case ellipticCurve + case rsa /// Initializes the Algorithm with a secAttr representation of an algorithm. /// - Parameter secAttr: the secAttr, represented as an NSNumber. @@ -28,8 +29,19 @@ public enum Algorithm: Hashable { switch secAttrString { case kSecAttrKeyTypeEC: self = .ellipticCurve + case kSecAttrKeyTypeRSA: + self = .rsa default: fatalError() } } + + public var secAttrKeyType: CFString { + switch self { + case .ellipticCurve: + return kSecAttrKeyTypeEC + case .rsa: + return kSecAttrKeyTypeRSA + } + } } diff --git a/Sources/Packages/Sources/SecretKit/Types/SecretStore.swift b/Sources/Packages/Sources/SecretKit/Types/SecretStore.swift index f13fec7..954fe21 100644 --- a/Sources/Packages/Sources/SecretKit/Types/SecretStore.swift +++ b/Sources/Packages/Sources/SecretKit/Types/SecretStore.swift @@ -23,6 +23,14 @@ public protocol SecretStore: ObservableObject, Identifiable { /// - Returns: The signed data. func sign(data: Data, with secret: SecretType, for provenance: SigningRequestProvenance) throws -> Data + /// Verifies that a signature is valid over a specified payload. + /// - Parameters: + /// - signature: The signature over the data. + /// - data: The data to verify the signature of. + /// - secret: The secret whose signature to verify. + /// - Returns: Whether the signature was verified. + func verify(signature: Data, for data: Data, with secret: SecretType) throws -> Bool + /// Checks to see if there is currently a valid persisted authentication for a given secret. /// - Parameters: /// - secret: The ``Secret`` to check if there is a persisted authentication for. @@ -71,3 +79,15 @@ extension NSNotification.Name { public static let secretStoreReloaded = NSNotification.Name("com.maxgoedjen.Secretive.secretStore.reloaded") } + +public typealias SecurityError = Unmanaged + +extension CFError { + + public static let verifyError = CFErrorCreate(nil, NSOSStatusErrorDomain as CFErrorDomain, CFIndex(errSecVerifyFailed), nil)! + + static public func ~=(lhs: CFError, rhs: CFError) -> Bool { + CFErrorGetDomain(lhs) == CFErrorGetDomain(rhs) && CFErrorGetCode(lhs) == CFErrorGetCode(rhs) + } + +} diff --git a/Sources/Packages/Sources/SecureEnclaveSecretKit/SecureEnclaveStore.swift b/Sources/Packages/Sources/SecureEnclaveSecretKit/SecureEnclaveStore.swift index 8c4784a..4cd32f9 100644 --- a/Sources/Packages/Sources/SecureEnclaveSecretKit/SecureEnclaveStore.swift +++ b/Sources/Packages/Sources/SecureEnclaveSecretKit/SecureEnclaveStore.swift @@ -101,7 +101,7 @@ extension SecureEnclave { reloadSecretsInternal() } - public func sign(data: Data, with secret: SecretType, for provenance: SigningRequestProvenance) throws -> Data { + public func sign(data: Data, with secret: Secret, for provenance: SigningRequestProvenance) throws -> Data { let context: LAContext if let existing = persistedAuthenticationContexts[secret], existing.valid { context = existing.context @@ -138,6 +138,41 @@ extension SecureEnclave { return signature as Data } + public func verify(signature: Data, for data: Data, with secret: Secret) throws -> Bool { + let context = LAContext() + context.localizedReason = "verify a signature using secret \"\(secret.name)\"" + context.localizedCancelTitle = "Deny" + let attributes = KeychainDictionary([ + kSecClass: kSecClassKey, + kSecAttrKeyClass: kSecAttrKeyClassPrivate, + kSecAttrApplicationLabel: secret.id as CFData, + kSecAttrKeyType: Constants.keyType, + kSecAttrTokenID: kSecAttrTokenIDSecureEnclave, + kSecAttrApplicationTag: Constants.keyTag, + kSecUseAuthenticationContext: context, + kSecReturnRef: true + ]) + var verifyError: SecurityError? + var untyped: CFTypeRef? + let status = SecItemCopyMatching(attributes, &untyped) + if status != errSecSuccess { + throw KeychainError(statusCode: status) + } + guard let untypedSafe = untyped else { + throw KeychainError(statusCode: errSecSuccess) + } + let key = untypedSafe as! SecKey + let verified = SecKeyVerifySignature(key, .ecdsaSignatureMessageX962SHA256, data as CFData, signature as CFData, &verifyError) + if !verified, let verifyError { + if verifyError.takeUnretainedValue() ~= .verifyError { + return false + } else { + throw SigningError(error: verifyError) + } + } + return verified + } + public func existingPersistedAuthenticationContext(secret: Secret) -> PersistedAuthenticationContext? { guard let persisted = persistedAuthenticationContexts[secret], persisted.valid else { return nil } return persisted @@ -282,11 +317,6 @@ extension SecureEnclave { } -extension SecureEnclave { - - public typealias SecurityError = Unmanaged - -} extension SecureEnclave { diff --git a/Sources/Packages/Sources/SmartCardSecretKit/SmartCardStore.swift b/Sources/Packages/Sources/SmartCardSecretKit/SmartCardStore.swift index 28026ce..156bc99 100644 --- a/Sources/Packages/Sources/SmartCardSecretKit/SmartCardStore.swift +++ b/Sources/Packages/Sources/SmartCardSecretKit/SmartCardStore.swift @@ -45,7 +45,7 @@ extension SmartCard { fatalError("Keys must be deleted on the smart card.") } - public func sign(data: Data, with secret: SecretType, for provenance: SigningRequestProvenance) throws -> Data { + public func sign(data: Data, with secret: Secret, for provenance: SigningRequestProvenance) throws -> Data { guard let tokenID = tokenID else { fatalError() } let context = LAContext() context.localizedReason = "sign a request from \"\(provenance.origin.displayName)\" using secret \"\(secret.name)\"" @@ -74,6 +74,10 @@ extension SmartCard { signatureAlgorithm = .ecdsaSignatureMessageX962SHA256 case (.ellipticCurve, 384): signatureAlgorithm = .ecdsaSignatureMessageX962SHA384 + case (.rsa, 1024): + signatureAlgorithm = .rsaSignatureMessagePKCS1v15SHA512 + case (.rsa, 2048): + signatureAlgorithm = .rsaSignatureMessagePKCS1v15SHA512 default: fatalError() } @@ -82,6 +86,41 @@ extension SmartCard { } return signature as Data } + public func verify(signature: Data, for data: Data, with secret: Secret) throws -> Bool { + let attributes = KeychainDictionary([ + kSecAttrKeyType: secret.algorithm.secAttrKeyType, + kSecAttrKeySizeInBits: secret.keySize, + kSecAttrKeyClass: kSecAttrKeyClassPublic + ]) + var verifyError: SecurityError? + let untyped: CFTypeRef? = SecKeyCreateWithData(secret.publicKey as CFData, attributes, &verifyError) + guard let untypedSafe = untyped else { + throw KeychainError(statusCode: errSecSuccess) + } + let key = untypedSafe as! SecKey + let signatureAlgorithm: SecKeyAlgorithm + switch (secret.algorithm, secret.keySize) { + case (.ellipticCurve, 256): + signatureAlgorithm = .ecdsaSignatureMessageX962SHA256 + case (.ellipticCurve, 384): + signatureAlgorithm = .ecdsaSignatureMessageX962SHA384 + case (.rsa, 1024): + signatureAlgorithm = .rsaSignatureMessagePKCS1v15SHA512 + case (.rsa, 2048): + signatureAlgorithm = .rsaSignatureMessagePKCS1v15SHA512 + default: + fatalError() + } + let verified = SecKeyVerifySignature(key, signatureAlgorithm, data as CFData, signature as CFData, &verifyError) + if !verified, let verifyError { + if verifyError.takeUnretainedValue() ~= .verifyError { + return false + } else { + throw SigningError(error: verifyError) + } + } + return verified + } public func existingPersistedAuthenticationContext(secret: SmartCard.Secret) -> PersistedAuthenticationContext? { nil @@ -140,7 +179,6 @@ extension SmartCard.Store { let attributes = KeychainDictionary([ kSecClass: kSecClassKey, kSecAttrTokenID: tokenID, - kSecAttrKeyType: kSecAttrKeyTypeEC, // Restrict to EC kSecReturnRef: true, kSecMatchLimit: kSecMatchLimitAll, kSecReturnAttributes: true @@ -164,6 +202,99 @@ extension SmartCard.Store { } + +// MARK: Smart Card specific encryption/decryption/verification +extension SmartCard.Store { + + /// Encrypts a payload with a specified key. + /// - Parameters: + /// - data: The payload to encrypt. + /// - secret: The secret to encrypt with. + /// - Returns: The encrypted data. + /// - Warning: Encryption functions are deliberately only exposed on a library level, and are not exposed in Secretive itself to prevent users from data loss. Any pull requests which expose this functionality in the app will not be merged. + public func encrypt(data: Data, with secret: SecretType) throws -> Data { + let context = LAContext() + context.localizedReason = "encrypt data using secret \"\(secret.name)\"" + context.localizedCancelTitle = "Deny" + let attributes = KeychainDictionary([ + kSecAttrKeyType: secret.algorithm.secAttrKeyType, + kSecAttrKeySizeInBits: secret.keySize, + kSecAttrKeyClass: kSecAttrKeyClassPublic, + kSecUseAuthenticationContext: context + ]) + var encryptError: SecurityError? + let untyped: CFTypeRef? = SecKeyCreateWithData(secret.publicKey as CFData, attributes, &encryptError) + guard let untypedSafe = untyped else { + throw SmartCard.KeychainError(statusCode: errSecSuccess) + } + let key = untypedSafe as! SecKey + let signatureAlgorithm: SecKeyAlgorithm + switch (secret.algorithm, secret.keySize) { + case (.ellipticCurve, 256): + signatureAlgorithm = .eciesEncryptionCofactorVariableIVX963SHA256AESGCM + case (.ellipticCurve, 384): + signatureAlgorithm = .eciesEncryptionCofactorVariableIVX963SHA256AESGCM + case (.rsa, 1024): + signatureAlgorithm = .rsaEncryptionOAEPSHA512AESGCM + case (.rsa, 2048): + signatureAlgorithm = .rsaEncryptionOAEPSHA512AESGCM + default: + fatalError() + } + guard let signature = SecKeyCreateEncryptedData(key, signatureAlgorithm, data as CFData, &encryptError) else { + throw SmartCard.SigningError(error: encryptError) + } + return signature as Data + } + + /// Decrypts a payload with a specified key. + /// - Parameters: + /// - data: The payload to decrypt. + /// - secret: The secret to decrypt with. + /// - Returns: The decrypted data. + /// - Warning: Encryption functions are deliberately only exposed on a library level, and are not exposed in Secretive itself to prevent users from data loss. Any pull requests which expose this functionality in the app will not be merged. + public func decrypt(data: Data, with secret: SecretType) throws -> Data { + guard let tokenID = tokenID else { fatalError() } + let context = LAContext() + context.localizedReason = "decrypt data using secret \"\(secret.name)\"" + context.localizedCancelTitle = "Deny" + let attributes = KeychainDictionary([ + kSecClass: kSecClassKey, + kSecAttrKeyClass: kSecAttrKeyClassPrivate, + kSecAttrApplicationLabel: secret.id as CFData, + kSecAttrTokenID: tokenID, + kSecUseAuthenticationContext: context, + kSecReturnRef: true + ]) + var untyped: CFTypeRef? + let status = SecItemCopyMatching(attributes, &untyped) + if status != errSecSuccess { + throw SmartCard.KeychainError(statusCode: status) + } + guard let untypedSafe = untyped else { + throw SmartCard.KeychainError(statusCode: errSecSuccess) + } + let key = untypedSafe as! SecKey + var encryptError: SecurityError? + let signatureAlgorithm: SecKeyAlgorithm + switch (secret.algorithm, secret.keySize) { + case (.ellipticCurve, 256): + signatureAlgorithm = .eciesEncryptionStandardX963SHA256AESGCM + case (.ellipticCurve, 384): + signatureAlgorithm = .eciesEncryptionStandardX963SHA384AESGCM + case (.rsa, 1024), (.rsa, 2048): + signatureAlgorithm = .rsaEncryptionOAEPSHA512AESGCM + default: + fatalError() + } + guard let signature = SecKeyCreateDecryptedData(key, signatureAlgorithm, data as CFData, &encryptError) else { + throw SmartCard.SigningError(error: encryptError) + } + return signature as Data + } + +} + extension TKTokenWatcher { /// All available tokens, excluding the Secure Enclave. @@ -188,9 +319,3 @@ extension SmartCard { } } - -extension SmartCard { - - public typealias SecurityError = Unmanaged - -} diff --git a/Sources/Packages/Tests/SecretAgentKitTests/AgentTests.swift b/Sources/Packages/Tests/SecretAgentKitTests/AgentTests.swift index 2c3e29e..398da9f 100644 --- a/Sources/Packages/Tests/SecretAgentKitTests/AgentTests.swift +++ b/Sources/Packages/Tests/SecretAgentKitTests/AgentTests.swift @@ -61,8 +61,17 @@ class AgentTests: XCTestCase { var rs = r rs.append(s) let signature = try! P256.Signing.ECDSASignature(rawRepresentation: rs) - let valid = try! P256.Signing.PublicKey(x963Representation: Constants.Secrets.ecdsa256Secret.publicKey).isValidSignature(signature, for: dataToSign) - XCTAssertTrue(valid) + let referenceValid = try! P256.Signing.PublicKey(x963Representation: Constants.Secrets.ecdsa256Secret.publicKey).isValidSignature(signature, for: dataToSign) + let store = list.stores.first! + let derVerifies = try! store.verify(signature: signature.derRepresentation, for: dataToSign, with: AnySecret(Constants.Secrets.ecdsa256Secret)) + let invalidRandomSignature = try? store.verify(signature: "invalid".data(using: .utf8)!, for: dataToSign, with: AnySecret(Constants.Secrets.ecdsa256Secret)) + let invalidRandomData = try? store.verify(signature: signature.derRepresentation, for: "invalid".data(using: .utf8)!, with: AnySecret(Constants.Secrets.ecdsa256Secret)) + let invalidWrongKey = try? store.verify(signature: signature.derRepresentation, for: dataToSign, with: AnySecret(Constants.Secrets.ecdsa384Secret)) + XCTAssertTrue(referenceValid) + XCTAssertTrue(derVerifies) + XCTAssert(invalidRandomSignature == false) + XCTAssert(invalidRandomData == false) + XCTAssert(invalidWrongKey == false) } // MARK: Witness protocol diff --git a/Sources/Packages/Tests/SecretAgentKitTests/StubStore.swift b/Sources/Packages/Tests/SecretAgentKitTests/StubStore.swift index cc6cb3b..acbda1d 100644 --- a/Sources/Packages/Tests/SecretAgentKitTests/StubStore.swift +++ b/Sources/Packages/Tests/SecretAgentKitTests/StubStore.swift @@ -70,6 +70,42 @@ extension Stub { return SecKeyCreateSignature(privateKey, signatureAlgorithm, data as CFData, nil)! as Data } + public func verify(signature: Data, for data: Data, with secret: Stub.Secret) throws -> Bool { + let attributes = KeychainDictionary([ + kSecAttrKeyType: secret.algorithm.secAttrKeyType, + kSecAttrKeySizeInBits: secret.keySize, + kSecAttrKeyClass: kSecAttrKeyClassPublic + ]) + var verifyError: Unmanaged? + let untyped: CFTypeRef? = SecKeyCreateWithData(secret.publicKey as CFData, attributes, &verifyError) + guard let untypedSafe = untyped else { + throw NSError(domain: "test", code: 0, userInfo: nil) + } + let key = untypedSafe as! SecKey + let signatureAlgorithm: SecKeyAlgorithm + switch (secret.algorithm, secret.keySize) { + case (.ellipticCurve, 256): + signatureAlgorithm = .ecdsaSignatureMessageX962SHA256 + case (.ellipticCurve, 384): + signatureAlgorithm = .ecdsaSignatureMessageX962SHA384 + case (.rsa, 1024): + signatureAlgorithm = .rsaSignatureMessagePKCS1v15SHA512 + case (.rsa, 2048): + signatureAlgorithm = .rsaSignatureMessagePKCS1v15SHA512 + default: + fatalError() + } + let verified = SecKeyVerifySignature(key, signatureAlgorithm, data as CFData, signature as CFData, &verifyError) + if let verifyError { + if verifyError.takeUnretainedValue() ~= .verifyError { + return false + } else { + throw NSError(domain: "test", code: 0, userInfo: nil) + } + } + return verified + } + public func existingPersistedAuthenticationContext(secret: Stub.Secret) -> PersistedAuthenticationContext? { nil } diff --git a/Sources/Secretive/Preview Content/PreviewStore.swift b/Sources/Secretive/Preview Content/PreviewStore.swift index 839273e..8006d6c 100644 --- a/Sources/Secretive/Preview Content/PreviewStore.swift +++ b/Sources/Secretive/Preview Content/PreviewStore.swift @@ -40,6 +40,10 @@ extension Preview { return data } + func verify(data: Data, signature: Data, with secret: Preview.Secret) throws -> Bool { + true + } + func existingPersistedAuthenticationContext(secret: Preview.Secret) -> PersistedAuthenticationContext? { nil } diff --git a/Sources/Secretive/Views/EmptyStoreView.swift b/Sources/Secretive/Views/EmptyStoreView.swift index 689b00b..5bd48c1 100644 --- a/Sources/Secretive/Views/EmptyStoreView.swift +++ b/Sources/Secretive/Views/EmptyStoreView.swift @@ -34,7 +34,7 @@ struct EmptyStoreImmutableView: View { VStack { Text("No Secrets").bold() Text("Use your Smart Card's management tool to create a secret.") - Text("Secretive only supports Elliptic Curve keys.") + Text("Secretive supports EC256, EC384, RSA1024, and RSA2048 keys.") }.frame(maxWidth: .infinity, maxHeight: .infinity) }