cryptomator / cryptofs

Java Filesystem Provider with integrated encryption
GNU Affero General Public License v3.0
94 stars 35 forks source link

Atomicity does not seem to be given in certain scenarios #246

Open Bios-Marcel opened 2 months ago

Bios-Marcel commented 2 months ago

Hey,

We are currently trying to use cryptofs as the backing filesystem for https://github.com/eclipse-store.

EclipseStore offers a type of transaction object, where you can store multiple things and then commit them, flushing them to the storage target.

In this scenario the storage target is a cryptofs instance. When killing the JVM abruptly (not graceful), this will cause broken files that can't be decrypted anymore.

I have created a reproducer repo here:

https://github.com/Bios-Marcel/cryptofs_bug_reproducer

You can switch between the uncrypted and encrypted versions of the reproducer by commenting / uncommenting the respective first line in EclipseStoreMain.

To cause the issue, you need to kill the JVM after the console puts out Committing, but before it says Committed. Given that cryptofs is a filesystem abstraction, I am doubting that EclipseStore does anything wrong here, especially since there is no issue in reading / writing the data without encryption.

Here's the exception:

Exception in thread "main" org.eclipse.store.storage.exceptions.StorageException: Problem in channel #0
    at org.eclipse.store.storage.types.StorageChannelTask$Abstract.checkForProblems(StorageChannelTask.java:106)
    at org.eclipse.store.storage.types.StorageChannelTask$Abstract.waitOnCompletion(StorageChannelTask.java:168)
    at org.eclipse.store.storage.types.StorageSystem$Default.startThreads(StorageSystem.java:332)
    at org.eclipse.store.storage.types.StorageSystem$Default.internalStartUp(StorageSystem.java:515)
    at org.eclipse.store.storage.types.StorageSystem$Default.start(StorageSystem.java:601)
    at org.eclipse.store.storage.types.StorageSystem$Default.start(StorageSystem.java:71)
    at org.eclipse.store.storage.embedded.types.EmbeddedStorageManager$Default.start(EmbeddedStorageManager.java:247)
    at org.eclipse.store.storage.embedded.types.EmbeddedStorageManager$Default.start(EmbeddedStorageManager.java:95)
    at EclipseStoreMain.main(EclipseStoreMain.java:33)
Caused by: org.eclipse.store.storage.exceptions.StorageExceptionIoReading
    at org.eclipse.store.storage.types.StorageEntityInitializer$Default.fillBuffer(StorageEntityInitializer.java:279)
    at org.eclipse.store.storage.types.StorageEntityInitializer$Default.indexEntities(StorageEntityInitializer.java:181)
    at org.eclipse.store.storage.types.StorageEntityInitializer$Default.registerFileEntities(StorageEntityInitializer.java:127)
    at org.eclipse.store.storage.types.StorageEntityInitializer$Default.registerEntities(StorageEntityInitializer.java:103)
    at org.eclipse.store.storage.types.StorageEntityInitializer$Default.registerEntities(StorageEntityInitializer.java:85)
    at org.eclipse.store.storage.types.StorageEntityInitializer$Default.registerEntities(StorageEntityInitializer.java:48)
    at org.eclipse.store.storage.types.StorageFileManager$Default.initializeForExistingFiles(StorageFileManager.java:1002)
    at org.eclipse.store.storage.types.StorageFileManager$Default.initializeStorage(StorageFileManager.java:886)
    at org.eclipse.store.storage.types.StorageChannel$Default.initializeStorage(StorageChannel.java:782)
    at org.eclipse.store.storage.types.StorageChannelTaskInitialize$Default.succeed(StorageChannelTaskInitialize.java:200)
    at org.eclipse.store.storage.types.StorageChannelTaskInitialize$Default.succeed(StorageChannelTaskInitialize.java:34)
    at org.eclipse.store.storage.types.StorageChannelSynchronizingTask$AbstractCompletingTask.synchronizedComplete(StorageChannelSynchronizingTask.java:78)
    at org.eclipse.store.storage.types.StorageChannelSynchronizingTask$AbstractCompletingTask.complete(StorageChannelSynchronizingTask.java:126)
    at org.eclipse.store.storage.types.StorageChannelTask$Abstract.processBy(StorageChannelTask.java:260)
    at org.eclipse.store.storage.types.StorageChannel$Default.work(StorageChannel.java:453)
    at org.eclipse.store.storage.types.StorageChannel$Default.run(StorageChannel.java:536)
    at java.base/java.lang.Thread.run(Thread.java:1583)
Caused by: org.eclipse.store.storage.exceptions.StorageExceptionIoReading
    at org.eclipse.store.storage.types.StorageFile$Abstract.readBytes(StorageFile.java:176)
    at org.eclipse.store.storage.types.StorageEntityInitializer$Default.fillBuffer(StorageEntityInitializer.java:275)
    ... 16 more
Caused by: org.eclipse.serializer.exceptions.IORuntimeException: java.io.IOException: org.cryptomator.cryptolib.api.AuthenticationFailedException: Content tag mismatch.
    at org.eclipse.store.afs.nio.types.NioIoHandler$Default.specificReadBytes(NioIoHandler.java:453)
    at org.eclipse.store.afs.nio.types.NioIoHandler$Default.specificReadBytes(NioIoHandler.java:81)
    at org.eclipse.serializer.afs.types.AIoHandler$Abstract.readBytes(AIoHandler.java:790)
    at org.eclipse.serializer.afs.types.AReadableFile.readBytes(AReadableFile.java:104)
    at org.eclipse.store.storage.types.StorageFile$Abstract.readBytes(StorageFile.java:172)
    ... 17 more
