Require that App ID Prefix be explicitly passed into Shared Access Group Valets (#218)

* Require that App ID Prefix be explicitly passed into Shared Access Group Valets

* Add App ID prefix to tests

* Create and adopt SharedAccessGroupIdentifier

* Update documentation

Author: dfed

README.md

@@ -128,14 +128,14 @@ In addition to allowing the storage of strings, Valet allows the storage of `Dat
 ### Sharing Secrets Among Multiple Applications
 
 ```swift
-let mySharedValet = Valet.sharedAccessGroupValet(with: Identifier(nonEmpty: "Druidia")!, accessibility: .whenUnlocked)
+let mySharedValet = Valet.sharedAccessGroupValet(with: SharedAccessGroupIdentifier(appIDPrefix: "AppID12345", nonEmptyGroup: "Druidia")!, accessibility: .whenUnlocked)
 ```
 
 ```objc
-VALValet *const mySharedValet = [VALValet valetWithSharedAccessGroupIdentifier:@"Druidia" accessibility:VALAccessibilityWhenUnlocked];
+VALValet *const mySharedValet = [VALValet sharedAccessGroupValetWithAppIDPrefix:@"AppID12345" sharedAccessGroupIdentifier:@"Druidia" accessibility:VALAccessibilityWhenUnlocked];
 ```
 
-This instance can be used to store and retrieve data securely across any app written by the same developer with the value `Druidia` under the `keychain-access-groups` key in the app’s `Entitlements` file, when the device is unlocked. Note that `myValet` and `mySharedValet` can not read or modify one another’s values because the two Valets were created with different initializers. All Valet types can share secrets across applications written by the same developer by using the `sharedAccessGroupValet` initializer.
+This instance can be used to store and retrieve data securely across any app written by the same developer that has `AppID12345.Druidia` (or `$(AppIdentifierPrefix)Druidia`) set as a value for the `keychain-access-groups` key in the app’s `Entitlements`, where `AppID12345` is the application’s [App ID prefix](https://developer.apple.com/documentation/security/keychain_services/keychain_items/sharing_access_to_keychain_items_among_a_collection_of_apps#2974920). This Valet is accessible when the device is unlocked. Note that `myValet` and `mySharedValet` can not read or modify one another’s values because the two Valets were created with different initializers. All Valet types can share secrets across applications written by the same developer by using the `sharedAccessGroupValet` initializer.
 
 ### Sharing Secrets Across Devices with iCloud
 

Sources/Valet/Internal/SecItem.swift

@@ -31,52 +31,7 @@ internal func execute<ReturnType>(in lock: NSLock, block: () throws -> ReturnTyp
 
 
 internal final class SecItem {
-    
-    // MARK: Internal Class Properties
-
-    /// Programatically grab the required prefix for the shared access group (i.e. Bundle Seed ID). The value for the kSecAttrAccessGroup key in queries for data that is shared between apps must be of the format bundleSeedID.sharedAccessGroup. For more information on the Bundle Seed ID, see https://developer.apple.com/library/ios/qa/qa1713/_index.html
-    internal static var sharedAccessGroupPrefix: String? {
-        var query: [CFString : Any] = [
-            kSecClass : kSecClassGenericPassword,
-            kSecAttrAccount : "SharedAccessGroupPrefixPlaceholder",
-            kSecReturnAttributes : true,
-            kSecAttrAccessible : Accessibility.afterFirstUnlockThisDeviceOnly.secAccessibilityAttribute
-        ]
-
-        if #available(iOS 13.0, tvOS 13.0, watchOS 6.0, macOS 10.15, *) {
-            // Add kSecUseDataProtectionKeychain to the query to ensure we can retrieve the shared access group prefix.
-            query[kSecUseDataProtectionKeychain] = true
-        }
-
-        secItemLock.lock()
-        defer {
-            secItemLock.unlock()
-        }
-        
-        var result: AnyObject? = nil
-        var status = SecItemCopyMatching(query as CFDictionary, &result)
 
-        if status == errSecItemNotFound {
-            status = SecItemAdd(query as CFDictionary, &result)
-        }
-        
-        guard status == errSecSuccess, let queryResult = result as? [CFString : AnyHashable], let accessGroup = queryResult[kSecAttrAccessGroup] as? String else {
-            // We may not be able to access the shared access group prefix because the accessibility of the above keychain data is set to `afterFirstUnlock`.
-            // Consumers should always check `canAccessKeychain()` after creating a Valet and before using it. Doing so will catch this error.
-            return nil
-        }
-        
-        let components = accessGroup.components(separatedBy: ".")
-        if let bundleSeedIdentifier = components.first, !bundleSeedIdentifier.isEmpty {
-            return bundleSeedIdentifier
-            
-        } else {
-            // We may not be able to access the shared access group prefix because the accessibility of the above keychain data is set to `afterFirstUnlock`.
-            // Consumers should always check `canAccessKeychain()` after creating a Valet and before using it. Doing so will catch this error.
-            return nil
-        }
-    }
-    
     // MARK: Internal Class Methods
 
     internal static func copy<DesiredType>(matching query: [String : AnyHashable]) throws -> DesiredType {

Sources/Valet/Internal/Service.swift

@@ -23,11 +23,11 @@ import Foundation
 
 internal enum Service: CustomStringConvertible, Equatable {
     case standard(Identifier, Configuration)
-    case sharedAccessGroup(Identifier, Configuration)
+    case sharedAccessGroup(SharedAccessGroupIdentifier, Configuration)
 
     #if os(macOS)
     case standardOverride(service: Identifier, Configuration)
-    case sharedAccessGroupOverride(service: Identifier, Configuration)
+    case sharedAccessGroupOverride(service: SharedAccessGroupIdentifier, Configuration)
     #endif
 
     // MARK: Equatable
@@ -48,13 +48,17 @@ internal enum Service: CustomStringConvertible, Equatable {
         "VAL_\(configuration.description)_initWithIdentifier:accessibility:_\(identifier)_\(accessibilityDescription)"
     }
 
-    internal static func sharedAccessGroup(with configuration: Configuration, identifier: Identifier, accessibilityDescription: String) -> String {
+    internal static func sharedAccessGroup(with configuration: Configuration, identifier: SharedAccessGroupIdentifier, accessibilityDescription: String) -> String {
+        "VAL_\(configuration.description)_initWithSharedAccessGroupIdentifier:accessibility:_\(identifier.groupIdentifier)_\(accessibilityDescription)"
+    }
+
+    internal static func sharedAccessGroup(with configuration: Configuration, explicitlySetIdentifier identifier: Identifier, accessibilityDescription: String) -> String {
         "VAL_\(configuration.description)_initWithSharedAccessGroupIdentifier:accessibility:_\(identifier)_\(accessibilityDescription)"
     }
 
     // MARK: Internal Methods
 
-    internal func generateBaseQuery() throws -> [String : AnyHashable] {
+    internal func generateBaseQuery() -> [String : AnyHashable] {
         var baseQuery: [String : AnyHashable] = [
             kSecClass as String : kSecClassGenericPassword as String,
             kSecAttrService as String : secService,
@@ -70,31 +74,15 @@ internal enum Service: CustomStringConvertible, Equatable {
             configuration = desiredConfiguration
 
         case let .sharedAccessGroup(identifier, desiredConfiguration):
-            guard let sharedAccessGroupPrefix = SecItem.sharedAccessGroupPrefix else {
-                throw KeychainError.couldNotAccessKeychain
-            }
-            if identifier.description.hasPrefix("\(sharedAccessGroupPrefix).") {
-                // The Bundle Seed ID was passed in as a prefix to the identifier.
-                baseQuery[kSecAttrAccessGroup as String] = identifier.description
-            } else {
-                baseQuery[kSecAttrAccessGroup as String] = "\(sharedAccessGroupPrefix).\(identifier.description)"
-            }
+            baseQuery[kSecAttrAccessGroup as String] = identifier.description
             configuration = desiredConfiguration
 
         #if os(macOS)
         case let .standardOverride(_, desiredConfiguration):
             configuration = desiredConfiguration
 
         case let .sharedAccessGroupOverride(identifier, desiredConfiguration):
-            guard let sharedAccessGroupPrefix = SecItem.sharedAccessGroupPrefix else {
-                throw KeychainError.couldNotAccessKeychain
-            }
-            if identifier.description.hasPrefix("\(sharedAccessGroupPrefix).") {
-                // The Bundle Seed ID was passed in as a prefix to the identifier.
-                baseQuery[kSecAttrAccessGroup as String] = identifier.description
-            } else {
-                baseQuery[kSecAttrAccessGroup as String] = "\(sharedAccessGroupPrefix).\(identifier.description)"
-            }
+            baseQuery[kSecAttrAccessGroup as String] = identifier.description
             configuration = desiredConfiguration
         #endif
         }
@@ -126,9 +114,10 @@ internal enum Service: CustomStringConvertible, Equatable {
         case let .sharedAccessGroup(identifier, configuration):
             service = Service.sharedAccessGroup(with: configuration, identifier: identifier, accessibilityDescription: configuration.accessibility.description)
         #if os(macOS)
-        case let .standardOverride(identifier, _),
-             let .sharedAccessGroupOverride(identifier, _):
+        case let .standardOverride(identifier, _):
             service = identifier.description
+        case let .sharedAccessGroupOverride(identifier, _):
+            service = identifier.groupIdentifier
         #endif
         }
 

Sources/Valet/SecureEnclave.swift

@@ -25,26 +25,24 @@ public final class SecureEnclave {
 
     // MARK: Internal Methods
 
-    /// - Parameters:
-    ///   - service: The service of the keychain slice we want to check if we can access.
-    ///   - identifier: A non-empty identifier that scopes the slice of keychain we want to access.
+    /// - Parameter service: The service of the keychain slice we want to check if we can access.
     /// - Returns: `true` if the keychain is accessible for reading and writing, `false` otherwise.
     /// - Note: Determined by writing a value to the keychain and then reading it back out.
-    internal static func canAccessKeychain(with service: Service, identifier: Identifier) -> Bool {
+    internal static func canAccessKeychain(with service: Service) -> Bool {
         // To avoid prompting the user for Touch ID or passcode, create a Valet with our identifier and accessibility and ask it if it can access the keychain.
         let noPromptValet: Valet
         switch service {
         #if os(macOS)
-        case .standardOverride:
-            fallthrough
+        case let .standardOverride(identifier, _):
+            noPromptValet = .valet(with: identifier, accessibility: .whenPasscodeSetThisDeviceOnly)
         #endif
-        case .standard:
+        case let .standard(identifier, _):
             noPromptValet = .valet(with: identifier, accessibility: .whenPasscodeSetThisDeviceOnly)
         #if os(macOS)
-        case .sharedAccessGroupOverride:
-            fallthrough
+        case let .sharedAccessGroupOverride(identifier, _):
+            noPromptValet = .sharedAccessGroupValet(withExplicitlySet: identifier, accessibility: .whenPasscodeSetThisDeviceOnly)
         #endif
-        case .sharedAccessGroup:
+        case let .sharedAccessGroup(identifier, _):
             noPromptValet = .sharedAccessGroupValet(with: identifier, accessibility: .whenPasscodeSetThisDeviceOnly)
         }
 

Sources/Valet/SecureEnclaveValet.swift

@@ -47,7 +47,7 @@ public final class SecureEnclaveValet: NSObject {
     ///   - identifier: A non-empty string that must correspond with the value for keychain-access-groups in your Entitlements file.
     ///   - accessControl: The desired access control for the SecureEnclaveValet.
     /// - Returns: A SecureEnclaveValet that reads/writes keychain elements that can be shared across applications written by the same development team.
-    public class func sharedAccessGroupValet(with identifier: Identifier, accessControl: SecureEnclaveAccessControl) -> SecureEnclaveValet {
+    public class func sharedAccessGroupValet(with identifier: SharedAccessGroupIdentifier, accessControl: SecureEnclaveAccessControl) -> SecureEnclaveValet {
         let key = Service.sharedAccessGroup(identifier, .secureEnclave(accessControl)).description as NSString
         if let existingValet = identifierToValetMap.object(forKey: key) {
             return existingValet
@@ -84,18 +84,18 @@ public final class SecureEnclaveValet: NSObject {
             accessControl: accessControl)
     }
 
-    private convenience init(sharedAccess identifier: Identifier, accessControl: SecureEnclaveAccessControl) {
+    private convenience init(sharedAccess groupIdentifier: SharedAccessGroupIdentifier, accessControl: SecureEnclaveAccessControl) {
         self.init(
-            identifier: identifier,
-            service: .sharedAccessGroup(identifier, .secureEnclave(accessControl)),
+            identifier: groupIdentifier.asIdentifier,
+            service: .sharedAccessGroup(groupIdentifier, .secureEnclave(accessControl)),
             accessControl: accessControl)
     }
 
     private init(identifier: Identifier, service: Service, accessControl: SecureEnclaveAccessControl) {
         self.identifier = identifier
         self.service = service
         self.accessControl = accessControl
-        _keychainQuery = try? service.generateBaseQuery()
+        baseKeychainQuery = service.generateBaseQuery()
     }
 
     // MARK: Hashable
@@ -116,7 +116,7 @@ public final class SecureEnclaveValet: NSObject {
     /// - Note: Determined by writing a value to the keychain and then reading it back out. Will never prompt the user for Face ID, Touch ID, or password.
     @objc
     public func canAccessKeychain() -> Bool {
-        SecureEnclave.canAccessKeychain(with: service, identifier: identifier)
+        SecureEnclave.canAccessKeychain(with: service)
     }
 
     /// - Parameters:
@@ -126,7 +126,7 @@ public final class SecureEnclaveValet: NSObject {
     @objc
     public func setObject(_ object: Data, forKey key: String) throws {
         try execute(in: lock) {
-            try SecureEnclave.setObject(object, forKey: key, options: try keychainQuery())
+            try SecureEnclave.setObject(object, forKey: key, options: baseKeychainQuery)
         }
     }
 
@@ -138,7 +138,7 @@ public final class SecureEnclaveValet: NSObject {
     @objc
     public func object(forKey key: String, withPrompt userPrompt: String) throws -> Data {
         try execute(in: lock) {
-            try SecureEnclave.object(forKey: key, withPrompt: userPrompt, options: try keychainQuery())
+            try SecureEnclave.object(forKey: key, withPrompt: userPrompt, options: baseKeychainQuery)
         }
     }
 
@@ -148,7 +148,7 @@ public final class SecureEnclaveValet: NSObject {
     /// - Note: Will never prompt the user for Face ID, Touch ID, or password.
     public func containsObject(forKey key: String) throws -> Bool {
         try execute(in: lock) {
-            try SecureEnclave.containsObject(forKey: key, options: try keychainQuery())
+            try SecureEnclave.containsObject(forKey: key, options: baseKeychainQuery)
         }
     }
 
@@ -159,7 +159,7 @@ public final class SecureEnclaveValet: NSObject {
     @objc
     public func setString(_ string: String, forKey key: String) throws {
         try execute(in: lock) {
-            try SecureEnclave.setString(string, forKey: key, options: try keychainQuery())
+            try SecureEnclave.setString(string, forKey: key, options: baseKeychainQuery)
         }
     }
 
@@ -171,7 +171,7 @@ public final class SecureEnclaveValet: NSObject {
     @objc
     public func string(forKey key: String, withPrompt userPrompt: String) throws -> String {
         try execute(in: lock) {
-            try SecureEnclave.string(forKey: key, withPrompt: userPrompt, options: try keychainQuery())
+            try SecureEnclave.string(forKey: key, withPrompt: userPrompt, options: baseKeychainQuery)
         }
     }
 
@@ -181,7 +181,7 @@ public final class SecureEnclaveValet: NSObject {
     @objc
     public func removeObject(forKey key: String) throws {
         try execute(in: lock) {
-            try Keychain.removeObject(forKey: key, options: try keychainQuery())
+            try Keychain.removeObject(forKey: key, options: baseKeychainQuery)
         }
     }
 
@@ -190,7 +190,7 @@ public final class SecureEnclaveValet: NSObject {
     @objc
     public func removeAllObjects() throws {
         try execute(in: lock) {
-            try Keychain.removeAllObjects(matching: try keychainQuery())
+            try Keychain.removeAllObjects(matching: baseKeychainQuery)
         }
     }
 
@@ -203,7 +203,7 @@ public final class SecureEnclaveValet: NSObject {
     @objc
     public func migrateObjects(matching query: [String : AnyHashable], removeOnCompletion: Bool) throws {
         try execute(in: lock) {
-            try Keychain.migrateObjects(matching: query, into: try keychainQuery(), removeOnCompletion: removeOnCompletion)
+            try Keychain.migrateObjects(matching: query, into: baseKeychainQuery, removeOnCompletion: removeOnCompletion)
         }
     }
 
@@ -215,7 +215,7 @@ public final class SecureEnclaveValet: NSObject {
     /// - Note: The keychain is not modified if an error is thrown.
     @objc
     public func migrateObjects(from valet: Valet, removeOnCompletion: Bool) throws {
-        try migrateObjects(matching: try valet.keychainQuery(), removeOnCompletion: removeOnCompletion)
+        try migrateObjects(matching: valet.baseKeychainQuery, removeOnCompletion: removeOnCompletion)
     }
 
     // MARK: Internal Properties
@@ -225,19 +225,8 @@ public final class SecureEnclaveValet: NSObject {
     // MARK: Private Properties
 
     private let lock = NSLock()
-    private var _keychainQuery: [String : AnyHashable]?
-
-    // MARK: Private Methods
+    private let baseKeychainQuery: [String : AnyHashable]
 
-    private func keychainQuery() throws -> [String : AnyHashable] {
-        if let keychainQuery = _keychainQuery {
-            return keychainQuery
-        } else {
-            let keychainQuery = try service.generateBaseQuery()
-            _keychainQuery = keychainQuery
-            return keychainQuery
-        }
-    }
 }
 
 
@@ -260,12 +249,14 @@ extension SecureEnclaveValet {
     }
 
     /// - Parameters:
-    ///   - identifier: A non-empty string that must correspond with the value for keychain-access-groups in your Entitlements file.
+    ///   - appIDPrefix: The application's App ID prefix. This string can be found by inspecting the application's provisioning profile, or viewing the application's App ID Configuration on developer.apple.com. This string must not be empty.
+    ///   - identifier: An identifier that cooresponds to a value in keychain-access-groups in the application's Entitlements file. This string must not be empty.
     ///   - accessControl: The desired access control for the SecureEnclaveValet.
     /// - Returns: A SecureEnclaveValet that reads/writes keychain elements that can be shared across applications written by the same development team.
-    @objc(sharedAccessGroupValetWithIdentifier:accessControl:)
-    public class func 🚫swift_sharedAccessGroupValet(with identifier: String, accessControl: SecureEnclaveAccessControl) -> SecureEnclaveValet? {
-        guard let identifier = Identifier(nonEmpty: identifier) else {
+    /// - SeeAlso: https://developer.apple.com/documentation/security/keychain_services/keychain_items/sharing_access_to_keychain_items_among_a_collection_of_apps
+    @objc(sharedAccessGroupValetWithAppIDPrefix:sharedAccessGroupIdentifier:accessControl:)
+    public class func 🚫swift_sharedAccessGroupValet(appIDPrefix: String, nonEmptyIdentifier identifier: String, accessControl: SecureEnclaveAccessControl) -> SecureEnclaveValet? {
+        guard let identifier = SharedAccessGroupIdentifier(appIDPrefix: appIDPrefix, nonEmptyGroup: identifier) else {
             return nil
         }
         return sharedAccessGroupValet(with: identifier, accessControl: accessControl)