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.

Sign up for GitHub

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

Jump to bottom

crypto/tls: missing alpnprotocol field's value of the message which SSL/TLS protocol's handshake phase sends back. #45918

Open
lynnyuan-arch opened this issue May 3, 2021 · 1 comment
Open

crypto/tls: missing alpnprotocol field's value of the message which SSL/TLS protocol's handshake phase sends back. #45918

lynnyuan-arch opened this issue May 3, 2021 · 1 comment
Labels
NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Milestone
Unplanned

Comments

@lynnyuan-arch
Copy link

lynnyuan-arch commented May 3, 2021 •
edited by seankhliao

Both go1.12 and go.1.15 occur the same bug which described as follows.
I read the codes of the master branch, the problem is also existed.

When I use grpc-go build a http server which eanble ssl/tls protocol, the browser visit the grpc service normally.
But I use grpc-java as a client to visit the grpc service, the exception information:

io.grpc.StatusRuntimeException: UNAVAILABLE: Failed ALPN negotiation: Unable to find compatible protocol
Channel Pipeline: [SslHandler#0, ProtocolNegotiators$ClientTlsHandler#0, WriteBufferingAndExceptionHandler#0, DefaultChannelPipeline$TailContext#0]
	at io.grpc.stub.ClientCalls.toStatusRuntimeException(ClientCalls.java:262)
	at io.grpc.stub.ClientCalls.getUnchecked(ClientCalls.java:243)
	at io.grpc.stub.ClientCalls.blockingUnaryCall(ClientCalls.java:156)
	at bini.server.serverpb.AdminGrpc$AdminBlockingStub.databases(AdminGrpc.java:1667)
	at com.inspur.znbase.rpc.AdminClient.main(AdminClient.java:72)

The reason is: the SSL/TLS of grpc-java will checked the alpnprotocol after the handshake success. But the message which sends back by grpc-go missing the alpnprotocol.

check codes of class ClientTlsHandler of io.grpc.netty.ProtocolNegotitators.java file:

        @Override
        protected void userEventTriggered0(ChannelHandlerContext ctx, Object evt) throws Exception {
            if (evt instanceof SslHandshakeCompletionEvent) {
                SslHandshakeCompletionEvent handshakeEvent = (SslHandshakeCompletionEvent) evt;
                if (handshakeEvent.isSuccess()) {
                    SslHandler handler = ctx.pipeline().get(SslHandler.class);

                  // The check code

                    if (sslContext.applicationProtocolNegotiator().protocols()
                            .contains(handler.applicationProtocol())) {
                        // Successfully negotiated the protocol.
                        logSslEngineDetails(Level.FINER, ctx, "TLS negotiation succeeded.", null);
                        propagateTlsComplete(ctx, handler.engine().getSession());
                    } else {
                        Exception ex =
                                unavailableException("Failed ALPN negotiation: Unable to find compatible protocol");
                        logSslEngineDetails(Level.FINE, ctx, "TLS negotiation failed.", ex);
                        ctx.fireExceptionCaught(ex);
                    }
                } else {
                    ctx.fireExceptionCaught(handshakeEvent.cause());
                }
            } else {
                super.userEventTriggered0(ctx, evt);
            }
        }

The codes of the method "doFullHandshake" of go sdk's source file: "crypto/tls/handshake_server.go":

    hs.hello.ticketSupported = hs.clientHello.ticketSupported && !c.config.SessionTicketsDisabled
hs.hello.cipherSuite = hs.suite.id

hs.finishedHash = newFinishedHash(hs.c.vers, hs.suite)
if c.config.ClientAuth == NoClientCert {
	// No need to keep a full record of the handshake if client
	// certificates won't be used.
	hs.finishedHash.discardHandshakeBuffer()
}

so add the codes to the method "doFullHandshake" of go sdk's source file: "crypto/tls/handshake_server.go:

	hs.hello.ticketSupported = hs.clientHello.ticketSupported && !c.config.SessionTicketsDisabled
	hs.hello.cipherSuite = hs.suite.id

       // fix grpc-java openssl check
	if len(hs.clientHello.alpnProtocols) > 0 {
		hs.hello.alpnProtocol = hs.clientHello.alpnProtocols[0]
	}

	hs.finishedHash = newFinishedHash(hs.c.vers, hs.suite)
	if c.config.ClientAuth == NoClientCert {
		// No need to keep a full record of the handshake if client
		// certificates won't be used.
		hs.finishedHash.discardHandshakeBuffer()
	}
@seankhliao seankhliao changed the title Missing alpnprotocol field's value of the message which SSL/TLS protocol's handshake phase sends back. crypto/tls: missing alpnprotocol field's value of the message which SSL/TLS protocol's handshake phase sends back. May 3, 2021
@seankhliao seankhliao added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label May 3, 2021
@networkimprov
Copy link

@seankhliao was there a reason not to cc this?

cc @FiloSottile @katiehockman

@seankhliao seankhliao added this to the Unplanned milestone Aug 20, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Projects
None yet
Milestone
Unplanned
Development

No branches or pull requests
3 participants
@networkimprov @seankhliao @lynnyuan-arch