diff --git a/.circleci/config.yml b/.circleci/config.yml index 511a6d40844..06d4389c8e8 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -8,7 +8,8 @@ jobs: - BAZEL_TEST_ARGS: "--test_env=ENVOY_IP_TEST_VERSIONS=v4only --test_output=all" resource_class: xlarge steps: - - run: sudo apt-get update && sudo apt-get install -y pkg-config + - run: curl -Lo /tmp/bazel.deb https://github.com/bazelbuild/bazel/releases/download/0.15.2/bazel_0.15.2-linux-x86_64.deb && sudo dpkg -i /tmp/bazel.deb && rm /tmp/bazel.deb + - run: sudo apt-get update && sudo apt-get install -y ninja-build pkg-config - checkout - restore_cache: keys: @@ -36,7 +37,8 @@ jobs: - BAZEL_TEST_ARGS: "--test_env=ENVOY_IP_TEST_VERSIONS=v4only --test_output=all" resource_class: xlarge steps: - - run: sudo apt-get update && sudo apt-get install -y pkg-config + - run: curl -Lo /tmp/bazel.deb https://github.com/bazelbuild/bazel/releases/download/0.15.2/bazel_0.15.2-linux-x86_64.deb && sudo dpkg -i /tmp/bazel.deb && rm /tmp/bazel.deb + - run: sudo apt-get update && sudo apt-get install -y ninja-build pkg-config - checkout - restore_cache: keys: @@ -56,7 +58,8 @@ jobs: - BAZEL_TEST_ARGS: "--test_env=ENVOY_IP_TEST_VERSIONS=v4only --test_output=all" resource_class: xlarge steps: - - run: sudo apt-get update && sudo apt-get install -y pkg-config + - run: curl -Lo /tmp/bazel.deb https://github.com/bazelbuild/bazel/releases/download/0.15.2/bazel_0.15.2-linux-x86_64.deb && sudo dpkg -i /tmp/bazel.deb && rm /tmp/bazel.deb + - run: sudo apt-get update && sudo apt-get install -y ninja-build pkg-config - checkout - restore_cache: keys: @@ -77,7 +80,7 @@ jobs: - BAZEL_TEST_ARGS: "--test_env=ENVOY_IP_TEST_VERSIONS=v4only --test_output=all" steps: - run: sudo ntpdate -vu time.apple.com - - run: brew install automake bazel cmake coreutils go libtool wget + - run: brew install automake bazel cmake coreutils go libtool ninja wget - checkout - restore_cache: keys: diff --git a/WORKSPACE b/WORKSPACE index 4239cd00615..4969bac17e8 100644 --- a/WORKSPACE +++ b/WORKSPACE @@ -30,7 +30,7 @@ bind( ) # When updating envoy sha manually please update the sha in istio.deps file also -ENVOY_SHA = "1aa2430f3bf49557e20edf4f4ad8c2c1072e4b8f" +ENVOY_SHA = "280baee6dc45e48a4bf38ac03d32711fe5eaeee1" http_archive( name = "envoy", diff --git a/istio.deps b/istio.deps index 6c80d622529..8ebc0904583 100644 --- a/istio.deps +++ b/istio.deps @@ -4,13 +4,13 @@ "name": "ISTIO_API", "repoName": "api", "file": "repositories.bzl", - "lastStableSHA": "6a128c56f81639e234cc34dbf40af9532008e467" + "lastStableSHA": "123b0a79a4dba5bd653dcac09cf731b8002e5341" }, { "_comment": "", "name": "ENVOY_SHA", "repoName": "envoyproxy/envoy", "file": "WORKSPACE", - "lastStableSHA": "1aa2430f3bf49557e20edf4f4ad8c2c1072e4b8f" + "lastStableSHA": "280baee6dc45e48a4bf38ac03d32711fe5eaeee1" } -] +] \ No newline at end of file diff --git a/repositories.bzl b/repositories.bzl index 799d93c1671..facc92a915f 100644 --- a/repositories.bzl +++ b/repositories.bzl @@ -113,7 +113,7 @@ cc_library( actual = "@googletest_git//:googletest_prod", ) -ISTIO_API = "6a128c56f81639e234cc34dbf40af9532008e467" +ISTIO_API = "123b0a79a4dba5bd653dcac09cf731b8002e5341" def mixerapi_repositories(bind=True): BUILD = """ diff --git a/src/envoy/http/jwt_auth/auth_store.h b/src/envoy/http/jwt_auth/auth_store.h index bc4286622d9..2410929e4eb 100644 --- a/src/envoy/http/jwt_auth/auth_store.h +++ b/src/envoy/http/jwt_auth/auth_store.h @@ -71,7 +71,7 @@ class JwtAuthStoreFactory : public Logger::Loggable { [this](Event::Dispatcher&) -> ThreadLocal::ThreadLocalObjectSharedPtr { return std::make_shared(config_); }); - ENVOY_LOG(info, "Loaded JwtAuthConfig: {}", + ENVOY_LOG(debug, "Loaded JwtAuthConfig: {}", MessageUtil::getJsonStringFromMessage(config_, true)); } diff --git a/src/envoy/http/jwt_auth/http_filter.cc b/src/envoy/http/jwt_auth/http_filter.cc index 1b7c35b621a..405cc611bab 100644 --- a/src/envoy/http/jwt_auth/http_filter.cc +++ b/src/envoy/http/jwt_auth/http_filter.cc @@ -31,22 +31,13 @@ JwtVerificationFilter::JwtVerificationFilter(Upstream::ClusterManager& cm, JwtVerificationFilter::~JwtVerificationFilter() {} -void JwtVerificationFilter::onDestroy() { - ENVOY_LOG(debug, "Called JwtVerificationFilter : {}", __func__); - jwt_auth_.onDestroy(); -} +void JwtVerificationFilter::onDestroy() { jwt_auth_.onDestroy(); } FilterHeadersStatus JwtVerificationFilter::decodeHeaders(HeaderMap& headers, bool) { - ENVOY_LOG(debug, "Called JwtVerificationFilter : {}", __func__); state_ = Calling; stopped_ = false; - // Sanitize the JWT verification result in the HTTP headers - // TODO (lei-tang): when the JWT verification result is in a configurable - // header, need to sanitize based on the configuration. - headers.remove(JwtAuth::JwtAuthenticator::JwtPayloadKey()); - // Verify the JWT token, onDone() will be called when completed. jwt_auth_.Verify(headers, this); @@ -59,8 +50,8 @@ FilterHeadersStatus JwtVerificationFilter::decodeHeaders(HeaderMap& headers, } void JwtVerificationFilter::onDone(const JwtAuth::Status& status) { - ENVOY_LOG(debug, "Called JwtVerificationFilter : check complete {}", - int(status)); + ENVOY_LOG(debug, "JwtVerificationFilter::onDone with status {}", + JwtAuth::StatusToString(status)); // This stream has been reset, abort the callback. if (state_ == Responded) { return; @@ -82,7 +73,6 @@ void JwtVerificationFilter::onDone(const JwtAuth::Status& status) { } FilterDataStatus JwtVerificationFilter::decodeData(Buffer::Instance&, bool) { - ENVOY_LOG(debug, "Called JwtVerificationFilter : {}", __func__); if (state_ == Calling) { return FilterDataStatus::StopIterationAndWatermark; } @@ -90,7 +80,6 @@ FilterDataStatus JwtVerificationFilter::decodeData(Buffer::Instance&, bool) { } FilterTrailersStatus JwtVerificationFilter::decodeTrailers(HeaderMap&) { - ENVOY_LOG(debug, "Called JwtVerificationFilter : {}", __func__); if (state_ == Calling) { return FilterTrailersStatus::StopIteration; } @@ -99,7 +88,6 @@ FilterTrailersStatus JwtVerificationFilter::decodeTrailers(HeaderMap&) { void JwtVerificationFilter::setDecoderFilterCallbacks( StreamDecoderFilterCallbacks& callbacks) { - ENVOY_LOG(debug, "Called JwtVerificationFilter : {}", __func__); decoder_callbacks_ = &callbacks; } diff --git a/src/envoy/http/jwt_auth/integration_test/envoy.conf.jwk b/src/envoy/http/jwt_auth/integration_test/envoy.conf.jwk index 21a8d136fc2..0262d21357e 100644 --- a/src/envoy/http/jwt_auth/integration_test/envoy.conf.jwk +++ b/src/envoy/http/jwt_auth/integration_test/envoy.conf.jwk @@ -46,7 +46,7 @@ "cluster": "example_issuer" } }, - "forward_payload_header": "sec-istio-auth-userinfo" + "forward_payload_header": "test-jwt-payload-output" } ] } diff --git a/src/envoy/http/jwt_auth/integration_test/envoy_allow_missing_or_failed_jwt.conf.jwk b/src/envoy/http/jwt_auth/integration_test/envoy_allow_missing_or_failed_jwt.conf.jwk index 91746d58809..172fa3f7a2c 100644 --- a/src/envoy/http/jwt_auth/integration_test/envoy_allow_missing_or_failed_jwt.conf.jwk +++ b/src/envoy/http/jwt_auth/integration_test/envoy_allow_missing_or_failed_jwt.conf.jwk @@ -45,7 +45,8 @@ "uri": "http://example.com/foobar_cert", "cluster": "example_issuer" } - } + }, + "forward_payload_header": "test-jwt-payload-output" } ], "allow_missing_or_failed": true diff --git a/src/envoy/http/jwt_auth/integration_test/http_filter_integration_test.cc b/src/envoy/http/jwt_auth/integration_test/http_filter_integration_test.cc index d606ee1b9c5..a5ae802d76d 100644 --- a/src/envoy/http/jwt_auth/integration_test/http_filter_integration_test.cc +++ b/src/envoy/http/jwt_auth/integration_test/http_filter_integration_test.cc @@ -19,9 +19,10 @@ namespace Envoy { namespace { -// The HTTP header key for the JWT verification result +// The HTTP header key for the JWT verification result. Should be the same as +// the one define for forward_payload_header in envoy.conf.jwk const Http::LowerCaseString kJwtVerificationResultHeaderKey( - "sec-istio-auth-userinfo"); + "test-jwt-payload-output"); // {"iss":"https://example.com","sub":"test@example.com","aud":"example_service","exp":2001001001} const std::string kJwtVerificationResult = "eyJpc3MiOiJodHRwczovL2V4YW1wbGUuY29tIiwic3ViIjoidGVz" @@ -260,7 +261,7 @@ TEST_P(JwtVerificationFilterIntegrationTestWithJwks, RSASuccess1) { auto expected_headers = BaseRequestHeaders(); expected_headers.addCopy( - "sec-istio-auth-userinfo", + kJwtVerificationResultHeaderKey, "eyJpc3MiOiJodHRwczovL2V4YW1wbGUuY29tIiwic3ViIjoidGVz" "dEBleGFtcGxlLmNvbSIsImF1ZCI6ImV4YW1wbGVfc2VydmljZSIs" "ImV4cCI6MjAwMTAwMTAwMX0"); @@ -282,7 +283,7 @@ TEST_P(JwtVerificationFilterIntegrationTestWithJwks, ES256Success1) { "T9ubWvRvNGGYOTuJ8T17Db68Qk3T8UNTK5lzfR_mw"; auto expected_headers = BaseRequestHeaders(); - expected_headers.addCopy("sec-istio-auth-userinfo", + expected_headers.addCopy(kJwtVerificationResultHeaderKey, "eyJpc3MiOiJo" "dHRwczovL2V4YW1wbGUuY29tIiwic3ViIjoidGVzdEBleGFtc" "GxlLmNvbSIsImV4cCI6MjAwMTAwMTAwMSwiYXVkIjoiZXhhbX" diff --git a/src/envoy/http/jwt_auth/jwt_authenticator.cc b/src/envoy/http/jwt_auth/jwt_authenticator.cc index b535d31f9a3..56b21356136 100644 --- a/src/envoy/http/jwt_auth/jwt_authenticator.cc +++ b/src/envoy/http/jwt_auth/jwt_authenticator.cc @@ -59,6 +59,16 @@ void JwtAuthenticator::Verify(HeaderMap& headers, headers_ = &headers; callback_ = callback; + // Sanitize the JWT verification result in the HTTP headers + for (const auto& rule : store_.config().rules()) { + if (!rule.forward_payload_header().empty()) { + ENVOY_LOG(debug, "Sanitize JWT authentication output header {}", + rule.forward_payload_header()); + const LowerCaseString key(rule.forward_payload_header()); + headers.remove(key); + } + } + ENVOY_LOG(debug, "Jwt authentication starts"); std::vector> tokens; store_.token_extractor().Extract(headers, &tokens); @@ -195,16 +205,10 @@ void JwtAuthenticator::VerifyKey(const PubkeyCacheItem& issuer_item) { return; } - // TODO(lei-tang): remove this backward compatibility. - // Tracking issue: https://github.com/istio/istio/issues/4744 - headers_->addReferenceKey(kJwtPayloadKey, jwt_->PayloadStrBase64Url()); - if (!issuer_item.jwt_config().forward_payload_header().empty()) { const LowerCaseString key( issuer_item.jwt_config().forward_payload_header()); - if (key.get() != kJwtPayloadKey.get()) { - headers_->addCopy(key, jwt_->PayloadStrBase64Url()); - } + headers_->addCopy(key, jwt_->PayloadStrBase64Url()); } if (!issuer_item.jwt_config().forward()) { diff --git a/src/envoy/http/jwt_auth/jwt_authenticator_test.cc b/src/envoy/http/jwt_auth/jwt_authenticator_test.cc index b5ec6ca974f..ae090943251 100644 --- a/src/envoy/http/jwt_auth/jwt_authenticator_test.cc +++ b/src/envoy/http/jwt_auth/jwt_authenticator_test.cc @@ -88,6 +88,8 @@ const std::string kPublicKey = " \"kid\": \"b3319a147514df7ee5e4bcdee51350cc890cc89e\"" "}]}"; +// Keep this same as forward_payload_header field in the config below. +const char kOutputHeadersKey[] = "test-output"; // A good JSON config. const char kExampleConfig[] = R"( { @@ -108,7 +110,7 @@ const char kExampleConfig[] = R"( "seconds": 600 } }, - "forward_payload_header": "sec-istio-auth-userinfo" + "forward_payload_header": "test-output" } ] } @@ -319,7 +321,7 @@ TEST_F(JwtAuthenticatorTest, TestOkJWTandCache) { auth_->Verify(headers, &mock_cb); - EXPECT_EQ(headers.get_("sec-istio-auth-userinfo"), + EXPECT_EQ(headers.get_(kOutputHeadersKey), "eyJpc3MiOiJodHRwczovL2V4YW1wbGUuY29tIiwic3ViIjoidGVzdEBleGFtcG" "xlLmNvbSIsImV4cCI6MjAwMTAwMTAwMSwiYXVkIjoiZXhhbXBsZV9zZXJ2" "aWNlIn0"); @@ -350,7 +352,7 @@ TEST_F(JwtAuthenticatorTest, TestOkJWTPubkeyNoAlg) { auth_->Verify(headers, &mock_cb); - EXPECT_EQ(headers.get_("sec-istio-auth-userinfo"), + EXPECT_EQ(headers.get_(kOutputHeadersKey), "eyJpc3MiOiJodHRwczovL2V4YW1wbGUuY29tIiwic3ViIjoidGVzdEBleGFtcG" "xlLmNvbSIsImV4cCI6MjAwMTAwMTAwMSwiYXVkIjoiZXhhbXBsZV9zZXJ2" "aWNlIn0"); @@ -383,7 +385,7 @@ TEST_F(JwtAuthenticatorTest, TestOkJWTPubkeyNoKid) { auth_->Verify(headers, &mock_cb); - EXPECT_EQ(headers.get_("sec-istio-auth-userinfo"), + EXPECT_EQ(headers.get_(kOutputHeadersKey), "eyJpc3MiOiJodHRwczovL2V4YW1wbGUuY29tIiwic3ViIjoidGVzdEBleGFtcG" "xlLmNvbSIsImV4cCI6MjAwMTAwMTAwMSwiYXVkIjoiZXhhbXBsZV9zZXJ2" "aWNlIn0"); @@ -409,7 +411,7 @@ TEST_F(JwtAuthenticatorTest, TestOkJWTAudService) { auth_->Verify(headers, &mock_cb); - EXPECT_EQ(headers.get_("sec-istio-auth-userinfo"), + EXPECT_EQ(headers.get_(kOutputHeadersKey), "eyJpc3MiOiJodHRwczovL2V4YW1wbGUuY29tIiwic3ViIjoidGVzdEBleGFtcGx" "lLmNvbSIsImV4cCI6MjAwMTAwMTAwMSwiYXVkIjoiaHR0cDovL2V4YW1wbG" "Vfc2VydmljZS8ifQ"); @@ -435,7 +437,7 @@ TEST_F(JwtAuthenticatorTest, TestOkJWTAudService1) { auth_->Verify(headers, &mock_cb); - EXPECT_EQ(headers.get_("sec-istio-auth-userinfo"), + EXPECT_EQ(headers.get_(kOutputHeadersKey), "eyJpc3MiOiJodHRwczovL2V4YW1wbGUuY29tIiwic3ViIjoidGVzdEBleGFtcGx" "lLmNvbSIsImV4cCI6MjAwMTAwMTAwMSwiYXVkIjoiaHR0cHM6Ly9leGFtcG" "xlX3NlcnZpY2UxLyJ9"); @@ -461,7 +463,7 @@ TEST_F(JwtAuthenticatorTest, TestOkJWTAudService2) { auth_->Verify(headers, &mock_cb); - EXPECT_EQ(headers.get_("sec-istio-auth-userinfo"), + EXPECT_EQ(headers.get_(kOutputHeadersKey), "eyJpc3MiOiJodHRwczovL2V4YW1wbGUuY29tIiwic3ViIjoidGVzdEBleGFtcGx" "lLmNvbSIsImV4cCI6MjAwMTAwMTAwMSwiYXVkIjoiaHR0cDovL2V4YW1wbG" "Vfc2VydmljZTIifQ"); @@ -728,11 +730,8 @@ TEST_F(JwtAuthenticatorTest, TestNoForwardPayloadHeader) { })); auth_->Verify(headers, &mock_cb); - // Test when forward_payload_header is not set, the output should still - // contain the sec-istio-auth-userinfo header for backward compatibility. - EXPECT_TRUE(headers.has("sec-istio-auth-userinfo")); - // In addition, the sec-istio-auth-userinfo header should be the only header - EXPECT_EQ(headers.size(), 1); + // Test when forward_payload_header is not set, nothing added to headers. + EXPECT_EQ(headers.size(), 0); } TEST_F(JwtAuthenticatorTest, TestInlineJwks) { diff --git a/src/istio/control/http/attributes_builder.cc b/src/istio/control/http/attributes_builder.cc index fdee19b5d81..efbe0bce199 100644 --- a/src/istio/control/http/attributes_builder.cc +++ b/src/istio/control/http/attributes_builder.cc @@ -67,9 +67,16 @@ void AttributesBuilder::ExtractRequestHeaderAttributes(CheckData *check_data) { } void AttributesBuilder::ExtractAuthAttributes(CheckData *check_data) { + utils::AttributesBuilder builder(&request_->attributes); + + std::string destination_principal; + if (check_data->GetPrincipal(false, &destination_principal)) { + builder.AddString(utils::AttributeName::kDestinationPrincipal, + destination_principal); + } + istio::authn::Result authn_result; if (check_data->GetAuthenticationResult(&authn_result)) { - utils::AttributesBuilder builder(&request_->attributes); if (!authn_result.principal().empty()) { builder.AddString(utils::AttributeName::kRequestAuthPrincipal, authn_result.principal()); @@ -110,7 +117,6 @@ void AttributesBuilder::ExtractAuthAttributes(CheckData *check_data) { // Fallback to extract from jwt filter directly. This can be removed once // authn filter is in place. std::map payload; - utils::AttributesBuilder builder(&request_->attributes); if (check_data->GetJWTPayload(&payload) && !payload.empty()) { // Populate auth attributes. if (payload.count("iss") > 0 && payload.count("sub") > 0) { @@ -134,12 +140,6 @@ void AttributesBuilder::ExtractAuthAttributes(CheckData *check_data) { builder.AddString(utils::AttributeName::kSourceUser, source_user); builder.AddString(utils::AttributeName::kSourcePrincipal, source_user); } - - std::string destination_principal; - if (check_data->GetPrincipal(false, &destination_principal)) { - builder.AddString(utils::AttributeName::kDestinationPrincipal, - destination_principal); - } } // namespace http void AttributesBuilder::ExtractForwardedAttributes(CheckData *check_data) { diff --git a/src/istio/control/http/attributes_builder_test.cc b/src/istio/control/http/attributes_builder_test.cc index 024fcb73fba..fee1fb4ee61 100644 --- a/src/istio/control/http/attributes_builder_test.cc +++ b/src/istio/control/http/attributes_builder_test.cc @@ -374,6 +374,15 @@ TEST(AttributesBuilderTest, TestCheckAttributesWithAuthNResult) { EXPECT_CALL(mock_data, IsMutualTLS()).WillOnce(Invoke([]() -> bool { return true; })); + EXPECT_CALL(mock_data, GetPrincipal(_, _)) + .WillRepeatedly(Invoke([](bool peer, std::string *user) -> bool { + if (peer) { + *user = "test_user"; + } else { + *user = "destination_user"; + } + return true; + })); EXPECT_CALL(mock_data, GetRequestedServerName(_)) .WillOnce(Invoke([](std::string *name) -> bool { *name = "www.google.com"; @@ -443,10 +452,6 @@ TEST(AttributesBuilderTest, TestCheckAttributesWithAuthNResult) { .mutable_attributes())[utils::AttributeName::kRequestAuthRawClaims] .set_string_value("test_raw_claims"); - // strip destination.principal for JWT-based authn - (*expected_attributes.mutable_attributes()) - .erase(utils::AttributeName::kDestinationPrincipal); - EXPECT_TRUE( MessageDifferencer::Equals(request.attributes, expected_attributes)); }