Commit 7da94ee9 authored by peter's avatar peter Committed by Commit bot

Disallow identical names for the Encryption and Crypto-Key headers

This is to be enforced per the following spec change:
https://github.com/martinthomson/http-encryption/commit/d11c8263ef3a9fa9cec261898b5d462162045099

BUG=538576

Review-Url: https://codereview.chromium.org/2106333003
Cr-Commit-Position: refs/heads/master@{#403352}
parent 24e3e377
...@@ -75,6 +75,10 @@ bool EncryptionHeaderIterator::GetNext() { ...@@ -75,6 +75,10 @@ bool EncryptionHeaderIterator::GetNext() {
net::HttpUtil::NameValuePairsIterator::Values::REQUIRED, net::HttpUtil::NameValuePairsIterator::Values::REQUIRED,
net::HttpUtil::NameValuePairsIterator::Quotes::NOT_STRICT); net::HttpUtil::NameValuePairsIterator::Quotes::NOT_STRICT);
bool found_keyid = false;
bool found_salt = false;
bool found_rs = false;
while (name_value_pairs.GetNext()) { while (name_value_pairs.GetNext()) {
const base::StringPiece name(name_value_pairs.name_begin(), const base::StringPiece name(name_value_pairs.name_begin(),
name_value_pairs.name_end()); name_value_pairs.name_end());
...@@ -82,13 +86,18 @@ bool EncryptionHeaderIterator::GetNext() { ...@@ -82,13 +86,18 @@ bool EncryptionHeaderIterator::GetNext() {
name_value_pairs.value_end()); name_value_pairs.value_end());
if (base::LowerCaseEqualsASCII(name, "keyid")) { if (base::LowerCaseEqualsASCII(name, "keyid")) {
if (found_keyid)
return false;
value.CopyToString(&keyid_); value.CopyToString(&keyid_);
found_keyid = true;
} else if (base::LowerCaseEqualsASCII(name, "salt")) { } else if (base::LowerCaseEqualsASCII(name, "salt")) {
if (!ValueToDecodedString(value, &salt_)) if (found_salt || !ValueToDecodedString(value, &salt_))
return false; return false;
found_salt = true;
} else if (base::LowerCaseEqualsASCII(name, "rs")) { } else if (base::LowerCaseEqualsASCII(name, "rs")) {
if (!RecordSizeToInt(value, &rs_)) if (found_rs || !RecordSizeToInt(value, &rs_))
return false; return false;
found_rs = true;
} else { } else {
// Silently ignore unknown directives for forward compatibility. // Silently ignore unknown directives for forward compatibility.
} }
...@@ -117,6 +126,10 @@ bool CryptoKeyHeaderIterator::GetNext() { ...@@ -117,6 +126,10 @@ bool CryptoKeyHeaderIterator::GetNext() {
net::HttpUtil::NameValuePairsIterator::Values::REQUIRED, net::HttpUtil::NameValuePairsIterator::Values::REQUIRED,
net::HttpUtil::NameValuePairsIterator::Quotes::NOT_STRICT); net::HttpUtil::NameValuePairsIterator::Quotes::NOT_STRICT);
bool found_keyid = false;
bool found_aesgcm128 = false;
bool found_dh = false;
while (name_value_pairs.GetNext()) { while (name_value_pairs.GetNext()) {
const base::StringPiece name(name_value_pairs.name_begin(), const base::StringPiece name(name_value_pairs.name_begin(),
name_value_pairs.name_end()); name_value_pairs.name_end());
...@@ -124,13 +137,18 @@ bool CryptoKeyHeaderIterator::GetNext() { ...@@ -124,13 +137,18 @@ bool CryptoKeyHeaderIterator::GetNext() {
name_value_pairs.value_end()); name_value_pairs.value_end());
if (base::LowerCaseEqualsASCII(name, "keyid")) { if (base::LowerCaseEqualsASCII(name, "keyid")) {
if (found_keyid)
return false;
value.CopyToString(&keyid_); value.CopyToString(&keyid_);
found_keyid = true;
} else if (base::LowerCaseEqualsASCII(name, "aesgcm128")) { } else if (base::LowerCaseEqualsASCII(name, "aesgcm128")) {
if (!ValueToDecodedString(value, &aesgcm128_)) if (found_aesgcm128 || !ValueToDecodedString(value, &aesgcm128_))
return false; return false;
found_aesgcm128 = true;
} else if (base::LowerCaseEqualsASCII(name, "dh")) { } else if (base::LowerCaseEqualsASCII(name, "dh")) {
if (!ValueToDecodedString(value, &dh_)) if (found_dh || !ValueToDecodedString(value, &dh_))
return false; return false;
found_dh = true;
} else { } else {
// Silently ignore unknown directives for forward compatibility. // Silently ignore unknown directives for forward compatibility.
} }
......
...@@ -42,7 +42,6 @@ TEST(EncryptionHeaderParsersTest, ParseValidEncryptionHeaders) { ...@@ -42,7 +42,6 @@ TEST(EncryptionHeaderParsersTest, ParseValidEncryptionHeaders) {
"", "sixteencoolbytes", kDefaultRecordSize }, "", "sixteencoolbytes", kDefaultRecordSize },
{ "rs=2048", "", "", 2048 }, { "rs=2048", "", "", 2048 },
{ "keyid=foo;someothervalue=1;rs=42", "foo", "", 42 }, { "keyid=foo;someothervalue=1;rs=42", "foo", "", 42 },
{ "keyid=foo;keyid=bar", "bar", "", kDefaultRecordSize },
}; };
for (size_t i = 0; i < arraysize(expected_results); i++) { for (size_t i = 0; i < arraysize(expected_results); i++) {
...@@ -116,6 +115,10 @@ TEST(EncryptionHeaderParsersTest, ParseInvalidEncryptionHeaders) { ...@@ -116,6 +115,10 @@ TEST(EncryptionHeaderParsersTest, ParseInvalidEncryptionHeaders) {
"rs", "rs",
"rs=", "rs=",
// Supplying the same name multiple times in the same value is invalid.
"keyid=foo;keyid=bar",
"keyid=foo;bar=baz;keyid=qux",
// The salt must be a URL-safe base64 decodable string. // The salt must be a URL-safe base64 decodable string.
"salt=YmV/2ZXJ-sMDA", "salt=YmV/2ZXJ-sMDA",
"salt=dHdlbHZlY29vbGJ5dGVz=====", "salt=dHdlbHZlY29vbGJ5dGVz=====",
...@@ -187,7 +190,6 @@ TEST(EncryptionHeaderParsersTest, ParseValidCryptoKeyHeaders) { ...@@ -187,7 +190,6 @@ TEST(EncryptionHeaderParsersTest, ParseValidCryptoKeyHeaders) {
{ "dh=dHdlbHZlY29vbGJ5dGVz", "", "", "twelvecoolbytes" }, { "dh=dHdlbHZlY29vbGJ5dGVz", "", "", "twelvecoolbytes" },
{ "keyid=foo;someothervalue=bar;aesgcm128=dHdlbHZlY29vbGJ5dGVz", { "keyid=foo;someothervalue=bar;aesgcm128=dHdlbHZlY29vbGJ5dGVz",
"foo", "twelvecoolbytes", "" }, "foo", "twelvecoolbytes", "" },
{ "keyid=foo;keyid=bar", "bar", "", "" },
}; };
for (size_t i = 0; i < arraysize(expected_results); i++) { for (size_t i = 0; i < arraysize(expected_results); i++) {
...@@ -262,6 +264,10 @@ TEST(EncryptionHeaderParsersTest, ParseInvalidCryptoKeyHeaders) { ...@@ -262,6 +264,10 @@ TEST(EncryptionHeaderParsersTest, ParseInvalidCryptoKeyHeaders) {
"dh", "dh",
"dh=", "dh=",
// Supplying the same name multiple times in the same value is invalid.
"keyid=foo;keyid=bar",
"keyid=foo;bar=baz;keyid=qux",
// The "aesgcm128" parameter must be a URL-safe base64 decodable string. // The "aesgcm128" parameter must be a URL-safe base64 decodable string.
"aesgcm128=123$xyz", "aesgcm128=123$xyz",
"aesgcm128=foobar;aesgcm128=123$xyz", "aesgcm128=foobar;aesgcm128=123$xyz",
......
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