From 21f177d69556dc771e765b2d2dc37678744827eb Mon Sep 17 00:00:00 2001
From: akwizgran <michael@briarproject.org>
Date: Fri, 19 Oct 2012 21:42:11 +0100
Subject: [PATCH] Clone secrets so each copy can be erased at the appropriate
 time.

---
 .../protocol/duplex/DuplexConnection.java     | 33 +++++++++++--------
 .../duplex/IncomingDuplexConnection.java      |  3 --
 .../simplex/IncomingSimplexConnection.java    | 18 ++++++----
 .../simplex/OutgoingSimplexConnection.java    | 24 +++++++-------
 .../TransportConnectionRecogniser.java        |  8 +++--
 5 files changed, 49 insertions(+), 37 deletions(-)

diff --git a/components/net/sf/briar/protocol/duplex/DuplexConnection.java b/components/net/sf/briar/protocol/duplex/DuplexConnection.java
index 6cb64768d3..02da9b9e43 100644
--- a/components/net/sf/briar/protocol/duplex/DuplexConnection.java
+++ b/components/net/sf/briar/protocol/duplex/DuplexConnection.java
@@ -51,6 +51,7 @@ import net.sf.briar.api.transport.ConnectionReaderFactory;
 import net.sf.briar.api.transport.ConnectionRegistry;
 import net.sf.briar.api.transport.ConnectionWriter;
 import net.sf.briar.api.transport.ConnectionWriterFactory;
+import net.sf.briar.util.ByteUtils;
 
 abstract class DuplexConnection implements DatabaseListener {
 
@@ -116,9 +117,7 @@ abstract class DuplexConnection implements DatabaseListener {
 			dbExecutor.execute(new GenerateAcks());
 		} else if(e instanceof ContactRemovedEvent) {
 			ContactId c = ((ContactRemovedEvent) e).getContactId();
-			if(contactId.equals(c)) {
-				if(!disposed.getAndSet(true)) transport.dispose(false, true);
-			}
+			if(contactId.equals(c)) dispose(false, true);
 		} else if(e instanceof MessagesAddedEvent) {
 			if(canSendOffer.getAndSet(false))
 				dbExecutor.execute(new GenerateOffer());
@@ -177,11 +176,11 @@ abstract class DuplexConnection implements DatabaseListener {
 					throw new FormatException();
 				}
 			}
-			// The writer will dispose of the transport
+			// The writer will dispose of the transport if no exceptions occur
 			writerTasks.add(CLOSE);
 		} catch(IOException e) {
 			if(LOG.isLoggable(Level.WARNING)) LOG.warning(e.toString());
-			if(!disposed.getAndSet(true)) transport.dispose(true, true);
+			dispose(true, true);
 		}
 	}
 
@@ -216,20 +215,26 @@ abstract class DuplexConnection implements DatabaseListener {
 			}
 			writer.flush();
 			writer.close();
-			if(!disposed.getAndSet(true)) transport.dispose(false, true);
+			dispose(false, true);
 		} catch(InterruptedException e) {
 			if(LOG.isLoggable(Level.INFO))
 				LOG.info("Interrupted while waiting for task");
-			if(!disposed.getAndSet(true)) transport.dispose(true, true);
+			dispose(true, true);
 		} catch(IOException e) {
 			if(LOG.isLoggable(Level.WARNING)) LOG.warning(e.toString());
-			if(!disposed.getAndSet(true)) transport.dispose(true, true);
+			dispose(true, true);
 		} finally {
 			connRegistry.unregisterConnection(contactId, transportId);
 			db.removeListener(this);
 		}
 	}
 
+	private void dispose(boolean exception, boolean recognised) {
+		if(disposed.getAndSet(true)) return;
+		ByteUtils.erase(ctx.getSecret());
+		transport.dispose(exception, recognised);
+	}
+
 	// This task runs on a database thread
 	private class ReceiveAck implements Runnable {
 
@@ -319,7 +324,7 @@ abstract class DuplexConnection implements DatabaseListener {
 				writer.writeRequest(request);
 			} catch(IOException e) {
 				if(LOG.isLoggable(Level.WARNING)) LOG.warning(e.toString());
-				transport.dispose(true, true);
+				dispose(true, true);
 			}
 		}
 	}
