diff --git a/src/envoy/http/authn/authenticator_base_test.cc b/src/envoy/http/authn/authenticator_base_test.cc index 39662318296..793fac11720 100644 --- a/src/envoy/http/authn/authenticator_base_test.cc +++ b/src/envoy/http/authn/authenticator_base_test.cc @@ -64,9 +64,10 @@ class ValidateX509Test : public testing::TestWithParam, NiceMock connection_{}; NiceMock ssl_{}; + Envoy::Http::HeaderMapImpl header_{}; FilterConfig filter_config_{}; FilterContext filter_context_{ - envoy::api::v2::core::Metadata::default_instance(), &connection_, + envoy::api::v2::core::Metadata::default_instance(), header_, &connection_, istio::envoy::config::filter::http::authn::v2alpha1::FilterConfig:: default_instance()}; @@ -168,8 +169,9 @@ class ValidateJwtTest : public testing::Test, envoy::api::v2::core::Metadata dynamic_metadata_; NiceMock connection_{}; // NiceMock ssl_{}; + Envoy::Http::HeaderMapImpl header_{}; FilterConfig filter_config_{}; - FilterContext filter_context_{dynamic_metadata_, &connection_, + FilterContext filter_context_{dynamic_metadata_, header_, &connection_, filter_config_}; MockAuthenticatorBase authenticator_{&filter_context_}; diff --git a/src/envoy/http/authn/authn_utils.cc b/src/envoy/http/authn/authn_utils.cc index 88d04327b94..4a022da8c1e 100644 --- a/src/envoy/http/authn/authn_utils.cc +++ b/src/envoy/http/authn/authn_utils.cc @@ -13,6 +13,8 @@ * limitations under the License. */ +#include + #include "authn_utils.h" #include "common/json/json_loader.h" #include "google/protobuf/struct.pb.h" @@ -98,6 +100,72 @@ bool AuthnUtils::ProcessJwtPayload(const std::string& payload_str, return true; } +bool AuthnUtils::MatchString(const char* const str, + const iaapi::StringMatch& match) { + if (str == nullptr) { + return false; + } + switch (match.match_type_case()) { + case iaapi::StringMatch::kExact: { + return match.exact().compare(str) == 0; + } + case iaapi::StringMatch::kPrefix: { + return StringUtil::startsWith(str, match.prefix()); + } + case iaapi::StringMatch::kSuffix: { + return StringUtil::endsWith(str, match.suffix()); + } + case iaapi::StringMatch::kRegex: { + return std::regex_match(str, std::regex(match.regex())); + } + default: + return false; + } +} + +static bool matchRule(const char* const path, + const iaapi::Jwt_TriggerRule& rule) { + for (const auto& excluded : rule.excluded_paths()) { + if (AuthnUtils::MatchString(path, excluded)) { + // The rule is not matched if any of excluded_paths matched. + return false; + } + } + + if (rule.included_paths_size() > 0) { + for (const auto& included : rule.included_paths()) { + if (AuthnUtils::MatchString(path, included)) { + // The rule is matched if any of included_paths matched. + return true; + } + } + + // The rule is not matched if included_paths is not empty and none of them + // matched. + return false; + } + + // The rule is matched if none of excluded_paths matched and included_paths is + // empty. + return true; +} + +bool AuthnUtils::ShouldValidateJwtPerPath(const char* const path, + const iaapi::Jwt& jwt) { + // If the path is nullptr which shouldn't happen for a HTTP request or if + // there are no trigger rules at all, then simply return true as if there're + // no per-path jwt support. + if (path == nullptr || jwt.trigger_rules_size() == 0) { + return true; + } + for (const auto& rule : jwt.trigger_rules()) { + if (matchRule(path, rule)) { + return true; + } + } + return false; +} + } // namespace AuthN } // namespace Istio } // namespace Http diff --git a/src/envoy/http/authn/authn_utils.h b/src/envoy/http/authn/authn_utils.h index 023b03e8f10..5a0f3fc4d45 100644 --- a/src/envoy/http/authn/authn_utils.h +++ b/src/envoy/http/authn/authn_utils.h @@ -15,11 +15,15 @@ #pragma once +#include "authentication/v1alpha1/policy.pb.h" #include "common/common/logger.h" +#include "common/common/utility.h" #include "envoy/http/header_map.h" #include "envoy/json/json_object.h" #include "src/istio/authn/context.pb.h" +namespace iaapi = istio::authentication::v1alpha1; + namespace Envoy { namespace Http { namespace Istio { @@ -33,6 +37,15 @@ class AuthnUtils : public Logger::Loggable { // successfully. Otherwise, return false. static bool ProcessJwtPayload(const std::string& jwt_payload_str, istio::authn::JwtPayload* payload); + + // Returns true if str is matched to match. + static bool MatchString(const char* const str, + const iaapi::StringMatch& match); + + // Returns true if the jwt should be validated. It will check if the request + // path is matched to the trigger rule in the jwt. + static bool ShouldValidateJwtPerPath(const char* const path, + const iaapi::Jwt& jwt); }; } // namespace AuthN diff --git a/src/envoy/http/authn/authn_utils_test.cc b/src/envoy/http/authn/authn_utils_test.cc index 6c79a6d73b4..019fc9d6d76 100644 --- a/src/envoy/http/authn/authn_utils_test.cc +++ b/src/envoy/http/authn/authn_utils_test.cc @@ -232,6 +232,101 @@ TEST(AuthnUtilsTest, ProcessJwtPayloadWithTwoAudTest) { EXPECT_TRUE(MessageDifferencer::Equals(expected_payload, payload)); } +TEST(AuthnUtilsTest, MatchString) { + iaapi::StringMatch match; + EXPECT_FALSE(AuthnUtils::MatchString(nullptr, match)); + EXPECT_FALSE(AuthnUtils::MatchString("", match)); + + match.set_exact("exact"); + EXPECT_TRUE(AuthnUtils::MatchString("exact", match)); + EXPECT_FALSE(AuthnUtils::MatchString("exac", match)); + EXPECT_FALSE(AuthnUtils::MatchString("exacy", match)); + + match.set_prefix("prefix"); + EXPECT_TRUE(AuthnUtils::MatchString("prefix-1", match)); + EXPECT_TRUE(AuthnUtils::MatchString("prefix", match)); + EXPECT_FALSE(AuthnUtils::MatchString("prefi", match)); + EXPECT_FALSE(AuthnUtils::MatchString("prefiy", match)); + + match.set_suffix("suffix"); + EXPECT_TRUE(AuthnUtils::MatchString("1-suffix", match)); + EXPECT_TRUE(AuthnUtils::MatchString("suffix", match)); + EXPECT_FALSE(AuthnUtils::MatchString("suffi", match)); + EXPECT_FALSE(AuthnUtils::MatchString("suffiy", match)); + + match.set_regex(".+abc.+"); + EXPECT_TRUE(AuthnUtils::MatchString("1-abc-1", match)); + EXPECT_FALSE(AuthnUtils::MatchString("1-abc", match)); + EXPECT_FALSE(AuthnUtils::MatchString("abc-1", match)); + EXPECT_FALSE(AuthnUtils::MatchString("1-ac-1", match)); +} + +TEST(AuthnUtilsTest, ShouldValidateJwtPerPathExcluded) { + iaapi::Jwt jwt; + + // Create a rule that triggers on everything except /good-x and /allow-x. + auto* rule = jwt.add_trigger_rules(); + rule->add_excluded_paths()->set_exact("/good-x"); + rule->add_excluded_paths()->set_exact("/allow-x"); + EXPECT_FALSE(AuthnUtils::ShouldValidateJwtPerPath("/good-x", jwt)); + EXPECT_FALSE(AuthnUtils::ShouldValidateJwtPerPath("/allow-x", jwt)); + EXPECT_TRUE(AuthnUtils::ShouldValidateJwtPerPath("/good-1", jwt)); + EXPECT_TRUE(AuthnUtils::ShouldValidateJwtPerPath("/allow-1", jwt)); + EXPECT_TRUE(AuthnUtils::ShouldValidateJwtPerPath("/other", jwt)); + + // Change the rule to only triggers on prefix /good and /allow. + rule->add_included_paths()->set_prefix("/good"); + rule->add_included_paths()->set_prefix("/allow"); + EXPECT_FALSE(AuthnUtils::ShouldValidateJwtPerPath("/good-x", jwt)); + EXPECT_FALSE(AuthnUtils::ShouldValidateJwtPerPath("/allow-x", jwt)); + EXPECT_TRUE(AuthnUtils::ShouldValidateJwtPerPath("/good-1", jwt)); + EXPECT_TRUE(AuthnUtils::ShouldValidateJwtPerPath("/allow-1", jwt)); + EXPECT_FALSE(AuthnUtils::ShouldValidateJwtPerPath("/other", jwt)); +} + +TEST(AuthnUtilsTest, ShouldValidateJwtPerPathIncluded) { + iaapi::Jwt jwt; + + // Create a rule that triggers on everything with prefix /good and /allow. + auto* rule = jwt.add_trigger_rules(); + rule->add_included_paths()->set_prefix("/good"); + rule->add_included_paths()->set_prefix("/allow"); + EXPECT_TRUE(AuthnUtils::ShouldValidateJwtPerPath("/good-x", jwt)); + EXPECT_TRUE(AuthnUtils::ShouldValidateJwtPerPath("/allow-x", jwt)); + EXPECT_TRUE(AuthnUtils::ShouldValidateJwtPerPath("/good-2", jwt)); + EXPECT_TRUE(AuthnUtils::ShouldValidateJwtPerPath("/allow-1", jwt)); + EXPECT_FALSE(AuthnUtils::ShouldValidateJwtPerPath("/other", jwt)); + + // Change the rule to also exclude /allow-x and /good-x. + rule->add_excluded_paths()->set_exact("/good-x"); + rule->add_excluded_paths()->set_exact("/allow-x"); + EXPECT_FALSE(AuthnUtils::ShouldValidateJwtPerPath("/good-x", jwt)); + EXPECT_FALSE(AuthnUtils::ShouldValidateJwtPerPath("/allow-x", jwt)); + EXPECT_TRUE(AuthnUtils::ShouldValidateJwtPerPath("/good-2", jwt)); + EXPECT_TRUE(AuthnUtils::ShouldValidateJwtPerPath("/allow-1", jwt)); + EXPECT_FALSE(AuthnUtils::ShouldValidateJwtPerPath("/other", jwt)); +} + +TEST(AuthnUtilsTest, ShouldValidateJwtPerPathDefault) { + iaapi::Jwt jwt; + + // Always trigger when path is unavailable. + EXPECT_TRUE(AuthnUtils::ShouldValidateJwtPerPath(nullptr, jwt)); + + // Always trigger when there is no rules in jwt. + EXPECT_TRUE(AuthnUtils::ShouldValidateJwtPerPath("/test", jwt)); + + // Add a rule that triggers on everything except /hello. + jwt.add_trigger_rules()->add_excluded_paths()->set_exact("/hello"); + EXPECT_FALSE(AuthnUtils::ShouldValidateJwtPerPath("/hello", jwt)); + EXPECT_TRUE(AuthnUtils::ShouldValidateJwtPerPath("/other", jwt)); + + // Add another rule that triggers on path /hello. + jwt.add_trigger_rules()->add_included_paths()->set_exact("/hello"); + EXPECT_TRUE(AuthnUtils::ShouldValidateJwtPerPath("/hello", jwt)); + EXPECT_TRUE(AuthnUtils::ShouldValidateJwtPerPath("/other", jwt)); +} + } // namespace } // namespace AuthN } // namespace Istio diff --git a/src/envoy/http/authn/filter_context.h b/src/envoy/http/authn/filter_context.h index 0b5e060f98e..6190ce7d076 100644 --- a/src/envoy/http/authn/filter_context.h +++ b/src/envoy/http/authn/filter_context.h @@ -19,6 +19,7 @@ #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/filter.h" #include "envoy/network/connection.h" #include "src/istio/authn/context.pb.h" @@ -33,10 +34,11 @@ class FilterContext : public Logger::Loggable { public: FilterContext( const envoy::api::v2::core::Metadata& dynamic_metadata, - const Network::Connection* connection, + const HeaderMap& header_map, const Network::Connection* connection, const istio::envoy::config::filter::http::authn::v2alpha1::FilterConfig& filter_config) : dynamic_metadata_(dynamic_metadata), + header_map_(header_map), connection_(connection), filter_config_(filter_config) {} virtual ~FilterContext() {} @@ -70,11 +72,17 @@ class FilterContext : public Logger::Loggable { // returns false. bool getJwtPayload(const std::string& issuer, std::string* payload) const; + const HeaderMap& headerMap() const { return header_map_; } + private: // 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_; + // Const reference to header map of the request. This provides request path + // that could be used to decide if a JWT should be used for validation. + const HeaderMap& header_map_; + // 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 6fb89866e15..96689e84897 100644 --- a/src/envoy/http/authn/filter_context_test.cc +++ b/src/envoy/http/authn/filter_context_test.cc @@ -34,8 +34,9 @@ class FilterContextTest : public testing::Test { virtual ~FilterContextTest() {} envoy::api::v2::core::Metadata metadata_; + Envoy::Http::TestHeaderMapImpl header_{}; // This test suit does not use connection, so ok to use null for it. - FilterContext filter_context_{metadata_, nullptr, + FilterContext filter_context_{metadata_, header_, 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 f1f372fa960..18a14a6f710 100644 --- a/src/envoy/http/authn/http_filter.cc +++ b/src/envoy/http/authn/http_filter.cc @@ -42,13 +42,14 @@ void AuthenticationFilter::onDestroy() { ENVOY_LOG(debug, "Called AuthenticationFilter : {}", __func__); } -FilterHeadersStatus AuthenticationFilter::decodeHeaders(HeaderMap&, bool) { +FilterHeadersStatus AuthenticationFilter::decodeHeaders(HeaderMap& headers, + bool) { ENVOY_LOG(debug, "AuthenticationFilter::decodeHeaders with config\n{}", filter_config_.DebugString()); state_ = State::PROCESSING; filter_context_.reset(new Istio::AuthN::FilterContext( - decoder_callbacks_->requestInfo().dynamicMetadata(), + decoder_callbacks_->requestInfo().dynamicMetadata(), headers, decoder_callbacks_->connection(), filter_config_)); Payload payload; diff --git a/src/envoy/http/authn/origin_authenticator.cc b/src/envoy/http/authn/origin_authenticator.cc index dca6d48af69..f5bb7a7cfac 100644 --- a/src/envoy/http/authn/origin_authenticator.cc +++ b/src/envoy/http/authn/origin_authenticator.cc @@ -15,6 +15,7 @@ #include "src/envoy/http/authn/origin_authenticator.h" #include "authentication/v1alpha1/policy.pb.h" +#include "src/envoy/http/authn/authn_utils.h" using istio::authn::Payload; @@ -57,10 +58,30 @@ bool OriginAuthenticator::run(Payload* payload) { } } + bool triggered = false; + const char* request_path = nullptr; + if (filter_context()->headerMap().Path() != nullptr) { + request_path = filter_context()->headerMap().Path()->value().c_str(); + ENVOY_LOG(debug, "Got request path {}", request_path); + } else { + ENVOY_LOG(error, + "Failed to get request path, JWT will always be used for " + "validation"); + } + for (const auto& method : policy_.origins()) { - if (validateJwt(method.jwt(), payload)) { - success = true; - break; + const auto& jwt = method.jwt(); + + if (AuthnUtils::ShouldValidateJwtPerPath(request_path, jwt)) { + ENVOY_LOG(debug, "Validating request path {} for jwt {}", request_path, + jwt.DebugString()); + // set triggered to true if any of the jwt trigger rule matched. + triggered = true; + if (validateJwt(jwt, payload)) { + ENVOY_LOG(debug, "JWT validation succeeded"); + success = true; + break; + } } } @@ -69,7 +90,9 @@ bool OriginAuthenticator::run(Payload* payload) { filter_context()->setPrincipal(policy_.principal_binding()); } - return success; + // If none of the JWT triggered, origin authentication will be ignored, as if + // it is not defined. + return !triggered || success; } } // namespace AuthN diff --git a/src/envoy/http/authn/origin_authenticator_test.cc b/src/envoy/http/authn/origin_authenticator_test.cc index a2a2cd7e858..dc3e88bff94 100644 --- a/src/envoy/http/authn/origin_authenticator_test.cc +++ b/src/envoy/http/authn/origin_authenticator_test.cc @@ -78,6 +78,54 @@ const char kPeerBinding[] = R"( } )"; +const char kSingleOriginMethodWithTriggerRulePolicy[] = R"( + principal_binding: USE_ORIGIN + origins { + jwt { + issuer: "abc.xyz" + trigger_rules: { + included_paths: { + exact: "/allow" + } + } + } + } +)"; + +const char kMultipleOriginMethodWithTriggerRulePolicy[] = R"( + principal_binding: USE_ORIGIN + origins { + jwt { + issuer: "one" + trigger_rules: { + excluded_paths: { + exact: "/bad" + } + } + } + } + origins { + jwt { + issuer: "two" + trigger_rules: { + included_paths: { + exact: "/two" + } + } + } + } + origins { + jwt { + issuer: "three" + trigger_rules: { + included_paths: { + exact: "/allow" + } + } + } + } +)"; + class MockOriginAuthenticator : public OriginAuthenticator { public: MockOriginAuthenticator(FilterContext* filter_context, @@ -121,8 +169,9 @@ class OriginAuthenticatorTest : public testing::TestWithParam { protected: std::unique_ptr> authenticator_; // envoy::api::v2::core::Metadata metadata_; + Envoy::Http::TestHeaderMapImpl header_{}; FilterContext filter_context_{ - envoy::api::v2::core::Metadata::default_instance(), nullptr, + envoy::api::v2::core::Metadata::default_instance(), header_, nullptr, istio::envoy::config::filter::http::authn::v2alpha1::FilterConfig:: default_instance()}; iaapi::Policy policy_; @@ -143,6 +192,11 @@ class OriginAuthenticatorTest : public testing::TestWithParam { // Indicates peer is set in the authN result before running. This is set from // test GetParam() bool set_peer_; + + void setPath(const std::string& path) { + header_.removePath(); + header_.addCopy(":path", path); + } }; TEST_P(OriginAuthenticatorTest, Empty) { @@ -185,6 +239,51 @@ TEST_P(OriginAuthenticatorTest, SingleMethodFail) { filter_context_.authenticationResult())); } +TEST_P(OriginAuthenticatorTest, TriggeredWithNullPath) { + ASSERT_TRUE(Protobuf::TextFormat::ParseFromString( + kSingleOriginMethodWithTriggerRulePolicy, &policy_)); + + createAuthenticator(); + + EXPECT_CALL(*authenticator_, validateJwt(_, _)) + .Times(1) + .WillOnce(DoAll(SetArgPointee<1>(jwt_payload_), Return(true))); + + EXPECT_TRUE(authenticator_->run(payload_)); + EXPECT_TRUE(TestUtility::protoEqual(expected_result_when_pass_, + filter_context_.authenticationResult())); +} + +TEST_P(OriginAuthenticatorTest, SingleRuleTriggered) { + ASSERT_TRUE(Protobuf::TextFormat::ParseFromString( + kSingleOriginMethodWithTriggerRulePolicy, &policy_)); + + createAuthenticator(); + + EXPECT_CALL(*authenticator_, validateJwt(_, _)) + .Times(1) + .WillOnce(DoAll(SetArgPointee<1>(jwt_payload_), Return(true))); + + setPath("/allow"); + EXPECT_TRUE(authenticator_->run(payload_)); + EXPECT_TRUE(TestUtility::protoEqual(expected_result_when_pass_, + filter_context_.authenticationResult())); +} + +TEST_P(OriginAuthenticatorTest, SingleRuleNotTriggered) { + ASSERT_TRUE(Protobuf::TextFormat::ParseFromString( + kSingleOriginMethodWithTriggerRulePolicy, &policy_)); + + createAuthenticator(); + + EXPECT_CALL(*authenticator_, validateJwt(_, _)).Times(0); + + setPath("/bad"); + EXPECT_TRUE(authenticator_->run(payload_)); + EXPECT_TRUE(TestUtility::protoEqual(initial_result_, + filter_context_.authenticationResult())); +} + TEST_P(OriginAuthenticatorTest, Multiple) { ASSERT_TRUE(Protobuf::TextFormat::ParseFromString( kMultipleOriginMethodsPolicy, &policy_)); @@ -219,6 +318,54 @@ TEST_P(OriginAuthenticatorTest, MultipleFail) { filter_context_.authenticationResult())); } +TEST_P(OriginAuthenticatorTest, MultipleRuleTriggeredValidationSucceeded) { + ASSERT_TRUE(Protobuf::TextFormat::ParseFromString( + kMultipleOriginMethodWithTriggerRulePolicy, &policy_)); + + createAuthenticator(); + // First method triggered but failed, second method not triggered, third + // method triggered and succeeded. + EXPECT_CALL(*authenticator_, validateJwt(_, _)) + .Times(2) + .WillOnce(DoAll(SetArgPointee<1>(jwt_extra_payload_), Return(false))) + .WillOnce(DoAll(SetArgPointee<1>(jwt_payload_), Return(true))); + + setPath("/allow"); + EXPECT_TRUE(authenticator_->run(payload_)); + EXPECT_TRUE(TestUtility::protoEqual(expected_result_when_pass_, + filter_context_.authenticationResult())); +} + +TEST_P(OriginAuthenticatorTest, MultipleRuleTriggeredValidationFailed) { + ASSERT_TRUE(Protobuf::TextFormat::ParseFromString( + kMultipleOriginMethodWithTriggerRulePolicy, &policy_)); + + createAuthenticator(); + // Triggered on first and second method but all failed. + EXPECT_CALL(*authenticator_, validateJwt(_, _)) + .Times(2) + .WillRepeatedly( + DoAll(SetArgPointee<1>(jwt_extra_payload_), Return(false))); + + setPath("/two"); + EXPECT_FALSE(authenticator_->run(payload_)); + EXPECT_TRUE(TestUtility::protoEqual(initial_result_, + filter_context_.authenticationResult())); +} + +TEST_P(OriginAuthenticatorTest, MultipleRuleNotTriggered) { + ASSERT_TRUE(Protobuf::TextFormat::ParseFromString( + kMultipleOriginMethodWithTriggerRulePolicy, &policy_)); + + createAuthenticator(); + EXPECT_CALL(*authenticator_, validateJwt(_, _)).Times(0); + + setPath("/bad"); + EXPECT_TRUE(authenticator_->run(payload_)); + EXPECT_TRUE(TestUtility::protoEqual(initial_result_, + filter_context_.authenticationResult())); +} + TEST_P(OriginAuthenticatorTest, PeerBindingPass) { ASSERT_TRUE(Protobuf::TextFormat::ParseFromString(kPeerBinding, &policy_)); // Expected principal is from peer_user. diff --git a/src/envoy/http/authn/peer_authenticator_test.cc b/src/envoy/http/authn/peer_authenticator_test.cc index cce09dbdc99..ffda3a6f749 100644 --- a/src/envoy/http/authn/peer_authenticator_test.cc +++ b/src/envoy/http/authn/peer_authenticator_test.cc @@ -66,8 +66,9 @@ class PeerAuthenticatorTest : public testing::Test { protected: std::unique_ptr> authenticator_; + Envoy::Http::TestHeaderMapImpl header_; FilterContext filter_context_{ - envoy::api::v2::core::Metadata::default_instance(), nullptr, + envoy::api::v2::core::Metadata::default_instance(), header_, nullptr, istio::envoy::config::filter::http::authn::v2alpha1::FilterConfig:: default_instance()};