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

TLS connection error messages are no longer returned #1917

Closed
mastersingh24 opened this issue Mar 15, 2018 · 3 comments
Closed

TLS connection error messages are no longer returned #1917

mastersingh24 opened this issue Mar 15, 2018 · 3 comments

Comments

@mastersingh24
Copy link

mastersingh24 commented Mar 15, 2018

Please answer these questions before submitting your issue.

What version of gRPC are you using?

1.10.0

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

1.9.2

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

macOS

What did you do?

Upgraded from 1.5.2 to 1.10.0

What did you expect to see?

Minor API changes but expected TLS error messages to remain the same

What did you see instead?

failed TLS connections all return context deadline exceeded

Here's a snippet:

//invoke the EmptyCall RPC
func invokeEmptyCall(address string, dialOptions []grpc.DialOption) (*testpb.Empty, error) {

	//add DialOptions
	dialOptions = append(dialOptions, grpc.WithBlock())
	ctx := context.Background()
	ctx, cancel := context.WithTimeout(ctx, timeout)
	defer cancel()
	//create GRPC client conn
	clientConn, err := grpc.DialContext(ctx, address, dialOptions...)
	if err != nil {
		return nil, err
	}
	defer clientConn.Close()

	//create GRPC client
	client := testpb.NewTestServiceClient(clientConn)

	//invoke service
	empty, err := client.EmptyCall(ctx, new(testpb.Empty))
	if err != nil {
		return nil, err
	}

	return empty, nil
}

...........

        // ensure that TLS 1.2 in required / enforced
	tlsVersions := []string{"SSL30", "TLS10", "TLS11"}
	for counter, tlsVersion := range []uint16{tls.VersionSSL30, tls.VersionTLS10, tls.VersionTLS11} {
		tlsVersion := tlsVersion
		t.Run(tlsVersions[counter], func(t *testing.T) {
			t.Parallel()
			_, err = invokeEmptyCall(testAddress,
				[]grpc.DialOption{grpc.WithTransportCredentials(
					credentials.NewTLS(&tls.Config{
						RootCAs:    certPool,
						MinVersion: tlsVersion,
						MaxVersion: tlsVersion,
					}))})
			t.Logf("TLSVersion [%d] failed with [%s]", tlsVersion, err)
			assert.Error(t, err, "Should not have been able to connect with TLS version < 1.2")
			assert.Contains(t, err.Error(), "protocol version not supported")
		})
	}

The invokeEmpty call dials a server which has the min/max TLS version set to 1.2. In 1.5.2, the error returned a message containing protocol version not supported but with 1.10.0 it returns context deadline exceeded (which is also returned for all other TLS-related connection errors)

I realize I was several releases behind, but this does seem a little odd to me.
Maybe I'm not configuring things correctly?

@lyuxuan
Copy link
Contributor

lyuxuan commented Mar 15, 2018

We have made some major changes to the way we manage connections recently. One of them is: if you set the WithBlock() dial option, we will retry on handshake error until deadline is exceeded no matter whether it is a temporary error or not, and that's why you see dial returns context deadline exceeded. (#1856)

We have a workaround (#1855) which was unfortunately not making the cut to our 1.10.0 release. In short, with the workaround, you dial without the dial option WithBlock(), and when making the first fail-fast rpc on the ClientConn returned from the dial, you will most likely get the error containing info about failed handshake (e.g. protocol version not supported). Note that if fail-fast is not specified, then rpc will be blocked until deadline exceeded or until it gets connected to server.

@mastersingh24
Copy link
Author

Got it. Thanks for the quick response.
I also tried setting

grpc.WithDefaultCallOptions(grpc.FailFast(true)),
grpc.FailOnNonTempDialError(true),

but I guess WithBlock() negates those options

@dfawley
Copy link
Member

dfawley commented Mar 16, 2018

@mastersingh24,

but I guess WithBlock() negates those options

WithBlock() makes Dial() block until a connection is successfully established, and either returns nil or context.DeadlineExceeded on success/timeout, respectively. It is conceptually similar to making a non-FailFast (AKA wait-for-ready) RPC.

Note that FailFast is the default, and always should be, so you don't need to explicitly set it.

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

No branches or pull requests

3 participants