From 679fd23d562f73845b968d5308052affcfec5b48 Mon Sep 17 00:00:00 2001 From: Tobias Hagemann Date: Fri, 1 Sep 2017 17:00:32 +0200 Subject: [PATCH 1/8] Update README.md --- README.md | 30 +++++++++++++----------------- 1 file changed, 13 insertions(+), 17 deletions(-) diff --git a/README.md b/README.md index 6b9f2e1c..37075310 100644 --- a/README.md +++ b/README.md @@ -5,27 +5,23 @@ [![Codacy Badge](https://api.codacy.com/project/badge/Coverage/7248ca7d466843f785f79f33374302c2)](https://www.codacy.com/app/cryptomator/cryptofs?utm_source=github.com&utm_medium=referral&utm_content=cryptomator/cryptofs&utm_campaign=Badge_Coverage) [![Coverity Scan Build Status](https://scan.coverity.com/projects/10006/badge.svg)](https://scan.coverity.com/projects/cryptomator-cryptofs) -**CryptoFS** - Implementation of the [Cryptomator](https://github.com/cryptomator/cryptomator) encryption scheme. - -## Disclaimer - -This project is in an early stage and not ready for production use. We recommend to use it only for testing and evaluation purposes. +**CryptoFS:** Implementation of the [Cryptomator](https://github.com/cryptomator/cryptomator) encryption scheme. ## Features - Access Cryptomator encrypted vaults from within your Java application -- Uses a ``java.nio.file.FileSystem`` so code written against the java.nio.file API can easily be adapted to work with encrypted data +- Uses a `java.nio.file.FileSystem` so code written against the `java.nio.file` API can easily be adapted to work with encrypted data - Open Source means: No backdoors, control is better than trust ### Security Architecture -For more information on the security details visit [cryptomator.org](https://cryptomator.org/architecture/). +For more information on the security details, visit [cryptomator.org](https://cryptomator.org/architecture/). ## Usage -CryptoFS depends on a Java 8 JRE/JDK. In addition the JCE unlimited strength policy files (needed for 256-bit keys) must be installed. +CryptoFS depends on Java 8 JRE/JDK. In addition, the JCE unlimited strength policy files (needed for 256-bit keys) must be installed. -### Vault initialization +### Vault Initialization ```java Path storageLocation = Paths.get("/home/cryptobot/vault"); @@ -33,9 +29,9 @@ Files.createDirectories(storageLocation); CryptoFileSystemProvider.initialize(storageLocation, "masterkey.cryptomator", "password"); ``` -### Obtaining a FileSystem instance +### Obtaining a FileSystem Instance -You have the option to use the convenience method ``CryptoFileSystemProvider#newFileSystem`` as follows: +You have the option to use the convenience method `CryptoFileSystemProvider#newFileSystem` as follows: ```java FileSystem fileSystem = CryptoFileSystemProvider.newFileSystem( @@ -46,7 +42,7 @@ FileSystem fileSystem = CryptoFileSystemProvider.newFileSystem( .build()); ``` -or to use one of the standard methods from ``FileSystems#newFileSystem``: +or to use one of the standard methods from `FileSystems#newFileSystem`: ```java URI uri = CryptoFileSystemUri.create(storageLocation); @@ -58,11 +54,11 @@ FileSystem fileSystem = FileSystems.newFileSystem( .build()); ``` -**Note** - Instead of CryptoFileSystemProperties you can always pass in a ``java.util.Map`` with entries set accordingly. +**Note:** Instead of `CryptoFileSystemProperties`, you can always pass in a `java.util.Map` with entries set accordingly. -For more details on construction have a look at the javadoc of ``CryptoFileSytemProvider``, ``CryptoFileSytemProperties`` and ``CryptoFileSytemUris``. +For more details on construction, have a look at the javadoc of `CryptoFileSytemProvider`, `CryptoFileSytemProperties`, and `CryptoFileSytemUris`. -### Using the constructed file system +### Using the Constructed FileSystem ```java try (FileSystem fileSystem = ...) { // see above @@ -84,7 +80,7 @@ try (FileSystem fileSystem = ...) { // see above } ``` -For more details on how to use the constructed file system you may consult the [javadocs of the java.nio.file package](http://docs.oracle.com/javase/8/docs/api/java/nio/file/package-summary.html). +For more details on how to use the constructed `FileSystem`, you may consult the [javadocs of the `java.nio.file` package](http://docs.oracle.com/javase/8/docs/api/java/nio/file/package-summary.html). ## Building @@ -101,7 +97,7 @@ mvn clean install ## Contributing to CryptoFS -Please read our [contribution guide](https://github.com/cryptomator/cryptomator/blob/master/CONTRIBUTING.md), if you would like to report a bug, ask a question or help us with coding. +Please read our [contribution guide](https://github.com/cryptomator/cryptomator/blob/master/CONTRIBUTING.md) if you would like to report a bug, ask a question, or help us with coding. ## Code of Conduct From efe55c68cf91f8c284f990c89e99e041c227a3cc Mon Sep 17 00:00:00 2001 From: Tobias Hagemann Date: Sun, 10 Sep 2017 23:25:21 +0200 Subject: [PATCH 2/8] Update README.md --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index 37075310..1f252552 100644 --- a/README.md +++ b/README.md @@ -105,4 +105,4 @@ Help us keep Cryptomator open and inclusive. Please read and follow our [Code of ## License -Distributed under the AGPLv3. See the `LICENSE.txt` file for more info. +This project is dual-licensed under the AGPLv3 for FOSS projects as well as a commercial license derived from the LGPL for independent software vendors and resellers. If you want to use this library in applications that are *not* licensed under the AGPL, feel free to contact our [sales team](https://cryptomator.org/enterprise/). From 17b2848d2f4cc867f0c065cd1305689ad9c9c30d Mon Sep 17 00:00:00 2001 From: Markus Kreusch Date: Thu, 9 Nov 2017 13:22:54 +0100 Subject: [PATCH 3/8] Fixed test --- .../cryptofs/CryptoFileSystemProviderIntegrationTest.java | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/test/java/org/cryptomator/cryptofs/CryptoFileSystemProviderIntegrationTest.java b/src/test/java/org/cryptomator/cryptofs/CryptoFileSystemProviderIntegrationTest.java index 32e3805c..4ff60b26 100644 --- a/src/test/java/org/cryptomator/cryptofs/CryptoFileSystemProviderIntegrationTest.java +++ b/src/test/java/org/cryptomator/cryptofs/CryptoFileSystemProviderIntegrationTest.java @@ -223,6 +223,9 @@ public void testDosFileAttributes() throws IOException { assertThat(Files.getAttribute(file, "dos:archive"), is(true)); assertThat(Files.getAttribute(file, "dos:readOnly"), is(true)); + // set readOnly to false again to allow deletion + Files.setAttribute(file, "dos:readOnly", false); + MoreFiles.deleteRecursively(tmpPath, RecursiveDeleteOption.ALLOW_INSECURE); } From 9c12313721e4ea473af129611106b3eb30c633a5 Mon Sep 17 00:00:00 2001 From: Markus Kreusch Date: Thu, 9 Nov 2017 15:05:37 +0100 Subject: [PATCH 4/8] Fixes #17 --- .../cryptofs/CiphertextDirectoryDeleter.java | 87 +++++++++++ .../cryptofs/CryptoDirectoryStream.java | 16 +- .../cryptofs/CryptoFileSystemImpl.java | 7 +- .../cryptomator/cryptofs/DeleteResult.java | 6 + .../cryptofs/DirectoryStreamFactory.java | 8 +- .../cryptofs/EncryptedNamePattern.java | 40 +++++ .../cryptofs/CryptoDirectoryStreamTest.java | 5 +- .../cryptofs/CryptoFileSystemImplTest.java | 9 +- ...ptyCiphertextDirectoryIntegrationTest.java | 144 ++++++++++++++++++ .../cryptofs/DirectoryStreamFactoryTest.java | 3 +- .../cryptofs/EncryptedNamePatternTest.java | 58 +++++++ 11 files changed, 364 insertions(+), 19 deletions(-) create mode 100644 src/main/java/org/cryptomator/cryptofs/CiphertextDirectoryDeleter.java create mode 100644 src/main/java/org/cryptomator/cryptofs/DeleteResult.java create mode 100644 src/main/java/org/cryptomator/cryptofs/EncryptedNamePattern.java create mode 100644 src/test/java/org/cryptomator/cryptofs/DeleteNonEmptyCiphertextDirectoryIntegrationTest.java create mode 100644 src/test/java/org/cryptomator/cryptofs/EncryptedNamePatternTest.java diff --git a/src/main/java/org/cryptomator/cryptofs/CiphertextDirectoryDeleter.java b/src/main/java/org/cryptomator/cryptofs/CiphertextDirectoryDeleter.java new file mode 100644 index 00000000..33f8d29b --- /dev/null +++ b/src/main/java/org/cryptomator/cryptofs/CiphertextDirectoryDeleter.java @@ -0,0 +1,87 @@ +package org.cryptomator.cryptofs; + +import static java.nio.file.FileVisitResult.CONTINUE; +import static org.cryptomator.cryptofs.DeleteResult.NO_FILES_EXISTED; +import static org.cryptomator.cryptofs.DeleteResult.SOME_FILES_EXISTED; + +import java.io.IOException; +import java.nio.file.DirectoryNotEmptyException; +import java.nio.file.FileVisitResult; +import java.nio.file.FileVisitor; +import java.nio.file.Files; +import java.nio.file.Path; +import java.nio.file.attribute.BasicFileAttributes; +import java.util.concurrent.atomic.AtomicInteger; + +import javax.inject.Inject; + +@PerProvider +class CiphertextDirectoryDeleter { + + private final EncryptedNamePattern encryptedNamePattern; + + @Inject + public CiphertextDirectoryDeleter(EncryptedNamePattern encryptedNamePattern) { + this.encryptedNamePattern = encryptedNamePattern; + } + + public void deleteCiphertextDirIncludingNonCiphertextFiles(Path ciphertextDir) throws IOException { + try { + Files.delete(ciphertextDir); + } catch (DirectoryNotEmptyException e) { + switch (deleteNonCiphertextFiles(ciphertextDir)) { + case NO_FILES_EXISTED: + throw e; + case SOME_FILES_EXISTED: + Files.delete(ciphertextDir); + } + } + } + + private DeleteResult deleteNonCiphertextFiles(Path ciphertextDir) throws IOException { + AtomicInteger counter = new AtomicInteger(0); + Files.walkFileTree(ciphertextDir, new FileVisitor() { + + private int level = 0; + + @Override + public FileVisitResult preVisitDirectory(Path dir, BasicFileAttributes attrs) throws IOException { + level++; + return CONTINUE; + } + + @Override + public FileVisitResult visitFile(Path file, BasicFileAttributes attrs) throws IOException { + if (level > 1 || !isEncryptedFile(file)) { + counter.incrementAndGet(); + Files.delete(file); + } + return CONTINUE; + } + + private boolean isEncryptedFile(Path file) { + return encryptedNamePattern.extractEncryptedName(file).isPresent(); + } + + @Override + public FileVisitResult visitFileFailed(Path file, IOException exc) throws IOException { + throw exc; + } + + @Override + public FileVisitResult postVisitDirectory(Path dir, IOException exc) throws IOException { + if (exc != null) { + throw exc; + } + level--; + if (level > 0) { + Files.delete(dir); + counter.incrementAndGet(); + } + return CONTINUE; + } + }); + return counter.get() == 0 ? NO_FILES_EXISTED : SOME_FILES_EXISTED; + } + +} diff --git a/src/main/java/org/cryptomator/cryptofs/CryptoDirectoryStream.java b/src/main/java/org/cryptomator/cryptofs/CryptoDirectoryStream.java index c98ec06a..1a0edd08 100644 --- a/src/main/java/org/cryptomator/cryptofs/CryptoDirectoryStream.java +++ b/src/main/java/org/cryptomator/cryptofs/CryptoDirectoryStream.java @@ -16,9 +16,8 @@ import java.nio.file.Path; import java.util.Iterator; import java.util.Objects; +import java.util.Optional; import java.util.function.Consumer; -import java.util.regex.Matcher; -import java.util.regex.Pattern; import java.util.stream.Stream; import java.util.stream.StreamSupport; @@ -30,7 +29,6 @@ class CryptoDirectoryStream implements DirectoryStream { - private static final Pattern BASE32_PATTERN = Pattern.compile("^0?(([A-Z2-7]{8})*[A-Z2-7=]{8})"); private static final Logger LOG = LoggerFactory.getLogger(CryptoDirectoryStream.class); private final String directoryId; @@ -43,11 +41,14 @@ class CryptoDirectoryStream implements DirectoryStream { private final DirectoryStream.Filter filter; private final Consumer onClose; private final FinallyUtil finallyUtil; + private final EncryptedNamePattern encryptedNamePattern; public CryptoDirectoryStream(Directory ciphertextDir, Path cleartextDir, FileNameCryptor filenameCryptor, CryptoPathMapper cryptoPathMapper, LongFileNameProvider longFileNameProvider, - ConflictResolver conflictResolver, DirectoryStream.Filter filter, Consumer onClose, FinallyUtil finallyUtil) throws IOException { + ConflictResolver conflictResolver, DirectoryStream.Filter filter, Consumer onClose, FinallyUtil finallyUtil, EncryptedNamePattern encryptedNamePattern) + throws IOException { this.onClose = onClose; this.finallyUtil = finallyUtil; + this.encryptedNamePattern = encryptedNamePattern; this.directoryId = ciphertextDir.dirId; this.ciphertextDirStream = Files.newDirectoryStream(ciphertextDir.path, p -> true); LOG.trace("OPEN {}", directoryId); @@ -122,10 +123,9 @@ private boolean isBrokenDirectoryFile(Path potentialDirectoryFile) { } private Path decrypt(Path ciphertextPath) { - String ciphertextFileName = ciphertextPath.getFileName().toString(); - Matcher m = BASE32_PATTERN.matcher(ciphertextFileName); - if (m.find()) { - String ciphertext = m.group(1); + Optional encryptedName = encryptedNamePattern.extractEncryptedNameFromStart(ciphertextPath); + if (encryptedName.isPresent()) { + String ciphertext = encryptedName.get(); try { String cleartext = filenameCryptor.decryptFilename(ciphertext, directoryId.getBytes(StandardCharsets.UTF_8)); return cleartextDir.resolve(cleartext); diff --git a/src/main/java/org/cryptomator/cryptofs/CryptoFileSystemImpl.java b/src/main/java/org/cryptomator/cryptofs/CryptoFileSystemImpl.java index 5905c7aa..9e94a38d 100644 --- a/src/main/java/org/cryptomator/cryptofs/CryptoFileSystemImpl.java +++ b/src/main/java/org/cryptomator/cryptofs/CryptoFileSystemImpl.java @@ -85,6 +85,7 @@ class CryptoFileSystemImpl extends CryptoFileSystem { private final CryptoPathFactory cryptoPathFactory; private final CryptoFileSystemStats stats; private final FinallyUtil finallyUtil; + private final CiphertextDirectoryDeleter ciphertextDirDeleter; private volatile boolean open = true; @@ -92,7 +93,8 @@ class CryptoFileSystemImpl extends CryptoFileSystem { public CryptoFileSystemImpl(@PathToVault Path pathToVault, CryptoFileSystemProperties properties, Cryptor cryptor, CryptoFileSystemProvider provider, CryptoFileSystems cryptoFileSystems, CryptoFileStore fileStore, OpenCryptoFiles openCryptoFiles, CryptoPathMapper cryptoPathMapper, DirectoryIdProvider dirIdProvider, CryptoFileAttributeProvider fileAttributeProvider, CryptoFileAttributeViewProvider fileAttributeViewProvider, PathMatcherFactory pathMatcherFactory, CryptoPathFactory cryptoPathFactory, CryptoFileSystemStats stats, - RootDirectoryInitializer rootDirectoryInitializer, CryptoFileAttributeByNameProvider fileAttributeByNameProvider, DirectoryStreamFactory directoryStreamFactory, FinallyUtil finallyUtil) { + RootDirectoryInitializer rootDirectoryInitializer, CryptoFileAttributeByNameProvider fileAttributeByNameProvider, DirectoryStreamFactory directoryStreamFactory, FinallyUtil finallyUtil, + CiphertextDirectoryDeleter ciphertextDirDeleter) { this.cryptor = cryptor; this.provider = provider; this.cryptoFileSystems = cryptoFileSystems; @@ -108,6 +110,7 @@ public CryptoFileSystemImpl(@PathToVault Path pathToVault, CryptoFileSystemPrope this.cryptoPathFactory = cryptoPathFactory; this.stats = stats; this.directoryStreamFactory = directoryStreamFactory; + this.ciphertextDirDeleter = ciphertextDirDeleter; this.rootPath = cryptoPathFactory.rootFor(this); this.emptyPath = cryptoPathFactory.emptyFor(this); this.finallyUtil = finallyUtil; @@ -348,7 +351,7 @@ void delete(CryptoPath cleartextPath) throws IOException { Path ciphertextDir = cryptoPathMapper.getCiphertextDirPath(cleartextPath); Path ciphertextDirFile = cryptoPathMapper.getCiphertextFilePath(cleartextPath, CiphertextFileType.DIRECTORY); try { - Files.delete(ciphertextDir); + ciphertextDirDeleter.deleteCiphertextDirIncludingNonCiphertextFiles(ciphertextDir); if (!Files.deleteIfExists(ciphertextDirFile)) { // should not happen. Nevertheless this is a valid state, so who no big deal... LOG.warn("Successfully deleted dir {}, but didn't find corresponding dir file {}", ciphertextDir, ciphertextDirFile); diff --git a/src/main/java/org/cryptomator/cryptofs/DeleteResult.java b/src/main/java/org/cryptomator/cryptofs/DeleteResult.java new file mode 100644 index 00000000..1e71ee0e --- /dev/null +++ b/src/main/java/org/cryptomator/cryptofs/DeleteResult.java @@ -0,0 +1,6 @@ +package org.cryptomator.cryptofs; + +enum DeleteResult { + NO_FILES_EXISTED, + SOME_FILES_EXISTED +} \ No newline at end of file diff --git a/src/main/java/org/cryptomator/cryptofs/DirectoryStreamFactory.java b/src/main/java/org/cryptomator/cryptofs/DirectoryStreamFactory.java index e3f88c1b..b44494f1 100644 --- a/src/main/java/org/cryptomator/cryptofs/DirectoryStreamFactory.java +++ b/src/main/java/org/cryptomator/cryptofs/DirectoryStreamFactory.java @@ -21,18 +21,21 @@ class DirectoryStreamFactory { private final ConflictResolver conflictResolver; private final CryptoPathMapper cryptoPathMapper; private final FinallyUtil finallyUtil; + private final EncryptedNamePattern encryptedNamePattern; private final ConcurrentMap streams = new ConcurrentHashMap<>(); private volatile boolean closed = false; @Inject - public DirectoryStreamFactory(Cryptor cryptor, LongFileNameProvider longFileNameProvider, ConflictResolver conflictResolver, CryptoPathMapper cryptoPathMapper, FinallyUtil finallyUtil) { + public DirectoryStreamFactory(Cryptor cryptor, LongFileNameProvider longFileNameProvider, ConflictResolver conflictResolver, CryptoPathMapper cryptoPathMapper, FinallyUtil finallyUtil, + EncryptedNamePattern encryptedNamePattern) { this.cryptor = cryptor; this.longFileNameProvider = longFileNameProvider; this.conflictResolver = conflictResolver; this.cryptoPathMapper = cryptoPathMapper; this.finallyUtil = finallyUtil; + this.encryptedNamePattern = encryptedNamePattern; } public DirectoryStream newDirectoryStream(CryptoPath cleartextDir, Filter filter) throws IOException { @@ -46,7 +49,8 @@ public DirectoryStream newDirectoryStream(CryptoPath cleartextDir, Filter< conflictResolver, // filter, // closed -> streams.remove(closed), // - finallyUtil); + finallyUtil, // + encryptedNamePattern); streams.put(stream, stream); if (closed) { stream.close(); diff --git a/src/main/java/org/cryptomator/cryptofs/EncryptedNamePattern.java b/src/main/java/org/cryptomator/cryptofs/EncryptedNamePattern.java new file mode 100644 index 00000000..7d1a9334 --- /dev/null +++ b/src/main/java/org/cryptomator/cryptofs/EncryptedNamePattern.java @@ -0,0 +1,40 @@ +package org.cryptomator.cryptofs; + +import java.nio.file.Path; +import java.util.Optional; +import java.util.regex.Matcher; +import java.util.regex.Pattern; + +import javax.inject.Inject; + +@PerProvider +class EncryptedNamePattern { + + private static final Pattern BASE32_PATTERN = Pattern.compile("0?(([A-Z2-7]{8})*[A-Z2-7=]{8})"); + private static final Pattern BASE32_PATTERN_AT_START_OF_NAME = Pattern.compile("^0?(([A-Z2-7]{8})*[A-Z2-7=]{8})"); + + @Inject + public EncryptedNamePattern() { + } + + public Optional extractEncryptedName(Path ciphertextFile) { + String name = ciphertextFile.getFileName().toString(); + Matcher matcher = BASE32_PATTERN.matcher(name); + if (matcher.find(0)) { + return Optional.of(matcher.group(1)); + } else { + return Optional.empty(); + } + } + + public Optional extractEncryptedNameFromStart(Path ciphertextFile) { + String name = ciphertextFile.getFileName().toString(); + Matcher matcher = BASE32_PATTERN_AT_START_OF_NAME.matcher(name); + if (matcher.find(0)) { + return Optional.of(matcher.group(1)); + } else { + return Optional.empty(); + } + } + +} diff --git a/src/test/java/org/cryptomator/cryptofs/CryptoDirectoryStreamTest.java b/src/test/java/org/cryptomator/cryptofs/CryptoDirectoryStreamTest.java index 809a5606..d1ffa13d 100644 --- a/src/test/java/org/cryptomator/cryptofs/CryptoDirectoryStreamTest.java +++ b/src/test/java/org/cryptomator/cryptofs/CryptoDirectoryStreamTest.java @@ -61,6 +61,7 @@ public static void setupClass() { private LongFileNameProvider longFileNameProvider; private ConflictResolver conflictResolver; private FinallyUtil finallyUtil; + private EncryptedNamePattern encryptedNamePattern = new EncryptedNamePattern(); @Before @SuppressWarnings("unchecked") @@ -126,7 +127,7 @@ public void testDirListing() throws IOException { Mockito.when(dirStream.spliterator()).thenReturn(ciphertextFileNames.stream().map(cleartextPath::resolve).spliterator()); try (CryptoDirectoryStream stream = new CryptoDirectoryStream(new Directory("foo", ciphertextDirPath), cleartextPath, filenameCryptor, cryptoPathMapper, longFileNameProvider, conflictResolver, ACCEPT_ALL, - DO_NOTHING_ON_CLOSE, finallyUtil)) { + DO_NOTHING_ON_CLOSE, finallyUtil, encryptedNamePattern)) { Iterator iter = stream.iterator(); Assert.assertTrue(iter.hasNext()); Assert.assertEquals(cleartextPath.resolve("one"), iter.next()); @@ -149,7 +150,7 @@ public void testDirListingForEmptyDir() throws IOException { Mockito.when(dirStream.spliterator()).thenReturn(Spliterators.emptySpliterator()); try (CryptoDirectoryStream stream = new CryptoDirectoryStream(new Directory("foo", ciphertextDirPath), cleartextPath, filenameCryptor, cryptoPathMapper, longFileNameProvider, conflictResolver, ACCEPT_ALL, - DO_NOTHING_ON_CLOSE, finallyUtil)) { + DO_NOTHING_ON_CLOSE, finallyUtil, encryptedNamePattern)) { Iterator iter = stream.iterator(); Assert.assertFalse(iter.hasNext()); iter.next(); diff --git a/src/test/java/org/cryptomator/cryptofs/CryptoFileSystemImplTest.java b/src/test/java/org/cryptomator/cryptofs/CryptoFileSystemImplTest.java index 5ca1173c..e1ab51a8 100644 --- a/src/test/java/org/cryptomator/cryptofs/CryptoFileSystemImplTest.java +++ b/src/test/java/org/cryptomator/cryptofs/CryptoFileSystemImplTest.java @@ -99,6 +99,7 @@ public class CryptoFileSystemImplTest { private final RootDirectoryInitializer rootDirectoryInitializer = mock(RootDirectoryInitializer.class); private final DirectoryStreamFactory directoryStreamFactory = mock(DirectoryStreamFactory.class); private final FinallyUtil finallyUtil = mock(FinallyUtil.class); + private final CiphertextDirectoryDeleter ciphertextDirDeleter = mock(CiphertextDirectoryDeleter.class); private final CryptoPath root = mock(CryptoPath.class); private final CryptoPath empty = mock(CryptoPath.class); @@ -111,7 +112,7 @@ public void setup() { when(cryptoPathFactory.emptyFor(any())).thenReturn(empty); inTest = new CryptoFileSystemImpl(pathToVault, properties, cryptor, provider, cryptoFileSystems, fileStore, openCryptoFiles, cryptoPathMapper, dirIdProvider, fileAttributeProvider, fileAttributeViewProvider, - pathMatcherFactory, cryptoPathFactory, stats, rootDirectoryInitializer, fileAttributeByNameProvider, directoryStreamFactory, finallyUtil); + pathMatcherFactory, cryptoPathFactory, stats, rootDirectoryInitializer, fileAttributeByNameProvider, directoryStreamFactory, finallyUtil, ciphertextDirDeleter); } @Test @@ -360,7 +361,7 @@ public void testDeleteExistingDirectory() throws IOException { when(physicalFsProv.deleteIfExists(ciphertextFilePath)).thenReturn(false); inTest.delete(cleartextPath); - verify(physicalFsProv).delete(ciphertextDirPath); + verify(ciphertextDirDeleter).deleteCiphertextDirIncludingNonCiphertextFiles(ciphertextDirPath); verify(physicalFsProv).deleteIfExists(ciphertextDirFilePath); verify(dirIdProvider).delete(ciphertextDirFilePath); } @@ -368,7 +369,7 @@ public void testDeleteExistingDirectory() throws IOException { @Test public void testDeleteNonExistingFileOrDir() throws IOException { when(physicalFsProv.deleteIfExists(ciphertextFilePath)).thenReturn(false); - Mockito.doThrow(new NoSuchFileException("cleartext")).when(physicalFsProv).delete(ciphertextDirPath); + Mockito.doThrow(new NoSuchFileException("cleartext")).when(ciphertextDirDeleter).deleteCiphertextDirIncludingNonCiphertextFiles(ciphertextDirPath); thrown.expect(NoSuchFileException.class); inTest.delete(cleartextPath); @@ -377,7 +378,7 @@ public void testDeleteNonExistingFileOrDir() throws IOException { @Test public void testDeleteNonEmptyDir() throws IOException { when(physicalFsProv.deleteIfExists(ciphertextFilePath)).thenReturn(false); - Mockito.doThrow(new DirectoryNotEmptyException("ciphertextDir")).when(physicalFsProv).delete(ciphertextDirPath); + Mockito.doThrow(new DirectoryNotEmptyException("ciphertextDir")).when(ciphertextDirDeleter).deleteCiphertextDirIncludingNonCiphertextFiles(ciphertextDirPath); thrown.expect(DirectoryNotEmptyException.class); inTest.delete(cleartextPath); diff --git a/src/test/java/org/cryptomator/cryptofs/DeleteNonEmptyCiphertextDirectoryIntegrationTest.java b/src/test/java/org/cryptomator/cryptofs/DeleteNonEmptyCiphertextDirectoryIntegrationTest.java new file mode 100644 index 00000000..c651bdf2 --- /dev/null +++ b/src/test/java/org/cryptomator/cryptofs/DeleteNonEmptyCiphertextDirectoryIntegrationTest.java @@ -0,0 +1,144 @@ +/******************************************************************************* + * Copyright (c) 2016 Sebastian Stenzel and others. + * All rights reserved. This program and the accompanying materials + * are made available under the terms of the accompanying LICENSE.txt. + * + * Contributors: + * Sebastian Stenzel - initial API and implementation + *******************************************************************************/ +package org.cryptomator.cryptofs; + +import static java.nio.file.Files.walkFileTree; +import static java.nio.file.StandardOpenOption.CREATE_NEW; +import static org.cryptomator.cryptofs.CryptoFileSystemProperties.cryptoFileSystemProperties; +import static org.cryptomator.cryptofs.CryptoFileSystemUri.create; + +import java.io.File; +import java.io.IOException; +import java.io.UncheckedIOException; +import java.nio.file.DirectoryNotEmptyException; +import java.nio.file.FileSystem; +import java.nio.file.Files; +import java.nio.file.Path; +import java.util.stream.Stream; + +import org.junit.AfterClass; +import org.junit.BeforeClass; +import org.junit.Rule; +import org.junit.Test; +import org.junit.rules.ExpectedException; + +/** + * Regression tests https://github.com/cryptomator/cryptofs/issues/17. + */ +public class DeleteNonEmptyCiphertextDirectoryIntegrationTest { + + @Rule + public ExpectedException thrown = ExpectedException.none(); + + private static Path tempDir; + private static Path pathToVault; + private static FileSystem fileSystem; + + @BeforeClass + public static void setupClass() throws IOException { + tempDir = Files.createTempDirectory("DNECDIT"); + pathToVault = tempDir.resolve("vault"); + Files.createDirectory(pathToVault); + fileSystem = new CryptoFileSystemProvider().newFileSystem(create(pathToVault), cryptoFileSystemProperties().withPassphrase("asd").build()); + } + + @AfterClass + public static void teardownClass() throws IOException { + walkFileTree(tempDir, new DeletingFileVisitor()); + } + + @Test + public void testDeleteCiphertextDirectoryContainingNonCryptoFile() throws IOException { + Path cleartextDirectory = fileSystem.getPath("/a"); + Files.createDirectory(cleartextDirectory); + + Path ciphertextDirectory = firstEmptyCiphertextDirectory(); + createFile(ciphertextDirectory, "foo01234.txt", new byte[] {65}); + + Files.delete(cleartextDirectory); + } + + @Test + public void testDeleteCiphertextDirectoryContainingDirectories() throws IOException { + Path cleartextDirectory = fileSystem.getPath("/a"); + Files.createDirectory(cleartextDirectory); + + Path ciphertextDirectory = firstEmptyCiphertextDirectory(); + // ciphertextDir + // .. foo0123 + // .... foobar + // ...... test.baz + // .... text.txt + // .... text.data + Path foo0123 = createFolder(ciphertextDirectory, "foo0123"); + Path foobar = createFolder(foo0123, "foobar"); + createFile(foo0123, "test.txt", new byte[] {65}); + createFile(foo0123, "text.data", new byte[] {65}); + createFile(foobar, "test.baz", new byte[] {65}); + + Files.delete(cleartextDirectory); + } + + @Test + public void testDeleteNonEmptyDir() throws IOException { + Path cleartextDirectory = fileSystem.getPath("/a"); + Files.createDirectory(cleartextDirectory); + createFile(cleartextDirectory, "test", new byte[] {65}); + + thrown.expect(DirectoryNotEmptyException.class); + + Files.delete(cleartextDirectory); + } + + @Test + public void testDeleteEmptyDir() throws IOException { + Path cleartextDirectory = fileSystem.getPath("/a"); + Files.createDirectory(cleartextDirectory); + + Files.delete(cleartextDirectory); + } + + private Path firstEmptyCiphertextDirectory() throws IOException { + try (Stream allFilesInVaultDir = Files.walk(pathToVault)) { + return allFilesInVaultDir // + .filter(Files::isDirectory) // + .filter(this::isEmptyDirectory) // + .filter(this::isEncryptedDirectory) // + .findFirst() // + .get(); + } + } + + private boolean isEmptyDirectory(Path path) { + try (Stream files = Files.list(path)) { + return files.count() == 0; + } catch (IOException e) { + throw new UncheckedIOException(e); + } + } + + private boolean isEncryptedDirectory(Path pathInVault) { + Path relativePath = pathToVault.relativize(pathInVault); + String relativePathAsString = relativePath.toString().replace(File.separatorChar, '/'); + return relativePathAsString.matches("d/[2-7A-Z]{2}/[2-7A-Z]{30}"); + } + + private Path createFolder(Path parent, String name) throws IOException { + Path result = parent.resolve(name); + Files.createDirectory(result); + return result; + } + + private Path createFile(Path parent, String name, byte[] data) throws IOException { + Path result = parent.resolve(name); + Files.write(result, data, CREATE_NEW); + return result; + } + +} diff --git a/src/test/java/org/cryptomator/cryptofs/DirectoryStreamFactoryTest.java b/src/test/java/org/cryptomator/cryptofs/DirectoryStreamFactoryTest.java index 6c93b3c2..cb72352b 100644 --- a/src/test/java/org/cryptomator/cryptofs/DirectoryStreamFactoryTest.java +++ b/src/test/java/org/cryptomator/cryptofs/DirectoryStreamFactoryTest.java @@ -41,8 +41,9 @@ public class DirectoryStreamFactoryTest { private final LongFileNameProvider longFileNameProvider = mock(LongFileNameProvider.class); private final ConflictResolver conflictResolver = mock(ConflictResolver.class); private final CryptoPathMapper cryptoPathMapper = mock(CryptoPathMapper.class); + private final EncryptedNamePattern encryptedNamePattern = new EncryptedNamePattern(); - private final DirectoryStreamFactory inTest = new DirectoryStreamFactory(cryptor, longFileNameProvider, conflictResolver, cryptoPathMapper, finallyUtil); + private final DirectoryStreamFactory inTest = new DirectoryStreamFactory(cryptor, longFileNameProvider, conflictResolver, cryptoPathMapper, finallyUtil, encryptedNamePattern); @SuppressWarnings("unchecked") diff --git a/src/test/java/org/cryptomator/cryptofs/EncryptedNamePatternTest.java b/src/test/java/org/cryptomator/cryptofs/EncryptedNamePatternTest.java new file mode 100644 index 00000000..d92f2285 --- /dev/null +++ b/src/test/java/org/cryptomator/cryptofs/EncryptedNamePatternTest.java @@ -0,0 +1,58 @@ +package org.cryptomator.cryptofs; + +import static org.hamcrest.Matchers.is; +import static org.junit.Assert.assertThat; + +import java.nio.file.Path; +import java.nio.file.Paths; +import java.util.Optional; + +import org.junit.Test; + +public class EncryptedNamePatternTest { + + private static final String ENCRYPTED_NAME = "ALKDUEEH2445375AUZEJFEFA"; + private static final Path PATH_WITHOUT_ENCRYPTED_NAME = Paths.get("foo.txt"); + private static final Path PATH_WITH_ENCRYPTED_NAME_AND_PREFIX_AND_SUFFIX = Paths.get("foo" + ENCRYPTED_NAME + ".txt"); + private static final Path PATH_WITH_ENCRYPTED_NAME_AND_SUFFIX = Paths.get(ENCRYPTED_NAME + ".txt"); + + private EncryptedNamePattern inTest = new EncryptedNamePattern(); + + @Test + public void testExtractEncryptedNameReturnsEmptyOptionalIfNoEncryptedNameIsPresent() { + Optional result = inTest.extractEncryptedName(PATH_WITHOUT_ENCRYPTED_NAME); + + assertThat(result.isPresent(), is(false)); + } + + @Test + public void testExtractEncryptedNameReturnsEncryptedNameIfItIsIsPresent() { + Optional result = inTest.extractEncryptedName(PATH_WITH_ENCRYPTED_NAME_AND_PREFIX_AND_SUFFIX); + + assertThat(result.isPresent(), is(true)); + assertThat(result.get(), is(ENCRYPTED_NAME)); + } + + @Test + public void testExtractEncryptedNameFromStartReturnsEmptyOptionalIfNoEncryptedNameIsPresent() { + Optional result = inTest.extractEncryptedNameFromStart(PATH_WITHOUT_ENCRYPTED_NAME); + + assertThat(result.isPresent(), is(false)); + } + + @Test + public void testExtractEncryptedNameFromStartReturnsEncryptedNameIfItIsPresent() { + Optional result = inTest.extractEncryptedName(PATH_WITH_ENCRYPTED_NAME_AND_SUFFIX); + + assertThat(result.isPresent(), is(true)); + assertThat(result.get(), is(ENCRYPTED_NAME)); + } + + @Test + public void testExtractEncryptedNameFromStartReturnsEmptyOptionalIfEncryptedNameIsPresentAfterStart() { + Optional result = inTest.extractEncryptedNameFromStart(PATH_WITH_ENCRYPTED_NAME_AND_PREFIX_AND_SUFFIX); + + assertThat(result.isPresent(), is(false)); + } + +} From a936f24e11037f2de3af9b0a730e9b21f3da63e8 Mon Sep 17 00:00:00 2001 From: Markus Kreusch Date: Fri, 10 Nov 2017 13:13:17 +0100 Subject: [PATCH 5/8] Fixes #16 --- .../cryptofs/CiphertextDirectoryDeleter.java | 127 +++++++++++------- .../cryptofs/CryptoDirectoryStream.java | 48 +++++-- .../cryptofs/CryptoFileSystemImpl.java | 2 +- .../cryptomator/cryptofs/DeleteResult.java | 6 - .../cryptofs/DirectoryStreamFactory.java | 3 +- .../cryptofs/CryptoFileSystemImplTest.java | 6 +- 6 files changed, 124 insertions(+), 68 deletions(-) delete mode 100644 src/main/java/org/cryptomator/cryptofs/DeleteResult.java diff --git a/src/main/java/org/cryptomator/cryptofs/CiphertextDirectoryDeleter.java b/src/main/java/org/cryptomator/cryptofs/CiphertextDirectoryDeleter.java index 33f8d29b..7022641a 100644 --- a/src/main/java/org/cryptomator/cryptofs/CiphertextDirectoryDeleter.java +++ b/src/main/java/org/cryptomator/cryptofs/CiphertextDirectoryDeleter.java @@ -1,8 +1,9 @@ package org.cryptomator.cryptofs; import static java.nio.file.FileVisitResult.CONTINUE; -import static org.cryptomator.cryptofs.DeleteResult.NO_FILES_EXISTED; -import static org.cryptomator.cryptofs.DeleteResult.SOME_FILES_EXISTED; +import static java.util.stream.Collectors.toSet; +import static org.cryptomator.cryptofs.CiphertextDirectoryDeleter.DeleteResult.NO_FILES_DELETED; +import static org.cryptomator.cryptofs.CiphertextDirectoryDeleter.DeleteResult.SOME_FILES_DELETED; import java.io.IOException; import java.nio.file.DirectoryNotEmptyException; @@ -11,77 +12,107 @@ import java.nio.file.Files; import java.nio.file.Path; import java.nio.file.attribute.BasicFileAttributes; -import java.util.concurrent.atomic.AtomicInteger; +import java.util.Set; import javax.inject.Inject; -@PerProvider +@PerFileSystem class CiphertextDirectoryDeleter { - private final EncryptedNamePattern encryptedNamePattern; + private final DirectoryStreamFactory directoryStreamFactory; @Inject - public CiphertextDirectoryDeleter(EncryptedNamePattern encryptedNamePattern) { - this.encryptedNamePattern = encryptedNamePattern; + public CiphertextDirectoryDeleter(DirectoryStreamFactory directoryStreamFactory) { + this.directoryStreamFactory = directoryStreamFactory; } - public void deleteCiphertextDirIncludingNonCiphertextFiles(Path ciphertextDir) throws IOException { + public void deleteCiphertextDirIncludingNonCiphertextFiles(Path ciphertextDir, CryptoPath cleartextDir) throws IOException { try { - Files.delete(ciphertextDir); + Files.deleteIfExists(ciphertextDir); } catch (DirectoryNotEmptyException e) { - switch (deleteNonCiphertextFiles(ciphertextDir)) { - case NO_FILES_EXISTED: + switch (deleteNonCiphertextFiles(ciphertextDir, cleartextDir)) { + case NO_FILES_DELETED: throw e; - case SOME_FILES_EXISTED: + case SOME_FILES_DELETED: Files.delete(ciphertextDir); + break; + default: + throw new IllegalStateException("Unexpected enum constant"); } } } - private DeleteResult deleteNonCiphertextFiles(Path ciphertextDir) throws IOException { - AtomicInteger counter = new AtomicInteger(0); - Files.walkFileTree(ciphertextDir, new FileVisitor() { + private DeleteResult deleteNonCiphertextFiles(Path ciphertextDir, CryptoPath cleartextDir) throws IOException { + NonCiphertextFilesDeletingFileVisitor visitor; + try (CryptoDirectoryStream directoryStream = directoryStreamFactory.newDirectoryStream(cleartextDir, ignored -> true)) { + Set ciphertextFiles = directoryStream.ciphertextDirectoryListing().collect(toSet()); + visitor = new NonCiphertextFilesDeletingFileVisitor(ciphertextFiles); + } + Files.walkFileTree(ciphertextDir, visitor); + return visitor.getNumDeleted() == 0 // + ? NO_FILES_DELETED // + : SOME_FILES_DELETED; + } + + static enum DeleteResult { + NO_FILES_DELETED, SOME_FILES_DELETED + } - private int level = 0; + private static class NonCiphertextFilesDeletingFileVisitor implements FileVisitor { - @Override - public FileVisitResult preVisitDirectory(Path dir, BasicFileAttributes attrs) throws IOException { - level++; - return CONTINUE; - } + private final Set ciphertextFiles; - @Override - public FileVisitResult visitFile(Path file, BasicFileAttributes attrs) throws IOException { - if (level > 1 || !isEncryptedFile(file)) { - counter.incrementAndGet(); - Files.delete(file); - } - return CONTINUE; - } + private int numDeleted = 0; + private int level = 0; + + public NonCiphertextFilesDeletingFileVisitor(Set ciphertextFiles) { + this.ciphertextFiles = ciphertextFiles; + } + + public int getNumDeleted() { + return numDeleted; + } + + @Override + public FileVisitResult preVisitDirectory(Path dir, BasicFileAttributes attrs) throws IOException { + level++; + return CONTINUE; + } - private boolean isEncryptedFile(Path file) { - return encryptedNamePattern.extractEncryptedName(file).isPresent(); + @Override + public FileVisitResult visitFile(Path file, BasicFileAttributes attrs) throws IOException { + if (!isOnRootLevel() || !isCiphertextFile(file)) { + Files.delete(file); + numDeleted++; } + return CONTINUE; + } + + private boolean isOnRootLevel() { + return level == 1; + } + + private boolean isCiphertextFile(Path file) throws IOException { + return ciphertextFiles.contains(file); + } - @Override - public FileVisitResult visitFileFailed(Path file, IOException exc) throws IOException { + @Override + public FileVisitResult visitFileFailed(Path file, IOException exc) throws IOException { + throw exc; + } + + @Override + public FileVisitResult postVisitDirectory(Path dir, IOException exc) throws IOException { + if (exc != null) { throw exc; } - - @Override - public FileVisitResult postVisitDirectory(Path dir, IOException exc) throws IOException { - if (exc != null) { - throw exc; - } - level--; - if (level > 0) { - Files.delete(dir); - counter.incrementAndGet(); - } - return CONTINUE; + level--; + if (level > 0) { + Files.delete(dir); + numDeleted++; } - }); - return counter.get() == 0 ? NO_FILES_EXISTED : SOME_FILES_EXISTED; - } + return CONTINUE; + } + }; } diff --git a/src/main/java/org/cryptomator/cryptofs/CryptoDirectoryStream.java b/src/main/java/org/cryptomator/cryptofs/CryptoDirectoryStream.java index 1a0edd08..f0f17128 100644 --- a/src/main/java/org/cryptomator/cryptofs/CryptoDirectoryStream.java +++ b/src/main/java/org/cryptomator/cryptofs/CryptoDirectoryStream.java @@ -62,13 +62,25 @@ public CryptoDirectoryStream(Directory ciphertextDir, Path cleartextDir, FileNam @Override public Iterator iterator() { + return cleartextDirectoryListing().iterator(); + } + + private Stream cleartextDirectoryListing() { + return directoryListing() // + .map(CiphertextAndCleartextPath::getCleartextPath) // + .filter(this::isAcceptableByFilter); + } + + public Stream ciphertextDirectoryListing() { + return directoryListing().map(CiphertextAndCleartextPath::getCiphertextPath); + } + + public Stream directoryListing() { Stream pathIter = StreamSupport.stream(ciphertextDirStream.spliterator(), false); Stream resolved = pathIter.map(this::resolveConflictingFileIfNeeded).filter(Objects::nonNull); Stream sanitized = resolved.filter(this::passesPlausibilityChecks); Stream inflated = sanitized.map(this::inflateIfNeeded).filter(Objects::nonNull); - Stream decrypted = inflated.map(this::decrypt).filter(Objects::nonNull); - Stream filtered = decrypted.filter(this::isAcceptableByFilter); - return filtered.iterator(); + return inflated.map(this::decrypt).filter(Objects::nonNull); } private Path resolveConflictingFileIfNeeded(Path potentiallyConflictingPath) { @@ -122,13 +134,13 @@ private boolean isBrokenDirectoryFile(Path potentialDirectoryFile) { return false; } - private Path decrypt(Path ciphertextPath) { - Optional encryptedName = encryptedNamePattern.extractEncryptedNameFromStart(ciphertextPath); - if (encryptedName.isPresent()) { - String ciphertext = encryptedName.get(); + private CiphertextAndCleartextPath decrypt(Path ciphertextPath) { + Optional ciphertextName = encryptedNamePattern.extractEncryptedNameFromStart(ciphertextPath); + if (ciphertextName.isPresent()) { + String ciphertext = ciphertextName.get(); try { String cleartext = filenameCryptor.decryptFilename(ciphertext, directoryId.getBytes(StandardCharsets.UTF_8)); - return cleartextDir.resolve(cleartext); + return new CiphertextAndCleartextPath(ciphertextPath, cleartextDir.resolve(cleartext)); } catch (AuthenticationFailedException e) { LOG.warn(ciphertextPath + " not decryptable due to an unauthentic ciphertext."); return null; @@ -155,4 +167,24 @@ public void close() throws IOException { () -> LOG.trace("CLOSE {}", directoryId)); } + private static class CiphertextAndCleartextPath { + + private final Path ciphertextPath; + private final Path cleartextPath; + + public CiphertextAndCleartextPath(Path ciphertextPath, Path cleartextPath) { + this.ciphertextPath = ciphertextPath; + this.cleartextPath = cleartextPath; + } + + public Path getCiphertextPath() { + return ciphertextPath; + } + + public Path getCleartextPath() { + return cleartextPath; + } + + } + } diff --git a/src/main/java/org/cryptomator/cryptofs/CryptoFileSystemImpl.java b/src/main/java/org/cryptomator/cryptofs/CryptoFileSystemImpl.java index 9e94a38d..3c844601 100644 --- a/src/main/java/org/cryptomator/cryptofs/CryptoFileSystemImpl.java +++ b/src/main/java/org/cryptomator/cryptofs/CryptoFileSystemImpl.java @@ -351,7 +351,7 @@ void delete(CryptoPath cleartextPath) throws IOException { Path ciphertextDir = cryptoPathMapper.getCiphertextDirPath(cleartextPath); Path ciphertextDirFile = cryptoPathMapper.getCiphertextFilePath(cleartextPath, CiphertextFileType.DIRECTORY); try { - ciphertextDirDeleter.deleteCiphertextDirIncludingNonCiphertextFiles(ciphertextDir); + ciphertextDirDeleter.deleteCiphertextDirIncludingNonCiphertextFiles(ciphertextDir, cleartextPath); if (!Files.deleteIfExists(ciphertextDirFile)) { // should not happen. Nevertheless this is a valid state, so who no big deal... LOG.warn("Successfully deleted dir {}, but didn't find corresponding dir file {}", ciphertextDir, ciphertextDirFile); diff --git a/src/main/java/org/cryptomator/cryptofs/DeleteResult.java b/src/main/java/org/cryptomator/cryptofs/DeleteResult.java deleted file mode 100644 index 1e71ee0e..00000000 --- a/src/main/java/org/cryptomator/cryptofs/DeleteResult.java +++ /dev/null @@ -1,6 +0,0 @@ -package org.cryptomator.cryptofs; - -enum DeleteResult { - NO_FILES_EXISTED, - SOME_FILES_EXISTED -} \ No newline at end of file diff --git a/src/main/java/org/cryptomator/cryptofs/DirectoryStreamFactory.java b/src/main/java/org/cryptomator/cryptofs/DirectoryStreamFactory.java index b44494f1..254f0033 100644 --- a/src/main/java/org/cryptomator/cryptofs/DirectoryStreamFactory.java +++ b/src/main/java/org/cryptomator/cryptofs/DirectoryStreamFactory.java @@ -2,7 +2,6 @@ import java.io.IOException; import java.nio.file.ClosedFileSystemException; -import java.nio.file.DirectoryStream; import java.nio.file.DirectoryStream.Filter; import java.nio.file.Path; import java.util.concurrent.ConcurrentHashMap; @@ -38,7 +37,7 @@ public DirectoryStreamFactory(Cryptor cryptor, LongFileNameProvider longFileName this.encryptedNamePattern = encryptedNamePattern; } - public DirectoryStream newDirectoryStream(CryptoPath cleartextDir, Filter filter) throws IOException { + public CryptoDirectoryStream newDirectoryStream(CryptoPath cleartextDir, Filter filter) throws IOException { Directory ciphertextDir = cryptoPathMapper.getCiphertextDir(cleartextDir); CryptoDirectoryStream stream = new CryptoDirectoryStream( // ciphertextDir, // diff --git a/src/test/java/org/cryptomator/cryptofs/CryptoFileSystemImplTest.java b/src/test/java/org/cryptomator/cryptofs/CryptoFileSystemImplTest.java index e1ab51a8..df7fd75d 100644 --- a/src/test/java/org/cryptomator/cryptofs/CryptoFileSystemImplTest.java +++ b/src/test/java/org/cryptomator/cryptofs/CryptoFileSystemImplTest.java @@ -361,7 +361,7 @@ public void testDeleteExistingDirectory() throws IOException { when(physicalFsProv.deleteIfExists(ciphertextFilePath)).thenReturn(false); inTest.delete(cleartextPath); - verify(ciphertextDirDeleter).deleteCiphertextDirIncludingNonCiphertextFiles(ciphertextDirPath); + verify(ciphertextDirDeleter).deleteCiphertextDirIncludingNonCiphertextFiles(ciphertextDirPath, cleartextPath); verify(physicalFsProv).deleteIfExists(ciphertextDirFilePath); verify(dirIdProvider).delete(ciphertextDirFilePath); } @@ -369,7 +369,7 @@ public void testDeleteExistingDirectory() throws IOException { @Test public void testDeleteNonExistingFileOrDir() throws IOException { when(physicalFsProv.deleteIfExists(ciphertextFilePath)).thenReturn(false); - Mockito.doThrow(new NoSuchFileException("cleartext")).when(ciphertextDirDeleter).deleteCiphertextDirIncludingNonCiphertextFiles(ciphertextDirPath); + Mockito.doThrow(new NoSuchFileException("cleartext")).when(ciphertextDirDeleter).deleteCiphertextDirIncludingNonCiphertextFiles(ciphertextDirPath, cleartextPath); thrown.expect(NoSuchFileException.class); inTest.delete(cleartextPath); @@ -378,7 +378,7 @@ public void testDeleteNonExistingFileOrDir() throws IOException { @Test public void testDeleteNonEmptyDir() throws IOException { when(physicalFsProv.deleteIfExists(ciphertextFilePath)).thenReturn(false); - Mockito.doThrow(new DirectoryNotEmptyException("ciphertextDir")).when(ciphertextDirDeleter).deleteCiphertextDirIncludingNonCiphertextFiles(ciphertextDirPath); + Mockito.doThrow(new DirectoryNotEmptyException("ciphertextDir")).when(ciphertextDirDeleter).deleteCiphertextDirIncludingNonCiphertextFiles(ciphertextDirPath, cleartextPath); thrown.expect(DirectoryNotEmptyException.class); inTest.delete(cleartextPath); From 6be4a51aa47bdc82cf61a7a546d0655824278054 Mon Sep 17 00:00:00 2001 From: Markus Kreusch Date: Fri, 10 Nov 2017 14:24:36 +0100 Subject: [PATCH 6/8] Improved handling of broken directory files and longnames --- .../cryptofs/CryptoDirectoryStream.java | 100 +++++++++++------- ...ptyCiphertextDirectoryIntegrationTest.java | 54 +++++++++- 2 files changed, 113 insertions(+), 41 deletions(-) diff --git a/src/main/java/org/cryptomator/cryptofs/CryptoDirectoryStream.java b/src/main/java/org/cryptomator/cryptofs/CryptoDirectoryStream.java index f0f17128..38b3651d 100644 --- a/src/main/java/org/cryptomator/cryptofs/CryptoDirectoryStream.java +++ b/src/main/java/org/cryptomator/cryptofs/CryptoDirectoryStream.java @@ -67,43 +67,60 @@ public Iterator iterator() { private Stream cleartextDirectoryListing() { return directoryListing() // - .map(CiphertextAndCleartextPath::getCleartextPath) // + .map(ProcessedPaths::getCleartextPath) // .filter(this::isAcceptableByFilter); } public Stream ciphertextDirectoryListing() { - return directoryListing().map(CiphertextAndCleartextPath::getCiphertextPath); + return directoryListing().map(ProcessedPaths::getCiphertextPath); } - public Stream directoryListing() { - Stream pathIter = StreamSupport.stream(ciphertextDirStream.spliterator(), false); - Stream resolved = pathIter.map(this::resolveConflictingFileIfNeeded).filter(Objects::nonNull); - Stream sanitized = resolved.filter(this::passesPlausibilityChecks); - Stream inflated = sanitized.map(this::inflateIfNeeded).filter(Objects::nonNull); - return inflated.map(this::decrypt).filter(Objects::nonNull); + public Stream directoryListing() { + Stream pathIter = StreamSupport.stream(ciphertextDirStream.spliterator(), false).map(ProcessedPaths::new); + Stream resolved = pathIter.map(this::resolveConflictingFileIfNeeded).filter(Objects::nonNull); + Stream inflated = resolved.map(this::inflateIfNeeded).filter(Objects::nonNull); + Stream decrypted = inflated.map(this::decrypt).filter(Objects::nonNull); + Stream sanitized = decrypted.filter(this::passesPlausibilityChecks); + return sanitized; } - private Path resolveConflictingFileIfNeeded(Path potentiallyConflictingPath) { + private ProcessedPaths resolveConflictingFileIfNeeded(ProcessedPaths paths) { try { - return conflictResolver.resolveConflictsIfNecessary(potentiallyConflictingPath, directoryId); + return paths.withCiphertextPath(conflictResolver.resolveConflictsIfNecessary(paths.getCiphertextPath(), directoryId)); } catch (IOException e) { - LOG.warn("I/O exception while finding potentially conflicting file versions for {}.", potentiallyConflictingPath); + LOG.warn("I/O exception while finding potentially conflicting file versions for {}.", paths.getCiphertextPath()); return null; } } - private Path inflateIfNeeded(Path ciphertextPath) { - String fileName = ciphertextPath.getFileName().toString(); + private ProcessedPaths inflateIfNeeded(ProcessedPaths paths) { + String fileName = paths.getCiphertextPath().getFileName().toString(); if (LongFileNameProvider.isDeflated(fileName)) { try { String longFileName = longFileNameProvider.inflate(fileName); - return ciphertextPath.resolveSibling(longFileName); + return paths.withInflatedPath(paths.getCiphertextPath().resolveSibling(longFileName)); } catch (IOException e) { - LOG.warn(ciphertextPath + " could not be inflated."); + LOG.warn(paths.getCiphertextPath() + " could not be inflated."); return null; } } else { - return ciphertextPath; + return paths.withInflatedPath(paths.getCiphertextPath()); + } + } + + private ProcessedPaths decrypt(ProcessedPaths paths) { + Optional ciphertextName = encryptedNamePattern.extractEncryptedNameFromStart(paths.getInflatedPath()); + if (ciphertextName.isPresent()) { + String ciphertext = ciphertextName.get(); + try { + String cleartext = filenameCryptor.decryptFilename(ciphertext, directoryId.getBytes(StandardCharsets.UTF_8)); + return paths.withCleartextPath(cleartextDir.resolve(cleartext)); + } catch (AuthenticationFailedException e) { + LOG.warn(paths.getInflatedPath() + " not decryptable due to an unauthentic ciphertext."); + return null; + } + } else { + return null; } } @@ -113,12 +130,13 @@ private Path inflateIfNeeded(Path ciphertextPath) { * @param ciphertextPath The path to check. * @return true if the file is an existing ciphertext or directory file. */ - private boolean passesPlausibilityChecks(Path ciphertextPath) { - return !isBrokenDirectoryFile(ciphertextPath); + private boolean passesPlausibilityChecks(ProcessedPaths paths) { + return !isBrokenDirectoryFile(paths); } - private boolean isBrokenDirectoryFile(Path potentialDirectoryFile) { - if (potentialDirectoryFile.getFileName().toString().startsWith(Constants.DIR_PREFIX)) { + private boolean isBrokenDirectoryFile(ProcessedPaths paths) { + Path potentialDirectoryFile = paths.getCiphertextPath(); + if (paths.getInflatedPath().getFileName().toString().startsWith(Constants.DIR_PREFIX)) { final Path dirPath; try { dirPath = cryptoPathMapper.resolveDirectory(potentialDirectoryFile).path; @@ -134,22 +152,6 @@ private boolean isBrokenDirectoryFile(Path potentialDirectoryFile) { return false; } - private CiphertextAndCleartextPath decrypt(Path ciphertextPath) { - Optional ciphertextName = encryptedNamePattern.extractEncryptedNameFromStart(ciphertextPath); - if (ciphertextName.isPresent()) { - String ciphertext = ciphertextName.get(); - try { - String cleartext = filenameCryptor.decryptFilename(ciphertext, directoryId.getBytes(StandardCharsets.UTF_8)); - return new CiphertextAndCleartextPath(ciphertextPath, cleartextDir.resolve(cleartext)); - } catch (AuthenticationFailedException e) { - LOG.warn(ciphertextPath + " not decryptable due to an unauthentic ciphertext."); - return null; - } - } else { - return null; - } - } - private boolean isAcceptableByFilter(Path path) { try { return filter.accept(path); @@ -167,13 +169,19 @@ public void close() throws IOException { () -> LOG.trace("CLOSE {}", directoryId)); } - private static class CiphertextAndCleartextPath { + private static class ProcessedPaths { private final Path ciphertextPath; + private final Path inflatedPath; private final Path cleartextPath; - public CiphertextAndCleartextPath(Path ciphertextPath, Path cleartextPath) { + public ProcessedPaths(Path ciphertextPath) { + this(ciphertextPath, null, null); + } + + private ProcessedPaths(Path ciphertextPath, Path inflatedPath, Path cleartextPath) { this.ciphertextPath = ciphertextPath; + this.inflatedPath = inflatedPath; this.cleartextPath = cleartextPath; } @@ -181,10 +189,26 @@ public Path getCiphertextPath() { return ciphertextPath; } + public Path getInflatedPath() { + return inflatedPath; + } + public Path getCleartextPath() { return cleartextPath; } + public ProcessedPaths withCiphertextPath(Path ciphertextPath) { + return new ProcessedPaths(ciphertextPath, inflatedPath, cleartextPath); + } + + public ProcessedPaths withInflatedPath(Path inflatedPath) { + return new ProcessedPaths(ciphertextPath, inflatedPath, cleartextPath); + } + + public ProcessedPaths withCleartextPath(Path cleartextPath) { + return new ProcessedPaths(ciphertextPath, inflatedPath, cleartextPath); + } + } } diff --git a/src/test/java/org/cryptomator/cryptofs/DeleteNonEmptyCiphertextDirectoryIntegrationTest.java b/src/test/java/org/cryptomator/cryptofs/DeleteNonEmptyCiphertextDirectoryIntegrationTest.java index c651bdf2..e352d589 100644 --- a/src/test/java/org/cryptomator/cryptofs/DeleteNonEmptyCiphertextDirectoryIntegrationTest.java +++ b/src/test/java/org/cryptomator/cryptofs/DeleteNonEmptyCiphertextDirectoryIntegrationTest.java @@ -10,6 +10,7 @@ import static java.nio.file.Files.walkFileTree; import static java.nio.file.StandardOpenOption.CREATE_NEW; +import static org.cryptomator.cryptofs.Constants.NAME_SHORTENING_THRESHOLD; import static org.cryptomator.cryptofs.CryptoFileSystemProperties.cryptoFileSystemProperties; import static org.cryptomator.cryptofs.CryptoFileSystemUri.create; @@ -20,6 +21,8 @@ import java.nio.file.FileSystem; import java.nio.file.Files; import java.nio.file.Path; +import java.util.stream.Collectors; +import java.util.stream.IntStream; import java.util.stream.Stream; import org.junit.AfterClass; @@ -38,13 +41,16 @@ public class DeleteNonEmptyCiphertextDirectoryIntegrationTest { private static Path tempDir; private static Path pathToVault; + private static Path mDir; private static FileSystem fileSystem; @BeforeClass public static void setupClass() throws IOException { tempDir = Files.createTempDirectory("DNECDIT"); pathToVault = tempDir.resolve("vault"); + mDir = pathToVault.resolve("m"); Files.createDirectory(pathToVault); + Files.createDirectories(mDir); fileSystem = new CryptoFileSystemProvider().newFileSystem(create(pathToVault), cryptoFileSystemProperties().withPassphrase("asd").build()); } @@ -55,7 +61,7 @@ public static void teardownClass() throws IOException { @Test public void testDeleteCiphertextDirectoryContainingNonCryptoFile() throws IOException { - Path cleartextDirectory = fileSystem.getPath("/a"); + Path cleartextDirectory = fileSystem.getPath("/z"); Files.createDirectory(cleartextDirectory); Path ciphertextDirectory = firstEmptyCiphertextDirectory(); @@ -85,9 +91,34 @@ public void testDeleteCiphertextDirectoryContainingDirectories() throws IOExcept Files.delete(cleartextDirectory); } + @Test + public void testDeleteDirectoryContainingLongNameFileWithoutMetadata() throws IOException { + Path cleartextDirectory = fileSystem.getPath("/b"); + Files.createDirectory(cleartextDirectory); + + Path ciphertextDirectory = firstEmptyCiphertextDirectory(); + createFile(ciphertextDirectory, "HHEZJURE.lng", new byte[] {65}); + + Files.delete(cleartextDirectory); + } + + @Test + public void testDeleteDirectoryContainingUnauthenticLongNameDirectoryFile() throws IOException { + Path cleartextDirectory = fileSystem.getPath("/c"); + Files.createDirectory(cleartextDirectory); + + Path ciphertextDirectory = firstEmptyCiphertextDirectory(); + createFile(ciphertextDirectory, "HHEZJURE.lng", new byte[] {65}); + Path mSubdir = mDir.resolve("HH").resolve("EZ"); + Files.createDirectories(mSubdir); + createFile(mSubdir, "HHEZJURE.lng", "0HHEZJUREHHEZJUREHHEZJURE".getBytes()); + + Files.delete(cleartextDirectory); + } + @Test public void testDeleteNonEmptyDir() throws IOException { - Path cleartextDirectory = fileSystem.getPath("/a"); + Path cleartextDirectory = fileSystem.getPath("/d"); Files.createDirectory(cleartextDirectory); createFile(cleartextDirectory, "test", new byte[] {65}); @@ -96,9 +127,26 @@ public void testDeleteNonEmptyDir() throws IOException { Files.delete(cleartextDirectory); } + @Test + public void testDeleteDirectoryContainingLongNamedDirectory() throws IOException { + Path cleartextDirectory = fileSystem.getPath("/e"); + Files.createDirectory(cleartextDirectory); + + // a + // .. LongNameaaa... + String name = "LongName" + IntStream.range(0, NAME_SHORTENING_THRESHOLD) // + .mapToObj(ignored -> "a") // + .collect(Collectors.joining()); + createFolder(cleartextDirectory, name); + + thrown.expect(DirectoryNotEmptyException.class); + + Files.delete(cleartextDirectory); + } + @Test public void testDeleteEmptyDir() throws IOException { - Path cleartextDirectory = fileSystem.getPath("/a"); + Path cleartextDirectory = fileSystem.getPath("/f"); Files.createDirectory(cleartextDirectory); Files.delete(cleartextDirectory); From 13fbd2f2126b7bf23c9a59cf984de72ed44ce1df Mon Sep 17 00:00:00 2001 From: Markus Kreusch Date: Fri, 10 Nov 2017 14:37:19 +0100 Subject: [PATCH 7/8] Creating m directory implicitly during vault initialization --- .../java/org/cryptomator/cryptofs/CryptoFileSystemProvider.java | 2 ++ .../DeleteNonEmptyCiphertextDirectoryIntegrationTest.java | 1 - 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/src/main/java/org/cryptomator/cryptofs/CryptoFileSystemProvider.java b/src/main/java/org/cryptomator/cryptofs/CryptoFileSystemProvider.java index d94df6f6..ea9595af 100644 --- a/src/main/java/org/cryptomator/cryptofs/CryptoFileSystemProvider.java +++ b/src/main/java/org/cryptomator/cryptofs/CryptoFileSystemProvider.java @@ -162,6 +162,8 @@ public static void initialize(Path pathToVault, String masterkeyFilename, byte[] String rootDirHash = cryptor.fileNameCryptor().hashDirectoryId(Constants.ROOT_DIR_ID); Path rootDirPath = pathToVault.resolve(Constants.DATA_DIR_NAME).resolve(rootDirHash.substring(0, 2)).resolve(rootDirHash.substring(2)); Files.createDirectories(rootDirPath); + // create "m": + Files.createDirectory(pathToVault.resolve(Constants.METADATA_DIR_NAME)); } assert containsVault(pathToVault, masterkeyFilename); } diff --git a/src/test/java/org/cryptomator/cryptofs/DeleteNonEmptyCiphertextDirectoryIntegrationTest.java b/src/test/java/org/cryptomator/cryptofs/DeleteNonEmptyCiphertextDirectoryIntegrationTest.java index e352d589..32f9cc2a 100644 --- a/src/test/java/org/cryptomator/cryptofs/DeleteNonEmptyCiphertextDirectoryIntegrationTest.java +++ b/src/test/java/org/cryptomator/cryptofs/DeleteNonEmptyCiphertextDirectoryIntegrationTest.java @@ -50,7 +50,6 @@ public static void setupClass() throws IOException { pathToVault = tempDir.resolve("vault"); mDir = pathToVault.resolve("m"); Files.createDirectory(pathToVault); - Files.createDirectories(mDir); fileSystem = new CryptoFileSystemProvider().newFileSystem(create(pathToVault), cryptoFileSystemProperties().withPassphrase("asd").build()); } From 7557e570c9ca34512e388e5a1ed6881df98680a6 Mon Sep 17 00:00:00 2001 From: Markus Kreusch Date: Fri, 10 Nov 2017 15:11:00 +0100 Subject: [PATCH 8/8] Updated dependencies and version --- pom.xml | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/pom.xml b/pom.xml index 3ef0ac48..f37d59a6 100644 --- a/pom.xml +++ b/pom.xml @@ -2,7 +2,7 @@ 4.0.0 org.cryptomator cryptofs - 1.5.0-SNAPSHOT + 1.4.3 Cryptomator Crypto Filesystem This library provides the Java filesystem provider used by Cryptomator. https://github.com/cryptomator/cryptofs @@ -15,9 +15,9 @@ 1.8 - 1.1.6 - 2.11 - 23.0 + 1.1.7 + 2.13 + 23.4-jre 1.7.25 UTF-8 @@ -102,7 +102,7 @@ org.mockito mockito-core - 2.8.47 + 2.11.0 test @@ -137,7 +137,7 @@ org.apache.maven.plugins maven-compiler-plugin - 3.6.2 + 3.7.0 ${java.version} ${java.version} @@ -162,7 +162,7 @@ org.owasp dependency-check-maven - 2.1.0 + 3.0.1 24 0 @@ -185,7 +185,7 @@ com.codacy codacy-coverage-reporter - 2.0.0 + 2.0.1 assembly @@ -276,7 +276,7 @@ maven-dependency-plugin - 3.0.1 + 3.0.2 generate-dependency-list