Downgrade/Resource exhaustion risk (HTTP protocol negotiation and idle connection binding)

PoC (explanation and example to reproduce prior to fix): Goal: Demonstrate potential resource exhaustion and protocol downgrade risk from unbounded idle HTTP keep-alive connections and HTTP/2 negotiation. Before this fix, a client could keep a large pool of idle connections open (or negotiate HTTP/2 unexpectedly) in a way that strains Grafana's resources or enables protocol downgrade pitfalls. Environment: A test Grafana server (or a staging instance) running version prior to this fix, with HTTP/2 enabled and no explicit idle-connection bound on the client side. PoC Steps (conceptual, safe lab setup only): 1) Target a Grafana REST endpoint that is accessible via HTTP keep-alive (e.g., /api/settings/v1/settings) behind a load balancer or reverse proxy. 2) Use a client that keeps many connections alive with HTTP/1.1 (or a mix) and does not aggressively close idle connections. 3) Sustain a large number N of idle connections, longer than typical idle timeouts (e.g., >30s) to observe server resource impact (memory/FD usage) due to idle connections. Sample Python script (conceptual): import http.client import threading import time import urllib.parse TARGET = 'https://grafana.example/api/settings/v1/settings' N = 200 # number of concurrent idle connections (adjust to lab capacity) IDLE_SECONDS = 60 class ConnHolder: def __init__(self, host, path): self.conn = http.client.HTTPSConnection(host, timeout=5) self.path = path def start(self): self.conn.connect() self.conn.request('GET', self.path, headers={'Connection': 'keep-alive'}) # get response headers to complete the request without closing the connection immediately self.conn.getresponse().read() host = urllib.parse.urlparse(TARGET).netloc path = urllib.parse.urlparse(TARGET).path threads = [] for i in range(N): t = threading.Thread(target=ConnHolder(host, path).start) t.daemon = True t.start() threads.append(t) print(f'Opened {N} idle HTTP/1.1 connections to {TARGET}. Keeping them open for {IDLE_SECONDS}s...') time.sleep(IDLE_SECONDS) print('PoC complete. If server resources degraded under load, this indicates potential idle-connection DoS risk.') Notes: - This PoC demonstrates potential resource exhaustion due to unbounded idle connections when the client keeps many connections alive. The fix in the commit caps idle connections via IdleConnTimeout (default 30s) and forces HTTP/1.1 when HTTP/2 is disabled, mitigating this risk. In a controlled lab, compare behavior with and without the fix to observe differences in resource usage and protocol negotiation.

