From 964e38e8e28878da8580d7ceea34c2ff20b27fdd Mon Sep 17 00:00:00 2001 From: lei-tang <32078630+lei-tang@users.noreply.github.com> Date: Wed, 4 Apr 2018 19:17:14 -0700 Subject: [PATCH] Remove JWT headers consumed by Istio authn and mixer filters (#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 #(, fixes #, ...)` format, will close that issue when PR gets merged)*: fixes #1363 **Special notes for your reviewer**: **Release note**: ```release-note ``` --- src/envoy/http/authn/http_filter.cc | 6 ++ .../authn/http_filter_integration_test.cc | 42 ++++++++ ...envoy_jwt_with_output_header_location.conf | 99 +++++++++++++++++++ src/envoy/http/jwt_auth/jwt_authenticator.cc | 3 + src/envoy/http/mixer/filter.cc | 15 ++- src/envoy/http/mixer/filter.h | 3 + src/istio/control/http/attributes_builder.cc | 1 - 7 files changed, 165 insertions(+), 4 deletions(-) create mode 100644 src/envoy/http/authn/testdata/envoy_jwt_with_output_header_location.conf diff --git a/src/envoy/http/authn/http_filter.cc b/src/envoy/http/authn/http_filter.cc index 175e6b677654..4320bea537bb 100644 --- a/src/envoy/http/authn/http_filter.cc +++ b/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) { diff --git a/src/envoy/http/authn/http_filter_integration_test.cc b/src/envoy/http/authn/http_filter_integration_test.cc index b8769fa20e8c..3ec0c1197e1f 100644 --- a/src/envoy/http/authn/http_filter_integration_test.cc +++ b/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": "issuer@foo.com", + "sub": "sub@foo.com", + "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 "issuer@foo.com" 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", diff --git a/src/envoy/http/authn/testdata/envoy_jwt_with_output_header_location.conf b/src/envoy/http/authn/testdata/envoy_jwt_with_output_header_location.conf new file mode 100644 index 000000000000..0a0c6e8cdd8d --- /dev/null +++ b/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": "issuer@foo.com", + "jwks_uri": "http://localhost:8081/" + } + } + ] + }, + "jwt_output_payload_locations": { + "issuer@foo.com": "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 }}" + } + ] + } + ] + } +} diff --git a/src/envoy/http/jwt_auth/jwt_authenticator.cc b/src/envoy/http/jwt_auth/jwt_authenticator.cc index 69c7252395e4..013a1c1775a2 100644 --- a/src/envoy/http/jwt_auth/jwt_authenticator.cc +++ b/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()) { diff --git a/src/envoy/http/mixer/filter.cc b/src/envoy/http/mixer/filter.cc index 2dee7d36e193..d320c9b805e2 100644 --- a/src/envoy/http/mixer/filter.cc +++ b/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& 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; diff --git a/src/envoy/http/mixer/filter.h b/src/envoy/http/mixer/filter.h index 86034e705c2f..7a03e01627ba 100644 --- a/src/envoy/http/mixer/filter.h +++ b/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_; }; diff --git a/src/istio/control/http/attributes_builder.cc b/src/istio/control/http/attributes_builder.cc index a5a092b7d0bd..cfa96bd682f2 100644 --- a/src/istio/control/http/attributes_builder.cc +++ b/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;