QUIC Crypto - return the reasons for reject message. Reject reason

contains failures due to invalid source address token client nonce and
server nonce. Will add UMA histogram in chrome when this CL is merged.

Send reject reason to client for debugging purposes. Protected behind
new flags: FLAGS_use_early_return_when_verifying_chlo and
FLAGS_send_quic_crypto_reject_reason

Merge internal change: 69227401

R=jar@chromium.org, wtc@chromium.org, avd@google.com

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

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@278523 0039d316-1c4b-4281-b951-d872f2087c98

net/quic/crypto/crypto_protocol.h

@@ -110,6 +110,9 @@ const QuicTag kRSEQ = TAG('R', 'S', 'E', 'Q');   // Rejected sequence number
 // Universal tags
 const QuicTag kPAD  = TAG('P', 'A', 'D', '\0');  // Padding
 
+// Reasons for server sending rejection message tag.
+const QuicTag kRREJ = TAG('R', 'R', 'E', 'J');
+
 // These tags have a special form so that they appear either at the beginning
 // or the end of a handshake message. Since handshake messages are sorted by
 // tag value, the tags with 0 at the end will sort first and those with 255 at

net/quic/crypto/crypto_server_test.cc

@@ -2,12 +2,16 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
+#include <ostream>
+#include <vector>
+
 #include "base/basictypes.h"
 #include "base/strings/string_number_conversions.h"
 #include "crypto/secure_hash.h"
 #include "net/quic/crypto/crypto_utils.h"
 #include "net/quic/crypto/quic_crypto_server_config.h"
 #include "net/quic/crypto/quic_random.h"
+#include "net/quic/quic_flags.h"
 #include "net/quic/quic_socket_address_coder.h"
 #include "net/quic/quic_utils.h"
 #include "net/quic/test_tools/crypto_test_utils.h"
@@ -18,7 +22,9 @@
 #include "testing/gtest/include/gtest/gtest.h"
 
 using base::StringPiece;
+using std::ostream;
 using std::string;
