Remove JWT headers consumed by Istio authn and mixer filters (envoypr…

…oxy#1364)

Automatic merge from submit-queue.

Remove JWT headers consumed by Istio authn and mixer filters

**What this PR does / why we need it**: This PR removes the JWT headers after they have been consumed by Istio authn and mixer filters.

**Which issue this PR fixes** *(optional, in `fixes #<issue number>(, fixes #<issue_number>, ...)` format, will close that issue when PR gets merged)*: fixes envoyproxy#1363 

**Special notes for your reviewer**:

**Release note**:

```release-note
```

src/envoy/http/authn/http_filter.cc

@@ -74,6 +74,12 @@ void AuthenticationFilter::onPeerAuthenticationDone(bool success) {
 }
 void AuthenticationFilter::onOriginAuthenticationDone(bool success) {
   ENVOY_LOG(debug, "{}: success = {}", __func__, success);
+  // After Istio authn, the JWT headers consumed by Istio authn should be
+  // removed.
+  for (auto const iter : filter_config_.jwt_output_payload_locations()) {
+    filter_context_->headers()->remove(LowerCaseString(iter.second));
+  }
+
   if (success) {
     // Put authentication result to headers.
     if (filter_context_ != nullptr) {

src/envoy/http/authn/http_filter_integration_test.cc

@@ -133,6 +133,48 @@ TEST_P(AuthenticationFilterIntegrationTest, CheckValidJwtPassAuthentication) {
   EXPECT_STREQ("200", response_->headers().Status()->value().c_str());
 }
 
+TEST_P(AuthenticationFilterIntegrationTest, CheckConsumedJwtHeadersAreRemoved) {
+  const Envoy::Http::LowerCaseString header_location(
+      "location-to-read-jwt-result");
+  const std::string jwt_header =
+      R"(
+     {
+       "iss": "[email protected]",
+       "sub": "[email protected]",
+       "aud": "aud1",
+       "non-string-will-be-ignored": 1512754205,
+       "some-other-string-claims": "some-claims-kept"
+     }
+   )";
+  std::string jwt_header_base64 =
+      Base64::encode(jwt_header.c_str(), jwt_header.size());
+  Http::TestHeaderMapImpl request_headers_with_jwt_at_specified_location{
+      {":method", "GET"},
+      {":path", "/"},
+      {":authority", "host"},
+      {"location-to-read-jwt-result", jwt_header_base64}};
+  // In this config, the JWT verification result for "[email protected]" is in the
+  // header "location-to-read-jwt-result"
+  createTestServer(
+      "src/envoy/http/authn/testdata/"
+      "envoy_jwt_with_output_header_location.conf",
+      {"http"});
+  // The AuthN filter requires JWT and the http request contains validated JWT.
+  // In this case, the authentication should succeed and an authn result
+  // should be generated.
+  codec_client_ =
+      makeHttpConnection(makeClientConnection((lookupPort("http"))));
+  codec_client_->makeHeaderOnlyRequest(
+      request_headers_with_jwt_at_specified_location, *response_);
+
+  // Wait for request to upstream[0] (backend)
+  waitForNextUpstreamRequest(0);
+
+  // After Istio authn, the JWT headers consumed by Istio authn should have
+  // been removed.
+  EXPECT_TRUE(nullptr == upstream_request_->headers().get(header_location));
+}
+
 TEST_P(AuthenticationFilterIntegrationTest, CheckAuthnResultIsExpected) {
   createTestServer(
       "src/envoy/http/authn/testdata/envoy_origin_jwt_authn_only.conf",

src/envoy/http/authn/testdata/envoy_jwt_with_output_header_location.conf

@@ -0,0 +1,99 @@
+{
+  "listeners": [
+    {
+      "address": "tcp://{{ ip_loopback_address }}:0",
+      "bind_to_port": true,
+      "filters": [
+        {
+          "type": "read",
+          "name": "http_connection_manager",
+          "config": {
+            "codec_type": "auto",
+            "stat_prefix": "ingress_http",
+            "route_config": {
+              "virtual_hosts": [
+                {
+                  "name": "backend",
+                  "domains": ["*"],
+                  "routes": [
+                    {
+                      "prefix": "/",
+                      "cluster": "backend_service"
+                    }
+                  ]
+                }
+              ]
+            },
+            "access_log": [
+              {
+                "path": "/dev/null"
+              }
+            ],
+            "filters": [
+              {
+                "type": "decoder",
+                "name": "istio_authn",
+                "config": {
+                  "policy": {
+                    "origins": [
+                      {
+                        "jwt": {
+                          "issuer": "[email protected]",
+                          "jwks_uri": "http://localhost:8081/"
+                        }
+                      }
+                    ]
+                  },
+                  "jwt_output_payload_locations": {
+                    "[email protected]": "location-to-read-jwt-result"
+                  }
+                }
+              },
+              {
+                "type": "decoder",
+                "name": "router",
+                "config": {}
+              }
+            ]
+          }
+        }
+      ]
+    }
+  ],
+  "admin": {
+    "access_log_path": "/dev/null",
+    "address": "tcp://{{ ip_loopback_address }}:0"
+  },
+  "cluster_manager": {
+    "clusters": [
+      {
+        "name": "backend_service",
+        "connect_timeout_ms": 5000,
+        "type": "static",
+        "lb_type": "round_robin",
+        "hosts": [
+          {
+            "url": "tcp://{{ ip_loopback_address }}:{{ upstream_0 }}"
+          }
+        ]
+      },
+      {
+        "name": "example_issuer",
+        "connect_timeout_ms": 5000,
+        "type": "static",
+        "circuit_breakers": {
+          "default": {
+            "max_pending_requests": 10000,
+            "max_requests": 10000
+          }
+        },
+        "lb_type": "round_robin",
+        "hosts": [
+          {
+            "url": "tcp://{{ ip_loopback_address }}:{{ upstream_1 }}"
+          }
+        ]
+      }
+    ]
+  }
+}

src/envoy/http/jwt_auth/jwt_authenticator.cc

@@ -195,6 +195,9 @@ void JwtAuthenticator::VerifyKey(const PubkeyCacheItem& issuer_item) {
     return;
   }
 
+  // kJwtPayloadKey is preserved for backward compatibility.
+  // Please refer to https://github.com/istio/istio/issues/4744
+  // for migrating to using Istio authentication.
   headers_->addReferenceKey(kJwtPayloadKey, jwt_->PayloadStrBase64Url());
 
   if (!issuer_item.jwt_config().forward_payload_header().empty()) {

src/envoy/http/mixer/filter.cc

@@ -20,8 +20,7 @@
 #include "src/envoy/http/mixer/check_data.h"
 #include "src/envoy/http/mixer/header_update.h"
 #include "src/envoy/http/mixer/report_data.h"
-#include "src/envoy/utils/grpc_transport.h"
-#include "src/envoy/utils/utils.h"
+#include "src/envoy/utils/authn.h"
 
 using ::google::protobuf::util::Status;
 using ::istio::mixer::v1::config::client::ServiceConfig;
@@ -52,7 +51,10 @@ bool ReadStringMap(const std::multimap<std::string, std::string>& string_map,
 }  // namespace
 
 Filter::Filter(Control& control)
-    : control_(control), state_(NotStarted), initiating_call_(false) {
+    : control_(control),
+      state_(NotStarted),
+      initiating_call_(false),
+      headers_(nullptr) {
   ENVOY_LOG(debug, "Called Mixer::Filter : {}", __func__);
 }
 
@@ -117,6 +119,7 @@ FilterHeadersStatus Filter::decodeHeaders(HeaderMap& headers, bool) {
   initiating_call_ = true;
   CheckData check_data(headers, decoder_callbacks_->connection());
   HeaderUpdate header_update(&headers);
+  headers_ = &headers;
   cancel_check_ = handler_->Check(
       &check_data, &header_update, control_.GetCheckTransport(&headers),
       [this](const Status& status) { completeCheck(status); });
@@ -155,6 +158,12 @@ void Filter::setDecoderFilterCallbacks(
 void Filter::completeCheck(const Status& status) {
   ENVOY_LOG(debug, "Called Mixer::Filter : check complete {}",
             status.ToString());
+  // Remove Istio authentication header after Check() is completed
+  if (nullptr != headers_) {
+    Envoy::Utils::Authentication::ClearResultInHeader(headers_);
+    headers_ = nullptr;
+  }
+
   // This stream has been reset, abort the callback.
   if (state_ == Responded) {
     return;

src/envoy/http/mixer/filter.h

@@ -64,6 +64,9 @@ class Filter : public Http::StreamDecoderFilter,
   State state_;
   bool initiating_call_;
 
+  // Point to the request HTTP headers
+  HeaderMap* headers_;
+
   // The filter callback.
   StreamDecoderFilterCallbacks* decoder_callbacks_;
 };

src/istio/control/http/attributes_builder.cc

@@ -17,7 +17,6 @@
 
 #include "include/istio/utils/attributes_builder.h"
 #include "include/istio/utils/status.h"
-#include "src/istio/authn/context.pb.h"
 #include "src/istio/control/attribute_names.h"
 
 using ::istio::mixer::v1::Attributes;