crypt: Refactor cipher creation. Make newCipher parameter explicit and introduce newCipherForTest

backend/crypt/cipher.go

@@ -36,6 +36,7 @@ const (
 	fileMagicSize       = len(fileMagic)
 	fileNonceSize       = 24
 	fileHeaderSize      = fileMagicSize + fileNonceSize
+	fileEncryptedSuffix = ".bin" // V1 cipher by default sets the suffix (only if name encryption disabled), but user can disable it completely by setting it to `none`.
 
 	fileMagicV2             = "RCLONE\x00\x01"
 	fileMagicSizeV2         = len(fileMagicV2)
@@ -212,14 +213,13 @@ 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, enc fileNameEncoding) (*Cipher, error) {
+func newCipher(mode NameEncryptionMode, password, salt string, dirNameEncrypt bool, enc fileNameEncoding, version string) (*Cipher, error) {
 	c := &Cipher{
-		mode:            mode,
+		mode:           mode,
-		fileNameEnc:     enc,
+		fileNameEnc:    enc,
-		cryptoRand:      rand.Reader,
+		cryptoRand:     rand.Reader,
-		dirNameEncrypt:  dirNameEncrypt,
+		dirNameEncrypt: dirNameEncrypt,
-		encryptedSuffix: ".bin",
+		version:        version,
-		version:         CipherVersionV1, // Default to satisfy existing: `cipher_test.go`. For Rclone overridden by: `setCipherVersion` inside `newCipherForConfig` call.
 	}
 	c.buffers.New = func() interface{} {
 		return new([blockSize]byte)

backend/crypt/cipher_test.go

@@ -190,7 +190,7 @@ func TestDecodeFileNameBase32768(t *testing.T) {
 
 func testEncryptSegment(t *testing.T, encoding string, testCases []EncodingTestCase, caseInsensitive bool) {
 	enc, _ := NewNameEncoding(encoding)
-	c, _ := newCipher(NameEncryptionStandard, "", "", true, enc)
+	c, _ := newCipherForTest(NameEncryptionStandard, "", "", true, enc)
 	for _, test := range testCases {
 		actual := c.encryptSegment(test.in)
 		assert.Equal(t, test.expected, actual, fmt.Sprintf("Testing %q", test.in))
@@ -279,7 +279,7 @@ func TestDecryptSegmentBase32(t *testing.T) {
 		longName[i] = 'a'
 	}
 	enc, _ := NewNameEncoding("base32")
-	c, _ := newCipher(NameEncryptionStandard, "", "", true, enc)
+	c, _ := newCipherForTest(NameEncryptionStandard, "", "", true, enc)
 	for _, test := range []struct {
 		in          string
 		expectedErr error
@@ -303,7 +303,7 @@ func TestDecryptSegmentBase64(t *testing.T) {
 		longName[i] = 'a'
 	}
 	enc, _ := NewNameEncoding("base64")
-	c, _ := newCipher(NameEncryptionStandard, "", "", true, enc)
+	c, _ := newCipherForTest(NameEncryptionStandard, "", "", true, enc)
 	for _, test := range []struct {
 		in          string
 		expectedErr error
@@ -324,7 +324,7 @@ func TestDecryptSegmentBase32768(t *testing.T) {
 	// We've tested the forwards above, now concentrate on the errors
 	longName := strings.Repeat("怪", 1280)
 	enc, _ := NewNameEncoding("base32768")
-	c, _ := newCipher(NameEncryptionStandard, "", "", true, enc)
+	c, _ := newCipherForTest(NameEncryptionStandard, "", "", true, enc)
 	for _, test := range []struct {
 		in          string
 		expectedErr error
@@ -344,12 +344,12 @@ func TestDecryptSegmentBase32768(t *testing.T) {
 func testStandardEncryptFileName(t *testing.T, encoding string, testCasesEncryptDir []EncodingTestCase, testCasesNoEncryptDir []EncodingTestCase) {
 	// First standard mode
 	enc, _ := NewNameEncoding(encoding)
-	c, _ := newCipher(NameEncryptionStandard, "", "", true, enc)
+	c, _ := newCipherForTest(NameEncryptionStandard, "", "", true, enc)
 	for _, test := range testCasesEncryptDir {
 		assert.Equal(t, test.expected, c.EncryptFileName(test.in))
 	}
 	// Standard mode with directory name encryption off
-	c, _ = newCipher(NameEncryptionStandard, "", "", false, enc)
+	c, _ = newCipherForTest(NameEncryptionStandard, "", "", false, enc)
 	for _, test := range testCasesNoEncryptDir {
 		assert.Equal(t, test.expected, c.EncryptFileName(test.in))
 	}
@@ -405,24 +405,24 @@ func TestStandardEncryptFileNameBase32768(t *testing.T) {
 
 func TestNonStandardEncryptFileName(t *testing.T) {
 	// Off mode
-	c, _ := newCipher(NameEncryptionOff, "", "", true, nil)
+	c, _ := newCipherForTest(NameEncryptionOff, "", "", true, nil)
 	assert.Equal(t, "1/12/123.bin", c.EncryptFileName("1/12/123"))
 	// Off mode with custom suffix
-	c, _ = newCipher(NameEncryptionOff, "", "", true, nil)
+	c, _ = newCipherForTest(NameEncryptionOff, "", "", true, nil)
 	c.setEncryptedSuffix(".jpg")
 	assert.Equal(t, "1/12/123.jpg", c.EncryptFileName("1/12/123"))
 	// Off mode with empty suffix
 	c.setEncryptedSuffix("none")
 	assert.Equal(t, "1/12/123", c.EncryptFileName("1/12/123"))
 	// Obfuscation mode
-	c, _ = newCipher(NameEncryptionObfuscated, "", "", true, nil)
+	c, _ = newCipherForTest(NameEncryptionObfuscated, "", "", true, nil)
 	assert.Equal(t, "49.6/99.23/150.890/53.!!lipps", c.EncryptFileName("1/12/123/!hello"))
 	assert.Equal(t, "49.6/99.23/150.890/53-v2001-02-03-040506-123.!!lipps", c.EncryptFileName("1/12/123/!hello-v2001-02-03-040506-123"))
 	assert.Equal(t, "49.6/99.23/150.890/162.uryyB-v2001-02-03-040506-123.GKG", c.EncryptFileName("1/12/123/hello-v2001-02-03-040506-123.txt"))
 	assert.Equal(t, "161.\u00e4", c.EncryptFileName("\u00a1"))
 	assert.Equal(t, "160.\u03c2", c.EncryptFileName("\u03a0"))
 	// Obfuscation mode with directory name encryption off
-	c, _ = newCipher(NameEncryptionObfuscated, "", "", false, nil)
+	c, _ = newCipherForTest(NameEncryptionObfuscated, "", "", false, nil)
 	assert.Equal(t, "1/12/123/53.!!lipps", c.EncryptFileName("1/12/123/!hello"))
 	assert.Equal(t, "1/12/123/53-v2001-02-03-040506-123.!!lipps", c.EncryptFileName("1/12/123/!hello-v2001-02-03-040506-123"))
 	assert.Equal(t, "161.\u00e4", c.EncryptFileName("\u00a1"))
@@ -433,12 +433,12 @@ func testStandardDecryptFileName(t *testing.T, encoding string, testCases []Enco
 	enc, _ := NewNameEncoding(encoding)
 	for _, test := range testCases {
 		// Test when dirNameEncrypt=true
-		c, _ := newCipher(NameEncryptionStandard, "", "", true, enc)
+		c, _ := newCipherForTest(NameEncryptionStandard, "", "", true, enc)
 		actual, actualErr := c.DecryptFileName(test.in)
 		assert.NoError(t, actualErr)
 		assert.Equal(t, test.expected, actual)
 		if caseInsensitive {
-			c, _ := newCipher(NameEncryptionStandard, "", "", true, enc)
+			c, _ := newCipherForTest(NameEncryptionStandard, "", "", true, enc)
 			actual, actualErr := c.DecryptFileName(strings.ToUpper(test.in))
 			assert.NoError(t, actualErr)
 			assert.Equal(t, test.expected, actual)
@@ -452,7 +452,7 @@ func testStandardDecryptFileName(t *testing.T, encoding string, testCases []Enco
 		if strings.LastIndex(test.expected, "/") != -1 {
 			noDirEncryptIn = test.expected[:strings.LastIndex(test.expected, "/")] + test.in[strings.LastIndex(test.in, "/"):]
 		}
-		c, _ = newCipher(NameEncryptionStandard, "", "", false, enc)
+		c, _ = newCipherForTest(NameEncryptionStandard, "", "", false, enc)
 		actual, actualErr = c.DecryptFileName(noDirEncryptIn)
 		assert.NoError(t, actualErr)
 		assert.Equal(t, test.expected, actual)
@@ -509,7 +509,7 @@ func TestNonStandardDecryptFileName(t *testing.T) {
 			{NameEncryptionObfuscated, false, "1/12/123/53.!!lipps", "1/12/123/!hello", nil, ""},
 			{NameEncryptionObfuscated, false, "1/12/123/53-v2001-02-03-040506-123.!!lipps", "1/12/123/!hello-v2001-02-03-040506-123", nil, ""},
 		} {
-			c, _ := newCipher(test.mode, "", "", test.dirNameEncrypt, enc)
+			c, _ := newCipherForTest(test.mode, "", "", test.dirNameEncrypt, enc)
 			if test.customSuffix != "" {
 				c.setEncryptedSuffix(test.customSuffix)
 			}
@@ -533,7 +533,7 @@ func TestEncDecMatches(t *testing.T) {
 			{NameEncryptionObfuscated, "1/2/3/4/!hello\u03a0"},
 			{NameEncryptionObfuscated, "Avatar The Last Airbender"},
 		} {
-			c, _ := newCipher(test.mode, "", "", true, enc)
+			c, _ := newCipherForTest(test.mode, "", "", true, enc)
 			out, err := c.DecryptFileName(c.EncryptFileName(test.in))
 			what := fmt.Sprintf("Testing %q (mode=%v)", test.in, test.mode)
 			assert.Equal(t, out, test.in, what)
@@ -544,7 +544,7 @@ func TestEncDecMatches(t *testing.T) {
 
 func testStandardEncryptDirName(t *testing.T, encoding string, testCases []EncodingTestCase) {
 	enc, _ := NewNameEncoding(encoding)
-	c, _ := newCipher(NameEncryptionStandard, "", "", true, enc)
+	c, _ := newCipherForTest(NameEncryptionStandard, "", "", true, enc)
 	// First standard mode
 	for _, test := range testCases {
 		assert.Equal(t, test.expected, c.EncryptDirName(test.in))
@@ -578,11 +578,11 @@ func TestStandardEncryptDirNameBase32768(t *testing.T) {
 func TestNonStandardEncryptDirName(t *testing.T) {
 	for _, encoding := range []string{"base32", "base64", "base32768"} {
 		enc, _ := NewNameEncoding(encoding)
-		c, _ := newCipher(NameEncryptionStandard, "", "", false, enc)
+		c, _ := newCipherForTest(NameEncryptionStandard, "", "", false, enc)
 		assert.Equal(t, "1/12", c.EncryptDirName("1/12"))
 		assert.Equal(t, "1/12/123", c.EncryptDirName("1/12/123"))
 		// Now off mode
-		c, _ = newCipher(NameEncryptionOff, "", "", true, enc)
+		c, _ = newCipherForTest(NameEncryptionOff, "", "", true, enc)
 		assert.Equal(t, "1/12/123", c.EncryptDirName("1/12/123"))
 	}
 }
@@ -591,7 +591,7 @@ func testStandardDecryptDirName(t *testing.T, encoding string, testCases []Encod
 	enc, _ := NewNameEncoding(encoding)
 	for _, test := range testCases {
 		// Test dirNameEncrypt=true
-		c, _ := newCipher(NameEncryptionStandard, "", "", true, enc)
+		c, _ := newCipherForTest(NameEncryptionStandard, "", "", true, enc)
 		actual, actualErr := c.DecryptDirName(test.in)
 		assert.Equal(t, test.expected, actual)
 		assert.NoError(t, actualErr)
@@ -604,7 +604,7 @@ func testStandardDecryptDirName(t *testing.T, encoding string, testCases []Encod
 		assert.Equal(t, "", actual)
 		assert.Equal(t, ErrorNotAMultipleOfBlocksize, actualErr)
 		// Test dirNameEncrypt=false
-		c, _ = newCipher(NameEncryptionStandard, "", "", false, enc)
+		c, _ = newCipherForTest(NameEncryptionStandard, "", "", false, enc)
 		actual, actualErr = c.DecryptDirName(test.in)
 		assert.Equal(t, test.in, actual)
 		assert.NoError(t, actualErr)
@@ -632,7 +632,7 @@ for _, test := range []struct {
 	{NameEncryptionStandard, false, "p0e52nreeaj0a5ea7s64m4j72s/l42g6771hnv3an9cgc8cr2n1ng", "p0e52nreeaj0a5ea7s64m4j72s/l42g6771hnv3an9cgc8cr2n1ng", nil},
 	{NameEncryptionStandard, false, "1/12/123", "1/12/123", nil},
 } {
-	c, _ := newCipher(test.mode, "", "", test.dirNameEncrypt, enc)
+	c, _ := newCipherForTest(test.mode, "", "", test.dirNameEncrypt, enc)
 	actual, actualErr := c.DecryptDirName(test.in)
 	what := fmt.Sprintf("Testing %q (mode=%v)", test.in, test.mode)
 	assert.Equal(t, test.expected, actual, what)
@@ -676,7 +676,7 @@ func TestNonStandardDecryptDirName(t *testing.T) {
 		{NameEncryptionOff, true, "1/12/123", "1/12/123", nil},
 		{NameEncryptionOff, true, ".bin", ".bin", nil},
 	} {
-		c, _ := newCipher(test.mode, "", "", test.dirNameEncrypt, nil)
+		c, _ := newCipherForTest(test.mode, "", "", test.dirNameEncrypt, nil)
 		actual, actualErr := c.DecryptDirName(test.in)
 		what := fmt.Sprintf("Testing %q (mode=%v)", test.in, test.mode)
 		assert.Equal(t, test.expected, actual, what)
@@ -685,7 +685,7 @@ func TestNonStandardDecryptDirName(t *testing.T) {
 }
 
 func TestEncryptedSize(t *testing.T) {
-	c, _ := newCipher(NameEncryptionStandard, "", "", true, nil)
+	c, _ := newCipherForTest(NameEncryptionStandard, "", "", true, nil)
 	for _, test := range []struct {
 		in       int64
 		expected int64
@@ -709,7 +709,7 @@ func TestEncryptedSize(t *testing.T) {
 
 func TestDecryptedSize(t *testing.T) {
 	// Test the errors since we tested the reverse above
-	c, _ := newCipher(NameEncryptionStandard, "", "", true, nil)
+	c, _ := newCipherForTest(NameEncryptionStandard, "", "", true, nil)
 	for _, test := range []struct {
 		in          int64
 		expectedErr error
@@ -1080,7 +1080,7 @@ func (z *zeroes) Read(p []byte) (n int, err error) {
 
 // Test encrypt decrypt with different buffer sizes
 func testEncryptDecrypt(t *testing.T, bufSize int, copySize int64) {
-	c, err := newCipher(NameEncryptionStandard, "", "", true, nil)
+	c, err := newCipherForTest(NameEncryptionStandard, "", "", true, nil)
 	assert.NoError(t, err)
 	c.cryptoRand = &zeroes{} // zero out the nonce
 	buf := make([]byte, bufSize)
@@ -1150,7 +1150,7 @@ func TestEncryptData(t *testing.T) {
 		{[]byte{1}, file1},
 		{[]byte{1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16}, file16},
 	} {
-		c, err := newCipher(NameEncryptionStandard, "", "", true, nil)
+		c, err := newCipherForTest(NameEncryptionStandard, "", "", true, nil)
 		assert.NoError(t, err)
 		c.cryptoRand = newRandomSource(1e8) // nodge the crypto rand generator
 
@@ -1173,7 +1173,7 @@ func TestEncryptData(t *testing.T) {
 }
 
 func TestNewEncrypter(t *testing.T) {
-	c, err := newCipher(NameEncryptionStandard, "", "", true, nil)
+	c, err := newCipherForTest(NameEncryptionStandard, "", "", true, nil)
 	assert.NoError(t, err)
 	c.cryptoRand = newRandomSource(1e8) // nodge the crypto rand generator
 
@@ -1197,7 +1197,7 @@ func TestNewEncrypterV2(t *testing.T) {
 	plaintextToEncrypt := []byte{1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16}
 	nonceUsed := []byte{0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x07, 0x08, 0x09, 0x0a, 0x0b, 0x0c, 0x0d, 0x0e, 0x0f, 0x10, 0x11, 0x12, 0x13, 0x14, 0x15, 0x16, 0x17}
 
-	c, err := newCipher(NameEncryptionStandard, encryptionPassword, "", true, nil)
+	c, err := newCipherForTest(NameEncryptionStandard, encryptionPassword, "", true, nil)
 	c.setCipherVersion(CipherVersionV2)
 	assert.NoError(t, err)
 
@@ -1266,7 +1266,7 @@ func TestNewEncrypterV2(t *testing.T) {
 // Test the stream returning 0, io.ErrUnexpectedEOF - this used to
 // cause a fatal loop
 func TestNewEncrypterErrUnexpectedEOF(t *testing.T) {
-	c, err := newCipher(NameEncryptionStandard, "", "", true, nil)
+	c, err := newCipherForTest(NameEncryptionStandard, "", "", true, nil)
 	assert.NoError(t, err)
 
 	in := &readers.ErrorReader{Err: io.ErrUnexpectedEOF}
@@ -1295,7 +1295,7 @@ func (c *closeDetector) Close() error {
 }
 
 func TestNewDecrypter(t *testing.T) {
-	c, err := newCipher(NameEncryptionStandard, "", "", true, nil)
+	c, err := newCipherForTest(NameEncryptionStandard, "", "", true, nil)
 	assert.NoError(t, err)
 	c.cryptoRand = newRandomSource(1e8) // nodge the crypto rand generator
 
@@ -1342,7 +1342,7 @@ func TestNewDecrypter(t *testing.T) {
 
 // Test the stream returning 0, io.ErrUnexpectedEOF
 func TestNewDecrypterErrUnexpectedEOF(t *testing.T) {
-	c, err := newCipher(NameEncryptionStandard, "", "", true, nil)
+	c, err := newCipherForTest(NameEncryptionStandard, "", "", true, nil)
 	assert.NoError(t, err)
 
 	in2 := &readers.ErrorReader{Err: io.ErrUnexpectedEOF}
@@ -1358,7 +1358,7 @@ func TestNewDecrypterErrUnexpectedEOF(t *testing.T) {
 }
 
 func TestNewDecrypterSeekLimit(t *testing.T) {
-	c, err := newCipher(NameEncryptionStandard, "", "", true, nil)
+	c, err := newCipherForTest(NameEncryptionStandard, "", "", true, nil)
 	assert.NoError(t, err)
 	c.cryptoRand = &zeroes{} // nodge the crypto rand generator
 
@@ -1564,7 +1564,7 @@ func TestDecrypterCalculateUnderlying(t *testing.T) {
 }
 
 func TestDecrypterRead(t *testing.T) {
-	c, err := newCipher(NameEncryptionStandard, "", "", true, nil)
+	c, err := newCipherForTest(NameEncryptionStandard, "", "", true, nil)
 	assert.NoError(t, err)
 
 	// Test truncating the file at each possible point
@@ -1641,7 +1641,7 @@ func TestDecrypterRead(t *testing.T) {
 }
 
 func TestDecrypterClose(t *testing.T) {
-	c, err := newCipher(NameEncryptionStandard, "", "", true, nil)
+	c, err := newCipherForTest(NameEncryptionStandard, "", "", true, nil)
 	assert.NoError(t, err)
 
 	cd := newCloseDetector(bytes.NewBuffer(file16))
@@ -1679,7 +1679,7 @@ func TestDecrypterClose(t *testing.T) {
 }
 
 func TestPutGetBlock(t *testing.T) {
-	c, err := newCipher(NameEncryptionStandard, "", "", true, nil)
+	c, err := newCipherForTest(NameEncryptionStandard, "", "", true, nil)
 	assert.NoError(t, err)
 
 	block := c.getBlock()
@@ -1688,7 +1688,7 @@ func TestPutGetBlock(t *testing.T) {
 }
 
 func TestKey(t *testing.T) {
-	c, err := newCipher(NameEncryptionStandard, "", "", true, nil)
+	c, err := newCipherForTest(NameEncryptionStandard, "", "", true, nil)
 	assert.NoError(t, err)
 
 	// Check zero keys OK
@@ -1721,3 +1721,11 @@ func TestKey(t *testing.T) {
 	assert.Equal(t, [32]byte{}, c.nameKey)
 	assert.Equal(t, [16]byte{}, c.nameTweak)
 }
+
+// newCipherForTest initialises the cipher.
+// For CLI these are set inside `newCipherForConfig`.
+func newCipherForTest(mode NameEncryptionMode, password, salt string, dirNameEncrypt bool, enc fileNameEncoding) (*Cipher, error) {
+	c, err := newCipher(mode, password, salt, dirNameEncrypt, enc, CipherVersionV1)
+	c.setEncryptedSuffix(fileEncryptedSuffix)
+	return c, err
+}

backend/crypt/crypt.go

@@ -187,7 +187,7 @@ length and if it's case sensitive.`,
 
 Setting suffix to "none" will result in an empty suffix. This may be useful 
 when the path length is critical.`,
-			Default:  ".bin",
+			Default:  fileEncryptedSuffix,
 			Advanced: true,
 		},
 			{
@@ -250,13 +250,12 @@ func newCipherForConfig(opt *Options) (*Cipher, error) {
 	if err != nil {
 		return nil, err
 	}
-	cipher, err := newCipher(mode, password, salt, opt.DirectoryNameEncryption, enc)
+	cipher, err := newCipher(mode, password, salt, opt.DirectoryNameEncryption, enc, opt.CipherVersion)
 	if err != nil {
 		return nil, fmt.Errorf("failed to make cipher: %w", err)
 	}
 	cipher.setEncryptedSuffix(opt.Suffix)
 	cipher.setPassBadBlocks(opt.PassBadBlocks)
-	cipher.setCipherVersion(opt.CipherVersion)
 	cipher.setExactSize(opt.ExactSize)
 	return cipher, nil
 }