vhost/http: fix auth bypass when routeByHTTPUser is used with proxy-style requests (#5285)

2026-04-15 05:29:10 +08:00 · 2026-04-14 00:58:19 +08:00
parent d667be7a0a
commit a9a4416ecf
5 changed files with 220 additions and 24 deletions
--- a/pkg/util/vhost/http.go
+++ b/pkg/util/vhost/http.go
@@ -187,16 +187,25 @@ func (rp *HTTPReverseProxy) CreateConnection(reqRouteInfo *RequestRouteInfo, byE
 	return nil, fmt.Errorf("%v: %s %s %s", ErrNoRouteFound, host, reqRouteInfo.URL, reqRouteInfo.HTTPUser)
 }

-func (rp *HTTPReverseProxy) CheckAuth(domain, location, routeByHTTPUser, user, passwd string) bool {
-	vr, ok := rp.getVhost(domain, location, routeByHTTPUser)
-	if ok {
-		checkUser := vr.payload.(*RouteConfig).Username
-		checkPasswd := vr.payload.(*RouteConfig).Password
-		if (checkUser != "" || checkPasswd != "") && (checkUser != user || checkPasswd != passwd) {
+func checkRouteAuthByRequest(req *http.Request, rc *RouteConfig) bool {
+	if rc == nil {
+		return true
+	}
+	if rc.Username == "" && rc.Password == "" {
+		return true
+	}
+
+	if req.URL.Host != "" {
+		proxyAuth := req.Header.Get("Proxy-Authorization")
+		if proxyAuth == "" {
 			return false
 		}
+		user, passwd, ok := httppkg.ParseBasicAuth(proxyAuth)
+		return ok && user == rc.Username && passwd == rc.Password
 	}
-	return true
+
+	user, passwd, ok := req.BasicAuth()
+	return ok && user == rc.Username && passwd == rc.Password
 }

 // getVhost tries to get vhost router by route policy.
@@ -266,18 +275,25 @@ func (rp *HTTPReverseProxy) connectHandler(rw http.ResponseWriter, req *http.Req
 	go libio.Join(remote, client)
 }

-func (rp *HTTPReverseProxy) injectRequestInfoToCtx(req *http.Request) *http.Request {
-	user := ""
-	// If url host isn't empty, it's a proxy request. Get http user from Proxy-Authorization header.
+func getRequestRouteUser(req *http.Request) string {
 	if req.URL.Host != "" {
 		proxyAuth := req.Header.Get("Proxy-Authorization")
-		if proxyAuth != "" {
-			user, _, _ = httppkg.ParseBasicAuth(proxyAuth)
+		if proxyAuth == "" {
+			// Preserve legacy proxy-mode routing when clients send only Authorization,
+			// so requests still hit the matched route and return 407 instead of 404.
+			// Auth validation intentionally does not share this fallback.
+			user, _, _ := req.BasicAuth()
+			return user
 		}
+		user, _, _ := httppkg.ParseBasicAuth(proxyAuth)
+		return user
 	}
-	if user == "" {
-		user, _, _ = req.BasicAuth()
-	}
+	user, _, _ := req.BasicAuth()
+	return user
+}
+
+func (rp *HTTPReverseProxy) injectRequestInfoToCtx(req *http.Request) *http.Request {
+	user := getRequestRouteUser(req)

 	reqRouteInfo := &RequestRouteInfo{
 		URL:        req.URL.Path,
@@ -297,16 +313,19 @@ func (rp *HTTPReverseProxy) injectRequestInfoToCtx(req *http.Request) *http.Requ
 }

 func (rp *HTTPReverseProxy) ServeHTTP(rw http.ResponseWriter, req *http.Request) {
-	domain, _ := httppkg.CanonicalHost(req.Host)
-	location := req.URL.Path
-	user, passwd, _ := req.BasicAuth()
-	if !rp.CheckAuth(domain, location, user, user, passwd) {
-		rw.Header().Set("WWW-Authenticate", `Basic realm="Restricted"`)
-		http.Error(rw, http.StatusText(http.StatusUnauthorized), http.StatusUnauthorized)
+	newreq := rp.injectRequestInfoToCtx(req)
+	rc := newreq.Context().Value(RouteConfigKey).(*RouteConfig)
+	if !checkRouteAuthByRequest(req, rc) {
+		if req.URL.Host != "" {
+			rw.Header().Set("Proxy-Authenticate", `Basic realm="Restricted"`)
+			http.Error(rw, http.StatusText(http.StatusProxyAuthRequired), http.StatusProxyAuthRequired)
+		} else {
+			rw.Header().Set("WWW-Authenticate", `Basic realm="Restricted"`)
+			http.Error(rw, http.StatusText(http.StatusUnauthorized), http.StatusUnauthorized)
+		}
 		return
 	}

-	newreq := rp.injectRequestInfoToCtx(req)
 	if req.Method == http.MethodConnect {
 		rp.connectHandler(rw, newreq)
 	} else {
--- a/pkg/util/vhost/http_test.go
+++ b/pkg/util/vhost/http_test.go
@@ -0,0 +1,102 @@
+package vhost
+
+import (
+	"net/http/httptest"
+	"testing"
+
+	"github.com/stretchr/testify/require"
+
+	httppkg "github.com/fatedier/frp/pkg/util/http"
+)
+
+func TestCheckRouteAuthByRequest(t *testing.T) {
+	rc := &RouteConfig{
+		Username: "alice",
+		Password: "secret",
+	}
+
+	t.Run("accepts nil route config", func(t *testing.T) {
+		req := httptest.NewRequest("GET", "/", nil)
+		require.True(t, checkRouteAuthByRequest(req, nil))
+	})
+
+	t.Run("accepts route without credentials", func(t *testing.T) {
+		req := httptest.NewRequest("GET", "/", nil)
+		require.True(t, checkRouteAuthByRequest(req, &RouteConfig{}))
+	})
+
+	t.Run("accepts authorization header", func(t *testing.T) {
+		req := httptest.NewRequest("GET", "/", nil)
+		req.SetBasicAuth("alice", "secret")
+		require.True(t, checkRouteAuthByRequest(req, rc))
+	})
+
+	t.Run("accepts proxy authorization header", func(t *testing.T) {
+		req := httptest.NewRequest("GET", "http://target.example.com/", nil)
+		req.Header.Set("Proxy-Authorization", httppkg.BasicAuth("alice", "secret"))
+		require.True(t, checkRouteAuthByRequest(req, rc))
+	})
+
+	t.Run("rejects authorization fallback for proxy request", func(t *testing.T) {
+		req := httptest.NewRequest("GET", "http://target.example.com/", nil)
+		req.SetBasicAuth("alice", "secret")
+		require.False(t, checkRouteAuthByRequest(req, rc))
+	})
+
+	t.Run("rejects wrong proxy authorization even when authorization matches", func(t *testing.T) {
+		req := httptest.NewRequest("GET", "http://target.example.com/", nil)
+		req.SetBasicAuth("alice", "secret")
+		req.Header.Set("Proxy-Authorization", httppkg.BasicAuth("alice", "wrong"))
+		require.False(t, checkRouteAuthByRequest(req, rc))
+	})
+
+	t.Run("rejects when neither header matches", func(t *testing.T) {
+		req := httptest.NewRequest("GET", "http://target.example.com/", nil)
+		req.SetBasicAuth("alice", "wrong")
+		req.Header.Set("Proxy-Authorization", httppkg.BasicAuth("alice", "wrong"))
+		require.False(t, checkRouteAuthByRequest(req, rc))
+	})
+
+	t.Run("rejects proxy authorization on direct request", func(t *testing.T) {
+		req := httptest.NewRequest("GET", "/", nil)
+		req.Header.Set("Proxy-Authorization", httppkg.BasicAuth("alice", "secret"))
+		require.False(t, checkRouteAuthByRequest(req, rc))
+	})
+}
+
+func TestGetRequestRouteUser(t *testing.T) {
+	t.Run("proxy request uses proxy authorization username", func(t *testing.T) {
+		req := httptest.NewRequest("GET", "http://target.example.com/", nil)
+		req.Host = "target.example.com"
+		req.Header.Set("Proxy-Authorization", httppkg.BasicAuth("proxy-user", "proxy-pass"))
+		req.SetBasicAuth("direct-user", "direct-pass")
+
+		require.Equal(t, "proxy-user", getRequestRouteUser(req))
+	})
+
+	t.Run("connect request keeps proxy authorization routing", func(t *testing.T) {
+		req := httptest.NewRequest("CONNECT", "http://target.example.com:443", nil)
+		req.Host = "target.example.com:443"
+		req.Header.Set("Proxy-Authorization", httppkg.BasicAuth("proxy-user", "proxy-pass"))
+		req.SetBasicAuth("direct-user", "direct-pass")
+
+		require.Equal(t, "proxy-user", getRequestRouteUser(req))
+	})
+
+	t.Run("direct request uses authorization username", func(t *testing.T) {
+		req := httptest.NewRequest("GET", "/", nil)
+		req.Host = "example.com"
+		req.SetBasicAuth("direct-user", "direct-pass")
+
+		require.Equal(t, "direct-user", getRequestRouteUser(req))
+	})
+
+	t.Run("proxy request does not fall back when proxy authorization is invalid", func(t *testing.T) {
+		req := httptest.NewRequest("GET", "http://target.example.com/", nil)
+		req.Host = "target.example.com"
+		req.Header.Set("Proxy-Authorization", "Basic !!!")
+		req.SetBasicAuth("direct-user", "direct-pass")
+
+		require.Empty(t, getRequestRouteUser(req))
+	})
+}