Commit 72d721e5 authored by Adam Langley's avatar Adam Langley

webauthn: only serialize CollectedClientData once.

This change causes |AuthenticatorImpl| to only serialize
|CollectedClientData| once for each request. Since the code then carries
it around as a string, the |CollectedClientData| object itself is never
used except to immediately call |SerializeToJson| on it. Thus
|CollectedClientData| is folded into a function on |AuthenticatorImpl| to
save two indirections.

Along the way, implement the changes to the Token Binding member[1] and
re-add the random hint to not template match[2] now that it doesn't
break things.

[1] https://github.com/w3c/webauthn/commit/a47fe1c4d53b123caa7abc76e9659b95dc1c1a16
[2] https://chromium.googlesource.com/chromium/src/+/eb234c8e7356f2239700c604ad7b3eda42aaac33

Bug: 814103
Change-Id: Ifa96f3eee11bcbf740c7adff43e092ce5361210a
Reviewed-on: https://chromium-review.googlesource.com/929807Reviewed-by: default avatarBalazs Engedy <engedy@chromium.org>
Cr-Commit-Position: refs/heads/master@{#538860}
parent 9aecd396
......@@ -2213,8 +2213,6 @@ jumbo_source_set("browser") {
# Most webauth code is non-Android
"webauth/authenticator_impl.cc",
"webauth/authenticator_impl.h",
"webauth/collected_client_data.cc",
"webauth/collected_client_data.h",
]
deps += [ "//third_party/flac" ]
}
......
......@@ -8,7 +8,10 @@
#include <utility>
#include <vector>
#include "base/base64url.h"
#include "base/json/json_writer.h"
#include "base/logging.h"
#include "base/rand_util.h"
#include "base/timer/timer.h"
#include "content/browser/bad_message.h"
#include "content/public/browser/content_browser_client.h"
......@@ -29,6 +32,11 @@
namespace content {
namespace client_data {
const char kCreateType[] = "webauthn.create";
const char kGetType[] = "webauthn.get";
} // namespace client_data
namespace {
constexpr int32_t kCoseEs256 = -7;
......@@ -113,11 +121,10 @@ std::vector<uint8_t> CreateAppId(const std::string& relying_party_id) {
}
webauth::mojom::MakeCredentialAuthenticatorResponsePtr
CreateMakeCredentialResponse(CollectedClientData client_data,
CreateMakeCredentialResponse(const std::string& client_data_json,
device::RegisterResponseData response_data) {
auto response = webauth::mojom::MakeCredentialAuthenticatorResponse::New();
auto common_info = webauth::mojom::CommonCredentialInfo::New();
std::string client_data_json = client_data.SerializeToJson();
common_info->client_data_json.assign(client_data_json.begin(),
client_data_json.end());
common_info->raw_id = response_data.raw_id();
......@@ -129,11 +136,10 @@ CreateMakeCredentialResponse(CollectedClientData client_data,
}
webauth::mojom::GetAssertionAuthenticatorResponsePtr CreateGetAssertionResponse(
CollectedClientData client_data,
const std::string& client_data_json,
device::SignResponseData response_data) {
auto response = webauth::mojom::GetAssertionAuthenticatorResponse::New();
auto common_info = webauth::mojom::CommonCredentialInfo::New();
std::string client_data_json = client_data.SerializeToJson();
common_info->client_data_json.assign(client_data_json.begin(),
client_data_json.end());
common_info->raw_id = response_data.raw_id();
......@@ -145,6 +151,15 @@ webauth::mojom::GetAssertionAuthenticatorResponsePtr CreateGetAssertionResponse(
return response;
}
std::string Base64UrlEncode(const base::span<const uint8_t> input) {
std::string ret;
base::Base64UrlEncode(
base::StringPiece(reinterpret_cast<const char*>(input.data()),
input.size()),
base::Base64UrlEncodePolicy::OMIT_PADDING, &ret);
return ret;
}
} // namespace
AuthenticatorImpl::AuthenticatorImpl(RenderFrameHost* render_frame_host)
......@@ -173,6 +188,61 @@ void AuthenticatorImpl::Bind(webauth::mojom::AuthenticatorRequest request) {
bindings_.AddBinding(this, std::move(request));
}
// static
std::string AuthenticatorImpl::SerializeCollectedClientDataToJson(
const std::string& type,
const url::Origin& origin,
base::span<const uint8_t> challenge,
base::Optional<base::span<const uint8_t>> token_binding) {
static constexpr char kTypeKey[] = "type";
static constexpr char kChallengeKey[] = "challenge";
static constexpr char kOriginKey[] = "origin";
static constexpr char kTokenBindingKey[] = "tokenBinding";
base::DictionaryValue client_data;
client_data.SetKey(kTypeKey, base::Value(type));
client_data.SetKey(kChallengeKey, base::Value(Base64UrlEncode(challenge)));
client_data.SetKey(kOriginKey, base::Value(origin.Serialize()));
base::DictionaryValue token_binding_dict;
static constexpr char kTokenBindingStatusKey[] = "status";
static constexpr char kTokenBindingIdKey[] = "id";
static constexpr char kTokenBindingSupportedStatus[] = "supported";
static constexpr char kTokenBindingNotSupportedStatus[] = "not-supported";
static constexpr char kTokenBindingPresentStatus[] = "present";
if (token_binding) {
if (token_binding->empty()) {
token_binding_dict.SetKey(kTokenBindingStatusKey,
base::Value(kTokenBindingSupportedStatus));
} else {
token_binding_dict.SetKey(kTokenBindingStatusKey,
base::Value(kTokenBindingPresentStatus));
token_binding_dict.SetKey(kTokenBindingIdKey,
base::Value(Base64UrlEncode(*token_binding)));
}
} else {
token_binding_dict.SetKey(kTokenBindingStatusKey,
base::Value(kTokenBindingNotSupportedStatus));
}
client_data.SetKey(kTokenBindingKey, std::move(token_binding_dict));
if (base::RandDouble() < 0.2) {
// An extra key is sometimes added to ensure that RPs do not make
// unreasonably specific assumptions about the clientData JSON. This is
// done in the fashion of
// https://tools.ietf.org/html/draft-davidben-tls-grease-01
client_data.SetKey("new_keys_may_be_added_here",
base::Value("do not compare clientDataJSON against a "
"template. See https://goo.gl/yabPex"));
}
std::string json;
base::JSONWriter::Write(client_data, &json);
return json;
}
// mojom::Authenticator
void AuthenticatorImpl::MakeCredential(
webauth::mojom::PublicKeyCredentialCreationOptionsPtr options,
......@@ -217,15 +287,6 @@ void AuthenticatorImpl::MakeCredential(
DCHECK(make_credential_response_callback_.is_null());
make_credential_response_callback_ = std::move(callback);
client_data_ = CollectedClientData::Create(client_data::kCreateType,
caller_origin.Serialize(),
std::move(options->challenge));
// SHA-256 hash of the JSON data structure.
std::vector<uint8_t> client_data_hash(crypto::kSHA256Length);
crypto::SHA256HashString(client_data_.SerializeToJson(),
client_data_hash.data(), client_data_hash.size());
timer_->Start(
FROM_HERE, options->adjusted_timeout,
base::Bind(&AuthenticatorImpl::OnTimeout, base::Unretained(this)));
......@@ -237,10 +298,13 @@ void AuthenticatorImpl::MakeCredential(
for (const auto& credential : options->exclude_credentials) {
registered_keys.push_back(credential->id);
}
// Save client data to return with the authenticator response.
client_data_ = CollectedClientData::Create(client_data::kCreateType,
caller_origin.Serialize(),
std::move(options->challenge));
// TODO(kpaulhamus): Fetch and add the Token Binding ID public key used to
// communicate with the origin.
client_data_json_ = SerializeCollectedClientDataToJson(
client_data::kCreateType, caller_origin, std::move(options->challenge),
base::nullopt);
const bool individual_attestation =
GetContentClient()
......@@ -259,7 +323,7 @@ void AuthenticatorImpl::MakeCredential(
// relying party (hence the name of the parameter).
u2f_request_ = device::U2fRegister::TryRegistration(
relying_party_id_, connector_, protocols_, registered_keys,
ConstructClientDataHash(client_data_.SerializeToJson()),
ConstructClientDataHash(client_data_json_),
CreateAppId(relying_party_id_), individual_attestation,
base::BindOnce(&AuthenticatorImpl::OnRegisterResponse,
weak_factory_.GetWeakPtr()));
......@@ -311,13 +375,15 @@ void AuthenticatorImpl::GetAssertion(
connector_ = ServiceManagerConnection::GetForProcess()->GetConnector();
// Save client data to return with the authenticator response.
client_data_ = CollectedClientData::Create(client_data::kGetType,
caller_origin.Serialize(),
std::move(options->challenge));
// TODO(kpaulhamus): Fetch and add the Token Binding ID public key used to
// communicate with the origin.
client_data_json_ = SerializeCollectedClientDataToJson(
client_data::kGetType, caller_origin, std::move(options->challenge),
base::nullopt);
u2f_request_ = device::U2fSign::TrySign(
options->relying_party_id, connector_, protocols_, handles,
ConstructClientDataHash(client_data_.SerializeToJson()),
ConstructClientDataHash(client_data_json_),
CreateAppId(options->relying_party_id),
base::BindOnce(&AuthenticatorImpl::OnSignResponse,
weak_factory_.GetWeakPtr()));
......@@ -365,7 +431,7 @@ void AuthenticatorImpl::OnRegisterResponse(
InvokeCallbackAndCleanup(
std::move(make_credential_response_callback_),
webauth::mojom::AuthenticatorStatus::SUCCESS,
CreateMakeCredentialResponse(std::move(client_data_),
CreateMakeCredentialResponse(std::move(client_data_json_),
std::move(*response_data)));
return;
}
......@@ -387,7 +453,7 @@ void AuthenticatorImpl::OnRegisterResponseAttestationDecided(
InvokeCallbackAndCleanup(
std::move(make_credential_response_callback_),
webauth::mojom::AuthenticatorStatus::SUCCESS,
CreateMakeCredentialResponse(std::move(client_data_),
CreateMakeCredentialResponse(std::move(client_data_json_),
std::move(response_data)));
}
}
......@@ -416,7 +482,7 @@ void AuthenticatorImpl::OnSignResponse(
InvokeCallbackAndCleanup(
std::move(get_assertion_response_callback_),
webauth::mojom::AuthenticatorStatus::SUCCESS,
CreateGetAssertionResponse(std::move(client_data_),
CreateGetAssertionResponse(std::move(client_data_json_),
std::move(*response_data)));
return;
}
......@@ -460,7 +526,7 @@ void AuthenticatorImpl::Cleanup() {
u2f_request_.reset();
make_credential_response_callback_.Reset();
get_assertion_response_callback_.Reset();
client_data_ = CollectedClientData();
client_data_json_.clear();
}
} // namespace content
......@@ -11,9 +11,9 @@
#include <string>
#include "base/containers/flat_set.h"
#include "base/containers/span.h"
#include "base/macros.h"
#include "base/optional.h"
#include "content/browser/webauth/collected_client_data.h"
#include "content/common/content_export.h"
#include "device/fido/register_response_data.h"
#include "device/fido/sign_response_data.h"
......@@ -35,10 +35,22 @@ namespace service_manager {
class Connector;
} // namespace service_manager
namespace url {
class Origin;
}
namespace content {
class RenderFrameHost;
namespace client_data {
// These enumerate the possible values for the `type` member of
// CollectedClientData. See
// https://w3c.github.io/webauthn/#dom-collectedclientdata-type
CONTENT_EXPORT extern const char kCreateType[];
CONTENT_EXPORT extern const char kGetType[];
} // namespace client_data
// Implementation of the public Authenticator interface.
class CONTENT_EXPORT AuthenticatorImpl : public webauth::mojom::Authenticator {
public:
......@@ -58,6 +70,17 @@ class CONTENT_EXPORT AuthenticatorImpl : public webauth::mojom::Authenticator {
void Bind(webauth::mojom::AuthenticatorRequest request);
private:
friend class AuthenticatorImplTest;
// Builds the CollectedClientData[1] dictionary with the given values,
// serializes it to JSON, and returns the resulting string.
// [1] https://w3c.github.io/webauthn/#dictdef-collectedclientdata
static std::string SerializeCollectedClientDataToJson(
const std::string& type,
const url::Origin& origin,
base::span<const uint8_t> challenge,
base::Optional<base::span<const uint8_t>> token_binding);
// mojom:Authenticator
void MakeCredential(
webauth::mojom::PublicKeyCredentialCreationOptionsPtr options,
......@@ -107,8 +130,8 @@ class CONTENT_EXPORT AuthenticatorImpl : public webauth::mojom::Authenticator {
MakeCredentialCallback make_credential_response_callback_;
GetAssertionCallback get_assertion_response_callback_;
// Holds the client data to be returned to the caller.
CollectedClientData client_data_;
// Holds the client data to be returned to the caller in JSON format.
std::string client_data_json_;
webauth::mojom::AttestationConveyancePreference attestation_preference_;
std::string relying_party_id_;
std::unique_ptr<base::OneShotTimer> timer_;
......
......@@ -17,7 +17,6 @@
#include "base/test/test_mock_time_task_runner.h"
#include "base/time/tick_clock.h"
#include "base/time/time.h"
#include "content/browser/webauth/collected_client_data.h"
#include "content/public/browser/render_frame_host.h"
#include "content/test/test_render_frame_host.h"
#include "device/fido/fake_hid_impl_for_testing.h"
......@@ -66,11 +65,13 @@ constexpr uint8_t kTestChallengeBytes[] = {
constexpr char kTestRegisterClientDataJsonString[] =
R"({"challenge":"aHE0loIi7BcgLkJQX47SsWriLxa7BbiMJdueYCZF8UE","origin":)"
R"("google.com","tokenBinding":"unused","type":"webauthn.create"})";
R"("https://a.google.com","tokenBinding":{"status":"not-supported"},)"
R"("type":"webauthn.create"})";
constexpr char kTestSignClientDataJsonString[] =
R"({"challenge":"aHE0loIi7BcgLkJQX47SsWriLxa7BbiMJdueYCZF8UE","origin":)"
R"("google.com","tokenBinding":"unused","type":"webauthn.get"})";
R"("https://a.google.com","tokenBinding":{"status":"not-supported"},)"
R"("type":"webauthn.get"})";
constexpr OriginRelyingPartyIdPair kValidRelyingPartyTestCases[] = {
{"http://localhost", "localhost"},
......@@ -212,45 +213,6 @@ GetTestPublicKeyCredentialRequestOptions() {
return options;
}
CollectedClientData GetTestClientData(std::string type) {
return CollectedClientData::Create(std::move(type), kTestRelyingPartyId,
GetTestChallengeBytes());
}
class AuthenticatorImplTest : public content::RenderViewHostTestHarness {
public:
AuthenticatorImplTest() {}
~AuthenticatorImplTest() override {}
protected:
// Simulates navigating to a page and getting the page contents and language
// for that navigation.
void SimulateNavigation(const GURL& url) {
if (main_rfh()->GetLastCommittedURL() != url)
NavigateAndCommit(url);
}
AuthenticatorPtr ConnectToAuthenticator() {
authenticator_impl_ = std::make_unique<AuthenticatorImpl>(main_rfh());
AuthenticatorPtr authenticator;
authenticator_impl_->Bind(mojo::MakeRequest(&authenticator));
return authenticator;
}
AuthenticatorPtr ConnectToAuthenticator(
service_manager::Connector* connector,
std::unique_ptr<base::OneShotTimer> timer) {
authenticator_impl_.reset(
new AuthenticatorImpl(main_rfh(), connector, std::move(timer)));
AuthenticatorPtr authenticator;
authenticator_impl_->Bind(mojo::MakeRequest(&authenticator));
return authenticator;
}
private:
std::unique_ptr<AuthenticatorImpl> authenticator_impl_;
};
class TestMakeCredentialCallback {
public:
TestMakeCredentialCallback()
......@@ -319,6 +281,59 @@ class TestGetAssertionCallback {
} // namespace
class AuthenticatorImplTest : public content::RenderViewHostTestHarness {
public:
AuthenticatorImplTest() {}
~AuthenticatorImplTest() override {}
protected:
// Simulates navigating to a page and getting the page contents and language
// for that navigation.
void SimulateNavigation(const GURL& url) {
if (main_rfh()->GetLastCommittedURL() != url)
NavigateAndCommit(url);
}
AuthenticatorPtr ConnectToAuthenticator() {
authenticator_impl_ = std::make_unique<AuthenticatorImpl>(main_rfh());
AuthenticatorPtr authenticator;
authenticator_impl_->Bind(mojo::MakeRequest(&authenticator));
return authenticator;
}
AuthenticatorPtr ConnectToAuthenticator(
service_manager::Connector* connector,
std::unique_ptr<base::OneShotTimer> timer) {
authenticator_impl_.reset(
new AuthenticatorImpl(main_rfh(), connector, std::move(timer)));
AuthenticatorPtr authenticator;
authenticator_impl_->Bind(mojo::MakeRequest(&authenticator));
return authenticator;
}
url::Origin GetTestOrigin() {
const GURL test_relying_party_url(kTestOrigin1);
CHECK(test_relying_party_url.is_valid());
return url::Origin::Create(test_relying_party_url);
}
std::string GetTestClientDataJSON(std::string type) {
return AuthenticatorImpl::SerializeCollectedClientDataToJson(
std::move(type), GetTestOrigin(), GetTestChallengeBytes(),
base::nullopt);
}
std::string GetTokenBindingTestClientDataJSON(
base::Optional<base::span<const uint8_t>> token_binding) {
return AuthenticatorImpl::SerializeCollectedClientDataToJson(
client_data::kGetType, GetTestOrigin(), GetTestChallengeBytes(),
token_binding);
}
private:
std::unique_ptr<AuthenticatorImpl> authenticator_impl_;
};
// Verify behavior for various combinations of origins and rp id's.
TEST_F(AuthenticatorImplTest, MakeCredentialOriginAndRpIds) {
// These instances should return security errors (for circumstances
......@@ -406,15 +421,36 @@ void CheckJSONIsSubsetOfJSON(base::StringPiece subset_str,
// Test that client data serializes to JSON properly.
TEST_F(AuthenticatorImplTest, TestSerializedRegisterClientData) {
CheckJSONIsSubsetOfJSON(
kTestRegisterClientDataJsonString,
GetTestClientData(client_data::kCreateType).SerializeToJson());
CheckJSONIsSubsetOfJSON(kTestRegisterClientDataJsonString,
GetTestClientDataJSON(client_data::kCreateType));
}
TEST_F(AuthenticatorImplTest, TestSerializedSignClientData) {
CheckJSONIsSubsetOfJSON(
kTestSignClientDataJsonString,
GetTestClientData(client_data::kGetType).SerializeToJson());
CheckJSONIsSubsetOfJSON(kTestSignClientDataJsonString,
GetTestClientDataJSON(client_data::kGetType));
}
TEST_F(AuthenticatorImplTest, TestTokenBindingClientData) {
const std::vector<
std::pair<base::Optional<std::vector<uint8_t>>, const char*>>
kTestCases = {
std::make_pair(base::nullopt,
R"({"tokenBinding":{"status":"not-supported"}})"),
std::make_pair(std::vector<uint8_t>{},
R"({"tokenBinding":{"status":"supported"}})"),
std::make_pair(
std::vector<uint8_t>{1, 2, 3, 4},
R"({"tokenBinding":{"status":"present","id":"AQIDBA"}})"),
};
for (const auto& test : kTestCases) {
const auto& token_binding = test.first;
const std::string expected_json_subset = test.second;
SCOPED_TRACE(expected_json_subset);
CheckJSONIsSubsetOfJSON(expected_json_subset,
GetTokenBindingTestClientDataJSON(token_binding));
}
}
TEST_F(AuthenticatorImplTest, TestMakeCredentialTimeout) {
......
// Copyright 2017 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 "content/browser/webauth/collected_client_data.h"
#include <utility>
#include "base/base64url.h"
#include "base/json/json_writer.h"
#include "base/rand_util.h"
#include "base/strings/string_piece.h"
#include "base/values.h"
namespace content {
namespace client_data {
const char kCreateType[] = "webauthn.create";
const char kGetType[] = "webauthn.get";
} // namespace client_data
namespace {
constexpr char kTypeKey[] = "type";
constexpr char kChallengeKey[] = "challenge";
constexpr char kOriginKey[] = "origin";
constexpr char kTokenBindingKey[] = "tokenBinding";
} // namespace
// static
CollectedClientData CollectedClientData::Create(
std::string type,
std::string relying_party_id,
base::span<const uint8_t> challenge) {
// The base64url encoding of options.challenge.
std::string encoded_challenge;
base::Base64UrlEncode(
base::StringPiece(reinterpret_cast<const char*>(challenge.data()),
challenge.size()),
base::Base64UrlEncodePolicy::OMIT_PADDING, &encoded_challenge);
// TokenBinding is present and set to the constant "unused" if the browser
// supports Token Binding, but is not using it to talk to the origin.
// TODO(kpaulhamus): Fetch and add the Token Binding ID public key used to
// communicate with the origin.
return CollectedClientData(std::move(type), std::move(encoded_challenge),
std::move(relying_party_id), "unused");
}
CollectedClientData::CollectedClientData() = default;
CollectedClientData::CollectedClientData(std::string type,
std::string base64_encoded_challenge,
std::string origin,
std::string token_binding_id)
: type_(std::move(type)),
base64_encoded_challenge_(std::move(base64_encoded_challenge)),
origin_(std::move(origin)),
token_binding_id_(std::move(token_binding_id)) {}
CollectedClientData::CollectedClientData(CollectedClientData&& other) = default;
CollectedClientData& CollectedClientData::operator=(
CollectedClientData&& other) = default;
CollectedClientData::~CollectedClientData() = default;
std::string CollectedClientData::SerializeToJson() const {
base::DictionaryValue client_data;
client_data.SetKey(kTypeKey, base::Value(type_));
client_data.SetKey(kChallengeKey, base::Value(base64_encoded_challenge_));
// The serialization of callerOrigin.
client_data.SetKey(kOriginKey, base::Value(origin_));
client_data.SetKey(kTokenBindingKey, base::Value(token_binding_id_));
std::string json;
base::JSONWriter::Write(client_data, &json);
return json;
}
} // namespace content
// Copyright 2017 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.
#ifndef CONTENT_BROWSER_WEBAUTH_COLLECTED_CLIENT_DATA_H_
#define CONTENT_BROWSER_WEBAUTH_COLLECTED_CLIENT_DATA_H_
#include <stdint.h>
#include <string>
#include "base/containers/span.h"
#include "base/macros.h"
#include "content/common/content_export.h"
namespace content {
namespace client_data {
CONTENT_EXPORT extern const char kCreateType[];
CONTENT_EXPORT extern const char kGetType[];
} // namespace client_data
// Represents the contextual bindings of both the Relying Party and the
// client platform that is used in authenticator signatures.
// https://www.w3.org/TR/2017/WD-webauthn-20170505/#dictdef-collectedclientdata
class CONTENT_EXPORT CollectedClientData {
public:
static CollectedClientData Create(std::string type,
std::string relying_party_id,
base::span<const uint8_t> challenge);
CollectedClientData();
CollectedClientData(std::string type_,
std::string base64_encoded_challenge_,
std::string origin,
std::string token_binding_id);
// Moveable.
CollectedClientData(CollectedClientData&& other);
CollectedClientData& operator=(CollectedClientData&& other);
~CollectedClientData();
// Builds a JSON-serialized string per step 13 of
// https://www.w3.org/TR/2017/WD-webauthn-20170505/#createCredential.
std::string SerializeToJson() const;
private:
std::string type_;
std::string base64_encoded_challenge_;
std::string origin_;
std::string token_binding_id_;
// TODO(kpaulhamus): Add extensions support. https://crbug/757502.
DISALLOW_COPY_AND_ASSIGN(CollectedClientData);
};
} // namespace content
#endif // CONTENT_BROWSER_WEBAUTH_COLLECTED_CLIENT_DATA_H_
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