From fc897bd1b95f615191c1fef5f81745ee6923d634 Mon Sep 17 00:00:00 2001
From: akwizgran <akwizgran@users.sourceforge.net>
Date: Fri, 18 Dec 2015 12:30:06 +0000
Subject: [PATCH] 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 219f5d4814..ae3a6c3daf 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 c212132873..0000000000
--- 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 66c3985162..e1b58dd9d8 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 3cdaf74041..35a4b01360 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 7a4a39d90b..bdfe361697 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;
  *     <li>http://cr.yp.to/highspeed/naclcrypto-20090310.pdf</li>
  * </ul>
  */
-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 723d7984ad..8828667699 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);
 	}
 }
-- 
GitLab