From b35d35a6af8a1916d8d57c6b33bc2351a910937e Mon Sep 17 00:00:00 2001 From: Sebastian Stenzel Date: Tue, 18 Feb 2020 13:56:15 +0100 Subject: [PATCH 1/3] Determine write access by actually attempting to write. This fixes #71 --- .../cryptofs/CryptoFileSystemComponent.java | 2 + .../cryptofs/CryptoFileSystemProvider.java | 4 +- .../CryptoFileSystemProviderComponent.java | 4 +- .../CryptoFileSystemProviderModule.java | 18 +++++ .../cryptofs/CryptoFileSystems.java | 38 +++++++++-- .../cryptomator/cryptofs/ReadonlyFlag.java | 5 +- .../cryptofs/attr/AttributeViewProvider.java | 3 +- .../common/FileSystemCapabilityChecker.java | 34 +++++++++- .../cryptofs/migration/Migrators.java | 2 +- .../cryptofs/CryptoFileSystemsTest.java | 6 +- .../cryptofs/ReadonlyFlagTest.java | 66 ++++--------------- .../cryptofs/migration/MigratorsTest.java | 2 +- 12 files changed, 105 insertions(+), 79 deletions(-) create mode 100644 src/main/java/org/cryptomator/cryptofs/CryptoFileSystemProviderModule.java diff --git a/src/main/java/org/cryptomator/cryptofs/CryptoFileSystemComponent.java b/src/main/java/org/cryptomator/cryptofs/CryptoFileSystemComponent.java index e1c0caa1..28ab66f4 100644 --- a/src/main/java/org/cryptomator/cryptofs/CryptoFileSystemComponent.java +++ b/src/main/java/org/cryptomator/cryptofs/CryptoFileSystemComponent.java @@ -3,10 +3,12 @@ import dagger.BindsInstance; import dagger.Subcomponent; import org.cryptomator.cryptofs.attr.AttributeViewComponent; +import org.cryptomator.cryptofs.common.FileSystemCapabilityChecker; import org.cryptomator.cryptofs.dir.DirectoryStreamComponent; import org.cryptomator.cryptofs.fh.OpenCryptoFileComponent; import java.nio.file.Path; +import java.util.Set; @CryptoFileSystemScoped @Subcomponent(modules = {CryptoFileSystemModule.class}) diff --git a/src/main/java/org/cryptomator/cryptofs/CryptoFileSystemProvider.java b/src/main/java/org/cryptomator/cryptofs/CryptoFileSystemProvider.java index 9cc9de44..45a93b33 100644 --- a/src/main/java/org/cryptomator/cryptofs/CryptoFileSystemProvider.java +++ b/src/main/java/org/cryptomator/cryptofs/CryptoFileSystemProvider.java @@ -164,7 +164,7 @@ public static void initialize(Path pathToVault, String masterkeyFilename, byte[] if (!Files.isDirectory(pathToVault)) { throw new NotDirectoryException(pathToVault.toString()); } - new FileSystemCapabilityChecker().checkCapabilities(pathToVault); + new FileSystemCapabilityChecker().assertReadWriteCapabilities(pathToVault); try (Cryptor cryptor = CRYPTOR_PROVIDER.createNew()) { // save masterkey file: Path masterKeyPath = pathToVault.resolve(masterkeyFilename); @@ -293,8 +293,6 @@ public String getScheme() { public CryptoFileSystem newFileSystem(URI uri, Map rawProperties) throws IOException { CryptoFileSystemUri parsedUri = CryptoFileSystemUri.parse(uri); CryptoFileSystemProperties properties = CryptoFileSystemProperties.wrap(rawProperties); - - new FileSystemCapabilityChecker().checkCapabilities(parsedUri.pathToVault()); // TODO remove implicit initialization in 2.0.0 initializeFileSystemIfRequired(parsedUri, properties); diff --git a/src/main/java/org/cryptomator/cryptofs/CryptoFileSystemProviderComponent.java b/src/main/java/org/cryptomator/cryptofs/CryptoFileSystemProviderComponent.java index dad25a59..ce52f6e6 100644 --- a/src/main/java/org/cryptomator/cryptofs/CryptoFileSystemProviderComponent.java +++ b/src/main/java/org/cryptomator/cryptofs/CryptoFileSystemProviderComponent.java @@ -7,7 +7,7 @@ import javax.inject.Singleton; @Singleton -@Component +@Component(modules = {CryptoFileSystemProviderModule.class}) interface CryptoFileSystemProviderComponent { CryptoFileSystems fileSystems(); @@ -16,8 +16,6 @@ interface CryptoFileSystemProviderComponent { CopyOperation copyOperation(); - CryptoFileSystemComponent.Builder newCryptoFileSystemComponent(); - @Component.Builder interface Builder { @BindsInstance diff --git a/src/main/java/org/cryptomator/cryptofs/CryptoFileSystemProviderModule.java b/src/main/java/org/cryptomator/cryptofs/CryptoFileSystemProviderModule.java new file mode 100644 index 00000000..9bd7b33e --- /dev/null +++ b/src/main/java/org/cryptomator/cryptofs/CryptoFileSystemProviderModule.java @@ -0,0 +1,18 @@ +package org.cryptomator.cryptofs; + +import dagger.Module; +import dagger.Provides; +import org.cryptomator.cryptofs.common.FileSystemCapabilityChecker; + +import javax.inject.Singleton; + +@Module(subcomponents = {CryptoFileSystemComponent.class}) +public class CryptoFileSystemProviderModule { + + @Provides + @Singleton + public FileSystemCapabilityChecker provideFileSystemCapabilityChecker() { + return new FileSystemCapabilityChecker(); + } + +} diff --git a/src/main/java/org/cryptomator/cryptofs/CryptoFileSystems.java b/src/main/java/org/cryptomator/cryptofs/CryptoFileSystems.java index 61e55295..c9f0fda6 100644 --- a/src/main/java/org/cryptomator/cryptofs/CryptoFileSystems.java +++ b/src/main/java/org/cryptomator/cryptofs/CryptoFileSystems.java @@ -1,5 +1,9 @@ package org.cryptomator.cryptofs; +import org.cryptomator.cryptofs.common.FileSystemCapabilityChecker; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + import javax.inject.Inject; import javax.inject.Singleton; import java.io.IOException; @@ -7,6 +11,8 @@ import java.nio.file.FileSystemAlreadyExistsException; import java.nio.file.FileSystemNotFoundException; import java.nio.file.Path; +import java.util.EnumSet; +import java.util.Set; import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.ConcurrentMap; @@ -14,25 +20,28 @@ @Singleton class CryptoFileSystems { - - private final CryptoFileSystemProviderComponent cryptoFileSystemProviderComponent; + + private static final Logger LOG = LoggerFactory.getLogger(CryptoFileSystems.class); private final ConcurrentMap fileSystems = new ConcurrentHashMap<>(); + private final CryptoFileSystemComponent.Builder cryptoFileSystemComponentBuilder; + private final FileSystemCapabilityChecker capabilityChecker; @Inject - public CryptoFileSystems(CryptoFileSystemProviderComponent cryptoFileSystemProviderComponent) { - this.cryptoFileSystemProviderComponent = cryptoFileSystemProviderComponent; + public CryptoFileSystems(CryptoFileSystemComponent.Builder cryptoFileSystemComponentBuilder, FileSystemCapabilityChecker capabilityChecker) { + this.cryptoFileSystemComponentBuilder = cryptoFileSystemComponentBuilder; + this.capabilityChecker = capabilityChecker; } public CryptoFileSystemImpl create(CryptoFileSystemProvider provider, Path pathToVault, CryptoFileSystemProperties properties) throws IOException { try { Path normalizedPathToVault = pathToVault.normalize(); + CryptoFileSystemProperties adjustedProperites = adjustForCapabilities(normalizedPathToVault, properties); return fileSystems.compute(normalizedPathToVault, (key, value) -> { if (value == null) { - return cryptoFileSystemProviderComponent // - .newCryptoFileSystemComponent() // + return cryptoFileSystemComponentBuilder // .pathToVault(key) // - .properties(properties) // + .properties(adjustedProperites) // .provider(provider) // .build() // .cryptoFileSystem(); @@ -44,6 +53,21 @@ public CryptoFileSystemImpl create(CryptoFileSystemProvider provider, Path pathT throw new IOException("Error during file system creation.", e); } } + + private CryptoFileSystemProperties adjustForCapabilities(Path pathToVault, CryptoFileSystemProperties originalProperties) throws FileSystemCapabilityChecker.MissingCapabilityException { + if (!originalProperties.readonly()) { + try { + capabilityChecker.assertReadWriteCapabilities(pathToVault); + return originalProperties; + } catch (FileSystemCapabilityChecker.MissingCapabilityException e) { + LOG.warn("Missing file system capabilities for read-write access. Fallback to read-only access."); + } + } + capabilityChecker.assertReadOnlyCapabilities(pathToVault); + Set flags = EnumSet.copyOf(originalProperties.flags()); + flags.add(CryptoFileSystemProperties.FileSystemFlags.READONLY); + return CryptoFileSystemProperties.cryptoFileSystemPropertiesFrom(originalProperties).withFlags(flags).build(); + } public void remove(CryptoFileSystemImpl cryptoFileSystem) { fileSystems.values().remove(cryptoFileSystem); diff --git a/src/main/java/org/cryptomator/cryptofs/ReadonlyFlag.java b/src/main/java/org/cryptomator/cryptofs/ReadonlyFlag.java index bd1de9fa..913ee717 100644 --- a/src/main/java/org/cryptomator/cryptofs/ReadonlyFlag.java +++ b/src/main/java/org/cryptomator/cryptofs/ReadonlyFlag.java @@ -18,13 +18,10 @@ public class ReadonlyFlag { private final boolean readonly; @Inject - public ReadonlyFlag(CryptoFileSystemProperties properties, @PathToVault Path pathToVault) { + public ReadonlyFlag(CryptoFileSystemProperties properties) { if (properties.readonly()) { LOG.info("Vault opened readonly."); readonly = true; - } else if (!Files.isWritable(pathToVault)) { - LOG.warn("Vault directory is write-protected."); - readonly = true; } else { LOG.debug("Vault opened for read and write."); readonly = false; diff --git a/src/main/java/org/cryptomator/cryptofs/attr/AttributeViewProvider.java b/src/main/java/org/cryptomator/cryptofs/attr/AttributeViewProvider.java index 28f07f72..c5df2ac6 100644 --- a/src/main/java/org/cryptomator/cryptofs/attr/AttributeViewProvider.java +++ b/src/main/java/org/cryptomator/cryptofs/attr/AttributeViewProvider.java @@ -8,9 +8,8 @@ *******************************************************************************/ package org.cryptomator.cryptofs.attr; -import org.cryptomator.cryptofs.CryptoFileSystemComponent; -import org.cryptomator.cryptofs.CryptoPath; import org.cryptomator.cryptofs.CryptoFileSystemScoped; +import org.cryptomator.cryptofs.CryptoPath; import org.slf4j.Logger; import org.slf4j.LoggerFactory; diff --git a/src/main/java/org/cryptomator/cryptofs/common/FileSystemCapabilityChecker.java b/src/main/java/org/cryptomator/cryptofs/common/FileSystemCapabilityChecker.java index d9531a6d..68fc4eae 100644 --- a/src/main/java/org/cryptomator/cryptofs/common/FileSystemCapabilityChecker.java +++ b/src/main/java/org/cryptomator/cryptofs/common/FileSystemCapabilityChecker.java @@ -17,17 +17,36 @@ public class FileSystemCapabilityChecker { private static final Logger LOG = LoggerFactory.getLogger(FileSystemCapabilityChecker.class); public enum Capability { + /** + * File system allows write access + * @since 1.9.3 + */ + WRITE_ACCESS, + /** * File system supports filenames with ≥ 230 chars. + * @since @since 1.9.2 */ LONG_FILENAMES, /** * File system supports paths with ≥ 400 chars. + * @since @since 1.9.2 */ LONG_PATHS, } + /** + * Checks whether the underlying filesystem has all required capabilities for readonly access. + * @param pathToVault Path to a vault's storage location + * @throws MissingCapabilityException if any check fails + * @implNote Only short-running tests with constant time are performed + * @since 1.9.3 + */ + public void assertReadOnlyCapabilities(Path pathToVault) throws MissingCapabilityException { + // no-op + } + /** * Checks whether the underlying filesystem has all required capabilities. * @@ -36,19 +55,30 @@ public enum Capability { * @implNote Only short-running tests with constant time are performed * @since 1.9.2 */ - public void checkCapabilities(Path pathToVault) throws MissingCapabilityException { + public void assertReadWriteCapabilities(Path pathToVault) throws MissingCapabilityException { Path checkDir = pathToVault.resolve("c"); try { + checkWriteAccess(checkDir); checkLongFilenames(checkDir); checkLongFilePaths(checkDir); } finally { try { - MoreFiles.deleteRecursively(checkDir, RecursiveDeleteOption.ALLOW_INSECURE); + if (Files.exists(checkDir)) { + MoreFiles.deleteRecursively(checkDir, RecursiveDeleteOption.ALLOW_INSECURE); + } } catch (IOException e) { LOG.warn("Failed to clean up " + checkDir, e); } } } + + private void checkWriteAccess(Path checkDir) throws MissingCapabilityException { + try { + Files.createDirectories(checkDir); + } catch (IOException e) { + throw new MissingCapabilityException(checkDir, Capability.WRITE_ACCESS); + } + } private void checkLongFilenames(Path checkDir) throws MissingCapabilityException { String longFileName = Strings.repeat("a", 226) + ".c9r"; diff --git a/src/main/java/org/cryptomator/cryptofs/migration/Migrators.java b/src/main/java/org/cryptomator/cryptofs/migration/Migrators.java index 8937eb60..2709f37a 100644 --- a/src/main/java/org/cryptomator/cryptofs/migration/Migrators.java +++ b/src/main/java/org/cryptomator/cryptofs/migration/Migrators.java @@ -94,7 +94,7 @@ public boolean needsMigration(Path pathToVault, String masterkeyFilename) throws * @throws IOException if an I/O error occurs migrating the vault */ public void migrate(Path pathToVault, String masterkeyFilename, CharSequence passphrase, MigrationProgressListener progressListener) throws NoApplicableMigratorException, InvalidPassphraseException, IOException { - fsCapabilityChecker.checkCapabilities(pathToVault); + fsCapabilityChecker.assertReadWriteCapabilities(pathToVault); Path masterKeyPath = pathToVault.resolve(masterkeyFilename); byte[] keyFileContents = Files.readAllBytes(masterKeyPath); diff --git a/src/test/java/org/cryptomator/cryptofs/CryptoFileSystemsTest.java b/src/test/java/org/cryptomator/cryptofs/CryptoFileSystemsTest.java index 9c1c909b..56687d7b 100644 --- a/src/test/java/org/cryptomator/cryptofs/CryptoFileSystemsTest.java +++ b/src/test/java/org/cryptomator/cryptofs/CryptoFileSystemsTest.java @@ -1,5 +1,6 @@ package org.cryptomator.cryptofs; +import org.cryptomator.cryptofs.common.FileSystemCapabilityChecker; import org.hamcrest.MatcherAssert; import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.BeforeEach; @@ -25,14 +26,13 @@ public class CryptoFileSystemsTest { private final CryptoFileSystemComponent cryptoFileSystemComponent = mock(CryptoFileSystemComponent.class); private final CryptoFileSystemImpl cryptoFileSystem = mock(CryptoFileSystemImpl.class); - private final CryptoFileSystemProviderComponent cryptoFileSystemProviderComponent = mock(CryptoFileSystemProviderComponent.class); private final CryptoFileSystemComponent.Builder cryptoFileSystemComponentBuilder = mock(CryptoFileSystemComponent.Builder.class); + private final FileSystemCapabilityChecker capabilityChecker = mock(FileSystemCapabilityChecker.class); - private final CryptoFileSystems inTest = new CryptoFileSystems(cryptoFileSystemProviderComponent); + private final CryptoFileSystems inTest = new CryptoFileSystems(cryptoFileSystemComponentBuilder, capabilityChecker); @BeforeEach public void setup() { - when(cryptoFileSystemProviderComponent.newCryptoFileSystemComponent()).thenReturn(cryptoFileSystemComponentBuilder); when(cryptoFileSystemComponentBuilder.provider(any())).thenReturn(cryptoFileSystemComponentBuilder); when(cryptoFileSystemComponentBuilder.pathToVault(any())).thenReturn(cryptoFileSystemComponentBuilder); when(cryptoFileSystemComponentBuilder.properties(any())).thenReturn(cryptoFileSystemComponentBuilder); diff --git a/src/test/java/org/cryptomator/cryptofs/ReadonlyFlagTest.java b/src/test/java/org/cryptomator/cryptofs/ReadonlyFlagTest.java index 27b132de..db400eb8 100644 --- a/src/test/java/org/cryptomator/cryptofs/ReadonlyFlagTest.java +++ b/src/test/java/org/cryptomator/cryptofs/ReadonlyFlagTest.java @@ -1,80 +1,40 @@ package org.cryptomator.cryptofs; -import org.hamcrest.CoreMatchers; -import org.hamcrest.MatcherAssert; import org.junit.jupiter.api.Assertions; -import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.DisplayName; -import org.junit.jupiter.api.Test; import org.junit.jupiter.params.ParameterizedTest; -import org.junit.jupiter.params.provider.CsvSource; -import org.mockito.Mockito; +import org.junit.jupiter.params.provider.ValueSource; import java.io.IOException; -import java.nio.file.AccessDeniedException; -import java.nio.file.AccessMode; -import java.nio.file.FileStore; -import java.nio.file.FileSystem; -import java.nio.file.Path; import java.nio.file.ReadOnlyFileSystemException; -import java.nio.file.spi.FileSystemProvider; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; public class ReadonlyFlagTest { - private FileStore fileStore = mock(FileStore.class); - private FileSystemProvider provider = mock(FileSystemProvider.class); - private FileSystem fileSystem = mock(FileSystem.class); - private Path path = mock(Path.class, "test-path"); - private CryptoFileSystemProperties properties = mock(CryptoFileSystemProperties.class); - @BeforeEach - public void setup() throws IOException { - when(path.getFileSystem()).thenReturn(fileSystem); - when(fileSystem.provider()).thenReturn(provider); - when(provider.getFileStore(path)).thenReturn(fileStore); - } - @DisplayName("isSet()") - @ParameterizedTest(name = "readonlyFlag: {0}, writeProtected: {1} -> mounted readonly {2}") - @CsvSource({ - "false, false, false", - "true, false, true", - "false, true, true", - "true, true, true", - }) - public void testIsSet(boolean readonlyFlag, boolean writeProtected, boolean expectedResult) throws IOException { - when(properties.readonly()).thenReturn(readonlyFlag); - if (writeProtected) { - Mockito.doThrow(new AccessDeniedException(path.toString())).when(provider).checkAccess(path, AccessMode.WRITE); - } - ReadonlyFlag inTest = new ReadonlyFlag(properties, path); + @ParameterizedTest(name = "readonlyFlag: {0} -> mounted readonly {0}") + @ValueSource(booleans = {true, false}) + public void testIsSet(boolean readonly) { + when(properties.readonly()).thenReturn(readonly); + ReadonlyFlag inTest = new ReadonlyFlag(properties); boolean result = inTest.isSet(); - MatcherAssert.assertThat(result, CoreMatchers.is(expectedResult)); - Assertions.assertEquals(expectedResult, result); + Assertions.assertEquals(readonly, result); } @DisplayName("assertWritable()") - @ParameterizedTest(name = "readonlyFlag: {0}, writeProtected: {1} -> mounted readonly {2}") - @CsvSource({ - "false, false, false", - "true, false, true", - "false, true, true", - "true, true, true", - }) - public void testAssertWritable(boolean readonlyFlag, boolean writeProtected, boolean expectedResult) throws IOException { - when(properties.readonly()).thenReturn(readonlyFlag); - if (writeProtected) { - Mockito.doThrow(new AccessDeniedException(path.toString())).when(provider).checkAccess(path, AccessMode.WRITE); - } - ReadonlyFlag inTest = new ReadonlyFlag(properties, path); + @ParameterizedTest(name = "readonlyFlag: {0} -> mounted readonly {0}") + @ValueSource(booleans = {true, false}) + public void testAssertWritable(boolean readonly) { + when(properties.readonly()).thenReturn(readonly); + ReadonlyFlag inTest = new ReadonlyFlag(properties); - if (expectedResult) { + if (readonly) { Assertions.assertThrows(ReadOnlyFileSystemException.class, () -> { inTest.assertWritable(); }); diff --git a/src/test/java/org/cryptomator/cryptofs/migration/MigratorsTest.java b/src/test/java/org/cryptomator/cryptofs/migration/MigratorsTest.java index f15ae16f..d3ca0bbc 100644 --- a/src/test/java/org/cryptomator/cryptofs/migration/MigratorsTest.java +++ b/src/test/java/org/cryptomator/cryptofs/migration/MigratorsTest.java @@ -89,7 +89,7 @@ public void testMigrateWithFailingCapabilitiesCheck() throws IOException { Migrators migrators = new Migrators(Collections.emptyMap(), fsCapabilityChecker); Exception expected = new FileSystemCapabilityChecker.MissingCapabilityException(pathToVault, FileSystemCapabilityChecker.Capability.LONG_FILENAMES); - Mockito.doThrow(expected).when(fsCapabilityChecker).checkCapabilities(pathToVault); + Mockito.doThrow(expected).when(fsCapabilityChecker).assertReadWriteCapabilities(pathToVault); Exception thrown = Assertions.assertThrows(FileSystemCapabilityChecker.MissingCapabilityException.class, () -> { migrators.migrate(pathToVault, "masterkey.cryptomator", "secret", (state, progress) -> {}); From 8a3d719b6e907a5e232d97424dd76081a8df1004 Mon Sep 17 00:00:00 2001 From: Sebastian Stenzel Date: Wed, 26 Feb 2020 13:31:33 +0100 Subject: [PATCH 2/3] Amendment to #71 --- .../cryptofs/CryptoFileSystemProvider.java | 2 +- .../cryptofs/CryptoFileSystems.java | 14 ++-- .../common/FileSystemCapabilityChecker.java | 81 ++++++++++++------- .../cryptofs/migration/Migrators.java | 2 +- .../cryptofs/migration/MigratorsTest.java | 2 +- 5 files changed, 64 insertions(+), 37 deletions(-) diff --git a/src/main/java/org/cryptomator/cryptofs/CryptoFileSystemProvider.java b/src/main/java/org/cryptomator/cryptofs/CryptoFileSystemProvider.java index 45a93b33..ef323f29 100644 --- a/src/main/java/org/cryptomator/cryptofs/CryptoFileSystemProvider.java +++ b/src/main/java/org/cryptomator/cryptofs/CryptoFileSystemProvider.java @@ -164,7 +164,7 @@ public static void initialize(Path pathToVault, String masterkeyFilename, byte[] if (!Files.isDirectory(pathToVault)) { throw new NotDirectoryException(pathToVault.toString()); } - new FileSystemCapabilityChecker().assertReadWriteCapabilities(pathToVault); + new FileSystemCapabilityChecker().assertAllCapabilities(pathToVault); try (Cryptor cryptor = CRYPTOR_PROVIDER.createNew()) { // save masterkey file: Path masterKeyPath = pathToVault.resolve(masterkeyFilename); diff --git a/src/main/java/org/cryptomator/cryptofs/CryptoFileSystems.java b/src/main/java/org/cryptomator/cryptofs/CryptoFileSystems.java index c9f0fda6..867afd1c 100644 --- a/src/main/java/org/cryptomator/cryptofs/CryptoFileSystems.java +++ b/src/main/java/org/cryptomator/cryptofs/CryptoFileSystems.java @@ -57,16 +57,18 @@ public CryptoFileSystemImpl create(CryptoFileSystemProvider provider, Path pathT private CryptoFileSystemProperties adjustForCapabilities(Path pathToVault, CryptoFileSystemProperties originalProperties) throws FileSystemCapabilityChecker.MissingCapabilityException { if (!originalProperties.readonly()) { try { - capabilityChecker.assertReadWriteCapabilities(pathToVault); + capabilityChecker.assertWriteAccess(pathToVault); return originalProperties; } catch (FileSystemCapabilityChecker.MissingCapabilityException e) { - LOG.warn("Missing file system capabilities for read-write access. Fallback to read-only access."); + capabilityChecker.assertReadAccess(pathToVault); + LOG.warn("No write access to vault. Fallback to read-only access."); + Set flags = EnumSet.copyOf(originalProperties.flags()); + flags.add(CryptoFileSystemProperties.FileSystemFlags.READONLY); + return CryptoFileSystemProperties.cryptoFileSystemPropertiesFrom(originalProperties).withFlags(flags).build(); } + } else { + return originalProperties; } - capabilityChecker.assertReadOnlyCapabilities(pathToVault); - Set flags = EnumSet.copyOf(originalProperties.flags()); - flags.add(CryptoFileSystemProperties.FileSystemFlags.READONLY); - return CryptoFileSystemProperties.cryptoFileSystemPropertiesFrom(originalProperties).withFlags(flags).build(); } public void remove(CryptoFileSystemImpl cryptoFileSystem) { diff --git a/src/main/java/org/cryptomator/cryptofs/common/FileSystemCapabilityChecker.java b/src/main/java/org/cryptomator/cryptofs/common/FileSystemCapabilityChecker.java index 68fc4eae..443bcbcb 100644 --- a/src/main/java/org/cryptomator/cryptofs/common/FileSystemCapabilityChecker.java +++ b/src/main/java/org/cryptomator/cryptofs/common/FileSystemCapabilityChecker.java @@ -8,6 +8,7 @@ import org.slf4j.LoggerFactory; import java.io.IOException; +import java.nio.file.DirectoryStream; import java.nio.file.FileSystemException; import java.nio.file.Files; import java.nio.file.Path; @@ -17,6 +18,12 @@ public class FileSystemCapabilityChecker { private static final Logger LOG = LoggerFactory.getLogger(FileSystemCapabilityChecker.class); public enum Capability { + /** + * File system allows read access + * @since 1.9.3 + */ + READ_ACCESS, + /** * File system allows write access * @since 1.9.3 @@ -37,67 +44,85 @@ public enum Capability { } /** - * Checks whether the underlying filesystem has all required capabilities for readonly access. + * Checks whether the underlying filesystem has all required capabilities. + * * @param pathToVault Path to a vault's storage location * @throws MissingCapabilityException if any check fails * @implNote Only short-running tests with constant time are performed - * @since 1.9.3 + * @since 1.9.2 */ - public void assertReadOnlyCapabilities(Path pathToVault) throws MissingCapabilityException { - // no-op + public void assertAllCapabilities(Path pathToVault) throws MissingCapabilityException { + assertReadAccess(pathToVault); + assertWriteAccess(pathToVault); + assertLongFilenameSupport(pathToVault); + assertLongFilePathSupport(pathToVault); } /** - * Checks whether the underlying filesystem has all required capabilities. - * + * Checks whether the underlying filesystem allows reading the given dir. * @param pathToVault Path to a vault's storage location - * @throws MissingCapabilityException if any check fails - * @implNote Only short-running tests with constant time are performed - * @since 1.9.2 + * @throws MissingCapabilityException if the check fails + * @since 1.9.3 */ - public void assertReadWriteCapabilities(Path pathToVault) throws MissingCapabilityException { - Path checkDir = pathToVault.resolve("c"); - try { - checkWriteAccess(checkDir); - checkLongFilenames(checkDir); - checkLongFilePaths(checkDir); - } finally { - try { - if (Files.exists(checkDir)) { - MoreFiles.deleteRecursively(checkDir, RecursiveDeleteOption.ALLOW_INSECURE); - } - } catch (IOException e) { - LOG.warn("Failed to clean up " + checkDir, e); - } + public void assertReadAccess(Path pathToVault) throws MissingCapabilityException { + try (DirectoryStream ds = Files.newDirectoryStream(pathToVault)) { + assert ds != null; + } catch (IOException e) { + throw new MissingCapabilityException(pathToVault, Capability.READ_ACCESS); } } - - private void checkWriteAccess(Path checkDir) throws MissingCapabilityException { + + /** + * Checks whether the underlying filesystem allows writing to the given dir. + * @param pathToVault Path to a vault's storage location + * @throws MissingCapabilityException if the check fails + * @since 1.9.3 + */ + public void assertWriteAccess(Path pathToVault) throws MissingCapabilityException { + Path checkDir = pathToVault.resolve("c"); try { - Files.createDirectories(checkDir); + Files.createDirectory(checkDir); } catch (IOException e) { throw new MissingCapabilityException(checkDir, Capability.WRITE_ACCESS); + } finally { + deleteSilently(checkDir); } } - private void checkLongFilenames(Path checkDir) throws MissingCapabilityException { + public void assertLongFilenameSupport(Path pathToVault) throws MissingCapabilityException { String longFileName = Strings.repeat("a", 226) + ".c9r"; + Path checkDir = pathToVault.resolve("c"); Path p = checkDir.resolve(longFileName); try { Files.createDirectories(p); } catch (IOException e) { throw new MissingCapabilityException(p, Capability.LONG_FILENAMES); + } finally { + deleteSilently(checkDir); } } - private void checkLongFilePaths(Path checkDir) throws MissingCapabilityException { + public void assertLongFilePathSupport(Path pathToVault) throws MissingCapabilityException { String longFileName = Strings.repeat("a", 96) + ".c9r"; String longPath = Joiner.on('/').join(longFileName, longFileName, longFileName, longFileName); + Path checkDir = pathToVault.resolve("c"); Path p = checkDir.resolve(longPath); try { Files.createDirectories(p); } catch (IOException e) { throw new MissingCapabilityException(p, Capability.LONG_PATHS); + } finally { + deleteSilently(checkDir); + } + } + + private void deleteSilently(Path dir) { + try { + if (Files.exists(dir)) { + MoreFiles.deleteRecursively(dir, RecursiveDeleteOption.ALLOW_INSECURE); + } + } catch (IOException e) { + LOG.warn("Failed to clean up " + dir, e); } } diff --git a/src/main/java/org/cryptomator/cryptofs/migration/Migrators.java b/src/main/java/org/cryptomator/cryptofs/migration/Migrators.java index 2709f37a..0cd5475c 100644 --- a/src/main/java/org/cryptomator/cryptofs/migration/Migrators.java +++ b/src/main/java/org/cryptomator/cryptofs/migration/Migrators.java @@ -94,7 +94,7 @@ public boolean needsMigration(Path pathToVault, String masterkeyFilename) throws * @throws IOException if an I/O error occurs migrating the vault */ public void migrate(Path pathToVault, String masterkeyFilename, CharSequence passphrase, MigrationProgressListener progressListener) throws NoApplicableMigratorException, InvalidPassphraseException, IOException { - fsCapabilityChecker.assertReadWriteCapabilities(pathToVault); + fsCapabilityChecker.assertAllCapabilities(pathToVault); Path masterKeyPath = pathToVault.resolve(masterkeyFilename); byte[] keyFileContents = Files.readAllBytes(masterKeyPath); diff --git a/src/test/java/org/cryptomator/cryptofs/migration/MigratorsTest.java b/src/test/java/org/cryptomator/cryptofs/migration/MigratorsTest.java index d3ca0bbc..6a90919d 100644 --- a/src/test/java/org/cryptomator/cryptofs/migration/MigratorsTest.java +++ b/src/test/java/org/cryptomator/cryptofs/migration/MigratorsTest.java @@ -89,7 +89,7 @@ public void testMigrateWithFailingCapabilitiesCheck() throws IOException { Migrators migrators = new Migrators(Collections.emptyMap(), fsCapabilityChecker); Exception expected = new FileSystemCapabilityChecker.MissingCapabilityException(pathToVault, FileSystemCapabilityChecker.Capability.LONG_FILENAMES); - Mockito.doThrow(expected).when(fsCapabilityChecker).assertReadWriteCapabilities(pathToVault); + Mockito.doThrow(expected).when(fsCapabilityChecker).assertAllCapabilities(pathToVault); Exception thrown = Assertions.assertThrows(FileSystemCapabilityChecker.MissingCapabilityException.class, () -> { migrators.migrate(pathToVault, "masterkey.cryptomator", "secret", (state, progress) -> {}); From ef3c0856b8c4668bcaa3c12ed4df3a422c2c17a5 Mon Sep 17 00:00:00 2001 From: Sebastian Stenzel Date: Wed, 26 Feb 2020 15:38:32 +0100 Subject: [PATCH 3/3] Preparing 1.9.3 --- pom.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pom.xml b/pom.xml index cd8fa368..a5d72544 100644 --- a/pom.xml +++ b/pom.xml @@ -2,7 +2,7 @@ 4.0.0 org.cryptomator cryptofs - 1.10.0-SNAPSHOT + 1.9.3 Cryptomator Crypto Filesystem This library provides the Java filesystem provider used by Cryptomator. https://github.com/cryptomator/cryptofs