Don't crash if QuicServerConfigProtobuf is invalid.

Return an error indication (false) if no keys are
passed to CryptoSecretBoxer::SetKeys, rather than corrupting
the state of the boxer, which would later result in a crash.

PiperOrigin-RevId: 416130497

quic/core/crypto/crypto_secret_boxer.cc

diff --git a/quic/core/crypto/crypto_secret_boxer.cc b/quic/core/crypto/crypto_secret_boxer.cc
index 1826212..a4a7a2a 100644
--- a/quic/core/crypto/crypto_secret_boxer.cc
+++ b/quic/core/crypto/crypto_secret_boxer.cc
@@ -44,8 +44,11 @@
 // kAEAD is the AEAD used for boxing: AES-256-GCM-SIV.
 static const EVP_AEAD* (*const kAEAD)() = EVP_aead_aes_256_gcm_siv;
 
-void CryptoSecretBoxer::SetKeys(const std::vector<std::string>& keys) {
-  QUICHE_DCHECK(!keys.empty());
+bool CryptoSecretBoxer::SetKeys(const std::vector<std::string>& keys) {
+  if (keys.empty()) {
+    QUIC_LOG(DFATAL) << "No keys supplied!";
+    return false;
+  }
   const EVP_AEAD* const aead = kAEAD();
   std::unique_ptr<State> new_state(new State);
 
@@ -57,7 +60,7 @@
     if (!ctx) {
       ERR_clear_error();
       QUIC_LOG(DFATAL) << "EVP_AEAD_CTX_init failed";
-      return;
+      return false;
     }
 
     new_state->ctxs.push_back(std::move(ctx));
@@ -65,6 +68,7 @@
 
   QuicWriterMutexLock l(&lock_);
   state_ = std::move(new_state);
+  return true;
 }
 
 std::string CryptoSecretBoxer::Box(QuicRandom* rand,

quic/core/crypto/crypto_secret_boxer.h

diff --git a/quic/core/crypto/crypto_secret_boxer.h b/quic/core/crypto/crypto_secret_boxer.h
index 5dd5cb2..e2cd037 100644
--- a/quic/core/crypto/crypto_secret_boxer.h
+++ b/quic/core/crypto/crypto_secret_boxer.h
@@ -34,13 +34,15 @@
   // SetKeys sets a list of encryption keys. The first key in the list will be
   // used by |Box|, but all supplied keys will be tried by |Unbox|, to handle
   // key skew across the fleet. This must be called before |Box| or |Unbox|.
-  // Keys must be |GetKeySize()| bytes long.
-  void SetKeys(const std::vector<std::string>& keys);
+  // Keys must be |GetKeySize()| bytes long. No change is made if any key is
+  // invalid, or if there are no keys supplied.
+  bool SetKeys(const std::vector<std::string>& keys);
 
   // Box encrypts |plaintext| using a random nonce generated from |rand| and
   // returns the resulting ciphertext. Since an authenticator and nonce are
   // included, the result will be slightly larger than |plaintext|. The first
-  // key in the vector supplied to |SetKeys| will be used.
+  // key in the vector supplied to |SetKeys| will be used. |SetKeys| must be
+  // called before calling this method.
   std::string Box(QuicRandom* rand, absl::string_view plaintext) const;
 
   // Unbox takes the result of a previous call to |Box| in |ciphertext| and

quic/core/crypto/crypto_secret_boxer_test.cc

diff --git a/quic/core/crypto/crypto_secret_boxer_test.cc b/quic/core/crypto/crypto_secret_boxer_test.cc
index 225bfc8..7b07d96 100644
--- a/quic/core/crypto/crypto_secret_boxer_test.cc
+++ b/quic/core/crypto/crypto_secret_boxer_test.cc
@@ -56,9 +56,9 @@
   std::string key_12(CryptoSecretBoxer::GetKeySize(), 0x12);
 
   CryptoSecretBoxer boxer_11, boxer_12, boxer;
-  boxer_11.SetKeys({key_11});
-  boxer_12.SetKeys({key_12});
-  boxer.SetKeys({key_12, key_11});
+  EXPECT_TRUE(boxer_11.SetKeys({key_11}));
+  EXPECT_TRUE(boxer_12.SetKeys({key_12}));
+  EXPECT_TRUE(boxer.SetKeys({key_12, key_11}));
 
   // Neither single-key boxer can decode the other's tokens.
   EXPECT_FALSE(CanDecode(boxer_11, boxer_12));
@@ -74,7 +74,7 @@
 
   // After we flush key_11 from |boxer|, it can no longer decode tokens from
   // |boxer_11|.
-  boxer.SetKeys({key_12});
+  EXPECT_TRUE(boxer.SetKeys({key_12}));
   EXPECT_FALSE(CanDecode(boxer, boxer_11));
 }
 

quic/core/crypto/quic_crypto_server_config.cc

diff --git a/quic/core/crypto/quic_crypto_server_config.cc b/quic/core/crypto/quic_crypto_server_config.cc
index d6eebbb..e8ac18e 100644
--- a/quic/core/crypto/quic_crypto_server_config.cc
+++ b/quic/core/crypto/quic_crypto_server_config.cc
@@ -512,6 +512,7 @@
 
 void QuicCryptoServerConfig::SetSourceAddressTokenKeys(
     const std::vector<std::string>& keys) {
+  // TODO(b/208866709)
   source_address_token_boxer_.SetKeys(keys);
 }
 
@@ -1576,9 +1577,14 @@
   std::unique_ptr<CryptoHandshakeMessage> msg =
       CryptoFramer::ParseMessage(protobuf.config());
 
+  if (!msg) {
+    QUIC_LOG(WARNING) << "Failed to parse server config message";
+    return nullptr;
+  }
+
   if (msg->tag() != kSCFG) {
     QUIC_LOG(WARNING) << "Server config message has tag " << msg->tag()
-                      << " expected " << kSCFG;
+                      << ", but expected " << kSCFG;
     return nullptr;
   }