Don't clone secrets until they're needed.

This commit is contained in:
akwizgran
2013-04-11 11:29:57 +01:00
parent 72fae48aef
commit 6f8982f3fd
3 changed files with 47 additions and 51 deletions

View File

@@ -217,6 +217,7 @@ class KeyManagerImpl extends TimerTask implements KeyManager, DatabaseListener {
if(LOG.isLoggable(WARNING)) LOG.log(WARNING, e.toString(), e); if(LOG.isLoggable(WARNING)) LOG.log(WARNING, e.toString(), e);
return null; return null;
} }
// Clone the secret - the original will be erased
byte[] secret = s.getSecret().clone(); byte[] secret = s.getSecret().clone();
return new ConnectionContext(c, t, secret, connection, s.getAlice()); return new ConnectionContext(c, t, secret, connection, s.getAlice());
} }

View File

@@ -18,7 +18,6 @@ import net.sf.briar.api.db.DatabaseComponent;
import net.sf.briar.api.db.DbException; import net.sf.briar.api.db.DbException;
import net.sf.briar.api.transport.ConnectionContext; import net.sf.briar.api.transport.ConnectionContext;
import net.sf.briar.api.transport.TemporarySecret; import net.sf.briar.api.transport.TemporarySecret;
import net.sf.briar.util.ByteUtils;
// FIXME: Don't make alien calls with a lock held // FIXME: Don't make alien calls with a lock held
/** A connection recogniser for a specific transport. */ /** A connection recogniser for a specific transport. */
@@ -41,46 +40,37 @@ class TransportConnectionRecogniser {
synchronized ConnectionContext acceptConnection(byte[] tag) synchronized ConnectionContext acceptConnection(byte[] tag)
throws DbException { throws DbException {
TagContext tctx = tagMap.remove(new Bytes(tag)); TagContext t = tagMap.remove(new Bytes(tag));
if(tctx == null) return null; // The tag was not expected if(t == null) return null; // The tag was not expected
ConnectionWindow window = tctx.window;
ConnectionContext ctx = tctx.context;
long period = tctx.period;
ContactId contactId = ctx.getContactId();
byte[] secret = ctx.getSecret();
long connection = ctx.getConnectionNumber();
boolean alice = ctx.getAlice();
// Update the connection window and the expected tags // Update the connection window and the expected tags
Cipher cipher = crypto.getTagCipher(); Cipher cipher = crypto.getTagCipher();
ErasableKey key = crypto.deriveTagKey(secret, !alice); ErasableKey key = crypto.deriveTagKey(t.secret, !t.alice);
for(long connection1 : window.setSeen(connection)) { for(long connection : t.window.setSeen(t.connection)) {
byte[] tag1 = new byte[TAG_LENGTH]; byte[] tag1 = new byte[TAG_LENGTH];
crypto.encodeTag(tag1, cipher, key, connection1); crypto.encodeTag(tag1, cipher, key, connection);
if(connection1 < connection) { if(connection < t.connection) {
TagContext removed = tagMap.remove(new Bytes(tag1)); TagContext removed = tagMap.remove(new Bytes(tag1));
assert removed != null; assert removed != null;
ByteUtils.erase(removed.context.getSecret());
} else { } else {
ConnectionContext ctx1 = new ConnectionContext(contactId, TagContext added = new TagContext(t, connection);
transportId, secret.clone(), connection1, alice); TagContext duplicate = tagMap.put(new Bytes(tag1), added);
TagContext tctx1 = new TagContext(window, ctx1, period);
TagContext duplicate = tagMap.put(new Bytes(tag1), tctx1);
assert duplicate == null; assert duplicate == null;
} }
} }
key.erase(); key.erase();
// Store the updated connection window in the DB // Store the updated connection window in the DB
long centre = window.getCentre(); db.setConnectionWindow(t.contactId, transportId, t.period,
byte[] bitmap = window.getBitmap(); t.window.getCentre(), t.window.getBitmap());
db.setConnectionWindow(contactId, transportId, period, centre, bitmap); // Clone the secret - the key manager will erase the original
return ctx; return new ConnectionContext(t.contactId, transportId,
t.secret.clone(), t.connection, t.alice);
} }
synchronized void addSecret(TemporarySecret s) { synchronized void addSecret(TemporarySecret s) {
ContactId contactId = s.getContactId(); ContactId contactId = s.getContactId();
boolean alice = s.getAlice();
long period = s.getPeriod(); long period = s.getPeriod();
byte[] secret = s.getSecret(); byte[] secret = s.getSecret();
boolean alice = s.getAlice();
long centre = s.getWindowCentre(); long centre = s.getWindowCentre();
byte[] bitmap = s.getWindowBitmap(); byte[] bitmap = s.getWindowBitmap();
// Create the connection window and the expected tags // Create the connection window and the expected tags
@@ -90,66 +80,73 @@ class TransportConnectionRecogniser {
for(long connection : window.getUnseen()) { for(long connection : window.getUnseen()) {
byte[] tag = new byte[TAG_LENGTH]; byte[] tag = new byte[TAG_LENGTH];
crypto.encodeTag(tag, cipher, key, connection); crypto.encodeTag(tag, cipher, key, connection);
ConnectionContext ctx = new ConnectionContext(contactId, TagContext added = new TagContext(contactId, alice, period,
transportId, secret.clone(), connection, alice); secret, window, connection);
TagContext tctx = new TagContext(window, ctx, period); TagContext duplicate = tagMap.put(new Bytes(tag), added);
TagContext duplicate = tagMap.put(new Bytes(tag), tctx);
assert duplicate == null; assert duplicate == null;
} }
key.erase(); key.erase();
// Create a removal context to remove the window later // Create a removal context to remove the window and the tags later
RemovalContext rctx = new RemovalContext(window, secret, alice); RemovalContext r = new RemovalContext(window, secret, alice);
removalMap.put(new RemovalKey(contactId, period), rctx); removalMap.put(new RemovalKey(contactId, period), r);
} }
synchronized void removeSecret(ContactId contactId, long period) { synchronized void removeSecret(ContactId contactId, long period) {
RemovalKey rk = new RemovalKey(contactId, period); RemovalKey k = new RemovalKey(contactId, period);
RemovalContext rctx = removalMap.remove(rk); RemovalContext removed = removalMap.remove(k);
if(rctx == null) throw new IllegalArgumentException(); if(removed == null) throw new IllegalArgumentException();
removeSecret(rctx); removeSecret(removed);
} }
// Locking: this // Locking: this
private void removeSecret(RemovalContext rctx) { private void removeSecret(RemovalContext r) {
// Remove the expected tags // Remove the expected tags
Cipher cipher = crypto.getTagCipher(); Cipher cipher = crypto.getTagCipher();
ErasableKey key = crypto.deriveTagKey(rctx.secret, !rctx.alice); ErasableKey key = crypto.deriveTagKey(r.secret, !r.alice);
byte[] tag = new byte[TAG_LENGTH]; byte[] tag = new byte[TAG_LENGTH];
for(long connection : rctx.window.getUnseen()) { for(long connection : r.window.getUnseen()) {
crypto.encodeTag(tag, cipher, key, connection); crypto.encodeTag(tag, cipher, key, connection);
TagContext removed = tagMap.remove(new Bytes(tag)); TagContext removed = tagMap.remove(new Bytes(tag));
assert removed != null; assert removed != null;
ByteUtils.erase(removed.context.getSecret());
} }
key.erase(); key.erase();
ByteUtils.erase(rctx.secret);
} }
synchronized void removeSecrets(ContactId c) { synchronized void removeSecrets(ContactId c) {
Collection<RemovalKey> keysToRemove = new ArrayList<RemovalKey>(); Collection<RemovalKey> keysToRemove = new ArrayList<RemovalKey>();
for(RemovalKey k : removalMap.keySet()) { for(RemovalKey k : removalMap.keySet())
if(k.contactId.equals(c)) keysToRemove.add(k); if(k.contactId.equals(c)) keysToRemove.add(k);
}
for(RemovalKey k : keysToRemove) removeSecret(k.contactId, k.period); for(RemovalKey k : keysToRemove) removeSecret(k.contactId, k.period);
} }
synchronized void removeSecrets() { synchronized void removeSecrets() {
for(RemovalContext rctx : removalMap.values()) removeSecret(rctx); for(RemovalContext r : removalMap.values()) removeSecret(r);
assert tagMap.isEmpty(); assert tagMap.isEmpty();
removalMap.clear(); removalMap.clear();
} }
private static class TagContext { private static class TagContext {
private final ConnectionWindow window; private final ContactId contactId;
private final ConnectionContext context; private final boolean alice;
private final long period; private final long period;
private final byte[] secret;
private final ConnectionWindow window;
private final long connection;
private TagContext(ConnectionWindow window, ConnectionContext context, private TagContext(ContactId contactId, boolean alice, long period,
long period) { byte[] secret, ConnectionWindow window, long connection) {
this.window = window; this.contactId = contactId;
this.context = context; this.alice = alice;
this.period = period; this.period = period;
this.secret = secret;
this.window = window;
this.connection = connection;
}
private TagContext(TagContext t, long connection) {
this(t.contactId, t.alice, t.period, t.secret, t.window,
connection);
} }
} }

View File

@@ -72,8 +72,6 @@ public class TransportConnectionRecogniserTest extends BriarTestCase {
new TransportConnectionRecogniser(crypto, db, transportId); new TransportConnectionRecogniser(crypto, db, transportId);
recogniser.addSecret(s); recogniser.addSecret(s);
recogniser.removeSecret(contactId, 0); recogniser.removeSecret(contactId, 0);
// The secret should have been erased
assertArrayEquals(new byte[32], secret);
context.assertIsSatisfied(); context.assertIsSatisfied();
} }