diff --git a/briar-core/src/net/sf/briar/transport/KeyManagerImpl.java b/briar-core/src/net/sf/briar/transport/KeyManagerImpl.java index 9e7629108c32ff849dbc4ee7d4fdc521b6275e99..eb0ee7a6dab8b0e8607d1f7b9157b11a5395a56a 100644 --- a/briar-core/src/net/sf/briar/transport/KeyManagerImpl.java +++ b/briar-core/src/net/sf/briar/transport/KeyManagerImpl.java @@ -10,6 +10,7 @@ import java.util.Collection; import java.util.HashMap; import java.util.Iterator; import java.util.Map; +import java.util.Map.Entry; import java.util.TimerTask; import java.util.logging.Logger; @@ -144,8 +145,27 @@ class KeyManagerImpl extends TimerTask implements KeyManager, DatabaseListener { // Locking: this private Collection<TemporarySecret> replaceDeadSecrets(long now, Collection<TemporarySecret> dead) { - Collection<TemporarySecret> created = new ArrayList<TemporarySecret>(); + // If there are several dead secrets for an endpoint, use the newest + Map<EndpointKey, TemporarySecret> newest = + new HashMap<EndpointKey, TemporarySecret>(); for(TemporarySecret s : dead) { + EndpointKey k = new EndpointKey(s); + TemporarySecret exists = newest.get(k); + if(exists == null) { + // There's no other secret for this endpoint + newest.put(k, s); + } else if(exists.getPeriod() < s.getPeriod()) { + // There's an older secret - erase it and use this one instead + ByteUtils.erase(exists.getSecret()); + newest.put(k, s); + } else { + // There's a newer secret - erase this one + ByteUtils.erase(s.getSecret()); + } + } + Collection<TemporarySecret> created = new ArrayList<TemporarySecret>(); + for(Entry<EndpointKey, TemporarySecret> e : newest.entrySet()) { + TemporarySecret s = e.getValue(); Long maxLatency = maxLatencies.get(s.getTransportId()); if(maxLatency == null) throw new IllegalStateException(); // Work out which rotation period we're in @@ -167,7 +187,7 @@ class KeyManagerImpl extends TimerTask implements KeyManager, DatabaseListener { // Add the secrets to their respective maps - copies may already // exist, in which case erase the new copies (the old copies are // referenced by the connection recogniser) - EndpointKey k = new EndpointKey(s); + EndpointKey k = e.getKey(); if(oldSecrets.containsKey(k)) { ByteUtils.erase(b1); } else { diff --git a/briar-tests/src/net/sf/briar/transport/KeyManagerImplTest.java b/briar-tests/src/net/sf/briar/transport/KeyManagerImplTest.java index 93c05d567b8ff4faa874ebcef9f7f5ed8522416b..0f7bed504be3aace2baec6af54eb0b393a60623e 100644 --- a/briar-tests/src/net/sf/briar/transport/KeyManagerImplTest.java +++ b/briar-tests/src/net/sf/briar/transport/KeyManagerImplTest.java @@ -355,15 +355,6 @@ public class KeyManagerImplTest extends BriarTestCase { // The current time is the end of period 3 oneOf(clock).currentTimeMillis(); will(returnValue(EPOCH + 3 * ROTATION_PERIOD_LENGTH - 1)); - // The secrets for periods 3 and 4 should be derived from secret 0 - oneOf(crypto).deriveNextSecret(secret0, 1); - will(returnValue(secret1.clone())); - oneOf(crypto).deriveNextSecret(secret1, 2); - will(returnValue(secret2.clone())); - oneOf(crypto).deriveNextSecret(secret2, 3); - will(returnValue(secret3.clone())); - oneOf(crypto).deriveNextSecret(secret3, 4); - will(returnValue(secret4.clone())); // The secrets for periods 3 and 4 should be derived from secret 1 oneOf(crypto).deriveNextSecret(secret1, 2); will(returnValue(secret2.clone())); @@ -371,9 +362,9 @@ public class KeyManagerImplTest extends BriarTestCase { will(returnValue(secret3.clone())); oneOf(crypto).deriveNextSecret(secret3, 4); will(returnValue(secret4.clone())); - // One copy of each of the new secrets should be stored + // The new secrets should be stored oneOf(db).addSecrets(Arrays.asList(s3, s4)); - // The secrets for periods 2 - 3 should be added to the recogniser + // The secrets for periods 2 - 4 should be added to the recogniser oneOf(connectionRecogniser).addSecret(s2); oneOf(connectionRecogniser).addSecret(s3); oneOf(connectionRecogniser).addSecret(s4); @@ -570,14 +561,6 @@ public class KeyManagerImplTest extends BriarTestCase { // run() during period 3 (late): the secrets should be rotated oneOf(clock).currentTimeMillis(); will(returnValue(EPOCH + 2 * ROTATION_PERIOD_LENGTH + 1)); - oneOf(crypto).deriveNextSecret(secret0, 1); - will(returnValue(secret1.clone())); - oneOf(crypto).deriveNextSecret(secret1, 2); - will(returnValue(secret2.clone())); - oneOf(crypto).deriveNextSecret(secret2, 3); - will(returnValue(secret3.clone())); - oneOf(crypto).deriveNextSecret(secret3, 4); - will(returnValue(secret4.clone())); oneOf(crypto).deriveNextSecret(secret1, 2); will(returnValue(secret2.clone())); oneOf(crypto).deriveNextSecret(secret2, 3); diff --git a/briar-tests/src/net/sf/briar/transport/KeyRotationIntegrationTest.java b/briar-tests/src/net/sf/briar/transport/KeyRotationIntegrationTest.java index ed5230b8337acac5580bdb7a548d7a0d83accee9..380c95c1ed426d282b6d07cc8fdd261054faa59b 100644 --- a/briar-tests/src/net/sf/briar/transport/KeyRotationIntegrationTest.java +++ b/briar-tests/src/net/sf/briar/transport/KeyRotationIntegrationTest.java @@ -848,15 +848,6 @@ public class KeyRotationIntegrationTest extends BriarTestCase { // The current time is the end of period 3 oneOf(clock).currentTimeMillis(); will(returnValue(EPOCH + 3 * ROTATION_PERIOD_LENGTH - 1)); - // The secrets for periods 3 and 4 should be derived from secret 0 - oneOf(crypto).deriveNextSecret(secret0, 1); - will(returnValue(secret1.clone())); - oneOf(crypto).deriveNextSecret(secret1, 2); - will(returnValue(secret2.clone())); - oneOf(crypto).deriveNextSecret(secret2, 3); - will(returnValue(secret3.clone())); - oneOf(crypto).deriveNextSecret(secret3, 4); - will(returnValue(secret4.clone())); // The secrets for periods 3 and 4 should be derived from secret 1 oneOf(crypto).deriveNextSecret(secret1, 2); will(returnValue(secret2.clone())); @@ -864,7 +855,7 @@ public class KeyRotationIntegrationTest extends BriarTestCase { will(returnValue(secret3.clone())); oneOf(crypto).deriveNextSecret(secret3, 4); will(returnValue(secret4.clone())); - // One copy of each of the new secrets should be stored + // The new secrets should be stored oneOf(db).addSecrets(Arrays.asList(s3, s4)); // The recogniser should derive the tags for period 2 oneOf(crypto).getTagCipher();