From e31db0f4facd1895a8c1116087c1bd66f889c677 Mon Sep 17 00:00:00 2001 From: Noah Berman <15199622+bermannoah@users.noreply.github.com> Date: Wed, 26 Oct 2022 09:48:31 +0100 Subject: [PATCH 1/5] Add line about help/setup tool to FAQ (#382) Co-authored-by: Max Goedjen --- FAQ.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/FAQ.md b/FAQ.md index ad8057e..6652ec9 100644 --- a/FAQ.md +++ b/FAQ.md @@ -12,6 +12,10 @@ Secretive relies on the `SSH_AUTH_SOCK` environment variable being respected. Th Please run `ssh -Tv git@github.com` in your terminal and paste the output in a [new GitHub issue](https://github.com/maxgoedjen/secretive/issues/new) with a description of your issue. +### Secretive was working for me, but now it has stopped + +Try running the "Setup Secretive" process by clicking on "Help", then "Setup Secretive." If that doesn't work, follow the process above. + ### Secretive prompts me to type my password instead of using my Apple Watch 1) Make sure you have enabled "Use your Apple Watch to unlock apps and your Mac" in System Preferences --> Security & Privacy: From fa0e81cd8e6f0b9a30d9a935574ad33967a78e34 Mon Sep 17 00:00:00 2001 From: unreality Date: Thu, 27 Oct 2022 13:19:21 +0800 Subject: [PATCH 2/5] Initial openssh certificate support (#416) * initial openssh certificate support * use the certificate to construct the public key instead of storing a map in memory * move certificate check into its own function and neaten up a few things * final requested changes Co-authored-by: Max Goedjen --- .../Sources/SecretAgentKit/Agent.swift | 112 +++++++++++++++++- 1 file changed, 108 insertions(+), 4 deletions(-) diff --git a/Sources/Packages/Sources/SecretAgentKit/Agent.swift b/Sources/Packages/Sources/SecretAgentKit/Agent.swift index acf3cde..bc37853 100644 --- a/Sources/Packages/Sources/SecretAgentKit/Agent.swift +++ b/Sources/Packages/Sources/SecretAgentKit/Agent.swift @@ -4,6 +4,25 @@ import OSLog import SecretKit import AppKit +enum OpenSSHCertificateError: Error { + case unsupportedType + case parsingFailed + case doesNotExist +} + +extension OpenSSHCertificateError: CustomStringConvertible { + public var description: String { + switch self { + case .unsupportedType: + return "The key type was unsupported" + case .parsingFailed: + return "Failed to properly parse the SSH certificate" + case .doesNotExist: + return "Certificate does not exist" + } + } +} + /// The `Agent` is an implementation of an SSH agent. It manages coordination and access between a socket, traces requests, notifies witnesses and passes requests to stores. public class Agent { @@ -11,6 +30,7 @@ public class Agent { private let witness: SigningWitness? private let writer = OpenSSHKeyWriter() private let requestTracer = SigningRequestTracer() + private let certsPath = (NSHomeDirectory() as NSString).appendingPathComponent("PublicKeys") as String /// Initializes an agent with a store list and a witness. /// - Parameters: @@ -83,12 +103,22 @@ extension Agent { var count = UInt32(secrets.count).bigEndian let countData = Data(bytes: &count, count: UInt32.bitWidth/8) var keyData = Data() - let writer = OpenSSHKeyWriter() + for secret in secrets { - let keyBlob = writer.data(secret: secret) + let keyBlob: Data + let curveData: Data + + if let (certBlob, certName) = try? checkForCert(secret: secret) { + keyBlob = certBlob + curveData = certName + } else { + keyBlob = writer.data(secret: secret) + curveData = writer.curveType(for: secret.algorithm, length: secret.keySize).data(using: .utf8)! + } + keyData.append(writer.lengthAndData(of: keyBlob)) - let curveData = writer.curveType(for: secret.algorithm, length: secret.keySize).data(using: .utf8)! keyData.append(writer.lengthAndData(of: curveData)) + } Logger().debug("Agent enumerated \(secrets.count) identities") return countData + keyData @@ -101,7 +131,13 @@ extension Agent { /// - Returns: An OpenSSH formatted Data payload containing the signed data response. func sign(data: Data, provenance: SigningRequestProvenance) throws -> Data { let reader = OpenSSHReader(data: data) - let hash = reader.readNextChunk() + var hash = reader.readNextChunk() + + // Check if hash is actually an openssh certificate and reconstruct the public key if it is + if let certPublicKey = try? getPublicKeyFromCert(certBlob: hash) { + hash = certPublicKey + } + guard let (store, secret) = secret(matching: hash) else { Logger().debug("Agent did not have a key matching \(hash as NSData)") throw AgentError.noMatchingKey @@ -161,6 +197,74 @@ extension Agent { return signedData } + + /// Reconstructs a public key from a ``Data`` object that contains an OpenSSH certificate. Currently only ecdsa certificates are supported + /// - Parameter certBlock: The openssh certificate to extract the public key from + /// - Returns: A ``Data`` object containing the public key in OpenSSH wire format + func getPublicKeyFromCert(certBlob: Data) throws -> Data { + let reader = OpenSSHReader(data: certBlob) + let certType = String(decoding: reader.readNextChunk(), as: UTF8.self) + + switch certType { + case "ecdsa-sha2-nistp256-cert-v01@openssh.com", + "ecdsa-sha2-nistp384-cert-v01@openssh.com", + "ecdsa-sha2-nistp521-cert-v01@openssh.com": + + _ = reader.readNextChunk() // nonce + let curveIdentifier = reader.readNextChunk() + let publicKey = reader.readNextChunk() + + if let curveType = certType.replacingOccurrences(of: "-cert-v01@openssh.com", with: "").data(using: .utf8) { + return writer.lengthAndData(of: curveType) + + writer.lengthAndData(of: curveIdentifier) + + writer.lengthAndData(of: publicKey) + } else { + throw OpenSSHCertificateError.parsingFailed + } + default: + throw OpenSSHCertificateError.unsupportedType + } + } + + + /// Attempts to find an OpenSSH Certificate that corresponds to a ``Secret`` + /// - Parameter secret: The secret to search for a certificate with + /// - Returns: Two ``Data`` objects containing the certificate and certificate name respectively + func checkForCert(secret: AnySecret) throws -> (Data, Data) { + let minimalHex = writer.openSSHMD5Fingerprint(secret: secret).replacingOccurrences(of: ":", with: "") + let certificatePath = certsPath.appending("/").appending("\(minimalHex)-cert.pub") + + if FileManager.default.fileExists(atPath: certificatePath) { + Logger().debug("Found certificate for \(secret.name)") + do { + let certContent = try String(contentsOfFile:certificatePath, encoding: .utf8) + let certElements = certContent.trimmingCharacters(in: .whitespacesAndNewlines).components(separatedBy: " ") + + if certElements.count >= 2 { + if let certDecoded = Data(base64Encoded: certElements[1] as String) { + if certElements.count >= 3 { + 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") + return (certDecoded, certName) + } else { + throw OpenSSHCertificateError.parsingFailed + } + } + } else { + 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") + throw OpenSSHCertificateError.parsingFailed + } + } + + throw OpenSSHCertificateError.doesNotExist + } } From 47d736cb0d9ee3baf9d66b8211e85b4b94a53d7e Mon Sep 17 00:00:00 2001 From: Max Goedjen Date: Wed, 26 Oct 2022 22:46:43 -0700 Subject: [PATCH 3/5] Don't delete public cert keys corrresponding to secretive-tracked keys (#417) --- .../PublicKeyStandinFileController.swift | 23 +++++++++++++++---- .../Secretive/Views/SecretDetailView.swift | 2 +- 2 files changed, 20 insertions(+), 5 deletions(-) diff --git a/Sources/Packages/Sources/SecretKit/PublicKeyStandinFileController.swift b/Sources/Packages/Sources/SecretKit/PublicKeyStandinFileController.swift index 3d84317..cbf40a6 100644 --- a/Sources/Packages/Sources/SecretKit/PublicKeyStandinFileController.swift +++ b/Sources/Packages/Sources/SecretKit/PublicKeyStandinFileController.swift @@ -15,15 +15,21 @@ public class PublicKeyFileStoreController { /// Writes out the keys specified to disk. /// - Parameter secrets: The Secrets to generate keys for. - /// - Parameter clear: Whether or not the directory should be erased before writing keys. + /// - Parameter clear: Whether or not any untracked files in the directory should be removed. public func generatePublicKeys(for secrets: [AnySecret], clear: Bool = false) throws { logger.log("Writing public keys to disk") if clear { - try? FileManager.default.removeItem(at: URL(fileURLWithPath: directory)) + let validPaths = Set(secrets.map { publicKeyPath(for: $0) }).union(Set(secrets.map { sshCertificatePath(for: $0) })) + let untracked = Set(try FileManager.default.contentsOfDirectory(atPath: directory) + .map { "\(directory)/\($0)" }) + .subtracting(validPaths) + for path in untracked { + try? FileManager.default.removeItem(at: URL(fileURLWithPath: path)) + } } try? FileManager.default.createDirectory(at: URL(fileURLWithPath: directory), withIntermediateDirectories: false, attributes: nil) for secret in secrets { - let path = path(for: secret) + let path = publicKeyPath(for: secret) guard let data = keyWriter.openSSHString(secret: secret).data(using: .utf8) else { continue } FileManager.default.createFile(atPath: path, contents: data, attributes: nil) } @@ -34,9 +40,18 @@ public class PublicKeyFileStoreController { /// - Parameter secret: The Secret to return the path for. /// - Returns: The path to the Secret's public key. /// - Warning: This method returning a path does not imply that a key has been written to disk already. This method only describes where it will be written to. - public func path(for secret: SecretType) -> String { + public func publicKeyPath(for secret: SecretType) -> String { let minimalHex = keyWriter.openSSHMD5Fingerprint(secret: secret).replacingOccurrences(of: ":", with: "") return directory.appending("/").appending("\(minimalHex).pub") } + /// The path for a Secret's SSH Certificate public key. + /// - Parameter secret: The Secret to return the path for. + /// - Returns: The path to the SSH Certificate public key. + /// - Warning: This method returning a path does not imply that a key has a SSH certificates. This method only describes where it will be. + public func sshCertificatePath(for secret: SecretType) -> String { + let minimalHex = keyWriter.openSSHMD5Fingerprint(secret: secret).replacingOccurrences(of: ":", with: "") + return directory.appending("/").appending("\(minimalHex)-cert.pub") + } + } diff --git a/Sources/Secretive/Views/SecretDetailView.swift b/Sources/Secretive/Views/SecretDetailView.swift index d756679..52978d7 100644 --- a/Sources/Secretive/Views/SecretDetailView.swift +++ b/Sources/Secretive/Views/SecretDetailView.swift @@ -21,7 +21,7 @@ struct SecretDetailView: View { CopyableView(title: "Public Key", image: Image(systemName: "key"), text: keyString) Spacer() .frame(height: 20) - CopyableView(title: "Public Key Path", image: Image(systemName: "lock.doc"), text: publicKeyFileStoreController.path(for: secret)) + CopyableView(title: "Public Key Path", image: Image(systemName: "lock.doc"), text: publicKeyFileStoreController.publicKeyPath(for: secret)) Spacer() } } From 20cbaac6f6f0ed4de3ee94f23bd4abd416ecfc3e Mon Sep 17 00:00:00 2001 From: Max Goedjen Date: Thu, 27 Oct 2022 00:20:22 -0700 Subject: [PATCH 4/5] Fix cstring init (#420) --- .../Sources/SecretAgentKit/SigningRequestTracer.swift | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/Sources/Packages/Sources/SecretAgentKit/SigningRequestTracer.swift b/Sources/Packages/Sources/SecretAgentKit/SigningRequestTracer.swift index 46917f8..7e87538 100644 --- a/Sources/Packages/Sources/SecretAgentKit/SigningRequestTracer.swift +++ b/Sources/Packages/Sources/SecretAgentKit/SigningRequestTracer.swift @@ -40,7 +40,10 @@ extension SigningRequestTracer { func process(from pid: Int32) -> SigningRequestProvenance.Process { var pidAndNameInfo = self.pidAndNameInfo(from: pid) let ppid = pidAndNameInfo.kp_eproc.e_ppid != 0 ? pidAndNameInfo.kp_eproc.e_ppid : nil - let procName = String(cString: &pidAndNameInfo.kp_proc.p_comm.0) + let procName = withUnsafeMutablePointer(to: &pidAndNameInfo.kp_proc.p_comm.0) { pointer in + String(cString: pointer) + } + let pathPointer = UnsafeMutablePointer.allocate(capacity: Int(MAXPATHLEN)) _ = proc_pidpath(pid, pathPointer, UInt32(MAXPATHLEN)) let path = String(cString: pathPointer) From 382913cb996dc01b5974dc6bb092b84dc00df61d Mon Sep 17 00:00:00 2001 From: Max Goedjen Date: Thu, 27 Oct 2022 00:40:01 -0700 Subject: [PATCH 5/5] Update workflows to macOS-latest and Xcode 14.1 (#421) * Update nightly.yml * Update release.yml * Update test.yml --- .github/workflows/nightly.yml | 6 +++--- .github/workflows/release.yml | 8 ++++---- .github/workflows/test.yml | 4 ++-- 3 files changed, 9 insertions(+), 9 deletions(-) diff --git a/.github/workflows/nightly.yml b/.github/workflows/nightly.yml index 9bf7a43..e18eb91 100644 --- a/.github/workflows/nightly.yml +++ b/.github/workflows/nightly.yml @@ -5,7 +5,7 @@ on: - cron: "0 8 * * *" jobs: build: - runs-on: macos-11.0 + runs-on: macOS-latest timeout-minutes: 10 steps: - uses: actions/checkout@v2 @@ -19,7 +19,7 @@ jobs: APPLE_API_KEY_ID: ${{ secrets.APPLE_API_KEY_ID }} run: ./.github/scripts/signing.sh - name: Set Environment - run: sudo xcrun xcode-select -s /Applications/Xcode_13.2.1.app + run: sudo xcrun xcode-select -s /Applications/Xcode_14.1.app - name: Update Build Number env: RUN_ID: ${{ github.run_id }} @@ -50,4 +50,4 @@ jobs: uses: actions/upload-artifact@v1 with: name: Secretive.zip - path: Secretive.zip \ No newline at end of file + path: Secretive.zip diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index d6beba2..7982ce9 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -6,7 +6,7 @@ on: - '*' jobs: test: - runs-on: macos-11.0 + runs-on: macOS-latest timeout-minutes: 10 steps: - uses: actions/checkout@v1 @@ -20,14 +20,14 @@ jobs: APPLE_API_KEY_ID: ${{ secrets.APPLE_API_KEY_ID }} run: ./.github/scripts/signing.sh - name: Set Environment - run: sudo xcrun xcode-select -s /Applications/Xcode_13.2.1.app + run: sudo xcrun xcode-select -s /Applications/Xcode_14.1.app - name: Test run: | pushd Sources/Packages swift test popd build: - runs-on: macos-11.0 + runs-on: macOS-latest timeout-minutes: 10 steps: - uses: actions/checkout@v2 @@ -41,7 +41,7 @@ jobs: APPLE_API_KEY_ID: ${{ secrets.APPLE_API_KEY_ID }} run: ./.github/scripts/signing.sh - name: Set Environment - run: sudo xcrun xcode-select -s /Applications/Xcode_13.2.1.app + run: sudo xcrun xcode-select -s /Applications/Xcode_14.1.app - name: Update Build Number env: TAG_NAME: ${{ github.ref }} diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index c21d0c5..ac6ad14 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -3,12 +3,12 @@ name: Test on: [push, pull_request] jobs: test: - runs-on: macos-11.0 + runs-on: macOS-latest timeout-minutes: 10 steps: - uses: actions/checkout@v2 - name: Set Environment - run: sudo xcrun xcode-select -s /Applications/Xcode_13.2.1.app + run: sudo xcrun xcode-select -s /Applications/Xcode_14.1.app - name: Test run: | pushd Sources/Packages