From 9d2dd2c49a54eefc263a5c603a5998c463a86f5d Mon Sep 17 00:00:00 2001 From: Nick Craig-Wood Date: Tue, 25 Oct 2016 15:15:44 +0100 Subject: [PATCH] crypt: Fix data corruption on seek This was caused by failing to reset the internal buffer on seek so old data was read first before the new data. The unit tests didn't detect this because they were reading to the end of the file to check integrity and thus emptying the internal buffer. Both code and unit tests were fixed up. --- crypt/cipher.go | 39 ++++++++++++++++++++++----------------- crypt/cipher_test.go | 24 ++++++++++++++++-------- 2 files changed, 38 insertions(+), 25 deletions(-) diff --git a/crypt/cipher.go b/crypt/cipher.go index 87747a553..9ce04e9e1 100644 --- a/crypt/cipher.go +++ b/crypt/cipher.go @@ -604,9 +604,20 @@ func (fh *decrypter) Seek(offset int64, whence int) (int64, error) { return 0, fh.err } - // Can we seek it directly? + // blocks we need to seek, plus bytes we need to discard + blocks, discard := offset/blockDataSize, offset%blockDataSize + + // Offset in underlying stream we need to seek + underlyingOffset := int64(fileHeaderSize) + blocks*(blockHeaderSize+blockDataSize) + + // Move the nonce on the correct number of blocks from the start + fh.nonce = fh.initialNonce + fh.nonce.add(uint64(blocks)) + + // Can we seek underlying stream directly? if do, ok := fh.rc.(io.Seeker); ok { - _, err := do.Seek(offset, 0) + // Seek underlying stream directly + _, err := do.Seek(underlyingOffset, 0) if err != nil { return 0, fh.finish(err) } @@ -615,16 +626,6 @@ func (fh *decrypter) Seek(offset int64, whence int) (int64, error) { _ = fh.rc.Close() // close underlying file fh.rc = nil - // blocks we need to seek, plus bytes we need to discard - blocks, discard := offset/blockDataSize, offset%blockDataSize - - // Offset in underlying stream we need to seek - underlyingOffset := int64(fileHeaderSize) + blocks*(blockHeaderSize+blockDataSize) - - // Move the nonce on the correct number of blocks from the start - fh.nonce = fh.initialNonce - fh.nonce.add(uint64(blocks)) - // Re-open the underlying object with the offset given rc, err := fh.open(underlyingOffset) if err != nil { @@ -633,12 +634,16 @@ func (fh *decrypter) Seek(offset int64, whence int) (int64, error) { // Set the file handle fh.rc = rc + } - // Discard excess bytes - _, err = io.CopyN(ioutil.Discard, fh, discard) - if err != nil { - return 0, fh.finish(err) - } + // Empty the buffer + fh.bufIndex = 0 + fh.bufSize = 0 + + // Discard excess bytes + _, err := io.CopyN(ioutil.Discard, fh, discard) + if err != nil { + return 0, fh.finish(err) } return offset, nil diff --git a/crypt/cipher_test.go b/crypt/cipher_test.go index 384b83155..5bacdcdcd 100644 --- a/crypt/cipher_test.go +++ b/crypt/cipher_test.go @@ -881,15 +881,26 @@ func TestNewDecrypterSeek(t *testing.T) { return ioutil.NopCloser(bytes.NewBuffer(ciphertext[int(underlyingOffset):])), nil } + inBlock := make([]byte, 1024) + + // Check the seek worked by reading a block and checking it + // against what it should be + check := func(rc ReadSeekCloser, offset int) { + n, err := io.ReadFull(rc, inBlock) + if err != nil && err != io.EOF && err != io.ErrUnexpectedEOF { + require.NoError(t, err) + } + seekedDecrypted := inBlock[:n] + + require.Equal(t, plaintext[offset:offset+n], seekedDecrypted) + } + // Now try decoding it with a open/seek for _, offset := range trials { rc, err := c.DecryptDataSeek(open, int64(offset)) assert.NoError(t, err) - seekedDecrypted, err := ioutil.ReadAll(rc) - assert.NoError(t, err) - - assert.Equal(t, plaintext[offset:], seekedDecrypted) + check(rc, offset) } // Now try decoding it with a single open and lots of seeks @@ -898,10 +909,7 @@ func TestNewDecrypterSeek(t *testing.T) { _, err := rc.Seek(int64(offset), 0) assert.NoError(t, err) - seekedDecrypted, err := ioutil.ReadAll(rc) - assert.NoError(t, err) - - assert.Equal(t, plaintext[offset:], seekedDecrypted) + check(rc, offset) } }