Skip to content
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

GRPC server: baseTLSConfig should include "h2" in NextProtos to support ALPN #3860

Open
3 tasks done
nr-tdeluna opened this issue Nov 22, 2024 · 0 comments
Open
3 tasks done

Comments

@nr-tdeluna
Copy link

nr-tdeluna commented Nov 22, 2024

Preflight Checklist

  • I agree to follow the Code of Conduct that this project adheres to.
  • I have searched the issue tracker for an issue that matches the one I want to file, without success.
  • I am not looking for support or already pursued the available support channels without success.

Version

2.41.1

Storage Type

Postgres

Installation Type

Official container image, Custom container image

Expected Behavior

go clients using grpc-go versions 1.67.0 and above should be able to connect to the Dex gRPC service via mTLS without having to set the environment variable GRPC_ENFORCE_ALPN_ENABLED=false .

Actual Behavior

When trying to connect the client fails with the log message:

transport: authentication handshake failed: credentials: cannot check peer: missing selected ALPN property

Steps To Reproduce

  1. Create mTLS certificates using examples/grpc-client/cert-gen
  2. Start the server with mTLS enabled as described in examples/grpc-client/README.md
  3. Create a new go module, copy the dex go client example in examples/grpc-client/client.go
  4. Ensure go-grpc dependency 1.67.0 or later with go get google.golang.org/[email protected]
  5. Try to connect and see the client error logs.

Additional Information

Summary

The dex gRPC server is not doing proper protocol negotiation when served through TLS. It's not advertising http2 during protocol negotiation.

The default value of the environment variable GRPC_ENFORCE_ALPN_ENABLED changed from false to true on version 1.67.0. This causes clients to fail when the server does not advertise http2.

grpc/grpc-go#7535

Details

The baseTLSConfig created for the gRPC service should include NextProtos: []string{"h2"},

dex/cmd/dex/serve.go

Lines 176 to 181 in fa0240d

baseTLSConfig := &tls.Config{
MinVersion: uint16(tlsMinVersion),
MaxVersion: uint16(tlsMaxVersion),
CipherSuites: allowedTLSCiphers,
PreferServerCipherSuites: true,
}

In most cases "h2" is added by go-grpc's credentials.NewTLS
https://github.com/grpc/grpc-go/blob/2da976983bbb33feb3e25b7daaa8f60b9769adb5/credentials/tls.go#L201-L204

But dex's fsnotify reloader returns a certificate based on the original baseTLSConfig. Any modifications by go-grpc are discarded.

dex/cmd/dex/serve.go

Lines 657 to 660 in fa0240d

} else {
// net/http only uses Certificates or GetCertificate
initialConfig.GetCertificate = func(chi *tls.ClientHelloInfo) (*tls.Certificate, error) { return &ptr.Load().Certificates[0], nil }
}

Configuration

No response

Logs

No response

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant