Skip to content

Commit

Permalink
[xDS] Protect RBAC audit logging options field with environment varia…
Browse files Browse the repository at this point in the history
…ble. (#33004)

The protection is added at `xds_http_rbac_filter.cc` where we read the
new field. With this disabling the feature, nothing from things like
`xds_audit_logger_registry.cc` shall be invoked.
  • Loading branch information
rockspore authored and wanlin31 committed May 18, 2023
1 parent bde4686 commit b424eff
Show file tree
Hide file tree
Showing 4 changed files with 42 additions and 1 deletion.
1 change: 1 addition & 0 deletions build_autogenerated.yaml

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

14 changes: 13 additions & 1 deletion src/core/ext/xds/xds_http_rbac_filter.cc
Original file line number Diff line number Diff line change
Expand Up @@ -49,13 +49,24 @@
#include "src/core/ext/xds/xds_bootstrap_grpc.h"
#include "src/core/ext/xds/xds_client.h"
#include "src/core/lib/channel/channel_args.h"
#include "src/core/lib/gpr/string.h"
#include "src/core/lib/gprpp/env.h"
#include "src/core/lib/json/json.h"
#include "src/core/lib/json/json_writer.h"

namespace grpc_core {

namespace {

// TODO(lwge): Remove once the feature is stable.
bool XdsRbacAuditLoggingEnabled() {
auto value = GetEnv("GRPC_EXPERIMENTAL_XDS_RBAC_AUDIT_LOGGING");
if (!value.has_value()) return false;
bool parsed_value;
bool parse_succeeded = gpr_parse_bool_value(value->c_str(), &parsed_value);
return parse_succeeded && parsed_value;
}

Json ParseRegexMatcherToJson(
const envoy_type_matcher_v3_RegexMatcher* regex_matcher) {
return Json::FromObject(
Expand Down Expand Up @@ -453,7 +464,8 @@ Json ParseHttpRbacToJson(const XdsResourceType::DecodeContext& context,
Json::FromObject(std::move(policies_object)));
}
// Flatten the nested messages defined in rbac.proto
if (envoy_config_rbac_v3_RBAC_has_audit_logging_options(rules)) {
if (XdsRbacAuditLoggingEnabled() &&
envoy_config_rbac_v3_RBAC_has_audit_logging_options(rules)) {
ValidationErrors::ScopedField field(errors, ".audit_logging_options");
const auto* audit_logging_options =
envoy_config_rbac_v3_RBAC_audit_logging_options(rules);
Expand Down
1 change: 1 addition & 0 deletions test/core/xds/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -243,6 +243,7 @@ grpc_cc_test(
"//src/proto/grpc/testing/xds/v3:stateful_session_proto",
"//src/proto/grpc/testing/xds/v3:typed_struct_proto",
"//test/core/util:grpc_test_util",
"//test/core/util:scoped_env_var",
"//test/cpp/util:grpc_cli_utils",
],
)
Expand Down
27 changes: 27 additions & 0 deletions test/core/xds/xds_http_filters_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,7 @@
#include "src/proto/grpc/testing/xds/v3/stateful_session_cookie.pb.h"
#include "src/proto/grpc/testing/xds/v3/string.pb.h"
#include "src/proto/grpc/testing/xds/v3/typed_struct.pb.h"
#include "test/core/util/scoped_env_var.h"
#include "test/core/util/test_config.h"

// IWYU pragma: no_include <google/protobuf/message.h>
Expand Down Expand Up @@ -895,7 +896,31 @@ TEST_P(XdsRbacFilterConfigTest, AllPrincipalTypes) {
"}}}}");
}

TEST_P(XdsRbacFilterConfigTest, AuditLoggingOptionsIgnoredWithFeatureDisabled) {
RBAC rbac;
auto* rules = rbac.mutable_rules();
rules->set_action(rules->ALLOW);
auto* logging_options = rules->mutable_audit_logging_options();
logging_options->set_audit_condition(
envoy::config::rbac::v3::RBAC_AuditLoggingOptions_AuditCondition_ON_DENY);
envoy::config::rbac::v3::RBAC_AuditLoggingOptions::AuditLoggerConfig
logger_config;
auto* audit_logger = logger_config.mutable_audit_logger();
audit_logger->mutable_typed_config()->set_type_url(
"/envoy.extensions.rbac.audit_loggers.stream.v3.StdoutAuditLog");
*logging_options->add_logger_configs() = logger_config;
auto config = GenerateConfig(rbac);
ASSERT_TRUE(errors_.ok()) << errors_.status(
absl::StatusCode::kInvalidArgument, "unexpected errors");
ASSERT_TRUE(config.has_value());
EXPECT_EQ(config->config_proto_type_name,
GetParam() ? filter_->OverrideConfigProtoName()
: filter_->ConfigProtoName());
EXPECT_EQ(JsonDump(config->config), "{\"rules\":{\"action\":0}}");
}

TEST_P(XdsRbacFilterConfigTest, AuditLoggingOptions) {
ScopedExperimentalEnvVar env_var("GRPC_EXPERIMENTAL_XDS_RBAC_AUDIT_LOGGING");
RBAC rbac;
auto* rules = rbac.mutable_rules();
rules->set_action(rules->ALLOW);
Expand Down Expand Up @@ -923,6 +948,7 @@ TEST_P(XdsRbacFilterConfigTest, AuditLoggingOptions) {
}

TEST_P(XdsRbacFilterConfigTest, InvalidAuditCondition) {
ScopedExperimentalEnvVar env_var("GRPC_EXPERIMENTAL_XDS_RBAC_AUDIT_LOGGING");
RBAC rbac;
auto* rules = rbac.mutable_rules();
rules->set_action(rules->ALLOW);
Expand All @@ -945,6 +971,7 @@ TEST_P(XdsRbacFilterConfigTest, InvalidAuditCondition) {
}

TEST_P(XdsRbacFilterConfigTest, InvalidAuditLoggerConfig) {
ScopedExperimentalEnvVar env_var("GRPC_EXPERIMENTAL_XDS_RBAC_AUDIT_LOGGING");
RBAC rbac;
auto* rules = rbac.mutable_rules();
rules->set_action(rules->ALLOW);
Expand Down

0 comments on commit b424eff

Please sign in to comment.