diff --git a/cmd/serve/proxy/proxy.go b/cmd/serve/proxy/proxy.go index dc4fce848..88d4327a3 100644 --- a/cmd/serve/proxy/proxy.go +++ b/cmd/serve/proxy/proxy.go @@ -3,6 +3,8 @@ package proxy import ( "bytes" + "crypto/sha256" + "crypto/subtle" "encoding/json" "os/exec" "strings" @@ -16,7 +18,6 @@ import ( libcache "github.com/rclone/rclone/lib/cache" "github.com/rclone/rclone/vfs" "github.com/rclone/rclone/vfs/vfsflags" - "golang.org/x/crypto/bcrypt" ) // Help contains text describing how to use the proxy @@ -107,8 +108,8 @@ type Proxy struct { // cacheEntry is what is stored in the vfsCache type cacheEntry struct { - vfs *vfs.VFS // stored VFS - pwHash []byte // bcrypt hash of the password/publicKey + vfs *vfs.VFS // stored VFS + pwHash [sha256.Size]byte // sha256 hash of the password/publicKey } // New creates a new proxy with the Options passed in @@ -218,16 +219,12 @@ func (p *Proxy) call(user, auth string, isPublicKey bool) (value interface{}, er return nil, false, err } - // The bcrypt cost is a compromise between security and speed. The password is looked up on every - // transaction for WebDAV so we store it lightly hashed. An attacker would find it easier to go after - // the unencrypted password in memory most likely. - pwHash, err := bcrypt.GenerateFromPassword([]byte(auth), bcrypt.MinCost) - if err != nil { - return nil, false, err - } + // We hash the auth here so we don't copy the auth more than we + // 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(f, &vfsflags.Opt), - pwHash: pwHash, + pwHash: sha256.Sum256([]byte(auth)), } return entry, true, nil }) @@ -262,9 +259,12 @@ func (p *Proxy) Call(user, auth string, isPublicKey bool) (VFS *vfs.VFS, vfsKey // user don't have their auth checked. It does mean that if // the password is changed, the user will have to wait for // cache expiry (5m) before trying again. - err = bcrypt.CompareHashAndPassword(entry.pwHash, []byte(auth)) - if err != nil { - return nil, "", errors.Wrap(err, "proxy: incorrect password / public key") + authHash := sha256.Sum256([]byte(auth)) + if subtle.ConstantTimeCompare(authHash[:], entry.pwHash[:]) != 1 { + if isPublicKey { + return nil, "", errors.New("proxy: incorrect public key") + } + return nil, "", errors.New("proxy: incorrect password") } return entry.vfs, user, nil diff --git a/cmd/serve/proxy/proxy_test.go b/cmd/serve/proxy/proxy_test.go index 5dd417e89..5e52e4b11 100644 --- a/cmd/serve/proxy/proxy_test.go +++ b/cmd/serve/proxy/proxy_test.go @@ -3,6 +3,7 @@ package proxy import ( "crypto/rand" "crypto/rsa" + "crypto/sha256" "encoding/base64" "log" "strings" @@ -13,7 +14,6 @@ import ( "github.com/rclone/rclone/fs/config/obscure" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" - "golang.org/x/crypto/bcrypt" "golang.org/x/crypto/ssh" ) @@ -85,8 +85,7 @@ func TestRun(t *testing.T) { require.True(t, ok) // check hash is correct in entry - err = bcrypt.CompareHashAndPassword(entry.pwHash, passwordBytes) - require.NoError(t, err) + assert.Equal(t, entry.pwHash, sha256.Sum256(passwordBytes)) require.NotNil(t, entry.vfs) f := entry.vfs.Fs() require.NotNil(t, f)