Commit 051979a8 authored by Kim Paulhamus's avatar Kim Paulhamus Committed by Commit Bot

[webauthn] Fix crash when rp.id is empty string

The renderer now checks for empty as well as null strings.
Crashing if an empty string is presented to the browser-side is
technically correct, but it is now  caught as a SecurityError in the
renderer first.

Bug: 830855
Change-Id: Id904864b8f6d57e9e0eb342d287337ba96d368d4
Reviewed-on: https://chromium-review.googlesource.com/1020187
Commit-Queue: Kim Paulhamus <kpaulhamus@chromium.org>
Reviewed-by: default avatarBalazs Engedy <engedy@chromium.org>
Cr-Commit-Position: refs/heads/master@{#552764}
parent fdd88a62
......@@ -4055,7 +4055,7 @@ crbug.com/785955 http/tests/credentialmanager/tools/virtual-authenticator-enviro
# virtual simulation thereof (work in progress, see: https://crbug.com/785955).
crbug.com/826936 external/wpt/webauthn/createcredential-badargs-authnrselection.https.html [ Pass Timeout ]
crbug.com/826936 external/wpt/webauthn/createcredential-badargs-challenge.https.html [ Pass Timeout ]
crbug.com/826936 external/wpt/webauthn/createcredential-badargs-rp.https.html [ Crash ]
crbug.com/826936 external/wpt/webauthn/createcredential-badargs-rp.https.html [ Pass Timeout ]
crbug.com/826936 external/wpt/webauthn/createcredential-badargs-user.https.html [ Pass Timeout ]
crbug.com/826936 external/wpt/webauthn/createcredential-excludecredentials.https.html [ Pass Timeout ]
crbug.com/826936 external/wpt/webauthn/createcredential-extensions.https.html [ Pass Timeout ]
......
......@@ -154,6 +154,24 @@ promise_test(_ => {
});
}, "navigator.credentials.create() with missing rp.id");
promise_test(t => {
mockAuthenticator.reset();
mockAuthenticator.setDefaultsForSuccessfulMakeCredential();
var customMakeCredOptions = deepCopy(MAKE_CREDENTIAL_OPTIONS);
customMakeCredOptions.rp = { name: "Acme", id: ""};
return promise_rejects(t, "SecurityError",
navigator.credentials.create({publicKey : customMakeCredOptions}));
}, "navigator.credentials.create() with empty rp.id");
promise_test(t => {
mockAuthenticator.reset();
mockAuthenticator.setDefaultsForSuccessfulMakeCredential();
var customMakeCredOptions = deepCopy(MAKE_CREDENTIAL_OPTIONS);
customMakeCredOptions.rp = { name: "Acme", id: null};
return promise_rejects(t, "SecurityError",
navigator.credentials.create({publicKey : customMakeCredOptions}));
}, "navigator.credentials.create() with null rp.id");
promise_test(_ => {
mockAuthenticator.reset();
mockAuthenticator.setDefaultsForSuccessfulMakeCredential();
......
......@@ -13,8 +13,6 @@
const VALID_ORIGIN_RPID_PAIRS = [
{ 'origin': 'https://google.test:8443',
'rpId': 'google.test' },
{ 'origin': 'https://google.test:8443',
'rpId': '' },
{'origin': 'https://subdomain.example.test:8443',
'rpId': 'example.test' },
{'origin': 'https://subdomain.example.test:8443',
......@@ -49,6 +47,8 @@ const INVALID_ORIGIN_RPID_PAIRS = [
'rpId': String(0) },
{ 'origin': 'https://google.test:8443',
'rpId': 'test' },
{ 'origin': 'https://google.test:8443',
'rpId': '' },
];
for (let test of INVALID_ORIGIN_RPID_PAIRS) {
......
......@@ -12,7 +12,7 @@
<script>
if (document.location.host != "subdomain.example.test:8443") {
document.location = "https://subdomain.example.test:8443/credentialmanager/credentialscontainer-get-from-nested-frame.html";
document.location = "https://subdomain.example.test:8443/credentialmanager/credentialscontainer-create-with-virtual-authenticator.html";
promise_test(_ => new Promise(_ => {}), "Stall tests on the wrong host.");
}
......@@ -27,7 +27,6 @@ promise_test(async t => {
assert_equals(authenticators.length, 1);
let testAuthenticator = authenticators[0];
assert_true(await testAuthenticator.generateAndRegisterKey(ACCEPTABLE_CREDENTIAL_ID, "subdomain.example.test"));
let keys = await testAuthenticator.registeredKeys();
var customMakeCredOptions = deepCopy(MAKE_CREDENTIAL_OPTIONS);
customMakeCredOptions.excludeCredentials = [ACCEPTABLE_CREDENTIAL];
......@@ -35,6 +34,13 @@ promise_test(async t => {
navigator.credentials.create({ publicKey : customMakeCredOptions}));
}, "navigator.credentials.create() re-registration returns InvalidStateError");
promise_test(async t => {
var customMakeCredOptions = deepCopy(MAKE_CREDENTIAL_OPTIONS);
customMakeCredOptions.rp.id = "";
return promise_rejects(t, "SecurityError",
navigator.credentials.create({ publicKey : customMakeCredOptions}));
}, "navigator.credentials.create() with empty rpId returns SecurityError");
promise_test(t => {
return navigator.credentials.test.clearAuthenticators();
}, "Clean up testing environment.");
......
......@@ -14,8 +14,6 @@
const VALID_ORIGIN_RPID_PAIRS = [
{ 'origin': 'https://google.test:8443',
'rpId': 'google.test' },
{ 'origin': 'https://google.test:8443',
'rpId': '' },
{'origin': 'https://subdomain.example.test:8443',
'rpId': 'example.test' },
{'origin': 'https://subdomain.example.test:8443',
......@@ -49,6 +47,8 @@ const INVALID_ORIGIN_RPID_PAIRS = [
'rpId': String(0) },
{ 'origin': 'https://google.test:8443',
'rpId': 'test' },
{ 'origin': 'https://google.test:8443',
'rpId': '' },
];
for (let test of INVALID_ORIGIN_RPID_PAIRS) {
......
......@@ -12,7 +12,7 @@
<script>
if (document.location.host != "subdomain.example.test:8443") {
document.location = "https://subdomain.example.test:8443/credentialmanager/credentialscontainer-get-from-nested-frame.html";
document.location = "https://subdomain.example.test:8443/credentialmanager/credentialscontainer-get-with-virtual-authenticator.html";
promise_test(_ => new Promise(_ => {}), "Stall tests on the wrong host.");
}
......
......@@ -200,8 +200,9 @@ bool CheckPublicKeySecurityRequirements(ScriptPromiseResolver* resolver,
if (!relying_party_id.IsNull()) {
OriginAccessEntry access_entry(origin->Protocol(), relying_party_id,
blink::OriginAccessEntry::kAllowSubdomains);
if (access_entry.MatchesDomain(*origin) !=
blink::OriginAccessEntry::kMatchesOrigin) {
if (relying_party_id.IsEmpty() ||
access_entry.MatchesDomain(*origin) !=
blink::OriginAccessEntry::kMatchesOrigin) {
resolver->Reject(DOMException::Create(
kSecurityError,
"The relying party ID '" + relying_party_id +
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment