From 14a63a26b9a673857fc37cba37aedc35a10ce634 Mon Sep 17 00:00:00 2001 From: WeidiDeng Date: Fri, 22 Aug 2025 22:30:42 +0800 Subject: [PATCH] caddyhttp: use the new http.Protocols to handle h1, h2 and h2c requests (#6961) * use the new http.Protocols to handle h1, h2 and h2c requests * fix lint * keep ConnCtxKey for now * fix handling for h2c * check http version while reading the connection * check if connection implements connectionStater when it should * add comments about either h1 or h2 must be used in the listener * fix if check * return a net.Conn that implements connectionStater if applicable * remove http/1.1 from alpn if h1 is disabled * fix matching if only h1 is enabled --------- Co-authored-by: Matt Holt --- modules/caddyhttp/app.go | 128 ++++++++++------------ modules/caddyhttp/http2listener.go | 164 +++++++++++++++-------------- modules/caddyhttp/server.go | 13 +-- 3 files changed, 146 insertions(+), 159 deletions(-) diff --git a/modules/caddyhttp/app.go b/modules/caddyhttp/app.go index 9eaf05d01..24a8415a5 100644 --- a/modules/caddyhttp/app.go +++ b/modules/caddyhttp/app.go @@ -28,7 +28,6 @@ import ( "go.uber.org/zap" "golang.org/x/net/http2" - "golang.org/x/net/http2/h2c" "github.com/caddyserver/caddy/v2" "github.com/caddyserver/caddy/v2/modules/caddyevents" @@ -236,15 +235,6 @@ func (app *App) Provision(ctx caddy.Context) error { for _, srvProtocol := range srv.Protocols { srvProtocolsUnique[srvProtocol] = struct{}{} } - _, h1ok := srvProtocolsUnique["h1"] - _, h2ok := srvProtocolsUnique["h2"] - _, h2cok := srvProtocolsUnique["h2c"] - - // the Go standard library does not let us serve only HTTP/2 using - // http.Server; we would probably need to write our own server - if !h1ok && (h2ok || h2cok) { - return fmt.Errorf("server %s: cannot enable HTTP/2 or H2C without enabling HTTP/1.1; add h1 to protocols or remove h2/h2c", srvName) - } if srv.ListenProtocols != nil { if len(srv.ListenProtocols) != len(srv.Listen) { @@ -278,19 +268,6 @@ func (app *App) Provision(ctx caddy.Context) error { } } - lnProtocolsIncludeUnique := map[string]struct{}{} - for _, lnProtocol := range lnProtocolsInclude { - lnProtocolsIncludeUnique[lnProtocol] = struct{}{} - } - _, h1ok := lnProtocolsIncludeUnique["h1"] - _, h2ok := lnProtocolsIncludeUnique["h2"] - _, h2cok := lnProtocolsIncludeUnique["h2c"] - - // check if any listener protocols contain h2 or h2c without h1 - if !h1ok && (h2ok || h2cok) { - return fmt.Errorf("server %s, listener %d: cannot enable HTTP/2 or H2C without enabling HTTP/1.1; add h1 to protocols or remove h2/h2c", srvName, i) - } - srv.ListenProtocols[i] = lnProtocolsInclude } } @@ -448,6 +425,25 @@ func (app *App) Validate() error { return nil } +func removeTLSALPN(srv *Server, target string) { + for _, cp := range srv.TLSConnPolicies { + // the TLSConfig was already provisioned, so... manually remove it + for i, np := range cp.TLSConfig.NextProtos { + if np == target { + cp.TLSConfig.NextProtos = append(cp.TLSConfig.NextProtos[:i], cp.TLSConfig.NextProtos[i+1:]...) + break + } + } + // remove it from the parent connection policy too, just to keep things tidy + for i, alpn := range cp.ALPN { + if alpn == target { + cp.ALPN = append(cp.ALPN[:i], cp.ALPN[i+1:]...) + break + } + } + } +} + // Start runs the app. It finishes automatic HTTPS if enabled, // including management of certificates. func (app *App) Start() error { @@ -466,32 +462,37 @@ func (app *App) Start() error { MaxHeaderBytes: srv.MaxHeaderBytes, Handler: srv, ErrorLog: serverLogger, + Protocols: new(http.Protocols), ConnContext: func(ctx context.Context, c net.Conn) context.Context { return context.WithValue(ctx, ConnCtxKey, c) }, } - h2server := new(http2.Server) // disable HTTP/2, which we enabled by default during provisioning if !srv.protocol("h2") { srv.server.TLSNextProto = make(map[string]func(*http.Server, *tls.Conn, http.Handler)) - for _, cp := range srv.TLSConnPolicies { - // the TLSConfig was already provisioned, so... manually remove it - for i, np := range cp.TLSConfig.NextProtos { - if np == "h2" { - cp.TLSConfig.NextProtos = append(cp.TLSConfig.NextProtos[:i], cp.TLSConfig.NextProtos[i+1:]...) - break - } - } - // remove it from the parent connection policy too, just to keep things tidy - for i, alpn := range cp.ALPN { - if alpn == "h2" { - cp.ALPN = append(cp.ALPN[:i], cp.ALPN[i+1:]...) - break - } - } - } - } else { + removeTLSALPN(srv, "h2") + } + if !srv.protocol("h1") { + removeTLSALPN(srv, "http/1.1") + } + + // configure the http versions the server will serve + if srv.protocol("h1") { + srv.server.Protocols.SetHTTP1(true) + } + + if srv.protocol("h2") || srv.protocol("h2c") { + // skip setting h2 because if NextProtos is present, it's list of alpn versions will take precedence. + // it will always be present because http2.ConfigureServer will populate that field + // enabling h2c because some listener wrapper will wrap the connection that is no longer *tls.Conn + // However, we need to handle the case that if the connection is h2c but h2c is not enabled. We identify + // this type of connection by checking if it's behind a TLS listener wrapper or if it implements tls.ConnectionState. + srv.server.Protocols.SetUnencryptedHTTP2(true) + // when h2c is enabled but h2 disabled, we already removed h2 from NextProtos + // the handshake will never succeed with h2 + // http2.ConfigureServer will enable the server to handle both h2 and h2c + h2server := new(http2.Server) //nolint:errcheck http2.ConfigureServer(srv.server, h2server) } @@ -501,11 +502,6 @@ func (app *App) Start() error { tlsCfg := srv.TLSConnPolicies.TLSConfig(app.ctx) srv.configureServer(srv.server) - // enable H2C if configured - if srv.protocol("h2c") { - srv.server.Handler = h2c.NewHandler(srv, h2server) - } - for lnIndex, lnAddr := range srv.Listen { listenAddr, err := caddy.ParseNetworkAddress(lnAddr) if err != nil { @@ -570,15 +566,13 @@ func (app *App) Start() error { ln = srv.listenerWrappers[i].WrapListener(ln) } - // handle http2 if use tls listener wrapper - if h2ok { - http2lnWrapper := &http2Listener{ - Listener: ln, - server: srv.server, - h2server: h2server, - } - srv.h2listeners = append(srv.h2listeners, http2lnWrapper) - ln = http2lnWrapper + // check if the connection is h2c + ln = &http2Listener{ + useTLS: useTLS, + useH1: h1ok, + useH2: h2ok || h2cok, + Listener: ln, + logger: app.logger, } // if binding to port 0, the OS chooses a port for us; @@ -596,11 +590,8 @@ func (app *App) Start() error { srv.listeners = append(srv.listeners, ln) - // enable HTTP/1 if configured - if h1ok { - //nolint:errcheck - go srv.server.Serve(ln) - } + //nolint:errcheck + go srv.server.Serve(ln) } if h2ok && !useTLS { @@ -756,25 +747,12 @@ func (app *App) Stop() error { } } } - stopH2Listener := func(server *Server) { - defer finishedShutdown.Done() - startedShutdown.Done() - - for i, s := range server.h2listeners { - if err := s.Shutdown(ctx); err != nil { - app.logger.Error("http2 listener shutdown", - zap.Error(err), - zap.Int("index", i)) - } - } - } for _, server := range app.Servers { - startedShutdown.Add(3) - finishedShutdown.Add(3) + startedShutdown.Add(2) + finishedShutdown.Add(2) go stopServer(server) go stopH3Server(server) - go stopH2Listener(server) } // block until all the goroutines have been run by the scheduler; diff --git a/modules/caddyhttp/http2listener.go b/modules/caddyhttp/http2listener.go index 51b356a77..afc5e51ab 100644 --- a/modules/caddyhttp/http2listener.go +++ b/modules/caddyhttp/http2listener.go @@ -1,102 +1,110 @@ package caddyhttp import ( - "context" "crypto/tls" - weakrand "math/rand" + "io" "net" - "net/http" - "sync/atomic" - "time" + "go.uber.org/zap" "golang.org/x/net/http2" ) -// http2Listener wraps the listener to solve the following problems: -// 1. server h2 natively without using h2c hack when listener handles tls connection but -// don't return *tls.Conn -// 2. graceful shutdown. the shutdown logic is copied from stdlib http.Server, it's an extra maintenance burden but -// whatever, the shutdown logic maybe extracted to be used with h2c graceful shutdown. http2.Server supports graceful shutdown -// sending GO_AWAY frame to connected clients, but doesn't track connection status. It requires explicit call of http2.ConfigureServer -type http2Listener struct { - cnt uint64 - net.Listener - server *http.Server - h2server *http2.Server -} - -type connectionStateConn interface { - net.Conn +type connectionStater interface { ConnectionState() tls.ConnectionState } +// http2Listener wraps the listener to solve the following problems: +// 1. prevent genuine h2c connections from succeeding if h2c is not enabled +// and the connection doesn't implment connectionStater or the resulting NegotiatedProtocol +// isn't http2. +// This does allow a connection to pass as tls enabled even if it's not, listener wrappers +// can do this. +// 2. After wrapping the connection doesn't implement connectionStater, emit a warning so that listener +// wrapper authors will hopefully implement it. +// 3. check if the connection matches a specific http version. h2/h2c has a distinct preface. +type http2Listener struct { + useTLS bool + useH1 bool + useH2 bool + net.Listener + logger *zap.Logger +} + func (h *http2Listener) Accept() (net.Conn, error) { - for { - conn, err := h.Listener.Accept() - if err != nil { - return nil, err - } + conn, err := h.Listener.Accept() + if err != nil { + return nil, err + } - if csc, ok := conn.(connectionStateConn); ok { - // *tls.Conn will return empty string because it's only populated after handshake is complete - if csc.ConnectionState().NegotiatedProtocol == http2.NextProtoTLS { - go h.serveHttp2(csc) - continue - } - } + _, isConnectionStater := conn.(connectionStater) + // emit a warning + if h.useTLS && !isConnectionStater { + h.logger.Warn("tls is enabled, but listener wrapper returns a connection that doesn't implement connectionStater") + } else if !h.useTLS && isConnectionStater { + h.logger.Warn("tls is disabled, but listener wrapper returns a connection that implements connectionStater") + } + // if both h1 and h2 are enabled, we don't need to check the preface + if h.useH1 && h.useH2 { return conn, nil } -} -func (h *http2Listener) serveHttp2(csc connectionStateConn) { - atomic.AddUint64(&h.cnt, 1) - h.runHook(csc, http.StateNew) - defer func() { - csc.Close() - atomic.AddUint64(&h.cnt, ^uint64(0)) - h.runHook(csc, http.StateClosed) - }() - h.h2server.ServeConn(csc, &http2.ServeConnOpts{ - Context: h.server.ConnContext(context.Background(), csc), - BaseConfig: h.server, - Handler: h.server.Handler, - }) -} - -const shutdownPollIntervalMax = 500 * time.Millisecond - -func (h *http2Listener) Shutdown(ctx context.Context) error { - pollIntervalBase := time.Millisecond - nextPollInterval := func() time.Duration { - // Add 10% jitter. - //nolint:gosec - interval := pollIntervalBase + time.Duration(weakrand.Intn(int(pollIntervalBase/10))) - // Double and clamp for next time. - pollIntervalBase *= 2 - if pollIntervalBase > shutdownPollIntervalMax { - pollIntervalBase = shutdownPollIntervalMax - } - return interval + // impossible both are false, either useH1 or useH2 must be true, + // or else the listener wouldn't be created + h2Conn := &http2Conn{ + h2Expected: h.useH2, + Conn: conn, } + if isConnectionStater { + return http2StateConn{h2Conn}, nil + } + return h2Conn, nil +} - timer := time.NewTimer(nextPollInterval()) - defer timer.Stop() - for { - if atomic.LoadUint64(&h.cnt) == 0 { - return nil +type http2StateConn struct { + *http2Conn +} + +func (conn http2StateConn) ConnectionState() tls.ConnectionState { + return conn.Conn.(connectionStater).ConnectionState() +} + +type http2Conn struct { + // current index where the preface should match, + // no matching is done if idx is >= len(http2.ClientPreface) + idx int + // whether the connection is expected to be h2/h2c + h2Expected bool + // log if one such connection is detected + logger *zap.Logger + net.Conn +} + +func (c *http2Conn) Read(p []byte) (int, error) { + if c.idx >= len(http2.ClientPreface) { + return c.Conn.Read(p) + } + n, err := c.Conn.Read(p) + for i := range n { + // first mismatch + if p[i] != http2.ClientPreface[c.idx] { + // close the connection if h2 is expected + if c.h2Expected { + c.logger.Debug("h1 connection detected, but h1 is not enabled") + _ = c.Conn.Close() + return 0, io.EOF + } + // no need to continue matching anymore + c.idx = len(http2.ClientPreface) + return n, err } - select { - case <-ctx.Done(): - return ctx.Err() - case <-timer.C: - timer.Reset(nextPollInterval()) + c.idx++ + // matching complete + if c.idx == len(http2.ClientPreface) && !c.h2Expected { + c.logger.Debug("h2/h2c connection detected, but h2/h2c is not enabled") + _ = c.Conn.Close() + return 0, io.EOF } } -} - -func (h *http2Listener) runHook(conn net.Conn, state http.ConnState) { - if h.server.ConnState != nil { - h.server.ConnState(conn, state) - } + return n, err } diff --git a/modules/caddyhttp/server.go b/modules/caddyhttp/server.go index 8aaf76f4a..f6666c14f 100644 --- a/modules/caddyhttp/server.go +++ b/modules/caddyhttp/server.go @@ -246,10 +246,9 @@ type Server struct { traceLogger *zap.Logger ctx caddy.Context - server *http.Server - h3server *http3.Server - h2listeners []*http2Listener - addresses []caddy.NetworkAddress + server *http.Server + h3server *http3.Server + addresses []caddy.NetworkAddress trustedProxies IPRangeSource @@ -266,11 +265,11 @@ type Server struct { // ServeHTTP is the entry point for all HTTP requests. func (s *Server) ServeHTTP(w http.ResponseWriter, r *http.Request) { // If there are listener wrappers that process tls connections but don't return a *tls.Conn, this field will be nil. - // TODO: Can be removed if https://github.com/golang/go/pull/56110 is ever merged. + // TODO: Scheduled to be removed later because https://github.com/golang/go/pull/56110 has been merged. if r.TLS == nil { // not all requests have a conn (like virtual requests) - see #5698 if conn, ok := r.Context().Value(ConnCtxKey).(net.Conn); ok { - if csc, ok := conn.(connectionStateConn); ok { + if csc, ok := conn.(connectionStater); ok { r.TLS = new(tls.ConnectionState) *r.TLS = csc.ConnectionState() } @@ -1083,6 +1082,8 @@ const ( OriginalRequestCtxKey caddy.CtxKey = "original_request" // For referencing underlying net.Conn + // This will eventually be deprecated and not used. To refer to the underlying connection, implement a middleware plugin + // that RegisterConnContext during provisioning. ConnCtxKey caddy.CtxKey = "conn" // For tracking whether the client is a trusted proxy