do not search too far for .gpg-id + lots of tests for that
This commit is contained in:
Lysann Tranvouez
parent
5c416bfb21
commit
cd6dd43dae
2 changed files with 177 additions and 22 deletions
|
|
@ -411,8 +411,7 @@ public class PasswordStore {
|
|||
return [keyID]
|
||||
}
|
||||
if Defaults.isEnableGPGIDOn {
|
||||
let encryptedDataPath = password.fileURL(in: storeURL)
|
||||
return findGPGIDs(from: encryptedDataPath)
|
||||
return findGPGIDs(underPath: password.path)
|
||||
}
|
||||
return []
|
||||
}()
|
||||
|
|
@ -430,6 +429,37 @@ public class PasswordStore {
|
|||
AppKeychain.shared.removeContent(for: SSHKey.PRIVATE.getKeychainKey())
|
||||
gitSSHPrivateKeyPassphrase = nil
|
||||
}
|
||||
|
||||
func findGPGIDs(underPath relativePath: String) -> [String] {
|
||||
guard let gpgIDFileURL = findGPGIDFile(atPath: relativePath) else {
|
||||
return []
|
||||
}
|
||||
let allKeysSeparatedByNewline = (try? String(contentsOf: gpgIDFileURL)) ?? ""
|
||||
return allKeysSeparatedByNewline
|
||||
.split(separator: "\n")
|
||||
.map { String($0).trimmed }
|
||||
.filter { !$0.isEmpty }
|
||||
}
|
||||
|
||||
func findGPGIDFile(atPath relativePath: String) -> URL? {
|
||||
// Walk up the directory hierarchy, but never escape the store root
|
||||
let storeRoot = storeURL.absoluteURL.resolvingSymlinksInPath()
|
||||
var current = storeRoot.appendingPathComponent(relativePath).resolvingSymlinksInPath()
|
||||
|
||||
while current.path.hasPrefix(storeRoot.path) {
|
||||
let candidate = current.appendingPathComponent(".gpg-id")
|
||||
var isDir: ObjCBool = false
|
||||
if FileManager.default.fileExists(atPath: candidate.path, isDirectory: &isDir), !isDir.boolValue {
|
||||
return candidate.standardizedFileURL
|
||||
}
|
||||
let parent = current.deletingLastPathComponent().resolvingSymlinksInPath()
|
||||
if parent.path == current.path {
|
||||
break
|
||||
}
|
||||
current = parent
|
||||
}
|
||||
return nil
|
||||
}
|
||||
}
|
||||
|
||||
extension PasswordStore {
|
||||
|
|
@ -462,18 +492,3 @@ extension PasswordStore {
|
|||
return try gitRepository.commit(signature: gitSignatureForNow, message: message)
|
||||
}
|
||||
}
|
||||
|
||||
func findGPGIDs(from url: URL) -> [String] {
|
||||
var path = url
|
||||
while !FileManager.default.fileExists(atPath: path.appendingPathComponent(".gpg-id").path),
|
||||
path.path != "file:///" {
|
||||
path = path.deletingLastPathComponent()
|
||||
}
|
||||
path = path.appendingPathComponent(".gpg-id")
|
||||
|
||||
let allKeysSeparatedByNewline = (try? String(contentsOf: path)) ?? ""
|
||||
return allKeysSeparatedByNewline
|
||||
.split(separator: "\n")
|
||||
.map { String($0).trimmed }
|
||||
.filter { !$0.isEmpty }
|
||||
}
|
||||
|
|
|
|||
|
|
@ -332,12 +332,85 @@ final class PasswordStoreTest: XCTestCase {
|
|||
waitForExpectations(timeout: 1, handler: nil)
|
||||
}
|
||||
|
||||
// MARK: - .gpg-id support
|
||||
// MARK: - Find .gpg-id
|
||||
|
||||
func testFindGPGIDFile() throws {
|
||||
try FileManager.default.createDirectory(at: localRepoURL, withIntermediateDirectories: true)
|
||||
XCTAssertTrue(FileManager.default.createFile(atPath: localRepoURL.appendingPathComponent(".gpg-id").path, contents: Data("under root".utf8)))
|
||||
|
||||
try FileManager.default.createDirectory(at: localRepoURL.appendingPathComponent("foo/bar/baz"), withIntermediateDirectories: true)
|
||||
XCTAssertTrue(FileManager.default.createFile(atPath: localRepoURL.appendingPathComponent("foo/.gpg-id").path, contents: Data("under foo".utf8)))
|
||||
|
||||
try FileManager.default.createDirectory(at: localRepoURL.appendingPathComponent("weird-subdir/.gpg-id/"), withIntermediateDirectories: true)
|
||||
try FileManager.default.createDirectory(at: localRepoURL.appendingPathComponent("weird-subdir/.gpg-id/hey"), withIntermediateDirectories: true)
|
||||
XCTAssertTrue(FileManager.default.createFile(atPath: localRepoURL.appendingPathComponent("weird-subdir/.gpg-id/hey/.gpg-id").path, contents: Data("under hey".utf8)))
|
||||
|
||||
XCTAssertEqual(passwordStore.findGPGIDFile(atPath: "")?.absoluteURL, URL(fileURLWithPath: localRepoURL.appendingPathComponent(".gpg-id").path))
|
||||
XCTAssertEqual(passwordStore.findGPGIDFile(atPath: "/")?.absoluteURL, URL(fileURLWithPath: localRepoURL.appendingPathComponent(".gpg-id").path))
|
||||
XCTAssertEqual(passwordStore.findGPGIDFile(atPath: "doesnt-exist")?.absoluteURL, URL(fileURLWithPath: localRepoURL.appendingPathComponent(".gpg-id").path))
|
||||
XCTAssertEqual(passwordStore.findGPGIDFile(atPath: "foo/..")?.absoluteURL, URL(fileURLWithPath: localRepoURL.appendingPathComponent(".gpg-id").path))
|
||||
|
||||
XCTAssertEqual(passwordStore.findGPGIDFile(atPath: "foo")?.absoluteURL, URL(fileURLWithPath: localRepoURL.appendingPathComponent("foo/.gpg-id").path))
|
||||
XCTAssertEqual(passwordStore.findGPGIDFile(atPath: "foo/bar")?.absoluteURL, URL(fileURLWithPath: localRepoURL.appendingPathComponent("foo/.gpg-id").path))
|
||||
XCTAssertEqual(passwordStore.findGPGIDFile(atPath: "foo/bar/baz")?.absoluteURL, URL(fileURLWithPath: localRepoURL.appendingPathComponent("foo/.gpg-id").path))
|
||||
XCTAssertEqual(passwordStore.findGPGIDFile(atPath: "foo/doesnt-exist")?.absoluteURL, URL(fileURLWithPath: localRepoURL.appendingPathComponent("foo/.gpg-id").path))
|
||||
|
||||
// there is a _drectory_ called .gpg-id in here
|
||||
XCTAssertEqual(passwordStore.findGPGIDFile(atPath: "weird-subdir")?.absoluteURL, URL(fileURLWithPath: localRepoURL.appendingPathComponent(".gpg-id").path))
|
||||
XCTAssertEqual(passwordStore.findGPGIDFile(atPath: "weird-subdir/.gpg-id")?.absoluteURL, URL(fileURLWithPath: localRepoURL.appendingPathComponent(".gpg-id").path))
|
||||
XCTAssertEqual(passwordStore.findGPGIDFile(atPath: "weird-subdir/.gpg-id/hey")?.absoluteURL, URL(fileURLWithPath: localRepoURL.appendingPathComponent("weird-subdir/.gpg-id/hey/.gpg-id").path))
|
||||
|
||||
// "foo/bar/../../baz" resolves to "baz" which has no .gpg-id, so should find root's.
|
||||
// Without path resolution, the walk ["foo","bar","..","..","baz"] → remove "baz" → remove ".." →
|
||||
// "foo/bar/.." → remove ".." → "foo/bar" → finds foo/.gpg-id (wrong).
|
||||
try FileManager.default.createDirectory(at: localRepoURL.appendingPathComponent("baz"), withIntermediateDirectories: true)
|
||||
XCTAssertEqual(passwordStore.findGPGIDFile(atPath: "foo/bar/../../baz")?.absoluteURL, URL(fileURLWithPath: localRepoURL.appendingPathComponent(".gpg-id").path))
|
||||
}
|
||||
|
||||
func testMissingGPGIDFile() throws {
|
||||
XCTAssertFalse(FileManager.default.fileExists(atPath: localRepoURL.appendingPathComponent(".gpg-id").path))
|
||||
try FileManager.default.createDirectory(at: localRepoURL.appendingPathComponent("subdir"), withIntermediateDirectories: true)
|
||||
|
||||
XCTAssertNil(passwordStore.findGPGIDFile(atPath: ""))
|
||||
XCTAssertNil(passwordStore.findGPGIDFile(atPath: "subdir"))
|
||||
XCTAssertNil(passwordStore.findGPGIDFile(atPath: "missing"))
|
||||
}
|
||||
|
||||
func testFindGPGIDFileStopsAtRoot() throws {
|
||||
// Place a .gpg-id file ABOVE the store root, this should not be found
|
||||
let parentDir = localRepoURL.deletingLastPathComponent()
|
||||
let escapedGPGIDURL = parentDir.appendingPathComponent(".gpg-id")
|
||||
XCTAssertTrue(FileManager.default.createFile(atPath: escapedGPGIDURL.path, contents: Data("ESCAPED_KEY".utf8)))
|
||||
defer { try? FileManager.default.removeItem(at: escapedGPGIDURL) }
|
||||
|
||||
// Store has no .gpg-id at all
|
||||
try FileManager.default.createDirectory(at: localRepoURL.appendingPathComponent("sub/deep"), withIntermediateDirectories: true)
|
||||
// Direct paths, should not find the escaped .gpg-id since it's outside the store root
|
||||
XCTAssertNil(passwordStore.findGPGIDFile(atPath: ""))
|
||||
XCTAssertNil(passwordStore.findGPGIDFile(atPath: "sub"))
|
||||
XCTAssertNil(passwordStore.findGPGIDFile(atPath: "sub/deep"))
|
||||
|
||||
// Path traversal attempts via ".."
|
||||
XCTAssertNil(passwordStore.findGPGIDFile(atPath: ".."))
|
||||
XCTAssertNil(passwordStore.findGPGIDFile(atPath: "../.."))
|
||||
XCTAssertNil(passwordStore.findGPGIDFile(atPath: "sub/../.."))
|
||||
XCTAssertNil(passwordStore.findGPGIDFile(atPath: "sub/deep/../../.."))
|
||||
XCTAssertNil(passwordStore.findGPGIDFile(atPath: "sub/deep/../../../../../etc"))
|
||||
|
||||
// Symlink escape: create a symlink inside the store pointing outside
|
||||
let evilDir = parentDir.appendingPathComponent("evil")
|
||||
try FileManager.default.createDirectory(at: evilDir, withIntermediateDirectories: true)
|
||||
XCTAssertTrue(FileManager.default.createFile(atPath: evilDir.appendingPathComponent(".gpg-id").path, contents: Data("EVIL_KEY".utf8)))
|
||||
defer { try? FileManager.default.removeItem(at: evilDir) }
|
||||
try FileManager.default.createSymbolicLink(at: localRepoURL.appendingPathComponent("sub/escape"), withDestinationURL: evilDir)
|
||||
// Following the symlink would find evil/.gpg-id — must not happen
|
||||
XCTAssertNil(passwordStore.findGPGIDFile(atPath: "sub/escape"))
|
||||
}
|
||||
|
||||
// MARK: Parse .gpg-id
|
||||
|
||||
func testReadGPGIDFile() throws {
|
||||
try cloneRepository(.withGPGID)
|
||||
Defaults.isEnableGPGIDOn = true
|
||||
|
||||
[
|
||||
("", [RSA4096.longFingerprint]),
|
||||
("family", [String(NISTP384.longFingerprint.suffix(16))]),
|
||||
|
|
@ -345,11 +418,36 @@ final class PasswordStoreTest: XCTestCase {
|
|||
("shared", [RSA2048.longFingerprint, RSA4096.longFingerprint]),
|
||||
("work", [RSA2048.longFingerprint]),
|
||||
].forEach { path, expectedKeyIDs in
|
||||
let foundKeyIDs = findGPGIDs(from: localRepoURL.appendingPathComponent(path))
|
||||
XCTAssertEqual(foundKeyIDs, expectedKeyIDs.map { $0.uppercased() })
|
||||
let foundKeyIDs = passwordStore.findGPGIDs(underPath: path)
|
||||
XCTAssertEqual(foundKeyIDs, expectedKeyIDs.map { $0.uppercased() }, "Failed for path: \(path)")
|
||||
}
|
||||
}
|
||||
|
||||
func testReadEmptyGPGIDFile() throws {
|
||||
try FileManager.default.createDirectory(at: localRepoURL, withIntermediateDirectories: true)
|
||||
|
||||
XCTAssertTrue(FileManager.default.createFile(atPath: localRepoURL.appendingPathComponent(".gpg-id").path, contents: nil))
|
||||
XCTAssertEqual(passwordStore.findGPGIDs(underPath: ""), [])
|
||||
|
||||
XCTAssertTrue(FileManager.default.createFile(atPath: localRepoURL.appendingPathComponent(".gpg-id").path, contents: Data(" \n\t".utf8)))
|
||||
XCTAssertEqual(passwordStore.findGPGIDs(underPath: ""), [])
|
||||
}
|
||||
|
||||
func testReadGPGIDFileWithWhitespace() throws {
|
||||
try FileManager.default.createDirectory(at: localRepoURL, withIntermediateDirectories: true)
|
||||
|
||||
XCTAssertTrue(FileManager.default.createFile(atPath: localRepoURL.appendingPathComponent(".gpg-id").path, contents: nil))
|
||||
XCTAssertEqual(passwordStore.findGPGIDs(underPath: ""), [])
|
||||
|
||||
XCTAssertTrue(FileManager.default.createFile(atPath: localRepoURL.appendingPathComponent(".gpg-id").path, contents: Data(" \n\t".utf8)))
|
||||
XCTAssertEqual(passwordStore.findGPGIDs(underPath: ""), [])
|
||||
|
||||
XCTAssertTrue(FileManager.default.createFile(atPath: localRepoURL.appendingPathComponent(".gpg-id").path, contents: Data(" \nbar foo\n\tbaz\n \n".utf8)))
|
||||
XCTAssertEqual(passwordStore.findGPGIDs(underPath: ""), ["bar foo", "baz"])
|
||||
}
|
||||
|
||||
// MARK: Handle .gpg-id
|
||||
|
||||
func testAddPasswordInRoot_WithSingleEntryInPGPIDFile_EncryptsWithThatKey() throws {
|
||||
let mockPGPInterface = setUpMockedPGPInterface()
|
||||
mockPGPInterface.publicKeyIDs = Set(RSA2048_RSA4096.fingerprints)
|
||||
|
|
@ -436,6 +534,48 @@ final class PasswordStoreTest: XCTestCase {
|
|||
}
|
||||
}
|
||||
|
||||
func testGPGIDDisabledIgnoresGPGIDFile() throws {
|
||||
try cloneRepository(.withGPGID)
|
||||
try importSinglePGPKey() // Only import RSA4096, but not RSA2048
|
||||
Defaults.isEnableGPGIDOn = false
|
||||
|
||||
// /work uses RSA2048, but we didn't import that one
|
||||
let testPassword = Password(name: "test", path: "work/test.gpg", plainText: "testpassword")
|
||||
// this would throw if isEnableGPGIDOn was true, since we are missing the key to encrypt it
|
||||
_ = try passwordStore.add(password: testPassword)
|
||||
|
||||
// check that we can decrypt it with the key we have, which confirms that it was encrypted without using the .gpg-id file
|
||||
let decryptedPassword = try decrypt(path: "work/test.gpg", keyID: RSA4096.longFingerprint)
|
||||
XCTAssertEqual(decryptedPassword.plainText, "testpassword")
|
||||
|
||||
// we can't even decrypt it with RSA2048
|
||||
try importMultiplePGPKeys()
|
||||
XCTAssertThrowsError(try decrypt(path: "work/test.gpg", keyID: RSA2048.longFingerprint)) {
|
||||
XCTAssertEqual($0 as? AppError, .keyExpiredOrIncompatible)
|
||||
}
|
||||
}
|
||||
|
||||
func testEncryptWithExplicitKeyID_OverridesGPGIDFile() throws {
|
||||
continueAfterFailure = false // avoid index out of bounds error below
|
||||
|
||||
let mockPGPInterface = setUpMockedPGPInterface()
|
||||
mockPGPInterface.publicKeyIDs = Set(RSA2048_RSA4096.fingerprints)
|
||||
try cloneRepository(.withGPGID)
|
||||
Defaults.isEnableGPGIDOn = true
|
||||
|
||||
// Even though /personal would normally use RSA4096 from the root .gpg-id file, if we explicitly specify a key ID then that should be used instead
|
||||
let testPassword1 = Password(name: "test1", path: "personal/test1.gpg", plainText: "testpassword1")
|
||||
_ = try passwordStore.add(password: testPassword1)
|
||||
let testPassword2 = Password(name: "test2", path: "personal/test2.gpg", plainText: "testpassword2")
|
||||
_ = try passwordStore.add(password: testPassword2, keyID: RSA2048.longFingerprint)
|
||||
|
||||
XCTAssertEqual(mockPGPInterface.encryptMultiKeyCalls.count, 2)
|
||||
XCTAssertEqual(mockPGPInterface.encryptMultiKeyCalls[0].plainData, testPassword1.plainData)
|
||||
XCTAssertEqual(mockPGPInterface.encryptMultiKeyCalls[0].keyIDs, [RSA4096.longFingerprint].map { $0.uppercased() })
|
||||
XCTAssertEqual(mockPGPInterface.encryptMultiKeyCalls[1].plainData, testPassword2.plainData)
|
||||
XCTAssertEqual(mockPGPInterface.encryptMultiKeyCalls[1].keyIDs, [RSA2048.longFingerprint])
|
||||
}
|
||||
|
||||
// MARK: - Helpers
|
||||
|
||||
private enum RemoteRepo {
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue