Skip to content

Commit

Permalink
[authz] get endpoint local/peer addresses via a handshaker (#36237)
Browse files Browse the repository at this point in the history
This paves the way for removing `GetEndpoint()` from the transport API, which is a prereq for removing `grpc_endpoint_shutdown()`.

Closes #36237

COPYBARA_INTEGRATE_REVIEW=#36237 from markdroth:endpoint_filter_cleanup_rbac 46a4140
PiperOrigin-RevId: 621537397
  • Loading branch information
markdroth authored and Copybara-Service committed Apr 3, 2024
1 parent e0cade4 commit 0aebe1c
Show file tree
Hide file tree
Showing 25 changed files with 205 additions and 43 deletions.
2 changes: 2 additions & 0 deletions BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -601,6 +601,7 @@ grpc_cc_library(
"//src/core:channel_stack_type",
"//src/core:client_channel_backup_poller",
"//src/core:default_event_engine",
"//src/core:endpoint_info_handshaker",
"//src/core:experiments",
"//src/core:forkable",
"//src/core:grpc_authorization_base",
Expand Down Expand Up @@ -689,6 +690,7 @@ grpc_cc_library(
"//src/core:channel_stack_type",
"//src/core:client_channel_backup_poller",
"//src/core:default_event_engine",
"//src/core:endpoint_info_handshaker",
"//src/core:experiments",
"//src/core:forkable",
"//src/core:grpc_authorization_base",
Expand Down
3 changes: 3 additions & 0 deletions CMakeLists.txt

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions Makefile

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 2 additions & 0 deletions Package.swift

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

6 changes: 6 additions & 0 deletions build_autogenerated.yaml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions config.m4

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions config.w32

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 2 additions & 0 deletions gRPC-C++.podspec

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 3 additions & 0 deletions gRPC-Core.podspec

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 2 additions & 0 deletions grpc.gemspec

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 2 additions & 0 deletions package.xml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

29 changes: 28 additions & 1 deletion src/core/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -1270,6 +1270,33 @@ grpc_cc_library(
],
)

grpc_cc_library(
name = "endpoint_info_handshaker",
srcs = [
"lib/transport/endpoint_info_handshaker.cc",
],
hdrs = [
"lib/transport/endpoint_info_handshaker.h",
],
external_deps = [
"absl/status",
],
language = "c++",
deps = [
"channel_args",
"closure",
"handshaker_factory",
"handshaker_registry",
"//:config",
"//:debug_location",
"//:exec_ctx",
"//:gpr",
"//:handshaker",
"//:iomgr",
"//:ref_counted_ptr",
],
)

grpc_cc_library(
name = "channel_creds_registry",
hdrs = [
Expand Down Expand Up @@ -3691,6 +3718,7 @@ grpc_cc_library(
"channel_args",
"channel_fwd",
"dual_ref_counted",
"endpoint_info_handshaker",
"load_file",
"metadata_batch",
"ref_counted",
Expand All @@ -3703,7 +3731,6 @@ grpc_cc_library(
"//:grpc_credentials_util",
"//:grpc_security_base",
"//:grpc_trace",
"//:iomgr",
"//:parse_address",
"//:promise",
"//:ref_counted_ptr",
Expand Down
15 changes: 4 additions & 11 deletions src/core/ext/filters/rbac/rbac_filter.cc
Original file line number Diff line number Diff line change
Expand Up @@ -88,17 +88,10 @@ absl::StatusOr<RbacFilter> RbacFilter::Create(const ChannelArgs& args,
if (auth_context == nullptr) {
return GRPC_ERROR_CREATE("No auth context found");
}
auto* transport = args.GetObject<Transport>();
if (transport == nullptr) {
// This should never happen since the transport is always set on the server
// side.
return GRPC_ERROR_CREATE("No transport configured");
}
return RbacFilter(
grpc_channel_stack_filter_instance_number(
filter_args.channel_stack(),
filter_args.uninitialized_channel_element()),
EvaluateArgs::PerChannelArgs(auth_context, transport->GetEndpoint()));
return RbacFilter(grpc_channel_stack_filter_instance_number(
filter_args.channel_stack(),
filter_args.uninitialized_channel_element()),
EvaluateArgs::PerChannelArgs(auth_context, args));
}

void RbacFilterRegister(CoreConfiguration::Builder* builder) {
Expand Down
11 changes: 6 additions & 5 deletions src/core/lib/security/authorization/evaluate_args.cc
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
#include "src/core/lib/gprpp/host_port.h"
#include "src/core/lib/security/credentials/tls/tls_utils.h"
#include "src/core/lib/slice/slice.h"
#include "src/core/lib/transport/endpoint_info_handshaker.h"
#include "src/core/lib/uri/uri_parser.h"

namespace grpc_core {
Expand Down Expand Up @@ -70,7 +71,7 @@ EvaluateArgs::PerChannelArgs::Address ParseEndpointUri(
} // namespace

EvaluateArgs::PerChannelArgs::PerChannelArgs(grpc_auth_context* auth_context,
grpc_endpoint* endpoint) {
const ChannelArgs& args) {
if (auth_context != nullptr) {
transport_security_type = GetAuthPropertyValue(
auth_context, GRPC_TRANSPORT_SECURITY_TYPE_PROPERTY_NAME);
Expand All @@ -83,10 +84,10 @@ EvaluateArgs::PerChannelArgs::PerChannelArgs(grpc_auth_context* auth_context,
subject =
GetAuthPropertyValue(auth_context, GRPC_X509_SUBJECT_PROPERTY_NAME);
}
if (endpoint != nullptr) {
local_address = ParseEndpointUri(grpc_endpoint_get_local_address(endpoint));
peer_address = ParseEndpointUri(grpc_endpoint_get_peer(endpoint));
}
local_address = ParseEndpointUri(
args.GetString(GRPC_ARG_ENDPOINT_LOCAL_ADDRESS).value_or(""));
peer_address = ParseEndpointUri(
args.GetString(GRPC_ARG_ENDPOINT_PEER_ADDRESS).value_or(""));
}

absl::string_view EvaluateArgs::GetPath() const {
Expand Down
6 changes: 3 additions & 3 deletions src/core/lib/security/authorization/evaluate_args.h
Original file line number Diff line number Diff line change
Expand Up @@ -25,13 +25,13 @@

#include <grpc/grpc_security.h>

#include "src/core/lib/iomgr/endpoint.h"
#include "src/core/lib/channel/channel_args.h"
#include "src/core/lib/iomgr/resolved_address.h"
#include "src/core/lib/transport/metadata_batch.h"

namespace grpc_core {

class EvaluateArgs {
class EvaluateArgs final {
public:
// Caller is responsible for ensuring auth_context outlives PerChannelArgs
// struct.
Expand All @@ -44,7 +44,7 @@ class EvaluateArgs {
int port = 0;
};

PerChannelArgs(grpc_auth_context* auth_context, grpc_endpoint* endpoint);
PerChannelArgs(grpc_auth_context* auth_context, const ChannelArgs& args);

absl::string_view transport_security_type;
absl::string_view spiffe_id;
Expand Down
11 changes: 4 additions & 7 deletions src/core/lib/security/authorization/grpc_server_authz_filter.cc
Original file line number Diff line number Diff line change
Expand Up @@ -46,10 +46,10 @@ const NoInterceptor GrpcServerAuthzFilter::Call::OnServerToClientMessage;
const NoInterceptor GrpcServerAuthzFilter::Call::OnFinalize;

GrpcServerAuthzFilter::GrpcServerAuthzFilter(
RefCountedPtr<grpc_auth_context> auth_context, grpc_endpoint* endpoint,
RefCountedPtr<grpc_auth_context> auth_context, const ChannelArgs& args,
RefCountedPtr<grpc_authorization_policy_provider> provider)
: auth_context_(std::move(auth_context)),
per_channel_evaluate_args_(auth_context_.get(), endpoint),
per_channel_evaluate_args_(auth_context_.get(), args),
provider_(std::move(provider)) {}

absl::StatusOr<GrpcServerAuthzFilter> GrpcServerAuthzFilter::Create(
Expand All @@ -59,12 +59,9 @@ absl::StatusOr<GrpcServerAuthzFilter> GrpcServerAuthzFilter::Create(
if (provider == nullptr) {
return absl::InvalidArgumentError("Failed to get authorization provider.");
}
// grpc_endpoint isn't needed because the current gRPC authorization policy
// does not support any rules that requires looking for source or destination
// addresses.
return GrpcServerAuthzFilter(
auth_context != nullptr ? auth_context->Ref() : nullptr,
/*endpoint=*/nullptr, provider->Ref());
auth_context != nullptr ? auth_context->Ref() : nullptr, args,
provider->Ref());
}

bool GrpcServerAuthzFilter::IsAuthorized(ClientMetadata& initial_metadata) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@
#include "src/core/lib/channel/channel_fwd.h"
#include "src/core/lib/channel/promise_based_filter.h"
#include "src/core/lib/gprpp/ref_counted_ptr.h"
#include "src/core/lib/iomgr/endpoint.h"
#include "src/core/lib/promise/arena_promise.h"
#include "src/core/lib/security/authorization/authorization_policy_provider.h"
#include "src/core/lib/security/authorization/evaluate_args.h"
Expand Down Expand Up @@ -55,7 +54,7 @@ class GrpcServerAuthzFilter final

private:
GrpcServerAuthzFilter(
RefCountedPtr<grpc_auth_context> auth_context, grpc_endpoint* endpoint,
RefCountedPtr<grpc_auth_context> auth_context, const ChannelArgs& args,
RefCountedPtr<grpc_authorization_policy_provider> provider);

bool IsAuthorized(ClientMetadata& initial_metadata);
Expand Down

0 comments on commit 0aebe1c

Please sign in to comment.