mirror of
https://code.briarproject.org/briar/briar.git
synced 2026-02-19 06:09:55 +01:00
Fixed a race conditon when adding a transport and then an endpoint.
To fix issue #3611966, KeyManagerImpl's handling of TransportAddedEvent was made asynchronous. This made it possible for a thread to call KeyManager.endpointAdded() before the KeyManager had asynchronously handled the TransportAddedEvent from a previous call to DatabaseComponent.addTransport().
This commit is contained in:
@@ -27,5 +27,5 @@ public interface KeyManager {
|
|||||||
* Called whenever an endpoint has been added. The initial secret
|
* Called whenever an endpoint has been added. The initial secret
|
||||||
* is erased before returning.
|
* is erased before returning.
|
||||||
*/
|
*/
|
||||||
void endpointAdded(Endpoint ep, byte[] initialSecret);
|
void endpointAdded(Endpoint ep, long maxLatency, byte[] initialSecret);
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -288,19 +288,25 @@ abstract class Connector extends Thread {
|
|||||||
db.setRemoteProperties(contactId, remoteProps);
|
db.setRemoteProperties(contactId, remoteProps);
|
||||||
// Create an endpoint for each transport shared with the contact
|
// Create an endpoint for each transport shared with the contact
|
||||||
List<TransportId> ids = new ArrayList<TransportId>();
|
List<TransportId> ids = new ArrayList<TransportId>();
|
||||||
for(TransportId id : localProps.keySet())
|
Map<TransportId, Long> latencies = db.getTransportLatencies();
|
||||||
if(remoteProps.containsKey(id)) ids.add(id);
|
for(TransportId id : localProps.keySet()) {
|
||||||
|
if(latencies.containsKey(id) && remoteProps.containsKey(id))
|
||||||
|
ids.add(id);
|
||||||
|
}
|
||||||
// Assign indices to the transports deterministically and derive keys
|
// Assign indices to the transports deterministically and derive keys
|
||||||
Collections.sort(ids, TransportIdComparator.INSTANCE);
|
Collections.sort(ids, TransportIdComparator.INSTANCE);
|
||||||
int size = ids.size();
|
int size = ids.size();
|
||||||
for(int i = 0; i < size; i++) {
|
for(int i = 0; i < size; i++) {
|
||||||
Endpoint ep = new Endpoint(contactId, ids.get(i), epoch, alice);
|
TransportId id = ids.get(i);
|
||||||
|
Endpoint ep = new Endpoint(contactId, id, epoch, alice);
|
||||||
|
long maxLatency = latencies.get(id);
|
||||||
try {
|
try {
|
||||||
db.addEndpoint(ep);
|
db.addEndpoint(ep);
|
||||||
} catch(NoSuchTransportException e) {
|
} catch(NoSuchTransportException e) {
|
||||||
continue;
|
continue;
|
||||||
}
|
}
|
||||||
keyManager.endpointAdded(ep, crypto.deriveInitialSecret(secret, i));
|
byte[] initialSecret = crypto.deriveInitialSecret(secret, i);
|
||||||
|
keyManager.endpointAdded(ep, maxLatency, initialSecret);
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
@@ -252,13 +252,9 @@ class KeyManagerImpl extends TimerTask implements KeyManager, DatabaseListener {
|
|||||||
return new ConnectionContext(c, t, secret, connection, s.getAlice());
|
return new ConnectionContext(c, t, secret, connection, s.getAlice());
|
||||||
}
|
}
|
||||||
|
|
||||||
public synchronized void endpointAdded(Endpoint ep, byte[] initialSecret) {
|
public synchronized void endpointAdded(Endpoint ep, long maxLatency,
|
||||||
Long maxLatency = maxLatencies.get(ep.getTransportId());
|
byte[] initialSecret) {
|
||||||
if(maxLatency == null) {
|
maxLatencies.put(ep.getTransportId(), maxLatency);
|
||||||
if(LOG.isLoggable(INFO))
|
|
||||||
LOG.info("No such transport, ignoring endpoint");
|
|
||||||
return;
|
|
||||||
}
|
|
||||||
// Work out which rotation period we're in
|
// Work out which rotation period we're in
|
||||||
long elapsed = clock.currentTimeMillis() - ep.getEpoch();
|
long elapsed = clock.currentTimeMillis() - ep.getEpoch();
|
||||||
long rotation = maxLatency + MAX_CLOCK_DIFFERENCE;
|
long rotation = maxLatency + MAX_CLOCK_DIFFERENCE;
|
||||||
|
|||||||
@@ -55,7 +55,7 @@
|
|||||||
</javac>
|
</javac>
|
||||||
</target>
|
</target>
|
||||||
<target name='test' depends='compile'>
|
<target name='test' depends='compile'>
|
||||||
<junit printsummary='on' fork='yes' forkmode='once'>
|
<junit showoutput='yes' printsummary='on' fork='yes' forkmode='once'>
|
||||||
<assertions>
|
<assertions>
|
||||||
<enable/>
|
<enable/>
|
||||||
</assertions>
|
</assertions>
|
||||||
|
|||||||
@@ -125,7 +125,7 @@ public class SimplexMessagingIntegrationTest extends BriarTestCase {
|
|||||||
db.addTransport(transportId, LATENCY);
|
db.addTransport(transportId, LATENCY);
|
||||||
Endpoint ep = new Endpoint(contactId, transportId, epoch, true);
|
Endpoint ep = new Endpoint(contactId, transportId, epoch, true);
|
||||||
db.addEndpoint(ep);
|
db.addEndpoint(ep);
|
||||||
km.endpointAdded(ep, initialSecret.clone());
|
km.endpointAdded(ep, LATENCY, initialSecret.clone());
|
||||||
// Send Bob a message
|
// Send Bob a message
|
||||||
String contentType = "text/plain";
|
String contentType = "text/plain";
|
||||||
byte[] body = "Hi Bob!".getBytes("UTF-8");
|
byte[] body = "Hi Bob!".getBytes("UTF-8");
|
||||||
@@ -179,7 +179,7 @@ public class SimplexMessagingIntegrationTest extends BriarTestCase {
|
|||||||
db.addTransport(transportId, LATENCY);
|
db.addTransport(transportId, LATENCY);
|
||||||
Endpoint ep = new Endpoint(contactId, transportId, epoch, false);
|
Endpoint ep = new Endpoint(contactId, transportId, epoch, false);
|
||||||
db.addEndpoint(ep);
|
db.addEndpoint(ep);
|
||||||
km.endpointAdded(ep, initialSecret.clone());
|
km.endpointAdded(ep, LATENCY, initialSecret.clone());
|
||||||
// Set up a database listener
|
// Set up a database listener
|
||||||
MessageListener listener = new MessageListener();
|
MessageListener listener = new MessageListener();
|
||||||
db.addListener(listener);
|
db.addListener(listener);
|
||||||
|
|||||||
@@ -141,7 +141,7 @@ public class KeyManagerImplTest extends BriarTestCase {
|
|||||||
}});
|
}});
|
||||||
|
|
||||||
assertTrue(keyManager.start());
|
assertTrue(keyManager.start());
|
||||||
keyManager.endpointAdded(ep, initialSecret.clone());
|
keyManager.endpointAdded(ep, MAX_LATENCY, initialSecret.clone());
|
||||||
keyManager.stop();
|
keyManager.stop();
|
||||||
|
|
||||||
context.assertIsSatisfied();
|
context.assertIsSatisfied();
|
||||||
@@ -202,7 +202,7 @@ public class KeyManagerImplTest extends BriarTestCase {
|
|||||||
}});
|
}});
|
||||||
|
|
||||||
assertTrue(keyManager.start());
|
assertTrue(keyManager.start());
|
||||||
keyManager.endpointAdded(ep, initialSecret.clone());
|
keyManager.endpointAdded(ep, MAX_LATENCY, initialSecret.clone());
|
||||||
ConnectionContext ctx =
|
ConnectionContext ctx =
|
||||||
keyManager.getConnectionContext(contactId, transportId);
|
keyManager.getConnectionContext(contactId, transportId);
|
||||||
assertNotNull(ctx);
|
assertNotNull(ctx);
|
||||||
|
|||||||
@@ -239,7 +239,7 @@ public class KeyRotationIntegrationTest extends BriarTestCase {
|
|||||||
}});
|
}});
|
||||||
|
|
||||||
assertTrue(keyManager.start());
|
assertTrue(keyManager.start());
|
||||||
keyManager.endpointAdded(ep, initialSecret.clone());
|
keyManager.endpointAdded(ep, MAX_LATENCY, initialSecret.clone());
|
||||||
keyManager.stop();
|
keyManager.stop();
|
||||||
|
|
||||||
context.assertIsSatisfied();
|
context.assertIsSatisfied();
|
||||||
@@ -377,7 +377,7 @@ public class KeyRotationIntegrationTest extends BriarTestCase {
|
|||||||
}});
|
}});
|
||||||
|
|
||||||
assertTrue(keyManager.start());
|
assertTrue(keyManager.start());
|
||||||
keyManager.endpointAdded(ep, initialSecret.clone());
|
keyManager.endpointAdded(ep, MAX_LATENCY, initialSecret.clone());
|
||||||
ConnectionContext ctx =
|
ConnectionContext ctx =
|
||||||
keyManager.getConnectionContext(contactId, transportId);
|
keyManager.getConnectionContext(contactId, transportId);
|
||||||
assertNotNull(ctx);
|
assertNotNull(ctx);
|
||||||
@@ -533,7 +533,7 @@ public class KeyRotationIntegrationTest extends BriarTestCase {
|
|||||||
}});
|
}});
|
||||||
|
|
||||||
assertTrue(keyManager.start());
|
assertTrue(keyManager.start());
|
||||||
keyManager.endpointAdded(ep, initialSecret.clone());
|
keyManager.endpointAdded(ep, MAX_LATENCY, initialSecret.clone());
|
||||||
// Recognise the tag for connection 0 in period 2
|
// Recognise the tag for connection 0 in period 2
|
||||||
byte[] tag = new byte[TAG_LENGTH];
|
byte[] tag = new byte[TAG_LENGTH];
|
||||||
encodeTag(tag, key2, 0);
|
encodeTag(tag, key2, 0);
|
||||||
|
|||||||
Reference in New Issue
Block a user