merge 1.0 branch to master

.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:

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",

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"
 	}
-]
+]

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 = """

src/envoy/http/jwt_auth/auth_store.h

@@ -71,7 +71,7 @@ class JwtAuthStoreFactory : public Logger::Loggable<Logger::Id::config> {
         [this](Event::Dispatcher&) -> ThreadLocal::ThreadLocalObjectSharedPtr {
           return std::make_shared<JwtAuthStore>(config_);
         });
-    ENVOY_LOG(info, "Loaded JwtAuthConfig: {}",
+    ENVOY_LOG(debug, "Loaded JwtAuthConfig: {}",
               MessageUtil::getJsonStringFromMessage(config_, true));
   }
 

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,15 +73,13 @@ 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;
   }
   return FilterDataStatus::Continue;
 }
 
 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;
 }
 

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"
                      }
                    ]
                  }

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

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"

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<std::unique_ptr<JwtTokenExtractor::Token>> 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()) {

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) {

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<std::string, std::string> 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) {

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));
 }