diff --git a/Release.md b/Release.md index 304a5d83..c42987d3 100644 --- a/Release.md +++ b/Release.md @@ -1 +1,3 @@ -## Features +## Fixes + +* Fixed a configuration-dependent authentication bypass in `type = "http"` proxies when `routeByHTTPUser` is used together with `httpUser` / `httpPassword`. This affected proxy-style requests. Proxy-style authentication failures now return `407 Proxy Authentication Required`. diff --git a/pkg/util/version/version.go b/pkg/util/version/version.go index c98b7cfb..14f04ede 100644 --- a/pkg/util/version/version.go +++ b/pkg/util/version/version.go @@ -14,7 +14,7 @@ package version -var version = "0.68.0" +var version = "0.68.1" func Full() string { return version diff --git a/pkg/util/vhost/http.go b/pkg/util/vhost/http.go index d12e7916..cac88e35 100644 --- 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 { diff --git a/pkg/util/vhost/http_test.go b/pkg/util/vhost/http_test.go new file mode 100644 index 00000000..237ae903 --- /dev/null +++ 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)) + }) +} diff --git a/test/e2e/v1/basic/http.go b/test/e2e/v1/basic/http.go index b594f1ea..1a44ba21 100644 --- a/test/e2e/v1/basic/http.go +++ b/test/e2e/v1/basic/http.go @@ -144,6 +144,79 @@ var _ = ginkgo.Describe("[Feature: HTTP]", func() { Ensure() }) + ginkgo.It("HTTP proxy mode uses proxy auth consistently", func() { + vhostHTTPPort := f.AllocPort() + serverConf := getDefaultServerConf(vhostHTTPPort) + + backendPort := f.AllocPort() + f.RunServer("", newHTTPServer(backendPort, "PRIVATE")) + + clientConf := consts.DefaultClientConfig + clientConf += fmt.Sprintf(` + [[proxies]] + name = "protected" + type = "http" + localPort = %d + customDomains = ["normal.example.com"] + routeByHTTPUser = "alice" + httpUser = "alice" + httpPassword = "secret" + `, backendPort) + + f.RunProcesses(serverConf, []string{clientConf}) + + proxyURLWithAuth := func(username, password string) string { + if username == "" { + return fmt.Sprintf("http://127.0.0.1:%d", vhostHTTPPort) + } + return fmt.Sprintf("http://%s:%s@127.0.0.1:%d", username, password, vhostHTTPPort) + } + + framework.NewRequestExpect(f).Explain("direct no auth").Port(vhostHTTPPort). + RequestModify(func(r *request.Request) { + r.HTTP().HTTPHost("normal.example.com") + }). + Ensure(framework.ExpectResponseCode(http.StatusNotFound)) + + framework.NewRequestExpect(f).Explain("direct correct auth").Port(vhostHTTPPort). + RequestModify(func(r *request.Request) { + r.HTTP().HTTPHost("normal.example.com").HTTPAuth("alice", "secret") + }). + ExpectResp([]byte("PRIVATE")). + Ensure() + + framework.NewRequestExpect(f).Explain("direct wrong auth").Port(vhostHTTPPort). + RequestModify(func(r *request.Request) { + r.HTTP().HTTPHost("normal.example.com").HTTPAuth("alice", "wrong") + }). + Ensure(framework.ExpectResponseCode(http.StatusUnauthorized)) + + framework.NewRequestExpect(f).Explain("proxy correct proxy auth"). + RequestModify(func(r *request.Request) { + r.HTTP().Addr("normal.example.com").Proxy(proxyURLWithAuth("alice", "secret")) + }). + ExpectResp([]byte("PRIVATE")). + Ensure() + + framework.NewRequestExpect(f).Explain("proxy wrong proxy auth"). + RequestModify(func(r *request.Request) { + r.HTTP().Addr("normal.example.com").Proxy(proxyURLWithAuth("alice", "wrong")) + }). + Ensure(framework.ExpectResponseCode(http.StatusProxyAuthRequired)) + + framework.NewRequestExpect(f).Explain("proxy request ignores authorization header"). + RequestModify(func(r *request.Request) { + r.HTTP().Addr("normal.example.com").Proxy(proxyURLWithAuth("", "")).HTTPAuth("alice", "secret") + }). + Ensure(framework.ExpectResponseCode(http.StatusProxyAuthRequired)) + + framework.NewRequestExpect(f).Explain("proxy wrong proxy auth with correct authorization"). + RequestModify(func(r *request.Request) { + r.HTTP().Addr("normal.example.com").Proxy(proxyURLWithAuth("alice", "wrong")).HTTPAuth("alice", "secret") + }). + Ensure(framework.ExpectResponseCode(http.StatusProxyAuthRequired)) + }) + ginkgo.It("HTTP Basic Auth", func() { vhostHTTPPort := f.AllocPort() serverConf := getDefaultServerConf(vhostHTTPPort)