Merging release-0.8 to master

WORKSPACE

@@ -38,7 +38,7 @@ git_repository(
 )
 
 # When updating envoy sha manually please update the sha in istio.deps file also
-ENVOY_SHA = "d55f4990889da5d6874ab70ec5f4e5e0a9053160"
+ENVOY_SHA = "2b2c299144600fb9e525d21aabf39bf48e64fb1f"
 
 http_archive(
     name = "envoy",
@@ -60,8 +60,6 @@ load("@com_lyft_protoc_gen_validate//bazel:go_proto_library.bzl", "go_proto_repo
 go_proto_repositories(shared=0)
 go_rules_dependencies()
 go_register_toolchains()
-load("@io_bazel_rules_go//proto:def.bzl", "proto_register_toolchains")
-proto_register_toolchains()
 
 load("@bazel_tools//tools/build_defs/repo:git.bzl", "git_repository")
 git_repository(

istio.deps

@@ -3,16 +3,14 @@
 		"_comment": "",
 		"name": "ISTIO_API",
 		"repoName": "api",
-		"prodBranch": "master",
 		"file": "repositories.bzl",
 		"lastStableSHA": "78da6e6eb4ad4f158fb58e02f94efde4abf4cabf"
 	},
 	{
 		"_comment": "",
 		"name": "ENVOY_SHA",
 		"repoName": "envoyproxy/envoy",
-		"prodBranch": "master",
 		"file": "WORKSPACE",
-		"lastStableSHA": "d55f4990889da5d6874ab70ec5f4e5e0a9053160"
+		"lastStableSHA": "2b2c299144600fb9e525d21aabf39bf48e64fb1f"
 	}
 ]

src/envoy/http/authn/authenticator_base.cc

@@ -14,6 +14,7 @@
  */
 
 #include "src/envoy/http/authn/authenticator_base.h"
+#include "common/common/assert.h"
 #include "src/envoy/http/authn/authn_utils.h"
 #include "src/envoy/utils/utils.h"
 
@@ -34,16 +35,31 @@ AuthenticatorBase::~AuthenticatorBase() {}
 bool AuthenticatorBase::validateX509(const iaapi::MutualTls& mtls,
                                      Payload* payload) const {
   const Network::Connection* connection = filter_context_.connection();
-  if (connection == nullptr || connection->ssl() == nullptr) {
-    // Not a TLS connection
+  if (connection == nullptr) {
+    // It's wrong if connection does not exist.
     return false;
   }
-
-  bool has_user =
+  // Always try to get principal and set to output if available.
+  const bool has_user =
+      connection->ssl() != nullptr &&
       connection->ssl()->peerCertificatePresented() &&
       Utils::GetSourceUser(connection, payload->mutable_x509()->mutable_user());
 
-  return has_user || mtls.allow_tls();
+  // Return value depend on mode:
+  // - PERMISSIVE: plaintext connection is acceptable, thus return true
+  // regardless.
+  // - TLS_PERMISSIVE: tls connection is required, but certificate is optional.
+  // - STRICT: must be TLS with valid certificate.
+  switch (mtls.mode()) {
+    case iaapi::MutualTls::PERMISSIVE:
+      return true;
+    case iaapi::MutualTls::TLS_PERMISSIVE:
+      return connection->ssl() != nullptr;
+    case iaapi::MutualTls::STRICT:
+      return has_user;
+    default:
+      NOT_REACHED;
+  }
 }
 
 bool AuthenticatorBase::validateJwt(const iaapi::Jwt& jwt, Payload* payload) {

src/envoy/http/authn/authenticator_base_test.cc

@@ -55,10 +55,10 @@ class MockAuthenticatorBase : public AuthenticatorBase {
   MOCK_METHOD1(run, bool(Payload*));
 };
 
-class AuthenticatorBaseTest : public testing::Test,
-                              public Logger::Loggable<Logger::Id::filter> {
+class ValidateX509Test : public testing::TestWithParam<iaapi::MutualTls::Mode>,
+                         public Logger::Loggable<Logger::Id::filter> {
  public:
-  virtual ~AuthenticatorBaseTest() {}
+  virtual ~ValidateX509Test() {}
 
   Http::TestHeaderMapImpl request_headers_{};
   NiceMock<Envoy::Network::MockConnection> connection_{};
@@ -70,7 +70,10 @@ class AuthenticatorBaseTest : public testing::Test,
 
   MockAuthenticatorBase authenticator_{&filter_context_};
 
-  void SetUp() override { payload_ = new Payload(); }
+  void SetUp() override {
+    mtls_params_.set_mode(GetParam());
+    payload_ = new Payload();
+  }
 
   void TearDown() override { delete (payload_); }
 
@@ -81,51 +84,34 @@ class AuthenticatorBaseTest : public testing::Test,
   Payload default_payload_;
 };
 
-Http::TestHeaderMapImpl CreateTestHeaderMap(const std::string& header_key,
-                                            const std::string& header_value) {
-  // The base64 encoding is done through Base64::encode().
-  // If the test input has special chars, may need to use the counterpart of
-  // Base64UrlDecode().
-  std::string value_base64 =
-      Base64::encode(header_value.c_str(), header_value.size());
-  return Http::TestHeaderMapImpl{{header_key, value_base64}};
-}
-
-TEST_F(AuthenticatorBaseTest, ValidateMtlsOnPlaintextConnection) {
-  EXPECT_FALSE(authenticator_.validateX509(mtls_params_, payload_));
-  EXPECT_TRUE(MessageDifferencer::Equals(*payload_, default_payload_));
-}
-
-TEST_F(AuthenticatorBaseTest, ValidateTlsOnPlaintextConnection) {
-  mtls_params_.set_allow_tls(true);  // allow TLS connection
-  // When requiring TLS, plaintext connection should fail.
-  EXPECT_FALSE(authenticator_.validateX509(mtls_params_, payload_));
+TEST_P(ValidateX509Test, PlaintextConnection) {
+  // Should return false except mode is PERMISSIVE (accept plaintext)
+  if (GetParam() == iaapi::MutualTls::PERMISSIVE) {
+    EXPECT_TRUE(authenticator_.validateX509(mtls_params_, payload_));
+  } else {
+    EXPECT_FALSE(authenticator_.validateX509(mtls_params_, payload_));
+  }
   EXPECT_TRUE(MessageDifferencer::Equals(*payload_, default_payload_));
 }
 
-TEST_F(AuthenticatorBaseTest, ValidateMtlsOnSslConnectionWithNoPeerCert) {
+TEST_P(ValidateX509Test, SslConnectionWithNoPeerCert) {
   EXPECT_CALL(Const(connection_), ssl()).WillRepeatedly(Return(&ssl_));
   EXPECT_CALL(Const(ssl_), peerCertificatePresented())
       .Times(1)
       .WillOnce(Return(false));
 
-  EXPECT_FALSE(authenticator_.validateX509(mtls_params_, payload_));
+  // Should return false except mode is PERMISSIVE (accept plaintext) or
+  // TLS_PERMISSIVE
+  if (GetParam() == iaapi::MutualTls::PERMISSIVE ||
+      GetParam() == iaapi::MutualTls::TLS_PERMISSIVE) {
+    EXPECT_TRUE(authenticator_.validateX509(mtls_params_, payload_));
+  } else {
+    EXPECT_FALSE(authenticator_.validateX509(mtls_params_, payload_));
+  }
   EXPECT_TRUE(MessageDifferencer::Equals(*payload_, default_payload_));
 }
 
-TEST_F(AuthenticatorBaseTest, ValidateTlsOnSslConnectionWithNoPeerCert) {
-  mtls_params_.set_allow_tls(true);  // allow TLS connection
-  EXPECT_CALL(Const(connection_), ssl()).WillRepeatedly(Return(&ssl_));
-  EXPECT_CALL(Const(ssl_), peerCertificatePresented())
-      .Times(1)
-      .WillOnce(Return(false));
-
-  // When client certificate is not present on TLS, authentication should still
-  // succeed.
-  EXPECT_TRUE(authenticator_.validateX509(mtls_params_, payload_));
-}
-
-TEST_F(AuthenticatorBaseTest, ValidateMtlsOnSslConnectionWithPeerCert) {
+TEST_P(ValidateX509Test, SslConnectionWithPeerCert) {
   EXPECT_CALL(Const(connection_), ssl()).WillRepeatedly(Return(&ssl_));
   EXPECT_CALL(Const(ssl_), peerCertificatePresented())
       .Times(1)
@@ -137,20 +123,7 @@ TEST_F(AuthenticatorBaseTest, ValidateMtlsOnSslConnectionWithPeerCert) {
   EXPECT_EQ(payload_->x509().user(), "foo");
 }
 
-TEST_F(AuthenticatorBaseTest, ValidateTlsOnSslConnectionWithPeerCert) {
-  mtls_params_.set_allow_tls(true);  // allow TLS connection
-  EXPECT_CALL(Const(connection_), ssl()).WillRepeatedly(Return(&ssl_));
-  EXPECT_CALL(Const(ssl_), peerCertificatePresented())
-      .Times(1)
-      .WillOnce(Return(true));
-  EXPECT_CALL(ssl_, uriSanPeerCertificate()).Times(1).WillOnce(Return("foo"));
-  EXPECT_TRUE(authenticator_.validateX509(mtls_params_, payload_));
-  // When client certificate is present on TLS, authenticated attribute should
-  // be extracted.
-  EXPECT_EQ(payload_->x509().user(), "foo");
-}
-
-TEST_F(AuthenticatorBaseTest, ValidateMtlsOnSslConnectionWithPeerSpiffeCert) {
+TEST_P(ValidateX509Test, SslConnectionWithPeerSpiffeCert) {
   EXPECT_CALL(Const(connection_), ssl()).WillRepeatedly(Return(&ssl_));
   EXPECT_CALL(Const(ssl_), peerCertificatePresented())
       .Times(1)
@@ -165,23 +138,7 @@ TEST_F(AuthenticatorBaseTest, ValidateMtlsOnSslConnectionWithPeerSpiffeCert) {
   EXPECT_EQ(payload_->x509().user(), "foo");
 }
 
-TEST_F(AuthenticatorBaseTest, ValidateTlsOnSslConnectionWithPeerSpiffeCert) {
-  mtls_params_.set_allow_tls(true);  // allow TLS connection
-  EXPECT_CALL(Const(connection_), ssl()).WillRepeatedly(Return(&ssl_));
-  EXPECT_CALL(Const(ssl_), peerCertificatePresented())
-      .Times(1)
-      .WillOnce(Return(true));
-  EXPECT_CALL(ssl_, uriSanPeerCertificate())
-      .Times(1)
-      .WillOnce(Return("spiffe://foo"));
-  EXPECT_TRUE(authenticator_.validateX509(mtls_params_, payload_));
-  // When client certificate is present on TLS, authenticated attribute should
-  // be extracted.
-  EXPECT_EQ(payload_->x509().user(), "foo");
-}
-
-TEST_F(AuthenticatorBaseTest,
-       ValidateMtlsOnSslConnectionWithPeerMalformedSpiffeCert) {
+TEST_P(ValidateX509Test, SslConnectionWithPeerMalformedSpiffeCert) {
   EXPECT_CALL(Const(connection_), ssl()).WillRepeatedly(Return(&ssl_));
   EXPECT_CALL(Const(ssl_), peerCertificatePresented())
       .Times(1)
@@ -198,26 +155,49 @@ TEST_F(AuthenticatorBaseTest,
   EXPECT_EQ(payload_->x509().user(), "spiffe:foo");
 }
 
-TEST_F(AuthenticatorBaseTest,
-       ValidateTlsOnSslConnectionWithPeerMalformedSpiffeCert) {
-  mtls_params_.set_allow_tls(true);  // allow TLS connection
-  EXPECT_CALL(Const(connection_), ssl()).WillRepeatedly(Return(&ssl_));
-  EXPECT_CALL(Const(ssl_), peerCertificatePresented())
-      .Times(1)
-      .WillOnce(Return(true));
-  EXPECT_CALL(ssl_, uriSanPeerCertificate())
-      .Times(1)
-      .WillOnce(Return("spiffe:foo"));
-  EXPECT_TRUE(authenticator_.validateX509(mtls_params_, payload_));
-  // When client certificate is present on TLS and the spiffe subject format is
-  // wrong
-  // ("spiffe:foo" instead of "spiffe://foo"), the user attribute should be
-  // extracted.
-  EXPECT_EQ(payload_->x509().user(), "spiffe:foo");
-}
+INSTANTIATE_TEST_CASE_P(ValidateX509Tests, ValidateX509Test,
+                        testing::Values(iaapi::MutualTls::STRICT,
+                                        iaapi::MutualTls::TLS_PERMISSIVE,
+                                        iaapi::MutualTls::PERMISSIVE));
+
+class ValidateJwtTest : public testing::Test,
+                        public Logger::Loggable<Logger::Id::filter> {
+ public:
+  virtual ~ValidateJwtTest() {}
+
+  Http::TestHeaderMapImpl request_headers_{};
+  NiceMock<Envoy::Network::MockConnection> connection_{};
+  NiceMock<Envoy::Ssl::MockConnection> ssl_{};
+  FilterConfig filter_config_{};
+  FilterContext filter_context_{&request_headers_, &connection_,
+                                istio::envoy::config::filter::http::authn::
+                                    v2alpha1::FilterConfig::default_instance()};
+
+  MockAuthenticatorBase authenticator_{&filter_context_};
+
+  void SetUp() override { payload_ = new Payload(); }
+
+  void TearDown() override { delete (payload_); }
+
+  Http::TestHeaderMapImpl CreateTestHeaderMap(const std::string& header_key,
+                                              const std::string& header_value) {
+    // The base64 encoding is done through Base64::encode().
+    // If the test input has special chars, may need to use the counterpart of
+    // Base64UrlDecode().
+    std::string value_base64 =
+        Base64::encode(header_value.c_str(), header_value.size());
+    return Http::TestHeaderMapImpl{{header_key, value_base64}};
+  }
+
+ protected:
+  iaapi::MutualTls mtls_params_;
+  iaapi::Jwt jwt_;
+  Payload* payload_;
+  Payload default_payload_;
+};
 
 // TODO: more tests for Jwt.
-TEST_F(AuthenticatorBaseTest, ValidateJwtWithNoIstioAuthnConfig) {
+TEST_F(ValidateJwtTest, ValidateJwtWithNoIstioAuthnConfig) {
   jwt_.set_issuer("issuer@foo.com");
   // authenticator_ has empty Istio authn config
   // When there is empty Istio authn config, validateJwt() should return
@@ -226,7 +206,7 @@ TEST_F(AuthenticatorBaseTest, ValidateJwtWithNoIstioAuthnConfig) {
   EXPECT_TRUE(MessageDifferencer::Equals(*payload_, default_payload_));
 }
 
-TEST_F(AuthenticatorBaseTest, ValidateJwtWithNoIssuer) {
+TEST_F(ValidateJwtTest, NoIssuer) {
   // no issuer in jwt
   google::protobuf::util::JsonParseOptions options;
   FilterConfig filter_config;
@@ -250,7 +230,7 @@ TEST_F(AuthenticatorBaseTest, ValidateJwtWithNoIssuer) {
   EXPECT_TRUE(MessageDifferencer::Equals(*payload_, default_payload_));
 }
 
-TEST_F(AuthenticatorBaseTest, ValidateJwtWithEmptyJwtOutputPayloadLocations) {
+TEST_F(ValidateJwtTest, EmptyJwtOutputPayloadLocations) {
   jwt_.set_issuer("issuer@foo.com");
   Http::TestHeaderMapImpl request_headers_with_jwt = CreateTestHeaderMap(
       kSecIstioAuthUserInfoHeaderKey, kSecIstioAuthUserinfoHeaderValue);
@@ -274,7 +254,7 @@ TEST_F(AuthenticatorBaseTest, ValidateJwtWithEmptyJwtOutputPayloadLocations) {
   EXPECT_TRUE(MessageDifferencer::Equals(*payload_, default_payload_));
 }
 
-TEST_F(AuthenticatorBaseTest, ValidateJwtWithNoJwtInHeader) {
+TEST_F(ValidateJwtTest, NoJwtInHeader) {
   jwt_.set_issuer("issuer@foo.com");
   google::protobuf::util::JsonParseOptions options;
   FilterConfig filter_config;
@@ -297,7 +277,7 @@ TEST_F(AuthenticatorBaseTest, ValidateJwtWithNoJwtInHeader) {
   EXPECT_TRUE(MessageDifferencer::Equals(*payload_, default_payload_));
 }
 
-TEST_F(AuthenticatorBaseTest, ValidateJwtWithJwtInHeader) {
+TEST_F(ValidateJwtTest, JwtInHeader) {
   jwt_.set_issuer("issuer@foo.com");
   Http::TestHeaderMapImpl request_headers_with_jwt = CreateTestHeaderMap(
       "sec-istio-auth-jwt-output", kSecIstioAuthUserinfoHeaderValue);
@@ -327,7 +307,8 @@ TEST_F(AuthenticatorBaseTest, ValidateJwtWithJwtInHeader) {
                  "iss": "issuer@foo.com",
                  "sub": "sub@foo.com",
                  "some-other-string-claims": "some-claims-kept"
-               }
+               },
+               raw_claims: "\n     {\n       \"iss\": \"issuer@foo.com\",\n       \"sub\": \"sub@foo.com\",\n       \"aud\": \"aud1\",\n       \"non-string-will-be-ignored\": 1512754205,\n       \"some-other-string-claims\": \"some-claims-kept\"\n     }\n   "
              }
            }
         )",

src/envoy/http/authn/authn_utils.cc

@@ -68,6 +68,7 @@ bool AuthnUtils::GetJWTPayloadFromHeaders(
               jwt_payload_key.get(), value);
     return false;
   }
+  *payload->mutable_raw_claims() = payload_str;
   ::google::protobuf::Map< ::std::string, ::std::string>* claims =
       payload->mutable_claims();
   Envoy::Json::ObjectSharedPtr json_obj;

src/envoy/http/authn/authn_utils_test.cc

@@ -14,6 +14,7 @@
  */
 #include "src/envoy/http/authn/authn_utils.h"
 #include "common/common/base64.h"
+#include "common/common/utility.h"
 #include "src/envoy/http/authn/test_utils.h"
 #include "test/test_common/utility.h"
 
@@ -91,7 +92,8 @@ TEST(AuthnUtilsTest, GetJwtPayloadFromHeaderTest) {
         key: "some-other-string-claims"
         value: "some-claims-kept"
       }
-    )",
+      raw_claims: ")" +
+          StringUtil::escape(kSecIstioAuthUserinfoHeaderValue) + R"(")",
       &expected_payload));
   // The payload returned from GetJWTPayloadFromHeaders() should be the same as
   // the expected.
@@ -121,7 +123,9 @@ TEST(AuthnUtilsTest, GetJwtPayloadFromHeaderWithNoAudTest) {
         key: "some-other-string-claims"
         value: "some-claims-kept"
       }
-    )",
+      raw_claims: ")" +
+          StringUtil::escape(kSecIstioAuthUserInfoHeaderWithNoAudValue) +
+          R"(")",
       &expected_payload));
   // The payload returned from GetJWTPayloadFromHeaders() should be the same as
   // the expected. When there is no aud,  the aud is not saved in the payload
@@ -154,7 +158,9 @@ TEST(AuthnUtilsTest, GetJwtPayloadFromHeaderWithTwoAudTest) {
         key: "some-other-string-claims"
         value: "some-claims-kept"
       }
-    )",
+      raw_claims: ")" +
+          StringUtil::escape(kSecIstioAuthUserInfoHeaderWithTwoAudValue) +
+          R"(")",
       &expected_payload));
 
   // The payload returned from GetJWTPayloadFromHeaders() should be the same as

src/envoy/http/authn/filter_context.h

@@ -18,6 +18,8 @@
 #include "authentication/v1alpha1/policy.pb.h"
 #include "common/common/logger.h"
 #include "envoy/config/filter/http/authn/v2alpha1/config.pb.h"
+#include "envoy/http/header_map.h"
+#include "envoy/network/connection.h"
 #include "src/istio/authn/context.pb.h"
 
 namespace Envoy {