Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
8 changes: 6 additions & 2 deletions WORKSPACE
Original file line number Diff line number Diff line change
Expand Up @@ -35,9 +35,9 @@ bind(
# When updating envoy sha manually please update the sha in istio.deps file also
#
# Determine SHA256 `wget https://github.com/envoyproxy/envoy/archive/COMMIT.tar.gz && sha256sum COMMIT.tar.gz`
ENVOY_SHA = "15a19b9cb1cc8bd5a5ec71d125177b3f6c9a3cf5"
ENVOY_SHA = "5ea1a0c1cb506ed3e80d52b572b0f767f55f9f39"

ENVOY_SHA256 = "5cfd67dcb8cd5d240ae1a7d6c5303bb385e4b4340705eba9e96f067f24e5e8d7"
ENVOY_SHA256 = "64beeb27f68ed644ff0bd37b193e5a85f49b883250940e292f6f150ec7173e38"

http_archive(
name = "envoy",
Expand All @@ -46,6 +46,10 @@ http_archive(
url = "https://github.com/envoyproxy/envoy/archive/" + ENVOY_SHA + ".tar.gz",
)

load("@envoy//bazel:api_repositories.bzl", "envoy_api_dependencies")

envoy_api_dependencies()

load("@envoy//bazel:repositories.bzl", "GO_VERSION", "envoy_dependencies")

envoy_dependencies()
Expand Down
2 changes: 1 addition & 1 deletion istio.deps
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,6 @@
"name": "ENVOY_SHA",
"repoName": "envoyproxy/envoy",
"file": "WORKSPACE",
"lastStableSHA": "15a19b9cb1cc8bd5a5ec71d125177b3f6c9a3cf5"
"lastStableSHA": "5ea1a0c1cb506ed3e80d52b572b0f767f55f9f39"
}
]
17 changes: 8 additions & 9 deletions src/envoy/http/authn/authenticator_base_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ class ValidateX509Test : public testing::TestWithParam<iaapi::MutualTls::Mode>,
virtual ~ValidateX509Test() {}

