fwd-auth: fix nginx-ingress forward-auth (#1505 / #1497)

Signed-off-by: Bobby DeSimone <bobbydesimone@gmail.com>
2025-04-29 10:26:29 +02:00 · 2020-10-19 08:09:13 -07:00 · 2020-10-19 08:09:13 -07:00 · aadbcd23bd
commit aadbcd23bd
parent c85b45cff6
8 changed files with 257 additions and 154 deletions
--- a/.devcontainer/envs/nginx.yaml
+++ b/.devcontainer/envs/nginx.yaml
@ -10,8 +10,6 @@ services:
      - ../../examples/nginx/httpbin.conf:/etc/nginx/conf.d/httpbin.conf
      - ../../examples/nginx/pomerium.conf:/etc/nginx/conf.d/pomerium.conf
      - ../../examples/nginx/proxy.conf:/etc/nginx/proxy.conf
-      - ../../examples/nginx/auth.conf:/etc/nginx/auth.conf
-      - ../../examples/nginx/ext_authz.conf:/etc/nginx/ext_authz.conf
      - ../../examples/nginx/_wildcard.localhost.pomerium.io.pem:/etc/nginx/nginx.pem
      - ../../examples/nginx/_wildcard.localhost.pomerium.io-key.pem:/etc/nginx/nginx-key.pem

--- a/authorize/grpc.go
+++ b/authorize/grpc.go
@ -39,9 +39,21 @@ func (a *Authorize) Check(ctx context.Context, in *envoy_service_auth_v2.CheckRe

 	state := a.state.Load()

-	// maybe rewrite http request for forward auth
-	isForwardAuth := a.handleForwardAuth(in)
+	// convert the incoming envoy-style http request into a go-style http request
 	hreq := getHTTPRequestFromCheckRequest(in)
+
+	isForwardAuth := a.isForwardAuth(in)
+	if isForwardAuth {
+		// update the incoming http request's uri to match the forwarded URI
+		fwdAuthURI := getForwardAuthURL(hreq)
+		in.Attributes.Request.Http.Scheme = fwdAuthURI.Scheme
+		in.Attributes.Request.Http.Host = fwdAuthURI.Host
+		in.Attributes.Request.Http.Path = fwdAuthURI.Path
+		if fwdAuthURI.RawQuery != "" {
+			in.Attributes.Request.Http.Path += "?" + fwdAuthURI.RawQuery
+		}
+	}
+
 	rawJWT, _ := loadRawSession(hreq, a.currentOptions.Load(), state.encoder)
 	sessionState, _ := loadSession(state.encoder, rawJWT)

@ -65,7 +77,7 @@ func (a *Authorize) Check(ctx context.Context, in *envoy_service_auth_v2.CheckRe
 	case reply.Status == http.StatusOK:
 		return a.okResponse(reply), nil
 	case reply.Status == http.StatusUnauthorized:
-		if isForwardAuth {
+		if isForwardAuth && hreq.URL.Path == "/verify" {
 			return a.deniedResponse(in, http.StatusUnauthorized, "Unauthenticated", nil), nil
 		}
 		return a.redirectResponse(in), nil
@ -172,7 +184,23 @@ func (a *Authorize) getEnvoyRequestHeaders(signedJWT string) ([]*envoy_api_v2_co
 	return hvos, nil
 }

-func (a *Authorize) handleForwardAuth(req *envoy_service_auth_v2.CheckRequest) bool {
+func getForwardAuthURL(r *http.Request) *url.URL {
+	urqQuery := r.URL.Query().Get("uri")
+	u, _ := urlutil.ParseAndValidateURL(urqQuery)
+	if u == nil {
+		u = &url.URL{
+			Scheme: r.Header.Get(httputil.HeaderForwardedProto),
+			Host:   r.Header.Get(httputil.HeaderForwardedHost),
+			Path:   r.Header.Get(httputil.HeaderForwardedURI),
+		}
+	}
+	// todo(bdd): handle httputil.HeaderOriginalURL which incorporates
+	// 			  path and query params
+	return u
+}
+
+// isForwardAuth returns if the current request is a forward auth route.
+func (a *Authorize) isForwardAuth(req *envoy_service_auth_v2.CheckRequest) bool {
 	opts := a.currentOptions.Load()

 	if opts.ForwardAuthURL == nil {
@ -180,37 +208,8 @@ func (a *Authorize) handleForwardAuth(req *envoy_service_auth_v2.CheckRequest) b
 	}

 	checkURL := getCheckRequestURL(req)
-	if urlutil.StripPort(checkURL.Host) != urlutil.StripPort(opts.GetForwardAuthURL().Host) {
-		return false
-	}

-	uriQuery := checkURL.Query().Get("uri")
-	if headers := req.GetAttributes().GetRequest().GetHttp().GetHeaders(); uriQuery == "" && headers != nil {
-		uriQuery = headers[http.CanonicalHeaderKey(httputil.HeaderForwardedProto)] + "://" +
-			headers[http.CanonicalHeaderKey(httputil.HeaderForwardedHost)]
-		if xfu := headers[http.CanonicalHeaderKey(httputil.HeaderForwardedURI)]; xfu != "/" {
-			uriQuery += xfu
-		}
-	}
-
-	if (checkURL.Path != "/" && checkURL.Path != "/verify") || uriQuery == "" {
-		return false
-	}
-	verifyURL, err := url.Parse(uriQuery)
-	if err != nil {
-		log.Warn().Str("uri", checkURL.Query().Get("uri")).Err(err).Msg("failed to parse uri for forward authentication")
-		return false
-	}
-
-	req.Attributes.Request.Http.Scheme = verifyURL.Scheme
-	req.Attributes.Request.Http.Host = verifyURL.Host
-	req.Attributes.Request.Http.Path = verifyURL.Path
-
-	// envoy sends the query string as part of the path
-	if verifyURL.RawQuery != "" {
-		req.Attributes.Request.Http.Path += "?" + verifyURL.RawQuery
-	}
-	return true
+	return urlutil.StripPort(checkURL.Host) == urlutil.StripPort(opts.GetForwardAuthURL().Host)
 }

 func (a *Authorize) getEvaluatorRequestFromCheckRequest(in *envoy_service_auth_v2.CheckRequest, sessionState *sessions.State) *evaluator.Request {
@ -291,20 +290,6 @@ func getCheckRequestURL(req *envoy_service_auth_v2.CheckRequest) *url.URL {
 	} else {
 		u.Path = path
 	}
-
-	// check to make sure this is _not_ a verify endpoint and that forwarding
-	// headers are set. If so, infer the true authorization location from thos
-	if u.Path != "/verify" && h.GetHeaders() != nil {
-		if val, ok := h.GetHeaders()["x-forwarded-proto"]; ok && val != "" {
-			u.Scheme = val
-		}
-		if val, ok := h.GetHeaders()["x-forwarded-host"]; ok && val != "" {
-			u.Host = val
-		}
-		if val, ok := h.GetHeaders()["x-forwarded-uri"]; ok && val != "" && val != "/" {
-			u.Path = val
-		}
-	}
 	return u
 }

--- a/authorize/grpc_test.go
+++ b/authorize/grpc_test.go
@ -8,8 +8,11 @@ import (

 	envoy_service_auth_v2 "github.com/envoyproxy/go-control-plane/envoy/service/auth/v2"
 	"github.com/golang/protobuf/ptypes"
+	"github.com/google/go-cmp/cmp"
+	"github.com/google/go-cmp/cmp/cmpopts"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
+	"google.golang.org/genproto/googleapis/rpc/status"
 	"google.golang.org/grpc"

 	"github.com/pomerium/pomerium/authorize/evaluator"
@ -95,7 +98,7 @@ func Test_getEvaluatorRequest(t *testing.T) {
 		},
 		HTTP: evaluator.RequestHTTP{
 			Method: "GET",
-			URL:    "https://example.com/some/path?qs=1",
+			URL:    "http://example.com/some/path?qs=1",
 			Headers: map[string]string{
 				"Accept":            "text/html",
 				"X-Forwarded-Proto": "https",
@ -108,12 +111,12 @@ func Test_getEvaluatorRequest(t *testing.T) {
 }

 func Test_handleForwardAuth(t *testing.T) {
+
 	tests := []struct {
 		name           string
 		checkReq       *envoy_service_auth_v2.CheckRequest
-		attrCtxHTTPReq *envoy_service_auth_v2.AttributeContext_HttpRequest
 		forwardAuthURL string
-		isForwardAuth  bool
+		want           bool
 	}{
 		{
 			name: "enabled",
@ -132,21 +135,14 @@ func Test_handleForwardAuth(t *testing.T) {
 					},
 				},
 			},
-			attrCtxHTTPReq: &envoy_service_auth_v2.AttributeContext_HttpRequest{
-				Method: "GET",
-				Path:   "/some/path?qs=1",
-				Host:   "example.com",
-				Scheme: "https",
-			},
 			forwardAuthURL: "https://forward-auth.example.com",
-			isForwardAuth:  true,
+			want:           true,
 		},
 		{
 			name:           "disabled",
 			checkReq:       nil,
-			attrCtxHTTPReq: nil,
 			forwardAuthURL: "",
-			isForwardAuth:  false,
+			want:           false,
 		},
 		{
 			name: "honor x-forwarded-uri set",
@ -170,19 +166,8 @@ func Test_handleForwardAuth(t *testing.T) {
 					},
 				},
 			},
-			attrCtxHTTPReq: &envoy_service_auth_v2.AttributeContext_HttpRequest{
-				Method: "GET",
-				Path:   "/foo/bar",
-				Host:   "example.com",
-				Scheme: "https",
-				Headers: map[string]string{
-					httputil.HeaderForwardedURI:   "/foo/bar",
-					httputil.HeaderForwardedProto: "https",
-					httputil.HeaderForwardedHost:  "example.com",
-				},
-			},
 			forwardAuthURL: "https://forward-auth.example.com",
-			isForwardAuth:  true,
+			want:           true,
 		},
 		{
 			name: "request with invalid forward auth url",
@ -201,9 +186,8 @@ func Test_handleForwardAuth(t *testing.T) {
 					},
 				},
 			},
-			attrCtxHTTPReq: nil,
 			forwardAuthURL: "https://forward-auth.example.com",
-			isForwardAuth:  false,
+			want:           false,
 		},
 		{
 			name: "request with invalid path",
@ -222,9 +206,8 @@ func Test_handleForwardAuth(t *testing.T) {
 					},
 				},
 			},
-			attrCtxHTTPReq: nil,
 			forwardAuthURL: "https://forward-auth.example.com",
-			isForwardAuth:  false,
+			want:           true,
 		},
 		{
 			name: "request with empty uri",
@ -243,9 +226,8 @@ func Test_handleForwardAuth(t *testing.T) {
 					},
 				},
 			},
-			attrCtxHTTPReq: nil,
 			forwardAuthURL: "https://forward-auth.example.com",
-			isForwardAuth:  false,
+			want:           true,
 		},
 		{
 			name: "request with invalid uri",
@ -264,9 +246,8 @@ func Test_handleForwardAuth(t *testing.T) {
 					},
 				},
 			},
