Use 16-byte auth tags when initial obfuscators are used

QuicFramer assumes that the auth tag is the same length for all encryption
levels. In Google QUIC crypto versions where initial obfuscators are used,
we should use IETF style crypters (different nonce/IV construction and 16
byte instead of 12 byte auth tags).

gfe-relnote: Change encryption used in QUIC. Protected by quic_enable_version_99
PiperOrigin-RevId: 271674606
Change-Id: Ic7736908068eeee8077bd3a17ec4f8b4112254f9
diff --git a/quic/core/crypto/aead_base_decrypter.cc b/quic/core/crypto/aead_base_decrypter.cc
index b5db0db..ab441b7 100644
--- a/quic/core/crypto/aead_base_decrypter.cc
+++ b/quic/core/crypto/aead_base_decrypter.cc
@@ -126,13 +126,18 @@
   }
 
   std::string key, nonce_prefix;
-  size_t prefix_size = nonce_size_ - sizeof(QuicPacketNumber);
+  size_t prefix_size = nonce_size_;
+  if (!use_ietf_nonce_construction_) {
+    prefix_size -= sizeof(QuicPacketNumber);
+  }
   DiversifyPreliminaryKey(
       QuicStringPiece(reinterpret_cast<const char*>(key_), key_size_),
       QuicStringPiece(reinterpret_cast<const char*>(iv_), prefix_size), nonce,
       key_size_, prefix_size, &key, &nonce_prefix);
 
-  if (!SetKey(key) || !SetNoncePrefix(nonce_prefix)) {
+  if (!SetKey(key) ||
+      (!use_ietf_nonce_construction_ && !SetNoncePrefix(nonce_prefix)) ||
+      (use_ietf_nonce_construction_ && !SetIV(nonce_prefix))) {
     DCHECK(false);
     return false;
   }
diff --git a/quic/core/crypto/crypto_utils.cc b/quic/core/crypto/crypto_utils.cc
index 4851748..96bc86c 100644
--- a/quic/core/crypto/crypto_utils.cc
+++ b/quic/core/crypto/crypto_utils.cc
@@ -117,28 +117,6 @@
 
 const char kPreSharedKeyLabel[] = "QUIC PSK";
 
-// This is the same as SetKeyAndIV, except it is for use with Google QUIC crypto
-// style crypters (which have a different nonce construction and auth tag
-// length). The labels for HkdfExpandLabel have also been changed to be prefixed
-// with "gquic" instead of "quic".
-void SetKeyAndNonceForGoogleQuicInitialCrypter(
-    const EVP_MD* prf,
-    const std::vector<uint8_t>& pp_secret,
-    QuicCrypter* crypter) {
-  std::vector<uint8_t> key =
-      HkdfExpandLabel(prf, pp_secret, "gquic key", crypter->GetKeySize());
-  std::vector<uint8_t> iv = HkdfExpandLabel(prf, pp_secret, "gquic iv",
-                                            crypter->GetNoncePrefixSize());
-  std::vector<uint8_t> pn =
-      HkdfExpandLabel(prf, pp_secret, "gquic hp", crypter->GetKeySize());
-  crypter->SetKey(
-      QuicStringPiece(reinterpret_cast<char*>(key.data()), key.size()));
-  crypter->SetNoncePrefix(
-      QuicStringPiece(reinterpret_cast<char*>(iv.data()), iv.size()));
-  crypter->SetHeaderProtectionKey(
-      QuicStringPiece(reinterpret_cast<char*>(pn.data()), pn.size()));
-}
-
 }  // namespace
 
 // static
@@ -185,25 +163,13 @@
   }
   std::vector<uint8_t> encryption_secret = HkdfExpandLabel(
       hash, handshake_secret, encryption_label, EVP_MD_size(hash));
+  crypters->encrypter = std::make_unique<Aes128GcmEncrypter>();
+  SetKeyAndIV(hash, encryption_secret, crypters->encrypter.get());
+
   std::vector<uint8_t> decryption_secret = HkdfExpandLabel(
       hash, handshake_secret, decryption_label, EVP_MD_size(hash));
