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

client does not recover after bad server certificate #1571

Closed
groob opened this issue Oct 11, 2017 · 3 comments
Closed

client does not recover after bad server certificate #1571

groob opened this issue Oct 11, 2017 · 3 comments
Assignees
Labels
P2 Status: Working As Intended Type: Behavior Change Behavior changes not categorized as bugs

Comments

@groob
Copy link
Contributor

groob commented Oct 11, 2017

Please answer these questions before submitting your issue.

What version of gRPC are you using?

  revision = "f92cdcd7dcdc69e81b2d7b338479a19a8723cfa3"
  version = "v1.6.0"

What version of Go are you using (go version)?

go version go1.9 darwin/amd64

What operating system (Linux, Windows, …) and version?

darwin and linux

What did you do?

If possible, provide a recipe for reproducing the error.

  1. Create a grpc client which uses TLS:
	creds := credentials.NewTLS(&tls.Config{
		ServerName: server.acme.co,
	})
       grpc.WithTransportCredentials(creds)
  1. have the server initially use the correct certificate, but then start issuing a bad one. In real life this corresponds to a wireless access point which MITM you for a short period. Finally serve the correct certificate again.
    The client fails to recover once the server connection is the correct one.

Logs:

2017/10/10 22:13:42 Greeting: Hello world
2017/10/10 22:13:43 Greeting: Hello world
2017/10/10 22:13:44 Greeting: Hello world 
// start serving bad cert

2017/10/10 22:13:45 could not greet: rpc error: code = Internal desc = connection error: desc = "transport: authentication handshake failed: x509: certificate is valid for not.acme.co:8443, not server.acme.co"
2017/10/10 22:13:46 could not greet: rpc error: code = Internal desc = connection error: desc = "transport: authentication handshake failed: x509: certificate is valid for not.acme.co:8443, not server.acme.co"

// start serving good cert again
2017/10/10 22:13:49 could not greet: rpc error: code = Internal desc = connection error: desc = "transport: authentication handshake failed: x509: certificate is valid for not.acme.co:8443, not server.acme.co"
2017/10/10 22:13:50 could not greet: rpc error: code = Internal desc = connection error: desc = "transport: authentication handshake failed: x509: certificate is valid for not.acme.co:8443, not server.acme.co"
// ... client never recovers

zip file attached with reproducible code. The certs/connection assume server.acme.co is localhost which you will need to set in /etc/hosts
grpc_repro.zip
playground main.go without certs/setup

@zwass
Copy link

zwass commented Oct 11, 2017

We found a workaround for this in our case: https://play.golang.org/p/VwcUcALPGN

The concept is to check for this specific error and explicitly mark it as temporary so that the gRPC dialer will retry dialing.

@menghanl
Copy link
Contributor

Credentials handshake errors are recognized as non-temporary errors by isTemporary function.
The purpose was to detect misconfigured certificates and return TLS errors (#768).

To make the client retry connections on creds errors, one way is to create a wrapper credentials, whose ClientHandshake always returns err implementing Temporary() bool { return true }.

A side note: with the new balancer APIs (#1388) we are working on, the feature to return creds errors from Dial introduced in (#768) will NOT continue to work.
So in a cleanup PR following the balancer PRs, we may make client always retry on creds errors.

@dfawley dfawley added P2 Type: Behavior Change Behavior changes not categorized as bugs labels Oct 12, 2017
@zwass
Copy link

zwass commented Oct 13, 2017

Sounds like our workaround is consistent with your suggestion. Thank you.

@dfawley dfawley closed this as completed Oct 26, 2017
@lock lock bot locked as resolved and limited conversation to collaborators Sep 26, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
P2 Status: Working As Intended Type: Behavior Change Behavior changes not categorized as bugs
Projects
None yet
Development

No branches or pull requests

4 participants