From 358166bc129650e588ae5fa3b64787083c5cf1cf Mon Sep 17 00:00:00 2001 From: akwizgran <akwizgran@users.sourceforge.net> Date: Mon, 29 Dec 2014 21:08:27 +0000 Subject: [PATCH] Don't try to erase secrets from memory. 1. The things we're really trying to protect - contact identities, message contents, etc - can't be erased from memory because they're encapsulated inside objects we don't control. 2. Long-term secrets can't be protected by erasing them from memory because they're stored in the database and the database key has to be held in memory whenever the app's running. 3. If the runtime uses a compacting garbage collector then we have no way to ensure an object is erased from memory. 4. Trying to erase secrets from memory makes the code more complex. Conclusion: Let's not try to protect secrets from an attacker who can read arbitrary memory locations. --- .../android/PasswordActivity.java | 5 +- .../briarproject/android/SetupActivity.java | 35 +-- .../api/crypto/CryptoComponent.java | 4 +- .../briarproject/api/crypto/KeyManager.java | 5 +- .../api/crypto/PasswordStrengthEstimator.java | 2 +- .../briarproject/api/crypto/SecretKey.java | 22 +- .../crypto/AuthenticatedCipherImpl.java | 2 +- .../crypto/CryptoComponentImpl.java | 57 +---- .../crypto/PasswordStrengthEstimatorImpl.java | 5 +- .../briarproject/crypto/SecretKeyImpl.java | 30 --- .../crypto/StreamDecrypterImpl.java | 15 +- .../crypto/StreamEncrypterFactoryImpl.java | 1 - .../crypto/StreamEncrypterImpl.java | 21 +- .../src/org/briarproject/db/H2Database.java | 26 +-- .../plugins/ConnectionManagerImpl.java | 41 ++-- .../transport/KeyManagerImpl.java | 90 +++---- .../transport/TransportTagRecogniser.java | 7 +- .../src/org/briarproject/util/ByteUtils.java | 4 - briar-tests/build.xml | 3 +- .../briarproject/ProtocolIntegrationTest.java | 8 +- .../crypto/KeyDerivationTest.java | 9 +- .../crypto/PasswordBasedKdfTest.java | 4 +- ...=> PasswordStrengthEstimatorImplTest.java} | 15 +- .../crypto/SecretKeyImplTest.java | 27 --- .../SimplexMessagingIntegrationTest.java | 4 +- .../transport/KeyManagerImplTest.java | 100 ++++---- .../transport/KeyRotationIntegrationTest.java | 219 ++++-------------- .../transport/TransportTagRecogniserTest.java | 7 +- 28 files changed, 211 insertions(+), 557 deletions(-) delete mode 100644 briar-core/src/org/briarproject/crypto/SecretKeyImpl.java rename briar-tests/src/org/briarproject/crypto/{PasswordStrengthEstimatorTest.java => PasswordStrengthEstimatorImplTest.java} (50%) delete mode 100644 briar-tests/src/org/briarproject/crypto/SecretKeyImplTest.java diff --git a/briar-android/src/org/briarproject/android/PasswordActivity.java b/briar-android/src/org/briarproject/android/PasswordActivity.java index 26ac3dee09..727985f1b1 100644 --- a/briar-android/src/org/briarproject/android/PasswordActivity.java +++ b/briar-android/src/org/briarproject/android/PasswordActivity.java @@ -133,10 +133,7 @@ public class PasswordActivity extends RoboActivity { continueButton.setVisibility(GONE); progress.setVisibility(VISIBLE); // Decrypt the database key in a background thread - int length = e.length(); - final char[] password = new char[length]; - e.getChars(0, length, password, 0); - e.delete(0, length); + final String password = e.toString(); cryptoExecutor.execute(new Runnable() { public void run() { byte[] key = crypto.decryptWithPassword(encrypted, password); diff --git a/briar-android/src/org/briarproject/android/SetupActivity.java b/briar-android/src/org/briarproject/android/SetupActivity.java index a71442eeb0..109ce0cee8 100644 --- a/briar-android/src/org/briarproject/android/SetupActivity.java +++ b/briar-android/src/org/briarproject/android/SetupActivity.java @@ -19,7 +19,6 @@ import static org.briarproject.android.util.CommonLayoutParams.WRAP_WRAP; import static org.briarproject.api.AuthorConstants.MAX_AUTHOR_NAME_LENGTH; import static org.briarproject.api.crypto.PasswordStrengthEstimator.WEAK; -import java.util.Arrays; import java.util.concurrent.Executor; import java.util.logging.Logger; @@ -43,7 +42,6 @@ import android.content.Intent; import android.content.SharedPreferences; import android.content.SharedPreferences.Editor; import android.os.Bundle; -import android.text.Editable; import android.view.KeyEvent; import android.view.View; import android.view.View.OnClickListener; @@ -187,18 +185,16 @@ OnEditorActionListener { else strengthMeter.setVisibility(INVISIBLE); String nickname = nicknameEntry.getText().toString(); int nicknameLength = StringUtils.toUtf8(nickname).length; - char[] firstPassword = getChars(passwordEntry.getText()); - char[] secondPassword = getChars(passwordConfirmation.getText()); - boolean passwordsMatch = Arrays.equals(firstPassword, secondPassword); + String firstPassword = passwordEntry.getText().toString(); + String secondPassword = passwordConfirmation.getText().toString(); + boolean passwordsMatch = firstPassword.equals(secondPassword); float strength = strengthEstimator.estimateStrength(firstPassword); - for(int i = 0; i < firstPassword.length; i++) firstPassword[i] = 0; - for(int i = 0; i < secondPassword.length; i++) secondPassword[i] = 0; strengthMeter.setStrength(strength); if(nicknameLength > MAX_AUTHOR_NAME_LENGTH) { feedback.setText(R.string.name_too_long); - } else if(firstPassword.length == 0) { + } else if(firstPassword.length() == 0) { feedback.setText(""); - } else if(secondPassword.length == 0 || passwordsMatch) { + } else if(secondPassword.length() == 0 || passwordsMatch) { if(strength < PasswordStrengthEstimator.WEAK) feedback.setText(R.string.password_too_weak); else feedback.setText(""); @@ -212,13 +208,6 @@ OnEditorActionListener { && passwordsMatch && strength >= WEAK); } - private char[] getChars(Editable e) { - int length = e.length(); - char[] c = new char[length]; - e.getChars(0, length, c, 0); - return c; - } - public boolean onEditorAction(TextView v, int actionId, KeyEvent event) { // Hide the soft keyboard Object o = getSystemService(INPUT_METHOD_SERVICE); @@ -231,18 +220,14 @@ OnEditorActionListener { feedback.setVisibility(GONE); continueButton.setVisibility(GONE); progress.setVisibility(VISIBLE); - // Copy the passwords and erase the originals final String nickname = nicknameEntry.getText().toString(); - final char[] password = getChars(passwordEntry.getText()); - delete(passwordEntry.getText()); - delete(passwordConfirmation.getText()); + final String password = passwordEntry.getText().toString(); // Store the DB key and create the identity in a background thread cryptoExecutor.execute(new Runnable() { public void run() { - byte[] key = crypto.generateSecretKey().getEncoded(); + byte[] key = crypto.generateSecretKey().getBytes(); databaseConfig.setEncryptionKey(key); byte[] encrypted = encryptDatabaseKey(key, password); - for(int i = 0; i < password.length; i++) password[i] = 0; storeEncryptedDatabaseKey(encrypted); LocalAuthor localAuthor = createLocalAuthor(nickname); showDashboard(referenceManager.putReference(localAuthor, @@ -251,10 +236,6 @@ OnEditorActionListener { }); } - private void delete(Editable e) { - e.delete(0, e.length()); - } - private void storeEncryptedDatabaseKey(final byte[] encrypted) { long now = System.currentTimeMillis(); SharedPreferences prefs = getSharedPreferences("db", MODE_PRIVATE); @@ -266,7 +247,7 @@ OnEditorActionListener { LOG.info("Key storage took " + duration + " ms"); } - private byte[] encryptDatabaseKey(byte[] key, char[] password) { + private byte[] encryptDatabaseKey(byte[] key, String password) { long now = System.currentTimeMillis(); byte[] encrypted = crypto.encryptWithPassword(key, password); long duration = System.currentTimeMillis() - now; diff --git a/briar-api/src/org/briarproject/api/crypto/CryptoComponent.java b/briar-api/src/org/briarproject/api/crypto/CryptoComponent.java index ba9c86224b..558077ae48 100644 --- a/briar-api/src/org/briarproject/api/crypto/CryptoComponent.java +++ b/briar-api/src/org/briarproject/api/crypto/CryptoComponent.java @@ -89,7 +89,7 @@ public interface CryptoComponent { * given password. The ciphertext will be decryptable using the same * password after the app restarts. */ - byte[] encryptWithPassword(byte[] plaintext, char[] password); + byte[] encryptWithPassword(byte[] plaintext, String password); /** * Decrypts and authenticates the given ciphertext that has been read from @@ -97,5 +97,5 @@ public interface CryptoComponent { * given password. Returns null if the ciphertext cannot be decrypted and * authenticated (for example, if the password is wrong). */ - byte[] decryptWithPassword(byte[] ciphertext, char[] password); + byte[] decryptWithPassword(byte[] ciphertext, String password); } diff --git a/briar-api/src/org/briarproject/api/crypto/KeyManager.java b/briar-api/src/org/briarproject/api/crypto/KeyManager.java index 77c41132e0..02a43e9706 100644 --- a/briar-api/src/org/briarproject/api/crypto/KeyManager.java +++ b/briar-api/src/org/briarproject/api/crypto/KeyManager.java @@ -16,9 +16,6 @@ public interface KeyManager extends Service { */ StreamContext getStreamContext(ContactId c, TransportId t); - /** - * Called whenever an endpoint has been added. The initial secret is erased - * before returning. - */ + /** Called whenever an endpoint has been added. */ void endpointAdded(Endpoint ep, int maxLatency, byte[] initialSecret); } diff --git a/briar-api/src/org/briarproject/api/crypto/PasswordStrengthEstimator.java b/briar-api/src/org/briarproject/api/crypto/PasswordStrengthEstimator.java index 9ffa2dc244..93d27a6b32 100644 --- a/briar-api/src/org/briarproject/api/crypto/PasswordStrengthEstimator.java +++ b/briar-api/src/org/briarproject/api/crypto/PasswordStrengthEstimator.java @@ -12,5 +12,5 @@ public interface PasswordStrengthEstimator { * Returns an estimate between 0 (weakest) and 1 (strongest), inclusive, * of the strength of the given password. */ - float estimateStrength(char[] password); + float estimateStrength(String password); } diff --git a/briar-api/src/org/briarproject/api/crypto/SecretKey.java b/briar-api/src/org/briarproject/api/crypto/SecretKey.java index 63d1f76617..62b75a9452 100644 --- a/briar-api/src/org/briarproject/api/crypto/SecretKey.java +++ b/briar-api/src/org/briarproject/api/crypto/SecretKey.java @@ -1,21 +1,15 @@ package org.briarproject.api.crypto; /** A secret key used for encryption and/or authentication. */ -public interface SecretKey { +public class SecretKey { - /** Returns the encoded representation of this key. */ - byte[] getEncoded(); + private final byte[] key; - /** - * Returns a copy of this key - erasing this key will erase the copy and - * vice versa. - */ - SecretKey copy(); + public SecretKey(byte[] key) { + this.key = key; + } - /** - * Erases this key from memory. Any copies derived from this key via the - * {@link #copy()} method, and any keys from which this key was derived via - * the {@link #copy()} method, are also erased. - */ - void erase(); + public byte[] getBytes() { + return key; + } } diff --git a/briar-core/src/org/briarproject/crypto/AuthenticatedCipherImpl.java b/briar-core/src/org/briarproject/crypto/AuthenticatedCipherImpl.java index 93807087d2..bf80ca8c5f 100644 --- a/briar-core/src/org/briarproject/crypto/AuthenticatedCipherImpl.java +++ b/briar-core/src/org/briarproject/crypto/AuthenticatedCipherImpl.java @@ -38,7 +38,7 @@ class AuthenticatedCipherImpl implements AuthenticatedCipher { public void init(boolean encrypt, SecretKey key, byte[] iv, byte[] aad) throws GeneralSecurityException { - KeyParameter k = new KeyParameter(key.getEncoded()); + KeyParameter k = new KeyParameter(key.getBytes()); AEADParameters params = new AEADParameters(k, macLength * 8, iv, aad); try { cipher.init(encrypt, params); diff --git a/briar-core/src/org/briarproject/crypto/CryptoComponentImpl.java b/briar-core/src/org/briarproject/crypto/CryptoComponentImpl.java index 4edd26a4fe..29442f311f 100644 --- a/briar-core/src/org/briarproject/crypto/CryptoComponentImpl.java +++ b/briar-core/src/org/briarproject/crypto/CryptoComponentImpl.java @@ -7,8 +7,6 @@ import static org.briarproject.crypto.EllipticCurveConstants.P; import static org.briarproject.crypto.EllipticCurveConstants.PARAMETERS; import static org.briarproject.util.ByteUtils.MAX_32_BIT_UNSIGNED; -import java.io.ByteArrayOutputStream; -import java.io.IOException; import java.security.GeneralSecurityException; import java.security.SecureRandom; import java.util.ArrayList; @@ -31,6 +29,7 @@ import org.briarproject.api.crypto.SecretKey; import org.briarproject.api.crypto.Signature; import org.briarproject.api.system.SeedProvider; import org.briarproject.util.ByteUtils; +import org.briarproject.util.StringUtils; import org.spongycastle.crypto.AsymmetricCipherKeyPair; import org.spongycastle.crypto.BlockCipher; import org.spongycastle.crypto.CipherParameters; @@ -48,7 +47,6 @@ import org.spongycastle.crypto.params.ECKeyGenerationParameters; import org.spongycastle.crypto.params.ECPrivateKeyParameters; import org.spongycastle.crypto.params.ECPublicKeyParameters; import org.spongycastle.crypto.params.KeyParameter; -import org.spongycastle.util.Strings; class CryptoComponentImpl implements CryptoComponent { @@ -114,7 +112,7 @@ class CryptoComponentImpl implements CryptoComponent { public SecretKey generateSecretKey() { byte[] b = new byte[CIPHER_KEY_BYTES]; secureRandom.nextBytes(b); - return new SecretKeyImpl(b); + return new SecretKey(b); } public MessageDigest getMessageDigest() { @@ -188,8 +186,6 @@ class CryptoComponentImpl implements CryptoComponent { int[] codes = new int[2]; codes[0] = ByteUtils.readUint(alice, CODE_BITS); codes[1] = ByteUtils.readUint(bob, CODE_BITS); - ByteUtils.erase(alice); - ByteUtils.erase(bob); return codes; } @@ -223,9 +219,7 @@ class CryptoComponentImpl implements CryptoComponent { byte[] raw = deriveSharedSecret(ourPriv, theirPub); // Derive the cooked secret from the raw secret using the // concatenation KDF - byte[] cooked = concatenationKdf(raw, MASTER, aliceInfo, bobInfo); - ByteUtils.erase(raw); - return cooked; + return concatenationKdf(raw, MASTER, aliceInfo, bobInfo); } // Package access for testing @@ -296,8 +290,7 @@ class CryptoComponentImpl implements CryptoComponent { } private SecretKey deriveKey(byte[] secret, byte[] label, long context) { - byte[] key = counterModeKdf(secret, label, context); - return new SecretKeyImpl(key); + return new SecretKey(counterModeKdf(secret, label, context)); } public AuthenticatedCipher getFrameCipher() { @@ -313,21 +306,19 @@ class CryptoComponentImpl implements CryptoComponent { ByteUtils.writeUint32(streamNumber, tag, 0); BlockCipher cipher = new AESLightEngine(); assert cipher.getBlockSize() == TAG_LENGTH; - KeyParameter k = new KeyParameter(tagKey.getEncoded()); + KeyParameter k = new KeyParameter(tagKey.getBytes()); cipher.init(true, k); cipher.processBlock(tag, 0, tag, 0); - ByteUtils.erase(k.getKey()); } - public byte[] encryptWithPassword(byte[] input, char[] password) { + public byte[] encryptWithPassword(byte[] input, String password) { // Generate a random salt byte[] salt = new byte[PBKDF_SALT_BYTES]; secureRandom.nextBytes(salt); // Calibrate the KDF int iterations = chooseIterationCount(PBKDF_TARGET_MILLIS); // Derive the key from the password - byte[] keyBytes = pbkdf2(password, salt, iterations); - SecretKey key = new SecretKeyImpl(keyBytes); + SecretKey key = new SecretKey(pbkdf2(password, salt, iterations)); // Generate a random IV byte[] iv = new byte[STORAGE_IV_BYTES]; secureRandom.nextBytes(iv); @@ -348,12 +339,10 @@ class CryptoComponentImpl implements CryptoComponent { return output; } catch(GeneralSecurityException e) { throw new RuntimeException(e); - } finally { - key.erase(); } } - public byte[] decryptWithPassword(byte[] input, char[] password) { + public byte[] decryptWithPassword(byte[] input, String password) { // The input contains the salt, iterations, IV, ciphertext and MAC if(input.length < PBKDF_SALT_BYTES + 4 + STORAGE_IV_BYTES + MAC_BYTES) return null; // Invalid @@ -365,8 +354,7 @@ class CryptoComponentImpl implements CryptoComponent { byte[] iv = new byte[STORAGE_IV_BYTES]; System.arraycopy(input, salt.length + 4, iv, 0, iv.length); // Derive the key from the password - byte[] keyBytes = pbkdf2(password, salt, (int) iterations); - SecretKey key = new SecretKeyImpl(keyBytes); + SecretKey key = new SecretKey(pbkdf2(password, salt, (int) iterations)); // Initialise the cipher AuthenticatedCipher cipher; try { @@ -374,7 +362,6 @@ class CryptoComponentImpl implements CryptoComponent { cipher = new AuthenticatedCipherImpl(a, MAC_BYTES); cipher.init(false, key, iv, null); } catch(GeneralSecurityException e) { - key.erase(); throw new RuntimeException(e); } // Try to decrypt the ciphertext (may be invalid) @@ -385,9 +372,7 @@ class CryptoComponentImpl implements CryptoComponent { cipher.doFinal(input, inputOff, inputLen, output, 0); return output; } catch(GeneralSecurityException e) { - return null; // Invalid - } finally { - key.erase(); + return null; // Invalid ciphertext } } @@ -417,7 +402,6 @@ class CryptoComponentImpl implements CryptoComponent { // The secret is the first CIPHER_KEY_BYTES bytes of the hash byte[] output = new byte[CIPHER_KEY_BYTES]; System.arraycopy(hash, 0, output, 0, output.length); - ByteUtils.erase(hash); return output; } @@ -447,20 +431,17 @@ class CryptoComponentImpl implements CryptoComponent { prf.update((byte) CIPHER_KEY_BYTES); // Output length prf.doFinal(mac, 0); System.arraycopy(mac, 0, output, 0, output.length); - ByteUtils.erase(mac); - ByteUtils.erase(k.getKey()); return output; } // Password-based key derivation function - see PKCS#5 v2.1, section 5.2 - private byte[] pbkdf2(char[] password, byte[] salt, int iterations) { - byte[] utf8 = toUtf8ByteArray(password); + private byte[] pbkdf2(String password, byte[] salt, int iterations) { + byte[] utf8 = StringUtils.toUtf8(password); Digest digest = new SHA384Digest(); PKCS5S2ParametersGenerator gen = new PKCS5S2ParametersGenerator(digest); gen.init(utf8, salt, iterations); int keyLengthInBits = CIPHER_KEY_BYTES * 8; CipherParameters p = gen.generateDerivedParameters(keyLengthInBits); - ByteUtils.erase(utf8); return ((KeyParameter) p).getKey(); } @@ -512,18 +493,4 @@ class CryptoComponentImpl implements CryptoComponent { if(size % 2 == 1) return list.get(size / 2); return list.get(size / 2 - 1) + list.get(size / 2) / 2; } - - private byte[] toUtf8ByteArray(char[] c) { - ByteArrayOutputStream out = new ByteArrayOutputStream(); - try { - Strings.toUTF8ByteArray(c, out); - byte[] utf8 = out.toByteArray(); - // Erase the output stream's buffer - out.reset(); - out.write(new byte[utf8.length]); - return utf8; - } catch(IOException e) { - throw new RuntimeException(e); - } - } } diff --git a/briar-core/src/org/briarproject/crypto/PasswordStrengthEstimatorImpl.java b/briar-core/src/org/briarproject/crypto/PasswordStrengthEstimatorImpl.java index aeec2ddc3b..1fd99f52bf 100644 --- a/briar-core/src/org/briarproject/crypto/PasswordStrengthEstimatorImpl.java +++ b/briar-core/src/org/briarproject/crypto/PasswordStrengthEstimatorImpl.java @@ -13,9 +13,10 @@ class PasswordStrengthEstimatorImpl implements PasswordStrengthEstimator { private static final double STRONG = Math.log(Math.pow(LOWER + UPPER + DIGIT + OTHER, 10)); - public float estimateStrength(char[] password) { + public float estimateStrength(String password) { HashSet<Character> unique = new HashSet<Character>(); - for(char c : password) unique.add(c); + int length = password.length(); + for(int i = 0; i < length; i++) unique.add(password.charAt(i)); boolean lower = false, upper = false, digit = false, other = false; for(char c : unique) { if(Character.isLowerCase(c)) lower = true; diff --git a/briar-core/src/org/briarproject/crypto/SecretKeyImpl.java b/briar-core/src/org/briarproject/crypto/SecretKeyImpl.java deleted file mode 100644 index d7cdbdf05d..0000000000 --- a/briar-core/src/org/briarproject/crypto/SecretKeyImpl.java +++ /dev/null @@ -1,30 +0,0 @@ -package org.briarproject.crypto; - -import org.briarproject.api.crypto.SecretKey; -import org.briarproject.util.ByteUtils; - -class SecretKeyImpl implements SecretKey { - - private final byte[] key; - - private boolean erased = false; // Locking: this - - SecretKeyImpl(byte[] key) { - this.key = key; - } - - public synchronized byte[] getEncoded() { - if(erased) throw new IllegalStateException(); - return key; - } - - public SecretKey copy() { - return new SecretKeyImpl(key.clone()); - } - - public synchronized void erase() { - if(erased) throw new IllegalStateException(); - ByteUtils.erase(key); - erased = true; - } -} diff --git a/briar-core/src/org/briarproject/crypto/StreamDecrypterImpl.java b/briar-core/src/org/briarproject/crypto/StreamDecrypterImpl.java index ff29697dfd..4127e99a70 100644 --- a/briar-core/src/org/briarproject/crypto/StreamDecrypterImpl.java +++ b/briar-core/src/org/briarproject/crypto/StreamDecrypterImpl.java @@ -44,16 +44,11 @@ class StreamDecrypterImpl implements StreamDecrypter { if(finalFrame) return -1; // Read the frame int ciphertextLength = 0; - try { - while(ciphertextLength < frameLength) { - int read = in.read(ciphertext, ciphertextLength, - frameLength - ciphertextLength); - if(read == -1) break; // We'll check the length later - ciphertextLength += read; - } - } catch(IOException e) { - frameKey.erase(); - throw e; + while(ciphertextLength < frameLength) { + int read = in.read(ciphertext, ciphertextLength, + frameLength - ciphertextLength); + if(read == -1) break; // We'll check the length later + ciphertextLength += read; } int plaintextLength = ciphertextLength - MAC_LENGTH; if(plaintextLength < HEADER_LENGTH) throw new EOFException(); diff --git a/briar-core/src/org/briarproject/crypto/StreamEncrypterFactoryImpl.java b/briar-core/src/org/briarproject/crypto/StreamEncrypterFactoryImpl.java index 979ebb17c8..07c9deaabf 100644 --- a/briar-core/src/org/briarproject/crypto/StreamEncrypterFactoryImpl.java +++ b/briar-core/src/org/briarproject/crypto/StreamEncrypterFactoryImpl.java @@ -30,7 +30,6 @@ class StreamEncrypterFactoryImpl implements StreamEncrypterFactory { byte[] tag = new byte[TAG_LENGTH]; SecretKey tagKey = crypto.deriveTagKey(secret, alice); crypto.encodeTag(tag, tagKey, streamNumber); - tagKey.erase(); // Derive the frame key SecretKey frameKey = crypto.deriveFrameKey(secret, streamNumber, alice); // Create the encrypter diff --git a/briar-core/src/org/briarproject/crypto/StreamEncrypterImpl.java b/briar-core/src/org/briarproject/crypto/StreamEncrypterImpl.java index d3e2e43ce1..43a6403912 100644 --- a/briar-core/src/org/briarproject/crypto/StreamEncrypterImpl.java +++ b/briar-core/src/org/briarproject/crypto/StreamEncrypterImpl.java @@ -45,12 +45,7 @@ class StreamEncrypterImpl implements StreamEncrypter { if(frameNumber > MAX_32_BIT_UNSIGNED) throw new IllegalStateException(); // Write the tag if required if(writeTag) { - try { - out.write(tag, 0, tag.length); - } catch(IOException e) { - frameKey.erase(); - throw e; - } + out.write(tag, 0, tag.length); writeTag = false; } // Don't pad the final frame @@ -81,24 +76,14 @@ class StreamEncrypterImpl implements StreamEncrypter { throw new RuntimeException(badCipher); } // Write the frame - try { - out.write(ciphertext, 0, ciphertextLength); - } catch(IOException e) { - frameKey.erase(); - throw e; - } + out.write(ciphertext, 0, ciphertextLength); frameNumber++; } public void flush() throws IOException { // Write the tag if required if(writeTag) { - try { - out.write(tag, 0, tag.length); - } catch(IOException e) { - frameKey.erase(); - throw e; - } + out.write(tag, 0, tag.length); writeTag = false; } out.flush(); diff --git a/briar-core/src/org/briarproject/db/H2Database.java b/briar-core/src/org/briarproject/db/H2Database.java index d7f064ddb2..fc91e20dca 100644 --- a/briar-core/src/org/briarproject/db/H2Database.java +++ b/briar-core/src/org/briarproject/db/H2Database.java @@ -81,34 +81,18 @@ class H2Database extends JdbcDatabase { } } + @Override protected Connection createConnection() throws SQLException { byte[] key = config.getEncryptionKey(); if(key == null) throw new IllegalStateException(); - char[] password = encodePassword(key); Properties props = new Properties(); props.setProperty("user", "user"); - props.put("password", password); - try { - return DriverManager.getConnection(url, props); - } finally { - for(int i = 0; i < password.length; i++) password[i] = 0; - } - } - - private char[] encodePassword(byte[] key) { - // The database password is the hex-encoded key - char[] hex = StringUtils.toHexChars(key); - // Separate the database password from the user password with a space - char[] user = "password".toCharArray(); - char[] combined = new char[hex.length + 1 + user.length]; - System.arraycopy(hex, 0, combined, 0, hex.length); - combined[hex.length] = ' '; - System.arraycopy(user, 0, combined, hex.length + 1, user.length); - // Erase the hex-encoded key - for(int i = 0; i < hex.length; i++) hex[i] = 0; - return combined; + // Separate the file password from the user password with a space + props.put("password", StringUtils.toHexString(key) + " password"); + return DriverManager.getConnection(url, props); } + @Override protected void flushBuffersToDisk(Statement s) throws SQLException { // FIXME: Remove this after implementing BTPv2? s.execute("CHECKPOINT SYNC"); diff --git a/briar-core/src/org/briarproject/plugins/ConnectionManagerImpl.java b/briar-core/src/org/briarproject/plugins/ConnectionManagerImpl.java index b72efb688d..2b1a5c633e 100644 --- a/briar-core/src/org/briarproject/plugins/ConnectionManagerImpl.java +++ b/briar-core/src/org/briarproject/plugins/ConnectionManagerImpl.java @@ -28,7 +28,6 @@ import org.briarproject.api.transport.StreamContext; import org.briarproject.api.transport.StreamReaderFactory; import org.briarproject.api.transport.StreamWriterFactory; import org.briarproject.api.transport.TagRecogniser; -import org.briarproject.util.ByteUtils; class ConnectionManagerImpl implements ConnectionManager { @@ -95,40 +94,28 @@ class ConnectionManagerImpl implements ConnectionManager { private MessagingSession createIncomingSession(StreamContext ctx, TransportConnectionReader r) throws IOException { - try { - InputStream streamReader = streamReaderFactory.createStreamReader( - r.getInputStream(), r.getMaxFrameLength(), ctx); - return messagingSessionFactory.createIncomingSession( - ctx.getContactId(), ctx.getTransportId(), streamReader); - } finally { - ByteUtils.erase(ctx.getSecret()); - } + InputStream streamReader = streamReaderFactory.createStreamReader( + r.getInputStream(), r.getMaxFrameLength(), ctx); + return messagingSessionFactory.createIncomingSession( + ctx.getContactId(), ctx.getTransportId(), streamReader); } private MessagingSession createSimplexOutgoingSession(StreamContext ctx, TransportConnectionWriter w) throws IOException { - try { - OutputStream streamWriter = streamWriterFactory.createStreamWriter( - w.getOutputStream(), w.getMaxFrameLength(), ctx); - return messagingSessionFactory.createSimplexOutgoingSession( - ctx.getContactId(), ctx.getTransportId(), w.getMaxLatency(), - streamWriter); - } finally { - ByteUtils.erase(ctx.getSecret()); - } + OutputStream streamWriter = streamWriterFactory.createStreamWriter( + w.getOutputStream(), w.getMaxFrameLength(), ctx); + return messagingSessionFactory.createSimplexOutgoingSession( + ctx.getContactId(), ctx.getTransportId(), w.getMaxLatency(), + streamWriter); } private MessagingSession createDuplexOutgoingSession(StreamContext ctx, TransportConnectionWriter w) throws IOException { - try { - OutputStream streamWriter = streamWriterFactory.createStreamWriter( - w.getOutputStream(), w.getMaxFrameLength(), ctx); - return messagingSessionFactory.createDuplexOutgoingSession( - ctx.getContactId(), ctx.getTransportId(), w.getMaxLatency(), - w.getMaxIdleTime(), streamWriter); - } finally { - ByteUtils.erase(ctx.getSecret()); - } + OutputStream streamWriter = streamWriterFactory.createStreamWriter( + w.getOutputStream(), w.getMaxFrameLength(), ctx); + return messagingSessionFactory.createDuplexOutgoingSession( + ctx.getContactId(), ctx.getTransportId(), w.getMaxLatency(), + w.getMaxIdleTime(), streamWriter); } private class ManageIncomingSimplexConnection implements Runnable { diff --git a/briar-core/src/org/briarproject/transport/KeyManagerImpl.java b/briar-core/src/org/briarproject/transport/KeyManagerImpl.java index 735e9f17ba..089f724f86 100644 --- a/briar-core/src/org/briarproject/transport/KeyManagerImpl.java +++ b/briar-core/src/org/briarproject/transport/KeyManagerImpl.java @@ -33,7 +33,6 @@ import org.briarproject.api.transport.Endpoint; import org.briarproject.api.transport.StreamContext; import org.briarproject.api.transport.TagRecogniser; import org.briarproject.api.transport.TemporarySecret; -import org.briarproject.util.ByteUtils; // FIXME: Don't make alien calls with a lock held class KeyManagerImpl extends TimerTask implements KeyManager, EventListener { @@ -119,7 +118,6 @@ class KeyManagerImpl extends TimerTask implements KeyManager, EventListener { Integer maxLatency = maxLatencies.get(s.getTransportId()); if(maxLatency == null) { LOG.info("Discarding obsolete secret"); - ByteUtils.erase(s.getSecret()); continue; } long rotation = maxLatency + MAX_CLOCK_DIFFERENCE; @@ -143,7 +141,7 @@ class KeyManagerImpl extends TimerTask implements KeyManager, EventListener { return dead; } - // Replaces and erases the given secrets and returns any secrets created + // Replaces the given secrets and returns any secrets created // Locking: this private Collection<TemporarySecret> replaceDeadSecrets(long now, Collection<TemporarySecret> dead) { @@ -157,12 +155,10 @@ class KeyManagerImpl extends TimerTask implements KeyManager, EventListener { // There's no other secret for this endpoint newest.put(k, s); } else if(exists.getPeriod() < s.getPeriod()) { - // There's an older secret - erase it and use this one instead - ByteUtils.erase(exists.getSecret()); + // There's an older secret - use this one instead newest.put(k, s); } else { - // There's a newer secret - erase this one - ByteUtils.erase(s.getSecret()); + // There's a newer secret - keep using it } } Collection<TemporarySecret> created = new ArrayList<TemporarySecret>(); @@ -179,34 +175,23 @@ class KeyManagerImpl extends TimerTask implements KeyManager, EventListener { throw new IllegalStateException(); // Derive the old, current and new secrets byte[] b1 = s.getSecret(); - for(long p = s.getPeriod() + 1; p < period; p++) { - byte[] temp = crypto.deriveNextSecret(b1, p); - ByteUtils.erase(b1); - b1 = temp; - } + for(long p = s.getPeriod() + 1; p < period; p++) + b1 = crypto.deriveNextSecret(b1, p); byte[] b2 = crypto.deriveNextSecret(b1, period); byte[] b3 = crypto.deriveNextSecret(b2, period + 1); - // Add the secrets to their respective maps - copies may already - // exist, in which case erase the new copies (the old copies are - // referenced by the connection recogniser) + // Add the secrets to their respective maps if not already present EndpointKey k = e.getKey(); - if(oldSecrets.containsKey(k)) { - ByteUtils.erase(b1); - } else { + if(!oldSecrets.containsKey(k)) { TemporarySecret s1 = new TemporarySecret(s, period - 1, b1); oldSecrets.put(k, s1); created.add(s1); } - if(currentSecrets.containsKey(k)) { - ByteUtils.erase(b2); - } else { + if(!currentSecrets.containsKey(k)) { TemporarySecret s2 = new TemporarySecret(s, period, b2); currentSecrets.put(k, s2); created.add(s2); } - if(newSecrets.containsKey(k)) { - ByteUtils.erase(b3); - } else { + if(!newSecrets.containsKey(k)) { TemporarySecret s3 = new TemporarySecret(s, period + 1, b3); newSecrets.put(k, s3); created.add(s3); @@ -220,18 +205,12 @@ class KeyManagerImpl extends TimerTask implements KeyManager, EventListener { timer.cancel(); tagRecogniser.removeSecrets(); maxLatencies.clear(); - removeAndEraseSecrets(oldSecrets); - removeAndEraseSecrets(currentSecrets); - removeAndEraseSecrets(newSecrets); + oldSecrets.clear(); + currentSecrets.clear(); + newSecrets.clear(); return true; } - // Locking: this - private void removeAndEraseSecrets(Map<?, TemporarySecret> m) { - for(TemporarySecret s : m.values()) ByteUtils.erase(s.getSecret()); - m.clear(); - } - public synchronized StreamContext getStreamContext(ContactId c, TransportId t) { TemporarySecret s = currentSecrets.get(new EndpointKey(c, t)); @@ -250,8 +229,7 @@ class KeyManagerImpl extends TimerTask implements KeyManager, EventListener { if(LOG.isLoggable(WARNING)) LOG.log(WARNING, e.toString(), e); return null; } - // Clone the secret - the original will be erased - byte[] secret = s.getSecret().clone(); + byte[] secret = s.getSecret(); return new StreamContext(c, t, secret, streamNumber, s.getAlice()); } @@ -265,11 +243,8 @@ class KeyManagerImpl extends TimerTask implements KeyManager, EventListener { if(period < 1) throw new IllegalStateException(); // Derive the old, current and new secrets byte[] b1 = initialSecret; - for(long p = 0; p < period; p++) { - byte[] temp = crypto.deriveNextSecret(b1, p); - ByteUtils.erase(b1); - b1 = temp; - } + for(long p = 0; p < period; p++) + b1 = crypto.deriveNextSecret(b1, p); byte[] b2 = crypto.deriveNextSecret(b1, period); byte[] b3 = crypto.deriveNextSecret(b2, period + 1); TemporarySecret s1 = new TemporarySecret(ep, period - 1, b1); @@ -341,28 +316,17 @@ class KeyManagerImpl extends TimerTask implements KeyManager, EventListener { } // Locking: this - private void removeAndEraseSecrets(ContactId c, Map<?, TemporarySecret> m) { + private void removeSecrets(ContactId c, Map<?, TemporarySecret> m) { Iterator<TemporarySecret> it = m.values().iterator(); - while(it.hasNext()) { - TemporarySecret s = it.next(); - if(s.getContactId().equals(c)) { - ByteUtils.erase(s.getSecret()); - it.remove(); - } - } + while(it.hasNext()) + if(it.next().getContactId().equals(c)) it.remove(); } // Locking: this - private void removeAndEraseSecrets(TransportId t, - Map<?, TemporarySecret> m) { + private void removeSecrets(TransportId t, Map<?, TemporarySecret> m) { Iterator<TemporarySecret> it = m.values().iterator(); - while(it.hasNext()) { - TemporarySecret s = it.next(); - if(s.getTransportId().equals(t)) { - ByteUtils.erase(s.getSecret()); - it.remove(); - } - } + while(it.hasNext()) + if(it.next().getTransportId().equals(t)) it.remove(); } private static class EndpointKey { @@ -408,9 +372,9 @@ class KeyManagerImpl extends TimerTask implements KeyManager, EventListener { ContactId c = event.getContactId(); tagRecogniser.removeSecrets(c); synchronized(KeyManagerImpl.this) { - removeAndEraseSecrets(c, oldSecrets); - removeAndEraseSecrets(c, currentSecrets); - removeAndEraseSecrets(c, newSecrets); + removeSecrets(c, oldSecrets); + removeSecrets(c, currentSecrets); + removeSecrets(c, newSecrets); } } } @@ -445,9 +409,9 @@ class KeyManagerImpl extends TimerTask implements KeyManager, EventListener { tagRecogniser.removeSecrets(t); synchronized(KeyManagerImpl.this) { maxLatencies.remove(t); - removeAndEraseSecrets(t, oldSecrets); - removeAndEraseSecrets(t, currentSecrets); - removeAndEraseSecrets(t, newSecrets); + removeSecrets(t, oldSecrets); + removeSecrets(t, currentSecrets); + removeSecrets(t, newSecrets); } } } diff --git a/briar-core/src/org/briarproject/transport/TransportTagRecogniser.java b/briar-core/src/org/briarproject/transport/TransportTagRecogniser.java index 3c9508397c..285b4465e4 100644 --- a/briar-core/src/org/briarproject/transport/TransportTagRecogniser.java +++ b/briar-core/src/org/briarproject/transport/TransportTagRecogniser.java @@ -56,13 +56,10 @@ class TransportTagRecogniser { assert duplicate == null; } } - key.erase(); // Store the updated reordering window in the DB db.setReorderingWindow(t.contactId, transportId, t.period, t.window.getCentre(), t.window.getBitmap()); - // Clone the secret - the key manager will erase the original - byte[] secret = t.secret.clone(); - return new StreamContext(t.contactId, transportId, secret, + return new StreamContext(t.contactId, transportId, t.secret, t.streamNumber, t.alice); } @@ -84,7 +81,6 @@ class TransportTagRecogniser { TagContext duplicate = tagMap.put(new Bytes(tag), added); assert duplicate == null; } - key.erase(); // Create a removal context to remove the window and the tags later RemovalContext r = new RemovalContext(window, secret, alice); removalMap.put(new RemovalKey(contactId, period), r); @@ -107,7 +103,6 @@ class TransportTagRecogniser { TagContext removed = tagMap.remove(new Bytes(tag)); assert removed != null; } - key.erase(); } synchronized void removeSecrets(ContactId c) { diff --git a/briar-core/src/org/briarproject/util/ByteUtils.java b/briar-core/src/org/briarproject/util/ByteUtils.java index ed40306e59..9cc9e6bc88 100644 --- a/briar-core/src/org/briarproject/util/ByteUtils.java +++ b/briar-core/src/org/briarproject/util/ByteUtils.java @@ -48,10 +48,6 @@ public class ByteUtils { | ((b[offset + 2] & 0xFFL) << 8) | (b[offset + 3] & 0xFFL); } - public static void erase(byte[] b) { - for(int i = 0; i < b.length; i++) b[i] = 0; - } - public static int readUint(byte[] b, int bits) { if(b.length << 3 < bits) throw new IllegalArgumentException(); int result = 0; diff --git a/briar-tests/build.xml b/briar-tests/build.xml index 561a3fbe55..eb4766a090 100644 --- a/briar-tests/build.xml +++ b/briar-tests/build.xml @@ -100,8 +100,7 @@ <test name='org.briarproject.crypto.KeyDerivationTest'/> <test name='org.briarproject.crypto.KeyEncodingAndParsingTest'/> <test name="org.briarproject.crypto.PasswordBasedKdfTest"/> - <test name="org.briarproject.crypto.PasswordStrengthEstimatorTest"/> - <test name='org.briarproject.crypto.SecretKeyImplTest'/> + <test name="org.briarproject.crypto.PasswordStrengthEstimatorImplTest"/> <test name='org.briarproject.crypto.StreamDecrypterImplTest'/> <test name='org.briarproject.crypto.StreamEncrypterImplTest'/> <test name='org.briarproject.db.BasicH2Test'/> diff --git a/briar-tests/src/org/briarproject/ProtocolIntegrationTest.java b/briar-tests/src/org/briarproject/ProtocolIntegrationTest.java index a3983b5a2e..cfd5af2d57 100644 --- a/briar-tests/src/org/briarproject/ProtocolIntegrationTest.java +++ b/briar-tests/src/org/briarproject/ProtocolIntegrationTest.java @@ -116,8 +116,8 @@ public class ProtocolIntegrationTest extends BriarTestCase { private byte[] write() throws Exception { ByteArrayOutputStream out = new ByteArrayOutputStream(); - StreamContext ctx = new StreamContext(contactId, transportId, - secret.clone(), 0, true); + StreamContext ctx = new StreamContext(contactId, transportId, secret, + 0, true); OutputStream streamWriter = streamWriterFactory.createStreamWriter(out, MAX_FRAME_LENGTH, ctx); PacketWriter packetWriter = packetWriterFactory.createPacketWriter( @@ -148,8 +148,8 @@ public class ProtocolIntegrationTest extends BriarTestCase { byte[] tag = new byte[TAG_LENGTH]; assertEquals(TAG_LENGTH, in.read(tag, 0, TAG_LENGTH)); // FIXME: Check that the expected tag was received - StreamContext ctx = new StreamContext(contactId, transportId, - secret.clone(), 0, false); + StreamContext ctx = new StreamContext(contactId, transportId, secret, + 0, false); InputStream streamReader = streamReaderFactory.createStreamReader(in, MAX_FRAME_LENGTH, ctx); PacketReader packetReader = packetReaderFactory.createPacketReader( diff --git a/briar-tests/src/org/briarproject/crypto/KeyDerivationTest.java b/briar-tests/src/org/briarproject/crypto/KeyDerivationTest.java index 54c4a3c156..d0d71a7eb7 100644 --- a/briar-tests/src/org/briarproject/crypto/KeyDerivationTest.java +++ b/briar-tests/src/org/briarproject/crypto/KeyDerivationTest.java @@ -30,9 +30,9 @@ public class KeyDerivationTest extends BriarTestCase { keys.add(crypto.deriveTagKey(secret, true)); keys.add(crypto.deriveTagKey(secret, false)); for(int i = 0; i < 4; i++) { - byte[] keyI = keys.get(i).getEncoded(); + byte[] keyI = keys.get(i).getBytes(); for(int j = 0; j < 4; j++) { - byte[] keyJ = keys.get(j).getEncoded(); + byte[] keyJ = keys.get(j).getBytes(); assertEquals(i == j, Arrays.equals(keyI, keyJ)); } } @@ -59,9 +59,8 @@ public class KeyDerivationTest extends BriarTestCase { @Test public void testStreamNumberAffectsDerivation() { List<byte[]> secrets = new ArrayList<byte[]>(); - for(int i = 0; i < 20; i++) { - secrets.add(crypto.deriveNextSecret(secret.clone(), i)); - } + for(int i = 0; i < 20; i++) + secrets.add(crypto.deriveNextSecret(secret, i)); for(int i = 0; i < 20; i++) { byte[] secretI = secrets.get(i); for(int j = 0; j < 20; j++) { diff --git a/briar-tests/src/org/briarproject/crypto/PasswordBasedKdfTest.java b/briar-tests/src/org/briarproject/crypto/PasswordBasedKdfTest.java index 151f78fb57..cdc59b3323 100644 --- a/briar-tests/src/org/briarproject/crypto/PasswordBasedKdfTest.java +++ b/briar-tests/src/org/briarproject/crypto/PasswordBasedKdfTest.java @@ -18,7 +18,7 @@ public class PasswordBasedKdfTest extends BriarTestCase { Random random = new Random(); byte[] input = new byte[1234]; random.nextBytes(input); - char[] password = "password".toCharArray(); + String password = "password"; byte[] ciphertext = crypto.encryptWithPassword(input, password); byte[] output = crypto.decryptWithPassword(ciphertext, password); assertArrayEquals(input, output); @@ -29,7 +29,7 @@ public class PasswordBasedKdfTest extends BriarTestCase { Random random = new Random(); byte[] input = new byte[1234]; random.nextBytes(input); - char[] password = "password".toCharArray(); + String password = "password"; byte[] ciphertext = crypto.encryptWithPassword(input, password); // Modify the ciphertext int position = random.nextInt(ciphertext.length); diff --git a/briar-tests/src/org/briarproject/crypto/PasswordStrengthEstimatorTest.java b/briar-tests/src/org/briarproject/crypto/PasswordStrengthEstimatorImplTest.java similarity index 50% rename from briar-tests/src/org/briarproject/crypto/PasswordStrengthEstimatorTest.java rename to briar-tests/src/org/briarproject/crypto/PasswordStrengthEstimatorImplTest.java index 9f703b378f..5407daf9ae 100644 --- a/briar-tests/src/org/briarproject/crypto/PasswordStrengthEstimatorTest.java +++ b/briar-tests/src/org/briarproject/crypto/PasswordStrengthEstimatorImplTest.java @@ -6,24 +6,23 @@ import org.briarproject.BriarTestCase; import org.briarproject.api.crypto.PasswordStrengthEstimator; import org.junit.Test; -public class PasswordStrengthEstimatorTest extends BriarTestCase { +public class PasswordStrengthEstimatorImplTest extends BriarTestCase { @Test public void testWeakPasswords() { PasswordStrengthEstimator e = new PasswordStrengthEstimatorImpl(); - assertTrue(e.estimateStrength("".toCharArray()) < QUITE_STRONG); - assertTrue(e.estimateStrength("password".toCharArray()) < QUITE_STRONG); - assertTrue(e.estimateStrength("letmein".toCharArray()) < QUITE_STRONG); - assertTrue(e.estimateStrength("123456".toCharArray()) < QUITE_STRONG); + assertTrue(e.estimateStrength("") < QUITE_STRONG); + assertTrue(e.estimateStrength("password") < QUITE_STRONG); + assertTrue(e.estimateStrength("letmein") < QUITE_STRONG); + assertTrue(e.estimateStrength("123456") < QUITE_STRONG); } @Test public void testStrongPasswords() { PasswordStrengthEstimator e = new PasswordStrengthEstimatorImpl(); // Industry standard - assertTrue(e.estimateStrength("Tr0ub4dor&3".toCharArray()) - > QUITE_STRONG); - assertTrue(e.estimateStrength("correcthorsebatterystaple".toCharArray()) + assertTrue(e.estimateStrength("Tr0ub4dor&3") > QUITE_STRONG); + assertTrue(e.estimateStrength("correcthorsebatterystaple") > QUITE_STRONG); } } diff --git a/briar-tests/src/org/briarproject/crypto/SecretKeyImplTest.java b/briar-tests/src/org/briarproject/crypto/SecretKeyImplTest.java deleted file mode 100644 index 20ee3605bf..0000000000 --- a/briar-tests/src/org/briarproject/crypto/SecretKeyImplTest.java +++ /dev/null @@ -1,27 +0,0 @@ -package org.briarproject.crypto; - -import static org.junit.Assert.assertArrayEquals; - -import java.util.Random; - -import org.briarproject.BriarTestCase; -import org.briarproject.api.crypto.SecretKey; -import org.junit.Test; - -public class SecretKeyImplTest extends BriarTestCase { - - private static final int KEY_BYTES = 32; // 256 bits - - @Test - public void testCopiesAreErased() { - byte[] master = new byte[KEY_BYTES]; - new Random().nextBytes(master); - SecretKey k = new SecretKeyImpl(master); - byte[] copy = k.getEncoded(); - assertArrayEquals(master, copy); - k.erase(); - byte[] blank = new byte[KEY_BYTES]; - assertArrayEquals(blank, master); - assertArrayEquals(blank, copy); - } -} diff --git a/briar-tests/src/org/briarproject/messaging/SimplexMessagingIntegrationTest.java b/briar-tests/src/org/briarproject/messaging/SimplexMessagingIntegrationTest.java index 3bce2a6e39..d0a8359af4 100644 --- a/briar-tests/src/org/briarproject/messaging/SimplexMessagingIntegrationTest.java +++ b/briar-tests/src/org/briarproject/messaging/SimplexMessagingIntegrationTest.java @@ -127,7 +127,7 @@ public class SimplexMessagingIntegrationTest extends BriarTestCase { db.addTransport(transportId, MAX_LATENCY); Endpoint ep = new Endpoint(contactId, transportId, epoch, true); db.addEndpoint(ep); - keyManager.endpointAdded(ep, MAX_LATENCY, initialSecret.clone()); + keyManager.endpointAdded(ep, MAX_LATENCY, initialSecret); // Send Bob a message String contentType = "text/plain"; long timestamp = System.currentTimeMillis(); @@ -190,7 +190,7 @@ public class SimplexMessagingIntegrationTest extends BriarTestCase { db.addTransport(transportId, MAX_LATENCY); Endpoint ep = new Endpoint(contactId, transportId, epoch, false); db.addEndpoint(ep); - keyManager.endpointAdded(ep, MAX_LATENCY, initialSecret.clone()); + keyManager.endpointAdded(ep, MAX_LATENCY, initialSecret); // Set up an event listener MessageListener listener = new MessageListener(); bob.getInstance(EventBus.class).addListener(listener); diff --git a/briar-tests/src/org/briarproject/transport/KeyManagerImplTest.java b/briar-tests/src/org/briarproject/transport/KeyManagerImplTest.java index c160b59db3..12880b1b8e 100644 --- a/briar-tests/src/org/briarproject/transport/KeyManagerImplTest.java +++ b/briar-tests/src/org/briarproject/transport/KeyManagerImplTest.java @@ -103,9 +103,9 @@ public class KeyManagerImplTest extends BriarTestCase { // The secrets for periods 0 - 2 should be derived Endpoint ep = new Endpoint(contactId, transportId, EPOCH, true); - final TemporarySecret s0 = new TemporarySecret(ep, 0, secret0.clone()); - final TemporarySecret s1 = new TemporarySecret(ep, 1, secret1.clone()); - final TemporarySecret s2 = new TemporarySecret(ep, 2, secret2.clone()); + final TemporarySecret s0 = new TemporarySecret(ep, 0, secret0); + final TemporarySecret s1 = new TemporarySecret(ep, 1, secret1); + final TemporarySecret s2 = new TemporarySecret(ep, 2, secret2); context.checking(new Expectations() {{ // start() @@ -123,11 +123,11 @@ public class KeyManagerImplTest extends BriarTestCase { oneOf(clock).currentTimeMillis(); will(returnValue(EPOCH)); oneOf(crypto).deriveNextSecret(initialSecret, 0); - will(returnValue(secret0.clone())); + will(returnValue(secret0)); oneOf(crypto).deriveNextSecret(secret0, 1); - will(returnValue(secret1.clone())); + will(returnValue(secret1)); oneOf(crypto).deriveNextSecret(secret1, 2); - will(returnValue(secret2.clone())); + will(returnValue(secret2)); oneOf(db).addSecrets(Arrays.asList(s0, s1, s2)); // The secrets for periods 0 - 2 should be added to the recogniser oneOf(tagRecogniser).addSecret(s0); @@ -140,7 +140,7 @@ public class KeyManagerImplTest extends BriarTestCase { }}); assertTrue(keyManager.start()); - keyManager.endpointAdded(ep, MAX_LATENCY, initialSecret.clone()); + keyManager.endpointAdded(ep, MAX_LATENCY, initialSecret); keyManager.stop(); context.assertIsSatisfied(); @@ -161,9 +161,9 @@ public class KeyManagerImplTest extends BriarTestCase { // The secrets for periods 0 - 2 should be derived Endpoint ep = new Endpoint(contactId, transportId, EPOCH, true); - final TemporarySecret s0 = new TemporarySecret(ep, 0, secret0.clone()); - final TemporarySecret s1 = new TemporarySecret(ep, 1, secret1.clone()); - final TemporarySecret s2 = new TemporarySecret(ep, 2, secret2.clone()); + final TemporarySecret s0 = new TemporarySecret(ep, 0, secret0); + final TemporarySecret s1 = new TemporarySecret(ep, 1, secret1); + final TemporarySecret s2 = new TemporarySecret(ep, 2, secret2); context.checking(new Expectations() {{ // start() @@ -181,11 +181,11 @@ public class KeyManagerImplTest extends BriarTestCase { oneOf(clock).currentTimeMillis(); will(returnValue(EPOCH)); oneOf(crypto).deriveNextSecret(initialSecret, 0); - will(returnValue(secret0.clone())); + will(returnValue(secret0)); oneOf(crypto).deriveNextSecret(secret0, 1); - will(returnValue(secret1.clone())); + will(returnValue(secret1)); oneOf(crypto).deriveNextSecret(secret1, 2); - will(returnValue(secret2.clone())); + will(returnValue(secret2)); oneOf(db).addSecrets(Arrays.asList(s0, s1, s2)); // The secrets for periods 0 - 2 should be added to the recogniser oneOf(tagRecogniser).addSecret(s0); @@ -201,7 +201,7 @@ public class KeyManagerImplTest extends BriarTestCase { }}); assertTrue(keyManager.start()); - keyManager.endpointAdded(ep, MAX_LATENCY, initialSecret.clone()); + keyManager.endpointAdded(ep, MAX_LATENCY, initialSecret); StreamContext ctx = keyManager.getStreamContext(contactId, transportId); assertNotNull(ctx); @@ -230,9 +230,9 @@ public class KeyManagerImplTest extends BriarTestCase { // The DB contains the secrets for periods 0 - 2 Endpoint ep = new Endpoint(contactId, transportId, EPOCH, true); - final TemporarySecret s0 = new TemporarySecret(ep, 0, secret0.clone()); - final TemporarySecret s1 = new TemporarySecret(ep, 1, secret1.clone()); - final TemporarySecret s2 = new TemporarySecret(ep, 2, secret2.clone()); + final TemporarySecret s0 = new TemporarySecret(ep, 0, secret0); + final TemporarySecret s1 = new TemporarySecret(ep, 1, secret1); + final TemporarySecret s2 = new TemporarySecret(ep, 2, secret2); context.checking(new Expectations() {{ // start() @@ -278,11 +278,11 @@ public class KeyManagerImplTest extends BriarTestCase { // The DB contains the secrets for periods 0 - 2 Endpoint ep = new Endpoint(contactId, transportId, EPOCH, true); - final TemporarySecret s0 = new TemporarySecret(ep, 0, secret0.clone()); - final TemporarySecret s1 = new TemporarySecret(ep, 1, secret1.clone()); - final TemporarySecret s2 = new TemporarySecret(ep, 2, secret2.clone()); + final TemporarySecret s0 = new TemporarySecret(ep, 0, secret0); + final TemporarySecret s1 = new TemporarySecret(ep, 1, secret1); + final TemporarySecret s2 = new TemporarySecret(ep, 2, secret2); // The secret for period 3 should be derived and stored - final TemporarySecret s3 = new TemporarySecret(ep, 3, secret3.clone()); + final TemporarySecret s3 = new TemporarySecret(ep, 3, secret3); context.checking(new Expectations() {{ // start() @@ -297,11 +297,11 @@ public class KeyManagerImplTest extends BriarTestCase { will(returnValue(EPOCH + ROTATION_PERIOD)); // The secret for period 3 should be derived and stored oneOf(crypto).deriveNextSecret(secret0, 1); - will(returnValue(secret1.clone())); + will(returnValue(secret1)); oneOf(crypto).deriveNextSecret(secret1, 2); - will(returnValue(secret2.clone())); + will(returnValue(secret2)); oneOf(crypto).deriveNextSecret(secret2, 3); - will(returnValue(secret3.clone())); + will(returnValue(secret3)); oneOf(db).addSecrets(Arrays.asList(s3)); // The secrets for periods 1 - 3 should be added to the recogniser oneOf(tagRecogniser).addSecret(s1); @@ -336,12 +336,12 @@ public class KeyManagerImplTest extends BriarTestCase { // The DB contains the secrets for periods 0 - 2 Endpoint ep = new Endpoint(contactId, transportId, EPOCH, true); - final TemporarySecret s0 = new TemporarySecret(ep, 0, secret0.clone()); - final TemporarySecret s1 = new TemporarySecret(ep, 1, secret1.clone()); - final TemporarySecret s2 = new TemporarySecret(ep, 2, secret2.clone()); + final TemporarySecret s0 = new TemporarySecret(ep, 0, secret0); + final TemporarySecret s1 = new TemporarySecret(ep, 1, secret1); + final TemporarySecret s2 = new TemporarySecret(ep, 2, secret2); // The secrets for periods 3 and 4 should be derived and stored - final TemporarySecret s3 = new TemporarySecret(ep, 3, secret3.clone()); - final TemporarySecret s4 = new TemporarySecret(ep, 4, secret4.clone()); + final TemporarySecret s3 = new TemporarySecret(ep, 3, secret3); + final TemporarySecret s4 = new TemporarySecret(ep, 4, secret4); context.checking(new Expectations() {{ // start() @@ -356,11 +356,11 @@ public class KeyManagerImplTest extends BriarTestCase { will(returnValue(EPOCH + 3 * ROTATION_PERIOD - 1)); // The secrets for periods 3 and 4 should be derived from secret 1 oneOf(crypto).deriveNextSecret(secret1, 2); - will(returnValue(secret2.clone())); + will(returnValue(secret2)); oneOf(crypto).deriveNextSecret(secret2, 3); - will(returnValue(secret3.clone())); + will(returnValue(secret3)); oneOf(crypto).deriveNextSecret(secret3, 4); - will(returnValue(secret4.clone())); + will(returnValue(secret4)); // The new secrets should be stored oneOf(db).addSecrets(Arrays.asList(s3, s4)); // The secrets for periods 2 - 4 should be added to the recogniser @@ -396,9 +396,9 @@ public class KeyManagerImplTest extends BriarTestCase { // The DB contains the secrets for periods 0 - 2 Endpoint ep = new Endpoint(contactId, transportId, EPOCH, true); - final TemporarySecret s0 = new TemporarySecret(ep, 0, secret0.clone()); - final TemporarySecret s1 = new TemporarySecret(ep, 1, secret1.clone()); - final TemporarySecret s2 = new TemporarySecret(ep, 2, secret2.clone()); + final TemporarySecret s0 = new TemporarySecret(ep, 0, secret0); + final TemporarySecret s1 = new TemporarySecret(ep, 1, secret1); + final TemporarySecret s2 = new TemporarySecret(ep, 2, secret2); context.checking(new Expectations() {{ // start() @@ -459,11 +459,11 @@ public class KeyManagerImplTest extends BriarTestCase { // The DB contains the secrets for periods 0 - 2 Endpoint ep = new Endpoint(contactId, transportId, EPOCH, true); - final TemporarySecret s0 = new TemporarySecret(ep, 0, secret0.clone()); - final TemporarySecret s1 = new TemporarySecret(ep, 1, secret1.clone()); - final TemporarySecret s2 = new TemporarySecret(ep, 2, secret2.clone()); + final TemporarySecret s0 = new TemporarySecret(ep, 0, secret0); + final TemporarySecret s1 = new TemporarySecret(ep, 1, secret1); + final TemporarySecret s2 = new TemporarySecret(ep, 2, secret2); // The secret for period 3 should be derived and stored - final TemporarySecret s3 = new TemporarySecret(ep, 3, secret3.clone()); + final TemporarySecret s3 = new TemporarySecret(ep, 3, secret3); context.checking(new Expectations() {{ // start() @@ -486,11 +486,11 @@ public class KeyManagerImplTest extends BriarTestCase { oneOf(clock).currentTimeMillis(); will(returnValue(EPOCH + ROTATION_PERIOD + 1)); oneOf(crypto).deriveNextSecret(secret0, 1); - will(returnValue(secret1.clone())); + will(returnValue(secret1)); oneOf(crypto).deriveNextSecret(secret1, 2); - will(returnValue(secret2.clone())); + will(returnValue(secret2)); oneOf(crypto).deriveNextSecret(secret2, 3); - will(returnValue(secret3.clone())); + will(returnValue(secret3)); oneOf(tagRecogniser).removeSecret(contactId, transportId, 0); oneOf(db).addSecrets(Arrays.asList(s3)); oneOf(tagRecogniser).addSecret(s3); @@ -533,12 +533,12 @@ public class KeyManagerImplTest extends BriarTestCase { // The DB contains the secrets for periods 0 - 2 Endpoint ep = new Endpoint(contactId, transportId, EPOCH, true); - final TemporarySecret s0 = new TemporarySecret(ep, 0, secret0.clone()); - final TemporarySecret s1 = new TemporarySecret(ep, 1, secret1.clone()); - final TemporarySecret s2 = new TemporarySecret(ep, 2, secret2.clone()); + final TemporarySecret s0 = new TemporarySecret(ep, 0, secret0); + final TemporarySecret s1 = new TemporarySecret(ep, 1, secret1); + final TemporarySecret s2 = new TemporarySecret(ep, 2, secret2); // The secrets for periods 3 and 4 should be derived and stored - final TemporarySecret s3 = new TemporarySecret(ep, 3, secret3.clone()); - final TemporarySecret s4 = new TemporarySecret(ep, 4, secret4.clone()); + final TemporarySecret s3 = new TemporarySecret(ep, 3, secret3); + final TemporarySecret s4 = new TemporarySecret(ep, 4, secret4); context.checking(new Expectations() {{ // start() @@ -561,11 +561,11 @@ public class KeyManagerImplTest extends BriarTestCase { oneOf(clock).currentTimeMillis(); will(returnValue(EPOCH + 2 * ROTATION_PERIOD + 1)); oneOf(crypto).deriveNextSecret(secret1, 2); - will(returnValue(secret2.clone())); + will(returnValue(secret2)); oneOf(crypto).deriveNextSecret(secret2, 3); - will(returnValue(secret3.clone())); + will(returnValue(secret3)); oneOf(crypto).deriveNextSecret(secret3, 4); - will(returnValue(secret4.clone())); + will(returnValue(secret4)); oneOf(tagRecogniser).removeSecret(contactId, transportId, 0); oneOf(tagRecogniser).removeSecret(contactId, transportId, 1); oneOf(db).addSecrets(Arrays.asList(s3, s4)); diff --git a/briar-tests/src/org/briarproject/transport/KeyRotationIntegrationTest.java b/briar-tests/src/org/briarproject/transport/KeyRotationIntegrationTest.java index 70e8cfea3c..84b6f89289 100644 --- a/briar-tests/src/org/briarproject/transport/KeyRotationIntegrationTest.java +++ b/briar-tests/src/org/briarproject/transport/KeyRotationIntegrationTest.java @@ -40,6 +40,7 @@ public class KeyRotationIntegrationTest extends BriarTestCase { private final TransportId transportId; private final byte[] secret0, secret1, secret2, secret3, secret4; private final byte[] key0, key1, key2, key3, key4; + private final SecretKey k0, k1, k2, k3, k4; private final byte[] initialSecret; public KeyRotationIntegrationTest() { @@ -60,6 +61,11 @@ public class KeyRotationIntegrationTest extends BriarTestCase { key2 = new byte[32]; key3 = new byte[32]; key4 = new byte[32]; + k0 = new SecretKey(key0); + k1 = new SecretKey(key1); + k2 = new SecretKey(key2); + k3 = new SecretKey(key3); + k4 = new SecretKey(key4); for(int i = 0; i < key0.length; i++) key0[i] = 1; for(int i = 0; i < key1.length; i++) key1[i] = 2; for(int i = 0; i < key2.length; i++) key2[i] = 3; @@ -112,9 +118,6 @@ public class KeyRotationIntegrationTest extends BriarTestCase { final EventBus eventBus = context.mock(EventBus.class); final Clock clock = context.mock(Clock.class); final Timer timer = context.mock(Timer.class); - final SecretKey k0 = context.mock(SecretKey.class, "k0"); - final SecretKey k1 = context.mock(SecretKey.class, "k1"); - final SecretKey k2 = context.mock(SecretKey.class, "k2"); final TagRecogniser tagRecogniser = new TagRecogniserImpl(crypto, db); final KeyManagerImpl keyManager = new KeyManagerImpl(crypto, db, @@ -122,9 +125,9 @@ public class KeyRotationIntegrationTest extends BriarTestCase { // The secrets for periods 0 - 2 should be derived Endpoint ep = new Endpoint(contactId, transportId, EPOCH, true); - final TemporarySecret s0 = new TemporarySecret(ep, 0, secret0.clone()); - final TemporarySecret s1 = new TemporarySecret(ep, 1, secret1.clone()); - final TemporarySecret s2 = new TemporarySecret(ep, 2, secret2.clone()); + final TemporarySecret s0 = new TemporarySecret(ep, 0, secret0); + final TemporarySecret s1 = new TemporarySecret(ep, 1, secret1); + final TemporarySecret s2 = new TemporarySecret(ep, 2, secret2); context.checking(new Expectations() {{ // start() @@ -142,11 +145,11 @@ public class KeyRotationIntegrationTest extends BriarTestCase { oneOf(clock).currentTimeMillis(); will(returnValue(EPOCH)); oneOf(crypto).deriveNextSecret(initialSecret, 0); - will(returnValue(secret0.clone())); + will(returnValue(secret0)); oneOf(crypto).deriveNextSecret(secret0, 1); - will(returnValue(secret1.clone())); + will(returnValue(secret1)); oneOf(crypto).deriveNextSecret(secret1, 2); - will(returnValue(secret2.clone())); + will(returnValue(secret2)); oneOf(db).addSecrets(Arrays.asList(s0, s1, s2)); // The recogniser should derive the tags for period 0 oneOf(crypto).deriveTagKey(secret0, false); @@ -155,10 +158,7 @@ public class KeyRotationIntegrationTest extends BriarTestCase { oneOf(crypto).encodeTag(with(any(byte[].class)), with(k0), with((long) i)); will(new EncodeTagAction()); - oneOf(k0).getEncoded(); - will(returnValue(key0)); } - oneOf(k0).erase(); // The recogniser should derive the tags for period 1 oneOf(crypto).deriveTagKey(secret1, false); will(returnValue(k1)); @@ -166,10 +166,7 @@ public class KeyRotationIntegrationTest extends BriarTestCase { oneOf(crypto).encodeTag(with(any(byte[].class)), with(k1), with((long) i)); will(new EncodeTagAction()); - oneOf(k1).getEncoded(); - will(returnValue(key1)); } - oneOf(k1).erase(); // The recogniser should derive the tags for period 2 oneOf(crypto).deriveTagKey(secret2, false); will(returnValue(k2)); @@ -177,10 +174,7 @@ public class KeyRotationIntegrationTest extends BriarTestCase { oneOf(crypto).encodeTag(with(any(byte[].class)), with(k2), with((long) i)); will(new EncodeTagAction()); - oneOf(k2).getEncoded(); - will(returnValue(key2)); } - oneOf(k2).erase(); // stop() // The recogniser should derive the tags for period 0 oneOf(crypto).deriveTagKey(secret0, false); @@ -189,10 +183,7 @@ public class KeyRotationIntegrationTest extends BriarTestCase { oneOf(crypto).encodeTag(with(any(byte[].class)), with(k0), with((long) i)); will(new EncodeTagAction()); - oneOf(k0).getEncoded(); - will(returnValue(key0)); } - oneOf(k0).erase(); // The recogniser should derive the tags for period 1 oneOf(crypto).deriveTagKey(secret1, false); will(returnValue(k1)); @@ -200,10 +191,7 @@ public class KeyRotationIntegrationTest extends BriarTestCase { oneOf(crypto).encodeTag(with(any(byte[].class)), with(k1), with((long) i)); will(new EncodeTagAction()); - oneOf(k1).getEncoded(); - will(returnValue(key1)); } - oneOf(k1).erase(); // The recogniser should derive the tags for period 2 oneOf(crypto).deriveTagKey(secret2, false); will(returnValue(k2)); @@ -211,17 +199,14 @@ public class KeyRotationIntegrationTest extends BriarTestCase { oneOf(crypto).encodeTag(with(any(byte[].class)), with(k2), with((long) i)); will(new EncodeTagAction()); - oneOf(k2).getEncoded(); - will(returnValue(key2)); } - oneOf(k2).erase(); // Remove the listener and stop the timer oneOf(eventBus).removeListener(with(any(EventListener.class))); oneOf(timer).cancel(); }}); assertTrue(keyManager.start()); - keyManager.endpointAdded(ep, MAX_LATENCY, initialSecret.clone()); + keyManager.endpointAdded(ep, MAX_LATENCY, initialSecret); keyManager.stop(); context.assertIsSatisfied(); @@ -235,9 +220,6 @@ public class KeyRotationIntegrationTest extends BriarTestCase { final EventBus eventBus = context.mock(EventBus.class); final Clock clock = context.mock(Clock.class); final Timer timer = context.mock(Timer.class); - final SecretKey k0 = context.mock(SecretKey.class, "k0"); - final SecretKey k1 = context.mock(SecretKey.class, "k1"); - final SecretKey k2 = context.mock(SecretKey.class, "k2"); final TagRecogniser tagRecogniser = new TagRecogniserImpl(crypto, db); final KeyManagerImpl keyManager = new KeyManagerImpl(crypto, db, @@ -245,9 +227,9 @@ public class KeyRotationIntegrationTest extends BriarTestCase { // The secrets for periods 0 - 2 should be derived Endpoint ep = new Endpoint(contactId, transportId, EPOCH, true); - final TemporarySecret s0 = new TemporarySecret(ep, 0, secret0.clone()); - final TemporarySecret s1 = new TemporarySecret(ep, 1, secret1.clone()); - final TemporarySecret s2 = new TemporarySecret(ep, 2, secret2.clone()); + final TemporarySecret s0 = new TemporarySecret(ep, 0, secret0); + final TemporarySecret s1 = new TemporarySecret(ep, 1, secret1); + final TemporarySecret s2 = new TemporarySecret(ep, 2, secret2); context.checking(new Expectations() {{ // start() @@ -265,11 +247,11 @@ public class KeyRotationIntegrationTest extends BriarTestCase { oneOf(clock).currentTimeMillis(); will(returnValue(EPOCH)); oneOf(crypto).deriveNextSecret(initialSecret, 0); - will(returnValue(secret0.clone())); + will(returnValue(secret0)); oneOf(crypto).deriveNextSecret(secret0, 1); - will(returnValue(secret1.clone())); + will(returnValue(secret1)); oneOf(crypto).deriveNextSecret(secret1, 2); - will(returnValue(secret2.clone())); + will(returnValue(secret2)); oneOf(db).addSecrets(Arrays.asList(s0, s1, s2)); // The recogniser should derive the tags for period 0 oneOf(crypto).deriveTagKey(secret0, false); @@ -278,10 +260,7 @@ public class KeyRotationIntegrationTest extends BriarTestCase { oneOf(crypto).encodeTag(with(any(byte[].class)), with(k0), with((long) i)); will(new EncodeTagAction()); - oneOf(k0).getEncoded(); - will(returnValue(key0)); } - oneOf(k0).erase(); // The recogniser should derive the tags for period 1 oneOf(crypto).deriveTagKey(secret1, false); will(returnValue(k1)); @@ -289,10 +268,7 @@ public class KeyRotationIntegrationTest extends BriarTestCase { oneOf(crypto).encodeTag(with(any(byte[].class)), with(k1), with((long) i)); will(new EncodeTagAction()); - oneOf(k1).getEncoded(); - will(returnValue(key1)); } - oneOf(k1).erase(); // The recogniser should derive the tags for period 2 oneOf(crypto).deriveTagKey(secret2, false); will(returnValue(k2)); @@ -300,10 +276,7 @@ public class KeyRotationIntegrationTest extends BriarTestCase { oneOf(crypto).encodeTag(with(any(byte[].class)), with(k2), with((long) i)); will(new EncodeTagAction()); - oneOf(k2).getEncoded(); - will(returnValue(key2)); } - oneOf(k2).erase(); // getConnectionContext() oneOf(db).incrementStreamCounter(contactId, transportId, 1); will(returnValue(0L)); @@ -315,10 +288,7 @@ public class KeyRotationIntegrationTest extends BriarTestCase { oneOf(crypto).encodeTag(with(any(byte[].class)), with(k0), with((long) i)); will(new EncodeTagAction()); - oneOf(k0).getEncoded(); - will(returnValue(key0)); } - oneOf(k0).erase(); // The recogniser should derive the tags for period 1 oneOf(crypto).deriveTagKey(secret1, false); will(returnValue(k1)); @@ -326,10 +296,7 @@ public class KeyRotationIntegrationTest extends BriarTestCase { oneOf(crypto).encodeTag(with(any(byte[].class)), with(k1), with((long) i)); will(new EncodeTagAction()); - oneOf(k1).getEncoded(); - will(returnValue(key1)); } - oneOf(k1).erase(); // The recogniser should derive the tags for period 2 oneOf(crypto).deriveTagKey(secret2, false); will(returnValue(k2)); @@ -337,17 +304,14 @@ public class KeyRotationIntegrationTest extends BriarTestCase { oneOf(crypto).encodeTag(with(any(byte[].class)), with(k2), with((long) i)); will(new EncodeTagAction()); - oneOf(k2).getEncoded(); - will(returnValue(key2)); } - oneOf(k2).erase(); // Remove the listener and stop the timer oneOf(eventBus).removeListener(with(any(EventListener.class))); oneOf(timer).cancel(); }}); assertTrue(keyManager.start()); - keyManager.endpointAdded(ep, MAX_LATENCY, initialSecret.clone()); + keyManager.endpointAdded(ep, MAX_LATENCY, initialSecret); StreamContext ctx = keyManager.getStreamContext(contactId, transportId); assertNotNull(ctx); @@ -369,9 +333,6 @@ public class KeyRotationIntegrationTest extends BriarTestCase { final EventBus eventBus = context.mock(EventBus.class); final Clock clock = context.mock(Clock.class); final Timer timer = context.mock(Timer.class); - final SecretKey k0 = context.mock(SecretKey.class, "k0"); - final SecretKey k1 = context.mock(SecretKey.class, "k1"); - final SecretKey k2 = context.mock(SecretKey.class, "k2"); final TagRecogniser tagRecogniser = new TagRecogniserImpl(crypto, db); final KeyManagerImpl keyManager = new KeyManagerImpl(crypto, db, @@ -379,9 +340,9 @@ public class KeyRotationIntegrationTest extends BriarTestCase { // The secrets for periods 0 - 2 should be derived Endpoint ep = new Endpoint(contactId, transportId, EPOCH, true); - final TemporarySecret s0 = new TemporarySecret(ep, 0, secret0.clone()); - final TemporarySecret s1 = new TemporarySecret(ep, 1, secret1.clone()); - final TemporarySecret s2 = new TemporarySecret(ep, 2, secret2.clone()); + final TemporarySecret s0 = new TemporarySecret(ep, 0, secret0); + final TemporarySecret s1 = new TemporarySecret(ep, 1, secret1); + final TemporarySecret s2 = new TemporarySecret(ep, 2, secret2); context.checking(new Expectations() {{ // start() @@ -399,11 +360,11 @@ public class KeyRotationIntegrationTest extends BriarTestCase { oneOf(clock).currentTimeMillis(); will(returnValue(EPOCH)); oneOf(crypto).deriveNextSecret(initialSecret, 0); - will(returnValue(secret0.clone())); + will(returnValue(secret0)); oneOf(crypto).deriveNextSecret(secret0, 1); - will(returnValue(secret1.clone())); + will(returnValue(secret1)); oneOf(crypto).deriveNextSecret(secret1, 2); - will(returnValue(secret2.clone())); + will(returnValue(secret2)); oneOf(db).addSecrets(Arrays.asList(s0, s1, s2)); // The recogniser should derive the tags for period 0 oneOf(crypto).deriveTagKey(secret0, false); @@ -412,10 +373,7 @@ public class KeyRotationIntegrationTest extends BriarTestCase { oneOf(crypto).encodeTag(with(any(byte[].class)), with(k0), with((long) i)); will(new EncodeTagAction()); - oneOf(k0).getEncoded(); - will(returnValue(key0)); } - oneOf(k0).erase(); // The recogniser should derive the tags for period 1 oneOf(crypto).deriveTagKey(secret1, false); will(returnValue(k1)); @@ -423,10 +381,7 @@ public class KeyRotationIntegrationTest extends BriarTestCase { oneOf(crypto).encodeTag(with(any(byte[].class)), with(k1), with((long) i)); will(new EncodeTagAction()); - oneOf(k1).getEncoded(); - will(returnValue(key1)); } - oneOf(k1).erase(); // The recogniser should derive the tags for period 2 oneOf(crypto).deriveTagKey(secret2, false); will(returnValue(k2)); @@ -434,21 +389,15 @@ public class KeyRotationIntegrationTest extends BriarTestCase { oneOf(crypto).encodeTag(with(any(byte[].class)), with(k2), with((long) i)); will(new EncodeTagAction()); - oneOf(k2).getEncoded(); - will(returnValue(key2)); } - oneOf(k2).erase(); // acceptConnection() oneOf(crypto).deriveTagKey(secret2, false); will(returnValue(k2)); - oneOf(crypto).encodeTag(with(any(byte[].class)), - with(k2), with(16L)); + oneOf(crypto).encodeTag(with(any(byte[].class)), with(k2), + with(16L)); will(new EncodeTagAction()); - oneOf(k2).getEncoded(); - will(returnValue(key2)); oneOf(db).setReorderingWindow(contactId, transportId, 2, 1, new byte[] {0, 1, 0, 0}); - oneOf(k2).erase(); // stop() // The recogniser should derive the tags for period 0 oneOf(crypto).deriveTagKey(secret0, false); @@ -457,10 +406,7 @@ public class KeyRotationIntegrationTest extends BriarTestCase { oneOf(crypto).encodeTag(with(any(byte[].class)), with(k0), with((long) i)); will(new EncodeTagAction()); - oneOf(k0).getEncoded(); - will(returnValue(key0)); } - oneOf(k0).erase(); // The recogniser should derive the tags for period 1 oneOf(crypto).deriveTagKey(secret1, false); will(returnValue(k1)); @@ -468,10 +414,7 @@ public class KeyRotationIntegrationTest extends BriarTestCase { oneOf(crypto).encodeTag(with(any(byte[].class)), with(k1), with((long) i)); will(new EncodeTagAction()); - oneOf(k1).getEncoded(); - will(returnValue(key1)); } - oneOf(k1).erase(); // The recogniser should derive the updated tags for period 2 oneOf(crypto).deriveTagKey(secret2, false); will(returnValue(k2)); @@ -479,17 +422,14 @@ public class KeyRotationIntegrationTest extends BriarTestCase { oneOf(crypto).encodeTag(with(any(byte[].class)), with(k2), with((long) i)); will(new EncodeTagAction()); - oneOf(k2).getEncoded(); - will(returnValue(key2)); } - oneOf(k2).erase(); // Remove the listener and stop the timer oneOf(eventBus).removeListener(with(any(EventListener.class))); oneOf(timer).cancel(); }}); assertTrue(keyManager.start()); - keyManager.endpointAdded(ep, MAX_LATENCY, initialSecret.clone()); + keyManager.endpointAdded(ep, MAX_LATENCY, initialSecret); // Recognise the tag for connection 0 in period 2 byte[] tag = new byte[TAG_LENGTH]; encodeTag(tag, key2, 0); @@ -513,9 +453,6 @@ public class KeyRotationIntegrationTest extends BriarTestCase { final EventBus eventBus = context.mock(EventBus.class); final Clock clock = context.mock(Clock.class); final Timer timer = context.mock(Timer.class); - final SecretKey k0 = context.mock(SecretKey.class, "k0"); - final SecretKey k1 = context.mock(SecretKey.class, "k1"); - final SecretKey k2 = context.mock(SecretKey.class, "k2"); final TagRecogniser tagRecogniser = new TagRecogniserImpl(crypto, db); final KeyManagerImpl keyManager = new KeyManagerImpl(crypto, db, @@ -523,9 +460,9 @@ public class KeyRotationIntegrationTest extends BriarTestCase { // The DB contains the secrets for periods 0 - 2 Endpoint ep = new Endpoint(contactId, transportId, EPOCH, true); - final TemporarySecret s0 = new TemporarySecret(ep, 0, secret0.clone()); - final TemporarySecret s1 = new TemporarySecret(ep, 1, secret1.clone()); - final TemporarySecret s2 = new TemporarySecret(ep, 2, secret2.clone()); + final TemporarySecret s0 = new TemporarySecret(ep, 0, secret0); + final TemporarySecret s1 = new TemporarySecret(ep, 1, secret1); + final TemporarySecret s2 = new TemporarySecret(ep, 2, secret2); context.checking(new Expectations() {{ // start() @@ -545,10 +482,7 @@ public class KeyRotationIntegrationTest extends BriarTestCase { oneOf(crypto).encodeTag(with(any(byte[].class)), with(k0), with((long) i)); will(new EncodeTagAction()); - oneOf(k0).getEncoded(); - will(returnValue(key0)); } - oneOf(k0).erase(); // The recogniser should derive the tags for period 1 oneOf(crypto).deriveTagKey(secret1, false); will(returnValue(k1)); @@ -556,10 +490,7 @@ public class KeyRotationIntegrationTest extends BriarTestCase { oneOf(crypto).encodeTag(with(any(byte[].class)), with(k1), with((long) i)); will(new EncodeTagAction()); - oneOf(k1).getEncoded(); - will(returnValue(key1)); } - oneOf(k1).erase(); // The recogniser should derive the tags for period 2 oneOf(crypto).deriveTagKey(secret2, false); will(returnValue(k2)); @@ -567,10 +498,7 @@ public class KeyRotationIntegrationTest extends BriarTestCase { oneOf(crypto).encodeTag(with(any(byte[].class)), with(k2), with((long) i)); will(new EncodeTagAction()); - oneOf(k2).getEncoded(); - will(returnValue(key2)); } - oneOf(k2).erase(); // Start the timer oneOf(timer).scheduleAtFixedRate(with(keyManager), with(any(long.class)), with(any(long.class))); @@ -582,10 +510,7 @@ public class KeyRotationIntegrationTest extends BriarTestCase { oneOf(crypto).encodeTag(with(any(byte[].class)), with(k0), with((long) i)); will(new EncodeTagAction()); - oneOf(k0).getEncoded(); - will(returnValue(key0)); } - oneOf(k0).erase(); // The recogniser should derive the tags for period 1 oneOf(crypto).deriveTagKey(secret1, false); will(returnValue(k1)); @@ -593,10 +518,7 @@ public class KeyRotationIntegrationTest extends BriarTestCase { oneOf(crypto).encodeTag(with(any(byte[].class)), with(k1), with((long) i)); will(new EncodeTagAction()); - oneOf(k1).getEncoded(); - will(returnValue(key1)); } - oneOf(k1).erase(); // The recogniser should derive the tags for period 2 oneOf(crypto).deriveTagKey(secret2, false); will(returnValue(k2)); @@ -604,10 +526,7 @@ public class KeyRotationIntegrationTest extends BriarTestCase { oneOf(crypto).encodeTag(with(any(byte[].class)), with(k2), with((long) i)); will(new EncodeTagAction()); - oneOf(k2).getEncoded(); - will(returnValue(key2)); } - oneOf(k2).erase(); // Remove the listener and stop the timer oneOf(eventBus).removeListener(with(any(EventListener.class))); oneOf(timer).cancel(); @@ -627,9 +546,6 @@ public class KeyRotationIntegrationTest extends BriarTestCase { final EventBus eventBus = context.mock(EventBus.class); final Clock clock = context.mock(Clock.class); final Timer timer = context.mock(Timer.class); - final SecretKey k1 = context.mock(SecretKey.class, "k1"); - final SecretKey k2 = context.mock(SecretKey.class, "k2"); - final SecretKey k3 = context.mock(SecretKey.class, "k3"); final TagRecogniser tagRecogniser = new TagRecogniserImpl(crypto, db); final KeyManagerImpl keyManager = new KeyManagerImpl(crypto, db, @@ -637,11 +553,11 @@ public class KeyRotationIntegrationTest extends BriarTestCase { // The DB contains the secrets for periods 0 - 2 Endpoint ep = new Endpoint(contactId, transportId, EPOCH, true); - final TemporarySecret s0 = new TemporarySecret(ep, 0, secret0.clone()); - final TemporarySecret s1 = new TemporarySecret(ep, 1, secret1.clone()); - final TemporarySecret s2 = new TemporarySecret(ep, 2, secret2.clone()); + final TemporarySecret s0 = new TemporarySecret(ep, 0, secret0); + final TemporarySecret s1 = new TemporarySecret(ep, 1, secret1); + final TemporarySecret s2 = new TemporarySecret(ep, 2, secret2); // The secret for period 3 should be derived and stored - final TemporarySecret s3 = new TemporarySecret(ep, 3, secret3.clone()); + final TemporarySecret s3 = new TemporarySecret(ep, 3, secret3); context.checking(new Expectations() {{ // start() @@ -656,11 +572,11 @@ public class KeyRotationIntegrationTest extends BriarTestCase { will(returnValue(EPOCH + ROTATION_PERIOD)); // The secret for period 3 should be derived and stored oneOf(crypto).deriveNextSecret(secret0, 1); - will(returnValue(secret1.clone())); + will(returnValue(secret1)); oneOf(crypto).deriveNextSecret(secret1, 2); - will(returnValue(secret2.clone())); + will(returnValue(secret2)); oneOf(crypto).deriveNextSecret(secret2, 3); - will(returnValue(secret3.clone())); + will(returnValue(secret3)); oneOf(db).addSecrets(Arrays.asList(s3)); // The recogniser should derive the tags for period 1 oneOf(crypto).deriveTagKey(secret1, false); @@ -669,10 +585,7 @@ public class KeyRotationIntegrationTest extends BriarTestCase { oneOf(crypto).encodeTag(with(any(byte[].class)), with(k1), with((long) i)); will(new EncodeTagAction()); - oneOf(k1).getEncoded(); - will(returnValue(key1)); } - oneOf(k1).erase(); // The recogniser should derive the tags for period 2 oneOf(crypto).deriveTagKey(secret2, false); will(returnValue(k2)); @@ -680,10 +593,7 @@ public class KeyRotationIntegrationTest extends BriarTestCase { oneOf(crypto).encodeTag(with(any(byte[].class)), with(k2), with((long) i)); will(new EncodeTagAction()); - oneOf(k2).getEncoded(); - will(returnValue(key2)); } - oneOf(k2).erase(); // The recogniser should derive the tags for period 3 oneOf(crypto).deriveTagKey(secret3, false); will(returnValue(k3)); @@ -691,10 +601,7 @@ public class KeyRotationIntegrationTest extends BriarTestCase { oneOf(crypto).encodeTag(with(any(byte[].class)), with(k3), with((long) i)); will(new EncodeTagAction()); - oneOf(k3).getEncoded(); - will(returnValue(key3)); } - oneOf(k3).erase(); // Start the timer oneOf(timer).scheduleAtFixedRate(with(keyManager), with(any(long.class)), with(any(long.class))); @@ -706,10 +613,7 @@ public class KeyRotationIntegrationTest extends BriarTestCase { oneOf(crypto).encodeTag(with(any(byte[].class)), with(k1), with((long) i)); will(new EncodeTagAction()); - oneOf(k1).getEncoded(); - will(returnValue(key1)); } - oneOf(k1).erase(); // The recogniser should derive the tags for period 2 oneOf(crypto).deriveTagKey(secret2, false); will(returnValue(k2)); @@ -717,10 +621,7 @@ public class KeyRotationIntegrationTest extends BriarTestCase { oneOf(crypto).encodeTag(with(any(byte[].class)), with(k2), with((long) i)); will(new EncodeTagAction()); - oneOf(k2).getEncoded(); - will(returnValue(key2)); } - oneOf(k2).erase(); // The recogniser should remove the tags for period 3 oneOf(crypto).deriveTagKey(secret3, false); will(returnValue(k3)); @@ -728,10 +629,7 @@ public class KeyRotationIntegrationTest extends BriarTestCase { oneOf(crypto).encodeTag(with(any(byte[].class)), with(k3), with((long) i)); will(new EncodeTagAction()); - oneOf(k3).getEncoded(); - will(returnValue(key3)); } - oneOf(k3).erase(); // Remove the listener and stop the timer oneOf(eventBus).removeListener(with(any(EventListener.class))); oneOf(timer).cancel(); @@ -751,9 +649,6 @@ public class KeyRotationIntegrationTest extends BriarTestCase { final EventBus eventBus = context.mock(EventBus.class); final Clock clock = context.mock(Clock.class); final Timer timer = context.mock(Timer.class); - final SecretKey k2 = context.mock(SecretKey.class, "k2"); - final SecretKey k3 = context.mock(SecretKey.class, "k3"); - final SecretKey k4 = context.mock(SecretKey.class, "k4"); final TagRecogniser tagRecogniser = new TagRecogniserImpl(crypto, db); final KeyManagerImpl keyManager = new KeyManagerImpl(crypto, db, @@ -761,12 +656,12 @@ public class KeyRotationIntegrationTest extends BriarTestCase { // The DB contains the secrets for periods 0 - 2 Endpoint ep = new Endpoint(contactId, transportId, EPOCH, true); - final TemporarySecret s0 = new TemporarySecret(ep, 0, secret0.clone()); - final TemporarySecret s1 = new TemporarySecret(ep, 1, secret1.clone()); - final TemporarySecret s2 = new TemporarySecret(ep, 2, secret2.clone()); + final TemporarySecret s0 = new TemporarySecret(ep, 0, secret0); + final TemporarySecret s1 = new TemporarySecret(ep, 1, secret1); + final TemporarySecret s2 = new TemporarySecret(ep, 2, secret2); // The secrets for periods 3 and 4 should be derived and stored - final TemporarySecret s3 = new TemporarySecret(ep, 3, secret3.clone()); - final TemporarySecret s4 = new TemporarySecret(ep, 4, secret4.clone()); + final TemporarySecret s3 = new TemporarySecret(ep, 3, secret3); + final TemporarySecret s4 = new TemporarySecret(ep, 4, secret4); context.checking(new Expectations() {{ // start() @@ -781,11 +676,11 @@ public class KeyRotationIntegrationTest extends BriarTestCase { will(returnValue(EPOCH + 3 * ROTATION_PERIOD - 1)); // The secrets for periods 3 and 4 should be derived from secret 1 oneOf(crypto).deriveNextSecret(secret1, 2); - will(returnValue(secret2.clone())); + will(returnValue(secret2)); oneOf(crypto).deriveNextSecret(secret2, 3); - will(returnValue(secret3.clone())); + will(returnValue(secret3)); oneOf(crypto).deriveNextSecret(secret3, 4); - will(returnValue(secret4.clone())); + will(returnValue(secret4)); // The new secrets should be stored oneOf(db).addSecrets(Arrays.asList(s3, s4)); // The recogniser should derive the tags for period 2 @@ -795,10 +690,7 @@ public class KeyRotationIntegrationTest extends BriarTestCase { oneOf(crypto).encodeTag(with(any(byte[].class)), with(k2), with((long) i)); will(new EncodeTagAction()); - oneOf(k2).getEncoded(); - will(returnValue(key2)); } - oneOf(k2).erase(); // The recogniser should derive the tags for period 3 oneOf(crypto).deriveTagKey(secret3, false); will(returnValue(k3)); @@ -806,10 +698,7 @@ public class KeyRotationIntegrationTest extends BriarTestCase { oneOf(crypto).encodeTag(with(any(byte[].class)), with(k3), with((long) i)); will(new EncodeTagAction()); - oneOf(k3).getEncoded(); - will(returnValue(key3)); } - oneOf(k3).erase(); // The recogniser should derive the tags for period 4 oneOf(crypto).deriveTagKey(secret4, false); will(returnValue(k4)); @@ -817,10 +706,7 @@ public class KeyRotationIntegrationTest extends BriarTestCase { oneOf(crypto).encodeTag(with(any(byte[].class)), with(k4), with((long) i)); will(new EncodeTagAction()); - oneOf(k4).getEncoded(); - will(returnValue(key4)); } - oneOf(k4).erase(); // Start the timer oneOf(timer).scheduleAtFixedRate(with(keyManager), with(any(long.class)), with(any(long.class))); @@ -832,10 +718,7 @@ public class KeyRotationIntegrationTest extends BriarTestCase { oneOf(crypto).encodeTag(with(any(byte[].class)), with(k2), with((long) i)); will(new EncodeTagAction()); - oneOf(k2).getEncoded(); - will(returnValue(key2)); } - oneOf(k2).erase(); // The recogniser should remove the tags for period 3 oneOf(crypto).deriveTagKey(secret3, false); will(returnValue(k3)); @@ -843,10 +726,7 @@ public class KeyRotationIntegrationTest extends BriarTestCase { oneOf(crypto).encodeTag(with(any(byte[].class)), with(k3), with((long) i)); will(new EncodeTagAction()); - oneOf(k3).getEncoded(); - will(returnValue(key3)); } - oneOf(k3).erase(); // The recogniser should derive the tags for period 4 oneOf(crypto).deriveTagKey(secret4, false); will(returnValue(k4)); @@ -854,10 +734,7 @@ public class KeyRotationIntegrationTest extends BriarTestCase { oneOf(crypto).encodeTag(with(any(byte[].class)), with(k4), with((long) i)); will(new EncodeTagAction()); - oneOf(k4).getEncoded(); - will(returnValue(key4)); } - oneOf(k4).erase(); // Remove the listener and stop the timer oneOf(eventBus).removeListener(with(any(EventListener.class))); oneOf(timer).cancel(); @@ -885,7 +762,7 @@ public class KeyRotationIntegrationTest extends BriarTestCase { byte[] tag = (byte[]) invocation.getParameter(0); SecretKey key = (SecretKey) invocation.getParameter(1); long streamNumber = (Long) invocation.getParameter(2); - encodeTag(tag, key.getEncoded(), streamNumber); + encodeTag(tag, key.getBytes(), streamNumber); return null; } } diff --git a/briar-tests/src/org/briarproject/transport/TransportTagRecogniserTest.java b/briar-tests/src/org/briarproject/transport/TransportTagRecogniserTest.java index ba1e84d48a..340745489d 100644 --- a/briar-tests/src/org/briarproject/transport/TransportTagRecogniserTest.java +++ b/briar-tests/src/org/briarproject/transport/TransportTagRecogniserTest.java @@ -25,6 +25,7 @@ public class TransportTagRecogniserTest extends BriarTestCase { private final ContactId contactId = new ContactId(234); private final TransportId transportId = new TransportId("id"); + private final SecretKey tagKey = new SecretKey(new byte[32]); @Test public void testAddAndRemoveSecret() { @@ -33,7 +34,6 @@ public class TransportTagRecogniserTest extends BriarTestCase { final byte[] secret = new byte[32]; new Random().nextBytes(secret); final boolean alice = false; - final SecretKey tagKey = context.mock(SecretKey.class); final DatabaseComponent db = context.mock(DatabaseComponent.class); context.checking(new Expectations() {{ // Add secret @@ -44,7 +44,6 @@ public class TransportTagRecogniserTest extends BriarTestCase { with((long) i)); will(new EncodeTagAction()); } - oneOf(tagKey).erase(); // Remove secret oneOf(crypto).deriveTagKey(secret, !alice); will(returnValue(tagKey)); @@ -53,7 +52,6 @@ public class TransportTagRecogniserTest extends BriarTestCase { with((long) i)); will(new EncodeTagAction()); } - oneOf(tagKey).erase(); }}); TemporarySecret s = new TemporarySecret(contactId, transportId, 123, alice, 0, secret, 0, 0, new byte[4]); @@ -71,7 +69,6 @@ public class TransportTagRecogniserTest extends BriarTestCase { final byte[] secret = new byte[32]; new Random().nextBytes(secret); final boolean alice = false; - final SecretKey tagKey = context.mock(SecretKey.class); final DatabaseComponent db = context.mock(DatabaseComponent.class); context.checking(new Expectations() {{ // Add secret @@ -82,7 +79,6 @@ public class TransportTagRecogniserTest extends BriarTestCase { with((long) i)); will(new EncodeTagAction()); } - oneOf(tagKey).erase(); // Recognise tag 0 oneOf(crypto).deriveTagKey(secret, !alice); will(returnValue(tagKey)); @@ -93,7 +89,6 @@ public class TransportTagRecogniserTest extends BriarTestCase { // The updated window should be stored oneOf(db).setReorderingWindow(contactId, transportId, 0, 1, new byte[] {0, 1, 0, 0}); - oneOf(tagKey).erase(); // Recognise tag again - no expectations }}); TemporarySecret s = new TemporarySecret(contactId, transportId, 123, -- GitLab