[WebAuthn] Relaxing signature length requirements for U2fRegister
authorjiewen_tan@apple.com <jiewen_tan@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 14 May 2020 23:49:04 +0000 (23:49 +0000)
committerjiewen_tan@apple.com <jiewen_tan@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 14 May 2020 23:49:04 +0000 (23:49 +0000)
https://bugs.webkit.org/show_bug.cgi?id=209645
<rdar://problem/63204591>

Reviewed by Brent Fulgham.

Source/WebCore:

It turns out the length range specified from the spec, i.e., [71, 73] is wrong.
https://fidoalliance.org/specs/fido-u2f-v1.2-ps-20170411/fido-u2f-raw-message-formats-v1.2-ps-20170411.html#registration-response-message-success

It should actually be [70, 72]. However, as a middleware to relay the messages, user agents
are not necessary to check the length. Therefore, the check is relaxed to make the code more robust.

Covered by existing tests.

* Modules/webauthn/fido/U2fResponseConverter.cpp:
(fido::WebCore::createFidoAttestationStatementFromU2fRegisterResponse):

Tools:

* TestWebKitAPI/Tests/WebCore/CtapResponseTest.cpp:
(TestWebKitAPI::TEST):

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

Source/WebCore/ChangeLog
Source/WebCore/Modules/webauthn/fido/U2fResponseConverter.cpp
Tools/ChangeLog
Tools/TestWebKitAPI/Tests/WebCore/CtapResponseTest.cpp

index 059117d..0dc737a 100644 (file)
@@ -1,3 +1,22 @@
+2020-05-14  Jiewen Tan  <jiewen_tan@apple.com>
+
+        [WebAuthn] Relaxing signature length requirements for U2fRegister
+        https://bugs.webkit.org/show_bug.cgi?id=209645
+        <rdar://problem/63204591>
+
+        Reviewed by Brent Fulgham.
+
+        It turns out the length range specified from the spec, i.e., [71, 73] is wrong.
+        https://fidoalliance.org/specs/fido-u2f-v1.2-ps-20170411/fido-u2f-raw-message-formats-v1.2-ps-20170411.html#registration-response-message-success
+
+        It should actually be [70, 72]. However, as a middleware to relay the messages, user agents
+        are not necessary to check the length. Therefore, the check is relaxed to make the code more robust.
+
+        Covered by existing tests.
+
+        * Modules/webauthn/fido/U2fResponseConverter.cpp:
+        (fido::WebCore::createFidoAttestationStatementFromU2fRegisterResponse):
+
 2020-05-14  Timothy Hatcher  <timothy@apple.com>
 
         Add sourceURL to _evaluateJavaScript: so the scripts appear in Web Inspector.
index 313eaf0..2b546d1 100644 (file)
@@ -49,9 +49,6 @@ namespace {
 const uint8_t uncompressedKey = 0x04;
 // https://www.w3.org/TR/webauthn/#flags
 const uint8_t makeCredentialFlags = 0b01000001; // UP and AT are set.
-// https://fidoalliance.org/specs/fido-u2f-v1.2-ps-20170411/fido-u2f-raw-message-formats-v1.2-ps-20170411.html#registration-response-message-success
-const uint8_t minSignatureLength = 71;
-const uint8_t maxSignatureLength = 73;
 // https://fidoalliance.org/specs/fido-u2f-v1.2-ps-20170411/fido-u2f-raw-message-formats-v1.2-ps-20170411.html#authentication-response-message-success
 const size_t flagIndex = 0;
 const size_t counterIndex = 1;
@@ -133,7 +130,7 @@ static cbor::CBORValue::MapValue createFidoAttestationStatementFromU2fRegisterRe
 
     Vector<uint8_t> signature;
     signature.append(u2fData.data() + offset, u2fData.size() - offset);
-    if (signature.size() < minSignatureLength || signature.size() > maxSignatureLength)
+    if (signature.isEmpty())
         return { };
 
     cbor::CBORValue::MapValue attestationStatementMap;
index 74ffb60..3d32d98 100644 (file)
@@ -1,3 +1,14 @@
+2020-05-14  Jiewen Tan  <jiewen_tan@apple.com>
+
+        [WebAuthn] Relaxing signature length requirements for U2fRegister
+        https://bugs.webkit.org/show_bug.cgi?id=209645
+        <rdar://problem/63204591>
+
+        Reviewed by Brent Fulgham.
+
+        * TestWebKitAPI/Tests/WebCore/CtapResponseTest.cpp:
+        (TestWebKitAPI::TEST):
+
 2020-05-14  Jonathan Bedard  <jbedard@apple.com>
 
         run-webkit-tests shouldn't need Xcode to run Mac tests
index a508e7f..c37fd68 100644 (file)
@@ -512,10 +512,6 @@ TEST(CTAPResponseTest, TestParseIncorrectRegisterResponseData4)
     const auto prefix = sizeof(TestData::kTestU2fRegisterResponse);
     auto response = readU2fRegisterResponse(TestData::kRelyingPartyId, getTestU2fRegisterResponse(prefix - 71, nullptr, 0));
     EXPECT_FALSE(response);
-
-    const uint8_t testData[] = { 0x40, 0x40, 0x40 };
-    response = readU2fRegisterResponse(TestData::kRelyingPartyId, getTestU2fRegisterResponse(prefix, testData, sizeof(testData)));
-    EXPECT_FALSE(response);
 }
 
 // Test malformed X.509 but pass.