diff --git a/include/envoy/ssl/connection.h b/include/envoy/ssl/connection.h index 8241c48ad8d7c..c0bca9c6ab321 100644 --- a/include/envoy/ssl/connection.h +++ b/include/envoy/ssl/connection.h @@ -137,17 +137,6 @@ class ConnectionInfo { * connection. **/ virtual const std::string& tlsVersion() const PURE; - - /** - * Retrieves the contents of the ``ASN.1`` object stored as an X.509 extension from the peer cert, - * if a peer cert exists and it contains the specified extension. - * - * Note: This is used out of tree, check with @snowp before removing. - * @param extension_name name of extension to look up. - * @return absl::optional the raw octets of the extension ``ASN.1`` object, if it - * exists. - */ - virtual absl::optional x509Extension(absl::string_view extension_name) const PURE; }; using ConnectionInfoConstSharedPtr = std::shared_ptr; diff --git a/source/extensions/transport_sockets/tls/ssl_socket.cc b/source/extensions/transport_sockets/tls/ssl_socket.cc index 36cc4886653d6..31dab2d3b6a10 100644 --- a/source/extensions/transport_sockets/tls/ssl_socket.cc +++ b/source/extensions/transport_sockets/tls/ssl_socket.cc @@ -506,15 +506,6 @@ const std::string& SslHandshakerImpl::tlsVersion() const { return cached_tls_version_; } -absl::optional -SslHandshakerImpl::x509Extension(absl::string_view extension_name) const { - bssl::UniquePtr cert(SSL_get_peer_certificate(ssl())); - if (!cert) { - return absl::nullopt; - } - return Utility::getX509ExtensionValue(*cert, extension_name); -} - Network::PostIoAction SslHandshakerImpl::doHandshake() { ASSERT(state_ != Ssl::SocketState::HandshakeComplete && state_ != Ssl::SocketState::ShutdownSent); int rc = SSL_do_handshake(ssl()); diff --git a/source/extensions/transport_sockets/tls/ssl_socket.h b/source/extensions/transport_sockets/tls/ssl_socket.h index 425e6644b209e..2fcc78445cd28 100644 --- a/source/extensions/transport_sockets/tls/ssl_socket.h +++ b/source/extensions/transport_sockets/tls/ssl_socket.h @@ -78,7 +78,6 @@ class SslHandshakerImpl : public Envoy::Ssl::ConnectionInfo, public Envoy::Ssl:: uint16_t ciphersuiteId() const override; std::string ciphersuiteString() const override; const std::string& tlsVersion() const override; - absl::optional x509Extension(absl::string_view extension_name) const override; // Ssl::Handshaker Network::PostIoAction doHandshake() override; diff --git a/source/extensions/transport_sockets/tls/utility.cc b/source/extensions/transport_sockets/tls/utility.cc index 476dda8c1b925..e5b8bc17085f8 100644 --- a/source/extensions/transport_sockets/tls/utility.cc +++ b/source/extensions/transport_sockets/tls/utility.cc @@ -4,6 +4,7 @@ #include "common/network/address_impl.h" #include "absl/strings/str_join.h" +#include "openssl/x509v3.h" namespace Envoy { namespace Extensions { @@ -152,38 +153,34 @@ int32_t Utility::getDaysUntilExpiration(const X509* cert, TimeSource& time_sourc return 0; } -absl::optional Utility::getX509ExtensionValue(const X509& cert, - absl::string_view extension_name) { - X509_EXTENSIONS* extensions(X509_get0_extensions(&cert)); - - if (extensions == nullptr) { - return absl::nullopt; +absl::string_view Utility::getCertificateExtensionValue(X509& cert, + absl::string_view extension_name) { + bssl::UniquePtr oid( + OBJ_txt2obj(std::string(extension_name).c_str(), 1 /* don't search names */)); + if (oid == nullptr) { + return {}; } - const size_t extension_count = sk_X509_EXTENSION_num(extensions); + int pos = X509_get_ext_by_OBJ(&cert, oid.get(), -1); + if (pos < 0) { + return {}; + } - for (size_t i = 0; i < extension_count; ++i) { - X509_EXTENSION* extension = sk_X509_EXTENSION_value(extensions, i); + X509_EXTENSION* extension = X509_get_ext(&cert, pos); + if (extension == nullptr) { + return {}; + } - ASN1_OBJECT* extension_object = X509_EXTENSION_get_object(extension); - const size_t size = OBJ_obj2txt(nullptr, 0, extension_object, 0); - std::vector buffer; - // +1 to allow for NULL byte. - buffer.resize(size + 1); - OBJ_obj2txt(buffer.data(), buffer.size(), extension_object, 0); + const ASN1_OCTET_STRING* octet_string = X509_EXTENSION_get_data(extension); + RELEASE_ASSERT(octet_string != nullptr, ""); - if (absl::string_view(buffer.data(), size) == extension_name) { - ASN1_OCTET_STRING* octet_string = X509_EXTENSION_get_data(extension); - const unsigned char* octet_string_data = octet_string->data; - long xlen; - int tag, xclass; - ASN1_get_object(&octet_string_data, &xlen, &tag, &xclass, octet_string->length); + // Return the entire DER-encoded value for this extension. Correct decoding depends on + // knowledge of the expected structure of the extension's value. + const unsigned char* octet_string_data = ASN1_STRING_get0_data(octet_string); + const int octet_string_length = ASN1_STRING_length(octet_string); - return std::string(reinterpret_cast(octet_string_data), xlen); - } - } - - return absl::nullopt; + return {reinterpret_cast(octet_string_data), + static_cast(octet_string_length)}; } SystemTime Utility::getValidFrom(const X509& cert) { diff --git a/source/extensions/transport_sockets/tls/utility.h b/source/extensions/transport_sockets/tls/utility.h index 211d02457975d..f4f06eb4cb234 100644 --- a/source/extensions/transport_sockets/tls/utility.h +++ b/source/extensions/transport_sockets/tls/utility.h @@ -55,11 +55,10 @@ std::string getSubjectFromCertificate(X509& cert); /** * Retrieves the value of a specific X509 extension from the cert, if present. * @param cert the certificate. - * @param extension_name the name of the extension to extract. - * @return absl::optional the value of the extension field, if present. + * @param extension_name the name of the extension to extract in dotted number format + * @return absl::string_view the DER-encoded value of the extension field or empty if not present. */ -absl::optional getX509ExtensionValue(const X509& cert, - absl::string_view extension_name); +absl::string_view getCertificateExtensionValue(X509& cert, absl::string_view extension_name); /** * Returns the days until this certificate is valid. diff --git a/test/extensions/transport_sockets/tls/ssl_socket_test.cc b/test/extensions/transport_sockets/tls/ssl_socket_test.cc index 69a721d3ec3fb..984d3d0e438ef 100644 --- a/test/extensions/transport_sockets/tls/ssl_socket_test.cc +++ b/test/extensions/transport_sockets/tls/ssl_socket_test.cc @@ -240,18 +240,6 @@ class TestUtilOptions : public TestUtilOptionsBase { Network::ConnectionEvent expectedServerCloseEvent() const { return expected_server_close_event_; } - TestUtilOptions& addExpected509Extension(absl::string_view name, - absl::optional value) { - expected_x509_extensions_[name] = std::move(value); - - return *this; - } - - const absl::flat_hash_map>& - expectedX509Extensions() const { - return expected_x509_extensions_; - } - private: const std::string client_ctx_yaml_; const std::string server_ctx_yaml_; @@ -271,7 +259,6 @@ class TestUtilOptions : public TestUtilOptionsBase { std::string expected_peer_cert_chain_; std::string expected_valid_from_peer_cert_; std::string expected_expiration_peer_cert_; - absl::flat_hash_map> expected_x509_extensions_; }; void testUtil(const TestUtilOptions& options) { @@ -427,10 +414,6 @@ void testUtil(const TestUtilOptions& options) { server_connection->ssl()->urlEncodedPemEncodedPeerCertificateChain()); } - for (const auto& expected_extension : options.expectedX509Extensions()) { - const auto& result = server_connection->ssl()->x509Extension(expected_extension.first); - EXPECT_EQ(expected_extension.second, result); - } // By default, the session is not created with session resumption. The // client should see a session ID but the server should not. EXPECT_EQ(EMPTY_STRING, server_connection->ssl()->sessionId()); @@ -1467,7 +1450,7 @@ TEST_P(SslSocketTest, FailedClientAuthSanVerification) { testUtil(test_options.setExpectedServerStats("ssl.fail_verify_san")); } -TEST_P(SslSocketTest, X509Extensions) { +TEST_P(SslSocketTest, X509ExtensionsCertificateSerialNumber) { const std::string client_ctx_yaml = R"EOF( common_tls_context: tls_certificates: @@ -1491,10 +1474,7 @@ TEST_P(SslSocketTest, X509Extensions) { )EOF"; TestUtilOptions test_options(client_ctx_yaml, server_ctx_yaml, true, GetParam()); - testUtil(test_options.addExpected509Extension("1.2.3.4.5.6.7.8", absl::make_optional("Something")) - .addExpected509Extension("1.2.3.4.5.6.7.9", "\x1\x1\xFF") - .addExpected509Extension("1.2.3.4", absl::nullopt) - .setExpectedSerialNumber(TEST_EXTENSIONS_CERT_SERIAL)); + testUtil(test_options.setExpectedSerialNumber(TEST_EXTENSIONS_CERT_SERIAL)); } // By default, expired certificates are not permitted. diff --git a/test/extensions/transport_sockets/tls/utility_test.cc b/test/extensions/transport_sockets/tls/utility_test.cc index 5adbe2614ba89..ca7f5cab7396a 100644 --- a/test/extensions/transport_sockets/tls/utility_test.cc +++ b/test/extensions/transport_sockets/tls/utility_test.cc @@ -127,6 +127,17 @@ TEST(UtilityTest, GetLastCryptoError) { EXPECT_FALSE(Utility::getLastCryptoError().has_value()); } +TEST(UtilityTest, TestGetCertificationExtensionValue) { + bssl::UniquePtr cert = readCertFromFile(TestEnvironment::substitute( + "{{ test_rundir }}/test/extensions/transport_sockets/tls/test_data/extensions_cert.pem")); + EXPECT_EQ("\xc\x9Something", Utility::getCertificateExtensionValue(*cert, "1.2.3.4.5.6.7.8")); + EXPECT_EQ("\x30\x3\x1\x1\xFF", Utility::getCertificateExtensionValue(*cert, "1.2.3.4.5.6.7.9")); + EXPECT_EQ("", Utility::getCertificateExtensionValue(*cert, "1.2.3.4.5.6.7.10")); + EXPECT_EQ("", Utility::getCertificateExtensionValue(*cert, "1.2.3.4")); + EXPECT_EQ("", Utility::getCertificateExtensionValue(*cert, "")); + EXPECT_EQ("", Utility::getCertificateExtensionValue(*cert, "foo")); +} + } // namespace } // namespace Tls } // namespace TransportSockets diff --git a/test/mocks/ssl/mocks.h b/test/mocks/ssl/mocks.h index 7567e5807cff0..5f25bd9a18cd6 100644 --- a/test/mocks/ssl/mocks.h +++ b/test/mocks/ssl/mocks.h @@ -58,7 +58,6 @@ class MockConnectionInfo : public ConnectionInfo { MOCK_METHOD(uint16_t, ciphersuiteId, (), (const)); MOCK_METHOD(std::string, ciphersuiteString, (), (const)); MOCK_METHOD(const std::string&, tlsVersion, (), (const)); - MOCK_METHOD(absl::optional, x509Extension, (absl::string_view), (const)); }; class MockClientContext : public ClientContext {