From b88dec51e51d9f0bf5c7f5e749e558255b31be18 Mon Sep 17 00:00:00 2001
From: Nick Craig-Wood <nick@craig-wood.com>
Date: Sun, 12 Jan 2020 11:36:39 +0000
Subject: [PATCH] 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.
---
 cmd/serve/proxy/proxy.go      | 28 ++++++++++++++--------------
 cmd/serve/proxy/proxy_test.go |  5 ++---
 2 files changed, 16 insertions(+), 17 deletions(-)

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)