Remove deprecated BHP handshake v0.0

https://briarproject.org/news/2023-three-security-issues-found-and-fixed/

bramble-api/src/main/java/org/briarproject/bramble/api/crypto/CryptoComponent.java

@@ -54,38 +54,6 @@ public interface CryptoComponent {
 			KeyPair ourKeyPair, byte[]... inputs)
 			throws GeneralSecurityException;
 
-	/**
-	 * Derives a shared secret from two static and two ephemeral key pairs.
-	 * <p>
-	 * Do not use this method for new protocols. The shared secret can be
-	 * re-derived using the ephemeral public keys and both static private
-	 * keys, so keys derived from the shared secret should not be used if
-	 * forward secrecy is required. Use {@link #deriveSharedSecret(String,
-	 * PublicKey, PublicKey, KeyPair, KeyPair, boolean, byte[]...)} instead.
-	 * <p>
-	 * TODO: Remove this after a reasonable migration period (added 2023-03-10).
-	 * <p>
-	 *
-	 * @param label A namespaced label indicating the purpose of this shared
-	 * secret, to prevent it from being repurposed or colliding with a shared
-	 * secret derived for another purpose
-	 * @param theirStaticPublicKey The static public key of the remote party
-	 * @param theirEphemeralPublicKey The ephemeral public key of the remote
-	 * party
-	 * @param ourStaticKeyPair The static key pair of the local party
-	 * @param ourEphemeralKeyPair The ephemeral key pair of the local party
-	 * @param alice True if the local party is Alice
-	 * @param inputs Additional inputs that will be included in the
-	 * derivation of the shared secret
-	 * @return The shared secret
-	 */
-	@Deprecated
-	SecretKey deriveSharedSecretBadly(String label,
-			PublicKey theirStaticPublicKey, PublicKey theirEphemeralPublicKey,
-			KeyPair ourStaticKeyPair, KeyPair ourEphemeralKeyPair,
-			boolean alice, byte[]... inputs)
-			throws GeneralSecurityException;
-
 	/**
 	 * Derives a shared secret from two static and two ephemeral key pairs.
 	 *

bramble-core/src/main/java/org/briarproject/bramble/contact/HandshakeConstants.java

@@ -14,16 +14,6 @@ interface HandshakeConstants {
 	 */
 	byte PROTOCOL_MINOR_VERSION = 1;
 
