Merge branch 'conflict-detection'

main/filesystem-crypto/src/main/java/org/cryptomator/crypto/engine/CryptoException.java

@@ -8,7 +8,7 @@
  *******************************************************************************/
 package org.cryptomator.crypto.engine;
 
-abstract class CryptoException extends RuntimeException {
+public abstract class CryptoException extends RuntimeException {
 
 	public CryptoException() {
 		super();

main/filesystem-crypto/src/main/java/org/cryptomator/crypto/engine/FilenameCryptor.java

@@ -8,6 +8,8 @@
  *******************************************************************************/
 package org.cryptomator.crypto.engine;
 
+import java.util.regex.Pattern;
+
 /**
  * Provides deterministic encryption capabilities as filenames must not change on subsequent encryption attempts,
  * otherwise each change results in major directory structure changes which would be a terrible idea for cloud storage encryption.
@@ -22,12 +24,9 @@ public interface FilenameCryptor {
 	String hashDirectoryId(String cleartextDirectoryId);
 
 	/**
-	 * Tests without an actual decryption attempt, if a name is a well-formed ciphertext.
-	 * 
-	 * @param ciphertextName Filename in question
-	 * @return <code>true</code> if the given name is likely to be a valid ciphertext
+	 * @return A Pattern that can be used to test, if a name is a well-formed ciphertext.
 	 */
-	boolean isEncryptedFilename(String ciphertextName);
+	Pattern encryptedNamePattern();
 
 	/**
 	 * @param cleartextName original filename including cleartext file extension

main/filesystem-crypto/src/main/java/org/cryptomator/crypto/engine/impl/FilenameCryptorImpl.java

@@ -12,6 +12,7 @@ import static java.nio.charset.StandardCharsets.UTF_8;
 
 import java.security.MessageDigest;
 import java.security.NoSuchAlgorithmException;
+import java.util.regex.Pattern;
 
 import javax.crypto.AEADBadTagException;
 import javax.crypto.SecretKey;
@@ -25,6 +26,7 @@ import org.cryptomator.siv.SivMode;
 class FilenameCryptorImpl implements FilenameCryptor {
 
 	private static final BaseNCodec BASE32 = new Base32();
+	private static final Pattern BASE32_PATTERN = Pattern.compile("([A-Z0-9]{8})*[A-Z0-9=]{8}");
 	private static final ThreadLocal<MessageDigest> SHA1 = new ThreadLocalSha1();
 	private static final ThreadLocal<SivMode> AES_SIV = new ThreadLocal<SivMode>() {
 		@Override
@@ -50,8 +52,8 @@ class FilenameCryptorImpl implements FilenameCryptor {
 	}
 
 	@Override
-	public boolean isEncryptedFilename(String ciphertextName) {
-		return BASE32.isInAlphabet(ciphertextName);
+	public Pattern encryptedNamePattern() {
+		return BASE32_PATTERN;
 	}
 
 	@Override

main/filesystem-crypto/src/main/java/org/cryptomator/filesystem/crypto/ConflictResolver.java

@@ -0,0 +1,81 @@
+package org.cryptomator.filesystem.crypto;
+
+import static org.cryptomator.filesystem.crypto.Constants.DIR_SUFFIX;
+
+import java.util.UUID;
+import java.util.function.UnaryOperator;
+import java.util.regex.MatchResult;
+import java.util.regex.Matcher;
+import java.util.regex.Pattern;
+
+import org.apache.commons.lang3.StringUtils;
+import org.cryptomator.crypto.engine.CryptoException;
+import org.cryptomator.filesystem.File;
+import org.cryptomator.filesystem.Folder;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+final class ConflictResolver {
+
+	private static final Logger LOG = LoggerFactory.getLogger(ConflictResolver.class);
+	private static final int UUID_FIRST_GROUP_STRLEN = 8;
+
+	private final Pattern encryptedNamePattern;
+	private final UnaryOperator<String> nameDecryptor;
+	private final UnaryOperator<String> nameEncryptor;
+
+	public ConflictResolver(Pattern encryptedNamePattern, UnaryOperator<String> nameDecryptor, UnaryOperator<String> nameEncryptor) {
+		this.encryptedNamePattern = encryptedNamePattern;
+		this.nameDecryptor = nameDecryptor;
+		this.nameEncryptor = nameEncryptor;
+	}
+
+	public File resolveIfNecessary(File file) {
+		Matcher m = encryptedNamePattern.matcher(StringUtils.removeEnd(file.name(), DIR_SUFFIX));
+		if (m.matches()) {
+			// full match, use file as is
+			return file;
+		} else if (m.find(0)) {
+			// partial match, might be conflicting
+			return resolveConflict(file, m.toMatchResult());
+		} else {
+			// no match, file not relevant
+			return file;
+		}
+	}
+
+	private File resolveConflict(File conflictingFile, MatchResult matchResult) {
+		String ciphertext = matchResult.group();
+		boolean isDirectory = conflictingFile.name().substring(matchResult.end()).startsWith(DIR_SUFFIX);
+		try {
+			String cleartext = nameDecryptor.apply(ciphertext);
+			Folder folder = conflictingFile.parent().get();
+			File canonicalFile = folder.file(isDirectory ? ciphertext + DIR_SUFFIX : ciphertext);
+			if (canonicalFile.exists()) {
+				// conflict detected! look for an alternative name:
+				File alternativeFile;
+				String conflictId;
+				do {
+					conflictId = createConflictId();
+					String alternativeCleartext = cleartext + " (Conflict " + conflictId + ")";
+					String alternativeCiphertext = nameEncryptor.apply(alternativeCleartext);
+					alternativeFile = folder.file(isDirectory ? alternativeCiphertext + DIR_SUFFIX : alternativeCiphertext);
+				} while (alternativeFile.exists());
+				LOG.info("Detected conflict {}:\n{}\n{}", conflictId, canonicalFile, conflictingFile);
+				conflictingFile.moveTo(alternativeFile);
+				return alternativeFile;
+			} else {
+				conflictingFile.moveTo(canonicalFile);
+				return canonicalFile;
+			}
+		} catch (CryptoException e) {
+			// not decryptable; false positive
+			return conflictingFile;
+		}
+	}
+
+	private String createConflictId() {
+		return UUID.randomUUID().toString().substring(0, UUID_FIRST_GROUP_STRLEN);
+	}
+
+}

main/filesystem-crypto/src/main/java/org/cryptomator/filesystem/crypto/CryptoFolder.java

@@ -18,6 +18,7 @@ import java.util.Optional;
 import java.util.UUID;
 import java.util.concurrent.atomic.AtomicReference;
 import java.util.function.Predicate;
+import java.util.regex.Pattern;
 import java.util.stream.Stream;
 
 import org.apache.commons.lang3.StringUtils;
@@ -36,11 +37,15 @@ class CryptoFolder extends CryptoNode implements Folder {
 	private final WeakValuedCache<String, CryptoFolder> folders = WeakValuedCache.usingLoader(this::newFolder);
 	private final WeakValuedCache<String, CryptoFile> files = WeakValuedCache.usingLoader(this::newFile);
 	private final AtomicReference<String> directoryId = new AtomicReference<>();
+	private final ConflictResolver conflictResolver;
 
 	public CryptoFolder(CryptoFolder parent, String name, Cryptor cryptor) {
 		super(parent, name, cryptor);
+		this.conflictResolver = new ConflictResolver(cryptor.getFilenameCryptor().encryptedNamePattern(), this::decryptChildName, this::encryptChildName);
 	}
 
+	/* ======================= name + directory id ======================= */
+
 	@Override
 	protected Optional<String> encryptedName() {
 		if (parent().isPresent()) {
@@ -73,49 +78,54 @@ class CryptoFolder extends CryptoNode implements Folder {
 		}));
 	}
 
+	/* ======================= children ======================= */
+
 	@Override
 	public Stream<? extends Node> children() {
 		return AutoClosingStream.from(Stream.concat(files(), folders()));
 	}
 
-	@Override
-	public Stream<CryptoFile> files() {
+	private Stream<File> nonConflictingFiles() {
 		final Stream<? extends File> files = physicalFolder().filter(Folder::exists).map(Folder::files).orElse(Stream.empty());
-		return files.map(File::name).filter(isEncryptedFileName()).map(this::decryptChildFileName).map(this::file);
+		return files.filter(containsEncryptedName()).map(conflictResolver::resolveIfNecessary);
 	}
 
-	private Predicate<String> isEncryptedFileName() {
-		return (String name) -> !name.endsWith(DIR_SUFFIX) && cryptor.getFilenameCryptor().isEncryptedFilename(name);
+	private Predicate<File> containsEncryptedName() {
+		final Pattern encryptedNamePattern = cryptor.getFilenameCryptor().encryptedNamePattern();
+		return (File file) -> encryptedNamePattern.matcher(file.name()).find();
 	}
 
-	private String decryptChildFileName(String encryptedFileName) {
+	private String decryptChildName(String ciphertextFileName) {
 		final byte[] dirId = getDirectoryId().get().getBytes(UTF_8);
-		return cryptor.getFilenameCryptor().decryptFilename(encryptedFileName, dirId);
+		return cryptor.getFilenameCryptor().decryptFilename(ciphertextFileName, dirId);
 	}
 
-	@Override
-	public CryptoFile file(String name) {
-		return files.get(name);
+	private String encryptChildName(String cleartextFileName) {
+		final byte[] dirId = getDirectoryId().get().getBytes(UTF_8);
+		return cryptor.getFilenameCryptor().encryptFilename(cleartextFileName, dirId);
 	}
 
-	public CryptoFile newFile(String name) {
-		return new CryptoFile(this, name, cryptor);
+	@Override
+	public Stream<CryptoFile> files() {
+		return nonConflictingFiles().map(File::name).filter(endsWithDirSuffix().negate()).map(this::decryptChildName).map(this::file);
 	}
 
 	@Override
 	public Stream<CryptoFolder> folders() {
-		final Stream<? extends File> files = physicalFolder().filter(Folder::exists).map(Folder::files).orElse(Stream.empty());
-		return files.map(File::name).filter(isEncryptedDirectoryName()).map(this::decryptChildFolderName).map(this::folder);
+		return nonConflictingFiles().map(File::name).filter(endsWithDirSuffix()).map(this::removeDirSuffix).map(this::decryptChildName).map(this::folder);
 	}
 
-	private Predicate<String> isEncryptedDirectoryName() {
-		return (String name) -> name.endsWith(DIR_SUFFIX) && cryptor.getFilenameCryptor().isEncryptedFilename(StringUtils.removeEnd(name, DIR_SUFFIX));
+	private Predicate<String> endsWithDirSuffix() {
+		return (String encryptedFolderName) -> StringUtils.endsWith(encryptedFolderName, DIR_SUFFIX);
 	}
 
-	private String decryptChildFolderName(String encryptedFolderName) {
-		final byte[] dirId = getDirectoryId().get().getBytes(UTF_8);
-		final String ciphertext = StringUtils.removeEnd(encryptedFolderName, DIR_SUFFIX);
-		return cryptor.getFilenameCryptor().decryptFilename(ciphertext, dirId);
+	private String removeDirSuffix(String encryptedFolderName) {
+		return StringUtils.removeEnd(encryptedFolderName, DIR_SUFFIX);
+	}
+
+	@Override
+	public CryptoFile file(String name) {
+		return files.get(name);
 	}
 
 	@Override
@@ -123,10 +133,16 @@ class CryptoFolder extends CryptoNode implements Folder {
 		return folders.get(name);
 	}
 
-	public CryptoFolder newFolder(String name) {
+	private CryptoFile newFile(String name) {
+		return new CryptoFile(this, name, cryptor);
+	}
+
+	private CryptoFolder newFolder(String name) {
 		return new CryptoFolder(this, name, cryptor);
 	}
 
+	/* ======================= create/move/delete ======================= */
+
 	@Override
 	public void create() {
 		parent.create();
@@ -176,7 +192,7 @@ class CryptoFolder extends CryptoNode implements Folder {
 		// cut all ties:
 		this.invalidateDirectoryIdsRecursively();
 
-		assert!exists();
+		assert !exists();
 		assert target.exists();
 	}
 

main/filesystem-crypto/src/test/java/org/cryptomator/crypto/engine/NoFilenameCryptor.java

@@ -12,6 +12,7 @@ import static java.nio.charset.StandardCharsets.UTF_8;
 
 import java.security.MessageDigest;
 import java.security.NoSuchAlgorithmException;
+import java.util.regex.Pattern;
 
 import org.apache.commons.codec.binary.Base32;
 import org.apache.commons.codec.binary.BaseNCodec;
@@ -19,6 +20,7 @@ import org.apache.commons.codec.binary.BaseNCodec;
 class NoFilenameCryptor implements FilenameCryptor {
 
 	private static final BaseNCodec BASE32 = new Base32();
+	private static final Pattern WILDCARD_PATTERN = Pattern.compile(".*");
 	private static final ThreadLocal<MessageDigest> SHA1 = new ThreadLocalSha1();
 
 	@Override
@@ -29,8 +31,8 @@ class NoFilenameCryptor implements FilenameCryptor {
 	}
 
 	@Override
-	public boolean isEncryptedFilename(String ciphertextName) {
-		return true;
+	public Pattern encryptedNamePattern() {
+		return WILDCARD_PATTERN;
 	}
 
 	@Override

main/filesystem-crypto/src/test/java/org/cryptomator/filesystem/crypto/ConflictResolverTest.java

@@ -0,0 +1,110 @@
+package org.cryptomator.filesystem.crypto;
+
+import java.nio.charset.StandardCharsets;
+import java.util.Optional;
+import java.util.function.UnaryOperator;
+import java.util.regex.Pattern;
+
+import org.apache.commons.codec.binary.Base32;
+import org.apache.commons.codec.binary.BaseNCodec;
+import org.cryptomator.filesystem.File;
+import org.cryptomator.filesystem.Folder;
+import org.junit.Assert;
+import org.junit.Before;
+import org.junit.Test;
+import org.mockito.Mockito;
+
+public class ConflictResolverTest {
+
+	private ConflictResolver conflictResolver;
+	private Folder folder;
+	private File canonicalFile;
+	private File canonicalFolder;
+	private File conflictingFile;
+	private File conflictingFolder;
+	private File resolved;
+	private File unrelatedFile;
+
+	@Before
+	public void setup() {
+		Pattern base32Pattern = Pattern.compile("([A-Z0-9]{8})*[A-Z0-9=]{8}");
+		BaseNCodec base32 = new Base32();
+		UnaryOperator<String> decode = (s) -> new String(base32.decode(s), StandardCharsets.UTF_8);
+		UnaryOperator<String> encode = (s) -> base32.encodeAsString(s.getBytes(StandardCharsets.UTF_8));
+		conflictResolver = new ConflictResolver(base32Pattern, decode, encode);
+
+		folder = Mockito.mock(Folder.class);
+		canonicalFile = Mockito.mock(File.class);
+		canonicalFolder = Mockito.mock(File.class);
+		conflictingFile = Mockito.mock(File.class);
+		conflictingFolder = Mockito.mock(File.class);
+		resolved = Mockito.mock(File.class);
+		unrelatedFile = Mockito.mock(File.class);
+
+		String canonicalFileName = encode.apply("test name");
+		String canonicalFolderName = encode.apply("test name") + Constants.DIR_SUFFIX;
+		String conflictingFileName = canonicalFileName + " (version 2)";
+		String conflictingFolderName = canonicalFolderName + " (version 2)";
+		String unrelatedName = "notBa$e32!";
+
+		Mockito.when(canonicalFile.name()).thenReturn(canonicalFileName);
+		Mockito.when(canonicalFolder.name()).thenReturn(canonicalFolderName);
+		Mockito.when(conflictingFile.name()).thenReturn(conflictingFileName);
+		Mockito.when(conflictingFolder.name()).thenReturn(conflictingFolderName);
+		Mockito.when(unrelatedFile.name()).thenReturn(unrelatedName);
+
+		Mockito.when(canonicalFile.exists()).thenReturn(true);
+		Mockito.when(canonicalFolder.exists()).thenReturn(true);
+		Mockito.when(conflictingFile.exists()).thenReturn(true);
+		Mockito.when(conflictingFolder.exists()).thenReturn(true);
+		Mockito.when(unrelatedFile.exists()).thenReturn(true);
+
+		Mockito.doReturn(Optional.of(folder)).when(canonicalFile).parent();
+		Mockito.doReturn(Optional.of(folder)).when(canonicalFolder).parent();
+		Mockito.doReturn(Optional.of(folder)).when(conflictingFile).parent();
+		Mockito.doReturn(Optional.of(folder)).when(conflictingFolder).parent();
+		Mockito.doReturn(Optional.of(folder)).when(unrelatedFile).parent();
+
+		Mockito.when(folder.file(Mockito.startsWith(canonicalFileName.substring(0, 8)))).thenReturn(resolved);
+		Mockito.when(folder.file(canonicalFileName)).thenReturn(canonicalFile);
+		Mockito.when(folder.file(canonicalFolderName)).thenReturn(canonicalFolder);
+		Mockito.when(folder.file(conflictingFileName)).thenReturn(conflictingFile);
+		Mockito.when(folder.file(conflictingFolderName)).thenReturn(conflictingFolder);
+		Mockito.when(folder.file(unrelatedName)).thenReturn(unrelatedFile);
+	}
+
+	@Test
+	public void testCanonicalName() {
+		File resolved = conflictResolver.resolveIfNecessary(canonicalFile);
+		Assert.assertSame(canonicalFile, resolved);
+	}
+
+	@Test
+	public void testUnrelatedName() {
+		File resolved = conflictResolver.resolveIfNecessary(unrelatedFile);
+		Assert.assertSame(unrelatedFile, resolved);
+	}
+
+	@Test
+	public void testConflictingFile() {
+		File resolved = conflictResolver.resolveIfNecessary(conflictingFile);
+		Mockito.verify(conflictingFile).moveTo(resolved);
+		Assert.assertSame(resolved, resolved);
+	}
+
+	@Test
+	public void testConflictingFileIfCanonicalDoesnExist() {
+		Mockito.when(canonicalFile.exists()).thenReturn(false);
+		File resolved = conflictResolver.resolveIfNecessary(conflictingFile);
+		Mockito.verify(conflictingFile).moveTo(canonicalFile);
+		Assert.assertSame(canonicalFile, resolved);
+	}
+
+	@Test
+	public void testConflictingFolder() {
+		File resolved = conflictResolver.resolveIfNecessary(conflictingFolder);
+		Mockito.verify(conflictingFolder).moveTo(resolved);
+		Assert.assertSame(resolved, resolved);
+	}
+
+}