From 7b4588b327d276a82c5f7ceb94797405a18bdadd Mon Sep 17 00:00:00 2001 From: Kuat Yessenov Date: Thu, 12 Sep 2019 16:39:35 -0700 Subject: [PATCH 1/4] improve attributes and docs Signed-off-by: Kuat Yessenov --- api/envoy/config/rbac/v2/rbac.proto | 5 +- .../arch_overview/security/rbac_filter.rst | 65 +++++++++++++++++++ .../extensions/filters/common/expr/context.cc | 31 +++++++-- .../extensions/filters/common/expr/context.h | 15 ++++- .../filters/common/expr/evaluator.cc | 2 + .../filters/common/expr/context_test.cc | 30 +++++++-- tools/spelling_dictionary.txt | 1 + 7 files changed, 134 insertions(+), 15 deletions(-) diff --git a/api/envoy/config/rbac/v2/rbac.proto b/api/envoy/config/rbac/v2/rbac.proto index f3546caa7bff..1d797d418681 100644 --- a/api/envoy/config/rbac/v2/rbac.proto +++ b/api/envoy/config/rbac/v2/rbac.proto @@ -91,8 +91,9 @@ message Policy { // Principal with the `any` field set to true should be used. repeated Principal principals = 2 [(validate.rules).repeated .min_items = 1]; - // An optional symbolic expression specifying an access control condition. - // The condition is combined with AND semantics. + // An optional symbolic expression specifying an access control + // :ref:`condition `. The condition is combined + // with the permissions and the principals as a clause with AND semantics. google.api.expr.v1alpha1.Expr condition = 3; } diff --git a/docs/root/intro/arch_overview/security/rbac_filter.rst b/docs/root/intro/arch_overview/security/rbac_filter.rst index a0fba7097e78..4dd925dffe25 100644 --- a/docs/root/intro/arch_overview/security/rbac_filter.rst +++ b/docs/root/intro/arch_overview/security/rbac_filter.rst @@ -33,3 +33,68 @@ The filter can be configured with a :ref:`shadow policy ` that doesn't have any effect (i.e. not deny the request) but only emit stats and log the result. This is useful for testing a rule before applying in production. + +.. _arch_overview_condition: + +Condition +--------- + +In addition to the pre-defined permissions and prinipals, a policy may optionally provide an +authorization condition written in the `Common Expression Language +`_. The condition specifies an extra +clause that must be satisfied for the policy to match. For example, the following condition checks +whether the request path starts with `/v1/`: + +.. code-block:: yaml + + call_expr: + function: startsWith + args: + - select_expr: + operand: + ident_expr: + name: request + field: path + - const_expr: + string_value: /v1/ + +The following attributes are exposed to the language runtime: + +.. csv-table:: + + :header: Attribute, Type, Description + :widths: 1, 1, 2 + + request.path, string, The path portion of the URL + request.url_path, string, The path portion of the URL without the query string + request.host, string, The host portion of the URL + request.scheme, string, The scheme portion of the URL + request.method, string, Request method + request.headers, map, All request headers + request.referer, string, Referer request header + request.useragent, string, User agent request header + request.time, timestamp, Time of the first byte received + request.duration, duration, Total duration of the request + request.id, string, Request ID + request.size, int, Size of the request body + request.total_size, int, Total size of the request including the headers + response.code, int, Response HTTP status code + response.headers, map, All response headers + response.trailers, map, All response trailers + response.size, int, Size of the response body + source.address, string, Downstream connection remote address + source.port, int, Downstream connection remote port + destination.address, string, Downstream connection local address + destination.port, int, Downstream connection local port + metadata, :ref:`Metadata`, Dynamic metadata + connection.mtls, bool, Indicates whether TLS is applied to the downstream connection and the peer ceritificate is presented + connection.requested_server_name, string, Requested server name in the downstream TLS connection + connection.tls_version, string, TLS version of the downstream TLS connection + upstream.address, string, Upstream connection remote address + upstream.port, int, Upstream connection remote port + upstream.mtls, bool, Indicates whether TLS is applied to the upstream connection and the peer ceritificate is presented + + +Most attributes are optional and will provide the default values based on the type of the attribute. +CEL supports presence checks for attributes and maps using `has()` syntax, e.g. +`has(request.referer)`. diff --git a/source/extensions/filters/common/expr/context.cc b/source/extensions/filters/common/expr/context.cc index 5ee1e91e69b3..cf2e3a1b0642 100644 --- a/source/extensions/filters/common/expr/context.cc +++ b/source/extensions/filters/common/expr/context.cc @@ -110,22 +110,41 @@ absl::optional ConnectionWrapper::operator[](CelValue key) const { return {}; } auto value = key.StringOrDie().value(); - if (value == UpstreamAddress) { + if (value == MTLS) { + return CelValue::CreateBool(info_.downstreamSslConnection() != nullptr && + info_.downstreamSslConnection()->peerCertificatePresented()); + } else if (value == RequestedServerName) { + return CelValue::CreateString(info_.requestedServerName()); + } + + if (info_.downstreamSslConnection() != nullptr) { + if (value == TLSVersion) { + return CelValue::CreateString(info_.downstreamSslConnection()->tlsVersion()); + } + } + + return {}; +} + +absl::optional UpstreamWrapper::operator[](CelValue key) const { + if (!key.IsString()) { + return {}; + } + auto value = key.StringOrDie().value(); + if (value == Address) { auto upstream_host = info_.upstreamHost(); if (upstream_host != nullptr && upstream_host->address() != nullptr) { return CelValue::CreateString(upstream_host->address()->asStringView()); } - } else if (value == UpstreamPort) { + } else if (value == Port) { auto upstream_host = info_.upstreamHost(); if (upstream_host != nullptr && upstream_host->address() != nullptr && upstream_host->address()->ip() != nullptr) { return CelValue::CreateInt64(upstream_host->address()->ip()->port()); } } else if (value == MTLS) { - return CelValue::CreateBool(info_.downstreamSslConnection() != nullptr && - info_.downstreamSslConnection()->peerCertificatePresented()); - } else if (value == RequestedServerName) { - return CelValue::CreateString(info_.requestedServerName()); + return CelValue::CreateBool(info_.upstreamSslConnection() != nullptr && + info_.upstreamSslConnection()->peerCertificatePresented()); } return {}; diff --git a/source/extensions/filters/common/expr/context.h b/source/extensions/filters/common/expr/context.h index 7b59c41a5a10..3d3ecfb4c4f1 100644 --- a/source/extensions/filters/common/expr/context.h +++ b/source/extensions/filters/common/expr/context.h @@ -40,10 +40,9 @@ constexpr absl::string_view Metadata = "metadata"; // Connection properties constexpr absl::string_view Connection = "connection"; -constexpr absl::string_view UpstreamAddress = "upstream_address"; -constexpr absl::string_view UpstreamPort = "upstream_port"; constexpr absl::string_view MTLS = "mtls"; constexpr absl::string_view RequestedServerName = "requested_server_name"; +constexpr absl::string_view TLSVersion = "tls_version"; // Source properties constexpr absl::string_view Source = "source"; @@ -53,6 +52,9 @@ constexpr absl::string_view Port = "port"; // Destination properties constexpr absl::string_view Destination = "destination"; +// Upstream properties +constexpr absl::string_view Upstream = "upstream"; + class RequestWrapper; class HeadersWrapper : public google::api::expr::runtime::CelMap { @@ -112,6 +114,15 @@ class ConnectionWrapper : public BaseWrapper { const StreamInfo::StreamInfo& info_; }; +class UpstreamWrapper : public BaseWrapper { +public: + UpstreamWrapper(const StreamInfo::StreamInfo& info) : info_(info) {} + absl::optional operator[](CelValue key) const override; + +private: + const StreamInfo::StreamInfo& info_; +}; + class PeerWrapper : public BaseWrapper { public: PeerWrapper(const StreamInfo::StreamInfo& info, bool local) : info_(info), local_(local) {} diff --git a/source/extensions/filters/common/expr/evaluator.cc b/source/extensions/filters/common/expr/evaluator.cc index bd25da52975f..0fb973a3689a 100644 --- a/source/extensions/filters/common/expr/evaluator.cc +++ b/source/extensions/filters/common/expr/evaluator.cc @@ -51,12 +51,14 @@ absl::optional evaluate(const Expression& expr, Protobuf::Arena* arena const RequestWrapper request(request_headers, info); const ResponseWrapper response(response_headers, response_trailers, info); const ConnectionWrapper connection(info); + const UpstreamWrapper upstream(info); const PeerWrapper source(info, false); const PeerWrapper destination(info, true); activation.InsertValue(Request, CelValue::CreateMap(&request)); activation.InsertValue(Response, CelValue::CreateMap(&response)); activation.InsertValue(Metadata, CelValue::CreateMessage(&info.dynamicMetadata(), arena)); activation.InsertValue(Connection, CelValue::CreateMap(&connection)); + activation.InsertValue(Upstream, CelValue::CreateMap(&upstream)); activation.InsertValue(Source, CelValue::CreateMap(&source)); activation.InsertValue(Destination, CelValue::CreateMap(&destination)); diff --git a/test/extensions/filters/common/expr/context_test.cc b/test/extensions/filters/common/expr/context_test.cc index d7ac41229d3d..1ca2669d5969 100644 --- a/test/extensions/filters/common/expr/context_test.cc +++ b/test/extensions/filters/common/expr/context_test.cc @@ -256,8 +256,10 @@ TEST(Context, ConnectionAttributes) { NiceMock info; std::shared_ptr> host( new NiceMock()); - auto connection_info = std::make_shared>(); + auto downstream_ssl_info = std::make_shared>(); + auto upstream_ssl_info = std::make_shared>(); ConnectionWrapper connection(info); + UpstreamWrapper upstream(info); PeerWrapper source(info, false); PeerWrapper destination(info, true); @@ -270,10 +272,14 @@ TEST(Context, ConnectionAttributes) { const std::string sni_name = "kittens.com"; EXPECT_CALL(info, downstreamLocalAddress()).WillRepeatedly(ReturnRef(local)); EXPECT_CALL(info, downstreamRemoteAddress()).WillRepeatedly(ReturnRef(remote)); - EXPECT_CALL(info, downstreamSslConnection()).WillRepeatedly(Return(connection_info)); + EXPECT_CALL(info, downstreamSslConnection()).WillRepeatedly(Return(downstream_ssl_info)); + EXPECT_CALL(info, upstreamSslConnection()).WillRepeatedly(Return(upstream_ssl_info)); EXPECT_CALL(info, upstreamHost()).WillRepeatedly(Return(host)); EXPECT_CALL(info, requestedServerName()).WillRepeatedly(ReturnRef(sni_name)); - EXPECT_CALL(*connection_info, peerCertificatePresented()).WillRepeatedly(Return(true)); + EXPECT_CALL(*downstream_ssl_info, peerCertificatePresented()).WillRepeatedly(Return(true)); + EXPECT_CALL(*upstream_ssl_info, peerCertificatePresented()).WillRepeatedly(Return(true)); + const std::string tls_version = "TLSv1"; + EXPECT_CALL(*downstream_ssl_info, tlsVersion()).WillRepeatedly(ReturnRef(tls_version)); EXPECT_CALL(*host, address()).WillRepeatedly(Return(upstream)); { @@ -325,19 +331,26 @@ TEST(Context, ConnectionAttributes) { } { - auto value = connection[CelValue::CreateString(UpstreamAddress)]; + auto value = upstream[CelValue::CreateString(Address)]; EXPECT_TRUE(value.has_value()); ASSERT_TRUE(value.value().IsString()); EXPECT_EQ("10.1.2.3:679", value.value().StringOrDie().value()); } { - auto value = connection[CelValue::CreateString(UpstreamPort)]; + auto value = upstream[CelValue::CreateString(Port)]; EXPECT_TRUE(value.has_value()); ASSERT_TRUE(value.value().IsInt64()); EXPECT_EQ(679, value.value().Int64OrDie()); } + { + auto value = upstream[CelValue::CreateString(MTLS)]; + EXPECT_TRUE(value.has_value()); + ASSERT_TRUE(value.value().IsBool()); + EXPECT_TRUE(value.value().BoolOrDie()); + } + { auto value = connection[CelValue::CreateString(MTLS)]; EXPECT_TRUE(value.has_value()); @@ -351,6 +364,13 @@ TEST(Context, ConnectionAttributes) { ASSERT_TRUE(value.value().IsString()); EXPECT_EQ(sni_name, value.value().StringOrDie().value()); } + + { + auto value = connection[CelValue::CreateString(TLSVersion)]; + EXPECT_TRUE(value.has_value()); + ASSERT_TRUE(value.value().IsString()); + EXPECT_EQ(tls_version, value.value().StringOrDie().value()); + } } } // namespace diff --git a/tools/spelling_dictionary.txt b/tools/spelling_dictionary.txt index 530f79bb325a..0cd9883bd93b 100644 --- a/tools/spelling_dictionary.txt +++ b/tools/spelling_dictionary.txt @@ -18,6 +18,7 @@ BSON CAS CB CBs +CEL CDS CHACHA CHLO From b8f40c9963a76900774d56562734765aa0ee065e Mon Sep 17 00:00:00 2001 From: Kuat Yessenov Date: Thu, 12 Sep 2019 16:49:06 -0700 Subject: [PATCH 2/4] fixes Signed-off-by: Kuat Yessenov --- .../arch_overview/security/rbac_filter.rst | 69 +++++++++---------- .../filters/common/expr/context_test.cc | 8 +-- 2 files changed, 38 insertions(+), 39 deletions(-) diff --git a/docs/root/intro/arch_overview/security/rbac_filter.rst b/docs/root/intro/arch_overview/security/rbac_filter.rst index 4dd925dffe25..366de99a8653 100644 --- a/docs/root/intro/arch_overview/security/rbac_filter.rst +++ b/docs/root/intro/arch_overview/security/rbac_filter.rst @@ -61,40 +61,39 @@ whether the request path starts with `/v1/`: The following attributes are exposed to the language runtime: .. csv-table:: - - :header: Attribute, Type, Description - :widths: 1, 1, 2 - - request.path, string, The path portion of the URL - request.url_path, string, The path portion of the URL without the query string - request.host, string, The host portion of the URL - request.scheme, string, The scheme portion of the URL - request.method, string, Request method - request.headers, map, All request headers - request.referer, string, Referer request header - request.useragent, string, User agent request header - request.time, timestamp, Time of the first byte received - request.duration, duration, Total duration of the request - request.id, string, Request ID - request.size, int, Size of the request body - request.total_size, int, Total size of the request including the headers - response.code, int, Response HTTP status code - response.headers, map, All response headers - response.trailers, map, All response trailers - response.size, int, Size of the response body - source.address, string, Downstream connection remote address - source.port, int, Downstream connection remote port - destination.address, string, Downstream connection local address - destination.port, int, Downstream connection local port - metadata, :ref:`Metadata`, Dynamic metadata - connection.mtls, bool, Indicates whether TLS is applied to the downstream connection and the peer ceritificate is presented - connection.requested_server_name, string, Requested server name in the downstream TLS connection - connection.tls_version, string, TLS version of the downstream TLS connection - upstream.address, string, Upstream connection remote address - upstream.port, int, Upstream connection remote port - upstream.mtls, bool, Indicates whether TLS is applied to the upstream connection and the peer ceritificate is presented - - -Most attributes are optional and will provide the default values based on the type of the attribute. + :header: Attribute, Type, Description + :widths: 1, 1, 2 + + request.path, string, The path portion of the URL + request.url_path, string, The path portion of the URL without the query string + request.host, string, The host portion of the URL + request.scheme, string, The scheme portion of the URL + request.method, string, Request method + request.headers, `map`, All request headers + request.referer, string, Referer request header + request.useragent, string, User agent request header + request.time, timestamp, Time of the first byte received + request.duration, duration, Total duration of the request + request.id, string, Request ID + request.size, int, Size of the request body + request.total_size, int, Total size of the request including the headers + response.code, int, Response HTTP status code + response.headers, `map`, All response headers + response.trailers, `map`, All response trailers + response.size, int, Size of the response body + source.address, string, Downstream connection remote address + source.port, int, Downstream connection remote port + destination.address, string, Downstream connection local address + destination.port, int, Downstream connection local port + metadata, :ref:`Metadata`, Dynamic metadata + connection.mtls, bool, Indicates whether TLS is applied to the downstream connection and the peer ceritificate is presented + connection.requested_server_name, string, Requested server name in the downstream TLS connection + connection.tls_version, string, TLS version of the downstream TLS connection + upstream.address, string, Upstream connection remote address + upstream.port, int, Upstream connection remote port + upstream.mtls, bool, Indicates whether TLS is applied to the upstream connection and the peer ceritificate is presented + + +Most attributes are optional and provide the default value based on the type of the attribute. CEL supports presence checks for attributes and maps using `has()` syntax, e.g. `has(request.referer)`. diff --git a/test/extensions/filters/common/expr/context_test.cc b/test/extensions/filters/common/expr/context_test.cc index 1ca2669d5969..7f11fa75bb55 100644 --- a/test/extensions/filters/common/expr/context_test.cc +++ b/test/extensions/filters/common/expr/context_test.cc @@ -254,7 +254,7 @@ TEST(Context, ResponseAttributes) { TEST(Context, ConnectionAttributes) { NiceMock info; - std::shared_ptr> host( + std::shared_ptr> upstream_host( new NiceMock()); auto downstream_ssl_info = std::make_shared>(); auto upstream_ssl_info = std::make_shared>(); @@ -267,20 +267,20 @@ TEST(Context, ConnectionAttributes) { Network::Utility::parseInternetAddress("1.2.3.4", 123, false); Network::Address::InstanceConstSharedPtr remote = Network::Utility::parseInternetAddress("10.20.30.40", 456, false); - Network::Address::InstanceConstSharedPtr upstream = + Network::Address::InstanceConstSharedPtr upstream_address = Network::Utility::parseInternetAddress("10.1.2.3", 679, false); const std::string sni_name = "kittens.com"; EXPECT_CALL(info, downstreamLocalAddress()).WillRepeatedly(ReturnRef(local)); EXPECT_CALL(info, downstreamRemoteAddress()).WillRepeatedly(ReturnRef(remote)); EXPECT_CALL(info, downstreamSslConnection()).WillRepeatedly(Return(downstream_ssl_info)); EXPECT_CALL(info, upstreamSslConnection()).WillRepeatedly(Return(upstream_ssl_info)); - EXPECT_CALL(info, upstreamHost()).WillRepeatedly(Return(host)); + EXPECT_CALL(info, upstreamHost()).WillRepeatedly(Return(upstream_host)); EXPECT_CALL(info, requestedServerName()).WillRepeatedly(ReturnRef(sni_name)); EXPECT_CALL(*downstream_ssl_info, peerCertificatePresented()).WillRepeatedly(Return(true)); EXPECT_CALL(*upstream_ssl_info, peerCertificatePresented()).WillRepeatedly(Return(true)); const std::string tls_version = "TLSv1"; EXPECT_CALL(*downstream_ssl_info, tlsVersion()).WillRepeatedly(ReturnRef(tls_version)); - EXPECT_CALL(*host, address()).WillRepeatedly(Return(upstream)); + EXPECT_CALL(*upstream_host, address()).WillRepeatedly(Return(upstream_address)); { auto value = connection[CelValue::CreateString(Undefined)]; From 0f8092fc9af86f58653506fb309a9028c121f4d8 Mon Sep 17 00:00:00 2001 From: Kuat Yessenov Date: Thu, 12 Sep 2019 16:57:22 -0700 Subject: [PATCH 3/4] ugh Signed-off-by: Kuat Yessenov --- docs/root/intro/arch_overview/security/rbac_filter.rst | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/docs/root/intro/arch_overview/security/rbac_filter.rst b/docs/root/intro/arch_overview/security/rbac_filter.rst index 366de99a8653..27a09432b618 100644 --- a/docs/root/intro/arch_overview/security/rbac_filter.rst +++ b/docs/root/intro/arch_overview/security/rbac_filter.rst @@ -69,7 +69,7 @@ The following attributes are exposed to the language runtime: request.host, string, The host portion of the URL request.scheme, string, The scheme portion of the URL request.method, string, Request method - request.headers, `map`, All request headers + request.headers, string map, All request headers request.referer, string, Referer request header request.useragent, string, User agent request header request.time, timestamp, Time of the first byte received @@ -78,8 +78,8 @@ The following attributes are exposed to the language runtime: request.size, int, Size of the request body request.total_size, int, Total size of the request including the headers response.code, int, Response HTTP status code - response.headers, `map`, All response headers - response.trailers, `map`, All response trailers + response.headers, string map, All response headers + response.trailers, string map, All response trailers response.size, int, Size of the response body source.address, string, Downstream connection remote address source.port, int, Downstream connection remote port From 59a2b8a7025dc8116cc4a01d27ccd6e969b52a5b Mon Sep 17 00:00:00 2001 From: Kuat Yessenov Date: Fri, 13 Sep 2019 15:00:07 -0700 Subject: [PATCH 4/4] typo Signed-off-by: Kuat Yessenov --- docs/root/intro/arch_overview/security/rbac_filter.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/root/intro/arch_overview/security/rbac_filter.rst b/docs/root/intro/arch_overview/security/rbac_filter.rst index 27a09432b618..138f2a6b9ed7 100644 --- a/docs/root/intro/arch_overview/security/rbac_filter.rst +++ b/docs/root/intro/arch_overview/security/rbac_filter.rst @@ -39,7 +39,7 @@ for testing a rule before applying in production. Condition --------- -In addition to the pre-defined permissions and prinipals, a policy may optionally provide an +In addition to the pre-defined permissions and principals, a policy may optionally provide an authorization condition written in the `Common Expression Language `_. The condition specifies an extra clause that must be satisfied for the policy to match. For example, the following condition checks