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();
