diff --git a/api/envoy/service/auth/v2/attribute_context.proto b/api/envoy/service/auth/v2/attribute_context.proto index 822e361dd81b..fe5bee033909 100644 --- a/api/envoy/service/auth/v2/attribute_context.proto +++ b/api/envoy/service/auth/v2/attribute_context.proto @@ -50,8 +50,8 @@ message AttributeContext { // The authenticated identity of this peer. // For example, the identity associated with the workload such as a service account. // If an X.509 certificate is used to assert the identity this field should be sourced from - // `Subject` or `Subject Alternative Names`. The primary identity should be the principal. - // The principal format is issuer specific. + // `URI Subject Alternative Names`, `DNS Subject Alternate Names` or `Subject` in that order. + // The primary identity should be the principal. The principal format is issuer specific. // // Example: // * SPIFFE format is `spiffe://trust-domain/path` diff --git a/source/extensions/filters/common/ext_authz/check_request_utils.cc b/source/extensions/filters/common/ext_authz/check_request_utils.cc index b98752cc5a14..707053f46bf0 100644 --- a/source/extensions/filters/common/ext_authz/check_request_utils.cc +++ b/source/extensions/filters/common/ext_authz/check_request_utils.cc @@ -37,24 +37,33 @@ void CheckRequestUtils::setAttrContextPeer(envoy::service::auth::v2::AttributeCo Envoy::Network::Utility::addressToProtobufAddress(*connection.remoteAddress(), *addr); } - // Set the principal - // Preferably the SAN from the peer's cert or - // Subject from the peer's cert. + // Set the principal. Preferably the URI SAN, DNS SAN or Subject in that order from the peer's + // cert. Ssl::ConnectionInfo* ssl = const_cast(connection.ssl()); if (ssl != nullptr) { if (local) { - const auto uriSans = ssl->uriSanLocalCertificate(); - if (uriSans.empty()) { - peer.set_principal(ssl->subjectLocalCertificate()); + const auto uri_sans = ssl->uriSanLocalCertificate(); + if (uri_sans.empty()) { + const auto dns_sans = ssl->dnsSansLocalCertificate(); + if (dns_sans.empty()) { + peer.set_principal(ssl->subjectLocalCertificate()); + } else { + peer.set_principal(dns_sans[0]); + } } else { - peer.set_principal(uriSans[0]); + peer.set_principal(uri_sans[0]); } } else { - const auto uriSans = ssl->uriSanPeerCertificate(); - if (uriSans.empty()) { - peer.set_principal(ssl->subjectPeerCertificate()); + const auto uri_sans = ssl->uriSanPeerCertificate(); + if (uri_sans.empty()) { + const auto dns_sans = ssl->dnsSansPeerCertificate(); + if (dns_sans.empty()) { + peer.set_principal(ssl->subjectPeerCertificate()); + } else { + peer.set_principal(dns_sans[0]); + } } else { - peer.set_principal(uriSans[0]); + peer.set_principal(uri_sans[0]); } } } 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 fa925bf6b8bf..bc5df7ec323d 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 @@ -30,7 +30,7 @@ class CheckRequestUtilsTest : public testing::Test { buffer_ = CheckRequestUtilsTest::newTestBuffer(8192); }; - void ExpectBasicHttp() { + void expectBasicHttp() { EXPECT_CALL(callbacks_, connection()).Times(2).WillRepeatedly(Return(&connection_)); EXPECT_CALL(connection_, remoteAddress()).WillOnce(ReturnRef(addr_)); EXPECT_CALL(connection_, localAddress()).WillOnce(ReturnRef(addr_)); @@ -41,6 +41,33 @@ class CheckRequestUtilsTest : public testing::Test { EXPECT_CALL(req_info_, protocol()).Times(2).WillRepeatedly(ReturnPointee(&protocol_)); } + void callHttpCheckAndValidateRequestAttributes() { + Http::TestHeaderMapImpl request_headers{{"x-envoy-downstream-service-cluster", "foo"}, + {":path", "/bar"}}; + envoy::service::auth::v2::CheckRequest request; + Protobuf::Map context_extensions; + context_extensions["key"] = "value"; + + envoy::api::v2::core::Metadata metadata_context; + auto metadata_val = MessageUtil::keyValueStruct("foo", "bar"); + (*metadata_context.mutable_filter_metadata())["meta.key"] = metadata_val; + + CheckRequestUtils::createHttpCheck(&callbacks_, request_headers, std::move(context_extensions), + std::move(metadata_context), request, false); + + EXPECT_EQ("source", request.attributes().source().principal()); + EXPECT_EQ("destination", request.attributes().destination().principal()); + EXPECT_EQ("foo", request.attributes().source().service()); + EXPECT_EQ("value", request.attributes().context_extensions().at("key")); + EXPECT_EQ("bar", request.attributes() + .metadata_context() + .filter_metadata() + .at("meta.key") + .fields() + .at("foo") + .string_value()); + } + static Buffer::InstancePtr newTestBuffer(uint64_t size) { auto buffer = std::make_unique(); while (buffer->length() < size) { @@ -84,7 +111,7 @@ TEST_F(CheckRequestUtilsTest, BasicHttp) { // A client supplied EnvoyAuthPartialBody header should be ignored. Http::TestHeaderMapImpl request_headers{{Http::Headers::get().EnvoyAuthPartialBody.get(), "1"}}; - ExpectBasicHttp(); + expectBasicHttp(); CheckRequestUtils::createHttpCheck(&callbacks_, request_headers, Protobuf::Map(), envoy::api::v2::core::Metadata(), request_, size); @@ -101,7 +128,7 @@ TEST_F(CheckRequestUtilsTest, BasicHttpWithPartialBody) { Http::HeaderMapImpl headers_; envoy::service::auth::v2::CheckRequest request_; - ExpectBasicHttp(); + expectBasicHttp(); CheckRequestUtils::createHttpCheck(&callbacks_, headers_, Protobuf::Map(), envoy::api::v2::core::Metadata(), request_, size); @@ -116,7 +143,7 @@ TEST_F(CheckRequestUtilsTest, BasicHttpWithFullBody) { Http::HeaderMapImpl headers_; envoy::service::auth::v2::CheckRequest request_; - ExpectBasicHttp(); + expectBasicHttp(); CheckRequestUtils::createHttpCheck(&callbacks_, headers_, Protobuf::Map(), envoy::api::v2::core::Metadata(), request_, buffer_->length()); @@ -127,45 +154,50 @@ TEST_F(CheckRequestUtilsTest, BasicHttpWithFullBody) { Http::Headers::get().EnvoyAuthPartialBody.get())); } -// Verify that createHttpCheck extract the proper attributes from the http request into CheckRequest -// proto object. -TEST_F(CheckRequestUtilsTest, CheckAttrContextPeer) { - Http::TestHeaderMapImpl request_headers{{"x-envoy-downstream-service-cluster", "foo"}, - {":path", "/bar"}}; - envoy::service::auth::v2::CheckRequest request; - EXPECT_CALL(callbacks_, connection()).WillRepeatedly(Return(&connection_)); - EXPECT_CALL(connection_, remoteAddress()).WillRepeatedly(ReturnRef(addr_)); - EXPECT_CALL(connection_, localAddress()).WillRepeatedly(ReturnRef(addr_)); - EXPECT_CALL(Const(connection_), ssl()).WillRepeatedly(Return(&ssl_)); - EXPECT_CALL(callbacks_, streamId()).WillRepeatedly(Return(0)); - EXPECT_CALL(callbacks_, streamInfo()).WillRepeatedly(ReturnRef(req_info_)); - EXPECT_CALL(callbacks_, decodingBuffer()).Times(1); - EXPECT_CALL(req_info_, protocol()).WillRepeatedly(ReturnPointee(&protocol_)); +// Verify that createHttpCheck extract the attributes from the HTTP request into CheckRequest +// proto object and URI SAN is used as principal if present. +TEST_F(CheckRequestUtilsTest, CheckAttrContextPeerUriSans) { + expectBasicHttp(); + EXPECT_CALL(ssl_, uriSanPeerCertificate()).WillOnce(Return(std::vector{"source"})); EXPECT_CALL(ssl_, uriSanLocalCertificate()) .WillOnce(Return(std::vector{"destination"})); + callHttpCheckAndValidateRequestAttributes(); +} + +// Verify that createHttpCheck extract the attributes from the HTTP request into CheckRequest +// proto object and DNS SAN is used as principal if URI SAN is absent. +TEST_F(CheckRequestUtilsTest, CheckAttrContextPeerDnsSans) { + expectBasicHttp(); + + EXPECT_CALL(ssl_, uriSanPeerCertificate()).WillOnce(Return(std::vector{})); + EXPECT_CALL(ssl_, dnsSansPeerCertificate()).WillOnce(Return(std::vector{"source"})); + + EXPECT_CALL(ssl_, uriSanLocalCertificate()).WillOnce(Return(std::vector{})); + EXPECT_CALL(ssl_, dnsSansLocalCertificate()) + .WillOnce(Return(std::vector{"destination"})); + Protobuf::Map context_extensions; context_extensions["key"] = "value"; - envoy::api::v2::core::Metadata metadata_context; - auto metadata_val = MessageUtil::keyValueStruct("foo", "bar"); - (*metadata_context.mutable_filter_metadata())["meta.key"] = metadata_val; - - CheckRequestUtils::createHttpCheck(&callbacks_, request_headers, std::move(context_extensions), - std::move(metadata_context), request, false); - - EXPECT_EQ("source", request.attributes().source().principal()); - EXPECT_EQ("destination", request.attributes().destination().principal()); - EXPECT_EQ("foo", request.attributes().source().service()); - EXPECT_EQ("value", request.attributes().context_extensions().at("key")); - EXPECT_EQ("bar", request.attributes() - .metadata_context() - .filter_metadata() - .at("meta.key") - .fields() - .at("foo") - .string_value()); + callHttpCheckAndValidateRequestAttributes(); +} + +// Verify that createHttpCheck extract the attributes from the HTTP request into CheckRequest +// proto object and Subject is used as principal if both URI SAN and DNS SAN are absent. +TEST_F(CheckRequestUtilsTest, CheckAttrContextSubject) { + expectBasicHttp(); + + EXPECT_CALL(ssl_, uriSanPeerCertificate()).WillOnce(Return(std::vector{})); + EXPECT_CALL(ssl_, dnsSansPeerCertificate()).WillOnce(Return(std::vector{})); + EXPECT_CALL(ssl_, subjectPeerCertificate()).WillOnce(Return("source")); + + EXPECT_CALL(ssl_, uriSanLocalCertificate()).WillOnce(Return(std::vector{})); + EXPECT_CALL(ssl_, dnsSansLocalCertificate()).WillOnce(Return(std::vector{})); + EXPECT_CALL(ssl_, subjectLocalCertificate()).WillOnce(Return("destination")); + + callHttpCheckAndValidateRequestAttributes(); } } // namespace