From 910396d1a6cfce27fdee5820fbed6584fcd7df20 Mon Sep 17 00:00:00 2001 From: Shikugawa Date: Fri, 8 Jan 2021 03:29:39 +0000 Subject: [PATCH 1/7] ssl: improve TLS handshake/read/write error log Signed-off-by: Shikugawa --- .../filters/listener/tls_inspector/tls_inspector.cc | 6 +++++- source/extensions/transport_sockets/tls/ssl_handshaker.cc | 2 ++ source/extensions/transport_sockets/tls/ssl_handshaker.h | 4 +++- source/extensions/transport_sockets/tls/ssl_socket.cc | 4 ++++ 4 files changed, 14 insertions(+), 2 deletions(-) diff --git a/source/extensions/filters/listener/tls_inspector/tls_inspector.cc b/source/extensions/filters/listener/tls_inspector/tls_inspector.cc index f773dade15d3..6552fe82d1ac 100644 --- a/source/extensions/filters/listener/tls_inspector/tls_inspector.cc +++ b/source/extensions/filters/listener/tls_inspector/tls_inspector.cc @@ -210,7 +210,11 @@ ParseState Filter::parseClientHello(const void* data, size_t len) { // This should never succeed because an error is always returned from the SNI callback. ASSERT(ret <= 0); - switch (SSL_get_error(ssl_.get(), ret)) { + int err = SSL_get_error(ssl_.get(), ret); + ENVOY_LOG(trace, "tls inspector error while client hello parsing: {}", + SSL_error_description(err)); + + switch (err) { case SSL_ERROR_WANT_READ: if (read_ == config_->maxClientHelloSize()) { // We've hit the specified size limit. This is an unreasonably large ClientHello; diff --git a/source/extensions/transport_sockets/tls/ssl_handshaker.cc b/source/extensions/transport_sockets/tls/ssl_handshaker.cc index 7d8def3ecfd1..9478829830a1 100644 --- a/source/extensions/transport_sockets/tls/ssl_handshaker.cc +++ b/source/extensions/transport_sockets/tls/ssl_handshaker.cc @@ -233,6 +233,8 @@ Network::PostIoAction SslHandshakerImpl::doHandshake() { : PostIoAction::Close; } else { int err = SSL_get_error(ssl(), rc); + ENVOY_CONN_LOG(trace, "ssl error occured while read: {}", handshake_callbacks_->connection(), + SSL_error_description(err)); switch (err) { case SSL_ERROR_WANT_READ: case SSL_ERROR_WANT_WRITE: diff --git a/source/extensions/transport_sockets/tls/ssl_handshaker.h b/source/extensions/transport_sockets/tls/ssl_handshaker.h index 50090f6f43a7..cb9be1e3190f 100644 --- a/source/extensions/transport_sockets/tls/ssl_handshaker.h +++ b/source/extensions/transport_sockets/tls/ssl_handshaker.h @@ -37,7 +37,9 @@ class SslExtendedSocketInfoImpl : public Envoy::Ssl::SslExtendedSocketInfo { Envoy::Ssl::ClientValidationStatus::NotValidated}; }; -class SslHandshakerImpl : public Ssl::ConnectionInfo, public Ssl::Handshaker { +class SslHandshakerImpl : public Ssl::ConnectionInfo, + public Ssl::Handshaker, + protected Logger::Loggable { public: SslHandshakerImpl(bssl::UniquePtr ssl, int ssl_extended_socket_info_index, Ssl::HandshakeCallbacks* handshake_callbacks); diff --git a/source/extensions/transport_sockets/tls/ssl_socket.cc b/source/extensions/transport_sockets/tls/ssl_socket.cc index ba408c2d417f..13a7e5b9d612 100644 --- a/source/extensions/transport_sockets/tls/ssl_socket.cc +++ b/source/extensions/transport_sockets/tls/ssl_socket.cc @@ -137,6 +137,8 @@ Network::IoResult SslSocket::doRead(Buffer::Instance& read_buffer) { if (result.error_.has_value()) { keep_reading = false; int err = SSL_get_error(rawSsl(), result.error_.value()); + ENVOY_CONN_LOG(trace, "ssl error occured while read: {}", callbacks_->connection(), + SSL_error_description(err)); switch (err) { case SSL_ERROR_WANT_READ: break; @@ -263,6 +265,8 @@ Network::IoResult SslSocket::doWrite(Buffer::Instance& write_buffer, bool end_st bytes_to_write = std::min(write_buffer.length(), static_cast(16384)); } else { int err = SSL_get_error(rawSsl(), rc); + ENVOY_CONN_LOG(trace, "ssl error occured while write: {}", callbacks_->connection(), + SSL_error_description(err)); switch (err) { case SSL_ERROR_WANT_WRITE: bytes_to_retry_ = bytes_to_write; From 622c900fe34934320d980888c08546d0e0cb9fcc Mon Sep 17 00:00:00 2001 From: Shikugawa Date: Fri, 8 Jan 2021 08:17:52 +0000 Subject: [PATCH 2/7] fix spell Signed-off-by: Shikugawa --- source/extensions/transport_sockets/tls/ssl_handshaker.cc | 2 +- source/extensions/transport_sockets/tls/ssl_socket.cc | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/source/extensions/transport_sockets/tls/ssl_handshaker.cc b/source/extensions/transport_sockets/tls/ssl_handshaker.cc index 9478829830a1..a3cb2794013d 100644 --- a/source/extensions/transport_sockets/tls/ssl_handshaker.cc +++ b/source/extensions/transport_sockets/tls/ssl_handshaker.cc @@ -233,7 +233,7 @@ Network::PostIoAction SslHandshakerImpl::doHandshake() { : PostIoAction::Close; } else { int err = SSL_get_error(ssl(), rc); - ENVOY_CONN_LOG(trace, "ssl error occured while read: {}", handshake_callbacks_->connection(), + ENVOY_CONN_LOG(trace, "ssl error occurred while read: {}", handshake_callbacks_->connection(), SSL_error_description(err)); switch (err) { case SSL_ERROR_WANT_READ: diff --git a/source/extensions/transport_sockets/tls/ssl_socket.cc b/source/extensions/transport_sockets/tls/ssl_socket.cc index 13a7e5b9d612..dfa8816210ff 100644 --- a/source/extensions/transport_sockets/tls/ssl_socket.cc +++ b/source/extensions/transport_sockets/tls/ssl_socket.cc @@ -137,7 +137,7 @@ Network::IoResult SslSocket::doRead(Buffer::Instance& read_buffer) { if (result.error_.has_value()) { keep_reading = false; int err = SSL_get_error(rawSsl(), result.error_.value()); - ENVOY_CONN_LOG(trace, "ssl error occured while read: {}", callbacks_->connection(), + ENVOY_CONN_LOG(trace, "ssl error occurred while read: {}", callbacks_->connection(), SSL_error_description(err)); switch (err) { case SSL_ERROR_WANT_READ: @@ -265,7 +265,7 @@ Network::IoResult SslSocket::doWrite(Buffer::Instance& write_buffer, bool end_st bytes_to_write = std::min(write_buffer.length(), static_cast(16384)); } else { int err = SSL_get_error(rawSsl(), rc); - ENVOY_CONN_LOG(trace, "ssl error occured while write: {}", callbacks_->connection(), + ENVOY_CONN_LOG(trace, "ssl error occurred while write: {}", callbacks_->connection(), SSL_error_description(err)); switch (err) { case SSL_ERROR_WANT_WRITE: From d83fc4dca118e0f7fdcdbd3aa70c5a62589684a4 Mon Sep 17 00:00:00 2001 From: Shikugawa Date: Sat, 9 Jan 2021 02:24:02 +0000 Subject: [PATCH 3/7] allow logs without fips Signed-off-by: Shikugawa --- .../filters/listener/tls_inspector/tls_inspector.cc | 3 ++- source/extensions/transport_sockets/tls/ssl_handshaker.cc | 2 ++ source/extensions/transport_sockets/tls/ssl_socket.cc | 4 ++++ 3 files changed, 8 insertions(+), 1 deletion(-) diff --git a/source/extensions/filters/listener/tls_inspector/tls_inspector.cc b/source/extensions/filters/listener/tls_inspector/tls_inspector.cc index 6552fe82d1ac..906694b7c120 100644 --- a/source/extensions/filters/listener/tls_inspector/tls_inspector.cc +++ b/source/extensions/filters/listener/tls_inspector/tls_inspector.cc @@ -211,9 +211,10 @@ ParseState Filter::parseClientHello(const void* data, size_t len) { // This should never succeed because an error is always returned from the SNI callback. ASSERT(ret <= 0); int err = SSL_get_error(ssl_.get(), ret); +#ifndef BORINGSSL_FIPS ENVOY_LOG(trace, "tls inspector error while client hello parsing: {}", SSL_error_description(err)); - +#endif switch (err) { case SSL_ERROR_WANT_READ: if (read_ == config_->maxClientHelloSize()) { diff --git a/source/extensions/transport_sockets/tls/ssl_handshaker.cc b/source/extensions/transport_sockets/tls/ssl_handshaker.cc index a3cb2794013d..de4ff0835fcf 100644 --- a/source/extensions/transport_sockets/tls/ssl_handshaker.cc +++ b/source/extensions/transport_sockets/tls/ssl_handshaker.cc @@ -233,8 +233,10 @@ Network::PostIoAction SslHandshakerImpl::doHandshake() { : PostIoAction::Close; } else { int err = SSL_get_error(ssl(), rc); +#ifndef BORINGSSL_FIPS ENVOY_CONN_LOG(trace, "ssl error occurred while read: {}", handshake_callbacks_->connection(), SSL_error_description(err)); +#endif switch (err) { case SSL_ERROR_WANT_READ: case SSL_ERROR_WANT_WRITE: diff --git a/source/extensions/transport_sockets/tls/ssl_socket.cc b/source/extensions/transport_sockets/tls/ssl_socket.cc index dfa8816210ff..e089b17a4d45 100644 --- a/source/extensions/transport_sockets/tls/ssl_socket.cc +++ b/source/extensions/transport_sockets/tls/ssl_socket.cc @@ -137,8 +137,10 @@ Network::IoResult SslSocket::doRead(Buffer::Instance& read_buffer) { if (result.error_.has_value()) { keep_reading = false; int err = SSL_get_error(rawSsl(), result.error_.value()); +#ifndef BORINGSSL_FIPS ENVOY_CONN_LOG(trace, "ssl error occurred while read: {}", callbacks_->connection(), SSL_error_description(err)); +#endif switch (err) { case SSL_ERROR_WANT_READ: break; @@ -265,8 +267,10 @@ Network::IoResult SslSocket::doWrite(Buffer::Instance& write_buffer, bool end_st bytes_to_write = std::min(write_buffer.length(), static_cast(16384)); } else { int err = SSL_get_error(rawSsl(), rc); +#ifndef BORINGSSL_FIPS ENVOY_CONN_LOG(trace, "ssl error occurred while write: {}", callbacks_->connection(), SSL_error_description(err)); +#endif switch (err) { case SSL_ERROR_WANT_WRITE: bytes_to_retry_ = bytes_to_write; From 08dea59e0738a013698ad8dfa8686815d430ef41 Mon Sep 17 00:00:00 2001 From: Shikugawa Date: Sat, 9 Jan 2021 04:53:27 +0000 Subject: [PATCH 4/7] add generic get error interface Signed-off-by: Shikugawa --- .../listener/tls_inspector/tls_inspector.cc | 7 +-- source/extensions/transport_sockets/tls/BUILD | 1 + .../transport_sockets/tls/ssl_handshaker.cc | 4 +- .../transport_sockets/tls/ssl_socket.cc | 8 +--- .../transport_sockets/tls/utility.cc | 44 +++++++++++++++++++ .../transport_sockets/tls/utility.h | 7 +++ .../transport_sockets/tls/handshaker_test.cc | 3 ++ 7 files changed, 59 insertions(+), 15 deletions(-) diff --git a/source/extensions/filters/listener/tls_inspector/tls_inspector.cc b/source/extensions/filters/listener/tls_inspector/tls_inspector.cc index 906694b7c120..f773dade15d3 100644 --- a/source/extensions/filters/listener/tls_inspector/tls_inspector.cc +++ b/source/extensions/filters/listener/tls_inspector/tls_inspector.cc @@ -210,12 +210,7 @@ ParseState Filter::parseClientHello(const void* data, size_t len) { // This should never succeed because an error is always returned from the SNI callback. ASSERT(ret <= 0); - int err = SSL_get_error(ssl_.get(), ret); -#ifndef BORINGSSL_FIPS - ENVOY_LOG(trace, "tls inspector error while client hello parsing: {}", - SSL_error_description(err)); -#endif - switch (err) { + switch (SSL_get_error(ssl_.get(), ret)) { case SSL_ERROR_WANT_READ: if (read_ == config_->maxClientHelloSize()) { // We've hit the specified size limit. This is an unreasonably large ClientHello; diff --git a/source/extensions/transport_sockets/tls/BUILD b/source/extensions/transport_sockets/tls/BUILD index 860b56203c30..2bcb141d5a63 100644 --- a/source/extensions/transport_sockets/tls/BUILD +++ b/source/extensions/transport_sockets/tls/BUILD @@ -177,6 +177,7 @@ envoy_cc_library( ], deps = [ "//source/common/common:assert_lib", + "//source/common/common:empty_string", "//source/common/common:utility_lib", "//source/common/network:address_lib", ], diff --git a/source/extensions/transport_sockets/tls/ssl_handshaker.cc b/source/extensions/transport_sockets/tls/ssl_handshaker.cc index de4ff0835fcf..766454bbd879 100644 --- a/source/extensions/transport_sockets/tls/ssl_handshaker.cc +++ b/source/extensions/transport_sockets/tls/ssl_handshaker.cc @@ -233,10 +233,8 @@ Network::PostIoAction SslHandshakerImpl::doHandshake() { : PostIoAction::Close; } else { int err = SSL_get_error(ssl(), rc); -#ifndef BORINGSSL_FIPS ENVOY_CONN_LOG(trace, "ssl error occurred while read: {}", handshake_callbacks_->connection(), - SSL_error_description(err)); -#endif + Utility::getErrorDescription(err)); switch (err) { case SSL_ERROR_WANT_READ: case SSL_ERROR_WANT_WRITE: diff --git a/source/extensions/transport_sockets/tls/ssl_socket.cc b/source/extensions/transport_sockets/tls/ssl_socket.cc index e089b17a4d45..b9bacd9caf44 100644 --- a/source/extensions/transport_sockets/tls/ssl_socket.cc +++ b/source/extensions/transport_sockets/tls/ssl_socket.cc @@ -137,10 +137,8 @@ Network::IoResult SslSocket::doRead(Buffer::Instance& read_buffer) { if (result.error_.has_value()) { keep_reading = false; int err = SSL_get_error(rawSsl(), result.error_.value()); -#ifndef BORINGSSL_FIPS ENVOY_CONN_LOG(trace, "ssl error occurred while read: {}", callbacks_->connection(), - SSL_error_description(err)); -#endif + Utility::getErrorDescription(err)); switch (err) { case SSL_ERROR_WANT_READ: break; @@ -267,10 +265,8 @@ Network::IoResult SslSocket::doWrite(Buffer::Instance& write_buffer, bool end_st bytes_to_write = std::min(write_buffer.length(), static_cast(16384)); } else { int err = SSL_get_error(rawSsl(), rc); -#ifndef BORINGSSL_FIPS ENVOY_CONN_LOG(trace, "ssl error occurred while write: {}", callbacks_->connection(), - SSL_error_description(err)); -#endif + Utility::getErrorDescription(err)); switch (err) { case SSL_ERROR_WANT_WRITE: bytes_to_retry_ = bytes_to_write; diff --git a/source/extensions/transport_sockets/tls/utility.cc b/source/extensions/transport_sockets/tls/utility.cc index e5b8bc17085f..ea69047725fc 100644 --- a/source/extensions/transport_sockets/tls/utility.cc +++ b/source/extensions/transport_sockets/tls/utility.cc @@ -1,6 +1,7 @@ #include "extensions/transport_sockets/tls/utility.h" #include "common/common/assert.h" +#include "common/common/empty_string.h" #include "common/network/address_impl.h" #include "absl/strings/str_join.h" @@ -214,6 +215,49 @@ absl::optional Utility::getLastCryptoError() { return absl::nullopt; } +std::string Utility::getErrorDescription(int err) { + switch (err) { + case SSL_ERROR_NONE: + return "NONE"; + case SSL_ERROR_SSL: + return "SSL"; + case SSL_ERROR_WANT_READ: + return "WANT_READ"; + case SSL_ERROR_WANT_WRITE: + return "WANT_WRITE"; + case SSL_ERROR_WANT_X509_LOOKUP: + return "WANT_X509_LOOKUP"; + case SSL_ERROR_SYSCALL: + return "SYSCALL"; + case SSL_ERROR_ZERO_RETURN: + return "ZERO_RETURN"; + case SSL_ERROR_WANT_CONNECT: + return "WANT_CONNECT"; + case SSL_ERROR_WANT_ACCEPT: + return "WANT_ACCEPT"; + case SSL_ERROR_WANT_CHANNEL_ID_LOOKUP: + return "WANT_CHANNEL_ID_LOOKUP"; + case SSL_ERROR_PENDING_SESSION: + return "PENDING_SESSION"; + case SSL_ERROR_PENDING_CERTIFICATE: + return "PENDING_CERTIFICATE"; + case SSL_ERROR_WANT_PRIVATE_KEY_OPERATION: + return "WANT_PRIVATE_KEY_OPERATION"; + case SSL_ERROR_PENDING_TICKET: + return "PENDING_TICKET"; + case SSL_ERROR_EARLY_DATA_REJECTED: + return "EARLY_DATA_REJECTED"; + case SSL_ERROR_WANT_CERTIFICATE_VERIFY: + return "WANT_CERTIFICATE_VERIFY"; + case SSL_ERROR_HANDOFF: + return "HANDOFF"; + case SSL_ERROR_HANDBACK: + return "HANDBACK"; + default: + return EMPTY_STRING; + } +} + } // namespace Tls } // namespace TransportSockets } // namespace Extensions diff --git a/source/extensions/transport_sockets/tls/utility.h b/source/extensions/transport_sockets/tls/utility.h index f4f06eb4cb23..f96881cb4d14 100644 --- a/source/extensions/transport_sockets/tls/utility.h +++ b/source/extensions/transport_sockets/tls/utility.h @@ -89,6 +89,13 @@ SystemTime getExpirationTime(const X509& cert); */ absl::optional getLastCryptoError(); +/** + * Returns error string corresponding error code derived from OpenSSL. + * @param err error code + * @return string message corresponding error code. + */ +std::string getErrorDescription(int err); + } // namespace Utility } // namespace Tls } // namespace TransportSockets diff --git a/test/extensions/transport_sockets/tls/handshaker_test.cc b/test/extensions/transport_sockets/tls/handshaker_test.cc index 96fdba339cd2..18e562727171 100644 --- a/test/extensions/transport_sockets/tls/handshaker_test.cc +++ b/test/extensions/transport_sockets/tls/handshaker_test.cc @@ -154,6 +154,9 @@ TEST_F(HandshakerTest, ErrorCbOnAbnormalOperation) { SSL_set_bio(client_ssl_.get(), bio, bio); StrictMock handshake_callbacks; + NiceMock mock_connection; + + EXPECT_CALL(handshake_callbacks, connection).WillOnce(ReturnRef(mock_connection)); EXPECT_CALL(handshake_callbacks, onFailure); SslHandshakerImpl handshaker(std::move(server_ssl_), 0, &handshake_callbacks); From eb3b529a13760a3a196ff97e63cd09f6e54a2de2 Mon Sep 17 00:00:00 2001 From: Shikugawa Date: Sat, 9 Jan 2021 08:08:42 +0000 Subject: [PATCH 5/7] fix test Signed-off-by: Shikugawa --- test/extensions/transport_sockets/tls/handshaker_test.cc | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/extensions/transport_sockets/tls/handshaker_test.cc b/test/extensions/transport_sockets/tls/handshaker_test.cc index 18e562727171..82484f56dda0 100644 --- a/test/extensions/transport_sockets/tls/handshaker_test.cc +++ b/test/extensions/transport_sockets/tls/handshaker_test.cc @@ -153,10 +153,10 @@ TEST_F(HandshakerTest, ErrorCbOnAbnormalOperation) { BIO* bio = BIO_new(BIO_s_socket()); SSL_set_bio(client_ssl_.get(), bio, bio); - StrictMock handshake_callbacks; + NiceMock handshake_callbacks; NiceMock mock_connection; - EXPECT_CALL(handshake_callbacks, connection).WillOnce(ReturnRef(mock_connection)); + ON_CALL(handshake_callbacks, connection).WillByDefault(ReturnRef(mock_connection)); EXPECT_CALL(handshake_callbacks, onFailure); SslHandshakerImpl handshaker(std::move(server_ssl_), 0, &handshake_callbacks); From b41e65598b89e513d5e1364b835e91f0147f8b27 Mon Sep 17 00:00:00 2001 From: Shikugawa Date: Sat, 9 Jan 2021 12:38:10 +0000 Subject: [PATCH 6/7] add test Signed-off-by: Shikugawa --- .../transport_sockets/tls/utility_test.cc | 28 +++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/test/extensions/transport_sockets/tls/utility_test.cc b/test/extensions/transport_sockets/tls/utility_test.cc index ca7f5cab7396..c6558266e9bb 100644 --- a/test/extensions/transport_sockets/tls/utility_test.cc +++ b/test/extensions/transport_sockets/tls/utility_test.cc @@ -138,6 +138,34 @@ TEST(UtilityTest, TestGetCertificationExtensionValue) { EXPECT_EQ("", Utility::getCertificateExtensionValue(*cert, "foo")); } +TEST(UtilityTest, SslErrorDescriptionTest) { + const std::vector> test_set = { + {0, "NONE"}, + {1, "SSL"}, + {2, "WANT_READ"}, + {3, "WANT_WRITE"}, + {4, "WANT_X509_LOOKUP"}, + {5, "SYSCALL"}, + {6, "ZERO_RETURN"}, + {7, "WANT_CONNECT"}, + {8, "WANT_ACCEPT"}, + {9, "WANT_CHANNEL_ID_LOOKUP"}, + {11, "PENDING_SESSION"}, + {12, "PENDING_CERTIFICATE"}, + {13, "WANT_PRIVATE_KEY_OPERATION"}, + {14, "PENDING_TICKET"}, + {15, "EARLY_DATA_REJECTED"}, + {16, "WANT_CERTIFICATE_VERIFY"}, + {17, "HANDOFF"}, + {18, "HANDBACK"}, + {19, ""}, + }; + + for (const auto& test_data : test_set) { + EXPECT_EQ(test_data.second, Utility::getErrorDescription(test_data.first)); + } +} + } // namespace } // namespace Tls } // namespace TransportSockets From 53f781030c4b239663c033c399ec712926f82295 Mon Sep 17 00:00:00 2001 From: Shikugawa Date: Fri, 15 Jan 2021 17:27:43 +0900 Subject: [PATCH 7/7] review Signed-off-by: Shikugawa --- .../transport_sockets/tls/utility.cc | 41 ++++++++++--------- .../transport_sockets/tls/utility.h | 25 ++++++++++- .../transport_sockets/tls/utility_test.cc | 7 +++- 3 files changed, 51 insertions(+), 22 deletions(-) diff --git a/source/extensions/transport_sockets/tls/utility.cc b/source/extensions/transport_sockets/tls/utility.cc index ea69047725fc..775fc763077b 100644 --- a/source/extensions/transport_sockets/tls/utility.cc +++ b/source/extensions/transport_sockets/tls/utility.cc @@ -215,46 +215,47 @@ absl::optional Utility::getLastCryptoError() { return absl::nullopt; } -std::string Utility::getErrorDescription(int err) { +absl::string_view Utility::getErrorDescription(int err) { switch (err) { case SSL_ERROR_NONE: - return "NONE"; + return SSL_ERROR_NONE_MESSAGE; case SSL_ERROR_SSL: - return "SSL"; + return SSL_ERROR_SSL_MESSAGE; case SSL_ERROR_WANT_READ: - return "WANT_READ"; + return SSL_ERROR_WANT_READ_MESSAGE; case SSL_ERROR_WANT_WRITE: - return "WANT_WRITE"; + return SSL_ERROR_WANT_WRITE_MESSAGE; case SSL_ERROR_WANT_X509_LOOKUP: - return "WANT_X509_LOOKUP"; + return SSL_ERROR_WANT_X509_LOOPUP_MESSAGE; case SSL_ERROR_SYSCALL: - return "SYSCALL"; + return SSL_ERROR_SYSCALL_MESSAGE; case SSL_ERROR_ZERO_RETURN: - return "ZERO_RETURN"; + return SSL_ERROR_ZERO_RETURN_MESSAGE; case SSL_ERROR_WANT_CONNECT: - return "WANT_CONNECT"; + return SSL_ERROR_WANT_CONNECT_MESSAGE; case SSL_ERROR_WANT_ACCEPT: - return "WANT_ACCEPT"; + return SSL_ERROR_WANT_ACCEPT_MESSAGE; case SSL_ERROR_WANT_CHANNEL_ID_LOOKUP: - return "WANT_CHANNEL_ID_LOOKUP"; + return SSL_ERROR_WANT_CHANNEL_ID_LOOKUP_MESSAGE; case SSL_ERROR_PENDING_SESSION: - return "PENDING_SESSION"; + return SSL_ERROR_PENDING_SESSION_MESSAGE; case SSL_ERROR_PENDING_CERTIFICATE: - return "PENDING_CERTIFICATE"; + return SSL_ERROR_PENDING_CERTIFICATE_MESSAGE; case SSL_ERROR_WANT_PRIVATE_KEY_OPERATION: - return "WANT_PRIVATE_KEY_OPERATION"; + return SSL_ERROR_WANT_PRIVATE_KEY_OPERATION_MESSAGE; case SSL_ERROR_PENDING_TICKET: - return "PENDING_TICKET"; + return SSL_ERROR_PENDING_TICKET_MESSAGE; case SSL_ERROR_EARLY_DATA_REJECTED: - return "EARLY_DATA_REJECTED"; + return SSL_ERROR_EARLY_DATA_REJECTED_MESSAGE; case SSL_ERROR_WANT_CERTIFICATE_VERIFY: - return "WANT_CERTIFICATE_VERIFY"; + return SSL_ERROR_WANT_CERTIFICATE_VERIFY_MESSAGE; case SSL_ERROR_HANDOFF: - return "HANDOFF"; + return SSL_ERROR_HANDOFF_MESSAGE; case SSL_ERROR_HANDBACK: - return "HANDBACK"; + return SSL_ERROR_HANDBACK_MESSAGE; default: - return EMPTY_STRING; + ENVOY_BUG(false, "Unknown BoringSSL error had occurred"); + return SSL_ERROR_UNKNOWN_ERROR_MESSAGE; } } diff --git a/source/extensions/transport_sockets/tls/utility.h b/source/extensions/transport_sockets/tls/utility.h index f96881cb4d14..68c82de170fe 100644 --- a/source/extensions/transport_sockets/tls/utility.h +++ b/source/extensions/transport_sockets/tls/utility.h @@ -15,6 +15,29 @@ namespace TransportSockets { namespace Tls { namespace Utility { +static constexpr absl::string_view SSL_ERROR_NONE_MESSAGE = "NONE"; +static constexpr absl::string_view SSL_ERROR_SSL_MESSAGE = "SSL"; +static constexpr absl::string_view SSL_ERROR_WANT_READ_MESSAGE = "WANT_READ"; +static constexpr absl::string_view SSL_ERROR_WANT_WRITE_MESSAGE = "WANT_WRITE"; +static constexpr absl::string_view SSL_ERROR_WANT_X509_LOOPUP_MESSAGE = "WANT_X509_LOOKUP"; +static constexpr absl::string_view SSL_ERROR_SYSCALL_MESSAGE = "SYSCALL"; +static constexpr absl::string_view SSL_ERROR_ZERO_RETURN_MESSAGE = "ZERO_RETURN"; +static constexpr absl::string_view SSL_ERROR_WANT_CONNECT_MESSAGE = "WANT_CONNECT"; +static constexpr absl::string_view SSL_ERROR_WANT_ACCEPT_MESSAGE = "WANT_ACCEPT"; +static constexpr absl::string_view SSL_ERROR_WANT_CHANNEL_ID_LOOKUP_MESSAGE = + "WANT_CHANNEL_ID_LOOKUP"; +static constexpr absl::string_view SSL_ERROR_PENDING_SESSION_MESSAGE = "PENDING_SESSION"; +static constexpr absl::string_view SSL_ERROR_PENDING_CERTIFICATE_MESSAGE = "PENDING_CERTIFICATE"; +static constexpr absl::string_view SSL_ERROR_WANT_PRIVATE_KEY_OPERATION_MESSAGE = + "WANT_PRIVATE_KEY_OPERATION"; +static constexpr absl::string_view SSL_ERROR_PENDING_TICKET_MESSAGE = "PENDING_TICKET"; +static constexpr absl::string_view SSL_ERROR_EARLY_DATA_REJECTED_MESSAGE = "EARLY_DATA_REJECTED"; +static constexpr absl::string_view SSL_ERROR_WANT_CERTIFICATE_VERIFY_MESSAGE = + "WANT_CERTIFICATE_VERIFY"; +static constexpr absl::string_view SSL_ERROR_HANDOFF_MESSAGE = "HANDOFF"; +static constexpr absl::string_view SSL_ERROR_HANDBACK_MESSAGE = "HANDBACK"; +static constexpr absl::string_view SSL_ERROR_UNKNOWN_ERROR_MESSAGE = "UNKNOWN_ERROR"; + /** * Retrieves the serial number of a certificate. * @param cert the certificate @@ -94,7 +117,7 @@ absl::optional getLastCryptoError(); * @param err error code * @return string message corresponding error code. */ -std::string getErrorDescription(int err); +absl::string_view getErrorDescription(int err); } // namespace Utility } // namespace Tls diff --git a/test/extensions/transport_sockets/tls/utility_test.cc b/test/extensions/transport_sockets/tls/utility_test.cc index c6558266e9bb..d1823952ac67 100644 --- a/test/extensions/transport_sockets/tls/utility_test.cc +++ b/test/extensions/transport_sockets/tls/utility_test.cc @@ -158,12 +158,17 @@ TEST(UtilityTest, SslErrorDescriptionTest) { {16, "WANT_CERTIFICATE_VERIFY"}, {17, "HANDOFF"}, {18, "HANDBACK"}, - {19, ""}, }; for (const auto& test_data : test_set) { EXPECT_EQ(test_data.second, Utility::getErrorDescription(test_data.first)); } + +#if defined(NDEBUG) + EXPECT_EQ(Utility::getErrorDescription(19), "UNKNOWN_ERROR"); +#else + EXPECT_DEATH(Utility::getErrorDescription(19), "Unknown BoringSSL error had occurred"); +#endif } } // namespace