Commit ceb92979 authored by Martin Kreichgauer's avatar Martin Kreichgauer Committed by Commit Bot

cryptotoken: check base64url decoding failures when proxying to WebAuthn

Cryptotoken used to tolerate challenges and key handles encoded in
either regular base64 or base64url, despite the spec only allowing
base64url. With the WebAuthenticationProxyCryptotoken feature enabled,
this is no longer the case: WebAuthn takes challenges and key handles as
raw byte inputs, and re-encodes them to base64url prior to signing.

Until now, if cryptotoken failed to base64url-decode a challenge for
keyHandle before sending it to WebAuthn, it would send an empty byte
sequence instead. Hence, if a non-compliant passed a challenge encoded
in regular base64, it would subsequently receive a response, but the
challenge field in the response would be empty and signature would be
over the empty challenge.

This change makes the failure to decode more explicit. If a challenge
(or keyHandle) fails to decode, cryptotoken will not forward the
request. Rather it will return a BAD_REQUEST error with a descriptive
error message.

Bug: 925738
Change-Id: I7e1d0bfca55765ccdfaf7b9762c8b79352fb2856
Reviewed-on: https://chromium-review.googlesource.com/c/1450400
Commit-Queue: Martin Kreichgauer <martinkr@google.com>
Reviewed-by: default avatarAdam Langley <agl@chromium.org>
Cr-Commit-Position: refs/heads/master@{#628508}
parent 82ef234c
...@@ -888,12 +888,29 @@ Enroller.prototype.doRegisterWebAuthnContinue_ = function( ...@@ -888,12 +888,29 @@ Enroller.prototype.doRegisterWebAuthnContinue_ = function(
const randomId = new Uint8Array(new ArrayBuffer(16)); const randomId = new Uint8Array(new ArrayBuffer(16));
crypto.getRandomValues(randomId); crypto.getRandomValues(randomId);
const decodedChallenge = B64_decode(challenge);
if (decodedChallenge.length == 0) {
this.notifyError_({
errorCode: ErrorCodes.BAD_REQUEST,
errorMessage: 'challenge must be base64url encoded',
});
return;
}
const excludeList = []; const excludeList = [];
for (let index = 0; index < request['signData'].length; index++) { for (let index = 0; index < request['signData'].length; index++) {
const element = request['signData'][index]; const element = request['signData'][index];
const decodedKeyHandle = B64_decode(element['keyHandle']);
if (decodedKeyHandle.length == 0) {
this.notifyError_({
errorCode: ErrorCodes.BAD_REQUEST,
errorMessage: 'keyHandle must be base64url encoded',
});
return;
}
excludeList.push({ excludeList.push({
type: 'public-key', type: 'public-key',
id: new Uint8Array(B64_decode(element['keyHandle'])).buffer, id: new Uint8Array(decodedKeyHandle).buffer,
transports: ['usb'], transports: ['usb'],
}); });
} }
...@@ -913,7 +930,7 @@ Enroller.prototype.doRegisterWebAuthnContinue_ = function( ...@@ -913,7 +930,7 @@ Enroller.prototype.doRegisterWebAuthnContinue_ = function(
displayName: this.sender_.origin, displayName: this.sender_.origin,
name: this.sender_.origin, name: this.sender_.origin,
}, },
challenge: new Uint8Array(B64_decode(challenge)).buffer, challenge: new Uint8Array(decodedChallenge).buffer,
pubKeyCredParams: [{ pubKeyCredParams: [{
type: 'public-key', type: 'public-key',
alg: -7, // ES-256 alg: -7, // ES-256
......
...@@ -495,11 +495,28 @@ Signer.prototype.doSignWebAuthn_ = function(encodedChallenges, challengeVal) { ...@@ -495,11 +495,28 @@ Signer.prototype.doSignWebAuthn_ = function(encodedChallenges, challengeVal) {
return false; return false;
} }
const decodedChallenge = B64_decode(challengeVal);
if (decodedChallenge.length == 0) {
this.notifyError_({
errorCode: ErrorCodes.BAD_REQUEST,
errorMessage: 'challenge must be base64url encoded',
});
return false;
}
const credentialList = []; const credentialList = [];
for (let i = 0; i < encodedChallenges.length; i++) { for (let i = 0; i < encodedChallenges.length; i++) {
const decodedKeyHandle = B64_decode(encodedChallenges[i]['keyHandle']);
if (decodedKeyHandle.length == 0) {
this.notifyError_({
errorCode: ErrorCodes.BAD_REQUEST,
errorMessage: 'keyHandle must be base64url encoded',
});
return false;
}
credentialList.push({ credentialList.push({
type: 'public-key', type: 'public-key',
id: new Uint8Array(B64_decode(encodedChallenges[i].keyHandle)).buffer, id: new Uint8Array(decodedKeyHandle).buffer,
}); });
} }
// App ID could be defined for each challenge or globally. // App ID could be defined for each challenge or globally.
...@@ -509,7 +526,7 @@ Signer.prototype.doSignWebAuthn_ = function(encodedChallenges, challengeVal) { ...@@ -509,7 +526,7 @@ Signer.prototype.doSignWebAuthn_ = function(encodedChallenges, challengeVal) {
const request = { const request = {
publicKey: { publicKey: {
challenge: new Uint8Array(B64_decode(challengeVal)).buffer, challenge: new Uint8Array(decodedChallenge).buffer,
timeout: this.timer_.millisecondsUntilExpired(), timeout: this.timer_.millisecondsUntilExpired(),
rpId: this.sender_.origin, rpId: this.sender_.origin,
allowCredentials: credentialList, allowCredentials: credentialList,
......
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