Skip to content

Use mTLS mode enum to decide authentication result. #1733

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
qiwzhang merged 5 commits into from
May 9, 2018
Merged

Use mTLS mode enum to decide authentication result. #1733

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
93ce9ad
Update_Dependencies (#1657)
istio-testing Apr 30, 2018
a8f88fa
Use mTLS mode enum to decide authentication result.
May 4, 2018
55c5d75
Clang
May 7, 2018
288d25c
Use NOT_REACHED macros for default case.
May 7, 2018
407fcf8
Clang
May 7, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion istio.deps
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,4 +13,4 @@
"file": "WORKSPACE",
"lastStableSHA": "2b2c299144600fb9e525d21aabf39bf48e64fb1f"
}
]
]
26 changes: 21 additions & 5 deletions src/envoy/http/authn/authenticator_base.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"

Expand All @@ -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) {
Expand Down
162 changes: 71 additions & 91 deletions src/envoy/http/authn/authenticator_base_test.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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_{};
Expand All @@ -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_); }

Expand All @@ -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)
Expand All @@ -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)
Expand All @@ -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)
Expand All @@ -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
Expand All @@ -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;
Expand All @@ -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);
Expand All @@ -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;
Expand All @@ -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);
Expand Down