-
Notifications
You must be signed in to change notification settings - Fork 5.3k
Support for external authorization grpc service. #2415
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.
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
Changes from all commits
79b97f0
06a0eec
bf7d7be
7834705
48edb72
1612396
4a197fb
141a203
0df4b9a
cea2ed6
b4c7fea
93776d8
195df83
e0e6b77
1f84caa
737171c
3737325
61f8f8f
8ffb69d
906ec7c
3d88e76
00c957e
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,18 @@ | ||
| licenses(["notice"]) # Apache 2 | ||
|
|
||
| load( | ||
| "//bazel:envoy_build_system.bzl", | ||
| "envoy_cc_library", | ||
| "envoy_package", | ||
| ) | ||
|
|
||
| envoy_package() | ||
|
|
||
| envoy_cc_library( | ||
| name = "ext_authz_interface", | ||
| hdrs = ["ext_authz.h"], | ||
| deps = [ | ||
| "//include/envoy/tracing:http_tracer_interface", | ||
| "@envoy_api//envoy/service/auth/v2:external_auth_cc", | ||
| ], | ||
| ) |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,67 @@ | ||
| #pragma once | ||
|
|
||
| #include <chrono> | ||
| #include <memory> | ||
| #include <string> | ||
| #include <vector> | ||
|
|
||
| #include "envoy/common/pure.h" | ||
| #include "envoy/service/auth/v2/external_auth.pb.h" | ||
| #include "envoy/tracing/http_tracer.h" | ||
|
|
||
| namespace Envoy { | ||
| namespace ExtAuthz { | ||
|
|
||
| /** | ||
| * Possible async results for a check call. | ||
| */ | ||
| enum class CheckStatus { | ||
| // The request is authorized. | ||
| OK, | ||
| // The authz service could not be queried. | ||
| Error, | ||
| // The request is denied. | ||
| Denied | ||
| }; | ||
|
|
||
| /** | ||
| * Async callbacks used during check() calls. | ||
| */ | ||
| class RequestCallbacks { | ||
| public: | ||
| virtual ~RequestCallbacks() {} | ||
|
|
||
| /** | ||
| * Called when a check request is complete. The resulting status is supplied. | ||
| */ | ||
| virtual void onComplete(CheckStatus status) PURE; | ||
| }; | ||
|
|
||
| class Client { | ||
| public: | ||
| // Destructor | ||
| virtual ~Client() {} | ||
|
|
||
| /** | ||
| * Cancel an inflight Check request. | ||
| */ | ||
| virtual void cancel() PURE; | ||
|
|
||
| /** | ||
| * Request a check call to an external authorization service which can use the | ||
| * passed request parameters to make a permit/deny decision. | ||
| * @param callback supplies the completion callbacks. | ||
| * NOTE: The callback may happen within the calling stack. | ||
| * @param request is the proto message with the attributes of the specific payload. | ||
| * @param parent_span source for generating an egress child span as part of the trace. | ||
| * | ||
| */ | ||
| virtual void check(RequestCallbacks& callback, | ||
| const envoy::service::auth::v2::CheckRequest& request, | ||
| Tracing::Span& parent_span) PURE; | ||
| }; | ||
|
|
||
| typedef std::unique_ptr<Client> ClientPtr; | ||
|
|
||
| } // namespace ExtAuthz | ||
| } // namespace Envoy | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -69,7 +69,7 @@ void HttpGrpcAccessLog::responseFlagsToAccessLogResponseFlags( | |
| envoy::config::filter::accesslog::v2::AccessLogCommon& common_access_log, | ||
| const RequestInfo::RequestInfo& request_info) { | ||
|
|
||
| static_assert(RequestInfo::ResponseFlag::LastFlag == 0x800, | ||
| static_assert(RequestInfo::ResponseFlag::LastFlag == 0x1000, | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Seems like gRPC access logging protos should be enhanced for unauthorized? Can you add a TODO and take care of this in one of your follow ups?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. yes. I'll do it in the follow up pr; thanks. |
||
| "A flag has been added. Fix this code."); | ||
|
|
||
| if (request_info.getResponseFlag(RequestInfo::ResponseFlag::FailedLocalHealthCheck)) { | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,35 @@ | ||
| licenses(["notice"]) # Apache 2 | ||
|
|
||
| load( | ||
| "//bazel:envoy_build_system.bzl", | ||
| "envoy_cc_library", | ||
| "envoy_package", | ||
| ) | ||
|
|
||
| envoy_package() | ||
|
|
||
| envoy_cc_library( | ||
| name = "ext_authz_lib", | ||
| srcs = ["ext_authz_impl.cc"], | ||
| hdrs = ["ext_authz_impl.h"], | ||
| deps = [ | ||
| "//include/envoy/ext_authz:ext_authz_interface", | ||
| "//include/envoy/grpc:async_client_interface", | ||
| "//include/envoy/grpc:async_client_manager_interface", | ||
| "//include/envoy/http:filter_interface", | ||
| "//include/envoy/http:header_map_interface", | ||
| "//include/envoy/http:protocol_interface", | ||
| "//include/envoy/network:address_interface", | ||
| "//include/envoy/network:connection_interface", | ||
| "//include/envoy/network:filter_interface", | ||
| "//include/envoy/ssl:connection_interface", | ||
| "//include/envoy/upstream:cluster_manager_interface", | ||
| "//source/common/common:assert_lib", | ||
| "//source/common/grpc:async_client_lib", | ||
| "//source/common/http:headers_lib", | ||
| "//source/common/http:utility_lib", | ||
| "//source/common/network:utility_lib", | ||
| "//source/common/protobuf", | ||
| "//source/common/tracing:http_tracer_lib", | ||
| ], | ||
| ) |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,189 @@ | ||
| #include "common/ext_authz/ext_authz_impl.h" | ||
|
|
||
| #include <chrono> | ||
| #include <cstdint> | ||
| #include <string> | ||
| #include <vector> | ||
|
|
||
| #include "envoy/access_log/access_log.h" | ||
| #include "envoy/ssl/connection.h" | ||
|
|
||
| #include "common/common/assert.h" | ||
| #include "common/grpc/async_client_impl.h" | ||
| #include "common/http/headers.h" | ||
| #include "common/http/utility.h" | ||
| #include "common/network/utility.h" | ||
| #include "common/protobuf/protobuf.h" | ||
|
|
||
| #include "fmt/format.h" | ||
|
|
||
| namespace Envoy { | ||
| namespace ExtAuthz { | ||
|
|
||
| GrpcClientImpl::GrpcClientImpl(Grpc::AsyncClientPtr&& async_client, | ||
| const Optional<std::chrono::milliseconds>& timeout) | ||
| : service_method_(*Protobuf::DescriptorPool::generated_pool()->FindMethodByName( | ||
| "envoy.service.auth.v2.Authorization.Check")), | ||
| async_client_(std::move(async_client)), timeout_(timeout) {} | ||
|
|
||
| GrpcClientImpl::~GrpcClientImpl() { ASSERT(!callbacks_); } | ||
|
|
||
| void GrpcClientImpl::cancel() { | ||
| ASSERT(callbacks_ != nullptr); | ||
| request_->cancel(); | ||
| callbacks_ = nullptr; | ||
| } | ||
|
|
||
| void GrpcClientImpl::check(RequestCallbacks& callbacks, | ||
| const envoy::service::auth::v2::CheckRequest& request, | ||
| Tracing::Span& parent_span) { | ||
| ASSERT(callbacks_ == nullptr); | ||
| callbacks_ = &callbacks; | ||
|
|
||
| request_ = async_client_->send(service_method_, request, *this, parent_span, timeout_); | ||
| } | ||
|
|
||
| void GrpcClientImpl::onSuccess(std::unique_ptr<envoy::service::auth::v2::CheckResponse>&& response, | ||
| Tracing::Span& span) { | ||
| CheckStatus status = CheckStatus::OK; | ||
| ASSERT(response->status().code() != Grpc::Status::GrpcStatus::Unknown); | ||
| if (response->status().code() != Grpc::Status::GrpcStatus::Ok) { | ||
| status = CheckStatus::Denied; | ||
| span.setTag(Constants::get().TraceStatus, Constants::get().TraceUnauthz); | ||
| } else { | ||
| span.setTag(Constants::get().TraceStatus, Constants::get().TraceOk); | ||
| } | ||
|
|
||
| callbacks_->onComplete(status); | ||
| callbacks_ = nullptr; | ||
| } | ||
|
|
||
| void GrpcClientImpl::onFailure(Grpc::Status::GrpcStatus status, const std::string&, | ||
| Tracing::Span&) { | ||
| ASSERT(status != Grpc::Status::GrpcStatus::Ok); | ||
| UNREFERENCED_PARAMETER(status); | ||
| callbacks_->onComplete(CheckStatus::Error); | ||
| callbacks_ = nullptr; | ||
| } | ||
|
|
||
| void CreateCheckRequest::setAttrContextPeer(envoy::service::auth::v2::AttributeContext_Peer& peer, | ||
| const Network::Connection& connection, | ||
| const std::string& service, const bool local) { | ||
|
|
||
| // Set the address | ||
| auto addr = peer.mutable_address(); | ||
| if (local) { | ||
| Envoy::Network::Utility::addressToProtobufAddress(*connection.localAddress(), *addr); | ||
| } else { | ||
| Envoy::Network::Utility::addressToProtobufAddress(*connection.remoteAddress(), *addr); | ||
| } | ||
|
|
||
| // Set the principal | ||
| // Preferably the SAN from the peer's cert or | ||
| // Subject from the peer's cert. | ||
| Ssl::Connection* ssl = const_cast<Ssl::Connection*>(connection.ssl()); | ||
| if (ssl != nullptr) { | ||
| if (local) { | ||
| peer.set_principal(ssl->uriSanLocalCertificate()); | ||
|
|
||
| if (peer.principal().empty()) { | ||
| peer.set_principal(ssl->subjectLocalCertificate()); | ||
| } | ||
| } else { | ||
| peer.set_principal(ssl->uriSanPeerCertificate()); | ||
|
|
||
| if (peer.principal().empty()) { | ||
| peer.set_principal(ssl->subjectPeerCertificate()); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| if (!service.empty()) { | ||
| peer.set_service(service); | ||
| } | ||
| } | ||
|
|
||
| std::string CreateCheckRequest::getHeaderStr(const Envoy::Http::HeaderEntry* entry) { | ||
| if (entry) { | ||
| return entry->value().getString(); | ||
| } | ||
| return ""; | ||
| } | ||
|
|
||
| void CreateCheckRequest::setHttpRequest( | ||
| ::envoy::service::auth::v2::AttributeContext_HttpRequest& httpreq, | ||
| const Envoy::Http::StreamDecoderFilterCallbacks* callbacks, | ||
| const Envoy::Http::HeaderMap& headers) { | ||
|
|
||
| // Set id | ||
| // The streamId is not qualified as a const. Although it is as it does not modify the object. | ||
| Envoy::Http::StreamDecoderFilterCallbacks* sdfc = | ||
| const_cast<Envoy::Http::StreamDecoderFilterCallbacks*>(callbacks); | ||
| httpreq.set_id(std::to_string(sdfc->streamId())); | ||
|
|
||
| // Set method | ||
| httpreq.set_method(getHeaderStr(headers.Method())); | ||
| // Set path | ||
| httpreq.set_path(getHeaderStr(headers.Path())); | ||
| // Set host | ||
| httpreq.set_host(getHeaderStr(headers.Host())); | ||
| // Set scheme | ||
| httpreq.set_scheme(getHeaderStr(headers.Scheme())); | ||
|
|
||
| // Set size | ||
| // need to convert to google buffer 64t; | ||
| httpreq.set_size(sdfc->requestInfo().bytesReceived()); | ||
|
|
||
| // Set protocol | ||
| if (sdfc->requestInfo().protocol().valid()) { | ||
| httpreq.set_protocol( | ||
| Envoy::Http::Utility::getProtocolString(sdfc->requestInfo().protocol().value())); | ||
| } | ||
|
|
||
| // Fill in the headers | ||
| auto mutable_headers = httpreq.mutable_headers(); | ||
| headers.iterate( | ||
| [](const Envoy::Http::HeaderEntry& e, void* ctx) { | ||
| Envoy::Protobuf::Map<::std::string, ::std::string>* mutable_headers = | ||
| static_cast<Envoy::Protobuf::Map<::std::string, ::std::string>*>(ctx); | ||
| (*mutable_headers)[e.key().getString()] = e.value().getString(); | ||
| return Envoy::Http::HeaderMap::Iterate::Continue; | ||
| }, | ||
| mutable_headers); | ||
| } | ||
|
|
||
| void CreateCheckRequest::setAttrContextRequest( | ||
| ::envoy::service::auth::v2::AttributeContext_Request& req, | ||
| const Envoy::Http::StreamDecoderFilterCallbacks* callbacks, | ||
| const Envoy::Http::HeaderMap& headers) { | ||
| setHttpRequest(*req.mutable_http(), callbacks, headers); | ||
| } | ||
|
|
||
| void CreateCheckRequest::createHttpCheck(const Envoy::Http::StreamDecoderFilterCallbacks* callbacks, | ||
| const Envoy::Http::HeaderMap& headers, | ||
| envoy::service::auth::v2::CheckRequest& request) { | ||
|
|
||
| auto attrs = request.mutable_attributes(); | ||
|
|
||
| Envoy::Http::StreamDecoderFilterCallbacks* cb = | ||
| const_cast<Envoy::Http::StreamDecoderFilterCallbacks*>(callbacks); | ||
|
|
||
| const std::string service = getHeaderStr(headers.EnvoyDownstreamServiceCluster()); | ||
|
|
||
| setAttrContextPeer(*attrs->mutable_source(), *cb->connection(), service, false); | ||
| setAttrContextPeer(*attrs->mutable_destination(), *cb->connection(), "", true); | ||
| setAttrContextRequest(*attrs->mutable_request(), callbacks, headers); | ||
| } | ||
|
|
||
| void CreateCheckRequest::createTcpCheck(const Network::ReadFilterCallbacks* callbacks, | ||
| envoy::service::auth::v2::CheckRequest& request) { | ||
|
|
||
| auto attrs = request.mutable_attributes(); | ||
|
|
||
| Network::ReadFilterCallbacks* cb = const_cast<Network::ReadFilterCallbacks*>(callbacks); | ||
| setAttrContextPeer(*attrs->mutable_source(), cb->connection(), "", false); | ||
| setAttrContextPeer(*attrs->mutable_destination(), cb->connection(), "", true); | ||
| } | ||
|
|
||
| } // namespace ExtAuthz | ||
| } // namespace Envoy |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Question: can there only ever be a single inflight request per client with this interface? Does this imply that each request must have its own
Client?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thinking about this a bit more, maybe we don't need to do the TLS thing at all here. This is unary RPC, so we don't have a significant cost in rebuilding the client each time, it should be a fairly small construction/destruction cost. I think what you have in this PR is fine, I would be more concerned if we had a bidi streaming auth check.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was thinking that the per-thread gRPC would save us setup time and hence would help even in the unary gRPC case. But if the cluster is going to do that for us then I agree changing it would not buy us much.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, for Envoy gRPC, this will come for free from the cluster pool management. In theory, Google gRPC also does this, based on good authority, but I haven't validated. Let's work on your other PRs first and we can circle back to this later.