Commit 96986fc8 authored by Adam Langley's avatar Adam Langley Committed by Commit Bot

Omit tokenBinding in ClientData if not supported.

https://github.com/w3c/webauthn/issues/907 notes that there are three
status values for tokenBinding in webauthn, including "not-supported",
but that tokenBinding itself is optional. Rather than having two ways to
indicate that it's not supported, we're going to eliminate the
"not-supported" status and represent that by omitting the value.

This matches with Firefox's behaviour. Spec change in
https://github.com/w3c/webauthn/pull/914.

Change-Id: If47bd218d61dc71eeddab3cc0d22da953b4cc370
Reviewed-on: https://chromium-review.googlesource.com/1062188Reviewed-by: default avatarBalazs Engedy <engedy@chromium.org>
Commit-Queue: Adam Langley <agl@chromium.org>
Cr-Commit-Position: refs/heads/master@{#561592}
parent d8e897e8
...@@ -400,14 +400,13 @@ std::string AuthenticatorImpl::SerializeCollectedClientDataToJson( ...@@ -400,14 +400,13 @@ std::string AuthenticatorImpl::SerializeCollectedClientDataToJson(
client_data.SetKey(kChallengeKey, base::Value(Base64UrlEncode(challenge))); client_data.SetKey(kChallengeKey, base::Value(Base64UrlEncode(challenge)));
client_data.SetKey(kOriginKey, base::Value(origin.Serialize())); 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) {
base::DictionaryValue token_binding_dict;
static constexpr char kTokenBindingStatusKey[] = "status";
static constexpr char kTokenBindingIdKey[] = "id";
static constexpr char kTokenBindingSupportedStatus[] = "supported";
static constexpr char kTokenBindingPresentStatus[] = "present";
if (token_binding->empty()) { if (token_binding->empty()) {
token_binding_dict.SetKey(kTokenBindingStatusKey, token_binding_dict.SetKey(kTokenBindingStatusKey,
base::Value(kTokenBindingSupportedStatus)); base::Value(kTokenBindingSupportedStatus));
...@@ -417,12 +416,9 @@ std::string AuthenticatorImpl::SerializeCollectedClientDataToJson( ...@@ -417,12 +416,9 @@ std::string AuthenticatorImpl::SerializeCollectedClientDataToJson(
token_binding_dict.SetKey(kTokenBindingIdKey, token_binding_dict.SetKey(kTokenBindingIdKey,
base::Value(Base64UrlEncode(*token_binding))); base::Value(Base64UrlEncode(*token_binding)));
} }
} else {
token_binding_dict.SetKey(kTokenBindingStatusKey,
base::Value(kTokenBindingNotSupportedStatus));
}
client_data.SetKey(kTokenBindingKey, std::move(token_binding_dict)); client_data.SetKey(kTokenBindingKey, std::move(token_binding_dict));
}
if (base::RandDouble() < 0.2) { if (base::RandDouble() < 0.2) {
// An extra key is sometimes added to ensure that RPs do not make // An extra key is sometimes added to ensure that RPs do not make
......
...@@ -80,13 +80,11 @@ constexpr uint8_t kTestChallengeBytes[] = { ...@@ -80,13 +80,11 @@ constexpr uint8_t kTestChallengeBytes[] = {
constexpr char kTestRegisterClientDataJsonString[] = constexpr char kTestRegisterClientDataJsonString[] =
R"({"challenge":"aHE0loIi7BcgLkJQX47SsWriLxa7BbiMJdueYCZF8UE","origin":)" R"({"challenge":"aHE0loIi7BcgLkJQX47SsWriLxa7BbiMJdueYCZF8UE","origin":)"
R"("https://a.google.com","tokenBinding":{"status":"not-supported"},)" R"("https://a.google.com", "type":"webauthn.create"})";
R"("type":"webauthn.create"})";
constexpr char kTestSignClientDataJsonString[] = constexpr char kTestSignClientDataJsonString[] =
R"({"challenge":"aHE0loIi7BcgLkJQX47SsWriLxa7BbiMJdueYCZF8UE","origin":)" R"({"challenge":"aHE0loIi7BcgLkJQX47SsWriLxa7BbiMJdueYCZF8UE","origin":)"
R"("https://a.google.com","tokenBinding":{"status":"not-supported"},)" R"("https://a.google.com", "type":"webauthn.get"})";
R"("type":"webauthn.get"})";
constexpr OriginClaimedAuthorityPair kValidRelyingPartyTestCases[] = { constexpr OriginClaimedAuthorityPair kValidRelyingPartyTestCases[] = {
{"http://localhost", "localhost"}, {"http://localhost", "localhost"},
...@@ -529,8 +527,7 @@ TEST_F(AuthenticatorImplTest, TestTokenBindingClientData) { ...@@ -529,8 +527,7 @@ TEST_F(AuthenticatorImplTest, TestTokenBindingClientData) {
const std::vector< const std::vector<
std::pair<base::Optional<std::vector<uint8_t>>, const char*>> std::pair<base::Optional<std::vector<uint8_t>>, const char*>>
kTestCases = { kTestCases = {
std::make_pair(base::nullopt, std::make_pair(base::nullopt, ""),
R"({"tokenBinding":{"status":"not-supported"}})"),
std::make_pair(std::vector<uint8_t>{}, std::make_pair(std::vector<uint8_t>{},
R"({"tokenBinding":{"status":"supported"}})"), R"({"tokenBinding":{"status":"supported"}})"),
std::make_pair( std::make_pair(
...@@ -542,9 +539,15 @@ TEST_F(AuthenticatorImplTest, TestTokenBindingClientData) { ...@@ -542,9 +539,15 @@ TEST_F(AuthenticatorImplTest, TestTokenBindingClientData) {
const auto& token_binding = test.first; const auto& token_binding = test.first;
const std::string expected_json_subset = test.second; const std::string expected_json_subset = test.second;
SCOPED_TRACE(expected_json_subset); SCOPED_TRACE(expected_json_subset);
const std::string client_data =
GetTokenBindingTestClientDataJSON(token_binding);
CheckJSONIsSubsetOfJSON(expected_json_subset, if (!expected_json_subset.empty()) {
GetTokenBindingTestClientDataJSON(token_binding)); CheckJSONIsSubsetOfJSON(expected_json_subset, client_data);
} else {
EXPECT_TRUE(client_data.find("tokenBinding") == std::string::npos)
<< client_data;
}
} }
} }
......
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