Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 5 additions & 6 deletions internal/adapter/proxy/core/common.go
Original file line number Diff line number Diff line change
Expand Up @@ -61,12 +61,11 @@ func CopyHeaders(proxyReq, originalReq *http.Request) {
proxyReq.Header[header] = values
}

// SCOUT-581: Host header missing for some requests
// preserve the original host header which is critical for virtual hosting
// many backend services rely on this to route requests correctly
if originalReq.Host != "" {
proxyReq.Host = originalReq.Host
}
// Deprecates SCOUT-581, superseded by OLLA-135:
// https://github.com/thushan/olla/issues/135
// Do not propagate the inbound Host onto the outbound request.
// req.URL.Host is authoritative for the backend; X-Forwarded-Host (set below) carries
// the original value for backends that need it.

// Add proxy identification headers
proxyReq.Header.Set(constants.HeaderXProxiedBy, GetProxiedByHeader())
Expand Down
23 changes: 23 additions & 0 deletions internal/adapter/proxy/core/common_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -776,6 +776,29 @@ func TestSetStickySessionHeaders(t *testing.T) {
}
}

// TestCopyHeaders_DoesNotPropagateInboundHost verifies that the inbound client Host is never
// forwarded to the backend. Go's transport uses req.URL.Host when req.Host is empty, which is
// the correct backend address. Propagating the inbound Host breaks HTTPS backends behind strict
// nginx server_name rules and is a Host-header injection vector.
func TestCopyHeaders_DoesNotPropagateInboundHost(t *testing.T) {
t.Parallel()

originalReq := httptest.NewRequest("POST", "http://olla.example.com/api/generate", nil)
originalReq.Host = "olla.example.com"

proxyReq := httptest.NewRequest("POST", "http://backend.internal:11434/api/generate", nil)
proxyReq.Host = "" // Transport will use URL.Host when this is empty

CopyHeaders(proxyReq, originalReq)

// Host must remain unset so Go's transport derives it from URL.Host (the backend address).
assert.Empty(t, proxyReq.Host, "outbound Host must not be overridden with the inbound client Host")

// The original host must still be available to backends via X-Forwarded-Host.
assert.Equal(t, "olla.example.com", proxyReq.Header.Get("X-Forwarded-Host"),
"X-Forwarded-Host must carry the original inbound Host")
}

// BenchmarkSetResponseHeaders benchmarks the SetResponseHeaders function
func BenchmarkSetResponseHeaders(b *testing.B) {
stats := &ports.RequestStats{
Expand Down
190 changes: 190 additions & 0 deletions internal/adapter/proxy/proxy_headers_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import (
"net/http"
"net/http/httptest"
"testing"
"time"

"github.com/stretchr/testify/assert"
"github.com/thushan/olla/internal/adapter/proxy/olla"
Expand Down Expand Up @@ -143,3 +144,192 @@ func TestProxyResponseHeaders_NoOverride(t *testing.T) {
assert.NotEmpty(t, w.Header().Get(constants.HeaderXServedBy))
assert.Contains(t, w.Header().Get(constants.HeaderXServedBy), "Olla/")
}

// hostHeaderSuites returns both proxy engines paired with a constructor so tests
// can iterate over both without duplicating setup.
func hostHeaderSuites() []struct {
name string
createProxy func(upstream *httptest.Server) ports.ProxyService
} {
make := func(name string, fn func(upstream *httptest.Server) ports.ProxyService) struct {
name string
createProxy func(upstream *httptest.Server) ports.ProxyService
} {
return struct {
name string
createProxy func(upstream *httptest.Server) ports.ProxyService
}{name, fn}
}

return []struct {
name string
createProxy func(upstream *httptest.Server) ports.ProxyService
}{
make("Sherpa", func(upstream *httptest.Server) ports.ProxyService {
ep := createTestEndpoint("be", upstream.URL, domain.StatusHealthy)
disc := &mockDiscoveryService{endpoints: []*domain.Endpoint{ep}}
sel := &mockEndpointSelector{endpoint: ep}
cfg := &sherpa.Configuration{} // zero value is fine for unit tests
svc, _ := sherpa.NewService(disc, sel, cfg, createTestStatsCollector(), nil, createTestLogger())
return svc
}),
make("Olla", func(upstream *httptest.Server) ports.ProxyService {
ep := createTestEndpoint("be", upstream.URL, domain.StatusHealthy)
disc := &mockDiscoveryService{endpoints: []*domain.Endpoint{ep}}
sel := &mockEndpointSelector{endpoint: ep}
cfg := &olla.Configuration{}
svc, _ := olla.NewService(disc, sel, cfg, createTestStatsCollector(), nil, createTestLogger())
return svc
}),
}
}

// TestProxy_OutboundHostMatchesBackend asserts that the Host seen by the backend
// is its own address, not the inbound client Host. Regression for issue #135.
func TestProxy_OutboundHostMatchesBackend(t *testing.T) {
t.Parallel()

for _, tc := range hostHeaderSuites() {
t.Run(tc.name, func(t *testing.T) {
t.Parallel()

var capturedHost string
upstream := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
capturedHost = r.Host
w.WriteHeader(http.StatusOK)
}))
defer upstream.Close()

proxy := tc.createProxy(upstream)

req := httptest.NewRequest("GET", "/test", nil)
req.Host = "olla.example.com" // inbound client host — must NOT reach the backend

stats := &ports.RequestStats{RequestID: "host-test", StartTime: time.Now()}
w := httptest.NewRecorder()
_ = proxy.ProxyRequest(req.Context(), w, req, stats, createTestLogger())

// The backend's listener address is the authoritative host.
assert.Equal(t, upstream.Listener.Addr().String(), capturedHost,
"backend must receive its own address as Host, not the inbound client Host")
Comment thread
thushan marked this conversation as resolved.
})
}
}

