tls: replace getX509ExtensionValue

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<std::string> the raw octets of the extension ``ASN.1`` object, if it
-   * exists.
-   */
-  virtual absl::optional<std::string> x509Extension(absl::string_view extension_name) const PURE;
 };
 
 using ConnectionInfoConstSharedPtr = std::shared_ptr<const ConnectionInfo>;

source/extensions/transport_sockets/tls/ssl_socket.cc

@@ -506,15 +506,6 @@ const std::string& SslHandshakerImpl::tlsVersion() const {
   return cached_tls_version_;
 }
 
-absl::optional<std::string>
-SslHandshakerImpl::x509Extension(absl::string_view extension_name) const {
-  bssl::UniquePtr<X509> 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());

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<std::string> x509Extension(absl::string_view extension_name) const override;
 
   // Ssl::Handshaker
   Network::PostIoAction doHandshake() override;

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<std::string> 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<ASN1_OBJECT> 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<char> 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<const char*>(octet_string_data), xlen);
-    }
-  }
-
-  return absl::nullopt;
+  return {reinterpret_cast<const char*>(octet_string_data),
+          static_cast<absl::string_view::size_type>(octet_string_length)};
 }
 
 SystemTime Utility::getValidFrom(const X509& cert) {

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<std::string> 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<std::string> 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.

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<std::string> value) {
-    expected_x509_extensions_[name] = std::move(value);
-
-    return *this;
-  }
-
-  const absl::flat_hash_map<std::string, absl::optional<std::string>>&
-  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<std::string, absl::optional<std::string>> 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.

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<X509> 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

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<std::string>, x509Extension, (absl::string_view), (const));
 };
 
 class MockClientContext : public ClientContext {