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

net/quic/crypto/crypto_server_test.cc

Index: net/quic/crypto/crypto_server_test.cc
diff --git a/net/quic/crypto/crypto_server_test.cc b/net/quic/crypto/crypto_server_test.cc
index f1892d3b6ac7813f9029df6a82829ef32ccddfca..19bb67bdc15e0898852df3998a5caf2974b75e02 100644
--- a/net/quic/crypto/crypto_server_test.cc
+++ b/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;

[wtc, 2014/06/19 19:58:42]

Nit: also rename this variable "reject_reasons".

[ramant (doing other things), 2014/06/20 23:20:47]

Fixed in CL: https://codereview.chromium.org/336273006/

Done.

+    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_F(CryptoServerTest, ReplayProtection) {
+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_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) {
-    ShouldFailMentioning("No config", InchoateClientHello(
-        "CHLO",
-        "VER\0", client_version_.data(),
-        NULL));
+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",