Add kDSNI connection option to permit sending debugging_sni despite ECH GREASE
Protected by QUIC connection option `DSNI`.
PiperOrigin-RevId: 831956111
diff --git a/quiche/quic/core/crypto/crypto_protocol.h b/quiche/quic/core/crypto/crypto_protocol.h
index a3fc864..47a5d8f 100644
--- a/quiche/quic/core/crypto/crypto_protocol.h
+++ b/quiche/quic/core/crypto/crypto_protocol.h
@@ -419,6 +419,8 @@
// it as the initial rtt.
DEFINE_STATIC_QUIC_TAG(SNI); // Server name
// indication
+DEFINE_STATIC_QUIC_TAG(DSNI); // Enables the debugging_sni transport parameter
+ // to be sent alongside ECH GREASE, but not ECH.
DEFINE_STATIC_QUIC_TAG(PUBS); // Public key values
DEFINE_STATIC_QUIC_TAG(SCID); // Server config id
DEFINE_STATIC_QUIC_TAG(OBIT); // Server orbit.
diff --git a/quiche/quic/core/tls_client_handshaker.cc b/quiche/quic/core/tls_client_handshaker.cc
index 8af37c8..85619b3 100644
--- a/quiche/quic/core/tls_client_handshaker.cc
+++ b/quiche/quic/core/tls_client_handshaker.cc
@@ -5,7 +5,7 @@
#include "quiche/quic/core/tls_client_handshaker.h"
#include <algorithm>
-#include <cstring>
+#include <cstdint>
#include <limits>
#include <memory>
#include <optional>
@@ -13,17 +13,27 @@
#include <utility>
#include <vector>
-#include "absl/strings/str_cat.h"
#include "absl/strings/string_view.h"
#include "openssl/ssl.h"
+#include "quiche/quic/core/crypto/client_proof_source.h"
+#include "quiche/quic/core/crypto/crypto_handshake.h"
+#include "quiche/quic/core/crypto/crypto_protocol.h"
+#include "quiche/quic/core/crypto/proof_verifier.h"
#include "quiche/quic/core/crypto/quic_crypto_client_config.h"
-#include "quiche/quic/core/crypto/quic_encrypter.h"
#include "quiche/quic/core/crypto/transport_parameters.h"
+#include "quiche/quic/core/quic_crypto_client_stream.h"
+#include "quiche/quic/core/quic_data_writer.h"
+#include "quiche/quic/core/quic_error_codes.h"
+#include "quiche/quic/core/quic_server_id.h"
#include "quiche/quic/core/quic_session.h"
#include "quiche/quic/core/quic_types.h"
+#include "quiche/quic/core/quic_versions.h"
+#include "quiche/quic/core/tls_handshaker.h"
#include "quiche/quic/platform/api/quic_bug_tracker.h"
#include "quiche/quic/platform/api/quic_flags.h"
#include "quiche/quic/platform/api/quic_hostname_utils.h"
+#include "quiche/quic/platform/api/quic_logging.h"
+#include "quiche/common/platform/api/quiche_logging.h"
#include "quiche/common/quiche_text_utils.h"
namespace quic {
@@ -289,11 +299,14 @@
}
// The `debugging_sni` field must not be sent when attempting Encrypted Client
- // Hello (ECH) because it would reveal the real SNI in cleartext. Further, it
- // must not be sent when GREASEing ECH because that would reveal to observers
- // whether the ECH was real or GREASE.
+ // Hello (ECH) because it would reveal the real SNI in cleartext. When only
+ // ECH GREASE will be sent, it's still sensible to omit `debugging_sni`
+ // because it would enable observers to discriminate real ECH from GREASE. The
+ // `kDSNI` option forces `debugging_sni` to be sent despite ECH GREASE.
if (!tls_connection_.ssl_config().ech_config_list.empty() ||
- tls_connection_.ssl_config().ech_grease_enabled) {
+ (tls_connection_.ssl_config().ech_grease_enabled &&
+ !session_->config()->HasClientSentConnectionOption(
+ kDSNI, Perspective::IS_CLIENT))) {
params.debugging_sni.reset();
}
diff --git a/quiche/quic/core/tls_client_handshaker_test.cc b/quiche/quic/core/tls_client_handshaker_test.cc
index 730e769..723c270 100644
--- a/quiche/quic/core/tls_client_handshaker_test.cc
+++ b/quiche/quic/core/tls_client_handshaker_test.cc
@@ -3,6 +3,8 @@
// found in the LICENSE file.
#include <algorithm>
+#include <cstddef>
+#include <cstdint>
#include <memory>
#include <optional>
#include <string>
@@ -10,22 +12,25 @@
#include <vector>
#include "absl/base/macros.h"
+#include "absl/strings/string_view.h"
#include "openssl/aead.h"
#include "openssl/hpke.h"
#include "openssl/ssl.h"
#include "openssl/tls1.h"
-#include "quiche/quic/core/crypto/quic_decrypter.h"
-#include "quiche/quic/core/crypto/quic_encrypter.h"
+#include "quiche/quic/core/crypto/crypto_handshake_message.h"
+#include "quiche/quic/core/crypto/crypto_protocol.h"
+#include "quiche/quic/core/crypto/proof_verifier.h"
+#include "quiche/quic/core/crypto/quic_compressed_certs_cache.h"
+#include "quiche/quic/core/crypto/quic_crypto_client_config.h"
#include "quiche/quic/core/crypto/transport_parameters.h"
#include "quiche/quic/core/quic_connection.h"
+#include "quiche/quic/core/quic_constants.h"
#include "quiche/quic/core/quic_error_codes.h"
-#include "quiche/quic/core/quic_packets.h"
#include "quiche/quic/core/quic_server_id.h"
+#include "quiche/quic/core/quic_time.h"
#include "quiche/quic/core/quic_types.h"
-#include "quiche/quic/core/quic_utils.h"
#include "quiche/quic/core/quic_versions.h"
#include "quiche/quic/platform/api/quic_expect_bug.h"
-#include "quiche/quic/platform/api/quic_flags.h"
#include "quiche/quic/platform/api/quic_test.h"
#include "quiche/quic/test_tools/crypto_test_utils.h"
#include "quiche/quic/test_tools/quic_connection_peer.h"
@@ -34,7 +39,6 @@
#include "quiche/quic/test_tools/quic_test_utils.h"
#include "quiche/quic/test_tools/simple_session_cache.h"
#include "quiche/quic/tools/fake_proof_verifier.h"
-#include "quiche/common/test_tools/quiche_test_utils.h"
using testing::_;
using testing::Eq;
@@ -983,6 +987,35 @@
EXPECT_EQ(debug_visitor.debugging_sni(), std::nullopt);
}
+TEST_P(TlsClientHandshakerTest, DebuggingSniDisabledByECHAndNotSavedByDSNI) {
+ ssl_config_.emplace();
+ bssl::UniquePtr<SSL_ECH_KEYS> ech_keys =
+ MakeTestEchKeys("public-name.example", /*max_name_len=*/64,
+ &ssl_config_->ech_config_list);
+ ASSERT_TRUE(ech_keys);
+
+ // Configure the server to use the test ECH keys.
+ ASSERT_TRUE(
+ SSL_CTX_set1_ech_keys(server_crypto_config_->ssl_ctx(), ech_keys.get()));
+
+ // Recreate the client to pick up the new `ssl_config_`.
+ CreateConnection();
+ session_->config()->SetDebuggingSniToSend("secret.example");
+ session_->config()->AddConnectionOptionsToSend({kDSNI});
+
+ DebuggingSniExtractor debug_visitor;
+ connection_->set_debug_visitor(&debug_visitor);
+
+ // The handshake should complete and negotiate ECH.
+ CompleteCryptoHandshake();
+ EXPECT_EQ(PROTOCOL_TLS1_3, stream()->handshake_protocol());
+ EXPECT_TRUE(stream()->encryption_established());
+ EXPECT_TRUE(stream()->one_rtt_keys_available());
+ EXPECT_TRUE(stream()->crypto_negotiated_params().encrypted_client_hello);
+
+ EXPECT_EQ(debug_visitor.debugging_sni(), std::nullopt);
+}
+
TEST_P(TlsClientHandshakerTest, DebuggingSniDisabledByECHGrease) {
ssl_config_.emplace();
ssl_config_->ech_grease_enabled = true;
@@ -1018,6 +1051,42 @@
EXPECT_EQ(debug_visitor.debugging_sni(), std::nullopt);
}
+TEST_P(TlsClientHandshakerTest, DebuggingSniDisabledByECHGreaseButSavedByDSNI) {
+ ssl_config_.emplace();
+ ssl_config_->ech_grease_enabled = true;
+ CreateConnection();
+ session_->config()->SetDebuggingSniToSend("secret.example");
+ session_->config()->AddConnectionOptionsToSend({kDSNI});
+
+ DebuggingSniExtractor debug_visitor;
+ connection_->set_debug_visitor(&debug_visitor);
+
+ stream()->CryptoConnect();
+
+ // Add a DoS callback on the server, to test that the client sent a GREASE
+ // message. This is a bit of a hack. TlsServerHandshaker already configures
+ // the certificate selection callback, but does not usefully expose any way
+ // for tests to inspect the ClientHello. So, instead, we register a different
+ // callback that also gets the ClientHello.
+ static bool callback_ran;
+ callback_ran = false;
+ SSL_CTX_set_dos_protection_cb(
+ server_crypto_config_->ssl_ctx(),
+ [](const SSL_CLIENT_HELLO* client_hello) -> int {
+ const uint8_t* data;
+ size_t len;
+ EXPECT_TRUE(SSL_early_callback_ctx_extension_get(
+ client_hello, TLSEXT_TYPE_encrypted_client_hello, &data, &len));
+ callback_ran = true;
+ return 1;
+ });
+
+ CompleteCryptoHandshake();
+ EXPECT_TRUE(callback_ran);
+
+ EXPECT_EQ(debug_visitor.debugging_sni(), "secret.example");
+}
+
TEST_P(TlsClientHandshakerTest, DebuggingSniSentWithoutECH) {
CreateConnection();
session_->config()->SetDebuggingSniToSend("secret.example");