-
-  // Create an encrypter and decrypter that have an auth tag of the same length
-  // as the encrypters/decrypters used with the handshake protocol for this
-  // version.
-  if (version.handshake_protocol == PROTOCOL_TLS1_3) {
-    crypters->encrypter = std::make_unique<Aes128GcmEncrypter>();
-    SetKeyAndIV(hash, encryption_secret, crypters->encrypter.get());
-    crypters->decrypter = std::make_unique<Aes128GcmDecrypter>();
-    SetKeyAndIV(hash, decryption_secret, crypters->decrypter.get());
-  } else {
-    crypters->encrypter = std::make_unique<Aes128Gcm12Encrypter>();
-    SetKeyAndNonceForGoogleQuicInitialCrypter(hash, encryption_secret,
-                                              crypters->encrypter.get());
-    crypters->decrypter = std::make_unique<Aes128Gcm12Decrypter>();
-    SetKeyAndNonceForGoogleQuicInitialCrypter(hash, decryption_secret,
-                                              crypters->decrypter.get());
-  }
+  crypters->decrypter = std::make_unique<Aes128GcmDecrypter>();
+  SetKeyAndIV(hash, decryption_secret, crypters->decrypter.get());
 }
 
 // static
@@ -234,7 +200,8 @@
 }
 
 // static
