Merge pull request #266 from openziti/use.all.certs.from.enrollment.resp

Use all certificates from enrollment response

Author: scareything

lib/Ziti.swift

@@ -332,7 +332,8 @@ import CZitiPrivate
             // Store certificate
             let cert = dropFirst("pem:", resp.id.cert)
             _ = zkc.deleteCertificate(silent: true)
-            guard zkc.storeCertificate(fromPem: cert) == nil else {
+            let (err, cns) = zkc.storeCertificates(cert)
+            guard err == nil else {
                 let errStr = "Unable to store certificate\n"
                 log.error(errStr, function:"enroll()")
                 enrollCallback(nil, ZitiError(errStr))
@@ -345,8 +346,8 @@ import CZitiPrivate
                 ca = dropFirst("pem:", idCa)
             }
 
-            let zid = ZitiIdentity(id: subj, ztAPIs: resp.ztAPIs, ca: ca)
-            log.info("Enrolled id:\(subj) with controller: \(zid.ztAPI)", function:"enroll()")
+            let zid = ZitiIdentity(id: subj, ztAPIs: resp.ztAPIs, certCNs: cns, ca: ca)
+            log.info("Enrolled id:\(subj) with controller: \(zid.ztAPI) with cns: \(zid.getCertCNs())", function:"enroll()")
 
             enrollCallback(zid, nil)
         }
