diff --git a/Package.swift b/Package.swift index 0c04c516..824d7206 100644 --- a/Package.swift +++ b/Package.swift @@ -23,4 +23,4 @@ let package = Package( ], swiftLanguageVersions: [.v4, .v4_2, .v5] ) -let version = Version(3, 2, 7) +let version = Version(3, 2, 8) diff --git a/README.md b/README.md index a11bc087..9113f9ce 100644 --- a/README.md +++ b/README.md @@ -171,6 +171,10 @@ However, because the Keychain is effectively disk storage, there is no guarantee Already using the Keychain and no longer want to maintain your own Keychain code? We feel you. That’s why we wrote `migrateObjects(matching query: [String : AnyHashable], removeOnCompletion: Bool)`. This method allows you to migrate all your existing Keychain entries to a Valet instance in one line. Just pass in a Dictionary with the `kSecClass`, `kSecAttrService`, and any other `kSecAttr*` attributes you use – we’ll migrate the data for you. +### Supporting macOS Catalina (10.15) and later + +If your macOS application supports macOS 10.14 or prior, you must run `myValet.migrateObjectsFromPreCatalina()` before reading values from a Valet. macOS Catalina introduced a breaking change to the macOS keychain, requiring that macOS keychain items that utilize `kSecAttrAccessible` or `kSecAttrAccessGroup` set `kSecUseDataProtectionKeychain` to `true` [when writing or accessing](https://developer.apple.com/documentation/security/ksecusedataprotectionkeychain) these items. Valet’s `migrateObjectsFromPreCatalina()` upgrades items entered into the keychain on older macOS devices or other operating systems to include the key:value pair `kSecUseDataProtectionKeychain:true`. Note that Valets that share keychain items between devices with iCloud are exempt from this requirement. Similarly, `SecureEnclaveValet` and `SinglePromptSecureEnclaveValet` are exempt from this requirement. + ### Debugging Valet guarantees it will never fail to write to or read from the keychain unless `canAccessKeychain()` returns `false`. There are only a few cases that can lead to the keychain being inaccessible: diff --git a/Sources/Valet/Internal/SecItem.swift b/Sources/Valet/Internal/SecItem.swift index af42034a..4cf4db2d 100644 --- a/Sources/Valet/Internal/SecItem.swift +++ b/Sources/Valet/Internal/SecItem.swift @@ -70,23 +70,32 @@ internal final class SecItem { /// 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 { - let query = [ + var query: [CFString : Any] = [ kSecClass : kSecClassGenericPassword, kSecAttrAccount : "SharedAccessGroupAlwaysAccessiblePrefixPlaceholder", kSecReturnAttributes : true, - kSecAttrAccessible : Accessibility.alwaysThisDeviceOnly.secAccessibilityAttribute - ] as CFDictionary - + kSecAttrAccessible : Accessibility.alwaysThisDeviceOnly.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. + #if swift(>=5.1) + query[kSecUseDataProtectionKeychain] = true + #else + query["nleg" as CFString] = true // kSecUseDataProtectionKeychain for Xcode 9 and Xcode 10 compatibility. + #endif + } + secItemLock.lock() defer { secItemLock.unlock() } var result: AnyObject? = nil - var status = SecItemCopyMatching(query, &result) + var status = SecItemCopyMatching(query as CFDictionary, &result) if status == errSecItemNotFound { - status = SecItemAdd(query, &result) + status = SecItemAdd(query as CFDictionary, &result) } guard status == errSecSuccess, let queryResult = result as? [CFString : AnyHashable], let accessGroup = queryResult[kSecAttrAccessGroup] as? String else { diff --git a/Sources/Valet/Internal/Service.swift b/Sources/Valet/Internal/Service.swift index 540bf8fc..f8d46947 100644 --- a/Sources/Valet/Internal/Service.swift +++ b/Sources/Valet/Internal/Service.swift @@ -42,9 +42,17 @@ internal enum Service: CustomStringConvertible, Equatable { internal func generateBaseQuery() -> [String : AnyHashable] { var baseQuery: [String : AnyHashable] = [ kSecClass as String : kSecClassGenericPassword as String, - kSecAttrService as String : secService + kSecAttrService as String : secService, ] - + + if #available(iOS 13.0, tvOS 13.0, watchOS 6.0, macOS 10.15, *) { + #if swift(>=5.1) + baseQuery[kSecUseDataProtectionKeychain as String] = true + #else + baseQuery["nleg"] = true // kSecUseDataProtectionKeychain for Xcode 9 and Xcode 10 compatibility. + #endif + } + let configuration: Configuration switch self { case let .standard(_, desiredConfiguration): diff --git a/Sources/Valet/Valet.swift b/Sources/Valet/Valet.swift index 51a2b3ba..dd77168d 100644 --- a/Sources/Valet/Valet.swift +++ b/Sources/Valet/Valet.swift @@ -242,6 +242,25 @@ public final class Valet: NSObject, KeychainQueryConvertible { return migrateObjects(matching: keychain.keychainQuery, removeOnCompletion: removeOnCompletion) } + #if os(macOS) + /// Migrates objects that were written to this Valet prior to macOS 10.15 to a format that can be read on macOS 10.15 and later. The new format is backwards compatible, allowing these values to be read on older operating systems. + /// - returns: Whether the migration succeeded or failed. + /// - note: The keychain is not modified if a failure occurs. This method can only be called from macOS 10.15 or later. + @available(macOS 10.15, *) + @objc(migrateObjectsFromPreCatalina) + public func migrateObjectsFromPreCatalina() -> MigrationResult { + var baseQuery = keychainQuery + #if swift(>=5.1) + baseQuery[kSecUseDataProtectionKeychain as String] = false + #else + baseQuery["nleg"] = false // kSecUseDataProtectionKeychain for Xcode 9 and Xcode 10 compatibility. + #endif + + // We do not need to remove these items on completion, since we are updating the kSecUseDataProtectionKeychain attribute in-place. + return migrateObjects(matching: baseQuery, removeOnCompletion: false) + } + #endif + // MARK: Internal Properties internal let configuration: Configuration diff --git a/Tests/ValetIntegrationTests/BackwardsCompatibilityTests/ValetBackwardsCompatibilityTests.swift b/Tests/ValetIntegrationTests/BackwardsCompatibilityTests/ValetBackwardsCompatibilityTests.swift index ec506186..16f2ba8a 100644 --- a/Tests/ValetIntegrationTests/BackwardsCompatibilityTests/ValetBackwardsCompatibilityTests.swift +++ b/Tests/ValetIntegrationTests/BackwardsCompatibilityTests/ValetBackwardsCompatibilityTests.swift @@ -93,6 +93,11 @@ class ValetBackwardsCompatibilityIntegrationTests: ValetIntegrationTests { legacyValet.setString(passcode, forKey: key) XCTAssertNotNil(legacyValet.string(forKey: key)) + if #available(OSX 10.15, *) { + #if os(macOS) + _ = permutation.migrateObjectsFromPreCatalina() + #endif + } XCTAssertEqual(legacyValet.string(forKey: key), permutation.string(forKey: key), "\(permutation) was not able to read from legacy counterpart: \(legacyValet)") } } @@ -102,6 +107,11 @@ class ValetBackwardsCompatibilityIntegrationTests: ValetIntegrationTests { legacyValet.setString(passcode, forKey: key) XCTAssertNotNil(legacyValet.string(forKey: key)) + if #available(OSX 10.15, *) { + #if os(macOS) + _ = permutation.migrateObjectsFromPreCatalina() + #endif + } XCTAssertEqual(legacyValet.string(forKey: key), permutation.string(forKey: key), "\(permutation) was not able to read from legacy counterpart: \(legacyValet)") } } diff --git a/Tests/ValetIntegrationTests/MacTests.swift b/Tests/ValetIntegrationTests/MacTests.swift index c727d827..a09d281b 100644 --- a/Tests/ValetIntegrationTests/MacTests.swift +++ b/Tests/ValetIntegrationTests/MacTests.swift @@ -91,5 +91,38 @@ class ValetMacTests: XCTestCase // If you add a breakpoint here then manually inspect the keychain via Keychain.app (search for "MacOSVulnTest"), "xctest" should be the only member of the Access Control list. // This is not be the case upon setting a breakpoint and inspecting before the valet.setString(, forKey:) call above. } + + // MARK: Migration - PreCatalina + + func test_migrateObjectsFromPreCatalina_migratesDataWrittenPreCatalina() { + guard #available(macOS 10.15, *) else { + return + } + guard testEnvironmentIsSigned() else { + return + } + + let valet = Valet.valet(with: Identifier(nonEmpty: "PreCatalinaTest")!, accessibility: .afterFirstUnlock) + var preCatalinaWriteQuery = valet.keychainQuery + #if swift(>=5.1) + preCatalinaWriteQuery[kSecUseDataProtectionKeychain as String] = nil + #else + preCatalinaWriteQuery["nleg"] = nil // kSecUseDataProtectionKeychain for Xcode 9 and Xcode 10 compatibility. + #endif + + let key = "PreCatalinaKey" + let object = Data("PreCatalinaValue".utf8) + preCatalinaWriteQuery[kSecAttrAccount as String] = key + preCatalinaWriteQuery[kSecValueData as String] = object + + // Make sure the item is not in the keychain before we start this test + SecItemDelete(preCatalinaWriteQuery as CFDictionary) + + XCTAssertEqual(SecItemAdd(preCatalinaWriteQuery as CFDictionary, nil), errSecSuccess) + XCTAssertNil(valet.object(forKey: key)) + XCTAssertEqual(valet.migrateObjectsFromPreCatalina(), .success) + XCTAssertEqual(valet.object(forKey: key), object) + } + } #endif diff --git a/Tests/ValetIntegrationTests/SecItemTests.swift b/Tests/ValetIntegrationTests/SecItemTests.swift index 4c47386c..d6579d54 100644 --- a/Tests/ValetIntegrationTests/SecItemTests.swift +++ b/Tests/ValetIntegrationTests/SecItemTests.swift @@ -34,7 +34,10 @@ class SecItemTests: XCTestCase { XCTAssertEqual(SecItem.sharedAccessGroupPrefix, "9XUJ7M53NG") #endif #elseif os(macOS) - // Do nothing. + guard testEnvironmentIsSigned() else { + return + } + XCTAssertEqual(SecItem.sharedAccessGroupPrefix, "9XUJ7M53NG") #else // Currently unsupported build configuration. This next line will compile-time error. doNotCommentOutThisLine() diff --git a/Tests/ValetIntegrationTests/ValetIntegrationTests.swift b/Tests/ValetIntegrationTests/ValetIntegrationTests.swift index d49e19e7..2ec21e6e 100644 --- a/Tests/ValetIntegrationTests/ValetIntegrationTests.swift +++ b/Tests/ValetIntegrationTests/ValetIntegrationTests.swift @@ -284,6 +284,29 @@ class ValetIntegrationTests: XCTestCase XCTAssertNil(anotherFlavor.string(forKey: "cookie")) } + + #if !os(macOS) + func test_objectForKey_canReadItemsWithout_kSecUseDataProtectionKeychain_when_kSecUseDataProtectionKeychain_isSetToTrueInKeychainQuery() { + let valet = Valet.valet(with: Identifier(nonEmpty: "DataProtectionTest")!, accessibility: .afterFirstUnlock) + var dataProtectionWriteQuery = valet.keychainQuery + #if swift(>=5.1) + dataProtectionWriteQuery[kSecUseDataProtectionKeychain as String] = nil + #else + dataProtectionWriteQuery["nleg"] = nil // kSecUseDataProtectionKeychain for Xcode 9 and Xcode 10 compatibility. + #endif + + let key = "DataProtectionKey" + let object = Data("DataProtectionValue".utf8) + dataProtectionWriteQuery[kSecAttrAccount as String] = key + dataProtectionWriteQuery[kSecValueData as String] = object + + // Make sure the item is not in the keychain before we start this test + SecItemDelete(dataProtectionWriteQuery as CFDictionary) + + XCTAssertEqual(SecItemAdd(dataProtectionWriteQuery as CFDictionary, nil), errSecSuccess) + XCTAssertEqual(valet.object(forKey: key), object) // If this breaks, it means Apple has changed behavior of SecItemCopy. It means that we need to remove `kSecUseDataProtectionKeychain` from our query on non-Mac platforms. + } + #endif // MARK: set(string:forKey:) diff --git a/Valet.podspec b/Valet.podspec index 2cd8d12c..be7b65ed 100644 --- a/Valet.podspec +++ b/Valet.podspec @@ -1,6 +1,6 @@ Pod::Spec.new do |s| s.name = 'Valet' - s.version = '3.2.6' + s.version = '3.2.8' s.license = 'Apache License, Version 2.0' s.summary = 'Securely store data on iOS, tvOS, watchOS, or macOS without knowing a thing about how the Keychain works. It\'s easy. We promise.' s.homepage = 'https://github.com/square/Valet'