diff --git a/include/istio/utils/BUILD b/include/istio/utils/BUILD index f343c5a51e7..92bb7e084bd 100644 --- a/include/istio/utils/BUILD +++ b/include/istio/utils/BUILD @@ -41,4 +41,4 @@ cc_library( "attribute_names.h", ], visibility = ["//visibility:public"], -) +) \ No newline at end of file diff --git a/src/envoy/http/authn/BUILD b/src/envoy/http/authn/BUILD index 6c9efa35324..3f4c3f296cd 100644 --- a/src/envoy/http/authn/BUILD +++ b/src/envoy/http/authn/BUILD @@ -46,6 +46,7 @@ envoy_cc_library( "//src/envoy/http/jwt_auth:jwt_lib", "//src/envoy/utils:utils_lib", "//src/istio/authn:context_proto", + "//src/envoy/utils:filter_names_lib", ], ) @@ -66,6 +67,7 @@ envoy_cc_library( "//src/envoy/utils:utils_lib", "//src/istio/authn:context_proto", "@envoy//source/exe:envoy_common_lib", + "//src/envoy/utils:filter_names_lib", ], ) @@ -85,6 +87,7 @@ envoy_cc_test( deps = [ ":authenticator", ":test_utils", + "@envoy//test/mocks/http:http_mocks", "@envoy//test/test_common:utility_lib", ], ) @@ -97,6 +100,7 @@ envoy_cc_test( ":authenticator", ":test_utils", "@envoy//test/mocks/network:network_mocks", + "//src/envoy/utils:filter_names_lib", "@envoy//test/mocks/ssl:ssl_mocks", "@envoy//test/test_common:utility_lib", ], @@ -158,6 +162,8 @@ envoy_cc_test( repository = "@envoy", deps = [ ":filter_lib", - "@envoy//test/integration:http_integration_lib", + "@envoy//source/common/common:utility_lib", + "@envoy//test/integration:http_protocol_integration_lib", + "//src/envoy/utils:filter_names_lib", ], ) diff --git a/src/envoy/http/authn/authenticator_base.cc b/src/envoy/http/authn/authenticator_base.cc index d53f8cc0119..9262d249e25 100644 --- a/src/envoy/http/authn/authenticator_base.cc +++ b/src/envoy/http/authn/authenticator_base.cc @@ -15,7 +15,9 @@ #include "src/envoy/http/authn/authenticator_base.h" #include "common/common/assert.h" +#include "common/config/metadata.h" #include "src/envoy/http/authn/authn_utils.h" +#include "src/envoy/utils/filter_names.h" #include "src/envoy/utils/utils.h" using istio::authn::Payload; @@ -64,21 +66,11 @@ bool AuthenticatorBase::validateX509(const iaapi::MutualTls& mtls, } bool AuthenticatorBase::validateJwt(const iaapi::Jwt& jwt, Payload* payload) { - Envoy::Http::HeaderMap& header = *filter_context()->headers(); - - auto iter = - filter_context()->filter_config().jwt_output_payload_locations().find( - jwt.issuer()); - if (iter == - filter_context()->filter_config().jwt_output_payload_locations().end()) { - ENVOY_LOG(warn, "No JWT payload header location is found for the issuer {}", - jwt.issuer()); - return false; + std::string jwt_payload; + if (filter_context()->getJwtPayload(jwt.issuer(), &jwt_payload)) { + return AuthnUtils::ProcessJwtPayload(jwt_payload, payload->mutable_jwt()); } - - LowerCaseString header_key(iter->second); - return AuthnUtils::GetJWTPayloadFromHeaders(header, header_key, - payload->mutable_jwt()); + return false; } } // namespace AuthN diff --git a/src/envoy/http/authn/authenticator_base_test.cc b/src/envoy/http/authn/authenticator_base_test.cc index 454e40e19c4..068e5db1edb 100644 --- a/src/envoy/http/authn/authenticator_base_test.cc +++ b/src/envoy/http/authn/authenticator_base_test.cc @@ -16,9 +16,11 @@ #include "src/envoy/http/authn/authenticator_base.h" #include "common/common/base64.h" #include "common/protobuf/protobuf.h" +#include "envoy/api/v2/core/base.pb.h" #include "envoy/config/filter/http/authn/v2alpha1/config.pb.h" #include "gmock/gmock.h" #include "src/envoy/http/authn/test_utils.h" +#include "src/envoy/utils/filter_names.h" #include "test/mocks/network/mocks.h" #include "test/mocks/ssl/mocks.h" @@ -27,6 +29,7 @@ using istio::authn::Payload; using istio::envoy::config::filter::http::authn::v2alpha1::FilterConfig; using testing::NiceMock; using testing::Return; +using testing::StrictMock; namespace iaapi = istio::authentication::v1alpha1; @@ -36,7 +39,6 @@ namespace Istio { namespace AuthN { namespace { -const std::string kSecIstioAuthUserInfoHeaderKey = "sec-istio-auth-userinfo"; const std::string kSecIstioAuthUserinfoHeaderValue = R"( { @@ -60,13 +62,13 @@ class ValidateX509Test : public testing::TestWithParam, public: virtual ~ValidateX509Test() {} - Http::TestHeaderMapImpl request_headers_{}; NiceMock connection_{}; NiceMock ssl_{}; FilterConfig filter_config_{}; - FilterContext filter_context_{&request_headers_, &connection_, - istio::envoy::config::filter::http::authn:: - v2alpha1::FilterConfig::default_instance()}; + FilterContext filter_context_{ + envoy::api::v2::core::Metadata::default_instance(), &connection_, + istio::envoy::config::filter::http::authn::v2alpha1::FilterConfig:: + default_instance()}; MockAuthenticatorBase authenticator_{&filter_context_}; @@ -162,30 +164,19 @@ class ValidateJwtTest : public testing::Test, public: virtual ~ValidateJwtTest() {} - Http::TestHeaderMapImpl request_headers_{}; + // StrictMock request_info_{}; + envoy::api::v2::core::Metadata dynamic_metadata_; NiceMock connection_{}; - NiceMock ssl_{}; + // NiceMock ssl_{}; FilterConfig filter_config_{}; - FilterContext filter_context_{&request_headers_, &connection_, - istio::envoy::config::filter::http::authn:: - v2alpha1::FilterConfig::default_instance()}; - + FilterContext filter_context_{dynamic_metadata_, &connection_, + filter_config_}; MockAuthenticatorBase authenticator_{&filter_context_}; void SetUp() override { payload_ = new Payload(); } void TearDown() override { delete (payload_); } - Http::TestHeaderMapImpl CreateTestHeaderMap(const std::string& header_key, - const std::string& header_value) { - // The base64 encoding is done through Base64::encode(). - // If the test input has special chars, may need to use the counterpart of - // Base64UrlDecode(). - std::string value_base64 = - Base64::encode(header_value.c_str(), header_value.size()); - return Http::TestHeaderMapImpl{{header_key, value_base64}}; - } - protected: iaapi::MutualTls mtls_params_; iaapi::Jwt jwt_; @@ -193,8 +184,7 @@ class ValidateJwtTest : public testing::Test, Payload default_payload_; }; -// TODO: more tests for Jwt. -TEST_F(ValidateJwtTest, ValidateJwtWithNoIstioAuthnConfig) { +TEST_F(ValidateJwtTest, NoIstioAuthnConfig) { jwt_.set_issuer("issuer@foo.com"); // authenticator_ has empty Istio authn config // When there is empty Istio authn config, validateJwt() should return @@ -206,7 +196,6 @@ TEST_F(ValidateJwtTest, ValidateJwtWithNoIstioAuthnConfig) { TEST_F(ValidateJwtTest, NoIssuer) { // no issuer in jwt google::protobuf::util::JsonParseOptions options; - FilterConfig filter_config; JsonStringToMessage( R"({ "jwt_output_payload_locations": @@ -215,11 +204,7 @@ TEST_F(ValidateJwtTest, NoIssuer) { } } )", - &filter_config, options); - Http::TestHeaderMapImpl empty_request_headers{}; - FilterContext filter_context{&empty_request_headers, &connection_, - filter_config}; - MockAuthenticatorBase authenticator{&filter_context}; + &filter_config_, options); // When there is no issuer in the JWT config, validateJwt() should return // nullptr and failure. @@ -227,12 +212,9 @@ TEST_F(ValidateJwtTest, NoIssuer) { EXPECT_TRUE(MessageDifferencer::Equals(*payload_, default_payload_)); } -TEST_F(ValidateJwtTest, EmptyJwtOutputPayloadLocations) { +TEST_F(ValidateJwtTest, OutputPayloadLocationNotDefine) { jwt_.set_issuer("issuer@foo.com"); - Http::TestHeaderMapImpl request_headers_with_jwt = CreateTestHeaderMap( - kSecIstioAuthUserInfoHeaderKey, kSecIstioAuthUserinfoHeaderValue); google::protobuf::util::JsonParseOptions options; - FilterConfig filter_config; JsonStringToMessage( R"({ "jwt_output_payload_locations": @@ -240,10 +222,8 @@ TEST_F(ValidateJwtTest, EmptyJwtOutputPayloadLocations) { } } )", - &filter_config, options); - FilterContext filter_context{&request_headers_with_jwt, &connection_, - filter_config}; - MockAuthenticatorBase authenticator{&filter_context}; + &filter_config_, options); + // authenticator has empty jwt_output_payload_locations in Istio authn config // When there is no matching jwt_output_payload_locations for the issuer in // the Istio authn config, validateJwt() should return nullptr and failure. @@ -251,47 +231,43 @@ TEST_F(ValidateJwtTest, EmptyJwtOutputPayloadLocations) { EXPECT_TRUE(MessageDifferencer::Equals(*payload_, default_payload_)); } -TEST_F(ValidateJwtTest, NoJwtInHeader) { +TEST_F(ValidateJwtTest, NoJwtPayloadOutput) { jwt_.set_issuer("issuer@foo.com"); - google::protobuf::util::JsonParseOptions options; - FilterConfig filter_config; - JsonStringToMessage( - R"({ - "jwt_output_payload_locations": - { - "issuer@foo.com": "sec-istio-auth-jwt-output" - } - } - )", - &filter_config, options); - Http::TestHeaderMapImpl empty_request_headers{}; - FilterContext filter_context{&empty_request_headers, &connection_, - filter_config}; - MockAuthenticatorBase authenticator{&filter_context}; - // When there is no JWT in the HTTP header, validateJwt() should return - // nullptr and failure. + + // When there is no JWT in request info dynamic metadata, validateJwt() should + // return nullptr and failure. EXPECT_FALSE(authenticator_.validateJwt(jwt_, payload_)); EXPECT_TRUE(MessageDifferencer::Equals(*payload_, default_payload_)); } -TEST_F(ValidateJwtTest, JwtInHeader) { +TEST_F(ValidateJwtTest, HasJwtPayloadOutputButNoDataForKey) { jwt_.set_issuer("issuer@foo.com"); - Http::TestHeaderMapImpl request_headers_with_jwt = CreateTestHeaderMap( - "sec-istio-auth-jwt-output", kSecIstioAuthUserinfoHeaderValue); - google::protobuf::util::JsonParseOptions options; - FilterConfig filter_config; - JsonStringToMessage( - R"({ - "jwt_output_payload_locations": - { - "issuer@foo.com": "sec-istio-auth-jwt-output" - } - } - )", - &filter_config, options); - FilterContext filter_context{&request_headers_with_jwt, &connection_, - filter_config}; - MockAuthenticatorBase authenticator{&filter_context}; + + (*dynamic_metadata_.mutable_filter_metadata())[Utils::IstioFilterName::kJwt] + .MergeFrom(MessageUtil::keyValueStruct("foo", "bar")); + + // When there is no JWT payload for given issuer in request info dynamic + // metadata, validateJwt() should return nullptr and failure. + EXPECT_FALSE(authenticator_.validateJwt(jwt_, payload_)); + EXPECT_TRUE(MessageDifferencer::Equals(*payload_, default_payload_)); +} + +TEST_F(ValidateJwtTest, JwtPayloadAvailableWithBadData) { + jwt_.set_issuer("issuer@foo.com"); + (*dynamic_metadata_.mutable_filter_metadata())[Utils::IstioFilterName::kJwt] + .MergeFrom(MessageUtil::keyValueStruct("issuer@foo.com", "bad-data")); + // EXPECT_CALL(request_info_, dynamicMetadata()); + + EXPECT_FALSE(authenticator_.validateJwt(jwt_, payload_)); + EXPECT_TRUE(MessageDifferencer::Equivalent(*payload_, default_payload_)); +} + +TEST_F(ValidateJwtTest, JwtPayloadAvailable) { + jwt_.set_issuer("issuer@foo.com"); + (*dynamic_metadata_.mutable_filter_metadata())[Utils::IstioFilterName::kJwt] + .MergeFrom(MessageUtil::keyValueStruct("issuer@foo.com", + kSecIstioAuthUserinfoHeaderValue)); + Payload expected_payload; JsonStringToMessage( R"({ @@ -309,9 +285,9 @@ TEST_F(ValidateJwtTest, JwtInHeader) { } } )", - &expected_payload, options); + &expected_payload, google::protobuf::util::JsonParseOptions{}); - EXPECT_TRUE(authenticator.validateJwt(jwt_, payload_)); + EXPECT_TRUE(authenticator_.validateJwt(jwt_, payload_)); EXPECT_TRUE(MessageDifferencer::Equals(expected_payload, *payload_)); } diff --git a/src/envoy/http/authn/authn_utils.cc b/src/envoy/http/authn/authn_utils.cc index 78c232e3108..98a1940f16e 100644 --- a/src/envoy/http/authn/authn_utils.cc +++ b/src/envoy/http/authn/authn_utils.cc @@ -50,27 +50,8 @@ void ExtractJwtAudience( } }; // namespace -// Retrieve the JwtPayload from the HTTP headers with the key -bool AuthnUtils::GetJWTPayloadFromHeaders( - const HeaderMap& headers, const LowerCaseString& jwt_payload_key, - istio::authn::JwtPayload* payload) { - const HeaderEntry* entry = headers.get(jwt_payload_key); - if (!entry) { - ENVOY_LOG(debug, "No JWT payload {} in the header", jwt_payload_key.get()); - return false; - } - std::string value(entry->value().c_str(), entry->value().size()); - // JwtAuth::Base64UrlDecode() is different from Base64::decode(). - std::string payload_str = JwtAuth::Base64UrlDecode(value); - // Return an empty string if Base64 decode fails. - if (payload_str.empty()) { - ENVOY_LOG(error, "Invalid {} header, invalid base64: {}", - jwt_payload_key.get(), value); - return false; - } - *payload->mutable_raw_claims() = payload_str; - ::google::protobuf::Map< ::std::string, ::std::string>* claims = - payload->mutable_claims(); +bool AuthnUtils::ProcessJwtPayload(const std::string& payload_str, + istio::authn::JwtPayload* payload) { Envoy::Json::ObjectSharedPtr json_obj; try { json_obj = Json::Factory::loadFromString(payload_str); @@ -80,6 +61,10 @@ bool AuthnUtils::GetJWTPayloadFromHeaders( return false; } + *payload->mutable_raw_claims() = payload_str; + ::google::protobuf::Map< ::std::string, ::std::string>* claims = + payload->mutable_claims(); + // Extract claims json_obj->iterate( [payload](const std::string& key, const Json::Object& obj) -> bool { diff --git a/src/envoy/http/authn/authn_utils.h b/src/envoy/http/authn/authn_utils.h index e156f5eb2c3..023b03e8f10 100644 --- a/src/envoy/http/authn/authn_utils.h +++ b/src/envoy/http/authn/authn_utils.h @@ -28,12 +28,11 @@ namespace AuthN { // AuthnUtils class provides utility functions used for authentication. class AuthnUtils : public Logger::Loggable { public: - // Retrieve the JWT payload from the HTTP header into the output payload map - // Return true if parsing the header payload key succeeds. - // Otherwise, return false. - static bool GetJWTPayloadFromHeaders(const HeaderMap& headers, - const LowerCaseString& jwt_payload_key, - istio::authn::JwtPayload* payload); + // Parse JWT payload string (which typically is the output from jwt filter) + // and populate JwtPayload object. Return true if input string can be parsed + // successfully. Otherwise, return false. + static bool ProcessJwtPayload(const std::string& jwt_payload_str, + istio::authn::JwtPayload* payload); }; } // namespace AuthN diff --git a/src/envoy/http/authn/authn_utils_test.cc b/src/envoy/http/authn/authn_utils_test.cc index 3663476ab0d..710a30e5bda 100644 --- a/src/envoy/http/authn/authn_utils_test.cc +++ b/src/envoy/http/authn/authn_utils_test.cc @@ -27,7 +27,6 @@ namespace Istio { namespace AuthN { namespace { -const LowerCaseString kSecIstioAuthUserInfoHeaderKey("sec-istio-auth-userinfo"); const std::string kSecIstioAuthUserinfoHeaderValue = R"( { @@ -58,20 +57,8 @@ const std::string kSecIstioAuthUserInfoHeaderWithTwoAudValue = } )"; -Http::TestHeaderMapImpl CreateTestHeaderMap(const LowerCaseString& header_key, - const std::string& header_value) { - // The base64 encoding is done through Base64::encode(). - // If the test input has special chars, may need to use the counterpart of - // Base64UrlDecode(). - std::string value_base64 = - Base64::encode(header_value.c_str(), header_value.size()); - return Http::TestHeaderMapImpl{{header_key.get(), value_base64}}; -} - TEST(AuthnUtilsTest, GetJwtPayloadFromHeaderTest) { JwtPayload payload, expected_payload; - Http::TestHeaderMapImpl request_headers_with_jwt = CreateTestHeaderMap( - kSecIstioAuthUserInfoHeaderKey, kSecIstioAuthUserinfoHeaderValue); ASSERT_TRUE(Protobuf::TextFormat::ParseFromString( R"( user: "issuer@foo.com/sub@foo.com" @@ -95,19 +82,16 @@ TEST(AuthnUtilsTest, GetJwtPayloadFromHeaderTest) { raw_claims: ")" + StringUtil::escape(kSecIstioAuthUserinfoHeaderValue) + R"(")", &expected_payload)); - // The payload returned from GetJWTPayloadFromHeaders() should be the same as + // The payload returned from ProcessJwtPayload() should be the same as // the expected. - bool ret = AuthnUtils::GetJWTPayloadFromHeaders( - request_headers_with_jwt, kSecIstioAuthUserInfoHeaderKey, &payload); + bool ret = + AuthnUtils::ProcessJwtPayload(kSecIstioAuthUserinfoHeaderValue, &payload); EXPECT_TRUE(ret); EXPECT_TRUE(MessageDifferencer::Equals(expected_payload, payload)); } -TEST(AuthnUtilsTest, GetJwtPayloadFromHeaderWithNoAudTest) { +TEST(AuthnUtilsTest, ProcessJwtPayloadWithNoAudTest) { JwtPayload payload, expected_payload; - Http::TestHeaderMapImpl request_headers_with_jwt = - CreateTestHeaderMap(kSecIstioAuthUserInfoHeaderKey, - kSecIstioAuthUserInfoHeaderWithNoAudValue); ASSERT_TRUE(Protobuf::TextFormat::ParseFromString( R"( user: "issuer@foo.com/sub@foo.com" @@ -127,20 +111,17 @@ TEST(AuthnUtilsTest, GetJwtPayloadFromHeaderWithNoAudTest) { StringUtil::escape(kSecIstioAuthUserInfoHeaderWithNoAudValue) + R"(")", &expected_payload)); - // The payload returned from GetJWTPayloadFromHeaders() should be the same as + // The payload returned from ProcessJwtPayload() should be the same as // the expected. When there is no aud, the aud is not saved in the payload // and claims. - bool ret = AuthnUtils::GetJWTPayloadFromHeaders( - request_headers_with_jwt, kSecIstioAuthUserInfoHeaderKey, &payload); + bool ret = AuthnUtils::ProcessJwtPayload( + kSecIstioAuthUserInfoHeaderWithNoAudValue, &payload); EXPECT_TRUE(ret); EXPECT_TRUE(MessageDifferencer::Equals(expected_payload, payload)); } -TEST(AuthnUtilsTest, GetJwtPayloadFromHeaderWithTwoAudTest) { +TEST(AuthnUtilsTest, ProcessJwtPayloadWithTwoAudTest) { JwtPayload payload, expected_payload; - Http::TestHeaderMapImpl request_headers_with_jwt = - CreateTestHeaderMap(kSecIstioAuthUserInfoHeaderKey, - kSecIstioAuthUserInfoHeaderWithTwoAudValue); ASSERT_TRUE(Protobuf::TextFormat::ParseFromString( R"( user: "issuer@foo.com/sub@foo.com" @@ -163,11 +144,11 @@ TEST(AuthnUtilsTest, GetJwtPayloadFromHeaderWithTwoAudTest) { R"(")", &expected_payload)); - // The payload returned from GetJWTPayloadFromHeaders() should be the same as + // The payload returned from ProcessJwtPayload() should be the same as // the expected. When the aud is a string array, the aud is not saved in the // claims. - bool ret = AuthnUtils::GetJWTPayloadFromHeaders( - request_headers_with_jwt, kSecIstioAuthUserInfoHeaderKey, &payload); + bool ret = AuthnUtils::ProcessJwtPayload( + kSecIstioAuthUserInfoHeaderWithTwoAudValue, &payload); EXPECT_TRUE(ret); EXPECT_TRUE(MessageDifferencer::Equals(expected_payload, payload)); } diff --git a/src/envoy/http/authn/filter_context.cc b/src/envoy/http/authn/filter_context.cc index ac72ae8bc89..38bf952db00 100644 --- a/src/envoy/http/authn/filter_context.cc +++ b/src/envoy/http/authn/filter_context.cc @@ -14,6 +14,7 @@ */ #include "src/envoy/http/authn/filter_context.h" +#include "src/envoy/utils/filter_names.h" #include "src/envoy/utils/utils.h" using istio::authn::Payload; @@ -71,6 +72,29 @@ void FilterContext::setPrincipal(const iaapi::PrincipalBinding& binding) { } } +bool FilterContext::getJwtPayload(const std::string& issuer, + std::string* payload) const { + const auto filter_it = + dynamic_metadata_.filter_metadata().find(Utils::IstioFilterName::kJwt); + if (filter_it == dynamic_metadata_.filter_metadata().end()) { + ENVOY_LOG(debug, "No dynamic_metadata found for filter {}", + Utils::IstioFilterName::kJwt); + return false; + } + + const auto& data_struct = filter_it->second; + const auto entry_it = data_struct.fields().find(issuer); + if (entry_it == data_struct.fields().end()) { + return false; + } + if (entry_it->second.string_value().empty()) { + return false; + } + + *payload = entry_it->second.string_value(); + return true; +} + } // namespace AuthN } // namespace Istio } // namespace Http diff --git a/src/envoy/http/authn/filter_context.h b/src/envoy/http/authn/filter_context.h index 7907479d55f..0b5e060f98e 100644 --- a/src/envoy/http/authn/filter_context.h +++ b/src/envoy/http/authn/filter_context.h @@ -17,8 +17,8 @@ #include "authentication/v1alpha1/policy.pb.h" #include "common/common/logger.h" +#include "envoy/api/v2/core/base.pb.h" #include "envoy/config/filter/http/authn/v2alpha1/config.pb.h" -#include "envoy/http/header_map.h" #include "envoy/network/connection.h" #include "src/istio/authn/context.pb.h" @@ -27,15 +27,16 @@ namespace Http { namespace Istio { namespace AuthN { -// FilterContext holds inputs, such as request header and connection and -// result data for authentication process. +// FilterContext holds inputs, such as request dynamic metadata and connection +// and result data for authentication process. class FilterContext : public Logger::Loggable { public: FilterContext( - HeaderMap* headers, const Network::Connection* connection, + const envoy::api::v2::core::Metadata& dynamic_metadata, + const Network::Connection* connection, const istio::envoy::config::filter::http::authn::v2alpha1::FilterConfig& filter_config) - : headers_(headers), + : dynamic_metadata_(dynamic_metadata), connection_(connection), filter_config_(filter_config) {} virtual ~FilterContext() {} @@ -56,8 +57,6 @@ class FilterContext : public Logger::Loggable { // Returns the authentication result. const istio::authn::Result& authenticationResult() { return result_; } - // Accessor to headers. - HeaderMap* headers() { return headers_; } // Accessor to connection const Network::Connection* connection() { return connection_; } // Accessor to the filter config @@ -66,9 +65,15 @@ class FilterContext : public Logger::Loggable { return filter_config_; } + // Gets JWT payload (output from JWT filter) for given issuer. If non-empty + // payload found, returns true and set the output payload string. Otherwise, + // returns false. + bool getJwtPayload(const std::string& issuer, std::string* payload) const; + private: - // Pointer to the headers of the request. - HeaderMap* headers_; + // Const reference to request info dynamic metadata. This provides data that + // output from other filters, e.g JWT. + const envoy::api::v2::core::Metadata& dynamic_metadata_; // Pointer to network connection of the request. const Network::Connection* connection_; diff --git a/src/envoy/http/authn/filter_context_test.cc b/src/envoy/http/authn/filter_context_test.cc index c00f3514ea0..6fb89866e15 100644 --- a/src/envoy/http/authn/filter_context_test.cc +++ b/src/envoy/http/authn/filter_context_test.cc @@ -14,6 +14,7 @@ */ #include "src/envoy/http/authn/filter_context.h" +#include "envoy/api/v2/core/base.pb.h" #include "src/envoy/http/authn/test_utils.h" #include "test/test_common/utility.h" @@ -32,9 +33,9 @@ class FilterContextTest : public testing::Test { public: virtual ~FilterContextTest() {} - // This test suit does not use headers nor connection, so ok to use null for - // them. - FilterContext filter_context_{nullptr, nullptr, + envoy::api::v2::core::Metadata metadata_; + // This test suit does not use connection, so ok to use null for it. + FilterContext filter_context_{metadata_, nullptr, istio::envoy::config::filter::http::authn:: v2alpha1::FilterConfig::default_instance()}; diff --git a/src/envoy/http/authn/http_filter.cc b/src/envoy/http/authn/http_filter.cc index 27e14b8a088..ddb0ed4f680 100644 --- a/src/envoy/http/authn/http_filter.cc +++ b/src/envoy/http/authn/http_filter.cc @@ -20,6 +20,7 @@ #include "src/envoy/http/authn/origin_authenticator.h" #include "src/envoy/http/authn/peer_authenticator.h" #include "src/envoy/utils/authn.h" +#include "src/envoy/utils/filter_names.h" #include "src/envoy/utils/utils.h" using istio::authn::Payload; @@ -48,14 +49,14 @@ FilterHeadersStatus AuthenticationFilter::decodeHeaders(HeaderMap& headers, state_ = State::PROCESSING; filter_context_.reset(new Istio::AuthN::FilterContext( - &headers, decoder_callbacks_->connection(), filter_config_)); + decoder_callbacks_->requestInfo().dynamicMetadata(), + decoder_callbacks_->connection(), filter_config_)); Payload payload; if (!filter_config_.policy().peer_is_optional() && !createPeerAuthenticator(filter_context_.get())->run(&payload)) { rejectRequest("Peer authentication failed."); - removeJwtPayloadFromHeaders(); return FilterHeadersStatus::StopIteration; } @@ -63,10 +64,6 @@ FilterHeadersStatus AuthenticationFilter::decodeHeaders(HeaderMap& headers, filter_config_.policy().origin_is_optional() || createOriginAuthenticator(filter_context_.get())->run(&payload); - // After Istio authn, the JWT headers consumed by Istio authn should be - // removed. - removeJwtPayloadFromHeaders(); - if (!success) { rejectRequest("Origin authentication failed."); return FilterHeadersStatus::StopIteration; @@ -74,28 +71,22 @@ FilterHeadersStatus AuthenticationFilter::decodeHeaders(HeaderMap& headers, // Put authentication result to headers. if (filter_context_ != nullptr) { - // TODO(yangminzhu): Remove the header and only use the metadata to pass the + // TODO(diemvu): Remove the header and only use the metadata to pass the // attributes. Utils::Authentication::SaveResultToHeader( - filter_context_->authenticationResult(), filter_context_->headers()); - + filter_context_->authenticationResult(), &headers); // Save auth results in the metadata, could be later used by RBAC filter. ProtobufWkt::Struct data; Utils::Authentication::SaveAuthAttributesToStruct( filter_context_->authenticationResult(), data); - decoder_callbacks_->requestInfo().setDynamicMetadata("istio_authn", data); + decoder_callbacks_->requestInfo().setDynamicMetadata( + Utils::IstioFilterName::kAuthentication, data); ENVOY_LOG(debug, "Saved Dynamic Metadata:\n{}", data.DebugString()); } state_ = State::COMPLETE; return FilterHeadersStatus::Continue; } -void AuthenticationFilter::removeJwtPayloadFromHeaders() { - for (auto const iter : filter_config_.jwt_output_payload_locations()) { - filter_context_->headers()->remove(LowerCaseString(iter.second)); - } -} - FilterDataStatus AuthenticationFilter::decodeData(Buffer::Instance&, bool) { if (state_ == State::PROCESSING) { return FilterDataStatus::StopIterationAndWatermark; diff --git a/src/envoy/http/authn/http_filter.h b/src/envoy/http/authn/http_filter.h index c7a29a02a8e..a710cfc0559 100644 --- a/src/envoy/http/authn/http_filter.h +++ b/src/envoy/http/authn/http_filter.h @@ -63,9 +63,6 @@ class AuthenticationFilter : public StreamDecoderFilter, createOriginAuthenticator(Istio::AuthN::FilterContext* filter_context); - // Removes output JWT valiation from headers. - void removeJwtPayloadFromHeaders(); - private: // Store the config. const istio::envoy::config::filter::http::authn::v2alpha1::FilterConfig& diff --git a/src/envoy/http/authn/http_filter_factory.cc b/src/envoy/http/authn/http_filter_factory.cc index 3fa6182f504..221cc78a6e5 100644 --- a/src/envoy/http/authn/http_filter_factory.cc +++ b/src/envoy/http/authn/http_filter_factory.cc @@ -18,6 +18,7 @@ #include "envoy/server/filter_config.h" #include "google/protobuf/util/json_util.h" #include "src/envoy/http/authn/http_filter.h" +#include "src/envoy/utils/filter_names.h" #include "src/envoy/utils/utils.h" using istio::envoy::config::filter::http::authn::v2alpha1::FilterConfig; @@ -26,11 +27,6 @@ namespace Envoy { namespace Server { namespace Configuration { -namespace { -// The name for the Istio authentication filter. -const std::string kAuthnFactoryName("istio_authn"); -} // namespace - class AuthnFilterConfig : public NamedHttpFilterConfigFactory, public Logger::Loggable { public: @@ -65,7 +61,9 @@ class AuthnFilterConfig : public NamedHttpFilterConfigFactory, return ProtobufTypes::MessagePtr{new FilterConfig}; } - std::string name() override { return kAuthnFactoryName; } + std::string name() override { + return Utils::IstioFilterName::kAuthentication; + } private: Http::FilterFactoryCb createFilterFactory(const FilterConfig& config_pb) { diff --git a/src/envoy/http/authn/http_filter_integration_test.cc b/src/envoy/http/authn/http_filter_integration_test.cc index 80416e135e0..0ba682b5007 100644 --- a/src/envoy/http/authn/http_filter_integration_test.cc +++ b/src/envoy/http/authn/http_filter_integration_test.cc @@ -14,8 +14,12 @@ */ #include "common/common/base64.h" +#include "common/common/utility.h" +#include "extensions/filters/http/well_known_names.h" +#include "fmt/printf.h" +#include "src/envoy/utils/filter_names.h" #include "src/istio/authn/context.pb.h" -#include "test/integration/http_integration.h" +#include "test/integration/http_protocol_integration.h" using google::protobuf::util::MessageDifferencer; using istio::authn::Payload; @@ -23,65 +27,72 @@ using istio::authn::Result; namespace Envoy { namespace { -const std::string kSecIstioAuthUserInfoHeaderKey = "sec-istio-auth-userinfo"; -const std::string kSecIstioAuthUserinfoHeaderValue = - "eyJpc3MiOiI2Mjg2NDU3NDE4ODEtbm9hYml1MjNmNWE4bThvdmQ4dWN2Njk4bGo3OH" - "Z2MGxAZGV2ZWxvcGVyLmdzZXJ2aWNlYWNjb3VudC5jb20iLCJzdWIiOiI2Mjg2NDU3" - "NDE4ODEtbm9hYml1MjNmNWE4bThvdmQ4dWN2Njk4bGo3OHZ2MGxAZGV2ZWxvcGVyLm" - "dzZXJ2aWNlYWNjb3VudC5jb20iLCJhdWQiOiJib29rc3RvcmUtZXNwLWVjaG8uY2xv" - "dWRlbmRwb2ludHNhcGlzLmNvbSIsImlhdCI6MTUxMjc1NDIwNSwiZXhwIjo1MTEyNz" - "U0MjA1fQ=="; -const Envoy::Http::LowerCaseString kSecIstioAuthnPayloadHeaderKey( + +static const Envoy::Http::LowerCaseString kSecIstioAuthnPayloadHeaderKey( "sec-istio-authn-payload"); -class AuthenticationFilterIntegrationTest - : public HttpIntegrationTest, - public testing::TestWithParam { - public: - AuthenticationFilterIntegrationTest() - : HttpIntegrationTest(Http::CodecClient::Type::HTTP1, GetParam()), - default_request_headers_{ - {":method", "GET"}, {":path", "/"}, {":authority", "host"}}, - request_headers_with_jwt_{{":method", "GET"}, - {":path", "/"}, - {":authority", "host"}, - {kSecIstioAuthUserInfoHeaderKey, - kSecIstioAuthUserinfoHeaderValue}} {} - - void SetUp() override { - fake_upstreams_.emplace_back( - new FakeUpstream(0, FakeHttpConnection::Type::HTTP1, version_)); - registerPort("upstream_0", - fake_upstreams_.back()->localAddress()->ip()->port()); - fake_upstreams_.emplace_back( - new FakeUpstream(0, FakeHttpConnection::Type::HTTP1, version_)); - registerPort("upstream_1", - fake_upstreams_.back()->localAddress()->ip()->port()); - } - - void TearDown() override { - test_server_.reset(); - fake_upstream_connection_.reset(); - fake_upstreams_.clear(); - } - - protected: - Http::TestHeaderMapImpl default_request_headers_; - Http::TestHeaderMapImpl request_headers_with_jwt_; -}; +// Default request for testing. +static const Http::TestHeaderMapImpl kSimpleRequestHeader{{ + {":method", "GET"}, + {":path", "/"}, + {":scheme", "http"}, + {":authority", "host"}, + {"x-forwarded-for", "10.0.0.1"}, +}}; + +// Keep the same as issuer in the policy below. +static const char kJwtIssuer[] = "some@issuer"; + +static const char kAuthnFilterWithJwt[] = R"( + name: istio_authn + config: + policy: + origins: + - jwt: + issuer: some@issuer + jwks_uri: http://localhost:8081/)"; + +// Payload data to inject. Note the iss claim intentionally set different from +// kJwtIssuer. +static const char kMockJwtPayload[] = + "{\"iss\":\"https://example.com\"," + "\"sub\":\"test@example.com\",\"exp\":2001001001," + "\"aud\":\"example_service\"}"; +// Returns a simple header-to-metadata filter config that can be used to inject +// data into request info dynamic metadata for testing. +std::string MakeHeaderToMetadataConfig() { + return fmt::sprintf( + R"( + name: %s + config: + request_rules: + - header: x-mock-metadata-injection + on_header_missing: + metadata_namespace: %s + key: %s + value: "%s" + type: STRING)", + Extensions::HttpFilters::HttpFilterNames::get().HeaderToMetadata, + Utils::IstioFilterName::kJwt, kJwtIssuer, + StringUtil::escape(kMockJwtPayload)); +} + +typedef HttpProtocolIntegrationTest AuthenticationFilterIntegrationTest; INSTANTIATE_TEST_CASE_P( - IpVersions, AuthenticationFilterIntegrationTest, - testing::ValuesIn(TestEnvironment::getIpVersionsForTest())); + Protocols, AuthenticationFilterIntegrationTest, + testing::ValuesIn(HttpProtocolIntegrationTest::getProtocolTestParams()), + HttpProtocolIntegrationTest::protocolTestParamsToString); TEST_P(AuthenticationFilterIntegrationTest, EmptyPolicy) { - createTestServer("src/envoy/http/authn/testdata/envoy_empty.conf", {"http"}); + config_helper_.addFilter("name: istio_authn"); + initialize(); codec_client_ = makeHttpConnection(makeClientConnection((lookupPort("http")))); - auto response = - codec_client_->makeHeaderOnlyRequest(default_request_headers_); - // Wait for request to upstream[0] (backend) - waitForNextUpstreamRequest(0); + auto response = codec_client_->makeHeaderOnlyRequest(kSimpleRequestHeader); + // Wait for request to upstream (backend) + waitForNextUpstreamRequest(); + // Send backend response. upstream_request_->encodeHeaders(Http::TestHeaderMapImpl{{":status", "200"}}, true); @@ -92,15 +103,19 @@ TEST_P(AuthenticationFilterIntegrationTest, EmptyPolicy) { } TEST_P(AuthenticationFilterIntegrationTest, SourceMTlsFail) { - createTestServer("src/envoy/http/authn/testdata/envoy_peer_mtls.conf", - {"http"}); + config_helper_.addFilter(R"( + name: istio_authn + config: + policy: + peers: + - mtls: {})"); + initialize(); // AuthN filter use MTls, but request doesn't have certificate, request // would be rejected. codec_client_ = makeHttpConnection(makeClientConnection((lookupPort("http")))); - auto response = - codec_client_->makeHeaderOnlyRequest(default_request_headers_); + auto response = codec_client_->makeHeaderOnlyRequest(kSimpleRequestHeader); // Request is rejected, there will be no upstream request (thus no // waitForNextUpstreamRequest). @@ -112,16 +127,14 @@ TEST_P(AuthenticationFilterIntegrationTest, SourceMTlsFail) { // TODO (diemtvu/lei-tang): add test for MTls success. TEST_P(AuthenticationFilterIntegrationTest, OriginJwtRequiredHeaderNoJwtFail) { - createTestServer( - "src/envoy/http/authn/testdata/envoy_origin_jwt_authn_only.conf", - {"http"}); + config_helper_.addFilter(kAuthnFilterWithJwt); + initialize(); // The AuthN filter requires JWT, but request doesn't have JWT, request // would be rejected. codec_client_ = makeHttpConnection(makeClientConnection((lookupPort("http")))); - auto response = - codec_client_->makeHeaderOnlyRequest(default_request_headers_); + auto response = codec_client_->makeHeaderOnlyRequest(kSimpleRequestHeader); // Request is rejected, there will be no upstream request (thus no // waitForNextUpstreamRequest). @@ -131,19 +144,18 @@ TEST_P(AuthenticationFilterIntegrationTest, OriginJwtRequiredHeaderNoJwtFail) { } TEST_P(AuthenticationFilterIntegrationTest, CheckValidJwtPassAuthentication) { - createTestServer( - "src/envoy/http/authn/testdata/envoy_origin_jwt_authn_only.conf", - {"http"}); + config_helper_.addFilter(kAuthnFilterWithJwt); + config_helper_.addFilter(MakeHeaderToMetadataConfig()); + initialize(); // The AuthN filter requires JWT. The http request contains validated JWT and // the authentication should succeed. codec_client_ = makeHttpConnection(makeClientConnection((lookupPort("http")))); - auto response = - codec_client_->makeHeaderOnlyRequest(request_headers_with_jwt_); + auto response = codec_client_->makeHeaderOnlyRequest(kSimpleRequestHeader); - // Wait for request to upstream[0] (backend) - waitForNextUpstreamRequest(0); + // Wait for request to upstream (backend) + waitForNextUpstreamRequest(); // Send backend response. upstream_request_->encodeHeaders(Http::TestHeaderMapImpl{{":status", "200"}}, true); @@ -151,98 +163,38 @@ TEST_P(AuthenticationFilterIntegrationTest, CheckValidJwtPassAuthentication) { response->waitForEndStream(); EXPECT_TRUE(response->complete()); EXPECT_STREQ("200", response->headers().Status()->value().c_str()); -} - -TEST_P(AuthenticationFilterIntegrationTest, CheckConsumedJwtHeadersAreRemoved) { - const Envoy::Http::LowerCaseString header_location( - "location-to-read-jwt-result"); - const std::string jwt_header = - R"( - { - "iss": "issuer@foo.com", - "sub": "sub@foo.com", - "aud": "aud1", - "non-string-will-be-ignored": 1512754205, - "some-other-string-claims": "some-claims-kept" - } - )"; - std::string jwt_header_base64 = - Base64::encode(jwt_header.c_str(), jwt_header.size()); - Http::TestHeaderMapImpl request_headers_with_jwt_at_specified_location{ - {":method", "GET"}, - {":path", "/"}, - {":authority", "host"}, - {"location-to-read-jwt-result", jwt_header_base64}}; - // In this config, the JWT verification result for "issuer@foo.com" is in the - // header "location-to-read-jwt-result" - createTestServer( - "src/envoy/http/authn/testdata/" - "envoy_jwt_with_output_header_location.conf", - {"http"}); - // The AuthN filter requires JWT and the http request contains validated JWT. - // In this case, the authentication should succeed and an authn result - // should be generated. - codec_client_ = - makeHttpConnection(makeClientConnection((lookupPort("http")))); - auto response = codec_client_->makeHeaderOnlyRequest( - request_headers_with_jwt_at_specified_location); - - // Wait for request to upstream[0] (backend) - waitForNextUpstreamRequest(0); - response->waitForEndStream(); - - // After Istio authn, the JWT headers consumed by Istio authn should have - // been removed. - EXPECT_TRUE(nullptr == upstream_request_->headers().get(header_location)); -} - -TEST_P(AuthenticationFilterIntegrationTest, CheckAuthnResultIsExpected) { - createTestServer( - "src/envoy/http/authn/testdata/envoy_origin_jwt_authn_only.conf", - {"http"}); - - // The AuthN filter requires JWT and the http request contains validated JWT. - // In this case, the authentication should succeed and an authn result - // should be generated. - codec_client_ = - makeHttpConnection(makeClientConnection((lookupPort("http")))); - auto response = - codec_client_->makeHeaderOnlyRequest(request_headers_with_jwt_); - - // Wait for request to upstream[0] (backend) - waitForNextUpstreamRequest(0); - response->waitForEndStream(); - // Authn result should be as expected + // Verify authn result in header. + // TODO(diemvu): find a way to verify data in request info as the use of + // header for authentication result will be removed soon. const Envoy::Http::HeaderString &header_value = upstream_request_->headers().get(kSecIstioAuthnPayloadHeaderKey)->value(); std::string value_base64(header_value.c_str(), header_value.size()); const std::string value = Base64::decode(value_base64); Result result; - google::protobuf::util::JsonParseOptions options; - Result expected_result; + EXPECT_TRUE(result.ParseFromString(value)); - bool parse_ret = result.ParseFromString(value); - EXPECT_TRUE(parse_ret); - JsonStringToMessage( - R"( - { - "origin": { - "user": "628645741881-noabiu23f5a8m8ovd8ucv698lj78vv0l@developer.gserviceaccount.com/628645741881-noabiu23f5a8m8ovd8ucv698lj78vv0l@developer.gserviceaccount.com", - "audiences": [ - "bookstore-esp-echo.cloudendpointsapis.com" - ], - "presenter": "", - "claims": { - "aud": "bookstore-esp-echo.cloudendpointsapis.com", - "iss": "628645741881-noabiu23f5a8m8ovd8ucv698lj78vv0l@developer.gserviceaccount.com", - "sub": "628645741881-noabiu23f5a8m8ovd8ucv698lj78vv0l@developer.gserviceaccount.com" - }, - raw_claims: "{\"iss\":\"628645741881-noabiu23f5a8m8ovd8ucv698lj78vv0l@developer.gserviceaccount.com\",\"sub\":\"628645741881-noabiu23f5a8m8ovd8ucv698lj78vv0l@developer.gserviceaccount.com\",\"aud\":\"bookstore-esp-echo.cloudendpointsapis.com\",\"iat\":1512754205,\"exp\":5112754205}" + Result expected_result; + ASSERT_TRUE(Protobuf::TextFormat::ParseFromString(R"( + origin { + user: "https://example.com/test@example.com" + audiences: "example_service" + claims { + key: "aud" + value: "example_service" } - } - )", - &expected_result, options); + claims { + key: "iss" + value: "https://example.com" + } + claims { + key: "sub" + value: "test@example.com" + } + raw_claims: "{\"iss\":\"https://example.com\",\"sub\":\"test@example.com\",\"exp\":2001001001,\"aud\":\"example_service\"}" + })", + &expected_result)); + // Note: TestUtility::protoEqual() uses SerializeAsString() and the output // is non-deterministic. Thus, MessageDifferencer::Equals() is used. EXPECT_TRUE(MessageDifferencer::Equals(expected_result, result)); diff --git a/src/envoy/http/authn/http_filter_test.cc b/src/envoy/http/authn/http_filter_test.cc index a71d60aeb56..eab12be2dc7 100644 --- a/src/envoy/http/authn/http_filter_test.cc +++ b/src/envoy/http/authn/http_filter_test.cc @@ -16,6 +16,7 @@ #include "src/envoy/http/authn/http_filter.h" #include "common/common/base64.h" #include "common/http/header_map_impl.h" +#include "common/request_info/request_info_impl.h" #include "envoy/config/filter/http/authn/v2alpha1/config.pb.h" #include "gmock/gmock.h" #include "gtest/gtest.h" @@ -32,6 +33,7 @@ using istio::authn::Result; using istio::envoy::config::filter::http::authn::v2alpha1::FilterConfig; using testing::Invoke; using testing::NiceMock; +using testing::ReturnRef; using testing::StrictMock; using testing::_; @@ -116,6 +118,9 @@ TEST_F(AuthenticationFilterTest, PeerFail) { EXPECT_CALL(filter_, createPeerAuthenticator(_)) .Times(1) .WillOnce(Invoke(createAlwaysFailAuthenticator)); + RequestInfo::RequestInfoImpl request_info(Http::Protocol::Http2); + EXPECT_CALL(decoder_callbacks_, requestInfo()) + .WillRepeatedly(ReturnRef(request_info)); EXPECT_CALL(decoder_callbacks_, encodeHeaders_(_, _)) .Times(1) .WillOnce(testing::Invoke([](Http::HeaderMap &headers, bool) { @@ -152,13 +157,34 @@ TEST_F(AuthenticationFilterTest, AllPass) { EXPECT_CALL(filter_, createOriginAuthenticator(_)) .Times(1) .WillOnce(Invoke(createAlwaysPassAuthenticator)); + RequestInfo::RequestInfoImpl request_info(Http::Protocol::Http2); + EXPECT_CALL(decoder_callbacks_, requestInfo()) + .WillRepeatedly(ReturnRef(request_info)); + EXPECT_EQ(Http::FilterHeadersStatus::Continue, filter_.decodeHeaders(request_headers_, true)); - Result authn; - EXPECT_TRUE( - Utils::Authentication::FetchResultFromHeader(request_headers_, &authn)); - EXPECT_TRUE(TestUtility::protoEqual( - TestUtilities::AuthNResultFromString(R"(peer_user: "foo")"), authn)); + + EXPECT_EQ(1, request_info.dynamicMetadata().filter_metadata_size()); + const auto *data = + Utils::Authentication::GetResultFromRequestInfo(request_info); + ASSERT_TRUE(data); + + ProtobufWkt::Struct expected_data; + ASSERT_TRUE(Protobuf::TextFormat::ParseFromString(R"( + fields { + key: "source.principal" + value { + string_value: "foo" + } + } + fields { + key: "source.user" + value { + string_value: "foo" + } + })", + &expected_data)); + EXPECT_TRUE(TestUtility::protoEqual(expected_data, *data)); } TEST_F(AuthenticationFilterTest, IgnoreBothFail) { diff --git a/src/envoy/http/authn/origin_authenticator_test.cc b/src/envoy/http/authn/origin_authenticator_test.cc index 8aeaa85215f..0a4211692f0 100644 --- a/src/envoy/http/authn/origin_authenticator_test.cc +++ b/src/envoy/http/authn/origin_authenticator_test.cc @@ -15,8 +15,8 @@ #include "src/envoy/http/authn/origin_authenticator.h" #include "authentication/v1alpha1/policy.pb.h" -#include "common/http/header_map_impl.h" #include "common/protobuf/protobuf.h" +#include "envoy/api/v2/core/base.pb.h" #include "gmock/gmock.h" #include "gtest/gtest.h" #include "src/envoy/http/authn/test_utils.h" @@ -90,8 +90,7 @@ class MockOriginAuthenticator : public OriginAuthenticator { class OriginAuthenticatorTest : public testing::TestWithParam { public: - OriginAuthenticatorTest() - : request_headers_{{":method", "GET"}, {":path", "/"}} {} + OriginAuthenticatorTest() {} virtual ~OriginAuthenticatorTest() {} void SetUp() override { @@ -121,10 +120,11 @@ class OriginAuthenticatorTest : public testing::TestWithParam { protected: std::unique_ptr> authenticator_; - Http::TestHeaderMapImpl request_headers_; - FilterContext filter_context_{&request_headers_, nullptr, - istio::envoy::config::filter::http::authn:: - v2alpha1::FilterConfig::default_instance()}; + // envoy::api::v2::core::Metadata metadata_; + FilterContext filter_context_{ + envoy::api::v2::core::Metadata::default_instance(), nullptr, + istio::envoy::config::filter::http::authn::v2alpha1::FilterConfig:: + default_instance()}; iaapi::Policy policy_; Payload* payload_; diff --git a/src/envoy/http/authn/peer_authenticator_test.cc b/src/envoy/http/authn/peer_authenticator_test.cc index d0cc95b6f6a..60f04a827ec 100644 --- a/src/envoy/http/authn/peer_authenticator_test.cc +++ b/src/envoy/http/authn/peer_authenticator_test.cc @@ -15,8 +15,8 @@ #include "src/envoy/http/authn/peer_authenticator.h" #include "authentication/v1alpha1/policy.pb.h" -#include "common/http/header_map_impl.h" #include "common/protobuf/protobuf.h" +#include "envoy/api/v2/core/base.pb.h" #include "gmock/gmock.h" #include "gtest/gtest.h" #include "src/envoy/http/authn/test_utils.h" @@ -52,8 +52,7 @@ class MockPeerAuthenticator : public PeerAuthenticator { class PeerAuthenticatorTest : public testing::Test { public: - PeerAuthenticatorTest() - : request_headers_{{":method", "GET"}, {":path", "/"}} {} + PeerAuthenticatorTest() {} virtual ~PeerAuthenticatorTest() {} void createAuthenticator() { @@ -67,10 +66,10 @@ class PeerAuthenticatorTest : public testing::Test { protected: std::unique_ptr> authenticator_; - Http::TestHeaderMapImpl request_headers_; - FilterContext filter_context_{&request_headers_, nullptr, - istio::envoy::config::filter::http::authn:: - v2alpha1::FilterConfig::default_instance()}; + FilterContext filter_context_{ + envoy::api::v2::core::Metadata::default_instance(), nullptr, + istio::envoy::config::filter::http::authn::v2alpha1::FilterConfig:: + default_instance()}; iaapi::Policy policy_; Payload* payload_; diff --git a/src/envoy/http/authn/testdata/envoy_empty.conf b/src/envoy/http/authn/testdata/envoy_empty.conf deleted file mode 100644 index c9f244f91ce..00000000000 --- a/src/envoy/http/authn/testdata/envoy_empty.conf +++ /dev/null @@ -1,85 +0,0 @@ -{ - "listeners": [ - { - "address": "tcp://{{ ip_loopback_address }}:0", - "bind_to_port": true, - "filters": [ - { - "type": "read", - "name": "http_connection_manager", - "config": { - "codec_type": "auto", - "stat_prefix": "ingress_http", - "route_config": { - "virtual_hosts": [ - { - "name": "backend", - "domains": ["*"], - "routes": [ - { - "prefix": "/", - "cluster": "backend_service" - } - ] - } - ] - }, - "access_log": [ - { - "path": "/dev/null" - } - ], - "filters": [ - { - "type": "decoder", - "name": "istio_authn", - "config": {} - }, - { - "type": "decoder", - "name": "router", - "config": {} - } - ] - } - } - ] - } - ], - "admin": { - "access_log_path": "/dev/null", - "address": "tcp://{{ ip_loopback_address }}:0" - }, - "cluster_manager": { - "clusters": [ - { - "name": "backend_service", - "connect_timeout_ms": 5000, - "type": "static", - "lb_type": "round_robin", - "hosts": [ - { - "url": "tcp://{{ ip_loopback_address }}:{{ upstream_0 }}" - } - ] - }, - { - "name": "example_issuer", - "connect_timeout_ms": 5000, - "type": "static", - "circuit_breakers": { - "default": { - "max_pending_requests": 10000, - "max_requests": 10000 - } - }, - "lb_type": "round_robin", - "hosts": [ - { - "url": "tcp://{{ ip_loopback_address }}:{{ upstream_1 }}" - } - ] - } - ] - } -} diff --git a/src/envoy/http/authn/testdata/envoy_jwt_with_output_header_location.conf b/src/envoy/http/authn/testdata/envoy_jwt_with_output_header_location.conf deleted file mode 100644 index 0a0c6e8cdd8..00000000000 --- a/src/envoy/http/authn/testdata/envoy_jwt_with_output_header_location.conf +++ /dev/null @@ -1,99 +0,0 @@ -{ - "listeners": [ - { - "address": "tcp://{{ ip_loopback_address }}:0", - "bind_to_port": true, - "filters": [ - { - "type": "read", - "name": "http_connection_manager", - "config": { - "codec_type": "auto", - "stat_prefix": "ingress_http", - "route_config": { - "virtual_hosts": [ - { - "name": "backend", - "domains": ["*"], - "routes": [ - { - "prefix": "/", - "cluster": "backend_service" - } - ] - } - ] - }, - "access_log": [ - { - "path": "/dev/null" - } - ], - "filters": [ - { - "type": "decoder", - "name": "istio_authn", - "config": { - "policy": { - "origins": [ - { - "jwt": { - "issuer": "issuer@foo.com", - "jwks_uri": "http://localhost:8081/" - } - } - ] - }, - "jwt_output_payload_locations": { - "issuer@foo.com": "location-to-read-jwt-result" - } - } - }, - { - "type": "decoder", - "name": "router", - "config": {} - } - ] - } - } - ] - } - ], - "admin": { - "access_log_path": "/dev/null", - "address": "tcp://{{ ip_loopback_address }}:0" - }, - "cluster_manager": { - "clusters": [ - { - "name": "backend_service", - "connect_timeout_ms": 5000, - "type": "static", - "lb_type": "round_robin", - "hosts": [ - { - "url": "tcp://{{ ip_loopback_address }}:{{ upstream_0 }}" - } - ] - }, - { - "name": "example_issuer", - "connect_timeout_ms": 5000, - "type": "static", - "circuit_breakers": { - "default": { - "max_pending_requests": 10000, - "max_requests": 10000 - } - }, - "lb_type": "round_robin", - "hosts": [ - { - "url": "tcp://{{ ip_loopback_address }}:{{ upstream_1 }}" - } - ] - } - ] - } -} diff --git a/src/envoy/http/authn/testdata/envoy_origin_jwt_authn_only.conf b/src/envoy/http/authn/testdata/envoy_origin_jwt_authn_only.conf deleted file mode 100644 index 5806fec16d2..00000000000 --- a/src/envoy/http/authn/testdata/envoy_origin_jwt_authn_only.conf +++ /dev/null @@ -1,99 +0,0 @@ -{ - "listeners": [ - { - "address": "tcp://{{ ip_loopback_address }}:0", - "bind_to_port": true, - "filters": [ - { - "type": "read", - "name": "http_connection_manager", - "config": { - "codec_type": "auto", - "stat_prefix": "ingress_http", - "route_config": { - "virtual_hosts": [ - { - "name": "backend", - "domains": ["*"], - "routes": [ - { - "prefix": "/", - "cluster": "backend_service" - } - ] - } - ] - }, - "access_log": [ - { - "path": "/dev/null" - } - ], - "filters": [ - { - "type": "decoder", - "name": "istio_authn", - "config": { - "policy": { - "origins": [ - { - "jwt": { - "issuer": "628645741881-noabiu23f5a8m8ovd8ucv698lj78vv0l@developer.gserviceaccount.com", - "jwks_uri": "http://localhost:8081/" - } - } - ] - }, - "jwt_output_payload_locations": { - "628645741881-noabiu23f5a8m8ovd8ucv698lj78vv0l@developer.gserviceaccount.com": "sec-istio-auth-userinfo" - } - } - }, - { - "type": "decoder", - "name": "router", - "config": {} - } - ] - } - } - ] - } - ], - "admin": { - "access_log_path": "/dev/null", - "address": "tcp://{{ ip_loopback_address }}:0" - }, - "cluster_manager": { - "clusters": [ - { - "name": "backend_service", - "connect_timeout_ms": 5000, - "type": "static", - "lb_type": "round_robin", - "hosts": [ - { - "url": "tcp://{{ ip_loopback_address }}:{{ upstream_0 }}" - } - ] - }, - { - "name": "example_issuer", - "connect_timeout_ms": 5000, - "type": "static", - "circuit_breakers": { - "default": { - "max_pending_requests": 10000, - "max_requests": 10000 - } - }, - "lb_type": "round_robin", - "hosts": [ - { - "url": "tcp://{{ ip_loopback_address }}:{{ upstream_1 }}" - } - ] - } - ] - } -} diff --git a/src/envoy/http/authn/testdata/envoy_peer_mtls.conf b/src/envoy/http/authn/testdata/envoy_peer_mtls.conf deleted file mode 100644 index 993de548707..00000000000 --- a/src/envoy/http/authn/testdata/envoy_peer_mtls.conf +++ /dev/null @@ -1,94 +0,0 @@ -{ - "listeners": [ - { - "address": "tcp://{{ ip_loopback_address }}:0", - "bind_to_port": true, - "filters": [ - { - "type": "read", - "name": "http_connection_manager", - "config": { - "codec_type": "auto", - "stat_prefix": "ingress_http", - "route_config": { - "virtual_hosts": [ - { - "name": "backend", - "domains": ["*"], - "routes": [ - { - "prefix": "/", - "cluster": "backend_service" - } - ] - } - ] - }, - "access_log": [ - { - "path": "/dev/null" - } - ], - "filters": [ - { - "type": "decoder", - "name": "istio_authn", - "config": { - "policy": { - "peers": [ - { - "mtls": { - } - } - ] - } - } - }, - { - "type": "decoder", - "name": "router", - "config": {} - } - ] - } - } - ] - } - ], - "admin": { - "access_log_path": "/dev/null", - "address": "tcp://{{ ip_loopback_address }}:0" - }, - "cluster_manager": { - "clusters": [ - { - "name": "backend_service", - "connect_timeout_ms": 5000, - "type": "static", - "lb_type": "round_robin", - "hosts": [ - { - "url": "tcp://{{ ip_loopback_address }}:{{ upstream_0 }}" - } - ] - }, - { - "name": "example_issuer", - "connect_timeout_ms": 5000, - "type": "static", - "circuit_breakers": { - "default": { - "max_pending_requests": 10000, - "max_requests": 10000 - } - }, - "lb_type": "round_robin", - "hosts": [ - { - "url": "tcp://{{ ip_loopback_address }}:{{ upstream_1 }}" - } - ] - } - ] - } -} diff --git a/src/envoy/http/jwt_auth/BUILD b/src/envoy/http/jwt_auth/BUILD index a0f80bf43e4..eca24ac187f 100644 --- a/src/envoy/http/jwt_auth/BUILD +++ b/src/envoy/http/jwt_auth/BUILD @@ -70,6 +70,7 @@ envoy_cc_library( deps = [ ":jwt_authenticator_lib", "@envoy//source/exe:envoy_common_lib", + "//src/envoy/utils:filter_names_lib", ], ) @@ -80,6 +81,7 @@ envoy_cc_library( deps = [ ":http_filter_lib", "@envoy//source/exe:envoy_common_lib", + "//src/envoy/utils:filter_names_lib", ], ) diff --git a/src/envoy/http/jwt_auth/http_filter.cc b/src/envoy/http/jwt_auth/http_filter.cc index 405cc611bab..40a50e9ffb5 100644 --- a/src/envoy/http/jwt_auth/http_filter.cc +++ b/src/envoy/http/jwt_auth/http_filter.cc @@ -18,6 +18,7 @@ #include "common/http/message_impl.h" #include "common/http/utility.h" #include "envoy/http/async_client.h" +#include "src/envoy/utils/filter_names.h" #include #include @@ -72,6 +73,12 @@ void JwtVerificationFilter::onDone(const JwtAuth::Status& status) { } } +void JwtVerificationFilter::savePayload(const std::string& key, + const std::string& payload) { + decoder_callbacks_->requestInfo().setDynamicMetadata( + Utils::IstioFilterName::kJwt, MessageUtil::keyValueStruct(key, payload)); +} + FilterDataStatus JwtVerificationFilter::decodeData(Buffer::Instance&, bool) { if (state_ == Calling) { return FilterDataStatus::StopIterationAndWatermark; diff --git a/src/envoy/http/jwt_auth/http_filter.h b/src/envoy/http/jwt_auth/http_filter.h index 838605faa6c..bcf8c583a54 100644 --- a/src/envoy/http/jwt_auth/http_filter.h +++ b/src/envoy/http/jwt_auth/http_filter.h @@ -47,6 +47,10 @@ class JwtVerificationFilter : public StreamDecoderFilter, // To be called when its Verify() call is completed. void onDone(const JwtAuth::Status& status) override; + // the function for JwtAuth::Authenticator::Callbacks interface. + // To be called when Jwt validation success to save payload for future use. + void savePayload(const std::string& key, const std::string& payload) override; + // The callback funcion. StreamDecoderFilterCallbacks* decoder_callbacks_; // The auth object. diff --git a/src/envoy/http/jwt_auth/http_filter_factory.cc b/src/envoy/http/jwt_auth/http_filter_factory.cc index 7c3b11fd86d..06daf039fb0 100644 --- a/src/envoy/http/jwt_auth/http_filter_factory.cc +++ b/src/envoy/http/jwt_auth/http_filter_factory.cc @@ -17,6 +17,7 @@ #include "google/protobuf/util/json_util.h" #include "src/envoy/http/jwt_auth/auth_store.h" #include "src/envoy/http/jwt_auth/http_filter.h" +#include "src/envoy/utils/filter_names.h" using ::istio::envoy::config::filter::http::jwt_auth::v2alpha1:: JwtAuthentication; @@ -46,7 +47,7 @@ class JwtVerificationFilterConfig : public NamedHttpFilterConfigFactory { return ProtobufTypes::MessagePtr{new JwtAuthentication}; } - std::string name() override { return "jwt-auth"; } + std::string name() override { return Utils::IstioFilterName::kJwt; } private: Http::FilterFactoryCb createFilter(const JwtAuthentication& proto_config, diff --git a/src/envoy/http/jwt_auth/integration_test/http_filter_integration_test.cc b/src/envoy/http/jwt_auth/integration_test/http_filter_integration_test.cc index a5ae802d76d..4a4d223faa4 100644 --- a/src/envoy/http/jwt_auth/integration_test/http_filter_integration_test.cc +++ b/src/envoy/http/jwt_auth/integration_test/http_filter_integration_test.cc @@ -260,11 +260,13 @@ TEST_P(JwtVerificationFilterIntegrationTestWithJwks, RSASuccess1) { "xfP590ACPyXrivtsxg"; auto expected_headers = BaseRequestHeaders(); - expected_headers.addCopy( - kJwtVerificationResultHeaderKey, - "eyJpc3MiOiJodHRwczovL2V4YW1wbGUuY29tIiwic3ViIjoidGVz" - "dEBleGFtcGxlLmNvbSIsImF1ZCI6ImV4YW1wbGVfc2VydmljZSIs" - "ImV4cCI6MjAwMTAwMTAwMX0"); + // TODO: JWT payload is not longer output to header. Find way to verify that + // data equivalent to this is added to dynamicMetadata. + // expected_headers.addCopy( + // kJwtVerificationResultHeaderKey, + // "eyJpc3MiOiJodHRwczovL2V4YW1wbGUuY29tIiwic3ViIjoidGVz" + // "dEBleGFtcGxlLmNvbSIsImF1ZCI6ImV4YW1wbGVfc2VydmljZSIs" + // "ImV4cCI6MjAwMTAwMTAwMX0"); TestVerification(createHeaders(kJwtNoKid), "", createIssuerHeaders(), kPublicKeyRSA, true, expected_headers, ""); @@ -283,11 +285,13 @@ TEST_P(JwtVerificationFilterIntegrationTestWithJwks, ES256Success1) { "T9ubWvRvNGGYOTuJ8T17Db68Qk3T8UNTK5lzfR_mw"; auto expected_headers = BaseRequestHeaders(); - expected_headers.addCopy(kJwtVerificationResultHeaderKey, - "eyJpc3MiOiJo" - "dHRwczovL2V4YW1wbGUuY29tIiwic3ViIjoidGVzdEBleGFtc" - "GxlLmNvbSIsImV4cCI6MjAwMTAwMTAwMSwiYXVkIjoiZXhhbX" - "BsZV9zZXJ2aWNlIn0"); + // TODO: JWT payload is not longer output to header. Find way to verify that + // data equivalent to this is added to dynamicMetadata. + // expected_headers.addCopy(kJwtVerificationResultHeaderKey, + // "eyJpc3MiOiJo" + // "dHRwczovL2V4YW1wbGUuY29tIiwic3ViIjoidGVzdEBleGFtc" + // "GxlLmNvbSIsImV4cCI6MjAwMTAwMTAwMSwiYXVkIjoiZXhhbX" + // "BsZV9zZXJ2aWNlIn0"); TestVerification(createHeaders(kJwtEC), "", createIssuerHeaders(), kPublicKeyEC, true, expected_headers, ""); @@ -380,11 +384,6 @@ TEST_P(JwtVerificationFilterIntegrationTestWithInjectedJwtResult, request_stream_backend->waitForEndStream(*dispatcher_); EXPECT_TRUE(request_stream_backend->complete()); - // With sanitization, the headers received by the backend should not - // contain the injected JWT verification header. - EXPECT_TRUE(request_stream_backend->headers().get( - kJwtVerificationResultHeaderKey) == nullptr); - response->waitForEndStream(); codec_client->close(); fake_upstream_connection_backend->close(); diff --git a/src/envoy/http/jwt_auth/jwt_authenticator.cc b/src/envoy/http/jwt_auth/jwt_authenticator.cc index 56b21356136..c47b8b6d036 100644 --- a/src/envoy/http/jwt_auth/jwt_authenticator.cc +++ b/src/envoy/http/jwt_auth/jwt_authenticator.cc @@ -59,16 +59,6 @@ void JwtAuthenticator::Verify(HeaderMap& headers, headers_ = &headers; callback_ = callback; - // Sanitize the JWT verification result in the HTTP headers - for (const auto& rule : store_.config().rules()) { - if (!rule.forward_payload_header().empty()) { - ENVOY_LOG(debug, "Sanitize JWT authentication output header {}", - rule.forward_payload_header()); - const LowerCaseString key(rule.forward_payload_header()); - headers.remove(key); - } - } - ENVOY_LOG(debug, "Jwt authentication starts"); std::vector> tokens; store_.token_extractor().Extract(headers, &tokens); @@ -205,11 +195,11 @@ void JwtAuthenticator::VerifyKey(const PubkeyCacheItem& issuer_item) { return; } - if (!issuer_item.jwt_config().forward_payload_header().empty()) { - const LowerCaseString key( - issuer_item.jwt_config().forward_payload_header()); - headers_->addCopy(key, jwt_->PayloadStrBase64Url()); - } + // TODO: can we save as proto or json object directly? + // User the issuer as the entry key for simplicity. The forward_payload_header + // field can be removed or replace by a boolean (to make `save` is + // conditional) + callback_->savePayload(issuer_item.jwt_config().issuer(), jwt_->PayloadStr()); if (!issuer_item.jwt_config().forward()) { // Remove JWT from headers. diff --git a/src/envoy/http/jwt_auth/jwt_authenticator.h b/src/envoy/http/jwt_auth/jwt_authenticator.h index 748b5752799..2796cf1ce01 100644 --- a/src/envoy/http/jwt_auth/jwt_authenticator.h +++ b/src/envoy/http/jwt_auth/jwt_authenticator.h @@ -36,6 +36,8 @@ class JwtAuthenticator : public Logger::Loggable, public: virtual ~Callbacks() {} virtual void onDone(const Status& status) PURE; + virtual void savePayload(const std::string& key, + const std::string& payload) PURE; }; void Verify(HeaderMap& headers, Callbacks* callback); diff --git a/src/envoy/http/jwt_auth/jwt_authenticator_test.cc b/src/envoy/http/jwt_auth/jwt_authenticator_test.cc index ae090943251..e3acc734f89 100644 --- a/src/envoy/http/jwt_auth/jwt_authenticator_test.cc +++ b/src/envoy/http/jwt_auth/jwt_authenticator_test.cc @@ -88,8 +88,8 @@ const std::string kPublicKey = " \"kid\": \"b3319a147514df7ee5e4bcdee51350cc890cc89e\"" "}]}"; -// Keep this same as forward_payload_header field in the config below. -const char kOutputHeadersKey[] = "test-output"; +// Keep this same as issuer field in the config below. +const char kJwtIssuer[] = "https://example.com"; // A good JSON config. const char kExampleConfig[] = R"( { @@ -216,6 +216,12 @@ const std::string kGoodToken = "EprqSZUzi_ZzzYzqBNVhIJujcNWij7JRra2sXXiSAfKjtxHQoxrX8n4V1ySWJ3_1T" "H_cJcdfS_RKP7YgXRWC0L16PNF5K7iqRqmjKALNe83ZFnFIw"; +// Payload output for kGoodToken. +const std::string kGoodTokenPayload = + "{\"iss\":\"https://" + "example.com\",\"sub\":\"test@example.com\",\"exp\":2001001001," + "\"aud\":\"example_service\"}"; + // Payload: // {"iss":"https://example.com","sub":"test@example.com","aud":"http://example_service/","exp":2001001001} const std::string kGoodTokenAudHasProtocolScheme = @@ -228,6 +234,12 @@ const std::string kGoodTokenAudHasProtocolScheme = "EpfDR3lAXSaug1NE22zX_tm0d9JnC5ZrIk3kwmPJPrnAS2_9RKTQW2e2skpAT8dUV" "T5aSpQxJmWIkyp4PKWmH6h4H2INS7hWyASZdX4oW-R0PMy3FAd8D6Y8740A"; +// Payload output for kGoodTokenAudHasProtocolScheme. +const std::string kGoodTokenAudHasProtocolSchemePayload = + "{\"iss\":\"https://" + "example.com\",\"sub\":\"test@example.com\",\"exp\":2001001001," + "\"aud\":\"http://example_service/\"}"; + // Payload: // {"iss":"https://example.com","sub":"test@example.com","aud":"https://example_service1/","exp":2001001001} const std::string kGoodTokenAudService1 = @@ -240,6 +252,12 @@ const std::string kGoodTokenAudService1 = "nzvwse8DINafa5kOhBmQcrIADiOyTVC1IqcOvaftVcS4MTkTeCyzfsqcNQ-VeNPKY" "3e6wTe9brxbii-IPZFNY-1osQNnfCtYpEDjfvMjwHTielF-b55xq_tUwuqaaQ"; +// Payload output for kGoodTokenAudService1. +const std::string kGoodTokenAudService1Payload = + "{\"iss\":\"https://" + "example.com\",\"sub\":\"test@example.com\",\"exp\":2001001001," + "\"aud\":\"https://example_service1/\"}"; + // Payload: // {"iss":"https://example.com","sub":"test@example.com","aud":"http://example_service2","exp":2001001001} const std::string kGoodTokenAudService2 = @@ -252,11 +270,18 @@ const std::string kGoodTokenAudService2 = "Thwt7f3DTmHOMBeO_3xrLOOZgNtuXipqupkp9sb-DcCRdSokoFpGSTibvV_8RwkQo" "W2fdqw_ZD7WOe4sTcK27Uma9exclisHVxzJJbQOW82WdPQGicYaR_EajYzA"; +// Payload output for kGoodTokenAudService2. +const std::string kGoodTokenAudService2Payload = + "{\"iss\":\"https://" + "example.com\",\"sub\":\"test@example.com\",\"exp\":2001001001," + "\"aud\":\"http://example_service2\"}"; } // namespace class MockJwtAuthenticatorCallbacks : public JwtAuthenticator::Callbacks { public: MOCK_METHOD1(onDone, void(const Status &status)); + MOCK_METHOD2(savePayload, + void(const std::string &key, const std::string &payload)); }; class JwtAuthenticatorTest : public ::testing::Test { @@ -318,13 +343,9 @@ TEST_F(JwtAuthenticatorTest, TestOkJWTandCache) { EXPECT_CALL(mock_cb, onDone(_)).WillOnce(Invoke([](const Status &status) { ASSERT_EQ(status, Status::OK); })); - + EXPECT_CALL(mock_cb, savePayload(kJwtIssuer, kGoodTokenPayload)); auth_->Verify(headers, &mock_cb); - EXPECT_EQ(headers.get_(kOutputHeadersKey), - "eyJpc3MiOiJodHRwczovL2V4YW1wbGUuY29tIiwic3ViIjoidGVzdEBleGFtcG" - "xlLmNvbSIsImV4cCI6MjAwMTAwMTAwMSwiYXVkIjoiZXhhbXBsZV9zZXJ2" - "aWNlIn0"); // Verify the token is removed. EXPECT_FALSE(headers.Authorization()); } @@ -349,13 +370,10 @@ TEST_F(JwtAuthenticatorTest, TestOkJWTPubkeyNoAlg) { EXPECT_CALL(mock_cb, onDone(_)).WillOnce(Invoke([](const Status &status) { ASSERT_EQ(status, Status::OK); })); + EXPECT_CALL(mock_cb, savePayload(kJwtIssuer, kGoodTokenPayload)); auth_->Verify(headers, &mock_cb); - EXPECT_EQ(headers.get_(kOutputHeadersKey), - "eyJpc3MiOiJodHRwczovL2V4YW1wbGUuY29tIiwic3ViIjoidGVzdEBleGFtcG" - "xlLmNvbSIsImV4cCI6MjAwMTAwMTAwMSwiYXVkIjoiZXhhbXBsZV9zZXJ2" - "aWNlIn0"); // Verify the token is removed. EXPECT_FALSE(headers.Authorization()); @@ -382,13 +400,10 @@ TEST_F(JwtAuthenticatorTest, TestOkJWTPubkeyNoKid) { EXPECT_CALL(mock_cb, onDone(_)).WillOnce(Invoke([](const Status &status) { ASSERT_EQ(status, Status::OK); })); + EXPECT_CALL(mock_cb, savePayload(kJwtIssuer, kGoodTokenPayload)); auth_->Verify(headers, &mock_cb); - EXPECT_EQ(headers.get_(kOutputHeadersKey), - "eyJpc3MiOiJodHRwczovL2V4YW1wbGUuY29tIiwic3ViIjoidGVzdEBleGFtcG" - "xlLmNvbSIsImV4cCI6MjAwMTAwMTAwMSwiYXVkIjoiZXhhbXBsZV9zZXJ2" - "aWNlIn0"); // Verify the token is removed. EXPECT_FALSE(headers.Authorization()); @@ -408,13 +423,11 @@ TEST_F(JwtAuthenticatorTest, TestOkJWTAudService) { EXPECT_CALL(mock_cb, onDone(_)).WillOnce(Invoke([](const Status &status) { ASSERT_EQ(status, Status::OK); })); + EXPECT_CALL(mock_cb, + savePayload(kJwtIssuer, kGoodTokenAudHasProtocolSchemePayload)); auth_->Verify(headers, &mock_cb); - EXPECT_EQ(headers.get_(kOutputHeadersKey), - "eyJpc3MiOiJodHRwczovL2V4YW1wbGUuY29tIiwic3ViIjoidGVzdEBleGFtcGx" - "lLmNvbSIsImV4cCI6MjAwMTAwMTAwMSwiYXVkIjoiaHR0cDovL2V4YW1wbG" - "Vfc2VydmljZS8ifQ"); // Verify the token is removed. EXPECT_FALSE(headers.Authorization()); @@ -434,13 +447,10 @@ TEST_F(JwtAuthenticatorTest, TestOkJWTAudService1) { EXPECT_CALL(mock_cb, onDone(_)).WillOnce(Invoke([](const Status &status) { ASSERT_EQ(status, Status::OK); })); + EXPECT_CALL(mock_cb, savePayload(kJwtIssuer, kGoodTokenAudService1Payload)); auth_->Verify(headers, &mock_cb); - EXPECT_EQ(headers.get_(kOutputHeadersKey), - "eyJpc3MiOiJodHRwczovL2V4YW1wbGUuY29tIiwic3ViIjoidGVzdEBleGFtcGx" - "lLmNvbSIsImV4cCI6MjAwMTAwMTAwMSwiYXVkIjoiaHR0cHM6Ly9leGFtcG" - "xlX3NlcnZpY2UxLyJ9"); // Verify the token is removed. EXPECT_FALSE(headers.Authorization()); @@ -460,13 +470,10 @@ TEST_F(JwtAuthenticatorTest, TestOkJWTAudService2) { EXPECT_CALL(mock_cb, onDone(_)).WillOnce(Invoke([](const Status &status) { ASSERT_EQ(status, Status::OK); })); + EXPECT_CALL(mock_cb, savePayload(kJwtIssuer, kGoodTokenAudService2Payload)); auth_->Verify(headers, &mock_cb); - EXPECT_EQ(headers.get_(kOutputHeadersKey), - "eyJpc3MiOiJodHRwczovL2V4YW1wbGUuY29tIiwic3ViIjoidGVzdEBleGFtcGx" - "lLmNvbSIsImV4cCI6MjAwMTAwMTAwMSwiYXVkIjoiaHR0cDovL2V4YW1wbG" - "Vfc2VydmljZTIifQ"); // Verify the token is removed. EXPECT_FALSE(headers.Authorization()); @@ -489,6 +496,7 @@ TEST_F(JwtAuthenticatorTest, TestForwardJwt) { EXPECT_CALL(mock_cb, onDone(_)).WillOnce(Invoke([](const Status &status) { ASSERT_EQ(status, Status::OK); })); + EXPECT_CALL(mock_cb, savePayload(kJwtIssuer, kGoodTokenPayload)); auth_->Verify(headers, &mock_cb); @@ -720,7 +728,9 @@ TEST_F(JwtAuthenticatorTest, TestOnDestroy) { } TEST_F(JwtAuthenticatorTest, TestNoForwardPayloadHeader) { - // In this config, there is no forward_payload_header + // The flag (forward_payload_header) is deprecated and have no impact. The + // current behavior is always save JWT payload to request info (dynamic + // metadata). In this config, there is no forward_payload_header. SetupConfig(kExampleConfigWithoutForwardPayloadHeader); MockUpstream mock_pubkey(mock_cm_, kPublicKey); auto headers = TestHeaderMapImpl{{"Authorization", "Bearer " + kGoodToken}}; @@ -728,6 +738,8 @@ TEST_F(JwtAuthenticatorTest, TestNoForwardPayloadHeader) { EXPECT_CALL(mock_cb, onDone(_)).WillOnce(Invoke([](const Status &status) { ASSERT_EQ(status, Status::OK); })); + // Note savePayload is still being called, as explain above. + EXPECT_CALL(mock_cb, savePayload(kJwtIssuer, kGoodTokenPayload)); auth_->Verify(headers, &mock_cb); // Test when forward_payload_header is not set, nothing added to headers. @@ -752,6 +764,7 @@ TEST_F(JwtAuthenticatorTest, TestInlineJwks) { EXPECT_CALL(mock_cb, onDone(_)).WillOnce(Invoke([](const Status &status) { ASSERT_EQ(status, Status::OK); })); + EXPECT_CALL(mock_cb, savePayload(kJwtIssuer, kGoodTokenPayload)); auth_->Verify(headers, &mock_cb); EXPECT_EQ(mock_pubkey.called_count(), 0); diff --git a/src/envoy/utils/BUILD b/src/envoy/utils/BUILD index 4b30761be45..adf671a1b20 100644 --- a/src/envoy/utils/BUILD +++ b/src/envoy/utils/BUILD @@ -35,6 +35,7 @@ envoy_cc_library( "//include/istio/utils:attribute_names_header", "//src/istio/authn:context_proto", "//src/istio/utils:attribute_names_lib", + ":filter_names_lib", "@envoy//source/exe:envoy_common_lib", ], ) @@ -89,3 +90,15 @@ envoy_cc_test( "@envoy//test/test_common:utility_lib", ], ) + + +cc_library( + name = "filter_names_lib", + srcs = [ + "filter_names.cc", + ], + hdrs = [ + "filter_names.h", + ], + visibility = ["//visibility:public"], +) \ No newline at end of file diff --git a/src/envoy/utils/authn.cc b/src/envoy/utils/authn.cc index a9b03bd67df..6bb73230263 100644 --- a/src/envoy/utils/authn.cc +++ b/src/envoy/utils/authn.cc @@ -16,6 +16,7 @@ #include "src/envoy/utils/authn.h" #include "common/common/base64.h" #include "include/istio/utils/attribute_names.h" +#include "src/envoy/utils/filter_names.h" #include "src/istio/authn/context.pb.h" using istio::authn::Result; @@ -53,6 +54,7 @@ bool Authentication::SaveResultToHeader(const istio::authn::Result& result, void Authentication::SaveAuthAttributesToStruct( const istio::authn::Result& result, ::google::protobuf::Struct& data) { + // TODO(diemvu): Refactor istio::authn::Result this conversion can be removed. if (!result.principal().empty()) { setKeyValue(data, istio::utils::AttributeName::kRequestAuthPrincipal, result.principal()); @@ -103,6 +105,17 @@ bool Authentication::FetchResultFromHeader(const Http::HeaderMap& headers, return result->ParseFromString(Base64::decode(value)); } +const ProtobufWkt::Struct* Authentication::GetResultFromRequestInfo( + const RequestInfo::RequestInfo& request_info) { + const auto& metadata = request_info.dynamicMetadata(); + const auto& iter = + metadata.filter_metadata().find(Utils::IstioFilterName::kAuthentication); + if (iter == metadata.filter_metadata().end()) { + return nullptr; + } + return &(iter->second); +} + void Authentication::ClearResultInHeader(Http::HeaderMap* headers) { headers->remove(kAuthenticationOutputHeaderLocation); } diff --git a/src/envoy/utils/authn.h b/src/envoy/utils/authn.h index 395eb88a817..eae74256610 100644 --- a/src/envoy/utils/authn.h +++ b/src/envoy/utils/authn.h @@ -15,6 +15,7 @@ #include "common/common/logger.h" #include "envoy/http/header_map.h" +#include "envoy/request_info/request_info.h" #include "google/protobuf/struct.pb.h" #include "src/istio/authn/context.pb.h" @@ -40,6 +41,11 @@ class Authentication : public Logger::Loggable { static bool FetchResultFromHeader(const Http::HeaderMap& headers, istio::authn::Result* result); + // Returns a pointer to the authentication result from request info, if + // available. Otherwise, return nullptrl + static const ProtobufWkt::Struct* GetResultFromRequestInfo( + const RequestInfo::RequestInfo& request_info); + // Clears authentication result in header, if exist. static void ClearResultInHeader(Http::HeaderMap* headers); diff --git a/src/envoy/utils/filter_names.cc b/src/envoy/utils/filter_names.cc new file mode 100644 index 00000000000..2626766b5c2 --- /dev/null +++ b/src/envoy/utils/filter_names.cc @@ -0,0 +1,26 @@ +/* Copyright 2018 Istio Authors. All Rights Reserved. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#include "src/envoy/utils/filter_names.h" + +namespace Envoy { +namespace Utils { + +// TODO: using more standard naming, e.g istio.jwt, istio.authn +const char IstioFilterName::kJwt[] = "jwt-auth"; +const char IstioFilterName::kAuthentication[] = "istio_authn"; + +} // namespace Utils +} // namespace Envoy diff --git a/src/envoy/utils/filter_names.h b/src/envoy/utils/filter_names.h new file mode 100644 index 00000000000..64b79dfff2c --- /dev/null +++ b/src/envoy/utils/filter_names.h @@ -0,0 +1,32 @@ +/* Copyright 2018 Istio Authors. All Rights Reserved. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#pragma once + +#include + +namespace Envoy { +namespace Utils { + +// These are name of (Istio) filters that currently output data to +// dynamicMetadata (by convention, under the the entry using filter name itself +// as key). Define them here for easy access. +struct IstioFilterName { + static const char kJwt[]; + static const char kAuthentication[]; +}; + +} // namespace Utils +} // namespace Envoy \ No newline at end of file