From 529897701506f1f4d02f7e16a864ecc1ff00646b Mon Sep 17 00:00:00 2001 From: akwizgran <michael@briarproject.org> Date: Thu, 15 Nov 2012 00:45:32 +0000 Subject: [PATCH] Refactored invitation code to allow the UI to save & restore its state. Android UI elements can be destroyed and recreated at any time, and they can only store serialisable state, so references to long-running tasks have to take the form of serialisable handles. This is pretty ugly - it's easy to create memory leaks if you don't clean up stale handle/reference mappings - but it's less ugly than the common solution of using static variables to hold references. --- lint.xml | 4 + .../invitation/AddContactActivity.java | 147 +++++++++++++++--- .../invitation/CodesDoNotMatchView.java | 2 +- .../invitation/ConnectionFailedView.java | 2 +- .../android/invitation/ContactAddedView.java | 2 +- .../invitation/InvitationCodeView.java | 8 +- .../api/invitation/ConfirmationCallback.java | 14 -- .../api/invitation/ConnectionCallback.java | 18 --- .../api/invitation/InvitationListener.java | 26 ++++ .../api/invitation/InvitationManager.java | 20 +-- .../briar/api/invitation/InvitationState.java | 62 ++++++++ .../briar/api/invitation/InvitationTask.java | 32 ++++ .../api/plugins/InvitationConstants.java | 4 +- .../sf/briar/invitation/AliceConnector.java | 43 ++--- src/net/sf/briar/invitation/BobConnector.java | 47 +++--- src/net/sf/briar/invitation/Connector.java | 58 +++---- .../sf/briar/invitation/ConnectorGroup.java | 145 +++++++++++++++++ .../sf/briar/invitation/FailureNotifier.java | 42 ----- .../invitation/InvitationManagerImpl.java | 62 ++++---- .../sf/briar/invitation/InvitationModule.java | 4 +- 20 files changed, 510 insertions(+), 232 deletions(-) create mode 100644 lint.xml delete mode 100644 src/net/sf/briar/api/invitation/ConfirmationCallback.java delete mode 100644 src/net/sf/briar/api/invitation/ConnectionCallback.java create mode 100644 src/net/sf/briar/api/invitation/InvitationListener.java create mode 100644 src/net/sf/briar/api/invitation/InvitationState.java create mode 100644 src/net/sf/briar/api/invitation/InvitationTask.java create mode 100644 src/net/sf/briar/invitation/ConnectorGroup.java delete mode 100644 src/net/sf/briar/invitation/FailureNotifier.java diff --git a/lint.xml b/lint.xml new file mode 100644 index 0000000000..48d75f9559 --- /dev/null +++ b/lint.xml @@ -0,0 +1,4 @@ +<?xml version="1.0" encoding="UTF-8"?> +<lint> + <issue id="UseSparseArrays" severity="ignore" /> +</lint> \ No newline at end of file diff --git a/src/net/sf/briar/android/invitation/AddContactActivity.java b/src/net/sf/briar/android/invitation/AddContactActivity.java index 20c7cdd63f..cc2486021f 100644 --- a/src/net/sf/briar/android/invitation/AddContactActivity.java +++ b/src/net/sf/briar/android/invitation/AddContactActivity.java @@ -1,34 +1,121 @@ package net.sf.briar.android.invitation; import net.sf.briar.api.crypto.CryptoComponent; -import net.sf.briar.api.invitation.ConfirmationCallback; -import net.sf.briar.api.invitation.ConnectionCallback; +import net.sf.briar.api.invitation.InvitationListener; import net.sf.briar.api.invitation.InvitationManager; +import net.sf.briar.api.invitation.InvitationState; +import net.sf.briar.api.invitation.InvitationTask; import roboguice.activity.RoboActivity; +import android.os.Bundle; import com.google.inject.Inject; public class AddContactActivity extends RoboActivity -implements ConnectionCallback, ConfirmationCallback { +implements InvitationListener { @Inject private CryptoComponent crypto; @Inject private InvitationManager invitationManager; // All of the following must be accessed on the UI thread private AddContactView view = null; + private InvitationTask task = null; private String networkName = null; private boolean useBluetooth = false; - private int localInvitationCode = -1; - private int localConfirmationCode = -1, remoteConfirmationCode = -1; - private ConfirmationCallback callback = null; - private boolean localMatched = false; - private boolean remoteCompared = false, remoteMatched = false; + private int localInvitationCode = -1, remoteInvitationCode = -1; + private int localConfirmationCode = -1, remoteConfirmationCode = -1; + private boolean connectionFailed = false; + private boolean localCompared = false, remoteCompared = false; + private boolean localMatched = false, remoteMatched = false; + + @Override + public void onCreate(Bundle state) { + super.onCreate(state); + if(state == null) { + // This is a new activity + setView(new NetworkSetupView(this)); + } else { + // Restore the activity's state + networkName = state.getString("net.sf.briar.NETWORK_NAME"); + useBluetooth = state.getBoolean("net.sf.briar.USE_BLUETOOTH"); + int handle = state.getInt("TASK_HANDLE", -1); + task = invitationManager.getTask(handle); + if(task == null) { + // No background task - we must be in an initial or final state + localInvitationCode = state.getInt("net.sf.briar.LOCAL_CODE"); + remoteInvitationCode = state.getInt("net.sf.briar.REMOTE_CODE"); + connectionFailed = state.getBoolean("net.sf.briar.FAILED"); + if(state.getBoolean("net.sf.briar.MATCHED")) { + localCompared = remoteCompared = true; + localMatched = remoteMatched = true; + } + // Set the appropriate view for the state + if(localInvitationCode == -1) { + setView(new NetworkSetupView(this)); + } else if(remoteInvitationCode == -1) { + setView(new InvitationCodeView(this)); + } else if(connectionFailed) { + setView(new ConnectionFailedView(this)); + } else if(localMatched && remoteMatched) { + setView(new ContactAddedView(this)); + } else { + setView(new CodesDoNotMatchView(this)); + } + } else { + // A background task exists - listen to it and get its state + InvitationState s = task.addListener(this); + localInvitationCode = s.getLocalInvitationCode(); + remoteInvitationCode = s.getRemoteInvitationCode(); + localConfirmationCode = s.getLocalConfirmationCode(); + remoteConfirmationCode = s.getRemoteConfirmationCode(); + connectionFailed = s.getConnectionFailed(); + localCompared = s.getLocalCompared(); + remoteCompared = s.getRemoteCompared(); + localMatched = s.getLocalMatched(); + remoteMatched = s.getRemoteMatched(); + // Set the appropriate view for the state + if(localInvitationCode == -1) { + setView(new NetworkSetupView(this)); + } else if(remoteInvitationCode == -1) { + setView(new InvitationCodeView(this)); + } else if(localConfirmationCode == -1) { + setView(new ConnectionView(this)); + } else if(connectionFailed) { + setView(new ConnectionFailedView(this)); + } else if(!localCompared) { + setView(new ConfirmationCodeView(this)); + } else if(!remoteCompared) { + setView(new WaitForContactView(this)); + } else if(localMatched && remoteMatched) { + setView(new ContactAddedView(this)); + } else { + setView(new CodesDoNotMatchView(this)); + } + } + } + } @Override public void onResume() { super.onResume(); - if(view == null) setView(new NetworkSetupView(this)); - else view.populate(); + view.populate(); + } + + @Override + public void onSaveInstanceState(Bundle state) { + super.onSaveInstanceState(state); + state.putString("net.sf.briar.NETWORK_NAME", networkName); + state.putBoolean("net.sf.briar.USE_BLUETOOTH", useBluetooth); + state.putInt("net.sf.briar.LOCAL_CODE", localInvitationCode); + state.putInt("net.sf.briar.REMOTE_CODE", remoteInvitationCode); + state.putBoolean("net.sf.briar.FAILED", connectionFailed); + state.putBoolean("net.sf.briar.MATCHED", localMatched && remoteMatched); + if(task != null) state.putInt("TASK_HANDLE", task.getHandle()); + } + + @Override + public void onDestroy() { + super.onDestroy(); + if(task != null) task.removeListener(this); } void setView(AddContactView view) { @@ -37,6 +124,18 @@ implements ConnectionCallback, ConfirmationCallback { setContentView(view); } + void reset(AddContactView view) { + task = null; + networkName = null; + useBluetooth = false; + localInvitationCode = -1; + localConfirmationCode = remoteConfirmationCode = -1; + connectionFailed = false; + localCompared = remoteCompared = false; + localMatched = remoteMatched = false; + setView(view); + } + void setNetworkName(String networkName) { this.networkName = networkName; } @@ -53,19 +152,18 @@ implements ConnectionCallback, ConfirmationCallback { return useBluetooth; } - int generateLocalInvitationCode() { - localInvitationCode = crypto.generateInvitationCode(); - return localInvitationCode; - } - int getLocalInvitationCode() { + if(localInvitationCode == -1) + localInvitationCode = crypto.generateInvitationCode(); return localInvitationCode; } void remoteInvitationCodeEntered(int code) { setView(new ConnectionView(this)); - localMatched = remoteCompared = remoteMatched = false; - invitationManager.connect(localInvitationCode, code, this); + // FIXME: These calls are blocking the UI thread for too long + task = invitationManager.createTask(localInvitationCode, code); + task.addListener(AddContactActivity.this); + task.connect(); } int getLocalConfirmationCode() { @@ -78,34 +176,33 @@ implements ConnectionCallback, ConfirmationCallback { if(remoteMatched) setView(new ContactAddedView(this)); else if(remoteCompared) setView(new CodesDoNotMatchView(this)); else setView(new WaitForContactView(this)); - callback.codesMatch(); + task.localConfirmationSucceeded(); } else { setView(new CodesDoNotMatchView(this)); - callback.codesDoNotMatch(); + task.localConfirmationFailed(); } } - public void connectionEstablished(final int localCode, final int remoteCode, - final ConfirmationCallback c) { + public void connectionSucceeded(final int localCode, final int remoteCode) { runOnUiThread(new Runnable() { public void run() { localConfirmationCode = localCode; remoteConfirmationCode = remoteCode; - callback = c; setView(new ConfirmationCodeView(AddContactActivity.this)); } }); } - public void connectionNotEstablished() { + public void connectionFailed() { runOnUiThread(new Runnable() { public void run() { + connectionFailed = true; setView(new ConnectionFailedView(AddContactActivity.this)); } }); } - public void codesMatch() { + public void remoteConfirmationSucceeded() { runOnUiThread(new Runnable() { public void run() { remoteCompared = true; @@ -116,7 +213,7 @@ implements ConnectionCallback, ConfirmationCallback { }); } - public void codesDoNotMatch() { + public void remoteConfirmationFailed() { runOnUiThread(new Runnable() { public void run() { remoteCompared = true; diff --git a/src/net/sf/briar/android/invitation/CodesDoNotMatchView.java b/src/net/sf/briar/android/invitation/CodesDoNotMatchView.java index d801d01ae9..d30cffde90 100644 --- a/src/net/sf/briar/android/invitation/CodesDoNotMatchView.java +++ b/src/net/sf/briar/android/invitation/CodesDoNotMatchView.java @@ -53,6 +53,6 @@ implements OnClickListener { public void onClick(View view) { // Try again - container.setView(new NetworkSetupView(container)); + container.reset(new NetworkSetupView(container)); } } diff --git a/src/net/sf/briar/android/invitation/ConnectionFailedView.java b/src/net/sf/briar/android/invitation/ConnectionFailedView.java index 970314cd94..6bc06b41cf 100644 --- a/src/net/sf/briar/android/invitation/ConnectionFailedView.java +++ b/src/net/sf/briar/android/invitation/ConnectionFailedView.java @@ -81,6 +81,6 @@ implements WifiStateListener, BluetoothStateListener, OnClickListener { public void onClick(View view) { // Try again - container.setView(new InvitationCodeView(container)); + container.reset(new InvitationCodeView(container)); } } diff --git a/src/net/sf/briar/android/invitation/ContactAddedView.java b/src/net/sf/briar/android/invitation/ContactAddedView.java index 25ad3a3ab8..fbc79b158b 100644 --- a/src/net/sf/briar/android/invitation/ContactAddedView.java +++ b/src/net/sf/briar/android/invitation/ContactAddedView.java @@ -86,6 +86,6 @@ OnEditorActionListener { public void onClick(View view) { if(view == done) container.finish(); // Done - else container.setView(new NetworkSetupView(container)); // Add another + else container.reset(new NetworkSetupView(container)); // Add another } } diff --git a/src/net/sf/briar/android/invitation/InvitationCodeView.java b/src/net/sf/briar/android/invitation/InvitationCodeView.java index fae6cca528..98a0067de0 100644 --- a/src/net/sf/briar/android/invitation/InvitationCodeView.java +++ b/src/net/sf/briar/android/invitation/InvitationCodeView.java @@ -9,17 +9,10 @@ import android.widget.TextView; public class InvitationCodeView extends AddContactView implements CodeEntryListener { - private int localCode = -1; - InvitationCodeView(Context ctx) { super(ctx); } - void init(AddContactActivity container) { - localCode = container.generateLocalInvitationCode(); - super.init(container); - } - void populate() { removeAllViews(); Context ctx = getContext(); @@ -31,6 +24,7 @@ implements CodeEntryListener { TextView code = new TextView(ctx); code.setGravity(CENTER_HORIZONTAL); code.setTextSize(50); + int localCode = container.getLocalInvitationCode(); code.setText(String.format("%06d", localCode)); addView(code); diff --git a/src/net/sf/briar/api/invitation/ConfirmationCallback.java b/src/net/sf/briar/api/invitation/ConfirmationCallback.java deleted file mode 100644 index 652773f863..0000000000 --- a/src/net/sf/briar/api/invitation/ConfirmationCallback.java +++ /dev/null @@ -1,14 +0,0 @@ -package net.sf.briar.api.invitation; - -/** An interface for informing a peer of whether confirmation codes match. */ -public interface ConfirmationCallback { - - /** Called to indicate that the confirmation codes match. */ - void codesMatch(); - - /** - * Called to indicate that either the confirmation codes do not match or - * the result of the comparison is unknown. - */ - void codesDoNotMatch(); -} diff --git a/src/net/sf/briar/api/invitation/ConnectionCallback.java b/src/net/sf/briar/api/invitation/ConnectionCallback.java deleted file mode 100644 index 56ce5c73bf..0000000000 --- a/src/net/sf/briar/api/invitation/ConnectionCallback.java +++ /dev/null @@ -1,18 +0,0 @@ -package net.sf.briar.api.invitation; - -/** An interface for monitoring the status of an invitation connection. */ -public interface ConnectionCallback extends ConfirmationCallback { - - /** - * Called if the connection is successfully established. - * @param localCode the local confirmation code. - * @param remoteCode the remote confirmation code. - * @param c a callback to inform the remote peer of the result of the local - * peer's confirmation code comparison. - */ - void connectionEstablished(int localCode, int remoteCode, - ConfirmationCallback c); - - /** Called if the connection cannot be established. */ - void connectionNotEstablished(); -} diff --git a/src/net/sf/briar/api/invitation/InvitationListener.java b/src/net/sf/briar/api/invitation/InvitationListener.java new file mode 100644 index 0000000000..1f5ecddb80 --- /dev/null +++ b/src/net/sf/briar/api/invitation/InvitationListener.java @@ -0,0 +1,26 @@ +package net.sf.briar.api.invitation; + +/** + * An interface for receiving updates about the state of an + * {@link InvitationTask}. + */ +public interface InvitationListener { + + /** Called if a connection is established and key agreement succeeds. */ + void connectionSucceeded(int localCode, int remoteCode); + + /** Called if a connection cannot be established. */ + void connectionFailed(); + + /** + * Informs the local peer that the remote peer's confirmation check + * succeeded. + */ + void remoteConfirmationSucceeded(); + + /** + * Informs the local peer that the remote peer's confirmation check did + * not succeed, or the connection was lost during confirmation. + */ + void remoteConfirmationFailed(); +} diff --git a/src/net/sf/briar/api/invitation/InvitationManager.java b/src/net/sf/briar/api/invitation/InvitationManager.java index e0ae1774ce..4cb6304e42 100644 --- a/src/net/sf/briar/api/invitation/InvitationManager.java +++ b/src/net/sf/briar/api/invitation/InvitationManager.java @@ -1,17 +1,17 @@ package net.sf.briar.api.invitation; -/** - * Allows invitation connections to be established and their status to be - * monitored. - */ +/** Creates and manages tasks for exchanging invitations with remote peers. */ public interface InvitationManager { + /** Creates a task using the given invitation codes. */ + InvitationTask createTask(int localCode, int remoteCode); + /** - * Tries to establish an invitation connection. - * @param localCode the local invitation code. - * @param remoteCode the remote invitation code. - * @param c1 a callback to be informed of the connection's status and the - * result of the remote peer's confirmation code comparison. + * Returns the previously created task with the given handle, unless the + * task has subsequently removed itself. */ - void connect(int localCode, int remoteCode, ConnectionCallback c); + InvitationTask getTask(int handle); + + /** Called by tasks to remove themselves when they terminate. */ + void removeTask(int handle); } diff --git a/src/net/sf/briar/api/invitation/InvitationState.java b/src/net/sf/briar/api/invitation/InvitationState.java new file mode 100644 index 0000000000..8862e3b666 --- /dev/null +++ b/src/net/sf/briar/api/invitation/InvitationState.java @@ -0,0 +1,62 @@ +package net.sf.briar.api.invitation; + +public class InvitationState { + + private final int localInvitationCode, remoteInvitationCode; + private final int localConfirmationCode, remoteConfirmationCode; + private final boolean connectionFailed; + private final boolean localCompared, remoteCompared; + private final boolean localMatched, remoteMatched; + + public InvitationState(int localInvitationCode, int remoteInvitationCode, + int localConfirmationCode, int remoteConfirmationCode, + boolean connectionFailed, boolean localCompared, + boolean remoteCompared, boolean localMatched, + boolean remoteMatched) { + this.localInvitationCode = localInvitationCode; + this.remoteInvitationCode = remoteInvitationCode; + this.localConfirmationCode = localConfirmationCode; + this.remoteConfirmationCode = remoteConfirmationCode; + this.connectionFailed = connectionFailed; + this.localCompared = localCompared; + this.remoteCompared = remoteCompared; + this.localMatched = localMatched; + this.remoteMatched = remoteMatched; + } + + public int getLocalInvitationCode() { + return localInvitationCode; + } + + public int getRemoteInvitationCode() { + return remoteInvitationCode; + } + + public int getLocalConfirmationCode() { + return localConfirmationCode; + } + + public int getRemoteConfirmationCode() { + return remoteConfirmationCode; + } + + public boolean getConnectionFailed() { + return connectionFailed; + } + + public boolean getLocalCompared() { + return localCompared; + } + + public boolean getRemoteCompared() { + return remoteCompared; + } + + public boolean getLocalMatched() { + return localMatched; + } + + public boolean getRemoteMatched() { + return remoteMatched; + } +} diff --git a/src/net/sf/briar/api/invitation/InvitationTask.java b/src/net/sf/briar/api/invitation/InvitationTask.java new file mode 100644 index 0000000000..43d39d7ffd --- /dev/null +++ b/src/net/sf/briar/api/invitation/InvitationTask.java @@ -0,0 +1,32 @@ +package net.sf.briar.api.invitation; + +/** A task for exchanging invitations with a remote peer. */ +public interface InvitationTask { + + /** Returns the task's unique handle. */ + int getHandle(); + + /** + * Adds a listener to be informed of state changes and returns the + * task's current state. + */ + InvitationState addListener(InvitationListener l); + + /** Removes the given listener. */ + void removeListener(InvitationListener l); + + /** Asynchronously starts the connection process. */ + void connect(); + + /** + * Asynchronously informs the remote peer that the local peer's + * confirmation codes matched. + */ + void localConfirmationSucceeded(); + + /** + * Asynchronously informs the remote peer that the local peer's + * confirmation codes did not match. + */ + void localConfirmationFailed(); +} diff --git a/src/net/sf/briar/api/plugins/InvitationConstants.java b/src/net/sf/briar/api/plugins/InvitationConstants.java index 8e76a32a92..9cdd4c6283 100644 --- a/src/net/sf/briar/api/plugins/InvitationConstants.java +++ b/src/net/sf/briar/api/plugins/InvitationConstants.java @@ -2,7 +2,9 @@ package net.sf.briar.api.plugins; public interface InvitationConstants { - long INVITATION_TIMEOUT = 30 * 1000; // Milliseconds + long CONNECTION_TIMEOUT = 15 * 1000; // Milliseconds + + long CONFIRMATION_TIMEOUT = 60 * 1000; // Milliseconds int CODE_BITS = 19; // Codes must fit into six decimal digits diff --git a/src/net/sf/briar/invitation/AliceConnector.java b/src/net/sf/briar/invitation/AliceConnector.java index 984e3e643a..a45d8b738c 100644 --- a/src/net/sf/briar/invitation/AliceConnector.java +++ b/src/net/sf/briar/invitation/AliceConnector.java @@ -2,18 +2,15 @@ package net.sf.briar.invitation; import static java.util.logging.Level.INFO; import static java.util.logging.Level.WARNING; -import static net.sf.briar.api.plugins.InvitationConstants.INVITATION_TIMEOUT; +import static net.sf.briar.api.plugins.InvitationConstants.CONNECTION_TIMEOUT; import java.io.IOException; import java.io.InputStream; import java.io.OutputStream; import java.security.GeneralSecurityException; -import java.util.concurrent.atomic.AtomicBoolean; import java.util.logging.Logger; import net.sf.briar.api.crypto.CryptoComponent; -import net.sf.briar.api.crypto.PseudoRandom; -import net.sf.briar.api.invitation.ConnectionCallback; import net.sf.briar.api.plugins.duplex.DuplexPlugin; import net.sf.briar.api.plugins.duplex.DuplexTransportConnection; import net.sf.briar.api.serial.Reader; @@ -21,23 +18,23 @@ import net.sf.briar.api.serial.ReaderFactory; import net.sf.briar.api.serial.Writer; import net.sf.briar.api.serial.WriterFactory; +/** A connection thread for the peer being Alice in the invitation protocol. */ class AliceConnector extends Connector { private static final Logger LOG = Logger.getLogger(AliceConnector.class.getName()); AliceConnector(CryptoComponent crypto, ReaderFactory readerFactory, - WriterFactory writerFactory, DuplexPlugin plugin, - PseudoRandom random, ConnectionCallback callback, - AtomicBoolean connected, AtomicBoolean succeeded) { - super(crypto, readerFactory, writerFactory, plugin, random, callback, - connected, succeeded); + WriterFactory writerFactory, ConnectorGroup group, + DuplexPlugin plugin, int localCode, int remoteCode) { + super(crypto, readerFactory, writerFactory, group, plugin, + crypto.getPseudoRandom(localCode, remoteCode)); } @Override public void run() { // Try an outgoing connection first, then an incoming connection - long halfTime = System.currentTimeMillis() + INVITATION_TIMEOUT / 2; + long halfTime = System.currentTimeMillis() + CONNECTION_TIMEOUT; DuplexTransportConnection conn = makeOutgoingConnection(); if(conn == null) { waitForHalfTime(halfTime); @@ -46,7 +43,7 @@ class AliceConnector extends Connector { if(conn == null) return; if(LOG.isLoggable(INFO)) LOG.info(pluginName + " connected"); // Don't proceed with more than one connection - if(connected.getAndSet(true)) { + if(group.getAndSetConnected()) { if(LOG.isLoggable(INFO)) LOG.info(pluginName + " redundant"); tryToClose(conn, false); return; @@ -77,21 +74,27 @@ class AliceConnector extends Connector { tryToClose(conn, true); return; } - // The key agreement succeeded + // The key agreement succeeded - derive the confirmation codes if(LOG.isLoggable(INFO)) LOG.info(pluginName + " succeeded"); - succeeded.set(true); - // Derive the confirmation codes int[] codes = crypto.deriveConfirmationCodes(secret); - callback.connectionEstablished(codes[0], codes[1], - new ConfirmationSender(w)); - // Check whether the remote peer's confirmation codes matched + group.connectionSucceeded(codes[0], codes[1]); + // Exchange confirmation results try { - if(r.readBoolean()) callback.codesMatch(); - else callback.codesDoNotMatch(); + sendConfirmation(w); + if(receiveConfirmation(r)) group.remoteConfirmationSucceeded(); + else group.remoteConfirmationFailed(); } catch(IOException e) { if(LOG.isLoggable(WARNING)) LOG.warning(e.toString()); tryToClose(conn, true); - callback.codesDoNotMatch(); + group.remoteConfirmationFailed(); + return; + } catch(InterruptedException e) { + if(LOG.isLoggable(WARNING)) + LOG.warning("Interrupted while waiting for confirmation"); + tryToClose(conn, true); + group.remoteConfirmationFailed(); + Thread.currentThread().interrupt(); + return; } } } \ No newline at end of file diff --git a/src/net/sf/briar/invitation/BobConnector.java b/src/net/sf/briar/invitation/BobConnector.java index 7b75dc3211..ea440d026b 100644 --- a/src/net/sf/briar/invitation/BobConnector.java +++ b/src/net/sf/briar/invitation/BobConnector.java @@ -2,18 +2,15 @@ package net.sf.briar.invitation; import static java.util.logging.Level.INFO; import static java.util.logging.Level.WARNING; -import static net.sf.briar.api.plugins.InvitationConstants.INVITATION_TIMEOUT; +import static net.sf.briar.api.plugins.InvitationConstants.CONNECTION_TIMEOUT; import java.io.IOException; import java.io.InputStream; import java.io.OutputStream; import java.security.GeneralSecurityException; -import java.util.concurrent.atomic.AtomicBoolean; import java.util.logging.Logger; import net.sf.briar.api.crypto.CryptoComponent; -import net.sf.briar.api.crypto.PseudoRandom; -import net.sf.briar.api.invitation.ConnectionCallback; import net.sf.briar.api.plugins.duplex.DuplexPlugin; import net.sf.briar.api.plugins.duplex.DuplexTransportConnection; import net.sf.briar.api.serial.Reader; @@ -21,23 +18,23 @@ import net.sf.briar.api.serial.ReaderFactory; import net.sf.briar.api.serial.Writer; import net.sf.briar.api.serial.WriterFactory; +/** A connection thread for the peer being Bob in the invitation protocol. */ class BobConnector extends Connector { private static final Logger LOG = Logger.getLogger(BobConnector.class.getName()); BobConnector(CryptoComponent crypto, ReaderFactory readerFactory, - WriterFactory writerFactory, DuplexPlugin plugin, - PseudoRandom random, ConnectionCallback callback, - AtomicBoolean connected, AtomicBoolean succeeded) { - super(crypto, readerFactory, writerFactory, plugin, random, callback, - connected, succeeded); + WriterFactory writerFactory, ConnectorGroup group, + DuplexPlugin plugin, int localCode, int remoteCode) { + super(crypto, readerFactory, writerFactory, group, plugin, + crypto.getPseudoRandom(remoteCode, localCode)); } @Override public void run() { // Try an incoming connection first, then an outgoing connection - long halfTime = System.currentTimeMillis() + INVITATION_TIMEOUT / 2; + long halfTime = System.currentTimeMillis() + CONNECTION_TIMEOUT; DuplexTransportConnection conn = acceptIncomingConnection(); if(conn == null) { waitForHalfTime(halfTime); @@ -58,6 +55,12 @@ class BobConnector extends Connector { w = writerFactory.createWriter(out); // Alice goes first byte[] hash = receivePublicKeyHash(r); + // Don't proceed with more than one connection + if(group.getAndSetConnected()) { + if(LOG.isLoggable(INFO)) LOG.info(pluginName + " redundant"); + tryToClose(conn, false); + return; + } sendPublicKeyHash(w); byte[] key = receivePublicKey(r); sendPublicKey(w); @@ -71,21 +74,27 @@ class BobConnector extends Connector { tryToClose(conn, true); return; } - // The key agreement succeeded + // The key agreement succeeded - derive the confirmation codes if(LOG.isLoggable(INFO)) LOG.info(pluginName + " succeeded"); - succeeded.set(true); - // Derive the confirmation codes int[] codes = crypto.deriveConfirmationCodes(secret); - callback.connectionEstablished(codes[1], codes[0], - new ConfirmationSender(w)); - // Check whether the remote peer's confirmation codes matched + group.connectionSucceeded(codes[1], codes[0]); + // Exchange confirmation results try { - if(r.readBoolean()) callback.codesMatch(); - else callback.codesDoNotMatch(); + sendConfirmation(w); + if(receiveConfirmation(r)) group.remoteConfirmationSucceeded(); + else group.remoteConfirmationFailed(); } catch(IOException e) { if(LOG.isLoggable(WARNING)) LOG.warning(e.toString()); tryToClose(conn, true); - callback.codesDoNotMatch(); + group.remoteConfirmationFailed(); + return; + } catch(InterruptedException e) { + if(LOG.isLoggable(WARNING)) + LOG.warning("Interrupted while waiting for confirmation"); + tryToClose(conn, true); + group.remoteConfirmationFailed(); + Thread.currentThread().interrupt(); + return; } } } diff --git a/src/net/sf/briar/invitation/Connector.java b/src/net/sf/briar/invitation/Connector.java index ce269ab8d5..1aed16e97e 100644 --- a/src/net/sf/briar/invitation/Connector.java +++ b/src/net/sf/briar/invitation/Connector.java @@ -3,14 +3,13 @@ package net.sf.briar.invitation; import static java.util.logging.Level.INFO; import static java.util.logging.Level.WARNING; import static net.sf.briar.api.plugins.InvitationConstants.HASH_LENGTH; -import static net.sf.briar.api.plugins.InvitationConstants.INVITATION_TIMEOUT; +import static net.sf.briar.api.plugins.InvitationConstants.CONNECTION_TIMEOUT; import static net.sf.briar.api.plugins.InvitationConstants.MAX_PUBLIC_KEY_LENGTH; import java.io.IOException; import java.security.GeneralSecurityException; import java.security.KeyPair; import java.util.Arrays; -import java.util.concurrent.atomic.AtomicBoolean; import java.util.logging.Logger; import net.sf.briar.api.FormatException; @@ -18,8 +17,6 @@ import net.sf.briar.api.crypto.CryptoComponent; import net.sf.briar.api.crypto.KeyParser; import net.sf.briar.api.crypto.MessageDigest; import net.sf.briar.api.crypto.PseudoRandom; -import net.sf.briar.api.invitation.ConfirmationCallback; -import net.sf.briar.api.invitation.ConnectionCallback; import net.sf.briar.api.plugins.duplex.DuplexPlugin; import net.sf.briar.api.plugins.duplex.DuplexTransportConnection; import net.sf.briar.api.serial.Reader; @@ -35,10 +32,9 @@ abstract class Connector extends Thread { protected final CryptoComponent crypto; protected final ReaderFactory readerFactory; protected final WriterFactory writerFactory; + protected final ConnectorGroup group; protected final DuplexPlugin plugin; protected final PseudoRandom random; - protected final ConnectionCallback callback; - protected final AtomicBoolean connected, succeeded; protected final String pluginName; private final KeyPair keyPair; @@ -46,17 +42,14 @@ abstract class Connector extends Thread { private final MessageDigest messageDigest; Connector(CryptoComponent crypto, ReaderFactory readerFactory, - WriterFactory writerFactory, DuplexPlugin plugin, - PseudoRandom random, ConnectionCallback callback, - AtomicBoolean connected, AtomicBoolean succeeded) { + WriterFactory writerFactory, ConnectorGroup group, + DuplexPlugin plugin, PseudoRandom random) { this.crypto = crypto; this.readerFactory = readerFactory; this.writerFactory = writerFactory; + this.group = group; this.plugin = plugin; this.random = random; - this.callback = callback; - this.connected = connected; - this.succeeded = succeeded; pluginName = plugin.getClass().getName(); keyPair = crypto.generateAgreementKeyPair(); keyParser = crypto.getAgreementKeyParser(); @@ -66,13 +59,13 @@ abstract class Connector extends Thread { protected DuplexTransportConnection acceptIncomingConnection() { if(LOG.isLoggable(INFO)) LOG.info(pluginName + " accepting incoming connection"); - return plugin.acceptInvitation(random, INVITATION_TIMEOUT / 2); + return plugin.acceptInvitation(random, CONNECTION_TIMEOUT); } protected DuplexTransportConnection makeOutgoingConnection() { if(LOG.isLoggable(INFO)) LOG.info(pluginName + " making outgoing connection"); - return plugin.sendInvitation(random, INVITATION_TIMEOUT / 2); + return plugin.sendInvitation(random, CONNECTION_TIMEOUT); } protected void waitForHalfTime(long halfTime) { @@ -125,7 +118,7 @@ abstract class Connector extends Thread { } catch(GeneralSecurityException e) { throw new FormatException(); } - if(LOG.isLoggable(INFO)) LOG.info(pluginName + " received hash"); + if(LOG.isLoggable(INFO)) LOG.info(pluginName + " received key"); return b; } @@ -141,29 +134,18 @@ abstract class Connector extends Thread { return crypto.deriveInitialSecret(key, keyPair, alice); } - protected static class ConfirmationSender implements ConfirmationCallback { - - private final Writer writer; - - protected ConfirmationSender(Writer writer) { - this.writer = writer; - } - - public void codesMatch() { - write(true); - } - - public void codesDoNotMatch() { - write(false); - } + protected void sendConfirmation(Writer w) throws IOException, + InterruptedException { + boolean matched = group.waitForLocalConfirmationResult(); + if(LOG.isLoggable(INFO)) LOG.info(pluginName + " sent confirmation"); + w.writeBoolean(matched); + w.flush(); + } - private void write(boolean match) { - try { - writer.writeBoolean(match); - writer.flush(); - } catch(IOException e) { - if(LOG.isLoggable(WARNING)) LOG.warning(e.toString()); - } - } + protected boolean receiveConfirmation(Reader r) throws IOException { + boolean matched = r.readBoolean(); + if(LOG.isLoggable(INFO)) + LOG.info(pluginName + " received confirmation"); + return matched; } } diff --git a/src/net/sf/briar/invitation/ConnectorGroup.java b/src/net/sf/briar/invitation/ConnectorGroup.java new file mode 100644 index 0000000000..d6d6c0045b --- /dev/null +++ b/src/net/sf/briar/invitation/ConnectorGroup.java @@ -0,0 +1,145 @@ +package net.sf.briar.invitation; + +import static java.util.concurrent.TimeUnit.MILLISECONDS; +import static java.util.logging.Level.WARNING; +import static net.sf.briar.api.plugins.InvitationConstants.CONFIRMATION_TIMEOUT; + +import java.util.Collection; +import java.util.concurrent.CopyOnWriteArrayList; +import java.util.concurrent.CountDownLatch; +import java.util.concurrent.atomic.AtomicBoolean; +import java.util.logging.Logger; + +import net.sf.briar.api.invitation.InvitationListener; +import net.sf.briar.api.invitation.InvitationManager; +import net.sf.briar.api.invitation.InvitationState; +import net.sf.briar.api.invitation.InvitationTask; + +/** A task consisting of one or more parallel connection attempts. */ +class ConnectorGroup implements InvitationTask { + + private static final Logger LOG = + Logger.getLogger(ConnectorGroup.class.getName()); + + private final InvitationManager manager; + private final int handle, localInvitationCode, remoteInvitationCode; + private final Collection<Connector> connectors; + private final Collection<InvitationListener> listeners; + private final AtomicBoolean connected; + private final CountDownLatch localConfirmationLatch; + + /* + * All of the following are locking: this. We don't want to call the + * listeners with a lock held, but we need to avoid a race condition in + * addListener(), so the state that's accessed there after calling + * listeners.add() must be guarded by a lock. + */ + private int localConfirmationCode = -1, remoteConfirmationCode = -1; + private boolean connectionFailed = false; + private boolean localCompared = false, remoteCompared = false; + private boolean localMatched = false, remoteMatched = false; + + ConnectorGroup(InvitationManager manager, int handle, + int localInvitationCode, int remoteInvitationCode) { + this.manager = manager; + this.handle = handle; + this.localInvitationCode = localInvitationCode; + this.remoteInvitationCode = remoteInvitationCode; + connectors = new CopyOnWriteArrayList<Connector>(); + listeners = new CopyOnWriteArrayList<InvitationListener>(); + connected = new AtomicBoolean(false); + localConfirmationLatch = new CountDownLatch(1); + } + + public int getHandle() { + return handle; + } + + public synchronized InvitationState addListener(InvitationListener l) { + listeners.add(l); + return new InvitationState(localInvitationCode, remoteInvitationCode, + localConfirmationCode, remoteConfirmationCode, connectionFailed, + localCompared, remoteCompared, localMatched, remoteMatched); + } + + public void removeListener(InvitationListener l) { + listeners.remove(l); + } + + // FIXME: The task isn't removed from the manager unless this is called + public void connect() { + for(Connector c : connectors) c.start(); + new Thread() { + @Override + public void run() { + try { + for(Connector c : connectors) c.join(); + } catch(InterruptedException e) { + if(LOG.isLoggable(WARNING)) + LOG.warning("Interrupted while waiting for connectors"); + } + if(!connected.get()) { + synchronized(ConnectorGroup.this) { + connectionFailed = true; + } + for(InvitationListener l : listeners) l.connectionFailed(); + } + manager.removeTask(handle); + } + }.start(); + } + + public void localConfirmationSucceeded() { + synchronized(this) { + localCompared = true; + localMatched = true; + } + localConfirmationLatch.countDown(); + } + + public void localConfirmationFailed() { + synchronized(this) { + localCompared = true; + } + localConfirmationLatch.countDown(); + } + + void addConnector(Connector c) { + connectors.add(c); + } + + boolean getAndSetConnected() { + return connected.getAndSet(true); + } + + void connectionSucceeded(int localCode, int remoteCode) { + synchronized(this) { + localConfirmationCode = localCode; + remoteConfirmationCode = remoteCode; + } + for(InvitationListener l : listeners) + l.connectionSucceeded(localCode, remoteCode); + } + + void remoteConfirmationSucceeded() { + synchronized(this) { + remoteCompared = true; + remoteMatched = true; + } + for(InvitationListener l : listeners) l.remoteConfirmationSucceeded(); + } + + void remoteConfirmationFailed() { + synchronized(this) { + remoteCompared = true; + } + for(InvitationListener l : listeners) l.remoteConfirmationFailed(); + } + + boolean waitForLocalConfirmationResult() throws InterruptedException { + localConfirmationLatch.await(CONFIRMATION_TIMEOUT, MILLISECONDS); + synchronized(this) { + return localMatched; + } + } +} diff --git a/src/net/sf/briar/invitation/FailureNotifier.java b/src/net/sf/briar/invitation/FailureNotifier.java deleted file mode 100644 index 7379a7e930..0000000000 --- a/src/net/sf/briar/invitation/FailureNotifier.java +++ /dev/null @@ -1,42 +0,0 @@ -package net.sf.briar.invitation; - -import static java.util.logging.Level.INFO; - -import java.util.Collection; -import java.util.concurrent.atomic.AtomicBoolean; -import java.util.logging.Logger; - -import net.sf.briar.api.invitation.ConnectionCallback; - -class FailureNotifier extends Thread { - - private static final Logger LOG = - Logger.getLogger(FailureNotifier.class.getName()); - - private final Collection<Thread> workers; - private final AtomicBoolean succeeded; - private final ConnectionCallback callback; - - FailureNotifier(Collection<Thread> workers, AtomicBoolean succeeded, - ConnectionCallback callback) { - this.workers = workers; - this.succeeded = succeeded; - this.callback = callback; - } - - @Override - public void run() { - if(LOG.isLoggable(INFO)) LOG.info(workers.size() + " workers"); - try { - for(Thread worker : workers) worker.join(); - if(!succeeded.get()) { - if(LOG.isLoggable(INFO)) LOG.info("No worker succeeded"); - callback.connectionNotEstablished(); - } - } catch(InterruptedException e) { - if(LOG.isLoggable(INFO)) - LOG.info("Interrupted while waiting for workers"); - callback.connectionNotEstablished(); - } - } -} diff --git a/src/net/sf/briar/invitation/InvitationManagerImpl.java b/src/net/sf/briar/invitation/InvitationManagerImpl.java index ade452233b..33c06fa2cd 100644 --- a/src/net/sf/briar/invitation/InvitationManagerImpl.java +++ b/src/net/sf/briar/invitation/InvitationManagerImpl.java @@ -1,13 +1,13 @@ package net.sf.briar.invitation; -import java.util.ArrayList; import java.util.Collection; -import java.util.concurrent.atomic.AtomicBoolean; +import java.util.Map; +import java.util.concurrent.ConcurrentHashMap; +import java.util.concurrent.atomic.AtomicInteger; import net.sf.briar.api.crypto.CryptoComponent; -import net.sf.briar.api.crypto.PseudoRandom; -import net.sf.briar.api.invitation.ConnectionCallback; import net.sf.briar.api.invitation.InvitationManager; +import net.sf.briar.api.invitation.InvitationTask; import net.sf.briar.api.plugins.PluginManager; import net.sf.briar.api.plugins.duplex.DuplexPlugin; import net.sf.briar.api.serial.ReaderFactory; @@ -22,6 +22,9 @@ class InvitationManagerImpl implements InvitationManager { private final WriterFactory writerFactory; private final PluginManager pluginManager; + private final AtomicInteger nextHandle; + private final Map<Integer, InvitationTask> tasks; + @Inject InvitationManagerImpl(CryptoComponent crypto, ReaderFactory readerFactory, WriterFactory writerFactory, PluginManager pluginManager) { @@ -29,45 +32,36 @@ class InvitationManagerImpl implements InvitationManager { this.readerFactory = readerFactory; this.writerFactory = writerFactory; this.pluginManager = pluginManager; + nextHandle = new AtomicInteger(0); + tasks = new ConcurrentHashMap<Integer, InvitationTask>(); } - public void connect(int localCode, int remoteCode, ConnectionCallback c) { + public InvitationTask createTask(int localCode, int remoteCode) { Collection<DuplexPlugin> plugins = pluginManager.getInvitationPlugins(); - // Alice is the party with the smaller invitation code + int handle = nextHandle.incrementAndGet(); + ConnectorGroup group = + new ConnectorGroup(this, handle, localCode, remoteCode); + // Alice is the peer with the lesser invitation code if(localCode < remoteCode) { - startAliceWorkers(plugins, localCode, remoteCode, c); + for(DuplexPlugin plugin : plugins) { + group.addConnector(new AliceConnector(crypto, readerFactory, + writerFactory, group, plugin, localCode, remoteCode)); + } } else { - startBobWorkers(plugins, localCode, remoteCode, c); + for(DuplexPlugin plugin : plugins) { + group.addConnector(new BobConnector(crypto, readerFactory, + writerFactory, group, plugin, localCode, remoteCode)); + } } + tasks.put(handle, group); + return group; } - private void startAliceWorkers(Collection<DuplexPlugin> plugins, - int localCode, int remoteCode, ConnectionCallback c) { - AtomicBoolean connected = new AtomicBoolean(false); - AtomicBoolean succeeded = new AtomicBoolean(false); - Collection<Thread> workers = new ArrayList<Thread>(); - for(DuplexPlugin p : plugins) { - PseudoRandom r = crypto.getPseudoRandom(localCode, remoteCode); - Thread worker = new AliceConnector(crypto, readerFactory, - writerFactory, p, r, c, connected, succeeded); - workers.add(worker); - worker.start(); - } - new FailureNotifier(workers, succeeded, c).start(); + public InvitationTask getTask(int handle) { + return tasks.get(handle); } - private void startBobWorkers(Collection<DuplexPlugin> plugins, - int localCode, int remoteCode, ConnectionCallback c) { - AtomicBoolean connected = new AtomicBoolean(false); - AtomicBoolean succeeded = new AtomicBoolean(false); - Collection<Thread> workers = new ArrayList<Thread>(); - for(DuplexPlugin p : plugins) { - PseudoRandom r = crypto.getPseudoRandom(remoteCode, localCode); - Thread worker = new BobConnector(crypto, readerFactory, - writerFactory, p, r, c, connected, succeeded); - workers.add(worker); - worker.start(); - } - new FailureNotifier(workers, succeeded, c).start(); + public void removeTask(int handle) { + tasks.remove(handle); } } diff --git a/src/net/sf/briar/invitation/InvitationModule.java b/src/net/sf/briar/invitation/InvitationModule.java index 41197dc7fc..89ae4ea49d 100644 --- a/src/net/sf/briar/invitation/InvitationModule.java +++ b/src/net/sf/briar/invitation/InvitationModule.java @@ -3,11 +3,13 @@ package net.sf.briar.invitation; import net.sf.briar.api.invitation.InvitationManager; import com.google.inject.AbstractModule; +import com.google.inject.Singleton; public class InvitationModule extends AbstractModule { @Override protected void configure() { - bind(InvitationManager.class).to(InvitationManagerImpl.class); + bind(InvitationManager.class).to(InvitationManagerImpl.class).in( + Singleton.class); } } -- GitLab