From 3b254d33a52d09191c3898000ca47d0761e68c6d Mon Sep 17 00:00:00 2001 From: Max Goedjen Date: Sat, 17 Dec 2022 23:16:56 -0800 Subject: [PATCH 1/2] Fix for SecretAgent acting as if it has no keys after updating macOS (#427) * Allow reload pre-op * Fix tests * Make sure standin keys get rewritten on force update * Stub store --- .../Sources/SecretAgentKit/Agent.swift | 41 ++++++++++++------- .../SecretKit/Erasers/AnySecretStore.swift | 6 +++ .../Sources/SecretKit/Types/SecretStore.swift | 3 ++ .../SecureEnclaveStore.swift | 23 +++++++---- .../SmartCardSecretKit/SmartCardStore.swift | 22 ++++++---- .../Tests/SecretAgentKitTests/StubStore.swift | 3 ++ .../Preview Content/PreviewStore.swift | 3 ++ 7 files changed, 70 insertions(+), 31 deletions(-) diff --git a/Sources/Packages/Sources/SecretAgentKit/Agent.swift b/Sources/Packages/Sources/SecretAgentKit/Agent.swift index bc37853..ce1c8b4 100644 --- a/Sources/Packages/Sources/SecretAgentKit/Agent.swift +++ b/Sources/Packages/Sources/SecretAgentKit/Agent.swift @@ -31,13 +31,14 @@ public class Agent { private let writer = OpenSSHKeyWriter() private let requestTracer = SigningRequestTracer() private let certsPath = (NSHomeDirectory() as NSString).appendingPathComponent("PublicKeys") as String + private let logger = Logger(subsystem: "com.maxgoedjen.secretive.secretagent.agent", category: "") /// Initializes an agent with a store list and a witness. /// - Parameters: /// - storeList: The `SecretStoreList` to make available. /// - witness: A witness to notify of requests. public init(storeList: SecretStoreList, witness: SigningWitness? = nil) { - Logger().debug("Agent is running") + logger.debug("Agent is running") self.storeList = storeList self.witness = witness } @@ -53,16 +54,16 @@ extension Agent { /// - Return value: /// - Boolean if data could be read @discardableResult public func handle(reader: FileHandleReader, writer: FileHandleWriter) -> Bool { - Logger().debug("Agent handling new data") + logger.debug("Agent handling new data") let data = Data(reader.availableData) guard data.count > 4 else { return false} let requestTypeInt = data[4] guard let requestType = SSHAgent.RequestType(rawValue: requestTypeInt) else { writer.write(OpenSSHKeyWriter().lengthAndData(of: SSHAgent.ResponseType.agentFailure.data)) - Logger().debug("Agent returned \(SSHAgent.ResponseType.agentFailure.debugDescription)") + logger.debug("Agent returned \(SSHAgent.ResponseType.agentFailure.debugDescription)") return true } - Logger().debug("Agent handling request of type \(requestType.debugDescription)") + logger.debug("Agent handling request of type \(requestType.debugDescription)") let subData = Data(data[5...]) let response = handle(requestType: requestType, data: subData, reader: reader) writer.write(response) @@ -70,23 +71,25 @@ extension Agent { } func handle(requestType: SSHAgent.RequestType, data: Data, reader: FileHandleReader) -> Data { + // Depending on the launch context (such as after macOS update), the agent may need to reload secrets before acting + reloadSecretsIfNeccessary() var response = Data() do { switch requestType { case .requestIdentities: response.append(SSHAgent.ResponseType.agentIdentitiesAnswer.data) response.append(identities()) - Logger().debug("Agent returned \(SSHAgent.ResponseType.agentIdentitiesAnswer.debugDescription)") + logger.debug("Agent returned \(SSHAgent.ResponseType.agentIdentitiesAnswer.debugDescription)") case .signRequest: let provenance = requestTracer.provenance(from: reader) response.append(SSHAgent.ResponseType.agentSignResponse.data) response.append(try sign(data: data, provenance: provenance)) - Logger().debug("Agent returned \(SSHAgent.ResponseType.agentSignResponse.debugDescription)") + logger.debug("Agent returned \(SSHAgent.ResponseType.agentSignResponse.debugDescription)") } } catch { response.removeAll() response.append(SSHAgent.ResponseType.agentFailure.data) - Logger().debug("Agent returned \(SSHAgent.ResponseType.agentFailure.debugDescription)") + logger.debug("Agent returned \(SSHAgent.ResponseType.agentFailure.debugDescription)") } let full = OpenSSHKeyWriter().lengthAndData(of: response) return full @@ -120,7 +123,7 @@ extension Agent { keyData.append(writer.lengthAndData(of: curveData)) } - Logger().debug("Agent enumerated \(secrets.count) identities") + logger.log("Agent enumerated \(secrets.count) identities") return countData + keyData } @@ -139,7 +142,7 @@ extension Agent { } guard let (store, secret) = secret(matching: hash) else { - Logger().debug("Agent did not have a key matching \(hash as NSData)") + logger.debug("Agent did not have a key matching \(hash as NSData)") throw AgentError.noMatchingKey } @@ -193,7 +196,7 @@ extension Agent { try witness.witness(accessTo: secret, from: store, by: provenance) } - Logger().debug("Agent signed request") + logger.debug("Agent signed request") return signedData } @@ -235,7 +238,7 @@ extension Agent { let certificatePath = certsPath.appending("/").appending("\(minimalHex)-cert.pub") if FileManager.default.fileExists(atPath: certificatePath) { - Logger().debug("Found certificate for \(secret.name)") + logger.debug("Found certificate for \(secret.name)") do { let certContent = try String(contentsOfFile:certificatePath, encoding: .utf8) let certElements = certContent.trimmingCharacters(in: .whitespacesAndNewlines).components(separatedBy: " ") @@ -246,19 +249,19 @@ extension Agent { if let certName = certElements[2].data(using: .utf8) { return (certDecoded, certName) } else if let certName = secret.name.data(using: .utf8) { - Logger().info("Certificate for \(secret.name) does not have a name tag, using secret name instead") + logger.info("Certificate for \(secret.name) does not have a name tag, using secret name instead") return (certDecoded, certName) } else { throw OpenSSHCertificateError.parsingFailed } } } else { - Logger().warning("Certificate found for \(secret.name) but failed to decode base64 key") + logger.warning("Certificate found for \(secret.name) but failed to decode base64 key") throw OpenSSHCertificateError.parsingFailed } } } catch { - Logger().warning("Certificate found for \(secret.name) but failed to load") + logger.warning("Certificate found for \(secret.name) but failed to load") throw OpenSSHCertificateError.parsingFailed } } @@ -270,6 +273,16 @@ extension Agent { extension Agent { + /// Gives any store with no loaded secrets a chance to reload. + func reloadSecretsIfNeccessary() { + for store in storeList.stores { + if store.secrets.isEmpty { + logger.debug("Store \(store.name, privacy: .public) has no loaded secrets. Reloading.") + store.reloadSecrets() + } + } + } + /// Finds a ``Secret`` matching a specified hash whos signature was requested. /// - Parameter hash: The hash to match against. /// - Returns: A ``Secret`` and the ``SecretStore`` containing it, if a match is found. diff --git a/Sources/Packages/Sources/SecretKit/Erasers/AnySecretStore.swift b/Sources/Packages/Sources/SecretKit/Erasers/AnySecretStore.swift index 4a05975..0cb6c40 100644 --- a/Sources/Packages/Sources/SecretKit/Erasers/AnySecretStore.swift +++ b/Sources/Packages/Sources/SecretKit/Erasers/AnySecretStore.swift @@ -12,6 +12,7 @@ public class AnySecretStore: SecretStore { private let _sign: (Data, AnySecret, SigningRequestProvenance) throws -> Data private let _existingPersistedAuthenticationContext: (AnySecret) -> PersistedAuthenticationContext? private let _persistAuthentication: (AnySecret, TimeInterval) throws -> Void + private let _reloadSecrets: () -> Void private var sink: AnyCancellable? @@ -24,6 +25,7 @@ public class AnySecretStore: SecretStore { _sign = { try secretStore.sign(data: $0, with: $1.base as! SecretStoreType.SecretType, for: $2) } _existingPersistedAuthenticationContext = { secretStore.existingPersistedAuthenticationContext(secret: $0.base as! SecretStoreType.SecretType) } _persistAuthentication = { try secretStore.persistAuthentication(secret: $0.base as! SecretStoreType.SecretType, forDuration: $1) } + _reloadSecrets = { secretStore.reloadSecrets() } sink = secretStore.objectWillChange.sink { _ in self.objectWillChange.send() } @@ -57,6 +59,10 @@ public class AnySecretStore: SecretStore { try _persistAuthentication(secret, duration) } + public func reloadSecrets() { + _reloadSecrets() + } + } public class AnySecretStoreModifiable: AnySecretStore, SecretStoreModifiable { diff --git a/Sources/Packages/Sources/SecretKit/Types/SecretStore.swift b/Sources/Packages/Sources/SecretKit/Types/SecretStore.swift index 2251f5e..f13fec7 100644 --- a/Sources/Packages/Sources/SecretKit/Types/SecretStore.swift +++ b/Sources/Packages/Sources/SecretKit/Types/SecretStore.swift @@ -36,6 +36,9 @@ public protocol SecretStore: ObservableObject, Identifiable { /// - Note: This is used for temporarily unlocking access to a secret which would otherwise require authentication every single use. This is useful for situations where the user anticipates several rapid accesses to a authorization-guarded secret. func persistAuthentication(secret: SecretType, forDuration duration: TimeInterval) throws + /// Requests that the store reload secrets from any backing store, if neccessary. + func reloadSecrets() + } /// A SecretStore that the Secretive admin app can modify. diff --git a/Sources/Packages/Sources/SecureEnclaveSecretKit/SecureEnclaveStore.swift b/Sources/Packages/Sources/SecureEnclaveSecretKit/SecureEnclaveStore.swift index 814d4af..13e15e2 100644 --- a/Sources/Packages/Sources/SecureEnclaveSecretKit/SecureEnclaveStore.swift +++ b/Sources/Packages/Sources/SecureEnclaveSecretKit/SecureEnclaveStore.swift @@ -24,7 +24,7 @@ extension SecureEnclave { /// Initializes a Store. public init() { DistributedNotificationCenter.default().addObserver(forName: .secretStoreUpdated, object: nil, queue: .main) { _ in - self.reloadSecrets(notifyAgent: false) + self.reloadSecretsInternal(notifyAgent: false) } loadSecrets() } @@ -68,7 +68,7 @@ extension SecureEnclave { throw KeychainError(statusCode: nil) } try savePublicKey(publicKey, name: name) - reloadSecrets() + reloadSecretsInternal() } public func delete(secret: Secret) throws { @@ -80,7 +80,7 @@ extension SecureEnclave { if status != errSecSuccess { throw KeychainError(statusCode: status) } - reloadSecrets() + reloadSecretsInternal() } public func update(secret: Secret, name: String) throws { @@ -97,7 +97,7 @@ extension SecureEnclave { if status != errSecSuccess { throw KeychainError(statusCode: status) } - reloadSecrets() + reloadSecretsInternal() } public func sign(data: Data, with secret: SecretType, for provenance: SigningRequestProvenance) throws -> Data { @@ -163,6 +163,10 @@ extension SecureEnclave { } } + public func reloadSecrets() { + reloadSecretsInternal(notifyAgent: false) + } + } } @@ -171,12 +175,15 @@ extension SecureEnclave.Store { /// Reloads all secrets from the store. /// - Parameter notifyAgent: A boolean indicating whether a distributed notification should be posted, notifying other processes (ie, the SecretAgent) to reload their stores as well. - private func reloadSecrets(notifyAgent: Bool = true) { + private func reloadSecretsInternal(notifyAgent: Bool = true) { + let before = secrets secrets.removeAll() loadSecrets() - NotificationCenter.default.post(name: .secretStoreReloaded, object: self) - if notifyAgent { - DistributedNotificationCenter.default().postNotificationName(.secretStoreUpdated, object: nil, deliverImmediately: true) + if secrets != before { + NotificationCenter.default.post(name: .secretStoreReloaded, object: self) + if notifyAgent { + DistributedNotificationCenter.default().postNotificationName(.secretStoreUpdated, object: nil, deliverImmediately: true) + } } } diff --git a/Sources/Packages/Sources/SmartCardSecretKit/SmartCardStore.swift b/Sources/Packages/Sources/SmartCardSecretKit/SmartCardStore.swift index 4f90983..ef7c23e 100644 --- a/Sources/Packages/Sources/SmartCardSecretKit/SmartCardStore.swift +++ b/Sources/Packages/Sources/SmartCardSecretKit/SmartCardStore.swift @@ -89,6 +89,19 @@ extension SmartCard { public func persistAuthentication(secret: SmartCard.Secret, forDuration: TimeInterval) throws { } + /// Reloads all secrets from the store. + public func reloadSecrets() { + DispatchQueue.main.async { + self.isAvailable = self.tokenID != nil + let before = self.secrets + self.secrets.removeAll() + self.loadSecrets() + if self.secrets != before { + NotificationCenter.default.post(name: .secretStoreReloaded, object: self) + } + } + } + } } @@ -102,15 +115,6 @@ extension SmartCard.Store { reloadSecrets() } - /// Reloads all secrets from the store. - private func reloadSecrets() { - DispatchQueue.main.async { - self.isAvailable = self.tokenID != nil - self.secrets.removeAll() - self.loadSecrets() - } - } - /// Loads all secrets from the store. private func loadSecrets() { guard let tokenID = tokenID else { return } diff --git a/Sources/Packages/Tests/SecretAgentKitTests/StubStore.swift b/Sources/Packages/Tests/SecretAgentKitTests/StubStore.swift index a4fd344..ae6af46 100644 --- a/Sources/Packages/Tests/SecretAgentKitTests/StubStore.swift +++ b/Sources/Packages/Tests/SecretAgentKitTests/StubStore.swift @@ -78,6 +78,9 @@ extension Stub { public func persistAuthentication(secret: Stub.Secret, forDuration duration: TimeInterval) throws { } + public func reloadSecrets() { + } + } } diff --git a/Sources/Secretive/Preview Content/PreviewStore.swift b/Sources/Secretive/Preview Content/PreviewStore.swift index a557f36..839273e 100644 --- a/Sources/Secretive/Preview Content/PreviewStore.swift +++ b/Sources/Secretive/Preview Content/PreviewStore.swift @@ -47,6 +47,9 @@ extension Preview { func persistAuthentication(secret: Preview.Secret, forDuration duration: TimeInterval) throws { } + func reloadSecrets() { + } + } class StoreModifiable: Store, SecretStoreModifiable { From 1a362ef9556591acd63d1c4856e1144b7a270082 Mon Sep 17 00:00:00 2001 From: Max Goedjen Date: Sun, 18 Dec 2022 15:16:08 -0800 Subject: [PATCH 2/2] Tighten up light-scheme colors (#429) * Tighten up light-scheme colors * Tweak more * Little more --- Sources/Secretive/Views/CopyableView.swift | 23 ++++++++++------------ 1 file changed, 10 insertions(+), 13 deletions(-) diff --git a/Sources/Secretive/Views/CopyableView.swift b/Sources/Secretive/Views/CopyableView.swift index f7cd9e9..bb1733d 100644 --- a/Sources/Secretive/Views/CopyableView.swift +++ b/Sources/Secretive/Views/CopyableView.swift @@ -8,6 +8,7 @@ struct CopyableView: View { var text: String @State private var interactionState: InteractionState = .normal + @Environment(\.colorScheme) private var colorScheme var body: some View { VStack(alignment: .leading) { @@ -77,38 +78,32 @@ struct CopyableView: View { } var backgroundColor: Color { - let color: NSColor switch interactionState { case .normal: - color = .windowBackgroundColor + return colorScheme == .dark ? Color(white: 0.2) : Color(white: 0.885) case .hovering: - color = .unemphasizedSelectedContentBackgroundColor + return colorScheme == .dark ? Color(white: 0.275) : Color(white: 0.82) case .clicking: - color = .selectedContentBackgroundColor + return .accentColor } - return Color(color) } var primaryTextColor: Color { - let color: NSColor switch interactionState { case .normal, .hovering: - color = .textColor + return Color(.textColor) case .clicking: - color = .white + return .white } - return Color(color) } var secondaryTextColor: Color { - let color: NSColor switch interactionState { case .normal, .hovering: - color = .secondaryLabelColor + return Color(.secondaryLabelColor) case .clicking: - color = .white + return .white } - return Color(color) } func copy() { @@ -128,7 +123,9 @@ struct CopyableView_Previews: PreviewProvider { static var previews: some View { Group { CopyableView(title: "Title", image: Image(systemName: "figure.wave"), text: "Hello world.") + .padding() CopyableView(title: "Title", image: Image(systemName: "figure.wave"), text: "Long text. Long text. Long text. Long text. Long text. Long text. Long text. Long text. Long text. Long text. Long text. Long text. Long text. Long text. Long text. Long text. Long text. Long text. Long text. Long text. Long text. Long text. Long text. Long text. Long text. Long text. Long text. Long text. Long text. ") + .padding() } } }