Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions api/envoy/config/rbac/v3/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ api_proto_package(
"//envoy/config/core/v3:pkg",
"//envoy/config/route/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",
Expand Down
6 changes: 5 additions & 1 deletion api/envoy/config/rbac/v3/rbac.proto
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand Down Expand Up @@ -145,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";

Expand Down Expand Up @@ -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;

Expand Down
1 change: 1 addition & 0 deletions api/envoy/config/rbac/v4alpha/BUILD

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

6 changes: 5 additions & 1 deletion api/envoy/config/rbac/v4alpha/rbac.proto

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion docs/root/version_history/current.rst
Original file line number Diff line number Diff line change
Expand Up @@ -53,8 +53,8 @@ New Features
------------
* http: added :ref:`string_match <envoy_v3_api_field_config.route.v3.HeaderMatcher.string_match>` in the header matcher.
* http: added support for :ref:`max_requests_per_connection <envoy_v3_api_field_config.core.v3.HttpProtocolOptions.max_requests_per_connection>` for both upstream and downstream connections.

* jwt_authn: added support for :ref:`Jwt Cache <envoy_v3_api_field_extensions.filters.http.jwt_authn.v3.JwtProvider.jwt_cache_config>` and its size can be specified by :ref:`jwt_cache_size <envoy_v3_api_field_extensions.filters.http.jwt_authn.v3.JwtCacheConfig.jwt_cache_size>`.
* rbac: added :ref:`destination_port_range <envoy_v3_api_field_config.rbac.v3.Permission.destination_port_range>` for matching range of destination ports.

Deprecated
----------
Expand Down
1 change: 1 addition & 0 deletions generated_api_shadow/envoy/config/rbac/v3/BUILD

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

6 changes: 5 additions & 1 deletion generated_api_shadow/envoy/config/rbac/v3/rbac.proto

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions generated_api_shadow/envoy/config/rbac/v4alpha/BUILD

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

6 changes: 5 additions & 1 deletion generated_api_shadow/envoy/config/rbac/v4alpha/rbac.proto

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

30 changes: 30 additions & 0 deletions source/extensions/filters/common/rbac/matchers.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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<const PortMatcher>(permission.destination_port());
case envoy::config::rbac::v3::Permission::RuleCase::kDestinationPortRange:
return std::make_shared<const PortRangeMatcher>(permission.destination_port_range());
case envoy::config::rbac::v3::Permission::RuleCase::kAny:
return std::make_shared<const AlwaysMatcher>();
case envoy::config::rbac::v3::Permission::RuleCase::kMetadata:
Expand Down Expand Up @@ -159,6 +161,34 @@ 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 {} is out of bounds", start));
}
if (end < 0 || end > 65536) {
throw EnvoyException(fmt::format("range end {} is 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 =
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 {
Expand Down
12 changes: 12 additions & 0 deletions source/extensions/filters/common/rbac/matchers.h
Original file line number Diff line number Diff line change
Expand Up @@ -163,6 +163,18 @@ class PortMatcher : public Matcher {
const uint32_t port_;
};

class PortRangeMatcher : public Matcher {
public:
PortRangeMatcher(const ::envoy::type::v3::Int32Range& range);

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.
Expand Down
63 changes: 63 additions & 0 deletions test/extensions/filters/common/rbac/matchers_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -33,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) {
Expand Down Expand Up @@ -101,6 +106,12 @@ TEST(OrMatcher, Permission_Set) {

checkMatcher(RBAC::OrMatcher(set), false, conn, headers, info);

perm = set.add_rules();
perm->mutable_destination_port_range()->set_start(123);
perm->mutable_destination_port_range()->set_end(456);
Comment on lines +110 to +111
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: why not copy range into destination_port_range?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's right. Unused variable has been removed.


checkMatcher(RBAC::OrMatcher(set), false, conn, headers, info);

perm = set.add_rules();
perm->set_any(true);

Expand Down Expand Up @@ -233,6 +244,58 @@ TEST(PortMatcher, PortMatcher) {
checkMatcher(PortMatcher(456), false, conn, headers, info);
}

// Test valid and invalid destination_port_range permission rule in RBAC.
TEST(PortRangeMatcher, PortRangeMatcher) {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add a short comment stating what the test validates.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, comment added.

Envoy::Network::MockConnection conn;
Envoy::Http::TestRequestHeaderMapImpl headers;
NiceMock<StreamInfo::MockStreamInfo> info;
Envoy::Network::Address::InstanceConstSharedPtr addr =
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);
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);

// Only IP address is valid for the permission rule.
NiceMock<StreamInfo::MockStreamInfo> info2;
Envoy::Network::Address::InstanceConstSharedPtr addr2 =
std::make_shared<const Envoy::Network::Address::PipeInstance>("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 .* is out of bounds");

range.set_start(80);
range.set_end(65537);
EXPECT_THROW_WITH_REGEX(createPortRangeMatcher(range), EnvoyException,
"range end .* is 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) {
Envoy::Network::MockConnection conn;
auto ssl = std::make_shared<Ssl::MockConnectionInfo>();
Expand Down