TLS does not enforce ALPN protocol

[carl-mastrangelo]

According to https://tools.ietf.org/html/rfc7540#section-3.3 connections over TLS use the "h2" application protocol identifier. Attempting to use another protocol identifier, such as "h2c", should fail the connection. Currently, the Grpc go server accepts using this invalid identifier when establishing a TLS connection.

Here is the test that fails:

https://github.com/grpc/grpc/blob/master/tools/http2_interop/http2interop.go#L235

[jroper]

Crazy that this issue hasn't been fixed 8 years on. Here's a work around that can be used:

import (
	"context"
	"crypto/tls"
	"fmt"
	"google.golang.org/grpc/credentials"
	"net"
)

func NewAlpnEnforcingTLSCredentials(config *tls.Config) credentials.TransportCredentials {
	return &alpnEnforcer{
		delegate: credentials.NewTLS(config),
	}
}

type alpnEnforcer struct {
	delegate credentials.TransportCredentials
}

func (a *alpnEnforcer) ClientHandshake(ctx context.Context, authority string, rawConn net.Conn) (net.Conn, credentials.AuthInfo, error) {
	conn, authInfo, err := a.delegate.ClientHandshake(ctx, authority, rawConn)
	if err != nil {
		return nil, nil, err
	}

	tlsInfo, ok := authInfo.(credentials.TLSInfo)
	if !ok {
		_ = conn.Close()
		return nil, nil, fmt.Errorf("TLS credentials did not return a TLS info")
	}
	if tlsInfo.State.NegotiatedProtocol == "" {
		_ = conn.Close()
		return nil, nil, fmt.Errorf("peer does not support ALPN and therefore can't support HTTP/2")
	}
	if tlsInfo.State.NegotiatedProtocol != "h2" {
		_ = conn.Close()
		return nil, nil, fmt.Errorf("negotiated protocol with peer that is not HTTP/2")
	}
	return conn, authInfo, err
}

func (a *alpnEnforcer) ServerHandshake(conn net.Conn) (net.Conn, credentials.AuthInfo, error) {
	return a.delegate.ServerHandshake(conn)
}

func (a *alpnEnforcer) Info() credentials.ProtocolInfo {
	return a.delegate.Info()
}

func (a *alpnEnforcer) Clone() credentials.TransportCredentials {
	return &alpnEnforcer{
		delegate: a.delegate.Clone(),
	}
}

func (a *alpnEnforcer) OverrideServerName(s string) error {
	return a.delegate.OverrideServerName(s)
}

[prakrit55]

@jroper, I would like to fix it then

[jroper]

One thing, the original bug posted was about server side, but I think client side validation is a bigger concern because this results is confusing errors when connecting to servers that don't support ALPN, particularly in corporate environments where transparent decrypting proxies such as zscaler are in use. I would create a separate issue for the client side, except #2742 was already created for this, but was closed by @dfawley as a duplicate of this issue.

[arvindbr8]

@prakrit55 -- thanks for the interest. which one would you like to pick up first?

[arvindbr8]

Sorry for the delay in getting back to this.

Looking at the current state of things every time a new connection is made over TLS, an h2 string is appended to NextProto in tls.config

the Go library itself handles negotiating ALPN supported by the server and client. See https://cs.opensource.google/go/go/+/refs/tags/go1.21.6:src/crypto/tls/handshake_server.go;l=224

@jroper -- It seems like this case is already handled by gRPC-go? I'm curious to know how did you face this issue?

[jroper]

Yes, h2 is always added, and the Go library itself does handle negotiating ALPN. However, if the server being connected to doesn't support ALPN, then the returned NegotiatedProtocol will be empty, see the docs here:

        // NextProtos is a list of supported application level protocols, in
	// order of preference. If both peers support ALPN, the selected
	// protocol will be one from this list, and the connection will fail
	// if there is no mutually supported protocol. If NextProtos is empty
	// or the peer doesn't support ALPN, the connection will succeed and
	// ConnectionState.NegotiatedProtocol will be empty.
	NextProtos []string

That feature is by design - when making ordinary HTTP/2 connections, an HTTP client must check ConnectionState.NegotiatedProtocol, and if it's not set, it will know that the server doesn't support ALPN and therefore can't support HTTP/2, and so use HTTP/1.1 instead. However, grpc-go does not do this check, instead it proceeds to try and use HTTP/2 when that hasn't been negotiated. That's the bug I encountered, and can be reproduced by connecting to an HTTPS server that doesn't support ALPN. Of course, since it doesn't support ALPN, the connection will eventually fail anyway when the server receives the HTTP/2 connection and doesn't know how to handle it, and respond to the client in HTTP/1.1, which the client doesn't know how to handle. But that's a very messy way to fail, much better to detect it up front and fail gracefully with meaningful error message, like "server does not support ALPN and therefore HTTP/2 could not be negotiated".

[prakrit55]

hey @arvindbr8 @jroper, can I try to fix it up, I understand it and wd like to give it a try

[arvindbr8]

@jroper -- thanks for the clarification. You are right, gRPC-Go does not check for ConnectionState.NegotiatedProtocol and err if nil

@prakrit55 -- Sure you can pick this up. Let me assign it you and please use this issue to track the issue. Also please feel free to ask any questions.

[arvindbr8]

@prakrit55 -- this is potentially a breaking change for some existing users. The change needs to be carefully rolled out. Let's have the implementation flag protected using an env variable and keep the default to keep the check turned off.

Once we make sure that this change does break existing users, we can flip the flag.

[arjan-bal]

@prakrit55 if you don't plan to raise a PR soon, I can pick up this issue.

[prakrit55]

@prakrit55 if you don't plan to raise a PR soon, I can pick up this issue

Sure: )

[dfawley]

Let's keep this open until the behavior is default and a subsequent release removes the option to disable it.

[arjan-bal]

We need to provide a mechanism for older users to disable ALPN enforcement before we remove the env variable: #7769