mirror of
https://github.com/rclone/rclone.git
synced 2025-01-11 00:40:03 +01:00
proxy: replace use of bcrypt with sha256
Unfortunately bcrypt only hashes the first 72 bytes of a given input which meant that using it on ssh keys which are longer than 72 bytes was incorrect. This swaps over to using sha256 which should be adequate for the purpose of protecting in memory passwords where the unencrypted password is likely in memory too.
This commit is contained in:
parent
f2a789ea98
commit
b88dec51e5
@ -3,6 +3,8 @@ package proxy
|
|||||||
|
|
||||||
import (
|
import (
|
||||||
"bytes"
|
"bytes"
|
||||||
|
"crypto/sha256"
|
||||||
|
"crypto/subtle"
|
||||||
"encoding/json"
|
"encoding/json"
|
||||||
"os/exec"
|
"os/exec"
|
||||||
"strings"
|
"strings"
|
||||||
@ -16,7 +18,6 @@ import (
|
|||||||
libcache "github.com/rclone/rclone/lib/cache"
|
libcache "github.com/rclone/rclone/lib/cache"
|
||||||
"github.com/rclone/rclone/vfs"
|
"github.com/rclone/rclone/vfs"
|
||||||
"github.com/rclone/rclone/vfs/vfsflags"
|
"github.com/rclone/rclone/vfs/vfsflags"
|
||||||
"golang.org/x/crypto/bcrypt"
|
|
||||||
)
|
)
|
||||||
|
|
||||||
// Help contains text describing how to use the proxy
|
// Help contains text describing how to use the proxy
|
||||||
@ -108,7 +109,7 @@ type Proxy struct {
|
|||||||
// cacheEntry is what is stored in the vfsCache
|
// cacheEntry is what is stored in the vfsCache
|
||||||
type cacheEntry struct {
|
type cacheEntry struct {
|
||||||
vfs *vfs.VFS // stored VFS
|
vfs *vfs.VFS // stored VFS
|
||||||
pwHash []byte // bcrypt hash of the password/publicKey
|
pwHash [sha256.Size]byte // sha256 hash of the password/publicKey
|
||||||
}
|
}
|
||||||
|
|
||||||
// New creates a new proxy with the Options passed in
|
// 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
|
return nil, false, err
|
||||||
}
|
}
|
||||||
|
|
||||||
// The bcrypt cost is a compromise between security and speed. The password is looked up on every
|
// We hash the auth here so we don't copy the auth more than we
|
||||||
// transaction for WebDAV so we store it lightly hashed. An attacker would find it easier to go after
|
// need to in memory. An attacker would find it easier to go
|
||||||
// the unencrypted password in memory most likely.
|
// after the unencrypted password in memory most likely.
|
||||||
pwHash, err := bcrypt.GenerateFromPassword([]byte(auth), bcrypt.MinCost)
|
|
||||||
if err != nil {
|
|
||||||
return nil, false, err
|
|
||||||
}
|
|
||||||
entry := cacheEntry{
|
entry := cacheEntry{
|
||||||
vfs: vfs.New(f, &vfsflags.Opt),
|
vfs: vfs.New(f, &vfsflags.Opt),
|
||||||
pwHash: pwHash,
|
pwHash: sha256.Sum256([]byte(auth)),
|
||||||
}
|
}
|
||||||
return entry, true, nil
|
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
|
// user don't have their auth checked. It does mean that if
|
||||||
// the password is changed, the user will have to wait for
|
// the password is changed, the user will have to wait for
|
||||||
// cache expiry (5m) before trying again.
|
// cache expiry (5m) before trying again.
|
||||||
err = bcrypt.CompareHashAndPassword(entry.pwHash, []byte(auth))
|
authHash := sha256.Sum256([]byte(auth))
|
||||||
if err != nil {
|
if subtle.ConstantTimeCompare(authHash[:], entry.pwHash[:]) != 1 {
|
||||||
return nil, "", errors.Wrap(err, "proxy: incorrect password / public key")
|
if isPublicKey {
|
||||||
|
return nil, "", errors.New("proxy: incorrect public key")
|
||||||
|
}
|
||||||
|
return nil, "", errors.New("proxy: incorrect password")
|
||||||
}
|
}
|
||||||
|
|
||||||
return entry.vfs, user, nil
|
return entry.vfs, user, nil
|
||||||
|
@ -3,6 +3,7 @@ package proxy
|
|||||||
import (
|
import (
|
||||||
"crypto/rand"
|
"crypto/rand"
|
||||||
"crypto/rsa"
|
"crypto/rsa"
|
||||||
|
"crypto/sha256"
|
||||||
"encoding/base64"
|
"encoding/base64"
|
||||||
"log"
|
"log"
|
||||||
"strings"
|
"strings"
|
||||||
@ -13,7 +14,6 @@ import (
|
|||||||
"github.com/rclone/rclone/fs/config/obscure"
|
"github.com/rclone/rclone/fs/config/obscure"
|
||||||
"github.com/stretchr/testify/assert"
|
"github.com/stretchr/testify/assert"
|
||||||
"github.com/stretchr/testify/require"
|
"github.com/stretchr/testify/require"
|
||||||
"golang.org/x/crypto/bcrypt"
|
|
||||||
"golang.org/x/crypto/ssh"
|
"golang.org/x/crypto/ssh"
|
||||||
)
|
)
|
||||||
|
|
||||||
@ -85,8 +85,7 @@ func TestRun(t *testing.T) {
|
|||||||
require.True(t, ok)
|
require.True(t, ok)
|
||||||
|
|
||||||
// check hash is correct in entry
|
// check hash is correct in entry
|
||||||
err = bcrypt.CompareHashAndPassword(entry.pwHash, passwordBytes)
|
assert.Equal(t, entry.pwHash, sha256.Sum256(passwordBytes))
|
||||||
require.NoError(t, err)
|
|
||||||
require.NotNil(t, entry.vfs)
|
require.NotNil(t, entry.vfs)
|
||||||
f := entry.vfs.Fs()
|
f := entry.vfs.Fs()
|
||||||
require.NotNil(t, f)
|
require.NotNil(t, f)
|
||||||
|
Loading…
Reference in New Issue
Block a user