Caused by: java.io.IOException: org.cryptomator.cryptolib.api.AuthenticationFailedException: Content tag mismatch.
    at org.cryptomator.cryptofs.fh.ChunkCache.getChunk(ChunkCache.java:123)
    at org.cryptomator.cryptofs.ch.CleartextFileChannel.readLocked(CleartextFileChannel.java:112)
    at org.cryptomator.cryptofs.ch.AbstractFileChannel.read(AbstractFileChannel.java:155)
    at org.eclipse.serializer.io.XIO.internalRead(XIO.java:1238)
    at org.eclipse.serializer.io.XIO.read(XIO.java:1198)
    at org.eclipse.store.afs.nio.types.NioIoHandler$Default.specificReadBytes(NioIoHandler.java:449)
    ... 21 more
Caused by: org.cryptomator.cryptolib.api.AuthenticationFailedException: Content tag mismatch.
    at org.cryptomator.cryptolib.v2.FileContentCryptorImpl.decryptChunk(FileContentCryptorImpl.java:146)
    at org.cryptomator.cryptolib.v2.FileContentCryptorImpl.decryptChunk(FileContentCryptorImpl.java:97)
    at org.cryptomator.cryptofs.fh.ChunkLoader.load(ChunkLoader.java:40)
    at org.cryptomator.cryptofs.fh.ChunkCache.loadChunk(ChunkCache.java:133)
    at org.cryptomator.cryptofs.fh.ChunkCache.lambda$getChunk$2(ChunkCache.java:117)
    at com.github.benmanes.caffeine.cache.BoundedLocalCache.lambda$remap$16(BoundedLocalCache.java:2858)
    at java.base/java.util.concurrent.ConcurrentHashMap.compute(ConcurrentHashMap.java:1916)
    at com.github.benmanes.caffeine.cache.BoundedLocalCache.remap(BoundedLocalCache.java:2853)
    at com.github.benmanes.caffeine.cache.BoundedLocalCache.compute(BoundedLocalCache.java:2803)
    at com.github.benmanes.caffeine.cache.LocalCache.compute(LocalCache.java:99)
    at org.cryptomator.cryptofs.fh.ChunkCache.getChunk(ChunkCache.java:115)
    ... 26 more
Caused by: javax.crypto.AEADBadTagException: Tag mismatch
    at java.base/com.sun.crypto.provider.GaloisCounterMode$GCMDecrypt.doFinal(GaloisCounterMode.java:1652)
    at java.base/com.sun.crypto.provider.GaloisCounterMode.engineDoFinal(GaloisCounterMode.java:458)
    at java.base/javax.crypto.Cipher.doFinal(Cipher.java:2543)
    at org.cryptomator.cryptolib.v2.FileContentCryptorImpl.decryptChunk(FileContentCryptorImpl.java:143)
    ... 36 more
tobihagemann commented 2 months ago

Hi there, very interesting use of this project. However, I'm not sure if atomicity is something that we can ever achieve. I mean... it's a filesystem and not a database.

The exception you're seeing makes sense. It means that the file/chunk cannot be authenticated, see file content encryption. The file/chunk was probably unable to finish writing and killed during the process, which would screw up the authentication tag, necessary for GCM.

What you're seeing is basically a broken file. Something that would also occur, when dealing with "regular" filesystems in such a scenario. The only difference is that cryptofs fails to deliver the contents of a file, if something goes wrong during decryption. #4 may be something that you're looking for, but was closed due to the introduction of GCM.

To be honest, this is where my knowledge about all this ends. You may have to catch the AuthenticationFailedException and somehow deal with unreadable files.

Maybe someone else can chime in on more specifics, not sure if I got something wrong.

Bios-Marcel commented 2 months ago

Well, after thinking about it, it makes sense that this does not work.

Maybe our usecase is just not a good idea.

bugabinga commented 2 months ago

@tobihagemann atomicity is not something a file system can provide, i think.

but even if it could, it would not solve the above issue. even atomic operations will fail, if power was cut in the middle of them.

isn't this issue rather about failure recovery? what is, for example, cryptofs stopping from ignoring broken files instead of bringing the whole file system down, just because one file could not be authenticated?

tobihagemann commented 2 months ago

but even if it could, it would not solve the above issue. even atomic operations will fail, if power was cut in the middle of them.

I agree.

what is, for example, cryptofs stopping from ignoring broken files instead of bringing the whole file system down, just because one file could not be authenticated?

Oh yeah, that's what I've got wrong. So... nothing, because this should already be the case. This exception doesn't bring the whole file system down. 😄 But the exception is obviously still reported.

Or what am I missing here?

Bios-Marcel commented 2 months ago

Oh yeah, that's what I've got wrong. So... nothing, because this should already be the case. This exception doesn't bring the whole file system down. 😄 But the exception is obviously still reported.

Yes, I think it is simply failing to read specific files. The filesystem as a whole is still functional, I have tested that. But even so, we can't really do anything here.

bugabinga commented 2 months ago

Oh yeah, that's what I've got wrong

I did not mean to insult your intelligence, only looking for potential solutions here.