Skip to content

Commit

Permalink
Made keychain a singleton and added cache as workaround for macOS bug.
Browse files Browse the repository at this point in the history
The SecItemCopyMatching function leaks on macOS. This has been known for years, and the bug is reported, but who knows when Apple fix it.
  • Loading branch information
erikdoe committed May 21, 2024
1 parent ee6516b commit 8202ebc
Show file tree
Hide file tree
Showing 5 changed files with 83 additions and 49 deletions.
122 changes: 78 additions & 44 deletions CCMenu/Source/Miscellaneous/Keychain.swift
Original file line number Diff line number Diff line change
Expand Up @@ -14,88 +14,122 @@ enum KeychainAccessError: Error {
case missingSchemeErr
case missingHostErr
case missingUserErr
case invalidKeychainType
}

class Keychain {

static let standard = Keychain()

private let lock: NSLock
private var cache: [String: String]

private init() {
lock = NSLock()
cache = Dictionary()
}

func setPassword(_ password: String, forURL urlString: String) throws {
let url = try getOrThrow(error: .invalidURLErr) { URL(string: urlString) }
let query: [String: Any] = [
kSecClass as String: kSecClassInternetPassword,
kSecAttrServer as String: try getOrThrow(error: .missingHostErr) { url.host() },
kSecAttrPort as String: url.port ?? 80,
kSecAttrAccount as String: try getOrThrow(error: .missingUserErr) { url.user }
]
var item: [String: Any] = [
kSecAttrProtocol as String: try getOrThrow(error: .missingSchemeErr) { url.scheme },
kSecValueData as String: try getOrThrow(error: .passwordEncodingErr) { password.data(using: .utf8) }
]
item.merge(query) { i, q in i }
let query = [
kSecClass: kSecClassInternetPassword,
kSecAttrServer: try getOrThrow(error: .missingHostErr) { url.host() },
kSecAttrPort: url.port ?? 80,
kSecAttrAccount: try getOrThrow(error: .missingUserErr) { url.user }
] as NSDictionary
let item = [
kSecClass: kSecClassInternetPassword,
kSecAttrServer: try getOrThrow(error: .missingHostErr) { url.host() },
kSecAttrPort: url.port ?? 80,
kSecAttrAccount: try getOrThrow(error: .missingUserErr) { url.user },
kSecAttrProtocol: try getOrThrow(error: .missingSchemeErr) { url.scheme },
kSecValueData: try getOrThrow(error: .passwordEncodingErr) { password.data(using: .utf8) }
] as NSDictionary
try setItem(item, forQuery: query)
cache[urlString] = nil
}

func getPassword(forURL url: URL) throws -> String? {
let query: [String: Any] = [
kSecClass as String: kSecClassInternetPassword,
kSecAttrServer as String: try getOrThrow(error: .missingHostErr) { url.host() },
kSecAttrPort as String: url.port ?? 80,
kSecAttrAccount as String: try getOrThrow(error: .missingUserErr) { url.user },
kSecReturnData as String: true
]
return try getStringForQuery(query)
if let password = cache[url.absoluteString] {
return password
}
let query = [
kSecClass: kSecClassInternetPassword,
kSecAttrServer: try getOrThrow(error: .missingHostErr) { url.host() },
kSecAttrPort: url.port ?? 80,
kSecAttrAccount: try getOrThrow(error: .missingUserErr) { url.user },
kSecMatchLimit: kSecMatchLimitOne,
kSecReturnData: true
] as NSDictionary
let password = try getStringForQuery(query)
cache[url.absoluteString] = password
return password
}


func setToken(_ token: String, forService service: String) throws {
let query: [String: Any] = [
kSecClass as String: kSecClassGenericPassword,
kSecAttrService as String: serviceForKeychain(service: service)
]
var item: [String: Any] = [
kSecValueData as String: try getOrThrow(error: .passwordEncodingErr) { token.data(using: .utf8) }
]
item.merge(query) { i, q in i }
let query = [
kSecClass: kSecClassGenericPassword,
kSecAttrService: serviceForKeychain(service: service)
] as NSDictionary
let item = [
kSecClass: kSecClassGenericPassword,
kSecAttrService: serviceForKeychain(service: service),
kSecValueData: try getOrThrow(error: .passwordEncodingErr) { token.data(using: .utf8) }
] as NSDictionary
try setItem(item, forQuery: query)
cache[service] = nil
}

func getToken(forService service: String) throws -> String? {
if service == "GitHub", let token = UserDefaults.active.string(forKey: "GitHubToken") {
return token.isEmpty ? nil : token
}
let query: [String: Any] = [
kSecClass as String: kSecClassGenericPassword,
kSecAttrService as String: serviceForKeychain(service: service),
kSecReturnData as String: true
]
return try getStringForQuery(query)
}

if let token = cache[service] {
return token
}
let query = [
kSecClass: kSecClassGenericPassword,
kSecAttrService: serviceForKeychain(service: service),
kSecMatchLimit: kSecMatchLimitOne,
kSecReturnData: true
] as NSDictionary
let token = try getStringForQuery(query)
cache[service] = token
return token
}


