ssl: make Ssl::Connection const everywhere (#4179)

Signed-off-by: Lizan Zhou <zlizan@google.com>

include/envoy/network/connection.h

@@ -176,11 +176,6 @@ class Connection : public Event::DeferredDeletable, public FilterManager {
    */
   virtual void setConnectionStats(const ConnectionStats& stats) PURE;
 
-  /**
-   * @return the SSL connection data if this is an SSL connection, or nullptr if it is not.
-   */
-  virtual Ssl::Connection* ssl() PURE;
-
   /**
    * @return the const SSL connection data if this is an SSL connection, or nullptr if it is not.
    */

include/envoy/network/transport_socket.h

@@ -128,11 +128,6 @@ class TransportSocket {
    */
   virtual void onConnected() PURE;
 
-  /**
-   * @return the SSL connection data if this is an SSL connection, or nullptr if it is not.
-   */
-  virtual Ssl::Connection* ssl() PURE;
-
   /**
    * @return the const SSL connection data if this is an SSL connection, or nullptr if it is not.
    */

include/envoy/ssl/connection.h

@@ -24,7 +24,7 @@ class Connection {
    * @return std::string the URI in the SAN feld of the local certificate. Returns "" if there is no
    *         local certificate, or no SAN field, or no URI.
    **/
-  virtual std::string uriSanLocalCertificate() PURE;
+  virtual std::string uriSanLocalCertificate() const PURE;
 
   /**
    * @return std::string the subject field of the local certificate in RFC 2253 format. Returns ""
@@ -66,13 +66,13 @@ class Connection {
    * @return std::vector<std::string> the DNS entries in the SAN field of the peer certificate.
    *         Returns {} if there is no peer certificate, or no SAN field, or no DNS.
    **/
-  virtual std::vector<std::string> dnsSansPeerCertificate() PURE;
+  virtual std::vector<std::string> dnsSansPeerCertificate() const PURE;
 
   /**
    * @return std::vector<std::string> the DNS entries in the SAN field of the local certificate.
    *         Returns {} if there is no local certificate, or no SAN field, or no DNS.
    **/
-  virtual std::vector<std::string> dnsSansLocalCertificate() PURE;
+  virtual std::vector<std::string> dnsSansLocalCertificate() const PURE;
 };
 
 } // namespace Ssl

source/common/network/connection_impl.h

@@ -78,7 +78,6 @@ class ConnectionImpl : public virtual Connection,
     return socket_->localAddress();
   }
   void setConnectionStats(const ConnectionStats& stats) override;
-  Ssl::Connection* ssl() override { return transport_socket_->ssl(); }
   const Ssl::Connection* ssl() const override { return transport_socket_->ssl(); }
   State state() const override;
   void write(Buffer::Instance& data, bool end_stream) override;

source/common/network/raw_buffer_socket.h

@@ -19,7 +19,6 @@ class RawBufferSocket : public TransportSocket, protected Logger::Loggable<Logge
   void onConnected() override;
   IoResult doRead(Buffer::Instance& buffer) override;
   IoResult doWrite(Buffer::Instance& buffer, bool end_stream) override;
-  Ssl::Connection* ssl() override { return nullptr; }
   const Ssl::Connection* ssl() const override { return nullptr; }
 
 private:

source/common/ssl/ssl_socket.cc

@@ -219,7 +219,7 @@ bool SslSocket::peerCertificatePresented() const {
   return cert != nullptr;
 }
 
-std::string SslSocket::uriSanLocalCertificate() {
+std::string SslSocket::uriSanLocalCertificate() const {
   // The cert object is not owned.
   X509* cert = SSL_get_certificate(ssl_.get());
   if (!cert) {
@@ -228,7 +228,7 @@ std::string SslSocket::uriSanLocalCertificate() {
   return getUriSanFromCertificate(cert);
 }
 
-std::vector<std::string> SslSocket::dnsSansLocalCertificate() {
+std::vector<std::string> SslSocket::dnsSansLocalCertificate() const {
   X509* cert = SSL_get_certificate(ssl_.get());
   if (!cert) {
     return {};
@@ -284,7 +284,7 @@ std::string SslSocket::uriSanPeerCertificate() const {
   return getUriSanFromCertificate(cert.get());
 }
 
-std::vector<std::string> SslSocket::dnsSansPeerCertificate() {
+std::vector<std::string> SslSocket::dnsSansPeerCertificate() const {
   bssl::UniquePtr<X509> cert(SSL_get_peer_certificate(ssl_.get()));
   if (!cert) {
     return {};
@@ -309,7 +309,7 @@ std::string SslSocket::getUriSanFromCertificate(X509* cert) const {
   return "";
 }
 
-std::vector<std::string> SslSocket::getDnsSansFromCertificate(X509* cert) {
+std::vector<std::string> SslSocket::getDnsSansFromCertificate(X509* cert) const {
   bssl::UniquePtr<GENERAL_NAMES> san_names(
       static_cast<GENERAL_NAMES*>(X509_get_ext_d2i(cert, NID_subject_alt_name, nullptr, nullptr)));
   if (san_names == nullptr) {

source/common/ssl/ssl_socket.h

@@ -25,15 +25,15 @@ class SslSocket : public Network::TransportSocket,
 
   // Ssl::Connection
   bool peerCertificatePresented() const override;
-  std::string uriSanLocalCertificate() override;
+  std::string uriSanLocalCertificate() const override;
   const std::string& sha256PeerCertificateDigest() const override;
   std::string serialNumberPeerCertificate() const override;
   std::string subjectPeerCertificate() const override;
   std::string subjectLocalCertificate() const override;
   std::string uriSanPeerCertificate() const override;
   const std::string& urlEncodedPemEncodedPeerCertificate() const override;
-  std::vector<std::string> dnsSansPeerCertificate() override;
-  std::vector<std::string> dnsSansLocalCertificate() override;
+  std::vector<std::string> dnsSansPeerCertificate() const override;
+  std::vector<std::string> dnsSansLocalCertificate() const override;
 
   // Network::TransportSocket
   void setTransportSocketCallbacks(Network::TransportSocketCallbacks& callbacks) override;
@@ -43,10 +43,9 @@ class SslSocket : public Network::TransportSocket,
   Network::IoResult doRead(Buffer::Instance& read_buffer) override;
   Network::IoResult doWrite(Buffer::Instance& write_buffer, bool end_stream) override;
   void onConnected() override;
-  Ssl::Connection* ssl() override { return this; }
   const Ssl::Connection* ssl() const override { return this; }
 
-  SSL* rawSslForTest() { return ssl_.get(); }
+  SSL* rawSslForTest() const { return ssl_.get(); }
 
 private:
   Network::PostIoAction doHandshake();
@@ -56,7 +55,7 @@ class SslSocket : public Network::TransportSocket,
   // TODO: Move helper functions to the `Ssl::Utility` namespace.
   std::string getUriSanFromCertificate(X509* cert) const;
   std::string getSubjectFromCertificate(X509* cert) const;
-  std::vector<std::string> getDnsSansFromCertificate(X509* cert);
+  std::vector<std::string> getDnsSansFromCertificate(X509* cert) const;
 
   Network::TransportSocketCallbacks* callbacks_{};
   ContextImplSharedPtr ctx_;

source/extensions/transport_sockets/capture/capture.cc

@@ -89,8 +89,6 @@ Network::IoResult CaptureSocket::doWrite(Buffer::Instance& buffer, bool end_stre
 
 void CaptureSocket::onConnected() { transport_socket_->onConnected(); }
 
-Ssl::Connection* CaptureSocket::ssl() { return transport_socket_->ssl(); }
-
 const Ssl::Connection* CaptureSocket::ssl() const { return transport_socket_->ssl(); }
 
 CaptureSocketFactory::CaptureSocketFactory(

source/extensions/transport_sockets/capture/capture.h

@@ -25,7 +25,6 @@ class CaptureSocket : public Network::TransportSocket {
   Network::IoResult doRead(Buffer::Instance& buffer) override;
   Network::IoResult doWrite(Buffer::Instance& buffer, bool end_stream) override;
   void onConnected() override;
-  Ssl::Connection* ssl() override;
   const Ssl::Connection* ssl() const override;
 
 private:

test/common/ssl/ssl_socket_test.cc

@@ -177,7 +177,8 @@ const std::string testUtilV2(const envoy::api::v2::Listener& server_proto,
       client_ssl_socket_factory.createTransportSocket(), nullptr);
 
   if (!client_session.empty()) {
-    Ssl::SslSocket* ssl_socket = dynamic_cast<Ssl::SslSocket*>(client_connection->ssl());
+    const Ssl::SslSocket* ssl_socket =
+        dynamic_cast<const Ssl::SslSocket*>(client_connection->ssl());
     SSL* client_ssl_socket = ssl_socket->rawSslForTest();
     SSL_CTX* client_ssl_context = SSL_get_SSL_CTX(client_ssl_socket);
     SSL_SESSION* client_ssl_session =
@@ -218,7 +219,8 @@ const std::string testUtilV2(const envoy::api::v2::Listener& server_proto,
         EXPECT_EQ(expected_alpn_protocol, client_connection->nextProtocol());
       }
       EXPECT_EQ(expected_client_cert_uri, server_connection->ssl()->uriSanPeerCertificate());
-      Ssl::SslSocket* ssl_socket = dynamic_cast<Ssl::SslSocket*>(client_connection->ssl());
+      const Ssl::SslSocket* ssl_socket =
+          dynamic_cast<const Ssl::SslSocket*>(client_connection->ssl());
       SSL* client_ssl_socket = ssl_socket->rawSslForTest();
       if (!expected_protocol_version.empty()) {
         EXPECT_EQ(expected_protocol_version, SSL_get_version(client_ssl_socket));
@@ -1705,7 +1707,7 @@ TEST_P(SslSocketTest, ClientAuthMultipleCAs) {
       ssl_socket_factory.createTransportSocket(), nullptr);
 
   // Verify that server sent list with 2 acceptable client certificate CA names.
-  Ssl::SslSocket* ssl_socket = dynamic_cast<Ssl::SslSocket*>(client_connection->ssl());
+  const Ssl::SslSocket* ssl_socket = dynamic_cast<const Ssl::SslSocket*>(client_connection->ssl());
   SSL_set_cert_cb(ssl_socket->rawSslForTest(),
                   [](SSL* ssl, void*) -> int {
                     STACK_OF(X509_NAME)* list = SSL_get_client_CA_list(ssl);
@@ -1805,7 +1807,8 @@ void testTicketSessionResumption(const std::string& server_ctx_json1,
 
   EXPECT_CALL(client_connection_callbacks, onEvent(Network::ConnectionEvent::Connected))
       .WillOnce(Invoke([&](Network::ConnectionEvent) -> void {
-        Ssl::SslSocket* ssl_socket = dynamic_cast<Ssl::SslSocket*>(client_connection->ssl());
+        const Ssl::SslSocket* ssl_socket =
+            dynamic_cast<const Ssl::SslSocket*>(client_connection->ssl());
         ssl_session = SSL_get1_session(ssl_socket->rawSslForTest());
         EXPECT_TRUE(SSL_SESSION_is_resumable(ssl_session));
         client_connection->close(Network::ConnectionCloseType::NoFlush);
@@ -1822,7 +1825,7 @@ void testTicketSessionResumption(const std::string& server_ctx_json1,
       socket2.localAddress(), Network::Address::InstanceConstSharedPtr(),
       ssl_socket_factory.createTransportSocket(), nullptr);
   client_connection->addConnectionCallbacks(client_connection_callbacks);
-  Ssl::SslSocket* ssl_socket = dynamic_cast<Ssl::SslSocket*>(client_connection->ssl());
+  const Ssl::SslSocket* ssl_socket = dynamic_cast<const Ssl::SslSocket*>(client_connection->ssl());
   SSL_set_session(ssl_socket->rawSslForTest(), ssl_session);
   SSL_SESSION_free(ssl_session);
 
@@ -2179,7 +2182,8 @@ TEST_P(SslSocketTest, ClientAuthCrossListenerSessionResumption) {
   EXPECT_CALL(server_connection_callbacks, onEvent(Network::ConnectionEvent::LocalClose));
   EXPECT_CALL(client_connection_callbacks, onEvent(Network::ConnectionEvent::Connected))
       .WillOnce(Invoke([&](Network::ConnectionEvent) -> void {
-        Ssl::SslSocket* ssl_socket = dynamic_cast<Ssl::SslSocket*>(client_connection->ssl());
+        const Ssl::SslSocket* ssl_socket =
+            dynamic_cast<const Ssl::SslSocket*>(client_connection->ssl());
         ssl_session = SSL_get1_session(ssl_socket->rawSslForTest());
         EXPECT_TRUE(SSL_SESSION_is_resumable(ssl_session));
         server_connection->close(Network::ConnectionCloseType::NoFlush);
@@ -2197,7 +2201,7 @@ TEST_P(SslSocketTest, ClientAuthCrossListenerSessionResumption) {
       socket2.localAddress(), Network::Address::InstanceConstSharedPtr(),
       ssl_socket_factory.createTransportSocket(), nullptr);
   client_connection->addConnectionCallbacks(client_connection_callbacks);
-  Ssl::SslSocket* ssl_socket = dynamic_cast<Ssl::SslSocket*>(client_connection->ssl());
+  const Ssl::SslSocket* ssl_socket = dynamic_cast<const Ssl::SslSocket*>(client_connection->ssl());
   SSL_set_session(ssl_socket->rawSslForTest(), ssl_session);
   SSL_SESSION_free(ssl_session);
 

test/mocks/network/mocks.h

@@ -77,7 +77,6 @@ class MockConnection : public Connection, public MockConnectionBase {
   MOCK_CONST_METHOD0(remoteAddress, const Address::InstanceConstSharedPtr&());
   MOCK_CONST_METHOD0(localAddress, const Address::InstanceConstSharedPtr&());
   MOCK_METHOD1(setConnectionStats, void(const ConnectionStats& stats));
-  MOCK_METHOD0(ssl, Ssl::Connection*());
   MOCK_CONST_METHOD0(ssl, const Ssl::Connection*());
   MOCK_CONST_METHOD0(requestedServerName, absl::string_view());
   MOCK_CONST_METHOD0(state, State());
@@ -117,7 +116,6 @@ class MockClientConnection : public ClientConnection, public MockConnectionBase
   MOCK_CONST_METHOD0(remoteAddress, const Address::InstanceConstSharedPtr&());
   MOCK_CONST_METHOD0(localAddress, const Address::InstanceConstSharedPtr&());
   MOCK_METHOD1(setConnectionStats, void(const ConnectionStats& stats));
-  MOCK_METHOD0(ssl, Ssl::Connection*());
   MOCK_CONST_METHOD0(ssl, const Ssl::Connection*());
   MOCK_CONST_METHOD0(requestedServerName, absl::string_view());
   MOCK_CONST_METHOD0(state, State());
@@ -435,7 +433,6 @@ class MockTransportSocket : public TransportSocket {
   MOCK_METHOD1(doRead, IoResult(Buffer::Instance& buffer));
   MOCK_METHOD2(doWrite, IoResult(Buffer::Instance& buffer, bool end_stream));
   MOCK_METHOD0(onConnected, void());
-  MOCK_METHOD0(ssl, Ssl::Connection*());
   MOCK_CONST_METHOD0(ssl, const Ssl::Connection*());
 };
 

test/mocks/ssl/mocks.h

@@ -36,15 +36,15 @@ class MockConnection : public Connection {
   ~MockConnection();
 
   MOCK_CONST_METHOD0(peerCertificatePresented, bool());
-  MOCK_METHOD0(uriSanLocalCertificate, std::string());
+  MOCK_CONST_METHOD0(uriSanLocalCertificate, std::string());
   MOCK_CONST_METHOD0(sha256PeerCertificateDigest, std::string&());
   MOCK_CONST_METHOD0(serialNumberPeerCertificate, std::string());
   MOCK_CONST_METHOD0(subjectPeerCertificate, std::string());
   MOCK_CONST_METHOD0(uriSanPeerCertificate, std::string());
   MOCK_CONST_METHOD0(subjectLocalCertificate, std::string());
   MOCK_CONST_METHOD0(urlEncodedPemEncodedPeerCertificate, std::string&());
-  MOCK_METHOD0(dnsSansPeerCertificate, std::vector<std::string>());
-  MOCK_METHOD0(dnsSansLocalCertificate, std::vector<std::string>());
+  MOCK_CONST_METHOD0(dnsSansPeerCertificate, std::vector<std::string>());
+  MOCK_CONST_METHOD0(dnsSansLocalCertificate, std::vector<std::string>());
 };
 
 class MockClientContext : public ClientContext {