From 71ec6cae2e66d877afd6945280d2ca8b0c6a00f5 Mon Sep 17 00:00:00 2001 From: Chris Smith Date: Tue, 19 Nov 2024 16:59:52 -0700 Subject: [PATCH 1/7] feat(auth): add universe domain support to mTLS * Remove EXPERIMENTAL field DefaultMTLSEndpoint from httptransport.InternalOptions * Add EXPERIMENTAL field DefaultMTLSEndpointTemplate to httptransport.InternalOptions * Remove EXPERIMENTAL field DefaultMTLSEndpoint from grpctransport.InternalOptions * Add EXPERIMENTAL field DefaultMTLSEndpointTemplate to grpctransport.InternalOptions * Remove runtime error for mTLS usage with non-GDU universe domain. refs: googleapis/google-api-go-client#2880 --- auth/grpctransport/grpctransport.go | 7 +- auth/httptransport/httptransport.go | 7 +- auth/internal/transport/cba.go | 52 +++--- auth/internal/transport/cba_test.go | 245 ++++++++++++++-------------- auth/internal/transport/s2a.go | 2 +- 5 files changed, 156 insertions(+), 157 deletions(-) diff --git a/auth/grpctransport/grpctransport.go b/auth/grpctransport/grpctransport.go index 38212ed0f82a..dea74e82ae32 100644 --- a/auth/grpctransport/grpctransport.go +++ b/auth/grpctransport/grpctransport.go @@ -197,8 +197,9 @@ type InternalOptions struct { // DefaultEndpointTemplate combined with UniverseDomain specifies // the default endpoint. DefaultEndpointTemplate string - // DefaultMTLSEndpoint specifies the default mTLS endpoint. - DefaultMTLSEndpoint string + // DefaultMTLSEndpointTemplate combined with UniverseDomain specifies the + // default mTLS endpoint. + DefaultMTLSEndpointTemplate string // DefaultScopes specifies the default OAuth2 scopes to be used for a // service. DefaultScopes []string @@ -244,7 +245,7 @@ func dial(ctx context.Context, secure bool, opts *Options) (*grpc.ClientConn, er } if io := opts.InternalOptions; io != nil { tOpts.DefaultEndpointTemplate = io.DefaultEndpointTemplate - tOpts.DefaultMTLSEndpoint = io.DefaultMTLSEndpoint + tOpts.DefaultMTLSEndpointTemplate = io.DefaultMTLSEndpointTemplate tOpts.EnableDirectPath = io.EnableDirectPath tOpts.EnableDirectPathXds = io.EnableDirectPathXds } diff --git a/auth/httptransport/httptransport.go b/auth/httptransport/httptransport.go index cbe5a7a40a77..2434859fcfd6 100644 --- a/auth/httptransport/httptransport.go +++ b/auth/httptransport/httptransport.go @@ -141,8 +141,9 @@ type InternalOptions struct { // DefaultEndpointTemplate combined with UniverseDomain specifies the // default endpoint. DefaultEndpointTemplate string - // DefaultMTLSEndpoint specifies the default mTLS endpoint. - DefaultMTLSEndpoint string + // DefaultMTLSEndpointTemplate combined with UniverseDomain specifies the + // default mTLS endpoint. + DefaultMTLSEndpointTemplate string // DefaultScopes specifies the default OAuth2 scopes to be used for a // service. DefaultScopes []string @@ -200,7 +201,7 @@ func NewClient(opts *Options) (*http.Client, error) { } if io := opts.InternalOptions; io != nil { tOpts.DefaultEndpointTemplate = io.DefaultEndpointTemplate - tOpts.DefaultMTLSEndpoint = io.DefaultMTLSEndpoint + tOpts.DefaultMTLSEndpointTemplate = io.DefaultMTLSEndpointTemplate } clientCertProvider, dialTLSContext, err := transport.GetHTTPTransportConfig(tOpts) if err != nil { diff --git a/auth/internal/transport/cba.go b/auth/internal/transport/cba.go index f606888f1204..c2d8e28fc21e 100644 --- a/auth/internal/transport/cba.go +++ b/auth/internal/transport/cba.go @@ -51,22 +51,18 @@ const ( mtlsMDSKey = "/run/google-mds-mtls/client.key" ) -var ( - errUniverseNotSupportedMTLS = errors.New("mTLS is not supported in any universe other than googleapis.com") -) - // Options is a struct that is duplicated information from the individual // transport packages in order to avoid cyclic deps. It correlates 1:1 with // fields on httptransport.Options and grpctransport.Options. type Options struct { - Endpoint string - DefaultMTLSEndpoint string - DefaultEndpointTemplate string - ClientCertProvider cert.Provider - Client *http.Client - UniverseDomain string - EnableDirectPath bool - EnableDirectPathXds bool + Endpoint string + DefaultEndpointTemplate string + DefaultMTLSEndpointTemplate string + ClientCertProvider cert.Provider + Client *http.Client + UniverseDomain string + EnableDirectPath bool + EnableDirectPathXds bool } // getUniverseDomain returns the default service domain for a given Cloud @@ -94,6 +90,16 @@ func (o *Options) defaultEndpoint() string { return strings.Replace(o.DefaultEndpointTemplate, universeDomainPlaceholder, o.getUniverseDomain(), 1) } +// defaultMTLSEndpoint returns the DefaultMTLSEndpointTemplate merged with the +// universe domain if the DefaultMTLSEndpointTemplate is set, otherwise returns an +// empty string. +func (o *Options) defaultMTLSEndpoint() string { + if o.DefaultMTLSEndpointTemplate == "" { + return "" + } + return strings.Replace(o.DefaultMTLSEndpointTemplate, universeDomainPlaceholder, o.getUniverseDomain(), 1) +} + // mergedEndpoint merges a user-provided Endpoint of format host[:port] with the // default endpoint. func (o *Options) mergedEndpoint() (string, error) { @@ -256,9 +262,6 @@ func getTransportConfig(opts *Options) (*transportConfig, error) { if !shouldUseS2A(clientCertSource, opts) { return &defaultTransportConfig, nil } - if !opts.isUniverseDomainGDU() { - return nil, errUniverseNotSupportedMTLS - } s2aAddress := GetS2AAddress() mtlsS2AAddress := GetMTLSS2AAddress() @@ -270,7 +273,7 @@ func getTransportConfig(opts *Options) (*transportConfig, error) { endpoint: endpoint, s2aAddress: s2aAddress, mtlsS2AAddress: mtlsS2AAddress, - s2aMTLSEndpoint: opts.DefaultMTLSEndpoint, + s2aMTLSEndpoint: opts.defaultMTLSEndpoint(), }, nil } @@ -316,24 +319,23 @@ type transportConfig struct { // getEndpoint returns the endpoint for the service, taking into account the // user-provided endpoint override "settings.Endpoint". // -// If no endpoint override is specified, we will either return the default endpoint or -// the default mTLS endpoint if a client certificate is available. +// If no endpoint override is specified, we will either return the default +// endpoint or the default mTLS endpoint if a client certificate is available. // -// You can override the default endpoint choice (mtls vs. regular) by setting the -// GOOGLE_API_USE_MTLS_ENDPOINT environment variable. +// You can override the default endpoint choice (mTLS vs. regular) by setting +// the GOOGLE_API_USE_MTLS_ENDPOINT environment variable. // // If the endpoint override is an address (host:port) rather than full base // URL (ex. https://...), then the user-provided address will be merged into // the default endpoint. For example, WithEndpoint("myhost:8000") and -// DefaultEndpointTemplate("https://UNIVERSE_DOMAIN/bar/baz") will return "https://myhost:8080/bar/baz" +// DefaultEndpointTemplate("https://UNIVERSE_DOMAIN/bar/baz") will return +// "https://myhost:8080/bar/baz". Note that this does not apply to the mTLS +// endpoint. func getEndpoint(opts *Options, clientCertSource cert.Provider) (string, error) { if opts.Endpoint == "" { mtlsMode := getMTLSMode() if mtlsMode == mTLSModeAlways || (clientCertSource != nil && mtlsMode == mTLSModeAuto) { - if !opts.isUniverseDomainGDU() { - return "", errUniverseNotSupportedMTLS - } - return opts.DefaultMTLSEndpoint, nil + return opts.defaultMTLSEndpoint(), nil } return opts.defaultEndpoint(), nil } diff --git a/auth/internal/transport/cba_test.go b/auth/internal/transport/cba_test.go index 3c0a90269ac4..e60d4e21e9fe 100644 --- a/auth/internal/transport/cba_test.go +++ b/auth/internal/transport/cba_test.go @@ -26,12 +26,14 @@ import ( ) const ( - testMTLSEndpoint = "https://test.mtls.googleapis.com/" - testEndpointTemplate = "https://test.UNIVERSE_DOMAIN/" - testRegularEndpoint = "https://test.googleapis.com/" - testOverrideEndpoint = "https://test.override.example.com/" - testUniverseDomain = "example.com" - testUniverseDomainEndpoint = "https://test.example.com/" + testEndpointTemplate = "https://test.UNIVERSE_DOMAIN/" + testMTLSEndpointTemplate = "https://test.mtls.UNIVERSE_DOMAIN/" + testDefaultUniverseEndpoint = "https://test.googleapis.com/" + testDefaultUniverseMTLSEndpoint = "https://test.mtls.googleapis.com/" + testOverrideEndpoint = "https://test.override.example.com/" + testUniverseDomain = "example.com" + testUniverseDomainEndpoint = "https://test.example.com/" + testUniverseDomainMTLSEndpoint = "https://test.mtls.example.com/" ) var ( @@ -260,9 +262,9 @@ func TestGetEndpointWithClientCertSource(t *testing.T) { for _, tc := range testCases { t.Run(tc.want, func(t *testing.T) { got, err := getEndpoint(&Options{ - Endpoint: tc.endpoint, - DefaultEndpointTemplate: tc.defaultEndpointTemplate, - DefaultMTLSEndpoint: tc.defaultMTLSEndpoint, + Endpoint: tc.endpoint, + DefaultEndpointTemplate: tc.defaultEndpointTemplate, + DefaultMTLSEndpointTemplate: tc.defaultMTLSEndpoint, }, fakeClientCertSource) if tc.wantErr && err == nil { t.Fatalf("want err, got nil err") @@ -287,57 +289,57 @@ func TestGetGRPCTransportConfigAndEndpoint_S2A(t *testing.T) { { name: "has client cert", opts: &Options{ - DefaultMTLSEndpoint: testMTLSEndpoint, - DefaultEndpointTemplate: testEndpointTemplate, - ClientCertProvider: fakeClientCertSource, + DefaultEndpointTemplate: testEndpointTemplate, + DefaultMTLSEndpointTemplate: testMTLSEndpointTemplate, + ClientCertProvider: fakeClientCertSource, }, s2ARespFn: validConfigResp, - want: testMTLSEndpoint, + want: testDefaultUniverseMTLSEndpoint, }, { name: "no client cert, S2A address not empty", opts: &Options{ - DefaultMTLSEndpoint: testMTLSEndpoint, - DefaultEndpointTemplate: testEndpointTemplate, + DefaultEndpointTemplate: testEndpointTemplate, + DefaultMTLSEndpointTemplate: testMTLSEndpointTemplate, }, s2ARespFn: validConfigResp, - want: testMTLSEndpoint, + want: testDefaultUniverseMTLSEndpoint, }, { name: "no client cert, S2A address not empty, EnableDirectPath == true", opts: &Options{ - DefaultMTLSEndpoint: testMTLSEndpoint, - DefaultEndpointTemplate: testEndpointTemplate, - EnableDirectPath: true, + DefaultEndpointTemplate: testEndpointTemplate, + DefaultMTLSEndpointTemplate: testMTLSEndpointTemplate, + EnableDirectPath: true, }, s2ARespFn: validConfigResp, - want: testRegularEndpoint, + want: testDefaultUniverseEndpoint, }, { name: "no client cert, S2A address not empty, EnableDirectPathXds == true", opts: &Options{ - DefaultMTLSEndpoint: testMTLSEndpoint, - DefaultEndpointTemplate: testEndpointTemplate, - EnableDirectPathXds: true, + DefaultEndpointTemplate: testEndpointTemplate, + DefaultMTLSEndpointTemplate: testMTLSEndpointTemplate, + EnableDirectPathXds: true, }, s2ARespFn: validConfigResp, - want: testRegularEndpoint, + want: testDefaultUniverseEndpoint, }, { name: "no client cert, S2A address empty", opts: &Options{ - DefaultMTLSEndpoint: testMTLSEndpoint, - DefaultEndpointTemplate: testEndpointTemplate, + DefaultEndpointTemplate: testEndpointTemplate, + DefaultMTLSEndpointTemplate: testMTLSEndpointTemplate, }, s2ARespFn: invalidConfigResp, - want: testRegularEndpoint, + want: testDefaultUniverseEndpoint, }, { name: "no client cert, S2A address not empty, override endpoint", opts: &Options{ - DefaultMTLSEndpoint: testMTLSEndpoint, - DefaultEndpointTemplate: testEndpointTemplate, - Endpoint: testOverrideEndpoint, + DefaultEndpointTemplate: testEndpointTemplate, + DefaultMTLSEndpointTemplate: testMTLSEndpointTemplate, + Endpoint: testOverrideEndpoint, }, s2ARespFn: validConfigResp, want: testOverrideEndpoint, @@ -345,29 +347,29 @@ func TestGetGRPCTransportConfigAndEndpoint_S2A(t *testing.T) { { "no client cert, S2A address not empty, DefaultMTLSEndpoint not set", &Options{ - DefaultMTLSEndpoint: "", - DefaultEndpointTemplate: testEndpointTemplate, + DefaultEndpointTemplate: testEndpointTemplate, + DefaultMTLSEndpointTemplate: "", }, validConfigResp, - testRegularEndpoint, + testDefaultUniverseEndpoint, }, { "no client cert, MTLS S2A address not empty, no MTLS MDS cert", &Options{ - DefaultMTLSEndpoint: testMTLSEndpoint, - DefaultEndpointTemplate: testEndpointTemplate, + DefaultEndpointTemplate: testEndpointTemplate, + DefaultMTLSEndpointTemplate: testMTLSEndpointTemplate, }, validConfigRespMTLSS2A, - testRegularEndpoint, + testDefaultUniverseEndpoint, }, { "no client cert, dual S2A addresses, no MTLS MDS cert", &Options{ - DefaultMTLSEndpoint: testMTLSEndpoint, - DefaultEndpointTemplate: testEndpointTemplate, + DefaultEndpointTemplate: testEndpointTemplate, + DefaultMTLSEndpointTemplate: testMTLSEndpointTemplate, }, validConfigRespDualS2A, - testMTLSEndpoint, + testDefaultUniverseMTLSEndpoint, }, } defer setupTest(t)() @@ -399,39 +401,39 @@ func TestGetHTTPTransportConfig_S2A(t *testing.T) { { name: "has client cert", opts: &Options{ - DefaultMTLSEndpoint: testMTLSEndpoint, - DefaultEndpointTemplate: testEndpointTemplate, - ClientCertProvider: fakeClientCertSource, + DefaultEndpointTemplate: testEndpointTemplate, + DefaultMTLSEndpointTemplate: testMTLSEndpointTemplate, + ClientCertProvider: fakeClientCertSource, }, s2ARespFn: validConfigResp, - want: testMTLSEndpoint, + want: testMTLSEndpointTemplate, isDialFnNil: true, }, { name: "no client cert, S2A address not empty", opts: &Options{ - DefaultMTLSEndpoint: testMTLSEndpoint, - DefaultEndpointTemplate: testEndpointTemplate, + DefaultEndpointTemplate: testEndpointTemplate, + DefaultMTLSEndpointTemplate: testMTLSEndpointTemplate, }, s2ARespFn: validConfigResp, - want: testMTLSEndpoint, + want: testMTLSEndpointTemplate, }, { name: "no client cert, S2A address empty", opts: &Options{ - DefaultMTLSEndpoint: testMTLSEndpoint, - DefaultEndpointTemplate: testEndpointTemplate, + DefaultEndpointTemplate: testEndpointTemplate, + DefaultMTLSEndpointTemplate: testMTLSEndpointTemplate, }, s2ARespFn: invalidConfigResp, - want: testRegularEndpoint, + want: testDefaultUniverseEndpoint, isDialFnNil: true, }, { name: "no client cert, S2A address not empty, override endpoint", opts: &Options{ - DefaultMTLSEndpoint: testMTLSEndpoint, - DefaultEndpointTemplate: testEndpointTemplate, - Endpoint: testOverrideEndpoint, + DefaultEndpointTemplate: testEndpointTemplate, + DefaultMTLSEndpointTemplate: testMTLSEndpointTemplate, + Endpoint: testOverrideEndpoint, }, s2ARespFn: validConfigResp, want: testOverrideEndpoint, @@ -440,42 +442,42 @@ func TestGetHTTPTransportConfig_S2A(t *testing.T) { { name: "no client cert, S2A address not empty, but DefaultMTLSEndpoint is not set", opts: &Options{ - DefaultMTLSEndpoint: "", - DefaultEndpointTemplate: testEndpointTemplate, + DefaultEndpointTemplate: testEndpointTemplate, + DefaultMTLSEndpointTemplate: "", }, s2ARespFn: validConfigResp, - want: testRegularEndpoint, + want: testDefaultUniverseEndpoint, isDialFnNil: true, }, { name: "no client cert, S2A address not empty, custom HTTP client", opts: &Options{ - DefaultMTLSEndpoint: testMTLSEndpoint, - DefaultEndpointTemplate: testEndpointTemplate, - Client: http.DefaultClient, + DefaultEndpointTemplate: testEndpointTemplate, + DefaultMTLSEndpointTemplate: testMTLSEndpointTemplate, + Client: http.DefaultClient, }, s2ARespFn: validConfigResp, - want: testRegularEndpoint, + want: testDefaultUniverseEndpoint, isDialFnNil: true, }, { name: "no client cert, MTLS S2A address not empty, no MTLS MDS cert", opts: &Options{ - DefaultMTLSEndpoint: testMTLSEndpoint, - DefaultEndpointTemplate: testEndpointTemplate, + DefaultEndpointTemplate: testEndpointTemplate, + DefaultMTLSEndpointTemplate: testMTLSEndpointTemplate, }, s2ARespFn: validConfigRespMTLSS2A, - want: testRegularEndpoint, + want: testDefaultUniverseEndpoint, isDialFnNil: true, }, { name: "no client cert, dual S2A addresses, no MTLS MDS cert", opts: &Options{ - DefaultMTLSEndpoint: testMTLSEndpoint, - DefaultEndpointTemplate: testEndpointTemplate, + DefaultEndpointTemplate: testEndpointTemplate, + DefaultMTLSEndpointTemplate: testMTLSEndpointTemplate, }, s2ARespFn: validConfigRespDualS2A, - want: testMTLSEndpoint, + want: testMTLSEndpointTemplate, isDialFnNil: false, }, } @@ -556,52 +558,50 @@ func TestGetTransportConfig_UniverseDomain(t *testing.T) { name string opts *Options wantEndpoint string - wantErr error }{ { name: "google default universe (GDU), no client cert, template is regular endpoint", opts: &Options{ - DefaultEndpointTemplate: testRegularEndpoint, - DefaultMTLSEndpoint: testMTLSEndpoint, + DefaultEndpointTemplate: testDefaultUniverseEndpoint, + DefaultMTLSEndpointTemplate: testMTLSEndpointTemplate, }, - wantEndpoint: testRegularEndpoint, + wantEndpoint: testDefaultUniverseEndpoint, }, { name: "google default universe (GDU), no client cert", opts: &Options{ - DefaultEndpointTemplate: testEndpointTemplate, - DefaultMTLSEndpoint: testMTLSEndpoint, + DefaultEndpointTemplate: testEndpointTemplate, + DefaultMTLSEndpointTemplate: testMTLSEndpointTemplate, }, - wantEndpoint: testRegularEndpoint, + wantEndpoint: testDefaultUniverseEndpoint, }, { name: "google default universe (GDU), client cert", opts: &Options{ - DefaultEndpointTemplate: testEndpointTemplate, - DefaultMTLSEndpoint: testMTLSEndpoint, - ClientCertProvider: fakeClientCertSource, + DefaultEndpointTemplate: testEndpointTemplate, + DefaultMTLSEndpointTemplate: testMTLSEndpointTemplate, + ClientCertProvider: fakeClientCertSource, }, - wantEndpoint: testMTLSEndpoint, + wantEndpoint: testDefaultUniverseMTLSEndpoint, }, { name: "UniverseDomain, no client cert", opts: &Options{ - DefaultEndpointTemplate: testEndpointTemplate, - DefaultMTLSEndpoint: testMTLSEndpoint, - UniverseDomain: testUniverseDomain, + DefaultEndpointTemplate: testEndpointTemplate, + DefaultMTLSEndpointTemplate: testMTLSEndpointTemplate, + UniverseDomain: testUniverseDomain, }, wantEndpoint: testUniverseDomainEndpoint, }, { name: "UniverseDomain, client cert", opts: &Options{ - DefaultEndpointTemplate: testEndpointTemplate, - DefaultMTLSEndpoint: testMTLSEndpoint, - UniverseDomain: testUniverseDomain, - ClientCertProvider: fakeClientCertSource, + DefaultEndpointTemplate: testEndpointTemplate, + DefaultMTLSEndpointTemplate: testMTLSEndpointTemplate, + UniverseDomain: testUniverseDomain, + ClientCertProvider: fakeClientCertSource, }, - wantEndpoint: testUniverseDomainEndpoint, - wantErr: errUniverseNotSupportedMTLS, + wantEndpoint: testUniverseDomainMTLSEndpoint, }, } @@ -614,9 +614,7 @@ func TestGetTransportConfig_UniverseDomain(t *testing.T) { } config, err := getTransportConfig(tc.opts) if err != nil { - if err != tc.wantErr { - t.Fatalf("err: %v", err) - } + t.Fatalf("err: %v", err) } else { if tc.wantEndpoint != config.endpoint { t.Errorf("want endpoint: %s, got %s", tc.wantEndpoint, config.endpoint) @@ -631,81 +629,80 @@ func TestGetGRPCTransportCredsAndEndpoint_UniverseDomain(t *testing.T) { name string opts *Options wantEndpoint string - wantErr error }{ { name: "google default universe (GDU), no client cert", opts: &Options{ - DefaultEndpointTemplate: testEndpointTemplate, - DefaultMTLSEndpoint: testMTLSEndpoint, + DefaultEndpointTemplate: testEndpointTemplate, + DefaultMTLSEndpointTemplate: testMTLSEndpointTemplate, }, - wantEndpoint: testRegularEndpoint, + wantEndpoint: testDefaultUniverseEndpoint, }, { name: "google default universe (GDU), no client cert, endpoint", opts: &Options{ - DefaultEndpointTemplate: testEndpointTemplate, - DefaultMTLSEndpoint: testMTLSEndpoint, - Endpoint: testOverrideEndpoint, + DefaultEndpointTemplate: testEndpointTemplate, + DefaultMTLSEndpointTemplate: testMTLSEndpointTemplate, + Endpoint: testOverrideEndpoint, }, wantEndpoint: testOverrideEndpoint, }, { name: "google default universe (GDU), client cert", opts: &Options{ - DefaultEndpointTemplate: testEndpointTemplate, - DefaultMTLSEndpoint: testMTLSEndpoint, - ClientCertProvider: fakeClientCertSource, + DefaultEndpointTemplate: testEndpointTemplate, + DefaultMTLSEndpointTemplate: testMTLSEndpointTemplate, + ClientCertProvider: fakeClientCertSource, }, - wantEndpoint: testMTLSEndpoint, + wantEndpoint: testDefaultUniverseMTLSEndpoint, }, { name: "google default universe (GDU), client cert, endpoint", opts: &Options{ - DefaultEndpointTemplate: testEndpointTemplate, - DefaultMTLSEndpoint: testMTLSEndpoint, - ClientCertProvider: fakeClientCertSource, - Endpoint: testOverrideEndpoint, + DefaultEndpointTemplate: testEndpointTemplate, + DefaultMTLSEndpointTemplate: testMTLSEndpointTemplate, + ClientCertProvider: fakeClientCertSource, + Endpoint: testOverrideEndpoint, }, wantEndpoint: testOverrideEndpoint, }, { name: "UniverseDomain, no client cert", opts: &Options{ - DefaultEndpointTemplate: testEndpointTemplate, - DefaultMTLSEndpoint: testMTLSEndpoint, - UniverseDomain: testUniverseDomain, + DefaultEndpointTemplate: testEndpointTemplate, + DefaultMTLSEndpointTemplate: testMTLSEndpointTemplate, + UniverseDomain: testUniverseDomain, }, wantEndpoint: testUniverseDomainEndpoint, }, { name: "UniverseDomain, no client cert, endpoint", opts: &Options{ - DefaultEndpointTemplate: testEndpointTemplate, - DefaultMTLSEndpoint: testMTLSEndpoint, - UniverseDomain: testUniverseDomain, - Endpoint: testOverrideEndpoint, + DefaultEndpointTemplate: testEndpointTemplate, + DefaultMTLSEndpointTemplate: testMTLSEndpointTemplate, + UniverseDomain: testUniverseDomain, + Endpoint: testOverrideEndpoint, }, wantEndpoint: testOverrideEndpoint, }, { name: "UniverseDomain, client cert", opts: &Options{ - DefaultEndpointTemplate: testEndpointTemplate, - DefaultMTLSEndpoint: testMTLSEndpoint, - UniverseDomain: testUniverseDomain, - ClientCertProvider: fakeClientCertSource, + DefaultEndpointTemplate: testEndpointTemplate, + DefaultMTLSEndpointTemplate: testMTLSEndpointTemplate, + UniverseDomain: testUniverseDomain, + ClientCertProvider: fakeClientCertSource, }, - wantErr: errUniverseNotSupportedMTLS, + wantEndpoint: testUniverseDomainMTLSEndpoint, }, { name: "UniverseDomain, client cert, endpoint", opts: &Options{ - DefaultEndpointTemplate: testEndpointTemplate, - DefaultMTLSEndpoint: testMTLSEndpoint, - UniverseDomain: testUniverseDomain, - ClientCertProvider: fakeClientCertSource, - Endpoint: testOverrideEndpoint, + DefaultEndpointTemplate: testEndpointTemplate, + DefaultMTLSEndpointTemplate: testMTLSEndpointTemplate, + UniverseDomain: testUniverseDomain, + ClientCertProvider: fakeClientCertSource, + Endpoint: testOverrideEndpoint, }, wantEndpoint: testOverrideEndpoint, }, @@ -720,9 +717,7 @@ func TestGetGRPCTransportCredsAndEndpoint_UniverseDomain(t *testing.T) { } _, endpoint, err := GetGRPCTransportCredsAndEndpoint(tc.opts) if err != nil { - if err != tc.wantErr { - t.Fatalf("err: %v", err) - } + t.Fatalf("err: %v", err) } else { if tc.wantEndpoint != endpoint { t.Errorf("want endpoint: %s, got %s", tc.wantEndpoint, endpoint) @@ -745,7 +740,7 @@ func TestGetClientCertificateProvider(t *testing.T) { opts: &Options{ UniverseDomain: internal.DefaultUniverseDomain, ClientCertProvider: fakeClientCertSource, - Endpoint: testRegularEndpoint, + Endpoint: testDefaultUniverseEndpoint, }, useCertEnvVar: "false", wantCertProvider: nil, @@ -765,7 +760,7 @@ func TestGetClientCertificateProvider(t *testing.T) { opts: &Options{ UniverseDomain: internal.DefaultUniverseDomain, ClientCertProvider: fakeClientCertSource, - Endpoint: testRegularEndpoint, + Endpoint: testDefaultUniverseEndpoint, }, useCertEnvVar: "unset", wantCertProvider: fakeClientCertSource, diff --git a/auth/internal/transport/s2a.go b/auth/internal/transport/s2a.go index 37894bfcd013..2119a20b064e 100644 --- a/auth/internal/transport/s2a.go +++ b/auth/internal/transport/s2a.go @@ -114,7 +114,7 @@ func shouldUseS2A(clientCertSource cert.Provider, opts *Options) bool { return false } // If DefaultMTLSEndpoint is not set or has endpoint override, skip S2A. - if opts.DefaultMTLSEndpoint == "" || opts.Endpoint != "" { + if opts.DefaultMTLSEndpointTemplate == "" || opts.Endpoint != "" { return false } // If custom HTTP client is provided, skip S2A. From da1cd11aeeef1c6a76b513da4c3b9c9d6237bf05 Mon Sep 17 00:00:00 2001 From: Chris Smith Date: Wed, 20 Nov 2024 11:51:00 -0700 Subject: [PATCH 2/7] restore InternalOptions.DefaultMTLSEndpoint for external compatibility --- auth/grpctransport/grpctransport.go | 6 +++--- auth/httptransport/httptransport.go | 6 +++--- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/auth/grpctransport/grpctransport.go b/auth/grpctransport/grpctransport.go index dea74e82ae32..3ad4718ae22d 100644 --- a/auth/grpctransport/grpctransport.go +++ b/auth/grpctransport/grpctransport.go @@ -197,9 +197,9 @@ type InternalOptions struct { // DefaultEndpointTemplate combined with UniverseDomain specifies // the default endpoint. DefaultEndpointTemplate string - // DefaultMTLSEndpointTemplate combined with UniverseDomain specifies the + // DefaultMTLSEndpoint combined with UniverseDomain specifies the // default mTLS endpoint. - DefaultMTLSEndpointTemplate string + DefaultMTLSEndpoint string // DefaultScopes specifies the default OAuth2 scopes to be used for a // service. DefaultScopes []string @@ -245,7 +245,7 @@ func dial(ctx context.Context, secure bool, opts *Options) (*grpc.ClientConn, er } if io := opts.InternalOptions; io != nil { tOpts.DefaultEndpointTemplate = io.DefaultEndpointTemplate - tOpts.DefaultMTLSEndpointTemplate = io.DefaultMTLSEndpointTemplate + tOpts.DefaultMTLSEndpointTemplate = io.DefaultMTLSEndpoint tOpts.EnableDirectPath = io.EnableDirectPath tOpts.EnableDirectPathXds = io.EnableDirectPathXds } diff --git a/auth/httptransport/httptransport.go b/auth/httptransport/httptransport.go index 2434859fcfd6..bda81dbe0936 100644 --- a/auth/httptransport/httptransport.go +++ b/auth/httptransport/httptransport.go @@ -141,9 +141,9 @@ type InternalOptions struct { // DefaultEndpointTemplate combined with UniverseDomain specifies the // default endpoint. DefaultEndpointTemplate string - // DefaultMTLSEndpointTemplate combined with UniverseDomain specifies the + // DefaultMTLSEndpoint combined with UniverseDomain specifies the // default mTLS endpoint. - DefaultMTLSEndpointTemplate string + DefaultMTLSEndpoint string // DefaultScopes specifies the default OAuth2 scopes to be used for a // service. DefaultScopes []string @@ -201,7 +201,7 @@ func NewClient(opts *Options) (*http.Client, error) { } if io := opts.InternalOptions; io != nil { tOpts.DefaultEndpointTemplate = io.DefaultEndpointTemplate - tOpts.DefaultMTLSEndpointTemplate = io.DefaultMTLSEndpointTemplate + tOpts.DefaultMTLSEndpointTemplate = io.DefaultMTLSEndpoint } clientCertProvider, dialTLSContext, err := transport.GetHTTPTransportConfig(tOpts) if err != nil { From 43f76192773683c88f0f723a8dce5948b54931af Mon Sep 17 00:00:00 2001 From: Chris Smith Date: Thu, 21 Nov 2024 10:35:54 -0700 Subject: [PATCH 3/7] Revert "fix(auth): Enable client certificates by default only for GDU (#10151)" This reverts commit 7c529786275a39b7e00525f7d5e7be0d963e9e15. --- auth/internal/transport/cba.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/auth/internal/transport/cba.go b/auth/internal/transport/cba.go index c2d8e28fc21e..5a7ac403951c 100644 --- a/auth/internal/transport/cba.go +++ b/auth/internal/transport/cba.go @@ -284,7 +284,7 @@ func getTransportConfig(opts *Options) (*transportConfig, error) { // encountered while initializing the default source will be reported as client // error (ex. corrupt metadata file). func GetClientCertificateProvider(opts *Options) (cert.Provider, error) { - if !isClientCertificateEnabled(opts) { + if !isClientCertificateEnabled() { return nil, nil } else if opts.ClientCertProvider != nil { return opts.ClientCertProvider, nil @@ -293,14 +293,14 @@ func GetClientCertificateProvider(opts *Options) (cert.Provider, error) { } -// isClientCertificateEnabled returns true by default for all GDU universe domain, unless explicitly overridden by env var -func isClientCertificateEnabled(opts *Options) bool { +// isClientCertificateEnabled returns true by default, unless explicitly set to false via env var. +func isClientCertificateEnabled() bool { if value, ok := os.LookupEnv(googleAPIUseCertSource); ok { // error as false is OK b, _ := strconv.ParseBool(value) return b } - return opts.isUniverseDomainGDU() + return true } type transportConfig struct { From dfbdabd7ec98bd47909307948d3b281b38901298 Mon Sep 17 00:00:00 2001 From: Chris Smith Date: Thu, 21 Nov 2024 10:42:31 -0700 Subject: [PATCH 4/7] revert internal/transport.Options.DefaultMTLSEndpointTemplate to DefaultMTLSEndpoint --- auth/grpctransport/grpctransport.go | 2 +- auth/httptransport/httptransport.go | 2 +- auth/internal/transport/cba.go | 20 ++-- auth/internal/transport/cba_test.go | 172 ++++++++++++++-------------- auth/internal/transport/s2a.go | 2 +- 5 files changed, 99 insertions(+), 99 deletions(-) diff --git a/auth/grpctransport/grpctransport.go b/auth/grpctransport/grpctransport.go index 3ad4718ae22d..a5dd320ac59b 100644 --- a/auth/grpctransport/grpctransport.go +++ b/auth/grpctransport/grpctransport.go @@ -245,7 +245,7 @@ func dial(ctx context.Context, secure bool, opts *Options) (*grpc.ClientConn, er } if io := opts.InternalOptions; io != nil { tOpts.DefaultEndpointTemplate = io.DefaultEndpointTemplate - tOpts.DefaultMTLSEndpointTemplate = io.DefaultMTLSEndpoint + tOpts.DefaultMTLSEndpoint = io.DefaultMTLSEndpoint tOpts.EnableDirectPath = io.EnableDirectPath tOpts.EnableDirectPathXds = io.EnableDirectPathXds } diff --git a/auth/httptransport/httptransport.go b/auth/httptransport/httptransport.go index bda81dbe0936..12ca6103328b 100644 --- a/auth/httptransport/httptransport.go +++ b/auth/httptransport/httptransport.go @@ -201,7 +201,7 @@ func NewClient(opts *Options) (*http.Client, error) { } if io := opts.InternalOptions; io != nil { tOpts.DefaultEndpointTemplate = io.DefaultEndpointTemplate - tOpts.DefaultMTLSEndpointTemplate = io.DefaultMTLSEndpoint + tOpts.DefaultMTLSEndpoint = io.DefaultMTLSEndpoint } clientCertProvider, dialTLSContext, err := transport.GetHTTPTransportConfig(tOpts) if err != nil { diff --git a/auth/internal/transport/cba.go b/auth/internal/transport/cba.go index 5a7ac403951c..910394fef2cf 100644 --- a/auth/internal/transport/cba.go +++ b/auth/internal/transport/cba.go @@ -55,14 +55,14 @@ const ( // transport packages in order to avoid cyclic deps. It correlates 1:1 with // fields on httptransport.Options and grpctransport.Options. type Options struct { - Endpoint string - DefaultEndpointTemplate string - DefaultMTLSEndpointTemplate string - ClientCertProvider cert.Provider - Client *http.Client - UniverseDomain string - EnableDirectPath bool - EnableDirectPathXds bool + Endpoint string + DefaultEndpointTemplate string + DefaultMTLSEndpoint string + ClientCertProvider cert.Provider + Client *http.Client + UniverseDomain string + EnableDirectPath bool + EnableDirectPathXds bool } // getUniverseDomain returns the default service domain for a given Cloud @@ -94,10 +94,10 @@ func (o *Options) defaultEndpoint() string { // universe domain if the DefaultMTLSEndpointTemplate is set, otherwise returns an // empty string. func (o *Options) defaultMTLSEndpoint() string { - if o.DefaultMTLSEndpointTemplate == "" { + if o.DefaultMTLSEndpoint == "" { return "" } - return strings.Replace(o.DefaultMTLSEndpointTemplate, universeDomainPlaceholder, o.getUniverseDomain(), 1) + return strings.Replace(o.DefaultMTLSEndpoint, universeDomainPlaceholder, o.getUniverseDomain(), 1) } // mergedEndpoint merges a user-provided Endpoint of format host[:port] with the diff --git a/auth/internal/transport/cba_test.go b/auth/internal/transport/cba_test.go index e60d4e21e9fe..3cf1389c8f1b 100644 --- a/auth/internal/transport/cba_test.go +++ b/auth/internal/transport/cba_test.go @@ -262,9 +262,9 @@ func TestGetEndpointWithClientCertSource(t *testing.T) { for _, tc := range testCases { t.Run(tc.want, func(t *testing.T) { got, err := getEndpoint(&Options{ - Endpoint: tc.endpoint, - DefaultEndpointTemplate: tc.defaultEndpointTemplate, - DefaultMTLSEndpointTemplate: tc.defaultMTLSEndpoint, + Endpoint: tc.endpoint, + DefaultEndpointTemplate: tc.defaultEndpointTemplate, + DefaultMTLSEndpoint: tc.defaultMTLSEndpoint, }, fakeClientCertSource) if tc.wantErr && err == nil { t.Fatalf("want err, got nil err") @@ -289,9 +289,9 @@ func TestGetGRPCTransportConfigAndEndpoint_S2A(t *testing.T) { { name: "has client cert", opts: &Options{ - DefaultEndpointTemplate: testEndpointTemplate, - DefaultMTLSEndpointTemplate: testMTLSEndpointTemplate, - ClientCertProvider: fakeClientCertSource, + DefaultEndpointTemplate: testEndpointTemplate, + DefaultMTLSEndpoint: testMTLSEndpointTemplate, + ClientCertProvider: fakeClientCertSource, }, s2ARespFn: validConfigResp, want: testDefaultUniverseMTLSEndpoint, @@ -299,8 +299,8 @@ func TestGetGRPCTransportConfigAndEndpoint_S2A(t *testing.T) { { name: "no client cert, S2A address not empty", opts: &Options{ - DefaultEndpointTemplate: testEndpointTemplate, - DefaultMTLSEndpointTemplate: testMTLSEndpointTemplate, + DefaultEndpointTemplate: testEndpointTemplate, + DefaultMTLSEndpoint: testMTLSEndpointTemplate, }, s2ARespFn: validConfigResp, want: testDefaultUniverseMTLSEndpoint, @@ -308,9 +308,9 @@ func TestGetGRPCTransportConfigAndEndpoint_S2A(t *testing.T) { { name: "no client cert, S2A address not empty, EnableDirectPath == true", opts: &Options{ - DefaultEndpointTemplate: testEndpointTemplate, - DefaultMTLSEndpointTemplate: testMTLSEndpointTemplate, - EnableDirectPath: true, + DefaultEndpointTemplate: testEndpointTemplate, + DefaultMTLSEndpoint: testMTLSEndpointTemplate, + EnableDirectPath: true, }, s2ARespFn: validConfigResp, want: testDefaultUniverseEndpoint, @@ -318,9 +318,9 @@ func TestGetGRPCTransportConfigAndEndpoint_S2A(t *testing.T) { { name: "no client cert, S2A address not empty, EnableDirectPathXds == true", opts: &Options{ - DefaultEndpointTemplate: testEndpointTemplate, - DefaultMTLSEndpointTemplate: testMTLSEndpointTemplate, - EnableDirectPathXds: true, + DefaultEndpointTemplate: testEndpointTemplate, + DefaultMTLSEndpoint: testMTLSEndpointTemplate, + EnableDirectPathXds: true, }, s2ARespFn: validConfigResp, want: testDefaultUniverseEndpoint, @@ -328,8 +328,8 @@ func TestGetGRPCTransportConfigAndEndpoint_S2A(t *testing.T) { { name: "no client cert, S2A address empty", opts: &Options{ - DefaultEndpointTemplate: testEndpointTemplate, - DefaultMTLSEndpointTemplate: testMTLSEndpointTemplate, + DefaultEndpointTemplate: testEndpointTemplate, + DefaultMTLSEndpoint: testMTLSEndpointTemplate, }, s2ARespFn: invalidConfigResp, want: testDefaultUniverseEndpoint, @@ -337,9 +337,9 @@ func TestGetGRPCTransportConfigAndEndpoint_S2A(t *testing.T) { { name: "no client cert, S2A address not empty, override endpoint", opts: &Options{ - DefaultEndpointTemplate: testEndpointTemplate, - DefaultMTLSEndpointTemplate: testMTLSEndpointTemplate, - Endpoint: testOverrideEndpoint, + DefaultEndpointTemplate: testEndpointTemplate, + DefaultMTLSEndpoint: testMTLSEndpointTemplate, + Endpoint: testOverrideEndpoint, }, s2ARespFn: validConfigResp, want: testOverrideEndpoint, @@ -347,8 +347,8 @@ func TestGetGRPCTransportConfigAndEndpoint_S2A(t *testing.T) { { "no client cert, S2A address not empty, DefaultMTLSEndpoint not set", &Options{ - DefaultEndpointTemplate: testEndpointTemplate, - DefaultMTLSEndpointTemplate: "", + DefaultEndpointTemplate: testEndpointTemplate, + DefaultMTLSEndpoint: "", }, validConfigResp, testDefaultUniverseEndpoint, @@ -356,8 +356,8 @@ func TestGetGRPCTransportConfigAndEndpoint_S2A(t *testing.T) { { "no client cert, MTLS S2A address not empty, no MTLS MDS cert", &Options{ - DefaultEndpointTemplate: testEndpointTemplate, - DefaultMTLSEndpointTemplate: testMTLSEndpointTemplate, + DefaultEndpointTemplate: testEndpointTemplate, + DefaultMTLSEndpoint: testMTLSEndpointTemplate, }, validConfigRespMTLSS2A, testDefaultUniverseEndpoint, @@ -365,8 +365,8 @@ func TestGetGRPCTransportConfigAndEndpoint_S2A(t *testing.T) { { "no client cert, dual S2A addresses, no MTLS MDS cert", &Options{ - DefaultEndpointTemplate: testEndpointTemplate, - DefaultMTLSEndpointTemplate: testMTLSEndpointTemplate, + DefaultEndpointTemplate: testEndpointTemplate, + DefaultMTLSEndpoint: testMTLSEndpointTemplate, }, validConfigRespDualS2A, testDefaultUniverseMTLSEndpoint, @@ -401,9 +401,9 @@ func TestGetHTTPTransportConfig_S2A(t *testing.T) { { name: "has client cert", opts: &Options{ - DefaultEndpointTemplate: testEndpointTemplate, - DefaultMTLSEndpointTemplate: testMTLSEndpointTemplate, - ClientCertProvider: fakeClientCertSource, + DefaultEndpointTemplate: testEndpointTemplate, + DefaultMTLSEndpoint: testMTLSEndpointTemplate, + ClientCertProvider: fakeClientCertSource, }, s2ARespFn: validConfigResp, want: testMTLSEndpointTemplate, @@ -412,8 +412,8 @@ func TestGetHTTPTransportConfig_S2A(t *testing.T) { { name: "no client cert, S2A address not empty", opts: &Options{ - DefaultEndpointTemplate: testEndpointTemplate, - DefaultMTLSEndpointTemplate: testMTLSEndpointTemplate, + DefaultEndpointTemplate: testEndpointTemplate, + DefaultMTLSEndpoint: testMTLSEndpointTemplate, }, s2ARespFn: validConfigResp, want: testMTLSEndpointTemplate, @@ -421,8 +421,8 @@ func TestGetHTTPTransportConfig_S2A(t *testing.T) { { name: "no client cert, S2A address empty", opts: &Options{ - DefaultEndpointTemplate: testEndpointTemplate, - DefaultMTLSEndpointTemplate: testMTLSEndpointTemplate, + DefaultEndpointTemplate: testEndpointTemplate, + DefaultMTLSEndpoint: testMTLSEndpointTemplate, }, s2ARespFn: invalidConfigResp, want: testDefaultUniverseEndpoint, @@ -431,9 +431,9 @@ func TestGetHTTPTransportConfig_S2A(t *testing.T) { { name: "no client cert, S2A address not empty, override endpoint", opts: &Options{ - DefaultEndpointTemplate: testEndpointTemplate, - DefaultMTLSEndpointTemplate: testMTLSEndpointTemplate, - Endpoint: testOverrideEndpoint, + DefaultEndpointTemplate: testEndpointTemplate, + DefaultMTLSEndpoint: testMTLSEndpointTemplate, + Endpoint: testOverrideEndpoint, }, s2ARespFn: validConfigResp, want: testOverrideEndpoint, @@ -442,8 +442,8 @@ func TestGetHTTPTransportConfig_S2A(t *testing.T) { { name: "no client cert, S2A address not empty, but DefaultMTLSEndpoint is not set", opts: &Options{ - DefaultEndpointTemplate: testEndpointTemplate, - DefaultMTLSEndpointTemplate: "", + DefaultEndpointTemplate: testEndpointTemplate, + DefaultMTLSEndpoint: "", }, s2ARespFn: validConfigResp, want: testDefaultUniverseEndpoint, @@ -452,9 +452,9 @@ func TestGetHTTPTransportConfig_S2A(t *testing.T) { { name: "no client cert, S2A address not empty, custom HTTP client", opts: &Options{ - DefaultEndpointTemplate: testEndpointTemplate, - DefaultMTLSEndpointTemplate: testMTLSEndpointTemplate, - Client: http.DefaultClient, + DefaultEndpointTemplate: testEndpointTemplate, + DefaultMTLSEndpoint: testMTLSEndpointTemplate, + Client: http.DefaultClient, }, s2ARespFn: validConfigResp, want: testDefaultUniverseEndpoint, @@ -463,8 +463,8 @@ func TestGetHTTPTransportConfig_S2A(t *testing.T) { { name: "no client cert, MTLS S2A address not empty, no MTLS MDS cert", opts: &Options{ - DefaultEndpointTemplate: testEndpointTemplate, - DefaultMTLSEndpointTemplate: testMTLSEndpointTemplate, + DefaultEndpointTemplate: testEndpointTemplate, + DefaultMTLSEndpoint: testMTLSEndpointTemplate, }, s2ARespFn: validConfigRespMTLSS2A, want: testDefaultUniverseEndpoint, @@ -473,8 +473,8 @@ func TestGetHTTPTransportConfig_S2A(t *testing.T) { { name: "no client cert, dual S2A addresses, no MTLS MDS cert", opts: &Options{ - DefaultEndpointTemplate: testEndpointTemplate, - DefaultMTLSEndpointTemplate: testMTLSEndpointTemplate, + DefaultEndpointTemplate: testEndpointTemplate, + DefaultMTLSEndpoint: testMTLSEndpointTemplate, }, s2ARespFn: validConfigRespDualS2A, want: testMTLSEndpointTemplate, @@ -562,44 +562,44 @@ func TestGetTransportConfig_UniverseDomain(t *testing.T) { { name: "google default universe (GDU), no client cert, template is regular endpoint", opts: &Options{ - DefaultEndpointTemplate: testDefaultUniverseEndpoint, - DefaultMTLSEndpointTemplate: testMTLSEndpointTemplate, + DefaultEndpointTemplate: testDefaultUniverseEndpoint, + DefaultMTLSEndpoint: testMTLSEndpointTemplate, }, wantEndpoint: testDefaultUniverseEndpoint, }, { name: "google default universe (GDU), no client cert", opts: &Options{ - DefaultEndpointTemplate: testEndpointTemplate, - DefaultMTLSEndpointTemplate: testMTLSEndpointTemplate, + DefaultEndpointTemplate: testEndpointTemplate, + DefaultMTLSEndpoint: testMTLSEndpointTemplate, }, wantEndpoint: testDefaultUniverseEndpoint, }, { name: "google default universe (GDU), client cert", opts: &Options{ - DefaultEndpointTemplate: testEndpointTemplate, - DefaultMTLSEndpointTemplate: testMTLSEndpointTemplate, - ClientCertProvider: fakeClientCertSource, + DefaultEndpointTemplate: testEndpointTemplate, + DefaultMTLSEndpoint: testMTLSEndpointTemplate, + ClientCertProvider: fakeClientCertSource, }, wantEndpoint: testDefaultUniverseMTLSEndpoint, }, { name: "UniverseDomain, no client cert", opts: &Options{ - DefaultEndpointTemplate: testEndpointTemplate, - DefaultMTLSEndpointTemplate: testMTLSEndpointTemplate, - UniverseDomain: testUniverseDomain, + DefaultEndpointTemplate: testEndpointTemplate, + DefaultMTLSEndpoint: testMTLSEndpointTemplate, + UniverseDomain: testUniverseDomain, }, wantEndpoint: testUniverseDomainEndpoint, }, { name: "UniverseDomain, client cert", opts: &Options{ - DefaultEndpointTemplate: testEndpointTemplate, - DefaultMTLSEndpointTemplate: testMTLSEndpointTemplate, - UniverseDomain: testUniverseDomain, - ClientCertProvider: fakeClientCertSource, + DefaultEndpointTemplate: testEndpointTemplate, + DefaultMTLSEndpoint: testMTLSEndpointTemplate, + UniverseDomain: testUniverseDomain, + ClientCertProvider: fakeClientCertSource, }, wantEndpoint: testUniverseDomainMTLSEndpoint, }, @@ -633,76 +633,76 @@ func TestGetGRPCTransportCredsAndEndpoint_UniverseDomain(t *testing.T) { { name: "google default universe (GDU), no client cert", opts: &Options{ - DefaultEndpointTemplate: testEndpointTemplate, - DefaultMTLSEndpointTemplate: testMTLSEndpointTemplate, + DefaultEndpointTemplate: testEndpointTemplate, + DefaultMTLSEndpoint: testMTLSEndpointTemplate, }, wantEndpoint: testDefaultUniverseEndpoint, }, { name: "google default universe (GDU), no client cert, endpoint", opts: &Options{ - DefaultEndpointTemplate: testEndpointTemplate, - DefaultMTLSEndpointTemplate: testMTLSEndpointTemplate, - Endpoint: testOverrideEndpoint, + DefaultEndpointTemplate: testEndpointTemplate, + DefaultMTLSEndpoint: testMTLSEndpointTemplate, + Endpoint: testOverrideEndpoint, }, wantEndpoint: testOverrideEndpoint, }, { name: "google default universe (GDU), client cert", opts: &Options{ - DefaultEndpointTemplate: testEndpointTemplate, - DefaultMTLSEndpointTemplate: testMTLSEndpointTemplate, - ClientCertProvider: fakeClientCertSource, + DefaultEndpointTemplate: testEndpointTemplate, + DefaultMTLSEndpoint: testMTLSEndpointTemplate, + ClientCertProvider: fakeClientCertSource, }, wantEndpoint: testDefaultUniverseMTLSEndpoint, }, { name: "google default universe (GDU), client cert, endpoint", opts: &Options{ - DefaultEndpointTemplate: testEndpointTemplate, - DefaultMTLSEndpointTemplate: testMTLSEndpointTemplate, - ClientCertProvider: fakeClientCertSource, - Endpoint: testOverrideEndpoint, + DefaultEndpointTemplate: testEndpointTemplate, + DefaultMTLSEndpoint: testMTLSEndpointTemplate, + ClientCertProvider: fakeClientCertSource, + Endpoint: testOverrideEndpoint, }, wantEndpoint: testOverrideEndpoint, }, { name: "UniverseDomain, no client cert", opts: &Options{ - DefaultEndpointTemplate: testEndpointTemplate, - DefaultMTLSEndpointTemplate: testMTLSEndpointTemplate, - UniverseDomain: testUniverseDomain, + DefaultEndpointTemplate: testEndpointTemplate, + DefaultMTLSEndpoint: testMTLSEndpointTemplate, + UniverseDomain: testUniverseDomain, }, wantEndpoint: testUniverseDomainEndpoint, }, { name: "UniverseDomain, no client cert, endpoint", opts: &Options{ - DefaultEndpointTemplate: testEndpointTemplate, - DefaultMTLSEndpointTemplate: testMTLSEndpointTemplate, - UniverseDomain: testUniverseDomain, - Endpoint: testOverrideEndpoint, + DefaultEndpointTemplate: testEndpointTemplate, + DefaultMTLSEndpoint: testMTLSEndpointTemplate, + UniverseDomain: testUniverseDomain, + Endpoint: testOverrideEndpoint, }, wantEndpoint: testOverrideEndpoint, }, { name: "UniverseDomain, client cert", opts: &Options{ - DefaultEndpointTemplate: testEndpointTemplate, - DefaultMTLSEndpointTemplate: testMTLSEndpointTemplate, - UniverseDomain: testUniverseDomain, - ClientCertProvider: fakeClientCertSource, + DefaultEndpointTemplate: testEndpointTemplate, + DefaultMTLSEndpoint: testMTLSEndpointTemplate, + UniverseDomain: testUniverseDomain, + ClientCertProvider: fakeClientCertSource, }, wantEndpoint: testUniverseDomainMTLSEndpoint, }, { name: "UniverseDomain, client cert, endpoint", opts: &Options{ - DefaultEndpointTemplate: testEndpointTemplate, - DefaultMTLSEndpointTemplate: testMTLSEndpointTemplate, - UniverseDomain: testUniverseDomain, - ClientCertProvider: fakeClientCertSource, - Endpoint: testOverrideEndpoint, + DefaultEndpointTemplate: testEndpointTemplate, + DefaultMTLSEndpoint: testMTLSEndpointTemplate, + UniverseDomain: testUniverseDomain, + ClientCertProvider: fakeClientCertSource, + Endpoint: testOverrideEndpoint, }, wantEndpoint: testOverrideEndpoint, }, diff --git a/auth/internal/transport/s2a.go b/auth/internal/transport/s2a.go index 2119a20b064e..37894bfcd013 100644 --- a/auth/internal/transport/s2a.go +++ b/auth/internal/transport/s2a.go @@ -114,7 +114,7 @@ func shouldUseS2A(clientCertSource cert.Provider, opts *Options) bool { return false } // If DefaultMTLSEndpoint is not set or has endpoint override, skip S2A. - if opts.DefaultMTLSEndpointTemplate == "" || opts.Endpoint != "" { + if opts.DefaultMTLSEndpoint == "" || opts.Endpoint != "" { return false } // If custom HTTP client is provided, skip S2A. From 3c37abc92ef404fc893a660ff8451edd4b73d3e7 Mon Sep 17 00:00:00 2001 From: Chris Smith Date: Thu, 21 Nov 2024 13:17:00 -0700 Subject: [PATCH 5/7] fix TestGetClientCertificateProvider --- auth/internal/transport/cba_test.go | 49 ++++++++++++++++++++++++++++- 1 file changed, 48 insertions(+), 1 deletion(-) diff --git a/auth/internal/transport/cba_test.go b/auth/internal/transport/cba_test.go index 3cf1389c8f1b..c111bd071321 100644 --- a/auth/internal/transport/cba_test.go +++ b/auth/internal/transport/cba_test.go @@ -27,6 +27,7 @@ import ( const ( testEndpointTemplate = "https://test.UNIVERSE_DOMAIN/" + testMTLSEndpoint = "https://test.mtls.googleapis.com/" testMTLSEndpointTemplate = "https://test.mtls.UNIVERSE_DOMAIN/" testDefaultUniverseEndpoint = "https://test.googleapis.com/" testDefaultUniverseMTLSEndpoint = "https://test.mtls.googleapis.com/" @@ -288,6 +289,16 @@ func TestGetGRPCTransportConfigAndEndpoint_S2A(t *testing.T) { }{ { name: "has client cert", + opts: &Options{ + DefaultEndpointTemplate: testEndpointTemplate, + DefaultMTLSEndpoint: testMTLSEndpoint, + ClientCertProvider: fakeClientCertSource, + }, + s2ARespFn: validConfigResp, + want: testDefaultUniverseMTLSEndpoint, + }, + { + name: "has client cert, MTLSEndpointTemplate", opts: &Options{ DefaultEndpointTemplate: testEndpointTemplate, DefaultMTLSEndpoint: testMTLSEndpointTemplate, @@ -298,6 +309,15 @@ func TestGetGRPCTransportConfigAndEndpoint_S2A(t *testing.T) { }, { name: "no client cert, S2A address not empty", + opts: &Options{ + DefaultEndpointTemplate: testEndpointTemplate, + DefaultMTLSEndpoint: testMTLSEndpoint, + }, + s2ARespFn: validConfigResp, + want: testDefaultUniverseMTLSEndpoint, + }, + { + name: "no client cert, S2A address not empty, MTLSEndpointTemplate", opts: &Options{ DefaultEndpointTemplate: testEndpointTemplate, DefaultMTLSEndpoint: testMTLSEndpointTemplate, @@ -364,6 +384,15 @@ func TestGetGRPCTransportConfigAndEndpoint_S2A(t *testing.T) { }, { "no client cert, dual S2A addresses, no MTLS MDS cert", + &Options{ + DefaultEndpointTemplate: testEndpointTemplate, + DefaultMTLSEndpoint: testMTLSEndpoint, + }, + validConfigRespDualS2A, + testDefaultUniverseMTLSEndpoint, + }, + { + "no client cert, dual S2A addresses, no MTLS MDS cert, MTLSEndpointTemplate", &Options{ DefaultEndpointTemplate: testEndpointTemplate, DefaultMTLSEndpoint: testMTLSEndpointTemplate, @@ -577,6 +606,15 @@ func TestGetTransportConfig_UniverseDomain(t *testing.T) { }, { name: "google default universe (GDU), client cert", + opts: &Options{ + DefaultEndpointTemplate: testEndpointTemplate, + DefaultMTLSEndpoint: testMTLSEndpoint, + ClientCertProvider: fakeClientCertSource, + }, + wantEndpoint: testDefaultUniverseMTLSEndpoint, + }, + { + name: "google default universe (GDU), client cert, MTLSEndpointTemplate", opts: &Options{ DefaultEndpointTemplate: testEndpointTemplate, DefaultMTLSEndpoint: testMTLSEndpointTemplate, @@ -649,6 +687,15 @@ func TestGetGRPCTransportCredsAndEndpoint_UniverseDomain(t *testing.T) { }, { name: "google default universe (GDU), client cert", + opts: &Options{ + DefaultEndpointTemplate: testEndpointTemplate, + DefaultMTLSEndpoint: testMTLSEndpoint, + ClientCertProvider: fakeClientCertSource, + }, + wantEndpoint: testDefaultUniverseMTLSEndpoint, + }, + { + name: "google default universe (GDU), client cert, MTLSEndpointTemplate", opts: &Options{ DefaultEndpointTemplate: testEndpointTemplate, DefaultMTLSEndpoint: testMTLSEndpointTemplate, @@ -753,7 +800,7 @@ func TestGetClientCertificateProvider(t *testing.T) { Endpoint: testOverrideEndpoint, }, useCertEnvVar: "unset", - wantCertProvider: nil, + wantCertProvider: fakeClientCertSource, }, { name: "UseCertEnvVar unset, Domain is GDU", From 930b67b67d6fb1f85213f6598326ad29cba0b00b Mon Sep 17 00:00:00 2001 From: Chris Smith Date: Thu, 21 Nov 2024 13:30:27 -0700 Subject: [PATCH 6/7] restore Enable client certificates by default only for GDU (#10151) --- auth/internal/transport/cba.go | 8 ++++---- auth/internal/transport/cba_test.go | 2 +- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/auth/internal/transport/cba.go b/auth/internal/transport/cba.go index 910394fef2cf..c879611a8d7f 100644 --- a/auth/internal/transport/cba.go +++ b/auth/internal/transport/cba.go @@ -284,7 +284,7 @@ func getTransportConfig(opts *Options) (*transportConfig, error) { // encountered while initializing the default source will be reported as client // error (ex. corrupt metadata file). func GetClientCertificateProvider(opts *Options) (cert.Provider, error) { - if !isClientCertificateEnabled() { + if !isClientCertificateEnabled(opts) { return nil, nil } else if opts.ClientCertProvider != nil { return opts.ClientCertProvider, nil @@ -293,14 +293,14 @@ func GetClientCertificateProvider(opts *Options) (cert.Provider, error) { } -// isClientCertificateEnabled returns true by default, unless explicitly set to false via env var. -func isClientCertificateEnabled() bool { +// isClientCertificateEnabled returns true by default for all GDU universe domain, unless explicitly overridden by env var +func isClientCertificateEnabled(opts *Options) bool { if value, ok := os.LookupEnv(googleAPIUseCertSource); ok { // error as false is OK b, _ := strconv.ParseBool(value) return b } - return true + return opts.isUniverseDomainGDU() } type transportConfig struct { diff --git a/auth/internal/transport/cba_test.go b/auth/internal/transport/cba_test.go index c111bd071321..a28f57f639a7 100644 --- a/auth/internal/transport/cba_test.go +++ b/auth/internal/transport/cba_test.go @@ -800,7 +800,7 @@ func TestGetClientCertificateProvider(t *testing.T) { Endpoint: testOverrideEndpoint, }, useCertEnvVar: "unset", - wantCertProvider: fakeClientCertSource, + wantCertProvider: nil, }, { name: "UseCertEnvVar unset, Domain is GDU", From 762d55189a37123b060091a15545679c6026d070 Mon Sep 17 00:00:00 2001 From: Chris Smith Date: Thu, 21 Nov 2024 13:33:01 -0700 Subject: [PATCH 7/7] restore previous docs for DefaultMTLSEndpoint --- auth/grpctransport/grpctransport.go | 3 +-- auth/httptransport/httptransport.go | 3 +-- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/auth/grpctransport/grpctransport.go b/auth/grpctransport/grpctransport.go index a5dd320ac59b..38212ed0f82a 100644 --- a/auth/grpctransport/grpctransport.go +++ b/auth/grpctransport/grpctransport.go @@ -197,8 +197,7 @@ type InternalOptions struct { // DefaultEndpointTemplate combined with UniverseDomain specifies // the default endpoint. DefaultEndpointTemplate string - // DefaultMTLSEndpoint combined with UniverseDomain specifies the - // default mTLS endpoint. + // DefaultMTLSEndpoint specifies the default mTLS endpoint. DefaultMTLSEndpoint string // DefaultScopes specifies the default OAuth2 scopes to be used for a // service. diff --git a/auth/httptransport/httptransport.go b/auth/httptransport/httptransport.go index 12ca6103328b..cbe5a7a40a77 100644 --- a/auth/httptransport/httptransport.go +++ b/auth/httptransport/httptransport.go @@ -141,8 +141,7 @@ type InternalOptions struct { // DefaultEndpointTemplate combined with UniverseDomain specifies the // default endpoint. DefaultEndpointTemplate string - // DefaultMTLSEndpoint combined with UniverseDomain specifies the - // default mTLS endpoint. + // DefaultMTLSEndpoint specifies the default mTLS endpoint. DefaultMTLSEndpoint string // DefaultScopes specifies the default OAuth2 scopes to be used for a // service.