diff --git a/app/artifact-cas/internal/server/http.go b/app/artifact-cas/internal/server/http.go index 9c8e6823f..6506129eb 100644 --- a/app/artifact-cas/internal/server/http.go +++ b/app/artifact-cas/internal/server/http.go @@ -40,9 +40,52 @@ import ( "github.com/go-kratos/kratos/v2/transport/grpc" "github.com/go-kratos/kratos/v2/transport/http" "github.com/golang-jwt/jwt/v5" + "golang.org/x/net/netutil" grpcLib "google.golang.org/grpc" ) +// Connection-liveness limits for the shared public listener. The single-port +// design serves HTTP/1.1 and native gRPC over HTTP/2 (including cleartext h2c) +// on one net/http listener, which does not set any of net/http's optional +// idle/liveness or connection limits by default. These give the shared listener +// the same connection hygiene the dedicated gRPC transport already had +// (PFM-7981). They are deliberately generous so they never trip on legitimate +// traffic; a fronting proxy or ingress can impose stricter limits. They apply to +// the public inbound server, not to the loopback reverse-proxy transport +// (grpcProxy), which is bounded separately. +const ( + // publicIdleTimeout bounds how long an established connection may stay idle + // before it is closed. net/http applies it to idle HTTP/1.x keep-alives and, + // since Go 1.24, to HTTP/2 connections with no open streams. The HTTP/2 idle + // timer is armed at connection start and is not refreshed by SETTINGS/PING + // frames, so an idle HTTP/2 connection is closed even while keepalive pings + // are answered. ReadHeaderTimeout does not apply on the HTTP/2 path, where + // Go clears the connection deadline. + publicIdleTimeout = 120 * time.Second + // publicHTTP2PingInterval sends an HTTP/2 PING after this much inactivity + // and publicHTTP2PingTimeout closes the connection if the PING goes + // unanswered. This closes connections whose peer has gone away without a + // clean shutdown, which the idle timer alone does not observe. + publicHTTP2PingInterval = 15 * time.Second + publicHTTP2PingTimeout = 15 * time.Second + // publicHTTP2MaxReadFrameSize bounds the largest HTTP/2 frame the server + // will read, capping the per-connection read buffer. 64 KiB sits above the + // 16 KiB floor the spec mandates and is ample for gRPC framing. + publicHTTP2MaxReadFrameSize = 64 << 10 + // publicHTTP2MaxConcurrentStreams bounds concurrent streams per connection. + // 250 matches net/http's own default, so it does not constrain legitimate + // multiplexing; it is set explicitly to document the bound and guard against + // the default changing. + publicHTTP2MaxConcurrentStreams = 250 + // maxPublicConns caps concurrent connections accepted on the shared + // endpoint. At roughly 2 goroutines + 1 FD + ~30 KiB of heap per connection, + // 4096 connections cost well under the smallest documented pod; a fronting + // proxy typically pools to far fewer. Connections beyond the cap wait in the + // accept backlog rather than being served, so a stricter per-client limit + // belongs at the edge. + maxPublicConns = 4096 +) + // APIServer owns the shared CAS listener on server.http.addr. The standard // HTTP server owns every connection and dispatches each request: // @@ -141,6 +184,9 @@ func NewHTTPServer(settings *Opts, grpcServers *GRPCServers) (*APIServer, error) _ = grpcServers.Close() return nil, fmt.Errorf("binding the shared CAS endpoint %q: %w", c.Http.GetAddr(), err) } + // Cap concurrent connections on the shared endpoint (PFM-7981). Wrap + // the raw TCP listener so the limit counts accepts; TLS is layered on top. + lis = netutil.LimitListener(lis, maxPublicConns) scheme := "http" if tlsConfig != nil { // One certificate covers both transports, with ALPN offering @@ -173,9 +219,23 @@ func NewHTTPServer(settings *Opts, grpcServers *GRPCServers) (*APIServer, error) srv.Protocols.SetHTTP1(true) srv.Protocols.SetHTTP2(true) srv.Protocols.SetUnencryptedHTTP2(true) - // Bound how long a client may take to deliver its request headers - // (slowloris protection). Response streaming is not affected. + // Bound how long a client may take to deliver its request headers over + // HTTP/1.x. On HTTP/2 this is inert — Go clears the connection deadline — so + // the idle and keepalive limits below apply to HTTP/2 instead. Response + // streaming is not affected. srv.ReadHeaderTimeout = 60 * time.Second + // Apply the shared listener's connection-liveness limits (PFM-7981): + // net/http keeps an idle HTTP/2 connection open indefinitely without an idle + // timeout or keepalive ping. No absolute read/write deadline is set, so + // multi-GB uploads and downloads are unaffected — only idle connections are + // closed. + srv.IdleTimeout = publicIdleTimeout + srv.HTTP2 = &h.HTTP2Config{ + MaxConcurrentStreams: publicHTTP2MaxConcurrentStreams, + MaxReadFrameSize: publicHTTP2MaxReadFrameSize, + SendPingTimeout: publicHTTP2PingInterval, + PingTimeout: publicHTTP2PingTimeout, + } // One proxy and one transport for the process: its connection pool is // reused across requests. Dispatch each request: native gRPC goes through // the standard reverse proxy to the private native server; everything else diff --git a/app/artifact-cas/internal/server/http_liveness_test.go b/app/artifact-cas/internal/server/http_liveness_test.go new file mode 100644 index 000000000..c4f422f16 --- /dev/null +++ b/app/artifact-cas/internal/server/http_liveness_test.go @@ -0,0 +1,63 @@ +// +// Copyright 2026 The Chainloop Authors. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package server + +import ( + "testing" + "time" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// TestSharedServerConnectionLiveness verifies the shared CAS endpoint is built +// with the connection-liveness limits — an idle timeout, HTTP/2 keepalive pings, +// per-connection stream and frame bounds, and a connection cap — on the +// h2c-enabled public listener (PFM-7981). net/http keeps an idle HTTP/2 +// connection open indefinitely without these, since ReadHeaderTimeout does not +// apply on the HTTP/2 path. No absolute read/write deadline is set, so multi-GB +// uploads and downloads remain unaffected. +func TestSharedServerConnectionLiveness(t *testing.T) { + opts := casTestOpts(t) + servers, err := NewGRPCServers(opts) + require.NoError(t, err) + api, err := NewHTTPServer(opts, servers) + require.NoError(t, err) + startCASTestTransport(t, api) + + require.NotNil(t, api.httpSrv.HTTP2, "the shared HTTP/2 config must be set so keepalive/liveness limits apply") + checks := []struct { + name string + got any + want any + }{ + {"IdleTimeout", api.httpSrv.IdleTimeout, publicIdleTimeout}, + {"HTTP2.SendPingTimeout", api.httpSrv.HTTP2.SendPingTimeout, publicHTTP2PingInterval}, + {"HTTP2.PingTimeout", api.httpSrv.HTTP2.PingTimeout, publicHTTP2PingTimeout}, + {"HTTP2.MaxReadFrameSize", api.httpSrv.HTTP2.MaxReadFrameSize, publicHTTP2MaxReadFrameSize}, + {"HTTP2.MaxConcurrentStreams", api.httpSrv.HTTP2.MaxConcurrentStreams, publicHTTP2MaxConcurrentStreams}, + } + for _, c := range checks { + t.Run(c.name, func(t *testing.T) { + assert.Equal(t, c.want, c.got) + }) + } + + // Sanity: the production value must stay generous enough never to trip on + // legitimate traffic. A misconfigured tiny cap would reject valid clients. + assert.GreaterOrEqual(t, maxPublicConns, 1024, "the connection cap must stay well above realistic concurrency") + assert.Equal(t, 120*time.Second, publicIdleTimeout) +} diff --git a/app/controlplane/internal/server/http.go b/app/controlplane/internal/server/http.go index 4ed4234a2..eef4894e9 100644 --- a/app/controlplane/internal/server/http.go +++ b/app/controlplane/internal/server/http.go @@ -19,6 +19,7 @@ import ( "context" "errors" "fmt" + "net" "strings" "time" @@ -34,6 +35,7 @@ import ( "github.com/go-kratos/kratos/v2/middleware/tracing" "github.com/improbable-eng/grpc-web/go/grpcweb" "go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp" + "golang.org/x/net/netutil" "google.golang.org/grpc/codes" "google.golang.org/grpc/status" "google.golang.org/protobuf/proto" @@ -44,6 +46,46 @@ import ( "github.com/go-kratos/kratos/v2/transport/http" ) +// Connection-liveness limits for the shared public listener. The single-port +// design serves HTTP/1.1, gRPC-Web and native gRPC over HTTP/2 (including +// cleartext h2c) on one net/http listener, which does not set any of net/http's +// optional idle/liveness or connection limits by default. These give the shared +// listener the same connection hygiene the dedicated gRPC transport already had +// (PFM-7981). They are deliberately generous so they never trip on legitimate +// traffic; a fronting proxy or ingress can impose stricter limits. +const ( + // publicIdleTimeout bounds how long an established connection may stay idle + // before it is closed. net/http applies it to idle HTTP/1.x keep-alives and, + // since Go 1.24, to HTTP/2 connections with no open streams. The HTTP/2 idle + // timer is armed at connection start and is not refreshed by SETTINGS/PING + // frames, so an idle HTTP/2 connection is closed even while keepalive pings + // are answered. ReadHeaderTimeout does not apply on the HTTP/2 path, where + // Go clears the connection deadline. + publicIdleTimeout = 120 * time.Second + // publicHTTP2PingInterval sends an HTTP/2 PING after this much inactivity + // and publicHTTP2PingTimeout closes the connection if the PING goes + // unanswered. This closes connections whose peer has gone away without a + // clean shutdown, which the idle timer alone does not observe. + publicHTTP2PingInterval = 15 * time.Second + publicHTTP2PingTimeout = 15 * time.Second + // publicHTTP2MaxReadFrameSize bounds the largest HTTP/2 frame the server + // will read, capping the per-connection read buffer. 64 KiB sits above the + // 16 KiB floor the spec mandates and is ample for gRPC framing. + publicHTTP2MaxReadFrameSize = 64 << 10 + // publicHTTP2MaxConcurrentStreams bounds concurrent streams per connection. + // 250 matches net/http's own default, so it does not constrain legitimate + // multiplexing; it is set explicitly to document the bound and guard against + // the default changing. + publicHTTP2MaxConcurrentStreams = 250 + // maxPublicConns caps concurrent connections accepted on the shared + // endpoint. At roughly 2 goroutines + 1 FD + ~30 KiB of heap per connection, + // 4096 connections cost well under the smallest documented pod; a fronting + // proxy typically pools to far fewer. Connections beyond the cap wait in the + // accept backlog rather than being served, so a stricter per-client limit + // belongs at the edge. + maxPublicConns = 4096 +) + // APIServer owns the public HTTP listener and its gRPC handler. The HTTP // server must drain before the handler is stopped, so in-flight RPCs finish. type APIServer struct { @@ -120,8 +162,21 @@ func newHTTPServer(opts *Opts, grpcSrv *grpc.Server) (*http.Server, error) { v1.RegisterStatusServiceHTTPServer(httpSrv, statusSvc) v1.RegisterReferrerServiceHTTPServer(httpSrv, service.NewReferrerService(opts.ReferrerUseCase)) + // Bind the shared listener here so it can be wrapped with a connection-count + // limiter before kratos serves it. Kratos applies TLS on top via ServeTLS + // when a TLS config is set, so the limit counts raw accepts. httpSrv keeps + // its address-based option and is never started — it only routes requests. + network := opts.ServerConfig.GetHttp().GetNetwork() + if network == "" { + network = "tcp" + } + lis, err := net.Listen(network, opts.ServerConfig.GetHttp().GetAddr()) + if err != nil { + return nil, fmt.Errorf("binding the shared control-plane endpoint %q: %w", opts.ServerConfig.GetHttp().GetAddr(), err) + } + // Wrap http server to handle grpc-web calls and we will return this new server - wrappedServer := http.NewServer(serverOpts...) + wrappedServer := http.NewServer(append(serverOpts, http.Listener(netutil.LimitListener(lis, maxPublicConns)))...) wrappedGrpc := grpcweb.WrapServer(grpcSrv.Server, // Be permissive about cors grpcweb.WithOriginFunc(func(_ string) bool { return true }), @@ -140,11 +195,20 @@ func newHTTPServer(opts *Opts, grpcSrv *grpc.Server) (*http.Server, error) { wrappedServer.Protocols.SetHTTP2(true) wrappedServer.Protocols.SetUnencryptedHTTP2(true) // Bound how long a client may take to deliver its request headers over - // HTTP/1.x, on either listener (slowloris protection). Response streaming - // is not affected. The bound does not protect HTTP/2 header completion: - // Go's HTTP/2 path clears the connection deadline, so that case is - // tracked separately. + // HTTP/1.x. On HTTP/2 this is inert — Go clears the connection deadline — so + // the idle and keepalive limits below apply to HTTP/2 instead. Response + // streaming is not affected. wrappedServer.ReadHeaderTimeout = 60 * time.Second + // Apply the shared listener's connection-liveness limits (PFM-7981): + // net/http keeps an idle HTTP/2 connection open indefinitely without an idle + // timeout or keepalive ping, and sets no connection cap by default. + wrappedServer.IdleTimeout = publicIdleTimeout + wrappedServer.HTTP2 = &h.HTTP2Config{ + MaxConcurrentStreams: publicHTTP2MaxConcurrentStreams, + MaxReadFrameSize: publicHTTP2MaxReadFrameSize, + SendPingTimeout: publicHTTP2PingInterval, + PingTimeout: publicHTTP2PingTimeout, + } wrappedServer.Handler = h.HandlerFunc(func(res http.ResponseWriter, req *http.Request) { // Native gRPC uses grpc-go's HTTP adapter and its existing middleware. diff --git a/app/controlplane/internal/server/http_liveness_test.go b/app/controlplane/internal/server/http_liveness_test.go new file mode 100644 index 000000000..b231a2afd --- /dev/null +++ b/app/controlplane/internal/server/http_liveness_test.go @@ -0,0 +1,76 @@ +// +// Copyright 2026 The Chainloop Authors. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package server + +import ( + "context" + "testing" + "time" + + conf "github.com/chainloop-dev/chainloop/app/controlplane/internal/conf/controlplane/config/v1" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// TestSharedServerConnectionLiveness verifies the shared control-plane endpoint +// is built with the connection-liveness limits — an idle timeout, HTTP/2 +// keepalive pings, per-connection stream and frame bounds, and a connection cap +// — on the h2c-enabled public listener (PFM-7981). net/http keeps an idle HTTP/2 +// connection open indefinitely without these, since ReadHeaderTimeout does not +// apply on the HTTP/2 path. +func TestSharedServerConnectionLiveness(t *testing.T) { + opts := testServerOpts(t, &conf.Server{ + Grpc: &conf.Server_GRPC{Addr: testAnyAddr}, + Http: &conf.Server_HTTP{Addr: testAnyAddr}, + }) + gs, err := NewGRPCServers(opts) + require.NoError(t, err) + api, err := NewHTTPServer(opts, gs) + require.NoError(t, err) + + // Start and stop the server so the bound listener is released cleanly. + errCh := make(chan error, 1) + go func() { errCh <- api.Start(context.Background()) }() + t.Cleanup(func() { + ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second) + defer cancel() + require.NoError(t, api.Stop(ctx)) + require.NoError(t, <-errCh) + }) + + require.NotNil(t, api.HTTP2, "the shared HTTP/2 config must be set so keepalive/liveness limits apply") + checks := []struct { + name string + got any + want any + }{ + {"IdleTimeout", api.IdleTimeout, publicIdleTimeout}, + {"HTTP2.SendPingTimeout", api.HTTP2.SendPingTimeout, publicHTTP2PingInterval}, + {"HTTP2.PingTimeout", api.HTTP2.PingTimeout, publicHTTP2PingTimeout}, + {"HTTP2.MaxReadFrameSize", api.HTTP2.MaxReadFrameSize, publicHTTP2MaxReadFrameSize}, + {"HTTP2.MaxConcurrentStreams", api.HTTP2.MaxConcurrentStreams, publicHTTP2MaxConcurrentStreams}, + } + for _, c := range checks { + t.Run(c.name, func(t *testing.T) { + assert.Equal(t, c.want, c.got) + }) + } + + // Sanity: the production value must stay generous enough never to trip on + // legitimate traffic. A misconfigured tiny cap would reject valid clients. + assert.GreaterOrEqual(t, maxPublicConns, 1024, "the connection cap must stay well above realistic concurrency") + assert.Equal(t, 120*time.Second, publicIdleTimeout) +} diff --git a/go.mod b/go.mod index 32aa47669..96d817272 100644 --- a/go.mod +++ b/go.mod @@ -115,6 +115,7 @@ require ( go.uber.org/zap v1.28.0 golang.org/x/crypto v0.58.0 golang.org/x/exp v0.0.0-20260908205506-85c1c2202aba + golang.org/x/net v0.61.0 golang.org/x/oauth2 v0.37.0 golang.org/x/sync v0.24.0 golang.org/x/term v0.47.0 @@ -459,7 +460,6 @@ require ( go4.org v0.0.0-20260112195520-a5071408f32f // indirect goa.design/goa/v3 v3.32.0 // indirect golang.org/x/mod v0.41.0 // indirect - golang.org/x/net v0.61.0 // indirect golang.org/x/sys v0.49.0 // indirect golang.org/x/time v0.16.0 // indirect golang.org/x/tools v0.51.0 // indirect