diff --git a/modules/caddyhttp/app.go b/modules/caddyhttp/app.go index b550904e2..e586556ca 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" @@ -231,15 +230,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) { @@ -273,19 +263,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 } } @@ -461,11 +438,8 @@ func (app *App) Start() error { MaxHeaderBytes: srv.MaxHeaderBytes, Handler: srv, ErrorLog: serverLogger, - ConnContext: func(ctx context.Context, c net.Conn) context.Context { - return context.WithValue(ctx, ConnCtxKey, c) - }, + Protocols: new(http.Protocols), } - h2server := new(http2.Server) // disable HTTP/2, which we enabled by default during provisioning if !srv.protocol("h2") { @@ -486,7 +460,24 @@ func (app *App) Start() error { } } } - } else { + } + + // 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) } @@ -496,11 +487,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 { @@ -560,15 +546,11 @@ 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, + Listener: ln, + logger: app.logger, } // if binding to port 0, the OS chooses a port for us; @@ -586,11 +568,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 { @@ -723,25 +702,12 @@ func (app *App) Stop() error { zap.Strings("addresses", server.Listen)) } } - 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..a4c6f8572 100644 --- a/modules/caddyhttp/http2listener.go +++ b/modules/caddyhttp/http2listener.go @@ -1,102 +1,87 @@ package caddyhttp import ( - "context" "crypto/tls" - weakrand "math/rand" + "go.uber.org/zap" + "io" "net" - "net/http" - "sync/atomic" - "time" "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. +type http2Listener struct { + useTLS 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 - } - - 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 - } - } - - 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 + conn, err := h.Listener.Accept() + if err != nil { + return nil, err } - timer := time.NewTimer(nextPollInterval()) - defer timer.Stop() - for { - if atomic.LoadUint64(&h.cnt) == 0 { - return nil - } - select { - case <-ctx.Done(): - return ctx.Err() - case <-timer.C: - timer.Reset(nextPollInterval()) + if h.useTLS { + // emit a warning + if _, ok := conn.(connectionStater); !ok { + h.logger.Warn("tls is enabled, but listener wrapper returns a connection that doesn't implement connectionStater") } + return &http2Conn{ + idx: len(http2.ClientPreface), + Conn: conn, + }, nil } + + if _, ok := conn.(connectionStater); ok { + h.logger.Warn("tls is disabled, but listener wrapper returns a connection that implements connectionStater") + return &http2Conn{ + idx: len(http2.ClientPreface), + Conn: conn, + }, nil + } + + return &http2Conn{ + Conn: conn, + }, nil } -func (h *http2Listener) runHook(conn net.Conn, state http.ConnState) { - if h.server.ConnState != nil { - h.server.ConnState(conn, state) - } +type http2Conn struct { + // check h2 preface if it's smaller that the preface + idx int + // 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 { + // mismatch + if p[i] != http2.ClientPreface[c.idx] { + c.idx = len(http2.ClientPreface) + return n, err + } + c.idx++ + if c.idx == len(http2.ClientPreface) { + c.logger.Warn("h2c connection detected, but h2c is not enabled") + _ = c.Conn.Close() + return 0, io.EOF + } + } + return n, err } diff --git a/modules/caddyhttp/server.go b/modules/caddyhttp/server.go index a2b29d658..32b06b271 100644 --- a/modules/caddyhttp/server.go +++ b/modules/caddyhttp/server.go @@ -245,10 +245,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 @@ -264,18 +263,6 @@ 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. - 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 { - r.TLS = new(tls.ConnectionState) - *r.TLS = csc.ConnectionState() - } - } - } - w.Header().Set("Server", "Caddy") // advertise HTTP/3, if enabled @@ -1080,6 +1067,8 @@ const ( OriginalRequestCtxKey caddy.CtxKey = "original_request" // For referencing underlying net.Conn + // DEPRECATED: Not used anymore. 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