diff --git a/Sources/ClavisCore/Crypto/KeychainManager.swift b/Sources/ClavisCore/Crypto/KeychainManager.swift index 4070848..9935188 100644 --- a/Sources/ClavisCore/Crypto/KeychainManager.swift +++ b/Sources/ClavisCore/Crypto/KeychainManager.swift @@ -78,12 +78,13 @@ public class KeychainManager { algorithm: String = "ECDSA P-256", storageType: KeyStorageType = .secureEnclave, biometricPolicy: BiometricPolicy? = nil, - keyPurpose: KeyPurpose = .general + keyPurpose: KeyPurpose = .general, + accessProfile: KeyAccessProfile? = nil ) throws -> SSHKeyInfo { #if DEBUG if let context = self.testContext, TestContextScope.current == nil { return try TestContextScope.$current.withValue(context) { - try self.generateKey(label: label, algorithm: algorithm, storageType: storageType, biometricPolicy: biometricPolicy, keyPurpose: keyPurpose) + try self.generateKey(label: label, algorithm: algorithm, storageType: storageType, biometricPolicy: biometricPolicy, keyPurpose: keyPurpose, accessProfile: accessProfile) } } #endif @@ -113,6 +114,7 @@ public class KeychainManager { storageType: storageType, biometricPolicy: policy, keyPurpose: keyPurpose, + accessProfile: accessProfile, keyData: keyPair.keyData ) defer { record.wipe() } @@ -144,7 +146,8 @@ public class KeychainManager { algorithmName: algorithm, storage: storageType, biometricPolicy: policy, - keyPurpose: keyPurpose + keyPurpose: keyPurpose, + accessProfile: record.accessProfile ) do { try PublicKeyStore.saveChecked(keyInfo) @@ -158,7 +161,7 @@ public class KeychainManager { type: .keyCreate, result: .allowed, keyFingerprint: keyInfo.fingerprint, - keyKind: .personal, + keyKind: record.accessProfile.type == .bot ? .agent : .personal, sensitive: AuditSensitive(keyLabel: keyInfo.label, processChain: [], host: nil) ) ) @@ -825,62 +828,8 @@ public class KeychainManager { throw KeyPurposeError.kindChangeNotAllowed } - var updatedRecord = StoredPrivateKeyRecord( - label: record.label, - algorithm: record.algorithm, - storageType: record.storageType, - biometricPolicy: record.biometricPolicy, - keyPurpose: newPurpose, - accessProfile: record.accessProfile.changingType(to: newPurpose == .agent ? .bot : .personal), - keyData: record.keyData, - createdAt: record.createdAt - ) - defer { updatedRecord.wipe() } - - let hasVaultRecord = vaultStore.hasRecord(label: label) - if hasVaultRecord { - try vaultStore.saveRecord(updatedRecord) - } - - var recordData = try updatedRecord.encode() - defer { Self.wipeData(&recordData) } - - do { - try privateKeyStore.save(label: label, data: recordData, accessControlFlags: []) - } catch { - if hasVaultRecord { - try? vaultStore.saveRecord(record) - } - throw error - } - - let updatedKeyInfo = SSHKeyInfo( - label: key.label, - publicKeyOpenSSH: key.publicKeyOpenSSH, - publicKeyBlob: key.publicKeyBlob, - fingerprint: key.fingerprint, - createdAt: key.createdAt, - algorithmName: key.algorithmName, - storage: key.storageType, - biometricPolicy: key.biometricPolicy, - keyPurpose: newPurpose, - accessProfile: updatedRecord.accessProfile - ) - - do { - try PublicKeyStore.saveChecked(updatedKeyInfo) - } catch { - if var originalRecordData = try? record.encode() { - defer { Self.wipeData(&originalRecordData) } - try? privateKeyStore.save(label: label, data: originalRecordData, accessControlFlags: []) - } - if hasVaultRecord { - try? vaultStore.saveRecord(record) - } - throw error - } - - try revokeKeyCapabilities(label: label) + try persistAccessProfile(record.accessProfile.changingType(to: newPurpose == .agent ? .bot : .personal), + purpose: newPurpose, key: key, record: record) auditRecorder.record( AuditEvent( @@ -895,6 +844,94 @@ public class KeychainManager { } } + private func persistAccessProfile(_ profile: KeyAccessProfile, purpose newPurpose: KeyPurpose, + key: SSHKeyInfo, record: StoredPrivateKeyRecord) throws { + let label = key.label + // Never persist a broader policy while the daemon can still reuse old approvals. + // Revocation failure leaves all stored copies untouched. + try revokeKeyCapabilities(label: label) + var updatedRecord = StoredPrivateKeyRecord( + label: record.label, + algorithm: record.algorithm, + storageType: record.storageType, + biometricPolicy: record.biometricPolicy, + keyPurpose: newPurpose, + accessProfile: profile, + keyData: record.keyData, + createdAt: record.createdAt + ) + defer { updatedRecord.wipe() } + + let hasVaultRecord = vaultStore.hasRecord(label: label) + if hasVaultRecord { + try vaultStore.saveRecord(updatedRecord) + } + + var recordData = try updatedRecord.encode() + defer { Self.wipeData(&recordData) } + + do { + try privateKeyStore.save(label: label, data: recordData, accessControlFlags: []) + } catch { + if hasVaultRecord { + try? vaultStore.saveRecord(record) + } + throw error + } + + let updatedKeyInfo = SSHKeyInfo( + label: key.label, + publicKeyOpenSSH: key.publicKeyOpenSSH, + publicKeyBlob: key.publicKeyBlob, + fingerprint: key.fingerprint, + createdAt: key.createdAt, + algorithmName: key.algorithmName, + storage: key.storageType, + biometricPolicy: key.biometricPolicy, + keyPurpose: newPurpose, + accessProfile: updatedRecord.accessProfile + ) + + do { + try PublicKeyStore.saveChecked(updatedKeyInfo) + } catch { + if var originalRecordData = try? record.encode() { + defer { Self.wipeData(&originalRecordData) } + try? privateKeyStore.save(label: label, data: originalRecordData, accessControlFlags: []) + } + if hasVaultRecord { + try? vaultStore.saveRecord(record) + } + throw error + } + + } + + /// A type-only change never changes operation capabilities or legacy Git restrictions. + /// Authentication cannot be supplied by the caller or reused from a previous unlock. + public func changeAccessProfile(label: String, type: KeyType? = nil, + capabilities: KeyCapabilities? = nil) throws { + guard let key = try fetchKeyInfo(label: label) else { + throw KeychainManagerError.keyNotFound(label: label) + } + let profile = KeyAccessProfile(type: type ?? key.effectiveAccessProfile.type, + capabilities: capabilities ?? key.effectiveAccessProfile.capabilities) + guard profile != key.effectiveAccessProfile else { return } + try withVerifiedPrivateKeyRecord(for: key, + prompt: "Change access policy for '\(label)'", + auditOnAuthFailure: true, auditKeyKind: profile.type == .bot ? .agent : .personal + ) { context, record in + defer { context.invalidate() } + // Retain Git-only restrictions unless the user explicitly enables SSH. + let purpose: KeyPurpose = record.purpose == .gitSigningOnly && !profile.capabilities.ssh + ? .gitSigningOnly : .general + try persistAccessProfile(profile, purpose: purpose, key: key, record: record) + auditRecorder.record(AuditEvent(type: .keyKindChange, result: .allowed, + keyFingerprint: key.fingerprint, keyKind: profile.type == .bot ? .agent : .personal, + sensitive: .init(keyLabel: label, processChain: [], host: nil))) + } + } + /// Rename an existing key label across the private key store, shadow vault, and public key store. @discardableResult public func renameKey(from oldLabel: String, to newLabel: String) throws -> SSHKeyInfo { diff --git a/Tests/ClavisCoreTests/KeyProfileMutationTests.swift b/Tests/ClavisCoreTests/KeyProfileMutationTests.swift new file mode 100644 index 0000000..843d8f6 --- /dev/null +++ b/Tests/ClavisCoreTests/KeyProfileMutationTests.swift @@ -0,0 +1,143 @@ +import Foundation +import LocalAuthentication +import Testing +@testable import ClavisCore +@testable import ClavisTestSupport + +@Suite(.serialized) struct KeyProfileMutationTests { + private struct Rejecting: UserAuthenticating { + func authenticate(reason: String, policy: LAPolicy) throws -> LAContext { throw LAError(.userCancel) } + func authenticate(reason: String, policy: LAPolicy) async throws -> LAContext { throw LAError(.userCancel) } + } + + @Test(arguments: KeyType.allCases) + func generationPersistsIndependentProfile(type: KeyType) async throws { + let fixture = try ClavisTestFixture() + defer { fixture.cleanup() } + try await fixture.run { + let store = InMemoryPrivateKeyStore() + let manager = fixture.makeKeyManager(privateKeyStore: store) + let profile = KeyAccessProfile(type: type, capabilities: .init(ssh: false, signature: true)) + let key = try manager.generateKey(label: "new-profile", accessProfile: profile) + let raw = try #require(try store.load(label: key.label, context: LAContext(), prompt: "")) + let record = try StoredPrivateKeyRecord.decode(from: raw) + #expect(record.accessProfile == profile) + #expect(key.effectiveAccessProfile == profile) + #expect(try manager.fetchKeyInfo(label: key.label)?.effectiveAccessProfile == profile) + #expect(throws: KeyAccessError.operationDisabled(.ssh)) { + try manager.signSSH(key: key, data: Data("ssh".utf8), prompt: "") + } + } + } + + @Test(arguments: KeyPurpose.allCases) + func typeChangePreservesCapabilitiesAndMaterial(purpose: KeyPurpose) async throws { + let fixture = try ClavisTestFixture() + defer { fixture.cleanup() } + try await fixture.run { + let store = InMemoryPrivateKeyStore() + let auth = CountingAuthenticator() + let manager = fixture.makeKeyManager(authenticator: auth, privateKeyStore: store) + let key = try manager.generateKey(label: "type-change", keyPurpose: purpose) + let raw = try #require(try store.load(label: key.label, context: LAContext(), prompt: "")) + let original = try StoredPrivateKeyRecord.decode(from: raw) + let target: KeyType = key.effectiveAccessProfile.type == .bot ? .personal : .bot + try manager.changeAccessProfile(label: key.label, type: target) + let changed = try #require(try manager.fetchKeyInfo(label: key.label)) + let updated = try StoredPrivateKeyRecord.decode(from: #require(try store.load(label: key.label, context: LAContext(), prompt: ""))) + #expect(auth.authenticationCount == 1) + #expect(changed.effectiveAccessProfile.type == target) + #expect(changed.effectiveAccessProfile.capabilities == key.effectiveAccessProfile.capabilities) + #expect(updated.accessProfile == changed.effectiveAccessProfile) + #expect(updated.keyData == original.keyData) + #expect(updated.createdAt == original.createdAt) + #expect(changed.fingerprint == key.fingerprint) + #expect(changed.publicKeyBlob == key.publicKeyBlob) + if purpose == .gitSigningOnly { #expect(changed.purpose == .gitSigningOnly) } + } + } + + @Test func everyActualChangeAuthenticatesAndNoOpDoesNot() async throws { + let fixture = try ClavisTestFixture() + defer { fixture.cleanup() } + try await fixture.run { + let auth = CountingAuthenticator() + let manager = fixture.makeKeyManager(authenticator: auth) + let key = try manager.generateKey(label: "fresh-auth") + try manager.changeAccessProfile(label: key.label, type: .personal) + #expect(auth.authenticationCount == 0) + try manager.changeAccessProfile(label: key.label, type: .bot) + try manager.changeAccessProfile(label: key.label, capabilities: .init(ssh: true, signature: false)) + try manager.changeAccessProfile(label: key.label, type: .personal) + #expect(auth.authenticationCount == 3) + let fresh = try #require(try manager.fetchKeyInfo(label: key.label)) + #expect(fresh.effectiveAccessProfile.type == .personal) + #expect(!fresh.effectiveAccessProfile.capabilities.signature) + } + } + + @Test func cancellationPreservesAllStoredPolicy() async throws { + let fixture = try ClavisTestFixture() + defer { fixture.cleanup() } + try await fixture.run { + let store = InMemoryPrivateKeyStore() + let manager = fixture.makeKeyManager(privateKeyStore: store) + let key = try manager.generateKey(label: "cancel-policy") + let raw = try store.load(label: key.label, context: LAContext(), prompt: "") + let failing = fixture.makeKeyManager(authenticator: Rejecting(), privateKeyStore: store) + #expect(throws: (any Error).self) { + try failing.changeAccessProfile(label: key.label, type: .bot, capabilities: .init(ssh: false, signature: false)) + } + #expect(try manager.fetchKeyInfo(label: key.label) == key) + #expect(try store.load(label: key.label, context: LAContext(), prompt: "") == raw) + } + } + + @Test func disablingBothOperationsIsARealPersistedPolicy() async throws { + let fixture = try ClavisTestFixture() + defer { fixture.cleanup() } + try await fixture.run { + let manager = fixture.makeKeyManager() + let key = try manager.generateKey(label: "disabled") + try manager.changeAccessProfile(label: key.label, capabilities: .init(ssh: false, signature: false)) + let fresh = try #require(try manager.fetchKeyInfo(label: key.label)) + for operation in KeyOperation.allCases { + #expect(throws: KeyAccessError.operationDisabled(operation)) { + try fresh.effectiveAccessProfile.require(operation) + } + } + } + } + + @Test func indexFailureRollsBackPrivateRecord() async throws { + let fixture = try ClavisTestFixture() + defer { fixture.cleanup() } + try await fixture.run { + let store = InMemoryPrivateKeyStore() + let manager = fixture.makeKeyManager(privateKeyStore: store) + let key = try manager.generateKey(label: "rollback-profile") + let raw = try store.load(label: key.label, context: LAContext(), prompt: "") + PublicKeyStore.forcedSaveErrorForTesting = NSError(domain: "test", code: 1) + defer { PublicKeyStore.forcedSaveErrorForTesting = nil } + #expect(throws: (any Error).self) { try manager.changeAccessProfile(label: key.label, type: .bot) } + #expect(try store.load(label: key.label, context: LAContext(), prompt: "") == raw) + #expect(try manager.fetchKeyInfo(label: key.label) == key) + } + } + @Test func revocationFailureDoesNotCommitNewPolicy() async throws { + let fixture = try ClavisTestFixture() + defer { fixture.cleanup() } + try await fixture.run { + let store = InMemoryPrivateKeyStore() + let initial = fixture.makeKeyManager(privateKeyStore: store) + let key = try initial.generateKey(label: "revoke-failure") + let raw = try store.load(label: key.label, context: LAContext(), prompt: "") + let manager = fixture.makeKeyManager(privateKeyStore: store, + agentGrantRevoker: { _ in throw NSError(domain: "revocation", code: 1) }) + #expect(throws: (any Error).self) { try manager.changeAccessProfile(label: key.label, type: .bot) } + #expect(try store.load(label: key.label, context: LAContext(), prompt: "") == raw) + #expect(try initial.fetchKeyInfo(label: key.label) == key) + } + } + +}