Commit 5c068ed0 authored by Torsten Grote's avatar Torsten Grote

Merge branch 'remove-unbound-transport-keys' into 'master'

Remove support for unbound transport keys

See merge request akwizgran/briar!791
parents 6ed16802 fec384c2
Pipeline #1503 passed with stage
in 13 minutes and 5 seconds
......@@ -45,9 +45,9 @@ public interface ContactManager {
*
* @param alice true if the local party is Alice
*/
ContactId addContact(Author remote, AuthorId local,
SecretKey master, long timestamp, boolean alice, boolean verified,
boolean active) throws DbException;
ContactId addContact(Author remote, AuthorId local, SecretKey master,
long timestamp, boolean alice, boolean verified, boolean active)
throws DbException;
/**
* Returns the contact with the given ID.
......
......@@ -104,18 +104,12 @@ public interface DatabaseComponent {
throws DbException;
/**
* Stores the given transport keys, optionally binding them to the given
* contact, and returns a key set ID.
* Stores the given transport keys for the given contact and returns a
* key set ID.
*/
KeySetId addTransportKeys(Transaction txn, @Nullable ContactId c,
KeySetId addTransportKeys(Transaction txn, 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.
......
......@@ -19,48 +19,24 @@ public interface KeyManager {
/**
* Informs the key manager that a new contact has been added. Derives and
* stores a set of transport keys for communicating with the contact over
* each transport.
* each transport and returns the key set IDs.
* <p/>
* {@link StreamContext StreamContexts} for the contact can be created
* after this method has returned.
*
* @param alice true if the local party is Alice
* @param active whether the derived keys can be used for outgoing streams
*/
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
* and returns the key set IDs.
* <p/>
* The keys must be bound before they can be used for incoming streams,
* and also activated before they can be used for outgoing streams.
*
* @param alice true if the local party is Alice
*/
Map<TransportId, KeySetId> 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<TransportId, KeySetId> keys)
Map<TransportId, KeySetId> addContact(Transaction txn, ContactId c,
SecretKey master, long timestamp, boolean alice, boolean active)
throws DbException;
/**
* Marks the given transport keys as usable for outgoing streams. Keys must
* be bound before they are activated.
* Marks the given transport keys as usable for outgoing streams.
*/
void activateKeys(Transaction txn, Map<TransportId, KeySetId> 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<TransportId, KeySetId> keys)
throws DbException;
/**
* Returns true if we have keys that can be used for outgoing streams to
* the given contact over the given transport.
......
......@@ -3,23 +3,20 @@ 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.
* A set of transport keys for communicating with a contact.
*/
@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,
public KeySet(KeySetId keySetId, ContactId contactId,
TransportKeys transportKeys) {
this.keySetId = keySetId;
this.contactId = contactId;
......@@ -30,7 +27,6 @@ public class KeySet {
return keySetId;
}
@Nullable
public ContactId getContactId() {
return contactId;
}
......
......@@ -46,7 +46,7 @@ class ContactManagerImpl implements ContactManager {
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);
keyManager.addContact(txn, c, master, timestamp, alice, active);
Contact contact = db.getContact(txn, c);
for (ContactHook hook : hooks) hook.addingContact(txn, contact);
return c;
......
......@@ -125,16 +125,10 @@ interface Database<T> {
throws DbException;
/**
* Stores the given transport keys, optionally binding them to the given
* contact, and returns a key set ID.
* Stores the given transport keys for the given contact and returns a
* key set ID.
*/
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)
KeySetId addTransportKeys(T txn, ContactId c, TransportKeys k)
throws DbException;
/**
......
......@@ -234,29 +234,17 @@ class DatabaseComponentImpl<T> implements DatabaseComponent {
}
@Override
public KeySetId addTransportKeys(Transaction transaction,
@Nullable ContactId c, TransportKeys k) throws DbException {
public KeySetId addTransportKeys(Transaction transaction, ContactId c,
TransportKeys k) throws DbException {
if (transaction.isReadOnly()) throw new IllegalArgumentException();
T txn = unbox(transaction);
if (c != null && !db.containsContact(txn, c))
if (!db.containsContact(txn, c))
throw new NoSuchContactException();
if (!db.containsTransport(txn, k.getTransportId()))
throw new NoSuchTransportException();
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 {
......
......@@ -53,6 +53,7 @@ import java.util.logging.Logger;
import javax.annotation.Nullable;
import static java.sql.Types.INTEGER;
import static java.util.Collections.singletonList;
import static java.util.logging.Level.INFO;
import static java.util.logging.Level.WARNING;
import static org.briarproject.bramble.api.db.Metadata.REMOVE;
......@@ -74,7 +75,7 @@ import static org.briarproject.bramble.db.ExponentialBackoff.calculateExpiry;
abstract class JdbcDatabase implements Database<Connection> {
// Package access for testing
static final int CODE_SCHEMA_VERSION = 38;
static final int CODE_SCHEMA_VERSION = 39;
// Rotation period offsets for incoming transport keys
private static final int OFFSET_PREV = -1;
......@@ -236,7 +237,7 @@ abstract class JdbcDatabase implements Database<Connection> {
+ " (transportId _STRING NOT NULL,"
+ " keySetId _COUNTER,"
+ " rotationPeriod BIGINT NOT NULL,"
+ " contactId INT," // Null if keys are not bound
+ " contactId INT NOT NULL,"
+ " tagKey _SECRET NOT NULL,"
+ " headerKey _SECRET NOT NULL,"
+ " stream BIGINT NOT NULL,"
......@@ -255,7 +256,7 @@ abstract class JdbcDatabase implements Database<Connection> {
+ " (transportId _STRING NOT NULL,"
+ " keySetId INT NOT NULL,"
+ " rotationPeriod BIGINT NOT NULL,"
+ " contactId INT," // Null if keys are not bound
+ " contactId INT NOT NULL,"
+ " tagKey _SECRET NOT NULL,"
+ " headerKey _SECRET NOT NULL,"
+ " base BIGINT NOT NULL,"
......@@ -389,7 +390,7 @@ abstract class JdbcDatabase implements Database<Connection> {
// Package access for testing
List<Migration<Connection>> getMigrations() {
return Collections.emptyList();
return singletonList(new Migration38_39());
}
private void storeSchemaVersion(Connection txn, int version)
......@@ -883,7 +884,7 @@ abstract class JdbcDatabase implements Database<Connection> {
}
@Override
public KeySetId addTransportKeys(Connection txn, @Nullable ContactId c,
public KeySetId addTransportKeys(Connection txn, ContactId c,
TransportKeys k) throws DbException {
PreparedStatement ps = null;
ResultSet rs = null;
......@@ -893,8 +894,7 @@ abstract class JdbcDatabase implements Database<Connection> {
+ " rotationPeriod, tagKey, headerKey, stream, active)"
+ " VALUES (?, ?, ?, ?, ?, ?, ?)";
ps = txn.prepareStatement(sql);
if (c == null) ps.setNull(1, INTEGER);
else ps.setInt(1, c.getInt());
ps.setInt(1, c.getInt());
ps.setString(2, k.getTransportId().getString());
OutgoingKeys outCurr = k.getCurrentOutgoingKeys();
ps.setLong(3, outCurr.getRotationPeriod());
......@@ -922,8 +922,7 @@ abstract class JdbcDatabase implements Database<Connection> {
+ " VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?)";
ps = txn.prepareStatement(sql);
ps.setInt(1, keySetId.getInt());
if (c == null) ps.setNull(2, INTEGER);
else ps.setInt(2, c.getInt());
ps.setInt(2, c.getInt());
ps.setString(3, k.getTransportId().getString());
// Previous rotation period
IncomingKeys inPrev = k.getPreviousIncomingKeys();
......@@ -965,33 +964,6 @@ abstract class JdbcDatabase implements Database<Connection> {
}
}
@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 {
......@@ -2172,7 +2144,6 @@ abstract class JdbcDatabase implements Database<Connection> {
if (inKeys.size() < (i + 1) * 3) throw new DbStateException();
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));
......
package org.briarproject.bramble.db;
import org.briarproject.bramble.api.db.DbException;
import java.sql.Connection;
import java.sql.SQLException;
import java.sql.Statement;
import java.util.logging.Logger;
import javax.annotation.Nullable;
import static java.util.logging.Level.WARNING;
class Migration38_39 implements Migration<Connection> {
private static final Logger LOG =
Logger.getLogger(Migration38_39.class.getName());
@Override
public int getStartVersion() {
return 38;
}
@Override
public int getEndVersion() {
return 39;
}
@Override
public void migrate(Connection txn) throws DbException {
Statement s = null;
try {
s = txn.createStatement();
// Add not null constraints
s.execute("ALTER TABLE outgoingKeys"
+ " ALTER COLUMN contactId"
+ " SET NOT NULL");
s.execute("ALTER TABLE incomingKeys"
+ " ALTER COLUMN contactId"
+ " SET NOT NULL");
} catch (SQLException e) {
tryToClose(s);
throw new DbException(e);
}
}
private void tryToClose(@Nullable Statement s) {
try {
if (s != null) s.close();
} catch (SQLException e) {
if (LOG.isLoggable(WARNING)) LOG.log(WARNING, e.toString(), e);
}
}
}
......@@ -99,39 +99,18 @@ class KeyManagerImpl implements KeyManager, Service, EventListener {
}
@Override
public void addContact(Transaction txn, ContactId c, SecretKey master,
long timestamp, boolean alice) throws DbException {
for (TransportKeyManager m : managers.values())
m.addContact(txn, c, master, timestamp, alice);
}
@Override
public Map<TransportId, KeySetId> addUnboundKeys(Transaction txn,
SecretKey master, long timestamp, boolean alice)
public Map<TransportId, KeySetId> addContact(Transaction txn, ContactId c,
SecretKey master, long timestamp, boolean alice, boolean active)
throws DbException {
Map<TransportId, KeySetId> ids = new HashMap<>();
for (Entry<TransportId, TransportKeyManager> e : managers.entrySet()) {
TransportId t = e.getKey();
TransportKeyManager m = e.getValue();
ids.put(t, m.addUnboundKeys(txn, master, timestamp, alice));
ids.put(t, m.addContact(txn, c, master, timestamp, alice, active));
}
return ids;
}
@Override
public void bindKeys(Transaction txn, ContactId c,
Map<TransportId, KeySetId> keys) throws DbException {
for (Entry<TransportId, KeySetId> 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 void activateKeys(Transaction txn, Map<TransportId, KeySetId> keys)
throws DbException {
......@@ -146,24 +125,10 @@ class KeyManagerImpl implements KeyManager, Service, EventListener {
}
}
@Override
public void removeKeys(Transaction txn, Map<TransportId, KeySetId> keys)
throws DbException {
for (Entry<TransportId, KeySetId> 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 boolean canSendOutgoingStreams(ContactId c, TransportId t) {
TransportKeyManager m = managers.get(t);
return m == null ? false : m.canSendOutgoingStreams(c);
return m != null && m.canSendOutgoingStreams(c);
}
@Override
......
......@@ -3,32 +3,28 @@ 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 {
class MutableKeySet {
private final KeySetId keySetId;
@Nullable
private final ContactId contactId;
private final MutableTransportKeys transportKeys;
public MutableKeySet(KeySetId keySetId, @Nullable ContactId contactId,
MutableKeySet(KeySetId keySetId, ContactId contactId,
MutableTransportKeys transportKeys) {
this.keySetId = keySetId;
this.contactId = contactId;
this.transportKeys = transportKeys;
}
public KeySetId getKeySetId() {
KeySetId getKeySetId() {
return keySetId;
}
@Nullable
public ContactId getContactId() {
ContactId getContactId() {
return contactId;
}
public MutableTransportKeys getTransportKeys() {
MutableTransportKeys getTransportKeys() {
return transportKeys;
}
}
......@@ -15,18 +15,11 @@ interface TransportKeyManager {
void start(Transaction txn) throws DbException;
void addContact(Transaction txn, ContactId c, SecretKey master,
long timestamp, boolean alice) throws DbException;
KeySetId addUnboundKeys(Transaction txn, SecretKey master, long timestamp,
boolean alice) throws DbException;
void bindKeys(Transaction txn, ContactId c, KeySetId k) throws DbException;
KeySetId addContact(Transaction txn, ContactId c, SecretKey master,
long timestamp, boolean alice, boolean active) throws DbException;
void activateKeys(Transaction txn, KeySetId k) throws DbException;
void removeKeys(Transaction txn, KeySetId k) throws DbException;
void removeContact(ContactId c);
boolean canSendOutgoingStreams(ContactId c);
......
......@@ -28,7 +28,6 @@ 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;
......@@ -119,16 +118,14 @@ class TransportKeyManagerImpl implements TransportKeyManager {
}
// Locking: lock
private void addKeys(KeySetId keySetId, @Nullable ContactId contactId,
private void addKeys(KeySetId keySetId, ContactId contactId,
MutableTransportKeys m) {
MutableKeySet ks = new MutableKeySet(keySetId, contactId, m);
keys.put(keySetId, ks);
if (contactId != null) {
encodeTags(keySetId, contactId, m.getPreviousIncomingKeys());
encodeTags(keySetId, contactId, m.getCurrentIncomingKeys());
encodeTags(keySetId, contactId, m.getNextIncomingKeys());
considerReplacingOutgoingKeys(ks);
}
encodeTags(keySetId, contactId, m.getPreviousIncomingKeys());
encodeTags(keySetId, contactId, m.getCurrentIncomingKeys());
encodeTags(keySetId, contactId, m.getNextIncomingKeys());
considerReplacingOutgoingKeys(ks);
}
// Locking: lock
......@@ -150,8 +147,9 @@ class TransportKeyManagerImpl implements TransportKeyManager {
if (ks.getTransportKeys().getCurrentOutgoingKeys().isActive()) {
MutableKeySet old = outContexts.get(ks.getContactId());
if (old == null ||
old.getKeySetId().getInt() < ks.getKeySetId().getInt())
old.getKeySetId().getInt() < ks.getKeySetId().getInt()) {
outContexts.put(ks.getContactId(), ks);
}
}
}
......@@ -177,20 +175,8 @@ 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, true);
}
@Override
public KeySetId addUnboundKeys(Transaction txn, SecretKey master,
long timestamp, boolean alice) throws DbException {
return deriveAndAddKeys(txn, null, master, timestamp, alice, false);
}
private KeySetId deriveAndAddKeys(Transaction txn, @Nullable ContactId c,
SecretKey master, long timestamp, boolean alice, boolean active)
throws DbException {
public KeySetId addContact(Transaction txn, ContactId c, SecretKey master,
long timestamp, boolean alice, boolean active) throws DbException {
lock.lock();
try {
// Work out what rotation period the timestamp belongs to
......@@ -211,31 +197,12 @@ 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();
// Check that the keys haven't already been bound
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 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);
......@@ -245,21 +212,6 @@ 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();
// Check that the keys haven't been bound
if (ks.getContactId() != null) throw new IllegalArgumentException();
TransportId t = ks.getTransportKeys().getTransportId();