This change introduces a way to tie source address token keys to specific QUIC server configs

net/quic/crypto/crypto_secret_boxer.cc

 // Copyright (c) 2013 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 "net/quic/crypto/crypto_secret_boxer.h"
 
 #include "base/logging.h"
 #include "base/memory/scoped_ptr.h"
 #include "crypto/secure_hash.h"
 #include "crypto/sha2.h"

[wtc, 2014/04/07 18:38:24]

Please remove "crypto/secure_hash.h" and "crypto/sha2.h".

[ramant (doing other things), 2014/04/21 22:39:29]

Done.

+#include "net/quic/crypto/crypto_protocol.h"
+#include "net/quic/crypto/quic_decrypter.h"
+#include "net/quic/crypto/quic_encrypter.h"
 #include "net/quic/crypto/quic_random.h"
 
 using base::StringPiece;
 using std::string;
 
 namespace net {
 
 // Defined kKeySize for GetKeySize() and SetKey().
 static const size_t kKeySize = 16;
 
 // kBoxNonceSize contains the number of bytes of nonce that we use in each box.
-static const size_t kBoxNonceSize = 16;
+// TODO(rtenneti): Add support for kBoxNonceSize to be 16 bytes.

[wtc, 2014/04/07 18:38:24]

Please explain why we want to switch to an algorithm that allows us to use a
16-byte nonce size. You can copy the explanation from Adam's email.

[ramant (doing other things), 2014/04/21 22:39:29]

Done.

+static const size_t kBoxNonceSize = 12;
 
 // static
 size_t CryptoSecretBoxer::GetKeySize() { return kKeySize; }

[wtc, 2014/04/07 18:38:24]

We should ideally get the key size from QuicEncrypter::GetKeySize(). But this
may require changing this method to a non-static method. (It is not necessary to
make this change.)

[ramant (doing other things), 2014/04/21 22:39:29]

Acknowledged.

 
 void CryptoSecretBoxer::SetKey(StringPiece key) {
   DCHECK_EQ(static_cast<size_t>(kKeySize), key.size());
   key_ = key.as_string();
 }
 
-// TODO(rtenneti): Delete sha256 based code. Use Aes128Gcm12Encrypter to Box the
-// plaintext. This is temporary solution for tests to pass.
 string CryptoSecretBoxer::Box(QuicRandom* rand, StringPiece plaintext) const {
+  scoped_ptr<QuicEncrypter> encrypter(QuicEncrypter::Create(kAESG));

[wtc, 2014/04/07 18:38:24]

It seems that |encrypter| and |decrypter| should be data members and created in
the constructor. Then, SetKey can just invoke encrypter_->SetKey() and
decrypter_->SetKey(). The only downside I can see is that we may create
encrypter_ without ever using it, if Box() is never called. (Similarly for
decrypter_.)

[ramant (doing other things), 2014/04/21 22:39:29]

Will make this change in internal source code and will merge in chromium code.

[ramant (doing other things), 2014/04/22 01:09:09]

Done.
"The only downside I can see is that we may create encrypter_ without ever using
it, if Box() is never called.". 

BTW: It looks like Box is not always called (the following are couple of
examples).

https://code.google.com/p/chromium/codesearch#chromium/src/net/quic/crypto/qu...


https://code.google.com/p/chromium/codesearch#chromium/src/net/quic/crypto/qu...

[ramant (doing other things), 2014/04/22 01:35:15]

Please ignore this comment. This change is n/a in the internal source.

+  if (!encrypter->SetKey(key_)) {
+    DLOG(DFATAL) << "CryptoSecretBoxer's Encrypt failed.";

[wtc, 2014/04/07 18:38:24]

This log message should say "SetKey" instead of "Encrypt".

[ramant (doing other things), 2014/04/21 22:39:29]

Done.

+    return string();
+  }
+  size_t ciphertext_size = encrypter->GetCiphertextSize(plaintext.length());
+
   string ret;
-  const size_t len = kBoxNonceSize + plaintext.size() + crypto::kSHA256Length;
+  const size_t len = kBoxNonceSize + ciphertext_size;
   ret.resize(len);
   char* data = &ret[0];
 
   // Generate nonce.
   rand->RandBytes(data, kBoxNonceSize);
   memcpy(data + kBoxNonceSize, plaintext.data(), plaintext.size());
 
-  // Compute sha256 for nonce + plaintext.
-  scoped_ptr<crypto::SecureHash> sha256(crypto::SecureHash::Create(
-      crypto::SecureHash::SHA256));
-  sha256->Update(data, kBoxNonceSize + plaintext.size());
-  sha256->Finish(data + kBoxNonceSize + plaintext.size(),
-                 crypto::kSHA256Length);
+  if (!encrypter->Encrypt(string(data, kBoxNonceSize), StringPiece(), plaintext,

[wtc, 2014/04/07 18:38:24]

1. Change
    string(data, kBoxNonceSize)
to
    StringPiece(data, kBoxNonceSize)

2. Make the same change to line 80.

[ramant (doing other things), 2014/04/21 22:39:29]

Done.

+                          reinterpret_cast<unsigned char*>(
+                              data + kBoxNonceSize))) {
+    DLOG(DFATAL) << "CryptoSecretBoxer's Encrypt failed.";
+    return string();
+  }
 
   return ret;
 }
 
-// TODO(rtenneti): Delete sha256 based code. Use Aes128Gcm12Decrypter to Unbox
-// the plaintext. This is temporary solution for tests to pass.
 bool CryptoSecretBoxer::Unbox(StringPiece ciphertext,
                               string* out_storage,
                               StringPiece* out) const {
-  if (ciphertext.size() < kBoxNonceSize + crypto::kSHA256Length) {
+  if (ciphertext.size() < kBoxNonceSize) {
     return false;
   }
 
-  const size_t plaintext_len =
-      ciphertext.size() - kBoxNonceSize - crypto::kSHA256Length;
-  out_storage->resize(plaintext_len);
+  char nonce[kBoxNonceSize];
+  memcpy(nonce, ciphertext.data(), kBoxNonceSize);
+  ciphertext.remove_prefix(kBoxNonceSize);
+
+  size_t len = ciphertext.size();
+  out_storage->resize(len);
   char* data = const_cast<char*>(out_storage->data());
 
-  // Copy plaintext from ciphertext.
-  if (plaintext_len != 0) {
-    memcpy(data, ciphertext.data() + kBoxNonceSize, plaintext_len);
-  }
-
-  // Compute sha256 for nonce + plaintext.
-  scoped_ptr<crypto::SecureHash> sha256(crypto::SecureHash::Create(
-      crypto::SecureHash::SHA256));
-  sha256->Update(ciphertext.data(), ciphertext.size() - crypto::kSHA256Length);
-  char sha256_bytes[crypto::kSHA256Length];
-  sha256->Finish(sha256_bytes, sizeof(sha256_bytes));
-
-  // Verify sha256.
-  if (0 != memcmp(ciphertext.data() + ciphertext.size() - crypto::kSHA256Length,
-                  sha256_bytes, crypto::kSHA256Length)) {
+  scoped_ptr<QuicDecrypter> decrypter(QuicDecrypter::Create(kAESG));
+  decrypter->SetKey(key_);
+  if (!decrypter->Decrypt(string(nonce, kBoxNonceSize), StringPiece(),
+                          ciphertext, reinterpret_cast<unsigned char*>(data),
+                          &len)) {
     return false;
   }
 
-  out->set(data, plaintext_len);
+  out->set(data, len);
   return true;
 }
 
 }  // namespace net