Skip to content
Permalink
Browse files
[WebAuthn] Should reject rp with empty id
https://bugs.webkit.org/show_bug.cgi?id=242091
rdar://90281653

Reviewed by Brent Fulgham.

Not specifying an rp.id should default to the caller’s origin's effective domain, but
empty / null values should be rejected per spec. https://www.w3.org/TR/webauthn-2/#sctn-createCredential

Updated tests.

* LayoutTests/http/wpt/webauthn/public-key-credential-create-failure.https-expected.txt:
* LayoutTests/http/wpt/webauthn/public-key-credential-create-failure.https.html:
* Source/WebCore/Modules/webauthn/AuthenticatorCoordinator.cpp:
(WebCore::AuthenticatorCoordinator::create const):
* Source/WebCore/Modules/webauthn/PublicKeyCredentialCreationOptions.h:
* Source/WebCore/Modules/webauthn/fido/DeviceRequestConverter.cpp:
(fido::convertRpEntityToCBOR):
* Source/WebCore/Modules/webauthn/fido/U2fCommandConstructor.cpp:
(fido::convertToU2fRegisterCommand):
(fido::convertToU2fCheckOnlySignCommand):
* Source/WebKit/UIProcess/API/Cocoa/_WKWebAuthenticationPanel.mm:
(publicKeyCredentialRpEntity):
* Source/WebKit/UIProcess/WebAuthentication/AuthenticatorManager.cpp:
(WebKit::WebCore::getRpId):
* Source/WebKit/UIProcess/WebAuthentication/Cocoa/LocalAuthenticator.mm:
(WebKit::LocalAuthenticator::makeCredential):
(WebKit::LocalAuthenticator::continueMakeCredentialAfterUserVerification):
* Source/WebKit/UIProcess/WebAuthentication/Cocoa/WebAuthenticatorCoordinatorProxy.mm:
(WebKit::configureRegistrationRequestContext):
* Source/WebKit/UIProcess/WebAuthentication/fido/U2fAuthenticator.cpp:
(WebKit::U2fAuthenticator::continueRegisterCommandAfterResponseReceived):
* Tools/TestWebKitAPI/Tests/WebKitCocoa/_WKWebAuthenticationPanel.mm:
(TestWebKitAPI::TEST):

Canonical link: https://commits.webkit.org/252142@main
  • Loading branch information
pascoej committed Jul 5, 2022
1 parent b29e62f commit 3b920f82563c500e5c4991cc04bf0b99b784b9c2
Show file tree
Hide file tree
Showing 12 changed files with 77 additions and 22 deletions.
@@ -7,6 +7,8 @@ CONSOLE MESSAGE: User gesture is not detected. To use the WebAuthn API, call 'na

PASS PublicKeyCredential's [[create]] with timeout
PASS PublicKeyCredential's [[create]] with a mismatched RP ID
PASS PublicKeyCredential's [[create]] with a empty string RP ID
PASS PublicKeyCredential's [[create]] with a null RP ID
PASS PublicKeyCredential's [[create]] with two consecutive requests
PASS PublicKeyCredential's [[create]] with two consecutive requests (2)
PASS PublicKeyCredential's [[create]] with new requests in a new page
@@ -50,6 +50,48 @@
navigator.credentials.create(options), "The provided RP ID is not a registrable domain suffix of the effective domain of the document.");
}, "PublicKeyCredential's [[create]] with a mismatched RP ID");

promise_test(function(t) {
const options = {
publicKey: {
rp: {
name: "example.com",
id: ""
},
user: {
name: "John Appleseed",
id: asciiToUint8Array("123456"),
displayName: "John",
},
challenge: asciiToUint8Array("123456"),
pubKeyCredParams: [{ type: "public-key", alg: -7 }],
}
};

return promiseRejects(t, "SecurityError",
navigator.credentials.create(options), "The provided RP ID is not a registrable domain suffix of the effective domain of the document.");
}, "PublicKeyCredential's [[create]] with a empty string RP ID");

promise_test(function(t) {
const options = {
publicKey: {
rp: {
name: "example.com",
id: null
},
user: {
name: "John Appleseed",
id: asciiToUint8Array("123456"),
displayName: "John",
},
challenge: asciiToUint8Array("123456"),
pubKeyCredParams: [{ type: "public-key", alg: -7 }],
}
};

return promiseRejects(t, "SecurityError",
navigator.credentials.create(options), "The provided RP ID is not a registrable domain suffix of the effective domain of the document.");
}, "PublicKeyCredential's [[create]] with a null RP ID");

