Validate ext_authz OkResponse header append actions

source/extensions/filters/common/ext_authz/ext_authz.h

@@ -110,6 +110,8 @@ struct Response {
   // "setCopy") to the response sent back to the downstream client on OK auth responses
   // only if the headers were returned from the authz server.
   UnsafeHeaderVector response_headers_to_overwrite_if_exists{};
+  // Whether the authorization server returned any headers with an invalid append action type.
+  bool saw_invalid_append_actions{false};
   // A set of HTTP headers consumed by the authorization server, will be removed
   // from the request to the upstream server.
   std::vector<std::string> headers_to_remove{};

source/extensions/filters/common/ext_authz/ext_authz_grpc_impl.cc

@@ -43,7 +43,6 @@ void copyOkResponseMutations(ResponsePtr& response,
       }
     } else {
       switch (header.append_action()) {
-        PANIC_ON_PROTO_ENUM_SENTINEL_VALUES;
       case Router::HeaderValueOption::APPEND_IF_EXISTS_OR_ADD:
         response->response_headers_to_add.emplace_back(header.header().key(),
                                                        header.header().value());
@@ -60,6 +59,9 @@ void copyOkResponseMutations(ResponsePtr& response,
         response->response_headers_to_set.emplace_back(header.header().key(),
                                                        header.header().value());
         break;
+      default:
+        response->saw_invalid_append_actions = true;
+        break;
       }
     }
   }

source/extensions/filters/common/ext_authz/ext_authz_http_impl.cc

