diff --git a/src/main/java/org/cryptomator/cryptofs/CiphertextPathValidations.java b/src/main/java/org/cryptomator/cryptofs/CiphertextPathValidations.java new file mode 100644 index 00000000..e0a9e635 --- /dev/null +++ b/src/main/java/org/cryptomator/cryptofs/CiphertextPathValidations.java @@ -0,0 +1,21 @@ +package org.cryptomator.cryptofs; + +import com.google.common.io.BaseEncoding; + +import java.nio.file.Path; + +public class CiphertextPathValidations { + + + private CiphertextPathValidations() {} + + public static boolean isCiphertextContentDir(Path p) { + var twoCharDir = p.getParent(); + if (twoCharDir == null) { + return false; + } + var testString = twoCharDir.getFileName().toString() + p.getFileName().toString(); + return testString.length() == 32 && BaseEncoding.base32().canDecode(testString); + } + +} diff --git a/src/main/java/org/cryptomator/cryptofs/DirectoryIdBackup.java b/src/main/java/org/cryptomator/cryptofs/DirectoryIdBackup.java index dc5029c7..da2f7dca 100644 --- a/src/main/java/org/cryptomator/cryptofs/DirectoryIdBackup.java +++ b/src/main/java/org/cryptomator/cryptofs/DirectoryIdBackup.java @@ -1,12 +1,12 @@ package org.cryptomator.cryptofs; +import jakarta.inject.Inject; import org.cryptomator.cryptofs.common.Constants; import org.cryptomator.cryptolib.api.CryptoException; import org.cryptomator.cryptolib.api.Cryptor; import org.cryptomator.cryptolib.common.DecryptingReadableByteChannel; import org.cryptomator.cryptolib.common.EncryptingWritableByteChannel; -import jakarta.inject.Inject; import java.io.IOException; import java.nio.ByteBuffer; import java.nio.channels.ByteChannel; @@ -65,6 +65,9 @@ public static void write(Cryptor cryptor, CiphertextDirectory ciphertextDirector * @throws IllegalStateException if the directory id exceeds {@value Constants#MAX_DIR_ID_LENGTH} chars */ public byte[] read(Path ciphertextContentDir) throws IOException, CryptoException, IllegalStateException { + if (!CiphertextPathValidations.isCiphertextContentDir(ciphertextContentDir)) { + throw new IllegalArgumentException("Directory %s is not a ciphertext content dir".formatted(ciphertextContentDir)); + } var dirIdBackupFile = getBackupFilePath(ciphertextContentDir); var dirIdBuffer = ByteBuffer.allocate(Constants.MAX_DIR_ID_LENGTH + 1); //a dir id contains at most 36 ascii chars, we add for security checks one more diff --git a/src/main/java/org/cryptomator/cryptofs/FileNameDecryptor.java b/src/main/java/org/cryptomator/cryptofs/FileNameDecryptor.java index 97296611..f87d0c34 100644 --- a/src/main/java/org/cryptomator/cryptofs/FileNameDecryptor.java +++ b/src/main/java/org/cryptomator/cryptofs/FileNameDecryptor.java @@ -43,7 +43,7 @@ public String decryptFilename(Path ciphertextNode) throws IOException, Unsupport String decryptFilenameInternal(Path ciphertextNode) throws IOException, UnsupportedOperationException { byte[] dirId = null; try { - dirId = dirIdBackup.read(ciphertextNode); + dirId = dirIdBackup.read(ciphertextNode.getParent()); } catch (NoSuchFileException e) { throw new UnsupportedOperationException("Directory does not have a " + Constants.DIR_ID_BACKUP_FILE_NAME + " file."); } catch (CryptoException | IllegalStateException e) { diff --git a/src/test/java/org/cryptomator/cryptofs/DirectoryIdBackupTest.java b/src/test/java/org/cryptomator/cryptofs/DirectoryIdBackupTest.java index fb120e4b..9b2f38b1 100644 --- a/src/test/java/org/cryptomator/cryptofs/DirectoryIdBackupTest.java +++ b/src/test/java/org/cryptomator/cryptofs/DirectoryIdBackupTest.java @@ -1,7 +1,7 @@ package org.cryptomator.cryptofs; +import com.google.common.io.BaseEncoding; import org.cryptomator.cryptofs.common.Constants; -import org.cryptomator.cryptofs.health.dirid.OrphanContentDirTest; import org.cryptomator.cryptofs.util.TestCryptoException; import org.cryptomator.cryptolib.api.CryptoException; import org.cryptomator.cryptolib.api.Cryptor; @@ -28,7 +28,7 @@ public class DirectoryIdBackupTest { @TempDir - Path contentPath; + Path testDir; private String dirId = "12345678"; private Cryptor cryptor; @@ -50,7 +50,7 @@ public class Write { @BeforeEach public void beforeEachWriteTest() { - ciphertextDirectoryObject = new CiphertextDirectory(dirId, contentPath); + ciphertextDirectoryObject = new CiphertextDirectory(dirId, testDir); encChannel = Mockito.mock(EncryptingWritableByteChannel.class); } @@ -62,7 +62,7 @@ public void testIdFileCreated() throws IOException { dirIdBackupSpy.write(ciphertextDirectoryObject); - Assertions.assertTrue(Files.exists(contentPath.resolve(Constants.DIR_ID_BACKUP_FILE_NAME))); + Assertions.assertTrue(Files.exists(testDir.resolve(Constants.DIR_ID_BACKUP_FILE_NAME))); } @Test @@ -83,22 +83,34 @@ public void testContentIsWritten() throws IOException { public class Read { private DecryptingReadableByteChannel decChannel; + private Path cipherContentDir; @BeforeEach public void beforeEachRead() throws IOException { - var backupFile = contentPath.resolve(Constants.DIR_ID_BACKUP_FILE_NAME); + var dirNames = BaseEncoding.base32().encode(new byte [20]); //a directory id hash is due to SHA1 always 20 bytes long + var twoCharDir = testDir.resolve(dirNames.substring(0,2)); + cipherContentDir = twoCharDir.resolve(dirNames.substring(2)); + var backupFile = cipherContentDir.resolve(Constants.DIR_ID_BACKUP_FILE_NAME); + Files.createDirectories(cipherContentDir); Files.writeString(backupFile, dirId, StandardCharsets.US_ASCII, StandardOpenOption.CREATE, StandardOpenOption.WRITE); decChannel = mock(DecryptingReadableByteChannel.class); } + @Test + @DisplayName("If the given path is not a cipherContentDir, throw IllegalArgumentException") + public void wrongPath() throws IOException { + var dirIdBackupSpy = spy(dirIdBackup); + Assertions.assertThrows(IllegalArgumentException.class, () -> dirIdBackupSpy.read(testDir)); + } + @Test @DisplayName("If the directory id is longer than 36 characters, throw IllegalStateException") public void contentLongerThan36Chars() throws IOException { var dirIdBackupSpy = spy(dirIdBackup); Mockito.when(dirIdBackupSpy.wrapDecryptionAround(Mockito.any(), Mockito.eq(cryptor))).thenReturn(decChannel); Mockito.when(decChannel.read(Mockito.any())).thenReturn(Constants.MAX_DIR_ID_LENGTH + 1); - Assertions.assertThrows(IllegalStateException.class, () -> dirIdBackupSpy.read(contentPath)); + Assertions.assertThrows(IllegalStateException.class, () -> dirIdBackupSpy.read(cipherContentDir)); } @Test @@ -108,7 +120,7 @@ public void invalidEncryptionThrowsCryptoException() throws IOException { var expectedException = new TestCryptoException(); Mockito.when(dirIdBackupSpy.wrapDecryptionAround(Mockito.any(), Mockito.eq(cryptor))).thenReturn(decChannel); Mockito.when(decChannel.read(Mockito.any())).thenThrow(expectedException); - var actual = Assertions.assertThrows(CryptoException.class, () -> dirIdBackupSpy.read(contentPath)); + var actual = Assertions.assertThrows(CryptoException.class, () -> dirIdBackupSpy.read(cipherContentDir)); Assertions.assertEquals(expectedException, actual); } @@ -119,7 +131,7 @@ public void ioException() throws IOException { var expectedException = new IOException("my oh my"); Mockito.when(dirIdBackupSpy.wrapDecryptionAround(Mockito.any(), Mockito.eq(cryptor))).thenReturn(decChannel); Mockito.when(decChannel.read(Mockito.any())).thenThrow(expectedException); - var actual = Assertions.assertThrows(IOException.class, () -> dirIdBackupSpy.read(contentPath)); + var actual = Assertions.assertThrows(IOException.class, () -> dirIdBackupSpy.read(cipherContentDir)); Assertions.assertEquals(expectedException, actual); } @@ -136,9 +148,10 @@ public void success() throws IOException { return expectedArray.length; }).when(decChannel).read(Mockito.any()); - var readDirId = dirIdBackupSpy.read(contentPath); + var readDirId = dirIdBackupSpy.read(cipherContentDir); Assertions.assertArrayEquals(expectedArray, readDirId); } + } diff --git a/src/test/java/org/cryptomator/cryptofs/FileNameDecryptorTest.java b/src/test/java/org/cryptomator/cryptofs/FileNameDecryptorTest.java index c5874d8b..0ae7422f 100644 --- a/src/test/java/org/cryptomator/cryptofs/FileNameDecryptorTest.java +++ b/src/test/java/org/cryptomator/cryptofs/FileNameDecryptorTest.java @@ -54,7 +54,7 @@ public void success(String fileExtension) throws IOException { var ciphertextNode = tmpPath.resolve(ciphertextNodeNameName + fileExtension); var dirId = new byte[]{'f', 'o', 'o', 'b', 'a', 'r'}; var expectedClearName = "veryClearText"; - when(dirIdBackup.read(ciphertextNode)).thenReturn(dirId); + when(dirIdBackup.read(tmpPath)).thenReturn(dirId); when(longFileNameProvider.inflate(ciphertextNode)).thenReturn(ciphertextNodeNameName); when(fileNameCryptor.decryptFilename(any(), eq(ciphertextNodeNameName), eq(dirId))).thenReturn(expectedClearName); @@ -78,7 +78,7 @@ public void validatePath() throws IOException { @DisplayName("If the dirId backup file does not exists, throw UnsupportedOperationException") public void notExistingDirIdFile() throws IOException { var ciphertextNode = tmpPath.resolve("toDecrypt.c9r"); - when(dirIdBackup.read(ciphertextNode)).thenThrow(NoSuchFileException.class); + when(dirIdBackup.read(tmpPath)).thenThrow(NoSuchFileException.class); Assertions.assertThrows(UnsupportedOperationException.class, () -> testObjSpy.decryptFilenameInternal(ciphertextNode)); } @@ -87,7 +87,7 @@ public void notExistingDirIdFile() throws IOException { @DisplayName("If the dirId cannot be read, throw FileSystemException") public void notReadableDirIdFile() throws IOException { var ciphertextNode = tmpPath.resolve("toDecrypt.c9r"); - when(dirIdBackup.read(ciphertextNode)) // + when(dirIdBackup.read(tmpPath)) // .thenThrow(TestCryptoException.class) // .thenThrow(IllegalStateException.class); Assertions.assertThrows(FileSystemException.class, () -> testObjSpy.decryptFilenameInternal(ciphertextNode)); @@ -101,7 +101,7 @@ public void notDecryptableCiphertext() throws IOException { var ciphertextNode = tmpPath.resolve(name + ".c9s"); var dirId = new byte[]{'f', 'o', 'o', 'b', 'a', 'r'}; var expectedException = new IOException("Inflation failed"); - when(dirIdBackup.read(ciphertextNode)).thenReturn(dirId); + when(dirIdBackup.read(tmpPath)).thenReturn(dirId); when(longFileNameProvider.inflate(ciphertextNode)).thenThrow(expectedException); var actual = Assertions.assertThrows(IOException.class, () -> testObjSpy.decryptFilenameInternal(ciphertextNode)); @@ -114,7 +114,7 @@ public void inflateThrows() throws IOException { var name = "toDecrypt"; var ciphertextNode = tmpPath.resolve(name + ".c9r"); var dirId = new byte[]{'f', 'o', 'o', 'b', 'a', 'r'}; - when(dirIdBackup.read(ciphertextNode)).thenReturn(dirId); + when(dirIdBackup.read(tmpPath)).thenReturn(dirId); when(fileNameCryptor.decryptFilename(any(), eq(name), eq(dirId))).thenThrow(TestCryptoException.class); Assertions.assertThrows(FileSystemException.class, () -> testObjSpy.decryptFilenameInternal(ciphertextNode)); diff --git a/src/test/java/org/cryptomator/cryptofs/health/dirid/OrphanContentDirTest.java b/src/test/java/org/cryptomator/cryptofs/health/dirid/OrphanContentDirTest.java index 934e1f58..81cb6fd1 100644 --- a/src/test/java/org/cryptomator/cryptofs/health/dirid/OrphanContentDirTest.java +++ b/src/test/java/org/cryptomator/cryptofs/health/dirid/OrphanContentDirTest.java @@ -32,6 +32,8 @@ import java.util.UUID; import java.util.concurrent.atomic.AtomicReference; +import static org.mockito.Mockito.mockStatic; + public class OrphanContentDirTest { @TempDir @@ -157,8 +159,8 @@ public void init() throws IOException { @Test @DisplayName("prepareStepParent() runs without error on not-existing stepparent") public void testPrepareStepParent() throws IOException { - try (var uuidClass = Mockito.mockStatic(UUID.class); // - var dirIdBackupClass = Mockito.mockStatic(DirectoryIdBackup.class)) { + try (var uuidClass = mockStatic(UUID.class); // + var dirIdBackupClass = mockStatic(DirectoryIdBackup.class)) { UUID uuid = Mockito.mock(UUID.class); uuidClass.when(UUID::randomUUID).thenReturn(uuid); Mockito.doReturn("aaaaaa").when(uuid).toString(); @@ -181,8 +183,8 @@ public void testPrepareStepParentExistingStepParentDir() throws IOException { Path cipherStepparent = dataDir.resolve("22/2222"); Files.createDirectories(cipherStepparent); - try (var uuidClass = Mockito.mockStatic(UUID.class); // - var dirIdBackupClass = Mockito.mockStatic(DirectoryIdBackup.class)) { + try (var uuidClass = mockStatic(UUID.class); // + var dirIdBackupClass = mockStatic(DirectoryIdBackup.class)) { UUID uuid = Mockito.mock(UUID.class); uuidClass.when(UUID::randomUUID).thenReturn(uuid); Mockito.doReturn("aaaaaa").when(uuid).toString(); @@ -205,8 +207,8 @@ public void testPrepareStepParentOrphanedStepParentDir() throws IOException { Path cipherStepparent = dataDir.resolve("22/2222"); Files.createDirectories(cipherStepparent); - try (var uuidClass = Mockito.mockStatic(UUID.class); // - var dirIdBackupClass = Mockito.mockStatic(DirectoryIdBackup.class)) { + try (var uuidClass = mockStatic(UUID.class); // + var dirIdBackupClass = mockStatic(DirectoryIdBackup.class)) { UUID uuid = Mockito.mock(UUID.class); uuidClass.when(UUID::randomUUID).thenReturn(uuid); Mockito.doReturn("aaaaaa").when(uuid).toString(); @@ -238,7 +240,7 @@ public void init() { @DisplayName("Successful reading dirId from backup file") public void success() { var dirId = new byte[]{'f', 'o', 'o'}; - try (var dirIdBackupMock = Mockito.mockStatic(DirectoryIdBackup.class)) { + try (var dirIdBackupMock = mockStatic(DirectoryIdBackup.class)) { dirIdBackupMock.when(() -> DirectoryIdBackup.read(cryptor, cipherOrphan)).thenReturn(dirId); var result = resultSpy.retrieveDirId(cipherOrphan, cryptor); Assertions.assertTrue(result.isPresent()); @@ -250,7 +252,7 @@ public void success() { @DisplayName("retrieveDirId returns an empty optional on any exception") @FieldSource("expectedExceptions") public void testRetrieveDirIdIOExceptionReadingFile(Throwable t) throws IOException { - try (var dirIdBackupMock = Mockito.mockStatic(DirectoryIdBackup.class)) { + try (var dirIdBackupMock = mockStatic(DirectoryIdBackup.class)) { dirIdBackupMock.when(() -> DirectoryIdBackup.read(cryptor, cipherOrphan)).thenThrow(t); var notExistingResult = resultSpy.retrieveDirId(cipherOrphan, cryptor); Assertions.assertTrue(notExistingResult.isEmpty()); @@ -339,7 +341,7 @@ public void testAdoptOrphanedShortened() throws IOException { Files.createDirectories(stepParentDir.path()); Mockito.doReturn("adopted").when(fileNameCryptor).encryptFilename(Mockito.any(), Mockito.any(), Mockito.any()); - try (var baseEncodingClass = Mockito.mockStatic(BaseEncoding.class)) { + try (var baseEncodingClass = mockStatic(BaseEncoding.class)) { MessageDigest sha1 = Mockito.mock(MessageDigest.class); Mockito.doReturn(new byte[]{}).when(sha1).digest(Mockito.any()); @@ -367,7 +369,7 @@ public void testAdoptOrphanedShortenedMissingNameC9s() throws IOException { Files.createDirectories(stepParentDir.path()); Mockito.doReturn("adopted").when(fileNameCryptor).encryptFilename(Mockito.any(), Mockito.any(), Mockito.any()); - try (var baseEncodingClass = Mockito.mockStatic(BaseEncoding.class)) { + try (var baseEncodingClass = mockStatic(BaseEncoding.class)) { MessageDigest sha1 = Mockito.mock(MessageDigest.class); Mockito.doReturn(new byte[]{}).when(sha1).digest(Mockito.any()); @@ -410,7 +412,11 @@ public void testFixNoDirId() throws IOException { return null; }).when(resultSpy).adoptOrphanedResource(Mockito.any(), Mockito.any(), Mockito.anyBoolean(), Mockito.eq(stepParentDir), Mockito.eq(fileNameCryptor), Mockito.any()); - resultSpy.fix(pathToVault, config, masterkey, cryptor); + try ( var dirIdBackup = mockStatic(DirectoryIdBackup.class)) { + dirIdBackup.when(() -> DirectoryIdBackup.read(cryptor, cipherOrphan)).thenThrow(IllegalStateException.class); + resultSpy.fix(pathToVault, config, masterkey, cryptor); + } + Mockito.verify(resultSpy, Mockito.times(2)).adoptOrphanedResource(Mockito.any(), Mockito.any(), Mockito.anyBoolean(), Mockito.eq(stepParentDir), Mockito.eq(fileNameCryptor), Mockito.any()); Assertions.assertTrue(Files.notExists(cipherOrphan));