-	/**
-	 * Label for deriving the master key when using the deprecated v0.0 key
-	 * derivation method.
-	 * <p>
-	 * TODO: Remove this after a reasonable migration period (added 2023-03-10).
-	 */
-	@Deprecated
-	String MASTER_KEY_LABEL_0_0 =
-			"org.briarproject.bramble.handshake/MASTER_KEY";
-
 	/**
 	 * Label for deriving the master key when using the v0.1 key derivation
 	 * method.

bramble-core/src/main/java/org/briarproject/bramble/contact/HandshakeCrypto.java

@@ -12,20 +12,6 @@ interface HandshakeCrypto {
 
 	KeyPair generateEphemeralKeyPair();
 
-	/**
-	 * Derives the master key from the given static and ephemeral keys using
-	 * the deprecated v0.0 key derivation method.
-	 * <p>
-	 * TODO: Remove this after a reasonable migration period (added 2023-03-10).
-	 *
-	 * @param alice Whether the local peer is Alice
-	 */
-	@Deprecated
-	SecretKey deriveMasterKey_0_0(PublicKey theirStaticPublicKey,
-			PublicKey theirEphemeralPublicKey, KeyPair ourStaticKeyPair,
-			KeyPair ourEphemeralKeyPair, boolean alice)
-			throws GeneralSecurityException;
-
 	/**
 	 * Derives the master key from the given static and ephemeral keys using
 	 * the v0.1 key derivation method.

bramble-core/src/main/java/org/briarproject/bramble/contact/HandshakeCryptoImpl.java

@@ -13,7 +13,6 @@ import javax.inject.Inject;
 
 import static org.briarproject.bramble.contact.HandshakeConstants.ALICE_PROOF_LABEL;
 import static org.briarproject.bramble.contact.HandshakeConstants.BOB_PROOF_LABEL;
-import static org.briarproject.bramble.contact.HandshakeConstants.MASTER_KEY_LABEL_0_0;
 import static org.briarproject.bramble.contact.HandshakeConstants.MASTER_KEY_LABEL_0_1;
 
 @Immutable
@@ -32,27 +31,6 @@ class HandshakeCryptoImpl implements HandshakeCrypto {
 		return crypto.generateAgreementKeyPair();
 	}
 
-	@Override
-	@Deprecated
-	public SecretKey deriveMasterKey_0_0(PublicKey theirStaticPublicKey,
-			PublicKey theirEphemeralPublicKey, KeyPair ourStaticKeyPair,
-			KeyPair ourEphemeralKeyPair, boolean alice) throws
-			GeneralSecurityException {
-		byte[] theirStatic = theirStaticPublicKey.getEncoded();
-		byte[] theirEphemeral = theirEphemeralPublicKey.getEncoded();
-		byte[] ourStatic = ourStaticKeyPair.getPublic().getEncoded();
-		byte[] ourEphemeral = ourEphemeralKeyPair.getPublic().getEncoded();
-		byte[][] inputs = {
-				alice ? ourStatic : theirStatic,
-				alice ? theirStatic : ourStatic,
-				alice ? ourEphemeral : theirEphemeral,
-				alice ? theirEphemeral : ourEphemeral
-		};
-		return crypto.deriveSharedSecretBadly(MASTER_KEY_LABEL_0_0,
-				theirStaticPublicKey, theirEphemeralPublicKey,
-				ourStaticKeyPair, ourEphemeralKeyPair, alice, inputs);
-	}
-
 	@Override
 	public SecretKey deriveMasterKey_0_1(PublicKey theirStaticPublicKey,
 			PublicKey theirEphemeralPublicKey, KeyPair ourStaticKeyPair,

bramble-core/src/main/java/org/briarproject/bramble/contact/HandshakeManagerImpl.java

@@ -2,6 +2,7 @@ package org.briarproject.bramble.contact;
 
 import org.briarproject.bramble.api.FormatException;
 import org.briarproject.bramble.api.Pair;
+import org.briarproject.bramble.api.UnsupportedVersionException;
 import org.briarproject.bramble.api.contact.ContactManager;
 import org.briarproject.bramble.api.contact.HandshakeManager;
 import org.briarproject.bramble.api.contact.PendingContact;
@@ -111,21 +112,12 @@ class HandshakeManagerImpl implements HandshakeManager {
 			sendMinorVersion(recordWriter);
 			sendPublicKey(recordWriter, ourEphemeralKeyPair.getPublic());
 		}
-		byte theirMinorVersion = theirMinorVersionAndKey.getFirst();
 		PublicKey theirEphemeralPublicKey = theirMinorVersionAndKey.getSecond();
 		SecretKey masterKey;
 		try {
-			if (theirMinorVersion > 0) {
-				masterKey = handshakeCrypto.deriveMasterKey_0_1(
-						theirStaticPublicKey, theirEphemeralPublicKey,
-						ourStaticKeyPair, ourEphemeralKeyPair, alice);
-			} else {
-				// TODO: Remove this branch after a reasonable migration
-				//  period (added 2023-03-10).
-				masterKey = handshakeCrypto.deriveMasterKey_0_0(
-						theirStaticPublicKey, theirEphemeralPublicKey,
-						ourStaticKeyPair, ourEphemeralKeyPair, alice);
-			}
+			masterKey = handshakeCrypto.deriveMasterKey_0_1(
+					theirStaticPublicKey, theirEphemeralPublicKey,
+					ourStaticKeyPair, ourEphemeralKeyPair, alice);
 		} catch (GeneralSecurityException e) {
 			throw new FormatException();
 		}
@@ -187,10 +179,11 @@ class HandshakeManagerImpl implements HandshakeManager {
 		} else {
 			// The remote peer did not send a minor version record, so the
 			// remote peer's protocol minor version is assumed to be zero
-			// TODO: Remove this branch after a reasonable migration period
-			//  (added 2023-03-10).
-			theirMinorVersion = 0;
-			theirEphemeralPublicKey = parsePublicKey(first);
+
+			// TODO: How communicate to user that contact seems to use a version
+			// of Briar that is too old? (be aware of MITM attacks)
+			// `RendezvousPollerImpl` broadcasts PendingContactState FAILED via EventBus
+			throw new UnsupportedVersionException(true);
 		}
 		return new Pair<>(theirMinorVersion, theirEphemeralPublicKey);
 	}

bramble-core/src/main/java/org/briarproject/bramble/crypto/CryptoComponentImpl.java

@@ -222,36 +222,6 @@ class CryptoComponentImpl implements CryptoComponent {
 		return new SecretKey(hash);
 	}
 
-	@Override
-	@Deprecated
-	public SecretKey deriveSharedSecretBadly(String label,
-			PublicKey theirStaticPublicKey, PublicKey theirEphemeralPublicKey,
-			KeyPair ourStaticKeyPair, KeyPair ourEphemeralKeyPair,
-			boolean alice, byte[]... inputs) throws GeneralSecurityException {
-		PrivateKey ourStaticPrivateKey = ourStaticKeyPair.getPrivate();
-		PrivateKey ourEphemeralPrivateKey = ourEphemeralKeyPair.getPrivate();
-		byte[][] hashInputs = new byte[inputs.length + 3][];
-		// Alice static/Bob static
-		hashInputs[0] = performRawKeyAgreement(ourStaticPrivateKey,
-				theirStaticPublicKey);
-		// Alice static/Bob ephemeral, Bob static/Alice ephemeral
-		if (alice) {
-			hashInputs[1] = performRawKeyAgreement(ourStaticPrivateKey,
-					theirEphemeralPublicKey);
-			hashInputs[2] = performRawKeyAgreement(ourEphemeralPrivateKey,
-					theirStaticPublicKey);
-		} else {
-			hashInputs[1] = performRawKeyAgreement(ourEphemeralPrivateKey,
-					theirStaticPublicKey);
-			hashInputs[2] = performRawKeyAgreement(ourStaticPrivateKey,
-					theirEphemeralPublicKey);
-		}
-		arraycopy(inputs, 0, hashInputs, 3, inputs.length);
-		byte[] hash = hash(label, hashInputs);
-		if (hash.length != SecretKey.LENGTH) throw new IllegalStateException();
-		return new SecretKey(hash);
-	}
-
 	@Override
 	public SecretKey deriveSharedSecret(String label,
 			PublicKey theirStaticPublicKey, PublicKey theirEphemeralPublicKey,

bramble-core/src/test/java/org/briarproject/bramble/contact/HandshakeManagerImplTest.java

@@ -1,6 +1,7 @@
 package org.briarproject.bramble.contact;
 
 import org.briarproject.bramble.api.FormatException;
+import org.briarproject.bramble.api.UnsupportedVersionException;
 import org.briarproject.bramble.api.contact.ContactManager;
 import org.briarproject.bramble.api.contact.HandshakeManager.HandshakeResult;
 import org.briarproject.bramble.api.contact.PendingContact;
@@ -27,6 +28,7 @@ import org.junit.Test;
 
 import java.io.ByteArrayInputStream;
 import java.io.ByteArrayOutputStream;
+import java.io.EOFException;
 import java.io.InputStream;
 import java.io.OutputStream;
 import java.util.Arrays;
@@ -123,12 +125,12 @@ public class HandshakeManagerImplTest extends BrambleMockTestCase {
 		assertEquals(alice, result.isAlice());
 	}
 
-	@Test
+	@Test(expected = UnsupportedVersionException.class)
 	public void testHandshakeAsAliceWithPeerVersion_0_0() throws Exception {
 		testHandshakeWithPeerVersion_0_0(true);
 	}
 
-	@Test
+	@Test(expected = UnsupportedVersionException.class)
 	public void testHandshakeAsBobWithPeerVersion_0_0() throws Exception {
 		testHandshakeWithPeerVersion_0_0(false);
 	}
@@ -140,20 +142,8 @@ public class HandshakeManagerImplTest extends BrambleMockTestCase {
 		expectSendKey();
 		// Remote peer does not send minor version, so use old key derivation
 		expectReceiveKey();
-		expectDeriveMasterKey_0_0(alice);
-		expectDeriveProof(alice);
-		expectSendProof();
-		expectReceiveProof();
-		expectSendEof();
-		expectReceiveEof();
-		expectVerifyOwnership(alice, true);
 
-		HandshakeResult result = handshakeManager.handshake(
-				pendingContact.getId(), in, streamWriter);
-
-		assertArrayEquals(masterKey.getBytes(),
-				result.getMasterKey().getBytes());
-		assertEquals(alice, result.isAlice());
+		handshakeManager.handshake(pendingContact.getId(), in, streamWriter);
 	}
 
 	@Test(expected = FormatException.class)
@@ -241,15 +231,6 @@ public class HandshakeManagerImplTest extends BrambleMockTestCase {
 		}});
 	}
 
-	private void expectDeriveMasterKey_0_0(boolean alice) throws Exception {
-		context.checking(new Expectations() {{
-			oneOf(handshakeCrypto).deriveMasterKey_0_0(theirStaticPublicKey,
-					theirEphemeralPublicKey, ourStaticKeyPair,
-					ourEphemeralKeyPair, alice);
-			will(returnValue(masterKey));
-		}});
-	}
-
 	private void expectDeriveProof(boolean alice) {
 		context.checking(new Expectations() {{
 			oneOf(handshakeCrypto).proveOwnership(masterKey, alice);

bramble-core/src/test/java/org/briarproject/bramble/crypto/KeyAgreementTest.java

@@ -60,22 +60,6 @@ public class KeyAgreementTest extends BrambleTestCase {
 		assertArrayEquals(aShared.getBytes(), bShared.getBytes());
 	}
 
-	@Test
-	public void testDerivesStaticEphemeralSharedSecretBadly() throws Exception {
-		String label = getRandomString(123);
-		KeyPair aStatic = crypto.generateAgreementKeyPair();
-		KeyPair aEphemeral = crypto.generateAgreementKeyPair();
-		KeyPair bStatic = crypto.generateAgreementKeyPair();
-		KeyPair bEphemeral = crypto.generateAgreementKeyPair();
-		SecretKey aShared = crypto.deriveSharedSecretBadly(label,
-				bStatic.getPublic(), bEphemeral.getPublic(), aStatic,
-				aEphemeral, true, inputs);
-		SecretKey bShared = crypto.deriveSharedSecretBadly(label,
-				aStatic.getPublic(), aEphemeral.getPublic(), bStatic,
-				bEphemeral, false, inputs);
-		assertArrayEquals(aShared.getBytes(), bShared.getBytes());
-	}
-
 	@Test
 	public void testDerivesStaticEphemeralSharedSecret() throws Exception {
 		String label = getRandomString(123);