- * 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. - *
- * TODO: Remove this after a reasonable migration period (added 2023-03-10). - *
- * - * @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. * diff --git a/bramble-core/src/main/java/org/briarproject/bramble/contact/HandshakeConstants.java b/bramble-core/src/main/java/org/briarproject/bramble/contact/HandshakeConstants.java index 45470f5b8..1184f91db 100644 --- a/bramble-core/src/main/java/org/briarproject/bramble/contact/HandshakeConstants.java +++ b/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. - *
- * 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. diff --git a/bramble-core/src/main/java/org/briarproject/bramble/contact/HandshakeCrypto.java b/bramble-core/src/main/java/org/briarproject/bramble/contact/HandshakeCrypto.java index ce9a656eb..81df4ac43 100644 --- a/bramble-core/src/main/java/org/briarproject/bramble/contact/HandshakeCrypto.java +++ b/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. - *
- * 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. diff --git a/bramble-core/src/main/java/org/briarproject/bramble/contact/HandshakeCryptoImpl.java b/bramble-core/src/main/java/org/briarproject/bramble/contact/HandshakeCryptoImpl.java index 7606ec938..3b21135c4 100644 --- a/bramble-core/src/main/java/org/briarproject/bramble/contact/HandshakeCryptoImpl.java +++ b/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, diff --git a/bramble-core/src/main/java/org/briarproject/bramble/contact/HandshakeManagerImpl.java b/bramble-core/src/main/java/org/briarproject/bramble/contact/HandshakeManagerImpl.java index 496fdc482..05b2b4354 100644 --- a/bramble-core/src/main/java/org/briarproject/bramble/contact/HandshakeManagerImpl.java +++ b/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); } diff --git a/bramble-core/src/main/java/org/briarproject/bramble/crypto/CryptoComponentImpl.java b/bramble-core/src/main/java/org/briarproject/bramble/crypto/CryptoComponentImpl.java index 5dd24a5f7..0b582dd28 100644 --- a/bramble-core/src/main/java/org/briarproject/bramble/crypto/CryptoComponentImpl.java +++ b/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, diff --git a/bramble-core/src/test/java/org/briarproject/bramble/contact/HandshakeManagerImplTest.java b/bramble-core/src/test/java/org/briarproject/bramble/contact/HandshakeManagerImplTest.java index 9597d486b..fb8062f98 100644 --- a/bramble-core/src/test/java/org/briarproject/bramble/contact/HandshakeManagerImplTest.java +++ b/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); diff --git a/bramble-core/src/test/java/org/briarproject/bramble/crypto/KeyAgreementTest.java b/bramble-core/src/test/java/org/briarproject/bramble/crypto/KeyAgreementTest.java index e179ac2b2..5612663ae 100644 --- a/bramble-core/src/test/java/org/briarproject/bramble/crypto/KeyAgreementTest.java +++ b/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);