diff --git a/cmd/cloudx/proxy/helpers.go b/cmd/cloudx/proxy/helpers.go index 8f2b4951..79e4824f 100644 --- a/cmd/cloudx/proxy/helpers.go +++ b/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(), diff --git a/cmd/cloudx/proxy/helpers_test.go b/cmd/cloudx/proxy/helpers_test.go new file mode 100644 index 00000000..8a694ea2 --- /dev/null +++ b/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") + }) +}