serve: pass the client IP address to the auth proxy - fixes #4499

The auth proxy was only given the user and their password or public
key, so a proxy program had no way to restrict logins to particular
networks, or to record where an authentication attempt came from.

The JSON sent to the program now has a client_ip key holding the bare
IP the client connected from, with the port stripped so IPv6 arrives
as 2001:db8::1 rather than [2001:db8::1]:52344. An IPv4-mapped IPv6
address is reported as plain IPv4 so that a client arriving over a
dual-stack listener still matches IPv4 networks. The key is omitted
when the client has no IP address.

The IP is also mixed into the backend cache key. That is needed as the
program is only run on a cache miss, so a client from a
non-allowlisted address presenting valid credentials within the 5
minute cache lifetime would get a cache hit and be let in without the
program being consulted at all.

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)
 	}

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
 	}

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
 		}

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`, ""},
+		{"@", ""},
+		{"<nil>", ""},
+		{"", ""},
+	} {
+		assert.Equal(t, test.want, ipFromAddr(test.in), test.in)
+	}
+}

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)
 		}

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

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
 	}