From ad60e44ef72b843842129856c82633d0ceddb78d Mon Sep 17 00:00:00 2001 From: Tomasz Raganowicz <7759113+tomekit@users.noreply.github.com> Date: Tue, 8 Oct 2024 11:30:11 +0200 Subject: [PATCH] crypt: Refactor cipher creation. Make newCipher parameter explicit and introduce newCipherForTest --- backend/crypt/cipher.go | 14 +++---- backend/crypt/cipher_test.go | 80 ++++++++++++++++++++---------------- backend/crypt/crypt.go | 5 +-- 3 files changed, 53 insertions(+), 46 deletions(-) diff --git a/backend/crypt/cipher.go b/backend/crypt/cipher.go index 9428c7ac4..c3abca9b0 100644 --- a/backend/crypt/cipher.go +++ b/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, - fileNameEnc: enc, - cryptoRand: rand.Reader, - dirNameEncrypt: dirNameEncrypt, - encryptedSuffix: ".bin", - version: CipherVersionV1, // Default to satisfy existing: `cipher_test.go`. For Rclone overridden by: `setCipherVersion` inside `newCipherForConfig` call. + mode: mode, + fileNameEnc: enc, + cryptoRand: rand.Reader, + dirNameEncrypt: dirNameEncrypt, + version: version, } c.buffers.New = func() interface{} { return new([blockSize]byte) diff --git a/backend/crypt/cipher_test.go b/backend/crypt/cipher_test.go index caf73135e..b568c5642 100644 --- a/backend/crypt/cipher_test.go +++ b/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 +} diff --git a/backend/crypt/crypt.go b/backend/crypt/crypt.go index 647b54998..97a336939 100644 --- a/backend/crypt/crypt.go +++ b/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 }