Skip to content

Commit

Permalink
Reland "webauthn: implement getPublicKey() and friends."
Browse files Browse the repository at this point in the history
This reverts commit e2c1b35.

https://chromium-review.googlesource.com/c/chromium/src/+/2238777 updates some Java tests with non-dummy data that should avoid causing problems.

Original change's description:
> Revert "webauthn: implement getPublicKey() and friends."
> 
> This reverts commit c5d2367.
> 
> Reason for revert: I suspect that this CL causes Fido2CredentialRequestTest failures on android-pie-x86-rel:
> https://ci.chromium.org/p/chromium/builders/ci/android-pie-x86-rel
> 
> Here is a snippet of callstack:
> java.lang.AssertionError: expected:<11> but was:<0>
> 	at org.junit.Assert.fail(Assert.java:88)
> 	at org.junit.Assert.failNotEquals(Assert.java:834)
> 	at org.junit.Assert.assertEquals(Assert.java:118)
> 	at org.junit.Assert.assertEquals(Assert.java:144)
> 	at org.chromium.chrome.browser.webauth.Fido2CredentialRequestTest.testMakeCredential_attestationIndirect(Fido2CredentialRequestTest.java:921)
> 
> Here is sample failure:
> https://00e9e64bac4dbda9c3b8a07931c85e48f4420a03929ffbcaf5-apidata.googleusercontent.com/download/storage/v1/b/chromium-result-details/o/html%2Fchrome_public_test_apk_android-pie-x86-rel_990_2020_06_09_T16_33_45-UTC?qk=AD5uMEvQvMK8K2ZyV-tHIDwdTtLEnQK-X1Mw0ohWLQeOpgogj0sVgm_7y9e5CdF-FMV6ObBmwjpVxD6Jh8_7VTv4z2FpOutuS8Gd6qQH5OxikfytS8cKFb_SRCCQXIgWq45BwD7DediOcKTF-UZea_JAoFuHO0ljO0PRqnkFYSoIHcPONuqaszXChp63b5o4VAKYsofAHDhpHru_hiAf6xUyfK91I8cr-2sdHjQU6Ks4XQIIWVnq2PSAiysd6hNeq51qWPEaX7HS8hCwWIpe55Xq9hIWLOtab3MY1WLLxM3UuWMY8IDCKfcvnrTiQMcRyeD7Iii33jHhC8-rDGttfDEl39kpFnWynDC-C0TmPFoCJVkl7mHxz0uCNlQJ3I0Rt_F2DTqA96qIKrpwEltNs96-0j1VgSOJCjU3s4od6h7juoIxpH5HThyg7G-fB1-MWzv7mQfAE5ZBilodU0CKMsFrnN_5Kf4thAbDj-RJUiUNpy0A9swLOhMLjEsfxQeDRldVvy9OVUNvcakhJziml5iA7LWoDB58E3eGTyUi-dA5aOLN3KWLR7T452j4MwnCHC5vTRNxxGxCkemgQQDkl1fHJas1u6GUcY4_ncvhIuGvLsfg8uqWpzFuau4CFamGC2USpyVpNIeNhpcf2w-lYzG1rkHjHWYgUKSvhoaItY3oeRFjnkYMgZrDsy8QmneuVVGTPd4nzoWWaCZbaG9ZaF80xRu9YhYnTUXI7GqQF1rlst09Mqu8IvDXo9dgYzodInIA6p3H7azZgnvCfzYUxL4QVrvskyjTQCITmz-o2W6CPUzBqWJhWLRTpZYcVSV7z8zjj1cFwDQPBKpJzQDRrtC1sZCM0WrmQ7oSNeU-AH3uBaNWeGBF2rkY8-9gx8dlr38DGQvXoKZVWHWi61jVnQhac27ctKBLKQ&isca=1
> 
> Original change's description:
> > webauthn: implement getPublicKey() and friends.
> > 
> > This change implements the getPublicKey(), getPublicKeyAlgorithm(), and
> > getAuthenticatorData() functions described in the editor's draft of
> > WebAuthn level two[1].
> > 
> > [1] https://w3c.github.io/webauthn/#sctn-public-key-easy
> > 
> > BUG=1083301
> > 
> > Change-Id: I3a03279d5239a9f8df50a78f2166f30d01d38d3e
> > Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2204087
> > Reviewed-by: Andrew Grieve <agrieve@chromium.org>
> > Reviewed-by: Ken Buchanan <kenrb@chromium.org>
> > Reviewed-by: Alex Russell <slightlyoff@chromium.org>
> > Reviewed-by: Jared Saul <jsaul@google.com>
> > Reviewed-by: Adam Langley <agl@chromium.org>
> > Reviewed-by: Nina Satragno <nsatragno@chromium.org>
> > Reviewed-by: Martin Kreichgauer <martinkr@google.com>
> > Commit-Queue: Adam Langley <agl@chromium.org>
> > Cr-Commit-Position: refs/heads/master@{#776517}
> 
> TBR=kenrb@chromium.org,agl@chromium.org,slightlyoff@chromium.org,nsatragno@chromium.org,agrieve@chromium.org,jsaul@google.com,martinkr@google.com
> 
> Change-Id: I7429b5b7ecd7fa38c4e1ec2905b80a15e7c64216
> No-Presubmit: true
> No-Tree-Checks: true
> No-Try: true
> Bug: 1083301
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2238749
> Reviewed-by: Pavel Yatsuk <pavely@chromium.org>
> Commit-Queue: Pavel Yatsuk <pavely@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#776637}

TBR=kenrb@chromium.org,agl@chromium.org,slightlyoff@chromium.org,pavely@chromium.org,nsatragno@chromium.org,agrieve@chromium.org,jsaul@google.com,martinkr@google.com

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug: 1083301
Change-Id: I9e3c8278dde4b61b7a45b6277f48fe018b3a8188
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2241341
Reviewed-by: Adam Langley <agl@chromium.org>
Commit-Queue: Adam Langley <agl@chromium.org>
Cr-Commit-Position: refs/heads/master@{#777424}
  • Loading branch information
Adam Langley authored and Commit Bot committed Jun 11, 2020
1 parent 5293d27 commit a09284e
Show file tree
Hide file tree
Showing 20 changed files with 333 additions and 139 deletions.
1 change: 1 addition & 0 deletions chrome/android/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -3066,6 +3066,7 @@ generate_jni("chrome_jni_headers") {
"java/src/org/chromium/chrome/browser/webapps/addtohomescreen/AddToHomescreenInstaller.java",
"java/src/org/chromium/chrome/browser/webapps/addtohomescreen/AddToHomescreenMediator.java",
"java/src/org/chromium/chrome/browser/webauth/AuthenticatorImpl.java",
"java/src/org/chromium/chrome/browser/webauth/Fido2Helper.java",
]

# Used for testing only, should not be shipped to end users.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -308,9 +308,13 @@ private void processPublicKeyCredential(Intent data) {
if (response instanceof AuthenticatorErrorResponse) {
processErrorResponse((AuthenticatorErrorResponse) response);
} else if (response instanceof AuthenticatorAttestationResponse) {
mCallback.onRegisterResponse(AuthenticatorStatus.SUCCESS,
Fido2Helper.toMakeCredentialResponse(publicKeyCredential));
mCallback = null;
try {
mCallback.onRegisterResponse(AuthenticatorStatus.SUCCESS,
Fido2Helper.toMakeCredentialResponse(publicKeyCredential));
mCallback = null;
} catch (NoSuchAlgorithmException e) {
returnErrorAndResetCallback(AuthenticatorStatus.ALGORITHM_UNSUPPORTED);
}
} else if (response instanceof AuthenticatorAssertionResponse) {
mCallback.onSignResponse(AuthenticatorStatus.SUCCESS,
Fido2Helper.toGetAssertionResponse(publicKeyCredential, mAppIdExtensionUsed));
Expand All @@ -331,10 +335,15 @@ private void processKeyResponse(Intent data) {
switch (mRequestStatus) {
case REGISTER_REQUEST:
Log.e(TAG, "Received a register response from Google Play Services FIDO2 API");
mCallback.onRegisterResponse(AuthenticatorStatus.SUCCESS,
Fido2Helper.toMakeCredentialResponse(
AuthenticatorAttestationResponse.deserializeFromBytes(
data.getByteArrayExtra(Fido.FIDO2_KEY_RESPONSE_EXTRA))));
try {
mCallback.onRegisterResponse(AuthenticatorStatus.SUCCESS,
Fido2Helper.toMakeCredentialResponse(
AuthenticatorAttestationResponse.deserializeFromBytes(
data.getByteArrayExtra(
Fido.FIDO2_KEY_RESPONSE_EXTRA))));
} catch (NoSuchAlgorithmException e) {
returnErrorAndResetCallback(AuthenticatorStatus.ALGORITHM_UNSUPPORTED);
}
break;
case SIGN_REQUEST:
Log.e(TAG, "Received a sign response from Google Play Services FIDO2 API");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,8 @@
import com.google.android.gms.fido.fido2.api.common.UvmEntries;

import org.chromium.base.Log;
import org.chromium.base.annotations.CalledByNative;
import org.chromium.base.annotations.NativeMethods;
import org.chromium.blink.mojom.AuthenticatorAttachment;
import org.chromium.blink.mojom.AuthenticatorStatus;
import org.chromium.blink.mojom.AuthenticatorTransport;
Expand Down Expand Up @@ -124,14 +126,25 @@ public static PublicKeyCredentialCreationOptions toMakeCredentialOptions(
* @return Response to be passed to the renderer.
*/
public static MakeCredentialAuthenticatorResponse toMakeCredentialResponse(
AuthenticatorAttestationResponse data) {
AuthenticatorAttestationResponse data) throws NoSuchAlgorithmException {
MakeCredentialAuthenticatorResponse response = new MakeCredentialAuthenticatorResponse();
CommonCredentialInfo info = new CommonCredentialInfo();

response.attestationObject = data.getAttestationObject();
AttestationObjectParts parts = new AttestationObjectParts();
if (!Fido2HelperJni.get().parseAttestationObject(response.attestationObject, parts)) {
// A failure to parse the attestation object is fatal to the request
// on desktop and so the same behavior is used here.
throw new NoSuchAlgorithmException();
}
response.publicKeyAlgo = parts.coseAlgorithm;
info.authenticatorData = parts.authenticatorData;
response.publicKeyDer = parts.spki;

// An empty transports array indicates that we don't have any
// information about the available transports.
response.transports = new int[] {};

info.id = encodeId(data.getKeyHandle());
info.rawId = data.getKeyHandle();
info.clientDataJson = data.getClientDataJSON();
Expand All @@ -140,7 +153,7 @@ public static MakeCredentialAuthenticatorResponse toMakeCredentialResponse(
}

public static MakeCredentialAuthenticatorResponse toMakeCredentialResponse(
PublicKeyCredential data) {
PublicKeyCredential data) throws NoSuchAlgorithmException {
MakeCredentialAuthenticatorResponse response =
toMakeCredentialResponse((AuthenticatorAttestationResponse) data.getResponse());
return response;
Expand Down Expand Up @@ -189,9 +202,9 @@ public static GetAssertionAuthenticatorResponse toGetAssertionResponse(
AuthenticatorAssertionResponse data, boolean appIdExtensionUsed) {
GetAssertionAuthenticatorResponse response = new GetAssertionAuthenticatorResponse();
CommonCredentialInfo info = new CommonCredentialInfo();
response.authenticatorData = data.getAuthenticatorData();
response.signature = data.getSignature();
response.echoAppidExtension = appIdExtensionUsed;
info.authenticatorData = data.getAuthenticatorData();
info.id = encodeId(data.getKeyHandle());
info.rawId = data.getKeyHandle();
info.clientDataJson = data.getClientDataJSON();
Expand Down Expand Up @@ -388,4 +401,29 @@ private static double adjustTimeout(TimeDelta timeout) {
Math.min(MAX_TIMEOUT_SECONDS,
TimeUnit.MICROSECONDS.toSeconds(timeout.microseconds)));
}

// AttestationObjectParts is used to group together the return values of
// |parseAttestationObject|, below.
public static final class AttestationObjectParts {
@CalledByNative("AttestationObjectParts")
void setAll(byte[] authenticatorData, byte[] spki, int coseAlgorithm) {
this.authenticatorData = authenticatorData;
this.spki = spki;
this.coseAlgorithm = coseAlgorithm;
}

public byte[] authenticatorData;
public byte[] spki;
public int coseAlgorithm;
}

@NativeMethods
interface Natives {
// parseAttestationObject parses a CTAP2 attestation[1] and extracts the
// parts that the browser provides via Javascript API [2].
//
// [1] https://www.w3.org/TR/webauthn/#attestation-object
// [2] https://w3c.github.io/webauthn/#sctn-public-key-easy
boolean parseAttestationObject(byte[] attestationObject, AttestationObjectParts result);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -390,7 +390,7 @@ public static void validateUserVerificationMethods(
* @param response The response from the Fido2 API.
*/
public static void validateGetAssertionResponse(GetAssertionAuthenticatorResponse response) {
Assert.assertArrayEquals(response.authenticatorData, TEST_AUTHENTICATOR_DATA);
Assert.assertArrayEquals(response.info.authenticatorData, TEST_AUTHENTICATOR_DATA);
Assert.assertArrayEquals(response.signature, TEST_SIGNATURE);
Assert.assertArrayEquals(response.info.rawId, TEST_KEY_HANDLE);
Assert.assertEquals(response.info.id, TEST_ENCODED_KEY_HANDLE);
Expand Down
1 change: 1 addition & 0 deletions chrome/browser/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -2794,6 +2794,7 @@ static_library("browser") {
"android/webapps/add_to_homescreen_params.h",
"android/webapps/webapp_registry.cc",
"android/webapps/webapp_registry.h",
"android/webauth/fido2helper_native_android.cc",
"autofill/accessory_controller.h",
"autofill/address_accessory_controller.h",
"autofill/address_accessory_controller_impl.cc",
Expand Down
77 changes: 77 additions & 0 deletions chrome/browser/android/webauth/fido2helper_native_android.cc
Original file line number Diff line number Diff line change
@@ -0,0 +1,77 @@
// Copyright 2020 The Chromium Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

#include <jni.h>

#include "base/android/jni_array.h"
#include "chrome/android/chrome_jni_headers/Fido2Helper_jni.h"
#include "components/cbor/reader.h"
#include "device/fido/attested_credential_data.h"
#include "device/fido/public_key.h"
#include "third_party/boringssl/src/include/openssl/bytestring.h"

using base::android::ScopedJavaLocalRef;
using base::android::ToJavaByteArray;

// Parses a CTAP2 attestation[1] and extracts the
// parts that the browser provides via Javascript API [2]. Called
// Fido2Helper.java when constructing the makeCredential reply.
//
// [1] https://www.w3.org/TR/webauthn/#attestation-object
// [2] https://w3c.github.io/webauthn/#sctn-public-key-easy
static jboolean JNI_Fido2Helper_ParseAttestationObject(
JNIEnv* env,
const base::android::JavaParamRef<jbyteArray>& jattestation_object_bytes,
const base::android::JavaParamRef<jobject>& out_result) {
std::vector<uint8_t> attestation_object_bytes;
JavaByteArrayToByteVector(env, jattestation_object_bytes,
&attestation_object_bytes);

base::Optional<cbor::Value> attestation_object =
cbor::Reader::Read(attestation_object_bytes);
if (!attestation_object || !attestation_object->is_map()) {
return false;
}

const cbor::Value::MapValue& map = attestation_object->GetMap();
// See https://www.w3.org/TR/webauthn/#generating-an-attestation-object
cbor::Value::MapValue::const_iterator it = map.find(cbor::Value("authData"));
if (it == map.end() || !it->second.is_bytestring()) {
return false;
}
const std::vector<uint8_t>& auth_data = it->second.GetBytestring();
// See https://www.w3.org/TR/webauthn/#sec-authenticator-data
CBS cbs;
CBS_init(&cbs, auth_data.data(), auth_data.size());
uint8_t flags;
if ( // RP ID hash.
!CBS_skip(&cbs, 32) || !CBS_get_u8(&cbs, &flags) ||
// Check AT flag is set.
((flags >> 6) & 1) == 0 ||
// Signature counter.
!CBS_skip(&cbs, 4)) {
return false;
}

const auto result = device::AttestedCredentialData::ConsumeFromCtapResponse(
base::span<const uint8_t>(CBS_data(&cbs), CBS_len(&cbs)));
if (!result) {
return false;
}

ScopedJavaLocalRef<jbyteArray> auth_data_java(
ToJavaByteArray(env, auth_data));

const device::PublicKey* pub_key = result->first.public_key();
const base::Optional<std::vector<uint8_t>>& der_bytes(pub_key->der_bytes);
ScopedJavaLocalRef<jbyteArray> spki_java;
if (der_bytes) {
spki_java.Reset(ToJavaByteArray(env, *der_bytes));
}

Java_AttestationObjectParts_setAll(env, out_result, auth_data_java, spki_java,
pub_key->algorithm);

return true;
}
Original file line number Diff line number Diff line change
Expand Up @@ -651,7 +651,7 @@ base::Value CreditCardFIDOAuthenticator::ParseAssertionResponse(
response.SetKey("credential_id",
BytesToBase64(assertion_response->info->raw_id));
response.SetKey("authenticator_data",
BytesToBase64(assertion_response->authenticator_data));
BytesToBase64(assertion_response->info->authenticator_data));
response.SetKey("client_data",
BytesToBase64(assertion_response->info->client_data_json));
response.SetKey("signature", BytesToBase64(assertion_response->signature));
Expand Down
17 changes: 16 additions & 1 deletion content/browser/webauth/authenticator_common.cc
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@
#include "device/fido/fido_transport_protocol.h"
#include "device/fido/get_assertion_request_handler.h"
#include "device/fido/make_credential_request_handler.h"
#include "device/fido/public_key.h"
#include "device/fido/public_key_credential_descriptor.h"
#include "device/fido/public_key_credential_params.h"
#include "net/base/registry_controlled_domains/registry_controlled_domain.h"
Expand Down Expand Up @@ -318,6 +319,9 @@ CreateMakeCredentialResponse(
auto common_info = blink::mojom::CommonCredentialInfo::New();
common_info->client_data_json.assign(client_data_json.begin(),
client_data_json.end());
common_info->authenticator_data = response_data.attestation_object()
.authenticator_data()
.SerializeToByteArray();
if (response_data.android_client_data_ext()) {
DCHECK(base::FeatureList::IsEnabled(device::kWebAuthPhoneSupport));
common_info->client_data_json = *response_data.android_client_data_ext();
Expand Down Expand Up @@ -373,6 +377,17 @@ CreateMakeCredentialResponse(
response->attestation_object =
response_data.GetCBOREncodedAttestationObject();

const device::PublicKey* public_key = response_data.attestation_object()
.authenticator_data()
.attested_data()
->public_key();
response->public_key_algo = public_key->algorithm;
const base::Optional<std::vector<uint8_t>>& public_key_der =
public_key->der_bytes;
if (public_key_der) {
response->public_key_der.emplace(public_key_der.value());
}

return response;
}

Expand All @@ -391,7 +406,7 @@ blink::mojom::GetAssertionAuthenticatorResponsePtr CreateGetAssertionResponse(
common_info->raw_id = response_data.raw_credential_id();
common_info->id = response_data.GetId();
response->info = std::move(common_info);
response->authenticator_data =
response->info->authenticator_data =
response_data.auth_data().SerializeToByteArray();
response->signature = response_data.signature();
if (echo_appid_extension) {
Expand Down
Loading

0 comments on commit a09284e

Please sign in to comment.