Skip to content

Commit

Permalink
Keychain search to return all matches (#6748)
Browse files Browse the repository at this point in the history
Motivation:
A registry URL (i.e., same scheme, host, and port) can have multiple entries in Keychain. Currently we only ask Keychain to return one match when doing a search, but that might not be the one we want.

Modifications:
- Have Keychain search return all matches
- Sort the matches by modification timestamp
- Return the most recently modified match

rdar://112556752
  • Loading branch information
yim-lee committed Jul 27, 2023
1 parent a2d4b44 commit 2054492
Showing 1 changed file with 88 additions and 15 deletions.
103 changes: 88 additions & 15 deletions Sources/Basics/AuthorizationProvider.swift
Expand Up @@ -11,6 +11,7 @@
//===----------------------------------------------------------------------===//

import struct Foundation.Data
import struct Foundation.Date
import struct Foundation.URL
#if canImport(Security)
import Security
Expand Down Expand Up @@ -194,7 +195,7 @@ public class KeychainAuthorizationProvider: AuthorizationProvider, Authorization
}

self.observabilityScope
.emit(debug: "Add/update credentials for '\(protocolHostPort)' [\(url.absoluteString)] in keychain")
.emit(debug: "add/update credentials for '\(protocolHostPort)' [\(url.absoluteString)] in keychain")

if !persist {
self.cache[protocolHostPort.description] = (user, password)
Expand All @@ -221,7 +222,7 @@ public class KeychainAuthorizationProvider: AuthorizationProvider, Authorization
}

self.observabilityScope
.emit(debug: "Remove credentials for '\(protocolHostPort)' [\(url.absoluteString)] from keychain")
.emit(debug: "remove credentials for '\(protocolHostPort)' [\(url.absoluteString)] from keychain")

do {
try self.delete(protocolHostPort: protocolHostPort)
Expand All @@ -241,28 +242,66 @@ public class KeychainAuthorizationProvider: AuthorizationProvider, Authorization
}

self.observabilityScope
.emit(debug: "Search credentials for '\(protocolHostPort)' [\(url.absoluteString)] in keychain")
.emit(debug: "search credentials for '\(protocolHostPort)' [\(url.absoluteString)] in keychain")

do {
guard let existingItem = try self
.search(protocolHostPort: protocolHostPort) as? [String: Any],
let passwordData = existingItem[kSecValueData as String] as? Data,
let password = String(data: passwordData, encoding: String.Encoding.utf8),
let account = existingItem[kSecAttrAccount as String] as? String
guard let existingItems = try self.search(protocolHostPort: protocolHostPort) as? [[String: Any]] else {
throw AuthorizationProviderError
.other("Failed to extract credentials for '\(protocolHostPort)' from keychain")
}

// Log warning if there is more than one result
if existingItems.count > 1 {
self.observabilityScope
.emit(
warning: "multiple (\(existingItems.count)) keychain entries found for '\(protocolHostPort)' [\(url.absoluteString)]"
)
}

// Sort by modification timestamp
let sortedItems = existingItems.sorted {
switch (
$0[kSecAttrModificationDate as String] as? Date,
$1[kSecAttrModificationDate as String] as? Date
) {
case (nil, nil):
return false
case (_, nil):
return true
case (nil, _):
return false
case (.some(let left), .some(let right)):
return left < right
}
}

// Return most recently modified item
guard let mostRecent = sortedItems.last,
let created = mostRecent[kSecAttrCreationDate as String] as? Date,
// Get password for this specific item
let existingItem = try self.get(
protocolHostPort: protocolHostPort,
created: created,
modified: mostRecent[kSecAttrModificationDate as String] as? Date
) as? [String: Any],
let passwordData = existingItem[kSecValueData as String] as? Data,
let password = String(data: passwordData, encoding: String.Encoding.utf8),
let account = existingItem[kSecAttrAccount as String] as? String
else {
throw AuthorizationProviderError
.other("Failed to extract credentials for '\(protocolHostPort)' from keychain")
}

return (user: account, password: password)
} catch {
switch error {
case AuthorizationProviderError.notFound:
self.observabilityScope.emit(debug: "No credentials found for '\(protocolHostPort)' in keychain")
self.observabilityScope.emit(debug: "no credentials found for '\(protocolHostPort)' in keychain")
case AuthorizationProviderError.other(let detail):
self.observabilityScope.emit(error: detail)
default:
self.observabilityScope.emit(
error: "Failed to find credentials for '\(protocolHostPort)' in keychain",
error: "failed to find credentials for '\(protocolHostPort)' in keychain",
underlyingError: error
)
}
Expand Down Expand Up @@ -331,13 +370,47 @@ public class KeychainAuthorizationProvider: AuthorizationProvider, Authorization
var query: [String: Any] = [kSecClass as String: kSecClassInternetPassword,
kSecAttrProtocol as String: protocolHostPort.protocolCFString,
kSecAttrServer as String: protocolHostPort.server,
kSecMatchLimit as String: kSecMatchLimitOne,
kSecMatchLimit as String: kSecMatchLimitAll, // returns all matches
kSecReturnAttributes as String: true]
// https://developer.apple.com/documentation/security/keychain_services/keychain_items/searching_for_keychain_items
// Can't combine `kSecMatchLimitAll` and `kSecReturnData` (which contains password)

if let port = protocolHostPort.port {
query[kSecAttrPort as String] = port
}

var items: CFTypeRef?
// Search keychain for server's username and password, if any.
let status = SecItemCopyMatching(query as CFDictionary, &items)
guard status != errSecItemNotFound else {
throw AuthorizationProviderError.notFound
}
guard status == errSecSuccess else {
throw AuthorizationProviderError
.other("Failed to find credentials for '\(protocolHostPort)' in keychain: status \(status)")
}

return items
}

private func get(protocolHostPort: ProtocolHostPort, created: Date, modified: Date?) throws -> CFTypeRef? {
self.observabilityScope
.emit(debug: "read credentials for '\(protocolHostPort)', created at \(created), in keychain")

var query: [String: Any] = [kSecClass as String: kSecClassInternetPassword,
kSecAttrProtocol as String: protocolHostPort.protocolCFString,
kSecAttrServer as String: protocolHostPort.server,
kSecAttrCreationDate as String: created,
kSecMatchLimit as String: kSecMatchLimitOne, // limit to one match
kSecReturnAttributes as String: true,
kSecReturnData as String: true]
kSecReturnData as String: true] // password

if let port = protocolHostPort.port {
query[kSecAttrPort as String] = port
}
if let modified {
query[kSecAttrModificationDate as String] = modified
}

var item: CFTypeRef?
// Search keychain for server's username and password, if any.
Expand Down Expand Up @@ -414,13 +487,13 @@ public struct CompositeAuthorizationProvider: AuthorizationProvider {
if let authentication = provider.authentication(for: url) {
switch provider {
case let provider as NetrcAuthorizationProvider:
self.observabilityScope.emit(info: "Credentials for \(url) found in netrc file at \(provider.path)")
self.observabilityScope.emit(info: "credentials for \(url) found in netrc file at \(provider.path)")
#if canImport(Security)
case is KeychainAuthorizationProvider:
self.observabilityScope.emit(info: "Credentials for \(url) found in keychain")
self.observabilityScope.emit(info: "credentials for \(url) found in keychain")
#endif
default:
self.observabilityScope.emit(info: "Credentials for \(url) found in \(provider)")
self.observabilityScope.emit(info: "credentials for \(url) found in \(provider)")
}
return authentication
}
Expand Down

0 comments on commit 2054492

Please sign in to comment.