-bool CryptoUtils::DeriveKeys(QuicStringPiece premaster_secret,
+bool CryptoUtils::DeriveKeys(const ParsedQuicVersion& version,
+                             QuicStringPiece premaster_secret,
                              QuicTag aead,
                              QuicStringPiece client_nonce,
                              QuicStringPiece server_nonce,
@@ -269,10 +236,14 @@
         QuicStringPiece(psk_premaster_secret.get(), psk_premaster_secret_size);
   }
 
-  crypters->encrypter = QuicEncrypter::Create(aead);
-  crypters->decrypter = QuicDecrypter::Create(aead);
+  crypters->encrypter = QuicEncrypter::Create(version, aead);
+  crypters->decrypter = QuicDecrypter::Create(version, aead);
+
   size_t key_bytes = crypters->encrypter->GetKeySize();
   size_t nonce_prefix_bytes = crypters->encrypter->GetNoncePrefixSize();
+  if (version.UsesInitialObfuscators()) {
+    nonce_prefix_bytes = crypters->encrypter->GetIVSize();
+  }
   size_t subkey_secret_bytes =
       subkey_secret == nullptr ? 0 : premaster_secret.length();
 
@@ -294,22 +265,26 @@
     case Diversification::NEVER: {
       if (perspective == Perspective::IS_SERVER) {
         if (!crypters->encrypter->SetKey(hkdf.server_write_key()) ||
-            !crypters->encrypter->SetNoncePrefix(hkdf.server_write_iv()) ||
+            !crypters->encrypter->SetNoncePrefixOrIV(version,
+                                                     hkdf.server_write_iv()) ||
             !crypters->encrypter->SetHeaderProtectionKey(
                 hkdf.server_hp_key()) ||
             !crypters->decrypter->SetKey(hkdf.client_write_key()) ||
-            !crypters->decrypter->SetNoncePrefix(hkdf.client_write_iv()) ||
+            !crypters->decrypter->SetNoncePrefixOrIV(version,
+                                                     hkdf.client_write_iv()) ||
             !crypters->decrypter->SetHeaderProtectionKey(
                 hkdf.client_hp_key())) {
           return false;
         }
       } else {
         if (!crypters->encrypter->SetKey(hkdf.client_write_key()) ||
-            !crypters->encrypter->SetNoncePrefix(hkdf.client_write_iv()) ||
+            !crypters->encrypter->SetNoncePrefixOrIV(version,
+                                                     hkdf.client_write_iv()) ||
             !crypters->encrypter->SetHeaderProtectionKey(
                 hkdf.client_hp_key()) ||
             !crypters->decrypter->SetKey(hkdf.server_write_key()) ||
-            !crypters->decrypter->SetNoncePrefix(hkdf.server_write_iv()) ||
+            !crypters->decrypter->SetNoncePrefixOrIV(version,
+                                                     hkdf.server_write_iv()) ||
             !crypters->decrypter->SetHeaderProtectionKey(
                 hkdf.server_hp_key())) {
           return false;
@@ -324,10 +299,12 @@
       }
 
       if (!crypters->encrypter->SetKey(hkdf.client_write_key()) ||
-          !crypters->encrypter->SetNoncePrefix(hkdf.client_write_iv()) ||
+          !crypters->encrypter->SetNoncePrefixOrIV(version,
+                                                   hkdf.client_write_iv()) ||
           !crypters->encrypter->SetHeaderProtectionKey(hkdf.client_hp_key()) ||
           !crypters->decrypter->SetPreliminaryKey(hkdf.server_write_key()) ||
-          !crypters->decrypter->SetNoncePrefix(hkdf.server_write_iv()) ||
+          !crypters->decrypter->SetNoncePrefixOrIV(version,
+                                                   hkdf.server_write_iv()) ||
           !crypters->decrypter->SetHeaderProtectionKey(hkdf.server_hp_key())) {
         return false;
       }
@@ -345,10 +322,11 @@
           *diversification.nonce(), key_bytes, nonce_prefix_bytes, &key,
           &nonce_prefix);
       if (!crypters->decrypter->SetKey(hkdf.client_write_key()) ||
-          !crypters->decrypter->SetNoncePrefix(hkdf.client_write_iv()) ||
+          !crypters->decrypter->SetNoncePrefixOrIV(version,
+                                                   hkdf.client_write_iv()) ||
           !crypters->decrypter->SetHeaderProtectionKey(hkdf.client_hp_key()) ||
           !crypters->encrypter->SetKey(key) ||
-          !crypters->encrypter->SetNoncePrefix(nonce_prefix) ||
+          !crypters->encrypter->SetNoncePrefixOrIV(version, nonce_prefix) ||
           !crypters->encrypter->SetHeaderProtectionKey(hkdf.server_hp_key())) {
         return false;
       }
diff --git a/quic/core/crypto/crypto_utils.h b/quic/core/crypto/crypto_utils.h
index 6f198d6..cb02dd7 100644
--- a/quic/core/crypto/crypto_utils.h
+++ b/quic/core/crypto/crypto_utils.h
@@ -123,7 +123,8 @@
   // decrypter will only be keyed to a preliminary state: a call to
   // |SetDiversificationNonce| with a diversification nonce will be needed to
   // complete keying.
-  static bool DeriveKeys(QuicStringPiece premaster_secret,
+  static bool DeriveKeys(const ParsedQuicVersion& version,
+                         QuicStringPiece premaster_secret,
                          QuicTag aead,
                          QuicStringPiece client_nonce,
                          QuicStringPiece server_nonce,
diff --git a/quic/core/crypto/crypto_utils_test.cc b/quic/core/crypto/crypto_utils_test.cc
index 94b5098..25adff2 100644
--- a/quic/core/crypto/crypto_utils_test.cc
+++ b/quic/core/crypto/crypto_utils_test.cc
@@ -149,6 +149,21 @@
           static_cast<HandshakeFailureReason>(MAX_FAILURE_REASON + 1)));
 }
 
+TEST_F(CryptoUtilsTest, AuthTagLengths) {
+  for (const auto& version : AllSupportedVersions()) {
+    for (QuicTag algo : {kAESG, kCC20}) {
+      SCOPED_TRACE(version);
+      std::unique_ptr<QuicEncrypter> encrypter(
+          QuicEncrypter::Create(version, algo));
+      size_t auth_tag_size = 12;
+      if (version.UsesInitialObfuscators()) {
+        auth_tag_size = 16;
+      }
+      EXPECT_EQ(encrypter->GetCiphertextSize(0), auth_tag_size);
+    }
+  }
+}
+
 }  // namespace
 }  // namespace test
 }  // namespace quic
