Removed double-encryption of shared secrets.

api/net/sf/briar/api/crypto/SecretStorageKey.java

@@ -1,18 +0,0 @@
-package net.sf.briar.api.crypto;
-
-import static java.lang.annotation.ElementType.PARAMETER;
-import static java.lang.annotation.RetentionPolicy.RUNTIME;
-
-import java.lang.annotation.Retention;
-import java.lang.annotation.Target;
-
-import com.google.inject.BindingAnnotation;
-
-/**
- * Annotation for injecting the key that is used for encrypting and decrypting
- * secrets stored in the database.
- */
-@BindingAnnotation
-@Target({ PARAMETER })
-@Retention(RUNTIME)
-public @interface SecretStorageKey {}

components/net/sf/briar/crypto/CryptoComponentImpl.java

@@ -1,8 +1,6 @@
 package net.sf.briar.crypto;
 
 import java.io.UnsupportedEncodingException;
-import java.security.InvalidAlgorithmParameterException;
-import java.security.InvalidKeyException;
 import java.security.KeyPair;
 import java.security.KeyPairGenerator;
 import java.security.NoSuchAlgorithmException;
@@ -10,22 +8,17 @@ import java.security.NoSuchProviderException;
 import java.security.SecureRandom;
 import java.security.Security;
 import java.security.Signature;
-import java.util.Arrays;
 
-import javax.crypto.BadPaddingException;
 import javax.crypto.Cipher;
-import javax.crypto.IllegalBlockSizeException;
 import javax.crypto.KeyGenerator;
 import javax.crypto.Mac;
 import javax.crypto.NoSuchPaddingException;
 import javax.crypto.SecretKey;
-import javax.crypto.spec.IvParameterSpec;
 import javax.crypto.spec.SecretKeySpec;
 
 import net.sf.briar.api.crypto.CryptoComponent;
 import net.sf.briar.api.crypto.KeyParser;
 import net.sf.briar.api.crypto.MessageDigest;
-import net.sf.briar.api.crypto.SecretStorageKey;
 
 import org.bouncycastle.jce.provider.BouncyCastleProvider;
 