private func setItem(_ item: [String: Any], forQuery query: [String: Any]) throws {
var status = SecItemAdd(item as CFDictionary, nil)
private func setItem(_ item: NSDictionary, forQuery query: NSDictionary) throws {
lock.lock()
defer { lock.unlock() }

var status = SecItemAdd(item, nil)
if status == errSecDuplicateItem {
status = SecItemUpdate(query as CFDictionary, item as CFDictionary)
status = SecItemUpdate(query, item)
}
if status != errSecSuccess {
throw NSError(domain: NSOSStatusErrorDomain, code: Int(status))
}
}

private func getStringForQuery(_ query: [String: Any]) throws -> String? {
var result: AnyObject?
let status = withUnsafeMutablePointer(to: &result) {
SecItemCopyMatching(query as CFDictionary, UnsafeMutablePointer($0))
}
private func getStringForQuery(_ query: NSDictionary) throws -> String? {
lock.lock()
defer { lock.unlock() }

var result: CFTypeRef?
let status = SecItemCopyMatching(query, &result)
if status == errSecItemNotFound {
return nil
}
if status != errSecSuccess {
throw NSError(domain: NSOSStatusErrorDomain, code: Int(status))
}
guard let data = result as? Data else { return nil }
guard let data = result as? Data else { throw KeychainAccessError.invalidKeychainType }
return String(data: data, encoding: .utf8)
}


private func getOrThrow<T>(error: KeychainAccessError, _ getter: () -> T?) throws -> T {
guard let v = getter() else { throw error }
return v
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ class CCTrayPipelineBuilder: ObservableObject {
components.user = credential.user
if !credential.password.isEmpty {
do {
try Keychain().setPassword(credential.password, forURL: url.absoluteString)
try Keychain.standard.setPassword(credential.password, forURL: url.absoluteString)
} catch {
// TODO: Figure out what to do here – so many errors...
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,7 @@ class GitHubAuthenticator: ObservableObject {

func fetchTokenFromKeychain() {
do {
token = try Keychain().getToken(forService: "GitHub")
token = try Keychain.standard.getToken(forService: "GitHub")
} catch {
// TODO: Figure out what to do here – so many errors...
token = nil
Expand All @@ -133,7 +133,7 @@ class GitHubAuthenticator: ObservableObject {
func storeTokenInKeychain() {
guard let token else { return }
do {
try Keychain().setToken(token, forService: "GitHub")
try Keychain.standard.setToken(token, forService: "GitHub")
} catch {
// TODO: Figure out what to do here – so many errors...
}
Expand Down
2 changes: 1 addition & 1 deletion CCMenu/Source/Server Monitor/CCTrayFeedReader.swift
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ class CCTrayFeedReader {
func requestForFeed(feed: Pipeline.Feed) throws -> URLRequest {
var credential: HTTPCredential?
if let user = feed.url.user() {
guard let password = try Keychain().getPassword(forURL: feed.url) else {
guard let password = try Keychain.standard.getPassword(forURL: feed.url) else {
throw CCTrayFeedReaderError.missingPasswordError
}
credential = HTTPCredential(user: user, password: password)
Expand Down
2 changes: 1 addition & 1 deletion CCMenu/Source/Server Monitor/GitHubFeedReader.swift
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ class GitHubFeedReader {

func updatePipelineStatus() async {
do {
let token = try Keychain().getToken(forService: "GitHub")
let token = try Keychain.standard.getToken(forService: "GitHub")
guard let request = GitHubAPI.requestForFeed(feed: pipeline.feed, token: token) else {
throw GithHubFeedReaderError.invalidURLError
}
Expand Down

0 comments on commit 8202ebc

Please sign in to comment.