From b2221070447f2ab6557ebd43815fde4e8740e744 Mon Sep 17 00:00:00 2001 From: akwizgran Date: Thu, 30 May 2019 14:30:16 +0100 Subject: [PATCH 01/10] Add static method for comparing byte arrays. --- .../org/briarproject/bramble/api/Bytes.java | 24 +++++++------------ .../client/ContactGroupFactoryImpl.java | 3 +-- 2 files changed, 9 insertions(+), 18 deletions(-) diff --git a/bramble-api/src/main/java/org/briarproject/bramble/api/Bytes.java b/bramble-api/src/main/java/org/briarproject/bramble/api/Bytes.java index 6da75fdff..d8a727b71 100644 --- a/bramble-api/src/main/java/org/briarproject/bramble/api/Bytes.java +++ b/bramble-api/src/main/java/org/briarproject/bramble/api/Bytes.java @@ -4,7 +4,6 @@ import org.briarproject.bramble.api.nullsafety.NotNullByDefault; import org.briarproject.bramble.util.StringUtils; import java.util.Arrays; -import java.util.Comparator; import javax.annotation.Nullable; import javax.annotation.concurrent.ThreadSafe; @@ -16,8 +15,6 @@ import javax.annotation.concurrent.ThreadSafe; @NotNullByDefault public class Bytes implements Comparable { - public static final BytesComparator COMPARATOR = new BytesComparator(); - private final byte[] bytes; private int hashCode = -1; @@ -45,14 +42,7 @@ public class Bytes implements Comparable { @Override public int compareTo(Bytes other) { - byte[] aBytes = bytes, bBytes = other.bytes; - int length = Math.min(aBytes.length, bBytes.length); - for (int i = 0; i < length; i++) { - int aUnsigned = aBytes[i] & 0xFF, bUnsigned = bBytes[i] & 0xFF; - if (aUnsigned < bUnsigned) return -1; - if (aUnsigned > bUnsigned) return 1; - } - return aBytes.length - bBytes.length; + return compare(bytes, other.bytes); } @Override @@ -61,11 +51,13 @@ public class Bytes implements Comparable { "(" + StringUtils.toHexString(getBytes()) + ")"; } - public static class BytesComparator implements Comparator { - - @Override - public int compare(Bytes a, Bytes b) { - return a.compareTo(b); + public static int compare(byte[] a, byte[] b) { + int length = Math.min(a.length, b.length); + for (int i = 0; i < length; i++) { + int aUnsigned = a[i] & 0xFF, bUnsigned = b[i] & 0xFF; + if (aUnsigned < bUnsigned) return -1; + if (aUnsigned > bUnsigned) return 1; } + return a.length - b.length; } } diff --git a/bramble-core/src/main/java/org/briarproject/bramble/client/ContactGroupFactoryImpl.java b/bramble-core/src/main/java/org/briarproject/bramble/client/ContactGroupFactoryImpl.java index 6b35c9281..091bc95f7 100644 --- a/bramble-core/src/main/java/org/briarproject/bramble/client/ContactGroupFactoryImpl.java +++ b/bramble-core/src/main/java/org/briarproject/bramble/client/ContactGroupFactoryImpl.java @@ -1,6 +1,5 @@ package org.briarproject.bramble.client; -import org.briarproject.bramble.api.Bytes; import org.briarproject.bramble.api.FormatException; import org.briarproject.bramble.api.client.ClientHelper; import org.briarproject.bramble.api.client.ContactGroupFactory; @@ -55,7 +54,7 @@ class ContactGroupFactoryImpl implements ContactGroupFactory { private byte[] createGroupDescriptor(AuthorId local, AuthorId remote) { try { - if (Bytes.COMPARATOR.compare(local, remote) < 0) + if (local.compareTo(remote) < 0) return clientHelper.toByteArray(BdfList.of(local, remote)); else return clientHelper.toByteArray(BdfList.of(remote, local)); } catch (FormatException e) { From 9b4f60088fa4f921c160700a0e97635b66ccf028 Mon Sep 17 00:00:00 2001 From: akwizgran Date: Thu, 30 May 2019 14:30:39 +0100 Subject: [PATCH 02/10] Add methods for deriving static master and root keys. --- .../bramble/api/crypto/TransportCrypto.java | 16 ++++++++++ .../api/transport/TransportConstants.java | 28 ++++++++++++----- .../bramble/crypto/TransportCryptoImpl.java | 30 +++++++++++++++++++ 3 files changed, 66 insertions(+), 8 deletions(-) diff --git a/bramble-api/src/main/java/org/briarproject/bramble/api/crypto/TransportCrypto.java b/bramble-api/src/main/java/org/briarproject/bramble/api/crypto/TransportCrypto.java index 6db16f7d5..a58645792 100644 --- a/bramble-api/src/main/java/org/briarproject/bramble/api/crypto/TransportCrypto.java +++ b/bramble-api/src/main/java/org/briarproject/bramble/api/crypto/TransportCrypto.java @@ -3,12 +3,28 @@ package org.briarproject.bramble.api.crypto; import org.briarproject.bramble.api.plugin.TransportId; import org.briarproject.bramble.api.transport.TransportKeys; +import java.security.GeneralSecurityException; + /** * Crypto operations for the transport security protocol - see * https://code.briarproject.org/briar/briar-spec/blob/master/protocols/BTP.md */ public interface TransportCrypto { + /** + * Derives the static master key shared with a contact or pending contact. + */ + SecretKey deriveStaticMasterKey(PublicKey theirHandshakePublicKey, + KeyPair ourHandshakeKeyPair) throws GeneralSecurityException; + + /** + * Derives the handshake mode root key from the static master key. + * @param pendingContact Whether the static master key is shared with a + * pending contact or a contact + */ + SecretKey deriveHandshakeRootKey(SecretKey staticMasterKey, + boolean pendingContact); + /** * Derives initial rotation mode transport keys for the given transport in * the given time period from the given root key. diff --git a/bramble-api/src/main/java/org/briarproject/bramble/api/transport/TransportConstants.java b/bramble-api/src/main/java/org/briarproject/bramble/api/transport/TransportConstants.java index ce2394d74..43e548208 100644 --- a/bramble-api/src/main/java/org/briarproject/bramble/api/transport/TransportConstants.java +++ b/bramble-api/src/main/java/org/briarproject/bramble/api/transport/TransportConstants.java @@ -63,14 +63,6 @@ public interface TransportConstants { int MAX_PAYLOAD_LENGTH = MAX_FRAME_LENGTH - FRAME_HEADER_LENGTH - MAC_LENGTH; - /** - * The minimum stream length in bytes that all transport plugins must - * support. Streams may be shorter than this length, but all transport - * plugins must support streams of at least this length. - */ - int MIN_STREAM_LENGTH = STREAM_HEADER_LENGTH + FRAME_HEADER_LENGTH - + MAC_LENGTH; - /** * The maximum difference in milliseconds between two peers' clocks. */ @@ -81,6 +73,26 @@ public interface TransportConstants { */ int REORDERING_WINDOW_SIZE = 32; + /** + * Label for deriving the static master key from handshake key pairs. + */ + String STATIC_MASTER_KEY_LABEL = + "org.briarproject.bramble.transport/STATIC_MASTER_KEY"; + + /** + * Label for deriving the handshake mode root key for a pending contact + * from the static master key. + */ + String PENDING_CONTACT_ROOT_KEY_LABEL = + "org.briarproject.bramble.transport/PENDING_CONTACT_ROOT_KEY"; + + /** + * Label for deriving the handshake mode root key for a contact from the + * static master key. + */ + String CONTACT_ROOT_KEY_LABEL = + "org.briarproject.bramble.transport/CONTACT_ROOT_KEY"; + /** * Label for deriving Alice's initial tag key from the root key in * rotation mode. diff --git a/bramble-core/src/main/java/org/briarproject/bramble/crypto/TransportCryptoImpl.java b/bramble-core/src/main/java/org/briarproject/bramble/crypto/TransportCryptoImpl.java index fab0c8fcd..aa8d4d665 100644 --- a/bramble-core/src/main/java/org/briarproject/bramble/crypto/TransportCryptoImpl.java +++ b/bramble-core/src/main/java/org/briarproject/bramble/crypto/TransportCryptoImpl.java @@ -1,6 +1,8 @@ package org.briarproject.bramble.crypto; import org.briarproject.bramble.api.crypto.CryptoComponent; +import org.briarproject.bramble.api.crypto.KeyPair; +import org.briarproject.bramble.api.crypto.PublicKey; import org.briarproject.bramble.api.crypto.SecretKey; import org.briarproject.bramble.api.crypto.TransportCrypto; import org.briarproject.bramble.api.plugin.TransportId; @@ -10,9 +12,12 @@ import org.briarproject.bramble.api.transport.TransportKeys; import org.spongycastle.crypto.Digest; import org.spongycastle.crypto.digests.Blake2bDigest; +import java.security.GeneralSecurityException; + import javax.inject.Inject; import static java.lang.System.arraycopy; +import static org.briarproject.bramble.api.Bytes.compare; import static org.briarproject.bramble.api.transport.TransportConstants.ALICE_HANDSHAKE_HEADER_LABEL; import static org.briarproject.bramble.api.transport.TransportConstants.ALICE_HANDSHAKE_TAG_LABEL; import static org.briarproject.bramble.api.transport.TransportConstants.ALICE_HEADER_LABEL; @@ -21,7 +26,10 @@ import static org.briarproject.bramble.api.transport.TransportConstants.BOB_HAND import static org.briarproject.bramble.api.transport.TransportConstants.BOB_HANDSHAKE_TAG_LABEL; import static org.briarproject.bramble.api.transport.TransportConstants.BOB_HEADER_LABEL; import static org.briarproject.bramble.api.transport.TransportConstants.BOB_TAG_LABEL; +import static org.briarproject.bramble.api.transport.TransportConstants.CONTACT_ROOT_KEY_LABEL; +import static org.briarproject.bramble.api.transport.TransportConstants.PENDING_CONTACT_ROOT_KEY_LABEL; import static org.briarproject.bramble.api.transport.TransportConstants.ROTATE_LABEL; +import static org.briarproject.bramble.api.transport.TransportConstants.STATIC_MASTER_KEY_LABEL; import static org.briarproject.bramble.api.transport.TransportConstants.TAG_LENGTH; import static org.briarproject.bramble.util.ByteUtils.INT_16_BYTES; import static org.briarproject.bramble.util.ByteUtils.INT_64_BYTES; @@ -40,6 +48,28 @@ class TransportCryptoImpl implements TransportCrypto { this.crypto = crypto; } + @Override + public SecretKey deriveStaticMasterKey(PublicKey theirHandshakePublicKey, + KeyPair ourHandshakeKeyPair) throws GeneralSecurityException { + byte[] theirPublic = theirHandshakePublicKey.getEncoded(); + byte[] ourPublic = ourHandshakeKeyPair.getPublic().getEncoded(); + boolean alice = compare(ourPublic, theirPublic) < 0; + byte[][] inputs = { + alice ? ourPublic : theirPublic, + alice ? theirPublic : ourPublic + }; + return crypto.deriveSharedSecret(STATIC_MASTER_KEY_LABEL, + theirHandshakePublicKey, ourHandshakeKeyPair, inputs); + } + + @Override + public SecretKey deriveHandshakeRootKey(SecretKey staticMasterKey, + boolean pendingContact) { + String label = pendingContact ? + PENDING_CONTACT_ROOT_KEY_LABEL : CONTACT_ROOT_KEY_LABEL; + return crypto.deriveKey(label, staticMasterKey); + } + @Override public TransportKeys deriveRotationKeys(TransportId t, SecretKey rootKey, long timePeriod, boolean weAreAlice, From 810d45d6b99e8baaf992ea6217008944ae72250a Mon Sep 17 00:00:00 2001 From: akwizgran Date: Thu, 30 May 2019 15:48:26 +0100 Subject: [PATCH 03/10] Derive handshake root key when adding a pending contact. --- .../bramble/api/crypto/TransportCrypto.java | 7 ++++ .../bramble/api/transport/KeyManager.java | 9 ++--- .../bramble/contact/ContactManagerImpl.java | 13 +++++++- .../bramble/crypto/TransportCryptoImpl.java | 12 +++++-- .../bramble/transport/KeyManagerImpl.java | 24 +++++++++++--- .../bramble/transport/KeyManagerImplTest.java | 33 +++++++++++++++---- 6 files changed, 79 insertions(+), 19 deletions(-) diff --git a/bramble-api/src/main/java/org/briarproject/bramble/api/crypto/TransportCrypto.java b/bramble-api/src/main/java/org/briarproject/bramble/api/crypto/TransportCrypto.java index a58645792..7439c0ec8 100644 --- a/bramble-api/src/main/java/org/briarproject/bramble/api/crypto/TransportCrypto.java +++ b/bramble-api/src/main/java/org/briarproject/bramble/api/crypto/TransportCrypto.java @@ -11,6 +11,12 @@ import java.security.GeneralSecurityException; */ public interface TransportCrypto { + /** + * Returns true if the local peer is Alice. + */ + boolean isAlice(PublicKey theirHandshakePublicKey, + KeyPair ourHandshakeKeyPair); + /** * Derives the static master key shared with a contact or pending contact. */ @@ -19,6 +25,7 @@ public interface TransportCrypto { /** * Derives the handshake mode root key from the static master key. + * * @param pendingContact Whether the static master key is shared with a * pending contact or a contact */ diff --git a/bramble-api/src/main/java/org/briarproject/bramble/api/transport/KeyManager.java b/bramble-api/src/main/java/org/briarproject/bramble/api/transport/KeyManager.java index 75ae88d86..898a45e5f 100644 --- a/bramble-api/src/main/java/org/briarproject/bramble/api/transport/KeyManager.java +++ b/bramble-api/src/main/java/org/briarproject/bramble/api/transport/KeyManager.java @@ -1,12 +1,15 @@ package org.briarproject.bramble.api.transport; import org.briarproject.bramble.api.contact.ContactId; +import org.briarproject.bramble.api.contact.PendingContact; import org.briarproject.bramble.api.contact.PendingContactId; +import org.briarproject.bramble.api.crypto.KeyPair; import org.briarproject.bramble.api.crypto.SecretKey; import org.briarproject.bramble.api.db.DbException; import org.briarproject.bramble.api.db.Transaction; import org.briarproject.bramble.api.plugin.TransportId; +import java.security.GeneralSecurityException; import java.util.Map; import javax.annotation.Nullable; @@ -53,12 +56,10 @@ public interface KeyManager { *

* {@link StreamContext StreamContexts} for the pending contact can be * created after this method has returned. - * - * @param alice True if the local party is Alice */ Map addPendingContact(Transaction txn, - PendingContactId p, SecretKey rootKey, boolean alice) - throws DbException; + PendingContact p, KeyPair ourKeyPair) + throws DbException, GeneralSecurityException; /** * Marks the given transport keys as usable for outgoing streams. diff --git a/bramble-core/src/main/java/org/briarproject/bramble/contact/ContactManagerImpl.java b/bramble-core/src/main/java/org/briarproject/bramble/contact/ContactManagerImpl.java index 7bea2930e..30c3b3c42 100644 --- a/bramble-core/src/main/java/org/briarproject/bramble/contact/ContactManagerImpl.java +++ b/bramble-core/src/main/java/org/briarproject/bramble/contact/ContactManagerImpl.java @@ -8,6 +8,7 @@ import org.briarproject.bramble.api.contact.ContactManager; import org.briarproject.bramble.api.contact.PendingContact; import org.briarproject.bramble.api.contact.PendingContactId; import org.briarproject.bramble.api.contact.PendingContactState; +import org.briarproject.bramble.api.crypto.KeyPair; import org.briarproject.bramble.api.crypto.SecretKey; import org.briarproject.bramble.api.db.DatabaseComponent; import org.briarproject.bramble.api.db.DbException; @@ -21,6 +22,7 @@ import org.briarproject.bramble.api.identity.LocalAuthor; import org.briarproject.bramble.api.nullsafety.NotNullByDefault; import org.briarproject.bramble.api.transport.KeyManager; +import java.security.GeneralSecurityException; import java.util.ArrayList; import java.util.Collection; import java.util.List; @@ -51,6 +53,7 @@ class ContactManagerImpl implements ContactManager { private final KeyManager keyManager; private final IdentityManager identityManager; private final PendingContactFactory pendingContactFactory; + private final List hooks; @Inject @@ -123,7 +126,15 @@ class ContactManagerImpl implements ContactManager { throws DbException, FormatException { PendingContact p = pendingContactFactory.createPendingContact(link, alias); - db.transaction(false, txn -> db.addPendingContact(txn, p)); + db.transaction(false, txn -> { + db.addPendingContact(txn, p); + KeyPair ourKeyPair = identityManager.getHandshakeKeys(txn); + try { + keyManager.addPendingContact(txn, p, ourKeyPair); + } catch (GeneralSecurityException e) { + throw new AssertionError(); + } + }); return p; } diff --git a/bramble-core/src/main/java/org/briarproject/bramble/crypto/TransportCryptoImpl.java b/bramble-core/src/main/java/org/briarproject/bramble/crypto/TransportCryptoImpl.java index aa8d4d665..2bf4c7b21 100644 --- a/bramble-core/src/main/java/org/briarproject/bramble/crypto/TransportCryptoImpl.java +++ b/bramble-core/src/main/java/org/briarproject/bramble/crypto/TransportCryptoImpl.java @@ -48,6 +48,14 @@ class TransportCryptoImpl implements TransportCrypto { this.crypto = crypto; } + @Override + public boolean isAlice(PublicKey theirHandshakePublicKey, + KeyPair ourHandshakeKeyPair) { + byte[] theirPublic = theirHandshakePublicKey.getEncoded(); + byte[] ourPublic = ourHandshakeKeyPair.getPublic().getEncoded(); + return compare(ourPublic, theirPublic) < 0; + } + @Override public SecretKey deriveStaticMasterKey(PublicKey theirHandshakePublicKey, KeyPair ourHandshakeKeyPair) throws GeneralSecurityException { @@ -55,8 +63,8 @@ class TransportCryptoImpl implements TransportCrypto { byte[] ourPublic = ourHandshakeKeyPair.getPublic().getEncoded(); boolean alice = compare(ourPublic, theirPublic) < 0; byte[][] inputs = { - alice ? ourPublic : theirPublic, - alice ? theirPublic : ourPublic + alice ? ourPublic : theirPublic, + alice ? theirPublic : ourPublic }; return crypto.deriveSharedSecret(STATIC_MASTER_KEY_LABEL, theirHandshakePublicKey, ourHandshakeKeyPair, inputs); diff --git a/bramble-core/src/main/java/org/briarproject/bramble/transport/KeyManagerImpl.java b/bramble-core/src/main/java/org/briarproject/bramble/transport/KeyManagerImpl.java index a508c2e83..a979e10b5 100644 --- a/bramble-core/src/main/java/org/briarproject/bramble/transport/KeyManagerImpl.java +++ b/bramble-core/src/main/java/org/briarproject/bramble/transport/KeyManagerImpl.java @@ -1,9 +1,12 @@ package org.briarproject.bramble.transport; import org.briarproject.bramble.api.contact.ContactId; +import org.briarproject.bramble.api.contact.PendingContact; import org.briarproject.bramble.api.contact.PendingContactId; import org.briarproject.bramble.api.contact.event.ContactRemovedEvent; +import org.briarproject.bramble.api.crypto.KeyPair; import org.briarproject.bramble.api.crypto.SecretKey; +import org.briarproject.bramble.api.crypto.TransportCrypto; import org.briarproject.bramble.api.db.DatabaseComponent; import org.briarproject.bramble.api.db.DatabaseExecutor; import org.briarproject.bramble.api.db.DbException; @@ -21,6 +24,7 @@ import org.briarproject.bramble.api.transport.KeyManager; import org.briarproject.bramble.api.transport.KeySetId; import org.briarproject.bramble.api.transport.StreamContext; +import java.security.GeneralSecurityException; import java.util.HashMap; import java.util.Map; import java.util.Map.Entry; @@ -46,17 +50,22 @@ class KeyManagerImpl implements KeyManager, Service, EventListener { private final Executor dbExecutor; private final PluginConfig pluginConfig; private final TransportKeyManagerFactory transportKeyManagerFactory; + private final TransportCrypto transportCrypto; + private final ConcurrentHashMap managers; private final AtomicBoolean used = new AtomicBoolean(false); @Inject - KeyManagerImpl(DatabaseComponent db, @DatabaseExecutor Executor dbExecutor, + KeyManagerImpl(DatabaseComponent db, + @DatabaseExecutor Executor dbExecutor, PluginConfig pluginConfig, - TransportKeyManagerFactory transportKeyManagerFactory) { + TransportKeyManagerFactory transportKeyManagerFactory, + TransportCrypto transportCrypto) { this.db = db; this.dbExecutor = dbExecutor; this.pluginConfig = pluginConfig; this.transportKeyManagerFactory = transportKeyManagerFactory; + this.transportCrypto = transportCrypto; managers = new ConcurrentHashMap<>(); } @@ -118,13 +127,18 @@ class KeyManagerImpl implements KeyManager, Service, EventListener { @Override public Map addPendingContact(Transaction txn, - PendingContactId p, SecretKey rootKey, boolean alice) - throws DbException { + PendingContact p, KeyPair ourKeyPair) + throws DbException, GeneralSecurityException { + SecretKey staticMasterKey = transportCrypto + .deriveStaticMasterKey(p.getPublicKey(), ourKeyPair); + SecretKey rootKey = + transportCrypto.deriveHandshakeRootKey(staticMasterKey, true); + boolean alice = transportCrypto.isAlice(p.getPublicKey(), ourKeyPair); Map ids = new HashMap<>(); for (Entry e : managers.entrySet()) { TransportId t = e.getKey(); TransportKeyManager m = e.getValue(); - ids.put(t, m.addPendingContact(txn, p, rootKey, alice)); + ids.put(t, m.addPendingContact(txn, p.getId(), rootKey, alice)); } return ids; } diff --git a/bramble-core/src/test/java/org/briarproject/bramble/transport/KeyManagerImplTest.java b/bramble-core/src/test/java/org/briarproject/bramble/transport/KeyManagerImplTest.java index 0220c358b..95c03a3fd 100644 --- a/bramble-core/src/test/java/org/briarproject/bramble/transport/KeyManagerImplTest.java +++ b/bramble-core/src/test/java/org/briarproject/bramble/transport/KeyManagerImplTest.java @@ -1,9 +1,12 @@ package org.briarproject.bramble.transport; import org.briarproject.bramble.api.contact.ContactId; +import org.briarproject.bramble.api.contact.PendingContact; import org.briarproject.bramble.api.contact.PendingContactId; import org.briarproject.bramble.api.contact.event.ContactRemovedEvent; +import org.briarproject.bramble.api.crypto.KeyPair; import org.briarproject.bramble.api.crypto.SecretKey; +import org.briarproject.bramble.api.crypto.TransportCrypto; import org.briarproject.bramble.api.db.DatabaseComponent; import org.briarproject.bramble.api.db.Transaction; import org.briarproject.bramble.api.plugin.PluginConfig; @@ -25,9 +28,11 @@ import java.util.Random; import static java.util.Collections.singletonList; import static java.util.Collections.singletonMap; import static org.briarproject.bramble.api.transport.TransportConstants.TAG_LENGTH; +import static org.briarproject.bramble.test.TestUtils.getAgreementPrivateKey; +import static org.briarproject.bramble.test.TestUtils.getAgreementPublicKey; import static org.briarproject.bramble.test.TestUtils.getContactId; +import static org.briarproject.bramble.test.TestUtils.getPendingContact; import static org.briarproject.bramble.test.TestUtils.getRandomBytes; -import static org.briarproject.bramble.test.TestUtils.getRandomId; import static org.briarproject.bramble.test.TestUtils.getSecretKey; import static org.briarproject.bramble.test.TestUtils.getTransportId; import static org.junit.Assert.assertEquals; @@ -41,12 +46,14 @@ public class KeyManagerImplTest extends BrambleMockTestCase { context.mock(TransportKeyManagerFactory.class); private final TransportKeyManager transportKeyManager = context.mock(TransportKeyManager.class); + private final TransportCrypto transportCrypto = + context.mock(TransportCrypto.class); private final DeterministicExecutor executor = new DeterministicExecutor(); private final Transaction txn = new Transaction(null, false); private final ContactId contactId = getContactId(); - private final PendingContactId pendingContactId = - new PendingContactId(getRandomId()); + private final PendingContact pendingContact = getPendingContact(); + private final PendingContactId pendingContactId = pendingContact.getId(); private final KeySetId keySetId = new KeySetId(345); private final TransportId transportId = getTransportId(); private final TransportId unknownTransportId = getTransportId(); @@ -60,7 +67,7 @@ public class KeyManagerImplTest extends BrambleMockTestCase { private final Random random = new Random(); private final KeyManagerImpl keyManager = new KeyManagerImpl(db, executor, - pluginConfig, transportKeyManagerFactory); + pluginConfig, transportKeyManagerFactory, transportCrypto); @Before public void testStartService() throws Exception { @@ -126,17 +133,29 @@ public class KeyManagerImplTest extends BrambleMockTestCase { @Test public void testAddPendingContact() throws Exception { - SecretKey secretKey = getSecretKey(); + KeyPair ourKeyPair = + new KeyPair(getAgreementPublicKey(), getAgreementPrivateKey()); + SecretKey staticMasterKey = getSecretKey(); + SecretKey rootKey = getSecretKey(); boolean alice = random.nextBoolean(); context.checking(new Expectations() {{ + oneOf(transportCrypto).deriveStaticMasterKey( + pendingContact.getPublicKey(), ourKeyPair); + will(returnValue(staticMasterKey)); + oneOf(transportCrypto) + .deriveHandshakeRootKey(staticMasterKey, true); + will(returnValue(rootKey)); + oneOf(transportCrypto).isAlice(pendingContact.getPublicKey(), + ourKeyPair); + will(returnValue(alice)); oneOf(transportKeyManager).addPendingContact(txn, pendingContactId, - secretKey, alice); + rootKey, alice); will(returnValue(keySetId)); }}); Map ids = keyManager.addPendingContact(txn, - pendingContactId, secretKey, alice); + pendingContact, ourKeyPair); assertEquals(singletonMap(transportId, keySetId), ids); } From 83dc52572d7a620b34660aa2ca4ec3bb589e80e0 Mon Sep 17 00:00:00 2001 From: akwizgran Date: Thu, 30 May 2019 16:20:57 +0100 Subject: [PATCH 04/10] Remove keys when pending contacts are removed. --- .../bramble/transport/KeyManagerImpl.java | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/bramble-core/src/main/java/org/briarproject/bramble/transport/KeyManagerImpl.java b/bramble-core/src/main/java/org/briarproject/bramble/transport/KeyManagerImpl.java index a979e10b5..c3f656768 100644 --- a/bramble-core/src/main/java/org/briarproject/bramble/transport/KeyManagerImpl.java +++ b/bramble-core/src/main/java/org/briarproject/bramble/transport/KeyManagerImpl.java @@ -4,6 +4,7 @@ import org.briarproject.bramble.api.contact.ContactId; import org.briarproject.bramble.api.contact.PendingContact; import org.briarproject.bramble.api.contact.PendingContactId; import org.briarproject.bramble.api.contact.event.ContactRemovedEvent; +import org.briarproject.bramble.api.contact.event.PendingContactRemovedEvent; import org.briarproject.bramble.api.crypto.KeyPair; import org.briarproject.bramble.api.crypto.SecretKey; import org.briarproject.bramble.api.crypto.TransportCrypto; @@ -12,6 +13,7 @@ import org.briarproject.bramble.api.db.DatabaseExecutor; import org.briarproject.bramble.api.db.DbException; import org.briarproject.bramble.api.db.Transaction; import org.briarproject.bramble.api.event.Event; +import org.briarproject.bramble.api.event.EventExecutor; import org.briarproject.bramble.api.event.EventListener; import org.briarproject.bramble.api.lifecycle.Service; import org.briarproject.bramble.api.lifecycle.ServiceException; @@ -194,15 +196,27 @@ class KeyManagerImpl implements KeyManager, Service, EventListener { public void eventOccurred(Event e) { if (e instanceof ContactRemovedEvent) { removeContact(((ContactRemovedEvent) e).getContactId()); + } else if (e instanceof PendingContactRemovedEvent) { + PendingContactRemovedEvent p = (PendingContactRemovedEvent) e; + removePendingContact(p.getId()); } } + @EventExecutor private void removeContact(ContactId c) { dbExecutor.execute(() -> { for (TransportKeyManager m : managers.values()) m.removeContact(c); }); } + @EventExecutor + private void removePendingContact(PendingContactId p) { + dbExecutor.execute(() -> { + for (TransportKeyManager m : managers.values()) + m.removePendingContact(p); + }); + } + @Nullable private T withManager(TransportId t, ManagerTask task) throws DbException { From 4a2936c6859e3b3f60a4871b52b7c1941c5697b3 Mon Sep 17 00:00:00 2001 From: akwizgran Date: Thu, 30 May 2019 16:36:25 +0100 Subject: [PATCH 05/10] Optionally include handshake public key for new contact. --- .../bramble/api/db/DatabaseComponent.java | 9 +- .../bramble/contact/ContactManagerImpl.java | 9 +- .../org/briarproject/bramble/db/Database.java | 12 +- .../bramble/db/DatabaseComponentImpl.java | 28 +---- .../briarproject/bramble/db/JdbcDatabase.java | 53 ++------- .../contact/ContactManagerImplTest.java | 2 +- .../bramble/db/DatabaseComponentImplTest.java | 89 ++------------ .../bramble/db/DatabasePerformanceTest.java | 2 +- .../bramble/db/JdbcDatabaseTest.java | 112 +++++------------- 9 files changed, 60 insertions(+), 256 deletions(-) diff --git a/bramble-api/src/main/java/org/briarproject/bramble/api/db/DatabaseComponent.java b/bramble-api/src/main/java/org/briarproject/bramble/api/db/DatabaseComponent.java index a953d7095..775a0d869 100644 --- a/bramble-api/src/main/java/org/briarproject/bramble/api/db/DatabaseComponent.java +++ b/bramble-api/src/main/java/org/briarproject/bramble/api/db/DatabaseComponent.java @@ -61,14 +61,7 @@ public interface DatabaseComponent extends TransactionManager { * and returns an ID for the contact. */ ContactId addContact(Transaction txn, Author remote, AuthorId local, - boolean verified) throws DbException; - - /** - * Stores a contact associated with the given local and remote pseudonyms, - * replacing the given pending contact, and returns an ID for the contact. - */ - ContactId addContact(Transaction txn, PendingContactId p, Author remote, - AuthorId local, boolean verified) throws DbException; + @Nullable PublicKey handshake, boolean verified) throws DbException; /** * Stores a group. diff --git a/bramble-core/src/main/java/org/briarproject/bramble/contact/ContactManagerImpl.java b/bramble-core/src/main/java/org/briarproject/bramble/contact/ContactManagerImpl.java index 30c3b3c42..b094e5272 100644 --- a/bramble-core/src/main/java/org/briarproject/bramble/contact/ContactManagerImpl.java +++ b/bramble-core/src/main/java/org/briarproject/bramble/contact/ContactManagerImpl.java @@ -9,6 +9,7 @@ import org.briarproject.bramble.api.contact.PendingContact; import org.briarproject.bramble.api.contact.PendingContactId; import org.briarproject.bramble.api.contact.PendingContactState; import org.briarproject.bramble.api.crypto.KeyPair; +import org.briarproject.bramble.api.crypto.PublicKey; import org.briarproject.bramble.api.crypto.SecretKey; import org.briarproject.bramble.api.db.DatabaseComponent; import org.briarproject.bramble.api.db.DbException; @@ -76,7 +77,7 @@ class ContactManagerImpl implements ContactManager { public ContactId addContact(Transaction txn, Author remote, AuthorId local, SecretKey rootKey, long timestamp, boolean alice, boolean verified, boolean active) throws DbException { - ContactId c = db.addContact(txn, remote, local, verified); + ContactId c = db.addContact(txn, remote, local, null, verified); keyManager.addContactWithRotationKeys(txn, c, rootKey, timestamp, alice, active); Contact contact = db.getContact(txn, c); @@ -89,7 +90,9 @@ class ContactManagerImpl implements ContactManager { Author remote, AuthorId local, SecretKey rootKey, long timestamp, boolean alice, boolean verified, boolean active) throws DbException { - ContactId c = db.addContact(txn, p, remote, local, verified); + PublicKey handshake = db.getPendingContact(txn, p).getPublicKey(); + db.removePendingContact(txn, p); + ContactId c = db.addContact(txn, remote, local, handshake, verified); keyManager.addContactWithRotationKeys(txn, c, rootKey, timestamp, alice, active); Contact contact = db.getContact(txn, c); @@ -100,7 +103,7 @@ class ContactManagerImpl implements ContactManager { @Override public ContactId addContact(Transaction txn, Author remote, AuthorId local, boolean verified) throws DbException { - ContactId c = db.addContact(txn, remote, local, verified); + ContactId c = db.addContact(txn, remote, local, null, verified); Contact contact = db.getContact(txn, c); for (ContactHook hook : hooks) hook.addingContact(txn, contact); return c; diff --git a/bramble-core/src/main/java/org/briarproject/bramble/db/Database.java b/bramble-core/src/main/java/org/briarproject/bramble/db/Database.java index 47ef5e0b3..65203229a 100644 --- a/bramble-core/src/main/java/org/briarproject/bramble/db/Database.java +++ b/bramble-core/src/main/java/org/briarproject/bramble/db/Database.java @@ -88,8 +88,8 @@ interface Database { * Stores a contact associated with the given local and remote pseudonyms, * and returns an ID for the contact. */ - ContactId addContact(T txn, Author remote, AuthorId local, boolean verified) - throws DbException; + ContactId addContact(T txn, Author remote, AuthorId local, + @Nullable PublicKey handshake, boolean verified) throws DbException; /** * Stores a group. @@ -695,14 +695,6 @@ interface Database { void setTransportKeysActive(T txn, TransportId t, KeySetId k) throws DbException; - /** - * Transfers ownership of any transport keys from the given pending contact - * to the given contact and copies the pending contact's handshake public - * key to the contact. - */ - void transferKeys(T txn, PendingContactId p, ContactId c) - throws DbException; - /** * Updates the transmission count, expiry time and estimated time of arrival * of the given message with respect to the given contact, using the latency diff --git a/bramble-core/src/main/java/org/briarproject/bramble/db/DatabaseComponentImpl.java b/bramble-core/src/main/java/org/briarproject/bramble/db/DatabaseComponentImpl.java index 876d419b2..666c54a23 100644 --- a/bramble-core/src/main/java/org/briarproject/bramble/db/DatabaseComponentImpl.java +++ b/bramble-core/src/main/java/org/briarproject/bramble/db/DatabaseComponentImpl.java @@ -85,6 +85,7 @@ import javax.annotation.concurrent.ThreadSafe; import javax.inject.Inject; import static java.util.logging.Level.WARNING; +import static java.util.logging.Logger.getLogger; import static org.briarproject.bramble.api.sync.Group.Visibility.INVISIBLE; import static org.briarproject.bramble.api.sync.Group.Visibility.SHARED; import static org.briarproject.bramble.api.sync.validation.MessageState.DELIVERED; @@ -99,7 +100,7 @@ import static org.briarproject.bramble.util.LogUtils.now; class DatabaseComponentImpl implements DatabaseComponent { private static final Logger LOG = - Logger.getLogger(DatabaseComponentImpl.class.getName()); + getLogger(DatabaseComponentImpl.class.getName()); private final Database db; private final Class txnClass; @@ -234,39 +235,18 @@ class DatabaseComponentImpl implements DatabaseComponent { @Override public ContactId addContact(Transaction transaction, Author remote, - AuthorId local, boolean verified) throws DbException { - if (transaction.isReadOnly()) throw new IllegalArgumentException(); - T txn = unbox(transaction); - if (!db.containsIdentity(txn, local)) - throw new NoSuchIdentityException(); - if (db.containsIdentity(txn, remote.getId())) - throw new ContactExistsException(local, remote); - if (db.containsContact(txn, remote.getId(), local)) - throw new ContactExistsException(local, remote); - ContactId c = db.addContact(txn, remote, local, verified); - transaction.attach(new ContactAddedEvent(c, verified)); - return c; - } - - @Override - public ContactId addContact(Transaction transaction, PendingContactId p, - Author remote, AuthorId local, boolean verified) + AuthorId local, @Nullable PublicKey handshake, boolean verified) throws DbException { if (transaction.isReadOnly()) throw new IllegalArgumentException(); T txn = unbox(transaction); - if (!db.containsPendingContact(txn, p)) - throw new NoSuchPendingContactException(); if (!db.containsIdentity(txn, local)) throw new NoSuchIdentityException(); if (db.containsIdentity(txn, remote.getId())) throw new ContactExistsException(local, remote); if (db.containsContact(txn, remote.getId(), local)) throw new ContactExistsException(local, remote); - ContactId c = db.addContact(txn, remote, local, verified); - db.transferKeys(txn, p, c); - db.removePendingContact(txn, p); + ContactId c = db.addContact(txn, remote, local, handshake, verified); transaction.attach(new ContactAddedEvent(c, verified)); - transaction.attach(new PendingContactRemovedEvent(p)); return c; } diff --git a/bramble-core/src/main/java/org/briarproject/bramble/db/JdbcDatabase.java b/bramble-core/src/main/java/org/briarproject/bramble/db/JdbcDatabase.java index f3d80a8e5..7b51a6e29 100644 --- a/bramble-core/src/main/java/org/briarproject/bramble/db/JdbcDatabase.java +++ b/bramble-core/src/main/java/org/briarproject/bramble/db/JdbcDatabase.java @@ -629,22 +629,25 @@ abstract class JdbcDatabase implements Database { @Override public ContactId addContact(Connection txn, Author remote, AuthorId local, - boolean verified) throws DbException { + @Nullable PublicKey handshake, boolean verified) + throws DbException { PreparedStatement ps = null; ResultSet rs = null; try { // Create a contact row String sql = "INSERT INTO contacts" + " (authorId, formatVersion, name, publicKey," - + " localAuthorId, verified)" - + " VALUES (?, ?, ?, ?, ?, ?)"; + + " localAuthorId, handshakePublicKey, verified)" + + " VALUES (?, ?, ?, ?, ?, ?, ?)"; ps = txn.prepareStatement(sql); ps.setBytes(1, remote.getId().getBytes()); ps.setInt(2, remote.getFormatVersion()); ps.setString(3, remote.getName()); ps.setBytes(4, remote.getPublicKey().getEncoded()); ps.setBytes(5, local.getBytes()); - ps.setBoolean(6, verified); + if (handshake == null) ps.setNull(6, BINARY); + else ps.setBytes(6, handshake.getEncoded()); + ps.setBoolean(7, verified); int affected = ps.executeUpdate(); if (affected != 1) throw new DbStateException(); ps.close(); @@ -3139,48 +3142,6 @@ abstract class JdbcDatabase implements Database { } } - @Override - public void transferKeys(Connection txn, PendingContactId p, ContactId c) - throws DbException { - PreparedStatement ps = null; - ResultSet rs = null; - try { - // Transfer the handshake public key - String sql = "SELECT publicKey from pendingContacts" - + " WHERE pendingContactId = ?"; - ps = txn.prepareStatement(sql); - ps.setBytes(1, p.getBytes()); - rs = ps.executeQuery(); - if (!rs.next()) throw new DbStateException(); - byte[] publicKey = rs.getBytes(1); - if (rs.next()) throw new DbStateException(); - rs.close(); - ps.close(); - sql = "UPDATE contacts SET handshakePublicKey = ?" - + " WHERE contactId = ?"; - ps = txn.prepareStatement(sql); - ps.setBytes(1, publicKey); - ps.setInt(2, c.getInt()); - int affected = ps.executeUpdate(); - if (affected < 0 || affected > 1) throw new DbStateException(); - ps.close(); - // Transfer the transport keys - sql = "UPDATE outgoingKeys" - + " SET contactId = ?, pendingContactId = NULL" - + " WHERE pendingContactId = ?"; - ps = txn.prepareStatement(sql); - ps.setInt(1, c.getInt()); - ps.setBytes(2, p.getBytes()); - affected = ps.executeUpdate(); - if (affected < 0) throw new DbStateException(); - ps.close(); - } catch (SQLException e) { - tryToClose(rs, LOG, WARNING); - tryToClose(ps, LOG, WARNING); - throw new DbException(e); - } - } - @Override public void updateExpiryTimeAndEta(Connection txn, ContactId c, MessageId m, int maxLatency) throws DbException { diff --git a/bramble-core/src/test/java/org/briarproject/bramble/contact/ContactManagerImplTest.java b/bramble-core/src/test/java/org/briarproject/bramble/contact/ContactManagerImplTest.java index 86a3932ee..059cb7aa9 100644 --- a/bramble-core/src/test/java/org/briarproject/bramble/contact/ContactManagerImplTest.java +++ b/bramble-core/src/test/java/org/briarproject/bramble/contact/ContactManagerImplTest.java @@ -71,7 +71,7 @@ public class ContactManagerImplTest extends BrambleMockTestCase { context.checking(new DbExpectations() {{ oneOf(db).transactionWithResult(with(false), withDbCallable(txn)); - oneOf(db).addContact(txn, remote, local, verified); + oneOf(db).addContact(txn, remote, local, null, verified); will(returnValue(contactId)); oneOf(keyManager).addContactWithRotationKeys(txn, contactId, rootKey, timestamp, alice, active); diff --git a/bramble-core/src/test/java/org/briarproject/bramble/db/DatabaseComponentImplTest.java b/bramble-core/src/test/java/org/briarproject/bramble/db/DatabaseComponentImplTest.java index 9afc37b18..1570d1b08 100644 --- a/bramble-core/src/test/java/org/briarproject/bramble/db/DatabaseComponentImplTest.java +++ b/bramble-core/src/test/java/org/briarproject/bramble/db/DatabaseComponentImplTest.java @@ -176,7 +176,8 @@ public class DatabaseComponentImplTest extends BrambleMockTestCase { oneOf(database).containsContact(txn, author.getId(), localAuthor.getId()); will(returnValue(false)); - oneOf(database).addContact(txn, author, localAuthor.getId(), true); + oneOf(database).addContact(txn, author, localAuthor.getId(), + null, true); will(returnValue(contactId)); oneOf(eventBus).broadcast(with(any(ContactAddedEvent.class))); // getContacts() @@ -224,7 +225,7 @@ public class DatabaseComponentImplTest extends BrambleMockTestCase { db.transaction(false, transaction -> { db.addIdentity(transaction, identity); assertEquals(contactId, db.addContact(transaction, author, - localAuthor.getId(), true)); + localAuthor.getId(), null, true)); assertEquals(singletonList(contact), db.getContacts(transaction)); db.addGroup(transaction, group); // First time - listeners called @@ -445,7 +446,7 @@ public class DatabaseComponentImplTest extends BrambleMockTestCase { try { db.transaction(false, transaction -> db.addContact(transaction, author, localAuthor.getId(), - true)); + null, true)); fail(); } catch (NoSuchIdentityException expected) { // Expected @@ -763,25 +764,16 @@ public class DatabaseComponentImplTest extends BrambleMockTestCase { throws Exception { context.checking(new Expectations() {{ // Check whether the pending contact is in the DB (which it's not) - exactly(4).of(database).startTransaction(); + exactly(3).of(database).startTransaction(); will(returnValue(txn)); - exactly(4).of(database).containsPendingContact(txn, + exactly(3).of(database).containsPendingContact(txn, pendingContactId); will(returnValue(false)); - exactly(4).of(database).abortTransaction(txn); + exactly(3).of(database).abortTransaction(txn); }}); DatabaseComponent db = createDatabaseComponent(database, eventBus, eventExecutor, shutdownManager); - try { - db.transaction(false, transaction -> - db.addContact(transaction, pendingContactId, author, - localAuthor.getId(), true)); - fail(); - } catch (NoSuchPendingContactException expected) { - // Expected - } - try { db.transaction(false, transaction -> db.addTransportKeys(transaction, pendingContactId, @@ -1473,7 +1465,7 @@ public class DatabaseComponentImplTest extends BrambleMockTestCase { try { db.transaction(false, transaction -> db.addContact(transaction, author, localAuthor.getId(), - true)); + null, true)); fail(); } catch (ContactExistsException expected) { assertEquals(localAuthor.getId(), expected.getLocalAuthorId()); @@ -1503,70 +1495,7 @@ public class DatabaseComponentImplTest extends BrambleMockTestCase { try { db.transaction(false, transaction -> db.addContact(transaction, author, localAuthor.getId(), - true)); - fail(); - } catch (ContactExistsException expected) { - assertEquals(localAuthor.getId(), expected.getLocalAuthorId()); - assertEquals(author, expected.getRemoteAuthor()); - } - } - - @Test - public void testCannotAddLocalIdentityAsContactFromPendingContact() - throws Exception { - context.checking(new Expectations() {{ - oneOf(database).startTransaction(); - will(returnValue(txn)); - oneOf(database).containsPendingContact(txn, pendingContactId); - will(returnValue(true)); - oneOf(database).containsIdentity(txn, localAuthor.getId()); - will(returnValue(true)); - // Contact is a local identity - oneOf(database).containsIdentity(txn, author.getId()); - will(returnValue(true)); - oneOf(database).abortTransaction(txn); - }}); - - DatabaseComponent db = createDatabaseComponent(database, eventBus, - eventExecutor, shutdownManager); - - try { - db.transaction(false, transaction -> - db.addContact(transaction, pendingContactId, author, - localAuthor.getId(), true)); - fail(); - } catch (ContactExistsException expected) { - assertEquals(localAuthor.getId(), expected.getLocalAuthorId()); - assertEquals(author, expected.getRemoteAuthor()); - } - } - - @Test - public void testCannotAddDuplicateContactFromPendingContact() - throws Exception { - context.checking(new Expectations() {{ - oneOf(database).startTransaction(); - will(returnValue(txn)); - oneOf(database).containsPendingContact(txn, pendingContactId); - will(returnValue(true)); - oneOf(database).containsIdentity(txn, localAuthor.getId()); - will(returnValue(true)); - oneOf(database).containsIdentity(txn, author.getId()); - will(returnValue(false)); - // Contact already exists for this local identity - oneOf(database).containsContact(txn, author.getId(), - localAuthor.getId()); - will(returnValue(true)); - oneOf(database).abortTransaction(txn); - }}); - - DatabaseComponent db = createDatabaseComponent(database, eventBus, - eventExecutor, shutdownManager); - - try { - db.transaction(false, transaction -> - db.addContact(transaction, pendingContactId, author, - localAuthor.getId(), true)); + null, true)); fail(); } catch (ContactExistsException expected) { assertEquals(localAuthor.getId(), expected.getLocalAuthorId()); diff --git a/bramble-core/src/test/java/org/briarproject/bramble/db/DatabasePerformanceTest.java b/bramble-core/src/test/java/org/briarproject/bramble/db/DatabasePerformanceTest.java index fe060e3df..556634431 100644 --- a/bramble-core/src/test/java/org/briarproject/bramble/db/DatabasePerformanceTest.java +++ b/bramble-core/src/test/java/org/briarproject/bramble/db/DatabasePerformanceTest.java @@ -548,7 +548,7 @@ public abstract class DatabasePerformanceTest extends BrambleTestCase { db.addIdentity(txn, identity); for (int i = 0; i < CONTACTS; i++) { ContactId c = db.addContact(txn, getAuthor(), localAuthor.getId(), - random.nextBoolean()); + null, random.nextBoolean()); contacts.add(db.getContact(txn, c)); contactGroups.put(c, new ArrayList<>()); for (int j = 0; j < GROUPS_PER_CONTACT; j++) { diff --git a/bramble-core/src/test/java/org/briarproject/bramble/db/JdbcDatabaseTest.java b/bramble-core/src/test/java/org/briarproject/bramble/db/JdbcDatabaseTest.java index bdbe53240..1bf92d83a 100644 --- a/bramble-core/src/test/java/org/briarproject/bramble/db/JdbcDatabaseTest.java +++ b/bramble-core/src/test/java/org/briarproject/bramble/db/JdbcDatabaseTest.java @@ -147,7 +147,7 @@ public abstract class JdbcDatabaseTest extends BrambleTestCase { assertFalse(db.containsContact(txn, contactId)); db.addIdentity(txn, identity); assertEquals(contactId, - db.addContact(txn, author, localAuthor.getId(), true)); + db.addContact(txn, author, localAuthor.getId(), null, true)); assertTrue(db.containsContact(txn, contactId)); assertFalse(db.containsGroup(txn, groupId)); db.addGroup(txn, group); @@ -210,7 +210,7 @@ public abstract class JdbcDatabaseTest extends BrambleTestCase { // Add a contact, a shared group and a shared message db.addIdentity(txn, identity); assertEquals(contactId, - db.addContact(txn, author, localAuthor.getId(), true)); + db.addContact(txn, author, localAuthor.getId(), null, true)); db.addGroup(txn, group); db.addGroupVisibility(txn, contactId, groupId, true); db.addMessage(txn, message, DELIVERED, true, null); @@ -241,7 +241,7 @@ public abstract class JdbcDatabaseTest extends BrambleTestCase { // Add a contact, a shared group and a shared but unvalidated message db.addIdentity(txn, identity); assertEquals(contactId, - db.addContact(txn, author, localAuthor.getId(), true)); + db.addContact(txn, author, localAuthor.getId(), null, true)); db.addGroup(txn, group); db.addGroupVisibility(txn, contactId, groupId, true); db.addMessage(txn, message, UNKNOWN, true, null); @@ -286,7 +286,7 @@ public abstract class JdbcDatabaseTest extends BrambleTestCase { // Add a contact, an invisible group and a shared message db.addIdentity(txn, identity); assertEquals(contactId, - db.addContact(txn, author, localAuthor.getId(), true)); + db.addContact(txn, author, localAuthor.getId(), null, true)); db.addGroup(txn, group); db.addMessage(txn, message, DELIVERED, true, null); @@ -337,7 +337,7 @@ public abstract class JdbcDatabaseTest extends BrambleTestCase { // Add a contact, a shared group and an unshared message db.addIdentity(txn, identity); assertEquals(contactId, - db.addContact(txn, author, localAuthor.getId(), true)); + db.addContact(txn, author, localAuthor.getId(), null, true)); db.addGroup(txn, group); db.addGroupVisibility(txn, contactId, groupId, true); db.addMessage(txn, message, DELIVERED, false, null); @@ -368,7 +368,7 @@ public abstract class JdbcDatabaseTest extends BrambleTestCase { // Add a contact, a shared group and a shared message db.addIdentity(txn, identity); assertEquals(contactId, - db.addContact(txn, author, localAuthor.getId(), true)); + db.addContact(txn, author, localAuthor.getId(), null, true)); db.addGroup(txn, group); db.addGroupVisibility(txn, contactId, groupId, true); db.addMessage(txn, message, DELIVERED, true, null); @@ -395,7 +395,7 @@ public abstract class JdbcDatabaseTest extends BrambleTestCase { // Add a contact and a visible group db.addIdentity(txn, identity); assertEquals(contactId, - db.addContact(txn, author, localAuthor.getId(), true)); + db.addContact(txn, author, localAuthor.getId(), null, true)); db.addGroup(txn, group); db.addGroupVisibility(txn, contactId, groupId, false); @@ -436,7 +436,7 @@ public abstract class JdbcDatabaseTest extends BrambleTestCase { // Add a contact, a shared group and a shared message db.addIdentity(txn, identity); assertEquals(contactId, - db.addContact(txn, author, localAuthor.getId(), true)); + db.addContact(txn, author, localAuthor.getId(), null, true)); db.addGroup(txn, group); db.addGroupVisibility(txn, contactId, groupId, true); db.addMessage(txn, message, DELIVERED, true, null); @@ -568,7 +568,7 @@ public abstract class JdbcDatabaseTest extends BrambleTestCase { // Add a contact and a shared group db.addIdentity(txn, identity); assertEquals(contactId, - db.addContact(txn, author, localAuthor.getId(), true)); + db.addContact(txn, author, localAuthor.getId(), null, true)); db.addGroup(txn, group); db.addGroupVisibility(txn, contactId, groupId, true); @@ -588,7 +588,7 @@ public abstract class JdbcDatabaseTest extends BrambleTestCase { // Add a contact db.addIdentity(txn, identity); assertEquals(contactId, - db.addContact(txn, author, localAuthor.getId(), true)); + db.addContact(txn, author, localAuthor.getId(), null, true)); // The group is not in the database assertFalse(db.containsVisibleMessage(txn, contactId, messageId)); @@ -606,7 +606,7 @@ public abstract class JdbcDatabaseTest extends BrambleTestCase { // Add a contact, an invisible group and a message db.addIdentity(txn, identity); assertEquals(contactId, - db.addContact(txn, author, localAuthor.getId(), true)); + db.addContact(txn, author, localAuthor.getId(), null, true)); db.addGroup(txn, group); db.addMessage(txn, message, DELIVERED, true, null); @@ -625,7 +625,7 @@ public abstract class JdbcDatabaseTest extends BrambleTestCase { // Add a contact and a group db.addIdentity(txn, identity); assertEquals(contactId, - db.addContact(txn, author, localAuthor.getId(), true)); + db.addContact(txn, author, localAuthor.getId(), null, true)); db.addGroup(txn, group); // The group should not be visible to the contact @@ -677,7 +677,7 @@ public abstract class JdbcDatabaseTest extends BrambleTestCase { // Add the contact, the transport and the transport keys db.addIdentity(txn, identity); assertEquals(contactId, - db.addContact(txn, author, localAuthor.getId(), true)); + db.addContact(txn, author, localAuthor.getId(), null, true)); db.addTransport(txn, transportId, 123); assertEquals(keySetId, db.addTransportKeys(txn, contactId, keys)); assertEquals(keySetId1, db.addTransportKeys(txn, contactId, keys1)); @@ -786,7 +786,7 @@ public abstract class JdbcDatabaseTest extends BrambleTestCase { // Add the contact, the transport and the handshake keys db.addIdentity(txn, identity); assertEquals(contactId, - db.addContact(txn, author, localAuthor.getId(), true)); + db.addContact(txn, author, localAuthor.getId(), null, true)); db.addTransport(txn, transportId, 123); assertEquals(keySetId, db.addTransportKeys(txn, contactId, keys)); assertEquals(keySetId1, db.addTransportKeys(txn, contactId, keys1)); @@ -920,7 +920,7 @@ public abstract class JdbcDatabaseTest extends BrambleTestCase { // Add the contact, transport and transport keys db.addIdentity(txn, identity); assertEquals(contactId, - db.addContact(txn, author, localAuthor.getId(), true)); + db.addContact(txn, author, localAuthor.getId(), null, true)); db.addTransport(txn, transportId, 123); assertEquals(keySetId, db.addTransportKeys(txn, contactId, keys)); @@ -964,7 +964,7 @@ public abstract class JdbcDatabaseTest extends BrambleTestCase { // Add the contact, transport and handshake keys db.addIdentity(txn, identity); assertEquals(contactId, - db.addContact(txn, author, localAuthor.getId(), true)); + db.addContact(txn, author, localAuthor.getId(), null, true)); db.addTransport(txn, transportId, 123); assertEquals(keySetId, db.addTransportKeys(txn, contactId, keys)); @@ -1011,7 +1011,7 @@ public abstract class JdbcDatabaseTest extends BrambleTestCase { // Add the contact, transport and transport keys db.addIdentity(txn, identity); assertEquals(contactId, - db.addContact(txn, author, localAuthor.getId(), true)); + db.addContact(txn, author, localAuthor.getId(), null, true)); db.addTransport(txn, transportId, 123); assertEquals(keySetId, db.addTransportKeys(txn, contactId, keys)); @@ -1058,7 +1058,7 @@ public abstract class JdbcDatabaseTest extends BrambleTestCase { // Add the contact, transport and handshake keys db.addIdentity(txn, identity); assertEquals(contactId, - db.addContact(txn, author, localAuthor.getId(), true)); + db.addContact(txn, author, localAuthor.getId(), null, true)); db.addTransport(txn, transportId, 123); assertEquals(keySetId, db.addTransportKeys(txn, contactId, keys)); @@ -1104,7 +1104,7 @@ public abstract class JdbcDatabaseTest extends BrambleTestCase { // Add a contact associated with the local author assertEquals(contactId, - db.addContact(txn, author, localAuthor.getId(), true)); + db.addContact(txn, author, localAuthor.getId(), null, true)); // Ensure contact is returned from database by author ID Collection contacts = @@ -1135,7 +1135,7 @@ public abstract class JdbcDatabaseTest extends BrambleTestCase { // Add a contact associated with the local author assertEquals(contactId, - db.addContact(txn, author, localAuthor.getId(), true)); + db.addContact(txn, author, localAuthor.getId(), null, true)); contacts = db.getContacts(txn, localAuthor.getId()); assertEquals(singletonList(contactId), contacts); @@ -1157,7 +1157,7 @@ public abstract class JdbcDatabaseTest extends BrambleTestCase { // Add a contact - initially there should be no offered messages db.addIdentity(txn, identity); assertEquals(contactId, - db.addContact(txn, author, localAuthor.getId(), true)); + db.addContact(txn, author, localAuthor.getId(), null, true)); assertEquals(0, db.countOfferedMessages(txn, contactId)); // Add some offered messages and count them @@ -1741,7 +1741,7 @@ public abstract class JdbcDatabaseTest extends BrambleTestCase { // Add a contact, a shared group and a shared message db.addIdentity(txn, identity); assertEquals(contactId, - db.addContact(txn, author, localAuthor.getId(), true)); + db.addContact(txn, author, localAuthor.getId(), null, true)); db.addGroup(txn, group); db.addGroupVisibility(txn, contactId, groupId, true); db.addMessage(txn, message, DELIVERED, true, null); @@ -1853,9 +1853,9 @@ public abstract class JdbcDatabaseTest extends BrambleTestCase { // Add the same contact for each local author ContactId contactId = - db.addContact(txn, author, localAuthor.getId(), true); + db.addContact(txn, author, localAuthor.getId(), null, true); ContactId contactId1 = - db.addContact(txn, author, localAuthor1.getId(), true); + db.addContact(txn, author, localAuthor1.getId(), null, true); // The contacts should be distinct assertNotEquals(contactId, contactId1); @@ -1875,7 +1875,7 @@ public abstract class JdbcDatabaseTest extends BrambleTestCase { // Add a contact, a shared group and a shared message db.addIdentity(txn, identity); assertEquals(contactId, - db.addContact(txn, author, localAuthor.getId(), true)); + db.addContact(txn, author, localAuthor.getId(), null, true)); db.addGroup(txn, group); db.addGroupVisibility(txn, contactId, groupId, true); db.addMessage(txn, message, DELIVERED, true, null); @@ -1929,7 +1929,7 @@ public abstract class JdbcDatabaseTest extends BrambleTestCase { // Add a contact db.addIdentity(txn, identity); assertEquals(contactId, - db.addContact(txn, author, localAuthor.getId(), true)); + db.addContact(txn, author, localAuthor.getId(), null, true)); // The contact should have no alias Contact contact = db.getContact(txn, contactId); @@ -1986,7 +1986,7 @@ public abstract class JdbcDatabaseTest extends BrambleTestCase { // Add a contact, a group and a message db.addIdentity(txn, identity); assertEquals(contactId, - db.addContact(txn, author, localAuthor.getId(), true)); + db.addContact(txn, author, localAuthor.getId(), null, true)); db.addGroup(txn, group); db.addMessage(txn, message, UNKNOWN, false, null); @@ -2070,7 +2070,7 @@ public abstract class JdbcDatabaseTest extends BrambleTestCase { // Add a contact, a shared group and a shared message db.addIdentity(txn, identity); assertEquals(contactId, - db.addContact(txn, author, localAuthor.getId(), true)); + db.addContact(txn, author, localAuthor.getId(), null, true)); db.addGroup(txn, group); db.addGroupVisibility(txn, contactId, groupId, true); db.addMessage(txn, message, DELIVERED, true, null); @@ -2115,7 +2115,7 @@ public abstract class JdbcDatabaseTest extends BrambleTestCase { // Add a contact, a shared group and a shared message db.addIdentity(txn, identity); assertEquals(contactId, - db.addContact(txn, author, localAuthor.getId(), true)); + db.addContact(txn, author, localAuthor.getId(), null, true)); db.addGroup(txn, group); db.addGroupVisibility(txn, contactId, groupId, true); db.addMessage(txn, message, DELIVERED, true, null); @@ -2229,60 +2229,6 @@ public abstract class JdbcDatabaseTest extends BrambleTestCase { assertEquals(expected.getTimestamp(), actual.getTimestamp()); } - @Test - public void testTransferKeys() throws Exception { - boolean alice = random.nextBoolean(); - TransportKeys transportKeys = - createHandshakeKeys(1000, getSecretKey(), alice); - - Database db = open(false); - Connection txn = db.startTransaction(); - - // Add the pending contact, the transport and the handshake keys - db.addPendingContact(txn, pendingContact); - db.addTransport(txn, transportId, 123); - assertEquals(keySetId, db.addTransportKeys(txn, pendingContact.getId(), - transportKeys)); - - Collection allKeys = - db.getTransportKeys(txn, transportId); - assertEquals(1, allKeys.size()); - TransportKeySet ks = allKeys.iterator().next(); - assertEquals(keySetId, ks.getKeySetId()); - assertNull(ks.getContactId()); - assertEquals(pendingContact.getId(), ks.getPendingContactId()); - - // Add a contact - db.addIdentity(txn, identity); - assertEquals(contactId, - db.addContact(txn, author, localAuthor.getId(), true)); - - // The contact shouldn't have a handshake public key - Contact contact = db.getContact(txn, contactId); - assertNull(contact.getHandshakePublicKey()); - - // Transfer the keys to the contact - db.transferKeys(txn, pendingContact.getId(), contactId); - - // The handshake public key should have been copied to the contact - contact = db.getContact(txn, contactId); - PublicKey handshakePublicKey = contact.getHandshakePublicKey(); - assertNotNull(handshakePublicKey); - assertArrayEquals(pendingContact.getPublicKey().getEncoded(), - handshakePublicKey.getEncoded()); - - // The transport keys should have been transferred to the contact - allKeys = db.getTransportKeys(txn, transportId); - assertEquals(1, allKeys.size()); - ks = allKeys.iterator().next(); - assertEquals(keySetId, ks.getKeySetId()); - assertEquals(contactId, ks.getContactId()); - assertNull(ks.getPendingContactId()); - - db.commitTransaction(txn); - db.close(); - } - @Test public void testSetHandshakeKeyPair() throws Exception { Identity withoutKeys = new Identity(localAuthor, null, null, From 430b530ca59cc3a006a0af90264443d3708e3f20 Mon Sep 17 00:00:00 2001 From: akwizgran Date: Thu, 30 May 2019 17:21:46 +0100 Subject: [PATCH 06/10] Derive handshake root key when converting pending contact. --- .../bramble/api/contact/ContactManager.java | 10 ++- .../bramble/api/transport/KeyManager.java | 19 +++--- .../bramble/contact/ContactManagerImpl.java | 32 ++++----- .../bramble/transport/KeyManagerImpl.java | 27 +++++--- .../transport/TransportKeyManager.java | 6 +- .../transport/TransportKeyManagerImpl.java | 23 ++++--- .../contact/ContactManagerImplTest.java | 4 +- .../bramble/transport/KeyManagerImplTest.java | 51 ++++++++------ .../TransportKeyManagerImplTest.java | 67 +++++++------------ .../add/remote/AddContactViewModel.java | 6 +- .../IntroduceeProtocolEngine.java | 2 +- 11 files changed, 125 insertions(+), 122 deletions(-) diff --git a/bramble-api/src/main/java/org/briarproject/bramble/api/contact/ContactManager.java b/bramble-api/src/main/java/org/briarproject/bramble/api/contact/ContactManager.java index 9aa8889f1..1391f3674 100644 --- a/bramble-api/src/main/java/org/briarproject/bramble/api/contact/ContactManager.java +++ b/bramble-api/src/main/java/org/briarproject/bramble/api/contact/ContactManager.java @@ -13,6 +13,7 @@ import org.briarproject.bramble.api.identity.AuthorInfo; import org.briarproject.bramble.api.lifecycle.LifecycleManager; import org.briarproject.bramble.api.nullsafety.NotNullByDefault; +import java.security.GeneralSecurityException; import java.util.Collection; import javax.annotation.Nullable; @@ -44,10 +45,13 @@ public interface ContactManager { * for each transport, and returns an ID for the contact. * * @param alice True if the local party is Alice + * @throws GeneralSecurityException If the pending contact's handshake + * public key is invalid */ ContactId addContact(Transaction txn, PendingContactId p, Author remote, AuthorId local, SecretKey rootKey, long timestamp, boolean alice, - boolean verified, boolean active) throws DbException; + boolean verified, boolean active) + throws DbException, GeneralSecurityException; /** * Stores a contact associated with the given local and remote pseudonyms @@ -83,9 +87,11 @@ public interface ContactManager { * @throws UnsupportedVersionException If the link uses a format version * that is not supported * @throws FormatException If the link is invalid + * @throws GeneralSecurityException If the pending contact's handshake + * public key is invalid */ PendingContact addPendingContact(String link, String alias) - throws DbException, FormatException; + throws DbException, FormatException, GeneralSecurityException; /** * Returns the pending contact with the given ID. diff --git a/bramble-api/src/main/java/org/briarproject/bramble/api/transport/KeyManager.java b/bramble-api/src/main/java/org/briarproject/bramble/api/transport/KeyManager.java index 898a45e5f..50f7d8aa0 100644 --- a/bramble-api/src/main/java/org/briarproject/bramble/api/transport/KeyManager.java +++ b/bramble-api/src/main/java/org/briarproject/bramble/api/transport/KeyManager.java @@ -1,9 +1,9 @@ package org.briarproject.bramble.api.transport; import org.briarproject.bramble.api.contact.ContactId; -import org.briarproject.bramble.api.contact.PendingContact; import org.briarproject.bramble.api.contact.PendingContactId; import org.briarproject.bramble.api.crypto.KeyPair; +import org.briarproject.bramble.api.crypto.PublicKey; import org.briarproject.bramble.api.crypto.SecretKey; import org.briarproject.bramble.api.db.DbException; import org.briarproject.bramble.api.db.Transaction; @@ -21,9 +21,9 @@ import javax.annotation.Nullable; public interface KeyManager { /** - * Informs the key manager that a new contact has been added. Derives and - * stores a set of rotation mode transport keys for communicating with the - * contact over each transport and returns the key set IDs. + * Derives and stores a set of rotation mode transport keys for + * communicating with the given contact over each transport and returns the + * key set IDs. *

* {@link StreamContext StreamContexts} for the contact can be created * after this method has returned. @@ -31,7 +31,7 @@ public interface KeyManager { * @param alice True if the local party is Alice * @param active Whether the derived keys can be used for outgoing streams */ - Map addContactWithRotationKeys(Transaction txn, + Map addRotationKeys(Transaction txn, ContactId c, SecretKey rootKey, long timestamp, boolean alice, boolean active) throws DbException; @@ -42,11 +42,10 @@ public interface KeyManager { *

* {@link StreamContext StreamContexts} for the contact can be created * after this method has returned. - * - * @param alice True if the local party is Alice */ - Map addContactWithHandshakeKeys(Transaction txn, - ContactId c, SecretKey rootKey, boolean alice) throws DbException; + Map addContact(Transaction txn, ContactId c, + PublicKey theirPublicKey, KeyPair ourKeyPair) + throws DbException, GeneralSecurityException; /** * Informs the key manager that a new pending contact has been added. @@ -58,7 +57,7 @@ public interface KeyManager { * created after this method has returned. */ Map addPendingContact(Transaction txn, - PendingContact p, KeyPair ourKeyPair) + PendingContactId p, PublicKey theirPublicKey, KeyPair ourKeyPair) throws DbException, GeneralSecurityException; /** diff --git a/bramble-core/src/main/java/org/briarproject/bramble/contact/ContactManagerImpl.java b/bramble-core/src/main/java/org/briarproject/bramble/contact/ContactManagerImpl.java index b094e5272..56a0694b0 100644 --- a/bramble-core/src/main/java/org/briarproject/bramble/contact/ContactManagerImpl.java +++ b/bramble-core/src/main/java/org/briarproject/bramble/contact/ContactManagerImpl.java @@ -78,8 +78,7 @@ class ContactManagerImpl implements ContactManager { SecretKey rootKey, long timestamp, boolean alice, boolean verified, boolean active) throws DbException { ContactId c = db.addContact(txn, remote, local, null, verified); - keyManager.addContactWithRotationKeys(txn, c, rootKey, timestamp, - alice, active); + keyManager.addRotationKeys(txn, c, rootKey, timestamp, alice, active); Contact contact = db.getContact(txn, c); for (ContactHook hook : hooks) hook.addingContact(txn, contact); return c; @@ -89,12 +88,14 @@ class ContactManagerImpl implements ContactManager { public ContactId addContact(Transaction txn, PendingContactId p, Author remote, AuthorId local, SecretKey rootKey, long timestamp, boolean alice, boolean verified, boolean active) - throws DbException { - PublicKey handshake = db.getPendingContact(txn, p).getPublicKey(); + throws DbException, GeneralSecurityException { + PublicKey theirPublicKey = db.getPendingContact(txn, p).getPublicKey(); db.removePendingContact(txn, p); - ContactId c = db.addContact(txn, remote, local, handshake, verified); - keyManager.addContactWithRotationKeys(txn, c, rootKey, timestamp, - alice, active); + ContactId c = + db.addContact(txn, remote, local, theirPublicKey, verified); + KeyPair ourKeyPair = identityManager.getHandshakeKeys(txn); + keyManager.addContact(txn, c, theirPublicKey, ourKeyPair); + keyManager.addRotationKeys(txn, c, rootKey, timestamp, alice, active); Contact contact = db.getContact(txn, c); for (ContactHook hook : hooks) hook.addingContact(txn, contact); return c; @@ -126,18 +127,19 @@ class ContactManagerImpl implements ContactManager { @Override public PendingContact addPendingContact(String link, String alias) - throws DbException, FormatException { + throws DbException, FormatException, GeneralSecurityException { PendingContact p = pendingContactFactory.createPendingContact(link, alias); - db.transaction(false, txn -> { + Transaction txn = db.startTransaction(false); + try { db.addPendingContact(txn, p); KeyPair ourKeyPair = identityManager.getHandshakeKeys(txn); - try { - keyManager.addPendingContact(txn, p, ourKeyPair); - } catch (GeneralSecurityException e) { - throw new AssertionError(); - } - }); + keyManager.addPendingContact(txn, p.getId(), p.getPublicKey(), + ourKeyPair); + db.commitTransaction(txn); + } finally { + db.endTransaction(txn); + } return p; } diff --git a/bramble-core/src/main/java/org/briarproject/bramble/transport/KeyManagerImpl.java b/bramble-core/src/main/java/org/briarproject/bramble/transport/KeyManagerImpl.java index c3f656768..b57713680 100644 --- a/bramble-core/src/main/java/org/briarproject/bramble/transport/KeyManagerImpl.java +++ b/bramble-core/src/main/java/org/briarproject/bramble/transport/KeyManagerImpl.java @@ -1,11 +1,11 @@ package org.briarproject.bramble.transport; import org.briarproject.bramble.api.contact.ContactId; -import org.briarproject.bramble.api.contact.PendingContact; import org.briarproject.bramble.api.contact.PendingContactId; import org.briarproject.bramble.api.contact.event.ContactRemovedEvent; import org.briarproject.bramble.api.contact.event.PendingContactRemovedEvent; import org.briarproject.bramble.api.crypto.KeyPair; +import org.briarproject.bramble.api.crypto.PublicKey; import org.briarproject.bramble.api.crypto.SecretKey; import org.briarproject.bramble.api.crypto.TransportCrypto; import org.briarproject.bramble.api.db.DatabaseComponent; @@ -101,46 +101,51 @@ class KeyManagerImpl implements KeyManager, Service, EventListener { } @Override - public Map addContactWithRotationKeys( + public Map addRotationKeys( Transaction txn, ContactId c, SecretKey rootKey, long timestamp, boolean alice, boolean active) throws DbException { Map ids = new HashMap<>(); for (Entry e : managers.entrySet()) { TransportId t = e.getKey(); TransportKeyManager m = e.getValue(); - ids.put(t, m.addContactWithRotationKeys(txn, c, rootKey, timestamp, + ids.put(t, m.addRotationKeys(txn, c, rootKey, timestamp, alice, active)); } return ids; } @Override - public Map addContactWithHandshakeKeys( - Transaction txn, ContactId c, SecretKey rootKey, boolean alice) - throws DbException { + public Map addContact(Transaction txn, ContactId c, + PublicKey theirPublicKey, KeyPair ourKeyPair) + throws DbException, GeneralSecurityException { + SecretKey staticMasterKey = transportCrypto + .deriveStaticMasterKey(theirPublicKey, ourKeyPair); + SecretKey rootKey = + transportCrypto.deriveHandshakeRootKey(staticMasterKey, true); + boolean alice = transportCrypto.isAlice(theirPublicKey, ourKeyPair); Map ids = new HashMap<>(); for (Entry e : managers.entrySet()) { TransportId t = e.getKey(); TransportKeyManager m = e.getValue(); - ids.put(t, m.addContactWithHandshakeKeys(txn, c, rootKey, alice)); + ids.put(t, m.addHandshakeKeys(txn, c, rootKey, alice)); } return ids; } @Override public Map addPendingContact(Transaction txn, - PendingContact p, KeyPair ourKeyPair) + PendingContactId p, PublicKey theirPublicKey, KeyPair ourKeyPair) throws DbException, GeneralSecurityException { SecretKey staticMasterKey = transportCrypto - .deriveStaticMasterKey(p.getPublicKey(), ourKeyPair); + .deriveStaticMasterKey(theirPublicKey, ourKeyPair); SecretKey rootKey = transportCrypto.deriveHandshakeRootKey(staticMasterKey, true); - boolean alice = transportCrypto.isAlice(p.getPublicKey(), ourKeyPair); + boolean alice = transportCrypto.isAlice(theirPublicKey, ourKeyPair); Map ids = new HashMap<>(); for (Entry e : managers.entrySet()) { TransportId t = e.getKey(); TransportKeyManager m = e.getValue(); - ids.put(t, m.addPendingContact(txn, p.getId(), rootKey, alice)); + ids.put(t, m.addHandshakeKeys(txn, p, rootKey, alice)); } return ids; } diff --git a/bramble-core/src/main/java/org/briarproject/bramble/transport/TransportKeyManager.java b/bramble-core/src/main/java/org/briarproject/bramble/transport/TransportKeyManager.java index 45be00197..7d2144260 100644 --- a/bramble-core/src/main/java/org/briarproject/bramble/transport/TransportKeyManager.java +++ b/bramble-core/src/main/java/org/briarproject/bramble/transport/TransportKeyManager.java @@ -16,14 +16,14 @@ interface TransportKeyManager { void start(Transaction txn) throws DbException; - KeySetId addContactWithRotationKeys(Transaction txn, ContactId c, + KeySetId addRotationKeys(Transaction txn, ContactId c, SecretKey rootKey, long timestamp, boolean alice, boolean active) throws DbException; - KeySetId addContactWithHandshakeKeys(Transaction txn, ContactId c, + KeySetId addHandshakeKeys(Transaction txn, ContactId c, SecretKey rootKey, boolean alice) throws DbException; - KeySetId addPendingContact(Transaction txn, PendingContactId p, + KeySetId addHandshakeKeys(Transaction txn, PendingContactId p, SecretKey rootKey, boolean alice) throws DbException; void activateKeys(Transaction txn, KeySetId k) throws DbException; diff --git a/bramble-core/src/main/java/org/briarproject/bramble/transport/TransportKeyManagerImpl.java b/bramble-core/src/main/java/org/briarproject/bramble/transport/TransportKeyManagerImpl.java index 81255dbc3..7c7f81d07 100644 --- a/bramble-core/src/main/java/org/briarproject/bramble/transport/TransportKeyManagerImpl.java +++ b/bramble-core/src/main/java/org/briarproject/bramble/transport/TransportKeyManagerImpl.java @@ -115,11 +115,14 @@ class TransportKeyManagerImpl implements TransportKeyManager { TransportKeys k = ks.getKeys(); TransportKeys k1 = transportCrypto.updateTransportKeys(k, timePeriod); - TransportKeySet ks1 = new TransportKeySet(ks.getKeySetId(), - ks.getContactId(), null, k1); - if (k1.getTimePeriod() > k.getTimePeriod()) + if (k1.getTimePeriod() > k.getTimePeriod()) { + TransportKeySet ks1 = new TransportKeySet(ks.getKeySetId(), + ks.getContactId(), ks.getPendingContactId(), k1); updateResult.updated.add(ks1); - updateResult.current.add(ks1); + updateResult.current.add(ks1); + } else { + updateResult.current.add(ks); + } } return updateResult; } @@ -207,7 +210,7 @@ class TransportKeyManagerImpl implements TransportKeyManager { } @Override - public KeySetId addContactWithRotationKeys(Transaction txn, ContactId c, + public KeySetId addRotationKeys(Transaction txn, ContactId c, SecretKey rootKey, long timestamp, boolean alice, boolean active) throws DbException { lock.lock(); @@ -222,7 +225,7 @@ class TransportKeyManagerImpl implements TransportKeyManager { k = transportCrypto.updateTransportKeys(k, timePeriod); // Write the keys back to the DB KeySetId keySetId = db.addTransportKeys(txn, c, k); - // Initialise mutable state for the contact + // Initialise mutable state for the keys addKeys(keySetId, c, null, new MutableTransportKeys(k)); return keySetId; } finally { @@ -231,7 +234,7 @@ class TransportKeyManagerImpl implements TransportKeyManager { } @Override - public KeySetId addContactWithHandshakeKeys(Transaction txn, ContactId c, + public KeySetId addHandshakeKeys(Transaction txn, ContactId c, SecretKey rootKey, boolean alice) throws DbException { lock.lock(); try { @@ -242,7 +245,7 @@ class TransportKeyManagerImpl implements TransportKeyManager { rootKey, timePeriod, alice); // Write the keys back to the DB KeySetId keySetId = db.addTransportKeys(txn, c, k); - // Initialise mutable state for the contact + // Initialise mutable state for the keys addKeys(keySetId, c, null, new MutableTransportKeys(k)); return keySetId; } finally { @@ -251,7 +254,7 @@ class TransportKeyManagerImpl implements TransportKeyManager { } @Override - public KeySetId addPendingContact(Transaction txn, PendingContactId p, + public KeySetId addHandshakeKeys(Transaction txn, PendingContactId p, SecretKey rootKey, boolean alice) throws DbException { lock.lock(); try { @@ -262,7 +265,7 @@ class TransportKeyManagerImpl implements TransportKeyManager { rootKey, timePeriod, alice); // Write the keys back to the DB KeySetId keySetId = db.addTransportKeys(txn, p, k); - // Initialise mutable state for the pending contact + // Initialise mutable state for the keys addKeys(keySetId, null, p, new MutableTransportKeys(k)); return keySetId; } finally { diff --git a/bramble-core/src/test/java/org/briarproject/bramble/contact/ContactManagerImplTest.java b/bramble-core/src/test/java/org/briarproject/bramble/contact/ContactManagerImplTest.java index 059cb7aa9..195de4ef3 100644 --- a/bramble-core/src/test/java/org/briarproject/bramble/contact/ContactManagerImplTest.java +++ b/bramble-core/src/test/java/org/briarproject/bramble/contact/ContactManagerImplTest.java @@ -73,8 +73,8 @@ public class ContactManagerImplTest extends BrambleMockTestCase { oneOf(db).transactionWithResult(with(false), withDbCallable(txn)); oneOf(db).addContact(txn, remote, local, null, verified); will(returnValue(contactId)); - oneOf(keyManager).addContactWithRotationKeys(txn, contactId, - rootKey, timestamp, alice, active); + oneOf(keyManager).addRotationKeys(txn, contactId, rootKey, + timestamp, alice, active); oneOf(db).getContact(txn, contactId); will(returnValue(contact)); }}); diff --git a/bramble-core/src/test/java/org/briarproject/bramble/transport/KeyManagerImplTest.java b/bramble-core/src/test/java/org/briarproject/bramble/transport/KeyManagerImplTest.java index 95c03a3fd..142f06538 100644 --- a/bramble-core/src/test/java/org/briarproject/bramble/transport/KeyManagerImplTest.java +++ b/bramble-core/src/test/java/org/briarproject/bramble/transport/KeyManagerImplTest.java @@ -1,10 +1,10 @@ package org.briarproject.bramble.transport; import org.briarproject.bramble.api.contact.ContactId; -import org.briarproject.bramble.api.contact.PendingContact; import org.briarproject.bramble.api.contact.PendingContactId; import org.briarproject.bramble.api.contact.event.ContactRemovedEvent; import org.briarproject.bramble.api.crypto.KeyPair; +import org.briarproject.bramble.api.crypto.PublicKey; import org.briarproject.bramble.api.crypto.SecretKey; import org.briarproject.bramble.api.crypto.TransportCrypto; import org.briarproject.bramble.api.db.DatabaseComponent; @@ -31,8 +31,8 @@ import static org.briarproject.bramble.api.transport.TransportConstants.TAG_LENG import static org.briarproject.bramble.test.TestUtils.getAgreementPrivateKey; import static org.briarproject.bramble.test.TestUtils.getAgreementPublicKey; import static org.briarproject.bramble.test.TestUtils.getContactId; -import static org.briarproject.bramble.test.TestUtils.getPendingContact; import static org.briarproject.bramble.test.TestUtils.getRandomBytes; +import static org.briarproject.bramble.test.TestUtils.getRandomId; import static org.briarproject.bramble.test.TestUtils.getSecretKey; import static org.briarproject.bramble.test.TestUtils.getTransportId; import static org.junit.Assert.assertEquals; @@ -52,8 +52,8 @@ public class KeyManagerImplTest extends BrambleMockTestCase { private final DeterministicExecutor executor = new DeterministicExecutor(); private final Transaction txn = new Transaction(null, false); private final ContactId contactId = getContactId(); - private final PendingContact pendingContact = getPendingContact(); - private final PendingContactId pendingContactId = pendingContact.getId(); + private final PendingContactId pendingContactId = + new PendingContactId(getRandomId()); private final KeySetId keySetId = new KeySetId(345); private final TransportId transportId = getTransportId(); private final TransportId unknownTransportId = getTransportId(); @@ -64,6 +64,11 @@ public class KeyManagerImplTest extends BrambleMockTestCase { new StreamContext(null, pendingContactId, transportId, getSecretKey(), getSecretKey(), 1, true); private final byte[] tag = getRandomBytes(TAG_LENGTH); + private final PublicKey theirPublicKey = getAgreementPublicKey(); + private final KeyPair ourKeyPair = + new KeyPair(getAgreementPublicKey(), getAgreementPrivateKey()); + private final SecretKey staticMasterKey = getSecretKey(); + private final SecretKey rootKey = getSecretKey(); private final Random random = new Random(); private final KeyManagerImpl keyManager = new KeyManagerImpl(db, executor, @@ -105,57 +110,59 @@ public class KeyManagerImplTest extends BrambleMockTestCase { boolean active = random.nextBoolean(); context.checking(new Expectations() {{ - oneOf(transportKeyManager).addContactWithRotationKeys(txn, + oneOf(transportKeyManager).addRotationKeys(txn, contactId, secretKey, timestamp, alice, active); will(returnValue(keySetId)); }}); - Map ids = keyManager.addContactWithRotationKeys( + Map ids = keyManager.addRotationKeys( txn, contactId, secretKey, timestamp, alice, active); assertEquals(singletonMap(transportId, keySetId), ids); } @Test - public void testAddContactWithHandshakeModeKeys() throws Exception { - SecretKey secretKey = getSecretKey(); + public void testAddContactWithHandshakePublicKey() throws Exception { boolean alice = random.nextBoolean(); context.checking(new Expectations() {{ - oneOf(transportKeyManager).addContactWithHandshakeKeys( - txn, contactId, secretKey, alice); + oneOf(transportCrypto) + .deriveStaticMasterKey(theirPublicKey, ourKeyPair); + will(returnValue(staticMasterKey)); + oneOf(transportCrypto) + .deriveHandshakeRootKey(staticMasterKey, false); + will(returnValue(rootKey)); + oneOf(transportCrypto).isAlice(theirPublicKey, ourKeyPair); + will(returnValue(alice)); + oneOf(transportKeyManager).addHandshakeKeys(txn, contactId, + rootKey, alice); will(returnValue(keySetId)); }}); - Map ids = keyManager.addContactWithHandshakeKeys( - txn, contactId, secretKey, alice); + Map ids = keyManager.addContact(txn, contactId, + theirPublicKey, ourKeyPair); assertEquals(singletonMap(transportId, keySetId), ids); } @Test public void testAddPendingContact() throws Exception { - KeyPair ourKeyPair = - new KeyPair(getAgreementPublicKey(), getAgreementPrivateKey()); - SecretKey staticMasterKey = getSecretKey(); - SecretKey rootKey = getSecretKey(); boolean alice = random.nextBoolean(); context.checking(new Expectations() {{ - oneOf(transportCrypto).deriveStaticMasterKey( - pendingContact.getPublicKey(), ourKeyPair); + oneOf(transportCrypto) + .deriveStaticMasterKey(theirPublicKey, ourKeyPair); will(returnValue(staticMasterKey)); oneOf(transportCrypto) .deriveHandshakeRootKey(staticMasterKey, true); will(returnValue(rootKey)); - oneOf(transportCrypto).isAlice(pendingContact.getPublicKey(), - ourKeyPair); + oneOf(transportCrypto).isAlice(theirPublicKey, ourKeyPair); will(returnValue(alice)); - oneOf(transportKeyManager).addPendingContact(txn, pendingContactId, + oneOf(transportKeyManager).addHandshakeKeys(txn, pendingContactId, rootKey, alice); will(returnValue(keySetId)); }}); Map ids = keyManager.addPendingContact(txn, - pendingContact, ourKeyPair); + pendingContactId, theirPublicKey, ourKeyPair); assertEquals(singletonMap(transportId, keySetId), ids); } diff --git a/bramble-core/src/test/java/org/briarproject/bramble/transport/TransportKeyManagerImplTest.java b/bramble-core/src/test/java/org/briarproject/bramble/transport/TransportKeyManagerImplTest.java index 660476610..a86d11a2b 100644 --- a/bramble-core/src/test/java/org/briarproject/bramble/transport/TransportKeyManagerImplTest.java +++ b/bramble-core/src/test/java/org/briarproject/bramble/transport/TransportKeyManagerImplTest.java @@ -21,6 +21,7 @@ import org.hamcrest.Description; import org.jmock.Expectations; import org.jmock.api.Action; import org.jmock.api.Invocation; +import org.junit.Before; import org.junit.Test; import java.util.ArrayList; @@ -72,6 +73,14 @@ public class TransportKeyManagerImplTest extends BrambleMockTestCase { private final SecretKey rootKey = getSecretKey(); private final Random random = new Random(); + private TransportKeyManager transportKeyManager; + + @Before + public void setUp() { + transportKeyManager = new TransportKeyManagerImpl(db, transportCrypto, + dbExecutor, scheduler, clock, transportId, maxLatency); + } + @Test public void testKeysAreUpdatedAtStartup() throws Exception { boolean active = random.nextBoolean(); @@ -112,16 +121,13 @@ public class TransportKeyManagerImplTest extends BrambleMockTestCase { with(timePeriodLength - 1), with(MILLISECONDS)); }}); - TransportKeyManager transportKeyManager = new TransportKeyManagerImpl( - db, transportCrypto, dbExecutor, scheduler, clock, transportId, - maxLatency); transportKeyManager.start(txn); assertEquals(active, transportKeyManager.canSendOutgoingStreams(contactId)); } @Test - public void testRotationKeysAreDerivedAndUpdatedWhenAddingContact() + public void testRotationKeysForContactAreDerivedAndUpdatedWhenAdded() throws Exception { boolean alice = random.nextBoolean(); boolean active = random.nextBoolean(); @@ -156,14 +162,14 @@ public class TransportKeyManagerImplTest extends BrambleMockTestCase { maxLatency); // The timestamp is 1 ms before the start of time period 1000 long timestamp = timePeriodLength * 1000 - 1; - assertEquals(keySetId, transportKeyManager.addContactWithRotationKeys( - txn, contactId, rootKey, timestamp, alice, active)); + assertEquals(keySetId, transportKeyManager.addRotationKeys(txn, + contactId, rootKey, timestamp, alice, active)); assertEquals(active, transportKeyManager.canSendOutgoingStreams(contactId)); } @Test - public void testHandshakeKeysAreDerivedWhenAddingContact() + public void testHandshakeKeysForContactAreDerivedWhenAdded() throws Exception { boolean alice = random.nextBoolean(); TransportKeys transportKeys = createHandshakeKeys(1000, 0, alice); @@ -189,16 +195,13 @@ public class TransportKeyManagerImplTest extends BrambleMockTestCase { will(returnValue(keySetId)); }}); - TransportKeyManager transportKeyManager = new TransportKeyManagerImpl( - db, transportCrypto, dbExecutor, scheduler, clock, transportId, - maxLatency); - assertEquals(keySetId, transportKeyManager.addContactWithHandshakeKeys( - txn, contactId, rootKey, alice)); + assertEquals(keySetId, transportKeyManager.addHandshakeKeys(txn, + contactId, rootKey, alice)); assertTrue(transportKeyManager.canSendOutgoingStreams(contactId)); } @Test - public void testHandshakeKeysAreDerivedWhenAddingPendingContact() + public void testHandshakeKeysForPendingContactAreDerivedWhenAdded() throws Exception { boolean alice = random.nextBoolean(); TransportKeys transportKeys = createHandshakeKeys(1000, 0, alice); @@ -224,10 +227,7 @@ public class TransportKeyManagerImplTest extends BrambleMockTestCase { will(returnValue(keySetId)); }}); - TransportKeyManager transportKeyManager = new TransportKeyManagerImpl( - db, transportCrypto, dbExecutor, scheduler, clock, transportId, - maxLatency); - assertEquals(keySetId, transportKeyManager.addPendingContact(txn, + assertEquals(keySetId, transportKeyManager.addHandshakeKeys(txn, pendingContactId, rootKey, alice)); assertTrue(transportKeyManager.canSendOutgoingStreams( pendingContactId)); @@ -269,12 +269,9 @@ public class TransportKeyManagerImplTest extends BrambleMockTestCase { expectAddContactKeysNotUpdated(alice, true, transportKeys, txn); - TransportKeyManager transportKeyManager = new TransportKeyManagerImpl( - db, transportCrypto, dbExecutor, scheduler, clock, transportId, - maxLatency); // The timestamp is at the start of time period 1000 long timestamp = timePeriodLength * 1000; - assertEquals(keySetId, transportKeyManager.addContactWithRotationKeys( + assertEquals(keySetId, transportKeyManager.addRotationKeys( txn, contactId, rootKey, timestamp, alice, true)); assertFalse(transportKeyManager.canSendOutgoingStreams(contactId)); assertNull(transportKeyManager.getStreamContext(txn, contactId)); @@ -295,12 +292,9 @@ public class TransportKeyManagerImplTest extends BrambleMockTestCase { oneOf(db).incrementStreamCounter(txn, transportId, keySetId); }}); - TransportKeyManager transportKeyManager = new TransportKeyManagerImpl( - db, transportCrypto, dbExecutor, scheduler, clock, transportId, - maxLatency); // The timestamp is at the start of time period 1000 long timestamp = timePeriodLength * 1000; - assertEquals(keySetId, transportKeyManager.addContactWithRotationKeys( + assertEquals(keySetId, transportKeyManager.addRotationKeys( txn, contactId, rootKey, timestamp, alice, true)); // The first request should return a stream context assertTrue(transportKeyManager.canSendOutgoingStreams(contactId)); @@ -327,12 +321,9 @@ public class TransportKeyManagerImplTest extends BrambleMockTestCase { expectAddContactKeysNotUpdated(alice, active, transportKeys, txn); - TransportKeyManager transportKeyManager = new TransportKeyManagerImpl( - db, transportCrypto, dbExecutor, scheduler, clock, transportId, - maxLatency); // The timestamp is at the start of time period 1000 long timestamp = timePeriodLength * 1000; - assertEquals(keySetId, transportKeyManager.addContactWithRotationKeys( + assertEquals(keySetId, transportKeyManager.addRotationKeys( txn, contactId, rootKey, timestamp, alice, active)); assertEquals(active, transportKeyManager.canSendOutgoingStreams(contactId)); @@ -380,12 +371,9 @@ public class TransportKeyManagerImplTest extends BrambleMockTestCase { 1, new byte[REORDERING_WINDOW_SIZE / 8]); }}); - TransportKeyManager transportKeyManager = new TransportKeyManagerImpl( - db, transportCrypto, dbExecutor, scheduler, clock, transportId, - maxLatency); // The timestamp is at the start of time period 1000 long timestamp = timePeriodLength * 1000; - assertEquals(keySetId, transportKeyManager.addContactWithRotationKeys( + assertEquals(keySetId, transportKeyManager.addRotationKeys( txn, contactId, rootKey, timestamp, alice, true)); assertTrue(transportKeyManager.canSendOutgoingStreams(contactId)); // Use the first tag (previous time period, stream number 0) @@ -461,9 +449,6 @@ public class TransportKeyManagerImplTest extends BrambleMockTestCase { with(timePeriodLength), with(MILLISECONDS)); }}); - TransportKeyManager transportKeyManager = new TransportKeyManagerImpl( - db, transportCrypto, dbExecutor, scheduler, clock, transportId, - maxLatency); transportKeyManager.start(txn); assertTrue(transportKeyManager.canSendOutgoingStreams(contactId)); } @@ -483,12 +468,9 @@ public class TransportKeyManagerImplTest extends BrambleMockTestCase { oneOf(db).incrementStreamCounter(txn, transportId, keySetId); }}); - TransportKeyManager transportKeyManager = new TransportKeyManagerImpl( - db, transportCrypto, dbExecutor, scheduler, clock, transportId, - maxLatency); // The timestamp is at the start of time period 1000 long timestamp = timePeriodLength * 1000; - assertEquals(keySetId, transportKeyManager.addContactWithRotationKeys( + assertEquals(keySetId, transportKeyManager.addRotationKeys( txn, contactId, rootKey, timestamp, alice, false)); // The keys are inactive so no stream context should be returned assertFalse(transportKeyManager.canSendOutgoingStreams(contactId)); @@ -549,12 +531,9 @@ public class TransportKeyManagerImplTest extends BrambleMockTestCase { oneOf(db).incrementStreamCounter(txn, transportId, keySetId); }}); - TransportKeyManager transportKeyManager = new TransportKeyManagerImpl( - db, transportCrypto, dbExecutor, scheduler, clock, transportId, - maxLatency); // The timestamp is at the start of time period 1000 long timestamp = timePeriodLength * 1000; - assertEquals(keySetId, transportKeyManager.addContactWithRotationKeys( + assertEquals(keySetId, transportKeyManager.addRotationKeys( txn, contactId, rootKey, timestamp, alice, false)); // The keys are inactive so no stream context should be returned assertFalse(transportKeyManager.canSendOutgoingStreams(contactId)); diff --git a/briar-android/src/main/java/org/briarproject/briar/android/contact/add/remote/AddContactViewModel.java b/briar-android/src/main/java/org/briarproject/briar/android/contact/add/remote/AddContactViewModel.java index ae5fd9255..00a9da2a9 100644 --- a/briar-android/src/main/java/org/briarproject/briar/android/contact/add/remote/AddContactViewModel.java +++ b/briar-android/src/main/java/org/briarproject/briar/android/contact/add/remote/AddContactViewModel.java @@ -16,6 +16,7 @@ import org.briarproject.briar.android.viewmodel.LiveEvent; import org.briarproject.briar.android.viewmodel.LiveResult; import org.briarproject.briar.android.viewmodel.MutableLiveEvent; +import java.security.GeneralSecurityException; import java.util.concurrent.Executor; import java.util.logging.Logger; @@ -102,14 +103,15 @@ public class AddContactViewModel extends AndroidViewModel { } catch (UnsupportedVersionException e) { logException(LOG, WARNING, e); addContactResult.postValue(new LiveResult<>(e)); - } catch (DbException | FormatException e) { + } catch (DbException | FormatException + | GeneralSecurityException e) { logException(LOG, WARNING, e); addContactResult.postValue(new LiveResult<>(e)); } }); } - public LiveData> getAddContactResult() { + LiveData> getAddContactResult() { return addContactResult; } diff --git a/briar-core/src/main/java/org/briarproject/briar/introduction/IntroduceeProtocolEngine.java b/briar-core/src/main/java/org/briarproject/briar/introduction/IntroduceeProtocolEngine.java index a1b0964ad..33750f358 100644 --- a/briar-core/src/main/java/org/briarproject/briar/introduction/IntroduceeProtocolEngine.java +++ b/briar-core/src/main/java/org/briarproject/briar/introduction/IntroduceeProtocolEngine.java @@ -444,7 +444,7 @@ class IntroduceeProtocolEngine // add the keys to the new contact //noinspection ConstantConditions - keys = keyManager.addContactWithRotationKeys(txn, c.getId(), + keys = keyManager.addRotationKeys(txn, c.getId(), new SecretKey(s.getMasterKey()), timestamp, s.getLocal().alice, false); From b2d2b1765a551dd8eefa0150f323c2812bffde16 Mon Sep 17 00:00:00 2001 From: akwizgran Date: Thu, 30 May 2019 17:37:09 +0100 Subject: [PATCH 07/10] Fix pending contact flag. Hooray for unit tests. --- .../java/org/briarproject/bramble/transport/KeyManagerImpl.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bramble-core/src/main/java/org/briarproject/bramble/transport/KeyManagerImpl.java b/bramble-core/src/main/java/org/briarproject/bramble/transport/KeyManagerImpl.java index b57713680..58aeb7cea 100644 --- a/bramble-core/src/main/java/org/briarproject/bramble/transport/KeyManagerImpl.java +++ b/bramble-core/src/main/java/org/briarproject/bramble/transport/KeyManagerImpl.java @@ -121,7 +121,7 @@ class KeyManagerImpl implements KeyManager, Service, EventListener { SecretKey staticMasterKey = transportCrypto .deriveStaticMasterKey(theirPublicKey, ourKeyPair); SecretKey rootKey = - transportCrypto.deriveHandshakeRootKey(staticMasterKey, true); + transportCrypto.deriveHandshakeRootKey(staticMasterKey, false); boolean alice = transportCrypto.isAlice(theirPublicKey, ourKeyPair); Map ids = new HashMap<>(); for (Entry e : managers.entrySet()) { From 57a70f411b14311432ae8ea9628988eed703e06d Mon Sep 17 00:00:00 2001 From: akwizgran Date: Fri, 31 May 2019 11:59:00 +0100 Subject: [PATCH 08/10] Update ContactManager javadocs. --- .../bramble/api/contact/ContactManager.java | 58 +++++++++++++------ 1 file changed, 40 insertions(+), 18 deletions(-) diff --git a/bramble-api/src/main/java/org/briarproject/bramble/api/contact/ContactManager.java b/bramble-api/src/main/java/org/briarproject/bramble/api/contact/ContactManager.java index 1391f3674..245f28f59 100644 --- a/bramble-api/src/main/java/org/briarproject/bramble/api/contact/ContactManager.java +++ b/bramble-api/src/main/java/org/briarproject/bramble/api/contact/ContactManager.java @@ -30,10 +30,16 @@ public interface ContactManager { /** * Stores a contact associated with the given local and remote pseudonyms, - * derives and stores transport keys for each transport, and returns an ID - * for the contact. + * derives and stores rotation mode transport keys for each transport, and + * returns an ID for the contact. * + * @param rootKey The root key for a set of rotation mode transport keys + * @param timestamp The timestamp for deriving rotation mode transport + * keys from the root key * @param alice True if the local party is Alice + * @param verified True if the contact's identity has been verified + * @param active True if the rotation mode transport keys can be used for + * outgoing streams */ ContactId addContact(Transaction txn, Author remote, AuthorId local, SecretKey rootKey, long timestamp, boolean alice, boolean verified, @@ -41,10 +47,17 @@ public interface ContactManager { /** * Stores a contact associated with the given local and remote pseudonyms, - * replacing the given pending contact, derives and stores transport keys - * for each transport, and returns an ID for the contact. + * replacing the given pending contact, derives and stores handshake mode + * and rotation mode transport keys for each transport, and returns an ID + * for the contact. * + * @param rootKey The root key for a set of rotation mode transport keys + * @param timestamp The timestamp for deriving rotation mode transport + * keys from the root key * @param alice True if the local party is Alice + * @param verified True if the contact's identity has been verified + * @param active True if the rotation mode transport keys can be used for + * outgoing streams * @throws GeneralSecurityException If the pending contact's handshake * public key is invalid */ @@ -56,16 +69,24 @@ public interface ContactManager { /** * Stores a contact associated with the given local and remote pseudonyms * and returns an ID for the contact. + * + * @param verified True if the contact's identity has been verified */ ContactId addContact(Transaction txn, Author remote, AuthorId local, boolean verified) throws DbException; /** * Stores a contact associated with the given local and remote pseudonyms, - * derives and stores transport keys for each transport, and returns an ID - * for the contact. + * derives and stores rotation mode transport keys for each transport, and + * returns an ID for the contact. * + * @param rootKey The root key for a set of rotation mode transport keys + * @param timestamp The timestamp for deriving rotation mode transport + * keys from the root key * @param alice True if the local party is Alice + * @param verified True if the contact's identity has been verified + * @param active True if the rotation mode transport keys can be used for + * outgoing streams */ ContactId addContact(Author remote, AuthorId local, SecretKey rootKey, long timestamp, boolean alice, boolean verified, boolean active) @@ -81,9 +102,8 @@ public interface ContactManager { * Creates a {@link PendingContact} from the given handshake link and * alias, adds it to the database and returns it. * - * @param link The handshake link received from the contact we want to add - * @param alias The alias the user has given this contact - * @return A PendingContact representing the contact to be added + * @param link The handshake link received from the pending contact + * @param alias The alias the user has given this pending contact * @throws UnsupportedVersionException If the link uses a format version * that is not supported * @throws FormatException If the link is invalid @@ -122,8 +142,8 @@ public interface ContactManager { Contact getContact(Transaction txn, ContactId c) throws DbException; /** - * Returns the contact with the given remoteAuthorId - * that was added by the LocalAuthor with the given localAuthorId + * Returns the contact with the given {@code remoteAuthorId} + * that belongs to the local pseudonym with the given {@code localAuthorId}. * * @throws NoSuchContactException If the contact is not in the database */ @@ -131,8 +151,8 @@ public interface ContactManager { throws DbException; /** - * Returns the contact with the given remoteAuthorId - * that was added by the LocalAuthor with the given localAuthorId + * Returns the contact with the given {@code remoteAuthorId} + * that belongs to the local pseudonym with the given {@code localAuthorId}. * * @throws NoSuchContactException If the contact is not in the database */ @@ -140,7 +160,7 @@ public interface ContactManager { AuthorId localAuthorId) throws DbException; /** - * Returns all active contacts. + * Returns all contacts. */ Collection getContacts() throws DbException; @@ -155,25 +175,27 @@ public interface ContactManager { void removeContact(Transaction txn, ContactId c) throws DbException; /** - * Sets an alias for the contact or unsets it if alias is null. + * Sets an alias for the contact or unsets it if {@code alias} is null. */ void setContactAlias(Transaction txn, ContactId c, @Nullable String alias) throws DbException; /** - * Sets an alias for the contact or unsets it if alias is null. + * Sets an alias for the contact or unsets it if {@code alias} is null. */ void setContactAlias(ContactId c, @Nullable String alias) throws DbException; /** - * Return true if a contact with this name and public key already exists + * Returns true if a contact with this {@code remoteAuthorId} belongs to + * the local pseudonym with this {@code localAuthorId}. */ boolean contactExists(Transaction txn, AuthorId remoteAuthorId, AuthorId localAuthorId) throws DbException; /** - * Return true if a contact with this name and public key already exists + * Returns true if a contact with this {@code remoteAuthorId} belongs to + * the local pseudonym with this {@code localAuthorId}. */ boolean contactExists(AuthorId remoteAuthorId, AuthorId localAuthorId) throws DbException; From 1b8692a2160f4f7f5f66cb2111cfade866910a98 Mon Sep 17 00:00:00 2001 From: akwizgran Date: Mon, 3 Jun 2019 12:40:49 +0100 Subject: [PATCH 09/10] Add longer explanation of 'verified' flag. --- .../bramble/api/contact/ContactManager.java | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/bramble-api/src/main/java/org/briarproject/bramble/api/contact/ContactManager.java b/bramble-api/src/main/java/org/briarproject/bramble/api/contact/ContactManager.java index 245f28f59..40b6ab534 100644 --- a/bramble-api/src/main/java/org/briarproject/bramble/api/contact/ContactManager.java +++ b/bramble-api/src/main/java/org/briarproject/bramble/api/contact/ContactManager.java @@ -37,7 +37,9 @@ public interface ContactManager { * @param timestamp The timestamp for deriving rotation mode transport * keys from the root key * @param alice True if the local party is Alice - * @param verified True if the contact's identity has been verified + * @param verified True if the contact's identity has been verified, which + * is true if the contact was added in person or false if the contact was + * introduced or added remotely * @param active True if the rotation mode transport keys can be used for * outgoing streams */ @@ -55,7 +57,9 @@ public interface ContactManager { * @param timestamp The timestamp for deriving rotation mode transport * keys from the root key * @param alice True if the local party is Alice - * @param verified True if the contact's identity has been verified + * @param verified True if the contact's identity has been verified, which + * is true if the contact was added in person or false if the contact was + * introduced or added remotely * @param active True if the rotation mode transport keys can be used for * outgoing streams * @throws GeneralSecurityException If the pending contact's handshake @@ -70,7 +74,9 @@ public interface ContactManager { * Stores a contact associated with the given local and remote pseudonyms * and returns an ID for the contact. * - * @param verified True if the contact's identity has been verified + * @param verified True if the contact's identity has been verified, which + * is true if the contact was added in person or false if the contact was + * introduced or added remotely */ ContactId addContact(Transaction txn, Author remote, AuthorId local, boolean verified) throws DbException; @@ -84,7 +90,9 @@ public interface ContactManager { * @param timestamp The timestamp for deriving rotation mode transport * keys from the root key * @param alice True if the local party is Alice - * @param verified True if the contact's identity has been verified + * @param verified True if the contact's identity has been verified, which + * is true if the contact was added in person or false if the contact was + * introduced or added remotely * @param active True if the rotation mode transport keys can be used for * outgoing streams */ From fe83a59d2ae7a75e60553f0f749e81ae192c82bb Mon Sep 17 00:00:00 2001 From: akwizgran Date: Mon, 3 Jun 2019 12:48:14 +0100 Subject: [PATCH 10/10] Add comment about tag reuse. --- .../org/briarproject/bramble/api/crypto/TransportCrypto.java | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/bramble-api/src/main/java/org/briarproject/bramble/api/crypto/TransportCrypto.java b/bramble-api/src/main/java/org/briarproject/bramble/api/crypto/TransportCrypto.java index 7439c0ec8..b971b309b 100644 --- a/bramble-api/src/main/java/org/briarproject/bramble/api/crypto/TransportCrypto.java +++ b/bramble-api/src/main/java/org/briarproject/bramble/api/crypto/TransportCrypto.java @@ -24,7 +24,9 @@ public interface TransportCrypto { KeyPair ourHandshakeKeyPair) throws GeneralSecurityException; /** - * Derives the handshake mode root key from the static master key. + * Derives the handshake mode root key from the static master key. To + * prevent tag reuse, separate root keys are derived for contacts and + * pending contacts. * * @param pendingContact Whether the static master key is shared with a * pending contact or a contact