Skip to content

Commit

Permalink
go/network: improve support for clients lacking ALPN
Browse files Browse the repository at this point in the history
We've seen a few legacy clients that don't support HTTP/2 or ALPN
negotiation. Improve the connector networking frontend to support
HTTP/1.1 transports for clients that don't support HTTP/2, or don't
support ALPN at all, while still preferring HTTP/2 where possible.

In particular, this lets connector networking work seamlessly with
NodeJS clients that are using the old "https" NodeJS module instead
of the newer "http2" module. This is less performant than HTTP/2,
but metrics can indicate which tasks are using large numbers of
user connections / TLS handshakes.
  • Loading branch information
jgraettinger committed Nov 18, 2024
1 parent dfc5b27 commit 8831666
Show file tree
Hide file tree
Showing 2 changed files with 87 additions and 26 deletions.
98 changes: 75 additions & 23 deletions go/network/frontend.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@ import (
lru "github.com/hashicorp/golang-lru/v2"
pb "go.gazette.dev/core/broker/protocol"
pc "go.gazette.dev/core/consumer/protocol"
"golang.org/x/net/http2"
)

// Frontend accepts connections over its configured Listener,
Expand Down Expand Up @@ -233,27 +232,53 @@ func (p *Frontend) getTLSConfigForClient(hello *tls.ClientHelloInfo) (*tls.Confi
}
}

if conn.sniErr == nil && conn.resolved.portProtocol != "" {
// We intend to TCP proxy to the connector. Dial the shard now so that
// we fail-fast during TLS handshake, instead of letting the client
// think it has a good connection.
var addr = conn.raw.RemoteAddr().String()
conn.dialed, conn.dialErr = dialShard(
hello.Context(), p.networkClient, p.shardClient, conn.parsed, conn.resolved, addr)
}
if conn.sniErr != nil {
// We failed to resolve to a shard, and will send a best-effort HTTP/1.1 error.
conn.resolved.portProtocol = protoHTTP11
} else if conn.resolved.portProtocol == protoHTTP11 {
// We intend to reverse proxy HTTP requests to connector shards.

if slices.Contains(hello.SupportedProtos, protoHTTP2) {
// We'll proxy client HTTP/2 to connector HTTP/1.1
conn.resolved.portProtocol = protoHTTP2
} else if len(hello.SupportedProtos) == 0 {
// Some clients, such as NodeJS's `https` module, don't support ALPN.
// We assume HTTP/1.1 and hope it works out.
} else if !slices.Contains(hello.SupportedProtos, protoHTTP11) {
conn.sniErr = fmt.Errorf(
"client supports ALPN protocols %v, but the connector requires `h2` or `http/1.1`",
hello.SupportedProtos,
)
}

var nextProtos []string
if conn.sniErr != nil || conn.dialErr != nil {
nextProtos = []string{"http/1.1"} // We'll send a descriptive HTTP/1.1 error.
} else if conn.dialed == nil {
nextProtos = []string{"h2"} // We'll reverse-proxy. The user MUST speak HTTP/2.
} else {
nextProtos = []string{conn.resolved.portProtocol} // We'll TCP proxy.
} else if len(hello.SupportedProtos) != 0 &&
!slices.Contains(hello.SupportedProtos, conn.resolved.portProtocol) {
// We intend to TCP proxy, but the client sent supported protocols and none match.
// We'll send a best-effort error.
conn.sniErr = fmt.Errorf(
"client supports ALPN protocols %v, but the connector requires protocol %s",
hello.SupportedProtos,
conn.resolved.portProtocol,
)
conn.resolved.portProtocol = protoHTTP11

} else if conn.dialed, conn.dialErr = dialShard(
hello.Context(),
p.networkClient,
p.shardClient,
conn.parsed,
conn.resolved,
conn.raw.RemoteAddr().String(),
); conn.dialErr != nil {
// We intend to TCP proxy to the connector. We dialed the shard so that
// we fail-fast during TLS handshake, instead of letting the client
// think it has a good connection, and now report a best-effort dial error.
conn.resolved.portProtocol = protoHTTP11
}

return &tls.Config{
Certificates: p.tlsConfig.Certificates,
NextProtos: nextProtos,
NextProtos: []string{conn.resolved.portProtocol},
}, nil
}

Expand Down Expand Up @@ -370,13 +395,23 @@ func (p *Frontend) serveConnHTTP(user *frontendConn) {
}
}

(&http2.Server{
// IdleTimeout can be generous: it's intended to catch broken TCP transports.
// MaxConcurrentStreams is an important setting left as the default (100).
IdleTimeout: time.Minute,
}).ServeConn(user.tls, &http2.ServeConnOpts{
var stubListener = &stubListener{conn: user.tls, done: make(chan struct{})}

_ = (&http.Server{
Handler: http.HandlerFunc(handle),
})
// ReadHeaderTimeout applies only to HTTP/1.1 connections.
ReadHeaderTimeout: time.Second * 30,
// IdleTimeout apples to HTTP/1.1 and HTTP/2. It can be generous: it's
// intended to catch broken TCP transports.
// MaxConcurrentStreams is another important HTTP/2 setting left as the default (100).
IdleTimeout: time.Minute,
// ConnState hook which stops this Server when its one connection is done.
ConnState: func(_ net.Conn, cs http.ConnState) {
if cs == http.StateClosed {
close(stubListener.done) // Accept now unblocks with an error.
}
},
}).Serve(stubListener)

userHandledCounter.WithLabelValues(task, port, proto, "OK").Inc()
}
Expand All @@ -395,3 +430,20 @@ func (f *Frontend) serveConnErr(conn net.Conn, status int, body string) {
_, _ = conn.Write(resp)
_ = conn.Close()
}

type stubListener struct {
conn net.Conn
done chan struct{}
}

func (l *stubListener) Accept() (net.Conn, error) {
if c := l.conn; c != nil {
l.conn = nil
return c, nil
}
<-l.done
return nil, io.EOF
}

func (l *stubListener) Close() error { return nil }
func (l *stubListener) Addr() net.Addr { return l.conn.LocalAddr() }
15 changes: 12 additions & 3 deletions go/network/sni.go
Original file line number Diff line number Diff line change
Expand Up @@ -79,9 +79,13 @@ func newResolvedSNI(parsed parsedSNI, shard *pc.ShardSpec) resolvedSNI {
var portProtocol = shard.LabelSet.ValueOf(labels.PortProtoPrefix + parsed.port)
var portIsPublic = shard.LabelSet.ValueOf(labels.PortPublicPrefix+parsed.port) == "true"

// Private ports MUST use the HTTP/1.1 reverse proxy.
if !portIsPublic {
portProtocol = ""
// HTTP/1.1 is the only protocol which we reverse proxy. It's the assumed
// protocol if none is specified, and is required if the port is private.
if portProtocol == "" || !portIsPublic {
portProtocol = protoHTTP11
} else if portProtocol == "h2c" {
// Connector expects cleartext HTTP/2. We terminate TLS and TCP proxy.
portProtocol = protoHTTP2
}

return resolvedSNI{
Expand Down Expand Up @@ -122,3 +126,8 @@ func listShards(ctx context.Context, shards pc.ShardClient, parsed parsedSNI, sh

return resp.Shards, nil
}

const (
protoHTTP11 = "http/1.1"
protoHTTP2 = "h2"
)

0 comments on commit 8831666

Please sign in to comment.