From 97263529a05f17a6f672b3687413fc2de1d33aea Mon Sep 17 00:00:00 2001 From: noahdietz Date: Mon, 21 Nov 2022 15:49:47 -0800 Subject: [PATCH 1/2] fix(transport/grpc): separate enforcement of WithoutAuthentication and WithInsecure --- transport/grpc/dial.go | 31 ++++++++++++++++++------------- 1 file changed, 18 insertions(+), 13 deletions(-) diff --git a/transport/grpc/dial.go b/transport/grpc/dial.go index c86f56507f5..074ef97facd 100644 --- a/transport/grpc/dial.go +++ b/transport/grpc/dial.go @@ -129,7 +129,15 @@ func dial(ctx context.Context, insecure bool, o *internal.DialSettings) (*grpc.C var grpcOpts []grpc.DialOption if insecure { grpcOpts = []grpc.DialOption{grpc.WithInsecure()} - } else if !o.NoAuth { + } else { + tlsConfig := &tls.Config{ + GetClientCertificate: clientCertSource, + } + grpcOpts = []grpc.DialOption{ + grpc.WithTransportCredentials(credentials.NewTLS(tlsConfig)), + } + } + if !o.NoAuth { if o.APIKey != "" { log.Print("API keys are not supported for gRPC APIs. Remove the WithAPIKey option from your client-creating call.") } @@ -142,8 +150,17 @@ func dial(ctx context.Context, insecure bool, o *internal.DialSettings) (*grpc.C o.QuotaProject = internal.QuotaProjectFromCreds(creds) } + grpcOpts = append(grpcOpts, + grpc.WithPerRPCCredentials(grpcTokenSource{ + TokenSource: oauth.TokenSource{creds.TokenSource}, + quotaProject: o.QuotaProject, + requestReason: o.RequestReason, + }), + ) + // Attempt Direct Path: if isDirectPathEnabled(endpoint, o) && isTokenSourceDirectPathCompatible(creds.TokenSource, o) && metadata.OnGCE() { + // Overwrite all of the previously specific DialOptions, DirectPath uses its own set of credentials and certificates. grpcOpts = []grpc.DialOption{ grpc.WithCredentialsBundle(grpcgoogle.NewDefaultCredentialsWithOptions(grpcgoogle.DefaultCredentialsOptions{oauth.TokenSource{creds.TokenSource}}))} if timeoutDialerOption != nil { @@ -169,18 +186,6 @@ func dial(ctx context.Context, insecure bool, o *internal.DialSettings) (*grpc.C grpc.WithDefaultServiceConfig(`{"loadBalancingConfig":[{"grpclb":{"childPolicy":[{"pick_first":{}}]}}]}`)) } // TODO(cbro): add support for system parameters (quota project, request reason) via chained interceptor. - } else { - tlsConfig := &tls.Config{ - GetClientCertificate: clientCertSource, - } - grpcOpts = []grpc.DialOption{ - grpc.WithPerRPCCredentials(grpcTokenSource{ - TokenSource: oauth.TokenSource{creds.TokenSource}, - quotaProject: o.QuotaProject, - requestReason: o.RequestReason, - }), - grpc.WithTransportCredentials(credentials.NewTLS(tlsConfig)), - } } } From ad1d1c103d5d34dd07d2d98c4ae6c2229bd0bc0f Mon Sep 17 00:00:00 2001 From: noahdietz Date: Tue, 22 Nov 2022 09:49:25 -0800 Subject: [PATCH 2/2] disallow NoAuth when insecure, refactor deprecated grpc option --- transport/grpc/dial.go | 25 +++++++++++++++++-------- 1 file changed, 17 insertions(+), 8 deletions(-) diff --git a/transport/grpc/dial.go b/transport/grpc/dial.go index 074ef97facd..2ae02ac7d00 100644 --- a/transport/grpc/dial.go +++ b/transport/grpc/dial.go @@ -25,6 +25,7 @@ import ( "google.golang.org/grpc" "google.golang.org/grpc/credentials" grpcgoogle "google.golang.org/grpc/credentials/google" + grpcinsecure "google.golang.org/grpc/credentials/insecure" "google.golang.org/grpc/credentials/oauth" // Install grpclb, which is required for direct path. @@ -126,18 +127,26 @@ func dial(ctx context.Context, insecure bool, o *internal.DialSettings) (*grpc.C if err != nil { return nil, err } - var grpcOpts []grpc.DialOption + + var transportCreds credentials.TransportCredentials if insecure { - grpcOpts = []grpc.DialOption{grpc.WithInsecure()} + transportCreds = grpcinsecure.NewCredentials() } else { - tlsConfig := &tls.Config{ + transportCreds = credentials.NewTLS(&tls.Config{ GetClientCertificate: clientCertSource, - } - grpcOpts = []grpc.DialOption{ - grpc.WithTransportCredentials(credentials.NewTLS(tlsConfig)), - } + }) } - if !o.NoAuth { + + // Initialize gRPC dial options with transport-level security options. + grpcOpts := []grpc.DialOption{ + grpc.WithTransportCredentials(transportCreds), + } + + // Authentication can only be sent when communicating over a secure connection. + // + // TODO: Should we be more lenient in the future and allow sending credentials + // when dialing an insecure connection? + if !o.NoAuth && !insecure { if o.APIKey != "" { log.Print("API keys are not supported for gRPC APIs. Remove the WithAPIKey option from your client-creating call.") }