Changeset 289059 in webkit

Timestamp:

Feb 3, 2022 10:01:45 AM (6 months ago)

Author:

J Pascoe

Message:

[WebAuthn] Allow use of hardware-fixed credentials while using alternate store
​https://bugs.webkit.org/show_bug.cgi?id=235923
rdar://88102108

Reviewed by Brent Fulgham.

Source/WebKit:

This patch allows use of credentials created before a user started using
the alternate credential store by searching regardless of status when
querying credentials.

Added API test + tested manually.

• UIProcess/WebAuthentication/Cocoa/LocalAuthenticator.mm:

(WebKit::LocalAuthenticatorInternal::getExistingCredentials):
(WebKit::LocalAuthenticator::continueGetAssertionAfterUserVerification):

Tools:

Add new test for querying credentials created both before and after enabling
alternative credential store.

• TestWebKitAPI/Tests/WebKitCocoa/_WKWebAuthenticationPanel.mm:

(TestWebKitAPI::WebCore::addKeyToKeychain):
(TestWebKitAPI::WebCore::cleanUpKeychain):
(TestWebKitAPI::TEST):

Location:

trunk

Files:

• Source/WebKit/ChangeLog (modified) (1 diff)
• Source/WebKit/UIProcess/WebAuthentication/Cocoa/LocalAuthenticator.mm (modified) (4 diffs)
• Tools/ChangeLog (modified) (1 diff)
• Tools/TestWebKitAPI/Tests/WebKitCocoa/_WKWebAuthenticationPanel.mm (modified) (5 diffs)

trunk/Source/WebKit/ChangeLog

@@ +1 @@
+2022-02-03  J Pascoe  <j_pascoe@apple.com>
+
+        [WebAuthn] Allow use of hardware-fixed credentials while using alternate store
+        https://bugs.webkit.org/show_bug.cgi?id=235923
+        rdar://88102108
+
+        Reviewed by Brent Fulgham.
+
+        This patch allows use of credentials created before a user started using
+        the alternate credential store by searching regardless of status when
+        querying credentials.
+
+        Added API test + tested manually.
+
+        * UIProcess/WebAuthentication/Cocoa/LocalAuthenticator.mm:
+        (WebKit::LocalAuthenticatorInternal::getExistingCredentials):
+        (WebKit::LocalAuthenticator::continueGetAssertionAfterUserVerification):
+
 2022-02-03  Per Arne Vollan  <pvollan@apple.com>
 

trunk/Source/WebKit/UIProcess/WebAuthentication/Cocoa/LocalAuthenticator.mm

