Commit 2e8eb55d authored by peter's avatar peter Committed by Commit bot

Ignore Crypto-Key header values that do not have "dh" values

This aligns Chrome with the latest change to the webpush-encryption
specification: https://github.com/webpush-wg/webpush-encryption/pull/7

BUG=

Review-Url: https://codereview.chromium.org/2114703002
Cr-Commit-Position: refs/heads/master@{#403264}
parent 9915862a
...@@ -150,7 +150,29 @@ void GCMEncryptionProvider::DecryptMessage( ...@@ -150,7 +150,29 @@ void GCMEncryptionProvider::DecryptMessage(
return; return;
} }
if (crypto_key_header_iterator.dh().empty()) { // Ignore values that don't include the "dh" property. When using VAPID, it is
// valid for the application server to supply multiple values.
while (crypto_key_header_iterator.dh().empty() &&
crypto_key_header_iterator.GetNext()) {}
bool valid_crypto_key_header = false;
std::string dh;
if (!crypto_key_header_iterator.dh().empty()) {
dh = crypto_key_header_iterator.dh();
valid_crypto_key_header = true;
// Guard against the "dh" property being included more than once.
while (crypto_key_header_iterator.GetNext()) {
if (crypto_key_header_iterator.dh().empty())
continue;
valid_crypto_key_header = false;
break;
}
}
if (!valid_crypto_key_header) {
DLOG(ERROR) << "Invalid values supplied in the Crypto-Key header"; DLOG(ERROR) << "Invalid values supplied in the Crypto-Key header";
callback.Run(DECRYPTION_RESULT_INVALID_CRYPTO_KEY_HEADER, callback.Run(DECRYPTION_RESULT_INVALID_CRYPTO_KEY_HEADER,
IncomingMessage()); IncomingMessage());
...@@ -164,8 +186,7 @@ void GCMEncryptionProvider::DecryptMessage( ...@@ -164,8 +186,7 @@ void GCMEncryptionProvider::DecryptMessage(
base::Bind(&GCMEncryptionProvider::DecryptMessageWithKey, base::Bind(&GCMEncryptionProvider::DecryptMessageWithKey,
weak_ptr_factory_.GetWeakPtr(), message, weak_ptr_factory_.GetWeakPtr(), message,
callback, encryption_header_iterator.salt(), callback, encryption_header_iterator.salt(),
crypto_key_header_iterator.dh(), dh, encryption_header_iterator.rs()));
encryption_header_iterator.rs()));
} }
void GCMEncryptionProvider::DidGetEncryptionInfo( void GCMEncryptionProvider::DidGetEncryptionInfo(
......
...@@ -40,7 +40,15 @@ const char kInvalidEncryptionHeader[] = "keyid"; ...@@ -40,7 +40,15 @@ const char kInvalidEncryptionHeader[] = "keyid";
const char kValidCryptoKeyHeader[] = const char kValidCryptoKeyHeader[] =
"keyid=foo;dh=BL_UGhfudEkXMUd4U4-D4nP5KHxKjQHsW6j88ybbehXM7fqi1OMFefDUEi0eJ" "keyid=foo;dh=BL_UGhfudEkXMUd4U4-D4nP5KHxKjQHsW6j88ybbehXM7fqi1OMFefDUEi0eJ"
"vsKfyVBWYkQjH-lSPJKxjAyslg"; "vsKfyVBWYkQjH-lSPJKxjAyslg";
const char kValidThreeValueCryptoKeyHeader[] =
"keyid=foo,keyid=bar,keyid=baz;dh=BL_UGhfudEkXMUd4U4-D4nP5KHxKjQHsW6j88ybbe"
"hXM7fqi1OMFefDUEi0eJvsKfyVBWYkQjH-lSPJKxjAyslg";
const char kInvalidCryptoKeyHeader[] = "keyid"; const char kInvalidCryptoKeyHeader[] = "keyid";
const char kInvalidThreeValueCryptoKeyHeader[] =
"keyid=foo,dh=BL_UGhfudEkXMUd4U4-D4nP5KHxKjQHsW6j88ybbehXM7fqi1OMFefDUEi0eJ"
"vsKfyVBWYkQjH-lSPJKxjAyslg,keyid=baz,dh=BL_UGhfudEkXMUd4U4-D4nP5KHxKjQHsW6"
"j88ybbehXM7fqi1OMFefDUEi0eJvsKfyVBWYkQjH-lSPJKxjAyslg";
} // namespace } // namespace
...@@ -204,7 +212,7 @@ TEST_F(GCMEncryptionProviderTest, VerifiesEncryptionHeaderParsing) { ...@@ -204,7 +212,7 @@ TEST_F(GCMEncryptionProviderTest, VerifiesEncryptionHeaderParsing) {
} }
TEST_F(GCMEncryptionProviderTest, VerifiesCryptoKeyHeaderParsing) { TEST_F(GCMEncryptionProviderTest, VerifiesCryptoKeyHeaderParsing) {
// The Encryption-Key header must be parsable and contain valid values. // The Crypto-Key header must be parsable and contain valid values.
// Note that this is more extensively tested in EncryptionHeaderParsersTest. // Note that this is more extensively tested in EncryptionHeaderParsersTest.
IncomingMessage invalid_message; IncomingMessage invalid_message;
...@@ -217,7 +225,7 @@ TEST_F(GCMEncryptionProviderTest, VerifiesCryptoKeyHeaderParsing) { ...@@ -217,7 +225,7 @@ TEST_F(GCMEncryptionProviderTest, VerifiesCryptoKeyHeaderParsing) {
decryption_result()); decryption_result());
IncomingMessage valid_message; IncomingMessage valid_message;
valid_message.data["encryption"] = kInvalidEncryptionHeader; valid_message.data["encryption"] = kValidEncryptionHeader;
valid_message.data["crypto-key"] = kValidCryptoKeyHeader; valid_message.data["crypto-key"] = kValidCryptoKeyHeader;
valid_message.raw_data = "foo"; valid_message.raw_data = "foo";
...@@ -226,6 +234,34 @@ TEST_F(GCMEncryptionProviderTest, VerifiesCryptoKeyHeaderParsing) { ...@@ -226,6 +234,34 @@ TEST_F(GCMEncryptionProviderTest, VerifiesCryptoKeyHeaderParsing) {
decryption_result()); decryption_result());
} }
TEST_F(GCMEncryptionProviderTest, VerifiesCryptoKeyHeaderParsingThirdValue) {
// The Crypto-Key header must be parsable and contain valid values, in which
// values will be ignored unless they contain a "dh" property.
IncomingMessage valid_message;
valid_message.data["encryption"] = kValidEncryptionHeader;
valid_message.data["crypto-key"] = kValidThreeValueCryptoKeyHeader;
valid_message.raw_data = "foo";
ASSERT_NO_FATAL_FAILURE(Decrypt(valid_message));
EXPECT_NE(GCMEncryptionProvider::DECRYPTION_RESULT_INVALID_CRYPTO_KEY_HEADER,
decryption_result());
}
TEST_F(GCMEncryptionProviderTest, VerifiesCryptoKeyHeaderSingleDhEntry) {
// The Crypto-Key header must include at most one value that contains the
// "dh" property. Having more than once occurrence is forbidden.
IncomingMessage valid_message;
valid_message.data["encryption"] = kValidEncryptionHeader;
valid_message.data["crypto-key"] = kInvalidThreeValueCryptoKeyHeader;
valid_message.raw_data = "foo";
ASSERT_NO_FATAL_FAILURE(Decrypt(valid_message));
EXPECT_EQ(GCMEncryptionProvider::DECRYPTION_RESULT_INVALID_CRYPTO_KEY_HEADER,
decryption_result());
}
TEST_F(GCMEncryptionProviderTest, VerifiesExistingKeys) { TEST_F(GCMEncryptionProviderTest, VerifiesExistingKeys) {
// When both headers are valid, the encryption keys still must be known to // When both headers are valid, the encryption keys still must be known to
// the GCM key store before the message can be decrypted. // the GCM key store before the message can be decrypted.
......
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