diff --git a/CODEOWNERS b/CODEOWNERS index 5ff830804e613..7e2d7dda5390d 100644 --- a/CODEOWNERS +++ b/CODEOWNERS @@ -108,6 +108,7 @@ extensions/filters/common/original_src @snowp @klarose /*/extensions/filters/common/fault @rshriram @alyssawilk /*/extensions/filters/http/grpc_json_transcoder @qiwzhang @lizan /*/extensions/filters/http/router @alyssawilk @mattklein123 @snowp +/*/extensions/filters/common/rbac/matchers @conqerAtapple @ggreenway @alyssawilk /*/extensions/filters/http/grpc_web @fengli79 @lizan /*/extensions/filters/http/grpc_stats @kyessenov @lizan /*/extensions/filters/common/original_src @klarose @snowp diff --git a/api/BUILD b/api/BUILD index 5f3637f0957bb..341cc48a22149 100644 --- a/api/BUILD +++ b/api/BUILD @@ -201,6 +201,7 @@ proto_library( "//envoy/extensions/quic/crypto_stream/v3:pkg", "//envoy/extensions/quic/proof_source/v3:pkg", "//envoy/extensions/rate_limit_descriptors/expr/v3:pkg", + "//envoy/extensions/rbac/matchers/upstream_ip_port/v3:pkg", "//envoy/extensions/request_id/uuid/v3:pkg", "//envoy/extensions/resource_monitors/fixed_heap/v3:pkg", "//envoy/extensions/resource_monitors/injected_resource/v3:pkg", diff --git a/api/envoy/config/rbac/v3/rbac.proto b/api/envoy/config/rbac/v3/rbac.proto index d66f9be2b4981..474f30a285633 100644 --- a/api/envoy/config/rbac/v3/rbac.proto +++ b/api/envoy/config/rbac/v3/rbac.proto @@ -3,6 +3,7 @@ syntax = "proto3"; package envoy.config.rbac.v3; import "envoy/config/core/v3/address.proto"; +import "envoy/config/core/v3/extension.proto"; import "envoy/config/route/v3/route_components.proto"; import "envoy/type/matcher/v3/metadata.proto"; import "envoy/type/matcher/v3/path.proto"; @@ -146,7 +147,7 @@ message Policy { } // Permission defines an action (or actions) that a principal can take. -// [#next-free-field: 12] +// [#next-free-field: 13] message Permission { option (udpa.annotations.versioning).previous_message_type = "envoy.config.rbac.v2.Permission"; @@ -218,6 +219,10 @@ message Permission { // Please refer to :ref:`this FAQ entry ` to learn to // setup SNI. type.matcher.v3.StringMatcher requested_server_name = 9; + + // Extension for configuring custom matchers for RBAC. + // [#extension-category: envoy.rbac.matchers] + core.v3.TypedExtensionConfig matcher = 12; } } diff --git a/api/envoy/extensions/filters/http/dynamic_forward_proxy/v3/dynamic_forward_proxy.proto b/api/envoy/extensions/filters/http/dynamic_forward_proxy/v3/dynamic_forward_proxy.proto index a5d7223b98d28..ecf2d271f952c 100644 --- a/api/envoy/extensions/filters/http/dynamic_forward_proxy/v3/dynamic_forward_proxy.proto +++ b/api/envoy/extensions/filters/http/dynamic_forward_proxy/v3/dynamic_forward_proxy.proto @@ -27,6 +27,12 @@ message FilterConfig { // `. common.dynamic_forward_proxy.v3.DnsCacheConfig dns_cache_config = 1 [(validate.rules).message = {required: true}]; + + // When this flag is set, the filter will add the resolved upstream address in the filter + // state. The state should be saved with key + // `envoy.stream.upstream_address` (See + // :repo:`upstream_address.h`). + bool save_upstream_address = 2; } // Per route Configuration for the dynamic forward proxy HTTP filter. diff --git a/api/envoy/extensions/rbac/matchers/upstream_ip_port/v3/BUILD b/api/envoy/extensions/rbac/matchers/upstream_ip_port/v3/BUILD new file mode 100644 index 0000000000000..ad2fc9a9a84fd --- /dev/null +++ b/api/envoy/extensions/rbac/matchers/upstream_ip_port/v3/BUILD @@ -0,0 +1,13 @@ +# DO NOT EDIT. This file is generated by tools/proto_format/proto_sync.py. + +load("@envoy_api//bazel:api_build_system.bzl", "api_proto_package") + +licenses(["notice"]) # Apache 2 + +api_proto_package( + deps = [ + "//envoy/config/core/v3:pkg", + "//envoy/type/v3:pkg", + "@com_github_cncf_udpa//udpa/annotations:pkg", + ], +) diff --git a/api/envoy/extensions/rbac/matchers/upstream_ip_port/v3/upstream_ip_port_matcher.proto b/api/envoy/extensions/rbac/matchers/upstream_ip_port/v3/upstream_ip_port_matcher.proto new file mode 100644 index 0000000000000..a4bdc73fa81a0 --- /dev/null +++ b/api/envoy/extensions/rbac/matchers/upstream_ip_port/v3/upstream_ip_port_matcher.proto @@ -0,0 +1,35 @@ +syntax = "proto3"; + +package envoy.extensions.rbac.matchers.upstream_ip_port.v3; + +import "envoy/config/core/v3/address.proto"; +import "envoy/type/v3/range.proto"; + +import "udpa/annotations/status.proto"; + +option java_package = "io.envoyproxy.envoy.extensions.rbac.matchers.upstream_ip_port.v3"; +option java_outer_classname = "UpstreamIpPortMatcherProto"; +option java_multiple_files = true; +option (udpa.annotations.file_status).package_version_status = ACTIVE; + +// [#protodoc-title: RBAC upstream IP and port matcher plugin] +// [#extension: envoy.rbac.matchers.upstream_ip_port] + +// This is configuration for matching upstream ip and port. +// Note that although both fields are optional, at least one of IP or port must be supplied. If only +// one is supplied the other is a wildcard match. +// This matcher requires a filter in the chain to have saved the upstream address in the +// filter state before the matcher is executed by RBAC filter. The state should be saved with key +// `envoy.stream.upstream_address` (See +// :repo:`upstream_address.h`). +// Also, See :repo:`proxy_filter.cc< +// source/extensions/filters/http/dynamic_forward_proxy/proxy_filter.cc>` for an example of a +// filter which populates the FilterState. +message UpstreamIpPortMatcher { + // A CIDR block that will be used to match the upstream IP. + // Both Ipv4 and Ipv6 ranges can be matched. + config.core.v3.CidrRange upstream_ip = 1; + + // A port range that will be used to match the upstream port. + type.v3.Int64Range upstream_port_range = 2; +} diff --git a/api/versioning/BUILD b/api/versioning/BUILD index 362c9789e9e7c..bf34f6f1eb431 100644 --- a/api/versioning/BUILD +++ b/api/versioning/BUILD @@ -153,6 +153,7 @@ proto_library( "//envoy/extensions/quic/crypto_stream/v3:pkg", "//envoy/extensions/quic/proof_source/v3:pkg", "//envoy/extensions/rate_limit_descriptors/expr/v3:pkg", + "//envoy/extensions/rbac/matchers/upstream_ip_port/v3:pkg", "//envoy/extensions/request_id/uuid/v3:pkg", "//envoy/extensions/resource_monitors/fixed_heap/v3:pkg", "//envoy/extensions/resource_monitors/injected_resource/v3:pkg", diff --git a/bazel/repository_locations.bzl b/bazel/repository_locations.bzl index 4e72e29e82d17..c27b5fa20fb46 100644 --- a/bazel/repository_locations.bzl +++ b/bazel/repository_locations.bzl @@ -877,6 +877,7 @@ REPOSITORY_LOCATIONS_SPEC = dict( "envoy.filters.network.rbac", "envoy.filters.network.wasm", "envoy.stat_sinks.wasm", + "envoy.rbac.matchers.upstream_ip_port", ], release_date = "2021-06-28", cpe = "N/A", @@ -899,6 +900,7 @@ REPOSITORY_LOCATIONS_SPEC = dict( "envoy.filters.network.rbac", "envoy.filters.network.wasm", "envoy.stat_sinks.wasm", + "envoy.rbac.matchers.upstream_ip_port", ], release_date = "2020-04-02", cpe = "N/A", diff --git a/docs/root/api-v3/config/config.rst b/docs/root/api-v3/config/config.rst index d627701ed62fc..d79d3cd79c175 100644 --- a/docs/root/api-v3/config/config.rst +++ b/docs/root/api-v3/config/config.rst @@ -32,3 +32,4 @@ Extensions quic/quic_extensions formatter/formatter contrib/contrib + rbac/matchers diff --git a/docs/root/api-v3/config/rbac/matchers.rst b/docs/root/api-v3/config/rbac/matchers.rst new file mode 100644 index 0000000000000..d32ce66750b86 --- /dev/null +++ b/docs/root/api-v3/config/rbac/matchers.rst @@ -0,0 +1,8 @@ +RBAC Matchers +============= + +.. toctree:: + :glob: + :maxdepth: 2 + + matchers/matchers diff --git a/docs/root/api-v3/config/rbac/matchers/matchers.rst b/docs/root/api-v3/config/rbac/matchers/matchers.rst new file mode 100644 index 0000000000000..ce6d5d4a20dd5 --- /dev/null +++ b/docs/root/api-v3/config/rbac/matchers/matchers.rst @@ -0,0 +1,8 @@ +RBAC Matchers +=== + +.. toctree:: + :glob: + :maxdepth: 2 + + upstream/upstream diff --git a/docs/root/api-v3/config/rbac/matchers/upstream/upstream.rst b/docs/root/api-v3/config/rbac/matchers/upstream/upstream.rst new file mode 100644 index 0000000000000..7bf4de6964520 --- /dev/null +++ b/docs/root/api-v3/config/rbac/matchers/upstream/upstream.rst @@ -0,0 +1,8 @@ +Upstream Matchers +================= + +.. toctree:: + :glob: + :maxdepth: 2 + + ../../../../extensions/rbac/matchers/upstream_ip_port/v3/upstream_ip_port_matcher.proto diff --git a/docs/root/version_history/current.rst b/docs/root/version_history/current.rst index 8a3a2d6a1f08c..12cc3b986cf1f 100644 --- a/docs/root/version_history/current.rst +++ b/docs/root/version_history/current.rst @@ -122,6 +122,7 @@ New Features * matcher: added :ref:`invert ` for inverting the match result in the metadata matcher. * overload: add a new overload action that resets streams using a lot of memory. To enable the tracking of allocated bytes in buffers that a stream is using we need to configure the minimum threshold for tracking via:ref:`buffer_factory_config `. We have an overload action ``Envoy::Server::OverloadActionNameValues::ResetStreams`` that takes advantage of the tracking to reset the most expensive stream first. * rbac: added :ref:`destination_port_range ` for matching range of destination ports. +* rbac: added :ref:`matcher` along with extension category ``extension_category_envoy.rbac.matchers`` for custom RBAC permission matchers. Added reference implementation for matchers :ref:`envoy.rbac.matchers.upstream_ip_port `. * route config: added :ref:`dynamic_metadata ` for routing based on dynamic metadata. * router: added retry options predicate extensions configured via :ref:` `. These diff --git a/source/common/stream_info/BUILD b/source/common/stream_info/BUILD index 8ed41ec327106..60f7ac62f3c02 100644 --- a/source/common/stream_info/BUILD +++ b/source/common/stream_info/BUILD @@ -49,3 +49,11 @@ envoy_cc_library( "//envoy/stream_info:uint32_accessor_interface", ], ) + +envoy_cc_library( + name = "upstream_address_lib", + hdrs = ["upstream_address.h"], + deps = [ + "//envoy/stream_info:filter_state_interface", + ], +) diff --git a/source/common/stream_info/upstream_address.h b/source/common/stream_info/upstream_address.h new file mode 100644 index 0000000000000..eb09a641ac9bf --- /dev/null +++ b/source/common/stream_info/upstream_address.h @@ -0,0 +1,24 @@ +#pragma once + +#include "envoy/network/address.h" +#include "envoy/stream_info/filter_state.h" + +#include "absl/container/flat_hash_set.h" + +namespace Envoy { +namespace StreamInfo { + +/* + * A FilterState object that wraps a network address shared pointer. + */ +class UpstreamAddress : public FilterState::Object { +public: + static const std::string& key() { + CONSTRUCT_ON_FIRST_USE(std::string, "envoy.stream.upstream_address"); + } + + Network::Address::InstanceConstSharedPtr address_; +}; + +} // namespace StreamInfo +} // namespace Envoy diff --git a/source/extensions/extensions_build_config.bzl b/source/extensions/extensions_build_config.bzl index d7447fbc82c8a..7e220b0a4a725 100644 --- a/source/extensions/extensions_build_config.bzl +++ b/source/extensions/extensions_build_config.bzl @@ -297,6 +297,12 @@ EXTENSIONS = { # "envoy.key_value.file_based": "//source/extensions/key_value/file_based:config_lib", + + # + # RBAC matchers + # + + "envoy.rbac.matchers.upstream_ip_port": "//source/extensions/filters/common/rbac/matchers:upstream_ip_port_lib", } # These can be changed to ["//visibility:public"], for downstream builds which diff --git a/source/extensions/extensions_metadata.yaml b/source/extensions/extensions_metadata.yaml index c8ad920b51e17..d8cec006bd6dd 100644 --- a/source/extensions/extensions_metadata.yaml +++ b/source/extensions/extensions_metadata.yaml @@ -704,3 +704,8 @@ envoy.key_value.file_based: - envoy.common.key_value security_posture: data_plane_agnostic status: alpha +envoy.rbac.matchers.upstream_ip_port: + categories: + - envoy.rbac.matchers + security_posture: unknown + status: alpha diff --git a/source/extensions/filters/common/rbac/BUILD b/source/extensions/filters/common/rbac/BUILD index 5d8e3712c0e2a..a49234a5111ac 100644 --- a/source/extensions/filters/common/rbac/BUILD +++ b/source/extensions/filters/common/rbac/BUILD @@ -22,13 +22,17 @@ envoy_cc_library( envoy_cc_library( name = "matchers_lib", srcs = ["matchers.cc"], - hdrs = ["matchers.h"], + hdrs = [ + "matcher_extension.h", + "matchers.h", + ], external_deps = ["abseil_optional"], deps = [ "//envoy/http:header_map_interface", "//envoy/network:connection_interface", "//source/common/common:assert_lib", "//source/common/common:matchers_lib", + "//source/common/config:utility_lib", "//source/common/http:header_utility_lib", "//source/common/network:cidr_range_lib", "//source/extensions/filters/common/expr:evaluator_lib", diff --git a/source/extensions/filters/common/rbac/engine_impl.cc b/source/extensions/filters/common/rbac/engine_impl.cc index dbac3dee1135a..eecc9d3412662 100644 --- a/source/extensions/filters/common/rbac/engine_impl.cc +++ b/source/extensions/filters/common/rbac/engine_impl.cc @@ -11,7 +11,8 @@ namespace Common { namespace RBAC { RoleBasedAccessControlEngineImpl::RoleBasedAccessControlEngineImpl( - const envoy::config::rbac::v3::RBAC& rules, const EnforcementMode mode) + const envoy::config::rbac::v3::RBAC& rules, + ProtobufMessage::ValidationVisitor& validation_visitor, const EnforcementMode mode) : action_(rules.action()), mode_(mode) { // guard expression builder by presence of a condition in policies for (const auto& policy : rules.policies()) { @@ -22,7 +23,8 @@ RoleBasedAccessControlEngineImpl::RoleBasedAccessControlEngineImpl( } for (const auto& policy : rules.policies()) { - policies_.emplace(policy.first, std::make_unique(policy.second, builder_.get())); + policies_.emplace(policy.first, std::make_unique(policy.second, builder_.get(), + validation_visitor)); } } diff --git a/source/extensions/filters/common/rbac/engine_impl.h b/source/extensions/filters/common/rbac/engine_impl.h index 237d4fd79868f..431763919f7ce 100644 --- a/source/extensions/filters/common/rbac/engine_impl.h +++ b/source/extensions/filters/common/rbac/engine_impl.h @@ -28,6 +28,7 @@ enum class EnforcementMode { Enforced, Shadow }; class RoleBasedAccessControlEngineImpl : public RoleBasedAccessControlEngine, NonCopyable { public: RoleBasedAccessControlEngineImpl(const envoy::config::rbac::v3::RBAC& rules, + ProtobufMessage::ValidationVisitor& validation_visitor, const EnforcementMode mode = EnforcementMode::Enforced); bool handleAction(const Network::Connection& connection, diff --git a/source/extensions/filters/common/rbac/matcher_extension.h b/source/extensions/filters/common/rbac/matcher_extension.h new file mode 100644 index 0000000000000..c4cd53b488a09 --- /dev/null +++ b/source/extensions/filters/common/rbac/matcher_extension.h @@ -0,0 +1,57 @@ +#pragma once + +#include "envoy/common/pure.h" +#include "envoy/config/typed_config.h" +#include "envoy/protobuf/message_validator.h" + +#include "source/extensions/filters/common/rbac/matchers.h" + +namespace Envoy { +namespace Extensions { +namespace Filters { +namespace Common { +namespace RBAC { + +// Matcher extension factory for RBAC filter. Matchers could be extended to support IP address, +// header value etc. +class MatcherExtensionFactory : public Envoy::Config::TypedFactory { +public: + /** + * Function to create Matchers from the specified config. + * @param config supplies the matcher configuration + * @return a new MatcherExtension + */ + virtual MatcherConstSharedPtr create(const Protobuf::Message& config, + ProtobufMessage::ValidationVisitor& validation_visitor) PURE; + + // @brief the category of the matcher extension type for factory registration. + std::string category() const override { return "envoy.rbac.matchers"; } +}; + +// Base RBAC matcher extension factory. This facilitates easy creation of matcher extension +// factories. The factory is templated by: +// M: Matcher extension implementation +// P: Protobuf definition of the matcher. +template +class BaseMatcherExtensionFactory : public Filters::Common::RBAC::MatcherExtensionFactory { +public: + Filters::Common::RBAC::MatcherConstSharedPtr + create(const Protobuf::Message& config, + ProtobufMessage::ValidationVisitor& validation_visitor) override { + const auto& matcher_typed_config = + MessageUtil::downcastAndValidate( + config, validation_visitor); + + const auto proto_message = MessageUtil::anyConvert

