STS configuration does not allow TLSConfig

[salrashid123]

The experimental sts credential support calls an external HTTP server using default certificate store and TLSConfigurations

https://github.com/grpc/grpc-go/blob/master/credentials/sts/sts.go#L195-L204

This prevents using this credential type with an STS server that returns Certificate Bound Access Tokens which requires making the connection to the STS server using client certificates.

Use case(s) - what problem will this feature solve?

• Allow mTLS connections to STS server
• Secondarily, allow easier configuration/overrides of the CA trust store (eg, if the STS server is using self-signed CA)
• Full control of the grpc conn to the STS server

Proposed Solution

I don't know enough about the grpc-go design to make a suggesiton...maybe allow setting the TLSConfig directly with the credential? (..i think you'd need to since the TransportCredentials TLSConfig applies to gRPC and this is just plain http?)

	stscreds, err := sts.NewCredentials(sts.Options{
           TLSConfig:  some_tls_config_for_the_sts_server,
	})
	if err != nil {
		log.Fatalf("unable to create TokenSource: %v", err)
	}

	conn, err = grpc.Dial(*address, grpc.WithTransportCredentials(creds), grpc.WithPerRPCCredentials(stscreds))

Alternatives Considered

Not sure, maybe fiddling a lot with the low-level conn objects somehow to allow differential treatment for grpc and the STS call

Additional Context

this'll help integrate STS and CertBound tokens to help w/ securit...for ref, i have a small git repo demonstrating this at cert_bound_sts_server

[dfawley]

Ping @easwars

[easwars]

@salrashid123 Yes, you are correct that we cannot use TransportCredentials configured on the ClientConn for communication with the STS server since:

• those TransportCredentials are meant for the gRPC server endpoint
• http package requires a tls.Config to be specified and there is no way to retrieve the underlying tls.Config from a given TransportCredentials implementation

Adding a field to specify the tls.Config as part of Options passed to NewCredentials seems like a valid thing to do.

We currently always use the system default cert pool here. If we add an option to specify the tls.Config, we could use that when specified and continue using the system default certs if the option is unspecified.

Also, we currently pass the system default roots to makeHTTPDoer here. If we are to support mTLS to the STS server, we would also need to change this function to instead accept a fully blown tls.Config.

Would be interested in sending a PR for this?

Thanks

[salrashid123]

unfortunately, i don't really have the background to writeup the sufficient test cases to make this happen in a PR.

fwiw, what i can say is i go it to 'just work' by overriding the source here

• google.golang.org/[email protected]/credentials/sts/sts.go

func NewCredentials(opts Options) (credentials.PerRPCCredentials, error) {
	if err := validateOptions(opts); err != nil {
		return nil, err
	}
	return &callCreds{
		opts:   opts,
		client: makeHTTPDoer(&opts.TLSConfig),
	}, nil
}

func makeHTTPClient(cfg *tls.Config) httpDoer {
	return &http.Client{
		Timeout: stsRequestTimeout,
		Transport: &http.Transport{
			TLSClientConfig: cfg,
		},
	}
}

btw, i didn't understand why the current imlementation was loading up the defaultcerts here https://github.com/grpc/grpc-go/blob/master/credentials/sts/sts.go#L125

i'm pretty sure that happens automatically even if tlsconfig is nil (line 167 of transport.go)

[salrashid123]

@dfawley @easwars @menghanl

the more natural structure to use here is to pass the entire httpclient to the sts credential as an option. That'll allow you to override evey aspect of the underlying STS connection (timeout, certs, etc).

that change was submitted on https://github.com/grpc/grpc-go/pull/5611 ( one presubmit thest there is failing but doent' seem to have to do with th is PR)

the origninal test cases were really very abstract/meta so instead the PR uses an actual grpc and sts server over TLS...to me thats a higher fidelity test

[easwars]

The team is unable to prioritize this, and therefore I'm going ahead and closing this. If there is more demand for making this configurable, we could get to this in the future.