gfe-relnote: Allow no SNI in TLS QUIC handshake, protected by quic_enable_version_t0XX flags
This allows using a QuicCryptoClientStream with TLS when the QuicServerId
has an empty hostname. It also modifies the server to expose the received
SNI in the crypto_negotiated_params.
//gfe/gfe2/quic:end_to_end_test runs with an empty servername; this change
will be needed to support TLS in that end to end test.
PiperOrigin-RevId: 285099863
Change-Id: I80e94a91824e8b53ed9fd5149a40dd63845fe9b5
diff --git a/quic/core/tls_client_handshaker.cc b/quic/core/tls_client_handshaker.cc
index a65fefc..a222c20 100644
--- a/quic/core/tls_client_handshaker.cc
+++ b/quic/core/tls_client_handshaker.cc
@@ -68,9 +68,10 @@
bool TlsClientHandshaker::CryptoConnect() {
state_ = STATE_HANDSHAKE_RUNNING;
- // Configure the SSL to be a client.
+ // Set the SNI to send, if any.
SSL_set_connect_state(ssl());
- if (SSL_set_tlsext_host_name(ssl(), server_id_.host().c_str()) != 1) {
+ if (!server_id_.host().empty() &&
+ SSL_set_tlsext_host_name(ssl(), server_id_.host().c_str()) != 1) {
return false;
}
diff --git a/quic/core/tls_handshaker_test.cc b/quic/core/tls_handshaker_test.cc
index 75f07aa..a0a3b19 100644
--- a/quic/core/tls_handshaker_test.cc
+++ b/quic/core/tls_handshaker_test.cc
@@ -17,6 +17,7 @@
#include "net/third_party/quiche/src/quic/test_tools/fake_proof_source.h"
#include "net/third_party/quiche/src/quic/test_tools/mock_quic_session_visitor.h"
#include "net/third_party/quiche/src/quic/test_tools/quic_test_utils.h"
+#include "net/third_party/quiche/src/quic/tools/fake_proof_verifier.h"
namespace quic {
namespace test {
@@ -26,9 +27,9 @@
using ::testing::ElementsAreArray;
using ::testing::Return;
-class FakeProofVerifier : public ProofVerifier {
+class TestProofVerifier : public ProofVerifier {
public:
- FakeProofVerifier()
+ TestProofVerifier()
: verifier_(crypto_test_utils::ProofVerifierForTesting()) {}
QuicAsyncStatus VerifyProof(
@@ -118,7 +119,7 @@
delegate_(delegate) {}
void Run() {
- // FakeProofVerifier depends on crypto_test_utils::ProofVerifierForTesting
+ // TestProofVerifier depends on crypto_test_utils::ProofVerifierForTesting
// running synchronously. It passes a FailingProofVerifierCallback and
// runs the original callback after asserting that the verification ran
// synchronously.
@@ -224,11 +225,18 @@
class TestQuicCryptoClientStream : public TestQuicCryptoStream {
public:
explicit TestQuicCryptoClientStream(QuicSession* session)
+ : TestQuicCryptoClientStream(session,
+ QuicServerId("test.example.com", 443),
+ std::make_unique<TestProofVerifier>()) {}
+
+ TestQuicCryptoClientStream(QuicSession* session,
+ const QuicServerId& server_id,
+ std::unique_ptr<ProofVerifier> proof_verifier)
: TestQuicCryptoStream(session),
- crypto_config_(std::make_unique<FakeProofVerifier>(),
+ crypto_config_(std::move(proof_verifier),
/*session_cache*/ nullptr),
handshaker_(new TlsClientHandshaker(
- QuicServerId("test.example.com", 443, false),
+ server_id,
this,
session,
crypto_test_utils::ProofVerifyContextForTesting(),
@@ -243,8 +251,8 @@
bool CryptoConnect() { return handshaker_->CryptoConnect(); }
- FakeProofVerifier* GetFakeProofVerifier() const {
- return static_cast<FakeProofVerifier*>(crypto_config_.proof_verifier());
+ TestProofVerifier* GetTestProofVerifier() const {
+ return static_cast<TestProofVerifier*>(crypto_config_.proof_verifier());
}
private:
@@ -425,9 +433,9 @@
TEST_F(TlsHandshakerTest, HandshakeWithAsyncProofVerifier) {
EXPECT_CALL(*client_conn_, CloseConnection(_, _, _)).Times(0);
EXPECT_CALL(*server_conn_, CloseConnection(_, _, _)).Times(0);
- // Enable FakeProofVerifier to capture call to VerifyCertChain and run it
+ // Enable TestProofVerifier to capture call to VerifyCertChain and run it
// asynchronously.
- FakeProofVerifier* proof_verifier = client_stream_->GetFakeProofVerifier();
+ TestProofVerifier* proof_verifier = client_stream_->GetTestProofVerifier();
proof_verifier->Activate();
EXPECT_CALL(client_stream_->proof_handler(), OnProofVerifyDetailsAvailable);
@@ -444,6 +452,34 @@
ExpectHandshakeSuccessful();
}
+TEST_F(TlsHandshakerTest, ClientSendsNoSNI) {
+ // Create a new client stream (and handshaker) with an empty server hostname.
+ client_stream_ =
+ new TestQuicCryptoClientStream(&client_session_, QuicServerId("", 443),
+ std::make_unique<FakeProofVerifier>());
+ client_session_.SetCryptoStream(client_stream_);
+
+ EXPECT_CALL(*client_conn_, CloseConnection(_, _, _)).Times(0);
+ EXPECT_CALL(*server_conn_, CloseConnection(_, _, _)).Times(0);
+ EXPECT_CALL(client_stream_->proof_handler(), OnProofVerifyDetailsAvailable);
+ client_stream_->CryptoConnect();
+ ExchangeHandshakeMessages(client_stream_, server_stream_);
+
+ ExpectHandshakeSuccessful();
+ EXPECT_EQ(server_stream_->crypto_negotiated_params().sni, "");
+}
+
+TEST_F(TlsHandshakerTest, ServerExtractSNI) {
+ EXPECT_CALL(*client_conn_, CloseConnection(_, _, _)).Times(0);
+ EXPECT_CALL(*server_conn_, CloseConnection(_, _, _)).Times(0);
+ EXPECT_CALL(client_stream_->proof_handler(), OnProofVerifyDetailsAvailable);
+ client_stream_->CryptoConnect();
+ ExchangeHandshakeMessages(client_stream_, server_stream_);
+ ExpectHandshakeSuccessful();
+
+ EXPECT_EQ(server_stream_->crypto_negotiated_params().sni, "test.example.com");
+}
+
TEST_F(TlsHandshakerTest, ClientConnectionClosedOnTlsError) {
// Have client send ClientHello.
client_stream_->CryptoConnect();
diff --git a/quic/core/tls_server_handshaker.cc b/quic/core/tls_server_handshaker.cc
index 24af98d..d9217c4 100644
--- a/quic/core/tls_server_handshaker.cc
+++ b/quic/core/tls_server_handshaker.cc
@@ -11,6 +11,7 @@
#include "third_party/boringssl/src/include/openssl/ssl.h"
#include "net/third_party/quiche/src/quic/core/crypto/quic_crypto_server_config.h"
#include "net/third_party/quiche/src/quic/core/crypto/transport_parameters.h"
+#include "net/third_party/quiche/src/quic/platform/api/quic_hostname_utils.h"
#include "net/third_party/quiche/src/quic/platform/api/quic_logging.h"
namespace quic {
@@ -314,6 +315,8 @@
const char* hostname = SSL_get_servername(ssl(), TLSEXT_NAMETYPE_host_name);
if (hostname) {
hostname_ = hostname;
+ crypto_negotiated_params_->sni =
+ QuicHostnameUtils::NormalizeHostname(hostname_);
} else {
QUIC_LOG(INFO) << "No hostname indicated in SNI";
}
