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

fix(transport/grpc): separate options for creds and certs #1759

Merged
merged 2 commits into from Nov 22, 2022

Conversation

noahdietz
Copy link
Contributor

@noahdietz noahdietz commented Nov 21, 2022

For gRPC, this separates the logic for credential dial options (associated with the WithoutAuthentication option) from the connection-level encryption dial options (associated with the WithInsecure option OR using DialInsecure). This allows a secure connection to be dialed without credentials.

Previously, using WithoutAuthentication alone (i.e. without WithInsecure or grpc.WithTransportCredentials(credentials.NewTLS(nil)), and while using Dial) would result in attempting to dial a secure connection but without any credentials or TLS config, resulting in an error. For example, the following code returns an error:

c, err := grpc.Dial(context.Background(),
  option.WithEndpoint("storage.googleapis.com:443"),
  option.WithoutAuthentication())
if err != nil {
  // grpc: no transport security set (use grpc.WithTransportCredentials(insecure.NewCredentials())
  // explicitly or set credentials)
}

This is not correct. It makes it very difficult for users to, for example, read a public GCS object without credentials, but using a secure connection.

This may seem like a weird use case, but the HTTP version of dial doesn't control connection-level encryption via the WithoutAuthentication option - connection certs loaded here while credentials are loaded here.

Note: This also refactors the use of grpc.WithInsecure, which is no deprecated, to the preferred grpcinsecure.NewCredentials() option.

transport/grpc/dial.go Outdated Show resolved Hide resolved
@noahdietz
Copy link
Contributor Author

Update to description: The internal param of dial is determined by use of either Dial or DialInsecure. That said, Dial + WithoutAuthentication but still "secure" is a valid use case and this change makes that possible. users should not need to provide the extra transport level security options to dial a connection without creds.

@noahdietz noahdietz marked this pull request as ready for review November 22, 2022 17:59
@noahdietz
Copy link
Contributor Author

The friction here around WithoutAuthentication but still dialing a secure connection is also mentioned in googleapis/google-cloud-go#4490. This will not fix that feature request, but it does reduce the friction of needing to add extra options to use WithoutAuthentication.

@noahdietz noahdietz merged commit c213153 into main Nov 22, 2022
@noahdietz noahdietz deleted the fix-grpc-without-auth branch November 22, 2022 18:11
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

Successfully merging this pull request may close these issues.

None yet

2 participants