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

Possible invalid ciphertext file when truncate-open a file while concurrently writing to it #165

Closed
infeo opened this issue Mar 31, 2023 · 0 comments · Fixed by #166
Closed
Assignees
Labels
Milestone

Comments

@infeo
Copy link
Member

infeo commented Mar 31, 2023

CryptoFs manages per encrypted file several (cleartext, ciphertext)-filechannelpairs. Hence, it is possible, that while one filechannel is continuously writing to an encrypted file, a different file channel opens the same file with the TRUNCATE_EXISTING flag.

This can lead to a rare race condition:

ciphertextFileChannel = path.getFileSystem().provider().newFileChannel(path, options.createOpenOptionsForEncryptedFile(), attrs);
final FileHeader header;
final boolean isNewHeader;
if (ciphertextFileChannel.size() == 0l) {
header = headerHolder.createNew();
isNewHeader = true;
} else {
header = headerHolder.loadExisting(ciphertextFileChannel);
isNewHeader = false;
}
initFileSize(ciphertextFileChannel);
cleartextFileChannel = component.newChannelComponent() //
.create(ciphertextFileChannel, header, isNewHeader, options, this::channelClosed) //
.channel();

Keep in mind, that Thread t1 writes in the background continuously to the file. When thread t2 now opens a new file channel to the same file with TRUNCATE_EXISTING, it will end up in the above code of OpenCryptoFile::newFileChannel. In line 78 the ciphertextfilechannel with the trunacte flag is opened, and in line 81 we decide what file header we use based on the size of the encrypted file.

If between these two steps thread t2 is hibernated and t1 flushes its content, the size of the encrypted file is != 0 and no new header is used. But on the other hand we truncated the encrytped file, so nilling any the existing fileheader on storage side. And since we set newHeader = false, the header won't get written anymore, making the file undecryptable.

@infeo infeo added the bug label Mar 31, 2023
@infeo infeo added this to the 2.6.3 milestone Mar 31, 2023
@infeo infeo self-assigned this Mar 31, 2023
infeo added a commit that referenced this issue Mar 31, 2023
@infeo infeo modified the milestones: 2.6.3, 2.6.4 Apr 4, 2023
infeo added a commit that referenced this issue Apr 11, 2023
…ader

Share file header across all OpenFile objects and truncate encrypted files only to fileHeader.size()

Fixes #165, fixes #168
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.

1 participant