@@ -409,7 +414,7 @@ abstract class DuplexConnection implements DatabaseListener {
 				dbExecutor.execute(new GenerateAcks());
 			} catch(IOException e) {
 				if(LOG.isLoggable(Level.WARNING)) LOG.warning(e.toString());
-				transport.dispose(true, true);
+				dispose(true, true);
 			}
 		}
 	}
@@ -455,7 +460,7 @@ abstract class DuplexConnection implements DatabaseListener {
 				else dbExecutor.execute(new GenerateBatches(requested));
 			} catch(IOException e) {
 				if(LOG.isLoggable(Level.WARNING)) LOG.warning(e.toString());
-				transport.dispose(true, true);
+				dispose(true, true);
 			}
 		}
 	}
@@ -498,7 +503,7 @@ abstract class DuplexConnection implements DatabaseListener {
 				writer.writeOffer(offer);
 			} catch(IOException e) {
 				if(LOG.isLoggable(Level.WARNING)) LOG.warning(e.toString());
-				transport.dispose(true, true);
+				dispose(true, true);
 			}
 		}
 	}
@@ -531,7 +536,7 @@ abstract class DuplexConnection implements DatabaseListener {
 				writer.writeSubscriptionUpdate(update);
 			} catch(IOException e) {
 				if(LOG.isLoggable(Level.WARNING)) LOG.warning(e.toString());
-				transport.dispose(true, true);
+				dispose(true, true);
 			}
 		}
 	}
@@ -564,7 +569,7 @@ abstract class DuplexConnection implements DatabaseListener {
 				writer.writeTransportUpdate(update);
 			} catch(IOException e) {
 				if(LOG.isLoggable(Level.WARNING)) LOG.warning(e.toString());
-				transport.dispose(true, true);
+				dispose(true, true);
 			}
 		}
 	}
diff --git a/components/net/sf/briar/protocol/duplex/IncomingDuplexConnection.java b/components/net/sf/briar/protocol/duplex/IncomingDuplexConnection.java
index 75fc8932aa..0f51a449c3 100644
--- a/components/net/sf/briar/protocol/duplex/IncomingDuplexConnection.java
+++ b/components/net/sf/briar/protocol/duplex/IncomingDuplexConnection.java
@@ -18,8 +18,6 @@ import net.sf.briar.api.transport.ConnectionWriterFactory;
 
 class IncomingDuplexConnection extends DuplexConnection {
 
-	private final ConnectionContext ctx;
-
 	IncomingDuplexConnection(@DatabaseExecutor Executor dbExecutor,
 			@VerificationExecutor Executor verificationExecutor,
 			DatabaseComponent db, ConnectionRegistry connRegistry,
@@ -31,7 +29,6 @@ class IncomingDuplexConnection extends DuplexConnection {
 		super(dbExecutor, verificationExecutor, db, connRegistry,
 				connReaderFactory, connWriterFactory, protoReaderFactory,
 				protoWriterFactory, ctx, transport);
-		this.ctx = ctx;
 	}
 
 	@Override
diff --git a/components/net/sf/briar/protocol/simplex/IncomingSimplexConnection.java b/components/net/sf/briar/protocol/simplex/IncomingSimplexConnection.java
index e83b36e1bf..7a23a90e85 100644
--- a/components/net/sf/briar/protocol/simplex/IncomingSimplexConnection.java
+++ b/components/net/sf/briar/protocol/simplex/IncomingSimplexConnection.java
@@ -26,6 +26,7 @@ import net.sf.briar.api.transport.ConnectionContext;
 import net.sf.briar.api.transport.ConnectionReader;
 import net.sf.briar.api.transport.ConnectionReaderFactory;
 import net.sf.briar.api.transport.ConnectionRegistry;
+import net.sf.briar.util.ByteUtils;
 
 class IncomingSimplexConnection {
 
@@ -85,19 +86,24 @@ class IncomingSimplexConnection {
 					throw new FormatException();
 				}
 			}
-			transport.dispose(false, true);
+			dispose(false, true);
 		} catch(IOException e) {
 			if(LOG.isLoggable(Level.WARNING)) LOG.warning(e.toString());
-			try {
-				transport.dispose(true, true);
-			} catch(IOException e1) {
-				if(LOG.isLoggable(Level.WARNING)) LOG.warning(e.toString());
-			}
+			dispose(true, true);
 		} finally {
 			connRegistry.unregisterConnection(contactId, transportId);
 		}
 	}
 
