diff --git a/bin/test_proxy.py b/bin/test_proxy.py index 1f3650fc8..dcd8fa3e1 100755 --- a/bin/test_proxy.py +++ b/bin/test_proxy.py @@ -4,13 +4,29 @@ A demo proxy for rclone serve sftp/webdav/ftp, etc. This takes the incoming user/pass and converts it into an sftp backend running on localhost. + +Logins from outside ALLOWED_NETWORKS are refused. """ import sys import json +import ipaddress + +ALLOWED_NETWORKS = ["127.0.0.0/8", "::1/128"] + +def allowed(ip): + """Return True if ip is in one of ALLOWED_NETWORKS.""" + if ip is None: + return False + address = ipaddress.ip_address(ip) + return any(address in ipaddress.ip_network(network) for network in ALLOWED_NETWORKS) def main(): i = json.load(sys.stdin) + # Exiting non zero refuses the login - rclone logs whatever we + # write on stderr, so say why. + if not allowed(i.get("client_ip")): + sys.exit("client_ip %s not allowed" % i.get("client_ip")) o = { "type": "sftp", # type of backend "_root": "", # root of the fs diff --git a/cmd/serve/ftp/ftp.go b/cmd/serve/ftp/ftp.go index 6d15d4cfc..d556ccca3 100644 --- a/cmd/serve/ftp/ftp.go +++ b/cmd/serve/ftp/ftp.go @@ -317,7 +317,7 @@ func (l *Logger) PrintResponse(sessionID string, code int, message string) { // CheckPasswd handle auth based on configuration func (d *driver) CheckPasswd(sctx *ftp.Context, user, pass string) (ok bool, err error) { if d.proxy != nil { - _, _, err = d.proxy.Call(user, pass, false) + _, _, err = d.proxy.Call(user, pass, false, sctx.Sess.RemoteAddr().String()) if err != nil { fs.Infof(nil, "proxy login failed: %v", err) return false, nil @@ -366,7 +366,7 @@ func (d *driver) getVFS(sctx *ftp.Context) (VFS *vfs.VFS, err error) { if err != nil { return nil, err } - VFS, _, err = d.proxy.Call(user, pass, false) + VFS, _, err = d.proxy.Call(user, pass, false, sctx.Sess.RemoteAddr().String()) if err != nil { return nil, fmt.Errorf("proxy login failed: %w", err) } diff --git a/cmd/serve/http/http.go b/cmd/serve/http/http.go index 6a20b7756..776e96a29 100644 --- a/cmd/serve/http/http.go +++ b/cmd/serve/http/http.go @@ -175,8 +175,8 @@ func (s *HTTP) getVFS(ctx context.Context) (VFS *vfs.VFS, err error) { } // auth does proxy authorization -func (s *HTTP) auth(user, pass string) (value any, err error) { - VFS, _, err := s.proxy.Call(user, pass, false) +func (s *HTTP) auth(r *http.Request, user, pass string) (value any, err error) { + VFS, _, err := s.proxy.Call(user, pass, false, r.RemoteAddr) if err != nil { return nil, err } diff --git a/cmd/serve/proxy/proxy.go b/cmd/serve/proxy/proxy.go index cb3283b14..25fe679fd 100644 --- a/cmd/serve/proxy/proxy.go +++ b/cmd/serve/proxy/proxy.go @@ -12,6 +12,7 @@ import ( "encoding/json" "errors" "fmt" + "net/netip" "os/exec" "strings" "time" @@ -62,7 +63,8 @@ process (on STDIN) would look similar to this: |||json { "user": "me", - "pass": "mypassword" + "pass": "mypassword", + "client_ip": "192.168.1.1" } ||| @@ -72,10 +74,18 @@ proxy process (on STDIN) would look similar to this: |||json { "user": "me", - "public_key": "AAAAB3NzaC1yc2EAAAADAQABAAABAQDuwESFdAe14hVS6omeyX7edc...JQdf" + "public_key": "AAAAB3NzaC1yc2EAAAADAQABAAABAQDuwESFdAe14hVS6omeyX7edc...JQdf", + "client_ip": "192.168.1.1" } ||| +The |client_ip| key holds the IP address the client connected from, +without a port number. It can be used to restrict logins to certain +networks, or to log authentication attempts centrally. It is omitted if +the client has no IP address, for example when connecting over a unix +socket. Note that if rclone is behind a reverse proxy this will be the +address of the reverse proxy and not the original client. + And as an example return this on STDOUT |||json @@ -101,11 +111,12 @@ to make proxy to many different sftp backends, you could make the in the output and the user to |user|. For security you'd probably want to restrict the |host| to a limited list. -An internal cache of backends is keyed on the |user| and a hash of the -|pass| or |public_key|. This means that if a user's password or -public-key changes, or the proxy returns different config parameters -(eg a rotated |api_key|), a fresh backend will be created on the next -request rather than the cached one being reused. +An internal cache of backends is keyed on the |user|, a hash of the +|pass| or |public_key|, and the |client_ip|. This means that if a +user's password or public-key changes, the client connects from a new IP +address, or the proxy returns different config parameters (eg a rotated +|api_key|), a fresh backend will be created on the next request rather +than the cached one being reused. This can be used to build general purpose proxies to any kind of backend that rclone supports. @@ -212,29 +223,41 @@ var cacheKeyHMACKey = func() []byte { return key }() -// generateCacheKey creates a composite cache key from user and auth credentials -func generateCacheKey(user, auth string) string { +// ipFromAddr returns the bare IP from a "host:port" address, or "" if it has none. +func ipFromAddr(addr string) string { + ap, err := netip.ParseAddrPort(addr) + if err != nil { + return "" + } + return ap.Addr().Unmap().String() +} + +// generateCacheKey creates a composite cache key from the user, the auth +// credentials and the client's IP address. +func generateCacheKey(user, auth, clientIP string) string { mac := hmac.New(sha256.New, cacheKeyHMACKey) mac.Write([]byte(auth)) + // Separate the two so ("ab", "c") can't collide with ("a", "bc") + mac.Write([]byte{0}) + mac.Write([]byte(clientIP)) return user + "-" + hex.EncodeToString(mac.Sum(nil)[:8]) } // call runs the auth proxy and returns a cacheEntry and an error -func (p *Proxy) call(user, auth string, isPublicKey bool) (value any, err error) { - var config configmap.Simple +func (p *Proxy) call(user, auth string, isPublicKey bool, clientIP string) (value any, err error) { // Contact the proxy - if isPublicKey { - config, err = p.run(map[string]string{ - "user": user, - "public_key": auth, - }) - } else { - config, err = p.run(map[string]string{ - "user": user, - "pass": auth, - }) + in := map[string]string{ + "user": user, } - + if isPublicKey { + in["public_key"] = auth + } else { + in["pass"] = auth + } + if clientIP != "" { + in["client_ip"] = clientIP + } + config, err := p.run(in) if err != nil { return nil, err } @@ -255,10 +278,10 @@ func (p *Proxy) call(user, auth string, isPublicKey bool) (value any, err error) return nil, fmt.Errorf("proxy: couldn't find backend for %q: %w", fsName, err) } - // Make the cache key include the auth so that changes to the - // auth (eg the proxy returning new config) create a fresh - // backend rather than reusing the cached one. - cacheKey := generateCacheKey(user, auth) + // Make the cache key include the auth and the client IP so that + // changes to either (eg the proxy returning new config) create a + // fresh backend rather than reusing the cached one. + cacheKey := generateCacheKey(user, auth, clientIP) // base name of config on user name and auth hash. This may appear in logs name := "proxy-" + cacheKey @@ -298,16 +321,23 @@ func (p *Proxy) call(user, auth string, isPublicKey bool) (value any, err error) // Call runs the auth proxy with the username and password/public key provided // returning a *vfs.VFS and the key used in the VFS cache. -func (p *Proxy) Call(user, auth string, isPublicKey bool) (VFS *vfs.VFS, vfsKey string, err error) { - // Cache key includes the auth so credential changes don't hit a stale entry - cacheKey := generateCacheKey(user, auth) +// +// remoteAddr is the address of the client as returned by net.Addr.String(). +// It may be empty if the client has no IP address, for example when +// connecting over a unix socket. +func (p *Proxy) Call(user, auth string, isPublicKey bool, remoteAddr string) (VFS *vfs.VFS, vfsKey string, err error) { + clientIP := ipFromAddr(remoteAddr) + + // Cache key includes the auth and the client IP so credential or + // address changes don't hit a stale entry + cacheKey := generateCacheKey(user, auth, clientIP) // Look in the cache first with the credential-aware key value, ok := p.vfsCache.GetMaybe(cacheKey) // If not found then call the proxy for a fresh answer if !ok { - value, err = p.call(user, auth, isPublicKey) + value, err = p.call(user, auth, isPublicKey, clientIP) if err != nil { return nil, "", err } diff --git a/cmd/serve/proxy/proxy_test.go b/cmd/serve/proxy/proxy_test.go index a7c2d7dc2..5f347284e 100644 --- a/cmd/serve/proxy/proxy_test.go +++ b/cmd/serve/proxy/proxy_test.go @@ -42,6 +42,25 @@ func TestRun(t *testing.T) { }, config) }) + t.Run("ClientIP", func(t *testing.T) { + config, err := p.run(map[string]string{ + "type": "ftp", + "user": "me", + "pass": "pass", + "host": "127.0.0.1", + "client_ip": "192.0.2.1", + }) + require.NoError(t, err) + assert.Equal(t, configmap.Simple{ + "type": "ftp", + "user": "me-test", + "pass": "pass", + "host": "127.0.0.1", + "client_ip": "192.0.2.1", + "_root": "", + }, config) + }) + t.Run("Error", func(t *testing.T) { config, err := p.run(map[string]string{ "error": "potato", @@ -74,6 +93,24 @@ func TestRun(t *testing.T) { const testUser = "testUser" const testPass = "testPass" + const testIP = "192.0.2.1" + const testAddr = testIP + ":1024" + const otherAddr = "198.51.100.1:1024" + + t.Run("CacheKey", func(t *testing.T) { + // The source port differs on every connection so it must not + // affect the cache key, otherwise the proxy would be run for + // every connection rather than once per client. + assert.Equal(t, + generateCacheKey(testUser, testPass, ipFromAddr(testIP+":1024")), + generateCacheKey(testUser, testPass, ipFromAddr(testIP+":2048"))) + + // A different client IP must produce a different key so the + // proxy is consulted again + assert.NotEqual(t, + generateCacheKey(testUser, testPass, ipFromAddr(testAddr)), + generateCacheKey(testUser, testPass, ipFromAddr(otherAddr))) + }) t.Run("call w/Password", func(t *testing.T) { // check cache empty @@ -81,7 +118,7 @@ func TestRun(t *testing.T) { defer p.vfsCache.Clear() passwordBytes := []byte(testPass) - value, err := p.call(testUser, testPass, false) + value, err := p.call(testUser, testPass, false, testIP) require.NoError(t, err) entry, ok := value.(cacheEntry) require.True(t, ok) @@ -91,7 +128,7 @@ func TestRun(t *testing.T) { require.NotNil(t, entry.vfs) f := entry.vfs.Fs() require.NotNil(t, f) - cacheKey := generateCacheKey(testUser, testPass) + cacheKey := generateCacheKey(testUser, testPass, testIP) assert.Equal(t, "proxy-"+cacheKey, f.Name()) assert.True(t, strings.HasPrefix(f.String(), "Local file system")) @@ -107,8 +144,8 @@ func TestRun(t *testing.T) { assert.Equal(t, 0, p.vfsCache.Entries()) defer p.vfsCache.Clear() - cacheKey := generateCacheKey(testUser, testPass) - vfs, vfsKey, err := p.Call(testUser, testPass, false) + cacheKey := generateCacheKey(testUser, testPass, testIP) + vfs, vfsKey, err := p.Call(testUser, testPass, false, testAddr) require.NoError(t, err) require.NotNil(t, vfs) assert.Equal(t, "proxy-"+cacheKey, vfs.Fs().Name()) @@ -129,7 +166,7 @@ func TestRun(t *testing.T) { }) // now try again from the cache - vfs, vfsKey, err = p.Call(testUser, testPass, false) + vfs, vfsKey, err = p.Call(testUser, testPass, false, testAddr) require.NoError(t, err) require.NotNil(t, vfs) assert.Equal(t, "proxy-"+cacheKey, vfs.Fs().Name()) @@ -141,7 +178,7 @@ func TestRun(t *testing.T) { // A different password produces a different cache key, so it // creates a fresh cache entry rather than hitting the existing // one. Authentication itself is the proxy script's job. - vfs2, vfsKey2, err := p.Call(testUser, testPass+"different", false) + vfs2, vfsKey2, err := p.Call(testUser, testPass+"different", false, testAddr) require.NoError(t, err) require.NotNil(t, vfs2) assert.NotEqual(t, cacheKey, vfsKey2) @@ -152,18 +189,40 @@ func TestRun(t *testing.T) { t.Error("fs/cache returned the stale backend after auth change") } + // A different client IP also produces a different cache key, so + // the proxy is consulted again rather than the cached backend + // being reused - the proxy may be filtering on the IP. + vfs3, vfsKey3, err := p.Call(testUser, testPass, false, otherAddr) + require.NoError(t, err) + require.NotNil(t, vfs3) + assert.NotEqual(t, cacheKey, vfsKey3) + assert.Equal(t, 3, p.vfsCache.Entries()) + // If a cached entry's pwHash somehow doesn't match the supplied // auth (eg a hash collision on the cache key), Call must reject // it. Simulate by corrupting the cached pwHash. entry := cacheEntry{vfs: vfs, pwHash: sha256.Sum256([]byte("tampered"))} p.vfsCache.Put(cacheKey, entry) - vfs, vfsKey, err = p.Call(testUser, testPass, false) + vfs, vfsKey, err = p.Call(testUser, testPass, false, testAddr) require.Error(t, err) require.Contains(t, err.Error(), "incorrect password") require.Nil(t, vfs) require.Equal(t, "", vfsKey) }) + t.Run("Call w/o Address", func(t *testing.T) { + // A client with no address, eg on a unix socket, must still + // authenticate + assert.Equal(t, 0, p.vfsCache.Entries()) + defer p.vfsCache.Clear() + + vfs, vfsKey, err := p.Call(testUser, testPass, false, "") + require.NoError(t, err) + require.NotNil(t, vfs) + assert.Equal(t, generateCacheKey(testUser, testPass, ""), vfsKey) + assert.Equal(t, 1, p.vfsCache.Entries()) + }) + privateKey, privateKeyErr := rsa.GenerateKey(rand.Reader, 2048) if privateKeyErr != nil { fs.Fatal(nil, "error generating test private key "+privateKeyErr.Error()) @@ -180,7 +239,7 @@ func TestRun(t *testing.T) { assert.Equal(t, 0, p.vfsCache.Entries()) defer p.vfsCache.Clear() - value, err := p.call(testUser, publicKeyString, true) + value, err := p.call(testUser, publicKeyString, true, testIP) require.NoError(t, err) entry, ok := value.(cacheEntry) require.True(t, ok) @@ -190,7 +249,7 @@ func TestRun(t *testing.T) { require.NotNil(t, entry.vfs) f := entry.vfs.Fs() require.NotNil(t, f) - cacheKey := generateCacheKey(testUser, publicKeyString) + cacheKey := generateCacheKey(testUser, publicKeyString, testIP) assert.Equal(t, "proxy-"+cacheKey, f.Name()) assert.True(t, strings.HasPrefix(f.String(), "Local file system")) @@ -206,11 +265,12 @@ func TestRun(t *testing.T) { assert.Equal(t, 0, p.vfsCache.Entries()) defer p.vfsCache.Clear() - cacheKey := generateCacheKey(testUser, publicKeyString) + cacheKey := generateCacheKey(testUser, publicKeyString, testIP) vfs, vfsKey, err := p.Call( testUser, publicKeyString, true, + testAddr, ) require.NoError(t, err) require.NotNil(t, vfs) @@ -232,7 +292,7 @@ func TestRun(t *testing.T) { }) // now try again from the cache - vfs, vfsKey, err = p.Call(testUser, publicKeyString, true) + vfs, vfsKey, err = p.Call(testUser, publicKeyString, true, testAddr) require.NoError(t, err) require.NotNil(t, vfs) assert.Equal(t, "proxy-"+cacheKey, vfs.Fs().Name()) @@ -244,7 +304,7 @@ func TestRun(t *testing.T) { // A different public key produces a different cache key, so it // creates a fresh cache entry rather than hitting the existing // one. Authentication itself is the proxy script's job. - vfs2, vfsKey2, err := p.Call(testUser, publicKeyString+"different", true) + vfs2, vfsKey2, err := p.Call(testUser, publicKeyString+"different", true, testAddr) require.NoError(t, err) require.NotNil(t, vfs2) assert.NotEqual(t, cacheKey, vfsKey2) @@ -260,10 +320,30 @@ func TestRun(t *testing.T) { // it. Simulate by corrupting the cached pwHash. entry := cacheEntry{vfs: vfs, pwHash: sha256.Sum256([]byte("tampered"))} p.vfsCache.Put(cacheKey, entry) - vfs, vfsKey, err = p.Call(testUser, publicKeyString, true) + vfs, vfsKey, err = p.Call(testUser, publicKeyString, true, testAddr) require.Error(t, err) require.Contains(t, err.Error(), "incorrect public key") require.Nil(t, vfs) require.Equal(t, "", vfsKey) }) } + +func TestIPFromAddr(t *testing.T) { + for _, test := range []struct { + in string + want string + }{ + {"192.0.2.1:1024", "192.0.2.1"}, + {"[2001:db8::1]:1024", "2001:db8::1"}, + {"[::ffff:192.0.2.1]:1024", "192.0.2.1"}, + {"[fe80::1%eth0]:1024", "fe80::1%eth0"}, + {"/tmp/rclone.sock", ""}, + {"/tmp/foo:bar.sock", ""}, + {`C:\Users\me\rclone.sock`, ""}, + {"@", ""}, + {"", ""}, + {"", ""}, + } { + assert.Equal(t, test.want, ipFromAddr(test.in), test.in) + } +} diff --git a/cmd/serve/s3/server.go b/cmd/serve/s3/server.go index f059ca062..260508a00 100644 --- a/cmd/serve/s3/server.go +++ b/cmd/serve/s3/server.go @@ -133,8 +133,8 @@ func (w *Server) getVFS(ctx context.Context) (VFS *vfs.VFS, err error) { } // auth does proxy authorization -func (w *Server) auth(accessKeyID string) (value any, err error) { - VFS, _, err := w.proxy.Call(stringToMd5Hash(accessKeyID), accessKeyID, false) +func (w *Server) auth(r *http.Request, accessKeyID string) (value any, err error) { + VFS, _, err := w.proxy.Call(stringToMd5Hash(accessKeyID), accessKeyID, false, r.RemoteAddr) if err != nil { return nil, err } @@ -179,7 +179,7 @@ func authPairMiddleware(next http.Handler, ws *Server) http.Handler { func proxyAuthMiddleware(next http.Handler, ws *Server) http.Handler { return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { accessKey, _ := parseAccessKeyID(r) - value, err := ws.auth(accessKey) + value, err := ws.auth(r, accessKey) if err != nil { fs.Infof(r.URL.Path, "%s: Auth failed: %v", r.RemoteAddr, err) } diff --git a/cmd/serve/sftp/server.go b/cmd/serve/sftp/server.go index 91fbc2bb4..b207ce2ce 100644 --- a/cmd/serve/sftp/server.go +++ b/cmd/serve/sftp/server.go @@ -172,7 +172,7 @@ func (s *server) configure() (err error) { fs.Debugf(describeConn(c), "Password login attempt for %s", c.User()) if s.proxy != nil { // query the proxy for the config - _, vfsKey, err := s.proxy.Call(c.User(), string(pass), false) + _, vfsKey, err := s.proxy.Call(c.User(), string(pass), false, c.RemoteAddr().String()) if err != nil { return nil, err } @@ -199,6 +199,7 @@ func (s *server) configure() (err error) { c.User(), base64.StdEncoding.EncodeToString(pubKey.Marshal()), true, + c.RemoteAddr().String(), ) if err != nil { return nil, err diff --git a/cmd/serve/webdav/webdav.go b/cmd/serve/webdav/webdav.go index 0a473e389..7011bef05 100644 --- a/cmd/serve/webdav/webdav.go +++ b/cmd/serve/webdav/webdav.go @@ -350,8 +350,8 @@ func (w *WebDAV) getVFS(ctx context.Context) (VFS *vfs.VFS, err error) { } // auth does proxy authorization -func (w *WebDAV) auth(user, pass string) (value any, err error) { - VFS, _, err := w.proxy.Call(user, pass, false) +func (w *WebDAV) auth(r *http.Request, user, pass string) (value any, err error) { + VFS, _, err := w.proxy.Call(user, pass, false, r.RemoteAddr) if err != nil { return nil, err } diff --git a/lib/http/auth.go b/lib/http/auth.go index d883cba64..a5358932d 100644 --- a/lib/http/auth.go +++ b/lib/http/auth.go @@ -4,6 +4,7 @@ import ( "bytes" "fmt" "html/template" + "net/http" "github.com/rclone/rclone/fs" "github.com/rclone/rclone/fs/config/flags" @@ -68,8 +69,11 @@ Use ` + "`--{{ .Prefix }}salt`" + ` to change the password hashing salt from the // CustomAuthFn if used will be used to authenticate user, pass. If an error // is returned then the user is not authenticated. // +// r is the request being authenticated, for example to find the address +// the client connected from in r.RemoteAddr. +// // If a non nil value is returned then it is added to the context under the key -type CustomAuthFn func(user, pass string) (value any, err error) +type CustomAuthFn func(r *http.Request, user, pass string) (value any, err error) // AuthConfigInfo descripts the Options in use var AuthConfigInfo = fs.Options{{ diff --git a/lib/http/middleware.go b/lib/http/middleware.go index e72668f8d..1c4721a25 100644 --- a/lib/http/middleware.go +++ b/lib/http/middleware.go @@ -142,7 +142,7 @@ func MiddlewareAuthCustom(fn CustomAuthFn, realm string, userFromContext bool) M return } - value, err := fn(user, pass) + value, err := fn(r, user, pass) if err != nil { fs.Infof(r.URL.Path, "%s: Auth failed from %s: %v", r.RemoteAddr, user, err) goauth.NewBasicAuthenticator(realm, func(user, realm string) string { return "" }).RequireAuth(w, r) //Reuse BasicAuth error reporting diff --git a/lib/http/middleware_test.go b/lib/http/middleware_test.go index 2623c0253..ed0607742 100644 --- a/lib/http/middleware_test.go +++ b/lib/http/middleware_test.go @@ -79,8 +79,8 @@ func TestMiddlewareAuth(t *testing.T) { }, auth: AuthConfig{ Realm: "test", - CustomAuthFn: func(user, pass string) (value any, err error) { - if user == "custom" && pass == "custom" { + CustomAuthFn: func(r *http.Request, user, pass string) (value any, err error) { + if user == "custom" && pass == "custom" && r.RemoteAddr != "" { return true, nil } return nil, errors.New("invalid credentials") @@ -304,7 +304,7 @@ func TestMiddlewareAuthCertificateUser(t *testing.T) { }, auth: AuthConfig{ Realm: "test", - CustomAuthFn: func(user, pass string) (value any, err error) { + CustomAuthFn: func(_ *http.Request, user, pass string) (value any, err error) { if user == "custom" && pass == "custom" { return true, nil } @@ -326,7 +326,7 @@ func TestMiddlewareAuthCertificateUser(t *testing.T) { }, auth: AuthConfig{ Realm: "test", - CustomAuthFn: func(user, pass string) (value any, err error) { + CustomAuthFn: func(_ *http.Request, user, pass string) (value any, err error) { fmt.Println("CUSTOMAUTH", user, pass) if user == "rclone-dev-client" && pass == "" { return true, nil