diff --git a/include/envoy/ssl/connection.h b/include/envoy/ssl/connection.h index d4e54ba92e41..d586d9fe09a5 100644 --- a/include/envoy/ssl/connection.h +++ b/include/envoy/ssl/connection.h @@ -33,7 +33,7 @@ class ConnectionInfo { * @return std::string the subject field of the local certificate in RFC 2253 format. Returns "" * if there is no local certificate, or no subject. **/ - virtual std::string subjectLocalCertificate() const PURE; + virtual const std::string& subjectLocalCertificate() const PURE; /** * @return std::string the SHA256 digest of the peer certificate. Returns "" if there is no peer @@ -45,19 +45,19 @@ class ConnectionInfo { * @return std::string the serial number field of the peer certificate. Returns "" if * there is no peer certificate, or no serial number. **/ - virtual std::string serialNumberPeerCertificate() const PURE; + virtual const std::string& serialNumberPeerCertificate() const PURE; /** * @return std::string the issuer field of the peer certificate in RFC 2253 format. Returns "" if * there is no peer certificate, or no issuer. **/ - virtual std::string issuerPeerCertificate() const PURE; + virtual const std::string& issuerPeerCertificate() const PURE; /** * @return std::string the subject field of the peer certificate in RFC 2253 format. Returns "" if * there is no peer certificate, or no subject. **/ - virtual std::string subjectPeerCertificate() const PURE; + virtual const std::string& subjectPeerCertificate() const PURE; /** * @return std::string the URIs in the SAN field of the peer certificate. Returns {} if there is @@ -105,7 +105,7 @@ class ConnectionInfo { /** * @return std::string the hex-encoded TLS session ID as defined in rfc5246. **/ - virtual std::string sessionId() const PURE; + virtual const std::string& sessionId() const PURE; /** * @return uint16_t the standard ID for the ciphers used in the established TLS connection. @@ -123,7 +123,7 @@ class ConnectionInfo { * @return std::string the TLS version (e.g., TLSv1.2, TLSv1.3) used in the established TLS * connection. **/ - virtual std::string tlsVersion() const PURE; + virtual const std::string& tlsVersion() 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 f0ec870245a9..7737d36b160f 100644 --- a/source/extensions/transport_sockets/tls/ssl_socket.cc +++ b/source/extensions/transport_sockets/tls/ssl_socket.cc @@ -475,11 +475,17 @@ std::string SslSocketInfo::ciphersuiteString() const { return SSL_CIPHER_get_name(cipher); } -std::string SslSocketInfo::tlsVersion() const { return SSL_get_version(ssl_.get()); } +const std::string& SslSocketInfo::tlsVersion() const { + if (!cached_tls_version_.empty()) { + return cached_tls_version_; + } + cached_tls_version_ = SSL_get_version(ssl_.get()); + return cached_tls_version_; +} absl::string_view SslSocket::failureReason() const { return failure_reason_; } -std::string SslSocketInfo::serialNumberPeerCertificate() const { +const std::string& SslSocketInfo::serialNumberPeerCertificate() const { if (!cached_serial_number_peer_certificate_.empty()) { return cached_serial_number_peer_certificate_; } @@ -492,7 +498,7 @@ std::string SslSocketInfo::serialNumberPeerCertificate() const { return cached_serial_number_peer_certificate_; } -std::string SslSocketInfo::issuerPeerCertificate() const { +const std::string& SslSocketInfo::issuerPeerCertificate() const { if (!cached_issuer_peer_certificate_.empty()) { return cached_issuer_peer_certificate_; } @@ -505,7 +511,7 @@ std::string SslSocketInfo::issuerPeerCertificate() const { return cached_issuer_peer_certificate_; } -std::string SslSocketInfo::subjectPeerCertificate() const { +const std::string& SslSocketInfo::subjectPeerCertificate() const { if (!cached_subject_peer_certificate_.empty()) { return cached_subject_peer_certificate_; } @@ -518,7 +524,7 @@ std::string SslSocketInfo::subjectPeerCertificate() const { return cached_subject_peer_certificate_; } -std::string SslSocketInfo::subjectLocalCertificate() const { +const std::string& SslSocketInfo::subjectLocalCertificate() const { if (!cached_subject_local_certificate_.empty()) { return cached_subject_local_certificate_; } @@ -547,7 +553,7 @@ absl::optional SslSocketInfo::expirationPeerCertificate() const { return Utility::getExpirationTime(*cert); } -std::string SslSocketInfo::sessionId() const { +const std::string& SslSocketInfo::sessionId() const { if (!cached_session_id_.empty()) { return cached_session_id_; } diff --git a/source/extensions/transport_sockets/tls/ssl_socket.h b/source/extensions/transport_sockets/tls/ssl_socket.h index 67ab1b28582a..c7b183451b4f 100644 --- a/source/extensions/transport_sockets/tls/ssl_socket.h +++ b/source/extensions/transport_sockets/tls/ssl_socket.h @@ -49,10 +49,10 @@ class SslSocketInfo : public Envoy::Ssl::ConnectionInfo { bool peerCertificatePresented() const override; std::vector uriSanLocalCertificate() const override; const std::string& sha256PeerCertificateDigest() const override; - std::string serialNumberPeerCertificate() const override; - std::string issuerPeerCertificate() const override; - std::string subjectPeerCertificate() const override; - std::string subjectLocalCertificate() const override; + const std::string& serialNumberPeerCertificate() const override; + const std::string& issuerPeerCertificate() const override; + const std::string& subjectPeerCertificate() const override; + const std::string& subjectLocalCertificate() const override; std::vector uriSanPeerCertificate() const override; const std::string& urlEncodedPemEncodedPeerCertificate() const override; const std::string& urlEncodedPemEncodedPeerCertificateChain() const override; @@ -60,10 +60,10 @@ class SslSocketInfo : public Envoy::Ssl::ConnectionInfo { std::vector dnsSansLocalCertificate() const override; absl::optional validFromPeerCertificate() const override; absl::optional expirationPeerCertificate() const override; - std::string sessionId() const override; + const std::string& sessionId() const override; uint16_t ciphersuiteId() const override; std::string ciphersuiteString() const override; - std::string tlsVersion() const override; + const std::string& tlsVersion() const override; SSL* rawSslForTest() const { return ssl_.get(); } @@ -82,6 +82,7 @@ class SslSocketInfo : public Envoy::Ssl::ConnectionInfo { mutable std::vector cached_dns_san_peer_certificate_; mutable std::vector cached_dns_san_local_certificate_; mutable std::string cached_session_id_; + mutable std::string cached_tls_version_; }; class SslSocket : public Network::TransportSocket, diff --git a/test/common/access_log/access_log_formatter_test.cc b/test/common/access_log/access_log_formatter_test.cc index db682288410b..e05f0911a129 100644 --- a/test/common/access_log/access_log_formatter_test.cc +++ b/test/common/access_log/access_log_formatter_test.cc @@ -279,14 +279,17 @@ TEST(AccessLogFormatterTest, streamInfoFormatter) { { StreamInfoFormatter upstream_format("DOWNSTREAM_LOCAL_SUBJECT"); auto connection_info = std::make_shared(); - EXPECT_CALL(*connection_info, subjectLocalCertificate()).WillRepeatedly(Return("subject")); + const std::string subject_local = "subject"; + EXPECT_CALL(*connection_info, subjectLocalCertificate()) + .WillRepeatedly(ReturnRef(subject_local)); EXPECT_CALL(stream_info, downstreamSslConnection()).WillRepeatedly(Return(connection_info)); EXPECT_EQ("subject", upstream_format.format(header, header, header, stream_info)); } { StreamInfoFormatter upstream_format("DOWNSTREAM_LOCAL_SUBJECT"); auto connection_info = std::make_shared(); - EXPECT_CALL(*connection_info, subjectLocalCertificate()).WillRepeatedly(Return("")); + EXPECT_CALL(*connection_info, subjectLocalCertificate()) + .WillRepeatedly(ReturnRef(EMPTY_STRING)); EXPECT_CALL(stream_info, downstreamSslConnection()).WillRepeatedly(Return(connection_info)); EXPECT_EQ("-", upstream_format.format(header, header, header, stream_info)); } @@ -298,14 +301,15 @@ TEST(AccessLogFormatterTest, streamInfoFormatter) { { StreamInfoFormatter upstream_format("DOWNSTREAM_PEER_SUBJECT"); auto connection_info = std::make_shared(); - EXPECT_CALL(*connection_info, subjectPeerCertificate()).WillRepeatedly(Return("subject")); + const std::string subject_peer = "subject"; + EXPECT_CALL(*connection_info, subjectPeerCertificate()).WillRepeatedly(ReturnRef(subject_peer)); EXPECT_CALL(stream_info, downstreamSslConnection()).WillRepeatedly(Return(connection_info)); EXPECT_EQ("subject", upstream_format.format(header, header, header, stream_info)); } { StreamInfoFormatter upstream_format("DOWNSTREAM_PEER_SUBJECT"); auto connection_info = std::make_shared(); - EXPECT_CALL(*connection_info, subjectPeerCertificate()).WillRepeatedly(Return("")); + EXPECT_CALL(*connection_info, subjectPeerCertificate()).WillRepeatedly(ReturnRef(EMPTY_STRING)); EXPECT_CALL(stream_info, downstreamSslConnection()).WillRepeatedly(Return(connection_info)); EXPECT_EQ("-", upstream_format.format(header, header, header, stream_info)); } @@ -317,14 +321,15 @@ TEST(AccessLogFormatterTest, streamInfoFormatter) { { StreamInfoFormatter upstream_format("DOWNSTREAM_TLS_SESSION_ID"); auto connection_info = std::make_shared(); - EXPECT_CALL(*connection_info, sessionId()).WillRepeatedly(Return("deadbeef")); + const std::string session_id = "deadbeef"; + EXPECT_CALL(*connection_info, sessionId()).WillRepeatedly(ReturnRef(session_id)); EXPECT_CALL(stream_info, downstreamSslConnection()).WillRepeatedly(Return(connection_info)); EXPECT_EQ("deadbeef", upstream_format.format(header, header, header, stream_info)); } { StreamInfoFormatter upstream_format("DOWNSTREAM_TLS_SESSION_ID"); auto connection_info = std::make_shared(); - EXPECT_CALL(*connection_info, sessionId()).WillRepeatedly(Return("")); + EXPECT_CALL(*connection_info, sessionId()).WillRepeatedly(ReturnRef(EMPTY_STRING)); EXPECT_CALL(stream_info, downstreamSslConnection()).WillRepeatedly(Return(connection_info)); EXPECT_EQ("-", upstream_format.format(header, header, header, stream_info)); } @@ -357,14 +362,15 @@ TEST(AccessLogFormatterTest, streamInfoFormatter) { { StreamInfoFormatter upstream_format("DOWNSTREAM_TLS_VERSION"); auto connection_info = std::make_shared(); - EXPECT_CALL(*connection_info, tlsVersion()).WillRepeatedly(Return("TLSv1.2")); + std::string tlsVersion = "TLSv1.2"; + EXPECT_CALL(*connection_info, tlsVersion()).WillRepeatedly(ReturnRef(tlsVersion)); EXPECT_CALL(stream_info, downstreamSslConnection()).WillRepeatedly(Return(connection_info)); EXPECT_EQ("TLSv1.2", upstream_format.format(header, header, header, stream_info)); } { StreamInfoFormatter upstream_format("DOWNSTREAM_TLS_VERSION"); auto connection_info = std::make_shared(); - EXPECT_CALL(*connection_info, tlsVersion()).WillRepeatedly(Return("")); + EXPECT_CALL(*connection_info, tlsVersion()).WillRepeatedly(ReturnRef(EMPTY_STRING)); EXPECT_CALL(stream_info, downstreamSslConnection()).WillRepeatedly(Return(connection_info)); EXPECT_EQ("-", upstream_format.format(header, header, header, stream_info)); } @@ -399,15 +405,17 @@ TEST(AccessLogFormatterTest, streamInfoFormatter) { { StreamInfoFormatter upstream_format("DOWNSTREAM_PEER_SERIAL"); auto connection_info = std::make_shared(); + const std::string serial_number = "b8b5ecc898f2124a"; EXPECT_CALL(*connection_info, serialNumberPeerCertificate()) - .WillRepeatedly(Return("b8b5ecc898f2124a")); + .WillRepeatedly(ReturnRef(serial_number)); EXPECT_CALL(stream_info, downstreamSslConnection()).WillRepeatedly(Return(connection_info)); EXPECT_EQ("b8b5ecc898f2124a", upstream_format.format(header, header, header, stream_info)); } { StreamInfoFormatter upstream_format("DOWNSTREAM_PEER_SERIAL"); auto connection_info = std::make_shared(); - EXPECT_CALL(*connection_info, serialNumberPeerCertificate()).WillRepeatedly(Return("")); + EXPECT_CALL(*connection_info, serialNumberPeerCertificate()) + .WillRepeatedly(ReturnRef(EMPTY_STRING)); EXPECT_CALL(stream_info, downstreamSslConnection()).WillRepeatedly(Return(connection_info)); EXPECT_EQ("-", upstream_format.format(header, header, header, stream_info)); } @@ -419,9 +427,9 @@ TEST(AccessLogFormatterTest, streamInfoFormatter) { { StreamInfoFormatter upstream_format("DOWNSTREAM_PEER_ISSUER"); auto connection_info = std::make_shared(); - EXPECT_CALL(*connection_info, issuerPeerCertificate()) - .WillRepeatedly( - Return("CN=Test CA,OU=Lyft Engineering,O=Lyft,L=San Francisco,ST=California,C=US")); + const std::string issuer_peer = + "CN=Test CA,OU=Lyft Engineering,O=Lyft,L=San Francisco,ST=California,C=US"; + EXPECT_CALL(*connection_info, issuerPeerCertificate()).WillRepeatedly(ReturnRef(issuer_peer)); EXPECT_CALL(stream_info, downstreamSslConnection()).WillRepeatedly(Return(connection_info)); EXPECT_EQ("CN=Test CA,OU=Lyft Engineering,O=Lyft,L=San Francisco,ST=California,C=US", upstream_format.format(header, header, header, stream_info)); @@ -429,7 +437,7 @@ TEST(AccessLogFormatterTest, streamInfoFormatter) { { StreamInfoFormatter upstream_format("DOWNSTREAM_PEER_ISSUER"); auto connection_info = std::make_shared(); - EXPECT_CALL(*connection_info, issuerPeerCertificate()).WillRepeatedly(Return("")); + EXPECT_CALL(*connection_info, issuerPeerCertificate()).WillRepeatedly(ReturnRef(EMPTY_STRING)); EXPECT_CALL(stream_info, downstreamSslConnection()).WillRepeatedly(Return(connection_info)); EXPECT_EQ("-", upstream_format.format(header, header, header, stream_info)); } @@ -441,9 +449,9 @@ TEST(AccessLogFormatterTest, streamInfoFormatter) { { StreamInfoFormatter upstream_format("DOWNSTREAM_PEER_SUBJECT"); auto connection_info = std::make_shared(); - EXPECT_CALL(*connection_info, subjectPeerCertificate()) - .WillRepeatedly( - Return("CN=Test Server,OU=Lyft Engineering,O=Lyft,L=San Francisco,ST=California,C=US")); + const std::string subject_peer = + "CN=Test Server,OU=Lyft Engineering,O=Lyft,L=San Francisco,ST=California,C=US"; + EXPECT_CALL(*connection_info, subjectPeerCertificate()).WillRepeatedly(ReturnRef(subject_peer)); EXPECT_CALL(stream_info, downstreamSslConnection()).WillRepeatedly(Return(connection_info)); EXPECT_EQ("CN=Test Server,OU=Lyft Engineering,O=Lyft,L=San Francisco,ST=California,C=US", upstream_format.format(header, header, header, stream_info)); @@ -451,7 +459,7 @@ TEST(AccessLogFormatterTest, streamInfoFormatter) { { StreamInfoFormatter upstream_format("DOWNSTREAM_PEER_SUBJECT"); auto connection_info = std::make_shared(); - EXPECT_CALL(*connection_info, subjectPeerCertificate()).WillRepeatedly(Return("")); + EXPECT_CALL(*connection_info, subjectPeerCertificate()).WillRepeatedly(ReturnRef(EMPTY_STRING)); EXPECT_CALL(stream_info, downstreamSslConnection()).WillRepeatedly(Return(connection_info)); EXPECT_EQ("-", upstream_format.format(header, header, header, stream_info)); } diff --git a/test/common/http/codec_client_test.cc b/test/common/http/codec_client_test.cc index 8c7d63a7f6bb..1de3c04cb424 100644 --- a/test/common/http/codec_client_test.cc +++ b/test/common/http/codec_client_test.cc @@ -32,6 +32,7 @@ using testing::NiceMock; using testing::Pointee; using testing::Ref; using testing::Return; +using testing::ReturnRef; using testing::Throw; namespace Envoy { @@ -260,9 +261,9 @@ TEST_F(CodecClientTest, WatermarkPassthrough) { } TEST_F(CodecClientTest, SSLConnectionInfo) { - const auto session_id = "D62A523A65695219D46FE1FFE285A4C371425ACE421B110B5B8D11D3EB4D5F0B"; + std::string session_id = "D62A523A65695219D46FE1FFE285A4C371425ACE421B110B5B8D11D3EB4D5F0B"; auto connection_info = std::make_shared>(); - ON_CALL(*connection_info, sessionId()).WillByDefault(Return(session_id)); + ON_CALL(*connection_info, sessionId()).WillByDefault(ReturnRef(session_id)); EXPECT_CALL(*connection_, ssl()).WillRepeatedly(Return(connection_info)); connection_cb_->onEvent(Network::ConnectionEvent::Connected); EXPECT_NE(nullptr, stream_info_.downstreamSslConnection()); diff --git a/test/common/http/conn_manager_utility_test.cc b/test/common/http/conn_manager_utility_test.cc index 1eb5ab52e5fc..2e591a77cf92 100644 --- a/test/common/http/conn_manager_utility_test.cc +++ b/test/common/http/conn_manager_utility_test.cc @@ -932,8 +932,8 @@ TEST_F(ConnectionManagerUtilityTest, MtlsSanitizeSetClientCert) { EXPECT_CALL(*ssl, uriSanLocalCertificate()).WillOnce(Return(local_uri_sans)); std::string expected_sha("abcdefg"); EXPECT_CALL(*ssl, sha256PeerCertificateDigest()).WillOnce(ReturnRef(expected_sha)); - EXPECT_CALL(*ssl, subjectPeerCertificate()) - .WillOnce(Return("/C=US/ST=CA/L=San Francisco/OU=Lyft/CN=test.lyft.com")); + std::string peer_subject("/C=US/ST=CA/L=San Francisco/OU=Lyft/CN=test.lyft.com"); + EXPECT_CALL(*ssl, subjectPeerCertificate()).WillOnce(ReturnRef(peer_subject)); const std::vector peer_uri_sans{"test://foo.com/fe"}; EXPECT_CALL(*ssl, uriSanPeerCertificate()).WillRepeatedly(Return(peer_uri_sans)); std::string expected_pem("abcde="); @@ -973,8 +973,8 @@ TEST_F(ConnectionManagerUtilityTest, MtlsSanitizeSetClientCertPeerSanEmpty) { EXPECT_CALL(*ssl, uriSanLocalCertificate()).WillOnce(Return(local_uri_sans)); std::string expected_sha("abcdefg"); EXPECT_CALL(*ssl, sha256PeerCertificateDigest()).WillOnce(ReturnRef(expected_sha)); - EXPECT_CALL(*ssl, subjectPeerCertificate()) - .WillOnce(Return("/C=US/ST=CA/L=San Francisco/OU=Lyft/CN=test.lyft.com")); + std::string peer_subject = "/C=US/ST=CA/L=San Francisco/OU=Lyft/CN=test.lyft.com"; + EXPECT_CALL(*ssl, subjectPeerCertificate()).WillOnce(ReturnRef(peer_subject)); EXPECT_CALL(*ssl, uriSanPeerCertificate()).WillRepeatedly(Return(std::vector())); ON_CALL(connection_, ssl()).WillByDefault(Return(ssl)); ON_CALL(config_, forwardClientCert()) diff --git a/test/common/router/header_formatter_test.cc b/test/common/router/header_formatter_test.cc index 3cbe129caea2..34584a4420ae 100644 --- a/test/common/router/header_formatter_test.cc +++ b/test/common/router/header_formatter_test.cc @@ -156,7 +156,8 @@ TEST_F(StreamInfoHeaderFormatterTest, TestFormatWithDownstreamLocalUriSanNoTls) TEST_F(StreamInfoHeaderFormatterTest, TestFormatWithDownstreamLocalSubject) { NiceMock stream_info; auto connection_info = std::make_shared>(); - ON_CALL(*connection_info, subjectLocalCertificate()).WillByDefault(Return("subject")); + std::string subject = "subject"; + ON_CALL(*connection_info, subjectLocalCertificate()).WillByDefault(ReturnRef(subject)); EXPECT_CALL(stream_info, downstreamSslConnection()).WillRepeatedly(Return(connection_info)); testFormatting(stream_info, "DOWNSTREAM_LOCAL_SUBJECT", "subject"); } @@ -164,7 +165,8 @@ TEST_F(StreamInfoHeaderFormatterTest, TestFormatWithDownstreamLocalSubject) { TEST_F(StreamInfoHeaderFormatterTest, TestFormatWithDownstreamLocalSubjectEmpty) { NiceMock stream_info; auto connection_info = std::make_shared>(); - ON_CALL(*connection_info, subjectLocalCertificate()).WillByDefault(Return("")); + std::string subject; + ON_CALL(*connection_info, subjectLocalCertificate()).WillByDefault(ReturnRef(subject)); EXPECT_CALL(stream_info, downstreamSslConnection()).WillRepeatedly(Return(connection_info)); testFormatting(stream_info, "DOWNSTREAM_LOCAL_SUBJECT", EMPTY_STRING); } @@ -178,7 +180,8 @@ TEST_F(StreamInfoHeaderFormatterTest, TestFormatWithDownstreamLocalSubjectNoTls) TEST_F(StreamInfoHeaderFormatterTest, TestFormatWithDownstreamTlsSessionId) { NiceMock stream_info; auto connection_info = std::make_shared>(); - ON_CALL(*connection_info, sessionId()).WillByDefault(Return("deadbeef")); + std::string session_id = "deadbeef"; + ON_CALL(*connection_info, sessionId()).WillByDefault(ReturnRef(session_id)); EXPECT_CALL(stream_info, downstreamSslConnection()).WillRepeatedly(Return(connection_info)); testFormatting(stream_info, "DOWNSTREAM_TLS_SESSION_ID", "deadbeef"); } @@ -186,7 +189,8 @@ TEST_F(StreamInfoHeaderFormatterTest, TestFormatWithDownstreamTlsSessionId) { TEST_F(StreamInfoHeaderFormatterTest, TestFormatWithDownstreamTlsSessionIdEmpty) { NiceMock stream_info; auto connection_info = std::make_shared>(); - ON_CALL(*connection_info, sessionId()).WillByDefault(Return("")); + std::string session_id; + ON_CALL(*connection_info, sessionId()).WillByDefault(ReturnRef(session_id)); EXPECT_CALL(stream_info, downstreamSslConnection()).WillRepeatedly(Return(connection_info)); testFormatting(stream_info, "DOWNSTREAM_TLS_SESSION_ID", EMPTY_STRING); } @@ -223,7 +227,8 @@ TEST_F(StreamInfoHeaderFormatterTest, TestFormatWithDownstreamTlsCipherNoTls) { TEST_F(StreamInfoHeaderFormatterTest, TestFormatWithDownstreamTlsVersion) { NiceMock stream_info; auto connection_info = std::make_shared>(); - ON_CALL(*connection_info, tlsVersion()).WillByDefault(Return("TLSv1.2")); + std::string tls_version = "TLSv1.2"; + ON_CALL(*connection_info, tlsVersion()).WillByDefault(ReturnRef(tls_version)); EXPECT_CALL(stream_info, downstreamSslConnection()).WillRepeatedly(Return(connection_info)); testFormatting(stream_info, "DOWNSTREAM_TLS_VERSION", "TLSv1.2"); } @@ -231,7 +236,7 @@ TEST_F(StreamInfoHeaderFormatterTest, TestFormatWithDownstreamTlsVersion) { TEST_F(StreamInfoHeaderFormatterTest, TestFormatWithDownstreamTlsVersionEmpty) { NiceMock stream_info; auto connection_info = std::make_shared>(); - ON_CALL(*connection_info, tlsVersion()).WillByDefault(Return("")); + ON_CALL(*connection_info, tlsVersion()).WillByDefault(ReturnRef(EMPTY_STRING)); EXPECT_CALL(stream_info, downstreamSslConnection()).WillRepeatedly(Return(connection_info)); testFormatting(stream_info, "DOWNSTREAM_TLS_VERSION", EMPTY_STRING); } @@ -270,8 +275,8 @@ TEST_F(StreamInfoHeaderFormatterTest, TestFormatWithDownstreamPeerFingerprintNoT TEST_F(StreamInfoHeaderFormatterTest, TestFormatWithDownstreamPeerSerial) { NiceMock stream_info; auto connection_info = std::make_shared>(); - ON_CALL(*connection_info, serialNumberPeerCertificate()) - .WillByDefault(Return("b8b5ecc898f2124a")); + const std::string serial_number = "b8b5ecc898f2124a"; + ON_CALL(*connection_info, serialNumberPeerCertificate()).WillByDefault(ReturnRef(serial_number)); EXPECT_CALL(stream_info, downstreamSslConnection()).WillRepeatedly(Return(connection_info)); testFormatting(stream_info, "DOWNSTREAM_PEER_SERIAL", "b8b5ecc898f2124a"); } @@ -279,7 +284,8 @@ TEST_F(StreamInfoHeaderFormatterTest, TestFormatWithDownstreamPeerSerial) { TEST_F(StreamInfoHeaderFormatterTest, TestFormatWithDownstreamPeerSerialEmpty) { NiceMock stream_info; auto connection_info = std::make_shared>(); - ON_CALL(*connection_info, serialNumberPeerCertificate()).WillByDefault(Return("")); + const std::string serial_number; + ON_CALL(*connection_info, serialNumberPeerCertificate()).WillByDefault(ReturnRef(serial_number)); EXPECT_CALL(stream_info, downstreamSslConnection()).WillRepeatedly(Return(connection_info)); testFormatting(stream_info, "DOWNSTREAM_PEER_SERIAL", EMPTY_STRING); } @@ -293,9 +299,9 @@ TEST_F(StreamInfoHeaderFormatterTest, TestFormatWithDownstreamPeerSerialNoTls) { TEST_F(StreamInfoHeaderFormatterTest, TestFormatWithDownstreamPeerIssuer) { NiceMock stream_info; auto connection_info = std::make_shared>(); - ON_CALL(*connection_info, issuerPeerCertificate()) - .WillByDefault( - Return("CN=Test CA,OU=Lyft Engineering,O=Lyft,L=San Francisco,ST=California,C=US")); + const std::string issuer_peer = + "CN=Test CA,OU=Lyft Engineering,O=Lyft,L=San Francisco,ST=California,C=US"; + ON_CALL(*connection_info, issuerPeerCertificate()).WillByDefault(ReturnRef(issuer_peer)); EXPECT_CALL(stream_info, downstreamSslConnection()).WillRepeatedly(Return(connection_info)); testFormatting(stream_info, "DOWNSTREAM_PEER_ISSUER", "CN=Test CA,OU=Lyft Engineering,O=Lyft,L=San Francisco,ST=California,C=US"); @@ -304,7 +310,8 @@ TEST_F(StreamInfoHeaderFormatterTest, TestFormatWithDownstreamPeerIssuer) { TEST_F(StreamInfoHeaderFormatterTest, TestFormatWithDownstreamPeerIssuerEmpty) { NiceMock stream_info; auto connection_info = std::make_shared>(); - ON_CALL(*connection_info, issuerPeerCertificate()).WillByDefault(Return("")); + const std::string issuer_peer; + ON_CALL(*connection_info, issuerPeerCertificate()).WillByDefault(ReturnRef(issuer_peer)); EXPECT_CALL(stream_info, downstreamSslConnection()).WillRepeatedly(Return(connection_info)); testFormatting(stream_info, "DOWNSTREAM_PEER_ISSUER", EMPTY_STRING); } @@ -318,9 +325,9 @@ TEST_F(StreamInfoHeaderFormatterTest, TestFormatWithDownstreamPeerIssuerNoTls) { TEST_F(StreamInfoHeaderFormatterTest, TestFormatWithDownstreamPeerSubject) { NiceMock stream_info; auto connection_info = std::make_shared>(); - ON_CALL(*connection_info, subjectPeerCertificate()) - .WillByDefault( - Return("CN=Test CA,OU=Lyft Engineering,O=Lyft,L=San Francisco,ST=California,C=US")); + const std::string subject_peer = + "CN=Test CA,OU=Lyft Engineering,O=Lyft,L=San Francisco,ST=California,C=US"; + ON_CALL(*connection_info, subjectPeerCertificate()).WillByDefault(ReturnRef(subject_peer)); EXPECT_CALL(stream_info, downstreamSslConnection()).WillRepeatedly(Return(connection_info)); testFormatting(stream_info, "DOWNSTREAM_PEER_SUBJECT", "CN=Test CA,OU=Lyft Engineering,O=Lyft,L=San Francisco,ST=California,C=US"); @@ -329,7 +336,8 @@ TEST_F(StreamInfoHeaderFormatterTest, TestFormatWithDownstreamPeerSubject) { TEST_F(StreamInfoHeaderFormatterTest, TestFormatWithDownstreamPeerSubjectEmpty) { NiceMock stream_info; auto connection_info = std::make_shared>(); - ON_CALL(*connection_info, subjectPeerCertificate()).WillByDefault(Return("")); + const std::string subject_peer; + ON_CALL(*connection_info, subjectPeerCertificate()).WillByDefault(ReturnRef(subject_peer)); EXPECT_CALL(stream_info, downstreamSslConnection()).WillRepeatedly(Return(connection_info)); testFormatting(stream_info, "DOWNSTREAM_PEER_SUBJECT", EMPTY_STRING); } diff --git a/test/common/router/router_test.cc b/test/common/router/router_test.cc index 2e8cfe69871f..3292f4ce9e87 100644 --- a/test/common/router/router_test.cc +++ b/test/common/router/router_test.cc @@ -3419,9 +3419,9 @@ TEST_F(RouterTest, UpstreamSSLConnection) { NiceMock encoder; Http::StreamDecoder* response_decoder = nullptr; - const auto session_id = "D62A523A65695219D46FE1FFE285A4C371425ACE421B110B5B8D11D3EB4D5F0B"; + std::string session_id = "D62A523A65695219D46FE1FFE285A4C371425ACE421B110B5B8D11D3EB4D5F0B"; auto connection_info = std::make_shared>(); - ON_CALL(*connection_info, sessionId()).WillByDefault(Return(session_id)); + ON_CALL(*connection_info, sessionId()).WillByDefault(ReturnRef(session_id)); upstream_stream_info_.setDownstreamSslConnection(connection_info); expectResponseTimerCreate(); diff --git a/test/common/tcp_proxy/tcp_proxy_test.cc b/test/common/tcp_proxy/tcp_proxy_test.cc index a7a5b415d867..07c505c2e901 100644 --- a/test/common/tcp_proxy/tcp_proxy_test.cc +++ b/test/common/tcp_proxy/tcp_proxy_test.cc @@ -967,7 +967,7 @@ TEST_F(TcpProxyTest, AccessLogTlsSessionId) { const std::string tlsSessionId{ "D62A523A65695219D46FE1FFE285A4C371425ACE421B110B5B8D11D3EB4D5F0B"}; auto mockConnectionInfo = std::make_shared(); - EXPECT_CALL(*mockConnectionInfo, sessionId()).WillOnce(Return(tlsSessionId)); + EXPECT_CALL(*mockConnectionInfo, sessionId()).WillOnce(ReturnRef(tlsSessionId)); EXPECT_CALL(filter_callbacks_.connection_, ssl()).WillRepeatedly(Return(mockConnectionInfo)); setup(1, accessLogConfig("%DOWNSTREAM_TLS_SESSION_ID%")); @@ -1016,7 +1016,7 @@ TEST_F(TcpProxyTest, AccessLogUpstreamSSLConnection) { NiceMock stream_info; const std::string session_id = "D62A523A65695219D46FE1FFE285A4C371425ACE421B110B5B8D11D3EB4D5F0B"; auto ssl_info = std::make_shared(); - EXPECT_CALL(*ssl_info, sessionId()).WillRepeatedly(Return(session_id)); + EXPECT_CALL(*ssl_info, sessionId()).WillRepeatedly(ReturnRef(session_id)); stream_info.setDownstreamSslConnection(ssl_info); EXPECT_CALL(*upstream_connections_.at(0), streamInfo()).WillRepeatedly(ReturnRef(stream_info)); diff --git a/test/extensions/access_loggers/grpc/http_grpc_access_log_impl_test.cc b/test/extensions/access_loggers/grpc/http_grpc_access_log_impl_test.cc index 1c5575186ab3..cee4378e4a21 100644 --- a/test/extensions/access_loggers/grpc/http_grpc_access_log_impl_test.cc +++ b/test/extensions/access_loggers/grpc/http_grpc_access_log_impl_test.cc @@ -19,6 +19,7 @@ using testing::InSequence; using testing::Invoke; using testing::NiceMock; using testing::Return; +using testing::ReturnRef; namespace Envoy { namespace Extensions { @@ -310,11 +311,15 @@ response: {} ON_CALL(*connection_info, uriSanPeerCertificate()).WillByDefault(Return(peerSans)); const std::vector localSans{"localSan1", "localSan2"}; ON_CALL(*connection_info, uriSanLocalCertificate()).WillByDefault(Return(localSans)); - ON_CALL(*connection_info, subjectPeerCertificate()).WillByDefault(Return("peerSubject")); - ON_CALL(*connection_info, subjectLocalCertificate()).WillByDefault(Return("localSubject")); - ON_CALL(*connection_info, sessionId()) - .WillByDefault(Return("D62A523A65695219D46FE1FFE285A4C371425ACE421B110B5B8D11D3EB4D5F0B")); - ON_CALL(*connection_info, tlsVersion()).WillByDefault(Return("TLSv1.3")); + const std::string peerSubject = "peerSubject"; + ON_CALL(*connection_info, subjectPeerCertificate()).WillByDefault(ReturnRef(peerSubject)); + const std::string localSubject = "localSubject"; + ON_CALL(*connection_info, subjectLocalCertificate()).WillByDefault(ReturnRef(localSubject)); + const std::string sessionId = + "D62A523A65695219D46FE1FFE285A4C371425ACE421B110B5B8D11D3EB4D5F0B"; + ON_CALL(*connection_info, sessionId()).WillByDefault(ReturnRef(sessionId)); + const std::string tlsVersion = "TLSv1.3"; + ON_CALL(*connection_info, tlsVersion()).WillByDefault(ReturnRef(tlsVersion)); ON_CALL(*connection_info, ciphersuiteId()).WillByDefault(Return(0x2CC0)); stream_info.setDownstreamSslConnection(connection_info); stream_info.requested_server_name_ = "sni"; @@ -365,7 +370,12 @@ response: {} stream_info.start_time_ = SystemTime(1h); auto connection_info = std::make_shared>(); - ON_CALL(*connection_info, tlsVersion()).WillByDefault(Return("TLSv1.2")); + const std::string empty; + ON_CALL(*connection_info, subjectPeerCertificate()).WillByDefault(ReturnRef(empty)); + ON_CALL(*connection_info, subjectLocalCertificate()).WillByDefault(ReturnRef(empty)); + ON_CALL(*connection_info, sessionId()).WillByDefault(ReturnRef(empty)); + const std::string tlsVersion = "TLSv1.2"; + ON_CALL(*connection_info, tlsVersion()).WillByDefault(ReturnRef(tlsVersion)); ON_CALL(*connection_info, ciphersuiteId()).WillByDefault(Return(0x2F)); stream_info.setDownstreamSslConnection(connection_info); stream_info.requested_server_name_ = "sni"; @@ -406,7 +416,12 @@ response: {} stream_info.start_time_ = SystemTime(1h); auto connection_info = std::make_shared>(); - ON_CALL(*connection_info, tlsVersion()).WillByDefault(Return("TLSv1.1")); + const std::string empty; + ON_CALL(*connection_info, subjectPeerCertificate()).WillByDefault(ReturnRef(empty)); + ON_CALL(*connection_info, subjectLocalCertificate()).WillByDefault(ReturnRef(empty)); + ON_CALL(*connection_info, sessionId()).WillByDefault(ReturnRef(empty)); + const std::string tlsVersion = "TLSv1.1"; + ON_CALL(*connection_info, tlsVersion()).WillByDefault(ReturnRef(tlsVersion)); ON_CALL(*connection_info, ciphersuiteId()).WillByDefault(Return(0x2F)); stream_info.setDownstreamSslConnection(connection_info); stream_info.requested_server_name_ = "sni"; @@ -447,7 +462,12 @@ response: {} stream_info.start_time_ = SystemTime(1h); auto connection_info = std::make_shared>(); - ON_CALL(*connection_info, tlsVersion()).WillByDefault(Return("TLSv1")); + const std::string empty; + ON_CALL(*connection_info, subjectPeerCertificate()).WillByDefault(ReturnRef(empty)); + ON_CALL(*connection_info, subjectLocalCertificate()).WillByDefault(ReturnRef(empty)); + ON_CALL(*connection_info, sessionId()).WillByDefault(ReturnRef(empty)); + const std::string tlsVersion = "TLSv1"; + ON_CALL(*connection_info, tlsVersion()).WillByDefault(ReturnRef(tlsVersion)); ON_CALL(*connection_info, ciphersuiteId()).WillByDefault(Return(0x2F)); stream_info.setDownstreamSslConnection(connection_info); stream_info.requested_server_name_ = "sni"; @@ -488,7 +508,12 @@ response: {} stream_info.start_time_ = SystemTime(1h); auto connection_info = std::make_shared>(); - ON_CALL(*connection_info, tlsVersion()).WillByDefault(Return("TLSv1.4")); + const std::string empty; + ON_CALL(*connection_info, subjectPeerCertificate()).WillByDefault(ReturnRef(empty)); + ON_CALL(*connection_info, subjectLocalCertificate()).WillByDefault(ReturnRef(empty)); + ON_CALL(*connection_info, sessionId()).WillByDefault(ReturnRef(empty)); + const std::string tlsVersion = "TLSv1.4"; + ON_CALL(*connection_info, tlsVersion()).WillByDefault(ReturnRef(tlsVersion)); ON_CALL(*connection_info, ciphersuiteId()).WillByDefault(Return(0x2F)); stream_info.setDownstreamSslConnection(connection_info); stream_info.requested_server_name_ = "sni"; diff --git a/test/extensions/filters/common/ext_authz/check_request_utils_test.cc b/test/extensions/filters/common/ext_authz/check_request_utils_test.cc index d04b54ad8ebe..de4ef025784e 100644 --- a/test/extensions/filters/common/ext_authz/check_request_utils_test.cc +++ b/test/extensions/filters/common/ext_authz/check_request_utils_test.cc @@ -97,6 +97,9 @@ TEST_F(CheckRequestUtilsTest, BasicTcp) { EXPECT_CALL(connection_, remoteAddress()).WillOnce(ReturnRef(addr_)); EXPECT_CALL(connection_, localAddress()).WillOnce(ReturnRef(addr_)); EXPECT_CALL(Const(connection_), ssl()).Times(2).WillRepeatedly(Return(ssl_)); + EXPECT_CALL(*ssl_, uriSanPeerCertificate()).WillOnce(Return(std::vector{"source"})); + EXPECT_CALL(*ssl_, uriSanLocalCertificate()) + .WillOnce(Return(std::vector{"destination"})); CheckRequestUtils::createTcpCheck(&net_callbacks_, request); } @@ -112,6 +115,9 @@ TEST_F(CheckRequestUtilsTest, BasicHttp) { // A client supplied EnvoyAuthPartialBody header should be ignored. Http::TestHeaderMapImpl request_headers{{Http::Headers::get().EnvoyAuthPartialBody.get(), "1"}}; + EXPECT_CALL(*ssl_, uriSanPeerCertificate()).WillOnce(Return(std::vector{"source"})); + EXPECT_CALL(*ssl_, uriSanLocalCertificate()) + .WillOnce(Return(std::vector{"destination"})); expectBasicHttp(); CheckRequestUtils::createHttpCheck(&callbacks_, request_headers, Protobuf::Map(), @@ -129,6 +135,9 @@ TEST_F(CheckRequestUtilsTest, BasicHttpWithPartialBody) { Http::HeaderMapImpl headers_; envoy::service::auth::v2::CheckRequest request_; + EXPECT_CALL(*ssl_, uriSanPeerCertificate()).WillOnce(Return(std::vector{"source"})); + EXPECT_CALL(*ssl_, uriSanLocalCertificate()) + .WillOnce(Return(std::vector{"destination"})); expectBasicHttp(); CheckRequestUtils::createHttpCheck(&callbacks_, headers_, Protobuf::Map(), @@ -144,6 +153,9 @@ TEST_F(CheckRequestUtilsTest, BasicHttpWithFullBody) { Http::HeaderMapImpl headers_; envoy::service::auth::v2::CheckRequest request_; + EXPECT_CALL(*ssl_, uriSanPeerCertificate()).WillOnce(Return(std::vector{"source"})); + EXPECT_CALL(*ssl_, uriSanLocalCertificate()) + .WillOnce(Return(std::vector{"destination"})); expectBasicHttp(); CheckRequestUtils::createHttpCheck(&callbacks_, headers_, Protobuf::Map(), @@ -213,11 +225,13 @@ TEST_F(CheckRequestUtilsTest, CheckAttrContextSubject) { EXPECT_CALL(*ssl_, uriSanPeerCertificate()).WillOnce(Return(std::vector{})); EXPECT_CALL(*ssl_, dnsSansPeerCertificate()).WillOnce(Return(std::vector{})); - EXPECT_CALL(*ssl_, subjectPeerCertificate()).WillOnce(Return("source")); + std::string subject_peer = "source"; + EXPECT_CALL(*ssl_, subjectPeerCertificate()).WillOnce(ReturnRef(subject_peer)); EXPECT_CALL(*ssl_, uriSanLocalCertificate()).WillOnce(Return(std::vector{})); EXPECT_CALL(*ssl_, dnsSansLocalCertificate()).WillOnce(Return(std::vector{})); - EXPECT_CALL(*ssl_, subjectLocalCertificate()).WillOnce(Return("destination")); + std::string subject_local = "destination"; + EXPECT_CALL(*ssl_, subjectLocalCertificate()).WillOnce(ReturnRef(subject_local)); callHttpCheckAndValidateRequestAttributes(); } diff --git a/test/extensions/filters/common/rbac/matchers_test.cc b/test/extensions/filters/common/rbac/matchers_test.cc index 7139ad1d1847..699b53d8b4e7 100644 --- a/test/extensions/filters/common/rbac/matchers_test.cc +++ b/test/extensions/filters/common/rbac/matchers_test.cc @@ -235,7 +235,8 @@ TEST(AuthenticatedMatcher, subjectPeerCertificate) { const std::vector sans; EXPECT_CALL(*ssl, uriSanPeerCertificate()).WillRepeatedly(Return(sans)); EXPECT_CALL(*ssl, dnsSansPeerCertificate()).WillRepeatedly(Return(sans)); - EXPECT_CALL(*ssl, subjectPeerCertificate()).WillRepeatedly(Return("bar")); + std::string peer_subject = "bar"; + EXPECT_CALL(*ssl, subjectPeerCertificate()).WillRepeatedly(ReturnRef(peer_subject)); EXPECT_CALL(Const(conn), ssl()).WillRepeatedly(Return(ssl)); envoy::config::rbac::v2::Principal_Authenticated auth; diff --git a/test/fuzz/utility.h b/test/fuzz/utility.h index 2fda4efb8de7..adc3912c0e82 100644 --- a/test/fuzz/utility.h +++ b/test/fuzz/utility.h @@ -103,6 +103,9 @@ inline test::fuzz::Headers toHeaders(const Http::HeaderMap& headers) { return fuzz_headers; } +const std::string TestSubjectPeer = + "CN=Test Server,OU=Lyft Engineering,O=Lyft,L=San Francisco,ST=California,C=US"; + inline TestStreamInfo fromStreamInfo(const test::fuzz::StreamInfo& stream_info) { // Set mocks' default string return value to be an empty string. testing::DefaultValue::Set(EMPTY_STRING); @@ -130,8 +133,7 @@ inline TestStreamInfo fromStreamInfo(const test::fuzz::StreamInfo& stream_info) test_stream_info.downstream_remote_address_ = address; auto connection_info = std::make_shared>(); ON_CALL(*connection_info, subjectPeerCertificate()) - .WillByDefault(testing::Return( - "CN=Test Server,OU=Lyft Engineering,O=Lyft,L=San Francisco,ST=California,C=US")); + .WillByDefault(testing::ReturnRef(TestSubjectPeer)); test_stream_info.setDownstreamSslConnection(connection_info); return test_stream_info; } diff --git a/test/mocks/ssl/mocks.h b/test/mocks/ssl/mocks.h index 957cbf05c87c..041888aa99c9 100644 --- a/test/mocks/ssl/mocks.h +++ b/test/mocks/ssl/mocks.h @@ -40,21 +40,21 @@ class MockConnectionInfo : public ConnectionInfo { MOCK_CONST_METHOD0(peerCertificatePresented, bool()); MOCK_CONST_METHOD0(uriSanLocalCertificate, std::vector()); MOCK_CONST_METHOD0(sha256PeerCertificateDigest, const std::string&()); - MOCK_CONST_METHOD0(serialNumberPeerCertificate, std::string()); - MOCK_CONST_METHOD0(issuerPeerCertificate, std::string()); - MOCK_CONST_METHOD0(subjectPeerCertificate, std::string()); + MOCK_CONST_METHOD0(serialNumberPeerCertificate, const std::string&()); + MOCK_CONST_METHOD0(issuerPeerCertificate, const std::string&()); + MOCK_CONST_METHOD0(subjectPeerCertificate, const std::string&()); MOCK_CONST_METHOD0(uriSanPeerCertificate, std::vector()); - MOCK_CONST_METHOD0(subjectLocalCertificate, std::string()); + MOCK_CONST_METHOD0(subjectLocalCertificate, const std::string&()); MOCK_CONST_METHOD0(urlEncodedPemEncodedPeerCertificate, const std::string&()); MOCK_CONST_METHOD0(urlEncodedPemEncodedPeerCertificateChain, const std::string&()); MOCK_CONST_METHOD0(dnsSansPeerCertificate, std::vector()); MOCK_CONST_METHOD0(dnsSansLocalCertificate, std::vector()); MOCK_CONST_METHOD0(validFromPeerCertificate, absl::optional()); MOCK_CONST_METHOD0(expirationPeerCertificate, absl::optional()); - MOCK_CONST_METHOD0(sessionId, std::string()); + MOCK_CONST_METHOD0(sessionId, const std::string&()); MOCK_CONST_METHOD0(ciphersuiteId, uint16_t()); MOCK_CONST_METHOD0(ciphersuiteString, std::string()); - MOCK_CONST_METHOD0(tlsVersion, std::string()); + MOCK_CONST_METHOD0(tlsVersion, const std::string&()); }; class MockClientContext : public ClientContext {