From 78f2d48bc4432ba8a00594f1187b3f623c0ba638 Mon Sep 17 00:00:00 2001 From: akwizgran Date: Tue, 13 Mar 2018 17:20:46 +0000 Subject: [PATCH 01/11] Support multiple sets of transport keys per contact. --- .../bramble/api/contact/ContactId.java | 2 +- .../bramble/api/db/DatabaseComponent.java | 21 +- .../bramble/api/transport/KeySet.java | 51 ++++ .../bramble/api/transport/KeySetId.java | 36 +++ .../org/briarproject/bramble/db/Database.java | 16 +- .../bramble/db/DatabaseComponentImpl.java | 36 ++- .../briarproject/bramble/db/JdbcDatabase.java | 220 ++++++++++-------- .../bramble/transport/MutableKeySet.java | 34 +++ .../transport/TransportKeyManagerImpl.java | 124 ++++++---- .../bramble/db/DatabaseComponentImplTest.java | 33 +-- .../bramble/db/JdbcDatabaseTest.java | 100 ++++---- .../TransportKeyManagerImplTest.java | 30 ++- 12 files changed, 430 insertions(+), 273 deletions(-) create mode 100644 bramble-api/src/main/java/org/briarproject/bramble/api/transport/KeySet.java create mode 100644 bramble-api/src/main/java/org/briarproject/bramble/api/transport/KeySetId.java create mode 100644 bramble-core/src/main/java/org/briarproject/bramble/transport/MutableKeySet.java 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 9e11e5557..21293599f 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 @@ -6,7 +6,7 @@ import javax.annotation.concurrent.Immutable; /** * Type-safe wrapper for an integer that uniquely identifies a contact within - * the scope of a single node. + * the scope of the local device. */ @Immutable @NotNullByDefault 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 08fbe4540..45f721f22 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 @@ -18,6 +18,8 @@ import org.briarproject.bramble.api.sync.MessageId; import org.briarproject.bramble.api.sync.MessageStatus; import org.briarproject.bramble.api.sync.Offer; import org.briarproject.bramble.api.sync.Request; +import org.briarproject.bramble.api.transport.KeySet; +import org.briarproject.bramble.api.transport.KeySetId; import org.briarproject.bramble.api.transport.TransportKeys; import java.util.Collection; @@ -102,10 +104,11 @@ public interface DatabaseComponent { throws DbException; /** - * Stores transport keys for a newly added contact. + * Stores the given transport keys, optionally binding them to the given + * contact, and returns a key set ID. */ - void addTransportKeys(Transaction txn, ContactId c, TransportKeys k) - throws DbException; + KeySetId addTransportKeys(Transaction txn, @Nullable ContactId c, + TransportKeys k) throws DbException; /** * Returns true if the database contains the given contact for the given @@ -394,8 +397,8 @@ public interface DatabaseComponent { *

* Read-only. */ - Map getTransportKeys(Transaction txn, - TransportId t) throws DbException; + Collection getTransportKeys(Transaction txn, TransportId t) + throws DbException; /** * Increments the outgoing stream counter for the given contact and @@ -507,15 +510,15 @@ public interface DatabaseComponent { Collection dependencies) throws DbException; /** - * Sets the reordering window for the given contact and transport in the + * Sets the reordering window for the given key set and transport in the * given rotation period. */ - void setReorderingWindow(Transaction txn, ContactId c, TransportId t, + void setReorderingWindow(Transaction txn, KeySetId k, TransportId t, long rotationPeriod, long base, byte[] bitmap) throws DbException; /** * Stores the given transport keys, deleting any keys they have replaced. */ - void updateTransportKeys(Transaction txn, - Map keys) throws DbException; + void updateTransportKeys(Transaction txn, Collection keys) + throws DbException; } diff --git a/bramble-api/src/main/java/org/briarproject/bramble/api/transport/KeySet.java b/bramble-api/src/main/java/org/briarproject/bramble/api/transport/KeySet.java new file mode 100644 index 000000000..9cc8f63c2 --- /dev/null +++ b/bramble-api/src/main/java/org/briarproject/bramble/api/transport/KeySet.java @@ -0,0 +1,51 @@ +package org.briarproject.bramble.api.transport; + +import org.briarproject.bramble.api.contact.ContactId; +import org.briarproject.bramble.api.nullsafety.NotNullByDefault; + +import javax.annotation.Nullable; +import javax.annotation.concurrent.Immutable; + +/** + * A set of transport keys for communicating with a contact. If the keys have + * not yet been bound to a contact, {@link #getContactId()}} returns null. + */ +@Immutable +@NotNullByDefault +public class KeySet { + + private final KeySetId keySetId; + @Nullable + private final ContactId contactId; + private final TransportKeys transportKeys; + + public KeySet(KeySetId keySetId, @Nullable ContactId contactId, + TransportKeys transportKeys) { + this.keySetId = keySetId; + this.contactId = contactId; + this.transportKeys = transportKeys; + } + + public KeySetId getKeySetId() { + return keySetId; + } + + @Nullable + public ContactId getContactId() { + return contactId; + } + + public TransportKeys getTransportKeys() { + return transportKeys; + } + + @Override + public int hashCode() { + return keySetId.hashCode(); + } + + @Override + public boolean equals(Object o) { + return o instanceof KeySet && keySetId.equals(((KeySet) o).keySetId); + } +} diff --git a/bramble-api/src/main/java/org/briarproject/bramble/api/transport/KeySetId.java b/bramble-api/src/main/java/org/briarproject/bramble/api/transport/KeySetId.java new file mode 100644 index 000000000..1f872e72a --- /dev/null +++ b/bramble-api/src/main/java/org/briarproject/bramble/api/transport/KeySetId.java @@ -0,0 +1,36 @@ +package org.briarproject.bramble.api.transport; + +import org.briarproject.bramble.api.nullsafety.NotNullByDefault; + +import javax.annotation.concurrent.Immutable; + +/** + * Type-safe wrapper for an integer that uniquely identifies a set of transport + * keys within the scope of the local device. + *

+ * Key sets created on a given device must have increasing identifiers. + */ +@Immutable +@NotNullByDefault +public class KeySetId { + + private final int id; + + public KeySetId(int id) { + this.id = id; + } + + public int getInt() { + return id; + } + + @Override + public int hashCode() { + return id; + } + + @Override + public boolean equals(Object o) { + return o instanceof KeySetId && id == ((KeySetId) o).id; + } +} 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 bc3166d56..e8597eaba 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 @@ -21,6 +21,8 @@ import org.briarproject.bramble.api.sync.Message; import org.briarproject.bramble.api.sync.MessageId; import org.briarproject.bramble.api.sync.MessageStatus; import org.briarproject.bramble.api.sync.ValidationManager.State; +import org.briarproject.bramble.api.transport.KeySet; +import org.briarproject.bramble.api.transport.KeySetId; import org.briarproject.bramble.api.transport.TransportKeys; import java.util.Collection; @@ -123,9 +125,10 @@ interface Database { throws DbException; /** - * Stores transport keys for a newly added contact. + * Stores the given transport keys, optionally binding them to the given + * contact, and returns a key set ID. */ - void addTransportKeys(T txn, ContactId c, TransportKeys k) + KeySetId addTransportKeys(T txn, @Nullable ContactId c, TransportKeys k) throws DbException; /** @@ -486,7 +489,7 @@ interface Database { *

* Read-only. */ - Map getTransportKeys(T txn, TransportId t) + Collection getTransportKeys(T txn, TransportId t) throws DbException; /** @@ -619,10 +622,10 @@ interface Database { void setMessageState(T txn, MessageId m, State state) throws DbException; /** - * Sets the reordering window for the given contact and transport in the + * Sets the reordering window for the given key set and transport in the * given rotation period. */ - void setReorderingWindow(T txn, ContactId c, TransportId t, + void setReorderingWindow(T txn, KeySetId k, TransportId t, long rotationPeriod, long base, byte[] bitmap) throws DbException; /** @@ -636,6 +639,5 @@ interface Database { /** * Stores the given transport keys, deleting any keys they have replaced. */ - void updateTransportKeys(T txn, Map keys) - throws DbException; + void updateTransportKeys(T txn, Collection keys) throws DbException; } 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 013233f39..f90f9e3ff 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 @@ -51,15 +51,15 @@ import org.briarproject.bramble.api.sync.event.MessageToAckEvent; import org.briarproject.bramble.api.sync.event.MessageToRequestEvent; import org.briarproject.bramble.api.sync.event.MessagesAckedEvent; import org.briarproject.bramble.api.sync.event.MessagesSentEvent; +import org.briarproject.bramble.api.transport.KeySet; +import org.briarproject.bramble.api.transport.KeySetId; import org.briarproject.bramble.api.transport.TransportKeys; import java.util.ArrayList; import java.util.Collection; import java.util.Collections; -import java.util.HashMap; import java.util.List; import java.util.Map; -import java.util.Map.Entry; import java.util.concurrent.atomic.AtomicBoolean; import java.util.concurrent.locks.ReentrantReadWriteLock; import java.util.logging.Logger; @@ -234,15 +234,15 @@ class DatabaseComponentImpl implements DatabaseComponent { } @Override - public void addTransportKeys(Transaction transaction, ContactId c, - TransportKeys k) throws DbException { + public KeySetId addTransportKeys(Transaction transaction, + @Nullable ContactId c, TransportKeys k) throws DbException { if (transaction.isReadOnly()) throw new IllegalArgumentException(); T txn = unbox(transaction); - if (!db.containsContact(txn, c)) + if (c != null && !db.containsContact(txn, c)) throw new NoSuchContactException(); if (!db.containsTransport(txn, k.getTransportId())) throw new NoSuchTransportException(); - db.addTransportKeys(txn, c, k); + return db.addTransportKeys(txn, c, k); } @Override @@ -586,8 +586,8 @@ class DatabaseComponentImpl implements DatabaseComponent { } @Override - public Map getTransportKeys( - Transaction transaction, TransportId t) throws DbException { + public Collection getTransportKeys(Transaction transaction, + TransportId t) throws DbException { T txn = unbox(transaction); if (!db.containsTransport(txn, t)) throw new NoSuchTransportException(); @@ -858,31 +858,25 @@ class DatabaseComponentImpl implements DatabaseComponent { } @Override - public void setReorderingWindow(Transaction transaction, ContactId c, + public void setReorderingWindow(Transaction transaction, KeySetId k, TransportId t, long rotationPeriod, long base, byte[] bitmap) throws DbException { if (transaction.isReadOnly()) throw new IllegalArgumentException(); T txn = unbox(transaction); - if (!db.containsContact(txn, c)) - throw new NoSuchContactException(); if (!db.containsTransport(txn, t)) throw new NoSuchTransportException(); - db.setReorderingWindow(txn, c, t, rotationPeriod, base, bitmap); + db.setReorderingWindow(txn, k, t, rotationPeriod, base, bitmap); } @Override public void updateTransportKeys(Transaction transaction, - Map keys) throws DbException { + Collection keys) throws DbException { if (transaction.isReadOnly()) throw new IllegalArgumentException(); T txn = unbox(transaction); - Map filtered = new HashMap<>(); - for (Entry e : keys.entrySet()) { - ContactId c = e.getKey(); - TransportKeys k = e.getValue(); - if (db.containsContact(txn, c) - && db.containsTransport(txn, k.getTransportId())) { - filtered.put(c, k); - } + Collection filtered = new ArrayList<>(); + for (KeySet ks : keys) { + TransportId t = ks.getTransportKeys().getTransportId(); + if (db.containsTransport(txn, t)) filtered.add(ks); } db.updateTransportKeys(txn, filtered); } 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 51fa9ae30..327203d63 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 @@ -25,6 +25,8 @@ import org.briarproject.bramble.api.sync.MessageStatus; import org.briarproject.bramble.api.sync.ValidationManager.State; import org.briarproject.bramble.api.system.Clock; import org.briarproject.bramble.api.transport.IncomingKeys; +import org.briarproject.bramble.api.transport.KeySet; +import org.briarproject.bramble.api.transport.KeySetId; import org.briarproject.bramble.api.transport.OutgoingKeys; import org.briarproject.bramble.api.transport.TransportKeys; @@ -223,37 +225,43 @@ abstract class JdbcDatabase implements Database { + " maxLatency INT NOT NULL," + " PRIMARY KEY (transportId))"; + private static final String CREATE_OUTGOING_KEYS = + "CREATE TABLE outgoingKeys" + + " (transportId _STRING NOT NULL," + + " keySetId _COUNTER," + + " rotationPeriod BIGINT NOT NULL," + + " contactId INT," // Null if keys are not bound + + " tagKey _SECRET NOT NULL," + + " headerKey _SECRET NOT NULL," + + " stream BIGINT NOT NULL," + + " PRIMARY KEY (transportId, keySetId)," + + " FOREIGN KEY (transportId)" + + " REFERENCES transports (transportId)" + + " ON DELETE CASCADE," + + " UNIQUE (keySetId)," + + " FOREIGN KEY (contactId)" + + " REFERENCES contacts (contactId)" + + " ON DELETE CASCADE)"; + private static final String CREATE_INCOMING_KEYS = "CREATE TABLE incomingKeys" - + " (contactId INT NOT NULL," - + " transportId _STRING NOT NULL," + + " (transportId _STRING NOT NULL," + + " keySetId INT NOT NULL," + " rotationPeriod BIGINT NOT NULL," + + " contactId INT," // Null if keys are not bound + " tagKey _SECRET NOT NULL," + " headerKey _SECRET NOT NULL," + " base BIGINT NOT NULL," + " bitmap _BINARY NOT NULL," - + " PRIMARY KEY (contactId, transportId, rotationPeriod)," - + " FOREIGN KEY (contactId)" - + " REFERENCES contacts (contactId)" - + " ON DELETE CASCADE," + + " PRIMARY KEY (transportId, keySetId, rotationPeriod)," + " FOREIGN KEY (transportId)" + " REFERENCES transports (transportId)" - + " ON DELETE CASCADE)"; - - private static final String CREATE_OUTGOING_KEYS = - "CREATE TABLE outgoingKeys" - + " (contactId INT NOT NULL," - + " transportId _STRING NOT NULL," - + " rotationPeriod BIGINT NOT NULL," - + " tagKey _SECRET NOT NULL," - + " headerKey _SECRET NOT NULL," - + " stream BIGINT NOT NULL," - + " PRIMARY KEY (contactId, transportId)," + + " ON DELETE CASCADE," + + " FOREIGN KEY (keySetId)" + + " REFERENCES outgoingKeys (keySetId)" + + " ON DELETE CASCADE," + " FOREIGN KEY (contactId)" + " REFERENCES contacts (contactId)" - + " ON DELETE CASCADE," - + " FOREIGN KEY (transportId)" - + " REFERENCES transports (transportId)" + " ON DELETE CASCADE)"; private static final String INDEX_CONTACTS_BY_AUTHOR_ID = @@ -415,8 +423,8 @@ abstract class JdbcDatabase implements Database { s.executeUpdate(insertTypeNames(CREATE_OFFERS)); s.executeUpdate(insertTypeNames(CREATE_STATUSES)); s.executeUpdate(insertTypeNames(CREATE_TRANSPORTS)); - s.executeUpdate(insertTypeNames(CREATE_INCOMING_KEYS)); s.executeUpdate(insertTypeNames(CREATE_OUTGOING_KEYS)); + s.executeUpdate(insertTypeNames(CREATE_INCOMING_KEYS)); s.close(); } catch (SQLException e) { tryToClose(s); @@ -865,52 +873,18 @@ abstract class JdbcDatabase implements Database { } @Override - public void addTransportKeys(Connection txn, ContactId c, TransportKeys k) - throws DbException { + public KeySetId addTransportKeys(Connection txn, @Nullable ContactId c, + TransportKeys k) throws DbException { PreparedStatement ps = null; + ResultSet rs = null; try { - // Store the incoming keys - String sql = "INSERT INTO incomingKeys (contactId, transportId," - + " rotationPeriod, tagKey, headerKey, base, bitmap)" - + " VALUES (?, ?, ?, ?, ?, ?, ?)"; - ps = txn.prepareStatement(sql); - ps.setInt(1, c.getInt()); - ps.setString(2, k.getTransportId().getString()); - // Previous rotation period - IncomingKeys inPrev = k.getPreviousIncomingKeys(); - ps.setLong(3, inPrev.getRotationPeriod()); - ps.setBytes(4, inPrev.getTagKey().getBytes()); - ps.setBytes(5, inPrev.getHeaderKey().getBytes()); - ps.setLong(6, inPrev.getWindowBase()); - ps.setBytes(7, inPrev.getWindowBitmap()); - ps.addBatch(); - // Current rotation period - IncomingKeys inCurr = k.getCurrentIncomingKeys(); - ps.setLong(3, inCurr.getRotationPeriod()); - ps.setBytes(4, inCurr.getTagKey().getBytes()); - ps.setBytes(5, inCurr.getHeaderKey().getBytes()); - ps.setLong(6, inCurr.getWindowBase()); - ps.setBytes(7, inCurr.getWindowBitmap()); - ps.addBatch(); - // Next rotation period - IncomingKeys inNext = k.getNextIncomingKeys(); - ps.setLong(3, inNext.getRotationPeriod()); - ps.setBytes(4, inNext.getTagKey().getBytes()); - ps.setBytes(5, inNext.getHeaderKey().getBytes()); - ps.setLong(6, inNext.getWindowBase()); - ps.setBytes(7, inNext.getWindowBitmap()); - ps.addBatch(); - int[] batchAffected = ps.executeBatch(); - if (batchAffected.length != 3) throw new DbStateException(); - for (int rows : batchAffected) - if (rows != 1) throw new DbStateException(); - ps.close(); // Store the outgoing keys - sql = "INSERT INTO outgoingKeys (contactId, transportId," + String sql = "INSERT INTO outgoingKeys (contactId, transportId," + " rotationPeriod, tagKey, headerKey, stream)" + " VALUES (?, ?, ?, ?, ?, ?)"; ps = txn.prepareStatement(sql); - ps.setInt(1, c.getInt()); + if (c == null) ps.setNull(1, INTEGER); + else ps.setInt(1, c.getInt()); ps.setString(2, k.getTransportId().getString()); OutgoingKeys outCurr = k.getCurrentOutgoingKeys(); ps.setLong(3, outCurr.getRotationPeriod()); @@ -920,7 +894,57 @@ abstract class JdbcDatabase implements Database { int affected = ps.executeUpdate(); if (affected != 1) throw new DbStateException(); ps.close(); + // Get the new (highest) key set ID + sql = "SELECT keySetId FROM outgoingKeys" + + " ORDER BY keySetId DESC LIMIT 1"; + ps = txn.prepareStatement(sql); + rs = ps.executeQuery(); + if (!rs.next()) throw new DbStateException(); + KeySetId keySetId = new KeySetId(rs.getInt(1)); + if (rs.next()) throw new DbStateException(); + rs.close(); + ps.close(); + // Store the incoming keys + sql = "INSERT INTO incomingKeys (keySetId, contactId, transportId," + + " rotationPeriod, tagKey, headerKey, base, bitmap)" + + " VALUES (?, ?, ?, ?, ?, ?, ?, ?)"; + ps = txn.prepareStatement(sql); + ps.setInt(1, keySetId.getInt()); + if (c == null) ps.setNull(2, INTEGER); + else ps.setInt(2, c.getInt()); + ps.setString(3, k.getTransportId().getString()); + // Previous rotation period + IncomingKeys inPrev = k.getPreviousIncomingKeys(); + ps.setLong(4, inPrev.getRotationPeriod()); + ps.setBytes(5, inPrev.getTagKey().getBytes()); + ps.setBytes(6, inPrev.getHeaderKey().getBytes()); + ps.setLong(7, inPrev.getWindowBase()); + ps.setBytes(8, inPrev.getWindowBitmap()); + ps.addBatch(); + // Current rotation period + IncomingKeys inCurr = k.getCurrentIncomingKeys(); + ps.setLong(4, inCurr.getRotationPeriod()); + ps.setBytes(5, inCurr.getTagKey().getBytes()); + ps.setBytes(6, inCurr.getHeaderKey().getBytes()); + ps.setLong(7, inCurr.getWindowBase()); + ps.setBytes(8, inCurr.getWindowBitmap()); + ps.addBatch(); + // Next rotation period + IncomingKeys inNext = k.getNextIncomingKeys(); + ps.setLong(4, inNext.getRotationPeriod()); + ps.setBytes(5, inNext.getTagKey().getBytes()); + ps.setBytes(6, inNext.getHeaderKey().getBytes()); + ps.setLong(7, inNext.getWindowBase()); + ps.setBytes(8, inNext.getWindowBitmap()); + ps.addBatch(); + int[] batchAffected = ps.executeBatch(); + if (batchAffected.length != 3) throw new DbStateException(); + for (int rows : batchAffected) + if (rows != 1) throw new DbStateException(); + ps.close(); + return keySetId; } catch (SQLException e) { + tryToClose(rs); tryToClose(ps); throw new DbException(e); } @@ -2078,8 +2102,8 @@ abstract class JdbcDatabase implements Database { } @Override - public Map getTransportKeys(Connection txn, - TransportId t) throws DbException { + public Collection getTransportKeys(Connection txn, TransportId t) + throws DbException { PreparedStatement ps = null; ResultSet rs = null; try { @@ -2088,7 +2112,7 @@ abstract class JdbcDatabase implements Database { + " base, bitmap" + " FROM incomingKeys" + " WHERE transportId = ?" - + " ORDER BY contactId, rotationPeriod"; + + " ORDER BY keySetId, rotationPeriod"; ps = txn.prepareStatement(sql); ps.setString(1, t.getString()); rs = ps.executeQuery(); @@ -2105,29 +2129,33 @@ abstract class JdbcDatabase implements Database { rs.close(); ps.close(); // Retrieve the outgoing keys in the same order - sql = "SELECT contactId, rotationPeriod, tagKey, headerKey, stream" + sql = "SELECT keySetId, contactId, rotationPeriod," + + " tagKey, headerKey, stream" + " FROM outgoingKeys" + " WHERE transportId = ?" - + " ORDER BY contactId, rotationPeriod"; + + " ORDER BY keySetId"; ps = txn.prepareStatement(sql); ps.setString(1, t.getString()); rs = ps.executeQuery(); - Map keys = new HashMap<>(); + Collection keys = new ArrayList<>(); for (int i = 0; rs.next(); i++) { // There should be three times as many incoming keys if (inKeys.size() < (i + 1) * 3) throw new DbStateException(); - ContactId contactId = new ContactId(rs.getInt(1)); - long rotationPeriod = rs.getLong(2); - SecretKey tagKey = new SecretKey(rs.getBytes(3)); - SecretKey headerKey = new SecretKey(rs.getBytes(4)); - long streamCounter = rs.getLong(5); + KeySetId keySetId = new KeySetId(rs.getInt(1)); + ContactId contactId = new ContactId(rs.getInt(2)); + if (rs.wasNull()) contactId = null; + long rotationPeriod = rs.getLong(3); + SecretKey tagKey = new SecretKey(rs.getBytes(4)); + SecretKey headerKey = new SecretKey(rs.getBytes(5)); + long streamCounter = rs.getLong(6); OutgoingKeys outCurr = new OutgoingKeys(tagKey, headerKey, rotationPeriod, streamCounter); IncomingKeys inPrev = inKeys.get(i * 3); IncomingKeys inCurr = inKeys.get(i * 3 + 1); IncomingKeys inNext = inKeys.get(i * 3 + 2); - keys.put(contactId, new TransportKeys(t, inPrev, inCurr, - inNext, outCurr)); + TransportKeys transportKeys = new TransportKeys(t, inPrev, + inCurr, inNext, outCurr); + keys.add(new KeySet(keySetId, contactId, transportKeys)); } rs.close(); ps.close(); @@ -2791,18 +2819,18 @@ abstract class JdbcDatabase implements Database { } @Override - public void setReorderingWindow(Connection txn, ContactId c, TransportId t, + public void setReorderingWindow(Connection txn, KeySetId k, TransportId t, long rotationPeriod, long base, byte[] bitmap) throws DbException { PreparedStatement ps = null; try { String sql = "UPDATE incomingKeys SET base = ?, bitmap = ?" - + " WHERE contactId = ? AND transportId = ?" + + " WHERE transportId = ? AND keySetId = ?" + " AND rotationPeriod = ?"; ps = txn.prepareStatement(sql); ps.setLong(1, base); ps.setBytes(2, bitmap); - ps.setInt(3, c.getInt()); - ps.setString(4, t.getString()); + ps.setString(3, t.getString()); + ps.setInt(4, k.getInt()); ps.setLong(5, rotationPeriod); int affected = ps.executeUpdate(); if (affected < 0 || affected > 1) throw new DbStateException(); @@ -2848,45 +2876,31 @@ abstract class JdbcDatabase implements Database { } @Override - public void updateTransportKeys(Connection txn, - Map keys) throws DbException { + public void updateTransportKeys(Connection txn, Collection keys) + throws DbException { PreparedStatement ps = null; try { - // Delete any existing incoming keys - String sql = "DELETE FROM incomingKeys" - + " WHERE contactId = ?" - + " AND transportId = ?"; + // Delete any existing outgoing keys - this will also remove any + // incoming keys with the same key set ID + String sql = "DELETE FROM outgoingKeys WHERE keySetId = ?"; ps = txn.prepareStatement(sql); - for (Entry e : keys.entrySet()) { - ps.setInt(1, e.getKey().getInt()); - ps.setString(2, e.getValue().getTransportId().getString()); + for (KeySet ks : keys) { + ps.setInt(1, ks.getKeySetId().getInt()); ps.addBatch(); } int[] batchAffected = ps.executeBatch(); if (batchAffected.length != keys.size()) throw new DbStateException(); - ps.close(); - // Delete any existing outgoing keys - sql = "DELETE FROM outgoingKeys" - + " WHERE contactId = ?" - + " AND transportId = ?"; - ps = txn.prepareStatement(sql); - for (Entry e : keys.entrySet()) { - ps.setInt(1, e.getKey().getInt()); - ps.setString(2, e.getValue().getTransportId().getString()); - ps.addBatch(); - } - batchAffected = ps.executeBatch(); - if (batchAffected.length != keys.size()) - throw new DbStateException(); + for (int rows: batchAffected) + if (rows < 0) throw new DbStateException(); ps.close(); } catch (SQLException e) { tryToClose(ps); throw new DbException(e); } // Store the new keys - for (Entry e : keys.entrySet()) { - addTransportKeys(txn, e.getKey(), e.getValue()); + for (KeySet ks : keys) { + addTransportKeys(txn, ks.getContactId(), ks.getTransportKeys()); } } } diff --git a/bramble-core/src/main/java/org/briarproject/bramble/transport/MutableKeySet.java b/bramble-core/src/main/java/org/briarproject/bramble/transport/MutableKeySet.java new file mode 100644 index 000000000..b55c5aef4 --- /dev/null +++ b/bramble-core/src/main/java/org/briarproject/bramble/transport/MutableKeySet.java @@ -0,0 +1,34 @@ +package org.briarproject.bramble.transport; + +import org.briarproject.bramble.api.contact.ContactId; +import org.briarproject.bramble.api.transport.KeySetId; + +import javax.annotation.Nullable; + +public class MutableKeySet { + + private final KeySetId keySetId; + @Nullable + private final ContactId contactId; + private final MutableTransportKeys transportKeys; + + public MutableKeySet(KeySetId keySetId, @Nullable ContactId contactId, + MutableTransportKeys transportKeys) { + this.keySetId = keySetId; + this.contactId = contactId; + this.transportKeys = transportKeys; + } + + public KeySetId getKeySetId() { + return keySetId; + } + + @Nullable + public ContactId getContactId() { + return contactId; + } + + public MutableTransportKeys getTransportKeys() { + return transportKeys; + } +} 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 60b48427f..1220beee5 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 @@ -11,19 +11,25 @@ import org.briarproject.bramble.api.nullsafety.NotNullByDefault; import org.briarproject.bramble.api.plugin.TransportId; import org.briarproject.bramble.api.system.Clock; import org.briarproject.bramble.api.system.Scheduler; +import org.briarproject.bramble.api.transport.KeySet; +import org.briarproject.bramble.api.transport.KeySetId; import org.briarproject.bramble.api.transport.StreamContext; import org.briarproject.bramble.api.transport.TransportKeys; import org.briarproject.bramble.transport.ReorderingWindow.Change; +import java.util.ArrayList; +import java.util.Collection; import java.util.HashMap; import java.util.Iterator; import java.util.Map; import java.util.Map.Entry; import java.util.concurrent.Executor; import java.util.concurrent.ScheduledExecutorService; +import java.util.concurrent.atomic.AtomicBoolean; import java.util.concurrent.locks.ReentrantLock; import java.util.logging.Logger; +import javax.annotation.Nullable; import javax.annotation.concurrent.ThreadSafe; import static java.util.concurrent.TimeUnit.MILLISECONDS; @@ -47,12 +53,13 @@ class TransportKeyManagerImpl implements TransportKeyManager { private final Clock clock; private final TransportId transportId; private final long rotationPeriodLength; - private final ReentrantLock lock; + private final AtomicBoolean used = new AtomicBoolean(false); + private final ReentrantLock lock = new ReentrantLock(); // The following are locking: lock - private final Map inContexts; - private final Map outContexts; - private final Map keys; + private final Collection keys = new ArrayList<>(); + private final Map inContexts = new HashMap<>(); + private final Map outContexts = new HashMap<>(); TransportKeyManagerImpl(DatabaseComponent db, TransportCrypto transportCrypto, Executor dbExecutor, @@ -65,20 +72,16 @@ class TransportKeyManagerImpl implements TransportKeyManager { this.clock = clock; this.transportId = transportId; rotationPeriodLength = maxLatency + MAX_CLOCK_DIFFERENCE; - lock = new ReentrantLock(); - inContexts = new HashMap<>(); - outContexts = new HashMap<>(); - keys = new HashMap<>(); } @Override public void start(Transaction txn) throws DbException { + if (used.getAndSet(true)) throw new IllegalStateException(); long now = clock.currentTimeMillis(); lock.lock(); try { // Load the transport keys from the DB - Map loaded = - db.getTransportKeys(txn, transportId); + Collection loaded = db.getTransportKeys(txn, transportId); // Rotate the keys to the current rotation period RotationResult rotationResult = rotateKeys(loaded, now); // Initialise mutable state for all contacts @@ -93,41 +96,51 @@ class TransportKeyManagerImpl implements TransportKeyManager { scheduleKeyRotation(now); } - private RotationResult rotateKeys(Map keys, - long now) { + private RotationResult rotateKeys(Collection keys, long now) { RotationResult rotationResult = new RotationResult(); long rotationPeriod = now / rotationPeriodLength; - for (Entry e : keys.entrySet()) { - ContactId c = e.getKey(); - TransportKeys k = e.getValue(); + for (KeySet ks : keys) { + TransportKeys k = ks.getTransportKeys(); TransportKeys k1 = transportCrypto.rotateTransportKeys(k, rotationPeriod); + KeySet ks1 = new KeySet(ks.getKeySetId(), ks.getContactId(), k1); if (k1.getRotationPeriod() > k.getRotationPeriod()) - rotationResult.rotated.put(c, k1); - rotationResult.current.put(c, k1); + rotationResult.rotated.add(ks1); + rotationResult.current.add(ks1); } return rotationResult; } // Locking: lock - private void addKeys(Map m) { - for (Entry e : m.entrySet()) - addKeys(e.getKey(), new MutableTransportKeys(e.getValue())); + private void addKeys(Collection keys) { + for (KeySet ks : keys) { + addKeys(ks.getKeySetId(), ks.getContactId(), + new MutableTransportKeys(ks.getTransportKeys())); + } } // Locking: lock - private void addKeys(ContactId c, MutableTransportKeys m) { - encodeTags(c, m.getPreviousIncomingKeys()); - encodeTags(c, m.getCurrentIncomingKeys()); - encodeTags(c, m.getNextIncomingKeys()); - outContexts.put(c, m.getCurrentOutgoingKeys()); - keys.put(c, m); + private void addKeys(KeySetId keySetId, @Nullable ContactId contactId, + MutableTransportKeys m) { + MutableKeySet ks = new MutableKeySet(keySetId, contactId, m); + keys.add(ks); + if (contactId != null) { + encodeTags(keySetId, contactId, m.getPreviousIncomingKeys()); + encodeTags(keySetId, contactId, m.getCurrentIncomingKeys()); + encodeTags(keySetId, contactId, m.getNextIncomingKeys()); + // Use the outgoing keys with the highest key set ID + MutableKeySet old = outContexts.get(contactId); + if (old == null || old.getKeySetId().getInt() < keySetId.getInt()) + outContexts.put(contactId, ks); + } } // Locking: lock - private void encodeTags(ContactId c, MutableIncomingKeys inKeys) { + private void encodeTags(KeySetId keySetId, ContactId contactId, + MutableIncomingKeys inKeys) { for (long streamNumber : inKeys.getWindow().getUnseen()) { - TagContext tagCtx = new TagContext(c, inKeys, streamNumber); + TagContext tagCtx = + new TagContext(keySetId, contactId, inKeys, streamNumber); byte[] tag = new byte[TAG_LENGTH]; transportCrypto.encodeTag(tag, inKeys.getTagKey(), PROTOCOL_VERSION, streamNumber); @@ -169,10 +182,10 @@ class TransportKeyManagerImpl implements TransportKeyManager { // Rotate the keys to the current rotation period if necessary rotationPeriod = clock.currentTimeMillis() / rotationPeriodLength; k = transportCrypto.rotateTransportKeys(k, rotationPeriod); - // Initialise mutable state for the contact - addKeys(c, new MutableTransportKeys(k)); // Write the keys back to the DB - db.addTransportKeys(txn, c, k); + KeySetId keySetId = db.addTransportKeys(txn, c, k); + // Initialise mutable state for the contact + addKeys(keySetId, c, new MutableTransportKeys(k)); } finally { lock.unlock(); } @@ -183,12 +196,18 @@ class TransportKeyManagerImpl implements TransportKeyManager { lock.lock(); try { // Remove mutable state for the contact - Iterator> it = + Iterator> inContextsIter = inContexts.entrySet().iterator(); - while (it.hasNext()) - if (it.next().getValue().contactId.equals(c)) it.remove(); + while (inContextsIter.hasNext()) { + ContactId c1 = inContextsIter.next().getValue().contactId; + if (c1.equals(c)) inContextsIter.remove(); + } outContexts.remove(c); - keys.remove(c); + Iterator keysIter = keys.iterator(); + while (keysIter.hasNext()) { + ContactId c1 = keysIter.next().getContactId(); + if (c1 != null && c1.equals(c)) keysIter.remove(); + } } finally { lock.unlock(); } @@ -200,8 +219,10 @@ class TransportKeyManagerImpl implements TransportKeyManager { lock.lock(); try { // Look up the outgoing keys for the contact - MutableOutgoingKeys outKeys = outContexts.get(c); - if (outKeys == null) return null; + MutableKeySet ks = outContexts.get(c); + if (ks == null) return null; + MutableOutgoingKeys outKeys = + ks.getTransportKeys().getCurrentOutgoingKeys(); if (outKeys.getStreamCounter() > MAX_32_BIT_UNSIGNED) return null; // Create a stream context StreamContext ctx = new StreamContext(c, transportId, @@ -238,8 +259,9 @@ class TransportKeyManagerImpl implements TransportKeyManager { byte[] addTag = new byte[TAG_LENGTH]; transportCrypto.encodeTag(addTag, inKeys.getTagKey(), PROTOCOL_VERSION, streamNumber); - inContexts.put(new Bytes(addTag), new TagContext( - tagCtx.contactId, inKeys, streamNumber)); + TagContext tagCtx1 = new TagContext(tagCtx.keySetId, + tagCtx.contactId, inKeys, streamNumber); + inContexts.put(new Bytes(addTag), tagCtx1); } // Remove tags for any stream numbers removed from the window for (long streamNumber : change.getRemoved()) { @@ -250,7 +272,7 @@ class TransportKeyManagerImpl implements TransportKeyManager { inContexts.remove(new Bytes(removeTag)); } // Write the window back to the DB - db.setReorderingWindow(txn, tagCtx.contactId, transportId, + db.setReorderingWindow(txn, tagCtx.keySetId, transportId, inKeys.getRotationPeriod(), window.getBase(), window.getBitmap()); return ctx; @@ -264,9 +286,11 @@ class TransportKeyManagerImpl implements TransportKeyManager { lock.lock(); try { // Rotate the keys to the current rotation period - Map snapshot = new HashMap<>(); - for (Entry e : keys.entrySet()) - snapshot.put(e.getKey(), e.getValue().snapshot()); + Collection snapshot = new ArrayList<>(keys.size()); + for (MutableKeySet ks : keys) { + snapshot.add(new KeySet(ks.getKeySetId(), ks.getContactId(), + ks.getTransportKeys().snapshot())); + } RotationResult rotationResult = rotateKeys(snapshot, now); // Rebuild the mutable state for all contacts inContexts.clear(); @@ -285,12 +309,14 @@ class TransportKeyManagerImpl implements TransportKeyManager { private static class TagContext { + private final KeySetId keySetId; private final ContactId contactId; private final MutableIncomingKeys inKeys; private final long streamNumber; - private TagContext(ContactId contactId, MutableIncomingKeys inKeys, - long streamNumber) { + private TagContext(KeySetId keySetId, ContactId contactId, + MutableIncomingKeys inKeys, long streamNumber) { + this.keySetId = keySetId; this.contactId = contactId; this.inKeys = inKeys; this.streamNumber = streamNumber; @@ -299,11 +325,7 @@ class TransportKeyManagerImpl implements TransportKeyManager { private static class RotationResult { - private final Map current, rotated; - - private RotationResult() { - current = new HashMap<>(); - rotated = new HashMap<>(); - } + private final Collection current = new ArrayList<>(); + private final Collection rotated = new ArrayList<>(); } } 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 0ed457461..eb31247ce 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 @@ -44,6 +44,8 @@ import org.briarproject.bramble.api.sync.event.MessageToRequestEvent; import org.briarproject.bramble.api.sync.event.MessagesAckedEvent; import org.briarproject.bramble.api.sync.event.MessagesSentEvent; import org.briarproject.bramble.api.transport.IncomingKeys; +import org.briarproject.bramble.api.transport.KeySet; +import org.briarproject.bramble.api.transport.KeySetId; import org.briarproject.bramble.api.transport.OutgoingKeys; import org.briarproject.bramble.api.transport.TransportKeys; import org.briarproject.bramble.test.BrambleMockTestCase; @@ -55,12 +57,10 @@ import org.junit.Test; import java.util.ArrayList; import java.util.Arrays; import java.util.Collection; -import java.util.Map; import java.util.concurrent.atomic.AtomicReference; import static java.util.Collections.emptyMap; import static java.util.Collections.singletonList; -import static java.util.Collections.singletonMap; 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.Group.Visibility.VISIBLE; @@ -100,6 +100,7 @@ public class DatabaseComponentImplTest extends BrambleMockTestCase { private final int maxLatency; private final ContactId contactId; private final Contact contact; + private final KeySetId keySetId; public DatabaseComponentImplTest() { clientId = new ClientId(getRandomString(123)); @@ -121,6 +122,7 @@ public class DatabaseComponentImplTest extends BrambleMockTestCase { contactId = new ContactId(234); contact = new Contact(contactId, author, localAuthor.getId(), true, true); + keySetId = new KeySetId(345); } private DatabaseComponent createDatabaseComponent(Database database, @@ -282,11 +284,11 @@ public class DatabaseComponentImplTest extends BrambleMockTestCase { throws Exception { context.checking(new Expectations() {{ // Check whether the contact is in the DB (which it's not) - exactly(18).of(database).startTransaction(); + exactly(17).of(database).startTransaction(); will(returnValue(txn)); - exactly(18).of(database).containsContact(txn, contactId); + exactly(17).of(database).containsContact(txn, contactId); will(returnValue(false)); - exactly(18).of(database).abortTransaction(txn); + exactly(17).of(database).abortTransaction(txn); }}); DatabaseComponent db = createDatabaseComponent(database, eventBus, shutdown); @@ -454,17 +456,6 @@ public class DatabaseComponentImplTest extends BrambleMockTestCase { db.endTransaction(transaction); } - transaction = db.startTransaction(false); - try { - db.setReorderingWindow(transaction, contactId, transportId, 0, 0, - new byte[REORDERING_WINDOW_SIZE / 8]); - fail(); - } catch (NoSuchContactException expected) { - // Expected - } finally { - db.endTransaction(transaction); - } - transaction = db.startTransaction(false); try { db.setGroupVisibility(transaction, contactId, groupId, SHARED); @@ -779,7 +770,7 @@ public class DatabaseComponentImplTest extends BrambleMockTestCase { // Check whether the transport is in the DB (which it's not) exactly(4).of(database).startTransaction(); will(returnValue(txn)); - exactly(2).of(database).containsContact(txn, contactId); + oneOf(database).containsContact(txn, contactId); will(returnValue(true)); exactly(4).of(database).containsTransport(txn, transportId); will(returnValue(false)); @@ -830,7 +821,7 @@ public class DatabaseComponentImplTest extends BrambleMockTestCase { transaction = db.startTransaction(false); try { - db.setReorderingWindow(transaction, contactId, transportId, 0, 0, + db.setReorderingWindow(transaction, keySetId, transportId, 0, 0, new byte[REORDERING_WINDOW_SIZE / 8]); fail(); } catch (NoSuchTransportException expected) { @@ -1303,15 +1294,13 @@ public class DatabaseComponentImplTest extends BrambleMockTestCase { @Test public void testTransportKeys() throws Exception { TransportKeys transportKeys = createTransportKeys(); - Map keys = - singletonMap(contactId, transportKeys); + Collection keys = + singletonList(new KeySet(keySetId, contactId, transportKeys)); context.checking(new Expectations() {{ // startTransaction() oneOf(database).startTransaction(); will(returnValue(txn)); // updateTransportKeys() - oneOf(database).containsContact(txn, contactId); - will(returnValue(true)); oneOf(database).containsTransport(txn, transportId); will(returnValue(true)); oneOf(database).updateTransportKeys(txn, keys); 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 d81cfd851..4237af191 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 @@ -19,6 +19,8 @@ import org.briarproject.bramble.api.sync.MessageStatus; import org.briarproject.bramble.api.sync.ValidationManager.State; import org.briarproject.bramble.api.system.Clock; import org.briarproject.bramble.api.transport.IncomingKeys; +import org.briarproject.bramble.api.transport.KeySet; +import org.briarproject.bramble.api.transport.KeySetId; import org.briarproject.bramble.api.transport.OutgoingKeys; import org.briarproject.bramble.api.transport.TransportKeys; import org.briarproject.bramble.system.SystemClock; @@ -34,7 +36,6 @@ import java.sql.Connection; import java.util.ArrayList; import java.util.Arrays; import java.util.Collection; -import java.util.Collections; import java.util.List; import java.util.Map; import java.util.Map.Entry; @@ -42,6 +43,10 @@ import java.util.Random; import java.util.concurrent.CountDownLatch; import java.util.concurrent.atomic.AtomicBoolean; +import static java.util.Collections.emptyList; +import static java.util.Collections.emptyMap; +import static java.util.Collections.singletonList; +import static java.util.Collections.singletonMap; import static java.util.concurrent.TimeUnit.SECONDS; import static org.briarproject.bramble.api.db.Metadata.REMOVE; import static org.briarproject.bramble.api.sync.Group.Visibility.INVISIBLE; @@ -86,6 +91,7 @@ public abstract class JdbcDatabaseTest extends BrambleTestCase { private final Message message; private final TransportId transportId; private final ContactId contactId; + private final KeySetId keySetId; JdbcDatabaseTest() throws Exception { groupId = new GroupId(getRandomId()); @@ -101,6 +107,7 @@ public abstract class JdbcDatabaseTest extends BrambleTestCase { message = new Message(messageId, groupId, timestamp, raw); transportId = new TransportId("id"); contactId = new ContactId(1); + keySetId = new KeySetId(1); } protected abstract JdbcDatabase createDatabase(DatabaseConfig config, @@ -190,9 +197,9 @@ public abstract class JdbcDatabaseTest extends BrambleTestCase { // The contact has not seen the message, so it should be sendable Collection ids = db.getMessagesToSend(txn, contactId, ONE_MEGABYTE); - assertEquals(Collections.singletonList(messageId), ids); + assertEquals(singletonList(messageId), ids); ids = db.getMessagesToOffer(txn, contactId, 100); - assertEquals(Collections.singletonList(messageId), ids); + assertEquals(singletonList(messageId), ids); // Changing the status to seen = true should make the message unsendable db.raiseSeenFlag(txn, contactId, messageId); @@ -228,9 +235,9 @@ public abstract class JdbcDatabaseTest extends BrambleTestCase { // Marking the message delivered should make it sendable db.setMessageState(txn, messageId, DELIVERED); ids = db.getMessagesToSend(txn, contactId, ONE_MEGABYTE); - assertEquals(Collections.singletonList(messageId), ids); + assertEquals(singletonList(messageId), ids); ids = db.getMessagesToOffer(txn, contactId, 100); - assertEquals(Collections.singletonList(messageId), ids); + assertEquals(singletonList(messageId), ids); // Marking the message invalid should make it unsendable db.setMessageState(txn, messageId, INVALID); @@ -279,9 +286,9 @@ public abstract class JdbcDatabaseTest extends BrambleTestCase { // Sharing the group should make the message sendable db.setGroupVisibility(txn, contactId, groupId, true); ids = db.getMessagesToSend(txn, contactId, ONE_MEGABYTE); - assertEquals(Collections.singletonList(messageId), ids); + assertEquals(singletonList(messageId), ids); ids = db.getMessagesToOffer(txn, contactId, 100); - assertEquals(Collections.singletonList(messageId), ids); + assertEquals(singletonList(messageId), ids); // Unsharing the group should make the message unsendable db.setGroupVisibility(txn, contactId, groupId, false); @@ -324,9 +331,9 @@ public abstract class JdbcDatabaseTest extends BrambleTestCase { // Sharing the message should make it sendable db.setMessageShared(txn, messageId); ids = db.getMessagesToSend(txn, contactId, ONE_MEGABYTE); - assertEquals(Collections.singletonList(messageId), ids); + assertEquals(singletonList(messageId), ids); ids = db.getMessagesToOffer(txn, contactId, 100); - assertEquals(Collections.singletonList(messageId), ids); + assertEquals(singletonList(messageId), ids); db.commitTransaction(txn); db.close(); @@ -352,7 +359,7 @@ public abstract class JdbcDatabaseTest extends BrambleTestCase { // The message is just the right size to send ids = db.getMessagesToSend(txn, contactId, size); - assertEquals(Collections.singletonList(messageId), ids); + assertEquals(singletonList(messageId), ids); db.commitTransaction(txn); db.close(); @@ -384,7 +391,7 @@ public abstract class JdbcDatabaseTest extends BrambleTestCase { db.lowerAckFlag(txn, contactId, Arrays.asList(messageId, messageId1)); // Both message IDs should have been removed - assertEquals(Collections.emptyList(), db.getMessagesToAck(txn, + assertEquals(emptyList(), db.getMessagesToAck(txn, contactId, 1234)); // Raise the ack flag again @@ -415,7 +422,7 @@ public abstract class JdbcDatabaseTest extends BrambleTestCase { // Retrieve the message from the database and mark it as sent Collection ids = db.getMessagesToSend(txn, contactId, ONE_MEGABYTE); - assertEquals(Collections.singletonList(messageId), ids); + assertEquals(singletonList(messageId), ids); db.updateExpiryTime(txn, contactId, messageId, Integer.MAX_VALUE); // The message should no longer be sendable @@ -626,31 +633,31 @@ public abstract class JdbcDatabaseTest extends BrambleTestCase { // The group should not be visible to the contact assertEquals(INVISIBLE, db.getGroupVisibility(txn, contactId, groupId)); - assertEquals(Collections.emptyMap(), + assertEquals(emptyMap(), db.getGroupVisibility(txn, groupId)); // Make the group visible to the contact db.addGroupVisibility(txn, contactId, groupId, false); assertEquals(VISIBLE, db.getGroupVisibility(txn, contactId, groupId)); - assertEquals(Collections.singletonMap(contactId, false), + assertEquals(singletonMap(contactId, false), db.getGroupVisibility(txn, groupId)); // Share the group with the contact db.setGroupVisibility(txn, contactId, groupId, true); assertEquals(SHARED, db.getGroupVisibility(txn, contactId, groupId)); - assertEquals(Collections.singletonMap(contactId, true), + assertEquals(singletonMap(contactId, true), db.getGroupVisibility(txn, groupId)); // Unshare the group with the contact db.setGroupVisibility(txn, contactId, groupId, false); assertEquals(VISIBLE, db.getGroupVisibility(txn, contactId, groupId)); - assertEquals(Collections.singletonMap(contactId, false), + assertEquals(singletonMap(contactId, false), db.getGroupVisibility(txn, groupId)); // Make the group invisible again db.removeGroupVisibility(txn, contactId, groupId); assertEquals(INVISIBLE, db.getGroupVisibility(txn, contactId, groupId)); - assertEquals(Collections.emptyMap(), + assertEquals(emptyMap(), db.getGroupVisibility(txn, groupId)); db.commitTransaction(txn); @@ -665,24 +672,22 @@ public abstract class JdbcDatabaseTest extends BrambleTestCase { Connection txn = db.startTransaction(); // Initially there should be no transport keys in the database - assertEquals(Collections.emptyMap(), - db.getTransportKeys(txn, transportId)); + assertEquals(emptyList(), db.getTransportKeys(txn, transportId)); // Add the contact, the transport and the transport keys db.addLocalAuthor(txn, localAuthor); assertEquals(contactId, db.addContact(txn, author, localAuthor.getId(), true, true)); db.addTransport(txn, transportId, 123); - db.addTransportKeys(txn, contactId, keys); + assertEquals(keySetId, db.addTransportKeys(txn, contactId, keys)); // Retrieve the transport keys - Map newKeys = - db.getTransportKeys(txn, transportId); + Collection newKeys = db.getTransportKeys(txn, transportId); assertEquals(1, newKeys.size()); - Entry e = - newKeys.entrySet().iterator().next(); - assertEquals(contactId, e.getKey()); - TransportKeys k = e.getValue(); + KeySet ks = newKeys.iterator().next(); + assertEquals(keySetId, ks.getKeySetId()); + assertEquals(contactId, ks.getContactId()); + TransportKeys k = ks.getTransportKeys(); assertEquals(transportId, k.getTransportId()); assertKeysEquals(keys.getPreviousIncomingKeys(), k.getPreviousIncomingKeys()); @@ -695,8 +700,7 @@ public abstract class JdbcDatabaseTest extends BrambleTestCase { // Removing the contact should remove the transport keys db.removeContact(txn, contactId); - assertEquals(Collections.emptyMap(), - db.getTransportKeys(txn, transportId)); + assertEquals(emptyList(), db.getTransportKeys(txn, transportId)); db.commitTransaction(txn); db.close(); @@ -735,18 +739,18 @@ public abstract class JdbcDatabaseTest extends BrambleTestCase { assertEquals(contactId, db.addContact(txn, author, localAuthor.getId(), true, true)); db.addTransport(txn, transportId, 123); - db.updateTransportKeys(txn, Collections.singletonMap(contactId, keys)); + db.updateTransportKeys(txn, + singletonList(new KeySet(keySetId, contactId, keys))); // Increment the stream counter twice and retrieve the transport keys db.incrementStreamCounter(txn, contactId, transportId, rotationPeriod); db.incrementStreamCounter(txn, contactId, transportId, rotationPeriod); - Map newKeys = - db.getTransportKeys(txn, transportId); + Collection newKeys = db.getTransportKeys(txn, transportId); assertEquals(1, newKeys.size()); - Entry e = - newKeys.entrySet().iterator().next(); - assertEquals(contactId, e.getKey()); - TransportKeys k = e.getValue(); + KeySet ks = newKeys.iterator().next(); + assertEquals(keySetId, ks.getKeySetId()); + assertEquals(contactId, ks.getContactId()); + TransportKeys k = ks.getTransportKeys(); assertEquals(transportId, k.getTransportId()); OutgoingKeys outCurr = k.getCurrentOutgoingKeys(); assertEquals(rotationPeriod, outCurr.getRotationPeriod()); @@ -771,19 +775,19 @@ public abstract class JdbcDatabaseTest extends BrambleTestCase { assertEquals(contactId, db.addContact(txn, author, localAuthor.getId(), true, true)); db.addTransport(txn, transportId, 123); - db.updateTransportKeys(txn, Collections.singletonMap(contactId, keys)); + db.updateTransportKeys(txn, + singletonList(new KeySet(keySetId, contactId, keys))); // Update the reordering window and retrieve the transport keys new Random().nextBytes(bitmap); - db.setReorderingWindow(txn, contactId, transportId, rotationPeriod, + db.setReorderingWindow(txn, keySetId, transportId, rotationPeriod, base + 1, bitmap); - Map newKeys = - db.getTransportKeys(txn, transportId); + Collection newKeys = db.getTransportKeys(txn, transportId); assertEquals(1, newKeys.size()); - Entry e = - newKeys.entrySet().iterator().next(); - assertEquals(contactId, e.getKey()); - TransportKeys k = e.getValue(); + KeySet ks = newKeys.iterator().next(); + assertEquals(keySetId, ks.getKeySetId()); + assertEquals(contactId, ks.getContactId()); + TransportKeys k = ks.getTransportKeys(); assertEquals(transportId, k.getTransportId()); IncomingKeys inCurr = k.getCurrentIncomingKeys(); assertEquals(rotationPeriod, inCurr.getRotationPeriod()); @@ -830,18 +834,18 @@ public abstract class JdbcDatabaseTest extends BrambleTestCase { db.addLocalAuthor(txn, localAuthor); Collection contacts = db.getContacts(txn, localAuthor.getId()); - assertEquals(Collections.emptyList(), contacts); + assertEquals(emptyList(), contacts); // Add a contact associated with the local author assertEquals(contactId, db.addContact(txn, author, localAuthor.getId(), true, true)); contacts = db.getContacts(txn, localAuthor.getId()); - assertEquals(Collections.singletonList(contactId), contacts); + assertEquals(singletonList(contactId), contacts); // Remove the local author - the contact should be removed db.removeLocalAuthor(txn, localAuthor.getId()); contacts = db.getContacts(txn, localAuthor.getId()); - assertEquals(Collections.emptyList(), contacts); + assertEquals(emptyList(), contacts); assertFalse(db.containsContact(txn, contactId)); db.commitTransaction(txn); @@ -1560,9 +1564,9 @@ public abstract class JdbcDatabaseTest extends BrambleTestCase { // The message should be sendable Collection ids = db.getMessagesToSend(txn, contactId, ONE_MEGABYTE); - assertEquals(Collections.singletonList(messageId), ids); + assertEquals(singletonList(messageId), ids); ids = db.getMessagesToOffer(txn, contactId, 100); - assertEquals(Collections.singletonList(messageId), ids); + assertEquals(singletonList(messageId), ids); // The raw message should not be null assertNotNull(db.getRawMessage(txn, messageId)); 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 4dc1f9802..213400f1e 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 @@ -8,6 +8,8 @@ import org.briarproject.bramble.api.db.Transaction; import org.briarproject.bramble.api.plugin.TransportId; import org.briarproject.bramble.api.system.Clock; import org.briarproject.bramble.api.transport.IncomingKeys; +import org.briarproject.bramble.api.transport.KeySet; +import org.briarproject.bramble.api.transport.KeySetId; import org.briarproject.bramble.api.transport.OutgoingKeys; import org.briarproject.bramble.api.transport.StreamContext; import org.briarproject.bramble.api.transport.TransportKeys; @@ -22,14 +24,13 @@ import org.junit.Test; import java.util.ArrayList; import java.util.Collection; -import java.util.Collections; -import java.util.LinkedHashMap; import java.util.List; -import java.util.Map; import java.util.Random; import java.util.concurrent.Executor; import java.util.concurrent.ScheduledExecutorService; +import static java.util.Arrays.asList; +import static java.util.Collections.singletonList; import static java.util.concurrent.TimeUnit.MILLISECONDS; import static org.briarproject.bramble.api.transport.TransportConstants.MAX_CLOCK_DIFFERENCE; import static org.briarproject.bramble.api.transport.TransportConstants.PROTOCOL_VERSION; @@ -55,6 +56,7 @@ public class TransportKeyManagerImplTest extends BrambleMockTestCase { private final long rotationPeriodLength = maxLatency + MAX_CLOCK_DIFFERENCE; private final ContactId contactId = new ContactId(123); private final ContactId contactId1 = new ContactId(234); + private final KeySetId keySetId = new KeySetId(345); private final SecretKey tagKey = TestUtils.getSecretKey(); private final SecretKey headerKey = TestUtils.getSecretKey(); private final SecretKey masterKey = TestUtils.getSecretKey(); @@ -62,11 +64,12 @@ public class TransportKeyManagerImplTest extends BrambleMockTestCase { @Test public void testKeysAreRotatedAtStartup() throws Exception { - Map loaded = new LinkedHashMap<>(); TransportKeys shouldRotate = createTransportKeys(900, 0); TransportKeys shouldNotRotate = createTransportKeys(1000, 0); - loaded.put(contactId, shouldRotate); - loaded.put(contactId1, shouldNotRotate); + Collection loaded = asList( + new KeySet(keySetId, contactId, shouldRotate), + new KeySet(keySetId, contactId1, shouldNotRotate) + ); TransportKeys rotated = createTransportKeys(1000, 0); Transaction txn = new Transaction(null, false); @@ -91,7 +94,7 @@ public class TransportKeyManagerImplTest extends BrambleMockTestCase { } // Save the keys that were rotated oneOf(db).updateTransportKeys(txn, - Collections.singletonMap(contactId, rotated)); + singletonList(new KeySet(keySetId, contactId, rotated))); // Schedule key rotation at the start of the next rotation period oneOf(scheduler).schedule(with(any(Runnable.class)), with(rotationPeriodLength - 1), with(MILLISECONDS)); @@ -129,6 +132,7 @@ public class TransportKeyManagerImplTest extends BrambleMockTestCase { } // Save the keys oneOf(db).addTransportKeys(txn, contactId, rotated); + will(returnValue(keySetId)); }}); TransportKeyManager transportKeyManager = new TransportKeyManagerImpl( @@ -179,6 +183,7 @@ public class TransportKeyManagerImplTest extends BrambleMockTestCase { will(returnValue(transportKeys)); // Save the keys oneOf(db).addTransportKeys(txn, contactId, transportKeys); + will(returnValue(keySetId)); }}); TransportKeyManager transportKeyManager = new TransportKeyManagerImpl( @@ -218,6 +223,7 @@ public class TransportKeyManagerImplTest extends BrambleMockTestCase { will(returnValue(transportKeys)); // Save the keys oneOf(db).addTransportKeys(txn, contactId, transportKeys); + will(returnValue(keySetId)); // Increment the stream counter oneOf(db).incrementStreamCounter(txn, contactId, transportId, 1000); }}); @@ -268,6 +274,7 @@ public class TransportKeyManagerImplTest extends BrambleMockTestCase { will(returnValue(transportKeys)); // Save the keys oneOf(db).addTransportKeys(txn, contactId, transportKeys); + will(returnValue(keySetId)); }}); TransportKeyManager transportKeyManager = new TransportKeyManagerImpl( @@ -308,13 +315,14 @@ public class TransportKeyManagerImplTest extends BrambleMockTestCase { will(returnValue(transportKeys)); // Save the keys oneOf(db).addTransportKeys(txn, contactId, transportKeys); + will(returnValue(keySetId)); // Encode a new tag after sliding the window oneOf(transportCrypto).encodeTag(with(any(byte[].class)), with(tagKey), with(PROTOCOL_VERSION), with((long) REORDERING_WINDOW_SIZE)); will(new EncodeTagAction(tags)); // Save the reordering window (previous rotation period, base 1) - oneOf(db).setReorderingWindow(txn, contactId, transportId, 999, + oneOf(db).setReorderingWindow(txn, keySetId, transportId, 999, 1, new byte[REORDERING_WINDOW_SIZE / 8]); }}); @@ -345,8 +353,8 @@ public class TransportKeyManagerImplTest extends BrambleMockTestCase { @Test public void testKeysAreRotatedToCurrentPeriod() throws Exception { TransportKeys transportKeys = createTransportKeys(1000, 0); - Map loaded = - Collections.singletonMap(contactId, transportKeys); + Collection loaded = + singletonList(new KeySet(keySetId, contactId, transportKeys)); TransportKeys rotated = createTransportKeys(1001, 0); Transaction txn = new Transaction(null, false); Transaction txn1 = new Transaction(null, false); @@ -393,7 +401,7 @@ public class TransportKeyManagerImplTest extends BrambleMockTestCase { } // Save the keys that were rotated oneOf(db).updateTransportKeys(txn1, - Collections.singletonMap(contactId, rotated)); + singletonList(new KeySet(keySetId, contactId, rotated))); // Schedule key rotation at the start of the next rotation period oneOf(scheduler).schedule(with(any(Runnable.class)), with(rotationPeriodLength), with(MILLISECONDS)); From bb2f94d5eb43b405edf50dcf72f3eab872b1b0ee Mon Sep 17 00:00:00 2001 From: akwizgran Date: Tue, 27 Mar 2018 15:47:01 +0100 Subject: [PATCH 02/11] Add methods for adding unbound keys. --- .../bramble/api/transport/KeyManager.java | 10 ++- .../bramble/contact/ContactManagerImpl.java | 2 +- .../bramble/transport/KeyManagerImpl.java | 7 ++ .../transport/TransportKeyManager.java | 3 + .../transport/TransportKeyManagerImpl.java | 12 ++++ .../bramble/transport/KeyManagerImplTest.java | 15 +++++ .../TransportKeyManagerImplTest.java | 65 ++++++++++++++++--- 7 files changed, 104 insertions(+), 10 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 97afdc133..1e26a7b3d 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 @@ -16,13 +16,21 @@ public interface KeyManager { /** * Informs the key manager that a new contact has been added. Derives and - * stores transport keys for communicating with the contact. + * stores a set of transport keys for communicating with the contact over + * each transport. + *

* {@link StreamContext StreamContexts} for the contact can be created * after this method has returned. */ void addContact(Transaction txn, ContactId c, SecretKey master, long timestamp, boolean alice) throws DbException; + /** + * Derives and stores a set of unbound transport keys for each transport. + */ + void addUnboundKeys(Transaction txn, SecretKey master, long timestamp, + boolean alice) throws DbException; + /** * Returns a {@link StreamContext} for sending a stream to the given * contact over the given transport, or null if an error occurs or the 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 25e0681c9..afac362ce 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 @@ -50,7 +50,7 @@ class ContactManagerImpl implements ContactManager { @Override public ContactId addContact(Transaction txn, Author remote, AuthorId local, - SecretKey master,long timestamp, boolean alice, boolean verified, + SecretKey master, long timestamp, boolean alice, boolean verified, boolean active) throws DbException { ContactId c = db.addContact(txn, remote, local, verified, active); keyManager.addContact(txn, c, master, timestamp, alice); 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 d0c6fd709..be50963e0 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 @@ -104,6 +104,13 @@ class KeyManagerImpl implements KeyManager, Service, EventListener { m.addContact(txn, c, master, timestamp, alice); } + @Override + public void addUnboundKeys(Transaction txn, SecretKey master, + long timestamp, boolean alice) throws DbException { + for (TransportKeyManager m : managers.values()) + m.addUnboundKeys(txn, master, timestamp, alice); + } + @Override public StreamContext getStreamContext(ContactId c, TransportId t) 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 6aa6d360f..8c9b03fd8 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 @@ -17,6 +17,9 @@ interface TransportKeyManager { void addContact(Transaction txn, ContactId c, SecretKey master, long timestamp, boolean alice) throws DbException; + void addUnboundKeys(Transaction txn, SecretKey master, long timestamp, + boolean alice) throws DbException; + void removeContact(ContactId c); @Nullable 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 1220beee5..7b4893e56 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 @@ -172,6 +172,18 @@ class TransportKeyManagerImpl implements TransportKeyManager { @Override public void addContact(Transaction txn, ContactId c, SecretKey master, long timestamp, boolean alice) throws DbException { + deriveAndAddKeys(txn, c, master, timestamp, alice); + } + + @Override + public void addUnboundKeys(Transaction txn, SecretKey master, + long timestamp, boolean alice) throws DbException { + deriveAndAddKeys(txn, null, master, timestamp, alice); + } + + private void deriveAndAddKeys(Transaction txn, @Nullable ContactId c, + SecretKey master, long timestamp, boolean alice) + throws DbException { lock.lock(); try { // Work out what rotation period the timestamp belongs to 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 edf073a4f..7320d60ce 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 @@ -113,6 +113,21 @@ public class KeyManagerImplTest extends BrambleTestCase { context.assertIsSatisfied(); } + @Test + public void testAddUnboundKeys() throws Exception { + SecretKey secretKey = getSecretKey(); + long timestamp = System.currentTimeMillis(); + boolean alice = new Random().nextBoolean(); + + context.checking(new Expectations() {{ + oneOf(transportKeyManager).addUnboundKeys(txn, secretKey, + timestamp, alice); + }}); + + keyManager.addUnboundKeys(txn, secretKey, timestamp, alice); + context.assertIsSatisfied(); + } + @Test public void testGetStreamContextForInactiveContact() throws Exception { assertEquals(null, 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 213400f1e..b926707df 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 @@ -30,7 +30,6 @@ import java.util.concurrent.Executor; import java.util.concurrent.ScheduledExecutorService; import static java.util.Arrays.asList; -import static java.util.Collections.singletonList; import static java.util.concurrent.TimeUnit.MILLISECONDS; import static org.briarproject.bramble.api.transport.TransportConstants.MAX_CLOCK_DIFFERENCE; import static org.briarproject.bramble.api.transport.TransportConstants.PROTOCOL_VERSION; @@ -57,6 +56,8 @@ public class TransportKeyManagerImplTest extends BrambleMockTestCase { private final ContactId contactId = new ContactId(123); private final ContactId contactId1 = new ContactId(234); private final KeySetId keySetId = new KeySetId(345); + private final KeySetId keySetId1 = new KeySetId(456); + private final KeySetId keySetId2 = new KeySetId(567); private final SecretKey tagKey = TestUtils.getSecretKey(); private final SecretKey headerKey = TestUtils.getSecretKey(); private final SecretKey masterKey = TestUtils.getSecretKey(); @@ -66,11 +67,14 @@ public class TransportKeyManagerImplTest extends BrambleMockTestCase { public void testKeysAreRotatedAtStartup() throws Exception { TransportKeys shouldRotate = createTransportKeys(900, 0); TransportKeys shouldNotRotate = createTransportKeys(1000, 0); + TransportKeys shouldRotate1 = createTransportKeys(999, 0); Collection loaded = asList( new KeySet(keySetId, contactId, shouldRotate), - new KeySet(keySetId, contactId1, shouldNotRotate) + new KeySet(keySetId1, contactId1, shouldNotRotate), + new KeySet(keySetId2, null, shouldRotate1) ); TransportKeys rotated = createTransportKeys(1000, 0); + TransportKeys rotated1 = createTransportKeys(1000, 0); Transaction txn = new Transaction(null, false); context.checking(new Expectations() {{ @@ -85,6 +89,8 @@ public class TransportKeyManagerImplTest extends BrambleMockTestCase { will(returnValue(rotated)); oneOf(transportCrypto).rotateTransportKeys(shouldNotRotate, 1000); will(returnValue(shouldNotRotate)); + oneOf(transportCrypto).rotateTransportKeys(shouldRotate1, 1000); + will(returnValue(rotated1)); // Encode the tags (3 sets per contact) for (long i = 0; i < REORDERING_WINDOW_SIZE; i++) { exactly(6).of(transportCrypto).encodeTag( @@ -93,8 +99,10 @@ public class TransportKeyManagerImplTest extends BrambleMockTestCase { will(new EncodeTagAction()); } // Save the keys that were rotated - oneOf(db).updateTransportKeys(txn, - singletonList(new KeySet(keySetId, contactId, rotated))); + oneOf(db).updateTransportKeys(txn, asList( + new KeySet(keySetId, contactId, rotated), + new KeySet(keySetId2, null, rotated1)) + ); // Schedule key rotation at the start of the next rotation period oneOf(scheduler).schedule(with(any(Runnable.class)), with(rotationPeriodLength - 1), with(MILLISECONDS)); @@ -144,6 +152,36 @@ public class TransportKeyManagerImplTest extends BrambleMockTestCase { alice); } + @Test + public void testKeysAreRotatedWhenAddingUnboundKeys() throws Exception { + boolean alice = random.nextBoolean(); + TransportKeys transportKeys = createTransportKeys(999, 0); + TransportKeys rotated = createTransportKeys(1000, 0); + Transaction txn = new Transaction(null, false); + + context.checking(new Expectations() {{ + oneOf(transportCrypto).deriveTransportKeys(transportId, masterKey, + 999, alice); + will(returnValue(transportKeys)); + // Get the current time (1 ms after start of rotation period 1000) + oneOf(clock).currentTimeMillis(); + will(returnValue(rotationPeriodLength * 1000 + 1)); + // Rotate the transport keys + oneOf(transportCrypto).rotateTransportKeys(transportKeys, 1000); + will(returnValue(rotated)); + // Save the keys + oneOf(db).addTransportKeys(txn, null, rotated); + will(returnValue(keySetId)); + }}); + + TransportKeyManager transportKeyManager = new TransportKeyManagerImpl( + db, transportCrypto, dbExecutor, scheduler, clock, transportId, + maxLatency); + // The timestamp is 1 ms before the start of rotation period 1000 + long timestamp = rotationPeriodLength * 1000 - 1; + transportKeyManager.addUnboundKeys(txn, masterKey, timestamp, alice); + } + @Test public void testOutgoingStreamContextIsNullIfContactIsNotFound() throws Exception { @@ -353,9 +391,13 @@ public class TransportKeyManagerImplTest extends BrambleMockTestCase { @Test public void testKeysAreRotatedToCurrentPeriod() throws Exception { TransportKeys transportKeys = createTransportKeys(1000, 0); - Collection loaded = - singletonList(new KeySet(keySetId, contactId, transportKeys)); + TransportKeys transportKeys1 = createTransportKeys(1000, 0); + Collection loaded = asList( + new KeySet(keySetId, contactId, transportKeys), + new KeySet(keySetId1, null, transportKeys1) + ); TransportKeys rotated = createTransportKeys(1001, 0); + TransportKeys rotated1 = createTransportKeys(1001, 0); Transaction txn = new Transaction(null, false); Transaction txn1 = new Transaction(null, false); @@ -369,6 +411,8 @@ public class TransportKeyManagerImplTest extends BrambleMockTestCase { // Rotate the transport keys (the keys are unaffected) oneOf(transportCrypto).rotateTransportKeys(transportKeys, 1000); will(returnValue(transportKeys)); + oneOf(transportCrypto).rotateTransportKeys(transportKeys1, 1000); + will(returnValue(transportKeys1)); // Encode the tags (3 sets) for (long i = 0; i < REORDERING_WINDOW_SIZE; i++) { exactly(3).of(transportCrypto).encodeTag( @@ -392,6 +436,9 @@ public class TransportKeyManagerImplTest extends BrambleMockTestCase { oneOf(transportCrypto).rotateTransportKeys( with(any(TransportKeys.class)), with(1001L)); will(returnValue(rotated)); + oneOf(transportCrypto).rotateTransportKeys( + with(any(TransportKeys.class)), with(1001L)); + will(returnValue(rotated1)); // Encode the tags (3 sets) for (long i = 0; i < REORDERING_WINDOW_SIZE; i++) { exactly(3).of(transportCrypto).encodeTag( @@ -400,8 +447,10 @@ public class TransportKeyManagerImplTest extends BrambleMockTestCase { will(new EncodeTagAction()); } // Save the keys that were rotated - oneOf(db).updateTransportKeys(txn1, - singletonList(new KeySet(keySetId, contactId, rotated))); + oneOf(db).updateTransportKeys(txn1, asList( + new KeySet(keySetId, contactId, rotated), + new KeySet(keySetId1, null, rotated1) + )); // Schedule key rotation at the start of the next rotation period oneOf(scheduler).schedule(with(any(Runnable.class)), with(rotationPeriodLength), with(MILLISECONDS)); From cb8f89db53b76b104932a1f8f936c8d19334302a Mon Sep 17 00:00:00 2001 From: akwizgran Date: Tue, 27 Mar 2018 15:56:35 +0100 Subject: [PATCH 03/11] Add method for adding a contact without transport keys. --- .../bramble/api/contact/ContactManager.java | 12 ++++++++++-- .../bramble/contact/ContactManagerImpl.java | 10 ++++++++++ 2 files changed, 20 insertions(+), 2 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 e6be9a4d2..dd3db0503 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 @@ -23,13 +23,21 @@ public interface ContactManager { void registerRemoveContactHook(RemoveContactHook hook); /** - * Stores a contact within the given transaction associated with the given - * local and remote pseudonyms, and returns an ID for the contact. + * 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. */ ContactId addContact(Transaction txn, Author remote, AuthorId local, SecretKey master, long timestamp, boolean alice, boolean verified, boolean active) throws DbException; + /** + * Stores a contact associated with the given local and remote pseudonyms + * and returns an ID for the contact. + */ + ContactId addContact(Transaction txn, Author remote, AuthorId local, + boolean verified, boolean active) throws DbException; + /** * Stores a contact associated with the given local and remote pseudonyms, * and returns an ID for the contact. 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 afac362ce..025a6d38d 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 @@ -60,6 +60,16 @@ class ContactManagerImpl implements ContactManager { return c; } + @Override + public ContactId addContact(Transaction txn, Author remote, AuthorId local, + boolean verified, boolean active) throws DbException { + ContactId c = db.addContact(txn, remote, local, verified, active); + Contact contact = db.getContact(txn, c); + for (AddContactHook hook : addHooks) + hook.addingContact(txn, contact); + return c; + } + @Override public ContactId addContact(Author remote, AuthorId local, SecretKey master, long timestamp, boolean alice, boolean verified, boolean active) From 5bd2092a03d70bd985d00277b086034e7312dda2 Mon Sep 17 00:00:00 2001 From: akwizgran Date: Tue, 27 Mar 2018 16:11:18 +0100 Subject: [PATCH 04/11] Return key set IDs when adding unbound keys. --- .../bramble/api/transport/KeyManager.java | 9 ++-- .../bramble/transport/KeyManagerImpl.java | 16 ++++--- .../transport/TransportKeyManager.java | 3 +- .../transport/TransportKeyManagerImpl.java | 7 +-- .../bramble/transport/KeyManagerImplTest.java | 43 ++++++++----------- .../TransportKeyManagerImplTest.java | 3 +- 6 files changed, 42 insertions(+), 39 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 1e26a7b3d..ef422d66e 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 @@ -6,6 +6,8 @@ import org.briarproject.bramble.api.db.DbException; import org.briarproject.bramble.api.db.Transaction; import org.briarproject.bramble.api.plugin.TransportId; +import java.util.Collection; + import javax.annotation.Nullable; /** @@ -26,10 +28,11 @@ public interface KeyManager { long timestamp, boolean alice) throws DbException; /** - * Derives and stores a set of unbound transport keys for each transport. + * Derives and stores a set of unbound transport keys for each transport + * and returns the key set IDs. */ - void addUnboundKeys(Transaction txn, SecretKey master, long timestamp, - boolean alice) throws DbException; + Collection addUnboundKeys(Transaction txn, SecretKey master, + long timestamp, boolean alice) throws DbException; /** * Returns a {@link StreamContext} for sending a stream to the given 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 be50963e0..993cb5c4c 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 @@ -19,8 +19,11 @@ import org.briarproject.bramble.api.plugin.TransportId; import org.briarproject.bramble.api.plugin.duplex.DuplexPluginFactory; import org.briarproject.bramble.api.plugin.simplex.SimplexPluginFactory; import org.briarproject.bramble.api.transport.KeyManager; +import org.briarproject.bramble.api.transport.KeySetId; import org.briarproject.bramble.api.transport.StreamContext; +import java.util.ArrayList; +import java.util.Collection; import java.util.HashMap; import java.util.Map; import java.util.Map.Entry; @@ -105,10 +108,13 @@ class KeyManagerImpl implements KeyManager, Service, EventListener { } @Override - public void addUnboundKeys(Transaction txn, SecretKey master, - long timestamp, boolean alice) throws DbException { + public Collection addUnboundKeys(Transaction txn, + SecretKey master, long timestamp, boolean alice) + throws DbException { + Collection ids = new ArrayList<>(); for (TransportKeyManager m : managers.values()) - m.addUnboundKeys(txn, master, timestamp, alice); + ids.add(m.addUnboundKeys(txn, master, timestamp, alice)); + return ids; } @Override @@ -121,7 +127,7 @@ class KeyManagerImpl implements KeyManager, Service, EventListener { if (LOG.isLoggable(INFO)) LOG.info("No key manager for " + t); return null; } - StreamContext ctx = null; + StreamContext ctx; Transaction txn = db.startTransaction(false); try { ctx = m.getStreamContext(txn, c); @@ -140,7 +146,7 @@ class KeyManagerImpl implements KeyManager, Service, EventListener { if (LOG.isLoggable(INFO)) LOG.info("No key manager for " + t); return null; } - StreamContext ctx = null; + StreamContext ctx; Transaction txn = db.startTransaction(false); try { ctx = m.getStreamContext(txn, tag); 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 8c9b03fd8..5da58633f 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 @@ -5,6 +5,7 @@ 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.nullsafety.NotNullByDefault; +import org.briarproject.bramble.api.transport.KeySetId; import org.briarproject.bramble.api.transport.StreamContext; import javax.annotation.Nullable; @@ -17,7 +18,7 @@ interface TransportKeyManager { void addContact(Transaction txn, ContactId c, SecretKey master, long timestamp, boolean alice) throws DbException; - void addUnboundKeys(Transaction txn, SecretKey master, long timestamp, + KeySetId addUnboundKeys(Transaction txn, SecretKey master, long timestamp, boolean alice) 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 7b4893e56..b1dbb6ced 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 @@ -176,12 +176,12 @@ class TransportKeyManagerImpl implements TransportKeyManager { } @Override - public void addUnboundKeys(Transaction txn, SecretKey master, + public KeySetId addUnboundKeys(Transaction txn, SecretKey master, long timestamp, boolean alice) throws DbException { - deriveAndAddKeys(txn, null, master, timestamp, alice); + return deriveAndAddKeys(txn, null, master, timestamp, alice); } - private void deriveAndAddKeys(Transaction txn, @Nullable ContactId c, + private KeySetId deriveAndAddKeys(Transaction txn, @Nullable ContactId c, SecretKey master, long timestamp, boolean alice) throws DbException { lock.lock(); @@ -198,6 +198,7 @@ class TransportKeyManagerImpl implements TransportKeyManager { KeySetId keySetId = db.addTransportKeys(txn, c, k); // Initialise mutable state for the contact addKeys(keySetId, c, new MutableTransportKeys(k)); + return keySetId; } finally { lock.unlock(); } 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 7320d60ce..a55a92bf7 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 @@ -12,19 +12,19 @@ import org.briarproject.bramble.api.identity.AuthorId; import org.briarproject.bramble.api.plugin.PluginConfig; import org.briarproject.bramble.api.plugin.TransportId; import org.briarproject.bramble.api.plugin.simplex.SimplexPluginFactory; +import org.briarproject.bramble.api.transport.KeySetId; import org.briarproject.bramble.api.transport.StreamContext; -import org.briarproject.bramble.test.BrambleTestCase; +import org.briarproject.bramble.test.BrambleMockTestCase; import org.jmock.Expectations; -import org.jmock.Mockery; import org.jmock.lib.concurrent.DeterministicExecutor; import org.junit.Before; import org.junit.Test; import java.util.ArrayList; import java.util.Collection; -import java.util.Collections; import java.util.Random; +import static java.util.Collections.singletonList; import static org.briarproject.bramble.api.transport.TransportConstants.TAG_LENGTH; import static org.briarproject.bramble.test.TestUtils.getAuthor; import static org.briarproject.bramble.test.TestUtils.getRandomBytes; @@ -32,31 +32,29 @@ import static org.briarproject.bramble.test.TestUtils.getRandomId; import static org.briarproject.bramble.test.TestUtils.getSecretKey; import static org.junit.Assert.assertEquals; -public class KeyManagerImplTest extends BrambleTestCase { +public class KeyManagerImplTest extends BrambleMockTestCase { - private final Mockery context = new Mockery(); - private final KeyManagerImpl keyManager; private final DatabaseComponent db = context.mock(DatabaseComponent.class); private final PluginConfig pluginConfig = context.mock(PluginConfig.class); private final TransportKeyManagerFactory transportKeyManagerFactory = context.mock(TransportKeyManagerFactory.class); private final TransportKeyManager transportKeyManager = context.mock(TransportKeyManager.class); + private final DeterministicExecutor executor = new DeterministicExecutor(); private final Transaction txn = new Transaction(null, false); - private final ContactId contactId = new ContactId(42); - private final ContactId inactiveContactId = new ContactId(43); - private final TransportId transportId = new TransportId("tId"); - private final TransportId unknownTransportId = new TransportId("id"); + private final ContactId contactId = new ContactId(123); + private final ContactId inactiveContactId = new ContactId(234); + private final KeySetId keySetId = new KeySetId(345); + private final TransportId transportId = new TransportId("known"); + private final TransportId unknownTransportId = new TransportId("unknown"); private final StreamContext streamContext = new StreamContext(contactId, transportId, getSecretKey(), getSecretKey(), 1); private final byte[] tag = getRandomBytes(TAG_LENGTH); - public KeyManagerImplTest() { - keyManager = new KeyManagerImpl(db, executor, pluginConfig, - transportKeyManagerFactory); - } + private final KeyManagerImpl keyManager = new KeyManagerImpl(db, executor, + pluginConfig, transportKeyManagerFactory); @Before public void testStartService() throws Exception { @@ -70,8 +68,8 @@ public class KeyManagerImplTest extends BrambleTestCase { true, false)); SimplexPluginFactory pluginFactory = context.mock(SimplexPluginFactory.class); - Collection factories = Collections - .singletonList(pluginFactory); + Collection factories = + singletonList(pluginFactory); int maxLatency = 1337; context.checking(new Expectations() {{ @@ -110,7 +108,6 @@ public class KeyManagerImplTest extends BrambleTestCase { }}); keyManager.addContact(txn, contactId, secretKey, timestamp, alice); - context.assertIsSatisfied(); } @Test @@ -122,10 +119,11 @@ public class KeyManagerImplTest extends BrambleTestCase { context.checking(new Expectations() {{ oneOf(transportKeyManager).addUnboundKeys(txn, secretKey, timestamp, alice); + will(returnValue(keySetId)); }}); - keyManager.addUnboundKeys(txn, secretKey, timestamp, alice); - context.assertIsSatisfied(); + assertEquals(singletonList(keySetId), + keyManager.addUnboundKeys(txn, secretKey, timestamp, alice)); } @Test @@ -153,7 +151,6 @@ public class KeyManagerImplTest extends BrambleTestCase { assertEquals(streamContext, keyManager.getStreamContext(contactId, transportId)); - context.assertIsSatisfied(); } @Test @@ -176,7 +173,6 @@ public class KeyManagerImplTest extends BrambleTestCase { assertEquals(streamContext, keyManager.getStreamContext(transportId, tag)); - context.assertIsSatisfied(); } @Test @@ -190,8 +186,6 @@ public class KeyManagerImplTest extends BrambleTestCase { keyManager.eventOccurred(event); executor.runUntilIdle(); assertEquals(null, keyManager.getStreamContext(contactId, transportId)); - - context.assertIsSatisfied(); } @Test @@ -211,8 +205,5 @@ public class KeyManagerImplTest extends BrambleTestCase { keyManager.eventOccurred(event); assertEquals(streamContext, keyManager.getStreamContext(inactiveContactId, transportId)); - - context.assertIsSatisfied(); } - } 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 b926707df..4a92d4eb3 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 @@ -179,7 +179,8 @@ public class TransportKeyManagerImplTest extends BrambleMockTestCase { maxLatency); // The timestamp is 1 ms before the start of rotation period 1000 long timestamp = rotationPeriodLength * 1000 - 1; - transportKeyManager.addUnboundKeys(txn, masterKey, timestamp, alice); + assertEquals(keySetId, transportKeyManager.addUnboundKeys(txn, + masterKey, timestamp, alice)); } @Test From 17fe358fd9efaaa05bd7292007e950390a64092b Mon Sep 17 00:00:00 2001 From: akwizgran Date: Tue, 27 Mar 2018 17:01:39 +0100 Subject: [PATCH 05/11] Add a method for binding transport keys to a contact. --- .../bramble/api/db/DatabaseComponent.java | 6 +++ .../bramble/api/transport/KeyManager.java | 10 ++++- .../org/briarproject/bramble/db/Database.java | 6 +++ .../bramble/db/DatabaseComponentImpl.java | 12 ++++++ .../briarproject/bramble/db/JdbcDatabase.java | 28 +++++++++++++ .../bramble/transport/KeyManagerImpl.java | 27 ++++++++++--- .../transport/TransportKeyManager.java | 2 + .../transport/TransportKeyManagerImpl.java | 39 ++++++++++++------- .../bramble/db/DatabaseComponentImplTest.java | 34 ++++++++++++---- .../bramble/transport/KeyManagerImplTest.java | 3 +- .../TransportKeyManagerImplTest.java | 4 +- 11 files changed, 139 insertions(+), 32 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 45f721f22..2a4f262de 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 @@ -110,6 +110,12 @@ public interface DatabaseComponent { KeySetId addTransportKeys(Transaction txn, @Nullable ContactId c, TransportKeys k) throws DbException; + /** + * Binds the given keys for the given transport to the given contact. + */ + void bindTransportKeys(Transaction txn, ContactId c, TransportId t, + KeySetId k) throws DbException; + /** * Returns true if the database contains the given contact for the given * local pseudonym. 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 ef422d66e..50a5cf948 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 @@ -6,7 +6,7 @@ import org.briarproject.bramble.api.db.DbException; import org.briarproject.bramble.api.db.Transaction; import org.briarproject.bramble.api.plugin.TransportId; -import java.util.Collection; +import java.util.Map; import javax.annotation.Nullable; @@ -31,9 +31,15 @@ public interface KeyManager { * Derives and stores a set of unbound transport keys for each transport * and returns the key set IDs. */ - Collection addUnboundKeys(Transaction txn, SecretKey master, + Map addUnboundKeys(Transaction txn, SecretKey master, long timestamp, boolean alice) throws DbException; + /** + * Binds the given transport keys to the given contact. + */ + void bindKeys(Transaction txn, ContactId c, Map keys) + throws DbException; + /** * Returns a {@link StreamContext} for sending a stream to the given * contact over the given transport, or null if an error occurs or the 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 e8597eaba..b11e1b828 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 @@ -131,6 +131,12 @@ interface Database { KeySetId addTransportKeys(T txn, @Nullable ContactId c, TransportKeys k) throws DbException; + /** + * Binds the given keys for the given transport to the given contact. + */ + void bindTransportKeys(T txn, ContactId c, TransportId t, KeySetId k) + throws DbException; + /** * Returns true if the database contains the given contact for the given * local pseudonym. 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 f90f9e3ff..305b67290 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 @@ -245,6 +245,18 @@ class DatabaseComponentImpl implements DatabaseComponent { return db.addTransportKeys(txn, c, k); } + @Override + public void bindTransportKeys(Transaction transaction, ContactId c, + TransportId t, KeySetId k) throws DbException { + if (transaction.isReadOnly()) throw new IllegalArgumentException(); + T txn = unbox(transaction); + if (!db.containsContact(txn, c)) + throw new NoSuchContactException(); + if (!db.containsTransport(txn, t)) + throw new NoSuchTransportException(); + db.bindTransportKeys(txn, c, t, k); + } + @Override public boolean containsContact(Transaction transaction, AuthorId remote, AuthorId local) throws DbException { 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 327203d63..747145a34 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 @@ -950,6 +950,33 @@ abstract class JdbcDatabase implements Database { } } + @Override + public void bindTransportKeys(Connection txn, ContactId c, TransportId t, + KeySetId k) throws DbException { + PreparedStatement ps = null; + try { + String sql = "UPDATE outgoingKeys SET contactId = ?" + + " WHERE keySetId = ?"; + ps = txn.prepareStatement(sql); + ps.setInt(1, c.getInt()); + ps.setInt(2, k.getInt()); + int affected = ps.executeUpdate(); + if (affected < 0) throw new DbStateException(); + ps.close(); + sql = "UPDATE incomingKeys SET contactId = ?" + + " WHERE keySetId = ?"; + ps = txn.prepareStatement(sql); + ps.setInt(1, c.getInt()); + ps.setInt(2, k.getInt()); + affected = ps.executeUpdate(); + if (affected < 0) throw new DbStateException(); + ps.close(); + } catch (SQLException e) { + tryToClose(ps); + throw new DbException(e); + } + } + @Override public boolean containsContact(Connection txn, AuthorId remote, AuthorId local) throws DbException { @@ -2882,6 +2909,7 @@ abstract class JdbcDatabase implements Database { try { // Delete any existing outgoing keys - this will also remove any // incoming keys with the same key set ID + // TODO: Add an index to speed this up? String sql = "DELETE FROM outgoingKeys WHERE keySetId = ?"; ps = txn.prepareStatement(sql); for (KeySet ks : keys) { 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 993cb5c4c..c07c9e1e7 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 @@ -22,8 +22,6 @@ import org.briarproject.bramble.api.transport.KeyManager; import org.briarproject.bramble.api.transport.KeySetId; import org.briarproject.bramble.api.transport.StreamContext; -import java.util.ArrayList; -import java.util.Collection; import java.util.HashMap; import java.util.Map; import java.util.Map.Entry; @@ -108,15 +106,32 @@ class KeyManagerImpl implements KeyManager, Service, EventListener { } @Override - public Collection addUnboundKeys(Transaction txn, + public Map addUnboundKeys(Transaction txn, SecretKey master, long timestamp, boolean alice) throws DbException { - Collection ids = new ArrayList<>(); - for (TransportKeyManager m : managers.values()) - ids.add(m.addUnboundKeys(txn, master, timestamp, alice)); + Map ids = new HashMap<>(); + for (Entry e : managers.entrySet()) { + TransportId t = e.getKey(); + TransportKeyManager m = e.getValue(); + ids.put(t, m.addUnboundKeys(txn, master, timestamp, alice)); + } return ids; } + @Override + public void bindKeys(Transaction txn, ContactId c, + 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 { + m.bindKeys(txn, c, e.getValue()); + } + } + } + @Override public StreamContext getStreamContext(ContactId c, TransportId t) 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 5da58633f..41e31fbe6 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 @@ -21,6 +21,8 @@ interface TransportKeyManager { KeySetId addUnboundKeys(Transaction txn, SecretKey master, long timestamp, boolean alice) throws DbException; + void bindKeys(Transaction txn, ContactId c, KeySetId k) throws DbException; + void removeContact(ContactId c); @Nullable 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 b1dbb6ced..6b9a1519e 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 @@ -22,7 +22,6 @@ import java.util.Collection; import java.util.HashMap; import java.util.Iterator; import java.util.Map; -import java.util.Map.Entry; import java.util.concurrent.Executor; import java.util.concurrent.ScheduledExecutorService; import java.util.concurrent.atomic.AtomicBoolean; @@ -57,7 +56,7 @@ class TransportKeyManagerImpl implements TransportKeyManager { private final ReentrantLock lock = new ReentrantLock(); // The following are locking: lock - private final Collection keys = new ArrayList<>(); + private final Map keys = new HashMap<>(); private final Map inContexts = new HashMap<>(); private final Map outContexts = new HashMap<>(); @@ -123,7 +122,7 @@ class TransportKeyManagerImpl implements TransportKeyManager { private void addKeys(KeySetId keySetId, @Nullable ContactId contactId, MutableTransportKeys m) { MutableKeySet ks = new MutableKeySet(keySetId, contactId, m); - keys.add(ks); + keys.put(keySetId, ks); if (contactId != null) { encodeTags(keySetId, contactId, m.getPreviousIncomingKeys()); encodeTags(keySetId, contactId, m.getCurrentIncomingKeys()); @@ -204,22 +203,34 @@ class TransportKeyManagerImpl implements TransportKeyManager { } } + @Override + public void bindKeys(Transaction txn, ContactId c, KeySetId k) + throws DbException { + lock.lock(); + try { + MutableKeySet ks = keys.get(k); + if (ks == null) throw new IllegalArgumentException(); + if (ks.getContactId() != null) throw new IllegalArgumentException(); + MutableTransportKeys m = ks.getTransportKeys(); + addKeys(k, c, m); + db.bindTransportKeys(txn, c, m.getTransportId(), k); + } finally { + lock.unlock(); + } + } + @Override public void removeContact(ContactId c) { lock.lock(); try { // Remove mutable state for the contact - Iterator> inContextsIter = - inContexts.entrySet().iterator(); - while (inContextsIter.hasNext()) { - ContactId c1 = inContextsIter.next().getValue().contactId; - if (c1.equals(c)) inContextsIter.remove(); - } + Iterator it = inContexts.values().iterator(); + while (it.hasNext()) if (it.next().contactId.equals(c)) it.remove(); outContexts.remove(c); - Iterator keysIter = keys.iterator(); - while (keysIter.hasNext()) { - ContactId c1 = keysIter.next().getContactId(); - if (c1 != null && c1.equals(c)) keysIter.remove(); + Iterator it1 = keys.values().iterator(); + while (it1.hasNext()) { + ContactId c1 = it1.next().getContactId(); + if (c1 != null && c1.equals(c)) it1.remove(); } } finally { lock.unlock(); @@ -300,7 +311,7 @@ class TransportKeyManagerImpl implements TransportKeyManager { try { // Rotate the keys to the current rotation period Collection snapshot = new ArrayList<>(keys.size()); - for (MutableKeySet ks : keys) { + for (MutableKeySet ks : keys.values()) { snapshot.add(new KeySet(ks.getKeySetId(), ks.getContactId(), ks.getTransportKeys().snapshot())); } 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 eb31247ce..4832d9d8b 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 @@ -284,11 +284,11 @@ public class DatabaseComponentImplTest extends BrambleMockTestCase { throws Exception { context.checking(new Expectations() {{ // Check whether the contact is in the DB (which it's not) - exactly(17).of(database).startTransaction(); + exactly(18).of(database).startTransaction(); will(returnValue(txn)); - exactly(17).of(database).containsContact(txn, contactId); + exactly(18).of(database).containsContact(txn, contactId); will(returnValue(false)); - exactly(17).of(database).abortTransaction(txn); + exactly(18).of(database).abortTransaction(txn); }}); DatabaseComponent db = createDatabaseComponent(database, eventBus, shutdown); @@ -303,6 +303,16 @@ public class DatabaseComponentImplTest extends BrambleMockTestCase { db.endTransaction(transaction); } + transaction = db.startTransaction(false); + try { + db.bindTransportKeys(transaction, contactId, transportId, keySetId); + fail(); + } catch (NoSuchContactException expected) { + // Expected + } finally { + db.endTransaction(transaction); + } + transaction = db.startTransaction(false); try { db.generateAck(transaction, contactId, 123); @@ -768,13 +778,13 @@ public class DatabaseComponentImplTest extends BrambleMockTestCase { // endTransaction() oneOf(database).commitTransaction(txn); // Check whether the transport is in the DB (which it's not) - exactly(4).of(database).startTransaction(); + exactly(5).of(database).startTransaction(); will(returnValue(txn)); - oneOf(database).containsContact(txn, contactId); + exactly(2).of(database).containsContact(txn, contactId); will(returnValue(true)); - exactly(4).of(database).containsTransport(txn, transportId); + exactly(5).of(database).containsTransport(txn, transportId); will(returnValue(false)); - exactly(4).of(database).abortTransaction(txn); + exactly(5).of(database).abortTransaction(txn); }}); DatabaseComponent db = createDatabaseComponent(database, eventBus, shutdown); @@ -789,6 +799,16 @@ public class DatabaseComponentImplTest extends BrambleMockTestCase { db.endTransaction(transaction); } + transaction = db.startTransaction(false); + try { + db.bindTransportKeys(transaction, contactId, transportId, keySetId); + fail(); + } catch (NoSuchTransportException expected) { + // Expected + } finally { + db.endTransaction(transaction); + } + transaction = db.startTransaction(false); try { db.getTransportKeys(transaction, transportId); 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 a55a92bf7..ed7a357ed 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 @@ -25,6 +25,7 @@ import java.util.Collection; 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.getAuthor; import static org.briarproject.bramble.test.TestUtils.getRandomBytes; @@ -122,7 +123,7 @@ public class KeyManagerImplTest extends BrambleMockTestCase { will(returnValue(keySetId)); }}); - assertEquals(singletonList(keySetId), + assertEquals(singletonMap(transportId, keySetId), keyManager.addUnboundKeys(txn, secretKey, timestamp, alice)); } 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 4a92d4eb3..a57ce4e74 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 @@ -449,8 +449,8 @@ public class TransportKeyManagerImplTest extends BrambleMockTestCase { } // Save the keys that were rotated oneOf(db).updateTransportKeys(txn1, asList( - new KeySet(keySetId, contactId, rotated), - new KeySet(keySetId1, null, rotated1) + new KeySet(keySetId1, null, rotated1), + new KeySet(keySetId, contactId, rotated) )); // Schedule key rotation at the start of the next rotation period oneOf(scheduler).schedule(with(any(Runnable.class)), From 0a802bbe0b4035accdb9d5e5bdeff275996ce7af Mon Sep 17 00:00:00 2001 From: akwizgran Date: Tue, 27 Mar 2018 17:30:25 +0100 Subject: [PATCH 06/11] Add a method for removing unbound transport keys. --- .../bramble/api/db/DatabaseComponent.java | 6 +++ .../bramble/api/transport/KeyManager.java | 7 +++ .../org/briarproject/bramble/db/Database.java | 6 +++ .../bramble/db/DatabaseComponentImpl.java | 10 ++++ .../briarproject/bramble/db/JdbcDatabase.java | 47 ++++++++++--------- .../bramble/transport/KeyManagerImpl.java | 14 ++++++ .../transport/TransportKeyManager.java | 2 + .../transport/TransportKeyManagerImpl.java | 14 ++++++ 8 files changed, 83 insertions(+), 23 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 2a4f262de..fcf5efe46 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 @@ -481,6 +481,12 @@ public interface DatabaseComponent { */ void removeTransport(Transaction txn, TransportId t) throws DbException; + /** + * Removes the given transport keys from the database. + */ + void removeTransportKeys(Transaction txn, TransportId t, KeySetId k) + throws DbException; + /** * Marks the given contact as verified. */ 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 50a5cf948..06b2f4725 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 @@ -40,6 +40,13 @@ public interface KeyManager { void bindKeys(Transaction txn, ContactId c, Map keys) throws DbException; + /** + * Removes the given transport keys, which must not have been bound, from + * the manager and the database. + */ + void removeKeys(Transaction txn, Map keys) + throws DbException; + /** * Returns a {@link StreamContext} for sending a stream to the given * contact over the given transport, or null if an error occurs or the 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 b11e1b828..6ee996b2b 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 @@ -593,6 +593,12 @@ interface Database { */ void removeTransport(T txn, TransportId t) throws DbException; + /** + * Removes the given transport keys from the database. + */ + void removeTransportKeys(T txn, TransportId t, KeySetId k) + throws DbException; + /** * Resets the transmission count and expiry time of the given message with * respect to the given contact. 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 305b67290..697bcdd3d 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 @@ -791,6 +791,16 @@ class DatabaseComponentImpl implements DatabaseComponent { db.removeTransport(txn, t); } + @Override + public void removeTransportKeys(Transaction transaction, + TransportId t, KeySetId k) throws DbException { + if (transaction.isReadOnly()) throw new IllegalArgumentException(); + T txn = unbox(transaction); + if (!db.containsTransport(txn, t)) + throw new NoSuchTransportException(); + db.removeTransportKeys(txn, t, k); + } + @Override public void setContactVerified(Transaction transaction, ContactId c) throws DbException { 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 747145a34..25cfb4d5a 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 @@ -2681,6 +2681,27 @@ abstract class JdbcDatabase implements Database { } } + @Override + public void removeTransportKeys(Connection txn, TransportId t, KeySetId k) + throws DbException { + PreparedStatement ps = null; + try { + // Delete any existing outgoing keys - this will also remove any + // incoming keys with the same key set ID + String sql = "DELETE FROM outgoingKeys" + + " WHERE transportId = ? AND keySetId = ?"; + ps = txn.prepareStatement(sql); + ps.setString(1, t.getString()); + ps.setInt(2, k.getInt()); + int affected = ps.executeUpdate(); + if (affected < 0) throw new DbStateException(); + ps.close(); + } catch (SQLException e) { + tryToClose(ps); + throw new DbException(e); + } + } + @Override public void resetExpiryTime(Connection txn, ContactId c, MessageId m) throws DbException { @@ -2905,30 +2926,10 @@ abstract class JdbcDatabase implements Database { @Override public void updateTransportKeys(Connection txn, Collection keys) throws DbException { - PreparedStatement ps = null; - try { - // Delete any existing outgoing keys - this will also remove any - // incoming keys with the same key set ID - // TODO: Add an index to speed this up? - String sql = "DELETE FROM outgoingKeys WHERE keySetId = ?"; - ps = txn.prepareStatement(sql); - for (KeySet ks : keys) { - ps.setInt(1, ks.getKeySetId().getInt()); - ps.addBatch(); - } - int[] batchAffected = ps.executeBatch(); - if (batchAffected.length != keys.size()) - throw new DbStateException(); - for (int rows: batchAffected) - if (rows < 0) throw new DbStateException(); - ps.close(); - } catch (SQLException e) { - tryToClose(ps); - throw new DbException(e); - } - // Store the new keys for (KeySet ks : keys) { - addTransportKeys(txn, ks.getContactId(), ks.getTransportKeys()); + TransportKeys k = ks.getTransportKeys(); + removeTransportKeys(txn, k.getTransportId(), ks.getKeySetId()); + addTransportKeys(txn, ks.getContactId(), k); } } } 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 c07c9e1e7..1a06ef4a6 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 @@ -132,6 +132,20 @@ class KeyManagerImpl implements KeyManager, Service, EventListener { } } + @Override + public void removeKeys(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 { + m.removeKeys(txn, e.getValue()); + } + } + } + @Override public StreamContext getStreamContext(ContactId c, TransportId t) 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 41e31fbe6..4eff0b289 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 @@ -23,6 +23,8 @@ interface TransportKeyManager { void bindKeys(Transaction txn, ContactId c, KeySetId k) throws DbException; + void removeKeys(Transaction txn, KeySetId k) throws DbException; + void removeContact(ContactId c); @Nullable 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 6b9a1519e..889485d0d 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 @@ -219,6 +219,20 @@ class TransportKeyManagerImpl implements TransportKeyManager { } } + @Override + public void removeKeys(Transaction txn, KeySetId k) throws DbException { + lock.lock(); + try { + MutableKeySet ks = keys.remove(k); + if (ks == null) throw new IllegalArgumentException(); + if (ks.getContactId() != null) throw new IllegalArgumentException(); + TransportId t = ks.getTransportKeys().getTransportId(); + db.removeTransportKeys(txn, t, k); + } finally { + lock.unlock(); + } + } + @Override public void removeContact(ContactId c) { lock.lock(); From 57e6f2ea9c5ff392fd34559f8b08416292b7439f Mon Sep 17 00:00:00 2001 From: akwizgran Date: Wed, 28 Mar 2018 10:51:30 +0100 Subject: [PATCH 07/11] Unit tests for removing unbound keys. --- .../bramble/db/DatabaseComponentImplTest.java | 16 +- .../bramble/db/JdbcDatabaseTest.java | 113 ++++++++++-- .../TransportKeyManagerImplTest.java | 164 +++++++++++------- 3 files changed, 213 insertions(+), 80 deletions(-) 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 4832d9d8b..61a499162 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 @@ -778,13 +778,13 @@ public class DatabaseComponentImplTest extends BrambleMockTestCase { // endTransaction() oneOf(database).commitTransaction(txn); // Check whether the transport is in the DB (which it's not) - exactly(5).of(database).startTransaction(); + exactly(6).of(database).startTransaction(); will(returnValue(txn)); exactly(2).of(database).containsContact(txn, contactId); will(returnValue(true)); - exactly(5).of(database).containsTransport(txn, transportId); + exactly(6).of(database).containsTransport(txn, transportId); will(returnValue(false)); - exactly(5).of(database).abortTransaction(txn); + exactly(6).of(database).abortTransaction(txn); }}); DatabaseComponent db = createDatabaseComponent(database, eventBus, shutdown); @@ -839,6 +839,16 @@ public class DatabaseComponentImplTest extends BrambleMockTestCase { db.endTransaction(transaction); } + transaction = db.startTransaction(false); + try { + db.removeTransportKeys(transaction, transportId, keySetId); + fail(); + } catch (NoSuchTransportException expected) { + // Expected + } finally { + db.endTransaction(transaction); + } + transaction = db.startTransaction(false); try { db.setReorderingWindow(transaction, keySetId, transportId, 0, 0, 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 4237af191..6babc6459 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 @@ -91,7 +91,7 @@ public abstract class JdbcDatabaseTest extends BrambleTestCase { private final Message message; private final TransportId transportId; private final ContactId contactId; - private final KeySetId keySetId; + private final KeySetId keySetId, keySetId1; JdbcDatabaseTest() throws Exception { groupId = new GroupId(getRandomId()); @@ -108,6 +108,7 @@ public abstract class JdbcDatabaseTest extends BrambleTestCase { transportId = new TransportId("id"); contactId = new ContactId(1); keySetId = new KeySetId(1); + keySetId1 = new KeySetId(2); } protected abstract JdbcDatabase createDatabase(DatabaseConfig config, @@ -667,6 +668,7 @@ public abstract class JdbcDatabaseTest extends BrambleTestCase { @Test public void testTransportKeys() throws Exception { TransportKeys keys = createTransportKeys(); + TransportKeys keys1 = createTransportKeys(); Database db = open(false); Connection txn = db.startTransaction(); @@ -680,23 +682,20 @@ public abstract class JdbcDatabaseTest extends BrambleTestCase { true, true)); db.addTransport(txn, transportId, 123); assertEquals(keySetId, db.addTransportKeys(txn, contactId, keys)); + assertEquals(keySetId1, db.addTransportKeys(txn, contactId, keys1)); // Retrieve the transport keys - Collection newKeys = db.getTransportKeys(txn, transportId); - assertEquals(1, newKeys.size()); - KeySet ks = newKeys.iterator().next(); - assertEquals(keySetId, ks.getKeySetId()); - assertEquals(contactId, ks.getContactId()); - TransportKeys k = ks.getTransportKeys(); - assertEquals(transportId, k.getTransportId()); - assertKeysEquals(keys.getPreviousIncomingKeys(), - k.getPreviousIncomingKeys()); - assertKeysEquals(keys.getCurrentIncomingKeys(), - k.getCurrentIncomingKeys()); - assertKeysEquals(keys.getNextIncomingKeys(), - k.getNextIncomingKeys()); - assertKeysEquals(keys.getCurrentOutgoingKeys(), - k.getCurrentOutgoingKeys()); + Collection allKeys = db.getTransportKeys(txn, transportId); + assertEquals(2, allKeys.size()); + for (KeySet ks : allKeys) { + assertEquals(contactId, ks.getContactId()); + if (ks.getKeySetId().equals(keySetId)) { + assertKeysEquals(keys, ks.getTransportKeys()); + } else { + assertEquals(keySetId1, ks.getKeySetId()); + assertKeysEquals(keys1, ks.getTransportKeys()); + } + } // Removing the contact should remove the transport keys db.removeContact(txn, contactId); @@ -706,6 +705,88 @@ public abstract class JdbcDatabaseTest extends BrambleTestCase { db.close(); } + @Test + public void testUnboundTransportKeys() throws Exception { + TransportKeys keys = createTransportKeys(); + TransportKeys keys1 = createTransportKeys(); + + Database db = open(false); + Connection txn = db.startTransaction(); + + // Initially there should be no transport keys in the database + assertEquals(emptyList(), db.getTransportKeys(txn, transportId)); + + // Add the contact, the transport and the unbound transport keys + db.addLocalAuthor(txn, localAuthor); + assertEquals(contactId, db.addContact(txn, author, localAuthor.getId(), + true, true)); + db.addTransport(txn, transportId, 123); + assertEquals(keySetId, db.addTransportKeys(txn, null, keys)); + assertEquals(keySetId1, db.addTransportKeys(txn, null, keys1)); + + // Retrieve the transport keys + Collection allKeys = db.getTransportKeys(txn, transportId); + assertEquals(2, allKeys.size()); + for (KeySet ks : allKeys) { + assertNull(ks.getContactId()); + if (ks.getKeySetId().equals(keySetId)) { + assertKeysEquals(keys, ks.getTransportKeys()); + } else { + assertEquals(keySetId1, ks.getKeySetId()); + assertKeysEquals(keys1, ks.getTransportKeys()); + } + } + + // Bind the first set of transport keys + db.bindTransportKeys(txn, contactId, transportId, keySetId); + + // Retrieve the keys again - the first set should be bound + allKeys = db.getTransportKeys(txn, transportId); + assertEquals(2, allKeys.size()); + for (KeySet ks : allKeys) { + if (ks.getKeySetId().equals(keySetId)) { + assertEquals(contactId, ks.getContactId()); + assertKeysEquals(keys, ks.getTransportKeys()); + } else { + assertEquals(keySetId1, ks.getKeySetId()); + assertNull(ks.getContactId()); + assertKeysEquals(keys1, ks.getTransportKeys()); + } + } + + // Remove the unbound transport keys + db.removeTransportKeys(txn, transportId, keySetId1); + + // Retrieve the keys again - the second set should be gone + allKeys = db.getTransportKeys(txn, transportId); + assertEquals(1, allKeys.size()); + KeySet ks = allKeys.iterator().next(); + assertEquals(keySetId, ks.getKeySetId()); + assertEquals(contactId, ks.getContactId()); + assertKeysEquals(keys, ks.getTransportKeys()); + + // Removing the transport should remove the remaining transport keys + db.removeTransport(txn, transportId); + assertEquals(emptyList(), db.getTransportKeys(txn, transportId)); + + db.commitTransaction(txn); + db.close(); + } + + private void assertKeysEquals(TransportKeys expected, + TransportKeys actual) { + assertEquals(expected.getTransportId(), actual.getTransportId()); + assertEquals(expected.getRotationPeriod(), actual.getRotationPeriod()); + assertKeysEquals(expected.getPreviousIncomingKeys(), + actual.getPreviousIncomingKeys()); + assertKeysEquals(expected.getCurrentIncomingKeys(), + actual.getCurrentIncomingKeys()); + assertKeysEquals(expected.getNextIncomingKeys(), + actual.getNextIncomingKeys()); + assertKeysEquals(expected.getCurrentOutgoingKeys(), + actual.getCurrentOutgoingKeys()); + } + private void assertKeysEquals(IncomingKeys expected, IncomingKeys actual) { assertArrayEquals(expected.getTagKey().getBytes(), actual.getTagKey().getBytes()); 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 a57ce4e74..afc5dfed2 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 @@ -203,27 +203,7 @@ public class TransportKeyManagerImplTest extends BrambleMockTestCase { MAX_32_BIT_UNSIGNED + 1); Transaction txn = new Transaction(null, false); - context.checking(new Expectations() {{ - oneOf(transportCrypto).deriveTransportKeys(transportId, masterKey, - 1000, alice); - will(returnValue(transportKeys)); - // Get the current time (the start of rotation period 1000) - oneOf(clock).currentTimeMillis(); - will(returnValue(rotationPeriodLength * 1000)); - // 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()); - } - // Rotate the transport keys (the keys are unaffected) - oneOf(transportCrypto).rotateTransportKeys(transportKeys, 1000); - will(returnValue(transportKeys)); - // Save the keys - oneOf(db).addTransportKeys(txn, contactId, transportKeys); - will(returnValue(keySetId)); - }}); + expectAddContactNoRotation(alice, transportKeys, txn); TransportKeyManager transportKeyManager = new TransportKeyManagerImpl( db, transportCrypto, dbExecutor, scheduler, clock, transportId, @@ -243,26 +223,9 @@ public class TransportKeyManagerImplTest extends BrambleMockTestCase { MAX_32_BIT_UNSIGNED); Transaction txn = new Transaction(null, false); + expectAddContactNoRotation(alice, transportKeys, txn); + context.checking(new Expectations() {{ - oneOf(transportCrypto).deriveTransportKeys(transportId, masterKey, - 1000, alice); - will(returnValue(transportKeys)); - // Get the current time (the start of rotation period 1000) - oneOf(clock).currentTimeMillis(); - will(returnValue(rotationPeriodLength * 1000)); - // 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()); - } - // Rotate the transport keys (the keys are unaffected) - oneOf(transportCrypto).rotateTransportKeys(transportKeys, 1000); - will(returnValue(transportKeys)); - // Save the keys - oneOf(db).addTransportKeys(txn, contactId, transportKeys); - will(returnValue(keySetId)); // Increment the stream counter oneOf(db).incrementStreamCounter(txn, contactId, transportId, 1000); }}); @@ -294,27 +257,7 @@ public class TransportKeyManagerImplTest extends BrambleMockTestCase { TransportKeys transportKeys = createTransportKeys(1000, 0); Transaction txn = new Transaction(null, false); - context.checking(new Expectations() {{ - oneOf(transportCrypto).deriveTransportKeys(transportId, masterKey, - 1000, alice); - will(returnValue(transportKeys)); - // Get the current time (the start of rotation period 1000) - oneOf(clock).currentTimeMillis(); - will(returnValue(rotationPeriodLength * 1000)); - // 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()); - } - // Rotate the transport keys (the keys are unaffected) - oneOf(transportCrypto).rotateTransportKeys(transportKeys, 1000); - will(returnValue(transportKeys)); - // Save the keys - oneOf(db).addTransportKeys(txn, contactId, transportKeys); - will(returnValue(keySetId)); - }}); + expectAddContactNoRotation(alice, transportKeys, txn); TransportKeyManager transportKeyManager = new TransportKeyManagerImpl( db, transportCrypto, dbExecutor, scheduler, clock, transportId, @@ -323,6 +266,7 @@ public class TransportKeyManagerImplTest extends BrambleMockTestCase { long timestamp = rotationPeriodLength * 1000; transportKeyManager.addContact(txn, contactId, masterKey, timestamp, alice); + // The tag should not be recognised assertNull(transportKeyManager.getStreamContext(txn, new byte[TAG_LENGTH])); } @@ -466,6 +410,104 @@ public class TransportKeyManagerImplTest extends BrambleMockTestCase { transportKeyManager.start(txn); } + @Test + public void testTagsAreEncodedWhenKeysAreBound() throws Exception { + boolean alice = random.nextBoolean(); + TransportKeys transportKeys = createTransportKeys(1000, 0); + Transaction txn = new Transaction(null, false); + + context.checking(new Expectations() {{ + oneOf(transportCrypto).deriveTransportKeys(transportId, masterKey, + 1000, alice); + will(returnValue(transportKeys)); + // Get the current time (the start of rotation period 1000) + oneOf(clock).currentTimeMillis(); + will(returnValue(rotationPeriodLength * 1000)); + // Rotate the transport keys (the keys are unaffected) + oneOf(transportCrypto).rotateTransportKeys(transportKeys, 1000); + will(returnValue(transportKeys)); + // Save the unbound keys + oneOf(db).addTransportKeys(txn, null, transportKeys); + will(returnValue(keySetId)); + // When the keys are bound, 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 key binding + oneOf(db).bindTransportKeys(txn, contactId, transportId, keySetId); + }}); + + TransportKeyManager transportKeyManager = new TransportKeyManagerImpl( + db, transportCrypto, dbExecutor, scheduler, clock, transportId, + maxLatency); + // The timestamp is at the start of rotation period 1000 + long timestamp = rotationPeriodLength * 1000; + assertEquals(keySetId, transportKeyManager.addUnboundKeys(txn, + masterKey, timestamp, alice)); + transportKeyManager.bindKeys(txn, contactId, keySetId); + } + + @Test + public void testRemovingUnboundKeys() throws Exception { + boolean alice = random.nextBoolean(); + TransportKeys transportKeys = createTransportKeys(1000, 0); + Transaction txn = new Transaction(null, false); + + context.checking(new Expectations() {{ + oneOf(transportCrypto).deriveTransportKeys(transportId, masterKey, + 1000, alice); + will(returnValue(transportKeys)); + // Get the current time (the start of rotation period 1000) + oneOf(clock).currentTimeMillis(); + will(returnValue(rotationPeriodLength * 1000)); + // Rotate the transport keys (the keys are unaffected) + oneOf(transportCrypto).rotateTransportKeys(transportKeys, 1000); + will(returnValue(transportKeys)); + // Save the unbound keys + oneOf(db).addTransportKeys(txn, null, transportKeys); + will(returnValue(keySetId)); + // Remove the unbound keys + oneOf(db).removeTransportKeys(txn, transportId, keySetId); + }}); + + TransportKeyManager transportKeyManager = new TransportKeyManagerImpl( + db, transportCrypto, dbExecutor, scheduler, clock, transportId, + maxLatency); + // The timestamp is at the start of rotation period 1000 + long timestamp = rotationPeriodLength * 1000; + assertEquals(keySetId, transportKeyManager.addUnboundKeys(txn, + masterKey, timestamp, alice)); + transportKeyManager.removeKeys(txn, keySetId); + } + + private void expectAddContactNoRotation(boolean alice, + TransportKeys transportKeys, Transaction txn) throws Exception { + context.checking(new Expectations() {{ + oneOf(transportCrypto).deriveTransportKeys(transportId, masterKey, + 1000, alice); + will(returnValue(transportKeys)); + // Get the current time (the start of rotation period 1000) + oneOf(clock).currentTimeMillis(); + will(returnValue(rotationPeriodLength * 1000)); + // 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()); + } + // Rotate the transport keys (the keys are unaffected) + oneOf(transportCrypto).rotateTransportKeys(transportKeys, 1000); + will(returnValue(transportKeys)); + // Save the keys + oneOf(db).addTransportKeys(txn, contactId, transportKeys); + will(returnValue(keySetId)); + }}); + } + private TransportKeys createTransportKeys(long rotationPeriod, long streamCounter) { IncomingKeys inPrev = new IncomingKeys(tagKey, headerKey, From 6787d29f11d876330f76c86f28dfc37d81f1c79c Mon Sep 17 00:00:00 2001 From: akwizgran Date: Wed, 28 Mar 2018 11:41:50 +0100 Subject: [PATCH 08/11] Add a flag to indicate whether outgoing keys are active. --- .../bramble/api/crypto/TransportCrypto.java | 3 +- .../bramble/api/db/DatabaseComponent.java | 6 ++ .../bramble/api/transport/KeyManager.java | 10 +++ .../bramble/api/transport/OutgoingKeys.java | 12 +++- .../bramble/crypto/TransportCryptoImpl.java | 8 ++- .../org/briarproject/bramble/db/Database.java | 6 ++ .../bramble/db/DatabaseComponentImpl.java | 10 +++ .../briarproject/bramble/db/JdbcDatabase.java | 28 ++++++-- .../bramble/transport/KeyManagerImpl.java | 14 ++++ .../transport/MutableOutgoingKeys.java | 12 +++- .../transport/TransportKeyManager.java | 2 + .../transport/TransportKeyManagerImpl.java | 43 ++++++++--- .../bramble/crypto/KeyDerivationTest.java | 22 +++--- .../bramble/db/DatabaseComponentImplTest.java | 19 ++--- .../bramble/db/JdbcDatabaseTest.java | 3 +- .../TransportKeyManagerImplTest.java | 72 +++++++++++-------- 16 files changed, 201 insertions(+), 69 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 6385d1f01..cbd6449b4 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 @@ -14,9 +14,10 @@ public interface TransportCrypto { * rotation period from the given master secret. * * @param alice whether the keys are for use by Alice or Bob. + * @param active whether the keys are usable for outgoing streams. */ TransportKeys deriveTransportKeys(TransportId t, SecretKey master, - long rotationPeriod, boolean alice); + long rotationPeriod, boolean alice, boolean active); /** * Rotates the given transport keys to the given rotation period. If the 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 fcf5efe46..27e379571 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 @@ -528,6 +528,12 @@ public interface DatabaseComponent { void setReorderingWindow(Transaction txn, KeySetId k, TransportId t, long rotationPeriod, long base, byte[] bitmap) throws DbException; + /** + * Marks the given transport keys as usable for outgoing streams. + */ + void setTransportKeysActive(Transaction txn, TransportId t, KeySetId k) + throws DbException; + /** * Stores the given transport keys, deleting any keys they have replaced. */ 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 06b2f4725..a3f0f6d60 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 @@ -30,6 +30,9 @@ public interface KeyManager { /** * Derives and stores a set of unbound transport keys for each transport * and returns the key set IDs. + *

+ * The keys must be bound before they can be used for incoming streams, + * and also activated before they can be used for outgoing streams. */ Map addUnboundKeys(Transaction txn, SecretKey master, long timestamp, boolean alice) throws DbException; @@ -40,6 +43,13 @@ public interface KeyManager { void bindKeys(Transaction txn, ContactId c, Map keys) throws DbException; + /** + * Marks the given transport keys as usable for outgoing streams. Keys must + * be bound before they are activated. + */ + void activateKeys(Transaction txn, Map keys) + throws DbException; + /** * Removes the given transport keys, which must not have been bound, from * the manager and the database. diff --git a/bramble-api/src/main/java/org/briarproject/bramble/api/transport/OutgoingKeys.java b/bramble-api/src/main/java/org/briarproject/bramble/api/transport/OutgoingKeys.java index 202c46e6a..4214ffd91 100644 --- a/bramble-api/src/main/java/org/briarproject/bramble/api/transport/OutgoingKeys.java +++ b/bramble-api/src/main/java/org/briarproject/bramble/api/transport/OutgoingKeys.java @@ -10,18 +10,20 @@ public class OutgoingKeys { private final SecretKey tagKey, headerKey; private final long rotationPeriod, streamCounter; + private final boolean active; public OutgoingKeys(SecretKey tagKey, SecretKey headerKey, - long rotationPeriod) { - this(tagKey, headerKey, rotationPeriod, 0); + long rotationPeriod, boolean active) { + this(tagKey, headerKey, rotationPeriod, 0, active); } public OutgoingKeys(SecretKey tagKey, SecretKey headerKey, - long rotationPeriod, long streamCounter) { + long rotationPeriod, long streamCounter, boolean active) { this.tagKey = tagKey; this.headerKey = headerKey; this.rotationPeriod = rotationPeriod; this.streamCounter = streamCounter; + this.active = active; } public SecretKey getTagKey() { @@ -39,4 +41,8 @@ public class OutgoingKeys { public long getStreamCounter() { return streamCounter; } + + public boolean isActive() { + return active; + } } \ No newline at end of file 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 db35c9d5e..2d4ffb7d3 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 @@ -36,7 +36,8 @@ class TransportCryptoImpl implements TransportCrypto { @Override public TransportKeys deriveTransportKeys(TransportId t, - SecretKey master, long rotationPeriod, boolean alice) { + SecretKey master, long rotationPeriod, boolean alice, + boolean active) { // Keys for the previous period are derived from the master secret SecretKey inTagPrev = deriveTagKey(master, t, !alice); SecretKey inHeaderPrev = deriveHeaderKey(master, t, !alice); @@ -57,7 +58,7 @@ class TransportCryptoImpl implements TransportCrypto { IncomingKeys inNext = new IncomingKeys(inTagNext, inHeaderNext, rotationPeriod + 1); OutgoingKeys outCurr = new OutgoingKeys(outTagCurr, outHeaderCurr, - rotationPeriod); + rotationPeriod, active); // Collect and return the keys return new TransportKeys(t, inPrev, inCurr, inNext, outCurr); } @@ -71,6 +72,7 @@ class TransportCryptoImpl implements TransportCrypto { IncomingKeys inNext = k.getNextIncomingKeys(); OutgoingKeys outCurr = k.getCurrentOutgoingKeys(); long startPeriod = outCurr.getRotationPeriod(); + boolean active = outCurr.isActive(); // Rotate the keys for (long p = startPeriod + 1; p <= rotationPeriod; p++) { inPrev = inCurr; @@ -80,7 +82,7 @@ class TransportCryptoImpl implements TransportCrypto { inNext = new IncomingKeys(inNextTag, inNextHeader, p + 1); SecretKey outCurrTag = rotateKey(outCurr.getTagKey(), p); SecretKey outCurrHeader = rotateKey(outCurr.getHeaderKey(), p); - outCurr = new OutgoingKeys(outCurrTag, outCurrHeader, p); + outCurr = new OutgoingKeys(outCurrTag, outCurrHeader, p, active); } // Collect and return the keys return new TransportKeys(k.getTransportId(), inPrev, inCurr, inNext, 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 6ee996b2b..ddd4860c2 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 @@ -640,6 +640,12 @@ interface Database { void setReorderingWindow(T txn, KeySetId k, TransportId t, long rotationPeriod, long base, byte[] bitmap) throws DbException; + /** + * Marks the given transport keys as usable for outgoing streams. + */ + void setTransportKeysActive(T txn, TransportId t, KeySetId k) + throws DbException; + /** * Updates the transmission count and expiry time of the given message * with respect to the given contact, using the latency of the transport 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 697bcdd3d..a97480200 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 @@ -890,6 +890,16 @@ class DatabaseComponentImpl implements DatabaseComponent { db.setReorderingWindow(txn, k, t, rotationPeriod, base, bitmap); } + @Override + public void setTransportKeysActive(Transaction transaction, TransportId t, + KeySetId k) throws DbException { + if (transaction.isReadOnly()) throw new IllegalArgumentException(); + T txn = unbox(transaction); + if (!db.containsTransport(txn, t)) + throw new NoSuchTransportException(); + db.setTransportKeysActive(txn, t, k); + } + @Override public void updateTransportKeys(Transaction transaction, Collection keys) throws DbException { 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 25cfb4d5a..953a0f160 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 @@ -234,6 +234,7 @@ abstract class JdbcDatabase implements Database { + " tagKey _SECRET NOT NULL," + " headerKey _SECRET NOT NULL," + " stream BIGINT NOT NULL," + + " active BOOLEAN NOT NULL," + " PRIMARY KEY (transportId, keySetId)," + " FOREIGN KEY (transportId)" + " REFERENCES transports (transportId)" @@ -880,8 +881,8 @@ abstract class JdbcDatabase implements Database { try { // Store the outgoing keys String sql = "INSERT INTO outgoingKeys (contactId, transportId," - + " rotationPeriod, tagKey, headerKey, stream)" - + " VALUES (?, ?, ?, ?, ?, ?)"; + + " rotationPeriod, tagKey, headerKey, stream, active)" + + " VALUES (?, ?, ?, ?, ?, ?, ?)"; ps = txn.prepareStatement(sql); if (c == null) ps.setNull(1, INTEGER); else ps.setInt(1, c.getInt()); @@ -891,6 +892,7 @@ abstract class JdbcDatabase implements Database { ps.setBytes(4, outCurr.getTagKey().getBytes()); ps.setBytes(5, outCurr.getHeaderKey().getBytes()); ps.setLong(6, outCurr.getStreamCounter()); + ps.setBoolean(7, outCurr.isActive()); int affected = ps.executeUpdate(); if (affected != 1) throw new DbStateException(); ps.close(); @@ -2157,7 +2159,7 @@ abstract class JdbcDatabase implements Database { ps.close(); // Retrieve the outgoing keys in the same order sql = "SELECT keySetId, contactId, rotationPeriod," - + " tagKey, headerKey, stream" + + " tagKey, headerKey, stream, active" + " FROM outgoingKeys" + " WHERE transportId = ?" + " ORDER BY keySetId"; @@ -2175,8 +2177,9 @@ abstract class JdbcDatabase implements Database { SecretKey tagKey = new SecretKey(rs.getBytes(4)); SecretKey headerKey = new SecretKey(rs.getBytes(5)); long streamCounter = rs.getLong(6); + boolean active = rs.getBoolean(7); OutgoingKeys outCurr = new OutgoingKeys(tagKey, headerKey, - rotationPeriod, streamCounter); + rotationPeriod, streamCounter, active); IncomingKeys inPrev = inKeys.get(i * 3); IncomingKeys inCurr = inKeys.get(i * 3 + 1); IncomingKeys inNext = inKeys.get(i * 3 + 2); @@ -2889,6 +2892,23 @@ abstract class JdbcDatabase implements Database { } } + @Override + public void setTransportKeysActive(Connection txn, TransportId t, + KeySetId k) throws DbException { + PreparedStatement ps = null; + try { + String sql = "UPDATE outgoingKeys SET active = true" + + " WHERE transportId = ? AND keySetId = ?"; + ps = txn.prepareStatement(sql); + int affected = ps.executeUpdate(); + if (affected < 0 || affected > 1) throw new DbStateException(); + ps.close(); + } catch (SQLException e) { + tryToClose(ps); + throw new DbException(e); + } + } + @Override public void updateExpiryTime(Connection txn, ContactId c, MessageId m, int maxLatency) throws DbException { 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 1a06ef4a6..860e0e402 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 @@ -132,6 +132,20 @@ class KeyManagerImpl implements KeyManager, Service, EventListener { } } + @Override + 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 { + m.activateKeys(txn, e.getValue()); + } + } + } + @Override public void removeKeys(Transaction txn, Map keys) throws DbException { diff --git a/bramble-core/src/main/java/org/briarproject/bramble/transport/MutableOutgoingKeys.java b/bramble-core/src/main/java/org/briarproject/bramble/transport/MutableOutgoingKeys.java index aaafec13b..c195f445c 100644 --- a/bramble-core/src/main/java/org/briarproject/bramble/transport/MutableOutgoingKeys.java +++ b/bramble-core/src/main/java/org/briarproject/bramble/transport/MutableOutgoingKeys.java @@ -13,17 +13,19 @@ class MutableOutgoingKeys { private final SecretKey tagKey, headerKey; private final long rotationPeriod; private long streamCounter; + private boolean active; MutableOutgoingKeys(OutgoingKeys out) { tagKey = out.getTagKey(); headerKey = out.getHeaderKey(); rotationPeriod = out.getRotationPeriod(); streamCounter = out.getStreamCounter(); + active = out.isActive(); } OutgoingKeys snapshot() { return new OutgoingKeys(tagKey, headerKey, rotationPeriod, - streamCounter); + streamCounter, active); } SecretKey getTagKey() { @@ -45,4 +47,12 @@ class MutableOutgoingKeys { void incrementStreamCounter() { streamCounter++; } + + boolean isActive() { + return active; + } + + void activate() { + active = true; + } } 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 4eff0b289..2af01fa4a 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 @@ -23,6 +23,8 @@ interface TransportKeyManager { void bindKeys(Transaction txn, ContactId c, KeySetId k) throws DbException; + void activateKeys(Transaction txn, KeySetId k) throws DbException; + void removeKeys(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 889485d0d..d7ccb49c2 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 @@ -127,10 +127,7 @@ class TransportKeyManagerImpl implements TransportKeyManager { encodeTags(keySetId, contactId, m.getPreviousIncomingKeys()); encodeTags(keySetId, contactId, m.getCurrentIncomingKeys()); encodeTags(keySetId, contactId, m.getNextIncomingKeys()); - // Use the outgoing keys with the highest key set ID - MutableKeySet old = outContexts.get(contactId); - if (old == null || old.getKeySetId().getInt() < keySetId.getInt()) - outContexts.put(contactId, ks); + considerReplacingOutgoingKeys(ks); } } @@ -147,6 +144,17 @@ class TransportKeyManagerImpl implements TransportKeyManager { } } + // Locking: lock + private void considerReplacingOutgoingKeys(MutableKeySet ks) { + // Use the active outgoing keys with the highest key set ID + if (ks.getTransportKeys().getCurrentOutgoingKeys().isActive()) { + MutableKeySet old = outContexts.get(ks.getContactId()); + if (old == null || + old.getKeySetId().getInt() < ks.getKeySetId().getInt()) + outContexts.put(ks.getContactId(), ks); + } + } + private void scheduleKeyRotation(long now) { long delay = rotationPeriodLength - now % rotationPeriodLength; scheduler.schedule((Runnable) this::rotateKeys, delay, MILLISECONDS); @@ -171,17 +179,17 @@ class TransportKeyManagerImpl implements TransportKeyManager { @Override public void addContact(Transaction txn, ContactId c, SecretKey master, long timestamp, boolean alice) throws DbException { - deriveAndAddKeys(txn, c, master, timestamp, alice); + deriveAndAddKeys(txn, c, master, timestamp, alice, true); } @Override public KeySetId addUnboundKeys(Transaction txn, SecretKey master, long timestamp, boolean alice) throws DbException { - return deriveAndAddKeys(txn, null, master, timestamp, alice); + return deriveAndAddKeys(txn, null, master, timestamp, alice, false); } private KeySetId deriveAndAddKeys(Transaction txn, @Nullable ContactId c, - SecretKey master, long timestamp, boolean alice) + SecretKey master, long timestamp, boolean alice, boolean active) throws DbException { lock.lock(); try { @@ -189,7 +197,7 @@ class TransportKeyManagerImpl implements TransportKeyManager { long rotationPeriod = timestamp / rotationPeriodLength; // Derive the transport keys TransportKeys k = transportCrypto.deriveTransportKeys(transportId, - master, rotationPeriod, alice); + master, rotationPeriod, alice, active); // Rotate the keys to the current rotation period if necessary rotationPeriod = clock.currentTimeMillis() / rotationPeriodLength; k = transportCrypto.rotateTransportKeys(k, rotationPeriod); @@ -210,6 +218,7 @@ class TransportKeyManagerImpl implements TransportKeyManager { try { MutableKeySet ks = keys.get(k); if (ks == null) throw new IllegalArgumentException(); + // Check that the keys haven't already been bound if (ks.getContactId() != null) throw new IllegalArgumentException(); MutableTransportKeys m = ks.getTransportKeys(); addKeys(k, c, m); @@ -219,12 +228,30 @@ class TransportKeyManagerImpl implements TransportKeyManager { } } + @Override + public void activateKeys(Transaction txn, KeySetId k) throws DbException { + lock.lock(); + try { + MutableKeySet ks = keys.get(k); + if (ks == null) throw new IllegalArgumentException(); + // Check that the keys have been bound + if (ks.getContactId() == null) throw new IllegalArgumentException(); + MutableTransportKeys m = ks.getTransportKeys(); + m.getCurrentOutgoingKeys().activate(); + considerReplacingOutgoingKeys(ks); + db.setTransportKeysActive(txn, m.getTransportId(), k); + } finally { + lock.unlock(); + } + } + @Override public void removeKeys(Transaction txn, KeySetId k) throws DbException { lock.lock(); try { MutableKeySet ks = keys.remove(k); if (ks == null) throw new IllegalArgumentException(); + // Check that the keys haven't been bound if (ks.getContactId() != null) throw new IllegalArgumentException(); TransportId t = ks.getTransportKeys().getTransportId(); db.removeTransportKeys(txn, t, k); diff --git a/bramble-core/src/test/java/org/briarproject/bramble/crypto/KeyDerivationTest.java b/bramble-core/src/test/java/org/briarproject/bramble/crypto/KeyDerivationTest.java index 81f73f8d2..dc51966ff 100644 --- a/bramble-core/src/test/java/org/briarproject/bramble/crypto/KeyDerivationTest.java +++ b/bramble-core/src/test/java/org/briarproject/bramble/crypto/KeyDerivationTest.java @@ -33,7 +33,7 @@ public class KeyDerivationTest extends BrambleTestCase { @Test public void testKeysAreDistinct() { TransportKeys k = transportCrypto.deriveTransportKeys(transportId, - master, 123, true); + master, 123, true, true); assertAllDifferent(k); } @@ -41,9 +41,9 @@ public class KeyDerivationTest extends BrambleTestCase { public void testCurrentKeysMatchCurrentKeysOfContact() { // Start in rotation period 123 TransportKeys kA = transportCrypto.deriveTransportKeys(transportId, - master, 123, true); + master, 123, true, true); TransportKeys kB = transportCrypto.deriveTransportKeys(transportId, - master, 123, false); + master, 123, false, true); // Alice's incoming keys should equal Bob's outgoing keys assertArrayEquals(kA.getCurrentIncomingKeys().getTagKey().getBytes(), kB.getCurrentOutgoingKeys().getTagKey().getBytes()); @@ -73,9 +73,9 @@ public class KeyDerivationTest extends BrambleTestCase { public void testPreviousKeysMatchPreviousKeysOfContact() { // Start in rotation period 123 TransportKeys kA = transportCrypto.deriveTransportKeys(transportId, - master, 123, true); + master, 123, true, true); TransportKeys kB = transportCrypto.deriveTransportKeys(transportId, - master, 123, false); + master, 123, false, true); // Compare Alice's previous keys in period 456 with Bob's current keys // in period 455 kA = transportCrypto.rotateTransportKeys(kA, 456); @@ -100,9 +100,9 @@ public class KeyDerivationTest extends BrambleTestCase { public void testNextKeysMatchNextKeysOfContact() { // Start in rotation period 123 TransportKeys kA = transportCrypto.deriveTransportKeys(transportId, - master, 123, true); + master, 123, true, true); TransportKeys kB = transportCrypto.deriveTransportKeys(transportId, - master, 123, false); + master, 123, false, true); // Compare Alice's current keys in period 456 with Bob's next keys in // period 455 kA = transportCrypto.rotateTransportKeys(kA, 456); @@ -127,9 +127,9 @@ public class KeyDerivationTest extends BrambleTestCase { SecretKey master1 = getSecretKey(); assertFalse(Arrays.equals(master.getBytes(), master1.getBytes())); TransportKeys k = transportCrypto.deriveTransportKeys(transportId, - master, 123, true); + master, 123, true, true); TransportKeys k1 = transportCrypto.deriveTransportKeys(transportId, - master1, 123, true); + master1, 123, true, true); assertAllDifferent(k, k1); } @@ -138,9 +138,9 @@ public class KeyDerivationTest extends BrambleTestCase { TransportId transportId1 = new TransportId("id1"); assertFalse(transportId.getString().equals(transportId1.getString())); TransportKeys k = transportCrypto.deriveTransportKeys(transportId, - master, 123, true); + master, 123, true, true); TransportKeys k1 = transportCrypto.deriveTransportKeys(transportId1, - master, 123, true); + master, 123, true, true); assertAllDifferent(k, k1); } 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 61a499162..d8f1f59fb 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 @@ -71,6 +71,7 @@ import static org.briarproject.bramble.api.transport.TransportConstants.REORDERI import static org.briarproject.bramble.db.DatabaseConstants.MAX_OFFERED_MESSAGES; import static org.briarproject.bramble.test.TestUtils.getAuthor; import static org.briarproject.bramble.test.TestUtils.getLocalAuthor; +import static org.briarproject.bramble.test.TestUtils.getSecretKey; import static org.briarproject.bramble.util.StringUtils.getRandomString; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; @@ -1356,22 +1357,22 @@ public class DatabaseComponentImplTest extends BrambleMockTestCase { } private TransportKeys createTransportKeys() { - SecretKey inPrevTagKey = TestUtils.getSecretKey(); - SecretKey inPrevHeaderKey = TestUtils.getSecretKey(); + SecretKey inPrevTagKey = getSecretKey(); + SecretKey inPrevHeaderKey = getSecretKey(); IncomingKeys inPrev = new IncomingKeys(inPrevTagKey, inPrevHeaderKey, 1, 123, new byte[4]); - SecretKey inCurrTagKey = TestUtils.getSecretKey(); - SecretKey inCurrHeaderKey = TestUtils.getSecretKey(); + SecretKey inCurrTagKey = getSecretKey(); + SecretKey inCurrHeaderKey = getSecretKey(); IncomingKeys inCurr = new IncomingKeys(inCurrTagKey, inCurrHeaderKey, 2, 234, new byte[4]); - SecretKey inNextTagKey = TestUtils.getSecretKey(); - SecretKey inNextHeaderKey = TestUtils.getSecretKey(); + SecretKey inNextTagKey = getSecretKey(); + SecretKey inNextHeaderKey = getSecretKey(); IncomingKeys inNext = new IncomingKeys(inNextTagKey, inNextHeaderKey, 3, 345, new byte[4]); - SecretKey outCurrTagKey = TestUtils.getSecretKey(); - SecretKey outCurrHeaderKey = TestUtils.getSecretKey(); + SecretKey outCurrTagKey = getSecretKey(); + SecretKey outCurrHeaderKey = getSecretKey(); OutgoingKeys outCurr = new OutgoingKeys(outCurrTagKey, outCurrHeaderKey, - 2, 456); + 2, 456, true); return new TransportKeys(transportId, inPrev, inCurr, inNext, outCurr); } 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 6babc6459..9e4047982 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 @@ -804,6 +804,7 @@ public abstract class JdbcDatabaseTest extends BrambleTestCase { actual.getHeaderKey().getBytes()); assertEquals(expected.getRotationPeriod(), actual.getRotationPeriod()); assertEquals(expected.getStreamCounter(), actual.getStreamCounter()); + assertEquals(expected.isActive(), actual.isActive()); } @Test @@ -1820,7 +1821,7 @@ public abstract class JdbcDatabaseTest extends BrambleTestCase { SecretKey outCurrTagKey = getSecretKey(); SecretKey outCurrHeaderKey = getSecretKey(); OutgoingKeys outCurr = new OutgoingKeys(outCurrTagKey, outCurrHeaderKey, - 2, 456); + 2, 456, true); return new TransportKeys(transportId, inPrev, inCurr, inNext, outCurr); } 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 afc5dfed2..8da8eefaa 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 @@ -65,16 +65,16 @@ public class TransportKeyManagerImplTest extends BrambleMockTestCase { @Test public void testKeysAreRotatedAtStartup() throws Exception { - TransportKeys shouldRotate = createTransportKeys(900, 0); - TransportKeys shouldNotRotate = createTransportKeys(1000, 0); - TransportKeys shouldRotate1 = createTransportKeys(999, 0); + TransportKeys shouldRotate = createTransportKeys(900, 0, true); + TransportKeys shouldNotRotate = createTransportKeys(1000, 0, true); + TransportKeys shouldRotate1 = createTransportKeys(999, 0, false); Collection loaded = asList( new KeySet(keySetId, contactId, shouldRotate), new KeySet(keySetId1, contactId1, shouldNotRotate), new KeySet(keySetId2, null, shouldRotate1) ); - TransportKeys rotated = createTransportKeys(1000, 0); - TransportKeys rotated1 = createTransportKeys(1000, 0); + TransportKeys rotated = createTransportKeys(1000, 0, true); + TransportKeys rotated1 = createTransportKeys(1000, 0, false); Transaction txn = new Transaction(null, false); context.checking(new Expectations() {{ @@ -117,13 +117,13 @@ public class TransportKeyManagerImplTest extends BrambleMockTestCase { @Test public void testKeysAreRotatedWhenAddingContact() throws Exception { boolean alice = random.nextBoolean(); - TransportKeys transportKeys = createTransportKeys(999, 0); - TransportKeys rotated = createTransportKeys(1000, 0); + TransportKeys transportKeys = createTransportKeys(999, 0, true); + TransportKeys rotated = createTransportKeys(1000, 0, true); Transaction txn = new Transaction(null, false); context.checking(new Expectations() {{ oneOf(transportCrypto).deriveTransportKeys(transportId, masterKey, - 999, alice); + 999, alice, true); will(returnValue(transportKeys)); // Get the current time (1 ms after start of rotation period 1000) oneOf(clock).currentTimeMillis(); @@ -155,13 +155,13 @@ public class TransportKeyManagerImplTest extends BrambleMockTestCase { @Test public void testKeysAreRotatedWhenAddingUnboundKeys() throws Exception { boolean alice = random.nextBoolean(); - TransportKeys transportKeys = createTransportKeys(999, 0); - TransportKeys rotated = createTransportKeys(1000, 0); + TransportKeys transportKeys = createTransportKeys(999, 0, false); + TransportKeys rotated = createTransportKeys(1000, 0, false); Transaction txn = new Transaction(null, false); context.checking(new Expectations() {{ oneOf(transportCrypto).deriveTransportKeys(transportId, masterKey, - 999, alice); + 999, alice, false); will(returnValue(transportKeys)); // Get the current time (1 ms after start of rotation period 1000) oneOf(clock).currentTimeMillis(); @@ -200,7 +200,7 @@ public class TransportKeyManagerImplTest extends BrambleMockTestCase { boolean alice = random.nextBoolean(); // The stream counter has been exhausted TransportKeys transportKeys = createTransportKeys(1000, - MAX_32_BIT_UNSIGNED + 1); + MAX_32_BIT_UNSIGNED + 1, true); Transaction txn = new Transaction(null, false); expectAddContactNoRotation(alice, transportKeys, txn); @@ -220,7 +220,7 @@ public class TransportKeyManagerImplTest extends BrambleMockTestCase { boolean alice = random.nextBoolean(); // The stream counter can be used one more time before being exhausted TransportKeys transportKeys = createTransportKeys(1000, - MAX_32_BIT_UNSIGNED); + MAX_32_BIT_UNSIGNED, true); Transaction txn = new Transaction(null, false); expectAddContactNoRotation(alice, transportKeys, txn); @@ -254,7 +254,7 @@ public class TransportKeyManagerImplTest extends BrambleMockTestCase { public void testIncomingStreamContextIsNullIfTagIsNotFound() throws Exception { boolean alice = random.nextBoolean(); - TransportKeys transportKeys = createTransportKeys(1000, 0); + TransportKeys transportKeys = createTransportKeys(1000, 0, true); Transaction txn = new Transaction(null, false); expectAddContactNoRotation(alice, transportKeys, txn); @@ -274,14 +274,14 @@ public class TransportKeyManagerImplTest extends BrambleMockTestCase { @Test public void testTagIsNotRecognisedTwice() throws Exception { boolean alice = random.nextBoolean(); - TransportKeys transportKeys = createTransportKeys(1000, 0); + TransportKeys transportKeys = createTransportKeys(1000, 0, true); // Keep a copy of the tags List tags = new ArrayList<>(); Transaction txn = new Transaction(null, false); context.checking(new Expectations() {{ oneOf(transportCrypto).deriveTransportKeys(transportId, masterKey, - 1000, alice); + 1000, alice, true); will(returnValue(transportKeys)); // Get the current time (the start of rotation period 1000) oneOf(clock).currentTimeMillis(); @@ -335,14 +335,14 @@ public class TransportKeyManagerImplTest extends BrambleMockTestCase { @Test public void testKeysAreRotatedToCurrentPeriod() throws Exception { - TransportKeys transportKeys = createTransportKeys(1000, 0); - TransportKeys transportKeys1 = createTransportKeys(1000, 0); + TransportKeys transportKeys = createTransportKeys(1000, 0, true); + TransportKeys transportKeys1 = createTransportKeys(1000, 0, false); Collection loaded = asList( new KeySet(keySetId, contactId, transportKeys), new KeySet(keySetId1, null, transportKeys1) ); - TransportKeys rotated = createTransportKeys(1001, 0); - TransportKeys rotated1 = createTransportKeys(1001, 0); + TransportKeys rotated = createTransportKeys(1001, 0, true); + TransportKeys rotated1 = createTransportKeys(1001, 0, false); Transaction txn = new Transaction(null, false); Transaction txn1 = new Transaction(null, false); @@ -411,14 +411,14 @@ public class TransportKeyManagerImplTest extends BrambleMockTestCase { } @Test - public void testTagsAreEncodedWhenKeysAreBound() throws Exception { + public void testBindingAndActivatingKeys() throws Exception { boolean alice = random.nextBoolean(); - TransportKeys transportKeys = createTransportKeys(1000, 0); + TransportKeys transportKeys = createTransportKeys(1000, 0, false); Transaction txn = new Transaction(null, false); context.checking(new Expectations() {{ oneOf(transportCrypto).deriveTransportKeys(transportId, masterKey, - 1000, alice); + 1000, alice, false); will(returnValue(transportKeys)); // Get the current time (the start of rotation period 1000) oneOf(clock).currentTimeMillis(); @@ -438,6 +438,10 @@ public class TransportKeyManagerImplTest extends BrambleMockTestCase { } // Save the key binding oneOf(db).bindTransportKeys(txn, contactId, transportId, keySetId); + // Activate the keys + oneOf(db).setTransportKeysActive(txn, transportId, keySetId); + // Increment the stream counter + oneOf(db).incrementStreamCounter(txn, contactId, transportId, 1000); }}); TransportKeyManager transportKeyManager = new TransportKeyManagerImpl( @@ -448,17 +452,29 @@ public class TransportKeyManagerImplTest extends BrambleMockTestCase { assertEquals(keySetId, transportKeyManager.addUnboundKeys(txn, masterKey, timestamp, alice)); transportKeyManager.bindKeys(txn, contactId, keySetId); + // The keys are inactive so no stream context should be returned + assertNull(transportKeyManager.getStreamContext(txn, contactId)); + transportKeyManager.activateKeys(txn, keySetId); + // The keys are active so a stream context should be returned + StreamContext ctx = transportKeyManager.getStreamContext(txn, + contactId); + assertNotNull(ctx); + assertEquals(contactId, ctx.getContactId()); + assertEquals(transportId, ctx.getTransportId()); + assertEquals(tagKey, ctx.getTagKey()); + assertEquals(headerKey, ctx.getHeaderKey()); + assertEquals(0, ctx.getStreamNumber()); } @Test public void testRemovingUnboundKeys() throws Exception { boolean alice = random.nextBoolean(); - TransportKeys transportKeys = createTransportKeys(1000, 0); + TransportKeys transportKeys = createTransportKeys(1000, 0, false); Transaction txn = new Transaction(null, false); context.checking(new Expectations() {{ oneOf(transportCrypto).deriveTransportKeys(transportId, masterKey, - 1000, alice); + 1000, alice, false); will(returnValue(transportKeys)); // Get the current time (the start of rotation period 1000) oneOf(clock).currentTimeMillis(); @@ -487,7 +503,7 @@ public class TransportKeyManagerImplTest extends BrambleMockTestCase { TransportKeys transportKeys, Transaction txn) throws Exception { context.checking(new Expectations() {{ oneOf(transportCrypto).deriveTransportKeys(transportId, masterKey, - 1000, alice); + 1000, alice, true); will(returnValue(transportKeys)); // Get the current time (the start of rotation period 1000) oneOf(clock).currentTimeMillis(); @@ -509,7 +525,7 @@ public class TransportKeyManagerImplTest extends BrambleMockTestCase { } private TransportKeys createTransportKeys(long rotationPeriod, - long streamCounter) { + long streamCounter, boolean active) { IncomingKeys inPrev = new IncomingKeys(tagKey, headerKey, rotationPeriod - 1); IncomingKeys inCurr = new IncomingKeys(tagKey, headerKey, @@ -517,7 +533,7 @@ public class TransportKeyManagerImplTest extends BrambleMockTestCase { IncomingKeys inNext = new IncomingKeys(tagKey, headerKey, rotationPeriod + 1); OutgoingKeys outCurr = new OutgoingKeys(tagKey, headerKey, - rotationPeriod, streamCounter); + rotationPeriod, streamCounter, active); return new TransportKeys(transportId, inPrev, inCurr, inNext, outCurr); } From 798b871cc916edeead0013bd79a29a090e990a5b Mon Sep 17 00:00:00 2001 From: akwizgran Date: Wed, 28 Mar 2018 11:52:14 +0100 Subject: [PATCH 09/11] Use key set ID to increment stream counter. --- .../bramble/api/db/DatabaseComponent.java | 7 +++---- .../org/briarproject/bramble/db/Database.java | 7 +++---- .../bramble/db/DatabaseComponentImpl.java | 8 +++----- .../briarproject/bramble/db/JdbcDatabase.java | 12 +++++------ .../transport/TransportKeyManagerImpl.java | 4 ++-- .../bramble/db/DatabaseComponentImplTest.java | 20 +++++-------------- .../bramble/db/JdbcDatabaseTest.java | 4 ++-- .../TransportKeyManagerImplTest.java | 4 ++-- 8 files changed, 25 insertions(+), 41 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 27e379571..ea05938d4 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 @@ -407,11 +407,10 @@ public interface DatabaseComponent { throws DbException; /** - * Increments the outgoing stream counter for the given contact and - * transport in the given rotation period . + * Increments the outgoing stream counter for the given transport keys. */ - void incrementStreamCounter(Transaction txn, ContactId c, TransportId t, - long rotationPeriod) throws DbException; + void incrementStreamCounter(Transaction txn, TransportId t, KeySetId k) + throws DbException; /** * Merges the given metadata with the existing metadata for the given 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 ddd4860c2..dcf96ea4e 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 @@ -499,11 +499,10 @@ interface Database { throws DbException; /** - * Increments the outgoing stream counter for the given contact and - * transport in the given rotation period. + * Increments the outgoing stream counter for the given transport keys. */ - void incrementStreamCounter(T txn, ContactId c, TransportId t, - long rotationPeriod) throws DbException; + void incrementStreamCounter(T txn, TransportId t, KeySetId k) + throws DbException; /** * Marks the given messages as not needing to be acknowledged to the 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 a97480200..00f0bb622 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 @@ -607,15 +607,13 @@ class DatabaseComponentImpl implements DatabaseComponent { } @Override - public void incrementStreamCounter(Transaction transaction, ContactId c, - TransportId t, long rotationPeriod) throws DbException { + public void incrementStreamCounter(Transaction transaction, TransportId t, + KeySetId k) throws DbException { if (transaction.isReadOnly()) throw new IllegalArgumentException(); T txn = unbox(transaction); - if (!db.containsContact(txn, c)) - throw new NoSuchContactException(); if (!db.containsTransport(txn, t)) throw new NoSuchTransportException(); - db.incrementStreamCounter(txn, c, t, rotationPeriod); + db.incrementStreamCounter(txn, t, k); } @Override 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 953a0f160..dd5874f6e 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 @@ -2198,17 +2198,15 @@ abstract class JdbcDatabase implements Database { } @Override - public void incrementStreamCounter(Connection txn, ContactId c, - TransportId t, long rotationPeriod) throws DbException { + public void incrementStreamCounter(Connection txn, TransportId t, + KeySetId k) throws DbException { PreparedStatement ps = null; try { String sql = "UPDATE outgoingKeys SET stream = stream + 1" - + " WHERE contactId = ? AND transportId = ?" - + " AND rotationPeriod = ?"; + + " WHERE transportId = ? AND keySetId = ?"; ps = txn.prepareStatement(sql); - ps.setInt(1, c.getInt()); - ps.setString(2, t.getString()); - ps.setLong(3, rotationPeriod); + ps.setString(1, t.getString()); + ps.setInt(2, k.getInt()); int affected = ps.executeUpdate(); if (affected != 1) throw new DbStateException(); ps.close(); 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 d7ccb49c2..f6abc1c54 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 @@ -288,6 +288,7 @@ class TransportKeyManagerImpl implements TransportKeyManager { if (ks == null) return null; MutableOutgoingKeys outKeys = ks.getTransportKeys().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, transportId, @@ -295,8 +296,7 @@ class TransportKeyManagerImpl implements TransportKeyManager { outKeys.getStreamCounter()); // Increment the stream counter and write it back to the DB outKeys.incrementStreamCounter(); - db.incrementStreamCounter(txn, c, transportId, - outKeys.getRotationPeriod()); + db.incrementStreamCounter(txn, transportId, ks.getKeySetId()); return ctx; } finally { lock.unlock(); 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 d8f1f59fb..f0149bb24 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 @@ -285,11 +285,11 @@ public class DatabaseComponentImplTest extends BrambleMockTestCase { throws Exception { context.checking(new Expectations() {{ // Check whether the contact is in the DB (which it's not) - exactly(18).of(database).startTransaction(); + exactly(17).of(database).startTransaction(); will(returnValue(txn)); - exactly(18).of(database).containsContact(txn, contactId); + exactly(17).of(database).containsContact(txn, contactId); will(returnValue(false)); - exactly(18).of(database).abortTransaction(txn); + exactly(17).of(database).abortTransaction(txn); }}); DatabaseComponent db = createDatabaseComponent(database, eventBus, shutdown); @@ -384,16 +384,6 @@ public class DatabaseComponentImplTest extends BrambleMockTestCase { db.endTransaction(transaction); } - transaction = db.startTransaction(false); - try { - db.incrementStreamCounter(transaction, contactId, transportId, 0); - fail(); - } catch (NoSuchContactException expected) { - // Expected - } finally { - db.endTransaction(transaction); - } - transaction = db.startTransaction(false); try { db.getGroupVisibility(transaction, contactId, groupId); @@ -781,7 +771,7 @@ public class DatabaseComponentImplTest extends BrambleMockTestCase { // Check whether the transport is in the DB (which it's not) exactly(6).of(database).startTransaction(); will(returnValue(txn)); - exactly(2).of(database).containsContact(txn, contactId); + oneOf(database).containsContact(txn, contactId); will(returnValue(true)); exactly(6).of(database).containsTransport(txn, transportId); will(returnValue(false)); @@ -822,7 +812,7 @@ public class DatabaseComponentImplTest extends BrambleMockTestCase { transaction = db.startTransaction(false); try { - db.incrementStreamCounter(transaction, contactId, transportId, 0); + db.incrementStreamCounter(transaction, transportId, keySetId); fail(); } catch (NoSuchTransportException expected) { // Expected 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 9e4047982..157b6902b 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 @@ -825,8 +825,8 @@ public abstract class JdbcDatabaseTest extends BrambleTestCase { singletonList(new KeySet(keySetId, contactId, keys))); // Increment the stream counter twice and retrieve the transport keys - db.incrementStreamCounter(txn, contactId, transportId, rotationPeriod); - db.incrementStreamCounter(txn, contactId, transportId, rotationPeriod); + db.incrementStreamCounter(txn, transportId, keySetId); + db.incrementStreamCounter(txn, transportId, keySetId); Collection newKeys = db.getTransportKeys(txn, transportId); assertEquals(1, newKeys.size()); KeySet ks = newKeys.iterator().next(); 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 8da8eefaa..df708df91 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 @@ -227,7 +227,7 @@ public class TransportKeyManagerImplTest extends BrambleMockTestCase { context.checking(new Expectations() {{ // Increment the stream counter - oneOf(db).incrementStreamCounter(txn, contactId, transportId, 1000); + oneOf(db).incrementStreamCounter(txn, transportId, keySetId); }}); TransportKeyManager transportKeyManager = new TransportKeyManagerImpl( @@ -441,7 +441,7 @@ public class TransportKeyManagerImplTest extends BrambleMockTestCase { // Activate the keys oneOf(db).setTransportKeysActive(txn, transportId, keySetId); // Increment the stream counter - oneOf(db).incrementStreamCounter(txn, contactId, transportId, 1000); + oneOf(db).incrementStreamCounter(txn, transportId, keySetId); }}); TransportKeyManager transportKeyManager = new TransportKeyManagerImpl( From f7c2f86499106fb4be5e34991be2741d5a77871b Mon Sep 17 00:00:00 2001 From: akwizgran Date: Wed, 28 Mar 2018 12:08:08 +0100 Subject: [PATCH 10/11] Add a method for checking whether we can send streams. --- .../bramble/api/transport/KeyManager.java | 6 +++ .../bramble/transport/KeyManagerImpl.java | 6 +++ .../transport/TransportKeyManager.java | 2 + .../transport/TransportKeyManagerImpl.java | 15 +++++++ .../TransportKeyManagerImplTest.java | 39 ++++++++++++------- 5 files changed, 53 insertions(+), 15 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 a3f0f6d60..065e5d2bc 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 @@ -57,6 +57,12 @@ public interface KeyManager { void removeKeys(Transaction txn, Map keys) throws DbException; + /** + * Returns true if we have keys that can be used for outgoing streams to + * the given contact over the given transport. + */ + boolean canSendOutgoingStreams(ContactId c, 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 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 860e0e402..bbd5e5ec1 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 @@ -160,6 +160,12 @@ class KeyManagerImpl implements KeyManager, Service, EventListener { } } + @Override + public boolean canSendOutgoingStreams(ContactId c, TransportId t) { + TransportKeyManager m = managers.get(t); + return m == null ? false : m.canSendOutgoingStreams(c); + } + @Override public StreamContext getStreamContext(ContactId c, TransportId t) 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 2af01fa4a..5ca159a42 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 @@ -29,6 +29,8 @@ interface TransportKeyManager { void removeContact(ContactId c); + boolean canSendOutgoingStreams(ContactId c); + @Nullable StreamContext getStreamContext(Transaction txn, ContactId c) 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 f6abc1c54..d521531b0 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 @@ -278,6 +278,21 @@ class TransportKeyManagerImpl implements TransportKeyManager { } } + @Override + public boolean canSendOutgoingStreams(ContactId c) { + lock.lock(); + try { + MutableKeySet ks = outContexts.get(c); + if (ks == null) return false; + MutableOutgoingKeys outKeys = + ks.getTransportKeys().getCurrentOutgoingKeys(); + if (!outKeys.isActive()) throw new AssertionError(); + return outKeys.getStreamCounter() <= MAX_32_BIT_UNSIGNED; + } finally { + lock.unlock(); + } + } + @Override public StreamContext getStreamContext(Transaction txn, ContactId c) throws DbException { 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 df708df91..84d6c3465 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 @@ -30,6 +30,7 @@ import java.util.concurrent.Executor; import java.util.concurrent.ScheduledExecutorService; import static java.util.Arrays.asList; +import static java.util.Collections.singletonList; import static java.util.concurrent.TimeUnit.MILLISECONDS; import static org.briarproject.bramble.api.transport.TransportConstants.MAX_CLOCK_DIFFERENCE; import static org.briarproject.bramble.api.transport.TransportConstants.PROTOCOL_VERSION; @@ -37,8 +38,10 @@ import static org.briarproject.bramble.api.transport.TransportConstants.REORDERI import static org.briarproject.bramble.api.transport.TransportConstants.TAG_LENGTH; import static org.briarproject.bramble.util.ByteUtils.MAX_32_BIT_UNSIGNED; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertNull; +import static org.junit.Assert.assertTrue; public class TransportKeyManagerImplTest extends BrambleMockTestCase { @@ -112,6 +115,7 @@ public class TransportKeyManagerImplTest extends BrambleMockTestCase { db, transportCrypto, dbExecutor, scheduler, clock, transportId, maxLatency); transportKeyManager.start(txn); + assertTrue(transportKeyManager.canSendOutgoingStreams(contactId)); } @Test @@ -150,6 +154,7 @@ public class TransportKeyManagerImplTest extends BrambleMockTestCase { long timestamp = rotationPeriodLength * 1000 - 1; transportKeyManager.addContact(txn, contactId, masterKey, timestamp, alice); + assertTrue(transportKeyManager.canSendOutgoingStreams(contactId)); } @Test @@ -181,6 +186,7 @@ public class TransportKeyManagerImplTest extends BrambleMockTestCase { long timestamp = rotationPeriodLength * 1000 - 1; assertEquals(keySetId, transportKeyManager.addUnboundKeys(txn, masterKey, timestamp, alice)); + assertFalse(transportKeyManager.canSendOutgoingStreams(contactId)); } @Test @@ -192,6 +198,7 @@ public class TransportKeyManagerImplTest extends BrambleMockTestCase { db, transportCrypto, dbExecutor, scheduler, clock, transportId, maxLatency); assertNull(transportKeyManager.getStreamContext(txn, contactId)); + assertFalse(transportKeyManager.canSendOutgoingStreams(contactId)); } @Test @@ -212,6 +219,7 @@ public class TransportKeyManagerImplTest extends BrambleMockTestCase { long timestamp = rotationPeriodLength * 1000; transportKeyManager.addContact(txn, contactId, masterKey, timestamp, alice); + assertFalse(transportKeyManager.canSendOutgoingStreams(contactId)); assertNull(transportKeyManager.getStreamContext(txn, contactId)); } @@ -238,6 +246,7 @@ public class TransportKeyManagerImplTest extends BrambleMockTestCase { transportKeyManager.addContact(txn, contactId, masterKey, timestamp, alice); // The first request should return a stream context + assertTrue(transportKeyManager.canSendOutgoingStreams(contactId)); StreamContext ctx = transportKeyManager.getStreamContext(txn, contactId); assertNotNull(ctx); @@ -247,6 +256,7 @@ public class TransportKeyManagerImplTest extends BrambleMockTestCase { assertEquals(headerKey, ctx.getHeaderKey()); assertEquals(MAX_32_BIT_UNSIGNED, ctx.getStreamNumber()); // The second request should return null, the counter is exhausted + assertFalse(transportKeyManager.canSendOutgoingStreams(contactId)); assertNull(transportKeyManager.getStreamContext(txn, contactId)); } @@ -266,6 +276,7 @@ public class TransportKeyManagerImplTest extends BrambleMockTestCase { long timestamp = rotationPeriodLength * 1000; transportKeyManager.addContact(txn, contactId, masterKey, timestamp, alice); + assertTrue(transportKeyManager.canSendOutgoingStreams(contactId)); // The tag should not be recognised assertNull(transportKeyManager.getStreamContext(txn, new byte[TAG_LENGTH])); @@ -316,6 +327,7 @@ public class TransportKeyManagerImplTest extends BrambleMockTestCase { long timestamp = rotationPeriodLength * 1000; transportKeyManager.addContact(txn, contactId, masterKey, timestamp, alice); + assertTrue(transportKeyManager.canSendOutgoingStreams(contactId)); // Use the first tag (previous rotation period, stream number 0) assertEquals(REORDERING_WINDOW_SIZE * 3, tags.size()); byte[] tag = tags.get(0); @@ -336,13 +348,9 @@ public class TransportKeyManagerImplTest extends BrambleMockTestCase { @Test public void testKeysAreRotatedToCurrentPeriod() throws Exception { TransportKeys transportKeys = createTransportKeys(1000, 0, true); - TransportKeys transportKeys1 = createTransportKeys(1000, 0, false); - Collection loaded = asList( - new KeySet(keySetId, contactId, transportKeys), - new KeySet(keySetId1, null, transportKeys1) - ); + Collection loaded = + singletonList(new KeySet(keySetId, contactId, transportKeys)); TransportKeys rotated = createTransportKeys(1001, 0, true); - TransportKeys rotated1 = createTransportKeys(1001, 0, false); Transaction txn = new Transaction(null, false); Transaction txn1 = new Transaction(null, false); @@ -356,8 +364,6 @@ public class TransportKeyManagerImplTest extends BrambleMockTestCase { // Rotate the transport keys (the keys are unaffected) oneOf(transportCrypto).rotateTransportKeys(transportKeys, 1000); will(returnValue(transportKeys)); - oneOf(transportCrypto).rotateTransportKeys(transportKeys1, 1000); - will(returnValue(transportKeys1)); // Encode the tags (3 sets) for (long i = 0; i < REORDERING_WINDOW_SIZE; i++) { exactly(3).of(transportCrypto).encodeTag( @@ -381,9 +387,6 @@ public class TransportKeyManagerImplTest extends BrambleMockTestCase { oneOf(transportCrypto).rotateTransportKeys( with(any(TransportKeys.class)), with(1001L)); will(returnValue(rotated)); - oneOf(transportCrypto).rotateTransportKeys( - with(any(TransportKeys.class)), with(1001L)); - will(returnValue(rotated1)); // Encode the tags (3 sets) for (long i = 0; i < REORDERING_WINDOW_SIZE; i++) { exactly(3).of(transportCrypto).encodeTag( @@ -392,10 +395,8 @@ public class TransportKeyManagerImplTest extends BrambleMockTestCase { will(new EncodeTagAction()); } // Save the keys that were rotated - oneOf(db).updateTransportKeys(txn1, asList( - new KeySet(keySetId1, null, rotated1), - new KeySet(keySetId, contactId, rotated) - )); + oneOf(db).updateTransportKeys(txn1, + singletonList(new KeySet(keySetId, contactId, rotated))); // Schedule key rotation at the start of the next rotation period oneOf(scheduler).schedule(with(any(Runnable.class)), with(rotationPeriodLength), with(MILLISECONDS)); @@ -408,6 +409,7 @@ public class TransportKeyManagerImplTest extends BrambleMockTestCase { db, transportCrypto, dbExecutor, scheduler, clock, transportId, maxLatency); transportKeyManager.start(txn); + assertTrue(transportKeyManager.canSendOutgoingStreams(contactId)); } @Test @@ -451,11 +453,16 @@ public class TransportKeyManagerImplTest extends BrambleMockTestCase { long timestamp = rotationPeriodLength * 1000; assertEquals(keySetId, transportKeyManager.addUnboundKeys(txn, masterKey, timestamp, alice)); + // The keys are unbound so no stream context should be returned + assertFalse(transportKeyManager.canSendOutgoingStreams(contactId)); + assertNull(transportKeyManager.getStreamContext(txn, contactId)); transportKeyManager.bindKeys(txn, contactId, keySetId); // The keys are inactive so no stream context should be returned + assertFalse(transportKeyManager.canSendOutgoingStreams(contactId)); assertNull(transportKeyManager.getStreamContext(txn, contactId)); transportKeyManager.activateKeys(txn, keySetId); // The keys are active so a stream context should be returned + assertTrue(transportKeyManager.canSendOutgoingStreams(contactId)); StreamContext ctx = transportKeyManager.getStreamContext(txn, contactId); assertNotNull(ctx); @@ -496,7 +503,9 @@ public class TransportKeyManagerImplTest extends BrambleMockTestCase { long timestamp = rotationPeriodLength * 1000; assertEquals(keySetId, transportKeyManager.addUnboundKeys(txn, masterKey, timestamp, alice)); + assertFalse(transportKeyManager.canSendOutgoingStreams(contactId)); transportKeyManager.removeKeys(txn, keySetId); + assertFalse(transportKeyManager.canSendOutgoingStreams(contactId)); } private void expectAddContactNoRotation(boolean alice, From b81058d6dae3a77cb0a6fa93347816fc291994da Mon Sep 17 00:00:00 2001 From: akwizgran Date: Wed, 28 Mar 2018 12:26:12 +0100 Subject: [PATCH 11/11] Activate outgoing keys when incoming tag is recognised. --- .../transport/TransportKeyManagerImpl.java | 10 ++ .../TransportKeyManagerImplTest.java | 118 ++++++++++++++---- 2 files changed, 102 insertions(+), 26 deletions(-) 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 d521531b0..63bcdbc77 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 @@ -355,6 +355,16 @@ class TransportKeyManagerImpl implements TransportKeyManager { db.setReorderingWindow(txn, tagCtx.keySetId, transportId, inKeys.getRotationPeriod(), window.getBase(), window.getBitmap()); + // If the outgoing keys are inactive, activate them + MutableKeySet ks = keys.get(tagCtx.keySetId); + MutableOutgoingKeys outKeys = + ks.getTransportKeys().getCurrentOutgoingKeys(); + if (!outKeys.isActive()) { + LOG.info("Activating outgoing keys"); + outKeys.activate(); + considerReplacingOutgoingKeys(ks); + db.setTransportKeysActive(txn, transportId, tagCtx.keySetId); + } return ctx; } finally { lock.unlock(); 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 84d6c3465..21350ae6d 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 @@ -286,9 +286,10 @@ public class TransportKeyManagerImplTest extends BrambleMockTestCase { public void testTagIsNotRecognisedTwice() throws Exception { boolean alice = random.nextBoolean(); TransportKeys transportKeys = createTransportKeys(1000, 0, true); + Transaction txn = new Transaction(null, false); + // Keep a copy of the tags List tags = new ArrayList<>(); - Transaction txn = new Transaction(null, false); context.checking(new Expectations() {{ oneOf(transportCrypto).deriveTransportKeys(transportId, masterKey, @@ -418,19 +419,9 @@ public class TransportKeyManagerImplTest extends BrambleMockTestCase { TransportKeys transportKeys = createTransportKeys(1000, 0, false); Transaction txn = new Transaction(null, false); + expectAddUnboundKeysNoRotation(alice, transportKeys, txn); + context.checking(new Expectations() {{ - oneOf(transportCrypto).deriveTransportKeys(transportId, masterKey, - 1000, alice, false); - will(returnValue(transportKeys)); - // Get the current time (the start of rotation period 1000) - oneOf(clock).currentTimeMillis(); - will(returnValue(rotationPeriodLength * 1000)); - // Rotate the transport keys (the keys are unaffected) - oneOf(transportCrypto).rotateTransportKeys(transportKeys, 1000); - will(returnValue(transportKeys)); - // Save the unbound keys - oneOf(db).addTransportKeys(txn, null, transportKeys); - will(returnValue(keySetId)); // When the keys are bound, encode the tags (3 sets) for (long i = 0; i < REORDERING_WINDOW_SIZE; i++) { exactly(3).of(transportCrypto).encodeTag( @@ -470,7 +461,74 @@ public class TransportKeyManagerImplTest extends BrambleMockTestCase { assertEquals(transportId, ctx.getTransportId()); assertEquals(tagKey, ctx.getTagKey()); assertEquals(headerKey, ctx.getHeaderKey()); - assertEquals(0, ctx.getStreamNumber()); + assertEquals(0L, ctx.getStreamNumber()); + } + + @Test + public void testRecognisingTagActivatesOutgoingKeys() throws Exception { + boolean alice = random.nextBoolean(); + TransportKeys transportKeys = createTransportKeys(1000, 0, false); + Transaction txn = new Transaction(null, false); + + // Keep a copy of the tags + List tags = new ArrayList<>(); + + expectAddUnboundKeysNoRotation(alice, transportKeys, txn); + + context.checking(new Expectations() {{ + // When the keys are bound, 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(tags)); + } + // Save the key binding + oneOf(db).bindTransportKeys(txn, contactId, transportId, keySetId); + // Encode a new tag after sliding the window + oneOf(transportCrypto).encodeTag(with(any(byte[].class)), + with(tagKey), with(PROTOCOL_VERSION), + with((long) REORDERING_WINDOW_SIZE)); + will(new EncodeTagAction(tags)); + // Save the reordering window (previous rotation period, base 1) + oneOf(db).setReorderingWindow(txn, keySetId, transportId, 999, + 1, new byte[REORDERING_WINDOW_SIZE / 8]); + // Activate the keys + oneOf(db).setTransportKeysActive(txn, transportId, keySetId); + // Increment the stream counter + oneOf(db).incrementStreamCounter(txn, transportId, keySetId); + }}); + + TransportKeyManager transportKeyManager = new TransportKeyManagerImpl( + db, transportCrypto, dbExecutor, scheduler, clock, transportId, + maxLatency); + // The timestamp is at the start of rotation period 1000 + long timestamp = rotationPeriodLength * 1000; + assertEquals(keySetId, transportKeyManager.addUnboundKeys(txn, + masterKey, timestamp, alice)); + transportKeyManager.bindKeys(txn, contactId, keySetId); + // The keys are inactive so no stream context should be returned + assertFalse(transportKeyManager.canSendOutgoingStreams(contactId)); + assertNull(transportKeyManager.getStreamContext(txn, contactId)); + // Recognising an incoming tag should activate the outgoing keys + assertEquals(REORDERING_WINDOW_SIZE * 3, tags.size()); + byte[] tag = tags.get(0); + StreamContext ctx = transportKeyManager.getStreamContext(txn, tag); + assertNotNull(ctx); + assertEquals(contactId, ctx.getContactId()); + assertEquals(transportId, ctx.getTransportId()); + assertEquals(tagKey, ctx.getTagKey()); + assertEquals(headerKey, ctx.getHeaderKey()); + assertEquals(0L, ctx.getStreamNumber()); + // The keys are active so a stream context should be returned + assertTrue(transportKeyManager.canSendOutgoingStreams(contactId)); + ctx = transportKeyManager.getStreamContext(txn, contactId); + assertNotNull(ctx); + assertEquals(contactId, ctx.getContactId()); + assertEquals(transportId, ctx.getTransportId()); + assertEquals(tagKey, ctx.getTagKey()); + assertEquals(headerKey, ctx.getHeaderKey()); + assertEquals(0L, ctx.getStreamNumber()); } @Test @@ -479,19 +537,9 @@ public class TransportKeyManagerImplTest extends BrambleMockTestCase { TransportKeys transportKeys = createTransportKeys(1000, 0, false); Transaction txn = new Transaction(null, false); + expectAddUnboundKeysNoRotation(alice, transportKeys, txn); + context.checking(new Expectations() {{ - oneOf(transportCrypto).deriveTransportKeys(transportId, masterKey, - 1000, alice, false); - will(returnValue(transportKeys)); - // Get the current time (the start of rotation period 1000) - oneOf(clock).currentTimeMillis(); - will(returnValue(rotationPeriodLength * 1000)); - // Rotate the transport keys (the keys are unaffected) - oneOf(transportCrypto).rotateTransportKeys(transportKeys, 1000); - will(returnValue(transportKeys)); - // Save the unbound keys - oneOf(db).addTransportKeys(txn, null, transportKeys); - will(returnValue(keySetId)); // Remove the unbound keys oneOf(db).removeTransportKeys(txn, transportId, keySetId); }}); @@ -533,6 +581,24 @@ public class TransportKeyManagerImplTest extends BrambleMockTestCase { }}); } + private void expectAddUnboundKeysNoRotation(boolean alice, + TransportKeys transportKeys, Transaction txn) throws Exception { + context.checking(new Expectations() {{ + oneOf(transportCrypto).deriveTransportKeys(transportId, masterKey, + 1000, alice, false); + will(returnValue(transportKeys)); + // Get the current time (the start of rotation period 1000) + oneOf(clock).currentTimeMillis(); + will(returnValue(rotationPeriodLength * 1000)); + // Rotate the transport keys (the keys are unaffected) + oneOf(transportCrypto).rotateTransportKeys(transportKeys, 1000); + will(returnValue(transportKeys)); + // Save the unbound keys + oneOf(db).addTransportKeys(txn, null, transportKeys); + will(returnValue(keySetId)); + }}); + } + private TransportKeys createTransportKeys(long rotationPeriod, long streamCounter, boolean active) { IncomingKeys inPrev = new IncomingKeys(tagKey, headerKey,