Skip to content

Commit bbdcd09

Browse files
authored
Allow same mTLS cert for multiple servers (#4603)
<!-- Thank you for submitting a Pull Request and helping to improve Home Assistant. Please complete the following sections to help the processing and review of your changes. Please do not delete anything from this template. --> ## Summary <!-- Provide a brief summary of the changes you have made and most importantly what they aim to achieve --> Fixes mTLS client certificate sharing across multiple servers when the same .p12 / SecIdentity is assigned to more than one server. - Store imported client certificate identities using a stable SHA-256 fingerprint of the leaf certificate instead of a server-specific identifier. - Update duplicate Keychain identity handling so re-importing the same certificate migrates the existing item to the stable fingerprint-based label. - Avoid deleting a shared Keychain identity when removing or replacing a certificate on one server if another configured server still references it. ## Screenshots <!-- If this is a user-facing change not in the frontend, please include screenshots in light and dark mode. --> ## Link to pull request in Documentation repository <!-- Pull requests that add, change or remove functionality must have a corresponding pull request in the Companion App Documentation repository (https://github.com/home-assistant/companion.home-assistant). Please add the number of this pull request after the "#" --> Documentation: home-assistant/companion.home-assistant# ## Any other notes <!-- If there is any other information of note, like if this Pull Request is part of a bigger change, please include it here. -->
1 parent a99cae4 commit bbdcd09

2 files changed

Lines changed: 54 additions & 13 deletions

File tree

Sources/App/Settings/Connection/ConnectionSettingsViewModel.swift

Lines changed: 27 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -290,11 +290,18 @@ final class ConnectionSettingsViewModel: ObservableObject {
290290

291291
switch result {
292292
case let .fulfilled(certificate):
293+
let replacedCertificate = server.info.connection.clientCertificate
294+
293295
// Update server connection info
294296
server.update { info in
295297
info.connection.clientCertificate = certificate
296298
}
297299

300+
if let replacedCertificate,
301+
replacedCertificate.keychainIdentifier != certificate.keychainIdentifier {
302+
deleteCertificateIfUnreferenced(replacedCertificate)
303+
}
304+
298305
clientCertificate = certificate
299306
Current.Log.info("Successfully imported client certificate: \(certificate.displayName)")
300307
case let .rejected(error):
@@ -309,17 +316,32 @@ final class ConnectionSettingsViewModel: ObservableObject {
309316
func removeCertificate() {
310317
guard let certificate = clientCertificate else { return }
311318

319+
server.update { info in
320+
info.connection.clientCertificate = nil
321+
}
322+
323+
deleteCertificateIfUnreferenced(certificate)
324+
325+
clientCertificate = nil
326+
Current.Log.info("Removed client certificate")
327+
}
328+
329+
private func deleteCertificateIfUnreferenced(_ certificate: ClientCertificate) {
330+
guard !isCertificateReferenced(certificate) else {
331+
Current.Log.info("Keeping shared client certificate in Keychain")
332+
return
333+
}
334+
312335
do {
313336
try ClientCertificateManager.shared.delete(certificate: certificate)
314337
} catch {
315338
Current.Log.error("Failed to delete certificate from Keychain: \(error)")
316339
}
340+
}
317341

318-
server.update { info in
319-
info.connection.clientCertificate = nil
342+
private func isCertificateReferenced(_ certificate: ClientCertificate) -> Bool {
343+
Current.servers.all.contains { server in
344+
server.info.connection.clientCertificate?.keychainIdentifier == certificate.keychainIdentifier
320345
}
321-
322-
clientCertificate = nil
323-
Current.Log.info("Removed client certificate")
324346
}
325347
}

Sources/Shared/API/mTLS/ClientCertificate.swift

Lines changed: 27 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
import CryptoKit
12
import Foundation
23
import Security
34

@@ -74,7 +75,7 @@ public final class ClientCertificateManager {
7475
/// - Parameters:
7576
/// - p12Data: The raw .p12 file data
7677
/// - password: The password to decrypt the .p12 file
77-
/// - identifier: A unique identifier for storing in Keychain
78+
/// - identifier: A fallback identifier for storing in Keychain if the certificate cannot be fingerprinted
7879
/// - Returns: A ClientCertificate reference
7980
public func importP12(data p12Data: Data, password: String, identifier: String) throws -> ClientCertificate {
8081
let options = Self.pkcs12ImportOptions(password: password)
@@ -123,8 +124,10 @@ public final class ClientCertificateManager {
123124
let certChain = firstItem[kSecImportItemCertChain as String] as? [SecCertificate] ?? []
124125
let intermediateCerts = Array(certChain.dropFirst())
125126

126-
// Store identity in Keychain
127-
let keychainIdentifier = "com.ha-ios.mtls.\(identifier)"
127+
// Store identities by certificate fingerprint rather than by server. The iOS Keychain
128+
// treats a certificate/private-key pair as one identity, so labeling the same identity
129+
// with different server IDs makes later imports move the item away from older servers.
130+
let keychainIdentifier = Self.keychainIdentifier(for: certificate, fallbackIdentifier: identifier)
128131
try storeIdentity(secIdentity, identifier: keychainIdentifier)
129132
storeIntermediateCertificates(intermediateCerts, for: keychainIdentifier)
130133

@@ -136,6 +139,21 @@ public final class ClientCertificateManager {
136139
)
137140
}
138141

142+
private static func keychainIdentifier(
143+
for certificate: SecCertificate?,
144+
fallbackIdentifier: String
145+
) -> String {
146+
guard let certificate,
147+
let certData = SecCertificateCopyData(certificate) as Data? else {
148+
return "com.ha-ios.mtls.\(fallbackIdentifier)"
149+
}
150+
151+
let fingerprint = SHA256.hash(data: certData)
152+
.map { String(format: "%02x", $0) }
153+
.joined()
154+
return "com.ha-ios.mtls.identity.\(fingerprint)"
155+
}
156+
139157
/// Store a SecIdentity in the Keychain
140158
private func storeIdentity(_ identity: SecIdentity, identifier: String) throws {
141159
// First, delete any existing identity with this identifier
@@ -155,9 +173,10 @@ public final class ClientCertificateManager {
155173

156174
let status = SecItemAdd(addQuery as CFDictionary, nil)
157175

158-
// Handle duplicate - the identity might already exist with different label
176+
// Handle duplicate - the same identity might already exist under a legacy server label.
159177
if status == errSecDuplicateItem {
160-
// Try to update instead
178+
// Move the existing identity to the certificate-derived label so every server that
179+
// imports the same P12 can resolve the same Keychain item.
161180
let updateQuery: [String: Any] = [
162181
kSecValueRef as String: identity,
163182
]
@@ -166,9 +185,9 @@ public final class ClientCertificateManager {
166185
kSecAttrAccessible as String: kSecAttrAccessibleAfterFirstUnlock,
167186
]
168187
let updateStatus = SecItemUpdate(updateQuery as CFDictionary, updateAttrs as CFDictionary)
169-
// If update also fails, the item exists which is fine for our purposes
170-
if updateStatus != errSecSuccess, updateStatus != errSecItemNotFound {
171-
Current.Log.warning("Keychain update returned \(updateStatus), but certificate may still work")
188+
if updateStatus != errSecSuccess {
189+
Current.Log.error("Failed to update duplicate client certificate identity label: \(updateStatus)")
190+
throw ClientCertificateError.keychainError(updateStatus)
172191
}
173192
return
174193
}

0 commit comments

Comments
 (0)