From 69aa887e4fef6ce6e1fba046f56a9a81a98e33f6 Mon Sep 17 00:00:00 2001 From: Richard Belleville Date: Fri, 26 Jun 2020 02:09:46 +0000 Subject: [PATCH] Expose gce tenancy as a function --- include/grpc/grpc_security.h | 6 ++++-- .../compute_engine_channel_credentials.cc | 20 +++++++------------ .../google_default_credentials.cc | 18 +++++++++++++---- .../google_default_credentials.h | 2 ++ 4 files changed, 27 insertions(+), 19 deletions(-) diff --git a/include/grpc/grpc_security.h b/include/grpc/grpc_security.h index 4d7850fc438..de78871e025 100644 --- a/include/grpc/grpc_security.h +++ b/include/grpc/grpc_security.h @@ -301,7 +301,8 @@ GRPCAPI grpc_call_credentials* grpc_composite_call_credentials_create( GRPCAPI grpc_call_credentials* grpc_google_compute_engine_credentials_create( void* reserved); -/** Creates compute engine channel credentials to connect to a google gRPC service. +/** Creates compute engine channel credentials to connect to a google gRPC + service. This channel credential is expected to be used within a composite credential alongside a compute_engine_credential. If used in conjunction with any call @@ -311,7 +312,8 @@ GRPCAPI grpc_call_credentials* grpc_google_compute_engine_credentials_create( WARNING: Do NOT use this credentials to connect to a non-google service as this could result in an oauth2 token leak. The security level of the resulting connection is GRPC_PRIVACY_AND_INTEGRITY. */ -GRPCAPI grpc_channel_credentials* grpc_compute_engine_channel_credentials_create(void* reserved); +GRPCAPI grpc_channel_credentials* +grpc_compute_engine_channel_credentials_create(void* reserved); GRPCAPI gpr_timespec grpc_max_auth_token_lifetime(void); diff --git a/src/core/lib/security/credentials/google_default/compute_engine_channel_credentials.cc b/src/core/lib/security/credentials/google_default/compute_engine_channel_credentials.cc index 0a39102b18e..b9607365ecb 100644 --- a/src/core/lib/security/credentials/google_default/compute_engine_channel_credentials.cc +++ b/src/core/lib/security/credentials/google_default/compute_engine_channel_credentials.cc @@ -44,18 +44,13 @@ #include "src/core/lib/slice/slice_string_helpers.h" #include "src/core/lib/surface/api_trace.h" -grpc_channel_credentials* grpc_compute_engine_channel_credentials_create(void* reserved) { - // If we haven't initialized the google_default_credentials singleton, - // then we don't know whether or not we're on GCE and can't safely - // created an ALTS connection. - // TODO: Fix. - auto default_warmer = grpc_google_default_credentials_create(); +grpc_channel_credentials* grpc_compute_engine_channel_credentials_create( + void* reserved) { grpc_core::ExecCtx exec_ctx; - GRPC_API_TRACE("grpc_gce_channel_credentials_create(%p)", 1, - (reserved)); + GRPC_API_TRACE("grpc_gce_channel_credentials_create(%p)", 1, (reserved)); - // TODO: Should we cache this here? + GPR_ASSERT(grpc_core::internal::is_on_gce()); grpc_channel_credentials* ssl_creds = grpc_ssl_credentials_create(nullptr, nullptr, nullptr, nullptr); GPR_ASSERT(ssl_creds != nullptr); @@ -64,10 +59,9 @@ grpc_channel_credentials* grpc_compute_engine_channel_credentials_create(void* r grpc_channel_credentials* alts_creds = grpc_alts_credentials_create(options); grpc_alts_credentials_options_destroy(options); - auto creds = - new grpc_google_default_channel_credentials( - alts_creds != nullptr ? alts_creds->Ref() : nullptr, - ssl_creds != nullptr ? ssl_creds->Ref() : nullptr); + auto creds = new grpc_google_default_channel_credentials( + alts_creds != nullptr ? alts_creds->Ref() : nullptr, + ssl_creds != nullptr ? ssl_creds->Ref() : nullptr); if (ssl_creds) ssl_creds->Unref(); if (alts_creds) alts_creds->Unref(); diff --git a/src/core/lib/security/credentials/google_default/google_default_credentials.cc b/src/core/lib/security/credentials/google_default/google_default_credentials.cc index f7dc85ec106..0476860b625 100644 --- a/src/core/lib/security/credentials/google_default/google_default_credentials.cc +++ b/src/core/lib/security/credentials/google_default/google_default_credentials.cc @@ -30,6 +30,7 @@ #include "src/core/lib/channel/channel_args.h" #include "src/core/lib/gpr/env.h" #include "src/core/lib/gpr/string.h" +#include "src/core/lib/gprpp/atomic.h" #include "src/core/lib/gprpp/ref_counted_ptr.h" #include "src/core/lib/http/httpcli.h" #include "src/core/lib/http/parser.h" @@ -57,7 +58,7 @@ using grpc_core::Json; * means the detection is done via network test that is unreliable and the * unreliable result should not be referred by successive calls. */ static int g_metadata_server_available = 0; -static int g_is_on_gce = 0; +static grpc_core::Atomic g_is_on_gce(false); static gpr_mu g_state_mu; /* Protect a metadata_server_detector instance that can be modified by more than * one gRPC threads */ @@ -89,7 +90,7 @@ grpc_google_default_channel_credentials::create_security_connector( bool use_alts = is_grpclb_load_balancer || is_backend_from_grpclb_load_balancer; /* Return failure if ALTS is selected but not running on GCE. */ - if (use_alts && !g_is_on_gce) { + if (use_alts && !grpc_core::internal::is_on_gce()) { gpr_log(GPR_ERROR, "ALTS is selected, but not running on GCE."); return nullptr; } @@ -301,8 +302,7 @@ grpc_channel_credentials* grpc_google_default_credentials_create() { /* Try a platform-provided hint for GCE. */ if (!g_metadata_server_available) { - g_is_on_gce = g_gce_tenancy_checker(); - g_metadata_server_available = g_is_on_gce; + g_metadata_server_available = grpc_core::internal::is_on_gce(); } /* TODO: Add a platform-provided hint for GAE. */ @@ -365,6 +365,16 @@ void grpc_flush_cached_google_default_credentials(void) { gpr_mu_unlock(&g_state_mu); } +bool is_on_gce(void) { + bool on_gce; + if (GPR_UNLIKELY( + !(on_gce = g_is_on_gce.Load(grpc_core::MemoryOrder::ACQUIRE)))) { + on_gce = g_gce_tenancy_checker(); + g_is_on_gce.Store(on_gce, grpc_core::MemoryOrder::RELEASE); + } + return on_gce; +} + } // namespace internal } // namespace grpc_core diff --git a/src/core/lib/security/credentials/google_default/google_default_credentials.h b/src/core/lib/security/credentials/google_default/google_default_credentials.h index 8a945da31e2..0be0fd3817e 100644 --- a/src/core/lib/security/credentials/google_default/google_default_credentials.h +++ b/src/core/lib/security/credentials/google_default/google_default_credentials.h @@ -77,6 +77,8 @@ typedef bool (*grpc_gce_tenancy_checker)(void); void set_gce_tenancy_checker_for_testing(grpc_gce_tenancy_checker checker); +inline bool is_on_gce(void); + // TEST-ONLY. Reset the internal global state. void grpc_flush_cached_google_default_credentials(void);