diff --git a/quic/core/crypto/quic_crypter.cc b/quic/core/crypto/quic_crypter.cc
new file mode 100644
index 0000000..dc5bfb3
--- /dev/null
+++ b/quic/core/crypto/quic_crypter.cc
@@ -0,0 +1,17 @@
+// Copyright (c) 2019 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/third_party/quiche/src/quic/core/crypto/quic_crypter.h"
+
+namespace quic {
+
+bool QuicCrypter::SetNoncePrefixOrIV(const ParsedQuicVersion& version,
+                                     QuicStringPiece nonce_prefix_or_iv) {
+  if (version.UsesInitialObfuscators()) {
+    return SetIV(nonce_prefix_or_iv);
+  }
+  return SetNoncePrefix(nonce_prefix_or_iv);
+}
+
+}  // namespace quic
diff --git a/quic/core/crypto/quic_crypter.h b/quic/core/crypto/quic_crypter.h
index 5f07836..25a91e4 100644
--- a/quic/core/crypto/quic_crypter.h
+++ b/quic/core/crypto/quic_crypter.h
@@ -5,6 +5,7 @@
 #ifndef QUICHE_QUIC_CORE_CRYPTO_QUIC_CRYPTER_H_
 #define QUICHE_QUIC_CORE_CRYPTO_QUIC_CRYPTER_H_
 
+#include "net/third_party/quiche/src/quic/core/quic_versions.h"
 #include "net/third_party/quiche/src/quic/platform/api/quic_export.h"
 #include "net/third_party/quiche/src/quic/platform/api/quic_string_piece.h"
 
@@ -69,6 +70,11 @@
   // packet number, even when retransmitting a lost packet.
   virtual bool SetIV(QuicStringPiece iv) = 0;
 
+  // Calls SetNoncePrefix or SetIV depending on whether |version| uses the
+  // Google QUIC crypto or IETF QUIC nonce construction.
+  virtual bool SetNoncePrefixOrIV(const ParsedQuicVersion& version,
+                                  QuicStringPiece nonce_prefix_or_iv);
+
   // Sets the key to use for header protection.
   virtual bool SetHeaderProtectionKey(QuicStringPiece key) = 0;
 
diff --git a/quic/core/crypto/quic_crypto_client_config.cc b/quic/core/crypto/quic_crypto_client_config.cc
index 2b458ff..75b2e6a 100644
--- a/quic/core/crypto/quic_crypto_client_config.cc
+++ b/quic/core/crypto/quic_crypto_client_config.cc
@@ -514,6 +514,7 @@
     const QuicServerId& server_id,
     QuicConnectionId connection_id,
     const ParsedQuicVersion preferred_version,
+    const ParsedQuicVersion actual_version,
     const CachedState* cached,
     QuicWallTime now,
     QuicRandom* rand,
@@ -649,12 +650,12 @@
 
   std::string* subkey_secret = &out_params->initial_subkey_secret;
 