-			attrCtxHTTPReq: nil,
 			forwardAuthURL: "https://forward-auth.example.com",
-			isForwardAuth:  false,
+			want:           true,
 		},
 	}

@ -279,9 +260,11 @@ func Test_handleForwardAuth(t *testing.T) {
 				fau = mustParseURL(tc.forwardAuthURL)
 			}
 			a.currentOptions.Store(&config.Options{ForwardAuthURL: fau})
-			assert.Equal(t, tc.isForwardAuth, a.handleForwardAuth(tc.checkReq))
-			if tc.attrCtxHTTPReq != nil {
-				assert.Equal(t, tc.attrCtxHTTPReq, tc.checkReq.Attributes.Request.Http)
+
+			got := a.isForwardAuth(tc.checkReq)
+
+			if diff := cmp.Diff(got, tc.want); diff != "" {
+				t.Errorf("Authorize.Check() = %s", diff)
 			}
 		})
 	}
@ -325,7 +308,7 @@ func Test_getEvaluatorRequestWithPortInHostHeader(t *testing.T) {
 		Session: evaluator.RequestSession{},
 		HTTP: evaluator.RequestHTTP{
 			Method: "GET",
-			URL:    "https://example.com/some/path?qs=1",
+			URL:    "http://example.com/some/path?qs=1",
 			Headers: map[string]string{
 				"Accept":            "text/html",
 				"X-Forwarded-Proto": "https",
@ -486,3 +469,95 @@ type mockDataBrokerServiceClient struct {
 func (m mockDataBrokerServiceClient) Get(ctx context.Context, in *databroker.GetRequest, opts ...grpc.CallOption) (*databroker.GetResponse, error) {
 	return m.get(ctx, in, opts...)
 }
+
+func TestAuthorize_Check(t *testing.T) {
+	opt := config.NewDefaultOptions()
+	opt.AuthenticateURL = mustParseURL("https://authenticate.example.com")
+	opt.DataBrokerURL = mustParseURL("https://databroker.example.com")
+	opt.SharedKey = "E8wWIMnihUx+AUfRegAQDNs8eRb3UrB5G3zlJW9XJDM="
+	a, err := New(&config.Config{Options: opt})
+	if err != nil {
+		t.Fatal(err)
+	}
+	a.currentOptions.Store(&config.Options{ForwardAuthURL: mustParseURL("https://forward-auth.example.com")})
+
+	cmpOpts := []cmp.Option{
+		cmpopts.IgnoreUnexported(envoy_service_auth_v2.CheckResponse{}),
+		cmpopts.IgnoreUnexported(status.Status{}),
+		cmpopts.IgnoreTypes(envoy_service_auth_v2.DeniedHttpResponse{}),
+	}
+	tests := []struct {
+		name    string
+		in      *envoy_service_auth_v2.CheckRequest
+		want    *envoy_service_auth_v2.CheckResponse
+		wantErr bool
+	}{
+		{"basic deny",
+			&envoy_service_auth_v2.CheckRequest{
+				Attributes: &envoy_service_auth_v2.AttributeContext{
+					Source: &envoy_service_auth_v2.AttributeContext_Peer{
+						Certificate: url.QueryEscape(certPEM),
+					},
+					Request: &envoy_service_auth_v2.AttributeContext_Request{
+						Http: &envoy_service_auth_v2.AttributeContext_HttpRequest{
+							Id:     "id-1234",
+							Method: "GET",
+							Headers: map[string]string{
+								"accept":            "application/json",
+								"x-forwarded-proto": "https",
+							},
+							Path:   "/some/path?qs=1",
+							Host:   "example.com",
+							Scheme: "http",
+							Body:   "BODY",
+						},
+					},
+				},
+			},
+			&envoy_service_auth_v2.CheckResponse{
+				Status: &status.Status{Code: 7, Message: "Access Denied"},
+				HttpResponse: &envoy_service_auth_v2.CheckResponse_DeniedResponse{
+					DeniedResponse: &envoy_service_auth_v2.DeniedHttpResponse{},
+				},
+			},
+			false},
+		{"basic forward-auth deny",
+			&envoy_service_auth_v2.CheckRequest{
+				Attributes: &envoy_service_auth_v2.AttributeContext{
+					Source: &envoy_service_auth_v2.AttributeContext_Peer{
+						Certificate: url.QueryEscape(certPEM),
+					},
+					Request: &envoy_service_auth_v2.AttributeContext_Request{
+						Http: &envoy_service_auth_v2.AttributeContext_HttpRequest{
+							Method: "GET",
+							Path:   "/verify?uri=" + url.QueryEscape("https://example.com/some/path?qs=1"),
+							Host:   "forward-auth.example.com",
+							Scheme: "https",
+						},
+					},
+				},
+			},
+			&envoy_service_auth_v2.CheckResponse{
+				Status: &status.Status{Code: 7, Message: "Access Denied"},
+				HttpResponse: &envoy_service_auth_v2.CheckResponse_DeniedResponse{
+					DeniedResponse: &envoy_service_auth_v2.DeniedHttpResponse{},
+				},
+			},
+			false},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+
+			got, err := a.Check(context.TODO(), tt.in)
+			if (err != nil) != tt.wantErr {
+				t.Errorf("Authorize.Check() error = %v, wantErr %v", err, tt.wantErr)
+				return
+			}
+
+			if diff := cmp.Diff(got, tt.want, cmpOpts...); diff != "" {
+				t.Errorf("NewStore() = %s", diff)
+			}
+
+		})
+	}
+}
--- a/examples/nginx/auth.conf
+++ b/examples/nginx/auth.conf
@ -1,11 +0,0 @@
-# Send auth check to /authorize location.
-auth_request /authorize;
-
-auth_request_set $target_url $scheme://$http_host$request_uri;
-
-# Set cookies we get back from the auth check
-auth_request_set $saved_set_cookie $upstream_http_set_cookie;
-add_header Set-Cookie $saved_set_cookie;
-
-# If we get a 401, respond with a named location
-error_page 401 =302 https://fwdauth.localhost.pomerium.io/?uri=$target_url;
--- a/examples/nginx/docker-compose.yaml
+++ b/examples/nginx/docker-compose.yaml
@ -12,8 +12,6 @@ services:
      - ./_wildcard.localhost.pomerium.io.pem:/etc/nginx/nginx.pem
      - ./_wildcard.localhost.pomerium.io-key.pem:/etc/nginx/nginx-key.pem
      - ./proxy.conf:/etc/nginx/proxy.conf
-      - ./auth.conf:/etc/nginx/auth.conf
-      - ./ext_authz.conf:/etc/nginx/ext_authz.conf

  httpbin:
    image: kennethreitz/httpbin:latest
--- a/examples/nginx/ext_authz.conf
+++ b/examples/nginx/ext_authz.conf
@ -1,16 +0,0 @@
-location /authorize {
-	proxy_pass http://pomerium/verify?uri=$scheme://$http_host$request_uri;
-	proxy_set_header Host fwdauth.localhost.pomerium.io;
-	proxy_http_version 1.1;
-
-	proxy_set_header X-Original-URL $scheme://$http_host$request_uri;
-	proxy_set_header X-Real-IP $remote_addr;
-	proxy_set_header X-Forwarded-For $remote_addr;
-	proxy_set_header X-Forwarded-Proto $scheme;
-	proxy_set_header X-Forwarded-Host $http_host;
-	proxy_set_header X-Forwarded-Uri $request_uri;
-	proxy_cache_bypass $cookie_session;
-	proxy_no_cache $cookie_session;
-	proxy_buffers 4 32k;
-	proxy_pass_request_body off;
-}
--- a/examples/nginx/httpbin.conf
+++ b/examples/nginx/httpbin.conf
@ -1,16 +1,62 @@
 # Protected application
 server {
-    listen 443 ssl;
-    listen 80;
-    server_name  httpbin.localhost.pomerium.io;
-    ssl_certificate /etc/nginx/nginx.pem;
-    ssl_certificate_key /etc/nginx/nginx-key.pem;
+  listen 80;
+  listen 443 ssl http2;

-    include /etc/nginx/ext_authz.conf;
+  server_name httpbin.localhost.pomerium.io;
+  ssl_certificate /etc/nginx/nginx.pem;
+  ssl_certificate_key /etc/nginx/nginx-key.pem;

-    location / {
-      proxy_pass http://httpbin;
-      include /etc/nginx/auth.conf;
-      include /etc/nginx/proxy.conf;
-    }
+
+  location = /ext_authz {
+    internal;
+
+    proxy_pass_request_body off;
+    proxy_set_header Content-Length "";
+    proxy_set_header X-Forwarded-Proto "";
+
+    proxy_set_header Host fwdauth.localhost.pomerium.io;
+    proxy_set_header X-Original-URL $scheme://$http_host$request_uri;
+    proxy_set_header X-Original-Method $request_method;
+    proxy_set_header X-Real-IP $remote_addr;
+
+    proxy_set_header X-Forwarded-For $remote_addr;
+
+    proxy_set_header X-Auth-Request-Redirect $request_uri;
+
+    proxy_buffering off;
+
+    proxy_buffer_size 4k;
+    proxy_buffers 4 4k;
+    proxy_request_buffering on;
+    proxy_http_version 1.1;
+
+    proxy_ssl_server_name on;
+    proxy_pass_request_headers on;
+
+    client_max_body_size 1m;
+
+    # Pass the extracted client certificate to the auth provider
+    set $target http://pomerium/verify?uri=$scheme://$http_host$request_uri;
+    proxy_pass $target;
+  }
+
+  location @authredirect {
+    internal;
+    add_header Set-Cookie $auth_cookie;
+    return 302
+      https://fwdauth.localhost.pomerium.io/?uri=$scheme://$host$request_uri;
+  }
+
+  location / {
+    proxy_pass http://httpbin;
+
+    include /etc/nginx/proxy.conf;
+    # If we get a 401, respond with a named location
+    error_page 401 = @authredirect;
+    # this location requires authentication
+    auth_request /ext_authz;
+    auth_request_set $auth_cookie $upstream_http_set_cookie;
+    add_header Set-Cookie $auth_cookie;
+  }
 }
--- a/examples/nginx/proxy.conf
+++ b/examples/nginx/proxy.conf
@ -1,33 +1,61 @@
-client_body_buffer_size 128k;
+set $pass_access_scheme $scheme;

-#Timeout if the real server is dead
-proxy_next_upstream error timeout invalid_header http_500 http_502 http_503;
+set $pass_server_port $server_port;

-# Advanced Proxy Config
-send_timeout 5m;
-proxy_read_timeout 360;
-proxy_send_timeout 360;
-proxy_connect_timeout 360;
+set $best_http_host $http_host;
+set $pass_port $pass_server_port;
+
+set $proxy_alternative_upstream_name "";
+
+client_max_body_size 1m;
+
+
+proxy_set_header Host $best_http_host;
+
+# Pass the extracted client certificate to the backend
+
+# Allow websocket connections
+proxy_set_header Upgrade $http_upgrade;

-# Basic Proxy Config
-proxy_set_header Host $host;
-proxy_set_header X-Real-IP $remote_addr;
-proxy_set_header X-Forwarded-For $proxy_add_x_forwarded_for;
-proxy_set_header X-Forwarded-Proto $scheme;
-proxy_set_header X-Forwarded-Host $http_host;
-proxy_set_header X-Forwarded-Uri $request_uri;
-proxy_set_header X-Forwarded-Ssl on;
-proxy_redirect  http://  $scheme://;
-proxy_http_version 1.1;
 proxy_set_header Connection "";
-proxy_cache_bypass $cookie_session;
-proxy_no_cache $cookie_session;
-proxy_buffers 64 256k;

-# If behind reverse proxy, forwards the correct IP
-set_real_ip_from 10.0.0.0/8;
-set_real_ip_from 172.0.0.0/8;
-set_real_ip_from 192.168.0.0/16;
-set_real_ip_from fc00::/7;
-real_ip_header X-Forwarded-For;
-real_ip_recursive on;
+# proxy_set_header X-Request-ID $req_id;
+proxy_set_header X-Real-IP $remote_addr;
+proxy_set_header X-Forwarded-For $remote_addr;
+proxy_set_header X-Forwarded-Host $best_http_host;
+proxy_set_header X-Forwarded-Port $pass_port;
+proxy_set_header X-Forwarded-Proto $pass_access_scheme;
+
+proxy_set_header X-Scheme $pass_access_scheme;
+
+# Pass the original X-Forwarded-For
+proxy_set_header X-Original-Forwarded-For $http_x_forwarded_for;
+
+# mitigate HTTPoxy Vulnerability
+# https://www.nginx.com/blog/mitigating-the-httpoxy-vulnerability-with-nginx/
+proxy_set_header Proxy "";
+
+# Custom headers to proxied server
+
+proxy_connect_timeout 5s;
+proxy_send_timeout 60s;
+proxy_read_timeout 60s;
+
+proxy_buffering off;
+proxy_buffer_size 4k;
+proxy_buffers 4 4k;
+
+proxy_max_temp_file_size 1024m;
+
+proxy_request_buffering on;
+proxy_http_version 1.1;
+
+proxy_cookie_domain off;
+proxy_cookie_path off;
+
+# In case of errors try the next upstream server before returning an error
+proxy_next_upstream error timeout;
+proxy_next_upstream_timeout 0;
+proxy_next_upstream_tries 3;
+
+proxy_redirect off;