promise_test(function(t) {
const options = {
publicKey: {
@@ -137,12 +137,12 @@ void AuthenticatorCoordinator::create(const Document& document, const PublicKeyC
}

// Step 8.
if (!options.rp.id.isEmpty() && !callerOrigin.isMatchingRegistrableDomainSuffix(options.rp.id)) {
if (!options.rp.id)
options.rp.id = callerOrigin.domain();
else if (!callerOrigin.isMatchingRegistrableDomainSuffix(*options.rp.id)) {
promise.reject(Exception { SecurityError, "The provided RP ID is not a registrable domain suffix of the effective domain of the document."_s });
return;
}
if (options.rp.id.isEmpty())
options.rp.id = callerOrigin.domain();

// Step 9-11.
// Most of the jobs are done by bindings.
@@ -161,7 +161,8 @@ void AuthenticatorCoordinator::create(const Document& document, const PublicKeyC

// Step 12-13.
// Only Google Legacy AppID Support Extension is supported.
options.extensions = AuthenticationExtensionsClientInputs { String(), processGoogleLegacyAppIdSupportExtension(options.extensions, options.rp.id), options.extensions && options.extensions->credProps };
ASSERT(options.rp.id);
options.extensions = AuthenticationExtensionsClientInputs { String(), processGoogleLegacyAppIdSupportExtension(options.extensions, *options.rp.id), options.extensions && options.extensions->credProps };

// Step 14-16.
auto clientDataJson = buildClientDataJson(ClientDataType::Create, options.challenge, callerOrigin, scope);
@@ -48,7 +48,7 @@ struct PublicKeyCredentialCreationOptions {
};

struct RpEntity : public Entity {
mutable String id;
mutable std::optional<String> id;
};

struct UserEntity : public Entity {
@@ -50,8 +50,8 @@ static CBORValue convertRpEntityToCBOR(const PublicKeyCredentialCreationOptions:
rpMap.emplace(CBORValue(kEntityNameMapKey), CBORValue(rpEntity.name));
if (!rpEntity.icon.isEmpty())
rpMap.emplace(CBORValue(kIconUrlMapKey), CBORValue(rpEntity.icon));
if (!rpEntity.id.isEmpty())
rpMap.emplace(CBORValue(kEntityIdMapKey), CBORValue(rpEntity.id));
if (rpEntity.id && !rpEntity.id->isEmpty())
rpMap.emplace(CBORValue(kEntityIdMapKey), CBORValue(*rpEntity.id));

return CBORValue(WTFMove(rpMap));
}
@@ -103,15 +103,17 @@ std::optional<Vector<uint8_t>> convertToU2fRegisterCommand(const Vector<uint8_t>
return std::nullopt;

auto appId = processGoogleLegacyAppIdSupportExtension(request.extensions);
return constructU2fRegisterCommand(produceRpIdHash(!appId ? request.rp.id : appId), clientDataHash);
ASSERT(request.rp.id);
return constructU2fRegisterCommand(produceRpIdHash(!appId ? *request.rp.id : appId), clientDataHash);
}

std::optional<Vector<uint8_t>> convertToU2fCheckOnlySignCommand(const Vector<uint8_t>& clientDataHash, const PublicKeyCredentialCreationOptions& request, const PublicKeyCredentialDescriptor& keyHandle)
{
if (keyHandle.type != PublicKeyCredentialType::PublicKey)
return std::nullopt;

return constructU2fSignCommand(produceRpIdHash(request.rp.id), clientDataHash, keyHandle.id, true /* checkOnly */);
ASSERT(request.rp.id);
return constructU2fSignCommand(produceRpIdHash(*request.rp.id), clientDataHash, keyHandle.id, true /* checkOnly */);
}

std::optional<Vector<uint8_t>> convertToU2fSignCommand(const Vector<uint8_t>& clientDataHash, const PublicKeyCredentialRequestOptions& request, const WebCore::BufferSource& keyHandle, bool isAppId)
@@ -650,6 +650,7 @@ - (void)cancel
WebCore::PublicKeyCredentialCreationOptions::RpEntity result;
result.name = rpEntity.name;
result.icon = rpEntity.icon;
ASSERT(rpEntity.identifier);
result.id = rpEntity.identifier;

return result;
@@ -157,8 +157,11 @@ static void processGoogleLegacyAppIdSupportExtension(const std::optional<Authent

static String getRpId(const std::variant<PublicKeyCredentialCreationOptions, PublicKeyCredentialRequestOptions>& options)
{
if (std::holds_alternative<PublicKeyCredentialCreationOptions>(options))
return std::get<PublicKeyCredentialCreationOptions>(options).rp.id;
if (std::holds_alternative<PublicKeyCredentialCreationOptions>(options)) {
auto& creationOptions = std::get<PublicKeyCredentialCreationOptions>(options);
ASSERT(creationOptions.rp.id);
return *creationOptions.rp.id;
}
return std::get<PublicKeyCredentialRequestOptions>(options).rpId;
}

@@ -248,7 +248,8 @@ static inline uint8_t authDataFlags(ClientDataType type, LocalConnection::UserVe
}

// Step 3.
auto existingCredentials = getExistingCredentials(creationOptions.rp.id);
ASSERT(creationOptions.rp.id);
auto existingCredentials = getExistingCredentials(*creationOptions.rp.id);
if (!existingCredentials) {
receiveException({ UnknownError, makeString("Couldn't get existing credentials") });
return;
@@ -336,7 +337,8 @@ static inline uint8_t authDataFlags(ClientDataType type, LocalConnection::UserVe
// kSecAttrApplicationTag: { "id": UserEntity.id, "name": UserEntity.name, "displayName": UserEntity.name} (CBOR encoded)
// Noted, the vale of kSecAttrApplicationLabel is automatically generated by the Keychain, which is a SHA-1 hash of
// the public key.
const auto& secAttrLabel = creationOptions.rp.id;
ASSERT(creationOptions.rp.id);
const auto& secAttrLabel = *creationOptions.rp.id;

// id, name, and displayName are required in PublicKeyCredentialUserEntity
// https://www.w3.org/TR/webauthn-2/#dictdef-publickeycredentialuserentity
@@ -414,7 +416,7 @@ static inline uint8_t authDataFlags(ClientDataType type, LocalConnection::UserVe
if (creationOptions.attestation == AttestationConveyancePreference::None) {
deleteDuplicateCredential();

auto authData = buildAuthData(creationOptions.rp.id, flags, counter, buildAttestedCredentialData(Vector<uint8_t>(aaguidLength, 0), credentialId, cosePublicKey));
auto authData = buildAuthData(*creationOptions.rp.id, flags, counter, buildAttestedCredentialData(Vector<uint8_t>(aaguidLength, 0), credentialId, cosePublicKey));
auto attestationObject = buildAttestationObject(WTFMove(authData), String { emptyString() }, { }, AttestationConveyancePreference::None);
auto response = AuthenticatorAttestationResponse::create(credentialId, attestationObject, AuthenticatorAttachment::Platform, transports());
processClientExtensions(response);
@@ -423,7 +425,7 @@ static inline uint8_t authDataFlags(ClientDataType type, LocalConnection::UserVe
}

// Step 13. Apple Attestation
auto authData = buildAuthData(creationOptions.rp.id, flags, counter, buildAttestedCredentialData(aaguidVector(), credentialId, cosePublicKey));
auto authData = buildAuthData(*creationOptions.rp.id, flags, counter, buildAttestedCredentialData(aaguidVector(), credentialId, cosePublicKey));
auto nsAuthData = toNSData(authData);
auto callback = [credentialId = WTFMove(credentialId), authData = WTFMove(authData), weakThis = WeakPtr { *this }] (NSArray * _Nullable certificates, NSError * _Nullable error) mutable {
ASSERT(RunLoop::isMain());
@@ -226,7 +226,8 @@ static inline ASPublicKeyCredentialResidentKeyPreference toASCResidentKeyPrefere
requestTypes &= ~ASCCredentialRequestTypePlatformPublicKeyRegistration;

auto requestContext = adoptNS([allocASCCredentialRequestContextInstance() initWithRequestTypes:requestTypes]);
[requestContext setRelyingPartyIdentifier:options.rp.id];
ASSERT(options.rp.id);
[requestContext setRelyingPartyIdentifier:*options.rp.id];
setGlobalFrameIDForContext(requestContext, globalFrameID);

auto credentialCreationOptions = adoptNS([allocASCPublicKeyCredentialCreationOptionsInstance() init]);
@@ -235,7 +236,7 @@ static inline ASPublicKeyCredentialResidentKeyPreference toASCResidentKeyPrefere
[credentialCreationOptions setClientDataHash:toNSData(hash).get()];
else
[credentialCreationOptions setChallenge:WebCore::toNSData(options.challenge).get()];
[credentialCreationOptions setRelyingPartyIdentifier:options.rp.id];
[credentialCreationOptions setRelyingPartyIdentifier:*options.rp.id];
[credentialCreationOptions setUserName:options.user.name];
[credentialCreationOptions setUserIdentifier:WebCore::toNSData(options.user.id).get()];
[credentialCreationOptions setUserDisplayName:options.user.displayName];
@@ -157,7 +157,8 @@ void U2fAuthenticator::continueRegisterCommandAfterResponseReceived(ApduResponse
case ApduResponse::Status::SW_NO_ERROR: {
auto& options = std::get<PublicKeyCredentialCreationOptions>(requestData().options);
auto appId = processGoogleLegacyAppIdSupportExtension(options.extensions);
auto response = readU2fRegisterResponse(!appId ? options.rp.id : appId, apduResponse.data(), AuthenticatorAttachment::CrossPlatform, { driver().transport() }, options.attestation);
ASSERT(options.rp.id);
auto response = readU2fRegisterResponse(!appId ? *options.rp.id : appId, apduResponse.data(), AuthenticatorAttachment::CrossPlatform, { driver().transport() }, options.attestation);
if (!response) {
receiveRespond(ExceptionData { UnknownError, "Couldn't parse the U2F register response."_s });
return;
@@ -1490,7 +1490,7 @@ HTTPServer server([parentFrame = String(parentFrame), subFrame = String(subFrame

EXPECT_WK_STREQ(result.rp.name, "example.com");
EXPECT_TRUE(result.rp.icon.isNull());
EXPECT_TRUE(result.rp.id.isNull());
EXPECT_TRUE(!result.rp.id);

EXPECT_WK_STREQ(result.user.name, "jappleseed@example.com");
EXPECT_TRUE(result.user.icon.isNull());
@@ -1534,7 +1534,7 @@ HTTPServer server([parentFrame = String(parentFrame), subFrame = String(subFrame

EXPECT_WK_STREQ(result.rp.name, "example.com");
EXPECT_TRUE(result.rp.icon.isNull());
EXPECT_TRUE(result.rp.id.isNull());
EXPECT_TRUE(!result.rp.id);

EXPECT_WK_STREQ(result.user.name, "jappleseed@example.com");
EXPECT_TRUE(result.user.icon.isNull());
@@ -1601,7 +1601,7 @@ HTTPServer server([parentFrame = String(parentFrame), subFrame = String(subFrame

EXPECT_WK_STREQ(result.rp.name, "example.com");
EXPECT_WK_STREQ(result.rp.icon, @"https//www.example.com/icon.jpg");
EXPECT_WK_STREQ(result.rp.id, "example.com");
EXPECT_WK_STREQ(*result.rp.id, "example.com");

EXPECT_WK_STREQ(result.user.name, "jappleseed@example.com");
EXPECT_WK_STREQ(result.user.icon, @"https//www.example.com/icon.jpg");
@@ -1671,7 +1671,7 @@ HTTPServer server([parentFrame = String(parentFrame), subFrame = String(subFrame

EXPECT_WK_STREQ(result.rp.name, "example.com");
EXPECT_WK_STREQ(result.rp.icon, @"https//www.example.com/icon.jpg");
EXPECT_WK_STREQ(result.rp.id, "example.com");
EXPECT_WK_STREQ(*result.rp.id, "example.com");

EXPECT_WK_STREQ(result.user.name, "jappleseed@example.com");
EXPECT_WK_STREQ(result.user.icon, @"https//www.example.com/icon.jpg");

0 comments on commit 3b920f8

Please sign in to comment.