+using std::vector;
 
 namespace net {
 namespace test {
@@ -36,7 +42,40 @@ class QuicCryptoServerConfigPeer {
   QuicCryptoServerConfig* server_config_;
 };
 
-class CryptoServerTest : public ::testing::Test {
+// Run tests with combinations of
+// {FLAGS_use_early_return_when_verifying_chlo,
+//  FLAGS_send_quic_crypto_reject_reason}.
+struct TestParams {
+  TestParams(bool use_early_return_when_verifying_chlo,
+             bool send_quic_crypto_reject_reason)
+      : use_early_return_when_verifying_chlo(
+            use_early_return_when_verifying_chlo),
+        send_quic_crypto_reject_reason(send_quic_crypto_reject_reason) {
+  }
+
+  friend ostream& operator<<(ostream& os, const TestParams& p) {
+    os << "{ use_early_return_when_verifying_chlo: "
+       << p.use_early_return_when_verifying_chlo
+       << " send_quic_crypto_reject_reason: "
+       << p.send_quic_crypto_reject_reason << " }";
+    return os;
+  }
+
+  bool use_early_return_when_verifying_chlo;
+  bool send_quic_crypto_reject_reason;
+};
+
+// Constructs various test permutations.
+vector<TestParams> GetTestParams() {
+  vector<TestParams> params;
+  params.push_back(TestParams(false, false));
+  params.push_back(TestParams(false, true));
+  params.push_back(TestParams(true, false));
+  params.push_back(TestParams(true, true));
+  return params;
+}
+
+class CryptoServerTest : public ::testing::TestWithParam<TestParams> {
  public:
   CryptoServerTest()
       : rand_(QuicRandom::GetInstance()),
@@ -46,6 +85,11 @@ class CryptoServerTest : public ::testing::Test {
     supported_versions_ = QuicSupportedVersions();
     client_version_ = QuicUtils::TagToString(
         QuicVersionToQuicTag(supported_versions_.front()));
+
+    FLAGS_use_early_return_when_verifying_chlo =
+        GetParam().use_early_return_when_verifying_chlo;
+    FLAGS_send_quic_crypto_reject_reason =
+        GetParam().send_quic_crypto_reject_reason;
   }
 
   virtual void SetUp() {
@@ -78,6 +122,10 @@ class CryptoServerTest : public ::testing::Test {
     // The message should be rejected because the source-address token is
     // missing.
     ASSERT_EQ(kREJ, out_.tag());
+    const HandshakeFailureReason kRejectReasons[] = {
+      SERVER_CONFIG_INCHOATE_HELLO_FAILURE
+    };
+    CheckRejectReasons(kRejectReasons, arraysize(kRejectReasons));
 
     StringPiece srct;
     ASSERT_TRUE(out_.GetStringPiece(kSourceAddressTokenTag, &srct));
@@ -219,6 +267,29 @@ class CryptoServerTest : public ::testing::Test {
     return nonce;
   }
 
+  void CheckRejectReasons(
+      const HandshakeFailureReason* expected_handshake_failures,
+      size_t expected_count) {
+    const QuicTag* reject_reason_tags;
+    size_t num_reject_reasons;
+    QuicErrorCode error_code = out_.GetTaglist(kRREJ, &reject_reason_tags,
+                                               &num_reject_reasons);
+    if (!FLAGS_send_quic_crypto_reject_reason) {
+      ASSERT_EQ(QUIC_CRYPTO_MESSAGE_PARAMETER_NOT_FOUND, error_code);
+      return;
+    }
+    ASSERT_EQ(QUIC_NO_ERROR, error_code);
+
+    if (FLAGS_use_early_return_when_verifying_chlo) {
+      EXPECT_EQ(1u, num_reject_reasons);
+    } else {
+      EXPECT_EQ(expected_count, num_reject_reasons);
+    }
+    for (size_t i = 0; i < num_reject_reasons; ++i) {
+      EXPECT_EQ(expected_handshake_failures[i], reject_reason_tags[i]);
+    }
+  }
+
  protected:
   QuicRandom* const rand_;
   MockClock clock_;
@@ -237,7 +308,14 @@ class CryptoServerTest : public ::testing::Test {
   scoped_ptr<CryptoHandshakeMessage> server_config_;
 };
 
-TEST_F(CryptoServerTest, BadSNI) {
+// Run all CryptoServerTest with all combinations of
+// FLAGS_use_early_return_when_verifying_chlo and
+// FLAGS_send_quic_crypto_reject_reason.
+INSTANTIATE_TEST_CASE_P(CryptoServerTests,
+                        CryptoServerTest,
+                        ::testing::ValuesIn(GetTestParams()));
+
+TEST_P(CryptoServerTest, BadSNI) {
   static const char* kBadSNIs[] = {
     "",
     "foo",
@@ -256,6 +334,10 @@ TEST_F(CryptoServerTest, BadSNI) {
         "SNI", kBadSNIs[i],
         "VER\0", client_version.data(),
         NULL));
+    const HandshakeFailureReason kRejectReasons[] = {
+      SERVER_CONFIG_INCHOATE_HELLO_FAILURE
+    };
+    CheckRejectReasons(kRejectReasons, arraysize(kRejectReasons));
   }
 }
 
@@ -281,16 +363,24 @@ TEST_F(CryptoServerTest, DISABLED_DefaultCert) {
   EXPECT_TRUE(out_.GetStringPiece(kPROF, &proof));
   EXPECT_NE(0u, cert.size());
   EXPECT_NE(0u, proof.size());
+  const HandshakeFailureReason kRejectReasons[] = {
+    CLIENT_NONCE_UNKNOWN_FAILURE
+  };
+  CheckRejectReasons(kRejectReasons, arraysize(kRejectReasons));
 }
 
-TEST_F(CryptoServerTest, TooSmall) {
+TEST_P(CryptoServerTest, TooSmall) {
   ShouldFailMentioning("too small", CryptoTestUtils::Message(
         "CHLO",
         "VER\0", client_version_.data(),
         NULL));
+  const HandshakeFailureReason kRejectReasons[] = {
+    SERVER_CONFIG_INCHOATE_HELLO_FAILURE
+  };
+  CheckRejectReasons(kRejectReasons, arraysize(kRejectReasons));
 }
 
-TEST_F(CryptoServerTest, BadSourceAddressToken) {
+TEST_P(CryptoServerTest, BadSourceAddressToken) {
   // Invalid source-address tokens should be ignored.
   static const char* kBadSourceAddressTokens[] = {
     "",
@@ -305,10 +395,14 @@ TEST_F(CryptoServerTest, BadSourceAddressToken) {
         "STK", kBadSourceAddressTokens[i],
         "VER\0", client_version_.data(),
         NULL));
+    const HandshakeFailureReason kRejectReasons[] = {
+      SERVER_CONFIG_INCHOATE_HELLO_FAILURE
+    };
+    CheckRejectReasons(kRejectReasons, arraysize(kRejectReasons));
   }
 }
 
-TEST_F(CryptoServerTest, BadClientNonce) {
+TEST_P(CryptoServerTest, BadClientNonce) {
   // Invalid nonces should be ignored.
   static const char* kBadNonces[] = {
     "",
@@ -322,10 +416,14 @@ TEST_F(CryptoServerTest, BadClientNonce) {
         "NONC", kBadNonces[i],
         "VER\0", client_version_.data(),
         NULL));
+    const HandshakeFailureReason kRejectReasons[] = {
+      SERVER_CONFIG_INCHOATE_HELLO_FAILURE
+    };
+    CheckRejectReasons(kRejectReasons, arraysize(kRejectReasons));
   }
 }
 
-TEST_F(CryptoServerTest, DowngradeAttack) {
+TEST_P(CryptoServerTest, DowngradeAttack) {
   if (supported_versions_.size() == 1) {
     // No downgrade attack is possible if the server only supports one version.
     return;
@@ -339,9 +437,101 @@ TEST_F(CryptoServerTest, DowngradeAttack) {
       "CHLO",
       "VER\0", bad_version.data(),
       NULL));
+  const HandshakeFailureReason kRejectReasons[] = {
+    SERVER_CONFIG_INCHOATE_HELLO_FAILURE
+  };
+  CheckRejectReasons(kRejectReasons, arraysize(kRejectReasons));
+}
+
+TEST_P(CryptoServerTest, CorruptServerConfig) {
+  // This tests corrupted server config.
+  CryptoHandshakeMessage msg = CryptoTestUtils::Message(
+      "CHLO",
+      "AEAD", "AESG",
+      "KEXS", "C255",
+      "SCID", (string(1, 'X') + scid_hex_).c_str(),
+      "#004b5453", srct_hex_.c_str(),
+      "PUBS", pub_hex_.c_str(),
+      "NONC", nonce_hex_.c_str(),
+      "VER\0", client_version_.data(),
+      "$padding", static_cast<int>(kClientHelloMinimumSize),
+      NULL);
+  ShouldSucceed(msg);
+  ASSERT_EQ(kREJ, out_.tag());
+  const HandshakeFailureReason kRejectReasons[] = {
+    SERVER_CONFIG_UNKNOWN_CONFIG_FAILURE
+  };
+  CheckRejectReasons(kRejectReasons, arraysize(kRejectReasons));
+}
+
+TEST_P(CryptoServerTest, CorruptSourceAddressToken) {
+  // This tests corrupted source address token.
+  CryptoHandshakeMessage msg = CryptoTestUtils::Message(
+      "CHLO",
+      "AEAD", "AESG",
+      "KEXS", "C255",
+      "SCID", scid_hex_.c_str(),
+      "#004b5453", (string(1, 'X') + srct_hex_).c_str(),
+      "PUBS", pub_hex_.c_str(),
+      "NONC", nonce_hex_.c_str(),
+      "VER\0", client_version_.data(),
+      "$padding", static_cast<int>(kClientHelloMinimumSize),
+      NULL);
+  ShouldSucceed(msg);
+  ASSERT_EQ(kREJ, out_.tag());
+  const HandshakeFailureReason kRejectReasons[] = {
+    SOURCE_ADDRESS_TOKEN_DECRYPTION_FAILURE
+  };
+  CheckRejectReasons(kRejectReasons, arraysize(kRejectReasons));
+}
+
+TEST_P(CryptoServerTest, CorruptClientNonceAndSourceAddressToken) {
+  // This test corrupts client nonce and source address token.
+  CryptoHandshakeMessage msg = CryptoTestUtils::Message(
+      "CHLO",
+      "AEAD", "AESG",
+      "KEXS", "C255",
+      "SCID", scid_hex_.c_str(),
+      "#004b5453", (string(1, 'X') + srct_hex_).c_str(),
+      "PUBS", pub_hex_.c_str(),
+      "NONC", (string(1, 'X') + nonce_hex_).c_str(),
+      "VER\0", client_version_.data(),
+      "$padding", static_cast<int>(kClientHelloMinimumSize),
+      NULL);
+  ShouldSucceed(msg);
+  ASSERT_EQ(kREJ, out_.tag());
+  const HandshakeFailureReason kRejectReasons[] = {
+    SOURCE_ADDRESS_TOKEN_DECRYPTION_FAILURE,
+    CLIENT_NONCE_INVALID_FAILURE
+  };
+  CheckRejectReasons(kRejectReasons, arraysize(kRejectReasons));
+}
+
+TEST_P(CryptoServerTest, CorruptMultipleTags) {
+  // This test corrupts client nonce, server nonce and source address token.
+  CryptoHandshakeMessage msg = CryptoTestUtils::Message(
+      "CHLO",
+      "AEAD", "AESG",
+      "KEXS", "C255",
+      "SCID", scid_hex_.c_str(),
+      "#004b5453", (string(1, 'X') + srct_hex_).c_str(),
+      "PUBS", pub_hex_.c_str(),
+      "NONC", (string(1, 'X') + nonce_hex_).c_str(),
+      "SNO\0", (string(1, 'X') + nonce_hex_).c_str(),
+      "VER\0", client_version_.data(),
+      "$padding", static_cast<int>(kClientHelloMinimumSize),
+      NULL);
+  ShouldSucceed(msg);
+  ASSERT_EQ(kREJ, out_.tag());
+  const HandshakeFailureReason kRejectReasons[] = {
+    SOURCE_ADDRESS_TOKEN_DECRYPTION_FAILURE,
+    CLIENT_NONCE_INVALID_FAILURE,
+    SERVER_NONCE_DECRYPTION_FAILURE,
+  };
+  CheckRejectReasons(kRejectReasons, arraysize(kRejectReasons));
 }
 
-TEST_F(CryptoServerTest, ReplayProtection) {
+TEST_P(CryptoServerTest, ReplayProtection) {
   // This tests that disabling replay protection works.
   CryptoHandshakeMessage msg = CryptoTestUtils::Message(
       "CHLO",
@@ -359,6 +549,11 @@ TEST_F(CryptoServerTest, ReplayProtection) {
   // quiescent.
   ASSERT_EQ(kREJ, out_.tag());
 
+  const HandshakeFailureReason kRejectReasons[] = {
+    CLIENT_NONCE_UNKNOWN_FAILURE
+  };
+  CheckRejectReasons(kRejectReasons, arraysize(kRejectReasons));
+
   config_.set_replay_protection(false);
 
   ShouldSucceed(msg);
@@ -449,11 +644,16 @@ class CryptoServerTestNoConfig : public CryptoServerTest {
   }
 };
 
-TEST_F(CryptoServerTestNoConfig, DontCrash) {
+TEST_P(CryptoServerTestNoConfig, DontCrash) {
   ShouldFailMentioning("No config", InchoateClientHello(
       "CHLO",
       "VER\0", client_version_.data(),
       NULL));
+
+  const HandshakeFailureReason kRejectReasons[] = {
+    CLIENT_NONCE_UNKNOWN_FAILURE
+  };
+  CheckRejectReasons(kRejectReasons, arraysize(kRejectReasons));
 }
 
 class AsyncStrikeServerVerificationTest : public CryptoServerTest {
@@ -478,7 +678,7 @@ class AsyncStrikeServerVerificationTest : public CryptoServerTest {
   DelayedVerifyStrikeRegisterClient* strike_register_client_;
 };
 
-TEST_F(AsyncStrikeServerVerificationTest, AsyncReplayProtection) {
+TEST_P(AsyncStrikeServerVerificationTest, AsyncReplayProtection) {
   // This tests async validation with a strike register works.
   CryptoHandshakeMessage msg = CryptoTestUtils::Message(
       "CHLO",

net/quic/crypto/quic_crypto_client_config.cc

@@ -587,6 +587,17 @@ QuicErrorCode QuicCryptoClientConfig::ProcessRejection(
     }
   }
 
+  const QuicTag* reject_reasons;
+  size_t num_reject_reasons;
+  if (rej.GetTaglist(kRREJ, &reject_reasons,
+                     &num_reject_reasons) == QUIC_NO_ERROR) {
+#if defined(DEBUG)
+    for (size_t i = 0; i < num_reject_reasons; ++i) {
+      DVLOG(1) << "Reasons for rejection: " << reject_reasons[i];
+    }
+#endif
+  }
+
   return QUIC_NO_ERROR;
 }
 

net/quic/crypto/quic_crypto_server_config.cc

@@ -33,6 +33,7 @@
 #include "net/quic/crypto/strike_register.h"
 #include "net/quic/crypto/strike_register_client.h"
 #include "net/quic/quic_clock.h"
+#include "net/quic/quic_flags.h"
 #include "net/quic/quic_protocol.h"
 #include "net/quic/quic_socket_address_coder.h"
 #include "net/quic/quic_utils.h"
@@ -81,6 +82,9 @@ struct ClientHelloInfo {
   StringPiece client_nonce;
   StringPiece server_nonce;
   StringPiece user_agent_id;
+
+  // Errors from EvaluateClientHello.
+  vector<uint32> reject_reasons;
 };
 
 struct ValidateClientHelloResultCallback::Result {
@@ -146,6 +150,12 @@ class VerifyNonceIsValidAndUniqueCallback
   virtual void RunImpl(bool nonce_is_valid_and_unique) OVERRIDE {
     DVLOG(1) << "Using client nonce, unique: " << nonce_is_valid_and_unique;
     result_->info.unique = nonce_is_valid_and_unique;
+    // TODO(rtenneti): Implement capturing of error from strike register.
+    // Temporarily treat them as CLIENT_NONCE_UNKNOWN_FAILURE.
+    if (!nonce_is_valid_and_unique) {
+      result_->info.reject_reasons.push_back(
+          static_cast<uint32>(CLIENT_NONCE_UNKNOWN_FAILURE));
+    }
     done_cb_->Run(result_);
   }
 
@@ -888,32 +898,65 @@ void QuicCryptoServerConfig::EvaluateClientHello(
 
   client_hello.GetStringPiece(kUAID, &info->user_agent_id);
 
+  if (!requested_config.get()) {
+    StringPiece requested_scid;
+    if (client_hello.GetStringPiece(kSCID, &requested_scid)) {
+      info->reject_reasons.push_back(
+          static_cast<uint32>(SERVER_CONFIG_UNKNOWN_CONFIG_FAILURE));
+    } else {
+      info->reject_reasons.push_back(
+          static_cast<uint32>(SERVER_CONFIG_INCHOATE_HELLO_FAILURE));
+    }
+    // No server config with the requested ID.
+    helper.ValidationComplete(QUIC_NO_ERROR, "");
+    return;
+  }
+
+  HandshakeFailureReason source_address_token_error;
   StringPiece srct;
-  if (requested_config.get() != NULL &&
-      client_hello.GetStringPiece(kSourceAddressTokenTag, &srct) &&
+  if (client_hello.GetStringPiece(kSourceAddressTokenTag, &srct)) {
+    source_address_token_error =
         ValidateSourceAddressToken(*requested_config,
                                    srct,
                                    info->client_ip,
-                                 info->now)) {
-    info->valid_source_address_token = true;
+                                   info->now);
+    info->valid_source_address_token =
+        (source_address_token_error == HANDSHAKE_OK);
   } else {
-    // No server config with the requested ID, or no valid source address token.
+    source_address_token_error = SOURCE_ADDRESS_TOKEN_INVALID_FAILURE;
+  }
+
+  bool found_error = false;
+  if (source_address_token_error != HANDSHAKE_OK) {
+    info->reject_reasons.push_back(
+        static_cast<uint32>(source_address_token_error));
+    // No valid source address token.
+    if (FLAGS_use_early_return_when_verifying_chlo) {
       helper.ValidationComplete(QUIC_NO_ERROR, "");
       return;
     }
+    found_error = true;
+  }
 
   if (client_hello.GetStringPiece(kNONC, &info->client_nonce) &&
       info->client_nonce.size() == kNonceSize) {
     info->client_nonce_well_formed = true;
   } else {
+    info->reject_reasons.push_back(
+        static_cast<uint32>(CLIENT_NONCE_INVALID_FAILURE));
     // Invalid client nonce.
     DVLOG(1) << "Invalid client nonce.";
+    if (FLAGS_use_early_return_when_verifying_chlo) {
       helper.ValidationComplete(QUIC_NO_ERROR, "");
       return;
     }
+    found_error = true;
+  }
 
   if (!replay_protection_) {
+    if (!found_error) {
       info->unique = true;
+    }
     DVLOG(1) << "No replay protection.";
     helper.ValidationComplete(QUIC_NO_ERROR, "");
     return;
@@ -922,12 +965,26 @@ void QuicCryptoServerConfig::EvaluateClientHello(
   client_hello.GetStringPiece(kServerNonceTag, &info->server_nonce);
   if (!info->server_nonce.empty()) {
     // If the server nonce is present, use it to establish uniqueness.
-    info->unique = ValidateServerNonce(info->server_nonce, info->now);
+    HandshakeFailureReason server_nonce_error =
+        ValidateServerNonce(info->server_nonce, info->now);
+    if (server_nonce_error == HANDSHAKE_OK) {
+      info->unique = true;
+    } else {
+      info->reject_reasons.push_back(static_cast<uint32>(server_nonce_error));
+      info->unique = false;
+    }
     DVLOG(1) << "Using server nonce, unique: " << info->unique;
     helper.ValidationComplete(QUIC_NO_ERROR, "");
     return;
   }
 
+  // We want to contact strike register if there are no errors because it is
+  // a RPC call and is expensive.
+  if (found_error) {
+    helper.ValidationComplete(QUIC_NO_ERROR, "");
+    return;
+  }
+
   // Use the client nonce to establish uniqueness.
   StrikeRegisterClient* strike_register_client;
   {
@@ -971,6 +1028,12 @@ void QuicCryptoServerConfig::BuildRejection(
     out->SetStringPiece(kServerNonceTag, NewServerNonce(rand, info.now));
   }
 
+  if (FLAGS_send_quic_crypto_reject_reason) {
+    // Send client the reject reason for debugging purposes.
+    DCHECK_LT(0u, info.reject_reasons.size());
+    out->SetVector(kRREJ, info.reject_reasons);
+  }
+
   // The client may have requested a certificate chain.
   const QuicTag* their_proof_demands;
   size_t num_their_proof_demands;
@@ -1288,7 +1351,7 @@ string QuicCryptoServerConfig::NewSourceAddressToken(const Config& config,
       rand, source_address_token.SerializeAsString());
 }
 
-bool QuicCryptoServerConfig::ValidateSourceAddressToken(
+HandshakeFailureReason QuicCryptoServerConfig::ValidateSourceAddressToken(
     const Config& config,
     StringPiece token,
     const IPEndPoint& ip,
@@ -1296,13 +1359,13 @@ bool QuicCryptoServerConfig::ValidateSourceAddressToken(
   string storage;
   StringPiece plaintext;
   if (!config.source_address_token_boxer->Unbox(token, &storage, &plaintext)) {
-    return false;
+    return SOURCE_ADDRESS_TOKEN_DECRYPTION_FAILURE;
   }
 
   SourceAddressToken source_address_token;
   if (!source_address_token.ParseFromArray(plaintext.data(),
                                            plaintext.size())) {
-    return false;
+    return SOURCE_ADDRESS_TOKEN_PARSE_FAILURE;
   }
 
   IPAddressNumber ip_address = ip.address();
@@ -1311,7 +1374,7 @@ bool QuicCryptoServerConfig::ValidateSourceAddressToken(
   }
   if (source_address_token.ip() != IPAddressToPackedString(ip_address)) {
     // It's for a different IP address.
-    return false;
+    return SOURCE_ADDRESS_TOKEN_DIFFERENT_IP_ADDRESS_FAILURE;
   }
 
   const QuicWallTime timestamp(
@@ -1320,15 +1383,15 @@ bool QuicCryptoServerConfig::ValidateSourceAddressToken(
 
   if (now.IsBefore(timestamp) &&
       delta.ToSeconds() > source_address_token_future_secs_) {
-    return false;
+    return SOURCE_ADDRESS_TOKEN_CLOCK_SKEW_FAILURE;
   }
 
   if (now.IsAfter(timestamp) &&
       delta.ToSeconds() > source_address_token_lifetime_secs_) {
-    return false;
+    return SOURCE_ADDRESS_TOKEN_EXPIRED_FAILURE;
   }
 
-  return true;
+  return HANDSHAKE_OK;
 }
 
 // kServerNoncePlaintextSize is the number of bytes in an unencrypted server
@@ -1354,12 +1417,13 @@ string QuicCryptoServerConfig::NewServerNonce(QuicRandom* rand,
       StringPiece(reinterpret_cast<char*>(server_nonce), sizeof(server_nonce)));
 }
 
-bool QuicCryptoServerConfig::ValidateServerNonce(StringPiece token,
+HandshakeFailureReason QuicCryptoServerConfig::ValidateServerNonce(
+    StringPiece token,
     QuicWallTime now) const {
   string storage;
   StringPiece plaintext;
   if (!server_nonce_boxer_.Unbox(token, &storage, &plaintext)) {
-    return false;
+    return SERVER_NONCE_DECRYPTION_FAILURE;
   }
 
   // plaintext contains:
@@ -1369,7 +1433,7 @@ bool QuicCryptoServerConfig::ValidateServerNonce(StringPiece token,
   if (plaintext.size() != kServerNoncePlaintextSize) {
     // This should never happen because the value decrypted correctly.
     LOG(DFATAL) << "Seemingly valid server nonce had incorrect length.";
-    return false;
+    return SERVER_NONCE_INVALID_FAILURE;
   }
 
   uint8 server_nonce[32];
@@ -1394,7 +1458,7 @@ bool QuicCryptoServerConfig::ValidateServerNonce(StringPiece token,
         server_nonce, static_cast<uint32>(now.ToUNIXSeconds()));
   }
 
-  return is_unique;
+  return is_unique ? HANDSHAKE_OK : SERVER_NONCE_NOT_UNIQUE_FAILURE;
 }
 
 QuicCryptoServerConfig::Config::Config()

net/quic/crypto/quic_crypto_server_config.h

@@ -40,6 +40,32 @@ namespace test {
 class QuicCryptoServerConfigPeer;
 }  // namespace test
 
+enum HandshakeFailureReason {
+  HANDSHAKE_OK = 0,
+
+  // Failure reasons for an invalid client nonce.
+  // TODO(rtenneti): Implement capturing of error from strike register.
+  CLIENT_NONCE_UNKNOWN_FAILURE = 100,
+  CLIENT_NONCE_INVALID_FAILURE,
+
+  // Failure reasons for an invalid server nonce.
+  SERVER_NONCE_INVALID_FAILURE = 200,
+  SERVER_NONCE_DECRYPTION_FAILURE,
+  SERVER_NONCE_NOT_UNIQUE_FAILURE,
+
+  // Failure reasons for an invalid server config.
+  SERVER_CONFIG_INCHOATE_HELLO_FAILURE = 300,
+  SERVER_CONFIG_UNKNOWN_CONFIG_FAILURE,
+
+  // Failure reasons for an invalid source adddress token.
+  SOURCE_ADDRESS_TOKEN_INVALID_FAILURE = 400,
+  SOURCE_ADDRESS_TOKEN_DECRYPTION_FAILURE,
+  SOURCE_ADDRESS_TOKEN_PARSE_FAILURE,
+  SOURCE_ADDRESS_TOKEN_DIFFERENT_IP_ADDRESS_FAILURE,
+  SOURCE_ADDRESS_TOKEN_CLOCK_SKEW_FAILURE,
+  SOURCE_ADDRESS_TOKEN_EXPIRED_FAILURE,
+};
+
 // Hook that allows application code to subscribe to primary config changes.
 class PrimaryConfigChangedCallback {
  public:
@@ -385,10 +411,10 @@ class NET_EXPORT_PRIVATE QuicCryptoServerConfig {
                                     QuicRandom* rand,
                                     QuicWallTime now) const;
 
-  // ValidateSourceAddressToken returns true if the source address token in
-  // |token| is a valid and timely token for the IP address |ip| given that the
-  // current time is |now|.
-  bool ValidateSourceAddressToken(const Config& config,
+  // ValidateSourceAddressToken returns HANDSHAKE_OK if the source address token
+  // in |token| is a valid and timely token for the IP address |ip| given that
+  // the current time is |now|. Otherwise it returns the reason for failure.
+  HandshakeFailureReason ValidateSourceAddressToken(const Config& config,
                                                     base::StringPiece token,
                                                     const IPEndPoint& ip,
                                                     QuicWallTime now) const;
@@ -399,9 +425,10 @@ class NET_EXPORT_PRIVATE QuicCryptoServerConfig {
   // ValidateServerNonce decrypts |token| and verifies that it hasn't been
   // previously used and is recent enough that it is plausible that it was part
   // of a very recently provided rejection ("recent" will be on the order of
-  // 10-30 seconds). If so, it records that it has been used and returns true.
-  // Otherwise it returns false.
-  bool ValidateServerNonce(base::StringPiece echoed_server_nonce,
+  // 10-30 seconds). If so, it records that it has been used and returns
+  // HANDSHAKE_OK. Otherwise it returns the reason for failure.
+  HandshakeFailureReason ValidateServerNonce(
+      base::StringPiece echoed_server_nonce,
       QuicWallTime now) const;
 
   // replay_protection_ controls whether the server enforces that handshakes

net/quic/crypto/quic_crypto_server_config_test.cc

@@ -9,6 +9,7 @@
 #include "base/stl_util.h"
 #include "net/quic/crypto/aes_128_gcm_12_encrypter.h"
 #include "net/quic/crypto/crypto_handshake_message.h"
+#include "net/quic/crypto/crypto_secret_boxer.h"
 #include "net/quic/crypto/crypto_server_config_protobuf.h"
 #include "net/quic/crypto/quic_random.h"
 #include "net/quic/crypto/strike_register_client.h"
@@ -58,7 +59,7 @@ class QuicCryptoServerConfigPeer {
         *GetConfig(config_id), ip, rand, now);
   }
 
-  bool ValidateSourceAddressToken(string config_id,
+  HandshakeFailureReason ValidateSourceAddressToken(string config_id,
                                                     StringPiece srct,
                                                     IPEndPoint ip,
                                                     QuicWallTime now) {
@@ -66,6 +67,15 @@ class QuicCryptoServerConfigPeer {
         *GetConfig(config_id), srct, ip, now);
   }
 
+  string NewServerNonce(QuicRandom* rand, QuicWallTime now) const {
+    return server_config_->NewServerNonce(rand, now);
+  }
+
+  HandshakeFailureReason ValidateServerNonce(StringPiece token,
+                                             QuicWallTime now) {
+    return server_config_->ValidateServerNonce(token, now);
+  }
+
   base::Lock* GetStrikeRegisterClientLock() {
     return &server_config_->strike_register_client_lock_;
   }
@@ -270,41 +280,84 @@ TEST(QuicCryptoServerConfigTest, SourceAddressTokens) {
   const string token4 = peer.NewSourceAddressToken(kPrimary, ip4, rand, now);
   const string token4d = peer.NewSourceAddressToken(kPrimary, ip4d, rand, now);
   const string token6 = peer.NewSourceAddressToken(kPrimary, ip6, rand, now);
-  EXPECT_TRUE(peer.ValidateSourceAddressToken(kPrimary, token4, ip4, now));
-  EXPECT_TRUE(peer.ValidateSourceAddressToken(kPrimary, token4, ip4d, now));
-  EXPECT_FALSE(peer.ValidateSourceAddressToken(kPrimary, token4, ip6, now));
-  EXPECT_TRUE(peer.ValidateSourceAddressToken(kPrimary, token4d, ip4, now));
-  EXPECT_TRUE(peer.ValidateSourceAddressToken(kPrimary, token4d, ip4d, now));
-  EXPECT_FALSE(peer.ValidateSourceAddressToken(kPrimary, token4d, ip6, now));
-  EXPECT_TRUE(peer.ValidateSourceAddressToken(kPrimary, token6, ip6, now));
+  EXPECT_EQ(HANDSHAKE_OK, peer.ValidateSourceAddressToken(
+      kPrimary, token4, ip4, now));
+  DCHECK_EQ(HANDSHAKE_OK, peer.ValidateSourceAddressToken(
+      kPrimary, token4, ip4d, now));
+  DCHECK_EQ(SOURCE_ADDRESS_TOKEN_DIFFERENT_IP_ADDRESS_FAILURE,
+            peer.ValidateSourceAddressToken(kPrimary, token4, ip6, now));
+  DCHECK_EQ(HANDSHAKE_OK, peer.ValidateSourceAddressToken(
+      kPrimary, token4d, ip4, now));
+  DCHECK_EQ(HANDSHAKE_OK, peer.ValidateSourceAddressToken(
+      kPrimary, token4d, ip4d, now));
+  DCHECK_EQ(SOURCE_ADDRESS_TOKEN_DIFFERENT_IP_ADDRESS_FAILURE,
+            peer.ValidateSourceAddressToken(kPrimary, token4d, ip6, now));
+  DCHECK_EQ(HANDSHAKE_OK, peer.ValidateSourceAddressToken(
+      kPrimary, token6, ip6, now));
 
   // Override config generates configs that validate successfully.
   const string override_token4 = peer.NewSourceAddressToken(
       kOverride, ip4, rand, now);
   const string override_token6 = peer.NewSourceAddressToken(
       kOverride, ip6, rand, now);
-  EXPECT_TRUE(peer.ValidateSourceAddressToken(
+  DCHECK_EQ(HANDSHAKE_OK, peer.ValidateSourceAddressToken(
       kOverride, override_token4, ip4, now));
-  EXPECT_FALSE(peer.ValidateSourceAddressToken(
-      kOverride, override_token4, ip6, now));
-  EXPECT_TRUE(peer.ValidateSourceAddressToken(
+  DCHECK_EQ(SOURCE_ADDRESS_TOKEN_DIFFERENT_IP_ADDRESS_FAILURE,
+            peer.ValidateSourceAddressToken(kOverride, override_token4, ip6,
+                                            now));
+  DCHECK_EQ(HANDSHAKE_OK, peer.ValidateSourceAddressToken(
       kOverride, override_token6, ip6, now));
 
   // Tokens generated by the primary config do not validate
   // successfully against the override config, and vice versa.
-  EXPECT_FALSE(peer.ValidateSourceAddressToken(kOverride, token4, ip4, now));
-  EXPECT_FALSE(peer.ValidateSourceAddressToken(kOverride, token6, ip6, now));
-  EXPECT_FALSE(peer.ValidateSourceAddressToken(
-      kPrimary, override_token4, ip4, now));
-  EXPECT_FALSE(peer.ValidateSourceAddressToken(
-      kPrimary, override_token6, ip6, now));
+  DCHECK_EQ(SOURCE_ADDRESS_TOKEN_DECRYPTION_FAILURE,
+            peer.ValidateSourceAddressToken(kOverride, token4, ip4, now));
+  DCHECK_EQ(SOURCE_ADDRESS_TOKEN_DECRYPTION_FAILURE,
+            peer.ValidateSourceAddressToken(kOverride, token6, ip6, now));
+  DCHECK_EQ(SOURCE_ADDRESS_TOKEN_DECRYPTION_FAILURE,
+            peer.ValidateSourceAddressToken(kPrimary, override_token4, ip4,
+                                            now));
+  DCHECK_EQ(SOURCE_ADDRESS_TOKEN_DECRYPTION_FAILURE,
+            peer.ValidateSourceAddressToken(kPrimary, override_token6, ip6,
+                                            now));
 
   // Validation fails after tokens expire.
   now = original_time.Add(QuicTime::Delta::FromSeconds(86400 * 7));
-  EXPECT_FALSE(peer.ValidateSourceAddressToken(kPrimary, token4, ip4, now));
+  DCHECK_EQ(SOURCE_ADDRESS_TOKEN_EXPIRED_FAILURE,
+            peer.ValidateSourceAddressToken(kPrimary, token4, ip4, now));
 
   now = original_time.Subtract(QuicTime::Delta::FromSeconds(3600 * 2));
-  EXPECT_FALSE(peer.ValidateSourceAddressToken(kPrimary, token4, ip4, now));
+  DCHECK_EQ(SOURCE_ADDRESS_TOKEN_CLOCK_SKEW_FAILURE,
+            peer.ValidateSourceAddressToken(kPrimary, token4, ip4, now));
+}
+
+TEST(QuicCryptoServerConfigTest, ValidateServerNonce) {
+  QuicRandom* rand = QuicRandom::GetInstance();
+  QuicCryptoServerConfig server(QuicCryptoServerConfig::TESTING, rand);
+  QuicCryptoServerConfigPeer peer(&server);
+
+  StringPiece message("hello world");
+  const size_t key_size = CryptoSecretBoxer::GetKeySize();
+  scoped_ptr<uint8[]> key(new uint8[key_size]);
+  memset(key.get(), 0x11, key_size);
+
+  CryptoSecretBoxer boxer;
+  boxer.SetKey(StringPiece(reinterpret_cast<char*>(key.get()), key_size));
+  const string box = boxer.Box(rand, message);
+  MockClock clock;
+  QuicWallTime now = clock.WallNow();
+  const QuicWallTime original_time = now;
+  EXPECT_EQ(SERVER_NONCE_DECRYPTION_FAILURE,
+            peer.ValidateServerNonce(box, now));
+
+  string server_nonce = peer.NewServerNonce(rand, now);
+  EXPECT_EQ(HANDSHAKE_OK, peer.ValidateServerNonce(server_nonce, now));
+  EXPECT_EQ(SERVER_NONCE_NOT_UNIQUE_FAILURE,
+            peer.ValidateServerNonce(server_nonce, now));
+
+  now = original_time.Add(QuicTime::Delta::FromSeconds(1000 * 7));
+  server_nonce = peer.NewServerNonce(rand, now);
+  EXPECT_EQ(HANDSHAKE_OK, peer.ValidateServerNonce(server_nonce, now));
 }
 
 class CryptoServerConfigsTest : public ::testing::Test {

net/quic/quic_flags.cc

@@ -39,3 +39,10 @@ bool FLAGS_quic_use_time_loss_detection = false;
 
 // If true, allow peer port migration of established QUIC connections.
 bool FLAGS_quic_allow_port_migration = true;
+
+// If true, it will return as soon as an error is detected while validating
+// CHLO.
+bool FLAGS_use_early_return_when_verifying_chlo = true;
+
+// If true, QUIC crypto reject message will include the reasons for rejection.
+bool FLAGS_send_quic_crypto_reject_reason = false;

net/quic/quic_flags.h

@@ -14,5 +14,7 @@ NET_EXPORT_PRIVATE extern bool FLAGS_enable_quic_connection_flow_control_2;
 NET_EXPORT_PRIVATE extern bool FLAGS_quic_allow_oversized_packets_for_test;
 NET_EXPORT_PRIVATE extern bool FLAGS_quic_use_time_loss_detection;
 NET_EXPORT_PRIVATE extern bool FLAGS_quic_allow_port_migration;
+NET_EXPORT_PRIVATE extern bool FLAGS_use_early_return_when_verifying_chlo;
+NET_EXPORT_PRIVATE extern bool FLAGS_send_quic_crypto_reject_reason;
 
 #endif  // NET_QUIC_QUIC_FLAGS_H_