-  if (!CryptoUtils::DeriveKeys(out_params->initial_premaster_secret,
-                               out_params->aead, out_params->client_nonce,
-                               out_params->server_nonce, pre_shared_key_,
-                               hkdf_input, Perspective::IS_CLIENT,
-                               CryptoUtils::Diversification::Pending(),
-                               &out_params->initial_crypters, subkey_secret)) {
+  if (!CryptoUtils::DeriveKeys(
+          actual_version, out_params->initial_premaster_secret,
+          out_params->aead, out_params->client_nonce, out_params->server_nonce,
+          pre_shared_key_, hkdf_input, Perspective::IS_CLIENT,
+          CryptoUtils::Diversification::Pending(),
+          &out_params->initial_crypters, subkey_secret)) {
     *error_details = "Symmetric key setup failed";
     return QUIC_CRYPTO_SYMMETRIC_KEY_SETUP_FAILED;
   }
@@ -767,7 +768,7 @@
 QuicErrorCode QuicCryptoClientConfig::ProcessServerHello(
     const CryptoHandshakeMessage& server_hello,
     QuicConnectionId /*connection_id*/,
-    ParsedQuicVersion /*version*/,
+    ParsedQuicVersion version,
     const ParsedQuicVersionVector& negotiated_versions,
     CachedState* cached,
     QuicReferenceCountedPointer<QuicCryptoNegotiatedParameters> out_params,
@@ -814,8 +815,8 @@
   hkdf_input.append(out_params->hkdf_input_suffix);
 
   if (!CryptoUtils::DeriveKeys(
-          out_params->forward_secure_premaster_secret, out_params->aead,
-          out_params->client_nonce,
+          version, out_params->forward_secure_premaster_secret,
+          out_params->aead, out_params->client_nonce,
           shlo_nonce.empty() ? out_params->server_nonce : shlo_nonce,
           pre_shared_key_, hkdf_input, Perspective::IS_CLIENT,
           CryptoUtils::Diversification::Never(),
diff --git a/quic/core/crypto/quic_crypto_client_config.h b/quic/core/crypto/quic_crypto_client_config.h
index 2de7cd0..838b2ee 100644
--- a/quic/core/crypto/quic_crypto_client_config.h
+++ b/quic/core/crypto/quic_crypto_client_config.h
@@ -253,6 +253,7 @@
       const QuicServerId& server_id,
       QuicConnectionId connection_id,
       const ParsedQuicVersion preferred_version,
+      const ParsedQuicVersion actual_version,
       const CachedState* cached,
       QuicWallTime now,
       QuicRandom* rand,
diff --git a/quic/core/crypto/quic_crypto_client_config_test.cc b/quic/core/crypto/quic_crypto_client_config_test.cc
index d337fdc..683180b 100644
--- a/quic/core/crypto/quic_crypto_client_config_test.cc
+++ b/quic/core/crypto/quic_crypto_client_config_test.cc
@@ -315,8 +315,8 @@
   MockRandom rand;
   CryptoHandshakeMessage chlo;
   QuicServerId server_id("www.google.com", 443, false);
-  config.FillClientHello(server_id, kConnectionId, QuicVersionMax(), &state,
-                         QuicWallTime::Zero(), &rand,
+  config.FillClientHello(server_id, kConnectionId, QuicVersionMax(),
+                         QuicVersionMax(), &state, QuicWallTime::Zero(), &rand,
                          params, &chlo, &error_details);
 
   // Verify that the version label has been set correctly in the CHLO.
@@ -336,8 +336,8 @@
   MockRandom rand;
   CryptoHandshakeMessage chlo;
   QuicServerId server_id("www.google.com", 443, false);
-  config.FillClientHello(server_id, kConnectionId, QuicVersionMax(), &state,
-                         QuicWallTime::Zero(), &rand,
+  config.FillClientHello(server_id, kConnectionId, QuicVersionMax(),
+                         QuicVersionMax(), &state, QuicWallTime::Zero(), &rand,
                          params, &chlo, &error_details);
 
   // Verify that the version label has been set correctly in the CHLO.
diff --git a/quic/core/crypto/quic_crypto_server_config.cc b/quic/core/crypto/quic_crypto_server_config.cc
index 95705e5..2ed0093 100644
--- a/quic/core/crypto/quic_crypto_server_config.cc
+++ b/quic/core/crypto/quic_crypto_server_config.cc
@@ -874,7 +874,7 @@
 
     CrypterPair crypters;
     if (!CryptoUtils::DeriveKeys(
-            context->params()->initial_premaster_secret,
+            context->version(), context->params()->initial_premaster_secret,
             context->params()->aead, context->info().client_nonce,
             context->info().server_nonce, pre_shared_key_, hkdf_input,
             Perspective::IS_SERVER, CryptoUtils::Diversification::Never(),
@@ -926,9 +926,10 @@
   CryptoUtils::Diversification diversification =
       CryptoUtils::Diversification::Now(out_diversification_nonce.get());
   if (!CryptoUtils::DeriveKeys(
-          context->params()->initial_premaster_secret, context->params()->aead,
-          context->info().client_nonce, context->info().server_nonce,
-          pre_shared_key_, hkdf_input, Perspective::IS_SERVER, diversification,
+          context->version(), context->params()->initial_premaster_secret,
+          context->params()->aead, context->info().client_nonce,
+          context->info().server_nonce, pre_shared_key_, hkdf_input,
+          Perspective::IS_SERVER, diversification,
           &context->params()->initial_crypters,
           &context->params()->initial_subkey_secret)) {
     context->Fail(QUIC_CRYPTO_SYMMETRIC_KEY_SETUP_FAILED,
@@ -967,6 +968,7 @@
   out->SetStringPiece(kServerNonceTag, shlo_nonce);
 
   if (!CryptoUtils::DeriveKeys(
+          context->version(),
           context->params()->forward_secure_premaster_secret,
           context->params()->aead, context->info().client_nonce,
           shlo_nonce.empty() ? context->info().server_nonce : shlo_nonce,
diff --git a/quic/core/crypto/quic_decrypter.cc b/quic/core/crypto/quic_decrypter.cc
index 2fb1deb..5802231 100644
--- a/quic/core/crypto/quic_decrypter.cc
+++ b/quic/core/crypto/quic_decrypter.cc
@@ -22,12 +22,22 @@
 namespace quic {
 
 // static
-std::unique_ptr<QuicDecrypter> QuicDecrypter::Create(QuicTag algorithm) {
+std::unique_ptr<QuicDecrypter> QuicDecrypter::Create(
+    const ParsedQuicVersion& version,
+    QuicTag algorithm) {
   switch (algorithm) {
     case kAESG:
-      return std::make_unique<Aes128Gcm12Decrypter>();
+      if (version.UsesInitialObfuscators()) {
+        return std::make_unique<Aes128GcmDecrypter>();
+      } else {
+        return std::make_unique<Aes128Gcm12Decrypter>();
+      }
     case kCC20:
-      return std::make_unique<ChaCha20Poly1305Decrypter>();
+      if (version.UsesInitialObfuscators()) {
+        return std::make_unique<ChaCha20Poly1305TlsDecrypter>();
+      } else {
+        return std::make_unique<ChaCha20Poly1305Decrypter>();
+      }
     default:
       QUIC_LOG(FATAL) << "Unsupported algorithm: " << algorithm;
       return nullptr;
diff --git a/quic/core/crypto/quic_decrypter.h b/quic/core/crypto/quic_decrypter.h
index 559420c..aed18e4 100644
--- a/quic/core/crypto/quic_decrypter.h
+++ b/quic/core/crypto/quic_decrypter.h
@@ -22,7 +22,8 @@
  public:
   virtual ~QuicDecrypter() {}
 
-  static std::unique_ptr<QuicDecrypter> Create(QuicTag algorithm);
+  static std::unique_ptr<QuicDecrypter> Create(const ParsedQuicVersion& version,
+                                               QuicTag algorithm);
 
   // Creates an IETF QuicDecrypter based on |cipher_suite| which must be an id
   // returned by SSL_CIPHER_get_id. The caller is responsible for taking
diff --git a/quic/core/crypto/quic_encrypter.cc b/quic/core/crypto/quic_encrypter.cc
index f6b2490..b4d3b0d 100644
--- a/quic/core/crypto/quic_encrypter.cc
+++ b/quic/core/crypto/quic_encrypter.cc
@@ -19,12 +19,22 @@
 namespace quic {
 
 // static
-std::unique_ptr<QuicEncrypter> QuicEncrypter::Create(QuicTag algorithm) {
+std::unique_ptr<QuicEncrypter> QuicEncrypter::Create(
+    const ParsedQuicVersion& version,
+    QuicTag algorithm) {
   switch (algorithm) {
     case kAESG:
-      return std::make_unique<Aes128Gcm12Encrypter>();
+      if (version.UsesInitialObfuscators()) {
+        return std::make_unique<Aes128GcmEncrypter>();
+      } else {
+        return std::make_unique<Aes128Gcm12Encrypter>();
+      }
     case kCC20:
-      return std::make_unique<ChaCha20Poly1305Encrypter>();
+      if (version.UsesInitialObfuscators()) {
+        return std::make_unique<ChaCha20Poly1305TlsEncrypter>();
+      } else {
+        return std::make_unique<ChaCha20Poly1305Encrypter>();
+      }
     default:
       QUIC_LOG(FATAL) << "Unsupported algorithm: " << algorithm;
       return nullptr;
diff --git a/quic/core/crypto/quic_encrypter.h b/quic/core/crypto/quic_encrypter.h
index 15acd02..d318329 100644
--- a/quic/core/crypto/quic_encrypter.h
+++ b/quic/core/crypto/quic_encrypter.h
@@ -19,7 +19,8 @@
  public:
   virtual ~QuicEncrypter() {}
 
-  static std::unique_ptr<QuicEncrypter> Create(QuicTag algorithm);
+  static std::unique_ptr<QuicEncrypter> Create(const ParsedQuicVersion& version,
+                                               QuicTag algorithm);
 
   // Creates an IETF QuicEncrypter based on |cipher_suite| which must be an id
   // returned by SSL_CIPHER_get_id. The caller is responsible for taking
diff --git a/quic/core/quic_connection_test.cc b/quic/core/quic_connection_test.cc
index 2f8f6a7..f65f0cd 100644
--- a/quic/core/quic_connection_test.cc
+++ b/quic/core/quic_connection_test.cc
@@ -4796,6 +4796,17 @@
   EXPECT_EQ(new_mtu, mtu_probe_size);
   EXPECT_EQ(QuicPacketNumber(1u), creator_->packet_number());
 
+  // QuicFramer::GetMaxPlaintextSize uses the smallest max plaintext size across
+  // all encrypters. The initial encrypter used with IETF QUIC has a 16-byte
+  // overhead, while the NullEncrypter used throughout this test has a 12-byte
+  // overhead. This test tests behavior that relies on computing the packet size
+  // correctly, so by unsetting the initial encrypter, we avoid having a
+  // mismatch between the overheads for the encrypters used. In non-test
+  // scenarios all encrypters used for a given connection have the same
+  // overhead, either 12 bytes for ones using Google QUIC crypto, or 16 bytes
+  // for ones using TLS.
+  connection_.SetEncrypter(ENCRYPTION_INITIAL, nullptr);
+
   // Send more than MTU worth of data.  No acknowledgement was received so far,
   // so the MTU should be at its old value.
   const std::string data(kDefaultMaxPacketSize + 1, '.');
@@ -4851,8 +4862,7 @@
   // scenarios all encrypters used for a given connection have the same
   // overhead, either 12 bytes for ones using Google QUIC crypto, or 16 bytes
   // for ones using TLS.
-  QuicConnectionPeer::GetFramer(&connection_)
-      ->SetEncrypter(ENCRYPTION_INITIAL, nullptr);
+  connection_.SetEncrypter(ENCRYPTION_INITIAL, nullptr);
 
   connection_.EnablePathMtuDiscovery(send_algorithm_);
 
@@ -4993,8 +5003,7 @@
   // scenarios all encrypters used for a given connection have the same
   // overhead, either 12 bytes for ones using Google QUIC crypto, or 16 bytes
   // for ones using TLS.
-  QuicConnectionPeer::GetFramer(&connection_)
-      ->SetEncrypter(ENCRYPTION_INITIAL, nullptr);
+  connection_.SetEncrypter(ENCRYPTION_INITIAL, nullptr);
 
   const QuicByteCount mtu_limit = kMtuDiscoveryTargetPacketSizeHigh - 1;
   writer_->set_max_packet_size(mtu_limit);
diff --git a/quic/core/quic_crypto_client_handshaker.cc b/quic/core/quic_crypto_client_handshaker.cc
index d993049..1f63042 100644
--- a/quic/core/quic_crypto_client_handshaker.cc
+++ b/quic/core/quic_crypto_client_handshaker.cc
@@ -288,7 +288,8 @@
   std::string error_details;
   QuicErrorCode error = crypto_config_->FillClientHello(
       server_id_, session()->connection()->connection_id(),
-      session()->supported_versions().front(), cached,
+      session()->supported_versions().front(),
+      session()->connection()->version(), cached,
       session()->connection()->clock()->WallNow(),
       session()->connection()->random_generator(), crypto_negotiated_params_,
       &out, &error_details);
diff --git a/quic/test_tools/simulator/quic_endpoint.cc b/quic/test_tools/simulator/quic_endpoint.cc
index 3048534..68c758f 100644
--- a/quic/test_tools/simulator/quic_endpoint.cc
+++ b/quic/test_tools/simulator/quic_endpoint.cc
@@ -86,6 +86,7 @@
   connection_.set_visitor(this);
   connection_.SetEncrypter(ENCRYPTION_FORWARD_SECURE,
                            std::make_unique<NullEncrypter>(perspective));
+  connection_.SetEncrypter(ENCRYPTION_INITIAL, nullptr);
   if (connection_.version().KnowsWhichDecrypterToUse()) {
     connection_.InstallDecrypter(ENCRYPTION_FORWARD_SECURE,
                                  std::make_unique<NullDecrypter>(perspective));