diff --git a/pkg/services/setting/service.go b/pkg/services/setting/service.go index 1fe6022a6e243..09570b074b9a7 100644 --- a/pkg/services/setting/service.go +++ b/pkg/services/setting/service.go @@ -55,6 +55,9 @@ const DefaultBurst = 300 const DefaultCacheTTL = 5 * time.Second const DefaultCacheMaxEntries = 1000 +// DefaultIdleConnTimeout caps how long idle keep-alive connections are pooled. +const DefaultIdleConnTimeout = 30 * time.Second + const ( ApiGroup = "setting.grafana.app" apiVersion = "v1beta1" @@ -171,6 +174,12 @@ type Config struct { CacheTTL time.Duration // CacheMaxEntries sets the max LRU cache entries. Defaults to DefaultCacheMaxEntries (1000). CacheMaxEntries int + // IdleConnTimeout caps how long idle keep-alive connections are pooled before + // being closed. Defaults to DefaultIdleConnTimeout. Set to a negative value to use the + // client-go default. + IdleConnTimeout time.Duration + // EnableHTTP2 allows HTTP/2 for the client connection. It is disabled by default. + EnableHTTP2 bool } // Setting represents the parsed spec of a Setting resource. @@ -526,9 +535,17 @@ func getRestClient(config Config, log logging.Logger, m clientMetrics) (*rest.RE } } + idleConnTimeout := DefaultIdleConnTimeout + if config.IdleConnTimeout != 0 { + idleConnTimeout = config.IdleConnTimeout + } + // Wrap with tracing middleware to propagate trace context to all outbound requests tracingMiddleware := httpclientprovider.TracingMiddleware(logging.NewNopLogger(), tracer) wrapTransport := func(rt http.RoundTripper) http.RoundTripper { + if baseTransport, ok := rt.(*http.Transport); ok && idleConnTimeout > 0 { + baseTransport.IdleConnTimeout = idleConnTimeout + } tracingRT := tracingMiddleware.CreateMiddleware(httpclient.Options{}, rt) return authTransport(tracingRT) } @@ -548,6 +565,13 @@ func getRestClient(config Config, log logging.Logger, m clientMetrics) (*rest.RE // Add a default scheme to handle K8s API error responses scheme := runtime.NewScheme() + // When HTTP/2 is disabled we restrict ALPN to HTTP/1.1 unconditionally. + // ALPN only applies to TLS; plain HTTP already uses HTTP/1.1. + tlsClientConfig := config.TLSClientConfig + if !config.EnableHTTP2 { + tlsClientConfig.NextProtos = []string{"http/1.1"} + } + // Create the rate limiter explicitly so we can wrap it with instrumentation rateLimiter := &instrumentedRateLimiter{ RateLimiter: flowcontrol.NewTokenBucketRateLimiter(qps, burst), @@ -556,7 +580,7 @@ func getRestClient(config Config, log logging.Logger, m clientMetrics) (*rest.RE restConfig := &rest.Config{ Host: config.URL, - TLSClientConfig: config.TLSClientConfig, + TLSClientConfig: tlsClientConfig, WrapTransport: wrapTransport, RateLimiter: rateLimiter, UserAgent: userAgent, diff --git a/pkg/services/setting/service_test.go b/pkg/services/setting/service_test.go index bf40a7596cbad..3e63ca4cc4b41 100644 --- a/pkg/services/setting/service_test.go +++ b/pkg/services/setting/service_test.go @@ -22,6 +22,7 @@ import ( sdktrace "go.opentelemetry.io/otel/sdk/trace" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apiserver/pkg/endpoints/request" + "k8s.io/client-go/rest" "k8s.io/client-go/util/flowcontrol" ) @@ -664,6 +665,73 @@ func TestNew(t *testing.T) { }) } +func TestNew_HTTPProtocol(t *testing.T) { + settings := []Setting{{Section: "server", Key: "port", Value: "3000"}} + + newHTTP2Server := func(t *testing.T, negotiatedProto *atomic.Int32) *httptest.Server { + t.Helper() + server := httptest.NewUnstartedServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + negotiatedProto.Store(int32(r.ProtoMajor)) + w.Header().Set("Content-Type", "application/json") + _, _ = w.Write([]byte(generateSettingsJSON(settings, ""))) + })) + // Offer HTTP/2 via ALPN so the client's protocol preference decides the outcome. + server.EnableHTTP2 = true + server.StartTLS() + t.Cleanup(server.Close) + return server + } + + newClient := func(t *testing.T, tlsConfig rest.TLSClientConfig, url string, enableHTTP2 bool) Service { + t.Helper() + client, err := New(Config{ + URL: url, + WrapTransport: func(rt http.RoundTripper) http.RoundTripper { return rt }, + TLSClientConfig: tlsConfig, + CacheTTL: -1, + EnableHTTP2: enableHTTP2, + }) + require.NoError(t, err) + return client + } + + insecure := rest.TLSClientConfig{Insecure: true} + ctx := request.WithNamespace(context.Background(), "test-namespace") + + t.Run("should negotiate HTTP/1.1 when HTTP/2 is not enabled", func(t *testing.T) { + var proto atomic.Int32 + server := newHTTP2Server(t, &proto) + + _, err := newClient(t, insecure, server.URL, false).List(ctx, metav1.LabelSelector{}) + + require.NoError(t, err) + assert.Equal(t, int32(1), proto.Load(), "client should negotiate HTTP/1.1 by default") + }) + + t.Run("should negotiate HTTP/1.1 when HTTP/2 is disabled but caller sets h2 in NextProtos", func(t *testing.T) { + var proto atomic.Int32 + server := newHTTP2Server(t, &proto) + + // EnableHTTP2 is the source of truth: a caller-supplied h2 preference must + // not re-admit HTTP/2 while the flag is off. + tlsConfig := rest.TLSClientConfig{Insecure: true, NextProtos: []string{"h2", "http/1.1"}} + _, err := newClient(t, tlsConfig, server.URL, false).List(ctx, metav1.LabelSelector{}) + + require.NoError(t, err) + assert.Equal(t, int32(1), proto.Load(), "EnableHTTP2=false should force HTTP/1.1 regardless of NextProtos") + }) + + t.Run("should negotiate HTTP/2 when EnableHTTP2 is set", func(t *testing.T) { + var proto atomic.Int32 + server := newHTTP2Server(t, &proto) + + _, err := newClient(t, insecure, server.URL, true).List(ctx, metav1.LabelSelector{}) + + require.NoError(t, err) + assert.Equal(t, int32(2), proto.Load(), "client should negotiate HTTP/2 when EnableHTTP2 is set") + }) +} + func TestListCache(t *testing.T) { t.Run("should return cached result on second call with same namespace and selector", func(t *testing.T) { var requestCount atomic.Int32