Avoid redundant derivation from dead secrets.

Find the newest dead secret for each endpoint, erase any others and
derive from the newest.

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 {

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);

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();