feat: flag to disable hop-by-hop defenses (#1120)

Ory Oathkeeper v0.44.4 uses the new Rewrite feature of Golang's reverse proxy. This will strip any X-Forwarded headers from upstream requests. This however is not always desirable which is why a new config flag `serve.proxy.trust_forwarded_headers` was introduced to optionally enable the forwarding of X-Forwarded headers.

driver/configuration/config_keys.go

@@ -11,6 +11,7 @@ const (
 	ProxyIdleTimeout                    Key = "serve.proxy.timeout.idle"
 	ProxyServeAddressHost               Key = "serve.proxy.host"
 	ProxyServeAddressPort               Key = "serve.proxy.port"
+	ProxyTrustForwardedHeaders          Key = "serve.proxy.trust_forwarded_headers"
 	APIServeAddressHost                 Key = "serve.api.host"
 	APIServeAddressPort                 Key = "serve.api.port"
 	APIReadTimeout                      Key = "serve.api.timeout.read"

driver/configuration/provider.go

@@ -43,6 +43,8 @@ type Provider interface {
 	CORSOptions(iface string) cors.Options
 	CORS(iface string) (cors.Options, bool)
 
+	ProxyTrustForwardedHeaders() bool
+
 	ProviderAuthenticators
 	ProviderErrorHandlers
 	ProviderAuthorizers

driver/configuration/provider_koanf.go

@@ -384,6 +384,10 @@ func (v *KoanfProvider) AuthenticatorIsEnabled(id string) bool {
 	return v.pipelineIsEnabled("authenticators", id)
 }
 
+func (v *KoanfProvider) ProxyTrustForwardedHeaders() bool {
+	return v.source.Bool(ProxyTrustForwardedHeaders)
+}
+
 func (v *KoanfProvider) AuthenticatorConfig(id string, override json.RawMessage, dest interface{}) error {
 	return v.PipelineConfig("authenticators", id, override, dest)
 }

driver/registry_memory.go

@@ -326,7 +326,7 @@ func (r *RegistryMemory) AvailablePipelineMutators() (available []string) {
 
 func (r *RegistryMemory) Proxy() *proxy.Proxy {
 	if r.proxyProxy == nil {
-		r.proxyProxy = proxy.NewProxy(r)
+		r.proxyProxy = proxy.NewProxy(r, r.c)
 	}
 
 	return r.proxyProxy

proxy/proxy.go

@@ -11,6 +11,8 @@ import (
 	"net/url"
 	"strings"
 
+	"github.com/ory/oathkeeper/driver/configuration"
+
 	"github.com/ory/oathkeeper/pipeline/authn"
 	"github.com/ory/oathkeeper/x"
 
@@ -22,17 +24,17 @@ import (
 type proxyRegistry interface {
 	x.RegistryLogger
 	x.RegistryWriter
-
 	ProxyRequestHandler() RequestHandler
 	RuleMatcher() rule.Matcher
 }
 
-func NewProxy(r proxyRegistry) *Proxy {
-	return &Proxy{r: r}
+func NewProxy(r proxyRegistry, c configuration.Provider) *Proxy {
+	return &Proxy{r: r, c: c}
 }
 
 type Proxy struct {
 	r proxyRegistry
+	c configuration.Provider
 }
 
 type key int
@@ -107,6 +109,10 @@ func (d *Proxy) RoundTrip(r *http.Request) (*http.Response, error) {
 }
 
 func (d *Proxy) Rewrite(r *httputil.ProxyRequest) {
+	if d.c.ProxyTrustForwardedHeaders() {
+		r.SetXForwarded()
+	}
+
 	EnrichRequestedURL(r)
 	rl, err := d.r.RuleMatcher().Match(r.Out.Context(), r.Out.Method, r.Out.URL, rule.ProtocolHTTP)
 	if err != nil {

proxy/proxy_test.go

@@ -93,10 +93,12 @@ func TestProxy(t *testing.T) {
 		url         string
 		code        int
 		messages    []string
+		messagesNot []string
 		rulesRegexp []rule.Rule
 		rulesGlob   []rule.Rule
 		transform   func(r *http.Request)
 		d           string
+		prep        func(t *testing.T)
 	}{
 		{
 			d:           "should fail because url does not exist in rule set",
@@ -201,6 +203,62 @@ func TestProxy(t *testing.T) {
 				"host=" + x.ParseURLOrPanic(backend.URL).Host,
 			},
 		},
+		{
+			d: "should pass and set x-forwarded headers",
+			prep: func(t *testing.T) {
+				conf.SetForTest(t, configuration.ProxyTrustForwardedHeaders, true)
+			},
+			transform: func(r *http.Request) {
+				r.Header.Set("X-Forwarded-For", "foobar.com")
+			},
+			url: ts.URL + "/authn-anon/authz-allow/cred-noop/1234",
+			rulesRegexp: []rule.Rule{{
+				Match:          &rule.Match{Methods: []string{"GET"}, URL: ts.URL + "/authn-anon/authz-allow/cred-noop/<[0-9]+>"},
+				Authenticators: []rule.Handler{{Handler: "anonymous"}},
+				Authorizer:     rule.Handler{Handler: "allow"},
+				Mutators:       []rule.Handler{{Handler: "noop"}},
+				Upstream:       rule.Upstream{URL: backend.URL},
+			}},
+			rulesGlob: []rule.Rule{{
+				Match:          &rule.Match{Methods: []string{"GET"}, URL: ts.URL + "/authn-anon/authz-allow/cred-noop/<[0-9]*>"},
+				Authenticators: []rule.Handler{{Handler: "anonymous"}},
+				Authorizer:     rule.Handler{Handler: "allow"},
+				Mutators:       []rule.Handler{{Handler: "noop"}},
+				Upstream:       rule.Upstream{URL: backend.URL},
+			}},
+			code: http.StatusOK,
+			messages: []string{
+				"authorization=",
+				"url=/authn-anon/authz-allow/cred-noop/1234",
+				"host=" + x.ParseURLOrPanic(backend.URL).Host,
+				"header X-Forwarded-Proto=http",
+			},
+		},
+		{
+			d: "should pass and remove x-forwarded headers",
+			transform: func(r *http.Request) {
+				r.Header.Set("X-Forwarded-For", "foobar.com")
+			},
+			url: ts.URL + "/authn-anon/authz-allow/cred-noop/1234",
+			rulesRegexp: []rule.Rule{{
+				Match:          &rule.Match{Methods: []string{"GET"}, URL: ts.URL + "/authn-anon/authz-allow/cred-noop/<[0-9]+>"},
+				Authenticators: []rule.Handler{{Handler: "anonymous"}},
+				Authorizer:     rule.Handler{Handler: "allow"},
+				Mutators:       []rule.Handler{{Handler: "noop"}},
+				Upstream:       rule.Upstream{URL: backend.URL},
+			}},
+			rulesGlob: []rule.Rule{{
+				Match:          &rule.Match{Methods: []string{"GET"}, URL: ts.URL + "/authn-anon/authz-allow/cred-noop/<[0-9]*>"},
+				Authenticators: []rule.Handler{{Handler: "anonymous"}},
+				Authorizer:     rule.Handler{Handler: "allow"},
+				Mutators:       []rule.Handler{{Handler: "noop"}},
+				Upstream:       rule.Upstream{URL: backend.URL},
+			}},
+			code: http.StatusOK,
+			messagesNot: []string{
+				"header X-Forwarded-Proto=http",
+			},
+		},
 		{
 			d:   "should fail when authorizer fails",
 			url: ts.URL + "/authn-anon/authz-deny/cred-noop/1234",
@@ -339,6 +397,10 @@ func TestProxy(t *testing.T) {
 	} {
 		t.Run(fmt.Sprintf("description=%s", tc.d), func(t *testing.T) {
 			testFunc := func(t *testing.T, strategy configuration.MatchingStrategy, rules []rule.Rule) {
+				if tc.prep != nil {
+					tc.prep(t)
+				}
+
 				reg.RuleRepository().(*rule.RepositoryMemory).WithRules(rules)
 				require.NoError(t, reg.RuleRepository().SetMatchingStrategy(context.Background(), strategy))
 
@@ -361,6 +423,13 @@ func TestProxy(t *testing.T) {
 %s
 proxy_url=%s
 backend_url=%s
+`, m, greeting, ts.URL, backend.URL)
+				}
+				for _, m := range tc.messagesNot {
+					assert.False(t, strings.Contains(string(greeting), m), `Value "%s" found in message but not expected:
+%s
+proxy_url=%s
+backend_url=%s
 `, m, greeting, ts.URL, backend.URL)
 				}
 

spec/config.schema.json

@@ -1176,6 +1176,12 @@
               "title": "Host",
               "description": "The network interface to listen on. Leave empty to listen on all interfaces."
             },
+            "trust_forwarded_headers": {
+              "type": "boolean",
+              "default": false,
+              "title": "Trust X-Forwarded Headers",
+              "description": "Trust the X-Forwarded-* headers from the reverse proxy. This is useful when running behind a load balancer or similar. Set this to false if you are not running behind a reverse proxy that prevents Hop-by-Hop attacks."
+            },
             "timeout": {
               "$ref": "#/definitions/serverTimeout"
             },