Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 13 additions & 2 deletions mac-app/Sources/CloudMachineCore/DriveEncryption.swift
Original file line number Diff line number Diff line change
Expand Up @@ -555,6 +555,15 @@ public enum DriveEncryption {
return Data(bytes).base64EncodedString()
}

/// What `setup` keeps in the Keychain: the password and the salt exactly as
/// `obscure` encrypted them. A trimmed copy of a password with an edge
/// space would not decrypt the backup.
static func keychainItems(password: String, salt: String)
-> [(account: String, value: String, exact: Bool)]
{
[(passwordAccount, password, true), (saltAccount, salt, true)]
}

/// `rclone obscure`, the secret through the environment (not `ps`).
static func obscure(_ secret: String) async -> String? {
let result = try? await ProcessRunner.run(
Expand Down Expand Up @@ -608,8 +617,10 @@ public enum DriveEncryption {
// the one mistake here that cannot be undone.
let service = keychainService(instance: VMLayout.instanceName)
do {
try await KeychainStore.save(password, account: passwordAccount, service: service)
try await KeychainStore.save(salt, account: saltAccount, service: service)
for item in keychainItems(password: password, salt: salt) {
try await KeychainStore.save(
item.value, account: item.account, service: service, exact: item.exact)
}
} catch {
return fail(error.localizedDescription)
}
Expand Down
21 changes: 17 additions & 4 deletions mac-app/Sources/CloudMachineCore/KeychainStore.swift
Original file line number Diff line number Diff line change
Expand Up @@ -37,19 +37,32 @@ public enum KeychainStore {
/// Saves or overwrites an item. `-U` means "replace if it already exists" -
/// without it, fixing a typo would end in an error with the old value still
/// in use.
public static func save(_ value: String, account: String, service: String) async throws {
let trimmed = value.trimmingCharacters(in: .whitespacesAndNewlines)
guard !trimmed.isEmpty else { throw StoreError.emptyValue }
///
/// `exact`: store the value as given. For a secret that is used elsewhere
/// untrimmed - the encryption password goes to `rclone obscure` exactly as
/// typed, so a trimmed Keychain copy would not decrypt the backup.
public static func save(
_ value: String, account: String, service: String, exact: Bool = false
) async throws {
let stored = try valueToStore(value, exact: exact)

let result = try? await ProcessRunner.run(
"/usr/bin/security",
["add-generic-password", "-a", account, "-s", service, "-w", trimmed, "-U"],
["add-generic-password", "-a", account, "-s", service, "-w", stored, "-U"],
timeout: 30)
guard result?.succeeded == true else {
throw StoreError.failed(result?.stderr ?? L10n.tr("unknown error"))
}
}

/// The value `save` hands to `security`: trimmed (pasted keys carry
/// newlines), or as given with `exact`. Blank is refused either way.
static func valueToStore(_ value: String, exact: Bool = false) throws -> String {
let trimmed = value.trimmingCharacters(in: .whitespacesAndNewlines)
guard !trimmed.isEmpty else { throw StoreError.emptyValue }
return exact ? value : trimmed
}

/// Whether the item exists - WITHOUT `-w`, i.e. without reaching for the
/// value itself.
///
Expand Down
46 changes: 46 additions & 0 deletions mac-app/Tests/CloudMachineAppTests/KeychainStoreValueTests.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
import XCTest

@testable import CloudMachineCore

/// What `KeychainStore.save` actually hands to `security` - without touching
/// this Mac's Keychain.
///
/// The encryption password goes to `rclone obscure` exactly as typed. If the
/// Keychain copy were trimmed, a password with a leading or trailing space
/// would be stored DIFFERENT from the one that encrypts the backup, and the
/// copy meant for recovery would not decrypt it.
final class KeychainStoreValueTests: XCTestCase {

func testExactValueKeepsLeadingAndTrailingSpaces() throws {
XCTAssertEqual(
try KeychainStore.valueToStore(" pass phrase with spaces ", exact: true),
" pass phrase with spaces ")
}

func testDefaultTrimsPastedNewlinesAndSpaces() throws {
// A pasted client_id / client_secret often carries a newline.
XCTAssertEqual(try KeychainStore.valueToStore(" client-id\n"), "client-id")
}

func testEmptyIsRefusedInBothModes() {
XCTAssertThrowsError(try KeychainStore.valueToStore(" \n"))
XCTAssertThrowsError(try KeychainStore.valueToStore("", exact: true))
}

/// What `DriveEncryption.setup` stores is what `obscure` encrypted with:
/// the value as `KeychainStore.save` would hand it to `security`.
func testEncryptionSetupStoresPasswordAndSaltExactly() throws {
let password = " edge spaces password "
let salt = "c2FsdA=="
let stored = try DriveEncryption.keychainItems(password: password, salt: salt).map {
($0.account, try KeychainStore.valueToStore($0.value, exact: $0.exact))
}
XCTAssertEqual(stored.map(\.0), [DriveEncryption.passwordAccount, DriveEncryption.saltAccount])
XCTAssertEqual(stored.map(\.1), [password, salt])
}

func testExactWhitespaceOnlyIsRefused() {
// Nothing meaningful to keep - same refusal as before.
XCTAssertThrowsError(try KeychainStore.valueToStore(" ", exact: true))
}
}
Loading