Implement support for draft-ietf-webpush-encryption-08

components/gcm_driver/crypto/gcm_message_cryptographer.cc

Index: components/gcm_driver/crypto/gcm_message_cryptographer.cc
diff --git a/components/gcm_driver/crypto/gcm_message_cryptographer.cc b/components/gcm_driver/crypto/gcm_message_cryptographer.cc
index 8b53cbdfdd4d9e97fe2eb2644a8b887390bf8e26..acae09bf2b903c5338667dd8241a82acb93e14ff 100644
--- a/components/gcm_driver/crypto/gcm_message_cryptographer.cc
+++ b/components/gcm_driver/crypto/gcm_message_cryptographer.cc
@@ -51,6 +51,8 @@ class WebPushEncryptionDraft03
 
   // GCMMessageCryptographer::EncryptionScheme implementation.
   std::string DerivePseudoRandomKey(
+      const base::StringPiece& /* recipient_public_key */,
+      const base::StringPiece& /* sender_public_key */,
       const base::StringPiece& ecdh_shared_secret,
       const base::StringPiece& auth_secret) override {
     std::stringstream info_stream;
@@ -157,6 +159,98 @@ class WebPushEncryptionDraft03
   DISALLOW_COPY_AND_ASSIGN(WebPushEncryptionDraft03);
 };
 
+// Implementation of draft 08 of the Web Push Encryption standard:
+// https://tools.ietf.org/html/draft-ietf-webpush-encryption-08
+// https://tools.ietf.org/html/draft-ietf-httpbis-encryption-encoding-07
+class WebPushEncryptionDraft08
+    : public GCMMessageCryptographer::EncryptionScheme {
+ public:
+  WebPushEncryptionDraft08() = default;
+  ~WebPushEncryptionDraft08() override = default;
+
+  // GCMMessageCryptographer::EncryptionScheme implementation.
+  std::string DerivePseudoRandomKey(
+      const base::StringPiece& recipient_public_key,
+      const base::StringPiece& sender_public_key,
+      const base::StringPiece& ecdh_shared_secret,
+      const base::StringPiece& auth_secret) override {
+    std::stringstream info_stream;

[eroman, 2017/05/16 21:03:27]

same comment as on other CLs -- i don't think stringstream is the best way to do
appends.

[Peter Beverloo, 2017/05/18 17:10:21]

Done.

+    info_stream << "WebPush: info" << '\x00';
+    info_stream << recipient_public_key;
+    info_stream << sender_public_key;
+
+    const std::string info = info_stream.str();
+    DCHECK_EQ(info.size(), 144u);
+
+    crypto::HKDF hkdf(ecdh_shared_secret, auth_secret, info_stream.str(),
+                      32, /* key_bytes_to_generate */
+                      0,  /* iv_bytes_to_generate */
+                      0 /* subkey_secret_bytes_to_generate */);
+
+    return hkdf.client_write_key().as_string();
+  }
+
+  // The info string used for generating the content encryption key and the
+  // nonce was simplified in draft-ietf-webpush-encryption-08, because the
+  // public keys of both the recipient and the sender are now in the PRK.
+  std::string GenerateInfoForContentEncoding(
+      EncodingType type,
+      const base::StringPiece& /* recipient_public_key */,
+      const base::StringPiece& /* sender_public_key */) override {
+    std::stringstream info_stream;
+    info_stream << "Content-Encoding: ";
+
+    switch (type) {
+      case EncodingType::CONTENT_ENCRYPTION_KEY:
+        info_stream << "aes128gcm";
+        break;
+      case EncodingType::NONCE:
+        info_stream << "nonce";
+        break;
+    }
+
+    info_stream << '\x00';
+    return info_stream.str();
+  }
+
+  // draft-ietf-webpush-encryption-08 defines that the padding follows the
+  // plaintext of a message. A delimiter byte (0x02 for the final record) will
+  // be added, and then zero or more bytes of padding.
+  //
+  // TODO(peter): Add support for message padding if the GCMMessageCryptographer
+  // starts encrypting payloads for reasons other than testing.
+  std::string CreateRecord(const base::StringPiece& plaintext) override {
+    std::string record;
+    record.reserve(plaintext.size() + sizeof(uint8_t));
+    plaintext.AppendToString(&record);
+
+    record.append(sizeof(uint8_t), '\x02');
+    return record;
+  }
+
+  // The record padding in draft-ietf-webpush-encryption-08 is included at the
+  // end of the record. The length is not defined, but all padding bytes must be
+  // zero until the delimiter (0x02) is found.
+  bool ValidateAndRemovePadding(base::StringPiece& record) override {
+    size_t padding_length = 1;
+    for (; padding_length < record.size(); ++padding_length) {
+      size_t offset = record.size() - padding_length;
+
+      if (record[offset] == 0x02 /* padding delimiter octet */)
+        break;
+
+      if (record[offset] != 0x00 /* valid padding byte */)
+        return false;
+    }
+
+    record.remove_suffix(padding_length);

[eroman, 2017/05/16 21:03:27]

Is this correct in the case where we don't loop?

i.e. if record.size() <= 1, we will remove 1 byte from record, without actually
having checked its value.

Maybe it would be clearer to structure it so the stripping is only done after
finding 0x02, and falling-through outside the loop returns false.

[Peter Beverloo, 2017/05/18 17:10:21]

I added a DCHECK to verify that record.size() >= 1, since the ciphertext length
validation covers that. To make sure we cover the record.size() == 1 case I've
changed the "<" on line 236 to "<=".

+    return true;
+  }
+
+ private:
+  DISALLOW_COPY_AND_ASSIGN(WebPushEncryptionDraft08);
+};
+
 }  // namespace
 
 const size_t GCMMessageCryptographer::kAuthenticationTagBytes = 16;
@@ -167,6 +261,9 @@ GCMMessageCryptographer::GCMMessageCryptographer(Version version) {
     case Version::DRAFT_03:
       encryption_scheme_ = base::MakeUnique<WebPushEncryptionDraft03>();
       return;
+    case Version::DRAFT_08:
+      encryption_scheme_ = base::MakeUnique<WebPushEncryptionDraft08>();
+      return;
   }
 
   NOTREACHED();
@@ -192,7 +289,7 @@ bool GCMMessageCryptographer::Encrypt(
   DCHECK(ciphertext);
 
   std::string prk = encryption_scheme_->DerivePseudoRandomKey(
-      ecdh_shared_secret, auth_secret);
+      recipient_public_key, sender_public_key, ecdh_shared_secret, auth_secret);
 
   std::string content_encryption_key = DeriveContentEncryptionKey(
       recipient_public_key, sender_public_key, prk, salt);
@@ -235,7 +332,7 @@ bool GCMMessageCryptographer::Decrypt(
     return false;
 
   std::string prk = encryption_scheme_->DerivePseudoRandomKey(
-      ecdh_shared_secret, auth_secret);
+      recipient_public_key, sender_public_key, ecdh_shared_secret, auth_secret);
 
   std::string content_encryption_key = DeriveContentEncryptionKey(
       recipient_public_key, sender_public_key, prk, salt);