diff --git a/bramble-android/src/main/java/org/briarproject/bramble/account/AndroidAccountManager.java b/bramble-android/src/main/java/org/briarproject/bramble/account/AndroidAccountManager.java index 8a4648bfd..012f76c4b 100644 --- a/bramble-android/src/main/java/org/briarproject/bramble/account/AndroidAccountManager.java +++ b/bramble-android/src/main/java/org/briarproject/bramble/account/AndroidAccountManager.java @@ -16,6 +16,7 @@ import java.util.Set; import java.util.logging.Logger; import javax.annotation.Nullable; +import javax.annotation.concurrent.GuardedBy; import javax.inject.Inject; import static android.os.Build.VERSION.SDK_INT; @@ -29,8 +30,6 @@ class AndroidAccountManager extends AccountManagerImpl private static final Logger LOG = Logger.getLogger(AndroidAccountManager.class.getName()); - private static final String PREF_DB_KEY = "key"; - protected final Context appContext; private final SharedPreferences prefs; @@ -53,36 +52,6 @@ class AndroidAccountManager extends AccountManagerImpl return exists; } - // Locking: stateChangeLock - @Override - @Nullable - protected String loadEncryptedDatabaseKey() { - String key = getDatabaseKeyFromPreferences(); - if (key == null) key = super.loadEncryptedDatabaseKey(); - else migrateDatabaseKeyToFile(key); - return key; - } - - // Locking: stateChangeLock - @Nullable - private String getDatabaseKeyFromPreferences() { - String key = prefs.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; - } - - // Locking: stateChangeLock - private void migrateDatabaseKeyToFile(String key) { - if (storeEncryptedDatabaseKey(key)) { - if (prefs.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 public void deleteAccount() { synchronized (stateChangeLock) { @@ -105,7 +74,7 @@ class AndroidAccountManager extends AccountManagerImpl return PreferenceManager.getDefaultSharedPreferences(appContext); } - // Locking: stateChangeLock + @GuardedBy("stateChangeLock") private void deleteAppData(SharedPreferences... clear) { // Clear and commit shared preferences for (SharedPreferences prefs : clear) { diff --git a/bramble-android/src/test/java/org/briarproject/bramble/account/AndroidAccountManagerTest.java b/bramble-android/src/test/java/org/briarproject/bramble/account/AndroidAccountManagerTest.java index 89065b89e..55d7b08ff 100644 --- a/bramble-android/src/test/java/org/briarproject/bramble/account/AndroidAccountManagerTest.java +++ b/bramble-android/src/test/java/org/briarproject/bramble/account/AndroidAccountManagerTest.java @@ -16,13 +16,10 @@ import org.junit.Test; import java.io.File; -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.getTestDirectory; -import static org.briarproject.bramble.util.StringUtils.toHexString; public class AndroidAccountManagerTest extends BrambleMockTestCase { @@ -40,11 +37,8 @@ public class AndroidAccountManagerTest extends BrambleMockTestCase { private final Application app; private final ApplicationInfo applicationInfo; - private final String encryptedKeyHex = toHexString(getRandomBytes(123)); 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"); private final File dbDir = new File(testDir, "db"); private AndroidAccountManager accountManager; @@ -75,29 +69,6 @@ public class AndroidAccountManagerTest extends BrambleMockTestCase { }; } - @Test - public void testDbKeyIsMigratedFromPreferencesToFile() { - context.checking(new Expectations() {{ - oneOf(prefs).getString("key", null); - will(returnValue(encryptedKeyHex)); - oneOf(prefs).edit(); - will(returnValue(editor)); - oneOf(editor).remove("key"); - will(returnValue(editor)); - oneOf(editor).commit(); - will(returnValue(true)); - }}); - - assertFalse(keyFile.exists()); - assertFalse(keyBackupFile.exists()); - - assertEquals(encryptedKeyHex, - accountManager.loadEncryptedDatabaseKey()); - - assertTrue(keyFile.exists()); - assertTrue(keyBackupFile.exists()); - } - @Test public void testDeleteAccountClearsSharedPrefsAndDeletesFiles() throws Exception { diff --git a/bramble-api/src/main/java/org/briarproject/bramble/api/crypto/CryptoComponent.java b/bramble-api/src/main/java/org/briarproject/bramble/api/crypto/CryptoComponent.java index 9d498ec9d..d1cbc63da 100644 --- a/bramble-api/src/main/java/org/briarproject/bramble/api/crypto/CryptoComponent.java +++ b/bramble-api/src/main/java/org/briarproject/bramble/api/crypto/CryptoComponent.java @@ -132,17 +132,32 @@ public interface CryptoComponent { * storage. The encryption and authentication keys are derived from the * given password. The ciphertext will be decryptable using the same * password after the app restarts. + * + * @param keyStrengthener Used to strengthen the password-based key. If + * null, the password-based key will not be strengthened */ - byte[] encryptWithPassword(byte[] plaintext, String password); + byte[] encryptWithPassword(byte[] plaintext, String password, + @Nullable KeyStrengthener keyStrengthener); /** * Decrypts and authenticates the given ciphertext that has been read from * storage. The encryption and authentication keys are derived from the * given password. Returns null if the ciphertext cannot be decrypted and * authenticated (for example, if the password is wrong). + * + * @param keyStrengthener Used to strengthen the password-based key. If + * null, or if strengthening was not used when encrypting the ciphertext, + * the password-based key will not be strengthened */ @Nullable - byte[] decryptWithPassword(byte[] ciphertext, String password); + byte[] decryptWithPassword(byte[] ciphertext, String password, + @Nullable KeyStrengthener keyStrengthener); + + /** + * Returns true if the given ciphertext was encrypted using a strengthened + * key. The validity of the ciphertext is not checked. + */ + boolean isEncryptedWithStrengthenedKey(byte[] ciphertext); /** * Encrypts the given plaintext to the given public key. diff --git a/bramble-api/src/main/java/org/briarproject/bramble/api/crypto/KeyStrengthener.java b/bramble-api/src/main/java/org/briarproject/bramble/api/crypto/KeyStrengthener.java new file mode 100644 index 000000000..5913aae5d --- /dev/null +++ b/bramble-api/src/main/java/org/briarproject/bramble/api/crypto/KeyStrengthener.java @@ -0,0 +1,23 @@ +package org.briarproject.bramble.api.crypto; + +import org.briarproject.bramble.api.nullsafety.NotNullByDefault; + +/** + * Interface for strengthening a password-based key, for example by using a + * key stored in a key management service or hardware security module. + */ +@NotNullByDefault +public interface KeyStrengthener { + + /** + * Returns true if the strengthener has been initialised. + */ + @SuppressWarnings("BooleanMethodIsAlwaysInverted") + boolean isInitialised(); + + /** + * Initialises the strengthener if necessary and returns a strong key + * derived from the given key. + */ + SecretKey strengthenKey(SecretKey k); +} diff --git a/bramble-api/src/main/java/org/briarproject/bramble/api/db/DatabaseConfig.java b/bramble-api/src/main/java/org/briarproject/bramble/api/db/DatabaseConfig.java index a01d12da5..16d587c1d 100644 --- a/bramble-api/src/main/java/org/briarproject/bramble/api/db/DatabaseConfig.java +++ b/bramble-api/src/main/java/org/briarproject/bramble/api/db/DatabaseConfig.java @@ -1,13 +1,29 @@ package org.briarproject.bramble.api.db; +import org.briarproject.bramble.api.crypto.KeyStrengthener; import org.briarproject.bramble.api.nullsafety.NotNullByDefault; import java.io.File; +import javax.annotation.Nullable; + @NotNullByDefault public interface DatabaseConfig { + /** + * Returns the directory where the database stores its data. + */ File getDatabaseDirectory(); + /** + * Returns the directory where the encrypted database key is stored. + */ File getDatabaseKeyDirectory(); + + /** + * Returns a {@link KeyStrengthener} for strengthening the encryption of + * the database key, or null if no strengthener should be used. + */ + @Nullable + KeyStrengthener getKeyStrengthener(); } diff --git a/bramble-core/src/main/java/org/briarproject/bramble/account/AccountManagerImpl.java b/bramble-core/src/main/java/org/briarproject/bramble/account/AccountManagerImpl.java index c4b75b6c3..c89353fa9 100644 --- a/bramble-core/src/main/java/org/briarproject/bramble/account/AccountManagerImpl.java +++ b/bramble-core/src/main/java/org/briarproject/bramble/account/AccountManagerImpl.java @@ -2,6 +2,7 @@ package org.briarproject.bramble.account; import org.briarproject.bramble.api.account.AccountManager; import org.briarproject.bramble.api.crypto.CryptoComponent; +import org.briarproject.bramble.api.crypto.KeyStrengthener; import org.briarproject.bramble.api.crypto.SecretKey; import org.briarproject.bramble.api.db.DatabaseConfig; import org.briarproject.bramble.api.identity.Identity; @@ -19,6 +20,7 @@ import java.io.InputStreamReader; import java.util.logging.Logger; import javax.annotation.Nullable; +import javax.annotation.concurrent.GuardedBy; import javax.inject.Inject; import static java.util.logging.Level.WARNING; @@ -68,9 +70,10 @@ class AccountManagerImpl implements AccountManager { return databaseKey; } - // Locking: stateChangeLock + // Package access for testing + @GuardedBy("stateChangeLock") @Nullable - protected String loadEncryptedDatabaseKey() { + String loadEncryptedDatabaseKey() { String key = readDbKeyFromFile(dbKeyFile); if (key == null) { LOG.info("No database key in primary file"); @@ -83,7 +86,7 @@ class AccountManagerImpl implements AccountManager { return key; } - // Locking: stateChangeLock + @GuardedBy("stateChangeLock") @Nullable private String readDbKeyFromFile(File f) { if (!f.exists()) { @@ -102,8 +105,9 @@ class AccountManagerImpl implements AccountManager { } } - // Locking: stateChangeLock - protected boolean storeEncryptedDatabaseKey(String hex) { + // Package access for testing + @GuardedBy("stateChangeLock") + boolean storeEncryptedDatabaseKey(String hex) { LOG.info("Storing database key in file"); // Create the directory if necessary if (databaseConfig.getDatabaseKeyDirectory().mkdirs()) @@ -140,7 +144,7 @@ class AccountManagerImpl implements AccountManager { } } - // Locking: stateChangeLock + @GuardedBy("stateChangeLock") private void writeDbKeyToFile(String key, File f) throws IOException { FileOutputStream out = new FileOutputStream(f); out.write(key.getBytes("UTF-8")); @@ -170,10 +174,11 @@ class AccountManagerImpl implements AccountManager { } } - // Locking: stateChangeLock + @GuardedBy("stateChangeLock") private boolean encryptAndStoreDatabaseKey(SecretKey key, String password) { byte[] plaintext = key.getBytes(); - byte[] ciphertext = crypto.encryptWithPassword(plaintext, password); + byte[] ciphertext = crypto.encryptWithPassword(plaintext, password, + databaseConfig.getKeyStrengthener()); return storeEncryptedDatabaseKey(toHexString(ciphertext)); } @@ -197,7 +202,7 @@ class AccountManagerImpl implements AccountManager { } } - // Locking: stateChangeLock + @GuardedBy("stateChangeLock") @Nullable private SecretKey loadAndDecryptDatabaseKey(String password) { String hex = loadEncryptedDatabaseKey(); @@ -206,12 +211,22 @@ class AccountManagerImpl implements AccountManager { return null; } byte[] ciphertext = fromHexString(hex); - byte[] plaintext = crypto.decryptWithPassword(ciphertext, password); + KeyStrengthener keyStrengthener = databaseConfig.getKeyStrengthener(); + byte[] plaintext = crypto.decryptWithPassword(ciphertext, password, + keyStrengthener); if (plaintext == null) { LOG.info("Failed to decrypt database key"); return null; } - return new SecretKey(plaintext); + SecretKey key = new SecretKey(plaintext); + // If the DB key was encrypted with a weak key and a key strengthener + // is now available, re-encrypt the DB key with a strengthened key + if (keyStrengthener != null && + !crypto.isEncryptedWithStrengthenedKey(ciphertext)) { + LOG.info("Re-encrypting database key with strengthened key"); + encryptAndStoreDatabaseKey(key, password); + } + return key; } @Override diff --git a/bramble-core/src/main/java/org/briarproject/bramble/crypto/CryptoComponentImpl.java b/bramble-core/src/main/java/org/briarproject/bramble/crypto/CryptoComponentImpl.java index 37aa5de3d..6763193ac 100644 --- a/bramble-core/src/main/java/org/briarproject/bramble/crypto/CryptoComponentImpl.java +++ b/bramble-core/src/main/java/org/briarproject/bramble/crypto/CryptoComponentImpl.java @@ -9,6 +9,7 @@ import org.briarproject.bramble.api.crypto.AgreementPublicKey; import org.briarproject.bramble.api.crypto.CryptoComponent; import org.briarproject.bramble.api.crypto.KeyPair; import org.briarproject.bramble.api.crypto.KeyParser; +import org.briarproject.bramble.api.crypto.KeyStrengthener; import org.briarproject.bramble.api.crypto.PrivateKey; import org.briarproject.bramble.api.crypto.PublicKey; import org.briarproject.bramble.api.crypto.SecretKey; @@ -51,7 +52,8 @@ class CryptoComponentImpl implements CryptoComponent { private static final int SIGNATURE_KEY_PAIR_BITS = 256; private static final int STORAGE_IV_BYTES = 24; // 196 bits private static final int PBKDF_SALT_BYTES = 32; // 256 bits - private static final int PBKDF_FORMAT_SCRYPT = 0; + private static final byte PBKDF_FORMAT_SCRYPT = 0; + private static final byte PBKDF_FORMAT_SCRYPT_STRENGTHENED = 1; private final SecureRandom secureRandom; private final PasswordBasedKdf passwordBasedKdf; @@ -311,7 +313,8 @@ class CryptoComponentImpl implements CryptoComponent { } @Override - public byte[] encryptWithPassword(byte[] input, String password) { + public byte[] encryptWithPassword(byte[] input, String password, + @Nullable KeyStrengthener keyStrengthener) { AuthenticatedCipher cipher = new XSalsa20Poly1305AuthenticatedCipher(); int macBytes = cipher.getMacBytes(); // Generate a random salt @@ -319,8 +322,9 @@ class CryptoComponentImpl implements CryptoComponent { secureRandom.nextBytes(salt); // Calibrate the KDF int cost = passwordBasedKdf.chooseCostParameter(); - // Derive the key from the password + // Derive the encryption key from the password SecretKey key = passwordBasedKdf.deriveKey(password, salt, cost); + if (keyStrengthener != null) key = keyStrengthener.strengthenKey(key); // Generate a random IV byte[] iv = new byte[STORAGE_IV_BYTES]; secureRandom.nextBytes(iv); @@ -331,7 +335,9 @@ class CryptoComponentImpl implements CryptoComponent { byte[] output = new byte[outputLen]; int outputOff = 0; // Format version - output[outputOff] = PBKDF_FORMAT_SCRYPT; + byte formatVersion = keyStrengthener == null + ? PBKDF_FORMAT_SCRYPT : PBKDF_FORMAT_SCRYPT_STRENGTHENED; + output[outputOff] = formatVersion; outputOff++; // Salt arraycopy(salt, 0, output, outputOff, salt.length); @@ -354,7 +360,8 @@ class CryptoComponentImpl implements CryptoComponent { @Override @Nullable - public byte[] decryptWithPassword(byte[] input, String password) { + public byte[] decryptWithPassword(byte[] input, String password, + @Nullable KeyStrengthener keyStrengthener) { AuthenticatedCipher cipher = new XSalsa20Poly1305AuthenticatedCipher(); int macBytes = cipher.getMacBytes(); // The input contains the format version, salt, cost parameter, IV, @@ -366,8 +373,11 @@ class CryptoComponentImpl implements CryptoComponent { // Format version byte formatVersion = input[inputOff]; inputOff++; - if (formatVersion != PBKDF_FORMAT_SCRYPT) - return null; // Unknown format + // Check whether we support this format version + if (formatVersion != PBKDF_FORMAT_SCRYPT && + formatVersion != PBKDF_FORMAT_SCRYPT_STRENGTHENED) { + return null; + } // Salt byte[] salt = new byte[PBKDF_SALT_BYTES]; arraycopy(input, inputOff, salt, 0, salt.length); @@ -381,8 +391,13 @@ class CryptoComponentImpl implements CryptoComponent { byte[] iv = new byte[STORAGE_IV_BYTES]; arraycopy(input, inputOff, iv, 0, iv.length); inputOff += iv.length; - // Derive the key from the password + // Derive the decryption key from the password SecretKey key = passwordBasedKdf.deriveKey(password, salt, (int) cost); + if (formatVersion == PBKDF_FORMAT_SCRYPT_STRENGTHENED) { + if (keyStrengthener == null || !keyStrengthener.isInitialised()) + return null; // Can't derive the same strengthened key + key = keyStrengthener.strengthenKey(key); + } // Initialise the cipher try { cipher.init(false, key, iv); @@ -400,6 +415,12 @@ class CryptoComponentImpl implements CryptoComponent { } } + @Override + public boolean isEncryptedWithStrengthenedKey(byte[] ciphertext) { + return ciphertext.length > 0 && + ciphertext[0] == PBKDF_FORMAT_SCRYPT_STRENGTHENED; + } + @Override public byte[] encryptToKey(PublicKey publicKey, byte[] plaintext) { try { diff --git a/bramble-core/src/test/java/org/briarproject/bramble/account/AccountManagerImplTest.java b/bramble-core/src/test/java/org/briarproject/bramble/account/AccountManagerImplTest.java index e1c1c1bb1..4b6b63e7e 100644 --- a/bramble-core/src/test/java/org/briarproject/bramble/account/AccountManagerImplTest.java +++ b/bramble-core/src/test/java/org/briarproject/bramble/account/AccountManagerImplTest.java @@ -1,6 +1,7 @@ package org.briarproject.bramble.account; import org.briarproject.bramble.api.crypto.CryptoComponent; +import org.briarproject.bramble.api.crypto.KeyStrengthener; import org.briarproject.bramble.api.crypto.SecretKey; import org.briarproject.bramble.api.db.DatabaseConfig; import org.briarproject.bramble.api.identity.Identity; @@ -39,6 +40,8 @@ public class AccountManagerImplTest extends BrambleMockTestCase { private final DatabaseConfig databaseConfig = context.mock(DatabaseConfig.class); + private final KeyStrengthener keyStrengthener = + context.mock(KeyStrengthener.class); private final CryptoComponent crypto = context.mock(CryptoComponent.class); private final IdentityManager identityManager = context.mock(IdentityManager.class); @@ -68,6 +71,8 @@ public class AccountManagerImplTest extends BrambleMockTestCase { will(returnValue(dbDir)); allowing(databaseConfig).getDatabaseKeyDirectory(); will(returnValue(keyDir)); + allowing(databaseConfig).getKeyStrengthener(); + will(returnValue(keyStrengthener)); }}); accountManager = @@ -89,7 +94,8 @@ public class AccountManagerImplTest extends BrambleMockTestCase { @Test public void testSignInReturnsFalseIfPasswordIsWrong() throws Exception { context.checking(new Expectations() {{ - oneOf(crypto).decryptWithPassword(encryptedKey, password); + oneOf(crypto).decryptWithPassword(encryptedKey, password, + keyStrengthener); will(returnValue(null)); }}); @@ -109,8 +115,11 @@ public class AccountManagerImplTest extends BrambleMockTestCase { @Test public void testSignInReturnsTrueIfPasswordIsRight() throws Exception { context.checking(new Expectations() {{ - oneOf(crypto).decryptWithPassword(encryptedKey, password); + oneOf(crypto).decryptWithPassword(encryptedKey, password, + keyStrengthener); will(returnValue(key.getBytes())); + oneOf(crypto).isEncryptedWithStrengthenedKey(encryptedKey); + will(returnValue(true)); }}); storeDatabaseKey(keyFile, encryptedKeyHex); @@ -129,6 +138,35 @@ public class AccountManagerImplTest extends BrambleMockTestCase { assertEquals(encryptedKeyHex, loadDatabaseKey(keyBackupFile)); } + @Test + public void testSignInReEncryptsKey() throws Exception { + context.checking(new Expectations() {{ + oneOf(crypto).decryptWithPassword(encryptedKey, password, + keyStrengthener); + will(returnValue(key.getBytes())); + oneOf(crypto).isEncryptedWithStrengthenedKey(encryptedKey); + will(returnValue(false)); + oneOf(crypto).encryptWithPassword(key.getBytes(), password, + keyStrengthener); + will(returnValue(newEncryptedKey)); + }}); + + storeDatabaseKey(keyFile, encryptedKeyHex); + storeDatabaseKey(keyBackupFile, encryptedKeyHex); + + assertEquals(encryptedKeyHex, loadDatabaseKey(keyFile)); + assertEquals(encryptedKeyHex, loadDatabaseKey(keyBackupFile)); + + assertTrue(accountManager.signIn(password)); + assertTrue(accountManager.hasDatabaseKey()); + SecretKey decrypted = accountManager.getDatabaseKey(); + assertNotNull(decrypted); + assertArrayEquals(key.getBytes(), decrypted.getBytes()); + + assertEquals(newEncryptedKeyHex, loadDatabaseKey(keyFile)); + assertEquals(newEncryptedKeyHex, loadDatabaseKey(keyBackupFile)); + } + @Test public void testDbKeyIsLoadedFromPrimaryFile() throws Exception { storeDatabaseKey(keyFile, encryptedKeyHex); @@ -258,7 +296,8 @@ public class AccountManagerImplTest extends BrambleMockTestCase { oneOf(identityManager).registerIdentity(identity); oneOf(crypto).generateSecretKey(); will(returnValue(key)); - oneOf(crypto).encryptWithPassword(key.getBytes(), password); + oneOf(crypto).encryptWithPassword(key.getBytes(), password, + keyStrengthener); will(returnValue(encryptedKey)); }}); @@ -287,7 +326,8 @@ public class AccountManagerImplTest extends BrambleMockTestCase { public void testChangePasswordReturnsFalseIfPasswordIsWrong() throws Exception { context.checking(new Expectations() {{ - oneOf(crypto).decryptWithPassword(encryptedKey, password); + oneOf(crypto).decryptWithPassword(encryptedKey, password, + keyStrengthener); will(returnValue(null)); }}); @@ -304,9 +344,13 @@ public class AccountManagerImplTest extends BrambleMockTestCase { public void testChangePasswordReturnsTrueIfPasswordIsRight() throws Exception { context.checking(new Expectations() {{ - oneOf(crypto).decryptWithPassword(encryptedKey, password); + oneOf(crypto).decryptWithPassword(encryptedKey, password, + keyStrengthener); will(returnValue(key.getBytes())); - oneOf(crypto).encryptWithPassword(key.getBytes(), newPassword); + oneOf(crypto).isEncryptedWithStrengthenedKey(encryptedKey); + will(returnValue(true)); + oneOf(crypto).encryptWithPassword(key.getBytes(), newPassword, + keyStrengthener); will(returnValue(newEncryptedKey)); }}); diff --git a/bramble-core/src/test/java/org/briarproject/bramble/crypto/PasswordBasedEncryptionTest.java b/bramble-core/src/test/java/org/briarproject/bramble/crypto/PasswordBasedEncryptionTest.java index 5efc3593e..e2380f655 100644 --- a/bramble-core/src/test/java/org/briarproject/bramble/crypto/PasswordBasedEncryptionTest.java +++ b/bramble-core/src/test/java/org/briarproject/bramble/crypto/PasswordBasedEncryptionTest.java @@ -21,8 +21,8 @@ public class PasswordBasedEncryptionTest extends BrambleTestCase { public void testEncryptionAndDecryption() { byte[] input = TestUtils.getRandomBytes(1234); String password = "password"; - byte[] ciphertext = crypto.encryptWithPassword(input, password); - byte[] output = crypto.decryptWithPassword(ciphertext, password); + byte[] ciphertext = crypto.encryptWithPassword(input, password, null); + byte[] output = crypto.decryptWithPassword(ciphertext, password, null); assertArrayEquals(input, output); } @@ -30,11 +30,11 @@ public class PasswordBasedEncryptionTest extends BrambleTestCase { public void testInvalidCiphertextReturnsNull() { byte[] input = TestUtils.getRandomBytes(1234); String password = "password"; - byte[] ciphertext = crypto.encryptWithPassword(input, password); + byte[] ciphertext = crypto.encryptWithPassword(input, password, null); // Modify the ciphertext int position = new Random().nextInt(ciphertext.length); ciphertext[position] = (byte) (ciphertext[position] ^ 0xFF); - byte[] output = crypto.decryptWithPassword(ciphertext, password); + byte[] output = crypto.decryptWithPassword(ciphertext, password, null); assertNull(output); } } diff --git a/bramble-core/src/test/java/org/briarproject/bramble/test/TestDatabaseConfig.java b/bramble-core/src/test/java/org/briarproject/bramble/test/TestDatabaseConfig.java index adf0712b1..ed4afd622 100644 --- a/bramble-core/src/test/java/org/briarproject/bramble/test/TestDatabaseConfig.java +++ b/bramble-core/src/test/java/org/briarproject/bramble/test/TestDatabaseConfig.java @@ -1,10 +1,13 @@ package org.briarproject.bramble.test; +import org.briarproject.bramble.api.crypto.KeyStrengthener; import org.briarproject.bramble.api.db.DatabaseConfig; import org.briarproject.bramble.api.nullsafety.NotNullByDefault; import java.io.File; +import javax.annotation.Nullable; + @NotNullByDefault public class TestDatabaseConfig implements DatabaseConfig { @@ -24,4 +27,10 @@ public class TestDatabaseConfig implements DatabaseConfig { public File getDatabaseKeyDirectory() { return keyDir; } + + @Nullable + @Override + public KeyStrengthener getKeyStrengthener() { + return null; + } } diff --git a/briar-android/src/main/java/org/briarproject/briar/android/AndroidDatabaseConfig.java b/briar-android/src/main/java/org/briarproject/briar/android/AndroidDatabaseConfig.java index f1fdc2492..e13b1463a 100644 --- a/briar-android/src/main/java/org/briarproject/briar/android/AndroidDatabaseConfig.java +++ b/briar-android/src/main/java/org/briarproject/briar/android/AndroidDatabaseConfig.java @@ -1,18 +1,25 @@ package org.briarproject.briar.android; +import org.briarproject.bramble.api.crypto.KeyStrengthener; import org.briarproject.bramble.api.db.DatabaseConfig; import org.briarproject.bramble.api.nullsafety.NotNullByDefault; import java.io.File; +import javax.annotation.Nullable; + @NotNullByDefault class AndroidDatabaseConfig implements DatabaseConfig { private final File dbDir, keyDir; + @Nullable + private final KeyStrengthener keyStrengthener; - AndroidDatabaseConfig(File dbDir, File keyDir) { + AndroidDatabaseConfig(File dbDir, File keyDir, + @Nullable KeyStrengthener keyStrengthener) { this.dbDir = dbDir; this.keyDir = keyDir; + this.keyStrengthener = keyStrengthener; } @Override @@ -24,4 +31,10 @@ class AndroidDatabaseConfig implements DatabaseConfig { public File getDatabaseKeyDirectory() { return keyDir; } + + @Nullable + @Override + public KeyStrengthener getKeyStrengthener() { + return keyStrengthener; + } } diff --git a/briar-android/src/main/java/org/briarproject/briar/android/AndroidKeyStrengthener.java b/briar-android/src/main/java/org/briarproject/briar/android/AndroidKeyStrengthener.java new file mode 100644 index 000000000..db44062f9 --- /dev/null +++ b/briar-android/src/main/java/org/briarproject/briar/android/AndroidKeyStrengthener.java @@ -0,0 +1,118 @@ +package org.briarproject.briar.android; + +import android.security.keystore.KeyGenParameterSpec; + +import org.briarproject.bramble.api.crypto.KeyStrengthener; +import org.briarproject.bramble.api.crypto.SecretKey; +import org.briarproject.bramble.api.nullsafety.NotNullByDefault; + +import java.io.IOException; +import java.security.GeneralSecurityException; +import java.security.KeyStore; +import java.security.KeyStore.Entry; +import java.security.KeyStore.SecretKeyEntry; +import java.security.spec.AlgorithmParameterSpec; +import java.util.List; +import java.util.logging.Logger; + +import javax.annotation.Nullable; +import javax.annotation.concurrent.GuardedBy; +import javax.crypto.KeyGenerator; +import javax.crypto.Mac; + +import androidx.annotation.RequiresApi; + +import static android.os.Build.VERSION.SDK_INT; +import static android.security.keystore.KeyProperties.KEY_ALGORITHM_HMAC_SHA256; +import static android.security.keystore.KeyProperties.PURPOSE_SIGN; +import static java.util.Arrays.asList; +import static java.util.Collections.singletonList; +import static java.util.logging.Level.INFO; +import static java.util.logging.Logger.getLogger; + +@RequiresApi(23) +@NotNullByDefault +class AndroidKeyStrengthener implements KeyStrengthener { + + private static final Logger LOG = + getLogger(AndroidKeyStrengthener.class.getName()); + + private static final String KEY_STORE_TYPE = "AndroidKeyStore"; + private static final String PROVIDER_NAME = "AndroidKeyStore"; + private static final String KEY_ALIAS = "db"; + private static final int KEY_BITS = 256; + + private final List specs; + + AndroidKeyStrengthener() { + KeyGenParameterSpec noStrongBox = + new KeyGenParameterSpec.Builder(KEY_ALIAS, PURPOSE_SIGN) + .setKeySize(KEY_BITS) + .build(); + if (SDK_INT >= 28) { + // Prefer StrongBox if available + KeyGenParameterSpec strongBox = + new KeyGenParameterSpec.Builder(KEY_ALIAS, PURPOSE_SIGN) + .setIsStrongBoxBacked(true) + .setKeySize(KEY_BITS) + .build(); + specs = asList(strongBox, noStrongBox); + } else { + specs = singletonList(noStrongBox); + } + } + + @GuardedBy("this") + @Nullable + private javax.crypto.SecretKey storedKey = null; + + @Override + public synchronized boolean isInitialised() { + if (storedKey != null) return true; + try { + KeyStore ks = KeyStore.getInstance(KEY_STORE_TYPE); + ks.load(null); + Entry entry = ks.getEntry(KEY_ALIAS, null); + if (entry instanceof SecretKeyEntry) { + storedKey = ((SecretKeyEntry) entry).getSecretKey(); + LOG.info("Loaded key from keystore"); + return true; + } + return false; + } catch (GeneralSecurityException | IOException e) { + throw new RuntimeException(e); + } + } + + @Override + public synchronized SecretKey strengthenKey(SecretKey k) { + try { + if (!isInitialised()) initialise(); + // Use the input key and the stored key to derive the output key + Mac mac = Mac.getInstance(KEY_ALGORITHM_HMAC_SHA256); + mac.init(storedKey); + return new SecretKey(mac.doFinal(k.getBytes())); + } catch (GeneralSecurityException e) { + throw new RuntimeException(e); + } + } + + private synchronized void initialise() throws GeneralSecurityException { + // Try the parameter specs in order of preference + for (AlgorithmParameterSpec spec : specs) { + try { + KeyGenerator kg = KeyGenerator.getInstance( + KEY_ALGORITHM_HMAC_SHA256, PROVIDER_NAME); + kg.init(spec); + storedKey = kg.generateKey(); + LOG.info("Stored key in keystore"); + return; + } catch (Exception e) { + if (LOG.isLoggable(INFO)) + LOG.info("Could not generate key: " + e); + // Fall back to next spec + } + } + throw new GeneralSecurityException("Could not generate key"); + } +} diff --git a/briar-android/src/main/java/org/briarproject/briar/android/AppModule.java b/briar-android/src/main/java/org/briarproject/briar/android/AppModule.java index 5da45a9c5..41c728e5e 100644 --- a/briar-android/src/main/java/org/briarproject/briar/android/AppModule.java +++ b/briar-android/src/main/java/org/briarproject/briar/android/AppModule.java @@ -10,6 +10,7 @@ import com.vanniktech.emoji.RecentEmoji; import org.briarproject.bramble.api.FeatureFlags; import org.briarproject.bramble.api.battery.BatteryManager; import org.briarproject.bramble.api.crypto.CryptoComponent; +import org.briarproject.bramble.api.crypto.KeyStrengthener; import org.briarproject.bramble.api.crypto.PublicKey; import org.briarproject.bramble.api.db.DatabaseConfig; import org.briarproject.bramble.api.event.EventBus; @@ -56,6 +57,7 @@ import dagger.Module; import dagger.Provides; import static android.content.Context.MODE_PRIVATE; +import static android.os.Build.VERSION.SDK_INT; import static java.util.Arrays.asList; import static java.util.Collections.emptyList; import static org.briarproject.bramble.api.reporting.ReportingConstants.DEV_ONION_ADDRESS; @@ -101,7 +103,9 @@ public class AppModule { File dbDir = app.getApplicationContext().getDir("db", MODE_PRIVATE); File keyDir = app.getApplicationContext().getDir("key", MODE_PRIVATE); StrictMode.setThreadPolicy(tp); - return new AndroidDatabaseConfig(dbDir, keyDir); + KeyStrengthener keyStrengthener = SDK_INT >= 23 + ? new AndroidKeyStrengthener() : null; + return new AndroidDatabaseConfig(dbDir, keyDir, keyStrengthener); } @Provides diff --git a/briar-headless/src/main/java/org/briarproject/briar/headless/HeadlessDatabaseConfig.kt b/briar-headless/src/main/java/org/briarproject/briar/headless/HeadlessDatabaseConfig.kt index d7082e998..620bdc826 100644 --- a/briar-headless/src/main/java/org/briarproject/briar/headless/HeadlessDatabaseConfig.kt +++ b/briar-headless/src/main/java/org/briarproject/briar/headless/HeadlessDatabaseConfig.kt @@ -1,16 +1,15 @@ package org.briarproject.briar.headless +import org.briarproject.bramble.api.crypto.KeyStrengthener import org.briarproject.bramble.api.db.DatabaseConfig import java.io.File internal class HeadlessDatabaseConfig(private val dbDir: File, private val keyDir: File) : DatabaseConfig { - override fun getDatabaseDirectory(): File { - return dbDir - } + override fun getDatabaseDirectory() = dbDir - override fun getDatabaseKeyDirectory(): File { - return keyDir - } + override fun getDatabaseKeyDirectory() = keyDir + + override fun getKeyStrengthener(): KeyStrengthener? = null }