+	private void dispose(boolean exception, boolean recognised) {
+		ByteUtils.erase(ctx.getSecret());
+		try {
+			transport.dispose(exception, recognised);
+		} catch(IOException e) {
+			if(LOG.isLoggable(Level.WARNING)) LOG.warning(e.toString());
+		}
+	}
+
 	private class ReceiveAck implements Runnable {
 
 		private final Ack ack;
diff --git a/components/net/sf/briar/protocol/simplex/OutgoingSimplexConnection.java b/components/net/sf/briar/protocol/simplex/OutgoingSimplexConnection.java
index fc0e926e4d..f6e2d40f20 100644
--- a/components/net/sf/briar/protocol/simplex/OutgoingSimplexConnection.java
+++ b/components/net/sf/briar/protocol/simplex/OutgoingSimplexConnection.java
@@ -23,6 +23,7 @@ import net.sf.briar.api.transport.ConnectionContext;
 import net.sf.briar.api.transport.ConnectionRegistry;
 import net.sf.briar.api.transport.ConnectionWriter;
 import net.sf.briar.api.transport.ConnectionWriterFactory;
+import net.sf.briar.util.ByteUtils;
 
 class OutgoingSimplexConnection {
 
@@ -96,23 +97,24 @@ class OutgoingSimplexConnection {
 			}
 			writer.flush();
 			writer.close();
-			transport.dispose(false);
+			dispose(false);
 		} catch(DbException e) {
 			if(LOG.isLoggable(Level.WARNING)) LOG.warning(e.toString());
-			try {
-				transport.dispose(true);
-			} catch(IOException e1) {
-				if(LOG.isLoggable(Level.WARNING)) LOG.warning(e.toString());
-			}
+			dispose(true);
 		} catch(IOException e) {
 			if(LOG.isLoggable(Level.WARNING)) LOG.warning(e.toString());
-			try {
-				transport.dispose(true);
-			} catch(IOException e1) {
-				if(LOG.isLoggable(Level.WARNING)) LOG.warning(e.toString());
-			}
+			dispose(true);
 		} finally {
 			connRegistry.unregisterConnection(contactId, transportId);
 		}
 	}
+
+	private void dispose(boolean exception) {
+		ByteUtils.erase(ctx.getSecret());
+		try {
+			transport.dispose(exception);
+		} catch(IOException e) {
+			if(LOG.isLoggable(Level.WARNING)) LOG.warning(e.toString());
+		}
+	}
 }
diff --git a/components/net/sf/briar/transport/TransportConnectionRecogniser.java b/components/net/sf/briar/transport/TransportConnectionRecogniser.java
index 33b8fe4a79..2eb5089672 100644
--- a/components/net/sf/briar/transport/TransportConnectionRecogniser.java
+++ b/components/net/sf/briar/transport/TransportConnectionRecogniser.java
@@ -56,10 +56,11 @@ class TransportConnectionRecogniser {
 			TagEncoder.encodeTag(tag1, cipher, key, connection1);
 			if(connection1 <= connection) {
 				WindowContext old = tagMap.remove(new Bytes(tag1));
-				assert old == null;
+				assert old != null;
+				ByteUtils.erase(old.context.getSecret());
 			} else {
 				ConnectionContext ctx1 = new ConnectionContext(contactId,
-						transportId, secret, connection1, alice);
+						transportId, secret.clone(), connection1, alice);
 				WindowContext wctx1 = new WindowContext(window, ctx1, period);
 				WindowContext old = tagMap.put(new Bytes(tag1), wctx1);
 				assert old == null;
@@ -83,7 +84,7 @@ class TransportConnectionRecogniser {
 			byte[] tag = new byte[TAG_LENGTH];
 			TagEncoder.encodeTag(tag, cipher, key, connection);
 			ConnectionContext ctx = new ConnectionContext(contactId,
-					transportId, secret, connection, alice);
+					transportId, secret.clone(), connection, alice);
 			WindowContext wctx = new WindowContext(window, ctx, period);
 			WindowContext old = tagMap.put(new Bytes(tag), wctx);
 			assert old == null;
@@ -107,6 +108,7 @@ class TransportConnectionRecogniser {
 			TagEncoder.encodeTag(tag, cipher, key, connection);
 			WindowContext old = tagMap.remove(new Bytes(tag));
 			assert old != null;
+			ByteUtils.erase(old.context.getSecret());
 		}
 		key.erase();
 		ByteUtils.erase(rctx.secret);
-- 
GitLab