Skip to content

Commit

Permalink
webauthn: require that PRF credential IDs are in the allow list.
Browse files Browse the repository at this point in the history
See w3c/webauthn@3b83189

Bug: 1106961
Change-Id: I64c309d53b2f9f5b9b717d5e887713aad0e0cfa9
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4291988
Reviewed-by: Nina Satragno <nsatragno@chromium.org>
Auto-Submit: Adam Langley <agl@chromium.org>
Commit-Queue: Adam Langley <agl@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1115908}
  • Loading branch information
Adam Langley authored and Chromium LUCI CQ committed Mar 10, 2023
1 parent a4ca524 commit 152c5f8
Show file tree
Hide file tree
Showing 2 changed files with 53 additions and 35 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@
#include "third_party/blink/renderer/core/inspector/console_message.h"
#include "third_party/blink/renderer/core/page/frame_tree.h"
#include "third_party/blink/renderer/core/typed_arrays/dom_array_buffer.h"
#include "third_party/blink/renderer/core/typed_arrays/dom_array_piece.h"
#include "third_party/blink/renderer/modules/credentialmanagement/authenticator_assertion_response.h"
#include "third_party/blink/renderer/modules/credentialmanagement/authenticator_attestation_response.h"
#include "third_party/blink/renderer/modules/credentialmanagement/credential.h"
Expand Down Expand Up @@ -351,18 +352,9 @@ bool IsArrayBufferOrViewBelowSizeLimit(
const V8UnionArrayBufferOrArrayBufferView* buffer_or_view) {
if (!buffer_or_view)
return true;
switch (buffer_or_view->GetContentType()) {
case V8UnionArrayBufferOrArrayBufferView::ContentType::kArrayBuffer:
return base::CheckedNumeric<wtf_size_t>(
buffer_or_view->GetAsArrayBuffer()->ByteLength())
.IsValid();
case V8UnionArrayBufferOrArrayBufferView::ContentType::kArrayBufferView:
return base::CheckedNumeric<wtf_size_t>(
buffer_or_view->GetAsArrayBufferView()->byteLength())
.IsValid();
}
NOTREACHED();
return false;
return base::CheckedNumeric<wtf_size_t>(
DOMArrayPiece(buffer_or_view).ByteLength())
.IsValid();
}

bool IsCredentialDescriptorListBelowSizeLimit(
Expand Down Expand Up @@ -1035,7 +1027,21 @@ bool IsPaymentExtensionValid(const CredentialCreationOptions* options,
}

const char* validateGetPublicKeyCredentialPRFExtension(
const AuthenticationExtensionsPRFInputs& prf) {
const AuthenticationExtensionsPRFInputs& prf,
const HeapVector<Member<PublicKeyCredentialDescriptor>>&
allow_credentials) {
std::vector<base::span<const uint8_t>> cred_ids;
cred_ids.reserve(allow_credentials.size());
for (const auto cred : allow_credentials) {
DOMArrayPiece piece(cred->id());
cred_ids.emplace_back(piece.Bytes(), piece.ByteLength());
}
const auto compare = [](const base::span<const uint8_t>& a,
const base::span<const uint8_t>& b) -> bool {
return std::lexicographical_compare(a.begin(), a.end(), b.begin(), b.end());
};
std::sort(cred_ids.begin(), cred_ids.end(), compare);

if (prf.hasEvalByCredential()) {
for (const auto& pair : prf.evalByCredential()) {
Vector<char> cred_id;
Expand All @@ -1048,6 +1054,12 @@ const char* validateGetPublicKeyCredentialPRFExtension(
return "'prf' extension contains an empty credential ID in "
"'evalByCredential'";
}
if (!std::binary_search(cred_ids.begin(), cred_ids.end(),
base::as_bytes(base::make_span(cred_id)),
compare)) {
return "'prf' extension contains 'evalByCredential' key that doesn't "
"match any in allowedCredentials";
}
}
}
return nullptr;
Expand Down Expand Up @@ -1227,14 +1239,10 @@ ScriptPromise CredentialsContainer::get(ScriptState* script_state,
return promise;
}
if (options->publicKey()->extensions()->largeBlob()->hasWrite()) {
V8UnionArrayBufferOrArrayBufferView* blob =
options->publicKey()->extensions()->largeBlob()->write();
size_t write_size;
if (blob->IsArrayBufferView()) {
write_size = blob->GetAsArrayBufferView()->byteLength();
} else {
write_size = blob->GetAsArrayBuffer()->ByteLength();
}
const size_t write_size =
DOMArrayPiece(
options->publicKey()->extensions()->largeBlob()->write())
.ByteLength();
if (write_size > kMaxLargeBlobSize) {
resolver->Reject(MakeGarbageCollected<DOMException>(
DOMExceptionCode::kNotSupportedError,
Expand All @@ -1245,14 +1253,6 @@ ScriptPromise CredentialsContainer::get(ScriptState* script_state,
}
}
if (options->publicKey()->extensions()->hasPrf()) {
const char* error = validateGetPublicKeyCredentialPRFExtension(
*options->publicKey()->extensions()->prf());
if (error != nullptr) {
resolver->Reject(MakeGarbageCollected<DOMException>(
DOMExceptionCode::kSyntaxError, error));
return promise;
}

if (options->publicKey()->extensions()->prf()->hasEvalByCredential() &&
options->publicKey()->allowCredentials().empty()) {
resolver->Reject(MakeGarbageCollected<DOMException>(
Expand All @@ -1261,6 +1261,16 @@ ScriptPromise CredentialsContainer::get(ScriptState* script_state,
"list"));
return promise;
}

const char* error = validateGetPublicKeyCredentialPRFExtension(
*options->publicKey()->extensions()->prf(),
options->publicKey()->allowCredentials());
if (error != nullptr) {
resolver->Reject(MakeGarbageCollected<DOMException>(
DOMExceptionCode::kSyntaxError, error));
return promise;
}

// Prohibiting uv=preferred is omitted. See
// https://github.com/w3c/webauthn/pull/1836.
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -116,13 +116,21 @@
byCred["Zm9v"] = {
first: new Uint8Array([1,2,3,4]).buffer,
};
const assertion = await assert(id, {
return promise_rejects_dom(t, "SyntaxError", assert(id, {
evalByCredential: byCred,
});
assert_own_property(assertion.getClientExtensionResults(), 'prf');
assert_not_own_property(assertion.getClientExtensionResults().prf,
'results');
}, "navigator.credentials.get() using wrong credential ID");
}));
}, "navigator.credentials.get() with credential ID not in allowedCredentials");

promise_test(async t => {
const id = (await credential).rawId;
const byCred = {};
byCred["Zm9v"] = {
first: new Uint8Array([1,2,3,4]),
};
return promise_rejects_dom(t, "SyntaxError", assert(id, {
evalByCredential: byCred,
}));
}, "navigator.credentials.get() with Uint8Array credential ID not in allowedCredentials");

promise_test(async t => {
const id = (await credential).rawId;
Expand Down

0 comments on commit 152c5f8

Please sign in to comment.