@@ -37,7 +30,6 @@ class CryptoComponentImpl implements CryptoComponent {
 	private static final String DIGEST_ALGO = "SHA-256";
 	private static final String KEY_PAIR_ALGO = "ECDSA";
 	private static final int KEY_PAIR_BITS = 256;
-	private static final String SECRET_STORAGE_ALGO = "AES/CTR/NoPadding";
 	private static final String FRAME_CIPHER_ALGO = "AES/CTR/NoPadding";
 	private static final String SECRET_KEY_ALGO = "AES";
 	private static final int SECRET_KEY_BITS = 256;
@@ -45,15 +37,13 @@ class CryptoComponentImpl implements CryptoComponent {
 	private static final String MAC_ALGO = "HMacSHA256";
 	private static final String SIGNATURE_ALGO = "ECDSA";
 
-	private final SecretKey secretStorageKey;
 	private final KeyParser keyParser;
 	private final KeyPairGenerator keyPairGenerator;
 	private final KeyGenerator keyGenerator;
 
 	@Inject
-	CryptoComponentImpl(@SecretStorageKey SecretKey secretStorageKey) {
+	CryptoComponentImpl() {
 		Security.addProvider(new BouncyCastleProvider());
-		this.secretStorageKey = secretStorageKey;
 		try {
 			keyParser = new KeyParserImpl(KEY_PAIR_ALGO, PROVIDER);
 			keyPairGenerator = KeyPairGenerator.getInstance(KEY_PAIR_ALGO,
@@ -75,54 +65,29 @@ class CryptoComponentImpl implements CryptoComponent {
 	}
 
 	private SecretKey deriveFrameKey(SharedSecret s, boolean alice) {
-		if(alice) return deriveKey("F_A", s.getIv(), s.getCiphertext());
+		if(alice) return deriveKey("F_A", s.getSecret());
-		else return deriveKey("F_B", s.getIv(), s.getCiphertext());
+		else return deriveKey("F_B", s.getSecret());
 	}
 
-	private SecretKey deriveKey(String name, IvParameterSpec iv,
+	private SecretKey deriveKey(String name, byte[] secret) {
-			byte[] ciphertext) {
 		MessageDigest digest = getMessageDigest();
 		try {
 			digest.update(name.getBytes("UTF-8"));
 		} catch(UnsupportedEncodingException e) {
 			throw new RuntimeException(e);
 		}
-		byte[] decrypted = decryptSharedSecret(iv, ciphertext);
+		digest.update(secret);
-		digest.update(decrypted);
-		Arrays.fill(decrypted, (byte) 0); // Destroy the plaintext secret
 		return new SecretKeySpec(digest.digest(), SECRET_KEY_ALGO);
 	}
 
-	private byte[] decryptSharedSecret(IvParameterSpec iv, byte[] ciphertext) {
-		try {
-			Cipher c = Cipher.getInstance(SECRET_STORAGE_ALGO, PROVIDER);
-			c.init(Cipher.DECRYPT_MODE, secretStorageKey, iv);
-			return c.doFinal(ciphertext);
-		} catch(BadPaddingException e) {
-			throw new RuntimeException(e);
-		} catch(IllegalBlockSizeException e) {
-			throw new RuntimeException(e);
-		} catch(InvalidAlgorithmParameterException e) {
-			throw new RuntimeException(e);
-		} catch(InvalidKeyException e) {
-			throw new RuntimeException(e);
-		} catch(NoSuchAlgorithmException e) {
-			throw new RuntimeException(e);
-		} catch(NoSuchPaddingException e) {
-			throw new RuntimeException(e);
-		} catch(NoSuchProviderException e) {
-			throw new RuntimeException(e);
-		}
-	}
-
 	public SecretKey deriveIncomingIvKey(byte[] secret) {
 		SharedSecret s = new SharedSecret(secret);
 		return deriveIvKey(s, !s.getAlice());
 	}
 
 	private SecretKey deriveIvKey(SharedSecret s, boolean alice) {
-		if(alice) return deriveKey("I_A", s.getIv(), s.getCiphertext());
+		if(alice) return deriveKey("I_A", s.getSecret());
-		else return deriveKey("I_B", s.getIv(), s.getCiphertext());
+		else return deriveKey("I_B", s.getSecret());
 	}
 
 	public SecretKey deriveIncomingMacKey(byte[] secret) {
@@ -131,8 +96,8 @@ class CryptoComponentImpl implements CryptoComponent {
 	}
 
 	private SecretKey deriveMacKey(SharedSecret s, boolean alice) {
-		if(alice) return deriveKey("M_A", s.getIv(), s.getCiphertext());
+		if(alice) return deriveKey("M_A", s.getSecret());
-		else return deriveKey("M_B", s.getIv(), s.getCiphertext());
+		else return deriveKey("M_B", s.getSecret());
 	}
 
 	public SecretKey deriveOutgoingFrameKey(byte[] secret) {

components/net/sf/briar/crypto/CryptoModule.java

@@ -1,10 +1,6 @@
 package net.sf.briar.crypto;
 
-import javax.crypto.SecretKey;
-import javax.crypto.spec.SecretKeySpec;
-
 import net.sf.briar.api.crypto.CryptoComponent;
-import net.sf.briar.api.crypto.SecretStorageKey;
 
 import com.google.inject.AbstractModule;
 import com.google.inject.Singleton;
@@ -15,8 +11,5 @@ public class CryptoModule extends AbstractModule {
 	protected void configure() {
 		bind(CryptoComponent.class).to(
 				CryptoComponentImpl.class).in(Singleton.class);
-		// FIXME: Use a real key
-		bind(SecretKey.class).annotatedWith(SecretStorageKey.class).toInstance(
-				new SecretKeySpec(new byte[32], "AES"));
 	}
 }

components/net/sf/briar/crypto/SharedSecret.java

@@ -1,29 +1,19 @@
 package net.sf.briar.crypto;
 
-import javax.crypto.spec.IvParameterSpec;
-
 /**
- * An encrypted shared secret from which authentication and encryption keys can
+ * A shared secret from which authentication and encryption keys can be derived.
- * be derived. The encrypted secret carries an IV for encrypting and decrypting
+ * The secret carries a flag indicating whether Alice's keys or Bob's keys
- * it and a flag indicating whether Alice's keys or Bob's keys should be
+ * should be derived from the secret. When two parties agree on a shared secret,
- * derived from the secret.
+ * they must decide which of them will derive Alice's keys and which Bob's.
- * <p>
- * When two parties agree on a shared secret, they must determine which of them
- * will derive Alice's keys and which Bob's. Each party then encrypts the
- * secret with an independent key and IV.
  */
 class SharedSecret {
 
-	static final int IV_BYTES = 16;
-
-	private final IvParameterSpec iv;
 	private final boolean alice;
-	private final byte[] ciphertext;
+	private final byte[] secret;
 
-	SharedSecret(byte[] secret) {
+	SharedSecret(byte[] b) {
-		if(secret.length < IV_BYTES + 2) throw new IllegalArgumentException();
+		if(b.length < 2) throw new IllegalArgumentException();
-		iv = new IvParameterSpec(secret, 0, IV_BYTES);
+		switch(b[0]) {
-		switch(secret[IV_BYTES]) {
 		case 0:
 			alice = false;
 			break;
@@ -33,21 +23,13 @@ class SharedSecret {
 		default:
 			throw new IllegalArgumentException();
 		}
-		ciphertext = new byte[secret.length - IV_BYTES - 1];
+		secret = new byte[b.length - 1];
-		System.arraycopy(secret, IV_BYTES + 1, ciphertext, 0,
+		System.arraycopy(b, 1, secret, 0, secret.length);
-				ciphertext.length);
 	}
 
-	SharedSecret(IvParameterSpec iv, boolean alice, byte[] ciphertext) {
+	SharedSecret(boolean alice, byte[] secret) {
-		if(iv.getIV().length != IV_BYTES) throw new IllegalArgumentException();
-		this.iv = iv;
 		this.alice = alice;
-		this.ciphertext = ciphertext;
+		this.secret = secret;
-	}
-
-	/** Returns the IV used for encrypting and decrypting the secret. */
-	IvParameterSpec getIv() {
-		return iv;
 	}
 
 	/**
@@ -58,22 +40,19 @@ class SharedSecret {
 		return alice;
 	}
 
-	/** Returns the encrypted shared secret. */
+	/** Returns the shared secret. */
-	byte[] getCiphertext() {
+	byte[] getSecret() {
-		return ciphertext;
+		return secret;
 	}
 
 	/**
-	 * Returns a raw representation of the encrypted shared secret, suitable
+	 * Returns a raw representation of this object, suitable for storing in the
-	 * for storing in the database.
+	 * database.
 	 */
 	byte[] getBytes() {
-		byte[] b = new byte[IV_BYTES + 1 + ciphertext.length];
+		byte[] b = new byte[1 + secret.length];
-		byte[] ivBytes = iv.getIV();
+		if(alice) b[0] = (byte) 1;
-		assert ivBytes.length == IV_BYTES;
+		System.arraycopy(secret, 0, b, 1, secret.length);
-		System.arraycopy(ivBytes, 0, b, 0, IV_BYTES);
-		if(alice) b[IV_BYTES] = (byte) 1;
-		System.arraycopy(ciphertext, 0, b, IV_BYTES + 1, ciphertext.length);
 		return b;
 	}
 }

test/net/sf/briar/ProtocolIntegrationTest.java

@@ -97,9 +97,9 @@ public class ProtocolIntegrationTest extends TestCase {
 		assertEquals(crypto.getMessageDigest().getDigestLength(),
 				UniqueId.LENGTH);
 		// Create matching secrets: one for Alice, one for Bob
-		aliceSecret = new byte[45];
+		aliceSecret = new byte[123];
-		aliceSecret[16] = (byte) 1;
+		aliceSecret[0] = (byte) 1;
-		bobSecret = new byte[45];
+		bobSecret = new byte[123];
 		// Create two groups: one restricted, one unrestricted
 		GroupFactory groupFactory = i.getInstance(GroupFactory.class);
 		group = groupFactory.createGroup("Unrestricted group", null);

test/net/sf/briar/crypto/CryptoComponentTest.java

@@ -22,7 +22,7 @@ public class CryptoComponentTest extends TestCase {
 	public void testKeyDerivation() {
 		// Create matching secrets: one for Alice, one for Bob
 		byte[] aliceSecret = new byte[123];
-		aliceSecret[SharedSecret.IV_BYTES] = (byte) 1;
+		aliceSecret[0] = (byte) 1;
 		byte[] bobSecret = new byte[123];
 		// Check that Alice's incoming keys match Bob's outgoing keys
 		assertEquals(crypto.deriveIncomingMacKey(aliceSecret),

test/net/sf/briar/crypto/SharedSecretTest.java

@@ -15,22 +15,22 @@ public class SharedSecretTest extends TestCase {
 		Random random = new Random();
 		byte[] secret = new byte[40];
 		random.nextBytes(secret);
-		secret[SharedSecret.IV_BYTES] = (byte) 0;
+		secret[0] = (byte) 0;
 		SharedSecret s = new SharedSecret(secret);
 		assertArrayEquals(secret, s.getBytes());
-		secret[SharedSecret.IV_BYTES] = (byte) 1;
+		secret[0] = (byte) 1;
 		s = new SharedSecret(secret);
 		assertArrayEquals(secret, s.getBytes());
 		// The Alice flag must be either 0 or 1
-		secret[SharedSecret.IV_BYTES] = (byte) 2;
+		secret[0] = (byte) 2;
 		try {
 			s = new SharedSecret(secret);
 			fail();
 		} catch(IllegalArgumentException expected) {}
-		// The ciphertext must be at least 1 byte long
+		// The secret must be at least 1 byte long
-		secret = new byte[SharedSecret.IV_BYTES + 1];
+		secret = new byte[1];
 		random.nextBytes(secret);
-		secret[SharedSecret.IV_BYTES] = (byte) 0;
+		secret[0] = (byte) 0;
 		try {
 			s = new SharedSecret(secret);
 			fail();

test/net/sf/briar/transport/batch/BatchConnectionReadWriteTest.java

@@ -63,9 +63,9 @@ public class BatchConnectionReadWriteTest extends TestCase {
 		transportId = new TransportId(TestUtils.getRandomId());
 		transportIndex = new TransportIndex(1);
 		// Create matching secrets for Alice and Bob
-		aliceSecret = new byte[100];
+		aliceSecret = new byte[123];
-		aliceSecret[16] = (byte) 1;
+		aliceSecret[0] = (byte) 1;
-		bobSecret = new byte[100];
+		bobSecret = new byte[123];
 	}
 
 	@Before