@@ -41,6 +41,7 @@ const Response& errorResponse() {
                                             UnsafeHeaderVector{},
                                             UnsafeHeaderVector{},
                                             UnsafeHeaderVector{},
+                                            false,
                                             {{}},
                                             Http::Utility::QueryParamsVector{},
                                             {},
@@ -385,6 +386,7 @@ ResponsePtr RawHttpClientImpl::toResponse(Http::ResponseMessagePtr message) {
                                 UnsafeHeaderVector{},
                                 UnsafeHeaderVector{},
                                 UnsafeHeaderVector{},
+                                false,
                                 std::move(headers_to_remove),
                                 Http::Utility::QueryParamsVector{},
                                 {},
@@ -408,6 +410,7 @@ ResponsePtr RawHttpClientImpl::toResponse(Http::ResponseMessagePtr message) {
                                   UnsafeHeaderVector{},
                                   UnsafeHeaderVector{},
                                   UnsafeHeaderVector{},
+                                  false,
                                   {{}},
                                   Http::Utility::QueryParamsVector{},
                                   {},

source/extensions/filters/http/ext_authz/ext_authz.cc

@@ -601,6 +601,17 @@ void Filter::onComplete(Filters::Common::ExtAuthz::ResponsePtr&& response) {
 
   updateLoggingInfo(response->grpc_status);
 
+  if (response->saw_invalid_append_actions) {
+    if (config_->validateMutations()) {
+      ENVOY_STREAM_LOG(trace, "Rejecting response with invalid header append action.",
+                       *decoder_callbacks_);
+      rejectResponse();
+      return;
+    }
+    ENVOY_STREAM_LOG(trace, "Ignoring response headers with invalid header append action.",
+                     *decoder_callbacks_);
+  }
+
   if (!response->dynamic_metadata.fields().empty()) {
     if (!config_->enableDynamicMetadataIngestion()) {
       ENVOY_STREAM_LOG(trace,

test/extensions/filters/common/ext_authz/ext_authz_grpc_impl_test.cc

@@ -445,6 +445,10 @@ TEST_F(ExtAuthzGrpcClientTest, AuthorizationOkWithAppendActions) {
       key: overwrite-if-exists-or-add
       value: overwrite-if-exists-or-add-value
     append_action: OVERWRITE_IF_EXISTS_OR_ADD
+  - header:
+      key: invalid-append-action
+      value: invalid-append-action-value
+    append_action: 404
 )EOF",
                             check_response);
 
@@ -458,6 +462,7 @@ TEST_F(ExtAuthzGrpcClientTest, AuthorizationOkWithAppendActions) {
           UnsafeHeaderVector{{"add-if-absent", "add-if-absent-value"}},
       .response_headers_to_overwrite_if_exists =
           UnsafeHeaderVector{{"overwrite-if-exists", "overwrite-if-exists-value"}},
+      .saw_invalid_append_actions = true,
       .status_code = Http::Code::OK,
       .grpc_status = Grpc::Status::WellKnownGrpcStatus::Ok,
   };

test/extensions/filters/common/ext_authz/test_common.h

@@ -133,6 +133,10 @@ MATCHER_P(AuthzOkResponse, response, "") {
     return false;
   }
 
+  if (response.saw_invalid_append_actions != arg->saw_invalid_append_actions) {
+    return false;
+  }
+
   if (!TestCommon::compareQueryParamsVector(response.query_parameters_to_set,
                                             arg->query_parameters_to_set)) {
     return false;

test/extensions/filters/http/ext_authz/BUILD

@@ -88,6 +88,7 @@ envoy_extension_cc_test(
         "//test/test_common:test_runtime_lib",
         "//test/test_common:utility_lib",
         "@envoy_api//envoy/config/bootstrap/v3:pkg_cc_proto",
+        "@envoy_api//envoy/config/core/v3:pkg_cc_proto",
         "@envoy_api//envoy/config/listener/v3:pkg_cc_proto",
         "@envoy_api//envoy/extensions/filters/http/ext_authz/v3:pkg_cc_proto",
         "@envoy_api//envoy/service/auth/v3:pkg_cc_proto",

test/extensions/filters/http/ext_authz/ext_authz_integration_test.cc

@@ -1,4 +1,5 @@
 #include "envoy/config/bootstrap/v3/bootstrap.pb.h"
+#include "envoy/config/core/v3/base.pb.h"
 #include "envoy/config/listener/v3/listener_components.pb.h"
 #include "envoy/extensions/filters/http/ext_authz/v3/ext_authz.pb.h"
 #include "envoy/service/auth/v3/external_auth.pb.h"
@@ -457,7 +458,8 @@ class ExtAuthzGrpcIntegrationTest
                             const Headers& response_headers_to_append,
                             const Headers& response_headers_to_set,
                             const Headers& response_headers_to_append_if_absent,
-                            const Headers& response_headers_to_set_if_exists = {}) {
+                            const Headers& response_headers_to_set_if_exists = {},
+                            bool add_sentinel_header_append_action = false) {
     ext_authz_request_->startGrpcStream();
     envoy::service::auth::v3::CheckResponse check_response;
     check_response.mutable_status()->set_code(Grpc::Status::WellKnownGrpcStatus::Ok);
@@ -518,6 +520,20 @@ class ExtAuthzGrpcIntegrationTest
       ENVOY_LOG_MISC(trace, "sendExtAuthzResponse: set response_header_to_add {}={}", key, value);
     }
 
+    if (add_sentinel_header_append_action) {
+      auto* entry = check_response.mutable_ok_response()->mutable_response_headers_to_add()->Add();
+      entry->set_append_action(
+          static_cast<envoy::config::core::v3::HeaderValueOption::HeaderAppendAction>(
+              std::numeric_limits<int32_t>::max()));
+      const auto key = std::string("invalid-append-action");
+      const auto value = std::string("invalid-append-action-value");
+      entry->mutable_header()->set_key(key);
+      entry->mutable_header()->set_value(value);
+      ENVOY_LOG_MISC(trace,
+                     "sendExtAuthzResponse: set response header with invalid append action {}={}",
+                     key, value);
+    }
+
     for (const auto& response_header_to_set : response_headers_to_set) {
       auto* entry = check_response.mutable_ok_response()->mutable_response_headers_to_add()->Add();
       const auto key = std::string(response_header_to_set.first);
@@ -1294,6 +1310,49 @@ TEST_P(ExtAuthzGrpcIntegrationTest, ValidateMutations) {
   cleanup();
 }
 
+TEST_P(ExtAuthzGrpcIntegrationTest, ValidateMutationsSentinelAppendAction) {
+  GrpcInitializeConfigOpts opts;
+  opts.validate_mutations = true;
+  initializeConfig(opts);
+
+  // Use h1, set up the test.
+  setDownstreamProtocol(Http::CodecType::HTTP1);
+  HttpIntegrationTest::initialize();
+
+  // Start a client connection and request.
+  initiateClientConnection(0);
+  waitForExtAuthzRequest(expectedCheckRequest(Http::CodecType::HTTP1));
+  sendExtAuthzResponse({}, {}, {}, {}, {}, {}, {}, {}, {},
+                       /*add_sentinel_header_append_action=*/true);
+
+  ASSERT_TRUE(response_->waitForEndStream());
+  EXPECT_TRUE(response_->complete());
+  EXPECT_EQ("500", response_->headers().getStatusValue());
+  test_server_->waitForCounterEq("cluster.cluster_0.ext_authz.invalid", 1);
+
+  cleanup();
+}
+
+// Ignore invalid header append actions when validate_mutations is false.
+TEST_P(ExtAuthzGrpcIntegrationTest, NoValidateMutationsSentinelAppendAction) {
+  GrpcInitializeConfigOpts opts;
+  opts.validate_mutations = false;
+  initializeConfig(opts);
+
+  // Use h1, set up the test.
+  setDownstreamProtocol(Http::CodecType::HTTP1);
+  HttpIntegrationTest::initialize();
+
+  // Start a client connection and request.
+  initiateClientConnection(0);
+
+  waitForExtAuthzRequest(expectedCheckRequest(Http::CodecType::HTTP1));
+  sendExtAuthzResponse({}, {}, {}, {}, {}, {}, {}, {}, {},
+                       /*add_sentinel_header_append_action=*/true);
+  waitForSuccessfulUpstreamResponse("200");
+  cleanup();
+}
+
 TEST_P(ExtAuthzGrpcIntegrationTest, TimeoutFailOpen) {
   GrpcInitializeConfigOpts init_opts;
   init_opts.stats_expect_response_bytes = false;

test/extensions/filters/http/ext_authz/ext_authz_test.cc

@@ -679,6 +679,13 @@ TEST_F(InvalidMutationTest, BasicInvalidKey) {
   testResponse(response);
 }
 
+TEST_F(InvalidMutationTest, InvalidHeaderAppendAction) {
+  Filters::Common::ExtAuthz::Response response;
+  response.status = Filters::Common::ExtAuthz::CheckStatus::OK;
+  response.saw_invalid_append_actions = true;
+  testResponse(response);
+}
+
 struct DecoderHeaderMutationRulesTestOpts {
   absl::optional<envoy::config::common::mutation_rules::v3::HeaderMutationRules> rules;
   bool expect_reject_response = false;