From 7b811c1c03c8e55f8bcb5488e672017009ca5de6 Mon Sep 17 00:00:00 2001 From: Kuat Yessenov Date: Thu, 17 Sep 2020 14:38:20 -0700 Subject: [PATCH 1/2] rbac: expose filter state Signed-off-by: Kuat Yessenov --- .../arch_overview/security/rbac_filter.rst | 1 + include/envoy/stream_info/filter_state.h | 8 +++++- source/common/router/string_accessor_impl.h | 2 ++ .../extensions/filters/common/expr/context.cc | 16 +++++++++++ .../extensions/filters/common/expr/context.h | 21 +++++++++++++- .../filters/common/expr/evaluator.cc | 2 ++ test/extensions/filters/common/expr/BUILD | 2 ++ .../filters/common/expr/context_test.cc | 28 +++++++++++++++++++ 8 files changed, 78 insertions(+), 2 deletions(-) diff --git a/docs/root/intro/arch_overview/security/rbac_filter.rst b/docs/root/intro/arch_overview/security/rbac_filter.rst index fc98580e4f84f..bdddca74837c7 100644 --- a/docs/root/intro/arch_overview/security/rbac_filter.rst +++ b/docs/root/intro/arch_overview/security/rbac_filter.rst @@ -90,6 +90,7 @@ The following attributes are exposed to the language runtime: destination.address, string, Downstream connection local address destination.port, int, Downstream connection local port metadata, :ref:`Metadata`, Dynamic metadata + filter_state, map string to bytes, Filter state mapping data names to their serialized string value 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 diff --git a/include/envoy/stream_info/filter_state.h b/include/envoy/stream_info/filter_state.h index ca16c0614fa86..612dc6994ec58 100644 --- a/include/envoy/stream_info/filter_state.h +++ b/include/envoy/stream_info/filter_state.h @@ -101,6 +101,13 @@ class FilterState { return *result; } + /** + * @param data_name the name of the data being looked up (mutable/readonly). + * @return a const reference to the stored data. + * An exception will be thrown if the data does not exist. + */ + virtual const Object* getDataReadOnlyGeneric(absl::string_view data_name) const PURE; + /** * @param data_name the name of the data being looked up (mutable only). * @return a non-const reference to the stored data if and only if the @@ -154,7 +161,6 @@ class FilterState { virtual FilterStateSharedPtr parent() const PURE; protected: - virtual const Object* getDataReadOnlyGeneric(absl::string_view data_name) const PURE; virtual Object* getDataMutableGeneric(absl::string_view data_name) PURE; }; diff --git a/source/common/router/string_accessor_impl.h b/source/common/router/string_accessor_impl.h index 251a714f4b142..d4851e9b1a754 100644 --- a/source/common/router/string_accessor_impl.h +++ b/source/common/router/string_accessor_impl.h @@ -19,6 +19,8 @@ class StringAccessorImpl : public StringAccessor { return message; } + absl::optional serializeAsString() const override { return value_; } + private: std::string value_; }; diff --git a/source/extensions/filters/common/expr/context.cc b/source/extensions/filters/common/expr/context.cc index 17a0bd88a5704..788da6cc47bf9 100644 --- a/source/extensions/filters/common/expr/context.cc +++ b/source/extensions/filters/common/expr/context.cc @@ -233,6 +233,22 @@ absl::optional PeerWrapper::operator[](CelValue key) const { return {}; } +absl::optional FilterStateWrapper::operator[](CelValue key) const { + if (!key.IsString()) { + return {}; + } + auto value = key.StringOrDie().value(); + if (filter_state_.hasDataWithName(value)) { + const StreamInfo::FilterState::Object* object = filter_state_.getDataReadOnlyGeneric(value); + absl::optional serialized = object->serializeAsString(); + if (serialized.has_value()) { + std::string* out = ProtobufWkt::Arena::Create(arena_, serialized.value()); + return CelValue::CreateBytes(out); + } + } + return {}; +} + } // namespace Expr } // namespace Common } // namespace Filters diff --git a/source/extensions/filters/common/expr/context.h b/source/extensions/filters/common/expr/context.h index 70da3d85c30fe..7beb3a04b693d 100644 --- a/source/extensions/filters/common/expr/context.h +++ b/source/extensions/filters/common/expr/context.h @@ -44,6 +44,9 @@ constexpr absl::string_view GrpcStatus = "grpc_status"; // Per-request or per-connection metadata constexpr absl::string_view Metadata = "metadata"; +// Per-request or per-connection filter state +constexpr absl::string_view FilterState = "filter_state"; + // Connection properties constexpr absl::string_view Connection = "connection"; constexpr absl::string_view MTLS = "mtls"; @@ -108,7 +111,14 @@ class BaseWrapper : public google::api::expr::runtime::CelMap, const google::api::expr::runtime::CelList* ListKeys() const override { NOT_IMPLEMENTED_GCOVR_EXCL_LINE; } - CelValue Produce(ProtobufWkt::Arena*) override { return CelValue::CreateMap(this); } + CelValue Produce(ProtobufWkt::Arena* arena) override { + // Producer is unique per evaluation arena since activation is re-created. + arena_ = arena; + return CelValue::CreateMap(this); + } + +protected: + ProtobufWkt::Arena* arena_; }; class RequestWrapper : public BaseWrapper { @@ -174,6 +184,15 @@ class MetadataProducer : public google::api::expr::runtime::CelValueProducer { const envoy::config::core::v3::Metadata& metadata_; }; +class FilterStateWrapper : public BaseWrapper { +public: + FilterStateWrapper(const StreamInfo::FilterState& filter_state) : filter_state_(filter_state) {} + absl::optional operator[](CelValue key) const override; + +private: + const StreamInfo::FilterState& filter_state_; +}; + } // namespace Expr } // namespace Common } // namespace Filters diff --git a/source/extensions/filters/common/expr/evaluator.cc b/source/extensions/filters/common/expr/evaluator.cc index e4920fd21fdab..fee62c0ad9fa1 100644 --- a/source/extensions/filters/common/expr/evaluator.cc +++ b/source/extensions/filters/common/expr/evaluator.cc @@ -25,6 +25,8 @@ ActivationPtr createActivation(const StreamInfo::StreamInfo& info, activation->InsertValueProducer(Destination, std::make_unique(info, true)); activation->InsertValueProducer(Metadata, std::make_unique(info.dynamicMetadata())); + activation->InsertValueProducer(FilterState, + std::make_unique(info.filterState())); return activation; } diff --git a/test/extensions/filters/common/expr/BUILD b/test/extensions/filters/common/expr/BUILD index d41dcb194806c..ac7dfeffce150 100644 --- a/test/extensions/filters/common/expr/BUILD +++ b/test/extensions/filters/common/expr/BUILD @@ -18,6 +18,8 @@ envoy_extension_cc_test( srcs = ["context_test.cc"], extension_name = "envoy.filters.http.rbac", deps = [ + "//source/common/router:string_accessor_lib", + "//source/common/stream_info:stream_info_lib", "//source/extensions/filters/common/expr:context_lib", "//test/mocks/ssl:ssl_mocks", "//test/mocks/stream_info:stream_info_mocks", diff --git a/test/extensions/filters/common/expr/context_test.cc b/test/extensions/filters/common/expr/context_test.cc index be7fa3b772f13..2e7587e638c5c 100644 --- a/test/extensions/filters/common/expr/context_test.cc +++ b/test/extensions/filters/common/expr/context_test.cc @@ -1,4 +1,6 @@ #include "common/network/utility.h" +#include "common/router/string_accessor_impl.h" +#include "common/stream_info/filter_state_impl.h" #include "extensions/filters/common/expr/context.h" @@ -606,6 +608,32 @@ TEST(Context, ConnectionAttributes) { } } +TEST(Context, FilterStateAttributes) { + StreamInfo::FilterStateImpl filter_state(StreamInfo::FilterState::LifeSpan::FilterChain); + FilterStateWrapper wrapper(filter_state); + ProtobufWkt::Arena arena; + wrapper.Produce(&arena); + + const auto key = "filter_state_key"; + const auto serialized = "filter_state_value"; + const auto missing = "missing_key"; + + auto accessor = std::make_shared(serialized); + filter_state.setData(key, accessor, StreamInfo::FilterState::StateType::ReadOnly); + + { + auto value = wrapper[CelValue::CreateStringView(missing)]; + EXPECT_FALSE(value.has_value()); + } + + { + auto value = wrapper[CelValue::CreateStringView(key)]; + EXPECT_TRUE(value.has_value()); + EXPECT_TRUE(value.value().IsBytes()); + EXPECT_EQ(serialized, value.value().BytesOrDie().value()); + } +} + } // namespace } // namespace Expr } // namespace Common From d678c8807917f2424fb43285d32e00e1ee82e9c3 Mon Sep 17 00:00:00 2001 From: Kuat Yessenov Date: Mon, 21 Sep 2020 08:17:59 -0700 Subject: [PATCH 2/2] use explicit type Signed-off-by: Kuat Yessenov --- test/extensions/filters/common/expr/context_test.cc | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/test/extensions/filters/common/expr/context_test.cc b/test/extensions/filters/common/expr/context_test.cc index 2e7587e638c5c..203ca3787c661 100644 --- a/test/extensions/filters/common/expr/context_test.cc +++ b/test/extensions/filters/common/expr/context_test.cc @@ -614,9 +614,9 @@ TEST(Context, FilterStateAttributes) { ProtobufWkt::Arena arena; wrapper.Produce(&arena); - const auto key = "filter_state_key"; - const auto serialized = "filter_state_value"; - const auto missing = "missing_key"; + const std::string key = "filter_state_key"; + const std::string serialized = "filter_state_value"; + const std::string missing = "missing_key"; auto accessor = std::make_shared(serialized); filter_state.setData(key, accessor, StreamInfo::FilterState::StateType::ReadOnly);