Revert "Update to rbac policy struct and end2end authz test. (#27074)" (#28552)

This reverts commit b64167a034.
pull/28563/head
Jan Tattermusch 3 years ago committed by GitHub
parent 3788b9142b
commit 8ca42ec6f8
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
  1. 10
      src/core/lib/security/authorization/matchers.cc
  2. 4
      src/core/lib/security/authorization/matchers.h
  3. 6
      src/core/lib/security/authorization/rbac_policy.cc
  4. 5
      src/core/lib/security/authorization/rbac_policy.h
  5. 6
      test/core/security/authorization_matchers_test.cc
  6. 8
      test/core/security/rbac_translator_test.cc
  7. 7
      test/cpp/end2end/BUILD
  8. 84
      test/cpp/end2end/sdk_authz_end2end_test.cc

@ -105,7 +105,7 @@ std::unique_ptr<AuthorizationMatcher> AuthorizationMatcher::Create(
std::move(principal.header_matcher));
case Rbac::Principal::RuleType::kPath:
return absl::make_unique<PathAuthorizationMatcher>(
std::move(principal.string_matcher.value()));
std::move(principal.string_matcher));
case Rbac::Principal::RuleType::kMetadata:
return absl::make_unique<MetadataAuthorizationMatcher>(principal.invert);
}
@ -184,14 +184,14 @@ bool AuthenticatedAuthorizationMatcher::Matches(
// Connection is not authenticated.
return false;
}
if (!matcher_.has_value()) {
if (matcher_.string_matcher().empty()) {
// Allows any authenticated user.
return true;
}
std::vector<absl::string_view> uri_sans = args.GetUriSans();
if (!uri_sans.empty()) {
for (const auto& uri : uri_sans) {
if (matcher_->Match(uri)) {
if (matcher_.Match(uri)) {
return true;
}
}
@ -199,12 +199,12 @@ bool AuthenticatedAuthorizationMatcher::Matches(
std::vector<absl::string_view> dns_sans = args.GetDnsSans();
if (!dns_sans.empty()) {
for (const auto& dns : dns_sans) {
if (matcher_->Match(dns)) {
if (matcher_.Match(dns)) {
return true;
}
}
}
return matcher_->Match(args.GetSubject());
return matcher_.Match(args.GetSubject());
}
bool ReqServerNameAuthorizationMatcher::Matches(const EvaluateArgs&) const {

@ -153,13 +153,13 @@ class PortAuthorizationMatcher : public AuthorizationMatcher {
// or DNS SAN in that order, otherwise uses subject field.
class AuthenticatedAuthorizationMatcher : public AuthorizationMatcher {
public:
explicit AuthenticatedAuthorizationMatcher(absl::optional<StringMatcher> auth)
explicit AuthenticatedAuthorizationMatcher(StringMatcher auth)
: matcher_(std::move(auth)) {}
bool Matches(const EvaluateArgs& args) const override;
private:
const absl::optional<StringMatcher> matcher_;
const StringMatcher matcher_;
};
// Perform a match against the request server from the client's connection

@ -278,7 +278,7 @@ Rbac::Principal Rbac::Principal::MakeAnyPrincipal() {
}
Rbac::Principal Rbac::Principal::MakeAuthenticatedPrincipal(
absl::optional<StringMatcher> string_matcher) {
StringMatcher string_matcher) {
Principal principal;
principal.type = Principal::RuleType::kPrincipalName;
principal.string_matcher = std::move(string_matcher);
@ -398,7 +398,7 @@ std::string Rbac::Principal::ToString() const {
case RuleType::kAny:
return "any";
case RuleType::kPrincipalName:
return absl::StrFormat("principal_name=%s", string_matcher->ToString());
return absl::StrFormat("principal_name=%s", string_matcher.ToString());
case RuleType::kSourceIp:
return absl::StrFormat("source_ip=%s", ip.ToString());
case RuleType::kDirectRemoteIp:
@ -408,7 +408,7 @@ std::string Rbac::Principal::ToString() const {
case RuleType::kHeader:
return absl::StrFormat("header=%s", header_matcher.ToString());
case RuleType::kPath:
return absl::StrFormat("path=%s", string_matcher->ToString());
return absl::StrFormat("path=%s", string_matcher.ToString());
case RuleType::kMetadata:
return absl::StrFormat("%smetadata", invert ? "invert " : "");
default:

@ -113,8 +113,7 @@ struct Rbac {
std::vector<std::unique_ptr<Principal>> principals);
static Principal MakeNotPrincipal(Principal principal);
static Principal MakeAnyPrincipal();
static Principal MakeAuthenticatedPrincipal(
absl::optional<StringMatcher> string_matcher);
static Principal MakeAuthenticatedPrincipal(StringMatcher string_matcher);
static Principal MakeSourceIpPrincipal(CidrRange ip);
static Principal MakeDirectRemoteIpPrincipal(CidrRange ip);
static Principal MakeRemoteIpPrincipal(CidrRange ip);
@ -132,7 +131,7 @@ struct Rbac {
RuleType type = RuleType::kAnd;
HeaderMatcher header_matcher;
absl::optional<StringMatcher> string_matcher;
StringMatcher string_matcher;
CidrRange ip;
// For type kAnd/kOr/kNot. For kNot type, the vector will have only one
// element.

@ -456,7 +456,11 @@ TEST_F(AuthorizationMatchersTest,
args_.AddPropertyToAuthContext(GRPC_TRANSPORT_SECURITY_TYPE_PROPERTY_NAME,
GRPC_SSL_TRANSPORT_SECURITY_TYPE);
EvaluateArgs args = args_.MakeEvaluateArgs();
AuthenticatedAuthorizationMatcher matcher(/*auth=*/absl::nullopt);
AuthenticatedAuthorizationMatcher matcher(
StringMatcher::Create(StringMatcher::Type::kExact,
/*matcher=*/"",
/*case_sensitive=*/false)
.value());
EXPECT_TRUE(matcher.Matches(args));
}

@ -24,12 +24,10 @@ namespace {
MATCHER_P3(EqualsPrincipalName, expected_matcher_type, expected_matcher_value,
is_regex, "") {
return arg->type == Rbac::Principal::RuleType::kPrincipalName &&
arg->string_matcher.value().type() == expected_matcher_type &&
is_regex
? arg->string_matcher.value().regex_matcher()->pattern() ==
arg->string_matcher.type() == expected_matcher_type && is_regex
? arg->string_matcher.regex_matcher()->pattern() ==
expected_matcher_value
: arg->string_matcher.value().string_matcher() ==
expected_matcher_value;
: arg->string_matcher.string_matcher() == expected_matcher_value;
}
MATCHER_P3(EqualsPath, expected_matcher_type, expected_matcher_value, is_regex,

@ -855,13 +855,6 @@ grpc_cc_test(
grpc_cc_test(
name = "sdk_authz_end2end_test",
srcs = ["sdk_authz_end2end_test.cc"],
data = [
"//src/core/tsi/test_creds:ca.pem",
"//src/core/tsi/test_creds:client.key",
"//src/core/tsi/test_creds:client.pem",
"//src/core/tsi/test_creds:server1.key",
"//src/core/tsi/test_creds:server1.pem",
],
external_deps = [
"gtest",
],

@ -22,7 +22,6 @@
#include <grpcpp/server.h>
#include <grpcpp/server_builder.h>
#include "src/core/lib/iomgr/load_file.h"
#include "src/core/lib/security/credentials/fake/fake_credentials.h"
#include "src/cpp/client/secure_credentials.h"
#include "src/cpp/server/secure_server_credentials.h"
@ -36,52 +35,19 @@ namespace grpc {
namespace testing {
namespace {
constexpr char kCaCertPath[] = "src/core/tsi/test_creds/ca.pem";
constexpr char kServerCertPath[] = "src/core/tsi/test_creds/server1.pem";
constexpr char kServerKeyPath[] = "src/core/tsi/test_creds/server1.key";
constexpr char kClientCertPath[] = "src/core/tsi/test_creds/client.pem";
constexpr char kClientKeyPath[] = "src/core/tsi/test_creds/client.key";
constexpr char kMessage[] = "Hello";
std::string ReadFile(const char* file_path) {
grpc_slice slice;
GPR_ASSERT(
GRPC_LOG_IF_ERROR("load_file", grpc_load_file(file_path, 0, &slice)));
std::string file_contents(grpc_core::StringViewFromSlice(slice));
grpc_slice_unref(slice);
return file_contents;
}
class SdkAuthzEnd2EndTest : public ::testing::Test {
protected:
SdkAuthzEnd2EndTest()
: server_address_(
absl::StrCat("localhost:", grpc_pick_unused_port_or_die())) {
std::string root_cert = ReadFile(kCaCertPath);
std::string identity_cert = ReadFile(kServerCertPath);
std::string private_key = ReadFile(kServerKeyPath);
std::vector<experimental::IdentityKeyCertPair>
server_identity_key_cert_pairs = {{private_key, identity_cert}};
grpc::experimental::TlsServerCredentialsOptions server_options(
std::make_shared<grpc::experimental::StaticDataCertificateProvider>(
root_cert, server_identity_key_cert_pairs));
server_options.watch_root_certs();
server_options.watch_identity_key_cert_pairs();
server_options.set_cert_request_type(
GRPC_SSL_REQUEST_CLIENT_CERTIFICATE_AND_VERIFY);
server_creds_ = grpc::experimental::TlsServerCredentials(server_options);
std::vector<experimental::IdentityKeyCertPair>
channel_identity_key_cert_pairs = {
{ReadFile(kClientKeyPath), ReadFile(kClientCertPath)}};
grpc::experimental::TlsChannelCredentialsOptions channel_options;
channel_options.set_certificate_provider(
std::make_shared<grpc::experimental::StaticDataCertificateProvider>(
ReadFile(kCaCertPath), channel_identity_key_cert_pairs));
channel_options.watch_identity_key_cert_pairs();
channel_options.watch_root_certs();
channel_creds_ = grpc::experimental::TlsCredentials(channel_options);
}
absl::StrCat("localhost:", grpc_pick_unused_port_or_die())),
server_creds_(
std::shared_ptr<ServerCredentials>(new SecureServerCredentials(
grpc_fake_transport_security_server_credentials_create()))),
channel_creds_(
std::shared_ptr<ChannelCredentials>(new SecureChannelCredentials(
grpc_fake_transport_security_credentials_create()))) {}
~SdkAuthzEnd2EndTest() override { server_->Shutdown(); }
@ -124,8 +90,6 @@ class SdkAuthzEnd2EndTest : public ::testing::Test {
std::shared_ptr<Channel> BuildChannel() {
ChannelArguments args;
// Override target name for host name check
args.SetSslTargetNameOverride("foo.test.google.fr");
return ::grpc::CreateCustomChannel(server_address_, channel_creds_, args);
}
@ -371,9 +335,16 @@ TEST_F(
" \"name\": \"authz\","
" \"allow_rules\": ["
" {"
" \"name\": \"allow_mtls\","
" \"name\": \"allow_echo\","
" \"source\": {"
" \"principals\": [\"*\"]"
" \"principals\": ["
" \"foo\""
" ]"
" },"
" \"request\": {"
" \"paths\": ["
" \"*/Echo\""
" ]"
" }"
" }"
" ]"
@ -389,29 +360,6 @@ TEST_F(
EXPECT_TRUE(resp.message().empty());
}
TEST_F(SdkAuthzEnd2EndTest,
StaticInitAllowsRpcRequestWithPrincipalsFieldOnAuthenticatedConnection) {
std::string policy =
"{"
" \"name\": \"authz\","
" \"allow_rules\": ["
" {"
" \"name\": \"allow_mtls\","
" \"source\": {"
" \"principals\": [\"*\"]"
" }"
" }"
" ]"
"}";
InitServer(CreateStaticAuthzPolicyProvider(policy));
auto channel = BuildChannel();
ClientContext context;
grpc::testing::EchoResponse resp;
grpc::Status status = SendRpc(channel, &context, &resp);
EXPECT_TRUE(status.ok());
EXPECT_EQ(resp.message(), kMessage);
}
TEST_F(SdkAuthzEnd2EndTest,
FileWatcherInitAllowsRpcRequestNoMatchInDenyMatchInAllow) {
std::string policy =

Loading…
Cancel
Save