Repository: rclone/rclone
Pull request #4107 (2020-04-04 15:12:58)

Box: Refresh token when using jwt auth - Fixes #4901 
#### What is the purpose of this change?

Refresh the auth token when using box, jwt and box config.json file

#### Was the change discussed in an issue or in the forum before?

[#4091 ](https://github.com/rclone/rclone/issues/4091)

#### Checklist

- [x] I have read the [contribution guidelines](https://github.com/rclone/rclone/blob/master/CONTRIBUTING.md#submitting-a-pull-request).
- [x] I have added tests for all changes in this PR if appropriate.
- [x] I have added documentation for the changes if appropriate.
- [x] All commit messages are in [house style](https://github.com/rclone/rclone/blob/master/CONTRIBUTING.md#commit-messages).
- [x] I'm done, this Pull Request is ready for review :-)


dbramwell: @ncw I wanted to just use the Renewer from oauthutil, passing a different function to do the renew. But then I realised that the renewer in there only runs when [uploads are pending](https://github.com/rclone/rclone/blob/master/lib/oauthutil/renew.go#L41), so when running a simple `ls`, rclone still failed to refresh the token. So I created a new renewer that does it whether there are uploads pending or not. I'm not sure that this is the correct thing to do but it works.

I'm unsure where the token currently get's refreshed in `ls` cases (seemed to be somewhere in the `pacer` calls [here](https://github.com/rclone/rclone/blob/master/backend/box/box.go#L547). Maybe it's better for that to be altered for jwt and the `jwtutil.Renew` to be removed? I'm unsure about how it all works though.
ncw: Apologies for the delay in looking at this.

> I'm unsure where the token currently get's refreshed in `ls` cases (seemed to be somewhere in the `pacer` calls [here](https://github.com/rclone/rclone/blob/master/backend/box/box.go#L547). Maybe it's better for that to be altered for jwt and the `jwtutil.Renew` to be removed? I'm unsure about how it all works though.

The token gets renewed by the oauth library golang.org/x/oauth2 - you use its http Transport and it magically refreshes the token when it expires.

The oauth library may be able to help you directly - it understands JWTs (unlike me :-)

> So I created a new renewer that does it whether there are uploads pending or not. I'm not sure that this is the correct thing to do but it works.

That will only work for uploads though as that is the only place we call the token renewer at the moment.

You could hook it into the Signer call for the rest library which is called before every HTTP transaction

https://github.com/rclone/rclone/blob/6893ce0bbf81f9d4264f12af3e62f21ae7ba5ced/backend/sugarsync/sugarsync.go#L422 (uploaded in 15-30 mins)

You have to be a little careful about recursion - I should probably add a "NoSigner" option to `rest.Opts`.

The most efficient hook might be to add a timer on refresh of the token to when it next needs refreshing since we know exactly when it expires. We should examine the expiry on startup and refresh if necessary then or set the timer, then set it again on every refresh.
dbramwell: No problem, thanks for taking a look.

> That will only work for uploads though as that is the only place we call the token renewer at the moment.

I removed the check for whether uploads are taking place in the jwt renewer [here](https://github.com/rclone/rclone/pull/4107/files#diff-e57f15e014bec6db19b4be39ba7eff78R41). It does work, but I don't like the idea of kicking off an async process to renew the token when running one short command like `ls` or whatever.

I'm still looking at this when I get time, currently hoping to use `golang.org/x/oauth2` like you said. I think I tried this last time but hit an issue with it [not supporting keys with a passphrase](https://github.com/rclone/rclone/blob/master/vendor/golang.org/x/oauth2/jwt/jwt.go#L42). Will see where I get to and hopefully have changes by the weekend. 
ncw: > > That will only work for uploads though as that is the only place we call the token renewer at the moment.
> 
> I removed the check for whether uploads are taking place in the jwt renewer [here](https://github.com/rclone/rclone/pull/4107/files#diff-e57f15e014bec6db19b4be39ba7eff78R41), compared to the oauthutil one [here](https://github.com/rclone/rclone/blob/master/lib/oauthutil/renew.go#L41). It does work, but I don't like the idea of kicking off an async process to renew the token when running one short command like `ls` or whatever.

You are right. I missed the fact that we kick off a go routine to received the expired notification so ignore what I said above, sorry!

Maybe the neatest thing to do would be to add a mode to the existing token renewer so that it always renews the token when it expires. That could be as simple as calling `Start()` as part of the initialisation (so uploads is always > 0) with no code changes needed which should simplify the code and mean that we don't need to duplicate the token renewer code.

> I'm still looking at this when I get time, currently hoping to use `golang.org/x/oauth2` like you said. I think I tried this last time but hit an issue with it [not supporting keys with a passphrase](https://github.com/rclone/rclone/blob/master/vendor/golang.org/x/oauth2/jwt/jwt.go#L42). Will see where I get to and hopefully have changes by the weekend.

Thanks
dbramwell: > Maybe the neatest thing to do would be to add a mode to the existing token renewer so that it always renews the token when it expires. That could be as simple as calling Start() as part of the initialisation (so uploads is always > 0) with no code changes needed which should simplify the code and mean that we don't need to duplicate the token renewer code.

Cool, I've taken that approach.

I took a look into using the jwt stuff in the oauth2 package and eventually hit the issue of not being able to set query params which will be solved by [this](https://github.com/golang/oauth2/pull/390/files), which once I saw I realised I'd definitely seen when I was trying to add the jwt stuff in the first place, I should have taken note! Once that's merged I think a lot my code from the Config function in jwtutil can be removed, and the renewer stuff could possibly be simplified too.
ncw: > Cool, I've taken that approach.

Rather than changing the interface of the token renewer, I'd I've just called `Start()` from the box backend. I'm sorry about my stream of consciousness ramble above - I should have been more explicit!

> I took a look into using the jwt stuff in the oauth2 package and eventually hit the issue of not being able to set query params which will be solved by [this](https://github.com/golang/oauth2/pull/390/files), which once I saw I realised I'd definitely seen when I was trying to add the jwt stuff in the first place, I should have taken note! Once that's merged I think a lot my code from the Config function in jwtutil can be removed, and the renewer stuff could possibly be simplified too.

That is good news! I put a note on the PR asking for a status check.
dbramwell: > Rather than changing the interface of the token renewer, I'd I've just called Start() from the box backend. I'm sorry about my stream of consciousness ramble above - I should have been more explicit!

No problem. Done!

I'm not sure why the two actions are failing, running on my local I don't seem to get errors.

Patch:
diff --git a/backend/box/box.go b/backend/box/box.go
index 520119c6a..6d687361c 100644
--- a/backend/box/box.go
+++ b/backend/box/box.go
@@ -89,22 +89,7 @@ func init() {
 			boxSubType, boxSubTypeOk := m.Get("box_sub_type")
 			var err error
 			if ok && boxSubTypeOk && jsonFile != "" && boxSubType != "" {
-				boxConfig, err := getBoxConfig(jsonFile)
-				if err != nil {
-					log.Fatalf("Failed to configure token: %v", err)
-				}
-				privateKey, err := getDecryptedPrivateKey(boxConfig)
-				if err != nil {
-					log.Fatalf("Failed to configure token: %v", err)
-				}
-				claims, err := getClaims(boxConfig, boxSubType)
-				if err != nil {
-					log.Fatalf("Failed to configure token: %v", err)
-				}
-				signingHeaders := getSigningHeaders(boxConfig)
-				queryParams := getQueryParams(boxConfig)
-				client := fshttp.NewClient(fs.Config)
-				err = jwtutil.Config("box", name, claims, signingHeaders, queryParams, privateKey, m, client)
+				err = refreshJWTToken(jsonFile, boxSubType, name, m)
 				if err != nil {
 					log.Fatalf("Failed to configure token with jwt authentication: %v", err)
 				}
@@ -163,6 +148,26 @@ func init() {
 	})
 }
 
+func refreshJWTToken(jsonFile string, boxSubType string, name string, m configmap.Mapper) error {
+	boxConfig, err := getBoxConfig(jsonFile)
+	if err != nil {
+		log.Fatalf("Failed to configure token: %v", err)
+	}
+	privateKey, err := getDecryptedPrivateKey(boxConfig)
+	if err != nil {
+		log.Fatalf("Failed to configure token: %v", err)
+	}
+	claims, err := getClaims(boxConfig, boxSubType)
+	if err != nil {
+		log.Fatalf("Failed to configure token: %v", err)
+	}
+	signingHeaders := getSigningHeaders(boxConfig)
+	queryParams := getQueryParams(boxConfig)
+	client := fshttp.NewClient(fs.Config)
+	err = jwtutil.Config("box", name, claims, signingHeaders, queryParams, privateKey, m, client)
+	return err
+}
+
 func getBoxConfig(configFile string) (boxConfig *api.ConfigJSON, err error) {
 	file, err := ioutil.ReadFile(configFile)
 	if err != nil {
@@ -393,11 +398,24 @@ func NewFs(name, root string, m configmap.Mapper) (fs.Fs, error) {
 	}).Fill(f)
 	f.srv.SetErrorHandler(errorHandler)
 
-	// Renew the token in the background
-	f.tokenRenewer = oauthutil.NewRenew(f.String(), ts, func() error {
-		_, err := f.readMetaDataForPath(ctx, "")
-		return err
-	})
+	jsonFile, ok := m.Get("box_config_file")
+	boxSubType, boxSubTypeOk := m.Get("box_sub_type")
+
+	// If using box config.json and JWT, renewing should just refresh the token and
+	// should do so whether there are uploads pending or not.
+	if ok && boxSubTypeOk && jsonFile != "" && boxSubType != "" {
+		f.tokenRenewer = oauthutil.NewRenew(f.String(), ts, func() error {
+			err := refreshJWTToken(jsonFile, boxSubType, name, m)
+			return err
+		})
+		f.tokenRenewer.Start()
+	} else {
+		// Renew the token in the background
+		f.tokenRenewer = oauthutil.NewRenew(f.String(), ts, func() error {
+			_, err := f.readMetaDataForPath(ctx, "")
+			return err
+		})
+	}
 
 	// Get rootID
 	f.dirCache = dircache.New(root, rootID, f)
diff --git a/backend/crypt/cipher.go b/backend/crypt/cipher.go
index afd107df3..5213a0c67 100644
--- a/backend/crypt/cipher.go
+++ b/backend/crypt/cipher.go
@@ -71,30 +71,6 @@ type ReadSeekCloser interface {
 // OpenRangeSeek opens the file handle at the offset with the limit given
 type OpenRangeSeek func(ctx context.Context, offset, limit int64) (io.ReadCloser, error)
 
-// Cipher is used to swap out the encryption implementations
-type Cipher interface {
-	// EncryptFileName encrypts a file path
-	EncryptFileName(string) string
-	// DecryptFileName decrypts a file path, returns error if decrypt was invalid
-	DecryptFileName(string) (string, error)
-	// EncryptDirName encrypts a directory path
-	EncryptDirName(string) string
-	// DecryptDirName decrypts a directory path, returns error if decrypt was invalid
-	DecryptDirName(string) (string, error)
-	// EncryptData
-	EncryptData(io.Reader) (io.Reader, error)
-	// DecryptData
-	DecryptData(io.ReadCloser) (io.ReadCloser, error)
-	// DecryptDataSeek decrypt at a given position
-	DecryptDataSeek(ctx context.Context, open OpenRangeSeek, offset, limit int64) (ReadSeekCloser, error)
-	// EncryptedSize calculates the size of the data when encrypted
-	EncryptedSize(int64) int64
-	// DecryptedSize calculates the size of the data when decrypted
-	DecryptedSize(int64) (int64, error)
-	// NameEncryptionMode returns the used mode for name handling
-	NameEncryptionMode() NameEncryptionMode
-}
-
 // NameEncryptionMode is the type of file name encryption in use
 type NameEncryptionMode int
 
@@ -136,7 +112,8 @@ func (mode NameEncryptionMode) String() (out string) {
 	return out
 }
 
-type cipher struct {
+// Cipher defines an encoding and decoding cipher for the crypt backend
+type Cipher struct {
 	dataKey        [32]byte                  // Key for secretbox
 	nameKey        [32]byte                  // 16,24 or 32 bytes
 	nameTweak      [nameCipherBlockSize]byte // used to tweak the name crypto
@@ -148,8 +125,8 @@ type cipher struct {
 }
 
 // newCipher initialises the cipher.  If salt is "" then it uses a built in salt val
-func newCipher(mode NameEncryptionMode, password, salt string, dirNameEncrypt bool) (*cipher, error) {
-	c := &cipher{
+func newCipher(mode NameEncryptionMode, password, salt string, dirNameEncrypt bool) (*Cipher, error) {
+	c := &Cipher{
 		mode:           mode,
 		cryptoRand:     rand.Reader,
 		dirNameEncrypt: dirNameEncrypt,
@@ -172,7 +149,7 @@ func newCipher(mode NameEncryptionMode, password, salt string, dirNameEncrypt bo
 //
 // Note that empty passsword makes all 0x00 keys which is used in the
 // tests.
-func (c *cipher) Key(password, salt string) (err error) {
+func (c *Cipher) Key(password, salt string) (err error) {
 	const keySize = len(c.dataKey) + len(c.nameKey) + len(c.nameTweak)
 	var saltBytes = defaultSalt
 	if salt != "" {
@@ -196,12 +173,12 @@ func (c *cipher) Key(password, salt string) (err error) {
 }
 
 // getBlock gets a block from the pool of size blockSize
-func (c *cipher) getBlock() []byte {
+func (c *Cipher) getBlock() []byte {
 	return c.buffers.Get().([]byte)
 }
 
 // putBlock returns a block to the pool of size blockSize
-func (c *cipher) putBlock(buf []byte) {
+func (c *Cipher) putBlock(buf []byte) {
 	if len(buf) != blockSize {
 		panic("bad blocksize returned to pool")
 	}
@@ -246,7 +223,7 @@ func decodeFileName(in string) ([]byte, error) {
 // This means that
 //  * filenames with the same name will encrypt the same
 //  * filenames which start the same won't have a common prefix
-func (c *cipher) encryptSegment(plaintext string) string {
+func (c *Cipher) encryptSegment(plaintext string) string {
 	if plaintext == "" {
 		return ""
 	}
@@ -256,7 +233,7 @@ func (c *cipher) encryptSegment(plaintext string) string {
 }
 
 // decryptSegment decrypts a path segment
-func (c *cipher) decryptSegment(ciphertext string) (string, error) {
+func (c *Cipher) decryptSegment(ciphertext string) (string, error) {
 	if ciphertext == "" {
 		return "", nil
 	}
@@ -283,7 +260,7 @@ func (c *cipher) decryptSegment(ciphertext string) (string, error) {
 }
 
 // Simple obfuscation routines
-func (c *cipher) obfuscateSegment(plaintext string) string {
+func (c *Cipher) obfuscateSegment(plaintext string) string {
 	if plaintext == "" {
 		return ""
 	}
@@ -370,7 +347,7 @@ func (c *cipher) obfuscateSegment(plaintext string) string {
 	return result.String()
 }
 
-func (c *cipher) deobfuscateSegment(ciphertext string) (string, error) {
+func (c *Cipher) deobfuscateSegment(ciphertext string) (string, error) {
 	if ciphertext == "" {
 		return "", nil
 	}
@@ -457,7 +434,7 @@ func (c *cipher) deobfuscateSegment(ciphertext string) (string, error) {
 }
 
 // encryptFileName encrypts a file path
-func (c *cipher) encryptFileName(in string) string {
+func (c *Cipher) encryptFileName(in string) string {
 	segments := strings.Split(in, "/")
 	for i := range segments {
 		// Skip directory name encryption if the user chose to
@@ -475,7 +452,7 @@ func (c *cipher) encryptFileName(in string) string {
 }
 
 // EncryptFileName encrypts a file path
-func (c *cipher) EncryptFileName(in string) string {
+func (c *Cipher) EncryptFileName(in string) string {
 	if c.mode == NameEncryptionOff {
 		return in + encryptedSuffix
 	}
@@ -483,7 +460,7 @@ func (c *cipher) EncryptFileName(in string) string {
 }
 
 // EncryptDirName encrypts a directory path
-func (c *cipher) EncryptDirName(in string) string {
+func (c *Cipher) EncryptDirName(in string) string {
 	if c.mode == NameEncryptionOff || !c.dirNameEncrypt {
 		return in
 	}
@@ -491,7 +468,7 @@ func (c *cipher) EncryptDirName(in string) string {
 }
 
 // decryptFileName decrypts a file path
-func (c *cipher) decryptFileName(in string) (string, error) {
+func (c *Cipher) decryptFileName(in string) (string, error) {
 	segments := strings.Split(in, "/")
 	for i := range segments {
 		var err error
@@ -514,7 +491,7 @@ func (c *cipher) decryptFileName(in string) (string, error) {
 }
 
 // DecryptFileName decrypts a file path
-func (c *cipher) DecryptFileName(in string) (string, error) {
+func (c *Cipher) DecryptFileName(in string) (string, error) {
 	if c.mode == NameEncryptionOff {
 		remainingLength := len(in) - len(encryptedSuffix)
 		if remainingLength > 0 && strings.HasSuffix(in, encryptedSuffix) {
@@ -526,14 +503,15 @@ func (c *cipher) DecryptFileName(in string) (string, error) {
 }
 
 // DecryptDirName decrypts a directory path
-func (c *cipher) DecryptDirName(in string) (string, error) {
+func (c *Cipher) DecryptDirName(in string) (string, error) {
 	if c.mode == NameEncryptionOff || !c.dirNameEncrypt {
 		return in, nil
 	}
 	return c.decryptFileName(in)
 }
 
-func (c *cipher) NameEncryptionMode() NameEncryptionMode {
+// NameEncryptionMode returns the encryption mode in use for names
+func (c *Cipher) NameEncryptionMode() NameEncryptionMode {
 	return c.mode
 }
 
@@ -601,7 +579,7 @@ func (n *nonce) add(x uint64) {
 type encrypter struct {
 	mu       sync.Mutex
 	in       io.Reader
-	c        *cipher
+	c        *Cipher
 	nonce    nonce
 	buf      []byte
 	readBuf  []byte
@@ -611,7 +589,7 @@ type encrypter struct {
 }
 
 // newEncrypter creates a new file handle encrypting on the fly
-func (c *cipher) newEncrypter(in io.Reader, nonce *nonce) (*encrypter, error) {
+func (c *Cipher) newEncrypter(in io.Reader, nonce *nonce) (*encrypter, error) {
 	fh := &encrypter{
 		in:      in,
 		c:       c,
@@ -683,13 +661,19 @@ func (fh *encrypter) finish(err error) (int, error) {
 }
 
 // Encrypt data encrypts the data stream
-func (c *cipher) EncryptData(in io.Reader) (io.Reader, error) {
+func (c *Cipher) encryptData(in io.Reader) (io.Reader, *encrypter, error) {
 	in, wrap := accounting.UnWrap(in) // unwrap the accounting off the Reader
 	out, err := c.newEncrypter(in, nil)
 	if err != nil {
-		return nil, err
+		return nil, nil, err
 	}
-	return wrap(out), nil // and wrap the accounting back on
+	return wrap(out), out, nil // and wrap the accounting back on
+}
+
+// EncryptData encrypts the data stream
+func (c *Cipher) EncryptData(in io.Reader) (io.Reader, error) {
+	out, _, err := c.encryptData(in)
+	return out, err
 }
 
 // decrypter decrypts an io.ReaderCloser on the fly
@@ -698,7 +682,7 @@ type decrypter struct {
 	rc           io.ReadCloser
 	nonce        nonce
 	initialNonce nonce
-	c            *cipher
+	c            *Cipher
 	buf          []byte
 	readBuf      []byte
 	bufIndex     int
@@ -709,7 +693,7 @@ type decrypter struct {
 }
 
 // newDecrypter creates a new file handle decrypting on the fly
-func (c *cipher) newDecrypter(rc io.ReadCloser) (*decrypter, error) {
+func (c *Cipher) newDecrypter(rc io.ReadCloser) (*decrypter, error) {
 	fh := &decrypter{
 		rc:      rc,
 		c:       c,
@@ -737,7 +721,7 @@ func (c *cipher) newDecrypter(rc io.ReadCloser) (*decrypter, error) {
 }
 
 // newDecrypterSeek creates a new file handle decrypting on the fly
-func (c *cipher) newDecrypterSeek(ctx context.Context, open OpenRangeSeek, offset, limit int64) (fh *decrypter, err error) {
+func (c *Cipher) newDecrypterSeek(ctx context.Context, open OpenRangeSeek, offset, limit int64) (fh *decrypter, err error) {
 	var rc io.ReadCloser
 	doRangeSeek := false
 	setLimit := false
@@ -1012,7 +996,7 @@ func (fh *decrypter) finishAndClose(err error) error {
 }
 
 // DecryptData decrypts the data stream
-func (c *cipher) DecryptData(rc io.ReadCloser) (io.ReadCloser, error) {
+func (c *Cipher) DecryptData(rc io.ReadCloser) (io.ReadCloser, error) {
 	out, err := c.newDecrypter(rc)
 	if err != nil {
 		return nil, err
@@ -1025,7 +1009,7 @@ func (c *cipher) DecryptData(rc io.ReadCloser) (io.ReadCloser, error) {
 // The open function must return a ReadCloser opened to the offset supplied
 //
 // You must use this form of DecryptData if you might want to Seek the file handle
-func (c *cipher) DecryptDataSeek(ctx context.Context, open OpenRangeSeek, offset, limit int64) (ReadSeekCloser, error) {
+func (c *Cipher) DecryptDataSeek(ctx context.Context, open OpenRangeSeek, offset, limit int64) (ReadSeekCloser, error) {
 	out, err := c.newDecrypterSeek(ctx, open, offset, limit)
 	if err != nil {
 		return nil, err
@@ -1034,7 +1018,7 @@ func (c *cipher) DecryptDataSeek(ctx context.Context, open OpenRangeSeek, offset
 }
 
 // EncryptedSize calculates the size of the data when encrypted
-func (c *cipher) EncryptedSize(size int64) int64 {
+func (c *Cipher) EncryptedSize(size int64) int64 {
 	blocks, residue := size/blockDataSize, size%blockDataSize
 	encryptedSize := int64(fileHeaderSize) + blocks*(blockHeaderSize+blockDataSize)
 	if residue != 0 {
@@ -1044,7 +1028,7 @@ func (c *cipher) EncryptedSize(size int64) int64 {
 }
 
 // DecryptedSize calculates the size of the data when decrypted
-func (c *cipher) DecryptedSize(size int64) (int64, error) {
+func (c *Cipher) DecryptedSize(size int64) (int64, error) {
 	size -= int64(fileHeaderSize)
 	if size < 0 {
 		return 0, ErrorEncryptedFileTooShort
@@ -1063,7 +1047,6 @@ func (c *cipher) DecryptedSize(size int64) (int64, error) {
 
 // check interfaces
 var (
-	_ Cipher         = (*cipher)(nil)
 	_ io.ReadCloser  = (*decrypter)(nil)
 	_ io.Seeker      = (*decrypter)(nil)
 	_ fs.RangeSeeker = (*decrypter)(nil)
diff --git a/backend/crypt/crypt.go b/backend/crypt/crypt.go
index a3bac32e9..f84eac6f3 100644
--- a/backend/crypt/crypt.go
+++ b/backend/crypt/crypt.go
@@ -90,7 +90,7 @@ names, or for debugging purposes.`,
 }
 
 // newCipherForConfig constructs a Cipher for the given config name
-func newCipherForConfig(opt *Options) (Cipher, error) {
+func newCipherForConfig(opt *Options) (*Cipher, error) {
 	mode, err := NewNameEncryptionMode(opt.FilenameEncryption)
 	if err != nil {
 		return nil, err
@@ -117,7 +117,7 @@ func newCipherForConfig(opt *Options) (Cipher, error) {
 }
 
 // NewCipher constructs a Cipher for the given config
-func NewCipher(m configmap.Mapper) (Cipher, error) {
+func NewCipher(m configmap.Mapper) (*Cipher, error) {
 	// Parse config into Options struct
 	opt := new(Options)
 	err := configstruct.Set(m, opt)
@@ -203,7 +203,7 @@ type Fs struct {
 	root     string
 	opt      Options
 	features *fs.Features // optional features
-	cipher   Cipher
+	cipher   *Cipher
 }
 
 // Name of the remote (as passed into NewFs)
@@ -327,7 +327,7 @@ type putFn func(ctx context.Context, in io.Reader, src fs.ObjectInfo, options ..
 // put implements Put or PutStream
 func (f *Fs) put(ctx context.Context, in io.Reader, src fs.ObjectInfo, options []fs.OpenOption, put putFn) (fs.Object, error) {
 	// Encrypt the data into wrappedIn
-	wrappedIn, err := f.cipher.EncryptData(in)
+	wrappedIn, encrypter, err := f.cipher.encryptData(in)
 	if err != nil {
 		return nil, err
 	}
@@ -351,7 +351,7 @@ func (f *Fs) put(ctx context.Context, in io.Reader, src fs.ObjectInfo, options [
 	}
 
 	// Transfer the data
-	o, err := put(ctx, wrappedIn, f.newObjectInfo(src), options...)
+	o, err := put(ctx, wrappedIn, f.newObjectInfo(src, encrypter.nonce), options...)
 	if err != nil {
 		return nil, err
 	}
@@ -504,11 +504,11 @@ func (f *Fs) PutUnchecked(ctx context.Context, in io.Reader, src fs.ObjectInfo,
 	if do == nil {
 		return nil, errors.New("can't PutUnchecked")
 	}
-	wrappedIn, err := f.cipher.EncryptData(in)
+	wrappedIn, encrypter, err := f.cipher.encryptData(in)
 	if err != nil {
 		return nil, err
 	}
-	o, err := do(ctx, wrappedIn, f.newObjectInfo(src))
+	o, err := do(ctx, wrappedIn, f.newObjectInfo(src, encrypter.nonce))
 	if err != nil {
 		return nil, err
 	}
@@ -561,6 +561,37 @@ func (f *Fs) DecryptFileName(encryptedFileName string) (string, error) {
 	return f.cipher.DecryptFileName(encryptedFileName)
 }
 
+// computeHashWithNonce takes the nonce and encrypts the contents of
+// src with it, and calculates the hash given by HashType on the fly
+//
+// Note that we break lots of encapsulation in this function.
+func (f *Fs) computeHashWithNonce(ctx context.Context, nonce nonce, src fs.Object, hashType hash.Type) (hashStr string, err error) {
+	// Open the src for input
+	in, err := src.Open(ctx)
+	if err != nil {
+		return "", errors.Wrap(err, "failed to open src")
+	}
+	defer fs.CheckClose(in, &err)
+
+	// Now encrypt the src with the nonce
+	out, err := f.cipher.newEncrypter(in, &nonce)
+	if err != nil {
+		return "", errors.Wrap(err, "failed to make encrypter")
+	}
+
+	// pipe into hash
+	m, err := hash.NewMultiHasherTypes(hash.NewHashSet(hashType))
+	if err != nil {
+		return "", errors.Wrap(err, "failed to make hasher")
+	}
+	_, err = io.Copy(m, out)
+	if err != nil {
+		return "", errors.Wrap(err, "failed to hash data")
+	}
+
+	return m.Sums()[hashType], nil
+}
+
 // ComputeHash takes the nonce from o, and encrypts the contents of
 // src with it, and calculates the hash given by HashType on the fly
 //
@@ -572,7 +603,7 @@ func (f *Fs) ComputeHash(ctx context.Context, o *Object, src fs.Object, hashType
 	if err != nil {
 		return "", errors.Wrap(err, "failed to open object to read nonce")
 	}
-	d, err := f.cipher.(*cipher).newDecrypter(in)
+	d, err := f.cipher.newDecrypter(in)
 	if err != nil {
 		_ = in.Close()
 		return "", errors.Wrap(err, "failed to open object to read nonce")
@@ -597,30 +628,7 @@ func (f *Fs) ComputeHash(ctx context.Context, o *Object, src fs.Object, hashType
 		return "", errors.Wrap(err, "failed to close nonce read")
 	}
 
-	// Open the src for input
-	in, err = src.Open(ctx)
-	if err != nil {
-		return "", errors.Wrap(err, "failed to open src")
-	}
-	defer fs.CheckClose(in, &err)
-
-	// Now encrypt the src with the nonce
-	out, err := f.cipher.(*cipher).newEncrypter(in, &nonce)
-	if err != nil {
-		return "", errors.Wrap(err, "failed to make encrypter")
-	}
-
-	// pipe into hash
-	m, err := hash.NewMultiHasherTypes(hash.NewHashSet(hashType))
-	if err != nil {
-		return "", errors.Wrap(err, "failed to make hasher")
-	}
-	_, err = io.Copy(m, out)
-	if err != nil {
-		return "", errors.Wrap(err, "failed to hash data")
-	}
-
-	return m.Sums()[hashType], nil
+	return f.computeHashWithNonce(ctx, nonce, src, hashType)
 }
 
 // MergeDirs merges the contents of all the directories passed
@@ -833,13 +841,15 @@ func (f *Fs) Disconnect(ctx context.Context) error {
 // This encrypts the remote name and adjusts the size
 type ObjectInfo struct {
 	fs.ObjectInfo
-	f *Fs
+	f     *Fs
+	nonce nonce
 }
 
-func (f *Fs) newObjectInfo(src fs.ObjectInfo) *ObjectInfo {
+func (f *Fs) newObjectInfo(src fs.ObjectInfo, nonce nonce) *ObjectInfo {
 	return &ObjectInfo{
 		ObjectInfo: src,
 		f:          f,
+		nonce:      nonce,
 	}
 }
 
@@ -865,6 +875,23 @@ func (o *ObjectInfo) Size() int64 {
 // Hash returns the selected checksum of the file
 // If no checksum is available it returns ""
 func (o *ObjectInfo) Hash(ctx context.Context, hash hash.Type) (string, error) {
+	var srcObj fs.Object
+	var ok bool
+	// Get the underlying object if there is one
+	if srcObj, ok = o.ObjectInfo.(fs.Object); ok {
+		// Prefer direct interface assertion
+	} else if do, ok := o.ObjectInfo.(fs.ObjectUnWrapper); ok {
+		// Otherwise likely is a operations.OverrideRemote
+		srcObj = do.UnWrap()
+	} else {
+		return "", nil
+	}
+	// if this is wrapping a local object then we work out the hash
+	if srcObj.Fs().Features().IsLocal {
+		// Read the data and encrypt it to calculate the hash
+		fs.Debugf(o, "Computing %v hash of encrypted source", hash)
+		return o.f.computeHashWithNonce(ctx, o.nonce, srcObj, hash)
+	}
 	return "", nil
 }
 
diff --git a/cmd/cmount/fs.go b/cmd/cmount/fs.go
index c4f0d6d65..6b51fd0cf 100644
--- a/cmd/cmount/fs.go
+++ b/cmd/cmount/fs.go
@@ -538,11 +538,11 @@ func translateError(err error) (errc int) {
 	switch errors.Cause(err) {
 	case vfs.OK:
 		return 0
-	case vfs.ENOENT:
+	case vfs.ENOENT, fs.ErrorDirNotFound, fs.ErrorObjectNotFound:
 		return -fuse.ENOENT
-	case vfs.EEXIST:
+	case vfs.EEXIST, fs.ErrorDirExists:
 		return -fuse.EEXIST
-	case vfs.EPERM:
+	case vfs.EPERM, fs.ErrorPermissionDenied:
 		return -fuse.EPERM
 	case vfs.ECLOSED:
 		return -fuse.EBADF
@@ -554,7 +554,7 @@ func translateError(err error) (errc int) {
 		return -fuse.EBADF
 	case vfs.EROFS:
 		return -fuse.EROFS
-	case vfs.ENOSYS:
+	case vfs.ENOSYS, fs.ErrorNotImplemented:
 		return -fuse.ENOSYS
 	case vfs.EINVAL:
 		return -fuse.EINVAL
diff --git a/cmd/cryptcheck/cryptcheck.go b/cmd/cryptcheck/cryptcheck.go
index 0e9b21043..97b155021 100644
--- a/cmd/cryptcheck/cryptcheck.go
+++ b/cmd/cryptcheck/cryptcheck.go
@@ -110,7 +110,6 @@ func cryptCheck(ctx context.Context, fdst, fsrc fs.Fs) error {
 			fs.Errorf(src, err.Error())
 			return true, false
 		}
-		fs.Debugf(src, "OK")
 		return false, false
 	}
 
diff --git a/cmd/cryptdecode/cryptdecode.go b/cmd/cryptdecode/cryptdecode.go
index 493f9edf6..7e7514a0c 100644
--- a/cmd/cryptdecode/cryptdecode.go
+++ b/cmd/cryptdecode/cryptdecode.go
@@ -60,7 +60,7 @@ use it like this
 }
 
 // cryptDecode returns the unencrypted file name
-func cryptDecode(cipher crypt.Cipher, args []string) error {
+func cryptDecode(cipher *crypt.Cipher, args []string) error {
 	output := ""
 
 	for _, encryptedFileName := range args {
@@ -78,7 +78,7 @@ func cryptDecode(cipher crypt.Cipher, args []string) error {
 }
 
 // cryptEncode returns the encrypted file name
-func cryptEncode(cipher crypt.Cipher, args []string) error {
+func cryptEncode(cipher *crypt.Cipher, args []string) error {
 	output := ""
 
 	for _, fileName := range args {
diff --git a/cmd/mount/fs.go b/cmd/mount/fs.go
index 016e97eac..0292db99b 100644
--- a/cmd/mount/fs.go
+++ b/cmd/mount/fs.go
@@ -77,11 +77,11 @@ func translateError(err error) error {
 	switch errors.Cause(err) {
 	case vfs.OK:
 		return nil
-	case vfs.ENOENT:
+	case vfs.ENOENT, fs.ErrorDirNotFound, fs.ErrorObjectNotFound:
 		return fuse.ENOENT
-	case vfs.EEXIST:
+	case vfs.EEXIST, fs.ErrorDirExists:
 		return fuse.EEXIST
-	case vfs.EPERM:
+	case vfs.EPERM, fs.ErrorPermissionDenied:
 		return fuse.EPERM
 	case vfs.ECLOSED:
 		return fuse.Errno(syscall.EBADF)
@@ -93,7 +93,7 @@ func translateError(err error) error {
 		return fuse.Errno(syscall.EBADF)
 	case vfs.EROFS:
 		return fuse.Errno(syscall.EROFS)
-	case vfs.ENOSYS:
+	case vfs.ENOSYS, fs.ErrorNotImplemented:
 		return fuse.ENOSYS
 	case vfs.EINVAL:
 		return fuse.Errno(syscall.EINVAL)
diff --git a/cmd/mount2/fs.go b/cmd/mount2/fs.go
index f672b48b5..0a24580e9 100644
--- a/cmd/mount2/fs.go
+++ b/cmd/mount2/fs.go
@@ -105,11 +105,11 @@ func translateError(err error) syscall.Errno {
 	switch errors.Cause(err) {
 	case vfs.OK:
 		return 0
-	case vfs.ENOENT:
+	case vfs.ENOENT, fs.ErrorDirNotFound, fs.ErrorObjectNotFound:
 		return syscall.ENOENT
-	case vfs.EEXIST:
+	case vfs.EEXIST, fs.ErrorDirExists:
 		return syscall.EEXIST
-	case vfs.EPERM:
+	case vfs.EPERM, fs.ErrorPermissionDenied:
 		return syscall.EPERM
 	case vfs.ECLOSED:
 		return syscall.EBADF
@@ -121,7 +121,7 @@ func translateError(err error) syscall.Errno {
 		return syscall.EBADF
 	case vfs.EROFS:
 		return syscall.EROFS
-	case vfs.ENOSYS:
+	case vfs.ENOSYS, fs.ErrorNotImplemented:
 		return syscall.ENOSYS
 	case vfs.EINVAL:
 		return syscall.EINVAL
diff --git a/fs/operations/lsjson.go b/fs/operations/lsjson.go
index f8a9e28bd..ffba3ff9d 100644
--- a/fs/operations/lsjson.go
+++ b/fs/operations/lsjson.go
@@ -81,7 +81,7 @@ type ListJSONOpt struct {
 
 // ListJSON lists fsrc using the options in opt calling callback for each item
 func ListJSON(ctx context.Context, fsrc fs.Fs, remote string, opt *ListJSONOpt, callback func(*ListJSONItem) error) error {
-	var cipher crypt.Cipher
+	var cipher *crypt.Cipher
 	if opt.ShowEncrypted {
 		fsInfo, _, _, config, err := fs.ConfigFs(fsrc.Name() + ":" + fsrc.Root())
 		if err != nil {


Test patch:
diff --git a/backend/crypt/crypt_internal_test.go b/backend/crypt/crypt_internal_test.go
new file mode 100644
index 000000000..33b093835
--- /dev/null
+++ b/backend/crypt/crypt_internal_test.go
@@ -0,0 +1,143 @@
+package crypt
+
+import (
+	"bytes"
+	"context"
+	"crypto/md5"
+	"fmt"
+	"io"
+	"testing"
+	"time"
+
+	"github.com/rclone/rclone/fs"
+	"github.com/rclone/rclone/fs/hash"
+	"github.com/rclone/rclone/fs/object"
+	"github.com/rclone/rclone/lib/random"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+type testWrapper struct {
+	fs.ObjectInfo
+}
+
+// UnWrap returns the Object that this Object is wrapping or nil if it
+// isn't wrapping anything
+func (o testWrapper) UnWrap() fs.Object {
+	if o, ok := o.ObjectInfo.(fs.Object); ok {
+		return o
+	}
+	return nil
+}
+
+// Create a temporary local fs to upload things from
+
+func makeTempLocalFs(t *testing.T) (localFs fs.Fs, cleanup func()) {
+	localFs, err := fs.TemporaryLocalFs()
+	require.NoError(t, err)
+	cleanup = func() {
+		require.NoError(t, localFs.Rmdir(context.Background(), ""))
+	}
+	return localFs, cleanup
+}
+
+// Upload a file to a remote
+func uploadFile(t *testing.T, f fs.Fs, remote, contents string) (obj fs.Object, cleanup func()) {
+	inBuf := bytes.NewBufferString(contents)
+	t1 := time.Date(2012, time.December, 17, 18, 32, 31, 0, time.UTC)
+	upSrc := object.NewStaticObjectInfo(remote, t1, int64(len(contents)), true, nil, nil)
+	obj, err := f.Put(context.Background(), inBuf, upSrc)
+	require.NoError(t, err)
+	cleanup = func() {
+		require.NoError(t, obj.Remove(context.Background()))
+	}
+	return obj, cleanup
+}
+
+// Test the ObjectInfo
+func testObjectInfo(t *testing.T, f *Fs, wrap bool) {
+	var (
+		contents = random.String(100)
+		path     = "hash_test_object"
+		ctx      = context.Background()
+	)
+	if wrap {
+		path = "_wrap"
+	}
+
+	localFs, cleanupLocalFs := makeTempLocalFs(t)
+	defer cleanupLocalFs()
+
+	obj, cleanupObj := uploadFile(t, localFs, path, contents)
+	defer cleanupObj()
+
+	// encrypt the data
+	inBuf := bytes.NewBufferString(contents)
+	var outBuf bytes.Buffer
+	enc, err := f.cipher.newEncrypter(inBuf, nil)
+	require.NoError(t, err)
+	nonce := enc.nonce // read the nonce at the start
+	_, err = io.Copy(&outBuf, enc)
+	require.NoError(t, err)
+
+	var oi fs.ObjectInfo = obj
+	if wrap {
+		// wrap the object in a fs.ObjectUnwrapper if required
+		oi = testWrapper{oi}
+	}
+
+	// wrap the object in a crypt for upload using the nonce we
+	// saved from the encryptor
+	src := f.newObjectInfo(oi, nonce)
+
+	// Test ObjectInfo methods
+	assert.Equal(t, int64(outBuf.Len()), src.Size())
+	assert.Equal(t, f, src.Fs())
+	assert.NotEqual(t, path, src.Remote())
+
+	// Test ObjectInfo.Hash
+	wantHash := md5.Sum(outBuf.Bytes())
+	gotHash, err := src.Hash(ctx, hash.MD5)
+	require.NoError(t, err)
+	assert.Equal(t, fmt.Sprintf("%x", wantHash), gotHash)
+}
+
+func testComputeHash(t *testing.T, f *Fs) {
+	var (
+		contents = random.String(100)
+		path     = "compute_hash_test"
+		ctx      = context.Background()
+		hashType = f.Fs.Hashes().GetOne()
+	)
+
+	if hashType == hash.None {
+		t.Skipf("%v: does not support hashes", f.Fs)
+	}
+
+	localFs, cleanupLocalFs := makeTempLocalFs(t)
+	defer cleanupLocalFs()
+
+	// Upload a file to localFs as a test object
+	localObj, cleanupLocalObj := uploadFile(t, localFs, path, contents)
+	defer cleanupLocalObj()
+
+	// Upload the same data to the remote Fs also
+	remoteObj, cleanupRemoteObj := uploadFile(t, f, path, contents)
+	defer cleanupRemoteObj()
+
+	// Calculate the expected Hash of the remote object
+	computedHash, err := f.ComputeHash(ctx, remoteObj.(*Object), localObj, hashType)
+	require.NoError(t, err)
+
+	// Test computed hash matches remote object hash
+	remoteObjHash, err := remoteObj.(*Object).Object.Hash(ctx, hashType)
+	require.NoError(t, err)
+	assert.Equal(t, remoteObjHash, computedHash)
+}
+
+// InternalTest is called by fstests.Run to extra tests
+func (f *Fs) InternalTest(t *testing.T) {
+	t.Run("ObjectInfo", func(t *testing.T) { testObjectInfo(t, f, false) })
+	t.Run("ObjectInfoWrap", func(t *testing.T) { testObjectInfo(t, f, true) })
+	t.Run("ComputeHash", func(t *testing.T) { testComputeHash(t, f) })
+}
diff --git a/backend/http/http_internal_test.go b/backend/http/http_internal_test.go
index e89e58b7b..e62a02b98 100644
--- a/backend/http/http_internal_test.go
+++ b/backend/http/http_internal_test.go
@@ -166,8 +166,7 @@ func TestNewObject(t *testing.T) {
 	require.NoError(t, err)
 	tFile := fi.ModTime()
 
-	dt, ok := fstest.CheckTimeEqualWithPrecision(tObj, tFile, time.Second)
-	assert.True(t, ok, fmt.Sprintf("%s: Modification time difference too big |%s| > %s (%s vs %s) (precision %s)", o.Remote(), dt, time.Second, tObj, tFile, time.Second))
+	fstest.AssertTimeEqualWithPrecision(t, o.Remote(), tFile, tObj, time.Second)
 
 	// check object not found
 	o, err = f.NewObject(context.Background(), "not found.txt")
diff --git a/fs/operations/operations_test.go b/fs/operations/operations_test.go
index 3616a8db0..8b8cae348 100644
--- a/fs/operations/operations_test.go
+++ b/fs/operations/operations_test.go
@@ -166,10 +166,7 @@ func TestLsLong(t *testing.T) {
 		if err != nil {
 			t.Errorf("Error parsing %q: %v", m, err)
 		} else {
-			dt, ok := fstest.CheckTimeEqualWithPrecision(expected, modTime, precision)
-			if !ok {
-				t.Errorf("%s: Modification time difference too big |%s| > %s (%s vs %s) (precision %s)", filename, dt, precision, modTime, expected, precision)
-			}
+			fstest.AssertTimeEqualWithPrecision(t, filename, expected, modTime, precision)
 		}
 	}
 
diff --git a/fstest/fstest.go b/fstest/fstest.go
index 81eefc828..ec5242895 100644
--- a/fstest/fstest.go
+++ b/fstest/fstest.go
@@ -117,10 +117,16 @@ func CheckTimeEqualWithPrecision(t0, t1 time.Time, precision time.Duration) (tim
 	return dt, true
 }
 
+// AssertTimeEqualWithPrecision checks that want is within precision
+// of got, asserting that with t and logging remote
+func AssertTimeEqualWithPrecision(t *testing.T, remote string, want, got time.Time, precision time.Duration) {
+	dt, ok := CheckTimeEqualWithPrecision(want, got, precision)
+	assert.True(t, ok, fmt.Sprintf("%s: Modification time difference too big |%s| > %s (want %s vs got %s) (precision %s)", remote, dt, precision, want, got, precision))
+}
+
 // CheckModTime checks the mod time to the given precision
 func (i *Item) CheckModTime(t *testing.T, obj fs.Object, modTime time.Time, precision time.Duration) {
-	dt, ok := CheckTimeEqualWithPrecision(modTime, i.ModTime, precision)
-	assert.True(t, ok, fmt.Sprintf("%s: Modification time difference too big |%s| > %s (%s vs %s) (precision %s)", obj.Remote(), dt, precision, modTime, i.ModTime, precision))
+	AssertTimeEqualWithPrecision(t, obj.Remote(), i.ModTime, modTime, precision)
 }
 
 // CheckHashes checks all the hashes the object supports are correct