Merge feature/#88-improve-backup-strategy

Backup Strategy refactoring:
* exception only thrown if masterkey cannot be read
* exceptions regarding backup creation or read are only logged
* renaming method to better show functionality

src/main/java/org/cryptomator/cryptofs/CryptoFileSystemModule.java

@@ -40,7 +40,7 @@ public Cryptor provideCryptor(CryptorProvider cryptorProvider, @PathToVault Path
 			byte[] keyFileContents = Files.readAllBytes(masterKeyPath);
 			Cryptor cryptor = cryptorProvider.createFromKeyFile(KeyFile.parse(keyFileContents), properties.passphrase(), properties.pepper(), Constants.VAULT_VERSION);
 			if (!readonlyFlag.isSet()) {
-				MasterkeyBackupHelper.backupMasterKey(masterKeyPath);
+				MasterkeyBackupHelper.attemptMasterKeyBackup(masterKeyPath);
 			}
 			return cryptor;
 		} catch (IOException e) {

src/main/java/org/cryptomator/cryptofs/common/MasterkeyBackupHelper.java

@@ -5,28 +5,22 @@
 import org.slf4j.LoggerFactory;
 
 import java.io.IOException;
-import java.io.InputStream;
 import java.nio.ByteBuffer;
-import java.nio.channels.FileChannel;
 import java.nio.channels.ReadableByteChannel;
-import java.nio.channels.SeekableByteChannel;
 import java.nio.channels.WritableByteChannel;
 import java.nio.file.AccessDeniedException;
+import java.nio.file.FileAlreadyExistsException;
 import java.nio.file.Files;
-import java.nio.file.NoSuchFileException;
 import java.nio.file.Path;
 import java.nio.file.StandardOpenOption;
 import java.security.MessageDigest;
 import java.security.NoSuchAlgorithmException;
-import java.util.Arrays;
-
-import static java.nio.file.StandardCopyOption.REPLACE_EXISTING;
 
 /**
  * Utility class for generating a suffix for the backup file to make it unique to its original master key file.
  */
 public final class MasterkeyBackupHelper {
-	
+
 	private static final Logger LOG = LoggerFactory.getLogger(MasterkeyBackupHelper.class);
 
 	/**
@@ -46,38 +40,42 @@ public static String generateFileIdSuffix(byte[] fileBytes) {
 	}
 
 	/**
-	 * Do a best-effort attempt to backup the masterkey at the given path. Fail silently if a valid backup already exists.
-	 * 
+	 * Do a best-effort attempt to backup the masterkey at the given path.
+	 * Fails silently if a _valid_ backup already exists and fails with a log entry, if any IO error occurs while creating or reading the backup file.
+	 *
 	 * @param masterKeyPath The masterkey file to backup
-	 * @throws IOException Any non-recoverable I/O exception that occurs during this attempt
+	 * @throws IOException If the masterkey cannot be read.
 	 */
-	public static Path backupMasterKey(Path masterKeyPath) throws IOException {
+	public static Path attemptMasterKeyBackup(Path masterKeyPath) throws IOException {
 		byte[] keyFileContents = Files.readAllBytes(masterKeyPath);
 		String backupFileName = masterKeyPath.getFileName().toString() + generateFileIdSuffix(keyFileContents) + Constants.MASTERKEY_BACKUP_SUFFIX;
 		Path backupFilePath = masterKeyPath.resolveSibling(backupFileName);
-		try (WritableByteChannel ch = Files.newByteChannel(backupFilePath, StandardOpenOption.CREATE, StandardOpenOption.WRITE, StandardOpenOption.TRUNCATE_EXISTING)) {
+		try (WritableByteChannel ch = Files.newByteChannel(backupFilePath, StandardOpenOption.CREATE_NEW, StandardOpenOption.WRITE)) {
 			ch.write(ByteBuffer.wrap(keyFileContents));
-		} catch (AccessDeniedException e) {
-			LOG.info("Storage device does not allow writing backup file. Comparing masterkey with backup directly.");
+		} catch (AccessDeniedException | FileAlreadyExistsException e) {
 			assertExistingBackupMatchesContent(backupFilePath, ByteBuffer.wrap(keyFileContents));
+		} catch (IOException e) {
+			LOG.warn("Failed to backup valid masterkey file.");
 		}
 		return backupFilePath;
 	}
-	
-	private static void assertExistingBackupMatchesContent(Path backupFilePath, ByteBuffer expectedContent) throws IOException {
+
+	private static void assertExistingBackupMatchesContent(Path backupFilePath, ByteBuffer expectedContent) {
 		if (Files.exists(backupFilePath)) {
 			// TODO replace by Files.mismatch() when using JDK > 12
 			ByteBuffer buf = ByteBuffer.allocateDirect(expectedContent.remaining() + 1);
 			try (ReadableByteChannel ch = Files.newByteChannel(backupFilePath, StandardOpenOption.READ)) {
 				ch.read(buf);
 				buf.flip();
 				if (buf.compareTo(expectedContent) != 0) {
-					throw new IllegalStateException("Corrupt masterkey backup: " + backupFilePath);
+					LOG.warn("Corrupt masterkey backup {}. Please replace it manually or unlock the vault on a writable storage device.", backupFilePath);
+				} else {
+					LOG.debug("Verified backup file: {}", backupFilePath);
 				}
-				LOG.debug("Verified backup file: {}", backupFilePath);
-			} catch (NoSuchFileException e) {
-				LOG.warn("Did not find backup file: {}", backupFilePath);
+			} catch (IOException e) {
+				LOG.warn("Failed to compare valid masterkey with backup file.", e);
 			}
 		}
 	}
+
 }

src/main/java/org/cryptomator/cryptofs/migration/v6/Version6Migrator.java

@@ -8,15 +8,13 @@
 import java.io.IOException;
 import java.nio.file.Files;
 import java.nio.file.Path;
-import java.nio.file.StandardCopyOption;
 import java.nio.file.StandardOpenOption;
 import java.text.Normalizer;
 import java.text.Normalizer.Form;
 
 import javax.inject.Inject;
 
 import org.cryptomator.cryptofs.common.MasterkeyBackupHelper;
-import org.cryptomator.cryptofs.common.Constants;
 import org.cryptomator.cryptofs.migration.api.MigrationContinuationListener;
 import org.cryptomator.cryptofs.migration.api.MigrationProgressListener;
 import org.cryptomator.cryptofs.migration.api.Migrator;
@@ -48,7 +46,7 @@ public void migrate(Path vaultRoot, String masterkeyFilename, CharSequence passp
 		KeyFile keyFile = KeyFile.parse(fileContentsBeforeUpgrade);
 		try (Cryptor cryptor = cryptorProvider.createFromKeyFile(keyFile, passphrase, 5)) {
 			// create backup, as soon as we know the password was correct:
-			Path masterkeyBackupFile = MasterkeyBackupHelper.backupMasterKey(masterkeyFile);
+			Path masterkeyBackupFile = MasterkeyBackupHelper.attemptMasterKeyBackup(masterkeyFile);
 			LOG.info("Backed up masterkey from {} to {}.", masterkeyFile.getFileName(), masterkeyBackupFile.getFileName());
 
 			progressListener.update(MigrationProgressListener.ProgressState.FINALIZING, 0.0);

src/main/java/org/cryptomator/cryptofs/migration/v7/Version7Migrator.java

@@ -6,7 +6,6 @@
 package org.cryptomator.cryptofs.migration.v7;
 
 import org.cryptomator.cryptofs.FileNameTooLongException;
-import org.cryptomator.cryptofs.common.Constants;
 import org.cryptomator.cryptofs.common.DeletingFileVisitor;
 import org.cryptomator.cryptofs.common.FileSystemCapabilityChecker;
 import org.cryptomator.cryptofs.common.MasterkeyBackupHelper;
@@ -28,7 +27,6 @@
 import java.nio.file.FileVisitOption;
 import java.nio.file.Files;
 import java.nio.file.Path;
-import java.nio.file.StandardCopyOption;
 import java.nio.file.StandardOpenOption;
 import java.util.EnumSet;
 
@@ -52,7 +50,7 @@ public void migrate(Path vaultRoot, String masterkeyFilename, CharSequence passp
 		KeyFile keyFile = KeyFile.parse(fileContentsBeforeUpgrade);
 		try (Cryptor cryptor = cryptorProvider.createFromKeyFile(keyFile, passphrase, 6)) {
 			// create backup, as soon as we know the password was correct:
-			Path masterkeyBackupFile = MasterkeyBackupHelper.backupMasterKey(masterkeyFile);
+			Path masterkeyBackupFile = MasterkeyBackupHelper.attemptMasterKeyBackup(masterkeyFile);
 			LOG.info("Backed up masterkey from {} to {}.", masterkeyFile.getFileName(), masterkeyBackupFile.getFileName());
 
 			// check file system capabilities:

src/test/java/org/cryptomator/cryptofs/common/MasterkeyBackupHelperTest.java

@@ -24,11 +24,11 @@ public void testBackupFilePosix(byte[] contents, @TempDir Path tmp) throws IOExc
 		Path originalFile = tmp.resolve("original");
 		Files.write(originalFile, contents);
 
-		Path backupFile = MasterkeyBackupHelper.backupMasterKey(originalFile);
+		Path backupFile = MasterkeyBackupHelper.attemptMasterKeyBackup(originalFile);
 		Assertions.assertArrayEquals(contents, Files.readAllBytes(backupFile));
 
 		Files.setPosixFilePermissions(backupFile, PosixFilePermissions.fromString("r--r--r--"));
-		Path backupFile2 = MasterkeyBackupHelper.backupMasterKey(originalFile);
+		Path backupFile2 = MasterkeyBackupHelper.attemptMasterKeyBackup(originalFile);
 		Assertions.assertEquals(backupFile, backupFile2);
 	}
 
@@ -39,11 +39,11 @@ public void testBackupFileWin(byte[] contents, @TempDir Path tmp) throws IOExcep
 		Path originalFile = tmp.resolve("original");
 		Files.write(originalFile, contents);
 
-		Path backupFile = MasterkeyBackupHelper.backupMasterKey(originalFile);
+		Path backupFile = MasterkeyBackupHelper.attemptMasterKeyBackup(originalFile);
 		Assertions.assertArrayEquals(contents, Files.readAllBytes(backupFile));
 
 		Files.getFileAttributeView(backupFile, DosFileAttributeView.class).setReadOnly(true);
-		Path backupFile2 = MasterkeyBackupHelper.backupMasterKey(originalFile);
+		Path backupFile2 = MasterkeyBackupHelper.attemptMasterKeyBackup(originalFile);
 		Assertions.assertEquals(backupFile, backupFile2);
 	}