From d5afcf9e34044e5b7bb1ae5553b6f2f98450b7d2 Mon Sep 17 00:00:00 2001
From: Nick Craig-Wood <nick@craig-wood.com>
Date: Mon, 6 Mar 2023 17:49:50 +0000
Subject: [PATCH] crypt: try not to return "unexpected EOF" error

Before this change the code wasn't taking into account the error
io.ErrUnexpectedEOF that io.ReadFull can return properly. Sometimes
that error was being returned instead of a more specific and useful
error.

To fix this, io.ReadFull was replaced with the simpler
readers.ReadFill which is much easier to use correctly.
---
 backend/crypt/cipher.go      | 21 +++++++++------------
 backend/crypt/cipher_test.go |  4 +++-
 2 files changed, 12 insertions(+), 13 deletions(-)

diff --git a/backend/crypt/cipher.go b/backend/crypt/cipher.go
index ae1e62393..e90d75615 100644
--- a/backend/crypt/cipher.go
+++ b/backend/crypt/cipher.go
@@ -21,6 +21,7 @@ import (
 	"github.com/rclone/rclone/backend/crypt/pkcs7"
 	"github.com/rclone/rclone/fs"
 	"github.com/rclone/rclone/fs/accounting"
+	"github.com/rclone/rclone/lib/readers"
 	"github.com/rclone/rclone/lib/version"
 	"github.com/rfjakob/eme"
 	"golang.org/x/crypto/nacl/secretbox"
@@ -609,7 +610,7 @@ func (n *nonce) pointer() *[fileNonceSize]byte {
 // fromReader fills the nonce from an io.Reader - normally the OSes
 // crypto random number generator
 func (n *nonce) fromReader(in io.Reader) error {
-	read, err := io.ReadFull(in, (*n)[:])
+	read, err := readers.ReadFill(in, (*n)[:])
 	if read != fileNonceSize {
 		return fmt.Errorf("short read of nonce: %w", err)
 	}
@@ -708,14 +709,12 @@ func (fh *encrypter) Read(p []byte) (n int, err error) {
 		// Read data
 		// FIXME should overlap the reads with a go-routine and 2 buffers?
 		readBuf := fh.readBuf[:blockDataSize]
-		n, err = io.ReadFull(fh.in, readBuf)
+		n, err = readers.ReadFill(fh.in, readBuf)
 		if n == 0 {
-			// err can't be nil since:
-			// n == len(buf) if and only if err == nil.
 			return fh.finish(err)
 		}
 		// possibly err != nil here, but we will process the
-		// data and the next call to ReadFull will return 0, err
+		// data and the next call to ReadFill will return 0, err
 		// Encrypt the block using the nonce
 		secretbox.Seal(fh.buf[:0], readBuf[:n], fh.nonce.pointer(), &fh.c.dataKey)
 		fh.bufIndex = 0
@@ -783,8 +782,8 @@ func (c *Cipher) newDecrypter(rc io.ReadCloser) (*decrypter, error) {
 	}
 	// Read file header (magic + nonce)
 	readBuf := fh.readBuf[:fileHeaderSize]
-	_, err := io.ReadFull(fh.rc, readBuf)
-	if err == io.EOF || err == io.ErrUnexpectedEOF {
+	n, err := readers.ReadFill(fh.rc, readBuf)
+	if n < fileHeaderSize && err == io.EOF {
 		// This read from 0..fileHeaderSize-1 bytes
 		return nil, fh.finishAndClose(ErrorEncryptedFileTooShort)
 	} else if err != nil {
@@ -845,10 +844,8 @@ func (c *Cipher) newDecrypterSeek(ctx context.Context, open OpenRangeSeek, offse
 func (fh *decrypter) fillBuffer() (err error) {
 	// FIXME should overlap the reads with a go-routine and 2 buffers?
 	readBuf := fh.readBuf
-	n, err := io.ReadFull(fh.rc, readBuf)
+	n, err := readers.ReadFill(fh.rc, readBuf)
 	if n == 0 {
-		// err can't be nil since:
-		// n == len(buf) if and only if err == nil.
 		return err
 	}
 	// possibly err != nil here, but we will process the data and
@@ -856,7 +853,7 @@ func (fh *decrypter) fillBuffer() (err error) {
 
 	// Check header + 1 byte exists
 	if n <= blockHeaderSize {
-		if err != nil {
+		if err != nil && err != io.EOF {
 			return err // return pending error as it is likely more accurate
 		}
 		return ErrorEncryptedFileBadHeader
@@ -864,7 +861,7 @@ func (fh *decrypter) fillBuffer() (err error) {
 	// Decrypt the block using the nonce
 	_, ok := secretbox.Open(fh.buf[:0], readBuf[:n], fh.nonce.pointer(), &fh.c.dataKey)
 	if !ok {
-		if err != nil {
+		if err != nil && err != io.EOF {
 			return err // return pending error as it is likely more accurate
 		}
 		return ErrorEncryptedBadBlock
diff --git a/backend/crypt/cipher_test.go b/backend/crypt/cipher_test.go
index 9ff4fdcc1..6b683b437 100644
--- a/backend/crypt/cipher_test.go
+++ b/backend/crypt/cipher_test.go
@@ -1495,8 +1495,10 @@ func TestDecrypterRead(t *testing.T) {
 		case i == fileHeaderSize:
 			// This would normally produce an error *except* on the first block
 			expectedErr = nil
+		case i <= fileHeaderSize+blockHeaderSize:
+			expectedErr = ErrorEncryptedFileBadHeader
 		default:
-			expectedErr = io.ErrUnexpectedEOF
+			expectedErr = ErrorEncryptedBadBlock
 		}
 		if expectedErr != nil {
 			assert.EqualError(t, err, expectedErr.Error(), what)