Skip to content

Commit 815ed6b

Browse files
infeoinfeo
authored
Fix DecryptFileName feature (#290)
fixes #289
1 parent 6a4ed6f commit 815ed6b

File tree

6 files changed

+70
-27
lines changed

6 files changed

+70
-27
lines changed

‎src/main/java/org/cryptomator/cryptofs/CiphertextPathValidations.java‎

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
package org.cryptomator.cryptofs;
2+
3+
import com.google.common.io.BaseEncoding;
4+
5+
import java.nio.file.Path;
6+
7+
public class CiphertextPathValidations {
8+
9+
10+
private CiphertextPathValidations() {}
11+
12+
public static boolean isCiphertextContentDir(Path p) {
13+
var twoCharDir = p.getParent();
14+
if (twoCharDir == null) {
15+
return false;
16+
}
17+
var testString = twoCharDir.getFileName().toString() + p.getFileName().toString();
18+
return testString.length() == 32 && BaseEncoding.base32().canDecode(testString);
19+
}
20+
21+
}

‎src/main/java/org/cryptomator/cryptofs/DirectoryIdBackup.java‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,12 @@
11
package org.cryptomator.cryptofs;
22

3+
import jakarta.inject.Inject;
34
import org.cryptomator.cryptofs.common.Constants;
45
import org.cryptomator.cryptolib.api.CryptoException;
56
import org.cryptomator.cryptolib.api.Cryptor;
67
import org.cryptomator.cryptolib.common.DecryptingReadableByteChannel;
78
import org.cryptomator.cryptolib.common.EncryptingWritableByteChannel;
89

9-
import jakarta.inject.Inject;
1010
import java.io.IOException;
1111
import java.nio.ByteBuffer;
1212
import java.nio.channels.ByteChannel;
@@ -65,6 +65,9 @@ public static void write(Cryptor cryptor, CiphertextDirectory ciphertextDirector
6565
* @throws IllegalStateException if the directory id exceeds {@value Constants#MAX_DIR_ID_LENGTH} chars
6666
*/
6767
public byte[] read(Path ciphertextContentDir) throws IOException, CryptoException, IllegalStateException {
68+
if (!CiphertextPathValidations.isCiphertextContentDir(ciphertextContentDir)) {
69+
throw new IllegalArgumentException("Directory %s is not a ciphertext content dir".formatted(ciphertextContentDir));
70+
}
6871
var dirIdBackupFile = getBackupFilePath(ciphertextContentDir);
6972
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
7073

‎src/main/java/org/cryptomator/cryptofs/FileNameDecryptor.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,7 @@ public String decryptFilename(Path ciphertextNode) throws IOException, Unsupport
4343
String decryptFilenameInternal(Path ciphertextNode) throws IOException, UnsupportedOperationException {
4444
byte[] dirId = null;
4545
try {
46-
dirId = dirIdBackup.read(ciphertextNode);
46+
dirId = dirIdBackup.read(ciphertextNode.getParent());
4747
} catch (NoSuchFileException e) {
4848
throw new UnsupportedOperationException("Directory does not have a " + Constants.DIR_ID_BACKUP_FILE_NAME + " file.");
4949
} catch (CryptoException | IllegalStateException e) {

‎src/test/java/org/cryptomator/cryptofs/DirectoryIdBackupTest.java‎

Lines changed: 22 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
package org.cryptomator.cryptofs;
22

3+
import com.google.common.io.BaseEncoding;
34
import org.cryptomator.cryptofs.common.Constants;
4-
import org.cryptomator.cryptofs.health.dirid.OrphanContentDirTest;
55
import org.cryptomator.cryptofs.util.TestCryptoException;
66
import org.cryptomator.cryptolib.api.CryptoException;
77
import org.cryptomator.cryptolib.api.Cryptor;
@@ -28,7 +28,7 @@
2828
public class DirectoryIdBackupTest {
2929

3030
@TempDir
31-
Path contentPath;
31+
Path testDir;
3232

3333
private String dirId = "12345678";
3434
private Cryptor cryptor;
@@ -50,7 +50,7 @@ public class Write {
5050

5151
@BeforeEach
5252
public void beforeEachWriteTest() {
53-
ciphertextDirectoryObject = new CiphertextDirectory(dirId, contentPath);
53+
ciphertextDirectoryObject = new CiphertextDirectory(dirId, testDir);
5454
encChannel = Mockito.mock(EncryptingWritableByteChannel.class);
5555
}
5656

@@ -62,7 +62,7 @@ public void testIdFileCreated() throws IOException {
6262

6363
dirIdBackupSpy.write(ciphertextDirectoryObject);
6464

65-
Assertions.assertTrue(Files.exists(contentPath.resolve(Constants.DIR_ID_BACKUP_FILE_NAME)));
65+
Assertions.assertTrue(Files.exists(testDir.resolve(Constants.DIR_ID_BACKUP_FILE_NAME)));
6666
}
6767

6868
@Test
@@ -83,22 +83,34 @@ public void testContentIsWritten() throws IOException {
8383
public class Read {
8484

8585
private DecryptingReadableByteChannel decChannel;
86+
private Path cipherContentDir;
8687

8788
@BeforeEach
8889
public void beforeEachRead() throws IOException {
89-
var backupFile = contentPath.resolve(Constants.DIR_ID_BACKUP_FILE_NAME);
90+
var dirNames = BaseEncoding.base32().encode(new byte [20]); //a directory id hash is due to SHA1 always 20 bytes long
91+
var twoCharDir = testDir.resolve(dirNames.substring(0,2));
92+
cipherContentDir = twoCharDir.resolve(dirNames.substring(2));
93+
var backupFile = cipherContentDir.resolve(Constants.DIR_ID_BACKUP_FILE_NAME);
94+
Files.createDirectories(cipherContentDir);
9095
Files.writeString(backupFile, dirId, StandardCharsets.US_ASCII, StandardOpenOption.CREATE, StandardOpenOption.WRITE);
9196

9297
decChannel = mock(DecryptingReadableByteChannel.class);
9398
}
9499

100+
@Test
101+
@DisplayName("If the given path is not a cipherContentDir, throw IllegalArgumentException")
102+
public void wrongPath() throws IOException {
103+
var dirIdBackupSpy = spy(dirIdBackup);
104+
Assertions.assertThrows(IllegalArgumentException.class, () -> dirIdBackupSpy.read(testDir));
105+
}
106+
95107
@Test
96108
@DisplayName("If the directory id is longer than 36 characters, throw IllegalStateException")
97109
public void contentLongerThan36Chars() throws IOException {
98110
var dirIdBackupSpy = spy(dirIdBackup);
99111
Mockito.when(dirIdBackupSpy.wrapDecryptionAround(Mockito.any(), Mockito.eq(cryptor))).thenReturn(decChannel);
100112
Mockito.when(decChannel.read(Mockito.any())).thenReturn(Constants.MAX_DIR_ID_LENGTH + 1);
101-
Assertions.assertThrows(IllegalStateException.class, () -> dirIdBackupSpy.read(contentPath));
113+
Assertions.assertThrows(IllegalStateException.class, () -> dirIdBackupSpy.read(cipherContentDir));
102114
}
103115

104116
@Test
@@ -108,7 +120,7 @@ public void invalidEncryptionThrowsCryptoException() throws IOException {
108120
var expectedException = new TestCryptoException();
109121
Mockito.when(dirIdBackupSpy.wrapDecryptionAround(Mockito.any(), Mockito.eq(cryptor))).thenReturn(decChannel);
110122
Mockito.when(decChannel.read(Mockito.any())).thenThrow(expectedException);
111-
var actual = Assertions.assertThrows(CryptoException.class, () -> dirIdBackupSpy.read(contentPath));
123+
var actual = Assertions.assertThrows(CryptoException.class, () -> dirIdBackupSpy.read(cipherContentDir));
112124
Assertions.assertEquals(expectedException, actual);
113125
}
114126

@@ -119,7 +131,7 @@ public void ioException() throws IOException {
119131
var expectedException = new IOException("my oh my");
120132
Mockito.when(dirIdBackupSpy.wrapDecryptionAround(Mockito.any(), Mockito.eq(cryptor))).thenReturn(decChannel);
121133
Mockito.when(decChannel.read(Mockito.any())).thenThrow(expectedException);
122-
var actual = Assertions.assertThrows(IOException.class, () -> dirIdBackupSpy.read(contentPath));
134+
var actual = Assertions.assertThrows(IOException.class, () -> dirIdBackupSpy.read(cipherContentDir));
123135
Assertions.assertEquals(expectedException, actual);
124136
}
125137

@@ -136,9 +148,10 @@ public void success() throws IOException {
136148
return expectedArray.length;
137149
}).when(decChannel).read(Mockito.any());
138150

139-
var readDirId = dirIdBackupSpy.read(contentPath);
151+
var readDirId = dirIdBackupSpy.read(cipherContentDir);
140152
Assertions.assertArrayEquals(expectedArray, readDirId);
141153
}
154+
142155
}
143156

144157

‎src/test/java/org/cryptomator/cryptofs/FileNameDecryptorTest.java‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,7 @@ public void success(String fileExtension) throws IOException {
5454
var ciphertextNode = tmpPath.resolve(ciphertextNodeNameName + fileExtension);
5555
var dirId = new byte[]{'f', 'o', 'o', 'b', 'a', 'r'};
5656
var expectedClearName = "veryClearText";
57-
when(dirIdBackup.read(ciphertextNode)).thenReturn(dirId);
57+
when(dirIdBackup.read(tmpPath)).thenReturn(dirId);
5858
when(longFileNameProvider.inflate(ciphertextNode)).thenReturn(ciphertextNodeNameName);
5959
when(fileNameCryptor.decryptFilename(any(), eq(ciphertextNodeNameName), eq(dirId))).thenReturn(expectedClearName);
6060

@@ -78,7 +78,7 @@ public void validatePath() throws IOException {
7878
@DisplayName("If the dirId backup file does not exists, throw UnsupportedOperationException")
7979
public void notExistingDirIdFile() throws IOException {
8080
var ciphertextNode = tmpPath.resolve("toDecrypt.c9r");
81-
when(dirIdBackup.read(ciphertextNode)).thenThrow(NoSuchFileException.class);
81+
when(dirIdBackup.read(tmpPath)).thenThrow(NoSuchFileException.class);
8282

8383
Assertions.assertThrows(UnsupportedOperationException.class, () -> testObjSpy.decryptFilenameInternal(ciphertextNode));
8484
}
@@ -87,7 +87,7 @@ public void notExistingDirIdFile() throws IOException {
8787
@DisplayName("If the dirId cannot be read, throw FileSystemException")
8888
public void notReadableDirIdFile() throws IOException {
8989
var ciphertextNode = tmpPath.resolve("toDecrypt.c9r");
90-
when(dirIdBackup.read(ciphertextNode)) //
90+
when(dirIdBackup.read(tmpPath)) //
9191
.thenThrow(TestCryptoException.class) //
9292
.thenThrow(IllegalStateException.class);
9393
Assertions.assertThrows(FileSystemException.class, () -> testObjSpy.decryptFilenameInternal(ciphertextNode));
@@ -101,7 +101,7 @@ public void notDecryptableCiphertext() throws IOException {
101101
var ciphertextNode = tmpPath.resolve(name + ".c9s");
102102
var dirId = new byte[]{'f', 'o', 'o', 'b', 'a', 'r'};
103103
var expectedException = new IOException("Inflation failed");
104-
when(dirIdBackup.read(ciphertextNode)).thenReturn(dirId);
104+
when(dirIdBackup.read(tmpPath)).thenReturn(dirId);
105105
when(longFileNameProvider.inflate(ciphertextNode)).thenThrow(expectedException);
106106

107107
var actual = Assertions.assertThrows(IOException.class, () -> testObjSpy.decryptFilenameInternal(ciphertextNode));
@@ -114,7 +114,7 @@ public void inflateThrows() throws IOException {
114114
var name = "toDecrypt";
115115
var ciphertextNode = tmpPath.resolve(name + ".c9r");
116116
var dirId = new byte[]{'f', 'o', 'o', 'b', 'a', 'r'};
117-
when(dirIdBackup.read(ciphertextNode)).thenReturn(dirId);
117+
when(dirIdBackup.read(tmpPath)).thenReturn(dirId);
118118
when(fileNameCryptor.decryptFilename(any(), eq(name), eq(dirId))).thenThrow(TestCryptoException.class);
119119

120120
Assertions.assertThrows(FileSystemException.class, () -> testObjSpy.decryptFilenameInternal(ciphertextNode));

‎src/test/java/org/cryptomator/cryptofs/health/dirid/OrphanContentDirTest.java‎

Lines changed: 17 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,8 @@
3232
import java.util.UUID;
3333
import java.util.concurrent.atomic.AtomicReference;
3434

35+
import static org.mockito.Mockito.mockStatic;
36+
3537
public class OrphanContentDirTest {
3638

3739
@TempDir
@@ -157,8 +159,8 @@ public void init() throws IOException {
157159
@Test
158160
@DisplayName("prepareStepParent() runs without error on not-existing stepparent")
159161
public void testPrepareStepParent() throws IOException {
160-
try (var uuidClass = Mockito.mockStatic(UUID.class); //
161-
var dirIdBackupClass = Mockito.mockStatic(DirectoryIdBackup.class)) {
162+
try (var uuidClass = mockStatic(UUID.class); //
163+
var dirIdBackupClass = mockStatic(DirectoryIdBackup.class)) {
162164
UUID uuid = Mockito.mock(UUID.class);
163165
uuidClass.when(UUID::randomUUID).thenReturn(uuid);
164166
Mockito.doReturn("aaaaaa").when(uuid).toString();
@@ -181,8 +183,8 @@ public void testPrepareStepParentExistingStepParentDir() throws IOException {
181183
Path cipherStepparent = dataDir.resolve("22/2222");
182184
Files.createDirectories(cipherStepparent);
183185

184-
try (var uuidClass = Mockito.mockStatic(UUID.class); //
185-
var dirIdBackupClass = Mockito.mockStatic(DirectoryIdBackup.class)) {
186+
try (var uuidClass = mockStatic(UUID.class); //
187+
var dirIdBackupClass = mockStatic(DirectoryIdBackup.class)) {
186188
UUID uuid = Mockito.mock(UUID.class);
187189
uuidClass.when(UUID::randomUUID).thenReturn(uuid);
188190
Mockito.doReturn("aaaaaa").when(uuid).toString();
@@ -205,8 +207,8 @@ public void testPrepareStepParentOrphanedStepParentDir() throws IOException {
205207
Path cipherStepparent = dataDir.resolve("22/2222");
206208
Files.createDirectories(cipherStepparent);
207209

208-
try (var uuidClass = Mockito.mockStatic(UUID.class); //
209-
var dirIdBackupClass = Mockito.mockStatic(DirectoryIdBackup.class)) {
210+
try (var uuidClass = mockStatic(UUID.class); //
211+
var dirIdBackupClass = mockStatic(DirectoryIdBackup.class)) {
210212
UUID uuid = Mockito.mock(UUID.class);
211213
uuidClass.when(UUID::randomUUID).thenReturn(uuid);
212214
Mockito.doReturn("aaaaaa").when(uuid).toString();
@@ -238,7 +240,7 @@ public void init() {
238240
@DisplayName("Successful reading dirId from backup file")
239241
public void success() {
240242
var dirId = new byte[]{'f', 'o', 'o'};
241-
try (var dirIdBackupMock = Mockito.mockStatic(DirectoryIdBackup.class)) {
243+
try (var dirIdBackupMock = mockStatic(DirectoryIdBackup.class)) {
242244
dirIdBackupMock.when(() -> DirectoryIdBackup.read(cryptor, cipherOrphan)).thenReturn(dirId);
243245
var result = resultSpy.retrieveDirId(cipherOrphan, cryptor);
244246
Assertions.assertTrue(result.isPresent());
@@ -250,7 +252,7 @@ public void success() {
250252
@DisplayName("retrieveDirId returns an empty optional on any exception")
251253
@FieldSource("expectedExceptions")
252254
public void testRetrieveDirIdIOExceptionReadingFile(Throwable t) throws IOException {
253-
try (var dirIdBackupMock = Mockito.mockStatic(DirectoryIdBackup.class)) {
255+
try (var dirIdBackupMock = mockStatic(DirectoryIdBackup.class)) {
254256
dirIdBackupMock.when(() -> DirectoryIdBackup.read(cryptor, cipherOrphan)).thenThrow(t);
255257
var notExistingResult = resultSpy.retrieveDirId(cipherOrphan, cryptor);
256258
Assertions.assertTrue(notExistingResult.isEmpty());
@@ -339,7 +341,7 @@ public void testAdoptOrphanedShortened() throws IOException {
339341
Files.createDirectories(stepParentDir.path());
340342

341343
Mockito.doReturn("adopted").when(fileNameCryptor).encryptFilename(Mockito.any(), Mockito.any(), Mockito.any());
342-
try (var baseEncodingClass = Mockito.mockStatic(BaseEncoding.class)) {
344+
try (var baseEncodingClass = mockStatic(BaseEncoding.class)) {
343345
MessageDigest sha1 = Mockito.mock(MessageDigest.class);
344346
Mockito.doReturn(new byte[]{}).when(sha1).digest(Mockito.any());
345347

@@ -367,7 +369,7 @@ public void testAdoptOrphanedShortenedMissingNameC9s() throws IOException {
367369
Files.createDirectories(stepParentDir.path());
368370

369371
Mockito.doReturn("adopted").when(fileNameCryptor).encryptFilename(Mockito.any(), Mockito.any(), Mockito.any());
370-
try (var baseEncodingClass = Mockito.mockStatic(BaseEncoding.class)) {
372+
try (var baseEncodingClass = mockStatic(BaseEncoding.class)) {
371373
MessageDigest sha1 = Mockito.mock(MessageDigest.class);
372374
Mockito.doReturn(new byte[]{}).when(sha1).digest(Mockito.any());
373375

@@ -410,7 +412,11 @@ public void testFixNoDirId() throws IOException {
410412
return null;
411413
}).when(resultSpy).adoptOrphanedResource(Mockito.any(), Mockito.any(), Mockito.anyBoolean(), Mockito.eq(stepParentDir), Mockito.eq(fileNameCryptor), Mockito.any());
412414

413-
resultSpy.fix(pathToVault, config, masterkey, cryptor);
415+
try ( var dirIdBackup = mockStatic(DirectoryIdBackup.class)) {
416+
dirIdBackup.when(() -> DirectoryIdBackup.read(cryptor, cipherOrphan)).thenThrow(IllegalStateException.class);
417+
resultSpy.fix(pathToVault, config, masterkey, cryptor);
418+
}
419+
414420

415421
Mockito.verify(resultSpy, Mockito.times(2)).adoptOrphanedResource(Mockito.any(), Mockito.any(), Mockito.anyBoolean(), Mockito.eq(stepParentDir), Mockito.eq(fileNameCryptor), Mockito.any());
416422
Assertions.assertTrue(Files.notExists(cipherOrphan));

0 commit comments

Comments
 (0)