Merge pull request #1159 from david-martin/cherry-pick-sni-fix-0.7.1

Cherry-pick per-server SNI fix for 0.7.1

Author: david-martin

cmd/mcp-broker-router/main.go

@@ -80,7 +80,7 @@ type app struct {
 	jwtMgr         *session.JWTManager
 	elicitMap      idmap.Map
 	tokenElicitMap elicitation.Map
-	hairpinClient  *http.Client
+	hairpinPool    *clients.HairpinClientPool
 	mcpBroker      broker.MCPBroker
 	tokenHandler   http.Handler
 	elicitHandler  http.Handler
@@ -250,15 +250,15 @@ func (a *app) setupSessionInfra(ctx context.Context) {
 }
 
 func (a *app) buildHairpinClient() {
-	hc, err := clients.BuildHairpinHTTPClient(
+	pool, err := clients.BuildHairpinHTTPClientPool(
 		a.brokerCfg.privateHost,
 		a.brokerCfg.publicHost,
 		a.brokerCfg.gatewayCACert,
 	)
 	if err != nil {
-		panic("failed to build hairpin HTTP client: " + err.Error())
+		panic("failed to build hairpin HTTP client pool: " + err.Error())
 	}
-	a.hairpinClient = hc
+	a.hairpinPool = pool
 }
 
 func (a *app) registerObservers() {

cmd/mcp-broker-router/router.go

@@ -15,7 +15,7 @@ func (a *app) createRouter() {
 		Logger:              a.logger.With("component", "router"),
 		JWTManager:          a.jwtMgr,
 		InitForClient:       clients.Initialize,
-		HairpinHTTPClient:   a.hairpinClient,
+		HairpinClientPool:   a.hairpinPool,
 		SessionCache:        a.sessionCache,
 		ElicitationMap:      a.elicitMap,
 		TokenElicitationMap: a.tokenElicitMap,

docs/guides/how-to-install-and-configure.md

@@ -109,6 +109,20 @@ kubectl patch deployment mcp-gateway -n mcp-system --type=json -p '[
 
 The operator preserves user-added volumes, volume mounts, and command flags across reconciliations.
 
+#### Supported TLS Configurations
+
+The gateway uses TLS termination (`tls.mode: Terminate`) on HTTPS listeners. Envoy decrypts incoming TLS, processes the request (ext_proc, AuthPolicy), then forwards **plain HTTP** to backends. This affects which backend setups work:
+
+**Fully supported — tool discovery and tool calls work:**
+
+- **HTTP listener → HTTP backend** — no TLS involved.
+- **HTTPS listener → HTTP backend** — Envoy terminates client TLS, forwards HTTP to backend. Requires `--gateway-ca-cert` if the listener uses a private CA.
+- **HTTPS listener → external HTTPS backend (with DestinationRule)** — Envoy terminates client TLS, then a DestinationRule configures TLS origination to the external service. See [External MCP Servers](./external-mcp-server.md).
+
+**Partial support — tool discovery works, tool calls require additional configuration:**
+
+- **Any listener → internal HTTPS backend (`caCertSecretRef`)** — the broker connects directly to the backend using the CA cert, so tool discovery works. However, `tools/call` is routed through Envoy, which sends plain HTTP to the TLS backend after termination. The backend rejects the non-TLS connection. To enable `tools/call`, create an Istio DestinationRule with TLS origination for that service — the same pattern used for [external MCP servers](./external-mcp-server.md). See the [Istio documentation on destination rule TLS settings](https://istio.io/latest/docs/reference/config/networking/destination-rule/#ClientTLSSettings) for configuring `credentialName` with a custom CA certificate.
+
 The `--mcp-gateway-public-host` flag tells the router which `Host` header to expect on incoming requests, so it avoids rewriting it during routing.
 
 The **EnvoyFilter** is configured to intercept traffic on the listener's port and route it through the ext_proc (external processor) running on port 50051.

internal/clients/clients.go

@@ -12,14 +12,63 @@ import (
 	"net/http"
 	"os"
 	"strings"
+	"sync"
 
 	"github.com/Kuadrant/mcp-gateway/internal/config"
-	mcprouter "github.com/Kuadrant/mcp-gateway/internal/mcp-router"
 	"github.com/mark3labs/mcp-go/client"
 	"github.com/mark3labs/mcp-go/client/transport"
 	"github.com/mark3labs/mcp-go/mcp"
 )
 
+// HairpinClientPool manages *http.Client instances for hairpin requests,
+// keyed by TLS ServerName (SNI). For HTTP or when all servers share the
+// same listener, a single default client is used. When a server attaches
+// to a different HTTPS listener, a client with that server's SNI is
+// created on demand and cached.
+type HairpinClientPool struct {
+	defaultClient *http.Client
+	baseTLSConfig *tls.Config // nil for HTTP
+	mu            sync.RWMutex
+	clients       map[string]*http.Client
+}
+
+// Get returns an *http.Client with the appropriate TLS ServerName.
+// If sniOverride is empty, the default client (gateway hostname SNI) is returned.
+func (p *HairpinClientPool) Get(sniOverride string) *http.Client {
+	if sniOverride == "" || p.baseTLSConfig == nil {
+		return p.defaultClient
+	}
+
+	sni := sniOverride
+	if h, _, err := net.SplitHostPort(sniOverride); err == nil {
+		sni = h
+	}
+
+	// double-checked locking: read lock for the fast path (concurrent readers),
+	// then write lock with a re-check to avoid duplicate creation when multiple
+	// goroutines race past the read lock simultaneously
+	p.mu.RLock()
+	c, ok := p.clients[sni]
+	p.mu.RUnlock()
+	if ok {
+		return c
+	}
+
+	p.mu.Lock()
+	defer p.mu.Unlock()
+	if c, ok = p.clients[sni]; ok {
+		return c
+	}
+
+	cfg := p.baseTLSConfig.Clone()
+	cfg.ServerName = sni
+	t := http.DefaultTransport.(*http.Transport).Clone()
+	t.TLSClientConfig = cfg
+	c = &http.Client{Transport: t}
+	p.clients[sni] = c
+	return c
+}
+
 // buildHairpinURL composes the hairpin URL the broker uses to send the internal
 // initialize request back through the gateway. gatewayHost may be either a
 // bare host[:port] (in which case http:// is assumed for backwards
@@ -34,21 +83,17 @@ func buildHairpinURL(gatewayHost, mcpPath string) string {
 	return "http://" + gatewayHost + mcpPath
 }
 
-// Initialize will create a new initialize and initialized request and return the associated http client for connection management
-// This method makes a request back to the gateway setting the target mcp server to initialize. We hairpin through the gateway to ensure any Auth applied to that host is triggered for the call.
-// The initToken is a short-lived JWT bound to conf.Hostname that the router will validate when the hairpin request re-enters the gateway.
-func Initialize(ctx context.Context, gatewayHost, initToken string, conf *config.MCPServer, passThroughHeaders map[string]string, clientElicitation bool, hairpinHTTPClient *http.Client) (*client.Client, error) {
-	// force the initialize to hairpin back through envoy with a token that
-	// proves the request originated from the gateway's own router.
-	passThroughHeaders[mcprouter.RoutingKey] = initToken
-	passThroughHeaders["mcp-init-host"] = conf.Hostname
-
+// Initialize will create a new initialize and initialized request and return the associated http client for connection management.
+// This method makes a request back through the gateway to ensure any AuthPolicy is triggered.
+// The caller must set the routing key and mcp-init-host headers in passThroughHeaders before calling.
+func Initialize(ctx context.Context, gatewayHost string, conf *config.MCPServer, passThroughHeaders map[string]string, clientElicitation bool, hairpinClientPool *HairpinClientPool) (*client.Client, error) {
 	mcpPath, err := conf.Path()
 	if err != nil {
 		return nil, err
 	}
 
 	url := buildHairpinURL(gatewayHost, mcpPath)
+	hairpinHTTPClient := hairpinClientPool.Get(conf.Hostname)
 
 	httpClient, err := client.NewStreamableHttpClient(url,
 		transport.WithHTTPHeaders(passThroughHeaders),
@@ -80,41 +125,49 @@ func Initialize(ctx context.Context, gatewayHost, initToken string, conf *config
 	return httpClient, nil
 }
 
-// BuildHairpinHTTPClient returns an *http.Client for hairpin requests. For HTTPS
-// private hosts it configures TLS with ServerName set to publicHost so the
-// handshake verifies the cert SANs against the public hostname while the TCP
-// connection goes to the internal address. For plain HTTP it returns http.DefaultClient.
-func BuildHairpinHTTPClient(privateHost, publicHost, caCertPath string) (*http.Client, error) {
+// BuildHairpinHTTPClientPool returns a HairpinClientPool for hairpin requests.
+// For HTTPS private hosts it configures TLS with the publicHost as the default
+// ServerName (SNI). Servers on a different HTTPS listener can obtain a client
+// with a different SNI via pool.Get(serverHostname).
+// For plain HTTP it returns a pool whose default client has no TLS.
+func BuildHairpinHTTPClientPool(privateHost, publicHost, caCertPath string) (*HairpinClientPool, error) {
 	if !strings.HasPrefix(strings.ToLower(privateHost), "https://") {
-		return &http.Client{}, nil
+		return &HairpinClientPool{
+			defaultClient: &http.Client{},
+			clients:       make(map[string]*http.Client),
+		}, nil
 	}
 
-	pool, err := x509.SystemCertPool()
+	certPool, err := x509.SystemCertPool()
 	if err != nil {
-		pool = x509.NewCertPool()
+		certPool = x509.NewCertPool()
 	}
 
 	if caCertPath != "" {
 		pem, err := os.ReadFile(caCertPath) //nolint:gosec // path comes from a CLI flag, not user input
 		if err != nil {
 			return nil, fmt.Errorf("failed to read gateway CA cert: %w", err)
 		}
-		if !pool.AppendCertsFromPEM(pem) {
+		if !certPool.AppendCertsFromPEM(pem) {
 			return nil, fmt.Errorf("failed to parse gateway CA cert PEM")
 		}
 	}
 
-	// TLS ServerName must be a bare hostname, never host:port
-	serverName := publicHost
+	defaultSNI := publicHost
 	if h, _, err := net.SplitHostPort(publicHost); err == nil {
-		serverName = h
+		defaultSNI = h
 	}
 
-	t := http.DefaultTransport.(*http.Transport).Clone()
-	t.TLSClientConfig = &tls.Config{
+	baseCfg := &tls.Config{
 		MinVersion: tls.VersionTLS12,
-		RootCAs:    pool,
-		ServerName: serverName,
+		RootCAs:    certPool,
+		ServerName: defaultSNI,
 	}
-	return &http.Client{Transport: t}, nil
+	t := http.DefaultTransport.(*http.Transport).Clone()
+	t.TLSClientConfig = baseCfg
+	return &HairpinClientPool{
+		defaultClient: &http.Client{Transport: t},
+		baseTLSConfig: baseCfg,
+		clients:       make(map[string]*http.Client),
+	}, nil
 }

internal/clients/clients_test.go

@@ -5,6 +5,7 @@ package clients
 
 import (
 	"context"
+	"crypto/tls"
 	"net/http"
 	"os"
 	"path/filepath"
@@ -65,15 +66,13 @@ func TestInitialize(t *testing.T) {
 	testCases := []struct {
 		name               string
 		gatewayHost        string
-		routerKey          string
 		conf               *config.MCPServer
 		passThroughHeaders map[string]string
 		expectedError      bool
 	}{
 		{
 			name:        "standard initialization",
 			gatewayHost: "%invalid",
-			routerKey:   "router-key-123",
 			conf: &config.MCPServer{
 				Name:     "test-server",
 				Prefix:   "test_",
@@ -87,7 +86,11 @@ func TestInitialize(t *testing.T) {
 
 	for _, tc := range testCases {
 		t.Run(tc.name, func(t *testing.T) {
-			client, err := Initialize(context.Background(), tc.gatewayHost, tc.routerKey, tc.conf, tc.passThroughHeaders, false, nil)
+			pool := &HairpinClientPool{
+				defaultClient: &http.Client{},
+				clients:       make(map[string]*http.Client),
+			}
+			client, err := Initialize(context.Background(), tc.gatewayHost, tc.conf, tc.passThroughHeaders, false, pool)
 			if tc.expectedError {
 				require.Error(t, err)
 				return
@@ -99,42 +102,87 @@ func TestInitialize(t *testing.T) {
 	}
 }
 
-func TestBuildHairpinHTTPClient(t *testing.T) {
-	t.Run("returns plain client for HTTP private host", func(t *testing.T) {
-		c, err := BuildHairpinHTTPClient("http://gw.svc:8080", "mcp.example.com", "")
+func TestBuildHairpinHTTPClientPool(t *testing.T) {
+	t.Run("returns plain pool for HTTP private host", func(t *testing.T) {
+		pool, err := BuildHairpinHTTPClientPool("http://gw.svc:8080", "mcp.example.com", "")
 		require.NoError(t, err)
-		require.NotNil(t, c)
+		require.NotNil(t, pool)
+		require.Nil(t, pool.baseTLSConfig)
+		require.NotNil(t, pool.Get(""))
 	})
 
-	t.Run("returns plain client for bare host without scheme", func(t *testing.T) {
-		c, err := BuildHairpinHTTPClient("gw.svc:8080", "mcp.example.com", "")
+	t.Run("returns plain pool for bare host without scheme", func(t *testing.T) {
+		pool, err := BuildHairpinHTTPClientPool("gw.svc:8080", "mcp.example.com", "")
 		require.NoError(t, err)
-		require.NotNil(t, c)
+		require.NotNil(t, pool)
+		require.Nil(t, pool.baseTLSConfig)
 	})
 
-	t.Run("HTTPS sets ServerName and TLS minimum version", func(t *testing.T) {
-		c, err := BuildHairpinHTTPClient("https://gw.svc:443", "mcp.example.com", "")
+	t.Run("HTTPS sets default ServerName and TLS minimum version", func(t *testing.T) {
+		pool, err := BuildHairpinHTTPClientPool("https://gw.svc:443", "mcp.example.com", "")
 		require.NoError(t, err)
-		require.NotNil(t, c)
+		require.NotNil(t, pool)
 
+		c := pool.Get("")
 		tr, ok := c.Transport.(*http.Transport)
 		require.True(t, ok)
 		require.Equal(t, "mcp.example.com", tr.TLSClientConfig.ServerName)
-		require.Equal(t, uint16(0x0303), tr.TLSClientConfig.MinVersion) // tls.VersionTLS12
+		require.Equal(t, uint16(tls.VersionTLS12), tr.TLSClientConfig.MinVersion)
 	})
 
 	t.Run("HTTPS strips port from publicHost for ServerName", func(t *testing.T) {
-		c, err := BuildHairpinHTTPClient("https://gw.svc:443", "mcp.example.com:8443", "")
+		pool, err := BuildHairpinHTTPClientPool("https://gw.svc:443", "mcp.example.com:8443", "")
 		require.NoError(t, err)
-		require.NotNil(t, c)
 
+		c := pool.Get("")
 		tr, ok := c.Transport.(*http.Transport)
 		require.True(t, ok)
 		require.Equal(t, "mcp.example.com", tr.TLSClientConfig.ServerName)
 	})
 
+	t.Run("Get with override returns client with different SNI", func(t *testing.T) {
+		pool, err := BuildHairpinHTTPClientPool("https://gw.svc:443", "mcp.example.com", "")
+		require.NoError(t, err)
+
+		defaultClient := pool.Get("")
+		overrideClient := pool.Get("server.mcp-alt.local")
+		require.NotEqual(t, defaultClient, overrideClient)
+
+		tr, ok := overrideClient.Transport.(*http.Transport)
+		require.True(t, ok)
+		require.Equal(t, "server.mcp-alt.local", tr.TLSClientConfig.ServerName)
+	})
+
+	t.Run("Get with override strips port", func(t *testing.T) {
+		pool, err := BuildHairpinHTTPClientPool("https://gw.svc:443", "mcp.example.com", "")
+		require.NoError(t, err)
+
+		c := pool.Get("server.mcp-alt.local:8443")
+		tr, ok := c.Transport.(*http.Transport)
+		require.True(t, ok)
+		require.Equal(t, "server.mcp-alt.local", tr.TLSClientConfig.ServerName)
+	})
+
+	t.Run("Get with override is cached", func(t *testing.T) {
+		pool, err := BuildHairpinHTTPClientPool("https://gw.svc:443", "mcp.example.com", "")
+		require.NoError(t, err)
+
+		c1 := pool.Get("server.mcp-alt.local")
+		c2 := pool.Get("server.mcp-alt.local")
+		require.Same(t, c1, c2)
+	})
+
+	t.Run("Get with override on HTTP pool returns default", func(t *testing.T) {
+		pool, err := BuildHairpinHTTPClientPool("http://gw.svc:8080", "mcp.example.com", "")
+		require.NoError(t, err)
+
+		defaultClient := pool.Get("")
+		overrideClient := pool.Get("server.mcp-alt.local")
+		require.Same(t, defaultClient, overrideClient)
+	})
+
 	t.Run("errors on non-existent CA cert path", func(t *testing.T) {
-		_, err := BuildHairpinHTTPClient("https://gw.svc:443", "mcp.example.com", "/nonexistent/ca.crt")
+		_, err := BuildHairpinHTTPClientPool("https://gw.svc:443", "mcp.example.com", "/nonexistent/ca.crt")
 		require.Error(t, err)
 		require.Contains(t, err.Error(), "failed to read gateway CA cert")
 	})
@@ -144,7 +192,7 @@ func TestBuildHairpinHTTPClient(t *testing.T) {
 		badCert := filepath.Join(tmpDir, "bad.crt")
 		require.NoError(t, os.WriteFile(badCert, []byte("not a certificate"), 0600))
 
-		_, err := BuildHairpinHTTPClient("https://gw.svc:443", "mcp.example.com", badCert)
+		_, err := BuildHairpinHTTPClientPool("https://gw.svc:443", "mcp.example.com", badCert)
 		require.Error(t, err)
 		require.Contains(t, err.Error(), "failed to parse gateway CA cert PEM")
 	})

internal/mcp-router/request_handlers.go

@@ -696,11 +696,10 @@ func (s *ExtProcServer) initializeMCPSeverSession(ctx context.Context, mcpReq *M
 		if mcpReq.Headers != nil {
 			// We don't want to pass through any pseudo routing headers (:authority,
 			// :path, etc.), the gateway-bound mcp-session-id, or the router-internal
-			// headers (mcp-init-host, router-key) which we set ourselves below via
-			// clients.Initialize. Dropping the router-internal headers here is
-			// defense-in-depth so a client-supplied value can never reach the
-			// hairpin request even if the override in clients.Initialize is later
-			// refactored. Everything else is passed through for custom headers.
+			// headers (mcp-init-host, router-key) which we set ourselves below.
+			// Dropping the router-internal headers here is defense-in-depth so a
+			// client-supplied value can never reach the hairpin request.
+			// Everything else is passed through for custom headers.
 			for _, h := range mcpReq.Headers.Headers {
 				key := strings.ToLower(h.Key)
 				if strings.HasPrefix(key, ":") ||
@@ -753,7 +752,9 @@ func (s *ExtProcServer) initializeMCPSeverSession(ctx context.Context, mcpReq *M
 			mcpotel.SpanError(initSpan, err, "failed to generate backend-init token")
 			return "", NewRouterErrorf(500, "failed to generate backend-init token: %w", err)
 		}
-		clientHandle, err := s.InitForClient(ctx, s.RoutingConfig.MCPGatewayInternalHostname, initToken, mcpServerConfig, passThroughHeaders, mcpReq.clientElicitation, s.HairpinHTTPClient)
+		passThroughHeaders[RoutingKey] = initToken
+		passThroughHeaders["mcp-init-host"] = mcpServerConfig.Hostname
+		clientHandle, err := s.InitForClient(ctx, s.RoutingConfig.MCPGatewayInternalHostname, mcpServerConfig, passThroughHeaders, mcpReq.clientElicitation, s.HairpinClientPool)
 		if err != nil {
 			s.Logger.ErrorContext(ctx, "failed to get remote session ", "error", err)
 			mcpotel.SpanError(initSpan, err, "failed to initialize backend session")