NiceMock<Envoy::Network::MockConnection> connection_{};
NiceMock<Envoy::Ssl::MockConnection> ssl_{};
NiceMock<Envoy::Ssl::MockConnectionInfo> ssl_{};
Envoy::Http::HeaderMapImpl header_{};
FilterConfig filter_config_{};
FilterContext filter_context_{
Expand Down Expand Up @@ -142,7 +142,8 @@ TEST_P(ValidateX509Test, SslConnectionWithPeerCert) {
EXPECT_CALL(Const(ssl_), peerCertificatePresented())
.Times(1)
.WillOnce(Return(true));
EXPECT_CALL(ssl_, uriSanPeerCertificate()).Times(1).WillOnce(Return("foo"));
const std::vector<std::string> sans{"foo", "bad"};
EXPECT_CALL(ssl_, uriSanPeerCertificate()).Times(1).WillOnce(Return(sans));
EXPECT_TRUE(authenticator_.validateX509(mtls_params_, payload_));
// When client certificate is present on mTLS, authenticated attribute should
// be extracted.
Expand All @@ -154,9 +155,8 @@ TEST_P(ValidateX509Test, SslConnectionWithPeerSpiffeCert) {
EXPECT_CALL(Const(ssl_), peerCertificatePresented())
.Times(1)
.WillOnce(Return(true));
EXPECT_CALL(ssl_, uriSanPeerCertificate())
.Times(1)
.WillOnce(Return("spiffe://foo"));
const std::vector<std::string> sans{"spiffe://foo", "bad"};
EXPECT_CALL(ssl_, uriSanPeerCertificate()).Times(1).WillOnce(Return(sans));
EXPECT_TRUE(authenticator_.validateX509(mtls_params_, payload_));

// When client certificate is present on mTLS, authenticated attribute should
Expand All @@ -169,9 +169,8 @@ TEST_P(ValidateX509Test, SslConnectionWithPeerMalformedSpiffeCert) {
EXPECT_CALL(Const(ssl_), peerCertificatePresented())
.Times(1)
.WillOnce(Return(true));
EXPECT_CALL(ssl_, uriSanPeerCertificate())
.Times(1)
.WillOnce(Return("spiffe:foo"));
const std::vector<std::string> sans{"spiffe:foo", "bad"};
EXPECT_CALL(ssl_, uriSanPeerCertificate()).Times(1).WillOnce(Return(sans));
EXPECT_TRUE(authenticator_.validateX509(mtls_params_, payload_));

// When client certificate is present on mTLS and the spiffe subject format is
Expand All @@ -193,7 +192,7 @@ class ValidateJwtTest : public testing::Test,
// StrictMock<Envoy::RequestInfo::MockRequestInfo> request_info_{};
envoy::api::v2::core::Metadata dynamic_metadata_;
NiceMock<Envoy::Network::MockConnection> connection_{};
// NiceMock<Envoy::Ssl::MockConnection> ssl_{};
// NiceMock<Envoy::Ssl::MockConnectionInfo> ssl_{};
Envoy::Http::HeaderMapImpl header_{};
FilterConfig filter_config_{};
FilterContext filter_context_{dynamic_metadata_, header_, &connection_,
Expand Down
17 changes: 7 additions & 10 deletions src/envoy/http/authn/authn_utils.cc
Original file line number Diff line number Diff line change
Expand Up @@ -135,14 +135,11 @@ bool AuthnUtils::ExtractOriginalPayload(const std::string& token,
return true;
}

bool AuthnUtils::MatchString(const char* const str,
bool AuthnUtils::MatchString(absl::string_view str,
const iaapi::StringMatch& match) {
if (str == nullptr) {
return false;
}
switch (match.match_type_case()) {
case iaapi::StringMatch::kExact: {
return match.exact().compare(str) == 0;
return match.exact() == str;
}
case iaapi::StringMatch::kPrefix: {
return absl::StartsWith(str, match.prefix());
Expand All @@ -151,14 +148,14 @@ bool AuthnUtils::MatchString(const char* const str,
return absl::EndsWith(str, match.suffix());
}
case iaapi::StringMatch::kRegex: {
return std::regex_match(str, std::regex(match.regex()));
return std::regex_match(std::string(str), std::regex(match.regex()));
}
default:
return false;
}
}

static bool matchRule(const char* const path,
static bool matchRule(absl::string_view path,
const iaapi::Jwt_TriggerRule& rule) {
for (const auto& excluded : rule.excluded_paths()) {
if (AuthnUtils::MatchString(path, excluded)) {
Expand All @@ -185,12 +182,12 @@ static bool matchRule(const char* const path,
return true;
}

bool AuthnUtils::ShouldValidateJwtPerPath(const char* const path,
bool AuthnUtils::ShouldValidateJwtPerPath(absl::string_view path,
const iaapi::Jwt& jwt) {
// If the path is nullptr which shouldn't happen for a HTTP request or if
// If the path is empty which shouldn't happen for a HTTP request or if
// there are no trigger rules at all, then simply return true as if there're
// no per-path jwt support.
if (path == nullptr || jwt.trigger_rules_size() == 0) {
if (path == "" || jwt.trigger_rules_size() == 0) {
return true;
}
for (const auto& rule : jwt.trigger_rules()) {
Expand Down
4 changes: 2 additions & 2 deletions src/envoy/http/authn/authn_utils.h
Original file line number Diff line number Diff line change
Expand Up @@ -45,12 +45,12 @@ class AuthnUtils : public Logger::Loggable<Logger::Id::filter> {
std::string* original_payload);

// Returns true if str is matched to match.
static bool MatchString(const char* const str,
static bool MatchString(absl::string_view str,
const iaapi::StringMatch& match);

// Returns true if the jwt should be validated. It will check if the request
// path is matched to the trigger rule in the jwt.
static bool ShouldValidateJwtPerPath(const char* const path,
static bool ShouldValidateJwtPerPath(absl::string_view path,
const iaapi::Jwt& jwt);
};

Expand Down
2 changes: 1 addition & 1 deletion src/envoy/http/authn/authn_utils_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -311,7 +311,7 @@ TEST(AuthnUtilsTest, ShouldValidateJwtPerPathDefault) {
iaapi::Jwt jwt;

// Always trigger when path is unavailable.
EXPECT_TRUE(AuthnUtils::ShouldValidateJwtPerPath(nullptr, jwt));
EXPECT_TRUE(AuthnUtils::ShouldValidateJwtPerPath("", jwt));

// Always trigger when there is no rules in jwt.
EXPECT_TRUE(AuthnUtils::ShouldValidateJwtPerPath("/test", jwt));
Expand Down
8 changes: 4 additions & 4 deletions src/envoy/http/authn/http_filter_integration_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,7 @@ TEST_P(AuthenticationFilterIntegrationTest, EmptyPolicy) {

response->waitForEndStream();
EXPECT_TRUE(response->complete());
EXPECT_STREQ("200", response->headers().Status()->value().c_str());
EXPECT_EQ("200", response->headers().Status()->value().getStringView());
}

TEST_P(AuthenticationFilterIntegrationTest, SourceMTlsFail) {
Expand All @@ -121,7 +121,7 @@ TEST_P(AuthenticationFilterIntegrationTest, SourceMTlsFail) {
// waitForNextUpstreamRequest).
response->waitForEndStream();
EXPECT_TRUE(response->complete());
EXPECT_STREQ("401", response->headers().Status()->value().c_str());
EXPECT_EQ("401", response->headers().Status()->value().getStringView());
}

// TODO (diemtvu/lei-tang): add test for MTls success.
Expand All @@ -140,7 +140,7 @@ TEST_P(AuthenticationFilterIntegrationTest, OriginJwtRequiredHeaderNoJwtFail) {
// waitForNextUpstreamRequest).
response->waitForEndStream();
EXPECT_TRUE(response->complete());
EXPECT_STREQ("401", response->headers().Status()->value().c_str());
EXPECT_EQ("401", response->headers().Status()->value().getStringView());
}

TEST_P(AuthenticationFilterIntegrationTest, CheckValidJwtPassAuthentication) {
Expand All @@ -162,7 +162,7 @@ TEST_P(AuthenticationFilterIntegrationTest, CheckValidJwtPassAuthentication) {

response->waitForEndStream();
EXPECT_TRUE(response->complete());
EXPECT_STREQ("200", response->headers().Status()->value().c_str());
EXPECT_EQ("200", response->headers().Status()->value().getStringView());
}

} // namespace
Expand Down
4 changes: 2 additions & 2 deletions src/envoy/http/authn/http_filter_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -130,7 +130,7 @@ TEST_F(AuthenticationFilterTest, PeerFail) {
EXPECT_CALL(decoder_callbacks_, encodeHeaders_(_, _))
.Times(1)
.WillOnce(testing::Invoke([](Http::HeaderMap &headers, bool) {
EXPECT_STREQ("401", headers.Status()->value().c_str());
EXPECT_EQ("401", headers.Status()->value().getStringView());
}));
EXPECT_EQ(Http::FilterHeadersStatus::StopIteration,
filter_.decodeHeaders(request_headers_, true));
Expand All @@ -156,7 +156,7 @@ TEST_F(AuthenticationFilterTest, PeerPassOriginFail) {
EXPECT_CALL(decoder_callbacks_, encodeHeaders_(_, _))
.Times(1)
.WillOnce(testing::Invoke([](Http::HeaderMap &headers, bool) {
EXPECT_STREQ("401", headers.Status()->value().c_str());
EXPECT_EQ("401", headers.Status()->value().getStringView());
}));
EXPECT_EQ(Http::FilterHeadersStatus::StopIteration,
filter_.decodeHeaders(request_headers_, true));
Expand Down
6 changes: 4 additions & 2 deletions src/envoy/http/authn/origin_authenticator.cc
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
*/

#include "src/envoy/http/authn/origin_authenticator.h"
#include "absl/strings/match.h"
#include "authentication/v1alpha1/policy.pb.h"
#include "src/envoy/http/authn/authn_utils.h"

Expand Down Expand Up @@ -44,9 +45,10 @@ bool OriginAuthenticator::run(Payload* payload) {
return false;
}

const char* request_path = nullptr;
absl::string_view request_path;
if (filter_context()->headerMap().Path() != nullptr) {
request_path = filter_context()->headerMap().Path()->value().c_str();
request_path =
filter_context()->headerMap().Path()->value().getStringView();
ENVOY_LOG(debug, "Got request path {}", request_path);
} else {
ENVOY_LOG(error,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -90,9 +90,10 @@ class JwtVerificationFilterIntegrationTest
[](const Http::HeaderEntry& entry,
void* context) -> Http::HeaderMap::Iterate {
auto ret = static_cast<std::map<std::string, std::string>*>(context);
Http::LowerCaseString lower_key{entry.key().c_str()};
const auto key = std::string(entry.key().getStringView());
Http::LowerCaseString lower_key{key};
(*ret)[std::string(lower_key.get())] =
std::string(entry.value().c_str());
std::string(entry.value().getStringView());
return Http::HeaderMap::Iterate::Continue;
},
&ret);
Expand Down
2 changes: 1 addition & 1 deletion src/envoy/http/jwt_auth/jwt_authenticator.cc
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ void JwtAuthenticator::Verify(HeaderMap &headers,
// request.
if (headers.Method() &&
Http::Headers::get().MethodValues.Options ==
headers.Method()->value().c_str() &&
headers.Method()->value().getStringView() &&
headers.Origin() && !headers.Origin()->value().empty() &&
headers.AccessControlRequestMethod() &&
!headers.AccessControlRequestMethod()->value().empty()) {
Expand Down
15 changes: 8 additions & 7 deletions src/envoy/http/jwt_auth/token_extractor.cc
Original file line number Diff line number Diff line change
Expand Up @@ -69,9 +69,10 @@ void JwtTokenExtractor::Extract(
const HeaderEntry *entry = headers.Authorization();
if (entry) {
// Extract token from header.
const HeaderString &value = entry->value();
if (absl::StartsWith(value.getStringView(), kBearerPrefix)) {
tokens->emplace_back(new Token(value.c_str() + kBearerPrefix.length(),
auto value = entry->value().getStringView();
if (absl::StartsWith(value, kBearerPrefix)) {
value.remove_prefix(kBearerPrefix.length());
tokens->emplace_back(new Token(std::string(value),
authorization_issuers_, true, nullptr));
// Only take the first one.
return;
Expand All @@ -88,9 +89,9 @@ void JwtTokenExtractor::Extract(
size_t pos = val.find(' ');
if (pos != absl::string_view::npos) {
// If the header value has prefix, trim the prefix.
token = entry->value().c_str() + pos + 1;
token = std::string(val.substr(pos + 1));
} else {
token = std::string(entry->value().c_str(), entry->value().size());
token = std::string(val);
}

tokens->emplace_back(
Expand All @@ -104,8 +105,8 @@ void JwtTokenExtractor::Extract(
return;
}

const auto &params = Utility::parseQueryString(std::string(
headers.Path()->value().c_str(), headers.Path()->value().size()));
const auto &params = Utility::parseQueryString(
std::string(headers.Path()->value().getStringView()));
for (const auto &param_it : param_maps_) {
const auto &it = params.find(param_it.first);
if (it != params.end()) {
Expand Down
Loading