From 6fab0e87e0cbf444b34756179135c8498f603459 Mon Sep 17 00:00:00 2001 From: akwizgran Date: Fri, 18 Dec 2015 12:25:07 +0000 Subject: [PATCH 1/4] Better variable names. --- .../org/briarproject/crypto/CryptoComponentImpl.java | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/briar-core/src/org/briarproject/crypto/CryptoComponentImpl.java b/briar-core/src/org/briarproject/crypto/CryptoComponentImpl.java index 4d62980cd..66c398516 100644 --- a/briar-core/src/org/briarproject/crypto/CryptoComponentImpl.java +++ b/briar-core/src/org/briarproject/crypto/CryptoComponentImpl.java @@ -94,16 +94,16 @@ class CryptoComponentImpl implements CryptoComponent { private final KeyParser agreementKeyParser, signatureKeyParser; @Inject - CryptoComponentImpl(SeedProvider r) { + CryptoComponentImpl(SeedProvider seedProvider) { if (!FortunaSecureRandom.selfTest()) throw new RuntimeException(); - SecureRandom secureRandom1 = new SecureRandom(); + SecureRandom platformSecureRandom = new SecureRandom(); if (LOG.isLoggable(INFO)) { - String provider = secureRandom1.getProvider().getName(); - String algorithm = secureRandom1.getAlgorithm(); + String provider = platformSecureRandom.getProvider().getName(); + String algorithm = platformSecureRandom.getAlgorithm(); LOG.info("Default SecureRandom: " + provider + " " + algorithm); } - SecureRandom secureRandom2 = new FortunaSecureRandom(r.getSeed()); - secureRandom = new CombinedSecureRandom(secureRandom1, secureRandom2); + SecureRandom fortuna = new FortunaSecureRandom(seedProvider.getSeed()); + secureRandom = new CombinedSecureRandom(platformSecureRandom, fortuna); ECKeyGenerationParameters params = new ECKeyGenerationParameters( PARAMETERS, secureRandom); agreementKeyPairGenerator = new ECKeyPairGenerator(); From fc897bd1b95f615191c1fef5f81745ee6923d634 Mon Sep 17 00:00:00 2001 From: akwizgran Date: Fri, 18 Dec 2015 12:30:06 +0000 Subject: [PATCH 2/4] Use XSalsa20-Poly1305 instead of AES-GCM. #111 --- .../api/transport/TransportConstants.java | 2 +- .../crypto/AuthenticatedCipherImpl.java | 57 ------------------- .../crypto/CryptoComponentImpl.java | 8 +-- .../org/briarproject/crypto/CryptoModule.java | 23 ++++---- ... XSalsa20Poly1305AuthenticatedCipher.java} | 18 +++--- ...lsa20Poly1305AuthenticatedCipherTest.java} | 25 ++++---- 6 files changed, 39 insertions(+), 94 deletions(-) delete mode 100644 briar-core/src/org/briarproject/crypto/AuthenticatedCipherImpl.java rename briar-core/src/org/briarproject/crypto/{XSalsa20Poly1305AC.java => XSalsa20Poly1305AuthenticatedCipher.java} (91%) rename briar-tests/src/org/briarproject/crypto/{XSalsa20Poly1305ACTest.java => XSalsa20Poly1305AuthenticatedCipherTest.java} (77%) diff --git a/briar-api/src/org/briarproject/api/transport/TransportConstants.java b/briar-api/src/org/briarproject/api/transport/TransportConstants.java index 219f5d481..ae3a6c3da 100644 --- a/briar-api/src/org/briarproject/api/transport/TransportConstants.java +++ b/briar-api/src/org/briarproject/api/transport/TransportConstants.java @@ -19,7 +19,7 @@ public interface TransportConstants { + MAC_LENGTH; /** The length of the frame initalisation vector (IV) in bytes. */ - int FRAME_IV_LENGTH = 12; + int FRAME_IV_LENGTH = 24; /** The length of the frame header in bytes. */ int FRAME_HEADER_LENGTH = 4 + MAC_LENGTH; diff --git a/briar-core/src/org/briarproject/crypto/AuthenticatedCipherImpl.java b/briar-core/src/org/briarproject/crypto/AuthenticatedCipherImpl.java deleted file mode 100644 index c21213287..000000000 --- a/briar-core/src/org/briarproject/crypto/AuthenticatedCipherImpl.java +++ /dev/null @@ -1,57 +0,0 @@ -package org.briarproject.crypto; - -import static org.briarproject.api.transport.TransportConstants.MAC_LENGTH; - -import java.security.GeneralSecurityException; - -import org.briarproject.api.crypto.SecretKey; -import org.spongycastle.crypto.DataLengthException; -import org.spongycastle.crypto.InvalidCipherTextException; -import org.spongycastle.crypto.engines.AESLightEngine; -import org.spongycastle.crypto.modes.AEADBlockCipher; -import org.spongycastle.crypto.modes.GCMBlockCipher; -import org.spongycastle.crypto.modes.gcm.BasicGCMMultiplier; -import org.spongycastle.crypto.params.AEADParameters; -import org.spongycastle.crypto.params.KeyParameter; - -class AuthenticatedCipherImpl implements AuthenticatedCipher { - - private final AEADBlockCipher cipher; - - AuthenticatedCipherImpl() { - cipher = new GCMBlockCipher(new AESLightEngine(), - new BasicGCMMultiplier()); - } - - public int process(byte[] input, int inputOff, int len, byte[] output, - int outputOff) throws GeneralSecurityException { - int processed = 0; - if (len != 0) { - processed = cipher.processBytes(input, inputOff, len, output, - outputOff); - } - try { - return processed + cipher.doFinal(output, outputOff + processed); - } catch (DataLengthException e) { - throw new GeneralSecurityException(e.getMessage()); - } catch (InvalidCipherTextException e) { - throw new GeneralSecurityException(e.getMessage()); - } - } - - public void init(boolean encrypt, SecretKey key, byte[] iv) - throws GeneralSecurityException { - KeyParameter k = new KeyParameter(key.getBytes()); - // Authenticate the IV by passing it as additional authenticated data - AEADParameters params = new AEADParameters(k, MAC_LENGTH * 8, iv, iv); - try { - cipher.init(encrypt, params); - } catch (IllegalArgumentException e) { - throw new GeneralSecurityException(e.getMessage()); - } - } - - public int getMacBytes() { - return MAC_LENGTH; - } -} diff --git a/briar-core/src/org/briarproject/crypto/CryptoComponentImpl.java b/briar-core/src/org/briarproject/crypto/CryptoComponentImpl.java index 66c398516..e1b58dd9d 100644 --- a/briar-core/src/org/briarproject/crypto/CryptoComponentImpl.java +++ b/briar-core/src/org/briarproject/crypto/CryptoComponentImpl.java @@ -55,8 +55,8 @@ class CryptoComponentImpl implements CryptoComponent { private static final int AGREEMENT_KEY_PAIR_BITS = 256; private static final int SIGNATURE_KEY_PAIR_BITS = 256; - private static final int STORAGE_IV_BYTES = 16; // 128 bits - private static final int PBKDF_SALT_BYTES = 16; // 128 bits + 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_TARGET_MILLIS = 500; private static final int PBKDF_SAMPLES = 30; @@ -325,7 +325,7 @@ class CryptoComponentImpl implements CryptoComponent { } public byte[] encryptWithPassword(byte[] input, String password) { - AuthenticatedCipher cipher = new AuthenticatedCipherImpl(); + AuthenticatedCipher cipher = new XSalsa20Poly1305AuthenticatedCipher(); int macBytes = cipher.getMacBytes(); // Generate a random salt byte[] salt = new byte[PBKDF_SALT_BYTES]; @@ -355,7 +355,7 @@ class CryptoComponentImpl implements CryptoComponent { } public byte[] decryptWithPassword(byte[] input, String password) { - AuthenticatedCipher cipher = new AuthenticatedCipherImpl(); + AuthenticatedCipher cipher = new XSalsa20Poly1305AuthenticatedCipher(); int macBytes = cipher.getMacBytes(); // The input contains the salt, iterations, IV, ciphertext and MAC if (input.length < PBKDF_SALT_BYTES + 4 + STORAGE_IV_BYTES + macBytes) diff --git a/briar-core/src/org/briarproject/crypto/CryptoModule.java b/briar-core/src/org/briarproject/crypto/CryptoModule.java index 3cdaf7404..35a4b0136 100644 --- a/briar-core/src/org/briarproject/crypto/CryptoModule.java +++ b/briar-core/src/org/briarproject/crypto/CryptoModule.java @@ -1,6 +1,14 @@ package org.briarproject.crypto; -import static java.util.concurrent.TimeUnit.SECONDS; +import com.google.inject.AbstractModule; +import com.google.inject.Provides; + +import org.briarproject.api.crypto.CryptoComponent; +import org.briarproject.api.crypto.CryptoExecutor; +import org.briarproject.api.crypto.PasswordStrengthEstimator; +import org.briarproject.api.crypto.StreamDecrypterFactory; +import org.briarproject.api.crypto.StreamEncrypterFactory; +import org.briarproject.api.lifecycle.LifecycleManager; import java.util.concurrent.BlockingQueue; import java.util.concurrent.Executor; @@ -11,15 +19,7 @@ import java.util.concurrent.ThreadPoolExecutor; import javax.inject.Singleton; -import org.briarproject.api.crypto.CryptoComponent; -import org.briarproject.api.crypto.CryptoExecutor; -import org.briarproject.api.crypto.PasswordStrengthEstimator; -import org.briarproject.api.crypto.StreamDecrypterFactory; -import org.briarproject.api.crypto.StreamEncrypterFactory; -import org.briarproject.api.lifecycle.LifecycleManager; - -import com.google.inject.AbstractModule; -import com.google.inject.Provides; +import static java.util.concurrent.TimeUnit.SECONDS; public class CryptoModule extends AbstractModule { @@ -42,7 +42,8 @@ public class CryptoModule extends AbstractModule { @Override protected void configure() { - bind(AuthenticatedCipher.class).to(AuthenticatedCipherImpl.class); + bind(AuthenticatedCipher.class).to( + XSalsa20Poly1305AuthenticatedCipher.class); bind(CryptoComponent.class).to( CryptoComponentImpl.class).in(Singleton.class); bind(PasswordStrengthEstimator.class).to( diff --git a/briar-core/src/org/briarproject/crypto/XSalsa20Poly1305AC.java b/briar-core/src/org/briarproject/crypto/XSalsa20Poly1305AuthenticatedCipher.java similarity index 91% rename from briar-core/src/org/briarproject/crypto/XSalsa20Poly1305AC.java rename to briar-core/src/org/briarproject/crypto/XSalsa20Poly1305AuthenticatedCipher.java index 7a4a39d90..bdfe36169 100644 --- a/briar-core/src/org/briarproject/crypto/XSalsa20Poly1305AC.java +++ b/briar-core/src/org/briarproject/crypto/XSalsa20Poly1305AuthenticatedCipher.java @@ -24,7 +24,8 @@ import static org.briarproject.api.transport.TransportConstants.MAC_LENGTH; *
  • http://cr.yp.to/highspeed/naclcrypto-20090310.pdf
    • * */ -public class XSalsa20Poly1305AC implements AuthenticatedCipher { +public class XSalsa20Poly1305AuthenticatedCipher + implements AuthenticatedCipher { /** Length of the padding to be used to generate the Poly1305 key */ private static final int SUBKEY_LENGTH = 32; @@ -34,13 +35,14 @@ public class XSalsa20Poly1305AC implements AuthenticatedCipher { private boolean encrypting; - XSalsa20Poly1305AC() { + XSalsa20Poly1305AuthenticatedCipher() { xSalsa20Engine = new XSalsa20Engine(); poly1305 = new Poly1305(); } @Override - public void init(boolean encrypt, SecretKey key, byte[] iv) throws GeneralSecurityException { + public void init(boolean encrypt, SecretKey key, byte[] iv) + throws GeneralSecurityException { encrypting = encrypt; KeyParameter k = new KeyParameter(key.getBytes()); ParametersWithIV params = new ParametersWithIV(k, iv); @@ -52,12 +54,10 @@ public class XSalsa20Poly1305AC implements AuthenticatedCipher { } @Override - public int process(byte[] input, int inputOff, int len, byte[] output, int outputOff) throws GeneralSecurityException { - if (len == 0) - return 0; - else if (!encrypting && len < MAC_LENGTH) + public int process(byte[] input, int inputOff, int len, byte[] output, + int outputOff) throws GeneralSecurityException { + if (!encrypting && len < MAC_LENGTH) throw new GeneralSecurityException("Invalid MAC"); - try { // Generate the Poly1305 subkey from an empty array byte[] zero = new byte[SUBKEY_LENGTH]; @@ -100,7 +100,7 @@ public class XSalsa20Poly1305AC implements AuthenticatedCipher { throw new GeneralSecurityException("Invalid MAC"); } - // Invert the stream encryption + // Apply or invert the stream encryption int processed = xSalsa20Engine.processBytes( input, encrypting ? inputOff : inputOff + MAC_LENGTH, encrypting ? len : len - MAC_LENGTH, diff --git a/briar-tests/src/org/briarproject/crypto/XSalsa20Poly1305ACTest.java b/briar-tests/src/org/briarproject/crypto/XSalsa20Poly1305AuthenticatedCipherTest.java similarity index 77% rename from briar-tests/src/org/briarproject/crypto/XSalsa20Poly1305ACTest.java rename to briar-tests/src/org/briarproject/crypto/XSalsa20Poly1305AuthenticatedCipherTest.java index 723d7984a..882866769 100644 --- a/briar-tests/src/org/briarproject/crypto/XSalsa20Poly1305ACTest.java +++ b/briar-tests/src/org/briarproject/crypto/XSalsa20Poly1305AuthenticatedCipherTest.java @@ -6,10 +6,11 @@ import org.briarproject.util.StringUtils; import org.junit.Test; import java.security.GeneralSecurityException; +import java.util.Random; import static org.junit.Assert.assertArrayEquals; -public class XSalsa20Poly1305ACTest extends BriarTestCase { +public class XSalsa20Poly1305AuthenticatedCipherTest extends BriarTestCase { // Test vectors from the NaCl paper // http://cr.yp.to/highspeed/naclcrypto-20090310.pdf @@ -47,9 +48,9 @@ public class XSalsa20Poly1305ACTest extends BriarTestCase { @Test public void testEncrypt() throws Exception { SecretKey k = new SecretKey(TEST_KEY); - AuthenticatedCipher cipher = new XSalsa20Poly1305AC(); + AuthenticatedCipher cipher = new XSalsa20Poly1305AuthenticatedCipher(); cipher.init(true, k, TEST_IV); - byte[] output = new byte[TEST_PLAINTEXT.length + cipher.getMacBytes()]; + byte[] output = new byte[TEST_CIPHERTEXT.length]; cipher.process(TEST_PLAINTEXT, 0, TEST_PLAINTEXT.length, output, 0); assertArrayEquals(TEST_CIPHERTEXT, output); } @@ -57,9 +58,9 @@ public class XSalsa20Poly1305ACTest extends BriarTestCase { @Test public void testDecrypt() throws Exception { SecretKey k = new SecretKey(TEST_KEY); - AuthenticatedCipher cipher = new XSalsa20Poly1305AC(); + AuthenticatedCipher cipher = new XSalsa20Poly1305AuthenticatedCipher(); cipher.init(false, k, TEST_IV); - byte[] output = new byte[TEST_CIPHERTEXT.length - cipher.getMacBytes()]; + byte[] output = new byte[TEST_PLAINTEXT.length]; cipher.process(TEST_CIPHERTEXT, 0, TEST_CIPHERTEXT.length, output, 0); assertArrayEquals(TEST_PLAINTEXT, output); } @@ -67,23 +68,23 @@ public class XSalsa20Poly1305ACTest extends BriarTestCase { @Test(expected = GeneralSecurityException.class) public void testDecryptFailsWithShortInput() throws Exception { SecretKey k = new SecretKey(TEST_KEY); - AuthenticatedCipher cipher = new XSalsa20Poly1305AC(); + AuthenticatedCipher cipher = new XSalsa20Poly1305AuthenticatedCipher(); cipher.init(false, k, TEST_IV); - byte[] input = new byte[8]; - System.arraycopy(TEST_CIPHERTEXT, 0, input, 0, 8); - byte[] output = new byte[TEST_CIPHERTEXT.length - cipher.getMacBytes()]; + byte[] input = new byte[cipher.getMacBytes() - 1]; + System.arraycopy(TEST_CIPHERTEXT, 0, input, 0, input.length); + byte[] output = new byte[TEST_PLAINTEXT.length]; cipher.process(input, 0, input.length, output, 0); } @Test(expected = GeneralSecurityException.class) public void testDecryptFailsWithAlteredCiphertext() throws Exception { SecretKey k = new SecretKey(TEST_KEY); - AuthenticatedCipher cipher = new XSalsa20Poly1305AC(); + AuthenticatedCipher cipher = new XSalsa20Poly1305AuthenticatedCipher(); cipher.init(false, k, TEST_IV); byte[] input = new byte[TEST_CIPHERTEXT.length]; System.arraycopy(TEST_CIPHERTEXT, 0, input, 0, TEST_CIPHERTEXT.length); - input[TEST_CIPHERTEXT.length - cipher.getMacBytes()] = 42; - byte[] output = new byte[TEST_CIPHERTEXT.length - cipher.getMacBytes()]; + input[new Random().nextInt(TEST_CIPHERTEXT.length)] ^= 0xFF; + byte[] output = new byte[TEST_PLAINTEXT.length]; cipher.process(input, 0, input.length, output, 0); } } From 73bb3b00653584da5c013f8f60aa67422e50bfa6 Mon Sep 17 00:00:00 2001 From: akwizgran Date: Fri, 18 Dec 2015 14:06:47 +0000 Subject: [PATCH 3/4] Fixed return value of process(). --- .../briarproject/crypto/AuthenticatedCipher.java | 16 ++++++++-------- .../XSalsa20Poly1305AuthenticatedCipher.java | 2 +- .../XSalsa20Poly1305AuthenticatedCipherTest.java | 7 +++++-- 3 files changed, 14 insertions(+), 11 deletions(-) diff --git a/briar-core/src/org/briarproject/crypto/AuthenticatedCipher.java b/briar-core/src/org/briarproject/crypto/AuthenticatedCipher.java index 5ee21e3e4..fe4f8bee0 100644 --- a/briar-core/src/org/briarproject/crypto/AuthenticatedCipher.java +++ b/briar-core/src/org/briarproject/crypto/AuthenticatedCipher.java @@ -26,14 +26,14 @@ interface AuthenticatedCipher { * including the MAC. * @param inputOff the offset into the input array where the data to be * processed starts. - * @param len the number of bytes to be processed. If decrypting, includes - * the MAC length. - * @param output the output buffer the processed bytes go into. If - * encrypting, the ciphertext including the MAC. If - * decrypting, the plaintext. - * @param outputOff the offset into the output byte array the processed - * data starts at. - * @return the number of bytes processed. + * @param len the length of the input. If decrypting, includes the MAC + * length. + * @param output the output byte array. If encrypting, the ciphertext + * including the MAC. If decrypting, the plaintext. + * @param outputOff the offset into the output byte array where the + * processed data starts. + * @return the length of the output. If encrypting, includes the MAC + * length. * @throws GeneralSecurityException on invalid input. */ int process(byte[] input, int inputOff, int len, byte[] output, diff --git a/briar-core/src/org/briarproject/crypto/XSalsa20Poly1305AuthenticatedCipher.java b/briar-core/src/org/briarproject/crypto/XSalsa20Poly1305AuthenticatedCipher.java index bdfe36169..8143b696a 100644 --- a/briar-core/src/org/briarproject/crypto/XSalsa20Poly1305AuthenticatedCipher.java +++ b/briar-core/src/org/briarproject/crypto/XSalsa20Poly1305AuthenticatedCipher.java @@ -112,7 +112,7 @@ public class XSalsa20Poly1305AuthenticatedCipher poly1305.doFinal(output, outputOff); } - return processed; + return encrypting ? processed + MAC_LENGTH : processed; } catch (DataLengthException e) { throw new GeneralSecurityException(e.getMessage()); } diff --git a/briar-tests/src/org/briarproject/crypto/XSalsa20Poly1305AuthenticatedCipherTest.java b/briar-tests/src/org/briarproject/crypto/XSalsa20Poly1305AuthenticatedCipherTest.java index 882866769..6d45fc975 100644 --- a/briar-tests/src/org/briarproject/crypto/XSalsa20Poly1305AuthenticatedCipherTest.java +++ b/briar-tests/src/org/briarproject/crypto/XSalsa20Poly1305AuthenticatedCipherTest.java @@ -9,6 +9,7 @@ import java.security.GeneralSecurityException; import java.util.Random; import static org.junit.Assert.assertArrayEquals; +import static org.junit.Assert.assertEquals; public class XSalsa20Poly1305AuthenticatedCipherTest extends BriarTestCase { @@ -51,7 +52,8 @@ public class XSalsa20Poly1305AuthenticatedCipherTest extends BriarTestCase { AuthenticatedCipher cipher = new XSalsa20Poly1305AuthenticatedCipher(); cipher.init(true, k, TEST_IV); byte[] output = new byte[TEST_CIPHERTEXT.length]; - cipher.process(TEST_PLAINTEXT, 0, TEST_PLAINTEXT.length, output, 0); + assertEquals(TEST_CIPHERTEXT.length, cipher.process(TEST_PLAINTEXT, 0, + TEST_PLAINTEXT.length, output, 0)); assertArrayEquals(TEST_CIPHERTEXT, output); } @@ -61,7 +63,8 @@ public class XSalsa20Poly1305AuthenticatedCipherTest extends BriarTestCase { AuthenticatedCipher cipher = new XSalsa20Poly1305AuthenticatedCipher(); cipher.init(false, k, TEST_IV); byte[] output = new byte[TEST_PLAINTEXT.length]; - cipher.process(TEST_CIPHERTEXT, 0, TEST_CIPHERTEXT.length, output, 0); + assertEquals(TEST_PLAINTEXT.length, cipher.process(TEST_CIPHERTEXT, 0, + TEST_CIPHERTEXT.length, output, 0)); assertArrayEquals(TEST_PLAINTEXT, output); } From ec3eafbb3ec37893e5b24622c5245cc3413da989 Mon Sep 17 00:00:00 2001 From: akwizgran Date: Fri, 18 Dec 2015 14:15:44 +0000 Subject: [PATCH 4/4] If Bluetooth was enabled, reuse invitation connection. --- .../android/invitation/AddContactActivity.java | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/briar-android/src/org/briarproject/android/invitation/AddContactActivity.java b/briar-android/src/org/briarproject/android/invitation/AddContactActivity.java index 011f7d38d..7d2ab1341 100644 --- a/briar-android/src/org/briarproject/android/invitation/AddContactActivity.java +++ b/briar-android/src/org/briarproject/android/invitation/AddContactActivity.java @@ -55,12 +55,12 @@ implements InvitationListener { private boolean localCompared = false, remoteCompared = false; private boolean localMatched = false, remoteMatched = false; private String contactName = null; - private boolean bluetoothWasEnabled = false; // Fields that are accessed from background threads must be volatile @Inject private volatile DatabaseComponent db; @Inject private volatile IdentityManager identityManager; - private volatile boolean leaveBluetoothEnabled = true; + private volatile boolean bluetoothWasEnabled = false; + private volatile boolean leaveBluetoothEnabled = false; @Override public void onCreate(Bundle state) { @@ -173,7 +173,8 @@ implements InvitationListener { long duration = System.currentTimeMillis() - now; if (LOG.isLoggable(INFO)) LOG.info("Loading setting took " + duration + " ms"); - leaveBluetoothEnabled = c.getBoolean("enable", false); + leaveBluetoothEnabled = bluetoothWasEnabled + || c.getBoolean("enable", false); } catch (DbException e) { if (LOG.isLoggable(WARNING)) LOG.log(WARNING, e.toString(), e); @@ -328,7 +329,7 @@ implements InvitationListener { } public void disableBluetooth() { - if (!bluetoothWasEnabled && !leaveBluetoothEnabled) { + if (!leaveBluetoothEnabled) { if (LOG.isLoggable(INFO)) LOG.info("Turning off Bluetooth again"); BluetoothAdapter adapter = BluetoothAdapter.getDefaultAdapter();