Skip to content

Commit

Permalink
ext authz: add dns san support for ext authz service (#7948)
Browse files Browse the repository at this point in the history 
Adds support for DNS SAN in ext authz peer validation

Risk Level: Low
Testing: Added
Docs Changes: Added
Release Notes: N/A

Signed-off-by: Rama Chavali <rama.rao@salesforce.com>
  • Loading branch information
@ramaraochavali @htuch
ramaraochavali authored and htuch committed Aug 22, 2019
1 parent ffeffd7 commit d4dc0a5
Show file tree
Hide file tree
Showing 3 changed files with 90 additions and 49 deletions.
4 changes: 2 additions & 2 deletions api/envoy/service/auth/v2/attribute_context.proto
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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`
Expand Down
31 changes: 20 additions & 11 deletions source/extensions/filters/common/ext_authz/check_request_utils.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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<Ssl::ConnectionInfo*>(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]);
}
}
}
Expand Down
104 changes: 68 additions & 36 deletions test/extensions/filters/common/ext_authz/check_request_utils_test.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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_));
Expand All @@ -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<std::string, std::string> 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<Buffer::OwnedImpl>();
while (buffer->length() < size) {
Expand Down Expand Up @@ -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<std::string, std::string>(),
envoy::api::v2::core::Metadata(), request_, size);
Expand All @@ -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<std::string, std::string>(),
envoy::api::v2::core::Metadata(), request_, size);
Expand All @@ -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<std::string, std::string>(),
envoy::api::v2::core::Metadata(), request_, buffer_->length());
Expand All @@ -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<std::string>{"source"}));
EXPECT_CALL(ssl_, uriSanLocalCertificate())
.WillOnce(Return(std::vector<std::string>{"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<std::string>{}));
EXPECT_CALL(ssl_, dnsSansPeerCertificate()).WillOnce(Return(std::vector<std::string>{"source"}));

EXPECT_CALL(ssl_, uriSanLocalCertificate()).WillOnce(Return(std::vector<std::string>{}));
EXPECT_CALL(ssl_, dnsSansLocalCertificate())
.WillOnce(Return(std::vector<std::string>{"destination"}));

Protobuf::Map<std::string, std::string> 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<std::string>{}));
EXPECT_CALL(ssl_, dnsSansPeerCertificate()).WillOnce(Return(std::vector<std::string>{}));
EXPECT_CALL(ssl_, subjectPeerCertificate()).WillOnce(Return("source"));

EXPECT_CALL(ssl_, uriSanLocalCertificate()).WillOnce(Return(std::vector<std::string>{}));
EXPECT_CALL(ssl_, dnsSansLocalCertificate()).WillOnce(Return(std::vector<std::string>{}));
EXPECT_CALL(ssl_, subjectLocalCertificate()).WillOnce(Return("destination"));

callHttpCheckAndValidateRequestAttributes();
}

} // namespace
Expand Down

0 comments on commit d4dc0a5

Please sign in to comment.