diff --git a/cmd/serve/proxy/proxy.go b/cmd/serve/proxy/proxy.go index 29532bf04..5718588b3 100644 --- a/cmd/serve/proxy/proxy.go +++ b/cmd/serve/proxy/proxy.go @@ -15,6 +15,8 @@ import ( "net/netip" "os/exec" "strings" + "sync" + "sync/atomic" "time" "github.com/rclone/rclone/fs" @@ -54,9 +56,10 @@ This config generated must have this extra parameter - |_root| - root to use for the backend -And it may have this parameter +And it may have these parameters - |_obscure| - comma separated strings for parameters to obscure +- |_secret_access_key| - the secret for S3 access key auth (see below) If password authentication was used by the client, input to the proxy process (on STDIN) would look similar to this: @@ -80,6 +83,32 @@ proxy process (on STDIN) would look similar to this: } ||| +If the client authenticated with an S3 access key (|rclone serve s3|), +the client never sends its secret, only a signature made with it, so +the input contains just the access key ID as the |user| with no |pass| +or |public_key|: + +|||json +{ + "user": "AKIAIOSFODNN7EXAMPLE", + "client_ip": "192.168.1.1" +} +||| + +In this case the program must look up the secret access key for that +access key ID and return it in the |_secret_access_key| field of the +output. Rclone then uses that secret to verify the signature on the +request, refusing the request if it does not match. This means the +proxy program is the source of truth for both the credentials and the +backend they map to. If the program does not return +|_secret_access_key| or returns it empty the request is refused. + +The program's answer for an access key ID is cached (see below) but +is checked with the program again after 5 minutes even if the access +key ID is in constant use, so revoking an access key ID in the +program takes effect within 5 minutes. A rotated secret takes effect +on the first request signed with it. + 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 @@ -145,19 +174,48 @@ func init() { // Proxy represents a proxy to turn auth requests into a VFS type Proxy struct { - cmdLine []string // broken down command line - vfsCache *libcache.Cache - ctx context.Context // for global config - Opt Options - vfsOpt vfscommon.Options + cmdLine []string // broken down command line + vfsCache *libcache.Cache + ctx context.Context // for global config + Opt Options + vfsOpt vfscommon.Options + accessKeyMu sync.Mutex // serialises replacing a cached access key entry } // cacheEntry is what is stored in the vfsCache type cacheEntry struct { - vfs *vfs.VFS // stored VFS - pwHash [sha256.Size]byte // sha256 hash of the password/publicKey + vfs *vfs.VFS // stored VFS + pwHash [sha256.Size]byte // sha256 hash of the password/publicKey + secret string // secret access key returned by the proxy for access key auth + refreshed *atomic.Int64 // unix nanoseconds when the proxy last confirmed this entry } +// accessKeyRefreshInterval is the minimum time between consulting the +// proxy again for a cached access key ID whose secret failed to +// verify a request, so a stream of bad signatures for a known access +// key ID can't make the proxy run for every request. +// +// A variable so tests can adjust it. +var accessKeyRefreshInterval = 10 * time.Second + +// accessKeyRevalidateInterval is how long a cached access key ID is +// trusted before the proxy is asked about it again even though its +// signatures are still verifying, so a revoked access key ID stops +// working within this time rather than for as long as it stays in +// use. +// +// A variable so tests can adjust it. +var accessKeyRevalidateInterval = 5 * time.Minute + +// authKind is the kind of credential the client presented +type authKind int + +const ( + authPassword authKind = iota // auth is a password + authPublicKey // auth is a public key + authAccessKey // user is an S3 access key ID and auth is empty +) + // New creates a new proxy with the Options passed in // // Any VFS are created with the vfsOpt passed in. @@ -251,14 +309,15 @@ func generateCacheKey(user, auth, clientIP string) string { } // call runs the auth proxy and returns a cacheEntry and an error -func (p *Proxy) call(user, auth string, isPublicKey bool, clientIP string) (value any, err error) { +func (p *Proxy) call(user, auth string, kind authKind, clientIP string) (value any, err error) { // Contact the proxy in := map[string]string{ "user": user, } - if isPublicKey { + switch kind { + case authPublicKey: in["public_key"] = auth - } else { + case authPassword: in["pass"] = auth } if clientIP != "" { @@ -278,6 +337,19 @@ func (p *Proxy) call(user, auth string, isPublicKey bool, clientIP string) (valu if !ok { return nil, errors.New("proxy: _root not set in result") } + var secret string + if kind == authAccessKey { + secret, ok = config.Get("_secret_access_key") + if !ok { + return nil, errors.New("proxy: _secret_access_key not set in result") + } + // An empty secret would let anyone sign for this access key ID + if secret == "" { + return nil, errors.New("proxy: _secret_access_key is empty in result") + } + // Keep the secret out of the backend config + delete(config, "_secret_access_key") + } // Find the backend fsInfo, err := fs.Find(fsName) @@ -290,6 +362,28 @@ func (p *Proxy) call(user, auth string, isPublicKey bool, clientIP string) (valu // fresh backend rather than reusing the cached one. cacheKey := generateCacheKey(user, auth, clientIP) + // For access key auth the secret isn't part of the cache key as + // it isn't known until the proxy has been run, so retire any + // cached entry whose secret the proxy has since changed. + // + // The stale entry is moved to a key including its old secret + // rather than deleted so that its VFS isn't shut down under + // requests still using it - it expires with the rest of the cache + // once it has been unused for long enough. + // + // The check, rename and the creation below are done under a lock + // so that two concurrent refreshes during a rotation can't have + // the second retire the backend the first has just created. + if kind == authAccessKey { + p.accessKeyMu.Lock() + defer p.accessKeyMu.Unlock() + if old, ok := p.vfsCache.GetMaybe(cacheKey); ok { + if entry, ok := old.(cacheEntry); ok && entry.secret != secret { + p.vfsCache.Rename(cacheKey, generateCacheKey(user, entry.secret, clientIP)) + } + } + } + // base name of config on user name and auth hash. This may appear in logs name := "proxy-" + cacheKey fsString := name + ":" + root @@ -315,14 +409,21 @@ func (p *Proxy) call(user, auth string, isPublicKey bool, clientIP string) (valu // need to in memory. An attacker would find it easier to go // after the unencrypted password in memory most likely. entry := cacheEntry{ - vfs: vfs.New(p.ctx, f, &p.vfsOpt), - pwHash: sha256.Sum256([]byte(auth)), + vfs: vfs.New(p.ctx, f, &p.vfsOpt), + pwHash: sha256.Sum256([]byte(auth)), + secret: secret, + refreshed: new(atomic.Int64), } return entry, true, nil }) if err != nil { return nil, fmt.Errorf("proxy: failed to create backend: %w", err) } + // The proxy has just confirmed this entry whether it was created + // or reused + if entry, ok := value.(cacheEntry); ok { + entry.refreshed.Store(time.Now().UnixNano()) + } return value, nil } @@ -344,7 +445,11 @@ func (p *Proxy) Call(user, auth string, isPublicKey bool, remoteAddr string) (VF // If not found then call the proxy for a fresh answer if !ok { - value, err = p.call(user, auth, isPublicKey, clientIP) + kind := authPassword + if isPublicKey { + kind = authPublicKey + } + value, err = p.call(user, auth, kind, clientIP) if err != nil { return nil, "", err } @@ -372,6 +477,53 @@ func (p *Proxy) Call(user, auth string, isPublicKey bool, remoteAddr string) (VF return entry.vfs, cacheKey, nil } +// CallAccessKey runs the auth proxy for an S3 access key ID returning +// a *vfs.VFS and the secret access key the proxy supplied for it. +// +// The caller must verify the request's signature against the returned +// secret - the proxy only maps the access key ID to a backend and +// secret, it cannot authenticate the client itself. +// +// If refresh is true the proxy is consulted rather than using a cached +// answer, unless it was consulted for this access key ID less than +// accessKeyRefreshInterval ago. A refresh never shuts down a cached +// backend, even if the proxy returns a different secret, so this is +// safe to do when a signature fails to verify in case the secret has +// been rotated. +// +// A cached answer older than accessKeyRevalidateInterval is always +// checked with the proxy so a revoked access key ID is refused even +// if it is in constant use. +// +// remoteAddr is the address of the client as returned by net.Addr.String(). +func (p *Proxy) CallAccessKey(accessKeyID, remoteAddr string, refresh bool) (VFS *vfs.VFS, secret string, err error) { + clientIP := ipFromAddr(remoteAddr) + cacheKey := generateCacheKey(accessKeyID, "", clientIP) + value, ok := p.vfsCache.GetMaybe(cacheKey) + if ok { + if entry, isEntry := value.(cacheEntry); isEntry { + age := time.Since(time.Unix(0, entry.refreshed.Load())) + switch { + case age >= accessKeyRevalidateInterval: + refresh = true + case age < accessKeyRefreshInterval: + refresh = false + } + } + } + if !ok || refresh { + value, err = p.call(accessKeyID, "", authAccessKey, clientIP) + if err != nil { + return nil, "", err + } + } + entry, ok := value.(cacheEntry) + if !ok { + return nil, "", fmt.Errorf("proxy: value is not cache entry: %#v", value) + } + return entry.vfs, entry.secret, nil +} + // Get VFS from the cache using key - returns nil if not found func (p *Proxy) Get(key string) *vfs.VFS { value, ok := p.vfsCache.GetMaybe(key) diff --git a/cmd/serve/proxy/proxy_code.go b/cmd/serve/proxy/proxy_code.go index cc703e209..4fe45508b 100644 --- a/cmd/serve/proxy/proxy_code.go +++ b/cmd/serve/proxy/proxy_code.go @@ -34,6 +34,25 @@ func main() { if out["_root"] == "" { out["_root"] = "" } + // S3 access key auth has neither pass nor public_key and needs + // the secret returned, unless the user asks for it to be omitted + // or empty. The secret's suffix can be changed to simulate a + // rotation and an access key ID can be revoked. + _, havePass := in["pass"] + _, havePublicKey := in["public_key"] + switch { + case havePass || havePublicKey || in["user"] == "nosecret": + case in["user"] == os.Getenv("RCLONE_TEST_PROXY_REVOKED"): + log.Fatalf("access key ID %q revoked", in["user"]) + case in["user"] == "emptysecret": + out["_secret_access_key"] = "" + default: + suffix := os.Getenv("RCLONE_TEST_PROXY_SECRET_SUFFIX") + if suffix == "" { + suffix = "-secret" + } + out["_secret_access_key"] = in["user"] + suffix + } json.NewEncoder(os.Stdout).Encode(&out) if err != nil { log.Fatal(err) diff --git a/cmd/serve/proxy/proxy_test.go b/cmd/serve/proxy/proxy_test.go index 5f347284e..411288773 100644 --- a/cmd/serve/proxy/proxy_test.go +++ b/cmd/serve/proxy/proxy_test.go @@ -7,12 +7,15 @@ import ( "crypto/sha256" "encoding/base64" "strings" + "sync" "testing" + "time" _ "github.com/rclone/rclone/backend/local" "github.com/rclone/rclone/fs" "github.com/rclone/rclone/fs/config/configmap" "github.com/rclone/rclone/fs/config/obscure" + "github.com/rclone/rclone/vfs" "github.com/rclone/rclone/vfs/vfscommon" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -118,7 +121,7 @@ func TestRun(t *testing.T) { defer p.vfsCache.Clear() passwordBytes := []byte(testPass) - value, err := p.call(testUser, testPass, false, testIP) + value, err := p.call(testUser, testPass, authPassword, testIP) require.NoError(t, err) entry, ok := value.(cacheEntry) require.True(t, ok) @@ -239,7 +242,7 @@ func TestRun(t *testing.T) { assert.Equal(t, 0, p.vfsCache.Entries()) defer p.vfsCache.Clear() - value, err := p.call(testUser, publicKeyString, true, testIP) + value, err := p.call(testUser, publicKeyString, authPublicKey, testIP) require.NoError(t, err) entry, ok := value.(cacheEntry) require.True(t, ok) @@ -328,6 +331,47 @@ func TestRun(t *testing.T) { }) } +// TestCallAccessKeyConcurrentRefresh checks that concurrent refreshes +// which all see a rotated secret end up sharing one backend and one +// cache entry rather than the later ones retiring the entry an +// earlier one created and returned. +func TestCallAccessKeyConcurrentRefresh(t *testing.T) { + opt := Opt + opt.AuthProxy = "go run proxy_code.go" + p := New(context.Background(), &opt, &vfscommon.Opt) + defer p.Shutdown() + const remoteAddr = "192.0.2.1:1234" + + oldInterval := accessKeyRefreshInterval + accessKeyRefreshInterval = 0 + defer func() { accessKeyRefreshInterval = oldInterval }() + + VFS, _, err := p.CallAccessKey("CONCURRENT", remoteAddr, false) + require.NoError(t, err) + + // Rotate the secret then refresh from many goroutines at once + t.Setenv("RCLONE_TEST_PROXY_SECRET_SUFFIX", "-rotated") + const n = 8 + results := make([]*vfs.VFS, n) + var wg sync.WaitGroup + for i := range n { + wg.Go(func() { + newVFS, secret, err := p.CallAccessKey("CONCURRENT", remoteAddr, true) + assert.NoError(t, err) + assert.Equal(t, "CONCURRENT-rotated", secret) + results[i] = newVFS + }) + } + wg.Wait() + + // Every caller must have got the same backend and only one + // entry was retired + for i := range n { + assert.Same(t, VFS, results[i], "goroutine %d got a different backend", i) + } + assert.Equal(t, 2, p.vfsCache.Entries()) +} + func TestIPFromAddr(t *testing.T) { for _, test := range []struct { in string @@ -347,3 +391,87 @@ func TestIPFromAddr(t *testing.T) { assert.Equal(t, test.want, ipFromAddr(test.in), test.in) } } + +func TestCallAccessKey(t *testing.T) { + opt := Opt + opt.AuthProxy = "go run proxy_code.go" + p := New(context.Background(), &opt, &vfscommon.Opt) + defer p.Shutdown() + const remoteAddr = "192.0.2.1:1234" + + // Disable refresh rate limiting for this test + oldInterval := accessKeyRefreshInterval + accessKeyRefreshInterval = 0 + defer func() { accessKeyRefreshInterval = oldInterval }() + + VFS, secret, err := p.CallAccessKey("AKID", remoteAddr, false) + require.NoError(t, err) + require.NotNil(t, VFS) + assert.Equal(t, "AKID-secret", secret) + + // Check the cached entry is returned on the next call + VFS2, secret2, err := p.CallAccessKey("AKID", remoteAddr, false) + require.NoError(t, err) + assert.Same(t, VFS, VFS2) + assert.Equal(t, secret, secret2) + + // Check a different access key ID gets a different backend + VFS3, secret3, err := p.CallAccessKey("OTHER", remoteAddr, false) + require.NoError(t, err) + assert.NotSame(t, VFS, VFS3) + assert.Equal(t, "OTHER-secret", secret3) + + // Check a refresh with an unchanged secret keeps the cached backend + VFS4, secret4, err := p.CallAccessKey("AKID", remoteAddr, true) + require.NoError(t, err) + assert.Same(t, VFS, VFS4) + assert.Equal(t, secret, secret4) + + // Check a refresh with a changed secret returns the new secret. + // The VFS is the same object as vfs.New shares a live VFS for the + // same backend and options, which keeps requests in flight under + // the old secret working. + t.Setenv("RCLONE_TEST_PROXY_SECRET_SUFFIX", "-rotated") + entries := p.vfsCache.Entries() + VFS5, secret5, err := p.CallAccessKey("AKID", remoteAddr, true) + require.NoError(t, err) + assert.Same(t, VFS, VFS5) + assert.Equal(t, "AKID-rotated", secret5) + // The old entry is retired rather than dropped + assert.Equal(t, entries+1, p.vfsCache.Entries()) + + // Check a proxy which doesn't return the secret is an error + _, _, err = p.CallAccessKey("nosecret", remoteAddr, false) + require.ErrorContains(t, err, "_secret_access_key not set") + + // Check a proxy which returns an empty secret is an error + _, _, err = p.CallAccessKey("emptysecret", remoteAddr, false) + require.ErrorContains(t, err, "_secret_access_key is empty") + + // Check refreshes are rate limited: with a long interval a + // refresh returns the cached secret without running the proxy + accessKeyRefreshInterval = time.Hour + t.Setenv("RCLONE_TEST_PROXY_SECRET_SUFFIX", "-rotated-again") + VFS6, secret6, err := p.CallAccessKey("AKID", remoteAddr, true) + require.NoError(t, err) + assert.Same(t, VFS5, VFS6) + assert.Equal(t, "AKID-rotated", secret6) + + // And with no interval the proxy is run and the rotation seen + accessKeyRefreshInterval = 0 + _, secret7, err := p.CallAccessKey("AKID", remoteAddr, true) + require.NoError(t, err) + assert.Equal(t, "AKID-rotated-again", secret7) + + // Check a cached entry is revalidated with the proxy once it is + // old enough even without a refresh being asked for + oldRevalidate := accessKeyRevalidateInterval + defer func() { accessKeyRevalidateInterval = oldRevalidate }() + t.Setenv("RCLONE_TEST_PROXY_REVOKED", "AKID") + _, secret8, err := p.CallAccessKey("AKID", remoteAddr, false) + require.NoError(t, err, "cached entry should still be trusted") + assert.Equal(t, "AKID-rotated-again", secret8) + accessKeyRevalidateInterval = 0 + _, _, err = p.CallAccessKey("AKID", remoteAddr, false) + require.ErrorContains(t, err, "revoked") +} diff --git a/cmd/serve/s3/s3.go b/cmd/serve/s3/s3.go index 94cb75ca5..5dd9df778 100644 --- a/cmd/serve/s3/s3.go +++ b/cmd/serve/s3/s3.go @@ -120,7 +120,7 @@ var Command = &cobra.Command{ }, Use: "s3 remote:path", Short: `Serve remote:path over s3.`, - Long: help() + strings.TrimSpace(httplib.AuthHelp(flagPrefix)+httplib.Help(flagPrefix)+vfs.Help()), + Long: help() + strings.TrimSpace(httplib.AuthHelp(flagPrefix)+httplib.Help(flagPrefix)+vfs.Help()+proxy.Help), RunE: func(command *cobra.Command, args []string) error { var f fs.Fs if proxy.Opt.AuthProxy == "" { diff --git a/cmd/serve/s3/s3_test.go b/cmd/serve/s3/s3_test.go index f789fc6e4..dbec07a8e 100644 --- a/cmd/serve/s3/s3_test.go +++ b/cmd/serve/s3/s3_test.go @@ -20,6 +20,7 @@ import ( v4 "github.com/aws/aws-sdk-go-v2/aws/signer/v4" "github.com/minio/minio-go/v7" "github.com/minio/minio-go/v7/pkg/credentials" + "github.com/rclone/gofakes3/signature" _ "github.com/rclone/rclone/backend/local" _ "github.com/rclone/rclone/backend/s3" // for TestS3Minio backing remote "github.com/rclone/rclone/cmd/serve/proxy" @@ -42,11 +43,19 @@ const ( ) // Configure and serve the server +// +// If f is nil the server is expected to be using an auth proxy, in +// which case the credentials are handed to the test proxy via the +// environment rather than --auth-key. func serveS3(t *testing.T, f fs.Fs) (testURL string, keyid string, keysec string, w *Server) { keyid = random.String(16) keysec = random.String(16) opt := Opt // copy default options - opt.AuthKey = []string{fmt.Sprintf("%s,%s", keyid, keysec)} + if f == nil { + t.Setenv("RCLONE_TEST_PROXY_AUTH_KEY", fmt.Sprintf("%s,%s", keyid, keysec)) + } else { + opt.AuthKey = []string{fmt.Sprintf("%s,%s", keyid, keysec)} + } opt.HTTP.ListenAddr = []string{endpoint} w, _ = newServer(context.Background(), f, &opt, &vfscommon.Opt, &proxy.Opt) go func() { @@ -286,9 +295,6 @@ func TestListBucketsAuthProxy(t *testing.T) { { description: "list buckets", bucket: "mybucket", - // request with random keyid - // instead of what was set in 'authPair' - keyID: random.String(16), files: []FileStuct{ { path: "", @@ -300,6 +306,12 @@ func TestListBucketsAuthProxy(t *testing.T) { }, }, }, + { + description: "list buckets: unknown s3 key", + bucket: "mybucket", + keyID: random.String(16), + shouldFail: true, + }, { description: "list buckets: wrong s3 secret", bucket: "mybucket", @@ -339,6 +351,85 @@ func TestNewServerPerServerAuthProxy(t *testing.T) { assert.Nil(t, w.provider.VFS(), "expected no fixed VFS when auth proxy is in use") } +// TestAuthProxyEmptySecret checks that a request for an arbitrary +// access key ID signed with an empty secret is refused when using an +// auth proxy without --auth-key. +func TestAuthProxyEmptySecret(t *testing.T) { + fstest.Initialise() + + prog, err := filepath.Abs("../servetest/proxy_code.go") + require.NoError(t, err) + files, err := filepath.Abs("testdata") + require.NoError(t, err) + proxy.Opt.AuthProxy = "go run " + prog + " " + files + defer func() { + proxy.Opt.AuthProxy = "" + }() + + endpoint, keyid, _, s := serveS3(t, nil) + defer func() { + assert.NoError(t, s.server.Shutdown()) + }() + + for _, accessKeyID := range []string{keyid, random.String(16)} { + req, err := http.NewRequest("GET", endpoint+"/", nil) + require.NoError(t, err) + req.Header.Set("X-Amz-Content-Sha256", "UNSIGNED-PAYLOAD") + signer := v4.NewSigner() + err = signer.SignHTTP(context.Background(), aws.Credentials{AccessKeyID: accessKeyID, SecretAccessKey: ""}, req, "UNSIGNED-PAYLOAD", "s3", "us-east-1", time.Now()) + require.NoError(t, err) + + resp, err := http.DefaultClient.Do(req) + require.NoError(t, err) + body, _ := io.ReadAll(resp.Body) + _ = resp.Body.Close() + assert.Equal(t, http.StatusForbidden, resp.StatusCode, string(body)) + assert.NotContains(t, string(body), "ListAllMyBucketsResult") + } +} + +// TestAuthProxyKeysNotRegistered checks that secrets supplied by the +// auth proxy are never registered in gofakes3's process wide key +// store where any other serve s3 instance in the process would honour +// them. +func TestAuthProxyKeysNotRegistered(t *testing.T) { + fstest.Initialise() + + prog, err := filepath.Abs("../servetest/proxy_code.go") + require.NoError(t, err) + files, err := filepath.Abs("testdata") + require.NoError(t, err) + proxy.Opt.AuthProxy = "go run " + prog + " " + files + defer func() { + proxy.Opt.AuthProxy = "" + }() + + endpoint, keyid, keysec, s := serveS3(t, nil) + defer func() { + assert.NoError(t, s.server.Shutdown()) + }() + + sign := func() *http.Request { + req, err := http.NewRequest("GET", endpoint+"/", nil) + require.NoError(t, err) + req.Header.Set("X-Amz-Content-Sha256", "UNSIGNED-PAYLOAD") + signer := v4.NewSigner() + err = signer.SignHTTP(context.Background(), aws.Credentials{AccessKeyID: keyid, SecretAccessKey: keysec}, req, "UNSIGNED-PAYLOAD", "s3", "us-east-1", time.Now()) + require.NoError(t, err) + return req + } + + // A correctly signed request is accepted by the server + resp, err := http.DefaultClient.Do(sign()) + require.NoError(t, err) + body, _ := io.ReadAll(resp.Body) + _ = resp.Body.Close() + assert.Equal(t, http.StatusOK, resp.StatusCode, string(body)) + + // But the key it used must not be in the global key store + assert.NotEqual(t, signature.ErrNone, signature.V4SignVerify(sign()), "proxy secret was registered in the gofakes3 key store") +} + // TestAuthKeyPerServer checks that two servers in the same process // with different --auth-key pairs only accept their own credentials. func TestAuthKeyPerServer(t *testing.T) { diff --git a/cmd/serve/s3/serve_s3.md b/cmd/serve/s3/serve_s3.md index 4ba573b88..495efb80a 100644 --- a/cmd/serve/s3/serve_s3.md +++ b/cmd/serve/s3/serve_s3.md @@ -13,6 +13,12 @@ docs](https://docs.aws.amazon.com/general/latest/gr/signature-version-4.html)). `--auth-key` is not provided then `serve s3` will allow anonymous access. +Alternatively `--auth-proxy` can be used to look up the secret for each +access key ID and choose the backend it maps to (see [Auth +Proxy](#auth-proxy) below). When an auth proxy is in use `--auth-key` +is ignored and every request must be signed with the secret the proxy +returns for its access key ID. + Like all rclone flags `--auth-key` can be set via environment variables, in this case `RCLONE_AUTH_KEY`. Since this flag can be repeated, the input to `RCLONE_AUTH_KEY` is CSV encoded. Because the diff --git a/cmd/serve/s3/server.go b/cmd/serve/s3/server.go index 6b5a58f74..eedf65c59 100644 --- a/cmd/serve/s3/server.go +++ b/cmd/serve/s3/server.go @@ -3,14 +3,11 @@ package s3 import ( "context" - "crypto/md5" - "encoding/hex" "errors" "fmt" "math/rand" "net" "net/http" - "strings" "time" "github.com/go-chi/chi/v5" @@ -40,7 +37,6 @@ type Server struct { backend *s3Backend handler http.Handler ctx context.Context // for global config - s3Secret string etagHashType hash.Type } @@ -76,14 +72,20 @@ func newServer(ctx context.Context, f fs.Fs, opt *Options, vfsOpt *vfscommon.Opt if len(opt.AuthKey) == 0 && !w.provider.IsProxy() { fs.Logf("serve s3", "No auth provided so allowing anonymous access") - } else if len(opt.AuthKey) > 0 { - w.s3Secret = getAuthSecret(opt.AuthKey) } authList, err := authlistResolver(opt.AuthKey) if err != nil { return nil, fmt.Errorf("parsing auth list failed: %q", err) } + if w.provider.IsProxy() { + // The proxy middleware authenticates every request itself so + // gofakes3's own auth must be left empty. + if len(authList) > 0 { + fs.Logf("serve s3", "--auth-key is ignored when --auth-proxy is set - the proxy must supply the secret for each access key ID") + } + authList = nil + } w.backend = newBackend(w) if w.opt.MultipartExpiry > 0 { @@ -104,9 +106,7 @@ func newServer(ctx context.Context, f fs.Fs, opt *Options, vfsOpt *vfscommon.Opt w.handler = w.faker.Server() if w.provider.IsProxy() { - // proxy auth middleware w.handler = proxyAuthMiddleware(w.handler, w) - w.handler = authPairMiddleware(w.handler, w) } else if len(opt.AuthKey) > 0 { w.faker.AddAuthKeys(authList) } @@ -142,13 +142,41 @@ func (w *Server) getVFS(ctx context.Context) (VFS *vfs.VFS, err error) { return VFS, nil } -// auth does proxy authorization -func (w *Server) auth(r *http.Request, accessKeyID string) (value any, err error) { - VFS, _, err := w.provider.Proxy().Call(stringToMd5Hash(accessKeyID), accessKeyID, false, r.RemoteAddr) +// auth authenticates the request via the auth proxy. +// +// The proxy maps the access key ID to a VFS and a secret access key +// and the request's signature is verified against that secret. If it +// fails against a cached secret the proxy is consulted again in case +// the secret has been rotated. +// +// The secret is only ever used here and is never registered with +// gofakes3, whose key store is shared by every instance in the +// process. +func (w *Server) auth(r *http.Request, accessKeyID string) (VFS *vfs.VFS, err error) { + p := w.provider.Proxy() + VFS, secret, err := p.CallAccessKey(accessKeyID, r.RemoteAddr, false) if err != nil { return nil, err } - return VFS, err + errCode := signature.V4SignVerifyWithSecret(r, secret) + if errCode == signature.ErrNone { + return VFS, nil + } + // Only a signature mismatch can be cured by a rotated secret, so + // only then is the proxy worth consulting again - other failures + // (expired request, bad date, missing headers) must not make + // every bad request run the proxy. + if signature.GetAPIError(errCode).Code == "SignatureDoesNotMatch" { + VFS, secret, err = p.CallAccessKey(accessKeyID, r.RemoteAddr, true) + if err != nil { + return nil, err + } + errCode = signature.V4SignVerifyWithSecret(r, secret) + if errCode == signature.ErrNone { + return VFS, nil + } + } + return nil, fmt.Errorf("signature verification failed: %s", signature.GetAPIError(errCode).Code) } // Bind register the handler to http.Router @@ -177,59 +205,49 @@ func (w *Server) Shutdown() error { return err } -func authPairMiddleware(next http.Handler, ws *Server) http.Handler { - return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - accessKey, _ := parseAccessKeyID(r) - // set the auth pair - authPair := map[string]string{ - accessKey: ws.s3Secret, - } - ws.faker.AddAuthKeys(authPair) - next.ServeHTTP(w, r) - }) -} - +// proxyAuthMiddleware authenticates each request via the auth proxy, +// storing the VFS it returns in the request context, and refuses +// requests the proxy does not accept. 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(r, accessKey) + accessKey, errCode := parseAccessKeyID(r) + if errCode != signature.ErrNone { + fs.Infof(r.URL.Path, "%s: Auth failed: no access key ID in request", r.RemoteAddr) + accessDenied(w) + return + } + VFS, err := ws.auth(r, accessKey) if err != nil { fs.Infof(r.URL.Path, "%s: Auth failed: %v", r.RemoteAddr, err) + accessDenied(w) + return } - if value != nil { - r = r.WithContext(context.WithValue(r.Context(), ctxKeyID, value)) - } - + r = r.WithContext(context.WithValue(r.Context(), ctxKeyID, VFS)) next.ServeHTTP(w, r) }) } -func parseAccessKeyID(r *http.Request) (accessKey string, error signature.ErrorCode) { - v4Auth := r.Header.Get("Authorization") - req, err := signature.ParseSignV4(v4Auth) - if err != signature.ErrNone { - return "", err - } +// accessDenied writes an S3 AccessDenied error response +func accessDenied(w http.ResponseWriter) { + w.Header().Set("Content-Type", "application/xml") + w.WriteHeader(http.StatusForbidden) + _, _ = w.Write([]byte(`AccessDeniedAccess Denied`)) +} +// parseAccessKeyID returns the access key ID from the request's +// Authorization header or presigned URL query parameters +func parseAccessKeyID(r *http.Request) (accessKey string, errCode signature.ErrorCode) { + v4Auth := r.Header.Get("Authorization") + if v4Auth == "" { + // Presigned URLs carry the credential in the query string + q := r.URL.Query() + if q.Get("X-Amz-Signature") != "" { + v4Auth = fmt.Sprintf("%s Credential=%s, SignedHeaders=%s, Signature=%s", q.Get("X-Amz-Algorithm"), q.Get("X-Amz-Credential"), q.Get("X-Amz-SignedHeaders"), q.Get("X-Amz-Signature")) + } + } + req, errCode := signature.ParseSignV4(v4Auth) + if errCode != signature.ErrNone { + return "", errCode + } return req.Credential.GetAccessKey(), signature.ErrNone } - -func stringToMd5Hash(s string) string { - hasher := md5.New() - hasher.Write([]byte(s)) - return hex.EncodeToString(hasher.Sum(nil)) -} - -func getAuthSecret(authPair []string) string { - if len(authPair) == 0 { - return "" - } - - splited := strings.Split(authPair[0], ",") - if len(splited) != 2 { - return "" - } - - secret := strings.TrimSpace(splited[1]) - return secret -} diff --git a/cmd/serve/servetest/proxy_code.go b/cmd/serve/servetest/proxy_code.go index 25499b6bc..10e9a3f98 100644 --- a/cmd/serve/servetest/proxy_code.go +++ b/cmd/serve/servetest/proxy_code.go @@ -1,12 +1,18 @@ //go:build ignore // A simple auth proxy for testing purposes +// +// For S3 access key auth (no "pass" or "public_key" in the input) the +// access key ID and secret are read from the environment variable +// RCLONE_TEST_PROXY_AUTH_KEY as "accessKeyID,secretAccessKey" and any +// other access key ID is refused. package main import ( "encoding/json" "log" "os" + "strings" ) func main() { @@ -28,6 +34,19 @@ func main() { "_root": root, "_obscure": "pass", } + + _, havePass := in["pass"] + _, havePublicKey := in["public_key"] + if !havePass && !havePublicKey { + accessKeyID, secret, ok := strings.Cut(os.Getenv("RCLONE_TEST_PROXY_AUTH_KEY"), ",") + if !ok { + log.Fatal("RCLONE_TEST_PROXY_AUTH_KEY not set") + } + if in["user"] != accessKeyID { + log.Fatalf("unknown access key ID %q", in["user"]) + } + out["_secret_access_key"] = secret + } json.NewEncoder(os.Stdout).Encode(&out) if err != nil { log.Fatal(err)