[WebAuthN] UserHandle can be null
authorjiewen_tan@apple.com <jiewen_tan@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 15 Nov 2018 20:01:52 +0000 (20:01 +0000)
committerjiewen_tan@apple.com <jiewen_tan@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 15 Nov 2018 20:01:52 +0000 (20:01 +0000)
https://bugs.webkit.org/show_bug.cgi?id=191521

Reviewed by Alex Christensen.

Source/WebCore:

According to the newest spec as of 7 August, 2018: https://www.w3.org/TR/webauthn/#conforming-authenticators-u2f.
UserHandle can now be null.

Covered by existing tests.

* Modules/webauthn/AuthenticatorAssertionResponse.h:
(WebCore::AuthenticatorAssertionResponse::create):
(WebCore::AuthenticatorAssertionResponse::userHandle const):
(WebCore::AuthenticatorAssertionResponse::AuthenticatorAssertionResponse):
* Modules/webauthn/AuthenticatorAssertionResponse.idl:
* Modules/webauthn/PublicKeyCredential.cpp:
(WebCore::PublicKeyCredential::tryCreate):
* Modules/webauthn/PublicKeyCredentialData.h:
(WebCore::PublicKeyCredentialData::encode const):
(WebCore::PublicKeyCredentialData::decode):
* Modules/webauthn/fido/DeviceResponseConverter.cpp:
(fido::readCTAPGetAssertionResponse):

LayoutTests:

* http/wpt/webauthn/public-key-credential-get-success-hid.https.html:

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@238238 268f45cc-cd09-0410-ab3c-d52691b4dbfc

LayoutTests/ChangeLog
LayoutTests/http/wpt/webauthn/public-key-credential-get-success-hid.https.html
Source/WebCore/ChangeLog
Source/WebCore/Modules/webauthn/AuthenticatorAssertionResponse.h
Source/WebCore/Modules/webauthn/AuthenticatorAssertionResponse.idl
Source/WebCore/Modules/webauthn/PublicKeyCredential.cpp
Source/WebCore/Modules/webauthn/PublicKeyCredentialData.h
Source/WebCore/Modules/webauthn/fido/DeviceResponseConverter.cpp

index cb53c4b..0ec9b47 100644 (file)
@@ -1,3 +1,12 @@
+2018-11-15  Jiewen Tan  <jiewen_tan@apple.com>
+
+        [WebAuthN] UserHandle can be null
+        https://bugs.webkit.org/show_bug.cgi?id=191521
+
+        Reviewed by Alex Christensen.
+
+        * http/wpt/webauthn/public-key-credential-get-success-hid.https.html:
+
 2018-11-15  Daniel Bates  <dabates@apple.com>
 
         [iOS] Shift + Tab does not focus previous field
index 7e56478..ae57add 100644 (file)
@@ -15,7 +15,7 @@
         assert_equals(credential.type, 'public-key');
         assert_array_equals(new Uint8Array(credential.rawId), Base64URL.parse(testHidCredentialIdBase64));
         assert_equals(bytesToASCIIString(credential.response.clientDataJSON), '{"type":"webauthn.get","challenge":"MTIzNDU2","origin":"https://localhost:9443","hashAlgorithm":"SHA-256"}');
-        assert_equals(bytesToHexString(credential.response.userHandle), "00");
+        assert_equals(credential.response.userHandle, null);
 
         // Check authData
         const authData = decodeAuthData(new Uint8Array(credential.response.authenticatorData), false);
index c7c2ba9..40ffaba 100644 (file)
@@ -1,3 +1,28 @@
+2018-11-15  Jiewen Tan  <jiewen_tan@apple.com>
+
+        [WebAuthN] UserHandle can be null
+        https://bugs.webkit.org/show_bug.cgi?id=191521
+
+        Reviewed by Alex Christensen.
+
+        According to the newest spec as of 7 August, 2018: https://www.w3.org/TR/webauthn/#conforming-authenticators-u2f.
+        UserHandle can now be null.
+
+        Covered by existing tests.
+
+        * Modules/webauthn/AuthenticatorAssertionResponse.h:
+        (WebCore::AuthenticatorAssertionResponse::create):
+        (WebCore::AuthenticatorAssertionResponse::userHandle const):
+        (WebCore::AuthenticatorAssertionResponse::AuthenticatorAssertionResponse):
+        * Modules/webauthn/AuthenticatorAssertionResponse.idl:
+        * Modules/webauthn/PublicKeyCredential.cpp:
+        (WebCore::PublicKeyCredential::tryCreate):
+        * Modules/webauthn/PublicKeyCredentialData.h:
+        (WebCore::PublicKeyCredentialData::encode const):
+        (WebCore::PublicKeyCredentialData::decode):
+        * Modules/webauthn/fido/DeviceResponseConverter.cpp:
+        (fido::readCTAPGetAssertionResponse):
+
 2018-11-15  Youenn Fablet  <youenn@apple.com>
 
         Modernize RTCPeerConnection handling of pendingActivity