// TestProxy_XForwardedHostPreservesInbound asserts that backends can still recover the
// original client Host via X-Forwarded-Host even though we don't propagate it as Host.
func TestProxy_XForwardedHostPreservesInbound(t *testing.T) {
t.Parallel()

for _, tc := range hostHeaderSuites() {
t.Run(tc.name, func(t *testing.T) {
t.Parallel()

var capturedXFH string
upstream := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
capturedXFH = r.Header.Get("X-Forwarded-Host")
w.WriteHeader(http.StatusOK)
}))
defer upstream.Close()

proxy := tc.createProxy(upstream)

req := httptest.NewRequest("GET", "/test", nil)
req.Host = "olla.example.com"

stats := &ports.RequestStats{RequestID: "xfh-test", StartTime: time.Now()}
w := httptest.NewRecorder()
_ = proxy.ProxyRequest(req.Context(), w, req, stats, createTestLogger())

assert.Equal(t, "olla.example.com", capturedXFH,
"X-Forwarded-Host must carry the original inbound Host for backends that need it")
})
}
}

// TestProxy_HostHeaderInjectionIsNeutralised is the security regression for issue #135.
// An attacker supplying a crafted Host must not influence which virtual host the backend serves.
func TestProxy_HostHeaderInjectionIsNeutralised(t *testing.T) {
t.Parallel()

for _, tc := range hostHeaderSuites() {
t.Run(tc.name, func(t *testing.T) {
t.Parallel()

var capturedHost string
upstream := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
capturedHost = r.Host
w.WriteHeader(http.StatusOK)
}))
defer upstream.Close()

proxy := tc.createProxy(upstream)

req := httptest.NewRequest("GET", "/test", nil)
req.Host = "attacker.evil.com" // injected host — must be neutralised

stats := &ports.RequestStats{RequestID: "inject-test", StartTime: time.Now()}
w := httptest.NewRecorder()
_ = proxy.ProxyRequest(req.Context(), w, req, stats, createTestLogger())

assert.NotEqual(t, "attacker.evil.com", capturedHost,
"injected Host must not reach the backend")
assert.Equal(t, upstream.Listener.Addr().String(), capturedHost,
"backend must receive its own address as Host regardless of inbound Host value")
})
}
}

// TestProxy_MultipleBackends_HostIsPerBackend verifies that when the selector routes to
// different backends, each request carries the correct backend's own Host — not a host
// value leaked from a prior request or the inbound client.
//
// Note: the mock selector always returns the pre-configured endpoint, so we use two separate
// proxy instances (one per backend) to simulate per-backend routing rather than within-proxy
// round-robin, which requires a real balancer. The invariant being tested — that URL.Host
// drives the outbound Host — is identical regardless of how the endpoint was selected.
func TestProxy_MultipleBackends_HostIsPerBackend(t *testing.T) {
t.Parallel()

var capturedHost1, capturedHost2 string

upstream1 := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
capturedHost1 = r.Host
w.WriteHeader(http.StatusOK)
}))
defer upstream1.Close()

upstream2 := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
capturedHost2 = r.Host
w.WriteHeader(http.StatusOK)
}))
defer upstream2.Close()

suites := hostHeaderSuites()
proxy1 := suites[0].createProxy(upstream1) // Sherpa → upstream1
proxy2 := suites[0].createProxy(upstream2) // Sherpa → upstream2

inboundHost := "olla.example.com"

req1 := httptest.NewRequest("GET", "/test", nil)
req1.Host = inboundHost
_ = proxy1.ProxyRequest(req1.Context(), httptest.NewRecorder(), req1,
&ports.RequestStats{RequestID: "mb1", StartTime: time.Now()}, createTestLogger())

req2 := httptest.NewRequest("GET", "/test", nil)
req2.Host = inboundHost
_ = proxy2.ProxyRequest(req2.Context(), httptest.NewRecorder(), req2,
&ports.RequestStats{RequestID: "mb2", StartTime: time.Now()}, createTestLogger())

assert.Equal(t, upstream1.Listener.Addr().String(), capturedHost1,
"backend 1 must receive its own address as Host")
assert.Equal(t, upstream2.Listener.Addr().String(), capturedHost2,
"backend 2 must receive its own address as Host")
assert.NotEqual(t, capturedHost1, capturedHost2,
"each backend must see a distinct, correct Host")

// HTTP/2: Go's httptest.NewServer is HTTP/1. The h2 :authority pseudo-header is derived
// from the same req.Host field on the server side, so the HTTP/1 assertions above are a
// sufficient proxy — the plumbing is identical. An h2 variant would require
// httptest.NewUnstartedServer + TLS, which adds noise without extra coverage.
}
Loading