@@ -383,14 +384,14 @@ import CZitiPrivate
     @objc public func run(_ postureChecks:ZitiPostureChecks?, _ initCallback: @escaping InitCallback) {
         // Get certificate
         let zkc = ZitiKeychain(tag: id.id)
-        let (maybeCert, zErr) = zkc.getCertificate()
-        guard let cert = maybeCert, zErr == nil else {
-            let errStr = zErr != nil ? zErr!.localizedDescription : "unable to retrieve certificate from keychain"
+        let (maybeCerts, zErr) = zkc.getCertificates(id.getCertCNs())
+        guard let certs = maybeCerts, zErr == nil else {
+            let errStr = zErr != nil ? zErr!.localizedDescription : "unable to retrieve certificates from keychain"
             log.error(errStr)
             initCallback(zErr ?? ZitiError(errStr))
             return
         }
-        let certPEM = zkc.convertToPEM("CERTIFICATE", der: cert)
+        let certPEM = zkc.convertToPEM("CERTIFICATE", ders: certs)
 
         // Get private key
         guard let privKey = zkc.getPrivateKey() else {

lib/ZitiIdentity.swift

@@ -34,6 +34,12 @@ import Foundation
     /// Initially the `sub` field from the  one-time enrollment JWT.  Used by `Ziti` to store and retrieve identity-related items in the Keychain
     @objc public let id:String
 
+    /// Certificate CNs
+    ///
+    /// Common names for the certificates in this identities certificate chain. The first element will always match `id`.
+    /// Used by `Ziti` to retrieve certificates from the Keychain.
+    @objc public var certCNs:[String]?
+
     /// scheme, host, and port used to communicate with Ziti controller
     @objc public var ztAPI:String
 
@@ -55,17 +61,29 @@ import Foundation
     ///
     /// - Parameters:
     ///     - id: unique identifier of this identity
-    ///     - ztAPI: URL for accessing Ziti controller API
+    ///     - ztAPIs: URLs for accessing Ziti controller API
+    ///     - certCNs: common names of certififcates 
     ///     - name: name currently configured for this identity
     ///     - ca: CA pool that can be used to verify trust of the Ziti controller
-    @objc public init(id:String, ztAPIs:[String], name:String?=nil, ca:String?=nil) {
+    @objc public init(id:String, ztAPIs:[String], certCNs:[String]?=nil, name:String?=nil, ca:String?=nil) {
         self.id = id
+        self.certCNs = [ id ]
+        if let certCNs {
+            for cn in certCNs {
+                if cn == id { continue }
+                self.certCNs?.append(cn)
+            }
+        }
         self.ztAPI = ztAPIs.first ?? ""
         self.ztAPIs = ztAPIs
         self.name = name
         self.ca = ca
     }
 
+    @objc public func getCertCNs() -> [String] {
+        return certCNs ?? [self.id]
+    }
+    
     /// Save this object to a JSON file
     ///
     /// This file can be used to initialize a `Ziti` object.

lib/ZitiKeychain.swift

@@ -178,23 +178,60 @@ public class ZitiKeychain : NSObject {
     ///
     /// - Returns: `true` if the certificates are successfully added to the keychain, otherwise `false`
     public func addCaPool(_ caPool:String) -> Bool {
-        let certs = extractCerts(caPool)
+        let (err, _) = storeCertificates(caPool, setAttrLabel: false)
+        return err == nil
+    }
+    
+    /// Add the certificates in the provided pem string to the keychain
+    ///
+    /// - Parameters
+    ///     - pem: PEM formatted certificate (or certificate chain)
+    ///
+    /// - Returns
+    ///     - An error, if one occurred.
+    ///     - An array of strings that contains the common name for each certificate in the pem.
+    public func storeCertificates(_ pem:String) -> (ZitiError?, [String]?) {
+        return storeCertificates(pem, setAttrLabel: true)
+    }
+    
+    private func storeCertificates(_ pem:String, setAttrLabel:Bool = true) -> (ZitiError?, [String]?) {
+        var cns:[String] = []
+        let certs = extractCerts(pem)
         for cert in certs {
+            var maybeCN: CFString?
+            var status = SecCertificateCopyCommonName(cert, &maybeCN)
+            guard let cn = maybeCN, status == errSecSuccess else {
+                let errStr = SecCopyErrorMessageString(status, nil) as String? ?? "\(status)"
+                log.error("Unable to retrieve certificate common name for \(tag): \(errStr)")
+                return (ZitiError("Unable to get certificate common name for \(tag): \(errStr)", errorCode: Int(status)), nil)
+            }
+            let cnStr = String(describing: cn)
             var parameters: [CFString: Any] = [
                 kSecClass: kSecClassCertificate,
                 kSecValueRef: cert]
+            if (setAttrLabel) {
+                // Setting kSecAttrLabel for a certificate is not effective, at least not on macOS. The cert is always stored with its
+                // common name as the label. This is true both for the file-based keychain (which can be verified with Keychain Access)
+                // and data protection (cloud) keychain (which cannot be inspected, but verified by adding/getting).
+                //
+                // Work around this by explicitly using the cn as the label, so the label is predictable on macOS and iOS, and hopefully
+                // on future macOS versions when/if `kSecAttrLabel` is respected. CNs
+                // to the caller so the app knows which certs to look for when it needs them.
+                parameters[kSecAttrLabel] = cnStr
+            }
             if #available(iOS 13.0, OSX 10.15, *) {
                 parameters[kSecUseDataProtectionKeychain] = true
             }
-            let status = SecItemAdd(parameters as CFDictionary, nil)
+            status = SecItemAdd(parameters as CFDictionary, nil)
             guard status == errSecSuccess || status == errSecDuplicateItem else {
                 let errStr = SecCopyErrorMessageString(status, nil) as String? ?? "\(status)"
-                log.error("Unable to store certificate for \(tag): \(errStr)")
-                return false
+                log.error("Unable to store certificate for \(cnStr): \(errStr)")
+                return (ZitiError("Unable to store certificate for \(cnStr): \(errStr)", errorCode: Int(status)), nil)
             }
             log.info("Added cert to keychain: \(String(describing: SecCertificateCopySubjectSummary(cert)))")
+            cns.append(cnStr)
         }
-        return true
+        return (nil, cns)
     }
 
 #if os(macOS) // if #available(iOS 13.0, OSX 10.15, *)
@@ -221,6 +258,7 @@ public class ZitiKeychain : NSObject {
     }
 #endif
 
+    @available(*, deprecated, message: "This function only stores the first certificate in `pem`. Use storeCertificates(pem:String) to store all certificates.")
     func storeCertificate(fromPem pem:String) -> ZitiError? {
         let (_, zErr) = storeCertificate(fromDer: convertToDER(pem))
         if zErr != nil {
@@ -229,6 +267,7 @@ public class ZitiKeychain : NSObject {
         return zErr
     }
 
+    @available(*, deprecated)
     func storeCertificate(fromDer der:Data) -> (SecCertificate?, ZitiError?) {
         guard let certificate = SecCertificateCreateWithData(nil, der as CFData) else {
             let errStr = "Unable to create certificate from data for \(tag)"
@@ -251,11 +290,16 @@ public class ZitiKeychain : NSObject {
         return (certificate, nil)
     }
 
+    @available(*, deprecated, message: "This function only gets one certificate, but an identity may have more than one. Use getCertificates( certCNs:[String]) instead")
     func getCertificate() -> (Data?, ZitiError?) {
+        return getCertificate(tag)
+    }
+    
+    private func getCertificate(_ label:String) -> (Data?, ZitiError?) {
         let params: [CFString: Any] = [
             kSecClass: kSecClassCertificate,
             kSecReturnRef: kCFBooleanTrue!,
-            kSecAttrLabel: tag]
+            kSecAttrLabel: label]
 
         var cert: CFTypeRef?
         let status = SecItemCopyMatching(params as CFDictionary, &cert)
@@ -272,6 +316,19 @@ public class ZitiKeychain : NSObject {
         return (certData, nil)
     }
 
+    func getCertificates(_ certCNs:[String]) -> ([Data]?, ZitiError?) {
+        var certs:[Data] = []
+        for cn in certCNs {
+            let (cert, err) = getCertificate(cn)
+            guard let cert = cert, err == nil else {
+                return (nil, err)
+            }
+            certs.append(cert)
+        }
+        
+        return (certs, nil)
+    }
+    
     func deleteCertificate(silent:Bool=false) -> ZitiError? {
         var params: [CFString: Any] = [
             kSecClass: kSecClassCertificate,
@@ -305,6 +362,14 @@ public class ZitiKeychain : NSObject {
         return nil
     }
 
+    func convertToPEM(_ type:String, ders:[Data]) -> String {
+        var pem = ""
+        ders.forEach { der in
+            pem.append(convertToPEM("CERTIFICATE", der: der))
+        }
+        return pem
+    }
+    
     func convertToPEM(_ type:String, der:Data) -> String {
         guard let str = der.base64EncodedString(options: NSData.Base64EncodingOptions(rawValue: 0)).addingPercentEncoding(withAllowedCharacters: CharacterSet.urlQueryAllowed) else {
             return ""