From c7ad00d980b6e7e3451c6a5e7348324763f0ec3f Mon Sep 17 00:00:00 2001 From: Xie Zhihao Date: Wed, 14 Jul 2021 15:40:31 +0800 Subject: [PATCH 01/12] rbac: add matcher for matching range of destination ports Signed-off-by: Xie Zhihao --- api/envoy/config/rbac/v3/BUILD | 1 + api/envoy/config/rbac/v3/rbac.proto | 4 ++++ generated_api_shadow/envoy/config/rbac/v3/BUILD | 1 + .../envoy/config/rbac/v3/rbac.proto | 4 ++++ source/extensions/filters/common/rbac/matchers.cc | 14 ++++++++++++++ source/extensions/filters/common/rbac/matchers.h | 13 +++++++++++++ 6 files changed, 37 insertions(+) diff --git a/api/envoy/config/rbac/v3/BUILD b/api/envoy/config/rbac/v3/BUILD index c5246439c7b55..41fe2eafeeb89 100644 --- a/api/envoy/config/rbac/v3/BUILD +++ b/api/envoy/config/rbac/v3/BUILD @@ -9,6 +9,7 @@ api_proto_package( "//envoy/annotations:pkg", "//envoy/config/core/v3:pkg", "//envoy/config/route/v3:pkg", + "//envoy/type/v3:pkg", "//envoy/type/matcher/v3:pkg", "@com_github_cncf_udpa//udpa/annotations:pkg", "@com_google_googleapis//google/api/expr/v1alpha1:checked_proto", diff --git a/api/envoy/config/rbac/v3/rbac.proto b/api/envoy/config/rbac/v3/rbac.proto index 44b3cf7cee6ec..24d6ad662e2f7 100644 --- a/api/envoy/config/rbac/v3/rbac.proto +++ b/api/envoy/config/rbac/v3/rbac.proto @@ -7,6 +7,7 @@ import "envoy/config/route/v3/route_components.proto"; import "envoy/type/matcher/v3/metadata.proto"; import "envoy/type/matcher/v3/path.proto"; import "envoy/type/matcher/v3/string.proto"; +import "envoy/type/v3/range.proto"; import "google/api/expr/v1alpha1/checked.proto"; import "google/api/expr/v1alpha1/syntax.proto"; @@ -185,6 +186,9 @@ message Permission { // A port number that describes the destination port connecting to. uint32 destination_port = 6 [(validate.rules).uint32 = {lte: 65535}]; + // A port number range that describes a range of destination ports connecting to. + type.v3.Int32Range destination_port_range = 11; + // Metadata that describes additional information about the action. type.matcher.v3.MetadataMatcher metadata = 7; diff --git a/generated_api_shadow/envoy/config/rbac/v3/BUILD b/generated_api_shadow/envoy/config/rbac/v3/BUILD index c5246439c7b55..41fe2eafeeb89 100644 --- a/generated_api_shadow/envoy/config/rbac/v3/BUILD +++ b/generated_api_shadow/envoy/config/rbac/v3/BUILD @@ -9,6 +9,7 @@ api_proto_package( "//envoy/annotations:pkg", "//envoy/config/core/v3:pkg", "//envoy/config/route/v3:pkg", + "//envoy/type/v3:pkg", "//envoy/type/matcher/v3:pkg", "@com_github_cncf_udpa//udpa/annotations:pkg", "@com_google_googleapis//google/api/expr/v1alpha1:checked_proto", diff --git a/generated_api_shadow/envoy/config/rbac/v3/rbac.proto b/generated_api_shadow/envoy/config/rbac/v3/rbac.proto index 44b3cf7cee6ec..24d6ad662e2f7 100644 --- a/generated_api_shadow/envoy/config/rbac/v3/rbac.proto +++ b/generated_api_shadow/envoy/config/rbac/v3/rbac.proto @@ -7,6 +7,7 @@ import "envoy/config/route/v3/route_components.proto"; import "envoy/type/matcher/v3/metadata.proto"; import "envoy/type/matcher/v3/path.proto"; import "envoy/type/matcher/v3/string.proto"; +import "envoy/type/v3/range.proto"; import "google/api/expr/v1alpha1/checked.proto"; import "google/api/expr/v1alpha1/syntax.proto"; @@ -185,6 +186,9 @@ message Permission { // A port number that describes the destination port connecting to. uint32 destination_port = 6 [(validate.rules).uint32 = {lte: 65535}]; + // A port number range that describes a range of destination ports connecting to. + type.v3.Int32Range destination_port_range = 11; + // Metadata that describes additional information about the action. type.matcher.v3.MetadataMatcher metadata = 7; diff --git a/source/extensions/filters/common/rbac/matchers.cc b/source/extensions/filters/common/rbac/matchers.cc index 25d8e2de63b54..3b76b24579424 100644 --- a/source/extensions/filters/common/rbac/matchers.cc +++ b/source/extensions/filters/common/rbac/matchers.cc @@ -23,6 +23,8 @@ MatcherConstSharedPtr Matcher::create(const envoy::config::rbac::v3::Permission& IPMatcher::Type::DownstreamLocal); case envoy::config::rbac::v3::Permission::RuleCase::kDestinationPort: return std::make_shared(permission.destination_port()); + case envoy::config::rbac::v3::Permission::RuleCase::kDestinationPortRange: + return std::make_shared(permission.destination_port_range()); case envoy::config::rbac::v3::Permission::RuleCase::kAny: return std::make_shared(); case envoy::config::rbac::v3::Permission::RuleCase::kMetadata: @@ -159,6 +161,18 @@ bool PortMatcher::matches(const Network::Connection&, const Envoy::Http::Request return ip && ip->port() == port_; } +bool PortRangeMatcher::matches(const Network::Connection&, const Envoy::Http::RequestHeaderMap&, + const StreamInfo::StreamInfo& info) const { + const Envoy::Network::Address::Ip* ip = + info.downstreamAddressProvider().localAddress().get()->ip(); + if (ip) { + const auto port = ip->port(); + return start_ <= port && port < end_; + } else { + return false; + } +} + bool AuthenticatedMatcher::matches(const Network::Connection& connection, const Envoy::Http::RequestHeaderMap&, const StreamInfo::StreamInfo&) const { diff --git a/source/extensions/filters/common/rbac/matchers.h b/source/extensions/filters/common/rbac/matchers.h index 472b4a2c9c17e..44186d64b5a7b 100644 --- a/source/extensions/filters/common/rbac/matchers.h +++ b/source/extensions/filters/common/rbac/matchers.h @@ -163,6 +163,19 @@ class PortMatcher : public Matcher { const uint32_t port_; }; +class PortRangeMatcher : public Matcher { +public: + PortRangeMatcher(const ::envoy::type::v3::Int32Range& range) + : start_(range.start()), end_(range.end()) {} + + bool matches(const Network::Connection&, const Envoy::Http::RequestHeaderMap&, + const StreamInfo::StreamInfo& info) const override; + +private: + const uint32_t start_; + const uint32_t end_; +}; + /** * Matches the principal name as described in the peer certificate. Uses the URI SAN first. If that * field is not present, uses the subject instead. From 4a72a94f7af0cce4853ae624a4134800a92da2e9 Mon Sep 17 00:00:00 2001 From: Xie Zhihao Date: Wed, 14 Jul 2021 18:04:49 +0800 Subject: [PATCH 02/12] test: add test for RBAC port range matching Signed-off-by: Xie Zhihao --- .../filters/common/rbac/matchers_test.cc | 26 +++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/test/extensions/filters/common/rbac/matchers_test.cc b/test/extensions/filters/common/rbac/matchers_test.cc index 87d22517dd428..a3d1fe4987e34 100644 --- a/test/extensions/filters/common/rbac/matchers_test.cc +++ b/test/extensions/filters/common/rbac/matchers_test.cc @@ -233,6 +233,32 @@ TEST(PortMatcher, PortMatcher) { checkMatcher(PortMatcher(456), false, conn, headers, info); } +TEST(PortRangeMatcher, PortRangeMatcher) { + Envoy::Network::MockConnection conn; + Envoy::Http::TestRequestHeaderMapImpl headers; + NiceMock info; + Envoy::Network::Address::InstanceConstSharedPtr addr = + Envoy::Network::Utility::parseInternetAddress("1.2.3.4", 456, false); + info.downstream_address_provider_->setLocalAddress(addr); + + envoy::type::v3::Int32Range range; + range.set_start(123); + range.set_end(789); + checkMatcher(PortRangeMatcher(range), true, conn, headers, info); + + range.set_start(456); + range.set_end(789); + checkMatcher(PortRangeMatcher(range), true, conn, headers, info); + + range.set_start(123); + range.set_end(456); + checkMatcher(PortRangeMatcher(range), false, conn, headers, info); + + range.set_start(12); + range.set_end(34); + checkMatcher(PortRangeMatcher(range), false, conn, headers, info); +} + TEST(AuthenticatedMatcher, uriSanPeerCertificate) { Envoy::Network::MockConnection conn; auto ssl = std::make_shared(); From f571b4bdb019092456d3a2d302800bba6297c458 Mon Sep 17 00:00:00 2001 From: Xie Zhihao Date: Thu, 15 Jul 2021 20:12:09 +0800 Subject: [PATCH 03/12] rbac: fix format in destination port range matcher Signed-off-by: Xie Zhihao --- api/envoy/config/rbac/v3/BUILD | 2 +- api/envoy/config/rbac/v3/rbac.proto | 2 +- api/envoy/config/rbac/v4alpha/BUILD | 1 + api/envoy/config/rbac/v4alpha/rbac.proto | 6 +++++- generated_api_shadow/envoy/config/rbac/v3/BUILD | 2 +- generated_api_shadow/envoy/config/rbac/v3/rbac.proto | 2 +- generated_api_shadow/envoy/config/rbac/v4alpha/BUILD | 1 + generated_api_shadow/envoy/config/rbac/v4alpha/rbac.proto | 6 +++++- source/extensions/filters/common/rbac/matchers.cc | 2 +- 9 files changed, 17 insertions(+), 7 deletions(-) diff --git a/api/envoy/config/rbac/v3/BUILD b/api/envoy/config/rbac/v3/BUILD index 41fe2eafeeb89..c289def1f11d2 100644 --- a/api/envoy/config/rbac/v3/BUILD +++ b/api/envoy/config/rbac/v3/BUILD @@ -9,8 +9,8 @@ api_proto_package( "//envoy/annotations:pkg", "//envoy/config/core/v3:pkg", "//envoy/config/route/v3:pkg", - "//envoy/type/v3:pkg", "//envoy/type/matcher/v3:pkg", + "//envoy/type/v3:pkg", "@com_github_cncf_udpa//udpa/annotations:pkg", "@com_google_googleapis//google/api/expr/v1alpha1:checked_proto", "@com_google_googleapis//google/api/expr/v1alpha1:syntax_proto", diff --git a/api/envoy/config/rbac/v3/rbac.proto b/api/envoy/config/rbac/v3/rbac.proto index 24d6ad662e2f7..d66f9be2b4981 100644 --- a/api/envoy/config/rbac/v3/rbac.proto +++ b/api/envoy/config/rbac/v3/rbac.proto @@ -146,7 +146,7 @@ message Policy { } // Permission defines an action (or actions) that a principal can take. -// [#next-free-field: 11] +// [#next-free-field: 12] message Permission { option (udpa.annotations.versioning).previous_message_type = "envoy.config.rbac.v2.Permission"; diff --git a/api/envoy/config/rbac/v4alpha/BUILD b/api/envoy/config/rbac/v4alpha/BUILD index f5683a61a2867..090d01f3cd17c 100644 --- a/api/envoy/config/rbac/v4alpha/BUILD +++ b/api/envoy/config/rbac/v4alpha/BUILD @@ -10,6 +10,7 @@ api_proto_package( "//envoy/config/rbac/v3:pkg", "//envoy/config/route/v4alpha:pkg", "//envoy/type/matcher/v4alpha:pkg", + "//envoy/type/v3:pkg", "@com_github_cncf_udpa//udpa/annotations:pkg", "@com_google_googleapis//google/api/expr/v1alpha1:checked_proto", "@com_google_googleapis//google/api/expr/v1alpha1:syntax_proto", diff --git a/api/envoy/config/rbac/v4alpha/rbac.proto b/api/envoy/config/rbac/v4alpha/rbac.proto index bd56c0c3dc326..6fbd5a90f37db 100644 --- a/api/envoy/config/rbac/v4alpha/rbac.proto +++ b/api/envoy/config/rbac/v4alpha/rbac.proto @@ -7,6 +7,7 @@ import "envoy/config/route/v4alpha/route_components.proto"; import "envoy/type/matcher/v4alpha/metadata.proto"; import "envoy/type/matcher/v4alpha/path.proto"; import "envoy/type/matcher/v4alpha/string.proto"; +import "envoy/type/v3/range.proto"; import "google/api/expr/v1alpha1/checked.proto"; import "google/api/expr/v1alpha1/syntax.proto"; @@ -143,7 +144,7 @@ message Policy { } // Permission defines an action (or actions) that a principal can take. -// [#next-free-field: 11] +// [#next-free-field: 12] message Permission { option (udpa.annotations.versioning).previous_message_type = "envoy.config.rbac.v3.Permission"; @@ -183,6 +184,9 @@ message Permission { // A port number that describes the destination port connecting to. uint32 destination_port = 6 [(validate.rules).uint32 = {lte: 65535}]; + // A port number range that describes a range of destination ports connecting to. + type.v3.Int32Range destination_port_range = 11; + // Metadata that describes additional information about the action. type.matcher.v4alpha.MetadataMatcher metadata = 7; diff --git a/generated_api_shadow/envoy/config/rbac/v3/BUILD b/generated_api_shadow/envoy/config/rbac/v3/BUILD index 41fe2eafeeb89..c289def1f11d2 100644 --- a/generated_api_shadow/envoy/config/rbac/v3/BUILD +++ b/generated_api_shadow/envoy/config/rbac/v3/BUILD @@ -9,8 +9,8 @@ api_proto_package( "//envoy/annotations:pkg", "//envoy/config/core/v3:pkg", "//envoy/config/route/v3:pkg", - "//envoy/type/v3:pkg", "//envoy/type/matcher/v3:pkg", + "//envoy/type/v3:pkg", "@com_github_cncf_udpa//udpa/annotations:pkg", "@com_google_googleapis//google/api/expr/v1alpha1:checked_proto", "@com_google_googleapis//google/api/expr/v1alpha1:syntax_proto", diff --git a/generated_api_shadow/envoy/config/rbac/v3/rbac.proto b/generated_api_shadow/envoy/config/rbac/v3/rbac.proto index 24d6ad662e2f7..d66f9be2b4981 100644 --- a/generated_api_shadow/envoy/config/rbac/v3/rbac.proto +++ b/generated_api_shadow/envoy/config/rbac/v3/rbac.proto @@ -146,7 +146,7 @@ message Policy { } // Permission defines an action (or actions) that a principal can take. -// [#next-free-field: 11] +// [#next-free-field: 12] message Permission { option (udpa.annotations.versioning).previous_message_type = "envoy.config.rbac.v2.Permission"; diff --git a/generated_api_shadow/envoy/config/rbac/v4alpha/BUILD b/generated_api_shadow/envoy/config/rbac/v4alpha/BUILD index ddf34cc1032bc..2b205e7373632 100644 --- a/generated_api_shadow/envoy/config/rbac/v4alpha/BUILD +++ b/generated_api_shadow/envoy/config/rbac/v4alpha/BUILD @@ -11,6 +11,7 @@ api_proto_package( "//envoy/config/rbac/v3:pkg", "//envoy/config/route/v4alpha:pkg", "//envoy/type/matcher/v4alpha:pkg", + "//envoy/type/v3:pkg", "@com_github_cncf_udpa//udpa/annotations:pkg", "@com_google_googleapis//google/api/expr/v1alpha1:checked_proto", "@com_google_googleapis//google/api/expr/v1alpha1:syntax_proto", diff --git a/generated_api_shadow/envoy/config/rbac/v4alpha/rbac.proto b/generated_api_shadow/envoy/config/rbac/v4alpha/rbac.proto index 3b27e68bba1dc..bff8576a27c84 100644 --- a/generated_api_shadow/envoy/config/rbac/v4alpha/rbac.proto +++ b/generated_api_shadow/envoy/config/rbac/v4alpha/rbac.proto @@ -7,6 +7,7 @@ import "envoy/config/route/v4alpha/route_components.proto"; import "envoy/type/matcher/v4alpha/metadata.proto"; import "envoy/type/matcher/v4alpha/path.proto"; import "envoy/type/matcher/v4alpha/string.proto"; +import "envoy/type/v3/range.proto"; import "google/api/expr/v1alpha1/checked.proto"; import "google/api/expr/v1alpha1/syntax.proto"; @@ -144,7 +145,7 @@ message Policy { } // Permission defines an action (or actions) that a principal can take. -// [#next-free-field: 11] +// [#next-free-field: 12] message Permission { option (udpa.annotations.versioning).previous_message_type = "envoy.config.rbac.v3.Permission"; @@ -184,6 +185,9 @@ message Permission { // A port number that describes the destination port connecting to. uint32 destination_port = 6 [(validate.rules).uint32 = {lte: 65535}]; + // A port number range that describes a range of destination ports connecting to. + type.v3.Int32Range destination_port_range = 11; + // Metadata that describes additional information about the action. type.matcher.v4alpha.MetadataMatcher metadata = 7; diff --git a/source/extensions/filters/common/rbac/matchers.cc b/source/extensions/filters/common/rbac/matchers.cc index 3b76b24579424..8643e647a6d9f 100644 --- a/source/extensions/filters/common/rbac/matchers.cc +++ b/source/extensions/filters/common/rbac/matchers.cc @@ -162,7 +162,7 @@ bool PortMatcher::matches(const Network::Connection&, const Envoy::Http::Request } bool PortRangeMatcher::matches(const Network::Connection&, const Envoy::Http::RequestHeaderMap&, - const StreamInfo::StreamInfo& info) const { + const StreamInfo::StreamInfo& info) const { const Envoy::Network::Address::Ip* ip = info.downstreamAddressProvider().localAddress().get()->ip(); if (ip) { From 4d615da03c80593154f76a231340a70280484d9b Mon Sep 17 00:00:00 2001 From: Xie Zhihao Date: Fri, 16 Jul 2021 16:05:52 +0800 Subject: [PATCH 04/12] test: add test coverage for RBAC port range matching Signed-off-by: Xie Zhihao --- .../filters/common/rbac/matchers_test.cc | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/test/extensions/filters/common/rbac/matchers_test.cc b/test/extensions/filters/common/rbac/matchers_test.cc index a3d1fe4987e34..6826f243d6f4a 100644 --- a/test/extensions/filters/common/rbac/matchers_test.cc +++ b/test/extensions/filters/common/rbac/matchers_test.cc @@ -4,6 +4,7 @@ #include "envoy/config/route/v3/route_components.pb.h" #include "envoy/type/matcher/v3/metadata.pb.h" +#include "source/common/network/address_impl.h" #include "source/common/network/utility.h" #include "source/extensions/filters/common/expr/evaluator.h" #include "source/extensions/filters/common/rbac/matchers.h" @@ -101,6 +102,15 @@ TEST(OrMatcher, Permission_Set) { checkMatcher(RBAC::OrMatcher(set), false, conn, headers, info); + perm = set.add_rules(); + envoy::type::v3::Int32Range range; + range.set_start(123); + range.set_end(456); + perm->mutable_destination_port_range()->set_start(123); + perm->mutable_destination_port_range()->set_end(456); + + checkMatcher(RBAC::OrMatcher(set), false, conn, headers, info); + perm = set.add_rules(); perm->set_any(true); @@ -257,6 +267,12 @@ TEST(PortRangeMatcher, PortRangeMatcher) { range.set_start(12); range.set_end(34); checkMatcher(PortRangeMatcher(range), false, conn, headers, info); + + NiceMock info2; + Envoy::Network::Address::InstanceConstSharedPtr addr2 = + std::make_shared("test"); + info2.downstream_address_provider_->setLocalAddress(addr2); + checkMatcher(PortRangeMatcher(range), false, conn, headers, info2); } TEST(AuthenticatedMatcher, uriSanPeerCertificate) { From 33c9d448fe39478fee1a5e08d38b5366cb7b429e Mon Sep 17 00:00:00 2001 From: Xie Zhihao Date: Mon, 19 Jul 2021 13:43:34 +0800 Subject: [PATCH 05/12] docs: add RBAC destination_port_range release note Signed-off-by: Xie Zhihao --- docs/root/version_history/current.rst | 1 + 1 file changed, 1 insertion(+) diff --git a/docs/root/version_history/current.rst b/docs/root/version_history/current.rst index 80eb72a7049ff..8754c98f5230f 100644 --- a/docs/root/version_history/current.rst +++ b/docs/root/version_history/current.rst @@ -30,6 +30,7 @@ Removed Config or Runtime New Features ------------ * http: added :ref:`string_match ` in the header matcher. +* rbac: added :ref:`destination_port_range ` for matching range of destination ports. Deprecated ---------- From af9846e1a705049a1127b2fa3ad4f6c9854b6ad9 Mon Sep 17 00:00:00 2001 From: Xie Zhihao Date: Mon, 26 Jul 2021 09:42:16 +0800 Subject: [PATCH 06/12] test: remove unused range in RBAC test Signed-off-by: Xie Zhihao --- test/extensions/filters/common/rbac/matchers_test.cc | 3 --- 1 file changed, 3 deletions(-) diff --git a/test/extensions/filters/common/rbac/matchers_test.cc b/test/extensions/filters/common/rbac/matchers_test.cc index 6826f243d6f4a..4a637eb49d5b6 100644 --- a/test/extensions/filters/common/rbac/matchers_test.cc +++ b/test/extensions/filters/common/rbac/matchers_test.cc @@ -103,9 +103,6 @@ TEST(OrMatcher, Permission_Set) { checkMatcher(RBAC::OrMatcher(set), false, conn, headers, info); perm = set.add_rules(); - envoy::type::v3::Int32Range range; - range.set_start(123); - range.set_end(456); perm->mutable_destination_port_range()->set_start(123); perm->mutable_destination_port_range()->set_end(456); From 1e9c88219dad02debb03c5e8865777577eb1afad Mon Sep 17 00:00:00 2001 From: Xie Zhihao Date: Mon, 26 Jul 2021 10:14:43 +0800 Subject: [PATCH 07/12] test: add comment for destination_port_range test in RBAC Signed-off-by: Xie Zhihao --- test/extensions/filters/common/rbac/matchers_test.cc | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/test/extensions/filters/common/rbac/matchers_test.cc b/test/extensions/filters/common/rbac/matchers_test.cc index 4a637eb49d5b6..c52083132e374 100644 --- a/test/extensions/filters/common/rbac/matchers_test.cc +++ b/test/extensions/filters/common/rbac/matchers_test.cc @@ -240,6 +240,7 @@ TEST(PortMatcher, PortMatcher) { checkMatcher(PortMatcher(456), false, conn, headers, info); } +// Test valid and invalid destination_port_range permission rule in RBAC. TEST(PortRangeMatcher, PortRangeMatcher) { Envoy::Network::MockConnection conn; Envoy::Http::TestRequestHeaderMapImpl headers; @@ -248,6 +249,8 @@ TEST(PortRangeMatcher, PortRangeMatcher) { Envoy::Network::Utility::parseInternetAddress("1.2.3.4", 456, false); info.downstream_address_provider_->setLocalAddress(addr); + // IP address with port 456 is in range [123, 789) and [456, 789), but not in range [123, 456) or + // [12, 34). envoy::type::v3::Int32Range range; range.set_start(123); range.set_end(789); @@ -265,6 +268,7 @@ TEST(PortRangeMatcher, PortRangeMatcher) { range.set_end(34); checkMatcher(PortRangeMatcher(range), false, conn, headers, info); + // Only IP address is valid for the permission rule. NiceMock info2; Envoy::Network::Address::InstanceConstSharedPtr addr2 = std::make_shared("test"); From f97bdf143f570e10bd919369235fc801f3a65fd2 Mon Sep 17 00:00:00 2001 From: Xie Zhihao Date: Mon, 26 Jul 2021 11:14:33 +0800 Subject: [PATCH 08/12] docs: rearrange order Signed-off-by: Xie Zhihao --- docs/root/version_history/current.rst | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/docs/root/version_history/current.rst b/docs/root/version_history/current.rst index 88b210895312b..6abcdac747fe2 100644 --- a/docs/root/version_history/current.rst +++ b/docs/root/version_history/current.rst @@ -53,9 +53,8 @@ New Features ------------ * http: added :ref:`string_match ` in the header matcher. * http: added support for :ref:`max_requests_per_connection ` for both upstream and downstream connections. -* rbac: added :ref:`destination_port_range ` for matching range of destination ports. - * jwt_authn: added support for :ref:`Jwt Cache ` and its size can be specified by :ref:`jwt_cache_size `. +* rbac: added :ref:`destination_port_range ` for matching range of destination ports. Deprecated ---------- From a4740b48c68844e5f8f4f44c22ac25dd33817876 Mon Sep 17 00:00:00 2001 From: Xie Zhihao Date: Fri, 30 Jul 2021 10:04:21 +0800 Subject: [PATCH 09/12] rbac: check destination port range validation Signed-off-by: Xie Zhihao --- .../extensions/filters/common/rbac/matchers.cc | 16 ++++++++++++++++ source/extensions/filters/common/rbac/matchers.h | 3 +-- 2 files changed, 17 insertions(+), 2 deletions(-) diff --git a/source/extensions/filters/common/rbac/matchers.cc b/source/extensions/filters/common/rbac/matchers.cc index 8643e647a6d9f..c7fa240ca5626 100644 --- a/source/extensions/filters/common/rbac/matchers.cc +++ b/source/extensions/filters/common/rbac/matchers.cc @@ -161,6 +161,22 @@ bool PortMatcher::matches(const Network::Connection&, const Envoy::Http::Request return ip && ip->port() == port_; } +PortRangeMatcher::PortRangeMatcher(const ::envoy::type::v3::Int32Range& range) + : start_(range.start()), end_(range.end()) { + auto start = range.start(); + auto end = range.end(); + if (start < 0 || start > 65536) { + throw EnvoyException(fmt::format("range start {} out of bounds", start)); + } + if (end < 0 || end > 65536) { + throw EnvoyException(fmt::format("range end {} out of bounds", end)); + } + if (start >= end) { + throw EnvoyException( + fmt::format("range start {} cannot be greater or equal than range end {}", start, end)); + } +} + bool PortRangeMatcher::matches(const Network::Connection&, const Envoy::Http::RequestHeaderMap&, const StreamInfo::StreamInfo& info) const { const Envoy::Network::Address::Ip* ip = diff --git a/source/extensions/filters/common/rbac/matchers.h b/source/extensions/filters/common/rbac/matchers.h index 44186d64b5a7b..79d43fb59f0c2 100644 --- a/source/extensions/filters/common/rbac/matchers.h +++ b/source/extensions/filters/common/rbac/matchers.h @@ -165,8 +165,7 @@ class PortMatcher : public Matcher { class PortRangeMatcher : public Matcher { public: - PortRangeMatcher(const ::envoy::type::v3::Int32Range& range) - : start_(range.start()), end_(range.end()) {} + PortRangeMatcher(const ::envoy::type::v3::Int32Range& range); bool matches(const Network::Connection&, const Envoy::Http::RequestHeaderMap&, const StreamInfo::StreamInfo& info) const override; From 4b99701a634ad6cdaff61a9d3a91e1ee273395b4 Mon Sep 17 00:00:00 2001 From: Xie Zhihao Date: Fri, 30 Jul 2021 11:41:36 +0800 Subject: [PATCH 10/12] test: add test for validating RBAC rules Signed-off-by: Xie Zhihao --- .../filters/common/rbac/matchers_test.cc | 20 +++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/test/extensions/filters/common/rbac/matchers_test.cc b/test/extensions/filters/common/rbac/matchers_test.cc index c52083132e374..2d9103aa145e0 100644 --- a/test/extensions/filters/common/rbac/matchers_test.cc +++ b/test/extensions/filters/common/rbac/matchers_test.cc @@ -34,6 +34,10 @@ void checkMatcher( EXPECT_EQ(expected, matcher.matches(connection, headers, info)); } +PortRangeMatcher createPortRangeMatcher(envoy::type::v3::Int32Range range) { + return PortRangeMatcher(range); +} + TEST(AlwaysMatcher, AlwaysMatches) { checkMatcher(RBAC::AlwaysMatcher(), true); } TEST(AndMatcher, Permission_Set) { @@ -274,6 +278,22 @@ TEST(PortRangeMatcher, PortRangeMatcher) { std::make_shared("test"); info2.downstream_address_provider_->setLocalAddress(addr2); checkMatcher(PortRangeMatcher(range), false, conn, headers, info2); + + // Invalid rule will cause an exception. + range.set_start(-1); + range.set_end(80); + EXPECT_THROW_WITH_REGEX(createPortRangeMatcher(range), EnvoyException, + "range start .* out of bounds"); + + range.set_start(80); + range.set_end(65537); + EXPECT_THROW_WITH_REGEX(createPortRangeMatcher(range), EnvoyException, + "range end .* out of bounds"); + + range.set_start(80); + range.set_end(80); + EXPECT_THROW_WITH_REGEX(createPortRangeMatcher(range), EnvoyException, + "range start .* cannot be greater or equal than range end .*"); } TEST(AuthenticatedMatcher, uriSanPeerCertificate) { From d51155f77a5f574af09f953d1a9ab24a7e235cb8 Mon Sep 17 00:00:00 2001 From: Xie Zhihao Date: Fri, 30 Jul 2021 11:44:16 +0800 Subject: [PATCH 11/12] rbac: fix grammar Signed-off-by: Xie Zhihao --- source/extensions/filters/common/rbac/matchers.cc | 4 ++-- test/extensions/filters/common/rbac/matchers_test.cc | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/source/extensions/filters/common/rbac/matchers.cc b/source/extensions/filters/common/rbac/matchers.cc index c7fa240ca5626..fc010bc46c68e 100644 --- a/source/extensions/filters/common/rbac/matchers.cc +++ b/source/extensions/filters/common/rbac/matchers.cc @@ -166,10 +166,10 @@ PortRangeMatcher::PortRangeMatcher(const ::envoy::type::v3::Int32Range& range) auto start = range.start(); auto end = range.end(); if (start < 0 || start > 65536) { - throw EnvoyException(fmt::format("range start {} out of bounds", start)); + throw EnvoyException(fmt::format("range start {} is out of bounds", start)); } if (end < 0 || end > 65536) { - throw EnvoyException(fmt::format("range end {} out of bounds", end)); + throw EnvoyException(fmt::format("range end {} is out of bounds", end)); } if (start >= end) { throw EnvoyException( diff --git a/test/extensions/filters/common/rbac/matchers_test.cc b/test/extensions/filters/common/rbac/matchers_test.cc index 2d9103aa145e0..0979445343df8 100644 --- a/test/extensions/filters/common/rbac/matchers_test.cc +++ b/test/extensions/filters/common/rbac/matchers_test.cc @@ -283,12 +283,12 @@ TEST(PortRangeMatcher, PortRangeMatcher) { range.set_start(-1); range.set_end(80); EXPECT_THROW_WITH_REGEX(createPortRangeMatcher(range), EnvoyException, - "range start .* out of bounds"); + "range start .* is out of bounds"); range.set_start(80); range.set_end(65537); EXPECT_THROW_WITH_REGEX(createPortRangeMatcher(range), EnvoyException, - "range end .* out of bounds"); + "range end .* is out of bounds"); range.set_start(80); range.set_end(80); From bbcf5b63b63b3253c34ef5aed565d2551b24bd64 Mon Sep 17 00:00:00 2001 From: Xie Zhihao Date: Sat, 31 Jul 2021 22:39:18 +0800 Subject: [PATCH 12/12] ci: kick ci Signed-off-by: Xie Zhihao