From f42fc5213e1ac83a714580df8fb06eba26fc28cd Mon Sep 17 00:00:00 2001 From: akwizgran Date: Thu, 9 May 2019 12:10:07 +0100 Subject: [PATCH 1/6] Add key manager method for contacts with handshake keys. --- .../bramble/api/transport/KeyManager.java | 21 +++++-- .../bramble/transport/KeyManagerImpl.java | 12 ++++ .../transport/TransportKeyManager.java | 3 + .../transport/TransportKeyManagerImpl.java | 20 +++++++ .../bramble/transport/KeyManagerImplTest.java | 18 +++++- .../TransportKeyManagerImplTest.java | 58 ++++++++++++++++--- 6 files changed, 118 insertions(+), 14 deletions(-) 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 827c94fb7..8a6e914b1 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 @@ -18,19 +18,32 @@ public interface KeyManager { /** * Informs the key manager that a new contact has been added. Derives and - * stores a set of transport keys for communicating with the contact over - * each transport and returns the key set IDs. + * stores a set of rotation mode transport keys for communicating with the + * contact over each transport and returns the key set IDs. *

* {@link StreamContext StreamContexts} for the contact can be created * after this method has returned. * - * @param alice true if the local party is Alice - * @param active whether the derived keys can be used for outgoing streams + * @param alice True if the local party is Alice + * @param active Whether the derived keys can be used for outgoing streams */ Map addContact(Transaction txn, ContactId c, SecretKey rootKey, long timestamp, boolean alice, boolean active) throws DbException; + /** + * Informs the key manager that a new contact has been added. Derives and + * stores a set of handshake mode transport keys for communicating with the + * contact over each transport and returns the key set IDs. + *

+ * {@link StreamContext StreamContexts} for the contact can be created + * after this method has returned. + * + * @param alice True if the local party is ALice + */ + Map addContact(Transaction txn, ContactId c, + SecretKey rootKey, boolean alice) throws DbException; + /** * Marks the given transport keys as usable for outgoing streams. */ 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 18d1b63ff..499e9dc22 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 @@ -100,6 +100,18 @@ class KeyManagerImpl implements KeyManager, Service, EventListener { return ids; } + @Override + public Map addContact(Transaction txn, + ContactId c, SecretKey rootKey, boolean alice) throws DbException { + Map ids = new HashMap<>(); + for (Entry e : managers.entrySet()) { + TransportId t = e.getKey(); + TransportKeyManager m = e.getValue(); + ids.put(t, m.addContact(txn, c, rootKey, alice)); + } + return ids; + } + @Override public void activateKeys(Transaction txn, Map keys) throws DbException { 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 fa3f54e11..d7758043f 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 @@ -18,6 +18,9 @@ interface TransportKeyManager { KeySetId addContact(Transaction txn, ContactId c, SecretKey rootKey, long timestamp, boolean alice, boolean active) throws DbException; + KeySetId addContact(Transaction txn, ContactId c, SecretKey rootKey, + boolean alice) throws DbException; + void activateKeys(Transaction txn, KeySetId k) throws DbException; void removeContact(ContactId c); 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 3e070e28f..86ce89e10 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 @@ -213,6 +213,26 @@ class TransportKeyManagerImpl implements TransportKeyManager { } } + @Override + public KeySetId addContact(Transaction txn, ContactId c, SecretKey rootKey, + boolean alice) throws DbException { + lock.lock(); + try { + // Work out what time period we're in + long timePeriod = clock.currentTimeMillis() / timePeriodLength; + // Derive the transport keys + TransportKeys k = transportCrypto.deriveHandshakeKeys(transportId, + rootKey, timePeriod, alice); + // Write the keys back to the DB + KeySetId keySetId = db.addTransportKeys(txn, c, k); + // Initialise mutable state for the contact + addKeys(keySetId, c, null, new MutableTransportKeys(k)); + return keySetId; + } finally { + lock.unlock(); + } + } + @Override public void activateKeys(Transaction txn, KeySetId k) throws DbException { lock.lock(); 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 e81264c3f..5f25ff777 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 @@ -83,7 +83,7 @@ public class KeyManagerImplTest extends BrambleMockTestCase { } @Test - public void testAddContact() throws Exception { + public void testAddContactWithRotationModeKeys() throws Exception { SecretKey secretKey = getSecretKey(); long timestamp = System.currentTimeMillis(); boolean alice = random.nextBoolean(); @@ -100,6 +100,22 @@ public class KeyManagerImplTest extends BrambleMockTestCase { assertEquals(singletonMap(transportId, keySetId), ids); } + @Test + public void testAddContactWithHandshakeModeKeys() throws Exception { + SecretKey secretKey = getSecretKey(); + boolean alice = random.nextBoolean(); + + context.checking(new Expectations() {{ + oneOf(transportKeyManager).addContact(txn, contactId, secretKey, + alice); + will(returnValue(keySetId)); + }}); + + Map ids = keyManager.addContact(txn, contactId, + secretKey, alice); + assertEquals(singletonMap(transportId, keySetId), ids); + } + @Test public void testGetStreamContextForUnknownTransport() throws Exception { assertNull(keyManager.getStreamContext(contactId, unknownTransportId)); 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 1aeabdc44..188be95f2 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 @@ -70,14 +70,15 @@ public class TransportKeyManagerImplTest extends BrambleMockTestCase { @Test public void testKeysAreUpdatedAtStartup() throws Exception { - TransportKeys shouldUpdate = createTransportKeys(900, 0, true); - TransportKeys shouldNotUpdate = createTransportKeys(1000, 0, true); + boolean active = random.nextBoolean(); + TransportKeys shouldUpdate = createTransportKeys(900, 0, active); + TransportKeys shouldNotUpdate = createTransportKeys(1000, 0, active); Collection loaded = asList( new TransportKeySet(keySetId, contactId, null, shouldUpdate), new TransportKeySet(keySetId1, contactId1, null, shouldNotUpdate) ); - TransportKeys updated = createTransportKeys(1000, 0, true); + TransportKeys updated = createTransportKeys(1000, 0, active); Transaction txn = new Transaction(null, false); context.checking(new Expectations() {{ @@ -111,19 +112,22 @@ public class TransportKeyManagerImplTest extends BrambleMockTestCase { db, transportCrypto, dbExecutor, scheduler, clock, transportId, maxLatency); transportKeyManager.start(txn); - assertTrue(transportKeyManager.canSendOutgoingStreams(contactId)); + assertEquals(active, + transportKeyManager.canSendOutgoingStreams(contactId)); } @Test - public void testKeysAreUpdatedWhenAddingContact() throws Exception { + public void testRotationKeysAreDerivedAndUpdatedWhenAddingContact() + throws Exception { boolean alice = random.nextBoolean(); - TransportKeys transportKeys = createTransportKeys(999, 0, true); - TransportKeys updated = createTransportKeys(1000, 0, true); + boolean active = random.nextBoolean(); + TransportKeys transportKeys = createTransportKeys(999, 0, active); + TransportKeys updated = createTransportKeys(1000, 0, active); Transaction txn = new Transaction(null, false); context.checking(new Expectations() {{ oneOf(transportCrypto).deriveRotationKeys(transportId, rootKey, - 999, alice, true); + 999, alice, active); will(returnValue(transportKeys)); // Get the current time (1 ms after start of time period 1000) oneOf(clock).currentTimeMillis(); @@ -149,7 +153,43 @@ public class TransportKeyManagerImplTest extends BrambleMockTestCase { // The timestamp is 1 ms before the start of time period 1000 long timestamp = timePeriodLength * 1000 - 1; assertEquals(keySetId, transportKeyManager.addContact(txn, contactId, - rootKey, timestamp, alice, true)); + rootKey, timestamp, alice, active)); + assertEquals(active, + transportKeyManager.canSendOutgoingStreams(contactId)); + } + + @Test + public void testHandshakeKeysAreDerivedWhenAddingContact() + throws Exception { + boolean alice = random.nextBoolean(); + TransportKeys transportKeys = createTransportKeys(1000, 0, true); + Transaction txn = new Transaction(null, false); + + context.checking(new Expectations() {{ + // Get the current time (1 ms after start of time period 1000) + oneOf(clock).currentTimeMillis(); + will(returnValue(timePeriodLength * 1000 + 1)); + // Derive the transport keys + oneOf(transportCrypto).deriveHandshakeKeys(transportId, rootKey, + 1000, alice); + will(returnValue(transportKeys)); + // Encode the tags (3 sets) + for (long i = 0; i < REORDERING_WINDOW_SIZE; i++) { + exactly(3).of(transportCrypto).encodeTag( + with(any(byte[].class)), with(tagKey), + with(PROTOCOL_VERSION), with(i)); + will(new EncodeTagAction()); + } + // Save the keys + oneOf(db).addTransportKeys(txn, contactId, transportKeys); + will(returnValue(keySetId)); + }}); + + TransportKeyManager transportKeyManager = new TransportKeyManagerImpl( + db, transportCrypto, dbExecutor, scheduler, clock, transportId, + maxLatency); + assertEquals(keySetId, transportKeyManager.addContact(txn, contactId, + rootKey, alice)); assertTrue(transportKeyManager.canSendOutgoingStreams(contactId)); } From dd50f4bcd4647f04d9bea7998d9201951e84ec13 Mon Sep 17 00:00:00 2001 From: akwizgran Date: Thu, 9 May 2019 13:07:15 +0100 Subject: [PATCH 2/6] Add key manager methods for pending contacts. --- .../org/briarproject/bramble/api/Bytes.java | 3 +- .../bramble/api/contact/ContactId.java | 3 +- .../bramble/api/contact/PendingContactId.java | 1 - .../bramble/api/transport/KeyManager.java | 35 +++++- .../bramble/transport/KeyManagerImpl.java | 32 +++++ .../transport/TransportKeyManager.java | 12 ++ .../transport/TransportKeyManagerImpl.java | 112 +++++++++++++++--- .../bramble/transport/KeyManagerImplTest.java | 59 +++++++-- .../TransportKeyManagerImplTest.java | 70 ++++++++++- 9 files changed, 294 insertions(+), 33 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 1a862bbd8..6da75fdff 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 @@ -6,6 +6,7 @@ import org.briarproject.bramble.util.StringUtils; import java.util.Arrays; import java.util.Comparator; +import javax.annotation.Nullable; import javax.annotation.concurrent.ThreadSafe; /** @@ -38,7 +39,7 @@ public class Bytes implements Comparable { } @Override - public boolean equals(Object o) { + public boolean equals(@Nullable Object o) { return o instanceof Bytes && Arrays.equals(bytes, ((Bytes) o).bytes); } diff --git a/bramble-api/src/main/java/org/briarproject/bramble/api/contact/ContactId.java b/bramble-api/src/main/java/org/briarproject/bramble/api/contact/ContactId.java index 21293599f..4a4f138e4 100644 --- a/bramble-api/src/main/java/org/briarproject/bramble/api/contact/ContactId.java +++ b/bramble-api/src/main/java/org/briarproject/bramble/api/contact/ContactId.java @@ -2,6 +2,7 @@ package org.briarproject.bramble.api.contact; import org.briarproject.bramble.api.nullsafety.NotNullByDefault; +import javax.annotation.Nullable; import javax.annotation.concurrent.Immutable; /** @@ -28,7 +29,7 @@ public class ContactId { } @Override - public boolean equals(Object o) { + public boolean equals(@Nullable Object o) { return o instanceof ContactId && id == ((ContactId) o).id; } } diff --git a/bramble-api/src/main/java/org/briarproject/bramble/api/contact/PendingContactId.java b/bramble-api/src/main/java/org/briarproject/bramble/api/contact/PendingContactId.java index 76526018e..a0a7c7f40 100644 --- a/bramble-api/src/main/java/org/briarproject/bramble/api/contact/PendingContactId.java +++ b/bramble-api/src/main/java/org/briarproject/bramble/api/contact/PendingContactId.java @@ -17,7 +17,6 @@ public class PendingContactId extends UniqueId { super(id); } - @Override public boolean equals(Object o) { return o instanceof PendingContactId && super.equals(o); 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 8a6e914b1..1651a135a 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,6 +1,7 @@ package org.briarproject.bramble.api.transport; import org.briarproject.bramble.api.contact.ContactId; +import org.briarproject.bramble.api.contact.PendingContactId; import org.briarproject.bramble.api.crypto.SecretKey; import org.briarproject.bramble.api.db.DbException; import org.briarproject.bramble.api.db.Transaction; @@ -39,11 +40,26 @@ 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 + * @param alice True if the local party is Alice */ Map addContact(Transaction txn, ContactId c, SecretKey rootKey, boolean alice) throws DbException; + /** + * Informs the key manager that a new pending contact has been added. + * Derives and stores a set of handshake mode transport keys for + * communicating with the pending contact over each transport and returns + * the key set IDs. + *

+ * {@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; + /** * Marks the given transport keys as usable for outgoing streams. */ @@ -56,15 +72,28 @@ public interface KeyManager { */ boolean canSendOutgoingStreams(ContactId c, TransportId t); + /** + * Returns true if we have keys that can be used for outgoing streams to + * the given pending contact over the given transport. + */ + boolean canSendOutgoingStreams(PendingContactId p, TransportId t); + /** * Returns a {@link StreamContext} for sending a stream to the given - * contact over the given transport, or null if an error occurs or the - * contact does not support the transport. + * contact over the given transport, or null if an error occurs. */ @Nullable StreamContext getStreamContext(ContactId c, TransportId t) throws DbException; + /** + * Returns a {@link StreamContext} for sending a stream to the given + * pending contact over the given transport, or null if an error occurs. + */ + @Nullable + StreamContext getStreamContext(PendingContactId p, TransportId t) + throws DbException; + /** * Looks up the given tag and returns a {@link StreamContext} for reading * from the corresponding stream, or null if an error occurs or the tag was 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 499e9dc22..036b9d2d9 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,6 +1,7 @@ package org.briarproject.bramble.transport; import org.briarproject.bramble.api.contact.ContactId; +import org.briarproject.bramble.api.contact.PendingContactId; import org.briarproject.bramble.api.contact.event.ContactRemovedEvent; import org.briarproject.bramble.api.crypto.SecretKey; import org.briarproject.bramble.api.db.DatabaseComponent; @@ -112,6 +113,19 @@ class KeyManagerImpl implements KeyManager, Service, EventListener { return ids; } + @Override + public Map addPendingContact(Transaction txn, + PendingContactId p, SecretKey rootKey, boolean alice) + throws DbException { + 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)); + } + return ids; + } + @Override public void activateKeys(Transaction txn, Map keys) throws DbException { @@ -132,6 +146,12 @@ class KeyManagerImpl implements KeyManager, Service, EventListener { return m != null && m.canSendOutgoingStreams(c); } + @Override + public boolean canSendOutgoingStreams(PendingContactId p, TransportId t) { + TransportKeyManager m = managers.get(t); + return m != null && m.canSendOutgoingStreams(p); + } + @Override public StreamContext getStreamContext(ContactId c, TransportId t) throws DbException { @@ -144,6 +164,18 @@ class KeyManagerImpl implements KeyManager, Service, EventListener { m.getStreamContext(txn, c)); } + @Override + public StreamContext getStreamContext(PendingContactId p, TransportId t) + throws DbException { + TransportKeyManager m = managers.get(t); + if (m == null) { + if (LOG.isLoggable(INFO)) LOG.info("No key manager for " + t); + return null; + } + return db.transactionWithNullableResult(false, txn -> + m.getStreamContext(txn, p)); + } + @Override public StreamContext getStreamContext(TransportId t, byte[] tag) throws DbException { 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 d7758043f..bed597398 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 @@ -1,6 +1,7 @@ package org.briarproject.bramble.transport; import org.briarproject.bramble.api.contact.ContactId; +import org.briarproject.bramble.api.contact.PendingContactId; import org.briarproject.bramble.api.crypto.SecretKey; import org.briarproject.bramble.api.db.DbException; import org.briarproject.bramble.api.db.Transaction; @@ -21,16 +22,27 @@ interface TransportKeyManager { KeySetId addContact(Transaction txn, ContactId c, SecretKey rootKey, boolean alice) throws DbException; + KeySetId addPendingContact(Transaction txn, PendingContactId p, + SecretKey rootKey, boolean alice) throws DbException; + void activateKeys(Transaction txn, KeySetId k) throws DbException; void removeContact(ContactId c); + void removePendingContact(PendingContactId p); + boolean canSendOutgoingStreams(ContactId c); + boolean canSendOutgoingStreams(PendingContactId p); + @Nullable StreamContext getStreamContext(Transaction txn, ContactId c) throws DbException; + @Nullable + StreamContext getStreamContext(Transaction txn, PendingContactId p) + throws DbException; + @Nullable StreamContext getStreamContext(Transaction txn, byte[] tag) 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 86ce89e10..7488d168c 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 @@ -64,8 +64,11 @@ class TransportKeyManagerImpl implements TransportKeyManager { @GuardedBy("lock") private final Map inContexts = new HashMap<>(); @GuardedBy("lock") - private final Map outContexts = - new HashMap<>(); + private final Map + contactOutContexts = new HashMap<>(); + @GuardedBy("lock") + private final Map + pendingContactOutContexts = new HashMap<>(); TransportKeyManagerImpl(DatabaseComponent db, TransportCrypto transportCrypto, Executor dbExecutor, @@ -162,19 +165,29 @@ class TransportKeyManagerImpl implements TransportKeyManager { @GuardedBy("lock") private void considerReplacingOutgoingKeys(MutableTransportKeySet ks) { - // Use the active outgoing keys with the highest key set ID - ContactId c = ks.getContactId(); - if (c != null && ks.getKeys().getCurrentOutgoingKeys().isActive()) { - MutableTransportKeySet old = outContexts.get(c); - if (old == null || - (old.getKeys().isHandshakeMode() && - !ks.getKeys().isHandshakeMode()) || + // Use the active outgoing keys with the highest key set ID, preferring + // rotation keys to handshake keys + if (ks.getKeys().getCurrentOutgoingKeys().isActive()) { + MutableTransportKeySet old = getOutgoingKeySet(ks.getContactId(), + ks.getPendingContactId()); + if (old == null || (old.getKeys().isHandshakeMode() && + !ks.getKeys().isHandshakeMode()) || old.getKeySetId().getInt() < ks.getKeySetId().getInt()) { - outContexts.put(c, ks); + if (ks.getContactId() == null) + pendingContactOutContexts.put(ks.getPendingContactId(), ks); + else contactOutContexts.put(ks.getContactId(), ks); } } } + @GuardedBy("lock") + @Nullable + private MutableTransportKeySet getOutgoingKeySet(@Nullable ContactId c, + @Nullable PendingContactId p) { + if (c == null) return pendingContactOutContexts.get(p); + else return contactOutContexts.get(c); + } + private void scheduleKeyUpdate(long now) { long delay = timePeriodLength - now % timePeriodLength; scheduler.schedule((Runnable) this::updateKeys, delay, MILLISECONDS); @@ -233,6 +246,26 @@ class TransportKeyManagerImpl implements TransportKeyManager { } } + @Override + public KeySetId addPendingContact(Transaction txn, PendingContactId p, + SecretKey rootKey, boolean alice) throws DbException { + lock.lock(); + try { + // Work out what time period we're in + long timePeriod = clock.currentTimeMillis() / timePeriodLength; + // Derive the transport keys + TransportKeys k = transportCrypto.deriveHandshakeKeys(transportId, + rootKey, timePeriod, alice); + // Write the keys back to the DB + KeySetId keySetId = db.addTransportKeys(txn, p, k); + // Initialise mutable state for the pending contact + addKeys(keySetId, null, p, new MutableTransportKeys(k)); + return keySetId; + } finally { + lock.unlock(); + } + } + @Override public void activateKeys(Transaction txn, KeySetId k) throws DbException { lock.lock(); @@ -254,8 +287,9 @@ class TransportKeyManagerImpl implements TransportKeyManager { try { // Remove mutable state for the contact Iterator it = inContexts.values().iterator(); - while (it.hasNext()) if (c.equals(it.next().contactId)) it.remove(); - outContexts.remove(c); + while (it.hasNext()) + if (c.equals(it.next().contactId)) it.remove(); + contactOutContexts.remove(c); Iterator it1 = keys.values().iterator(); while (it1.hasNext()) if (c.equals(it1.next().getContactId())) it1.remove(); @@ -265,13 +299,39 @@ class TransportKeyManagerImpl implements TransportKeyManager { } @Override - public boolean canSendOutgoingStreams(ContactId c) { + public void removePendingContact(PendingContactId p) { lock.lock(); try { - MutableTransportKeySet ks = outContexts.get(c); + // Remove mutable state for the pending contact + Iterator it = inContexts.values().iterator(); + while (it.hasNext()) + if (p.equals(it.next().pendingContactId)) it.remove(); + pendingContactOutContexts.remove(p); + Iterator it1 = keys.values().iterator(); + while (it1.hasNext()) + if (p.equals(it1.next().getPendingContactId())) it1.remove(); + } finally { + lock.unlock(); + } + } + + @Override + public boolean canSendOutgoingStreams(ContactId c) { + return canSendOutgoingStreams(c, null); + } + + @Override + public boolean canSendOutgoingStreams(PendingContactId p) { + return canSendOutgoingStreams(null, p); + } + + private boolean canSendOutgoingStreams(@Nullable ContactId c, + @Nullable PendingContactId p) { + lock.lock(); + try { + MutableTransportKeySet ks = getOutgoingKeySet(c, p); if (ks == null) return false; - MutableOutgoingKeys outKeys = - ks.getKeys().getCurrentOutgoingKeys(); + MutableOutgoingKeys outKeys = ks.getKeys().getCurrentOutgoingKeys(); if (!outKeys.isActive()) throw new AssertionError(); return outKeys.getStreamCounter() <= MAX_32_BIT_UNSIGNED; } finally { @@ -282,17 +342,30 @@ class TransportKeyManagerImpl implements TransportKeyManager { @Override public StreamContext getStreamContext(Transaction txn, ContactId c) throws DbException { + return getStreamContext(txn, c, null); + } + + @Override + public StreamContext getStreamContext(Transaction txn, PendingContactId p) + throws DbException { + return getStreamContext(txn, null, p); + } + + @Nullable + private StreamContext getStreamContext(Transaction txn, + @Nullable ContactId c, @Nullable PendingContactId p) + throws DbException { lock.lock(); try { // Look up the outgoing keys for the contact - MutableTransportKeySet ks = outContexts.get(c); + MutableTransportKeySet ks = getOutgoingKeySet(c, p); if (ks == null) return null; MutableTransportKeys keys = ks.getKeys(); MutableOutgoingKeys outKeys = keys.getCurrentOutgoingKeys(); if (!outKeys.isActive()) throw new AssertionError(); if (outKeys.getStreamCounter() > MAX_32_BIT_UNSIGNED) return null; // Create a stream context - StreamContext ctx = new StreamContext(c, null, transportId, + StreamContext ctx = new StreamContext(c, p, transportId, outKeys.getTagKey(), outKeys.getHeaderKey(), outKeys.getStreamCounter(), keys.isHandshakeMode()); // Increment the stream counter and write it back to the DB @@ -373,7 +446,8 @@ class TransportKeyManagerImpl implements TransportKeyManager { UpdateResult updateResult = updateKeys(snapshot, now); // Rebuild the mutable state for all contacts inContexts.clear(); - outContexts.clear(); + contactOutContexts.clear(); + pendingContactOutContexts.clear(); keys.clear(); addKeys(updateResult.current); // Write any updated keys back to the DB 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 5f25ff777..7d212aff5 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,6 +1,7 @@ package org.briarproject.bramble.transport; import org.briarproject.bramble.api.contact.ContactId; +import org.briarproject.bramble.api.contact.PendingContactId; import org.briarproject.bramble.api.contact.event.ContactRemovedEvent; import org.briarproject.bramble.api.crypto.SecretKey; import org.briarproject.bramble.api.db.DatabaseComponent; @@ -26,6 +27,7 @@ import static java.util.Collections.singletonMap; import static org.briarproject.bramble.api.transport.TransportConstants.TAG_LENGTH; import static org.briarproject.bramble.test.TestUtils.getContactId; 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; @@ -43,11 +45,17 @@ 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 PendingContactId pendingContactId = + new PendingContactId(getRandomId()); private final KeySetId keySetId = new KeySetId(345); private final TransportId transportId = getTransportId(); private final TransportId unknownTransportId = getTransportId(); - private final StreamContext streamContext = new StreamContext(contactId, - null, transportId, getSecretKey(), getSecretKey(), 1, false); + private final StreamContext contactStreamContext = + new StreamContext(contactId, null, transportId, getSecretKey(), + getSecretKey(), 1, false); + private final StreamContext pendingContactStreamContext = + new StreamContext(null, pendingContactId, transportId, + getSecretKey(), getSecretKey(), 1, true); private final byte[] tag = getRandomBytes(TAG_LENGTH); private final Random random = new Random(); @@ -117,23 +125,60 @@ public class KeyManagerImplTest extends BrambleMockTestCase { } @Test - public void testGetStreamContextForUnknownTransport() throws Exception { + public void testAddPendingContact() throws Exception { + SecretKey secretKey = getSecretKey(); + boolean alice = random.nextBoolean(); + + context.checking(new Expectations() {{ + oneOf(transportKeyManager).addPendingContact(txn, pendingContactId, + secretKey, alice); + will(returnValue(keySetId)); + }}); + + Map ids = keyManager.addPendingContact(txn, + pendingContactId, secretKey, alice); + assertEquals(singletonMap(transportId, keySetId), ids); + } + + @Test + public void testGetStreamContextForContactWithUnknownTransport() + throws Exception { assertNull(keyManager.getStreamContext(contactId, unknownTransportId)); } + @Test + public void testGetStreamContextForPendingContactWithUnknownTransport() + throws Exception { + assertNull(keyManager.getStreamContext(pendingContactId, + unknownTransportId)); + } + @Test public void testGetStreamContextForContact() throws Exception { context.checking(new DbExpectations() {{ oneOf(db).transactionWithNullableResult(with(false), withNullableDbCallable(txn)); oneOf(transportKeyManager).getStreamContext(txn, contactId); - will(returnValue(streamContext)); + will(returnValue(contactStreamContext)); }}); - assertEquals(streamContext, + assertEquals(contactStreamContext, keyManager.getStreamContext(contactId, transportId)); } + @Test + public void testGetStreamContextForPendingContact() throws Exception { + context.checking(new DbExpectations() {{ + oneOf(db).transactionWithNullableResult(with(false), + withNullableDbCallable(txn)); + oneOf(transportKeyManager).getStreamContext(txn, pendingContactId); + will(returnValue(pendingContactStreamContext)); + }}); + + assertEquals(pendingContactStreamContext, + keyManager.getStreamContext(pendingContactId, transportId)); + } + @Test public void testGetStreamContextForTagAndUnknownTransport() throws Exception { @@ -146,10 +191,10 @@ public class KeyManagerImplTest extends BrambleMockTestCase { oneOf(db).transactionWithNullableResult(with(false), withNullableDbCallable(txn)); oneOf(transportKeyManager).getStreamContext(txn, tag); - will(returnValue(streamContext)); + will(returnValue(contactStreamContext)); }}); - assertEquals(streamContext, + assertEquals(contactStreamContext, keyManager.getStreamContext(transportId, tag)); } 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 188be95f2..57abdab52 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 @@ -1,6 +1,7 @@ package org.briarproject.bramble.transport; import org.briarproject.bramble.api.contact.ContactId; +import org.briarproject.bramble.api.contact.PendingContactId; import org.briarproject.bramble.api.crypto.SecretKey; import org.briarproject.bramble.api.crypto.TransportCrypto; import org.briarproject.bramble.api.db.DatabaseComponent; @@ -37,6 +38,7 @@ import static org.briarproject.bramble.api.transport.TransportConstants.PROTOCOL import static org.briarproject.bramble.api.transport.TransportConstants.REORDERING_WINDOW_SIZE; import static org.briarproject.bramble.api.transport.TransportConstants.TAG_LENGTH; import static org.briarproject.bramble.test.TestUtils.getContactId; +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.briarproject.bramble.util.ByteUtils.MAX_32_BIT_UNSIGNED; @@ -61,6 +63,8 @@ public class TransportKeyManagerImplTest extends BrambleMockTestCase { private final long timePeriodLength = maxLatency + MAX_CLOCK_DIFFERENCE; private final ContactId contactId = getContactId(); private final ContactId contactId1 = getContactId(); + private final PendingContactId pendingContactId = + new PendingContactId(getRandomId()); private final KeySetId keySetId = new KeySetId(345); private final KeySetId keySetId1 = new KeySetId(456); private final SecretKey tagKey = getSecretKey(); @@ -162,7 +166,7 @@ public class TransportKeyManagerImplTest extends BrambleMockTestCase { public void testHandshakeKeysAreDerivedWhenAddingContact() throws Exception { boolean alice = random.nextBoolean(); - TransportKeys transportKeys = createTransportKeys(1000, 0, true); + TransportKeys transportKeys = createHandshakeKeys(1000, 0, alice); Transaction txn = new Transaction(null, false); context.checking(new Expectations() {{ @@ -193,6 +197,42 @@ public class TransportKeyManagerImplTest extends BrambleMockTestCase { assertTrue(transportKeyManager.canSendOutgoingStreams(contactId)); } + @Test + public void testHandshakeKeysAreDerivedWhenAddingPendingContact() + throws Exception { + boolean alice = random.nextBoolean(); + TransportKeys transportKeys = createHandshakeKeys(1000, 0, alice); + Transaction txn = new Transaction(null, false); + + context.checking(new Expectations() {{ + // Get the current time (1 ms after start of time period 1000) + oneOf(clock).currentTimeMillis(); + will(returnValue(timePeriodLength * 1000 + 1)); + // Derive the transport keys + oneOf(transportCrypto).deriveHandshakeKeys(transportId, rootKey, + 1000, alice); + will(returnValue(transportKeys)); + // Encode the tags (3 sets) + for (long i = 0; i < REORDERING_WINDOW_SIZE; i++) { + exactly(3).of(transportCrypto).encodeTag( + with(any(byte[].class)), with(tagKey), + with(PROTOCOL_VERSION), with(i)); + will(new EncodeTagAction()); + } + // Save the keys + oneOf(db).addTransportKeys(txn, pendingContactId, transportKeys); + will(returnValue(keySetId)); + }}); + + TransportKeyManager transportKeyManager = new TransportKeyManagerImpl( + db, transportCrypto, dbExecutor, scheduler, clock, transportId, + maxLatency); + assertEquals(keySetId, transportKeyManager.addPendingContact(txn, + pendingContactId, rootKey, alice)); + assertTrue(transportKeyManager.canSendOutgoingStreams( + pendingContactId)); + } + @Test public void testOutgoingStreamContextIsNullIfContactIsNotFound() throws Exception { @@ -205,6 +245,19 @@ public class TransportKeyManagerImplTest extends BrambleMockTestCase { assertFalse(transportKeyManager.canSendOutgoingStreams(contactId)); } + @Test + public void testOutgoingStreamContextIsNullIfPendingContactIsNotFound() + throws Exception { + Transaction txn = new Transaction(null, false); + + TransportKeyManager transportKeyManager = new TransportKeyManagerImpl( + db, transportCrypto, dbExecutor, scheduler, clock, transportId, + maxLatency); + assertNull(transportKeyManager.getStreamContext(txn, pendingContactId)); + assertFalse(transportKeyManager.canSendOutgoingStreams( + pendingContactId)); + } + @Test public void testOutgoingStreamContextIsNullIfStreamCounterIsExhausted() throws Exception { @@ -565,6 +618,21 @@ public class TransportKeyManagerImplTest extends BrambleMockTestCase { return new TransportKeys(transportId, inPrev, inCurr, inNext, outCurr); } + @SuppressWarnings("SameParameterValue") + private TransportKeys createHandshakeKeys(long timePeriod, + long streamCounter, boolean alice) { + IncomingKeys inPrev = new IncomingKeys(tagKey, headerKey, + timePeriod - 1); + IncomingKeys inCurr = new IncomingKeys(tagKey, headerKey, + timePeriod); + IncomingKeys inNext = new IncomingKeys(tagKey, headerKey, + timePeriod + 1); + OutgoingKeys outCurr = new OutgoingKeys(tagKey, headerKey, + timePeriod, streamCounter, true); + return new TransportKeys(transportId, inPrev, inCurr, inNext, outCurr, + rootKey, alice); + } + private class EncodeTagAction implements Action { private final Collection tags; From afa0b96293a591232d3803f0f48e0bca67f5fb07 Mon Sep 17 00:00:00 2001 From: akwizgran Date: Thu, 9 May 2019 13:17:23 +0100 Subject: [PATCH 3/6] Add utility method for null checks. --- .../bramble/api/nullsafety/NullSafety.java | 12 +++++++++++- .../bramble/api/transport/StreamContext.java | 5 +++-- .../bramble/api/transport/TransportKeySet.java | 5 +++-- .../bramble/transport/MutableTransportKeySet.java | 5 +++-- .../bramble/transport/TransportKeyManagerImpl.java | 4 ++++ 5 files changed, 24 insertions(+), 7 deletions(-) diff --git a/bramble-api/src/main/java/org/briarproject/bramble/api/nullsafety/NullSafety.java b/bramble-api/src/main/java/org/briarproject/bramble/api/nullsafety/NullSafety.java index 3aed01e5a..8e98561ee 100644 --- a/bramble-api/src/main/java/org/briarproject/bramble/api/nullsafety/NullSafety.java +++ b/bramble-api/src/main/java/org/briarproject/bramble/api/nullsafety/NullSafety.java @@ -6,10 +6,20 @@ import javax.annotation.Nullable; public class NullSafety { /** - * Stand-in for `Objects.requireNonNull()`. + * Stand-in for {@code Objects.requireNonNull()}. */ public static T requireNonNull(@Nullable T t) { if (t == null) throw new NullPointerException(); return t; } + + /** + * Checks that exactly one of the arguments is null. + * + * @throws AssertionError If both or neither of the arguments are null + */ + public static void requireExactlyOneNull(@Nullable Object a, + @Nullable Object b) { + if ((a == null) == (b == null)) throw new AssertionError(); + } } diff --git a/bramble-api/src/main/java/org/briarproject/bramble/api/transport/StreamContext.java b/bramble-api/src/main/java/org/briarproject/bramble/api/transport/StreamContext.java index 0290d1eb8..c27abc891 100644 --- a/bramble-api/src/main/java/org/briarproject/bramble/api/transport/StreamContext.java +++ b/bramble-api/src/main/java/org/briarproject/bramble/api/transport/StreamContext.java @@ -9,6 +9,8 @@ import org.briarproject.bramble.api.plugin.TransportId; import javax.annotation.Nullable; import javax.annotation.concurrent.Immutable; +import static org.briarproject.bramble.api.nullsafety.NullSafety.requireExactlyOneNull; + @Immutable @NotNullByDefault public class StreamContext { @@ -26,8 +28,7 @@ public class StreamContext { @Nullable PendingContactId pendingContactId, TransportId transportId, SecretKey tagKey, SecretKey headerKey, long streamNumber, boolean handshakeMode) { - if ((contactId == null) == (pendingContactId == null)) - throw new IllegalArgumentException(); + requireExactlyOneNull(contactId, pendingContactId); this.contactId = contactId; this.pendingContactId = pendingContactId; this.transportId = transportId; diff --git a/bramble-api/src/main/java/org/briarproject/bramble/api/transport/TransportKeySet.java b/bramble-api/src/main/java/org/briarproject/bramble/api/transport/TransportKeySet.java index b5a22775f..7a3b1dc12 100644 --- a/bramble-api/src/main/java/org/briarproject/bramble/api/transport/TransportKeySet.java +++ b/bramble-api/src/main/java/org/briarproject/bramble/api/transport/TransportKeySet.java @@ -7,6 +7,8 @@ import org.briarproject.bramble.api.nullsafety.NotNullByDefault; import javax.annotation.Nullable; import javax.annotation.concurrent.Immutable; +import static org.briarproject.bramble.api.nullsafety.NullSafety.requireExactlyOneNull; + /** * A set of keys for communicating with a given contact or pending contact * over a given transport. @@ -24,8 +26,7 @@ public class TransportKeySet { public TransportKeySet(KeySetId keySetId, @Nullable ContactId contactId, @Nullable PendingContactId pendingContactId, TransportKeys keys) { - if ((contactId == null) == (pendingContactId == null)) - throw new IllegalArgumentException(); + requireExactlyOneNull(contactId, pendingContactId); this.keySetId = keySetId; this.contactId = contactId; this.pendingContactId = pendingContactId; diff --git a/bramble-core/src/main/java/org/briarproject/bramble/transport/MutableTransportKeySet.java b/bramble-core/src/main/java/org/briarproject/bramble/transport/MutableTransportKeySet.java index 89ef17726..55480107b 100644 --- a/bramble-core/src/main/java/org/briarproject/bramble/transport/MutableTransportKeySet.java +++ b/bramble-core/src/main/java/org/briarproject/bramble/transport/MutableTransportKeySet.java @@ -8,6 +8,8 @@ import org.briarproject.bramble.api.transport.KeySetId; import javax.annotation.Nullable; import javax.annotation.concurrent.NotThreadSafe; +import static org.briarproject.bramble.api.nullsafety.NullSafety.requireExactlyOneNull; + @NotThreadSafe @NotNullByDefault class MutableTransportKeySet { @@ -22,8 +24,7 @@ class MutableTransportKeySet { MutableTransportKeySet(KeySetId keySetId, @Nullable ContactId contactId, @Nullable PendingContactId pendingContactId, MutableTransportKeys keys) { - if ((contactId == null) == (pendingContactId == null)) - throw new IllegalArgumentException(); + requireExactlyOneNull(contactId, pendingContactId); this.keySetId = keySetId; this.contactId = contactId; this.pendingContactId = pendingContactId; 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 7488d168c..ba31f851c 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 @@ -36,6 +36,7 @@ import javax.annotation.concurrent.ThreadSafe; import static java.util.concurrent.TimeUnit.MILLISECONDS; import static java.util.logging.Level.WARNING; import static java.util.logging.Logger.getLogger; +import static org.briarproject.bramble.api.nullsafety.NullSafety.requireExactlyOneNull; import static org.briarproject.bramble.api.transport.TransportConstants.MAX_CLOCK_DIFFERENCE; import static org.briarproject.bramble.api.transport.TransportConstants.PROTOCOL_VERSION; import static org.briarproject.bramble.api.transport.TransportConstants.TAG_LENGTH; @@ -136,6 +137,7 @@ class TransportKeyManagerImpl implements TransportKeyManager { private void addKeys(KeySetId keySetId, @Nullable ContactId contactId, @Nullable PendingContactId pendingContactId, MutableTransportKeys keys) { + requireExactlyOneNull(contactId, pendingContactId); MutableTransportKeySet ks = new MutableTransportKeySet(keySetId, contactId, pendingContactId, keys); this.keys.put(keySetId, ks); @@ -184,6 +186,7 @@ class TransportKeyManagerImpl implements TransportKeyManager { @Nullable private MutableTransportKeySet getOutgoingKeySet(@Nullable ContactId c, @Nullable PendingContactId p) { + requireExactlyOneNull(c, p); if (c == null) return pendingContactOutContexts.get(p); else return contactOutContexts.get(c); } @@ -475,6 +478,7 @@ class TransportKeyManagerImpl implements TransportKeyManager { @Nullable PendingContactId pendingContactId, MutableIncomingKeys inKeys, long streamNumber, boolean handshakeMode) { + requireExactlyOneNull(contactId, pendingContactId); this.keySetId = keySetId; this.contactId = contactId; this.pendingContactId = pendingContactId; From f9b928c12ae65900cbaf91079c290463bfa08eba Mon Sep 17 00:00:00 2001 From: akwizgran Date: Tue, 14 May 2019 10:48:45 +0100 Subject: [PATCH 4/6] Annotate equals() argument as nullable. --- .../org/briarproject/bramble/api/contact/PendingContactId.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/bramble-api/src/main/java/org/briarproject/bramble/api/contact/PendingContactId.java b/bramble-api/src/main/java/org/briarproject/bramble/api/contact/PendingContactId.java index a0a7c7f40..535f8f531 100644 --- a/bramble-api/src/main/java/org/briarproject/bramble/api/contact/PendingContactId.java +++ b/bramble-api/src/main/java/org/briarproject/bramble/api/contact/PendingContactId.java @@ -3,6 +3,7 @@ package org.briarproject.bramble.api.contact; import org.briarproject.bramble.api.UniqueId; import org.briarproject.bramble.api.nullsafety.NotNullByDefault; +import javax.annotation.Nullable; import javax.annotation.concurrent.ThreadSafe; /** @@ -18,7 +19,7 @@ public class PendingContactId extends UniqueId { } @Override - public boolean equals(Object o) { + public boolean equals(@Nullable Object o) { return o instanceof PendingContactId && super.equals(o); } } From 0b30a0786e77addec89f6deb2dcf670538edc279 Mon Sep 17 00:00:00 2001 From: akwizgran Date: Tue, 14 May 2019 10:57:48 +0100 Subject: [PATCH 5/6] Rename key manager methods for clarity. --- .../bramble/api/transport/KeyManager.java | 10 +++--- .../bramble/contact/ContactManagerImpl.java | 3 +- .../bramble/transport/KeyManagerImpl.java | 16 ++++++---- .../transport/TransportKeyManager.java | 9 +++--- .../transport/TransportKeyManagerImpl.java | 9 +++--- .../contact/ContactManagerImplTest.java | 4 +-- .../bramble/transport/KeyManagerImplTest.java | 16 +++++----- .../TransportKeyManagerImplTest.java | 32 +++++++++---------- .../IntroduceeProtocolEngine.java | 4 +-- 9 files changed, 54 insertions(+), 49 deletions(-) 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 1651a135a..75ae88d86 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 @@ -28,9 +28,9 @@ 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 addContact(Transaction txn, ContactId c, - SecretKey rootKey, long timestamp, boolean alice, boolean active) - throws DbException; + Map addContactWithRotationKeys(Transaction txn, + ContactId c, SecretKey rootKey, long timestamp, boolean alice, + boolean active) throws DbException; /** * Informs the key manager that a new contact has been added. Derives and @@ -42,8 +42,8 @@ public interface KeyManager { * * @param alice True if the local party is Alice */ - Map addContact(Transaction txn, ContactId c, - SecretKey rootKey, boolean alice) throws DbException; + Map addContactWithHandshakeKeys(Transaction txn, + ContactId c, SecretKey rootKey, boolean alice) throws DbException; /** * Informs the key manager that a new pending contact has been added. 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 632d9226b..985793843 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 @@ -70,7 +70,8 @@ class ContactManagerImpl implements ContactManager { SecretKey rootKey, long timestamp, boolean alice, boolean verified, boolean active) throws DbException { ContactId c = db.addContact(txn, remote, local, verified); - keyManager.addContact(txn, c, rootKey, timestamp, alice, active); + keyManager.addContactWithRotationKeys(txn, c, rootKey, timestamp, + alice, active); 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/transport/KeyManagerImpl.java b/bramble-core/src/main/java/org/briarproject/bramble/transport/KeyManagerImpl.java index 036b9d2d9..b5adf450d 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 @@ -89,26 +89,28 @@ class KeyManagerImpl implements KeyManager, Service, EventListener { } @Override - public Map addContact(Transaction txn, - ContactId c, SecretKey rootKey, long timestamp, boolean alice, - boolean active) throws DbException { + public Map addContactWithRotationKeys( + 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.addContact(txn, c, rootKey, timestamp, alice, active)); + ids.put(t, m.addContactWithRotationKeys(txn, c, rootKey, timestamp, + alice, active)); } return ids; } @Override - public Map addContact(Transaction txn, - ContactId c, SecretKey rootKey, boolean alice) throws DbException { + public Map addContactWithHandshakeKeys( + Transaction txn, ContactId c, SecretKey rootKey, boolean alice) + throws DbException { Map ids = new HashMap<>(); for (Entry e : managers.entrySet()) { TransportId t = e.getKey(); TransportKeyManager m = e.getValue(); - ids.put(t, m.addContact(txn, c, rootKey, alice)); + ids.put(t, m.addContactWithHandshakeKeys(txn, c, 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 bed597398..45be00197 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,11 +16,12 @@ interface TransportKeyManager { void start(Transaction txn) throws DbException; - KeySetId addContact(Transaction txn, ContactId c, SecretKey rootKey, - long timestamp, boolean alice, boolean active) throws DbException; + KeySetId addContactWithRotationKeys(Transaction txn, ContactId c, + SecretKey rootKey, long timestamp, boolean alice, boolean active) + throws DbException; - KeySetId addContact(Transaction txn, ContactId c, SecretKey rootKey, - boolean alice) throws DbException; + KeySetId addContactWithHandshakeKeys(Transaction txn, ContactId c, + SecretKey rootKey, boolean alice) throws DbException; KeySetId addPendingContact(Transaction txn, PendingContactId p, SecretKey rootKey, boolean alice) 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 ba31f851c..81255dbc3 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 @@ -207,8 +207,9 @@ class TransportKeyManagerImpl implements TransportKeyManager { } @Override - public KeySetId addContact(Transaction txn, ContactId c, SecretKey rootKey, - long timestamp, boolean alice, boolean active) throws DbException { + public KeySetId addContactWithRotationKeys(Transaction txn, ContactId c, + SecretKey rootKey, long timestamp, boolean alice, boolean active) + throws DbException { lock.lock(); try { // Work out what time period the timestamp belongs to @@ -230,8 +231,8 @@ class TransportKeyManagerImpl implements TransportKeyManager { } @Override - public KeySetId addContact(Transaction txn, ContactId c, SecretKey rootKey, - boolean alice) throws DbException { + public KeySetId addContactWithHandshakeKeys(Transaction txn, ContactId c, + SecretKey rootKey, boolean alice) throws DbException { lock.lock(); try { // Work out what time period we're in 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 df7463983..86a3932ee 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, verified); will(returnValue(contactId)); - oneOf(keyManager).addContact(txn, contactId, rootKey, timestamp, - alice, active); + oneOf(keyManager).addContactWithRotationKeys(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 7d212aff5..0220c358b 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 @@ -98,13 +98,13 @@ public class KeyManagerImplTest extends BrambleMockTestCase { boolean active = random.nextBoolean(); context.checking(new Expectations() {{ - oneOf(transportKeyManager).addContact(txn, contactId, secretKey, - timestamp, alice, active); + oneOf(transportKeyManager).addContactWithRotationKeys(txn, + contactId, secretKey, timestamp, alice, active); will(returnValue(keySetId)); }}); - Map ids = keyManager.addContact(txn, contactId, - secretKey, timestamp, alice, active); + Map ids = keyManager.addContactWithRotationKeys( + txn, contactId, secretKey, timestamp, alice, active); assertEquals(singletonMap(transportId, keySetId), ids); } @@ -114,13 +114,13 @@ public class KeyManagerImplTest extends BrambleMockTestCase { boolean alice = random.nextBoolean(); context.checking(new Expectations() {{ - oneOf(transportKeyManager).addContact(txn, contactId, secretKey, - alice); + oneOf(transportKeyManager).addContactWithHandshakeKeys( + txn, contactId, secretKey, alice); will(returnValue(keySetId)); }}); - Map ids = keyManager.addContact(txn, contactId, - secretKey, alice); + Map ids = keyManager.addContactWithHandshakeKeys( + txn, contactId, secretKey, alice); 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 57abdab52..660476610 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 @@ -156,8 +156,8 @@ 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.addContact(txn, contactId, - rootKey, timestamp, alice, active)); + assertEquals(keySetId, transportKeyManager.addContactWithRotationKeys( + txn, contactId, rootKey, timestamp, alice, active)); assertEquals(active, transportKeyManager.canSendOutgoingStreams(contactId)); } @@ -192,8 +192,8 @@ public class TransportKeyManagerImplTest extends BrambleMockTestCase { TransportKeyManager transportKeyManager = new TransportKeyManagerImpl( db, transportCrypto, dbExecutor, scheduler, clock, transportId, maxLatency); - assertEquals(keySetId, transportKeyManager.addContact(txn, contactId, - rootKey, alice)); + assertEquals(keySetId, transportKeyManager.addContactWithHandshakeKeys( + txn, contactId, rootKey, alice)); assertTrue(transportKeyManager.canSendOutgoingStreams(contactId)); } @@ -274,8 +274,8 @@ public class TransportKeyManagerImplTest extends BrambleMockTestCase { maxLatency); // The timestamp is at the start of time period 1000 long timestamp = timePeriodLength * 1000; - assertEquals(keySetId, transportKeyManager.addContact(txn, contactId, - rootKey, timestamp, alice, true)); + assertEquals(keySetId, transportKeyManager.addContactWithRotationKeys( + txn, contactId, rootKey, timestamp, alice, true)); assertFalse(transportKeyManager.canSendOutgoingStreams(contactId)); assertNull(transportKeyManager.getStreamContext(txn, contactId)); } @@ -300,8 +300,8 @@ public class TransportKeyManagerImplTest extends BrambleMockTestCase { maxLatency); // The timestamp is at the start of time period 1000 long timestamp = timePeriodLength * 1000; - assertEquals(keySetId, transportKeyManager.addContact(txn, contactId, - rootKey, timestamp, alice, true)); + assertEquals(keySetId, transportKeyManager.addContactWithRotationKeys( + txn, contactId, rootKey, timestamp, alice, true)); // The first request should return a stream context assertTrue(transportKeyManager.canSendOutgoingStreams(contactId)); StreamContext ctx = transportKeyManager.getStreamContext(txn, @@ -332,8 +332,8 @@ public class TransportKeyManagerImplTest extends BrambleMockTestCase { maxLatency); // The timestamp is at the start of time period 1000 long timestamp = timePeriodLength * 1000; - assertEquals(keySetId, transportKeyManager.addContact(txn, contactId, - rootKey, timestamp, alice, active)); + assertEquals(keySetId, transportKeyManager.addContactWithRotationKeys( + txn, contactId, rootKey, timestamp, alice, active)); assertEquals(active, transportKeyManager.canSendOutgoingStreams(contactId)); // The tag should not be recognised @@ -385,8 +385,8 @@ public class TransportKeyManagerImplTest extends BrambleMockTestCase { maxLatency); // The timestamp is at the start of time period 1000 long timestamp = timePeriodLength * 1000; - assertEquals(keySetId, transportKeyManager.addContact(txn, contactId, - rootKey, timestamp, alice, true)); + assertEquals(keySetId, transportKeyManager.addContactWithRotationKeys( + txn, contactId, rootKey, timestamp, alice, true)); assertTrue(transportKeyManager.canSendOutgoingStreams(contactId)); // Use the first tag (previous time period, stream number 0) assertEquals(REORDERING_WINDOW_SIZE * 3, tags.size()); @@ -488,8 +488,8 @@ public class TransportKeyManagerImplTest extends BrambleMockTestCase { maxLatency); // The timestamp is at the start of time period 1000 long timestamp = timePeriodLength * 1000; - assertEquals(keySetId, transportKeyManager.addContact(txn, contactId, - rootKey, timestamp, alice, false)); + assertEquals(keySetId, transportKeyManager.addContactWithRotationKeys( + txn, contactId, rootKey, timestamp, alice, false)); // The keys are inactive so no stream context should be returned assertFalse(transportKeyManager.canSendOutgoingStreams(contactId)); assertNull(transportKeyManager.getStreamContext(txn, contactId)); @@ -554,8 +554,8 @@ public class TransportKeyManagerImplTest extends BrambleMockTestCase { maxLatency); // The timestamp is at the start of time period 1000 long timestamp = timePeriodLength * 1000; - assertEquals(keySetId, transportKeyManager.addContact(txn, contactId, - rootKey, timestamp, alice, false)); + assertEquals(keySetId, transportKeyManager.addContactWithRotationKeys( + txn, contactId, rootKey, timestamp, alice, false)); // The keys are inactive so no stream context should be returned assertFalse(transportKeyManager.canSendOutgoingStreams(contactId)); assertNull(transportKeyManager.getStreamContext(txn, contactId)); 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 49ed450e8..b2fd5170d 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 @@ -5,6 +5,7 @@ import org.briarproject.bramble.api.client.ClientHelper; import org.briarproject.bramble.api.client.ContactGroupFactory; import org.briarproject.bramble.api.contact.Contact; import org.briarproject.bramble.api.contact.ContactManager; +import org.briarproject.bramble.api.contact.event.ContactAddedRemotelyEvent; import org.briarproject.bramble.api.crypto.KeyPair; import org.briarproject.bramble.api.crypto.SecretKey; import org.briarproject.bramble.api.data.BdfDictionary; @@ -30,7 +31,6 @@ import org.briarproject.briar.api.client.SessionId; import org.briarproject.briar.api.introduction.IntroductionRequest; import org.briarproject.briar.api.introduction.event.IntroductionAbortedEvent; import org.briarproject.briar.api.introduction.event.IntroductionRequestReceivedEvent; -import org.briarproject.bramble.api.contact.event.ContactAddedRemotelyEvent; import java.security.GeneralSecurityException; import java.util.Map; @@ -443,7 +443,7 @@ class IntroduceeProtocolEngine // add the keys to the new contact //noinspection ConstantConditions - keys = keyManager.addContact(txn, c.getId(), + keys = keyManager.addContactWithRotationKeys(txn, c.getId(), new SecretKey(s.getMasterKey()), timestamp, s.getLocal().alice, false); From 2c4188caf5ad0be56d2815b73f1458f6a5034267 Mon Sep 17 00:00:00 2001 From: akwizgran Date: Tue, 14 May 2019 11:16:37 +0100 Subject: [PATCH 6/6] Use lambdas for tasks requiring a manager lookup. --- .../bramble/transport/KeyManagerImpl.java | 56 ++++++++++--------- 1 file changed, 29 insertions(+), 27 deletions(-) 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 b5adf450d..a508c2e83 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 @@ -29,6 +29,7 @@ import java.util.concurrent.Executor; import java.util.concurrent.atomic.AtomicBoolean; import java.util.logging.Logger; +import javax.annotation.Nullable; import javax.annotation.concurrent.ThreadSafe; import javax.inject.Inject; @@ -132,13 +133,10 @@ class KeyManagerImpl implements KeyManager, Service, EventListener { public void activateKeys(Transaction txn, Map keys) throws DbException { for (Entry e : keys.entrySet()) { - TransportId t = e.getKey(); - TransportKeyManager m = managers.get(t); - if (m == null) { - if (LOG.isLoggable(INFO)) LOG.info("No key manager for " + t); - } else { + withManager(e.getKey(), m -> { m.activateKeys(txn, e.getValue()); - } + return null; + }); } } @@ -157,37 +155,25 @@ class KeyManagerImpl implements KeyManager, Service, EventListener { @Override public StreamContext getStreamContext(ContactId c, TransportId t) throws DbException { - TransportKeyManager m = managers.get(t); - if (m == null) { - if (LOG.isLoggable(INFO)) LOG.info("No key manager for " + t); - return null; - } - return db.transactionWithNullableResult(false, txn -> - m.getStreamContext(txn, c)); + return withManager(t, m -> + db.transactionWithNullableResult(false, txn -> + m.getStreamContext(txn, c))); } @Override public StreamContext getStreamContext(PendingContactId p, TransportId t) throws DbException { - TransportKeyManager m = managers.get(t); - if (m == null) { - if (LOG.isLoggable(INFO)) LOG.info("No key manager for " + t); - return null; - } - return db.transactionWithNullableResult(false, txn -> - m.getStreamContext(txn, p)); + return withManager(t, m -> + db.transactionWithNullableResult(false, txn -> + m.getStreamContext(txn, p))); } @Override public StreamContext getStreamContext(TransportId t, byte[] tag) throws DbException { - TransportKeyManager m = managers.get(t); - if (m == null) { - if (LOG.isLoggable(INFO)) LOG.info("No key manager for " + t); - return null; - } - return db.transactionWithNullableResult(false, txn -> - m.getStreamContext(txn, tag)); + return withManager(t, m -> + db.transactionWithNullableResult(false, txn -> + m.getStreamContext(txn, tag))); } @Override @@ -202,4 +188,20 @@ class KeyManagerImpl implements KeyManager, Service, EventListener { for (TransportKeyManager m : managers.values()) m.removeContact(c); }); } + + @Nullable + private T withManager(TransportId t, ManagerTask task) + throws DbException { + TransportKeyManager m = managers.get(t); + if (m == null) { + if (LOG.isLoggable(INFO)) LOG.info("No key manager for " + t); + return null; + } + return task.run(m); + } + + private interface ManagerTask { + @Nullable + T run(TransportKeyManager m) throws DbException; + } }