fix: scope Ory-Base-URL-Rewrite* headers to Ory-bound requests

cmd/cloudx/proxy/helpers.go

@@ -194,28 +194,7 @@ func runReverseProxy(ctx context.Context, h *client.CommandHelper, stdErr io.Wri
 				PathPrefix:     "",
 			}, nil
 		},
-		proxy.WithReqMiddleware(func(r *httputil.ProxyRequest, c *proxy.HostConfig, body []byte) ([]byte, error) {
-			if r.Out.URL.Host == oryURL.Host {
-				r.Out.URL.Path = strings.TrimPrefix(r.Out.URL.Path, conf.pathPrefix)
-				r.Out.Host = oryURL.Host
-			} else if conf.rewriteHost {
-				r.Out.Header.Set("X-Forwarded-Host", r.In.Host)
-				r.Out.Host = c.UpstreamHost
-			}
-
-			publicURL := conf.publicURL
-			if conf.pathPrefix != "" {
-				publicURL = urlx.AppendPaths(publicURL, conf.pathPrefix)
-			}
-
-			r.Out.Header.Set("Ory-No-Custom-Domain-Redirect", "true")
-			r.Out.Header.Set("Ory-Base-URL-Rewrite", publicURL.String())
-			if len(apiKey) > 0 {
-				r.Out.Header.Set("Ory-Base-URL-Rewrite-Token", apiKey)
-			}
-
-			return body, nil
-		}),
+		proxy.WithReqMiddleware(reqMiddleware(conf, oryURL, apiKey)),
 		proxy.WithRespMiddleware(func(resp *http.Response, config *proxy.HostConfig, body []byte) ([]byte, error) {
 			l, err := resp.Location()
 			if err == nil {
@@ -313,6 +292,44 @@ and configure your SDKs to point to it, for example in JavaScript:
 	return nil
 }
 
+// reqMiddleware returns the request middleware used by the reverse proxy. The
+// Ory-* headers (including the temporary API key in Ory-Base-URL-Rewrite-Token)
+// are only attached to Ory-bound requests. Requests forwarded to the developer's
+// upstream application do not need — and must not receive — these headers.
+func reqMiddleware(conf *config, oryURL *url.URL, apiKey string) proxy.ReqMiddleware {
+	return func(r *httputil.ProxyRequest, c *proxy.HostConfig, body []byte) ([]byte, error) {
+		// Strip any client-supplied Ory-* headers before selectively re-applying
+		// them below. Otherwise a client could spoof these headers: they would be
+		// forwarded unchanged to the developer's upstream app, or — when apiKey is
+		// empty — an attacker-supplied Ory-Base-URL-Rewrite-Token would be passed
+		// through to Ory.
+		r.Out.Header.Del("Ory-No-Custom-Domain-Redirect")
+		r.Out.Header.Del("Ory-Base-URL-Rewrite")
+		r.Out.Header.Del("Ory-Base-URL-Rewrite-Token")
+
+		if r.Out.URL.Host == oryURL.Host {
+			r.Out.URL.Path = strings.TrimPrefix(r.Out.URL.Path, conf.pathPrefix)
+			r.Out.Host = oryURL.Host
+
+			publicURL := conf.publicURL
+			if conf.pathPrefix != "" {
+				publicURL = urlx.AppendPaths(publicURL, conf.pathPrefix)
+			}
+
+			r.Out.Header.Set("Ory-No-Custom-Domain-Redirect", "true")
+			r.Out.Header.Set("Ory-Base-URL-Rewrite", publicURL.String())
+			if len(apiKey) > 0 {
+				r.Out.Header.Set("Ory-Base-URL-Rewrite-Token", apiKey)
+			}
+		} else if conf.rewriteHost {
+			r.Out.Header.Set("X-Forwarded-Host", r.In.Host)
+			r.Out.Host = c.UpstreamHost
+		}
+
+		return body, nil
+	}
+}
+
 func newJWTSigner() (jose.Signer, *jose.JSONWebKeySet, error) {
 	key, err := jwksx.GenerateSigningKeys(
 		uuid.Must(uuid.NewV4()).String(),

cmd/cloudx/proxy/helpers_test.go

@@ -0,0 +1,110 @@
+// Copyright © 2023 Ory Corp
+// SPDX-License-Identifier: Apache-2.0
+
+package proxy
+
+import (
+	"net/http"
+	"net/http/httputil"
+	"net/url"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+
+	"github.com/ory/x/proxy"
+)
+
+func TestReqMiddleware(t *testing.T) {
+	oryURL := &url.URL{Scheme: "https", Host: "example.projects.oryapis.com"}
+	publicURL := &url.URL{Scheme: "http", Host: "localhost:4000"}
+	const apiKey = "ory_apikey_sentinel"
+
+	const (
+		headerToken    = "Ory-Base-URL-Rewrite-Token"
+		headerRewrite  = "Ory-Base-URL-Rewrite"
+		headerNoCustom = "Ory-No-Custom-Domain-Redirect"
+	)
+
+	newRequest := func(t *testing.T, outHost string) *httputil.ProxyRequest {
+		t.Helper()
+		in, err := http.NewRequest(http.MethodGet, "http://localhost:4000/foo", nil)
+		require.NoError(t, err)
+		out, err := http.NewRequest(http.MethodGet, "http://"+outHost+"/foo", nil)
+		require.NoError(t, err)
+		return &httputil.ProxyRequest{In: in, Out: out}
+	}
+
+	t.Run("case=ory-bound request receives Ory headers", func(t *testing.T) {
+		conf := &config{publicURL: publicURL}
+		r := newRequest(t, oryURL.Host)
+
+		_, err := reqMiddleware(conf, oryURL, apiKey)(r, &proxy.HostConfig{}, nil)
+		require.NoError(t, err)
+
+		assert.Equal(t, apiKey, r.Out.Header.Get(headerToken))
+		assert.Equal(t, publicURL.String(), r.Out.Header.Get(headerRewrite))
+		assert.Equal(t, "true", r.Out.Header.Get(headerNoCustom))
+	})
+
+	t.Run("case=upstream-bound request does not receive Ory headers", func(t *testing.T) {
+		conf := &config{publicURL: publicURL}
+		r := newRequest(t, "localhost:3000")
+
+		_, err := reqMiddleware(conf, oryURL, apiKey)(r, &proxy.HostConfig{}, nil)
+		require.NoError(t, err)
+
+		assert.Empty(t, r.Out.Header.Get(headerToken), "API key must not leak to the upstream app")
+		assert.Empty(t, r.Out.Header.Get(headerRewrite))
+		assert.Empty(t, r.Out.Header.Get(headerNoCustom))
+	})
+
+	t.Run("case=rewriteHost upstream sets X-Forwarded-Host but not the API key", func(t *testing.T) {
+		conf := &config{publicURL: publicURL, rewriteHost: true}
+		r := newRequest(t, "localhost:3000")
+
+		_, err := reqMiddleware(conf, oryURL, apiKey)(r, &proxy.HostConfig{UpstreamHost: "upstream.internal"}, nil)
+		require.NoError(t, err)
+
+		assert.Equal(t, r.In.Host, r.Out.Header.Get("X-Forwarded-Host"))
+		assert.Equal(t, "upstream.internal", r.Out.Host)
+		assert.Empty(t, r.Out.Header.Get(headerToken), "API key must not leak to the upstream app")
+	})
+
+	t.Run("case=client-spoofed Ory headers are stripped from upstream-bound requests", func(t *testing.T) {
+		conf := &config{publicURL: publicURL}
+		r := newRequest(t, "localhost:3000")
+		r.Out.Header.Set(headerToken, "ory_apikey_spoofed")
+		r.Out.Header.Set(headerRewrite, "http://evil.example")
+		r.Out.Header.Set(headerNoCustom, "true")
+
+		_, err := reqMiddleware(conf, oryURL, apiKey)(r, &proxy.HostConfig{}, nil)
+		require.NoError(t, err)
+
+		assert.Empty(t, r.Out.Header.Get(headerToken), "spoofed token must not be forwarded to the upstream app")
+		assert.Empty(t, r.Out.Header.Get(headerRewrite), "spoofed header must not be forwarded to the upstream app")
+		assert.Empty(t, r.Out.Header.Get(headerNoCustom), "spoofed header must not be forwarded to the upstream app")
+	})
+
+	t.Run("case=client-spoofed token is stripped from Ory-bound requests when apiKey is empty", func(t *testing.T) {
+		conf := &config{publicURL: publicURL}
+		r := newRequest(t, oryURL.Host)
+		r.Out.Header.Set(headerToken, "ory_apikey_spoofed")
+
+		_, err := reqMiddleware(conf, oryURL, "")(r, &proxy.HostConfig{}, nil)
+		require.NoError(t, err)
+
+		assert.Empty(t, r.Out.Header.Get(headerToken), "spoofed token must not be passed through to Ory when apiKey is empty")
+	})
+
+	t.Run("case=real apiKey overwrites a client-spoofed token on Ory-bound requests", func(t *testing.T) {
+		conf := &config{publicURL: publicURL}
+		r := newRequest(t, oryURL.Host)
+		r.Out.Header.Set(headerToken, "ory_apikey_spoofed")
+
+		_, err := reqMiddleware(conf, oryURL, apiKey)(r, &proxy.HostConfig{}, nil)
+		require.NoError(t, err)
+
+		assert.Equal(t, apiKey, r.Out.Header.Get(headerToken), "genuine key must overwrite any spoofed token")
+	})
+}