Add code counts and error details to debug QUIC TLS handshake failures. NOT generated via copybara Protected by FLAGS_quic_reloadable_flag_quic_add_ssl_error_stack_to_error_detail. PiperOrigin-RevId: 673853213
diff --git a/quiche/common/quiche_feature_flags_list.h b/quiche/common/quiche_feature_flags_list.h index 329cb7a..d526bac 100755 --- a/quiche/common/quiche_feature_flags_list.h +++ b/quiche/common/quiche_feature_flags_list.h
@@ -10,6 +10,7 @@ QUICHE_FLAG(bool, quiche_reloadable_flag_enable_h3_origin_frame, false, true, "If true, enables support for parsing HTTP/3 ORIGIN frames.") QUICHE_FLAG(bool, quiche_reloadable_flag_quic_act_upon_invalid_header, true, true, "If true, reject or send error response code upon receiving invalid request or response headers.") +QUICHE_FLAG(bool, quiche_reloadable_flag_quic_add_ssl_error_stack_to_error_detail, false, false, "If true, when TlsHandshaker::AdvanceHandshake closes connection due to BoringSSL error, it will include a formatted BoringSSL error stack in the error details.") QUICHE_FLAG(bool, quiche_reloadable_flag_quic_add_stream_info_to_idle_close_detail, false, true, "If true, include stream information in idle timeout connection close detail.") QUICHE_FLAG(bool, quiche_reloadable_flag_quic_allow_client_enabled_bbr_v2, true, true, "If true, allow client to enable BBRv2 on server via connection option 'B2ON'.") QUICHE_FLAG(bool, quiche_reloadable_flag_quic_allow_host_in_request2, false, false, "If true, requests with a host header will be allowed.")
diff --git a/quiche/quic/core/crypto/crypto_utils.cc b/quiche/quic/core/crypto/crypto_utils.cc index ad224c2..2cbdb83 100644 --- a/quiche/quic/core/crypto/crypto_utils.cc +++ b/quiche/quic/core/crypto/crypto_utils.cc
@@ -5,6 +5,7 @@ #include "quiche/quic/core/crypto/crypto_utils.h" #include <algorithm> +#include <cstddef> #include <memory> #include <optional> #include <string> @@ -15,6 +16,7 @@ #include "absl/strings/str_cat.h" #include "absl/strings/string_view.h" #include "openssl/bytestring.h" +#include "openssl/err.h" #include "openssl/hkdf.h" #include "openssl/mem.h" #include "openssl/sha.h" @@ -805,4 +807,29 @@ return payload; } +std::string CryptoUtils::GetSSLErrorStack() { + std::string result; + const char* file; + const char* data; + int line; + int flags; + int packed_error = ERR_get_error_line_data(&file, &line, &data, &flags); + if (packed_error != 0) { + char buffer[ERR_ERROR_STRING_BUF_LEN]; + while (packed_error != 0) { + ERR_error_string_n(packed_error, buffer, sizeof(buffer)); + absl::StrAppendFormat(&result, "[%s:%d] %s", PosixBasename(file), line, + buffer); + if (data && (flags & ERR_TXT_STRING)) { + absl::StrAppendFormat(&result, "(%s)", data); + } + packed_error = ERR_get_error_line_data(&file, &line, &data, &flags); + if (packed_error != 0) { + absl::StrAppend(&result, ", "); + } + } + } + return result; +} + } // namespace quic
diff --git a/quiche/quic/core/crypto/crypto_utils.h b/quiche/quic/core/crypto/crypto_utils.h index 41823fe..31ec380 100644 --- a/quiche/quic/core/crypto/crypto_utils.h +++ b/quiche/quic/core/crypto/crypto_utils.h
@@ -252,6 +252,10 @@ // protocol using the certificate key. static std::optional<std::string> GenerateProofPayloadToBeSigned( absl::string_view chlo_hash, absl::string_view server_config); + + // Returns the SSL error queue in a human-readable string. The error queue is + // cleared by the function. + static std::string GetSSLErrorStack(); }; } // namespace quic
diff --git a/quiche/quic/core/crypto/crypto_utils_test.cc b/quiche/quic/core/crypto/crypto_utils_test.cc index 707c3e2..6ddc522 100644 --- a/quiche/quic/core/crypto/crypto_utils_test.cc +++ b/quiche/quic/core/crypto/crypto_utils_test.cc
@@ -9,7 +9,10 @@ #include "absl/base/macros.h" #include "absl/strings/escaping.h" +#include "absl/strings/str_cat.h" #include "absl/strings/string_view.h" +#include "openssl/err.h" +#include "openssl/ssl.h" #include "quiche/quic/core/quic_utils.h" #include "quiche/quic/platform/api/quic_test.h" #include "quiche/quic/test_tools/quic_test_utils.h" @@ -19,6 +22,9 @@ namespace test { namespace { +using ::testing::AllOf; +using ::testing::HasSubstr; + class CryptoUtilsTest : public QuicTest {}; TEST_F(CryptoUtilsTest, HandshakeFailureReasonToString) { @@ -258,6 +264,16 @@ } } +TEST_F(CryptoUtilsTest, GetSSLErrorStack) { + ERR_clear_error(); + const int line = (OPENSSL_PUT_ERROR(SSL, SSL_R_WRONG_SSL_VERSION), __LINE__); + std::string error_location = absl::StrCat("crypto_utils_test.cc:", line); + EXPECT_THAT(CryptoUtils::GetSSLErrorStack(), + AllOf(HasSubstr(error_location), HasSubstr("WRONG_SSL_VERSION"))); + EXPECT_TRUE(CryptoUtils::GetSSLErrorStack().empty()); + ERR_clear_error(); +} + } // namespace } // namespace test } // namespace quic
diff --git a/quiche/quic/core/quic_dispatcher.cc b/quiche/quic/core/quic_dispatcher.cc index 654e58c..0685c64 100644 --- a/quiche/quic/core/quic_dispatcher.cc +++ b/quiche/quic/core/quic_dispatcher.cc
@@ -584,7 +584,7 @@ uint8_t tls_alert = *extract_chlo_result.tls_alert; connection_close_error_code = TlsAlertToQuicErrorCode(tls_alert); tls_alert_error_detail = - absl::StrCat("TLS handshake failure (", + absl::StrCat("TLS handshake failure from dispatcher (", EncryptionLevelToString(ENCRYPTION_INITIAL), ") ", static_cast<int>(tls_alert), ": ", SSL_alert_desc_string_long(tls_alert));
diff --git a/quiche/quic/core/quic_utils.cc b/quiche/quic/core/quic_utils.cc index 5a86673..e2663f3 100644 --- a/quiche/quic/core/quic_utils.cc +++ b/quiche/quic/core/quic_utils.cc
@@ -618,6 +618,23 @@ return total; } +absl::string_view PosixBasename(absl::string_view path) { + constexpr char kPathSeparator = '/'; + size_t pos = path.find_last_of(kPathSeparator); + + // Handle the case with no `kPathSeparator` in `path`. + if (pos == absl::string_view::npos) { + return path; + } + + // Handle the case with a single leading `kPathSeparator` in `path`. + if (pos == 0) { + return absl::ClippedSubstr(path, 1); + } + + return absl::ClippedSubstr(path, pos + 1); +} + std::string RawSha256(absl::string_view input) { std::string raw_hash; raw_hash.resize(SHA256_DIGEST_LENGTH);
diff --git a/quiche/quic/core/quic_utils.h b/quiche/quic/core/quic_utils.h index 2c39781..5bd6121 100644 --- a/quiche/quic/core/quic_utils.h +++ b/quiche/quic/core/quic_utils.h
@@ -226,6 +226,15 @@ QuicByteCount MemSliceSpanTotalSize(absl::Span<quiche::QuicheMemSlice> span); +// Returns the part of the path after the final "/". If there is no "/" in the +// path, the result is the same as the input. +// Note that this function's behavior differs from the POSIX standard basename +// function if path ends with "/". For such paths, this function returns the +// empty string. +// This function returns path as-is, if it's windows path with backslash +// separators. +absl::string_view PosixBasename(absl::string_view path); + // Computes a SHA-256 hash and returns the raw bytes of the hash. QUICHE_EXPORT std::string RawSha256(absl::string_view input);
diff --git a/quiche/quic/core/quic_utils_test.cc b/quiche/quic/core/quic_utils_test.cc index e29f769..55cf72e 100644 --- a/quiche/quic/core/quic_utils_test.cc +++ b/quiche/quic/core/quic_utils_test.cc
@@ -236,6 +236,19 @@ EXPECT_EQ(EcnCodepointToString(ECN_CE), "CE"); } +TEST_F(QuicUtilsTest, PosixBasename) { + EXPECT_EQ("", PosixBasename("/hello/")); + EXPECT_EQ("hello", PosixBasename("/hello")); + EXPECT_EQ("world", PosixBasename("hello/world")); + EXPECT_EQ("", PosixBasename("hello/")); + EXPECT_EQ("world", PosixBasename("world")); + EXPECT_EQ("", PosixBasename("/")); + EXPECT_EQ("", PosixBasename("")); + // "\\" is not treated as a path separator. + EXPECT_EQ("C:\\hello", PosixBasename("C:\\hello")); + EXPECT_EQ("world", PosixBasename("C:\\hello/world")); +} + enum class TestEnumClassBit : uint8_t { BIT_ZERO = 0, BIT_ONE,
diff --git a/quiche/quic/core/tls_chlo_extractor.cc b/quiche/quic/core/tls_chlo_extractor.cc index 6c17b7c..996cd79 100644 --- a/quiche/quic/core/tls_chlo_extractor.cc +++ b/quiche/quic/core/tls_chlo_extractor.cc
@@ -397,6 +397,7 @@ alpn_len); absl::string_view alpns_payload; if (!alpns_reader.ReadStringPiece16(&alpns_payload)) { + QUIC_CODE_COUNT_N(quic_chlo_alpns_invalid, 1, 2); HandleUnrecoverableError("Failed to read alpns_payload"); return; } @@ -404,6 +405,7 @@ while (!alpns_payload_reader.IsDoneReading()) { absl::string_view alpn_payload; if (!alpns_payload_reader.ReadStringPiece8(&alpn_payload)) { + QUIC_CODE_COUNT_N(quic_chlo_alpns_invalid, 2, 2); HandleUnrecoverableError("Failed to read alpn_payload"); return; }
diff --git a/quiche/quic/core/tls_client_handshaker_test.cc b/quiche/quic/core/tls_client_handshaker_test.cc index fa7a3c8..40ff7f7 100644 --- a/quiche/quic/core/tls_client_handshaker_test.cc +++ b/quiche/quic/core/tls_client_handshaker_test.cc
@@ -33,6 +33,7 @@ #include "quiche/common/test_tools/quiche_test_utils.h" using testing::_; +using testing::HasSubstr; namespace quic { namespace test { @@ -647,13 +648,14 @@ return std::find(alpns.cbegin(), alpns.cend(), kTestAlpn); }); - EXPECT_CALL(*server_connection_, - CloseConnection(QUIC_HANDSHAKE_FAILED, - static_cast<QuicIetfTransportErrorCodes>( - CRYPTO_ERROR_FIRST + 120), - "TLS handshake failure (ENCRYPTION_INITIAL) 120: " - "no application protocol", - _)); + EXPECT_CALL( + *server_connection_, + CloseConnection( + QUIC_HANDSHAKE_FAILED, + static_cast<QuicIetfTransportErrorCodes>(CRYPTO_ERROR_FIRST + 120), + HasSubstr("TLS handshake failure (ENCRYPTION_INITIAL) 120: " + "no application protocol"), + _)); stream()->CryptoConnect(); crypto_test_utils::AdvanceHandshake(connection_, stream(), 0,
diff --git a/quiche/quic/core/tls_handshaker.cc b/quiche/quic/core/tls_handshaker.cc index ff9bfe5..dffb4d9 100644 --- a/quiche/quic/core/tls_handshaker.cc +++ b/quiche/quic/core/tls_handshaker.cc
@@ -154,22 +154,31 @@ } if (ShouldCloseConnectionOnUnexpectedError(ssl_error) && !is_connection_closed()) { + std::string ssl_error_stack; + if (GetQuicReloadableFlag(quic_add_ssl_error_stack_to_error_detail)) { + QUIC_RELOADABLE_FLAG_COUNT(quic_add_ssl_error_stack_to_error_detail); + ssl_error_stack = CryptoUtils::GetSSLErrorStack(); + } else { + ERR_print_errors_fp(stderr); + } QUIC_VLOG(1) << "SSL_do_handshake failed; SSL_get_error returns " - << ssl_error; - ERR_print_errors_fp(stderr); + << ssl_error << ", SSLErrorStack: " << ssl_error_stack; if (last_tls_alert_.has_value()) { std::string error_details = absl::StrCat("TLS handshake failure (", EncryptionLevelToString(last_tls_alert_->level), ") ", static_cast<int>(last_tls_alert_->desc), ": ", - SSL_alert_desc_string_long(last_tls_alert_->desc)); + SSL_alert_desc_string_long(last_tls_alert_->desc), + ". SSLErrorStack:", ssl_error_stack); QUIC_DLOG(ERROR) << error_details; CloseConnection(TlsAlertToQuicErrorCode(last_tls_alert_->desc), static_cast<QuicIetfTransportErrorCodes>( CRYPTO_ERROR_FIRST + last_tls_alert_->desc), error_details); } else { - CloseConnection(QUIC_HANDSHAKE_FAILED, "TLS handshake failed"); + CloseConnection(QUIC_HANDSHAKE_FAILED, + absl::StrCat("TLS handshake failed. SSLErrorStack:", + ssl_error_stack)); } } }
diff --git a/quiche/quic/core/tls_server_handshaker_test.cc b/quiche/quic/core/tls_server_handshaker_test.cc index b76ea32..319832e 100644 --- a/quiche/quic/core/tls_server_handshaker_test.cc +++ b/quiche/quic/core/tls_server_handshaker_test.cc
@@ -42,6 +42,7 @@ } // namespace quic using testing::_; +using testing::HasSubstr; using testing::NiceMock; using testing::Return; @@ -698,13 +699,14 @@ EXPECT_CALL(*client_session_, GetAlpnsToOffer()) .WillOnce(Return(std::vector<std::string>({kTestBadClientAlpn}))); - EXPECT_CALL(*server_connection_, - CloseConnection(QUIC_HANDSHAKE_FAILED, - static_cast<QuicIetfTransportErrorCodes>( - CRYPTO_ERROR_FIRST + 120), - "TLS handshake failure (ENCRYPTION_INITIAL) 120: " - "no application protocol", - _)); + EXPECT_CALL( + *server_connection_, + CloseConnection( + QUIC_HANDSHAKE_FAILED, + static_cast<QuicIetfTransportErrorCodes>(CRYPTO_ERROR_FIRST + 120), + HasSubstr("TLS handshake failure (ENCRYPTION_INITIAL) 120: " + "no application protocol"), + _)); AdvanceHandshakeWithFakeClient();