-
Notifications
You must be signed in to change notification settings - Fork 5.5k
filter: add conditions to access control filter #7716
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 2 commits
3d4f7aa
beb197c
e914f46
ec74571
97efd6d
f3668b2
dca0933
8e80999
970b361
ebf4c4a
d1fd462
1235c7d
41220da
982e3fd
c31b4a7
e309dd8
6cdbe8e
f934dda
bf99900
6861b98
b27790a
9393ca9
5d44fea
784a970
c081695
9f812b5
184fe6b
811fba6
64c9201
684d473
08fe702
580a79f
8f72a50
a2a9a7f
f86eadf
b9c755c
532ed67
c1f1890
aed67b6
76b6788
a0daefa
753b352
8df3414
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,15 @@ | ||
| load("@envoy_api//bazel:api_build_system.bzl", "api_proto_library_internal") | ||
|
|
||
| licenses(["notice"]) # Apache 2 | ||
|
|
||
| api_proto_library_internal( | ||
| name = "abac", | ||
| srcs = ["abac.proto"], | ||
| external_cc_proto_deps = [ | ||
| "@com_google_googleapis//google/api/expr/v1alpha1:syntax_cc_proto", | ||
| ], | ||
| external_proto_deps = [ | ||
| "@com_google_googleapis//google/api/expr/v1alpha1:syntax_proto", | ||
| ], | ||
| require_py = False, | ||
| ) |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,18 @@ | ||
| syntax = "proto3"; | ||
|
|
||
| package envoy.config.filter.http.abac.v2alpha; | ||
|
|
||
| option java_outer_classname = "AbacProto"; | ||
| option java_multiple_files = true; | ||
| option java_package = "io.envoyproxy.envoy.config.filter.http.abac.v2alpha"; | ||
| option go_package = "v2alpha"; | ||
|
|
||
| import "google/api/expr/v1alpha1/syntax.proto"; | ||
|
|
||
| // [#protodoc-title: ABAC] | ||
| // Attribute-Based Access Control. | ||
|
|
||
| // ABAC filter config. | ||
| message ABAC { | ||
| google.api.expr.v1alpha1.Expr expr = 1; | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -24,10 +24,10 @@ REPOSITORY_LOCATIONS = dict( | |
| urls = ["https://commondatastorage.googleapis.com/chromium-boringssl-docs/fips/boringssl-66005f41fbc3529ffe8d007708756720529da20d.tar.xz"], | ||
| ), | ||
| com_google_absl = dict( | ||
| sha256 = "7ddf863ddced6fa5bf7304103f9c7aa619c20a2fcf84475512c8d3834b9d14fa", | ||
| strip_prefix = "abseil-cpp-61c9bf3e3e1c28a4aa6d7f1be4b37fd473bb5529", | ||
| # 2019-06-05 | ||
| urls = ["https://github.com/abseil/abseil-cpp/archive/61c9bf3e3e1c28a4aa6d7f1be4b37fd473bb5529.tar.gz"], | ||
| sha256 = "05a97ad5bb123ee3f6d65c7b06f6d7de5ce62e4fb971cbe4b5e391dd69704bb7", | ||
| strip_prefix = "abseil-cpp-ad1485c8986246b2ae9105e512738d0e97aec887", | ||
| # 2019-07-24 | ||
| urls = ["https://github.com/abseil/abseil-cpp/archive/ad1485c8986246b2ae9105e512738d0e97aec887.tar.gz"], | ||
| ), | ||
| com_github_apache_thrift = dict( | ||
| sha256 = "7d59ac4fdcb2c58037ebd4a9da5f9a49e3e034bf75b3f26d9fe48ba3d8806e6b", | ||
|
|
@@ -247,4 +247,27 @@ REPOSITORY_LOCATIONS = dict( | |
| sha256 = "fcdebf54c89d839ffa7eefae166c8e4b551c765559db13ff15bff98047f344fb", | ||
| urls = ["https://storage.googleapis.com/quiche-envoy-integration/2a930469533c3b541443488a629fe25cd8ff53d0.tar.gz"], | ||
| ), | ||
| com_github_gflags_gflags = dict( | ||
| sha256 = "6e16c8bc91b1310a44f3965e616383dbda48f83e8c1eaa2370a215057b00cabe", | ||
| strip_prefix = "gflags-77592648e3f3be87d6c7123eb81cbad75f9aef5a", | ||
| urls = [ | ||
| "https://mirror.bazel.build/github.com/gflags/gflags/archive/77592648e3f3be87d6c7123eb81cbad75f9aef5a.tar.gz", | ||
| "https://github.com/gflags/gflags/archive/77592648e3f3be87d6c7123eb81cbad75f9aef5a.tar.gz", | ||
| ], | ||
| ), | ||
| com_google_glog = dict( | ||
| sha256 = "819cb075d6b02b8e9c9c77c2be1d55cef7fec47e7c94359d2626f46268ae67bf", | ||
| strip_prefix = "glog-ba8a9f6952d04d1403b97df24e6836227751454e", | ||
| urls = ["https://github.com/google/glog/archive/ba8a9f6952d04d1403b97df24e6836227751454e.tar.gz"], | ||
| ), | ||
| com_google_cel_cpp = dict( | ||
| sha256 = "37df0e66c84ddffe7bbac6e659856f48b34dbd9e1e869f57275493a20b271d11", | ||
| strip_prefix = "cel-cpp-592129c73f70de6d5ff8977aa94e030a1a9c4e8a", | ||
| urls = ["https://github.com/google/cel-cpp/archive/592129c73f70de6d5ff8977aa94e030a1a9c4e8a.tar.gz"], | ||
| ), | ||
| com_google_re2 = dict( | ||
|
kyessenov marked this conversation as resolved.
Outdated
|
||
| sha256 = "f31db9cd224d018a7e4fe88ef84aaa874b0b3ed91d4d98ee5a1531101d3fdc64", | ||
| strip_prefix = "re2-87e2ad45e7b18738e1551474f7ee5886ff572059", | ||
| urls = ["https://github.com/google/re2/archive/87e2ad45e7b18738e1551474f7ee5886ff572059.tar.gz"], | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @kyessenov qq: does cel-cpp rely on specific commit of re2? Asking because it might conflict with #7878, or latest release (2019-08-01) is fine? cc @mattklein123
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There should be no difference between which version is used. I think I chose the latest version which I started this PR.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. OK that's fine.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I was going to ask the same question. I'll switch this back a release version of re2 on a subsequent dependency PR.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I will just fix this when I merge master.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thanks. Happy to help if necessary. Google3 doesn't really have versions for its repositories, and the upstream cel-cpp is continuously tested against head. |
||
| ), | ||
| ) | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,37 @@ | ||
| licenses(["notice"]) # Apache 2 | ||
|
|
||
| load( | ||
| "//bazel:envoy_build_system.bzl", | ||
| "envoy_cc_library", | ||
| "envoy_package", | ||
| ) | ||
|
|
||
| envoy_package() | ||
|
|
||
| envoy_cc_library( | ||
| name = "config", | ||
| srcs = ["config.cc"], | ||
| hdrs = ["config.h"], | ||
| deps = [ | ||
| ":abac_filter_lib", | ||
| "//include/envoy/registry", | ||
| "//source/extensions/filters/http:well_known_names", | ||
| "//source/extensions/filters/http/common:factory_base_lib", | ||
| "@com_google_cel_cpp//eval/public:builtin_func_registrar", | ||
| "@com_google_cel_cpp//eval/public:cel_expr_builder_factory", | ||
| ], | ||
| ) | ||
|
|
||
| envoy_cc_library( | ||
| name = "abac_filter_lib", | ||
| srcs = ["abac_filter.cc"], | ||
| hdrs = ["abac_filter.h"], | ||
| deps = [ | ||
| "//include/envoy/http:filter_interface", | ||
| #"//include/envoy/stats:stats_macros", | ||
| "//source/common/http:utility_lib", | ||
| "//source/common/protobuf", | ||
| "@com_google_cel_cpp//eval/public:cel_expression", | ||
| "@envoy_api//envoy/config/filter/http/abac/v2alpha:abac_cc", | ||
| ], | ||
| ) |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,171 @@ | ||
| #include "extensions/filters/http/abac/abac_filter.h" | ||
|
|
||
| #include "common/protobuf/protobuf.h" | ||
|
|
||
| namespace Envoy { | ||
| namespace Extensions { | ||
| namespace HttpFilters { | ||
| namespace ABACFilter { | ||
|
|
||
| using CelValue = google::api::expr::runtime::CelValue; | ||
| using CelMap = google::api::expr::runtime::CelMap; | ||
| using CelList = google::api::expr::runtime::CelList; | ||
|
|
||
| namespace { | ||
|
|
||
| constexpr absl::string_view kMetadata = "metadata"; | ||
|
kyessenov marked this conversation as resolved.
Outdated
|
||
| constexpr absl::string_view kConnection = "connection"; | ||
| constexpr absl::string_view kRequestedServerName = "requested_server_name"; | ||
| constexpr absl::string_view kLocalAddress = "local_address"; | ||
| constexpr absl::string_view kRemoteAddress = "remote_address"; | ||
| constexpr absl::string_view kCertificatePresented = "certificate_presented"; | ||
| constexpr absl::string_view kSubjectLocalCertificate = "subject_local_certificate"; | ||
| constexpr absl::string_view kIssuerPeerCertificate = "issuer_peer_certificate"; | ||
| constexpr absl::string_view kSubjectPeerCertificate = "subject_peer_certificate"; | ||
| constexpr absl::string_view kTlsVersion = "tls_version"; | ||
| constexpr absl::string_view kRequest = "request"; | ||
| constexpr absl::string_view kPath = "path"; | ||
| constexpr absl::string_view kHost = "host"; | ||
| constexpr absl::string_view kScheme = "scheme"; | ||
| constexpr absl::string_view kMethod = "method"; | ||
| constexpr absl::string_view kReferer = "referer"; | ||
| constexpr absl::string_view kHeaders = "headers"; | ||
| constexpr absl::string_view kTime = "time"; | ||
|
|
||
| class ConnectionWrapper : public CelMap { | ||
| public: | ||
| ConnectionWrapper(const StreamInfo::StreamInfo& info) : info_(info) {} | ||
|
|
||
| absl::optional<CelValue> operator[](CelValue key) const override { | ||
| if (!key.IsString()) { | ||
| return {}; | ||
| } | ||
| auto value = key.StringOrDie().value(); | ||
| if (value == kRequestedServerName) { | ||
|
kyessenov marked this conversation as resolved.
Outdated
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. can those be something like flat_hash_map<string_view, std::function>? I think we do similar in access_log_formatter.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'll need to measure it. std::function are surprisingly expensive in terms of unexpected allocations. The way it's done here is trivially allocation-free.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You don't have to create a map every time, have a static map like: flat_hash_map<string_view, std::function<CelValue(const StreamInfo&)>>, so you just call them every time, not allocating.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm not sure that's more readable. Anyways, I need to measure before making the change.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Lizan, is this what you had in mind https://github.com/kyessenov/envoy/blob/abac_filter_feedback/test/extensions/filters/common/expr/context_speed_test.cc#L34?
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. yep, thanks for testing that out.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. After adding all functions to the flat_hash_map, here is the performance comparison: So it's roughly the same (I couldn't trivially wire a memory allocation profiler), and it comes down to readability and code style. WDYT? Should we change it to a hashmap of functions? |
||
| return CelValue::CreateString(info_.requestedServerName()); | ||
| } else if (value == kLocalAddress) { | ||
| return CelValue::CreateString(info_.downstreamLocalAddress()->asString()); | ||
| } else if (value == kRemoteAddress) { | ||
| return CelValue::CreateString(info_.downstreamRemoteAddress()->asString()); | ||
| } | ||
|
|
||
| auto ssl_info = info_.downstreamSslConnection(); | ||
| if (ssl_info != nullptr) { | ||
| if (value == kCertificatePresented) { | ||
| return CelValue::CreateBool(ssl_info->peerCertificatePresented()); | ||
| } else if (value == kSubjectLocalCertificate) { | ||
| return CelValue::CreateString(ssl_info->subjectLocalCertificate()); | ||
|
kyessenov marked this conversation as resolved.
Outdated
|
||
| } else if (value == kIssuerPeerCertificate) { | ||
| return CelValue::CreateString(ssl_info->issuerPeerCertificate()); | ||
| } else if (value == kSubjectPeerCertificate) { | ||
| return CelValue::CreateString(ssl_info->subjectPeerCertificate()); | ||
| } else if (value == kTlsVersion) { | ||
| return CelValue::CreateString(ssl_info->tlsVersion()); | ||
| } | ||
| } | ||
| return {}; | ||
| } | ||
| int size() const override { return 0; } | ||
| bool empty() const override { return false; } | ||
| const CelList* ListKeys() const override { return nullptr; } | ||
|
|
||
| private: | ||
| const StreamInfo::StreamInfo& info_; | ||
| }; | ||
|
|
||
| class HeadersWrapper : public CelMap { | ||
| public: | ||
| HeadersWrapper(const Http::HeaderMap& headers) : headers_(headers) {} | ||
|
|
||
| absl::optional<CelValue> operator[](CelValue key) const override { | ||
| if (!key.IsString()) { | ||
| return {}; | ||
| } | ||
| auto out = headers_.get(Http::LowerCaseString(std::string(key.StringOrDie().value()))); | ||
| if (out == nullptr) { | ||
| return {}; | ||
| } | ||
| return CelValue::CreateString(out->value().getStringView()); | ||
| } | ||
| int size() const override { return headers_.size(); } | ||
| bool empty() const override { return headers_.empty(); } | ||
| const CelList* ListKeys() const override { return nullptr; } | ||
| const Http::HeaderMap& headers_; | ||
| }; | ||
|
|
||
| class RequestWrapper : public CelMap { | ||
| public: | ||
| RequestWrapper(const Http::HeaderMap& headers, const StreamInfo::StreamInfo& info) | ||
| : wrapper_(headers), info_(info) {} | ||
|
|
||
| absl::optional<CelValue> operator[](CelValue key) const override { | ||
| if (!key.IsString()) { | ||
| return {}; | ||
| } | ||
| auto value = key.StringOrDie().value(); | ||
| if (value == kPath) { | ||
| return CelValue::CreateString(wrapper_.headers_.Path()->value().getStringView()); | ||
| } else if (value == kHost) { | ||
| return CelValue::CreateString(wrapper_.headers_.Host()->value().getStringView()); | ||
| } else if (value == kScheme) { | ||
| return CelValue::CreateString(wrapper_.headers_.Scheme()->value().getStringView()); | ||
| } else if (value == kMethod) { | ||
| return CelValue::CreateString(wrapper_.headers_.Method()->value().getStringView()); | ||
| } else if (value == kReferer) { | ||
| return CelValue::CreateString(wrapper_.headers_.Referer()->value().getStringView()); | ||
| } else if (value == kHeaders) { | ||
| return CelValue::CreateMap(&wrapper_); | ||
| } else if (value == kTime) { | ||
| return CelValue::CreateTimestamp(absl::FromChrono(info_.startTime())); | ||
| } | ||
| return {}; | ||
| } | ||
|
|
||
| int size() const override { return 0; } | ||
| bool empty() const override { return false; } | ||
| const CelList* ListKeys() const override { return nullptr; } | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why are all listkeys returning null?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Not implemented. For wrappers, it's undesired. For headers, it might be useful for comprehension operations ( |
||
|
|
||
| private: | ||
| const HeadersWrapper wrapper_; | ||
| const StreamInfo::StreamInfo& info_; | ||
| }; | ||
|
|
||
| } // namespace | ||
|
|
||
| Http::FilterHeadersStatus AttributeBasedAccessControlFilter::decodeHeaders(Http::HeaderMap& headers, | ||
| bool) { | ||
| Protobuf::Arena arena; | ||
| google::api::expr::runtime::Activation activation; | ||
| activation.InsertValue( | ||
| kMetadata, CelValue::CreateMessage(&callbacks_->streamInfo().dynamicMetadata(), &arena)); | ||
| const ConnectionWrapper connection(callbacks_->streamInfo()); | ||
| activation.InsertValue(kConnection, CelValue::CreateMap(&connection)); | ||
| const RequestWrapper request(headers, callbacks_->streamInfo()); | ||
| activation.InsertValue(kRequest, CelValue::CreateMap(&request)); | ||
|
|
||
| auto eval_status = expr_->Evaluate(activation, &arena); | ||
| if (!eval_status.ok()) { | ||
| ENVOY_LOG(debug, "evaluation failed: {}", eval_status.message()); | ||
| return Http::FilterHeadersStatus::StopIteration; | ||
|
kyessenov marked this conversation as resolved.
Outdated
|
||
| } | ||
|
|
||
| auto result = eval_status.ValueOrDie(); | ||
| if (result.IsBool() && result.BoolOrDie()) { | ||
| return Http::FilterHeadersStatus::Continue; | ||
| } | ||
|
|
||
| if (result.IsError()) { | ||
| ENVOY_LOG(debug, "evaluation error: {}", result.ErrorOrDie()->message()); | ||
| } else if (result.IsString()) { | ||
| ENVOY_LOG(debug, "evaluation resulted in a string: {}", result.StringOrDie().value()); | ||
| } else { | ||
| ENVOY_LOG(debug, "expression did not evaluate to 'true': type {}", | ||
| CelValue::TypeName(result.type())); | ||
| } | ||
| return Http::FilterHeadersStatus::StopIteration; | ||
|
kyessenov marked this conversation as resolved.
Outdated
|
||
| } | ||
|
|
||
| } // namespace ABACFilter | ||
| } // namespace HttpFilters | ||
| } // namespace Extensions | ||
| } // namespace Envoy | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,51 @@ | ||
| #pragma once | ||
|
|
||
| #include <memory> | ||
|
|
||
| #include "envoy/config/filter/http/abac/v2alpha/abac.pb.h" | ||
| #include "envoy/http/filter.h" | ||
|
|
||
| #include "common/common/logger.h" | ||
|
|
||
| #include "eval/public/cel_expression.h" | ||
|
|
||
| namespace Envoy { | ||
| namespace Extensions { | ||
| namespace HttpFilters { | ||
| namespace ABACFilter { | ||
|
|
||
| /** | ||
| * A filter that provides attribute-based access control authorization for HTTP requests. | ||
| */ | ||
| class AttributeBasedAccessControlFilter : public Http::StreamDecoderFilter, | ||
| public Logger::Loggable<Logger::Id::abac> { | ||
| public: | ||
| AttributeBasedAccessControlFilter(std::shared_ptr<google::api::expr::runtime::CelExpression> expr) | ||
| : expr_(expr) {} | ||
|
|
||
| Http::FilterHeadersStatus decodeHeaders(Http::HeaderMap& headers, bool) override; | ||
|
|
||
| Http::FilterDataStatus decodeData(Buffer::Instance&, bool) override { | ||
| return Http::FilterDataStatus::Continue; | ||
| } | ||
|
|
||
| Http::FilterTrailersStatus decodeTrailers(Http::HeaderMap&) override { | ||
| return Http::FilterTrailersStatus::Continue; | ||
| } | ||
|
|
||
| void setDecoderFilterCallbacks(Http::StreamDecoderFilterCallbacks& callbacks) override { | ||
| callbacks_ = &callbacks; | ||
| } | ||
|
|
||
| // Http::StreamFilterBase | ||
| void onDestroy() override {} | ||
|
|
||
| private: | ||
| Http::StreamDecoderFilterCallbacks* callbacks_{}; | ||
| std::shared_ptr<google::api::expr::runtime::CelExpression> expr_; | ||
| }; | ||
|
|
||
| } // namespace ABACFilter | ||
| } // namespace HttpFilters | ||
| } // namespace Extensions | ||
| } // namespace Envoy |
Uh oh!
There was an error while loading. Please reload this page.