Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Flushing ChunkCache might write incomplete data #173

Closed
infeo opened this issue Jun 6, 2023 · 1 comment · Fixed by #174
Closed

Flushing ChunkCache might write incomplete data #173

infeo opened this issue Jun 6, 2023 · 1 comment · Fixed by #174
Assignees
Labels
Milestone

Comments

@infeo
Copy link
Member

infeo commented Jun 6, 2023

When writing to a file channel, for performance reasons cryptofs differs writing on the condition "Does the content to be written fits exaclty in a chunk?"

if (offsetInChunk == 0 && len == cleartextChunkSize) {
// complete chunk, no need to load and decrypt from file
ByteBuffer cleartextChunkData = bufferPool.getCleartextBuffer();
src.copyTo(cleartextChunkData);
cleartextChunkData.flip();
chunkCache.putChunk(chunkIndex, cleartextChunkData).close();
} else {

If the condition is true, we directly replace the data in the cache:

public Chunk putChunk(long chunkIndex, ByteBuffer chunkData) throws IllegalArgumentException {
sharedLock.lock();
try {
return cachedChunks.compute(chunkIndex, (index, chunk) -> {
if (chunk == null) {
chunk = new Chunk(chunkData, true, () -> releaseChunk(chunkIndex));
} else {
var dst = chunk.data().duplicate().clear();
Preconditions.checkArgument(chunkData.remaining() == dst.remaining());
dst.put(chunkData);
chunk.dirty().set(true);
}
chunk.currentAccesses().incrementAndGet();
return chunk;
});
} finally {
sharedLock.unlock();
}
}

Also here a case distinction is made: Is the Chunk not present? On false, we overwrite the data of the existing chunk by are using JDKs ByteBuffer::duplicate (getting an independet positon and limit.) and placing the data in the duplicate.

But this approach has the problem, that the chunks data limit is not adjusted! Hence, directly flushing the cache afterwards, this chunk contents will only be saved to the outdated limit before the write, leading to invalid ciphertext.

@infeo infeo added the bug label Jun 6, 2023
@infeo infeo added this to the 2.6.5 milestone Jun 6, 2023
@infeo infeo self-assigned this Jun 6, 2023
infeo added a commit that referenced this issue Jun 6, 2023
@overheadhunter
Copy link
Member

For the record, here is a visualization of what happens in the chunk cache:

// write 1.5 chunks of data:
|xxxx|xx--|

// write 2.5 chunks of data at same pos
|xxxx|xxxx|xx--|

chunks[1] is still cached from the first write, however as it was the EOF chunk, its limit was set to 1/2 of its capacity.

Due to the mentioned „full chunk update“ mechanism, while its bytes were correctly updated during the second write, the limit has not been updated. It still behaves as if it was the last chunk.

@infeo infeo closed this as completed in #174 Jun 7, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants