Title: [289059] trunk
Revision
289059
Author
j_pas...@apple.com
Date
2022-02-03 10:01:45 -0800 (Thu, 03 Feb 2022)

Log 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):

Modified Paths

Diff

Modified: trunk/Source/WebKit/ChangeLog (289058 => 289059)


--- trunk/Source/WebKit/ChangeLog	2022-02-03 17:58:13 UTC (rev 289058)
+++ trunk/Source/WebKit/ChangeLog	2022-02-03 18:01:45 UTC (rev 289059)
@@ -1,3 +1,21 @@
+2022-02-03  J Pascoe  <j_pas...@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  <pvol...@apple.com>
 
         [iOS][WP] Add file-ioctl telemetry

Modified: trunk/Source/WebKit/UIProcess/WebAuthentication/Cocoa/LocalAuthenticator.mm (289058 => 289059)


--- trunk/Source/WebKit/UIProcess/WebAuthentication/Cocoa/LocalAuthenticator.mm	2022-02-03 17:58:13 UTC (rev 289058)
+++ trunk/Source/WebKit/UIProcess/WebAuthentication/Cocoa/LocalAuthenticator.mm	2022-02-03 18:01:45 UTC (rev 289059)
@@ -120,19 +120,18 @@
 static std::optional<Vector<Ref<AuthenticatorAssertionResponse>>> getExistingCredentials(const String& rpId)
 {
     // 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;
     auto retainAttributesArray = adoptCF(attributesArrayRef);
@@ -599,6 +598,7 @@
         NSMutableDictionary *queryDictionary = [@{
             (id)kSecClass: (id)kSecClassKey,
             (id)kSecAttrKeyClass: (id)kSecAttrKeyClassPrivate,
+            (id)kSecAttrSynchronizable: (id)kSecAttrSynchronizableAny,
             (id)kSecAttrApplicationLabel: nsCredentialId.get(),
             (id)kSecReturnRef: @YES,
             (id)kSecUseDataProtectionKeychain: @YES
@@ -608,7 +608,6 @@
             queryDictionary[(id)kSecUseAuthenticationContext] = context;
 
         auto query = adoptNS(queryDictionary);
-        updateQueryIfNecessary(query.get());
 
         CFTypeRef privateKeyRef = nullptr;
         OSStatus status = SecItemCopyMatching((__bridge CFDictionaryRef)query.get(), &privateKeyRef);
@@ -633,19 +632,18 @@
 
     // 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);
 

Modified: trunk/Tools/ChangeLog (289058 => 289059)


--- trunk/Tools/ChangeLog	2022-02-03 17:58:13 UTC (rev 289058)
+++ trunk/Tools/ChangeLog	2022-02-03 18:01:45 UTC (rev 289059)
@@ -1,3 +1,19 @@
+2022-02-03  J Pascoe  <j_pas...@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  <cgar...@igalia.com>
 
         WebDriver: selenium tests are executed more than once

Modified: trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/_WKWebAuthenticationPanel.mm (289058 => 289059)


--- trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/_WKWebAuthenticationPanel.mm	2022-02-03 17:58:13 UTC (rev 289058)
+++ trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/_WKWebAuthenticationPanel.mm	2022-02-03 18:01:45 UTC (rev 289059)
@@ -362,7 +362,7 @@
 
 #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 = @{
         (id)kSecAttrKeyType: (id)kSecAttrKeyTypeECSECPrimeRandom,
@@ -378,7 +378,8 @@
     if (errorRef)
         return false;
 
-    NSDictionary* addQuery = @{
+    auto addQuery = adoptNS([[NSMutableDictionary alloc] init]);
+    [addQuery setDictionary:@{
         (id)kSecValueRef: (id)key.get(),
         (id)kSecClass: (id)kSecClassKey,
         (id)kSecAttrLabel: rpId,
@@ -385,8 +386,11 @@
         (id)kSecAttrApplicationTag: adoptNS([[NSData alloc] initWithBase64EncodedString:userHandleBase64 options:NSDataBase64DecodingIgnoreUnknownCharacters]).get(),
         (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;
 
@@ -398,6 +402,7 @@
     NSDictionary* deleteQuery = @{
         (id)kSecClass: (id)kSecClassKey,
         (id)kSecAttrLabel: rpId,
+        (id)kSecAttrSynchronizable: (id)kSecAttrSynchronizableAny,
         (id)kSecAttrAccessible: (id)kSecAttrAccessibleAfterFirstUnlock,
         (id)kSecUseDataProtectionKeychain: @YES
     };
@@ -1510,6 +1515,33 @@
     cleanUpKeychain("");
 }
 
+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)
 {
     reset();
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to