From 525d909d0830a4071774187038ebf338681b8c3e Mon Sep 17 00:00:00 2001
From: akwizgran <michael@briarproject.org>
Date: Thu, 11 Apr 2013 12:09:35 +0100
Subject: [PATCH] Avoid redundant derivation from dead secrets.
Find the newest dead secret for each endpoint, erase any others and
derive from the newest.
---
.../sf/briar/transport/KeyManagerImpl.java | 24 +++++++++++++++++--
.../briar/transport/KeyManagerImplTest.java | 21 ++--------------
.../transport/KeyRotationIntegrationTest.java | 11 +--------
3 files changed, 25 insertions(+), 31 deletions(-)
diff --git a/briar-core/src/net/sf/briar/transport/KeyManagerImpl.java b/briar-core/src/net/sf/briar/transport/KeyManagerImpl.java
index 9e7629108c..eb0ee7a6da 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 93c05d567b..0f7bed504b 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 ed5230b833..380c95c1ed 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();
--
GitLab