@@ -121 +121 @@
 {
     // Search Keychain for existing credential matched the RP ID.
-    auto query = adoptNS([[NSMutableDictionary alloc] init]);
-    [query setDictionary:@{
+    NSDictionary *query = @{
         (id)kSecClass: (id)kSecClassKey,
         (id)kSecAttrKeyClass: (id)kSecAttrKeyClassPrivate,
+        (id)kSecAttrSynchronizable: (id)kSecAttrSynchronizableAny,
         (id)kSecAttrLabel: rpId,
         (id)kSecReturnAttributes: @YES,
         (id)kSecMatchLimit: (id)kSecMatchLimitAll,
         (id)kSecUseDataProtectionKeychain: @YES
-    }];
-    updateQueryIfNecessary(query.get());
+    };
 
     CFTypeRef attributesArrayRef = nullptr;
-    OSStatus status = SecItemCopyMatching((__bridge CFDictionaryRef)query.get(), &attributesArrayRef);
+    OSStatus status = SecItemCopyMatching((__bridge CFDictionaryRef)query, &attributesArrayRef);
     if (status && status != errSecItemNotFound)
         return std::nullopt;
@@ -600 +599 @@
             (id)kSecClass: (id)kSecClassKey,
             (id)kSecAttrKeyClass: (id)kSecAttrKeyClassPrivate,
+            (id)kSecAttrSynchronizable: (id)kSecAttrSynchronizableAny,
             (id)kSecAttrApplicationLabel: nsCredentialId.get(),
             (id)kSecReturnRef: @YES,
@@ -609 +609 @@
 
         auto query = adoptNS(queryDictionary);
-        updateQueryIfNecessary(query.get());
 
         CFTypeRef privateKeyRef = nullptr;
@@ -634 +633 @@
     // Extra step: update the Keychain item with the same value to update its modification date such that LRU can be used
     // for selectAssertionResponse
-    auto query = adoptNS([[NSMutableDictionary alloc] init]);
-    [query setDictionary:@{
+    NSDictionary *query = @{
         (id)kSecClass: (id)kSecClassKey,
         (id)kSecAttrKeyClass: (id)kSecAttrKeyClassPrivate,
+        (id)kSecAttrSynchronizable: (id)kSecAttrSynchronizableAny,
         (id)kSecAttrApplicationLabel: nsCredentialId.get(),
         (id)kSecUseDataProtectionKeychain: @YES
-    }];
-    updateQueryIfNecessary(query.get());
+    };
 
     NSDictionary *updateParams = @{
         (id)kSecAttrLabel: requestOptions.rpId,
     };
-    auto status = SecItemUpdate((__bridge CFDictionaryRef)query.get(), (__bridge CFDictionaryRef)updateParams);
+    auto status = SecItemUpdate((__bridge CFDictionaryRef)query, (__bridge CFDictionaryRef)updateParams);
     if (status)
         LOG_ERROR("Couldn't update the Keychain item: %d", status);

trunk/Tools/ChangeLog

@@ +1 @@
+2022-02-03  J Pascoe  <j_pascoe@apple.com>
+
+        [WebAuthn] Allow use of hardware-fixed credentials while using alternate store
+        https://bugs.webkit.org/show_bug.cgi?id=235923
+        rdar://88102108
+
+        Reviewed by Brent Fulgham.
+
+        Add new test for querying credentials created both before and after enabling
+        alternative credential store.
+
+        * TestWebKitAPI/Tests/WebKitCocoa/_WKWebAuthenticationPanel.mm:
+        (TestWebKitAPI::WebCore::addKeyToKeychain):
+        (TestWebKitAPI::WebCore::cleanUpKeychain):
+        (TestWebKitAPI::TEST):
+
 2022-02-03  Carlos Garcia Campos  <cgarcia@igalia.com>
 

trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/_WKWebAuthenticationPanel.mm

@@ -363 +363 @@
 #if USE(APPLE_INTERNAL_SDK) || PLATFORM(IOS)
 
-bool addKeyToKeychain(const String& privateKeyBase64, const String& rpId, const String& userHandleBase64)
+bool addKeyToKeychain(const String& privateKeyBase64, const String& rpId, const String& userHandleBase64, bool synchronizable = false)
 {
     NSDictionary* options = @{
@@ -379 +379 @@
         return false;
 
-    NSDictionary* addQuery = @{
+    auto addQuery = adoptNS([[NSMutableDictionary alloc] init]);
+    [addQuery setDictionary:@{
         (id)kSecValueRef: (id)key.get(),
         (id)kSecClass: (id)kSecClassKey,
@@ -386 +387 @@
         (id)kSecAttrAccessible: (id)kSecAttrAccessibleAfterFirstUnlock,
         (id)kSecUseDataProtectionKeychain: @YES
-    };
-    OSStatus status = SecItemAdd((__bridge CFDictionaryRef)addQuery, NULL);
+    }];
+    if (synchronizable)
+        [addQuery.get() setObject:@YES forKey:(__bridge id)kSecAttrSynchronizable];
+
+    OSStatus status = SecItemAdd((__bridge CFDictionaryRef)addQuery.get(), NULL);
     if (status)
         return false;
@@ -399 +403 @@
         (id)kSecClass: (id)kSecClassKey,
         (id)kSecAttrLabel: rpId,
+        (id)kSecAttrSynchronizable: (id)kSecAttrSynchronizableAny,
         (id)kSecAttrAccessible: (id)kSecAttrAccessibleAfterFirstUnlock,
         (id)kSecUseDataProtectionKeychain: @YES
@@ -1511 +1516 @@
 }
 
+TEST(WebAuthenticationPanel, LAGetAssertionMultipleCredentialStore)
+{
+    reset();
+    RetainPtr<NSURL> testURL = [[NSBundle mainBundle] URLForResource:@"web-authentication-get-assertion-la" withExtension:@"html" subdirectory:@"TestWebKitAPI.resources"];
+
+    auto *configuration = [WKWebViewConfiguration _test_configurationWithTestPlugInClassName:@"WebProcessPlugInWithInternals" configureJSCForTesting:YES];
+    [[configuration preferences] _setEnabled:NO forExperimentalFeature:webAuthenticationModernExperimentalFeature()];
+
+    auto webView = adoptNS([[TestWKWebView alloc] initWithFrame:NSZeroRect configuration:configuration]);
+    auto delegate = adoptNS([[TestWebAuthenticationPanelUIDelegate alloc] init]);
+    [webView setUIDelegate:delegate.get()];
+    [webView focus];
+
+    ASSERT_TRUE(addKeyToKeychain(testES256PrivateKeyBase64, "", testUserEntityBundleBase64));
+    ASSERT_TRUE(addKeyToKeychain("BBRoi2JbR0IXTeJmvXUp1YIuM4sph/Lu3eGf75F7n+HojHKG70a4R0rB2PQce5/SJle6T7OO5Cqet/LJZVM6NQ8yDDxWvayf71GTDp2yUtuIbqJLFVbpWymlj9WRizgX3A==", "", "omJpZEoAAQIDBAUGBwgJZG5hbWVkSmFuZQ=="/* { "id": h'00010203040506070809', "name": "Jane" } */, true /* synchronizable */));
+
+    [webView loadRequest:[NSURLRequest requestWithURL:testURL.get()]];
+    [webView waitForMessage:@"Succeeded!"];
+    EXPECT_WK_STREQ(webAuthenticationPanelSelectedCredentialName, "John");
+
+    [webView loadRequest:[NSURLRequest requestWithURL:testURL.get()]];
+    [webView waitForMessage:@"Succeeded!"];
+    EXPECT_WK_STREQ(webAuthenticationPanelSelectedCredentialName, "Jane");
+
+    cleanUpKeychain("");
+}
+
 TEST(WebAuthenticationPanel, LAGetAssertionNoMockNoUserGesture)
 {