From 29fd89418965d375322886f7f198f689931e9cac Mon Sep 17 00:00:00 2001 From: Moises Lima Date: Thu, 10 Oct 2024 14:42:42 -0300 Subject: [PATCH] lib/http: disable automatic authentication skipping for unix sockets Disabling the authentication for unix sockets makes it impossible to use `rclone serve` behind a proxy that that communicates with rclone via a unix socket. Re-enabling the authentication should not have any effect on most users of unix sockets as they do not set authentication up with a unix socket anyway. --- lib/http/context.go | 6 --- lib/http/middleware.go | 15 ------ lib/http/server.go | 3 +- lib/http/server_test.go | 107 +++++++++++++++++++++++++++++----------- 4 files changed, 80 insertions(+), 51 deletions(-) diff --git a/lib/http/context.go b/lib/http/context.go index 239572c75..cbb551568 100644 --- a/lib/http/context.go +++ b/lib/http/context.go @@ -36,12 +36,6 @@ func IsAuthenticated(r *http.Request) bool { return false } -// IsUnixSocket checks if the request was received on a unix socket, used to skip auth & CORS -func IsUnixSocket(r *http.Request) bool { - v, _ := r.Context().Value(ctxKeyUnixSock).(bool) - return v -} - // PublicURL returns the URL defined in NewBaseContext, used for logging & CORS func PublicURL(r *http.Request) string { v, _ := r.Context().Value(ctxKeyPublicURL).(string) diff --git a/lib/http/middleware.go b/lib/http/middleware.go index 06b82278b..8030ec4f1 100644 --- a/lib/http/middleware.go +++ b/lib/http/middleware.go @@ -57,11 +57,6 @@ func NewLoggedBasicAuthenticator(realm string, secrets goauth.SecretProvider) *L func basicAuth(authenticator *LoggedBasicAuth) func(next http.Handler) http.Handler { return func(next http.Handler) http.Handler { return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - // skip auth for unix socket - if IsUnixSocket(r) { - next.ServeHTTP(w, r) - return - } // skip auth for CORS preflight if r.Method == "OPTIONS" { next.ServeHTTP(w, r) @@ -123,11 +118,6 @@ func MiddlewareAuthBasic(user, pass, realm, salt string) Middleware { func MiddlewareAuthCustom(fn CustomAuthFn, realm string, userFromContext bool) Middleware { return func(next http.Handler) http.Handler { return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - // skip auth for unix socket - if IsUnixSocket(r) { - next.ServeHTTP(w, r) - return - } // skip auth for CORS preflight if r.Method == "OPTIONS" { next.ServeHTTP(w, r) @@ -175,11 +165,6 @@ func MiddlewareCORS(allowOrigin string) Middleware { return func(next http.Handler) http.Handler { return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - // skip cors for unix sockets - if IsUnixSocket(r) { - next.ServeHTTP(w, r) - return - } if allowOrigin != "" { w.Header().Add("Access-Control-Allow-Origin", allowOrigin) diff --git a/lib/http/server.go b/lib/http/server.go index acc07c4b2..9964c297e 100644 --- a/lib/http/server.go +++ b/lib/http/server.go @@ -39,8 +39,7 @@ If you set ` + "`--{{ .Prefix }}addr`" + ` to listen on a public or LAN accessib then using Authentication is advised - see the next section for info. You can use a unix socket by setting the url to ` + "`unix:///path/to/socket`" + ` -or just by using an absolute path name. Note that unix sockets bypass the -authentication - this is expected to be done with file system permissions. +or just by using an absolute path name. ` + "`--{{ .Prefix }}addr`" + ` may be repeated to listen on multiple IPs/ports/sockets. Socket activation, described further below, can also be used to accomplish the same. diff --git a/lib/http/server_test.go b/lib/http/server_test.go index 0c636d274..22e49580b 100644 --- a/lib/http/server_test.go +++ b/lib/http/server_test.go @@ -3,6 +3,7 @@ package http import ( "context" "crypto/tls" + "fmt" "io" "net" "net/http" @@ -63,46 +64,96 @@ func testReadTestdataFile(t *testing.T, path string) []byte { } func TestNewServerUnix(t *testing.T) { - ctx := context.Background() - tempDir := t.TempDir() path := filepath.Join(tempDir, "rclone.sock") - cfg := DefaultCfg() - cfg.ListenAddr = []string{path} - - auth := AuthConfig{ - BasicUser: "test", - BasicPass: "test", + servers := []struct { + name string + status int + result string + cfg Config + auth AuthConfig + user string + pass string + }{ + { + name: "ServerWithoutAuth/NoCreds", + status: http.StatusOK, + result: "hello world", + cfg: Config{ + ListenAddr: []string{path}, + }, + }, { + name: "ServerWithAuth/NoCreds", + status: http.StatusUnauthorized, + cfg: Config{ + ListenAddr: []string{path}, + }, + auth: AuthConfig{ + BasicUser: "test", + BasicPass: "test", + }, + }, { + name: "ServerWithAuth/GoodCreds", + status: http.StatusOK, + result: "hello world", + cfg: Config{ + ListenAddr: []string{path}, + }, + auth: AuthConfig{ + BasicUser: "test", + BasicPass: "test", + }, + user: "test", + pass: "test", + }, { + name: "ServerWithAuth/BadCreds", + status: http.StatusUnauthorized, + cfg: Config{ + ListenAddr: []string{path}, + }, + auth: AuthConfig{ + BasicUser: "test", + BasicPass: "test", + }, + user: "testBAD", + pass: "testBAD", + }, } - s, err := NewServer(ctx, WithConfig(cfg), WithAuth(auth)) - require.NoError(t, err) - defer func() { - require.NoError(t, s.Shutdown()) - _, err := os.Stat(path) - require.ErrorIs(t, err, os.ErrNotExist, "shutdown should remove socket") - }() + for _, ss := range servers { + t.Run(ss.name, func(t *testing.T) { + s, err := NewServer(context.Background(), WithConfig(ss.cfg), WithAuth(ss.auth)) + require.NoError(t, err) + defer func() { + require.NoError(t, s.Shutdown()) + _, err := os.Stat(path) + require.ErrorIs(t, err, os.ErrNotExist, "shutdown should remove socket") + }() - require.Empty(t, s.URLs(), "unix socket should not appear in URLs") + require.Empty(t, s.URLs(), "unix socket should not appear in URLs") - expected := []byte("hello world") - s.Router().Mount("/", testEchoHandler(expected)) - s.Serve() + s.Router().Mount("/", testEchoHandler([]byte(ss.result))) + s.Serve() - client := testNewHTTPClientUnix(path) - req, err := http.NewRequest("GET", "http://unix", nil) - require.NoError(t, err) + client := testNewHTTPClientUnix(path) + req, err := http.NewRequest("GET", "http://unix", nil) + require.NoError(t, err) - resp, err := client.Do(req) - require.NoError(t, err) + req.SetBasicAuth(ss.user, ss.pass) - testExpectRespBody(t, resp, expected) + resp, err := client.Do(req) + require.NoError(t, err) + defer func() { + _ = resp.Body.Close() + }() - require.Equal(t, http.StatusOK, resp.StatusCode, "unix sockets should ignore auth") + require.Equal(t, ss.status, resp.StatusCode, fmt.Sprintf("should return status %d", ss.status)) - for _, key := range _testCORSHeaderKeys { - require.NotContains(t, resp.Header, key, "unix sockets should not be sent CORS headers") + if ss.result != "" { + testExpectRespBody(t, resp, []byte(ss.result)) + } + }) } }