From dddd15cd10239e1cfcc97694ac029870e1828f63 Mon Sep 17 00:00:00 2001
From: akwizgran <michael@briarproject.org>
Date: Tue, 14 May 2013 20:54:23 +0100
Subject: [PATCH] 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().
---
.../src/net/sf/briar/api/crypto/KeyManager.java | 2 +-
.../src/net/sf/briar/invitation/Connector.java | 14 ++++++++++----
.../src/net/sf/briar/transport/KeyManagerImpl.java | 10 +++-------
briar-tests/build.xml | 2 +-
.../simplex/SimplexMessagingIntegrationTest.java | 4 ++--
.../net/sf/briar/transport/KeyManagerImplTest.java | 4 ++--
.../transport/KeyRotationIntegrationTest.java | 6 +++---
7 files changed, 22 insertions(+), 20 deletions(-)
diff --git a/briar-api/src/net/sf/briar/api/crypto/KeyManager.java b/briar-api/src/net/sf/briar/api/crypto/KeyManager.java
index 8964f665f8..781ad3b139 100644
--- a/briar-api/src/net/sf/briar/api/crypto/KeyManager.java
+++ b/briar-api/src/net/sf/briar/api/crypto/KeyManager.java
@@ -27,5 +27,5 @@ public interface KeyManager {
* Called whenever an endpoint has been added. The initial secret
* is erased before returning.
*/
- void endpointAdded(Endpoint ep, byte[] initialSecret);
+ void endpointAdded(Endpoint ep, long maxLatency, byte[] initialSecret);
}
diff --git a/briar-core/src/net/sf/briar/invitation/Connector.java b/briar-core/src/net/sf/briar/invitation/Connector.java
index e54f1518d8..c58d9d4b3c 100644
--- a/briar-core/src/net/sf/briar/invitation/Connector.java
+++ b/briar-core/src/net/sf/briar/invitation/Connector.java
@@ -288,19 +288,25 @@ abstract class Connector extends Thread {
db.setRemoteProperties(contactId, remoteProps);
// Create an endpoint for each transport shared with the contact
List<TransportId> ids = new ArrayList<TransportId>();
- for(TransportId id : localProps.keySet())
- if(remoteProps.containsKey(id)) ids.add(id);
+ Map<TransportId, Long> latencies = db.getTransportLatencies();
+ for(TransportId id : localProps.keySet()) {
+ if(latencies.containsKey(id) && remoteProps.containsKey(id))
+ ids.add(id);
+ }
// Assign indices to the transports deterministically and derive keys
Collections.sort(ids, TransportIdComparator.INSTANCE);
int size = ids.size();
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 {
db.addEndpoint(ep);
} catch(NoSuchTransportException e) {
continue;
}
- keyManager.endpointAdded(ep, crypto.deriveInitialSecret(secret, i));
+ byte[] initialSecret = crypto.deriveInitialSecret(secret, i);
+ keyManager.endpointAdded(ep, maxLatency, initialSecret);
}
}
diff --git a/briar-core/src/net/sf/briar/transport/KeyManagerImpl.java b/briar-core/src/net/sf/briar/transport/KeyManagerImpl.java
index 9597af9ca0..778607b9a3 100644
--- a/briar-core/src/net/sf/briar/transport/KeyManagerImpl.java
+++ b/briar-core/src/net/sf/briar/transport/KeyManagerImpl.java
@@ -252,13 +252,9 @@ class KeyManagerImpl extends TimerTask implements KeyManager, DatabaseListener {
return new ConnectionContext(c, t, secret, connection, s.getAlice());
}
- public synchronized void endpointAdded(Endpoint ep, byte[] initialSecret) {
- Long maxLatency = maxLatencies.get(ep.getTransportId());
- if(maxLatency == null) {
- if(LOG.isLoggable(INFO))
- LOG.info("No such transport, ignoring endpoint");
- return;
- }
+ public synchronized void endpointAdded(Endpoint ep, long maxLatency,
+ byte[] initialSecret) {
+ maxLatencies.put(ep.getTransportId(), maxLatency);
// Work out which rotation period we're in
long elapsed = clock.currentTimeMillis() - ep.getEpoch();
long rotation = maxLatency + MAX_CLOCK_DIFFERENCE;
diff --git a/briar-tests/build.xml b/briar-tests/build.xml
index 655e6f80ed..e2d8d749fb 100644
--- a/briar-tests/build.xml
+++ b/briar-tests/build.xml
@@ -55,7 +55,7 @@
</javac>
</target>
<target name='test' depends='compile'>
- <junit printsummary='on' fork='yes' forkmode='once'>
+ <junit showoutput='yes' printsummary='on' fork='yes' forkmode='once'>
<assertions>
<enable/>
</assertions>
diff --git a/briar-tests/src/net/sf/briar/messaging/simplex/SimplexMessagingIntegrationTest.java b/briar-tests/src/net/sf/briar/messaging/simplex/SimplexMessagingIntegrationTest.java
index 3d4c0bc6ad..882245d819 100644
--- a/briar-tests/src/net/sf/briar/messaging/simplex/SimplexMessagingIntegrationTest.java
+++ b/briar-tests/src/net/sf/briar/messaging/simplex/SimplexMessagingIntegrationTest.java
@@ -125,7 +125,7 @@ public class SimplexMessagingIntegrationTest extends BriarTestCase {
db.addTransport(transportId, LATENCY);
Endpoint ep = new Endpoint(contactId, transportId, epoch, true);
db.addEndpoint(ep);
- km.endpointAdded(ep, initialSecret.clone());
+ km.endpointAdded(ep, LATENCY, initialSecret.clone());
// Send Bob a message
String contentType = "text/plain";
byte[] body = "Hi Bob!".getBytes("UTF-8");
@@ -179,7 +179,7 @@ public class SimplexMessagingIntegrationTest extends BriarTestCase {
db.addTransport(transportId, LATENCY);
Endpoint ep = new Endpoint(contactId, transportId, epoch, false);
db.addEndpoint(ep);
- km.endpointAdded(ep, initialSecret.clone());
+ km.endpointAdded(ep, LATENCY, initialSecret.clone());
// Set up a database listener
MessageListener listener = new MessageListener();
db.addListener(listener);
diff --git a/briar-tests/src/net/sf/briar/transport/KeyManagerImplTest.java b/briar-tests/src/net/sf/briar/transport/KeyManagerImplTest.java
index 0f7bed504b..32b418f796 100644
--- a/briar-tests/src/net/sf/briar/transport/KeyManagerImplTest.java
+++ b/briar-tests/src/net/sf/briar/transport/KeyManagerImplTest.java
@@ -141,7 +141,7 @@ public class KeyManagerImplTest extends BriarTestCase {
}});
assertTrue(keyManager.start());
- keyManager.endpointAdded(ep, initialSecret.clone());
+ keyManager.endpointAdded(ep, MAX_LATENCY, initialSecret.clone());
keyManager.stop();
context.assertIsSatisfied();
@@ -202,7 +202,7 @@ public class KeyManagerImplTest extends BriarTestCase {
}});
assertTrue(keyManager.start());
- keyManager.endpointAdded(ep, initialSecret.clone());
+ keyManager.endpointAdded(ep, MAX_LATENCY, initialSecret.clone());
ConnectionContext ctx =
keyManager.getConnectionContext(contactId, transportId);
assertNotNull(ctx);
diff --git a/briar-tests/src/net/sf/briar/transport/KeyRotationIntegrationTest.java b/briar-tests/src/net/sf/briar/transport/KeyRotationIntegrationTest.java
index 380c95c1ed..93dfb3b429 100644
--- a/briar-tests/src/net/sf/briar/transport/KeyRotationIntegrationTest.java
+++ b/briar-tests/src/net/sf/briar/transport/KeyRotationIntegrationTest.java
@@ -239,7 +239,7 @@ public class KeyRotationIntegrationTest extends BriarTestCase {
}});
assertTrue(keyManager.start());
- keyManager.endpointAdded(ep, initialSecret.clone());
+ keyManager.endpointAdded(ep, MAX_LATENCY, initialSecret.clone());
keyManager.stop();
context.assertIsSatisfied();
@@ -377,7 +377,7 @@ public class KeyRotationIntegrationTest extends BriarTestCase {
}});
assertTrue(keyManager.start());
- keyManager.endpointAdded(ep, initialSecret.clone());
+ keyManager.endpointAdded(ep, MAX_LATENCY, initialSecret.clone());
ConnectionContext ctx =
keyManager.getConnectionContext(contactId, transportId);
assertNotNull(ctx);
@@ -533,7 +533,7 @@ public class KeyRotationIntegrationTest extends BriarTestCase {
}});
assertTrue(keyManager.start());
- keyManager.endpointAdded(ep, initialSecret.clone());
+ keyManager.endpointAdded(ep, MAX_LATENCY, initialSecret.clone());
// Recognise the tag for connection 0 in period 2
byte[] tag = new byte[TAG_LENGTH];
encodeTag(tag, key2, 0);
--
GitLab