From 32e0b3977116c41f076dc063450790958869cccd Mon Sep 17 00:00:00 2001
From: akwizgran <michael@briarproject.org>
Date: Tue, 28 Nov 2017 11:20:15 +0000
Subject: [PATCH] Include protocol version in shared secret derivation.
---
.../bramble/api/crypto/CryptoComponent.java | 4 +--
.../bramble/crypto/CryptoComponentImpl.java | 19 ++++++-------
.../keyagreement/KeyAgreementProtocol.java | 10 ++++++-
.../bramble/crypto/KeyAgreementTest.java | 11 ++++++--
.../KeyAgreementProtocolTest.java | 28 +++++++++++++++----
.../introduction/IntroductionConstants.java | 5 ++++
.../briar/introduction/IntroduceeManager.java | 8 +++++-
.../IntroductionIntegrationTest.java | 20 +++++++++----
8 files changed, 78 insertions(+), 27 deletions(-)
diff --git a/bramble-api/src/main/java/org/briarproject/bramble/api/crypto/CryptoComponent.java b/bramble-api/src/main/java/org/briarproject/bramble/api/crypto/CryptoComponent.java
index 3a2be304a6..c104f61d13 100644
--- a/bramble-api/src/main/java/org/briarproject/bramble/api/crypto/CryptoComponent.java
+++ b/bramble-api/src/main/java/org/briarproject/bramble/api/crypto/CryptoComponent.java
@@ -41,11 +41,11 @@ public interface CryptoComponent {
* secret derived for another purpose
* @param theirPublicKey the public key of the remote party
* @param ourKeyPair the key pair of the local party
- * @param alice true if the local party is Alice
* @return the shared secret
*/
SecretKey deriveSharedSecret(String label, PublicKey theirPublicKey,
- KeyPair ourKeyPair, boolean alice) throws GeneralSecurityException;
+ KeyPair ourKeyPair, byte[]... inputs)
+ throws GeneralSecurityException;
/**
* Signs the given byte[] with the given ECDSA private key.
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 37af381df9..7eed7b5b7f 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
@@ -227,18 +227,15 @@ class CryptoComponentImpl implements CryptoComponent {
@Override
public SecretKey deriveSharedSecret(String label, PublicKey theirPublicKey,
- KeyPair ourKeyPair, boolean alice) throws GeneralSecurityException {
+ KeyPair ourKeyPair, byte[]... inputs)
+ throws GeneralSecurityException {
PrivateKey ourPriv = ourKeyPair.getPrivate();
- byte[] raw = performRawKeyAgreement(ourPriv, theirPublicKey);
- byte[] alicePub, bobPub;
- if (alice) {
- alicePub = ourKeyPair.getPublic().getEncoded();
- bobPub = theirPublicKey.getEncoded();
- } else {
- alicePub = theirPublicKey.getEncoded();
- bobPub = ourKeyPair.getPublic().getEncoded();
- }
- return new SecretKey(hash(label, raw, alicePub, bobPub));
+ byte[][] hashInputs = new byte[inputs.length + 1][];
+ hashInputs[0] = performRawKeyAgreement(ourPriv, theirPublicKey);
+ System.arraycopy(inputs, 0, hashInputs, 1, inputs.length);
+ byte[] hash = hash(label, hashInputs);
+ if (hash.length != SecretKey.LENGTH) throw new IllegalStateException();
+ return new SecretKey(hash);
}
@Override
diff --git a/bramble-core/src/main/java/org/briarproject/bramble/keyagreement/KeyAgreementProtocol.java b/bramble-core/src/main/java/org/briarproject/bramble/keyagreement/KeyAgreementProtocol.java
index a5c24c6e42..ab5ea4f921 100644
--- a/bramble-core/src/main/java/org/briarproject/bramble/keyagreement/KeyAgreementProtocol.java
+++ b/bramble-core/src/main/java/org/briarproject/bramble/keyagreement/KeyAgreementProtocol.java
@@ -15,6 +15,7 @@ import java.security.GeneralSecurityException;
import java.util.Arrays;
import static org.briarproject.bramble.api.keyagreement.KeyAgreementConstants.MASTER_SECRET_LABEL;
+import static org.briarproject.bramble.api.keyagreement.KeyAgreementConstants.PROTOCOL_VERSION;
import static org.briarproject.bramble.api.keyagreement.KeyAgreementConstants.SHARED_SECRET_LABEL;
/**
@@ -142,8 +143,15 @@ class KeyAgreementProtocol {
private SecretKey deriveSharedSecret(PublicKey theirPublicKey)
throws AbortException {
try {
+ byte[] ourPublicKeyBytes = ourKeyPair.getPublic().getEncoded();
+ byte[] theirPublicKeyBytes = theirPublicKey.getEncoded();
+ byte[][] inputs = {
+ new byte[] {PROTOCOL_VERSION},
+ alice ? ourPublicKeyBytes : theirPublicKeyBytes,
+ alice ? theirPublicKeyBytes : ourPublicKeyBytes
+ };
return crypto.deriveSharedSecret(SHARED_SECRET_LABEL,
- theirPublicKey, ourKeyPair, alice);
+ theirPublicKey, ourKeyPair, inputs);
} catch (GeneralSecurityException e) {
throw new AbortException(e);
}
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 1aec3b78fe..5a3568631c 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
@@ -7,7 +7,10 @@ import org.briarproject.bramble.test.BrambleTestCase;
import org.briarproject.bramble.test.TestSecureRandomProvider;
import org.junit.Test;
+import java.util.Random;
+
import static org.briarproject.bramble.api.keyagreement.KeyAgreementConstants.SHARED_SECRET_LABEL;
+import static org.briarproject.bramble.test.TestUtils.getRandomBytes;
import static org.junit.Assert.assertArrayEquals;
public class KeyAgreementTest extends BrambleTestCase {
@@ -18,10 +21,14 @@ public class KeyAgreementTest extends BrambleTestCase {
new CryptoComponentImpl(new TestSecureRandomProvider());
KeyPair aPair = crypto.generateAgreementKeyPair();
KeyPair bPair = crypto.generateAgreementKeyPair();
+ Random random = new Random();
+ byte[][] inputs = new byte[random.nextInt(10) + 1][];
+ for (int i = 0; i < inputs.length; i++)
+ inputs[i] = getRandomBytes(random.nextInt(256));
SecretKey aShared = crypto.deriveSharedSecret(SHARED_SECRET_LABEL,
- bPair.getPublic(), aPair, true);
+ bPair.getPublic(), aPair, inputs);
SecretKey bShared = crypto.deriveSharedSecret(SHARED_SECRET_LABEL,
- aPair.getPublic(), bPair, false);
+ aPair.getPublic(), bPair, inputs);
assertArrayEquals(aShared.getBytes(), bShared.getBytes());
}
}
diff --git a/bramble-core/src/test/java/org/briarproject/bramble/keyagreement/KeyAgreementProtocolTest.java b/bramble-core/src/test/java/org/briarproject/bramble/keyagreement/KeyAgreementProtocolTest.java
index c6c55ce040..e070db6a94 100644
--- a/bramble-core/src/test/java/org/briarproject/bramble/keyagreement/KeyAgreementProtocolTest.java
+++ b/bramble-core/src/test/java/org/briarproject/bramble/keyagreement/KeyAgreementProtocolTest.java
@@ -18,6 +18,7 @@ import org.junit.Test;
import static org.briarproject.bramble.api.keyagreement.KeyAgreementConstants.COMMIT_LENGTH;
import static org.briarproject.bramble.api.keyagreement.KeyAgreementConstants.MASTER_SECRET_LABEL;
+import static org.briarproject.bramble.api.keyagreement.KeyAgreementConstants.PROTOCOL_VERSION;
import static org.briarproject.bramble.api.keyagreement.KeyAgreementConstants.SHARED_SECRET_LABEL;
import static org.briarproject.bramble.test.TestUtils.getRandomBytes;
import static org.briarproject.bramble.test.TestUtils.getSecretKey;
@@ -90,6 +91,10 @@ public class KeyAgreementProtocolTest extends BrambleTestCase {
will(returnValue(alicePubKeyBytes));
allowing(crypto).getAgreementKeyParser();
will(returnValue(keyParser));
+ allowing(alicePubKey).getEncoded();
+ will(returnValue(alicePubKeyBytes));
+ allowing(bobPubKey).getEncoded();
+ will(returnValue(bobPubKeyBytes));
// Alice sends her public key
oneOf(transport).sendKey(alicePubKeyBytes);
@@ -108,7 +113,8 @@ public class KeyAgreementProtocolTest extends BrambleTestCase {
// Alice computes shared secret
oneOf(crypto).deriveSharedSecret(SHARED_SECRET_LABEL, bobPubKey,
- ourKeyPair, true);
+ ourKeyPair, new byte[] {PROTOCOL_VERSION},
+ alicePubKeyBytes, bobPubKeyBytes);
will(returnValue(sharedSecret));
// Alice sends her confirmation record
@@ -161,6 +167,10 @@ public class KeyAgreementProtocolTest extends BrambleTestCase {
will(returnValue(bobPubKeyBytes));
allowing(crypto).getAgreementKeyParser();
will(returnValue(keyParser));
+ allowing(alicePubKey).getEncoded();
+ will(returnValue(alicePubKeyBytes));
+ allowing(bobPubKey).getEncoded();
+ will(returnValue(bobPubKeyBytes));
// Bob receives Alice's public key
oneOf(transport).receiveKey();
@@ -178,7 +188,8 @@ public class KeyAgreementProtocolTest extends BrambleTestCase {
// Bob computes shared secret
oneOf(crypto).deriveSharedSecret(SHARED_SECRET_LABEL, alicePubKey,
- ourKeyPair, false);
+ ourKeyPair, new byte[] {PROTOCOL_VERSION},
+ alicePubKeyBytes, bobPubKeyBytes);
will(returnValue(sharedSecret));
// Bob receives Alices's confirmation record
@@ -246,7 +257,8 @@ public class KeyAgreementProtocolTest extends BrambleTestCase {
// Alice never computes shared secret
never(crypto).deriveSharedSecret(SHARED_SECRET_LABEL, badPubKey,
- ourKeyPair, true);
+ ourKeyPair, new byte[] {PROTOCOL_VERSION},
+ alicePubKeyBytes, bobPubKeyBytes);
}});
// execute
@@ -317,6 +329,8 @@ public class KeyAgreementProtocolTest extends BrambleTestCase {
will(returnValue(alicePubKeyBytes));
allowing(crypto).getAgreementKeyParser();
will(returnValue(keyParser));
+ allowing(bobPubKey).getEncoded();
+ will(returnValue(bobPubKeyBytes));
// Alice sends her public key
oneOf(transport).sendKey(alicePubKeyBytes);
@@ -335,7 +349,8 @@ public class KeyAgreementProtocolTest extends BrambleTestCase {
// Alice computes shared secret
oneOf(crypto).deriveSharedSecret(SHARED_SECRET_LABEL, bobPubKey,
- ourKeyPair, true);
+ ourKeyPair, new byte[] {PROTOCOL_VERSION},
+ alicePubKeyBytes, bobPubKeyBytes);
will(returnValue(sharedSecret));
// Alice sends her confirmation record
@@ -389,6 +404,8 @@ public class KeyAgreementProtocolTest extends BrambleTestCase {
will(returnValue(bobPubKeyBytes));
allowing(crypto).getAgreementKeyParser();
will(returnValue(keyParser));
+ allowing(alicePubKey).getEncoded();
+ will(returnValue(alicePubKeyBytes));
// Bob receives Alice's public key
oneOf(transport).receiveKey();
@@ -406,7 +423,8 @@ public class KeyAgreementProtocolTest extends BrambleTestCase {
// Bob computes shared secret
oneOf(crypto).deriveSharedSecret(SHARED_SECRET_LABEL, alicePubKey,
- ourKeyPair, false);
+ ourKeyPair, new byte[] {PROTOCOL_VERSION},
+ alicePubKeyBytes, bobPubKeyBytes);
will(returnValue(sharedSecret));
// Bob receives a bad confirmation record
diff --git a/briar-api/src/main/java/org/briarproject/briar/api/introduction/IntroductionConstants.java b/briar-api/src/main/java/org/briarproject/briar/api/introduction/IntroductionConstants.java
index 72e6030b10..8ae24bc505 100644
--- a/briar-api/src/main/java/org/briarproject/briar/api/introduction/IntroductionConstants.java
+++ b/briar-api/src/main/java/org/briarproject/briar/api/introduction/IntroductionConstants.java
@@ -86,6 +86,11 @@ public interface IntroductionConstants {
int TASK_ACTIVATE_CONTACT = 1;
int TASK_ABORT = 2;
+ /**
+ * The current version of the introduction protocol.
+ */
+ int PROTOCOL_VERSION = 0;
+
/**
* Label for deriving the shared secret.
*/
diff --git a/briar-core/src/main/java/org/briarproject/briar/introduction/IntroduceeManager.java b/briar-core/src/main/java/org/briarproject/briar/introduction/IntroduceeManager.java
index 6ecfcef49c..aa89289858 100644
--- a/briar-core/src/main/java/org/briarproject/briar/introduction/IntroduceeManager.java
+++ b/briar-core/src/main/java/org/briarproject/briar/introduction/IntroduceeManager.java
@@ -76,6 +76,7 @@ import static org.briarproject.briar.api.introduction.IntroductionConstants.OUR_
import static org.briarproject.briar.api.introduction.IntroductionConstants.OUR_SIGNATURE;
import static org.briarproject.briar.api.introduction.IntroductionConstants.OUR_TIME;
import static org.briarproject.briar.api.introduction.IntroductionConstants.OUR_TRANSPORT;
+import static org.briarproject.briar.api.introduction.IntroductionConstants.PROTOCOL_VERSION;
import static org.briarproject.briar.api.introduction.IntroductionConstants.PUBLIC_KEY;
import static org.briarproject.briar.api.introduction.IntroductionConstants.REMOTE_AUTHOR_ID;
import static org.briarproject.briar.api.introduction.IntroductionConstants.REMOTE_AUTHOR_IS_US;
@@ -433,8 +434,13 @@ class IntroduceeManager {
// The shared secret is derived from the local ephemeral key pair
// and the remote ephemeral public key
+ byte[][] inputs = {
+ new byte[] {PROTOCOL_VERSION},
+ alice ? ourPublicKeyBytes : theirPublicKeyBytes,
+ alice ? theirPublicKeyBytes : ourPublicKeyBytes
+ };
return cryptoComponent.deriveSharedSecret(SHARED_SECRET_LABEL,
- theirPublicKey, ourKeyPair, alice);
+ theirPublicKey, ourKeyPair, inputs);
}
/**
diff --git a/briar-core/src/test/java/org/briarproject/briar/introduction/IntroductionIntegrationTest.java b/briar-core/src/test/java/org/briarproject/briar/introduction/IntroductionIntegrationTest.java
index b6b088f9d5..d6478286fc 100644
--- a/briar-core/src/test/java/org/briarproject/briar/introduction/IntroductionIntegrationTest.java
+++ b/briar-core/src/test/java/org/briarproject/briar/introduction/IntroductionIntegrationTest.java
@@ -65,6 +65,7 @@ import static org.briarproject.briar.api.introduction.IntroductionConstants.MAC_
import static org.briarproject.briar.api.introduction.IntroductionConstants.MAC_LABEL;
import static org.briarproject.briar.api.introduction.IntroductionConstants.NAME;
import static org.briarproject.briar.api.introduction.IntroductionConstants.NONCE;
+import static org.briarproject.briar.api.introduction.IntroductionConstants.PROTOCOL_VERSION;
import static org.briarproject.briar.api.introduction.IntroductionConstants.PUBLIC_KEY;
import static org.briarproject.briar.api.introduction.IntroductionConstants.SESSION_ID;
import static org.briarproject.briar.api.introduction.IntroductionConstants.SHARED_SECRET_LABEL;
@@ -753,8 +754,13 @@ public class IntroductionIntegrationTest
KeyPair eKeyPair2 = crypto.generateAgreementKeyPair();
// Nonce 1
+ byte[][] inputs = {
+ new byte[] {PROTOCOL_VERSION},
+ eKeyPair1.getPublic().getEncoded(),
+ eKeyPair2.getPublic().getEncoded()
+ };
SecretKey sharedSecret = crypto.deriveSharedSecret(SHARED_SECRET_LABEL,
- eKeyPair2.getPublic(), eKeyPair1, true);
+ eKeyPair2.getPublic(), eKeyPair1, inputs);
byte[] nonce1 = crypto.mac(ALICE_NONCE_LABEL, sharedSecret);
// Signature 1
@@ -787,20 +793,24 @@ public class IntroductionIntegrationTest
// replace ephemeral key pair and recalculate matching keys and nonce
KeyPair eKeyPair1f = crypto.generateAgreementKeyPair();
- byte[] ePublicKeyBytes1f = eKeyPair1f.getPublic().getEncoded();
+ byte[][] fakeInputs = {
+ new byte[] {PROTOCOL_VERSION},
+ eKeyPair1f.getPublic().getEncoded(),
+ eKeyPair2.getPublic().getEncoded()
+ };
sharedSecret = crypto.deriveSharedSecret(SHARED_SECRET_LABEL,
- eKeyPair2.getPublic(), eKeyPair1f, true);
+ eKeyPair2.getPublic(), eKeyPair1f, fakeInputs);
nonce1 = crypto.mac(ALICE_NONCE_LABEL, sharedSecret);
// recalculate MAC
macKey1 = crypto.deriveKey(ALICE_MAC_KEY_LABEL, sharedSecret);
toMacList = BdfList.of(keyPair1.getPublic().getEncoded(),
- ePublicKeyBytes1f, tp1, time1);
+ eKeyPair1f.getPublic().getEncoded(), tp1, time1);
toMac = clientHelper.toByteArray(toMacList);
mac1 = crypto.mac(MAC_LABEL, macKey1, toMac);
// update state with faked information
- state.put(E_PUBLIC_KEY, ePublicKeyBytes1f);
+ state.put(E_PUBLIC_KEY, eKeyPair1f.getPublic().getEncoded());
state.put(MAC, mac1);
state.put(MAC_KEY, macKey1.getBytes());
state.put(NONCE, nonce1);
--
GitLab