Store database key in a file rather than shared prefs.

bramble-api/src/main/java/org/briarproject/bramble/api/db/DatabaseConfig.java

@@ -14,6 +14,8 @@ public interface DatabaseConfig {
 
 	File getDatabaseDirectory();
 
+	File getDatabaseKeyDirectory();
+
 	void setEncryptionKey(SecretKey key);
 
 	@Nullable

bramble-core/src/test/java/org/briarproject/bramble/test/TestDatabaseConfig.java

@@ -9,25 +9,31 @@ import java.io.File;
 @NotNullByDefault
 public class TestDatabaseConfig implements DatabaseConfig {
 
-	private final File dir;
+	private final File dbDir, keyDir;
 	private final long maxSize;
 	private volatile SecretKey key = new SecretKey(new byte[SecretKey.LENGTH]);
 
-	public TestDatabaseConfig(File dir, long maxSize) {
+	public TestDatabaseConfig(File testDir, long maxSize) {
-		this.dir = dir;
+		dbDir = new File(testDir, "db");
+		keyDir = new File(testDir, "key");
 		this.maxSize = maxSize;
 	}
 
 	@Override
 	public boolean databaseExists() {
-		if (!dir.isDirectory()) return false;
+		if (!dbDir.isDirectory()) return false;
-		File[] files = dir.listFiles();
+		File[] files = dbDir.listFiles();
 		return files != null && files.length > 0;
 	}
 
 	@Override
 	public File getDatabaseDirectory() {
-		return dir;
+		return dbDir;
+	}
+
+	@Override
+	public File getDatabaseKeyDirectory() {
+		return keyDir;
 	}
 
 	@Override

briar-android/src/main/java/org/briarproject/briar/android/AndroidDatabaseConfig.java

@@ -17,31 +17,32 @@ class AndroidDatabaseConfig implements DatabaseConfig {
 	private static final Logger LOG =
 			Logger.getLogger(AndroidDatabaseConfig.class.getName());
 
-	private final File dir;
+	private final File dbDir, keyDir;
 
 	@Nullable
 	private volatile SecretKey key = null;
 	@Nullable
 	private volatile String nickname = null;
 
-	AndroidDatabaseConfig(File dir) {
+	AndroidDatabaseConfig(File dbDir, File keyDir) {
-		this.dir = dir;
+		this.dbDir = dbDir;
+		this.keyDir = keyDir;
 	}
 
 	@Override
 	public boolean databaseExists() {
 		// FIXME should not run on UiThread #620
-		if (!dir.isDirectory()) {
+		if (!dbDir.isDirectory()) {
 			if (LOG.isLoggable(INFO))
-				LOG.info(dir.getAbsolutePath() + " is not a directory");
+				LOG.info(dbDir.getAbsolutePath() + " is not a directory");
 			return false;
 		}
-		File[] files = dir.listFiles();
+		File[] files = dbDir.listFiles();
 		if (LOG.isLoggable(INFO)) {
 			if (files == null) {
-				LOG.info("Could not list files in " + dir.getAbsolutePath());
+				LOG.info("Could not list files in " + dbDir.getAbsolutePath());
 			} else {
-				LOG.info("Files in " + dir.getAbsolutePath() + ":");
+				LOG.info("Files in " + dbDir.getAbsolutePath() + ":");
 				for (File f : files) LOG.info(f.getName());
 			}
 			LOG.info("Database exists: " + (files != null && files.length > 0));
@@ -51,10 +52,16 @@ class AndroidDatabaseConfig implements DatabaseConfig {
 
 	@Override
 	public File getDatabaseDirectory() {
-		File dir = this.dir;
 		if (LOG.isLoggable(INFO))
-			LOG.info("Database directory: " + dir.getAbsolutePath());
+			LOG.info("Database directory: " + dbDir.getAbsolutePath());
-		return dir;
+		return dbDir;
+	}
+
+	@Override
+	public File getDatabaseKeyDirectory() {
+		if (LOG.isLoggable(INFO))
+			LOG.info("Database key directory: " + keyDir.getAbsolutePath());
+		return keyDir;
 	}
 
 	@Override

briar-android/src/main/java/org/briarproject/briar/android/AppModule.java

@@ -87,11 +87,13 @@ public class AppModule {
 		//FIXME: StrictMode
 		StrictMode.ThreadPolicy tp = StrictMode.allowThreadDiskReads();
 		StrictMode.allowThreadDiskWrites();
-		File dir = app.getApplicationContext().getDir("db", MODE_PRIVATE);
+		File dbDir = app.getApplicationContext().getDir("db", MODE_PRIVATE);
+		File keyDir = app.getApplicationContext().getDir("key", MODE_PRIVATE);
 		StrictMode.setThreadPolicy(tp);
 		@MethodsNotNullByDefault
 		@ParametersNotNullByDefault
-		DatabaseConfig databaseConfig = new AndroidDatabaseConfig(dir);
+		DatabaseConfig databaseConfig =
+				new AndroidDatabaseConfig(dbDir, keyDir);
 		return databaseConfig;
 	}
 

briar-android/src/main/java/org/briarproject/briar/android/controller/ConfigController.java

@@ -12,7 +12,7 @@ public interface ConfigController {
 	@Nullable
 	String getEncryptedDatabaseKey();
 
-	void storeEncryptedDatabaseKey(String hex);
+	boolean storeEncryptedDatabaseKey(String hex);
 
 	void deleteAccount(Context ctx);
 

briar-android/src/main/java/org/briarproject/briar/android/controller/ConfigControllerImpl.java

@@ -1,6 +1,5 @@
 package org.briarproject.briar.android.controller;
 
-import android.annotation.SuppressLint;
 import android.content.Context;
 import android.content.SharedPreferences;
 import android.support.v7.preference.PreferenceManager;
@@ -9,12 +8,19 @@ import org.briarproject.bramble.api.db.DatabaseConfig;
 import org.briarproject.bramble.api.nullsafety.NotNullByDefault;
 import org.briarproject.bramble.util.AndroidUtils;
 
+import java.io.BufferedReader;
+import java.io.File;
+import java.io.FileInputStream;
+import java.io.FileOutputStream;
+import java.io.IOException;
+import java.io.InputStreamReader;
 import java.util.logging.Logger;
 
 import javax.annotation.Nullable;
 import javax.inject.Inject;
 
 import static java.util.logging.Level.INFO;
+import static java.util.logging.Level.WARNING;
 
 @NotNullByDefault
 public class ConfigControllerImpl implements ConfigController {
@@ -23,6 +29,8 @@ public class ConfigControllerImpl implements ConfigController {
 			Logger.getLogger(ConfigControllerImpl.class.getName());
 
 	private static final String PREF_DB_KEY = "key";
+	private static final String DB_KEY_FILENAME = "db.key";
+	private static final String DB_KEY_BACKUP_FILENAME = "db.key.bak";
 
 	private final SharedPreferences briarPrefs;
 	protected final DatabaseConfig databaseConfig;
@@ -37,17 +45,101 @@ public class ConfigControllerImpl implements ConfigController {
 	@Override
 	@Nullable
 	public String getEncryptedDatabaseKey() {
-		String key = briarPrefs.getString(PREF_DB_KEY, null);
+		String key = getDatabaseKeyFromPreferences();
-		if (LOG.isLoggable(INFO))
+		if (key == null) key = getDatabaseKeyFromFile();
-			LOG.info("Got database key from preferences: " + (key != null));
+		else migrateDatabaseKeyToFile(key);
 		return key;
 	}
 
+	@Nullable
+	private String getDatabaseKeyFromPreferences() {
+		String key = briarPrefs.getString(PREF_DB_KEY, null);
+		if (key == null) LOG.info("No database key in preferences");
+		else LOG.info("Found database key in preferences");
+		return key;
+	}
+
+	@Nullable
+	private String getDatabaseKeyFromFile() {
+		String key = readDbKeyFromFile(getDbKeyFile());
+		if (key == null) {
+			LOG.info("No database key in primary file");
+			key = readDbKeyFromFile(getDbKeyBackupFile());
+			if (key == null) LOG.info("No database key in backup file");
+			else LOG.warning("Found database key in backup file");
+		} else {
+			LOG.info("Found database key in primary file");
+		}
+		return key;
+	}
+
+	@Nullable
+	private String readDbKeyFromFile(File f) {
+		if (!f.exists()) {
+			LOG.info("Key file does not exist");
+			return null;
+		}
+		try {
+			BufferedReader reader = new BufferedReader(new InputStreamReader(
+					new FileInputStream(f), "UTF-8"));
+			String key = reader.readLine();
+			reader.close();
+			return key;
+		} catch (IOException e) {
+			if (LOG.isLoggable(WARNING)) LOG.log(WARNING, e.toString(), e);
+			return null;
+		}
+	}
+
+	private File getDbKeyFile() {
+		return new File(databaseConfig.getDatabaseKeyDirectory(),
+				DB_KEY_FILENAME);
+	}
+
+	private File getDbKeyBackupFile() {
+		return new File(databaseConfig.getDatabaseKeyDirectory(),
+				DB_KEY_BACKUP_FILENAME);
+	}
+
+	private void migrateDatabaseKeyToFile(String key) {
+		if (storeEncryptedDatabaseKey(key)) {
+			if (briarPrefs.edit().remove(PREF_DB_KEY).commit())
+				LOG.info("Database key migrated to file");
+			else LOG.warning("Database key not removed from preferences");
+		} else {
+			LOG.warning("Database key not migrated to file");
+		}
+	}
+
 	@Override
-	@SuppressLint("ApplySharedPref")
+	public boolean storeEncryptedDatabaseKey(String hex) {
-	public void storeEncryptedDatabaseKey(String hex) {
+		LOG.info("Storing database key in file");
-		LOG.info("Storing database key in preferences");
+		File dbKey = getDbKeyFile();
-		briarPrefs.edit().putString(PREF_DB_KEY, hex).commit();
+		File dbKeyBackup = getDbKeyBackupFile();
+		try {
+			// Create the directory if necessary
+			if (databaseConfig.getDatabaseKeyDirectory().mkdirs())
+				LOG.info("Created database key directory");
+			// Write to the backup file
+			FileOutputStream out = new FileOutputStream(dbKeyBackup);
+			out.write(hex.getBytes("UTF-8"));
+			out.flush();
+			out.close();
+			LOG.info("Stored database key in backup file");
+			// Delete the old key file, if it exists
+			if (dbKey.exists()) {
+				if (dbKey.delete()) LOG.info("Deleted primary file");
+				else LOG.warning("Failed to delete primary file");
+			}
+			// The backup file becomes the new key file
+			boolean renamed = dbKeyBackup.renameTo(dbKey);
+			if (renamed) LOG.info("Renamed backup file to primary");
+			else LOG.warning("Failed to rename backup file to primary");
+			return renamed;
+		} catch (IOException e) {
+			if (LOG.isLoggable(WARNING)) LOG.log(WARNING, e.toString(), e);
+			return false;
+		}
 	}
 
 	@Override
@@ -73,5 +165,4 @@ public class ConfigControllerImpl implements ConfigController {
 		if (LOG.isLoggable(INFO)) LOG.info("Signed in: " + signedIn);
 		return signedIn;
 	}
-
 }

briar-android/src/main/java/org/briarproject/briar/android/login/PasswordControllerImpl.java

@@ -72,8 +72,8 @@ public class PasswordControllerImpl extends ConfigControllerImpl
 			} else {
 				String hex =
 						encryptDatabaseKey(new SecretKey(key), newPassword);
-				storeEncryptedDatabaseKey(hex);
+				boolean stored = storeEncryptedDatabaseKey(hex);
-				resultHandler.onResult(true);
+				resultHandler.onResult(stored);
 			}
 		});
 	}

briar-android/src/test/java/org/briarproject/briar/android/TestDatabaseKeyUtils.java

@@ -0,0 +1,35 @@
+package org.briarproject.briar.android;
+
+import org.briarproject.bramble.api.nullsafety.NotNullByDefault;
+
+import java.io.BufferedReader;
+import java.io.File;
+import java.io.FileInputStream;
+import java.io.FileOutputStream;
+import java.io.IOException;
+import java.io.InputStreamReader;
+
+import javax.annotation.Nullable;
+
+import static junit.framework.Assert.assertTrue;
+
+@NotNullByDefault
+public class TestDatabaseKeyUtils {
+
+	public static void storeDatabaseKey(File f, String hex) throws IOException {
+		assertTrue(f.getParentFile().mkdirs());
+		FileOutputStream out = new FileOutputStream(f);
+		out.write(hex.getBytes("UTF-8"));
+		out.flush();
+		out.close();
+	}
+
+	@Nullable
+	public static String loadDatabaseKey(File f) throws IOException {
+		BufferedReader reader = new BufferedReader(new InputStreamReader(
+				new FileInputStream(f), "UTF-8"));
+		String hex = reader.readLine();
+		reader.close();
+		return hex;
+	}
+}

briar-android/src/test/java/org/briarproject/briar/android/login/PasswordControllerImplTest.java

@@ -8,14 +8,23 @@ import org.briarproject.bramble.api.db.DatabaseConfig;
 import org.briarproject.bramble.test.BrambleMockTestCase;
 import org.briarproject.bramble.test.ImmediateExecutor;
 import org.jmock.Expectations;
+import org.junit.After;
 import org.junit.Test;
 
+import java.io.File;
 import java.util.concurrent.Executor;
 import java.util.concurrent.atomic.AtomicBoolean;
 
+import static junit.framework.Assert.assertEquals;
 import static junit.framework.Assert.assertFalse;
 import static junit.framework.Assert.assertTrue;
+import static org.briarproject.bramble.test.TestUtils.deleteTestDirectory;
+import static org.briarproject.bramble.test.TestUtils.getRandomBytes;
 import static org.briarproject.bramble.test.TestUtils.getSecretKey;
+import static org.briarproject.bramble.test.TestUtils.getTestDirectory;
+import static org.briarproject.bramble.util.StringUtils.toHexString;
+import static org.briarproject.briar.android.TestDatabaseKeyUtils.loadDatabaseKey;
+import static org.briarproject.briar.android.TestDatabaseKeyUtils.storeDatabaseKey;
 
 public class PasswordControllerImplTest extends BrambleMockTestCase {
 
@@ -26,62 +35,84 @@ public class PasswordControllerImplTest extends BrambleMockTestCase {
 	private final CryptoComponent crypto = context.mock(CryptoComponent.class);
 	private final PasswordStrengthEstimator estimator =
 			context.mock(PasswordStrengthEstimator.class);
-	private final SharedPreferences.Editor editor =
-			context.mock(SharedPreferences.Editor.class);
 
 	private final Executor cryptoExecutor = new ImmediateExecutor();
 
 	private final String oldPassword = "some.old.pass";
 	private final String newPassword = "some.new.pass";
-	private final String oldEncryptedHex = "010203";
+	private final byte[] oldEncryptedKey = getRandomBytes(123);
-	private final String newEncryptedHex = "020304";
+	private final byte[] newEncryptedKey = getRandomBytes(123);
-	private final byte[] oldEncryptedBytes = new byte[] {1, 2, 3};
+	private final byte[] key = getSecretKey().getBytes();
-	private final byte[] newEncryptedBytes = new byte[] {2, 3, 4};
+	private final File testDir = getTestDirectory();
-	private final byte[] keyBytes = getSecretKey().getBytes();
+	private final File keyDir = new File(testDir, "key");
+	private final File keyFile = new File(keyDir, "db.key");
+	private final File keyBackupFile = new File(keyDir, "db.key.bak");
 
 	@Test
-	public void testChangePasswordReturnsTrue() {
+	public void testChangePasswordReturnsTrue() throws Exception {
 		context.checking(new Expectations() {{
 			// Look up the encrypted DB key
 			oneOf(briarPrefs).getString("key", null);
-			will(returnValue(oldEncryptedHex));
+			will(returnValue(null));
+			allowing(databaseConfig).getDatabaseKeyDirectory();
+			will(returnValue(keyDir));
 			// Decrypt and re-encrypt the key
-			oneOf(crypto).decryptWithPassword(oldEncryptedBytes, oldPassword);
+			oneOf(crypto).decryptWithPassword(oldEncryptedKey, oldPassword);
-			will(returnValue(keyBytes));
+			will(returnValue(key));
-			oneOf(crypto).encryptWithPassword(keyBytes, newPassword);
+			oneOf(crypto).encryptWithPassword(key, newPassword);
-			will(returnValue(newEncryptedBytes));
+			will(returnValue(newEncryptedKey));
-			// Store the re-encrypted key
-			oneOf(briarPrefs).edit();
-			will(returnValue(editor));
-			oneOf(editor).putString("key", newEncryptedHex);
-			will(returnValue(editor));
-			oneOf(editor).commit();
 		}});
 
+		assertFalse(keyFile.exists());
+		assertFalse(keyBackupFile.exists());
+
+		storeDatabaseKey(keyFile, toHexString(oldEncryptedKey));
+
 		PasswordControllerImpl p = new PasswordControllerImpl(briarPrefs,
 				databaseConfig, cryptoExecutor, crypto, estimator);
 
 		AtomicBoolean capturedResult = new AtomicBoolean(false);
 		p.changePassword(oldPassword, newPassword, capturedResult::set);
 		assertTrue(capturedResult.get());
+
+		assertTrue(keyFile.exists());
+		assertFalse(keyBackupFile.exists());
+		assertEquals(toHexString(newEncryptedKey), loadDatabaseKey(keyFile));
 	}
 
 	@Test
-	public void testChangePasswordReturnsFalseIfOldPasswordIsWrong() {
+	public void testChangePasswordReturnsFalseIfOldPasswordIsWrong()
+			throws Exception {
 		context.checking(new Expectations() {{
 			// Look up the encrypted DB key
 			oneOf(briarPrefs).getString("key", null);
-			will(returnValue(oldEncryptedHex));
+			will(returnValue(null));
+			allowing(databaseConfig).getDatabaseKeyDirectory();
+			will(returnValue(keyDir));
 			// Try to decrypt the key - the password is wrong
-			oneOf(crypto).decryptWithPassword(oldEncryptedBytes, oldPassword);
+			oneOf(crypto).decryptWithPassword(oldEncryptedKey, oldPassword);
 			will(returnValue(null));
 		}});
 
+		assertFalse(keyFile.exists());
+		assertFalse(keyBackupFile.exists());
+
+		storeDatabaseKey(keyFile, toHexString(oldEncryptedKey));
+
 		PasswordControllerImpl p = new PasswordControllerImpl(briarPrefs,
 				databaseConfig, cryptoExecutor, crypto, estimator);
 
 		AtomicBoolean capturedResult = new AtomicBoolean(true);
 		p.changePassword(oldPassword, newPassword, capturedResult::set);
 		assertFalse(capturedResult.get());
+
+		assertTrue(keyFile.exists());
+		assertFalse(keyBackupFile.exists());
+		assertEquals(toHexString(oldEncryptedKey), loadDatabaseKey(keyFile));
+	}
+
+	@After
+	public void tearDown() {
+		deleteTestDirectory(testDir);
 	}
 }

briar-android/src/test/java/org/briarproject/briar/android/login/SetupControllerImplTest.java

@@ -9,7 +9,6 @@ import org.briarproject.bramble.api.crypto.SecretKey;
 import org.briarproject.bramble.api.db.DatabaseConfig;
 import org.briarproject.bramble.test.BrambleMockTestCase;
 import org.briarproject.bramble.test.ImmediateExecutor;
-import org.briarproject.bramble.test.TestUtils;
 import org.jmock.Expectations;
 import org.jmock.lib.legacy.ClassImposteriser;
 import org.junit.After;
@@ -19,10 +18,17 @@ import java.io.File;
 import java.util.concurrent.Executor;
 import java.util.concurrent.atomic.AtomicBoolean;
 
+import static junit.framework.Assert.assertEquals;
+import static junit.framework.Assert.assertFalse;
 import static junit.framework.Assert.assertTrue;
 import static org.briarproject.bramble.api.identity.AuthorConstants.MAX_AUTHOR_NAME_LENGTH;
+import static org.briarproject.bramble.test.TestUtils.deleteTestDirectory;
+import static org.briarproject.bramble.test.TestUtils.getRandomBytes;
 import static org.briarproject.bramble.test.TestUtils.getSecretKey;
+import static org.briarproject.bramble.test.TestUtils.getTestDirectory;
 import static org.briarproject.bramble.util.StringUtils.getRandomString;
+import static org.briarproject.bramble.util.StringUtils.toHexString;
+import static org.briarproject.briar.android.TestDatabaseKeyUtils.loadDatabaseKey;
 
 public class SetupControllerImplTest extends BrambleMockTestCase {
 
@@ -33,18 +39,18 @@ public class SetupControllerImplTest extends BrambleMockTestCase {
 	private final CryptoComponent crypto = context.mock(CryptoComponent.class);
 	private final PasswordStrengthEstimator estimator =
 			context.mock(PasswordStrengthEstimator.class);
-	private final SharedPreferences.Editor editor =
-			context.mock(SharedPreferences.Editor.class);
 	private final SetupActivity setupActivity;
 
 	private final Executor cryptoExecutor = new ImmediateExecutor();
 
 	private final String authorName = getRandomString(MAX_AUTHOR_NAME_LENGTH);
 	private final String password = "some.strong.pass";
-	private final String encryptedHex = "010203";
+	private final byte[] encryptedKey = getRandomBytes(123);
-	private final byte[] encryptedBytes = new byte[] {1, 2, 3};
 	private final SecretKey key = getSecretKey();
-	private final File testDir = TestUtils.getTestDirectory();
+	private final File testDir = getTestDirectory();
+	private final File keyDir = new File(testDir, "key");
+	private final File keyFile = new File(keyDir, "db.key");
+	private final File keyBackupFile = new File(keyDir, "db.key.bak");
 
 	public SetupControllerImplTest() {
 		context.setImposteriser(ClassImposteriser.INSTANCE);
@@ -53,7 +59,7 @@ public class SetupControllerImplTest extends BrambleMockTestCase {
 
 	@Test
 	@SuppressWarnings("ResultOfMethodCallIgnored")
-	public void testCreateAccount() {
+	public void testCreateAccount() throws Exception {
 		context.checking(new Expectations() {{
 			// Allow the contents of the data directory to be logged
 			allowing(setupActivity).getApplicationInfo();
@@ -76,15 +82,15 @@ public class SetupControllerImplTest extends BrambleMockTestCase {
 			oneOf(databaseConfig).setEncryptionKey(key);
 			// Encrypt the key with the password
 			oneOf(crypto).encryptWithPassword(key.getBytes(), password);
-			will(returnValue(encryptedBytes));
+			will(returnValue(encryptedKey));
 			// Store the encrypted key
-			oneOf(briarPrefs).edit();
+			allowing(databaseConfig).getDatabaseKeyDirectory();
-			will(returnValue(editor));
+			will(returnValue(keyDir));
-			oneOf(editor).putString("key", encryptedHex);
-			will(returnValue(editor));
-			oneOf(editor).commit();
 		}});
 
+		assertFalse(keyFile.exists());
+		assertFalse(keyBackupFile.exists());
+
 		SetupControllerImpl s = new SetupControllerImpl(briarPrefs,
 				databaseConfig, cryptoExecutor, crypto, estimator);
 		s.setSetupActivity(setupActivity);
@@ -94,10 +100,14 @@ public class SetupControllerImplTest extends BrambleMockTestCase {
 		s.setPassword(password);
 		s.createAccount(result -> called.set(true));
 		assertTrue(called.get());
+
+		assertTrue(keyFile.exists());
+		assertFalse(keyBackupFile.exists());
+		assertEquals(toHexString(encryptedKey), loadDatabaseKey(keyFile));
 	}
 
 	@After
 	public void tearDown() {
-		TestUtils.deleteTestDirectory(testDir);
+		deleteTestDirectory(testDir);
 	}
 }