Change the QuicEncrypter to accept a pre-allocated buffer instead of

allocating it's own buffer.  No functional change.

Merge internal change: 85952670

R=rch@chromium.org

Review URL: https://codereview.chromium.org/912163002

Cr-Commit-Position: refs/heads/master@{#316320}

11 files changed, 124 insertions, 88 deletions

diff --git a/net/quic/crypto/aead_base_encrypter.h b/net/quic/crypto/aead_base_encrypter.h
index 7d9b6fc..8108744 100644
--- a/net/quic/crypto/aead_base_encrypter.h
+++ b/net/quic/crypto/aead_base_encrypter.h
@@ -46,9 +46,12 @@ class NET_EXPORT_PRIVATE AeadBaseEncrypter : public QuicEncrypter {
                base::StringPiece associated_data,
                base::StringPiece plaintext,
                unsigned char* output) override;
-  QuicData* EncryptPacket(QuicPacketSequenceNumber sequence_number,
-                          base::StringPiece associated_data,
-                          base::StringPiece plaintext) override;
+  bool EncryptPacket(QuicPacketSequenceNumber sequence_number,
+                     base::StringPiece associated_data,
+                     base::StringPiece plaintext,
+                     char* output,
+                     size_t* output_length,
+                     size_t max_output_length) override;
   size_t GetKeySize() const override;
   size_t GetNoncePrefixSize() const override;
   size_t GetMaxPlaintextSize(size_t ciphertext_size) const override;
diff --git a/net/quic/crypto/aead_base_encrypter_nss.cc b/net/quic/crypto/aead_base_encrypter_nss.cc
index 1e408ad..ca2e2a9 100644
--- a/net/quic/crypto/aead_base_encrypter_nss.cc
+++ b/net/quic/crypto/aead_base_encrypter_nss.cc
@@ -110,27 +110,28 @@ bool AeadBaseEncrypter::Encrypt(StringPiece nonce,
   return true;
 }
 
-QuicData* AeadBaseEncrypter::EncryptPacket(
-    QuicPacketSequenceNumber sequence_number,
-    StringPiece associated_data,
-    StringPiece plaintext) {
+bool AeadBaseEncrypter::EncryptPacket(QuicPacketSequenceNumber sequence_number,
+                                      StringPiece associated_data,
+                                      StringPiece plaintext,
+                                      char* output,
+                                      size_t* output_length,
+                                      size_t max_output_length) {
   size_t ciphertext_size = GetCiphertextSize(plaintext.length());
-  scoped_ptr<char[]> ciphertext(new char[ciphertext_size]);
-
+  if (max_output_length < ciphertext_size) {
+    return false;
+  }
   // TODO(ianswett): Introduce a check to ensure that we don't encrypt with the
   // same sequence number twice.
-  uint8 nonce[sizeof(nonce_prefix_) + sizeof(sequence_number)];
   const size_t nonce_size = nonce_prefix_size_ + sizeof(sequence_number);
-  DCHECK_LE(nonce_size, sizeof(nonce));
-  memcpy(nonce, nonce_prefix_, nonce_prefix_size_);
-  memcpy(nonce + nonce_prefix_size_, &sequence_number, sizeof(sequence_number));
-  if (!Encrypt(StringPiece(reinterpret_cast<char*>(nonce), nonce_size),
-               associated_data, plaintext,
-               reinterpret_cast<unsigned char*>(ciphertext.get()))) {
-    return nullptr;
+  memcpy(output, nonce_prefix_, nonce_prefix_size_);
+  memcpy(output + nonce_prefix_size_, &sequence_number,
+         sizeof(sequence_number));
+  if (!Encrypt(StringPiece(output, nonce_size), associated_data, plaintext,
+               reinterpret_cast<unsigned char*>(output))) {
+    return false;
   }
-
-  return new QuicData(ciphertext.release(), ciphertext_size, true);
+  *output_length = ciphertext_size;
+  return true;
 }
 
 size_t AeadBaseEncrypter::GetKeySize() const { return key_size_; }
diff --git a/net/quic/crypto/aead_base_encrypter_openssl.cc b/net/quic/crypto/aead_base_encrypter_openssl.cc
index 25d9967..be4b1e8 100644
--- a/net/quic/crypto/aead_base_encrypter_openssl.cc
+++ b/net/quic/crypto/aead_base_encrypter_openssl.cc
@@ -81,16 +81,12 @@ bool AeadBaseEncrypter::Encrypt(StringPiece nonce,
     return false;
   }
 
-  size_t len;
+  size_t ciphertext_len;
   if (!EVP_AEAD_CTX_seal(
-          ctx_.get(),
-          output,
-          &len,
+          ctx_.get(), output, &ciphertext_len,
           plaintext.size() + auth_tag_size_,
-          reinterpret_cast<const uint8_t*>(nonce.data()),
-          nonce.size(),
-          reinterpret_cast<const uint8_t*>(plaintext.data()),
-          plaintext.size(),
+          reinterpret_cast<const uint8_t*>(nonce.data()), nonce.size(),
+          reinterpret_cast<const uint8_t*>(plaintext.data()), plaintext.size(),
           reinterpret_cast<const uint8_t*>(associated_data.data()),
           associated_data.size())) {
     DLogOpenSslErrors();
@@ -100,27 +96,28 @@ bool AeadBaseEncrypter::Encrypt(StringPiece nonce,
   return true;
 }
 
-QuicData* AeadBaseEncrypter::EncryptPacket(
-    QuicPacketSequenceNumber sequence_number,
-    StringPiece associated_data,
-    StringPiece plaintext) {
+bool AeadBaseEncrypter::EncryptPacket(QuicPacketSequenceNumber sequence_number,
+                                      StringPiece associated_data,
+                                      StringPiece plaintext,
+                                      char* output,
+                                      size_t* output_length,
+                                      size_t max_output_length) {
   size_t ciphertext_size = GetCiphertextSize(plaintext.length());
-  scoped_ptr<char[]> ciphertext(new char[ciphertext_size]);
-
+  if (max_output_length < ciphertext_size) {
+    return false;
+  }
   // TODO(ianswett): Introduce a check to ensure that we don't encrypt with the
   // same sequence number twice.
-  uint8 nonce[sizeof(nonce_prefix_) + sizeof(sequence_number)];
   const size_t nonce_size = nonce_prefix_size_ + sizeof(sequence_number);
-  DCHECK_LE(nonce_size, sizeof(nonce));
-  memcpy(nonce, nonce_prefix_, nonce_prefix_size_);
-  memcpy(nonce + nonce_prefix_size_, &sequence_number, sizeof(sequence_number));
-  if (!Encrypt(StringPiece(reinterpret_cast<char*>(nonce), nonce_size),
-               associated_data, plaintext,
-               reinterpret_cast<unsigned char*>(ciphertext.get()))) {
-    return nullptr;
+  memcpy(output, nonce_prefix_, nonce_prefix_size_);
+  memcpy(output + nonce_prefix_size_, &sequence_number,
+         sizeof(sequence_number));
+  if (!Encrypt(StringPiece(output, nonce_size), associated_data, plaintext,
+               reinterpret_cast<unsigned char*>(output))) {
+    return false;
   }
-
-  return new QuicData(ciphertext.release(), ciphertext_size, true);
+  *output_length = ciphertext_size;
+  return true;
 }
 
 size_t AeadBaseEncrypter::GetKeySize() const { return key_size_; }
diff --git a/net/quic/crypto/null_encrypter.cc b/net/quic/crypto/null_encrypter.cc
index 5f1a85b..286694a 100644
--- a/net/quic/crypto/null_encrypter.cc
+++ b/net/quic/crypto/null_encrypter.cc
@@ -34,14 +34,20 @@ bool NullEncrypter::Encrypt(
   return true;
 }
 
-QuicData* NullEncrypter::EncryptPacket(
-    QuicPacketSequenceNumber /*sequence_number*/,
-    StringPiece associated_data,
-    StringPiece plaintext) {
+bool NullEncrypter::EncryptPacket(QuicPacketSequenceNumber /*sequence_number*/,
+                                  StringPiece associated_data,
+                                  StringPiece plaintext,
+                                  char* output,
+                                  size_t* output_length,
+                                  size_t max_output_length) {
   const size_t len = plaintext.size() + GetHashLength();
-  uint8* buffer = new uint8[len];
-  Encrypt(StringPiece(), associated_data, plaintext, buffer);
-  return new QuicData(reinterpret_cast<char*>(buffer), len, true);
+  if (max_output_length < len) {
+    return false;
+  }
+  Encrypt(StringPiece(), associated_data, plaintext,
+          reinterpret_cast<unsigned char*>(output));
+  *output_length = len;
+  return true;
 }
 
 size_t NullEncrypter::GetKeySize() const { return 0; }
diff --git a/net/quic/crypto/null_encrypter.h b/net/quic/crypto/null_encrypter.h
index b52bd9c..40bcacc 100644
--- a/net/quic/crypto/null_encrypter.h
+++ b/net/quic/crypto/null_encrypter.h
@@ -26,9 +26,12 @@ class NET_EXPORT_PRIVATE NullEncrypter : public QuicEncrypter {
                base::StringPiece associated_data,
                base::StringPiece plaintext,
                unsigned char* output) override;
-  QuicData* EncryptPacket(QuicPacketSequenceNumber sequence_number,
-                          base::StringPiece associated_data,
-                          base::StringPiece plaintext) override;
+  bool EncryptPacket(QuicPacketSequenceNumber sequence_number,
+                     base::StringPiece associated_data,
+                     base::StringPiece plaintext,
+                     char* output,
+                     size_t* output_length,
+                     size_t max_output_length) override;
   size_t GetKeySize() const override;
   size_t GetNoncePrefixSize() const override;
   size_t GetMaxPlaintextSize(size_t ciphertext_size) const override;
diff --git a/net/quic/crypto/null_encrypter_test.cc b/net/quic/crypto/null_encrypter_test.cc
index 4f4dae7..e9ae773 100644
--- a/net/quic/crypto/null_encrypter_test.cc
+++ b/net/quic/crypto/null_encrypter_test.cc
@@ -24,13 +24,13 @@ TEST_F(NullEncrypterTest, Encrypt) {
     'b',  'y',  'e',  '!',
   };
   NullEncrypter encrypter;
-  scoped_ptr<QuicData> encrypted(
-      encrypter.EncryptPacket(0, "hello world!", "goodbye!"));
-  ASSERT_TRUE(encrypted.get());
+  char encrypted[256];
+  size_t encrypted_len = 0;
+  ASSERT_TRUE(encrypter.EncryptPacket(0, "hello world!", "goodbye!", encrypted,
+                                      &encrypted_len, 256));
   test::CompareCharArraysWithHexError(
-      "encrypted data", encrypted->data(), encrypted->length(),
-      reinterpret_cast<const char*>(expected),
-      arraysize(expected));
+      "encrypted data", encrypted, encrypted_len,
+      reinterpret_cast<const char*>(expected), arraysize(expected));
 }
 
 TEST_F(NullEncrypterTest, GetMaxPlaintextSize) {
diff --git a/net/quic/crypto/quic_crypto_client_config.cc b/net/quic/crypto/quic_crypto_client_config.cc
index 1e12424..18e9e60 100644
--- a/net/quic/crypto/quic_crypto_client_config.cc
+++ b/net/quic/crypto/quic_crypto_client_config.cc
@@ -545,16 +545,19 @@ QuicErrorCode QuicCryptoClientConfig::FillClientHello(
     }
 
     const QuicData& cetv_plaintext = cetv.GetSerialized();
-    scoped_ptr<QuicData> cetv_ciphertext(crypters.encrypter->EncryptPacket(
-        0 /* sequence number */,
-        StringPiece() /* associated data */,
-        cetv_plaintext.AsStringPiece()));
-    if (!cetv_ciphertext.get()) {
+    const size_t encrypted_len =
+        crypters.encrypter->GetCiphertextSize(cetv_plaintext.length());
+    scoped_ptr<char[]> output(new char[encrypted_len]);
+    size_t output_size = 0;
+    if (!crypters.encrypter->EncryptPacket(
+            0 /* sequence number */, StringPiece() /* associated data */,
+            cetv_plaintext.AsStringPiece(), output.get(), &output_size,
+            encrypted_len)) {
       *error_details = "Packet encryption failed";
       return QUIC_ENCRYPTION_FAILURE;
     }
 
-    out->SetStringPiece(kCETV, cetv_ciphertext->AsStringPiece());
+    out->SetStringPiece(kCETV, StringPiece(output.get(), output_size));
     out->MarkDirty();
 
     out->set_minimum_size(orig_min_size);
diff --git a/net/quic/crypto/quic_encrypter.h b/net/quic/crypto/quic_encrypter.h
index 5cb40a1..2e565c5 100644
--- a/net/quic/crypto/quic_encrypter.h
+++ b/net/quic/crypto/quic_encrypter.h
@@ -53,9 +53,12 @@ class NET_EXPORT_PRIVATE QuicEncrypter {
   // |plaintext| as well as a MAC over both |plaintext| and |associated_data|,
   // or nullptr if there is an error. |sequence_number| is appended to the
   // |nonce_prefix| value provided in SetNoncePrefix() to form the nonce.
-  virtual QuicData* EncryptPacket(QuicPacketSequenceNumber sequence_number,
-                                  base::StringPiece associated_data,
-                                  base::StringPiece plaintext) = 0;
+  virtual bool EncryptPacket(QuicPacketSequenceNumber sequence_number,
+                             base::StringPiece associated_data,
+                             base::StringPiece plaintext,
+                             char* output,
+                             size_t* output_length,
+                             size_t max_output_length) = 0;
 
   // GetKeySize() and GetNoncePrefixSize() tell the HKDF class how many bytes
   // of key material needs to be derived from the master secret.
diff --git a/net/quic/quic_connection_test.cc b/net/quic/quic_connection_test.cc
index 5a76ec4..a25065e 100644
--- a/net/quic/quic_connection_test.cc
+++ b/net/quic/quic_connection_test.cc
@@ -87,13 +87,20 @@ class TaggingEncrypter : public QuicEncrypter {
     return true;
   }
 
-  QuicData* EncryptPacket(QuicPacketSequenceNumber sequence_number,
-                          StringPiece associated_data,
-                          StringPiece plaintext) override {
+  bool EncryptPacket(QuicPacketSequenceNumber sequence_number,
+                     StringPiece associated_data,
+                     StringPiece plaintext,
+                     char* output,
+                     size_t* output_length,
+                     size_t max_output_length) override {
     const size_t len = plaintext.size() + kTagSize;
-    uint8* buffer = new uint8[len];
-    Encrypt(StringPiece(), associated_data, plaintext, buffer);
-    return new QuicData(reinterpret_cast<char*>(buffer), len, true);
+    if (max_output_length < len) {
+      return false;
+    }
+    Encrypt(StringPiece(), associated_data, plaintext,
+            reinterpret_cast<unsigned char*>(output));
+    *output_length = len;
+    return true;
   }
 
   size_t GetKeySize() const override { return 0; }
diff --git a/net/quic/quic_framer.cc b/net/quic/quic_framer.cc
index 364bdac..3eda6b3 100644
--- a/net/quic/quic_framer.cc
+++ b/net/quic/quic_framer.cc
@@ -1625,19 +1625,27 @@ QuicEncryptedPacket* QuicFramer::EncryptPacket(
     const QuicPacket& packet) {
   DCHECK(encrypter_[level].get() != nullptr);
 
-  scoped_ptr<QuicData> out(encrypter_[level]->EncryptPacket(
-      packet_sequence_number, packet.AssociatedData(), packet.Plaintext()));
-  if (out.get() == nullptr) {
-    RaiseError(QUIC_ENCRYPTION_FAILURE);
-    return nullptr;
-  }
+  // Allocate a large enough buffer for the header and the encrypted data.
+  const size_t encrypted_len =
+      encrypter_[level]->GetCiphertextSize(packet.Plaintext().length());
   StringPiece header_data = packet.BeforePlaintext();
-  size_t len =  header_data.length() + out->length();
+  const size_t len = header_data.length() + encrypted_len;
+  // TODO(ianswett): Consider allocating this on the stack in the typical case.
   char* buffer = new char[len];
-  // TODO(rch): eliminate this buffer copy by passing in a buffer to Encrypt().
+  // Copy in the header, because the encrypter only populates the encrypted
+  // plaintext content.
   memcpy(buffer, header_data.data(), header_data.length());
-  memcpy(buffer + header_data.length(), out->data(), out->length());
-  return new QuicEncryptedPacket(buffer, len, true);
+  // Encrypt the plaintext into the buffer.
+  size_t output_length = 0;
+  if (!encrypter_[level]->EncryptPacket(
+          packet_sequence_number, packet.AssociatedData(), packet.Plaintext(),
+          buffer + header_data.length(), &output_length, encrypted_len)) {
+    RaiseError(QUIC_ENCRYPTION_FAILURE);
+    return nullptr;
+  }
+
+  return new QuicEncryptedPacket(buffer, header_data.length() + output_length,
+                                 true);
 }
 
 size_t QuicFramer::GetMaxPlaintextSize(size_t ciphertext_size) {
diff --git a/net/quic/quic_framer_test.cc b/net/quic/quic_framer_test.cc
index 678e69d..07a3510 100644
--- a/net/quic/quic_framer_test.cc
+++ b/net/quic/quic_framer_test.cc
@@ -113,13 +113,18 @@ class TestEncrypter : public QuicEncrypter {
     CHECK(false) << "Not implemented";
     return false;
   }
-  QuicData* EncryptPacket(QuicPacketSequenceNumber sequence_number,
-                          StringPiece associated_data,
-                          StringPiece plaintext) override {
+  bool EncryptPacket(QuicPacketSequenceNumber sequence_number,
+                     StringPiece associated_data,
+                     StringPiece plaintext,
+                     char* output,
+                     size_t* output_length,
+                     size_t max_output_length) override {
     sequence_number_ = sequence_number;
     associated_data_ = associated_data.as_string();
     plaintext_ = plaintext.as_string();
-    return new QuicData(plaintext.data(), plaintext.length());
+    memcpy(output, plaintext.data(), plaintext.length());
+    *output_length = plaintext.length();
+    return true;
   }
   size_t GetKeySize() const override { return 0; }
   size_t GetNoncePrefixSize() const override { return 0; }