(matcher_typed_config.typed_config()); + + return std::make_shared(proto_message); + } + + ProtobufTypes::MessagePtr createEmptyConfigProto() override { return std::make_unique

(); } +}; + +} // namespace RBAC +} // namespace Common +} // namespace Filters +} // namespace Extensions +} // namespace Envoy diff --git a/source/extensions/filters/common/rbac/matchers.cc b/source/extensions/filters/common/rbac/matchers.cc index f0efb5c7d668b..1cfa006ff38d9 100644 --- a/source/extensions/filters/common/rbac/matchers.cc +++ b/source/extensions/filters/common/rbac/matchers.cc @@ -1,8 +1,10 @@ #include "source/extensions/filters/common/rbac/matchers.h" #include "envoy/config/rbac/v3/rbac.pb.h" +#include "envoy/upstream/upstream.h" -#include "source/common/common/assert.h" +#include "source/common/config/utility.h" +#include "source/extensions/filters/common/rbac/matcher_extension.h" namespace Envoy { namespace Extensions { @@ -10,12 +12,13 @@ namespace Filters { namespace Common { namespace RBAC { -MatcherConstSharedPtr Matcher::create(const envoy::config::rbac::v3::Permission& permission) { +MatcherConstSharedPtr Matcher::create(const envoy::config::rbac::v3::Permission& permission, + ProtobufMessage::ValidationVisitor& validation_visitor) { switch (permission.rule_case()) { case envoy::config::rbac::v3::Permission::RuleCase::kAndRules: - return std::make_shared(permission.and_rules()); + return std::make_shared(permission.and_rules(), validation_visitor); case envoy::config::rbac::v3::Permission::RuleCase::kOrRules: - return std::make_shared(permission.or_rules()); + return std::make_shared(permission.or_rules(), validation_visitor); case envoy::config::rbac::v3::Permission::RuleCase::kHeader: return std::make_shared(permission.header()); case envoy::config::rbac::v3::Permission::RuleCase::kDestinationIp: @@ -30,11 +33,16 @@ MatcherConstSharedPtr Matcher::create(const envoy::config::rbac::v3::Permission& case envoy::config::rbac::v3::Permission::RuleCase::kMetadata: return std::make_shared(permission.metadata()); case envoy::config::rbac::v3::Permission::RuleCase::kNotRule: - return std::make_shared(permission.not_rule()); + return std::make_shared(permission.not_rule(), validation_visitor); case envoy::config::rbac::v3::Permission::RuleCase::kRequestedServerName: return std::make_shared(permission.requested_server_name()); case envoy::config::rbac::v3::Permission::RuleCase::kUrlPath: return std::make_shared(permission.url_path()); + case envoy::config::rbac::v3::Permission::RuleCase::kMatcher: { + auto& factory = + Config::Utility::getAndCheckFactory(permission.matcher()); + return factory.create(permission.matcher(), validation_visitor); + } default: NOT_REACHED_GCOVR_EXCL_LINE; } @@ -72,9 +80,10 @@ MatcherConstSharedPtr Matcher::create(const envoy::config::rbac::v3::Principal& } } -AndMatcher::AndMatcher(const envoy::config::rbac::v3::Permission::Set& set) { +AndMatcher::AndMatcher(const envoy::config::rbac::v3::Permission::Set& set, + ProtobufMessage::ValidationVisitor& validation_visitor) { for (const auto& rule : set.rules()) { - matchers_.push_back(Matcher::create(rule)); + matchers_.push_back(Matcher::create(rule, validation_visitor)); } } @@ -96,9 +105,10 @@ bool AndMatcher::matches(const Network::Connection& connection, return true; } -OrMatcher::OrMatcher(const Protobuf::RepeatedPtrField& rules) { +OrMatcher::OrMatcher(const Protobuf::RepeatedPtrField& rules, + ProtobufMessage::ValidationVisitor& validation_visitor) { for (const auto& rule : rules) { - matchers_.push_back(Matcher::create(rule)); + matchers_.push_back(Matcher::create(rule, validation_visitor)); } } diff --git a/source/extensions/filters/common/rbac/matchers.h b/source/extensions/filters/common/rbac/matchers.h index 5623dee2b70a9..a43dbce5c72cf 100644 --- a/source/extensions/filters/common/rbac/matchers.h +++ b/source/extensions/filters/common/rbac/matchers.h @@ -47,7 +47,8 @@ class Matcher { * Creates a shared instance of a matcher based off the rules defined in the Permission config * proto message. */ - static MatcherConstSharedPtr create(const envoy::config::rbac::v3::Permission& permission); + static MatcherConstSharedPtr create(const envoy::config::rbac::v3::Permission& permission, + ProtobufMessage::ValidationVisitor& validation_visitor); /** * Creates a shared instance of a matcher based off the rules defined in the Principal config @@ -73,7 +74,8 @@ class AlwaysMatcher : public Matcher { */ class AndMatcher : public Matcher { public: - AndMatcher(const envoy::config::rbac::v3::Permission::Set& rules); + AndMatcher(const envoy::config::rbac::v3::Permission::Set& rules, + ProtobufMessage::ValidationVisitor& validation_visitor); AndMatcher(const envoy::config::rbac::v3::Principal::Set& ids); bool matches(const Network::Connection& connection, const Envoy::Http::RequestHeaderMap& headers, @@ -89,9 +91,12 @@ class AndMatcher : public Matcher { */ class OrMatcher : public Matcher { public: - OrMatcher(const envoy::config::rbac::v3::Permission::Set& set) : OrMatcher(set.rules()) {} + OrMatcher(const envoy::config::rbac::v3::Permission::Set& set, + ProtobufMessage::ValidationVisitor& validation_visitor) + : OrMatcher(set.rules(), validation_visitor) {} OrMatcher(const envoy::config::rbac::v3::Principal::Set& set) : OrMatcher(set.ids()) {} - OrMatcher(const Protobuf::RepeatedPtrField& rules); + OrMatcher(const Protobuf::RepeatedPtrField& rules, + ProtobufMessage::ValidationVisitor& validation_visitor); OrMatcher(const Protobuf::RepeatedPtrField& ids); bool matches(const Network::Connection& connection, const Envoy::Http::RequestHeaderMap& headers, @@ -103,8 +108,9 @@ class OrMatcher : public Matcher { class NotMatcher : public Matcher { public: - NotMatcher(const envoy::config::rbac::v3::Permission& permission) - : matcher_(Matcher::create(permission)) {} + NotMatcher(const envoy::config::rbac::v3::Permission& permission, + ProtobufMessage::ValidationVisitor& validation_visitor) + : matcher_(Matcher::create(permission, validation_visitor)) {} NotMatcher(const envoy::config::rbac::v3::Principal& principal) : matcher_(Matcher::create(principal)) {} @@ -203,8 +209,9 @@ class AuthenticatedMatcher : public Matcher { */ class PolicyMatcher : public Matcher, NonCopyable { public: - PolicyMatcher(const envoy::config::rbac::v3::Policy& policy, Expr::Builder* builder) - : permissions_(policy.permissions()), principals_(policy.principals()), + PolicyMatcher(const envoy::config::rbac::v3::Policy& policy, Expr::Builder* builder, + ProtobufMessage::ValidationVisitor& validation_visitor) + : permissions_(policy.permissions(), validation_visitor), principals_(policy.principals()), condition_(policy.condition()) { if (policy.has_condition()) { expr_ = Expr::createExpression(*builder, condition_); @@ -217,7 +224,6 @@ class PolicyMatcher : public Matcher, NonCopyable { private: const OrMatcher permissions_; const OrMatcher principals_; - const google::api::expr::v1alpha1::Expr condition_; Expr::ExpressionPtr expr_; }; diff --git a/source/extensions/filters/common/rbac/matchers/BUILD b/source/extensions/filters/common/rbac/matchers/BUILD new file mode 100644 index 0000000000000..3bfe2c93635f8 --- /dev/null +++ b/source/extensions/filters/common/rbac/matchers/BUILD @@ -0,0 +1,27 @@ +load( + "//bazel:envoy_build_system.bzl", + "envoy_cc_extension", + "envoy_extension_package", +) + +licenses(["notice"]) # Apache 2 + +envoy_extension_package() + +envoy_cc_extension( + name = "upstream_ip_port_lib", + srcs = [ + "upstream_ip_port.cc", + ], + hdrs = [ + "upstream_ip_port.h", + ], + deps = [ + "//source/common/common:logger_lib", + "//source/common/stream_info:upstream_address_lib", + "//source/extensions/filters/common/rbac:matchers_lib", + "//source/extensions/filters/common/rbac:utility_lib", + "@envoy_api//envoy/config/core/v3:pkg_cc_proto", + "@envoy_api//envoy/extensions/rbac/matchers/upstream_ip_port/v3:pkg_cc_proto", + ], +) diff --git a/source/extensions/filters/common/rbac/matchers/upstream_ip_port.cc b/source/extensions/filters/common/rbac/matchers/upstream_ip_port.cc new file mode 100644 index 0000000000000..11396b754fee5 --- /dev/null +++ b/source/extensions/filters/common/rbac/matchers/upstream_ip_port.cc @@ -0,0 +1,86 @@ +#include "source/extensions/filters/common/rbac/matchers/upstream_ip_port.h" + +#include "envoy/config/core/v3/extension.pb.validate.h" +#include "envoy/registry/registry.h" + +#include "source/common/stream_info/upstream_address.h" + +namespace Envoy { +namespace Extensions { +namespace Filters { +namespace Common { +namespace RBAC { +namespace Matchers { + +using namespace Filters::Common::RBAC; + +UpstreamIpPortMatcher::UpstreamIpPortMatcher( + const envoy::extensions::rbac::matchers::upstream_ip_port::v3::UpstreamIpPortMatcher& proto) { + if (!proto.has_upstream_ip() && !proto.has_upstream_port_range()) { + throw EnvoyException( + "Invalid UpstreamIpPortMatcher configuration - missing `upstream_ip` and/or" + " `upstream_port_range`"); + } + + if (proto.has_upstream_ip()) { + cidr_ = Network::Address::CidrRange::create(proto.upstream_ip()); + } + if (proto.has_upstream_port_range()) { + port_ = proto.upstream_port_range(); + } +} + +bool UpstreamIpPortMatcher::matches(const Network::Connection&, + const Envoy::Http::RequestHeaderMap&, + const StreamInfo::StreamInfo& info) const { + + if (!info.filterState().hasDataWithName(StreamInfo::UpstreamAddress::key())) { + ENVOY_LOG_EVERY_POW_2( + warn, + "Did not find filter state with key: {}. Do you have a filter in the filter chain " + "before the RBAC filter which populates the filter state with upstream addresses ?", + StreamInfo::UpstreamAddress::key()); + + return false; + } + + const StreamInfo::UpstreamAddress& address_obj = + info.filterState().getDataReadOnly( + StreamInfo::UpstreamAddress::key()); + + if (cidr_) { + if (cidr_->isInRange(*address_obj.address_)) { + ENVOY_LOG(debug, "UpstreamIpPort matcher for cidr range: {} evaluated to: true", + cidr_->asString()); + + } else { + ENVOY_LOG(debug, "UpstreamIpPort matcher for cidr range: {} evaluated to: false", + cidr_->asString()); + return false; + } + } + + if (port_) { + const auto port = address_obj.address_->ip()->port(); + if (port >= port_->start() && port <= port_->end()) { + ENVOY_LOG(debug, "UpstreamIpPort matcher for port range: {{}, {}} evaluated to: true", + port_->start(), port_->end()); + } else { + ENVOY_LOG(debug, "UpstreamIpPort matcher for port range: {{}, {}} evaluated to: false", + port_->start(), port_->end()); + return false; + } + } + + ENVOY_LOG(trace, "UpstreamIpPort matcher evaluated to: true"); + return true; +} + +REGISTER_FACTORY(UpstreamIpPortMatcherFactory, MatcherExtensionFactory); + +} // namespace Matchers +} // namespace RBAC +} // namespace Common +} // namespace Filters +} // namespace Extensions +} // namespace Envoy diff --git a/source/extensions/filters/common/rbac/matchers/upstream_ip_port.h b/source/extensions/filters/common/rbac/matchers/upstream_ip_port.h new file mode 100644 index 0000000000000..3a01a8a7afab3 --- /dev/null +++ b/source/extensions/filters/common/rbac/matchers/upstream_ip_port.h @@ -0,0 +1,47 @@ +#pragma once + +#include "envoy/extensions/rbac/matchers/upstream_ip_port/v3/upstream_ip_port_matcher.pb.validate.h" + +#include "source/common/common/logger.h" +#include "source/common/network/cidr_range.h" +#include "source/extensions/filters/common/rbac/matcher_extension.h" + +namespace Envoy { +namespace Extensions { +namespace Filters { +namespace Common { +namespace RBAC { +namespace Matchers { + +// RBAC matcher extension for matching upstream's IP address (and port range if configured). +// configuration with the resolved upstream IP (v4 and v6). +class UpstreamIpPortMatcher : public Filters::Common::RBAC::Matcher, + public Logger::Loggable { +public: + UpstreamIpPortMatcher( + const envoy::extensions::rbac::matchers::upstream_ip_port::v3::UpstreamIpPortMatcher& proto); + + // Matcher interface. + bool matches(const Network::Connection&, const Envoy::Http::RequestHeaderMap&, + const StreamInfo::StreamInfo&) const override; + +private: + absl::optional cidr_; + absl::optional port_; +}; + +// Extension factory for UpstreamIpPortMatcher. +class UpstreamIpPortMatcherFactory + : public Filters::Common::RBAC::BaseMatcherExtensionFactory< + UpstreamIpPortMatcher, + envoy::extensions::rbac::matchers::upstream_ip_port::v3::UpstreamIpPortMatcher> { +public: + std::string name() const override { return "envoy.rbac.matchers.upstream.upstream_ip_port"; } +}; + +} // namespace Matchers +} // namespace RBAC +} // namespace Common +} // namespace Filters +} // namespace Extensions +} // namespace Envoy diff --git a/source/extensions/filters/common/rbac/utility.h b/source/extensions/filters/common/rbac/utility.h index f4acc822a36fb..b96205648d290 100644 --- a/source/extensions/filters/common/rbac/utility.h +++ b/source/extensions/filters/common/rbac/utility.h @@ -38,17 +38,21 @@ RoleBasedAccessControlFilterStats generateStats(const std::string& prefix, const std::string& shadow_prefix, Stats::Scope& scope); template -std::unique_ptr createEngine(const ConfigType& config) { +std::unique_ptr +createEngine(const ConfigType& config, ProtobufMessage::ValidationVisitor& validation_visitor) { return config.has_rules() ? std::make_unique( - config.rules(), EnforcementMode::Enforced) + config.rules(), validation_visitor, EnforcementMode::Enforced) : nullptr; } template -std::unique_ptr createShadowEngine(const ConfigType& config) { - return config.has_shadow_rules() ? std::make_unique( - config.shadow_rules(), EnforcementMode::Shadow) - : nullptr; +std::unique_ptr +createShadowEngine(const ConfigType& config, + ProtobufMessage::ValidationVisitor& validation_visitor) { + return config.has_shadow_rules() + ? std::make_unique( + config.shadow_rules(), validation_visitor, EnforcementMode::Shadow) + : nullptr; } std::string responseDetail(const std::string& policy_id); diff --git a/source/extensions/filters/http/dynamic_forward_proxy/BUILD b/source/extensions/filters/http/dynamic_forward_proxy/BUILD index ef0a6fb5c67ac..1bc118a341f8b 100644 --- a/source/extensions/filters/http/dynamic_forward_proxy/BUILD +++ b/source/extensions/filters/http/dynamic_forward_proxy/BUILD @@ -16,6 +16,7 @@ envoy_cc_library( deps = [ "//envoy/http:filter_interface", "//source/common/http:header_utility_lib", + "//source/common/stream_info:upstream_address_lib", "//source/extensions/common/dynamic_forward_proxy:dns_cache_interface", "//source/extensions/filters/http/common:pass_through_filter_lib", "@envoy_api//envoy/config/cluster/v3:pkg_cc_proto", diff --git a/source/extensions/filters/http/dynamic_forward_proxy/proxy_filter.cc b/source/extensions/filters/http/dynamic_forward_proxy/proxy_filter.cc index 9c90194f265ca..a25f157ea9eae 100644 --- a/source/extensions/filters/http/dynamic_forward_proxy/proxy_filter.cc +++ b/source/extensions/filters/http/dynamic_forward_proxy/proxy_filter.cc @@ -5,6 +5,7 @@ #include "envoy/extensions/filters/http/dynamic_forward_proxy/v3/dynamic_forward_proxy.pb.h" #include "source/common/http/utility.h" +#include "source/common/stream_info/upstream_address.h" #include "source/extensions/common/dynamic_forward_proxy/dns_cache.h" namespace Envoy { @@ -29,7 +30,8 @@ ProxyFilterConfig::ProxyFilterConfig( Upstream::ClusterManager& cluster_manager) : dns_cache_manager_(cache_manager_factory.get()), dns_cache_(dns_cache_manager_->getCache(proto_config.dns_cache_config())), - cluster_manager_(cluster_manager) {} + cluster_manager_(cluster_manager), + save_upstream_address_(proto_config.save_upstream_address()) {} ProxyPerRouteConfig::ProxyPerRouteConfig( const envoy::extensions::filters::http::dynamic_forward_proxy::v3::PerRouteConfig& config) @@ -64,6 +66,8 @@ Http::FilterHeadersStatus ProxyFilter::decodeHeaders(Http::RequestHeaderMap& hea return Http::FilterHeadersStatus::Continue; } if (cluster_type->name() != "envoy.clusters.dynamic_forward_proxy") { + ENVOY_STREAM_LOG(debug, "cluster_type->name(): {} ", *this->decoder_callbacks_, + cluster_type->name()); return Http::FilterHeadersStatus::Continue; } @@ -119,10 +123,17 @@ Http::FilterHeadersStatus ProxyFilter::decodeHeaders(Http::RequestHeaderMap& hea } switch (result.status_) { - case LoadDnsCacheEntryStatus::InCache: + case LoadDnsCacheEntryStatus::InCache: { ASSERT(cache_load_handle_ == nullptr); ENVOY_STREAM_LOG(debug, "DNS cache entry already loaded, continuing", *decoder_callbacks_); + + auto const& host = config_->cache().getHost(headers.Host()->value().getStringView()); + if (host.has_value()) { + addHostAddressToFilterState(host.value()->address()); + } + return Http::FilterHeadersStatus::Continue; + } case LoadDnsCacheEntryStatus::Loading: ASSERT(cache_load_handle_ != nullptr); ENVOY_STREAM_LOG(debug, "waiting to load DNS cache entry", *decoder_callbacks_); @@ -138,10 +149,45 @@ Http::FilterHeadersStatus ProxyFilter::decodeHeaders(Http::RequestHeaderMap& hea NOT_REACHED_GCOVR_EXCL_LINE; } -void ProxyFilter::onLoadDnsCacheComplete(const Common::DynamicForwardProxy::DnsHostInfoSharedPtr&) { - ENVOY_STREAM_LOG(debug, "load DNS cache complete, continuing", *decoder_callbacks_); +void ProxyFilter::addHostAddressToFilterState( + const Network::Address::InstanceConstSharedPtr& address) { + + if (!config_->saveUpstreamAddress()) { + return; + } + + // `onLoadDnsCacheComplete` is called by DNS cache on first resolution even if there was a + // resolution failure (null address). This check makes sure that we do not add null address to + // FilterState when this happens. + if (!address) { + ENVOY_STREAM_LOG(debug, "Cannot add address to filter state: invalid address", + *decoder_callbacks_); + return; + } + + ENVOY_STREAM_LOG(trace, "Adding resolved host {} to filter state", *decoder_callbacks_, + address->asString()); + + const Envoy::StreamInfo::FilterStateSharedPtr& filter_state = + decoder_callbacks_->streamInfo().filterState(); + + auto address_obj = std::make_unique(); + address_obj->address_ = address; + + filter_state->setData(StreamInfo::UpstreamAddress::key(), std::move(address_obj), + StreamInfo::FilterState::StateType::Mutable, + StreamInfo::FilterState::LifeSpan::Request); +} + +void ProxyFilter::onLoadDnsCacheComplete( + const Common::DynamicForwardProxy::DnsHostInfoSharedPtr& host_info) { + ENVOY_STREAM_LOG(debug, "load DNS cache complete, continuing after adding resolved host: {}", + *decoder_callbacks_, host_info->resolvedHost()); ASSERT(circuit_breaker_ != nullptr); circuit_breaker_.reset(); + + addHostAddressToFilterState(host_info->address()); + decoder_callbacks_->continueDecoding(); } diff --git a/source/extensions/filters/http/dynamic_forward_proxy/proxy_filter.h b/source/extensions/filters/http/dynamic_forward_proxy/proxy_filter.h index d97e3efeed54c..a0920e553c704 100644 --- a/source/extensions/filters/http/dynamic_forward_proxy/proxy_filter.h +++ b/source/extensions/filters/http/dynamic_forward_proxy/proxy_filter.h @@ -20,11 +20,13 @@ class ProxyFilterConfig { Extensions::Common::DynamicForwardProxy::DnsCache& cache() { return *dns_cache_; } Upstream::ClusterManager& clusterManager() { return cluster_manager_; } + bool saveUpstreamAddress() const { return save_upstream_address_; }; private: const Extensions::Common::DynamicForwardProxy::DnsCacheManagerSharedPtr dns_cache_manager_; const Extensions::Common::DynamicForwardProxy::DnsCacheSharedPtr dns_cache_; Upstream::ClusterManager& cluster_manager_; + const bool save_upstream_address_; }; using ProxyFilterConfigSharedPtr = std::shared_ptr; @@ -59,6 +61,8 @@ class ProxyFilter const Extensions::Common::DynamicForwardProxy::DnsHostInfoSharedPtr&) override; private: + void addHostAddressToFilterState(const Network::Address::InstanceConstSharedPtr& address); + const ProxyFilterConfigSharedPtr config_; Upstream::ClusterInfoConstSharedPtr cluster_info_; Upstream::ResourceAutoIncDecPtr circuit_breaker_; diff --git a/source/extensions/filters/http/rbac/BUILD b/source/extensions/filters/http/rbac/BUILD index 603f17ef8a4ff..39d9e481a1095 100644 --- a/source/extensions/filters/http/rbac/BUILD +++ b/source/extensions/filters/http/rbac/BUILD @@ -31,6 +31,7 @@ envoy_cc_library( "//source/common/http:utility_lib", "//source/extensions/filters/common/rbac:engine_lib", "//source/extensions/filters/common/rbac:utility_lib", + "//source/extensions/filters/common/rbac/matchers:upstream_ip_port_lib", "@envoy_api//envoy/extensions/filters/http/rbac/v3:pkg_cc_proto", ], ) diff --git a/source/extensions/filters/http/rbac/config.cc b/source/extensions/filters/http/rbac/config.cc index e389ee1030f13..1559328594fa0 100644 --- a/source/extensions/filters/http/rbac/config.cc +++ b/source/extensions/filters/http/rbac/config.cc @@ -15,8 +15,8 @@ Http::FilterFactoryCb RoleBasedAccessControlFilterConfigFactory::createFilterFac const envoy::extensions::filters::http::rbac::v3::RBAC& proto_config, const std::string& stats_prefix, Server::Configuration::FactoryContext& context) { - auto config = std::make_shared(proto_config, stats_prefix, - context.scope()); + auto config = std::make_shared( + proto_config, stats_prefix, context.scope(), context.messageValidationVisitor()); return [config](Http::FilterChainFactoryCallbacks& callbacks) -> void { callbacks.addStreamDecoderFilter(std::make_shared(config)); @@ -26,8 +26,9 @@ Http::FilterFactoryCb RoleBasedAccessControlFilterConfigFactory::createFilterFac Router::RouteSpecificFilterConfigConstSharedPtr RoleBasedAccessControlFilterConfigFactory::createRouteSpecificFilterConfigTyped( const envoy::extensions::filters::http::rbac::v3::RBACPerRoute& proto_config, - Server::Configuration::ServerFactoryContext&, ProtobufMessage::ValidationVisitor&) { - return std::make_shared(proto_config); + Server::Configuration::ServerFactoryContext&, ProtobufMessage::ValidationVisitor& validator) { + return std::make_shared(proto_config, + validator); } /** diff --git a/source/extensions/filters/http/rbac/rbac_filter.cc b/source/extensions/filters/http/rbac/rbac_filter.cc index da1d64d1f520a..85669c2e3a9c8 100644 --- a/source/extensions/filters/http/rbac/rbac_filter.cc +++ b/source/extensions/filters/http/rbac/rbac_filter.cc @@ -14,12 +14,13 @@ namespace RBACFilter { RoleBasedAccessControlFilterConfig::RoleBasedAccessControlFilterConfig( const envoy::extensions::filters::http::rbac::v3::RBAC& proto_config, - const std::string& stats_prefix, Stats::Scope& scope) + const std::string& stats_prefix, Stats::Scope& scope, + ProtobufMessage::ValidationVisitor& validation_visitor) : stats_(Filters::Common::RBAC::generateStats(stats_prefix, proto_config.shadow_rules_stat_prefix(), scope)), shadow_rules_stat_prefix_(proto_config.shadow_rules_stat_prefix()), - engine_(Filters::Common::RBAC::createEngine(proto_config)), - shadow_engine_(Filters::Common::RBAC::createShadowEngine(proto_config)) {} + engine_(Filters::Common::RBAC::createEngine(proto_config, validation_visitor)), + shadow_engine_(Filters::Common::RBAC::createShadowEngine(proto_config, validation_visitor)) {} const Filters::Common::RBAC::RoleBasedAccessControlEngineImpl* RoleBasedAccessControlFilterConfig::engine(const Router::RouteConstSharedPtr route, @@ -35,9 +36,15 @@ RoleBasedAccessControlFilterConfig::engine(const Router::RouteConstSharedPtr rou } RoleBasedAccessControlRouteSpecificFilterConfig::RoleBasedAccessControlRouteSpecificFilterConfig( - const envoy::extensions::filters::http::rbac::v3::RBACPerRoute& per_route_config) - : engine_(Filters::Common::RBAC::createEngine(per_route_config.rbac())), - shadow_engine_(Filters::Common::RBAC::createShadowEngine(per_route_config.rbac())) {} + const envoy::extensions::filters::http::rbac::v3::RBACPerRoute& per_route_config, + ProtobufMessage::ValidationVisitor& validation_visitor) { + // Moved from member initializer to ctor body to overcome clang false warning about memory + // leak (clang-analyzer-cplusplus.NewDeleteLeaks,-warnings-as-errors). + // Potentially https://lists.llvm.org/pipermail/llvm-bugs/2018-July/066769.html + engine_ = Filters::Common::RBAC::createEngine(per_route_config.rbac(), validation_visitor); + shadow_engine_ = + Filters::Common::RBAC::createShadowEngine(per_route_config.rbac(), validation_visitor); +} Http::FilterHeadersStatus RoleBasedAccessControlFilter::decodeHeaders(Http::RequestHeaderMap& headers, bool) { diff --git a/source/extensions/filters/http/rbac/rbac_filter.h b/source/extensions/filters/http/rbac/rbac_filter.h index 41d38a9a37c1e..a29fa532dc073 100644 --- a/source/extensions/filters/http/rbac/rbac_filter.h +++ b/source/extensions/filters/http/rbac/rbac_filter.h @@ -19,7 +19,8 @@ namespace RBACFilter { class RoleBasedAccessControlRouteSpecificFilterConfig : public Router::RouteSpecificFilterConfig { public: RoleBasedAccessControlRouteSpecificFilterConfig( - const envoy::extensions::filters::http::rbac::v3::RBACPerRoute& per_route_config); + const envoy::extensions::filters::http::rbac::v3::RBACPerRoute& per_route_config, + ProtobufMessage::ValidationVisitor& validation_visitor); const Filters::Common::RBAC::RoleBasedAccessControlEngineImpl* engine(Filters::Common::RBAC::EnforcementMode mode) const { @@ -39,7 +40,8 @@ class RoleBasedAccessControlFilterConfig { public: RoleBasedAccessControlFilterConfig( const envoy::extensions::filters::http::rbac::v3::RBAC& proto_config, - const std::string& stats_prefix, Stats::Scope& scope); + const std::string& stats_prefix, Stats::Scope& scope, + ProtobufMessage::ValidationVisitor& validation_visitor); Filters::Common::RBAC::RoleBasedAccessControlFilterStats& stats() { return stats_; } std::string shadowEffectivePolicyIdField() const { diff --git a/source/extensions/filters/network/rbac/config.cc b/source/extensions/filters/network/rbac/config.cc index b5fc803f5202e..3c95a5987956b 100644 --- a/source/extensions/filters/network/rbac/config.cc +++ b/source/extensions/filters/network/rbac/config.cc @@ -79,7 +79,8 @@ RoleBasedAccessControlNetworkFilterConfigFactory::createFilterFactoryFromProtoTy validateRbacRules(proto_config.rules()); validateRbacRules(proto_config.shadow_rules()); RoleBasedAccessControlFilterConfigSharedPtr config( - std::make_shared(proto_config, context.scope())); + std::make_shared(proto_config, context.scope(), + context.messageValidationVisitor())); return [config](Network::FilterManager& filter_manager) -> void { filter_manager.addReadFilter(std::make_shared(config)); }; diff --git a/source/extensions/filters/network/rbac/rbac_filter.cc b/source/extensions/filters/network/rbac/rbac_filter.cc index 9bd6b6723c244..6eb21b13495e2 100644 --- a/source/extensions/filters/network/rbac/rbac_filter.cc +++ b/source/extensions/filters/network/rbac/rbac_filter.cc @@ -14,12 +14,13 @@ namespace NetworkFilters { namespace RBACFilter { RoleBasedAccessControlFilterConfig::RoleBasedAccessControlFilterConfig( - const envoy::extensions::filters::network::rbac::v3::RBAC& proto_config, Stats::Scope& scope) + const envoy::extensions::filters::network::rbac::v3::RBAC& proto_config, Stats::Scope& scope, + ProtobufMessage::ValidationVisitor& validation_visitor) : stats_(Filters::Common::RBAC::generateStats(proto_config.stat_prefix(), proto_config.shadow_rules_stat_prefix(), scope)), shadow_rules_stat_prefix_(proto_config.shadow_rules_stat_prefix()), - engine_(Filters::Common::RBAC::createEngine(proto_config)), - shadow_engine_(Filters::Common::RBAC::createShadowEngine(proto_config)), + engine_(Filters::Common::RBAC::createEngine(proto_config, validation_visitor)), + shadow_engine_(Filters::Common::RBAC::createShadowEngine(proto_config, validation_visitor)), enforcement_type_(proto_config.enforcement_type()) {} Network::FilterStatus RoleBasedAccessControlFilter::onData(Buffer::Instance&, bool) { diff --git a/source/extensions/filters/network/rbac/rbac_filter.h b/source/extensions/filters/network/rbac/rbac_filter.h index d5eabcc2b4737..f43b3853b8884 100644 --- a/source/extensions/filters/network/rbac/rbac_filter.h +++ b/source/extensions/filters/network/rbac/rbac_filter.h @@ -27,7 +27,8 @@ struct Result { class RoleBasedAccessControlFilterConfig { public: RoleBasedAccessControlFilterConfig( - const envoy::extensions::filters::network::rbac::v3::RBAC& proto_config, Stats::Scope& scope); + const envoy::extensions::filters::network::rbac::v3::RBAC& proto_config, Stats::Scope& scope, + ProtobufMessage::ValidationVisitor& validation_visitor); Filters::Common::RBAC::RoleBasedAccessControlFilterStats& stats() { return stats_; } std::string shadowEffectivePolicyIdField() const { diff --git a/test/extensions/filters/common/rbac/engine_impl_test.cc b/test/extensions/filters/common/rbac/engine_impl_test.cc index 8e56a2023b4f0..24c727245f5dc 100644 --- a/test/extensions/filters/common/rbac/engine_impl_test.cc +++ b/test/extensions/filters/common/rbac/engine_impl_test.cc @@ -67,11 +67,13 @@ void onMetadata(NiceMock& info) { TEST(RoleBasedAccessControlEngineImpl, Disabled) { envoy::config::rbac::v3::RBAC rbac; rbac.set_action(envoy::config::rbac::v3::RBAC::ALLOW); - RBAC::RoleBasedAccessControlEngineImpl engine_allow(rbac); + RBAC::RoleBasedAccessControlEngineImpl engine_allow( + rbac, ProtobufMessage::getStrictValidationVisitor()); checkEngine(engine_allow, false, LogResult::Undecided); rbac.set_action(envoy::config::rbac::v3::RBAC::DENY); - RBAC::RoleBasedAccessControlEngineImpl engine_deny(rbac); + RBAC::RoleBasedAccessControlEngineImpl engine_deny(rbac, + ProtobufMessage::getStrictValidationVisitor()); checkEngine(engine_deny, true, LogResult::Undecided); } @@ -169,7 +171,8 @@ TEST(RoleBasedAccessControlEngineImpl, AllowedAllowlist) { envoy::config::rbac::v3::RBAC rbac; rbac.set_action(envoy::config::rbac::v3::RBAC::ALLOW); (*rbac.mutable_policies())["foo"] = policy; - RBAC::RoleBasedAccessControlEngineImpl engine(rbac); + RBAC::RoleBasedAccessControlEngineImpl engine(rbac, + ProtobufMessage::getStrictValidationVisitor()); Envoy::Network::MockConnection conn; Envoy::Http::TestRequestHeaderMapImpl headers; @@ -192,7 +195,8 @@ TEST(RoleBasedAccessControlEngineImpl, DeniedDenylist) { envoy::config::rbac::v3::RBAC rbac; rbac.set_action(envoy::config::rbac::v3::RBAC::DENY); (*rbac.mutable_policies())["foo"] = policy; - RBAC::RoleBasedAccessControlEngineImpl engine(rbac); + RBAC::RoleBasedAccessControlEngineImpl engine(rbac, + ProtobufMessage::getStrictValidationVisitor()); Envoy::Network::MockConnection conn; Envoy::Http::TestRequestHeaderMapImpl headers; @@ -220,7 +224,8 @@ TEST(RoleBasedAccessControlEngineImpl, BasicCondition) { envoy::config::rbac::v3::RBAC rbac; rbac.set_action(envoy::config::rbac::v3::RBAC::ALLOW); (*rbac.mutable_policies())["foo"] = policy; - RBAC::RoleBasedAccessControlEngineImpl engine(rbac); + RBAC::RoleBasedAccessControlEngineImpl engine(rbac, + ProtobufMessage::getStrictValidationVisitor()); checkEngine(engine, false, LogResult::Undecided); } @@ -241,12 +246,14 @@ TEST(RoleBasedAccessControlEngineImpl, MalformedCondition) { rbac.set_action(envoy::config::rbac::v3::RBAC::ALLOW); (*rbac.mutable_policies())["foo"] = policy; - EXPECT_THROW_WITH_REGEX(RBAC::RoleBasedAccessControlEngineImpl engine(rbac), EnvoyException, - "failed to create an expression: .*"); + EXPECT_THROW_WITH_REGEX(RBAC::RoleBasedAccessControlEngineImpl engine( + rbac, ProtobufMessage::getStrictValidationVisitor()), + EnvoyException, "failed to create an expression: .*"); rbac.set_action(envoy::config::rbac::v3::RBAC::LOG); - EXPECT_THROW_WITH_REGEX(RBAC::RoleBasedAccessControlEngineImpl engine_log(rbac), EnvoyException, - "failed to create an expression: .*"); + EXPECT_THROW_WITH_REGEX(RBAC::RoleBasedAccessControlEngineImpl engine_log( + rbac, ProtobufMessage::getStrictValidationVisitor()), + EnvoyException, "failed to create an expression: .*"); } TEST(RoleBasedAccessControlEngineImpl, MistypedCondition) { @@ -262,7 +269,8 @@ TEST(RoleBasedAccessControlEngineImpl, MistypedCondition) { envoy::config::rbac::v3::RBAC rbac; rbac.set_action(envoy::config::rbac::v3::RBAC::ALLOW); (*rbac.mutable_policies())["foo"] = policy; - RBAC::RoleBasedAccessControlEngineImpl engine(rbac); + RBAC::RoleBasedAccessControlEngineImpl engine(rbac, + ProtobufMessage::getStrictValidationVisitor()); checkEngine(engine, false, LogResult::Undecided); } @@ -282,7 +290,8 @@ TEST(RoleBasedAccessControlEngineImpl, EvaluationFailure) { envoy::config::rbac::v3::RBAC rbac; rbac.set_action(envoy::config::rbac::v3::RBAC::ALLOW); (*rbac.mutable_policies())["foo"] = policy; - RBAC::RoleBasedAccessControlEngineImpl engine(rbac); + RBAC::RoleBasedAccessControlEngineImpl engine(rbac, + ProtobufMessage::getStrictValidationVisitor()); checkEngine(engine, false, LogResult::Undecided); } @@ -307,7 +316,8 @@ TEST(RoleBasedAccessControlEngineImpl, ErrorCondition) { envoy::config::rbac::v3::RBAC rbac; rbac.set_action(envoy::config::rbac::v3::RBAC::ALLOW); (*rbac.mutable_policies())["foo"] = policy; - RBAC::RoleBasedAccessControlEngineImpl engine(rbac); + RBAC::RoleBasedAccessControlEngineImpl engine(rbac, + ProtobufMessage::getStrictValidationVisitor()); checkEngine(engine, false, LogResult::Undecided, Envoy::Network::MockConnection()); } @@ -337,7 +347,8 @@ TEST(RoleBasedAccessControlEngineImpl, HeaderCondition) { envoy::config::rbac::v3::RBAC rbac; rbac.set_action(envoy::config::rbac::v3::RBAC::ALLOW); (*rbac.mutable_policies())["foo"] = policy; - RBAC::RoleBasedAccessControlEngineImpl engine(rbac); + RBAC::RoleBasedAccessControlEngineImpl engine(rbac, + ProtobufMessage::getStrictValidationVisitor()); Envoy::Http::TestRequestHeaderMapImpl headers; Envoy::Http::LowerCaseString key("foo"); @@ -378,7 +389,8 @@ TEST(RoleBasedAccessControlEngineImpl, MetadataCondition) { envoy::config::rbac::v3::RBAC rbac; rbac.set_action(envoy::config::rbac::v3::RBAC::ALLOW); (*rbac.mutable_policies())["foo"] = policy; - RBAC::RoleBasedAccessControlEngineImpl engine(rbac); + RBAC::RoleBasedAccessControlEngineImpl engine(rbac, + ProtobufMessage::getStrictValidationVisitor()); Envoy::Http::TestRequestHeaderMapImpl headers; NiceMock info; @@ -405,7 +417,8 @@ TEST(RoleBasedAccessControlEngineImpl, ConjunctiveCondition) { envoy::config::rbac::v3::RBAC rbac; rbac.set_action(envoy::config::rbac::v3::RBAC::ALLOW); (*rbac.mutable_policies())["foo"] = policy; - RBAC::RoleBasedAccessControlEngineImpl engine(rbac); + RBAC::RoleBasedAccessControlEngineImpl engine(rbac, + ProtobufMessage::getStrictValidationVisitor()); Envoy::Network::MockConnection conn; Envoy::Http::TestRequestHeaderMapImpl headers; @@ -423,7 +436,8 @@ TEST(RoleBasedAccessControlEngineImpl, DisabledLog) { envoy::config::rbac::v3::RBAC rbac; rbac.set_action(envoy::config::rbac::v3::RBAC::LOG); - RBAC::RoleBasedAccessControlEngineImpl engine(rbac); + RBAC::RoleBasedAccessControlEngineImpl engine(rbac, + ProtobufMessage::getStrictValidationVisitor()); checkEngine(engine, true, RBAC::LogResult::No, info); } @@ -435,7 +449,8 @@ TEST(RoleBasedAccessControlEngineImpl, LogIfMatched) { envoy::config::rbac::v3::RBAC rbac; rbac.set_action(envoy::config::rbac::v3::RBAC::LOG); (*rbac.mutable_policies())["foo"] = policy; - RBAC::RoleBasedAccessControlEngineImpl engine(rbac); + RBAC::RoleBasedAccessControlEngineImpl engine(rbac, + ProtobufMessage::getStrictValidationVisitor()); Envoy::Network::MockConnection conn; Envoy::Http::TestRequestHeaderMapImpl headers; diff --git a/test/extensions/filters/common/rbac/matchers_test.cc b/test/extensions/filters/common/rbac/matchers_test.cc index 09f9f75d08863..7d96e355d9e53 100644 --- a/test/extensions/filters/common/rbac/matchers_test.cc +++ b/test/extensions/filters/common/rbac/matchers_test.cc @@ -45,7 +45,7 @@ TEST(AndMatcher, Permission_Set) { envoy::config::rbac::v3::Permission* perm = set.add_rules(); perm->set_any(true); - checkMatcher(RBAC::AndMatcher(set), true); + checkMatcher(RBAC::AndMatcher(set, ProtobufMessage::getStrictValidationVisitor()), true); perm = set.add_rules(); perm->set_destination_port(123); @@ -57,12 +57,14 @@ TEST(AndMatcher, Permission_Set) { Envoy::Network::Utility::parseInternetAddress("1.2.3.4", 123, false); info.downstream_connection_info_provider_->setLocalAddress(addr); - checkMatcher(RBAC::AndMatcher(set), true, conn, headers, info); + checkMatcher(RBAC::AndMatcher(set, ProtobufMessage::getStrictValidationVisitor()), true, conn, + headers, info); addr = Envoy::Network::Utility::parseInternetAddress("1.2.3.4", 8080, false); info.downstream_connection_info_provider_->setLocalAddress(addr); - checkMatcher(RBAC::AndMatcher(set), false, conn, headers, info); + checkMatcher(RBAC::AndMatcher(set, ProtobufMessage::getStrictValidationVisitor()), false, conn, + headers, info); } TEST(AndMatcher, Principal_Set) { @@ -104,18 +106,21 @@ TEST(OrMatcher, Permission_Set) { Envoy::Network::Utility::parseInternetAddress("1.2.3.4", 456, false); info.downstream_connection_info_provider_->setLocalAddress(addr); - checkMatcher(RBAC::OrMatcher(set), false, conn, headers, info); + checkMatcher(RBAC::OrMatcher(set, ProtobufMessage::getStrictValidationVisitor()), false, conn, + headers, info); perm = set.add_rules(); perm->mutable_destination_port_range()->set_start(123); perm->mutable_destination_port_range()->set_end(456); - checkMatcher(RBAC::OrMatcher(set), false, conn, headers, info); + checkMatcher(RBAC::OrMatcher(set, ProtobufMessage::getStrictValidationVisitor()), false, conn, + headers, info); perm = set.add_rules(); perm->set_any(true); - checkMatcher(RBAC::OrMatcher(set), true, conn, headers, info); + checkMatcher(RBAC::OrMatcher(set, ProtobufMessage::getStrictValidationVisitor()), true, conn, + headers, info); } TEST(OrMatcher, Principal_Set) { @@ -144,7 +149,8 @@ TEST(NotMatcher, Permission) { envoy::config::rbac::v3::Permission perm; perm.set_any(true); - checkMatcher(RBAC::NotMatcher(perm), false, Envoy::Network::MockConnection()); + checkMatcher(RBAC::NotMatcher(perm, ProtobufMessage::getStrictValidationVisitor()), false, + Envoy::Network::MockConnection()); } TEST(NotMatcher, Principal) { @@ -419,7 +425,7 @@ TEST(PolicyMatcher, PolicyMatcher) { policy.add_principals()->mutable_authenticated()->mutable_principal_name()->set_exact("bar"); Expr::BuilderPtr builder = Expr::createBuilder(nullptr); - RBAC::PolicyMatcher matcher(policy, builder.get()); + RBAC::PolicyMatcher matcher(policy, builder.get(), ProtobufMessage::getStrictValidationVisitor()); Envoy::Network::MockConnection conn; Envoy::Http::TestRequestHeaderMapImpl headers; diff --git a/test/extensions/filters/common/rbac/mocks.h b/test/extensions/filters/common/rbac/mocks.h index 354a1e5dab153..99503af52f4e4 100644 --- a/test/extensions/filters/common/rbac/mocks.h +++ b/test/extensions/filters/common/rbac/mocks.h @@ -2,6 +2,7 @@ #include "envoy/config/rbac/v3/rbac.pb.h" +#include "source/common/protobuf/message_validator_impl.h" #include "source/extensions/filters/common/rbac/engine_impl.h" #include "gmock/gmock.h" @@ -16,7 +17,8 @@ class MockEngine : public RoleBasedAccessControlEngineImpl { public: MockEngine(const envoy::config::rbac::v3::RBAC& rules, const EnforcementMode mode = EnforcementMode::Enforced) - : RoleBasedAccessControlEngineImpl(rules, mode){}; + : RoleBasedAccessControlEngineImpl(rules, ProtobufMessage::getStrictValidationVisitor(), + mode){}; MOCK_METHOD(bool, handleAction, (const Envoy::Network::Connection&, const Envoy::Http::RequestHeaderMap&, diff --git a/test/extensions/filters/http/dynamic_forward_proxy/proxy_filter_test.cc b/test/extensions/filters/http/dynamic_forward_proxy/proxy_filter_test.cc index 72a611f703a2a..c2150e500dd91 100644 --- a/test/extensions/filters/http/dynamic_forward_proxy/proxy_filter_test.cc +++ b/test/extensions/filters/http/dynamic_forward_proxy/proxy_filter_test.cc @@ -1,6 +1,7 @@ #include "envoy/config/cluster/v3/cluster.pb.h" #include "envoy/extensions/filters/http/dynamic_forward_proxy/v3/dynamic_forward_proxy.pb.h" +#include "source/common/stream_info/upstream_address.h" #include "source/extensions/common/dynamic_forward_proxy/dns_cache_impl.h" #include "source/extensions/filters/http/dynamic_forward_proxy/proxy_filter.h" @@ -9,7 +10,6 @@ #include "test/mocks/upstream/basic_resource_limit.h" #include "test/mocks/upstream/cluster_manager.h" #include "test/mocks/upstream/transport_socket_match.h" -#include "test/test_common/test_runtime.h" using testing::AtLeast; using testing::Eq; @@ -31,19 +31,31 @@ using MockLoadDnsCacheEntryResult = class ProxyFilterTest : public testing::Test, public Extensions::Common::DynamicForwardProxy::DnsCacheManagerFactory { public: - ProxyFilterTest() { + void SetUp() override { + setupSocketMatcher(); + setupFilter(); + setupCluster(); + } + + void setupSocketMatcher() { cm_.initializeThreadLocalClusters({"fake_cluster"}); transport_socket_match_ = new NiceMock( Network::TransportSocketFactoryPtr(transport_socket_factory_)); cm_.thread_local_cluster_.cluster_.info_->transport_socket_matcher_.reset( transport_socket_match_); + } - envoy::extensions::filters::http::dynamic_forward_proxy::v3::FilterConfig proto_config; + virtual void setupFilter() { EXPECT_CALL(*dns_cache_manager_, getCache(_)); + + envoy::extensions::filters::http::dynamic_forward_proxy::v3::FilterConfig proto_config; filter_config_ = std::make_shared(proto_config, *this, cm_); filter_ = std::make_unique(filter_config_); + filter_->setDecoderFilterCallbacks(callbacks_); + } + void setupCluster() { // Allow for an otherwise strict mock. EXPECT_CALL(callbacks_, connection()).Times(AtLeast(0)); EXPECT_CALL(callbacks_, streamId()).Times(AtLeast(0)); @@ -330,6 +342,175 @@ TEST_F(ProxyFilterTest, HostRewriteViaHeader) { filter_->onDestroy(); } +class UpstreamResolvedHostFilterStateHelper : public ProxyFilterTest { +public: + void setupFilter() override { + EXPECT_CALL(*dns_cache_manager_, getCache(_)); + + envoy::extensions::filters::http::dynamic_forward_proxy::v3::FilterConfig proto_config; + proto_config.set_save_upstream_address(true); + + filter_config_ = std::make_shared(proto_config, *this, cm_); + filter_ = std::make_unique(filter_config_); + + filter_->setDecoderFilterCallbacks(callbacks_); + } +}; + +// Tests if address set is populated in the filter state when an upstream host is resolved +// successfully. +TEST_F(UpstreamResolvedHostFilterStateHelper, AddResolvedHostFilterStateMetadata) { + Upstream::ResourceAutoIncDec* circuit_breakers_( + new Upstream::ResourceAutoIncDec(pending_requests_)); + + EXPECT_CALL(callbacks_, streamInfo()); + auto& filter_state = callbacks_.streamInfo().filterState(); + + InSequence s; + + // Setup test host + auto host_info = std::make_shared(); + host_info->address_ = Network::Utility::parseInternetAddress("1.2.3.4", 80); + + EXPECT_CALL(callbacks_, route()); + EXPECT_CALL(cm_, getThreadLocalCluster(_)); + EXPECT_CALL(*dns_cache_manager_->dns_cache_, canCreateDnsRequest_()) + .WillOnce(Return(circuit_breakers_)); + EXPECT_CALL(*transport_socket_factory_, implementsSecureTransport()).WillOnce(Return(false)); + + EXPECT_CALL(*dns_cache_manager_->dns_cache_, loadDnsCacheEntry_(Eq("foo"), 80, _)) + .WillOnce(Invoke([&](absl::string_view, uint16_t, ProxyFilter::LoadDnsCacheEntryCallbacks&) { + return MockLoadDnsCacheEntryResult{LoadDnsCacheEntryStatus::InCache, nullptr, host_info}; + })); + + EXPECT_CALL(*dns_cache_manager_->dns_cache_, getHost(_)) + .WillOnce( + Invoke([&](absl::string_view) + -> absl::optional { + return host_info; + })); + + EXPECT_CALL(*host_info, address()); + + EXPECT_CALL(callbacks_, streamInfo()); + + // Host was resolved successfully, so continue filter iteration. + EXPECT_EQ(Http::FilterHeadersStatus::Continue, filter_->decodeHeaders(request_headers_, false)); + + // We expect FilterState to be populated + EXPECT_TRUE( + filter_state->hasData(StreamInfo::UpstreamAddress::key())); + + filter_->onDestroy(); +} + +// Tests if an already existing address set in filter state is updated when upstream host is +// resolved successfully. +TEST_F(UpstreamResolvedHostFilterStateHelper, UpdateResolvedHostFilterStateMetadata) { + Upstream::ResourceAutoIncDec* circuit_breakers_( + new Upstream::ResourceAutoIncDec(pending_requests_)); + + EXPECT_CALL(callbacks_, streamInfo()); + + // Pre-populate the filter state with an address. + auto& filter_state = callbacks_.streamInfo().filterState(); + const auto pre_address = Network::Utility::parseInternetAddress("1.2.3.3", 80); + auto address_obj = std::make_unique(); + address_obj->address_ = pre_address; + filter_state->setData(StreamInfo::UpstreamAddress::key(), std::move(address_obj), + StreamInfo::FilterState::StateType::Mutable, + StreamInfo::FilterState::LifeSpan::Request); + + InSequence s; + + // Setup test host + auto host_info = std::make_shared(); + host_info->address_ = Network::Utility::parseInternetAddress("1.2.3.4", 80); + + EXPECT_CALL(callbacks_, route()); + EXPECT_CALL(cm_, getThreadLocalCluster(_)); + EXPECT_CALL(*dns_cache_manager_->dns_cache_, canCreateDnsRequest_()) + .WillOnce(Return(circuit_breakers_)); + EXPECT_CALL(*transport_socket_factory_, implementsSecureTransport()).WillOnce(Return(false)); + + EXPECT_CALL(*dns_cache_manager_->dns_cache_, loadDnsCacheEntry_(Eq("foo"), 80, _)) + .WillOnce(Invoke([&](absl::string_view, uint16_t, ProxyFilter::LoadDnsCacheEntryCallbacks&) { + return MockLoadDnsCacheEntryResult{LoadDnsCacheEntryStatus::InCache, nullptr, host_info}; + })); + + EXPECT_CALL(*dns_cache_manager_->dns_cache_, getHost(_)) + .WillOnce( + Invoke([&](absl::string_view) + -> absl::optional { + return host_info; + })); + + EXPECT_CALL(*host_info, address()); + + EXPECT_CALL(callbacks_, streamInfo()); + + // Host was resolved successfully, so continue filter iteration. + EXPECT_EQ(Http::FilterHeadersStatus::Continue, filter_->decodeHeaders(request_headers_, false)); + + // We expect FilterState to be populated + EXPECT_TRUE( + filter_state->hasData(StreamInfo::UpstreamAddress::key())); + + const StreamInfo::UpstreamAddress& updated_address_obj = + filter_state->getDataReadOnly( + StreamInfo::UpstreamAddress::key()); + + // Verify the data + EXPECT_TRUE(updated_address_obj.address_); + EXPECT_EQ(updated_address_obj.address_->asStringView(), host_info->address_->asStringView()); + + filter_->onDestroy(); +} + +// Tests if address set is populated in the filter state when an upstream host is resolved +// successfully but is null. +TEST_F(UpstreamResolvedHostFilterStateHelper, IgnoreFilterStateMetadataNullAddress) { + Upstream::ResourceAutoIncDec* circuit_breakers_( + new Upstream::ResourceAutoIncDec(pending_requests_)); + + EXPECT_CALL(callbacks_, streamInfo()); + auto& filter_state = callbacks_.streamInfo().filterState(); + + InSequence s; + + // Setup test host + auto host_info = std::make_shared(); + host_info->address_ = nullptr; + + EXPECT_CALL(callbacks_, route()); + EXPECT_CALL(cm_, getThreadLocalCluster(_)); + EXPECT_CALL(*dns_cache_manager_->dns_cache_, canCreateDnsRequest_()) + .WillOnce(Return(circuit_breakers_)); + EXPECT_CALL(*transport_socket_factory_, implementsSecureTransport()).WillOnce(Return(false)); + + EXPECT_CALL(*dns_cache_manager_->dns_cache_, loadDnsCacheEntry_(Eq("foo"), 80, _)) + .WillOnce(Invoke([&](absl::string_view, uint16_t, ProxyFilter::LoadDnsCacheEntryCallbacks&) { + return MockLoadDnsCacheEntryResult{LoadDnsCacheEntryStatus::InCache, nullptr, host_info}; + })); + + EXPECT_CALL(*dns_cache_manager_->dns_cache_, getHost(_)) + .WillOnce( + Invoke([&](absl::string_view) + -> absl::optional { + return host_info; + })); + + EXPECT_CALL(*host_info, address()); + + EXPECT_EQ(Http::FilterHeadersStatus::Continue, filter_->decodeHeaders(request_headers_, false)); + + // We do not expect FilterState to be populated + EXPECT_FALSE( + filter_state->hasData(StreamInfo::UpstreamAddress::key())); + + filter_->onDestroy(); +} + } // namespace } // namespace DynamicForwardProxy } // namespace HttpFilters diff --git a/test/extensions/filters/http/rbac/BUILD b/test/extensions/filters/http/rbac/BUILD index e37cc1971f608..0e2cf13fc8cc7 100644 --- a/test/extensions/filters/http/rbac/BUILD +++ b/test/extensions/filters/http/rbac/BUILD @@ -38,6 +38,7 @@ envoy_extension_cc_test( "//test/mocks/network:network_mocks", "@envoy_api//envoy/config/rbac/v3:pkg_cc_proto", "@envoy_api//envoy/extensions/filters/http/rbac/v3:pkg_cc_proto", + "@envoy_api//envoy/extensions/rbac/matchers/upstream_ip_port/v3:pkg_cc_proto", ], ) @@ -46,7 +47,10 @@ envoy_extension_cc_test( srcs = ["rbac_filter_integration_test.cc"], extension_names = ["envoy.filters.http.rbac"], deps = [ + "//source/extensions/clusters/dynamic_forward_proxy:cluster", + "//source/extensions/filters/http/dynamic_forward_proxy:config", "//source/extensions/filters/http/rbac:config", + "//source/extensions/key_value/file_based:config_lib", "//test/config:utility_lib", "//test/integration:http_protocol_integration_lib", "@envoy_api//envoy/extensions/filters/http/rbac/v3:pkg_cc_proto", diff --git a/test/extensions/filters/http/rbac/mocks.h b/test/extensions/filters/http/rbac/mocks.h index 7932a02fea4dc..3a079fc5758d6 100644 --- a/test/extensions/filters/http/rbac/mocks.h +++ b/test/extensions/filters/http/rbac/mocks.h @@ -2,6 +2,7 @@ #include "envoy/extensions/filters/http/rbac/v3/rbac.pb.h" +#include "source/common/protobuf/message_validator_impl.h" #include "source/extensions/filters/common/rbac/utility.h" #include "source/extensions/filters/http/rbac/rbac_filter.h" @@ -18,7 +19,8 @@ class MockRoleBasedAccessControlRouteSpecificFilterConfig public: MockRoleBasedAccessControlRouteSpecificFilterConfig( const envoy::extensions::filters::http::rbac::v3::RBACPerRoute& r) - : RoleBasedAccessControlRouteSpecificFilterConfig(r){}; + : RoleBasedAccessControlRouteSpecificFilterConfig( + r, ProtobufMessage::getStrictValidationVisitor()){}; MOCK_METHOD(Filters::Common::RBAC::RoleBasedAccessControlEngineImpl&, engine, (), (const)); }; diff --git a/test/extensions/filters/http/rbac/rbac_filter_integration_test.cc b/test/extensions/filters/http/rbac/rbac_filter_integration_test.cc index 242ad7684071d..8894afc1303a9 100644 --- a/test/extensions/filters/http/rbac/rbac_filter_integration_test.cc +++ b/test/extensions/filters/http/rbac/rbac_filter_integration_test.cc @@ -540,5 +540,186 @@ TEST_P(RBACIntegrationTest, HeaderMatchConditionDuplicateHeaderMatch) { EXPECT_EQ("200", response->headers().getStatusValue()); } +// Helper for integration testing of RBAC filter with dynamic forward proxy. +class RbacDynamicForwardProxyIntegrationHelper + : public testing::TestWithParam, + public Event::TestUsingSimulatedTime, + public HttpIntegrationTest { +public: + RbacDynamicForwardProxyIntegrationHelper() + : HttpIntegrationTest(Http::CodecType::HTTP1, GetParam()) {} + + void initializeWithFilterConfigs(bool save_filter_state, const std::string& rbac_config) { + setUpstreamProtocol(Http::CodecType::HTTP1); + + const std::string save_upstream_config = + save_filter_state ? "save_upstream_address: true " : ""; + const std::string dfp_config = + fmt::format(R"EOF( +name: dynamic_forward_proxy +typed_config: + "@type": type.googleapis.com/envoy.extensions.filters.http.dynamic_forward_proxy.v3.FilterConfig + {} + dns_cache_config: + name: foo + dns_lookup_family: {} +)EOF", + save_upstream_config, Network::Test::ipVersionToDnsFamily(GetParam())); + + config_helper_.prependFilter(rbac_config); + + config_helper_.prependFilter(dfp_config); + config_helper_.addConfigModifier([this](envoy::config::bootstrap::v3::Bootstrap& bootstrap) { + // Switch predefined cluster_0 to CDS filesystem sourcing. + bootstrap.mutable_dynamic_resources()->mutable_cds_config()->set_resource_api_version( + envoy::config::core::v3::ApiVersion::V3); + bootstrap.mutable_dynamic_resources()->mutable_cds_config()->set_path(cds_helper_.cds_path()); + bootstrap.mutable_static_resources()->clear_clusters(); + }); + + // Set validate_clusters to false to allow us to reference a CDS cluster. + config_helper_.addConfigModifier( + [](envoy::extensions::filters::network::http_connection_manager::v3::HttpConnectionManager& + hcm) { hcm.mutable_route_config()->mutable_validate_clusters()->set_value(false); }); + + // Setup the initial CDS cluster. + cluster_.mutable_connect_timeout()->CopyFrom( + Protobuf::util::TimeUtil::MillisecondsToDuration(100)); + cluster_.set_name("cluster_0"); + cluster_.set_lb_policy(envoy::config::cluster::v3::Cluster::CLUSTER_PROVIDED); + + ConfigHelper::HttpProtocolOptions protocol_options; + protocol_options.mutable_upstream_http_protocol_options()->set_auto_sni(true); + protocol_options.mutable_upstream_http_protocol_options()->set_auto_san_validation(true); + protocol_options.mutable_explicit_http_config()->mutable_http_protocol_options(); + ConfigHelper::setProtocolOptions(cluster_, protocol_options); + + const std::string cluster_type_config = fmt::format( + R"EOF( +name: envoy.clusters.dynamic_forward_proxy +typed_config: + "@type": type.googleapis.com/envoy.extensions.clusters.dynamic_forward_proxy.v3.ClusterConfig + dns_cache_config: + name: foo + dns_lookup_family: {} +)EOF", + Network::Test::ipVersionToDnsFamily(GetParam())); + + TestUtility::loadFromYaml(cluster_type_config, *cluster_.mutable_cluster_type()); + // Load the CDS cluster and wait for it to initialize. + cds_helper_.setCds({cluster_}); + HttpIntegrationTest::initialize(); + test_server_->waitForCounterEq("cluster_manager.cluster_added", 1); + test_server_->waitForGaugeEq("cluster_manager.warming_clusters", 0); + } + + CdsHelper cds_helper_; + envoy::config::cluster::v3::Cluster cluster_; + bool write_cache_file_{}; +}; + +INSTANTIATE_TEST_SUITE_P(IpVersions, RbacDynamicForwardProxyIntegrationHelper, + testing::ValuesIn(TestEnvironment::getIpVersionsForTest()), + TestUtility::ipTestParamsToString); + +// Verify that if upstream ip matcher is configured, upstream address is saved by a filter(dynamic +// forward proxy in this case). If not saved, the request would be denied. +TEST_P(RbacDynamicForwardProxyIntegrationHelper, AllowIpWithNoFilterState) { + const std::string rbac_config = R"EOF( +name: rbac +typed_config: + "@type": type.googleapis.com/envoy.extensions.filters.http.rbac.v3.RBAC + rules: + policies: + foo: + permissions: + - or_rules: + rules: + - matcher: + name: envoy.filters.http.rbac.matchers.upstream_ip_port + typed_config: + "@type": type.googleapis.com/envoy.extensions.rbac.matchers.upstream_ip_port.v3.UpstreamIpPortMatcher + upstream_ip: + address_prefix: 127.0.0.1 + prefix_len: 24 + principals: + - any: true +)EOF"; + + initializeWithFilterConfigs(false, rbac_config); + codec_client_ = makeHttpConnection(lookupPort("http")); + const Http::TestRequestHeaderMapImpl request_headers{ + {":method", "POST"}, + {":path", "/test/long/url"}, + {":scheme", "http"}, + {":authority", + fmt::format("localhost:{}", fake_upstreams_[0]->localAddress()->ip()->port())}}; + + auto response = codec_client_->makeRequestWithBody(request_headers, 1024); + + ASSERT_TRUE(response->waitForEndStream()); + ASSERT_TRUE(response->complete()); + EXPECT_EQ("403", response->headers().getStatusValue()); +} + +// Verify that if upstream ip matcher is configured and upstream address is saved by dynamic +// forward proxy, then RBAC policy is evaluated correctly for `or_rules`. +#ifndef WIN32 +// TODO(conqerAtapple) figure out why this test doesn't pass on windows. +TEST_P(RbacDynamicForwardProxyIntegrationHelper, DenyIpOrPortWithFilterState) { + const std::string rbac_config = R"EOF( +name: rbac +typed_config: + "@type": type.googleapis.com/envoy.extensions.filters.http.rbac.v3.RBAC + rules: + action: DENY + policies: + foo: + permissions: + - or_rules: + rules: + - matcher: + name: envoy.filters.http.rbac.matchers.upstream_ip_port + typed_config: + "@type": type.googleapis.com/envoy.extensions.rbac.matchers.upstream_ip_port.v3.UpstreamIpPortMatcher + upstream_ip: + address_prefix: 127.2.1.1 + prefix_len: 24 + - matcher: + name: envoy.filters.http.rbac.matchers.upstream_ip_port + typed_config: + "@type": type.googleapis.com/envoy.extensions.rbac.matchers.upstream_ip_port.v3.UpstreamIpPortMatcher + upstream_ip: + address_prefix: 127.0.0.1 + prefix_len: 24 + - matcher: + name: envoy.filters.http.rbac.matchers.upstream_ip_port + typed_config: + "@type": type.googleapis.com/envoy.extensions.rbac.matchers.upstream_ip_port.v3.UpstreamIpPortMatcher + upstream_ip: + address_prefix: ::1 + prefix_len: 24 + principals: + - any: true +)EOF"; + + initializeWithFilterConfigs(true, rbac_config); + + codec_client_ = makeHttpConnection(lookupPort("http")); + const Http::TestRequestHeaderMapImpl request_headers{ + {":method", "POST"}, + {":path", "/test/long/url"}, + {":scheme", "http"}, + {":authority", + fmt::format("localhost:{}", fake_upstreams_[0]->localAddress()->ip()->port())}}; + + auto response = codec_client_->makeRequestWithBody(request_headers, 1024); + + ASSERT_TRUE(response->waitForEndStream()); + ASSERT_TRUE(response->complete()); + EXPECT_EQ("403", response->headers().getStatusValue()); +} +#endif + } // namespace } // namespace Envoy diff --git a/test/extensions/filters/http/rbac/rbac_filter_test.cc b/test/extensions/filters/http/rbac/rbac_filter_test.cc index 4d50a70421da0..9b2dc930d71a6 100644 --- a/test/extensions/filters/http/rbac/rbac_filter_test.cc +++ b/test/extensions/filters/http/rbac/rbac_filter_test.cc @@ -1,8 +1,10 @@ #include "envoy/config/rbac/v3/rbac.pb.h" #include "envoy/extensions/filters/http/rbac/v3/rbac.pb.h" +#include "envoy/extensions/rbac/matchers/upstream_ip_port/v3/upstream_ip_port_matcher.pb.h" #include "source/common/config/metadata.h" #include "source/common/network/utility.h" +#include "source/common/stream_info/upstream_address.h" #include "source/extensions/filters/common/rbac/utility.h" #include "source/extensions/filters/http/rbac/rbac_filter.h" @@ -49,18 +51,30 @@ class RoleBasedAccessControlFilterTest : public testing::Test { (*config.mutable_shadow_rules()->mutable_policies())["bar"] = shadow_policy; config.set_shadow_rules_stat_prefix("prefix_"); - return std::make_shared(config, "test", store_); + return std::make_shared( + config, "test", store_, ProtobufMessage::getStrictValidationVisitor()); } RoleBasedAccessControlFilterTest() : config_(setupConfig(envoy::config::rbac::v3::RBAC::ALLOW)), filter_(config_) {} void SetUp() override { + config_ = setupConfig(envoy::config::rbac::v3::RBAC::ALLOW); + filter_ = RoleBasedAccessControlFilter(config_); + EXPECT_CALL(callbacks_, connection()).WillRepeatedly(Return(&connection_)); EXPECT_CALL(callbacks_, streamInfo()).WillRepeatedly(ReturnRef(req_info_)); filter_.setDecoderFilterCallbacks(callbacks_); } + void SetUp(RoleBasedAccessControlFilterConfigSharedPtr config) { + config_ = config; + filter_ = RoleBasedAccessControlFilter(config_); + + EXPECT_CALL(callbacks_, connection()).WillRepeatedly(Return(&connection_)); + EXPECT_CALL(callbacks_, streamInfo()).WillRepeatedly(ReturnRef(req_info_)); + filter_.setDecoderFilterCallbacks(callbacks_); + } void setDestinationPort(uint16_t port) { address_ = Envoy::Network::Utility::parseInternetAddress("1.2.3.4", port, false); req_info_.downstream_connection_info_provider_->setLocalAddress(address_); @@ -266,6 +280,321 @@ TEST_F(RoleBasedAccessControlFilterTest, ShouldNotLog) { checkAccessLogMetadata(LogResult::No); } +// Upstream Ip and Port matcher tests. +class UpstreamIpPortMatcherTests : public RoleBasedAccessControlFilterTest { +public: + struct UpstreamIpPortMatcherConfig { + UpstreamIpPortMatcherConfig() = default; + + UpstreamIpPortMatcherConfig(const std::string& ip) : ip_(ip) {} + + UpstreamIpPortMatcherConfig(uint16_t start, uint16_t end) { + envoy::type::v3::Int64Range port_range; + port_range.set_start(start); + port_range.set_end(end); + port_range_ = port_range; + } + + UpstreamIpPortMatcherConfig(const std::string& ip, uint16_t start, uint16_t end) : ip_(ip) { + envoy::type::v3::Int64Range port_range; + port_range.set_start(start); + port_range.set_end(end); + port_range_ = port_range; + } + + absl::optional ip_; + absl::optional port_range_; + }; + + void upstreamIpTestsBasicPolicySetup(const std::vector& configs, + const envoy::config::rbac::v3::RBAC::Action& action) { + envoy::config::rbac::v3::Policy policy; + + auto policy_rules = policy.add_permissions()->mutable_or_rules(); + policy_rules->add_rules()->mutable_requested_server_name()->MergeFrom( + TestUtility::createRegexMatcher(".*cncf.io")); + + for (const auto& config : configs) { + envoy::extensions::rbac::matchers::upstream_ip_port::v3::UpstreamIpPortMatcher matcher; + + if (config.ip_) { + matcher.mutable_upstream_ip()->set_address_prefix(*config.ip_); + matcher.mutable_upstream_ip()->mutable_prefix_len()->set_value(32); + } + + if (config.port_range_) { + *matcher.mutable_upstream_port_range() = config.port_range_.value(); + } + + auto* matcher_ext_config = policy_rules->add_rules()->mutable_matcher(); + + *matcher_ext_config->mutable_name() = "envoy.rbac.matchers.upstream.upstream_ip_port"; + + matcher_ext_config->mutable_typed_config()->PackFrom(matcher); + } + + policy.add_principals()->set_any(true); + + envoy::extensions::filters::http::rbac::v3::RBAC config; + config.mutable_rules()->set_action(action); + (*config.mutable_rules()->mutable_policies())["foo"] = policy; + + auto config_ptr = std::make_shared( + config, "test", store_, ProtobufMessage::getStrictValidationVisitor()); + + // Setup test with the policy config. + SetUp(config_ptr); + } + + void upstreamIpTestsFilterStateSetup(NiceMock& callback, + const std::vector& upstream_ips) { + auto address_obj = std::make_unique(); + + for (const auto& ip : upstream_ips) { + Network::Address::InstanceConstSharedPtr address = + Envoy::Network::Utility::parseInternetAddressAndPort(ip, false); + + address_obj->address_ = address; + } + + // Set the filter state data. + callback.streamInfo().filterState()->setData( + StreamInfo::UpstreamAddress::key(), std::move(address_obj), + StreamInfo::FilterState::StateType::ReadOnly, StreamInfo::FilterState::LifeSpan::Request); + } +}; + +// Tests simple permission policy with no upstream ip metadata in the filter state. +TEST_F(UpstreamIpPortMatcherTests, UpstreamIpNoFilterStateMetadata) { + const std::vector configs = { + {"1.2.3.4"}, + }; + // Setup policy config. + upstreamIpTestsBasicPolicySetup(configs, envoy::config::rbac::v3::RBAC::ALLOW); + + // Filter iteration should be stopped as there is no filter state metadata. + EXPECT_EQ(Http::FilterHeadersStatus::StopIteration, filter_.decodeHeaders(headers_, false)); + + // Expect `denied` stats to be incremented. + EXPECT_EQ(1U, config_->stats().denied_.value()); +} + +// Tests simple upstream_ip ALLOW permission policy with ONLY upstream ip metadata in the filter +// state. +TEST_F(UpstreamIpPortMatcherTests, UpstreamIpWithFilterStateAllow) { + // Setup filter state with the upstream address. + upstreamIpTestsFilterStateSetup(callbacks_, {"1.2.3.4:123"}); + + // Setup policy config. + const std::vector configs = { + {"1.2.3.4"}, + }; + upstreamIpTestsBasicPolicySetup(configs, envoy::config::rbac::v3::RBAC::ALLOW); + + // Filter iteration should continue since the policy is ALLOW. + EXPECT_EQ(Http::FilterHeadersStatus::Continue, filter_.decodeHeaders(headers_, false)); + + // Expect `allowed` stats to be incremented. + EXPECT_EQ(1U, config_->stats().allowed_.value()); +} + +// Tests simple upstream_ip DENY permission policy with ONLY upstream ip metadata in the filter +// state. +TEST_F(UpstreamIpPortMatcherTests, UpstreamIpWithFilterStateDeny) { + // Setup filter state with the upstream address. + upstreamIpTestsFilterStateSetup(callbacks_, {"1.2.3.4:123"}); + + // Setup policy config. + const std::vector configs = { + {"1.2.3.4"}, + }; + + upstreamIpTestsBasicPolicySetup(configs, envoy::config::rbac::v3::RBAC::DENY); + + // Filter iteration should stop since the policy is DENY. + EXPECT_EQ(Http::FilterHeadersStatus::StopIteration, filter_.decodeHeaders(headers_, false)); + + // Expect `denied` stats to be incremented. + EXPECT_EQ(1U, config_->stats().denied_.value()); +} + +// Tests simple upstream_ip DENY permission policy with BOTH upstream ip and port matching the +// policy. +TEST_F(UpstreamIpPortMatcherTests, UpstreamIpPortMatchDeny) { + // Setup filter state with the upstream address. + upstreamIpTestsFilterStateSetup(callbacks_, {"1.2.3.4:123"}); + + const std::vector configs = { + {"1.2.3.4", 120, 123}, + }; + + // Setup policy config. + upstreamIpTestsBasicPolicySetup(configs, envoy::config::rbac::v3::RBAC::DENY); + + // Filter iteration should stop since the policy is DENY. + EXPECT_EQ(Http::FilterHeadersStatus::StopIteration, filter_.decodeHeaders(headers_, false)); + + // Expect `denied` stats to be incremented. + EXPECT_EQ(1U, config_->stats().denied_.value()); +} + +TEST_F(UpstreamIpPortMatcherTests, UpstreamIpPortMatchAllow) { + // Setup filter state with the upstream address. + upstreamIpTestsFilterStateSetup(callbacks_, {"1.2.3.4:123"}); + + const std::vector configs = { + {"1.2.3.4", 120, 123}, + }; + + // Setup policy config. + upstreamIpTestsBasicPolicySetup(configs, envoy::config::rbac::v3::RBAC::ALLOW); + + EXPECT_EQ(Http::FilterHeadersStatus::Continue, filter_.decodeHeaders(headers_, false)); + + // Expect `allowed` stats to be incremented. + EXPECT_EQ(1U, config_->stats().allowed_.value()); +} + +TEST_F(UpstreamIpPortMatcherTests, UpstreamPortMatchDeny) { + // Setup filter state with the upstream address. + upstreamIpTestsFilterStateSetup(callbacks_, {"1.2.3.4:123"}); + + const std::vector configs = { + {120, 123}, + }; + + // Setup policy config. + upstreamIpTestsBasicPolicySetup(configs, envoy::config::rbac::v3::RBAC::DENY); + + // Filter iteration should stop since the policy is DENY. + EXPECT_EQ(Http::FilterHeadersStatus::StopIteration, filter_.decodeHeaders(headers_, false)); + + // Expect `denied` stats to be incremented. + EXPECT_EQ(1U, config_->stats().denied_.value()); +} + +TEST_F(UpstreamIpPortMatcherTests, UpstreamPortMatchAllow) { + // Setup filter state with the upstream address. + upstreamIpTestsFilterStateSetup(callbacks_, {"1.2.3.4:123"}); + + const std::vector configs = { + {120, 123}, + }; + + // Setup policy config. + upstreamIpTestsBasicPolicySetup(configs, envoy::config::rbac::v3::RBAC::ALLOW); + + EXPECT_EQ(Http::FilterHeadersStatus::Continue, filter_.decodeHeaders(headers_, false)); + + // Expect `allowed` stats to be incremented. + EXPECT_EQ(1U, config_->stats().allowed_.value()); +} + +// Tests upstream_ip DENY permission policy with multiple upstream ips to match in the policy. +// If any of the configured upstream ip addresses match the metadata, the policy is enforced (DENY). +TEST_F(UpstreamIpPortMatcherTests, MultiUpstreamIpsAnyPolicyDeny) { + // Setup filter state with the upstream address. + upstreamIpTestsFilterStateSetup(callbacks_, {"1.2.3.4:123"}); + + // Setup policy config. + const std::vector configs = { + {"1.1.1.2"}, {"1.2.3.4", 120, 123}, {"1.2.3.5"}}; + + upstreamIpTestsBasicPolicySetup(configs, envoy::config::rbac::v3::RBAC::DENY); + + // Filter iteration should stop since the policy is DENY. + EXPECT_EQ(Http::FilterHeadersStatus::StopIteration, filter_.decodeHeaders(headers_, false)); + + // Expect `denied` stats to be incremented. + EXPECT_EQ(1U, config_->stats().denied_.value()); +} + +// Tests upstream_ip DENY permission policy with multiple upstream ips to match in the policy. +// If ONLY port is configured in the policy, a match should enforce the policy. +TEST_F(UpstreamIpPortMatcherTests, MultiUpstreamIpsNoIpMatchPortMatchDeny) { + // Setup filter state with the upstream address. + upstreamIpTestsFilterStateSetup(callbacks_, {"2.2.3.4:123"}); + + // Setup policy config. + const std::vector configs = {{"1.1.1.2"}, {120, 123}, {"1.2.3.5"}}; + + upstreamIpTestsBasicPolicySetup(configs, envoy::config::rbac::v3::RBAC::DENY); + + // Filter iteration should stop since the policy is DENY. + EXPECT_EQ(Http::FilterHeadersStatus::StopIteration, filter_.decodeHeaders(headers_, false)); + + // Expect `denied` stats to be incremented. + EXPECT_EQ(1U, config_->stats().denied_.value()); +} + +// Tests upstream_ip DENY permission policy with multiple upstream ips to match in the policy. +// If NONE of the configured upstream ip addresses or port match the metadata, the policy is NOT +// enforced. +TEST_F(UpstreamIpPortMatcherTests, MultiUpstreamIpsNoIpMatchNoPortMatchDeny) { + // Setup filter state with the upstream address. + upstreamIpTestsFilterStateSetup(callbacks_, {"2.2.3.4:123"}); + + // Setup policy config. + const std::vector configs = {{"1.1.1.2"}, {124, 125}, {"1.2.3.5"}}; + + upstreamIpTestsBasicPolicySetup(configs, envoy::config::rbac::v3::RBAC::DENY); + + EXPECT_EQ(Http::FilterHeadersStatus::Continue, filter_.decodeHeaders(headers_, false)); + + // Expect `allowed` stats to be incremented. + EXPECT_EQ(1U, config_->stats().allowed_.value()); +} + +// Tests upstream_ip DENY permission policy with multiple upstream ips to match in the policy. +// If NONE of the configured upstream ip addresses or port match the metadata, the policy is NOT +// enforced. +TEST_F(UpstreamIpPortMatcherTests, MultiUpstreamIpsAnyPolicyNoMatchDeny) { + // Setup filter state with the upstream address. + upstreamIpTestsFilterStateSetup(callbacks_, {"1.2.3.4:123"}); + + // Setup policy config. + const std::vector configs = { + {"1.1.1.2"}, {"1.2.3.4", 124, 125}, {"1.2.3.5"}}; + + upstreamIpTestsBasicPolicySetup(configs, envoy::config::rbac::v3::RBAC::DENY); + + EXPECT_EQ(Http::FilterHeadersStatus::Continue, filter_.decodeHeaders(headers_, false)); + + // Expect `allowed` stats to be incremented. + EXPECT_EQ(1U, config_->stats().allowed_.value()); +} + +// Tests simple DENY permission policy with misconfigured port range. +TEST_F(UpstreamIpPortMatcherTests, UpstreamPortBadRangeDeny) { + // Setup filter state with the upstream address. + upstreamIpTestsFilterStateSetup(callbacks_, {"1.2.3.4:8080"}); + + const std::vector configs = { + {8080, 0}, + }; + + // Setup policy config. + upstreamIpTestsBasicPolicySetup(configs, envoy::config::rbac::v3::RBAC::DENY); + + EXPECT_EQ(Http::FilterHeadersStatus::Continue, filter_.decodeHeaders(headers_, false)); + + EXPECT_EQ(0, config_->stats().denied_.value()); +} + +// Verifies that if no IP or port is configured, EnvoyException is thrown. +TEST_F(UpstreamIpPortMatcherTests, EmptyUpstreamConfigPolicyDeny) { + // Setup filter state with the upstream address. + upstreamIpTestsFilterStateSetup(callbacks_, {"1.2.3.4:123"}); + + // Setup policy config. + const std::vector configs = {{}}; + + EXPECT_THROW_WITH_MESSAGE( + upstreamIpTestsBasicPolicySetup(configs, envoy::config::rbac::v3::RBAC::DENY), EnvoyException, + "Invalid UpstreamIpPortMatcher configuration - missing `upstream_ip` " + "and/or `upstream_port_range`"); +} + } // namespace } // namespace RBACFilter } // namespace HttpFilters diff --git a/test/extensions/filters/network/rbac/filter_test.cc b/test/extensions/filters/network/rbac/filter_test.cc index 0d62c804b5c59..7d2bcf4dc3c42 100644 --- a/test/extensions/filters/network/rbac/filter_test.cc +++ b/test/extensions/filters/network/rbac/filter_test.cc @@ -52,7 +52,8 @@ class RoleBasedAccessControlNetworkFilterTest : public testing::Test { config.set_enforcement_type(envoy::extensions::filters::network::rbac::v3::RBAC::CONTINUOUS); } - return std::make_shared(config, store_); + return std::make_shared( + config, store_, ProtobufMessage::getStrictValidationVisitor()); } RoleBasedAccessControlNetworkFilterTest() : config_(setupConfig()) { diff --git a/tools/extensions/extensions_check.py b/tools/extensions/extensions_check.py index b4d35c6b218cb..80c348ae63a9e 100644 --- a/tools/extensions/extensions_check.py +++ b/tools/extensions/extensions_check.py @@ -54,7 +54,8 @@ "envoy.resource_monitors", "envoy.retry_host_predicates", "envoy.retry_priorities", "envoy.stats_sinks", "envoy.thrift_proxy.filters", "envoy.tracers", "envoy.sip_proxy.filters", "envoy.transport_sockets.downstream", "envoy.transport_sockets.upstream", - "envoy.tls.cert_validator", "envoy.upstreams", "envoy.wasm.runtime", "envoy.common.key_value") + "envoy.tls.cert_validator", "envoy.upstreams", "envoy.wasm.runtime", "envoy.common.key_value", + "envoy.rbac.matchers") EXTENSION_STATUS_VALUES = ( # This extension is stable and is expected to be production usable.