index cf28c1a..9c56520 100644 (file)
@@ -33,7 +33,7 @@ namespace WebCore {
 
 class AuthenticatorAssertionResponse : public AuthenticatorResponse {
 public:
-    static Ref<AuthenticatorAssertionResponse> create(Ref<ArrayBuffer>&& clientDataJSON, Ref<ArrayBuffer>&& authenticatorData, Ref<ArrayBuffer>&& signature, Ref<ArrayBuffer>&& userHandle)
+    static Ref<AuthenticatorAssertionResponse> create(Ref<ArrayBuffer>&& clientDataJSON, Ref<ArrayBuffer>&& authenticatorData, Ref<ArrayBuffer>&& signature, RefPtr<ArrayBuffer>&& userHandle)
     {
         return adoptRef(*new AuthenticatorAssertionResponse(WTFMove(clientDataJSON), WTFMove(authenticatorData), WTFMove(signature), WTFMove(userHandle)));
     }
@@ -42,10 +42,10 @@ public:
 
     ArrayBuffer* authenticatorData() const { return m_authenticatorData.ptr(); }
     ArrayBuffer* signature() const { return m_signature.ptr(); }
-    ArrayBuffer* userHandle() const { return m_userHandle.ptr(); }
+    ArrayBuffer* userHandle() const { return m_userHandle.get(); }
 
 private:
-    AuthenticatorAssertionResponse(Ref<ArrayBuffer>&& clientDataJSON, Ref<ArrayBuffer>&& authenticatorData, Ref<ArrayBuffer>&& signature, Ref<ArrayBuffer>&& userHandle)
+    AuthenticatorAssertionResponse(Ref<ArrayBuffer>&& clientDataJSON, Ref<ArrayBuffer>&& authenticatorData, Ref<ArrayBuffer>&& signature, RefPtr<ArrayBuffer>&& userHandle)
         : AuthenticatorResponse(WTFMove(clientDataJSON))
         , m_authenticatorData(WTFMove(authenticatorData))
         , m_signature(WTFMove(signature))
@@ -57,7 +57,7 @@ private:
 
     Ref<ArrayBuffer> m_authenticatorData;
     Ref<ArrayBuffer> m_signature;
-    Ref<ArrayBuffer> m_userHandle;
+    RefPtr<ArrayBuffer> m_userHandle;
 };
 
 } // namespace WebCore
index 6917585..f8e7e89 100644 (file)
@@ -31,5 +31,5 @@
 ] interface AuthenticatorAssertionResponse : AuthenticatorResponse {
     [SameObject] readonly attribute ArrayBuffer authenticatorData;
     [SameObject] readonly attribute ArrayBuffer signature;
-    [SameObject] readonly attribute ArrayBuffer userHandle;
+    [SameObject] readonly attribute ArrayBuffer? userHandle;
 };
index cff64dd..c6b6844 100644 (file)
@@ -57,10 +57,10 @@ RefPtr<PublicKeyCredential> PublicKeyCredential::tryCreate(const PublicKeyCreden
         return adoptRef(*new PublicKeyCredential(data.rawId.releaseNonNull(), AuthenticatorAttestationResponse::create(data.clientDataJSON.releaseNonNull(), data.attestationObject.releaseNonNull())));
     }
 
-    if (!data.authenticatorData || !data.signature || !data.userHandle)
+    if (!data.authenticatorData || !data.signature)
         return nullptr;
 
-    return adoptRef(*new PublicKeyCredential(data.rawId.releaseNonNull(), AuthenticatorAssertionResponse::create(data.clientDataJSON.releaseNonNull(), data.authenticatorData.releaseNonNull(), data.signature.releaseNonNull(), data.userHandle.releaseNonNull())));
+    return adoptRef(*new PublicKeyCredential(data.rawId.releaseNonNull(), AuthenticatorAssertionResponse::create(data.clientDataJSON.releaseNonNull(), data.authenticatorData.releaseNonNull(), data.signature.releaseNonNull(), WTFMove(data.userHandle))));
 }
 
 PublicKeyCredential::PublicKeyCredential(Ref<ArrayBuffer>&& id, Ref<AuthenticatorResponse>&& response)
index 33928a8..9bbef17 100644 (file)
@@ -74,12 +74,18 @@ void PublicKeyCredentialData::encode(Encoder& encoder) const
         return;
     }
 
-    if (!authenticatorData || !signature || !userHandle)
+    if (!authenticatorData || !signature)
         return;
     encoder << static_cast<uint64_t>(authenticatorData->byteLength());
     encoder.encodeFixedLengthData(reinterpret_cast<const uint8_t*>(authenticatorData->data()), authenticatorData->byteLength(), 1);
     encoder << static_cast<uint64_t>(signature->byteLength());
     encoder.encodeFixedLengthData(reinterpret_cast<const uint8_t*>(signature->data()), signature->byteLength(), 1);
+
+    if (!userHandle) {
+        encoder << false;
+        return;
+    }
+    encoder << true;
     encoder << static_cast<uint64_t>(userHandle->byteLength());
     encoder.encodeFixedLengthData(reinterpret_cast<const uint8_t*>(userHandle->data()), userHandle->byteLength(), 1);
 }
@@ -142,6 +148,13 @@ std::optional<PublicKeyCredentialData> PublicKeyCredentialData::decode(Decoder&
     if (!decoder.decodeFixedLengthData(reinterpret_cast<uint8_t*>(result.signature->data()), signatureLength.value(), 1))
         return std::nullopt;
 
+    std::optional<bool> hasUserHandle;
+    decoder >> hasUserHandle;
+    if (!hasUserHandle)
+        return std::nullopt;
+    if (!*hasUserHandle)
+        return result;
+
     std::optional<uint64_t> userHandleLength;
     decoder >> userHandleLength;
     if (!userHandleLength)
index 45365c9..80cf6de 100644 (file)
@@ -158,8 +158,7 @@ std::optional<PublicKeyCredentialData> readCTAPGetAssertionResponse(const Vector
         return std::nullopt;
     auto& signature = it->second.getByteString();
 
-    // FIXME(191521): Properly handle null userHandle.
-    RefPtr<ArrayBuffer> userHandle = ArrayBuffer::create(1, 1);
+    RefPtr<ArrayBuffer> userHandle;
     it = responseMap.find(CBOR(4));
     if (it != responseMap.end() && it->second.isMap()) {
         auto& user = it->second.getMap();