From 9efb6ab38f8b450af7bedfb042511b5b782ec46f Mon Sep 17 00:00:00 2001 From: akwizgran <michael@briarproject.org> Date: Tue, 13 Feb 2018 17:19:05 +0000 Subject: [PATCH] Don't allow BT contact connections during key agreement. --- .../bluetooth/AndroidBluetoothPlugin.java | 9 +- .../AndroidBluetoothPluginFactory.java | 8 +- .../AndroidBluetoothTransportConnection.java | 12 +- .../KeyAgreementStoppedListeningEvent.java | 9 ++ .../keyagreement/KeyAgreementTaskImpl.java | 2 + .../bluetooth/BluetoothConnectionLimiter.java | 47 +++++++ .../BluetoothConnectionLimiterImpl.java | 115 ++++++++++++++++++ .../plugin/bluetooth/BluetoothPlugin.java | 31 ++++- .../plugin/bluetooth/JavaBluetoothPlugin.java | 8 +- .../bluetooth/JavaBluetoothPluginFactory.java | 6 +- .../JavaBluetoothTransportConnection.java | 12 +- 11 files changed, 239 insertions(+), 20 deletions(-) create mode 100644 bramble-api/src/main/java/org/briarproject/bramble/api/keyagreement/event/KeyAgreementStoppedListeningEvent.java create mode 100644 bramble-core/src/main/java/org/briarproject/bramble/plugin/bluetooth/BluetoothConnectionLimiter.java create mode 100644 bramble-core/src/main/java/org/briarproject/bramble/plugin/bluetooth/BluetoothConnectionLimiterImpl.java diff --git a/bramble-android/src/main/java/org/briarproject/bramble/plugin/bluetooth/AndroidBluetoothPlugin.java b/bramble-android/src/main/java/org/briarproject/bramble/plugin/bluetooth/AndroidBluetoothPlugin.java index 2c318a6493..a6761c770e 100644 --- a/bramble-android/src/main/java/org/briarproject/bramble/plugin/bluetooth/AndroidBluetoothPlugin.java +++ b/bramble-android/src/main/java/org/briarproject/bramble/plugin/bluetooth/AndroidBluetoothPlugin.java @@ -55,10 +55,12 @@ class AndroidBluetoothPlugin extends BluetoothPlugin<BluetoothServerSocket> { // Non-null if the plugin started successfully private volatile BluetoothAdapter adapter = null; - AndroidBluetoothPlugin(Executor ioExecutor, AndroidExecutor androidExecutor, + AndroidBluetoothPlugin(BluetoothConnectionLimiter connectionLimiter, + Executor ioExecutor, AndroidExecutor androidExecutor, Context appContext, SecureRandom secureRandom, Backoff backoff, DuplexPluginCallback callback, int maxLatency) { - super(ioExecutor, secureRandom, backoff, callback, maxLatency); + super(connectionLimiter, ioExecutor, secureRandom, backoff, callback, + maxLatency); this.androidExecutor = androidExecutor; this.appContext = appContext; } @@ -154,7 +156,8 @@ class AndroidBluetoothPlugin extends BluetoothPlugin<BluetoothServerSocket> { } private DuplexTransportConnection wrapSocket(BluetoothSocket s) { - return new AndroidBluetoothTransportConnection(this, s); + return new AndroidBluetoothTransportConnection(this, + connectionLimiter, s); } @Override diff --git a/bramble-android/src/main/java/org/briarproject/bramble/plugin/bluetooth/AndroidBluetoothPluginFactory.java b/bramble-android/src/main/java/org/briarproject/bramble/plugin/bluetooth/AndroidBluetoothPluginFactory.java index 57ae5caa89..c4b5467398 100644 --- a/bramble-android/src/main/java/org/briarproject/bramble/plugin/bluetooth/AndroidBluetoothPluginFactory.java +++ b/bramble-android/src/main/java/org/briarproject/bramble/plugin/bluetooth/AndroidBluetoothPluginFactory.java @@ -59,11 +59,13 @@ public class AndroidBluetoothPluginFactory implements DuplexPluginFactory { @Override public DuplexPlugin createPlugin(DuplexPluginCallback callback) { + BluetoothConnectionLimiter connectionLimiter = + new BluetoothConnectionLimiterImpl(); Backoff backoff = backoffFactory.createBackoff(MIN_POLLING_INTERVAL, MAX_POLLING_INTERVAL, BACKOFF_BASE); - AndroidBluetoothPlugin plugin = new AndroidBluetoothPlugin(ioExecutor, - androidExecutor, appContext, secureRandom, backoff, callback, - MAX_LATENCY); + AndroidBluetoothPlugin plugin = new AndroidBluetoothPlugin( + connectionLimiter, ioExecutor, androidExecutor, appContext, + secureRandom, backoff, callback, MAX_LATENCY); eventBus.addListener(plugin); return plugin; } diff --git a/bramble-android/src/main/java/org/briarproject/bramble/plugin/bluetooth/AndroidBluetoothTransportConnection.java b/bramble-android/src/main/java/org/briarproject/bramble/plugin/bluetooth/AndroidBluetoothTransportConnection.java index e9b615e0b1..989c80d47e 100644 --- a/bramble-android/src/main/java/org/briarproject/bramble/plugin/bluetooth/AndroidBluetoothTransportConnection.java +++ b/bramble-android/src/main/java/org/briarproject/bramble/plugin/bluetooth/AndroidBluetoothTransportConnection.java @@ -14,10 +14,14 @@ import java.io.OutputStream; class AndroidBluetoothTransportConnection extends AbstractDuplexTransportConnection { + private final BluetoothConnectionLimiter connectionManager; private final BluetoothSocket socket; - AndroidBluetoothTransportConnection(Plugin plugin, BluetoothSocket socket) { + AndroidBluetoothTransportConnection(Plugin plugin, + BluetoothConnectionLimiter connectionManager, + BluetoothSocket socket) { super(plugin); + this.connectionManager = connectionManager; this.socket = socket; } @@ -33,6 +37,10 @@ class AndroidBluetoothTransportConnection @Override protected void closeConnection(boolean exception) throws IOException { - socket.close(); + try { + socket.close(); + } finally { + connectionManager.connectionClosed(this); + } } } diff --git a/bramble-api/src/main/java/org/briarproject/bramble/api/keyagreement/event/KeyAgreementStoppedListeningEvent.java b/bramble-api/src/main/java/org/briarproject/bramble/api/keyagreement/event/KeyAgreementStoppedListeningEvent.java new file mode 100644 index 0000000000..70016db83d --- /dev/null +++ b/bramble-api/src/main/java/org/briarproject/bramble/api/keyagreement/event/KeyAgreementStoppedListeningEvent.java @@ -0,0 +1,9 @@ +package org.briarproject.bramble.api.keyagreement.event; + +import org.briarproject.bramble.api.event.Event; + +/** + * An event that is broadcast when a BQP task stops listening. + */ +public class KeyAgreementStoppedListeningEvent extends Event { +} diff --git a/bramble-core/src/main/java/org/briarproject/bramble/keyagreement/KeyAgreementTaskImpl.java b/bramble-core/src/main/java/org/briarproject/bramble/keyagreement/KeyAgreementTaskImpl.java index f5dfa0dc1b..1b3d9abea8 100644 --- a/bramble-core/src/main/java/org/briarproject/bramble/keyagreement/KeyAgreementTaskImpl.java +++ b/bramble-core/src/main/java/org/briarproject/bramble/keyagreement/KeyAgreementTaskImpl.java @@ -14,6 +14,7 @@ import org.briarproject.bramble.api.keyagreement.event.KeyAgreementFailedEvent; import org.briarproject.bramble.api.keyagreement.event.KeyAgreementFinishedEvent; import org.briarproject.bramble.api.keyagreement.event.KeyAgreementListeningEvent; import org.briarproject.bramble.api.keyagreement.event.KeyAgreementStartedEvent; +import org.briarproject.bramble.api.keyagreement.event.KeyAgreementStoppedListeningEvent; import org.briarproject.bramble.api.keyagreement.event.KeyAgreementWaitingEvent; import org.briarproject.bramble.api.nullsafety.MethodsNotNullByDefault; import org.briarproject.bramble.api.nullsafety.ParametersNotNullByDefault; @@ -71,6 +72,7 @@ class KeyAgreementTaskImpl extends Thread implements KeyAgreementTask, if (localPayload != null) { if (remotePayload == null) connector.stopListening(); else interrupt(); + eventBus.broadcast(new KeyAgreementStoppedListeningEvent()); } } diff --git a/bramble-core/src/main/java/org/briarproject/bramble/plugin/bluetooth/BluetoothConnectionLimiter.java b/bramble-core/src/main/java/org/briarproject/bramble/plugin/bluetooth/BluetoothConnectionLimiter.java new file mode 100644 index 0000000000..548d7c3980 --- /dev/null +++ b/bramble-core/src/main/java/org/briarproject/bramble/plugin/bluetooth/BluetoothConnectionLimiter.java @@ -0,0 +1,47 @@ +package org.briarproject.bramble.plugin.bluetooth; + +import org.briarproject.bramble.api.nullsafety.NotNullByDefault; +import org.briarproject.bramble.api.plugin.duplex.DuplexTransportConnection; + +@NotNullByDefault +interface BluetoothConnectionLimiter { + + /** + * Informs the limiter that key agreement has started. + */ + void keyAgreementStarted(); + + /** + * Informs the limiter that key agreement has ended. + */ + void keyAgreementEnded(); + + /** + * Returns true if a contact connection can be opened. This method does not + * need to be called for key agreement connections. + */ + boolean canOpenContactConnection(); + + /** + * Informs the limiter that a contact connection has been opened. The + * limiter may close the new connection if key agreement is in progress. + * <p/> + * Returns false if the limiter has closed the new connection. + */ + boolean contactConnectionOpened(DuplexTransportConnection conn); + + /** + * Informs the limiter that a key agreement connection has been opened. + */ + void keyAgreementConnectionOpened(DuplexTransportConnection conn); + + /** + * Informs the limiter that the given connection has been closed. + */ + void connectionClosed(DuplexTransportConnection conn); + + /** + * Informs the limiter that all connections have been closed. + */ + void allConnectionsClosed(); +} diff --git a/bramble-core/src/main/java/org/briarproject/bramble/plugin/bluetooth/BluetoothConnectionLimiterImpl.java b/bramble-core/src/main/java/org/briarproject/bramble/plugin/bluetooth/BluetoothConnectionLimiterImpl.java new file mode 100644 index 0000000000..99e8c3c8e7 --- /dev/null +++ b/bramble-core/src/main/java/org/briarproject/bramble/plugin/bluetooth/BluetoothConnectionLimiterImpl.java @@ -0,0 +1,115 @@ +package org.briarproject.bramble.plugin.bluetooth; + +import org.briarproject.bramble.api.nullsafety.NotNullByDefault; +import org.briarproject.bramble.api.plugin.duplex.DuplexTransportConnection; + +import java.io.IOException; +import java.util.ArrayList; +import java.util.LinkedList; +import java.util.List; +import java.util.logging.Logger; + +import javax.annotation.concurrent.ThreadSafe; + +import static java.util.logging.Level.INFO; +import static java.util.logging.Level.WARNING; + +@NotNullByDefault +@ThreadSafe +class BluetoothConnectionLimiterImpl implements BluetoothConnectionLimiter { + + private static final Logger LOG = + Logger.getLogger(BluetoothConnectionLimiterImpl.class.getName()); + + private final Object lock = new Object(); + // The following are locking: lock + private final LinkedList<DuplexTransportConnection> connections = + new LinkedList<>(); + private boolean keyAgreementInProgress = false; + + @Override + public void keyAgreementStarted() { + List<DuplexTransportConnection> close; + synchronized (lock) { + keyAgreementInProgress = true; + close = new ArrayList<>(connections); + connections.clear(); + } + if (LOG.isLoggable(INFO)) { + LOG.info("Key agreement started, closing " + close.size() + + " connections"); + } + for (DuplexTransportConnection conn : close) tryToClose(conn); + } + + @Override + public void keyAgreementEnded() { + synchronized (lock) { + keyAgreementInProgress = false; + } + LOG.info("Key agreement ended"); + } + + @Override + public boolean canOpenContactConnection() { + synchronized (lock) { + if (keyAgreementInProgress) { + LOG.info("Can't open contact connection during key agreement"); + return false; + } else { + LOG.info("Can open contact connection"); + return true; + } + } + } + + @Override + public boolean contactConnectionOpened(DuplexTransportConnection conn) { + boolean accept = true; + synchronized (lock) { + if (keyAgreementInProgress) { + LOG.info("Refusing contact connection during key agreement"); + accept = false; + } else { + LOG.info("Accepting contact connection"); + connections.add(conn); + } + } + if (!accept) tryToClose(conn); + return accept; + } + + @Override + public void keyAgreementConnectionOpened(DuplexTransportConnection conn) { + synchronized (lock) { + LOG.info("Accepting key agreement connection"); + connections.add(conn); + } + } + + private void tryToClose(DuplexTransportConnection conn) { + try { + conn.getWriter().dispose(false); + conn.getReader().dispose(false, false); + } catch (IOException e) { + if (LOG.isLoggable(WARNING)) LOG.log(WARNING, e.toString(), e); + } + } + + @Override + public void connectionClosed(DuplexTransportConnection conn) { + synchronized (lock) { + connections.remove(conn); + if (LOG.isLoggable(INFO)) + LOG.info("Connection closed, " + connections.size() + " open"); + } + } + + @Override + public void allConnectionsClosed() { + synchronized (lock) { + connections.clear(); + LOG.info("All connections closed"); + } + } +} diff --git a/bramble-core/src/main/java/org/briarproject/bramble/plugin/bluetooth/BluetoothPlugin.java b/bramble-core/src/main/java/org/briarproject/bramble/plugin/bluetooth/BluetoothPlugin.java index 8c9ed0646d..9b4ba5bb93 100644 --- a/bramble-core/src/main/java/org/briarproject/bramble/plugin/bluetooth/BluetoothPlugin.java +++ b/bramble-core/src/main/java/org/briarproject/bramble/plugin/bluetooth/BluetoothPlugin.java @@ -7,6 +7,8 @@ import org.briarproject.bramble.api.event.Event; import org.briarproject.bramble.api.event.EventListener; import org.briarproject.bramble.api.keyagreement.KeyAgreementConnection; import org.briarproject.bramble.api.keyagreement.KeyAgreementListener; +import org.briarproject.bramble.api.keyagreement.event.KeyAgreementListeningEvent; +import org.briarproject.bramble.api.keyagreement.event.KeyAgreementStoppedListeningEvent; import org.briarproject.bramble.api.nullsafety.MethodsNotNullByDefault; import org.briarproject.bramble.api.nullsafety.ParametersNotNullByDefault; import org.briarproject.bramble.api.plugin.Backoff; @@ -51,6 +53,8 @@ abstract class BluetoothPlugin<SS> implements DuplexPlugin, EventListener { private static final Logger LOG = Logger.getLogger(BluetoothPlugin.class.getName()); + final BluetoothConnectionLimiter connectionLimiter; + private final Executor ioExecutor; private final SecureRandom secureRandom; private final Backoff backoff; @@ -91,8 +95,10 @@ abstract class BluetoothPlugin<SS> implements DuplexPlugin, EventListener { abstract DuplexTransportConnection connectTo(String address, String uuid) throws IOException; - BluetoothPlugin(Executor ioExecutor, SecureRandom secureRandom, + BluetoothPlugin(BluetoothConnectionLimiter connectionLimiter, + Executor ioExecutor, SecureRandom secureRandom, Backoff backoff, DuplexPluginCallback callback, int maxLatency) { + this.connectionLimiter = connectionLimiter; this.ioExecutor = ioExecutor; this.secureRandom = secureRandom; this.backoff = backoff; @@ -110,6 +116,7 @@ abstract class BluetoothPlugin<SS> implements DuplexPlugin, EventListener { void onAdapterDisabled() { LOG.info("Bluetooth disabled"); tryToClose(socket); + connectionLimiter.allConnectionsClosed(); callback.transportDisabled(); } @@ -213,7 +220,8 @@ abstract class BluetoothPlugin<SS> implements DuplexPlugin, EventListener { return; } backoff.reset(); - callback.incomingConnectionCreated(conn); + if (connectionLimiter.contactConnectionOpened(conn)) + callback.incomingConnectionCreated(conn); if (!running) return; } } @@ -257,10 +265,12 @@ abstract class BluetoothPlugin<SS> implements DuplexPlugin, EventListener { if (StringUtils.isNullOrEmpty(uuid)) continue; ioExecutor.execute(() -> { if (!isRunning() || !shouldAllowContactConnections()) return; + if (!connectionLimiter.canOpenContactConnection()) return; DuplexTransportConnection conn = connect(address, uuid); if (conn != null) { backoff.reset(); - callback.outgoingConnectionCreated(c, conn); + if (connectionLimiter.contactConnectionOpened(conn)) + callback.outgoingConnectionCreated(c, conn); } }); } @@ -300,12 +310,16 @@ abstract class BluetoothPlugin<SS> implements DuplexPlugin, EventListener { @Override public DuplexTransportConnection createConnection(ContactId c) { if (!isRunning() || !shouldAllowContactConnections()) return null; + if (!connectionLimiter.canOpenContactConnection()) return null; TransportProperties p = callback.getRemoteProperties(c); String address = p.get(PROP_ADDRESS); if (StringUtils.isNullOrEmpty(address)) return null; String uuid = p.get(PROP_UUID); if (StringUtils.isNullOrEmpty(uuid)) return null; - return connect(address, uuid); + DuplexTransportConnection conn = connect(address, uuid); + if (conn == null) return null; + // TODO: Why don't we reset the backoff here? + return connectionLimiter.contactConnectionOpened(conn) ? conn : null; } @Override @@ -355,7 +369,9 @@ abstract class BluetoothPlugin<SS> implements DuplexPlugin, EventListener { String uuid = UUID.nameUUIDFromBytes(commitment).toString(); if (LOG.isLoggable(INFO)) LOG.info("Connecting to key agreement UUID " + uuid); - return connect(address, uuid); + DuplexTransportConnection conn = connect(address, uuid); + if (conn != null) connectionLimiter.keyAgreementConnectionOpened(conn); + return conn; } private String parseAddress(BdfList descriptor) throws FormatException { @@ -376,6 +392,10 @@ abstract class BluetoothPlugin<SS> implements DuplexPlugin, EventListener { SettingsUpdatedEvent s = (SettingsUpdatedEvent) e; if (s.getNamespace().equals(ID.getString())) ioExecutor.execute(this::onSettingsUpdated); + } else if (e instanceof KeyAgreementListeningEvent) { + ioExecutor.execute(connectionLimiter::keyAgreementStarted); + } else if (e instanceof KeyAgreementStoppedListeningEvent) { + ioExecutor.execute(connectionLimiter::keyAgreementEnded); } } @@ -408,6 +428,7 @@ abstract class BluetoothPlugin<SS> implements DuplexPlugin, EventListener { public KeyAgreementConnection accept() throws IOException { DuplexTransportConnection conn = acceptConnection(ss); if (LOG.isLoggable(INFO)) LOG.info(ID + ": Incoming connection"); + connectionLimiter.keyAgreementConnectionOpened(conn); return new KeyAgreementConnection(conn, ID); } diff --git a/bramble-j2se/src/main/java/org/briarproject/bramble/plugin/bluetooth/JavaBluetoothPlugin.java b/bramble-j2se/src/main/java/org/briarproject/bramble/plugin/bluetooth/JavaBluetoothPlugin.java index 487603e2da..f69fab3945 100644 --- a/bramble-j2se/src/main/java/org/briarproject/bramble/plugin/bluetooth/JavaBluetoothPlugin.java +++ b/bramble-j2se/src/main/java/org/briarproject/bramble/plugin/bluetooth/JavaBluetoothPlugin.java @@ -31,9 +31,11 @@ class JavaBluetoothPlugin extends BluetoothPlugin<StreamConnectionNotifier> { // Non-null if the plugin started successfully private volatile LocalDevice localDevice = null; - JavaBluetoothPlugin(Executor ioExecutor, SecureRandom secureRandom, + JavaBluetoothPlugin(BluetoothConnectionLimiter connectionManager, + Executor ioExecutor, SecureRandom secureRandom, Backoff backoff, DuplexPluginCallback callback, int maxLatency) { - super(ioExecutor, secureRandom, backoff, callback, maxLatency); + super(connectionManager, ioExecutor, secureRandom, backoff, callback, + maxLatency); } @Override @@ -110,6 +112,6 @@ class JavaBluetoothPlugin extends BluetoothPlugin<StreamConnectionNotifier> { } private DuplexTransportConnection wrapSocket(StreamConnection s) { - return new JavaBluetoothTransportConnection(this, s); + return new JavaBluetoothTransportConnection(this, connectionLimiter, s); } } diff --git a/bramble-j2se/src/main/java/org/briarproject/bramble/plugin/bluetooth/JavaBluetoothPluginFactory.java b/bramble-j2se/src/main/java/org/briarproject/bramble/plugin/bluetooth/JavaBluetoothPluginFactory.java index 56e7234f62..3008e77670 100644 --- a/bramble-j2se/src/main/java/org/briarproject/bramble/plugin/bluetooth/JavaBluetoothPluginFactory.java +++ b/bramble-j2se/src/main/java/org/briarproject/bramble/plugin/bluetooth/JavaBluetoothPluginFactory.java @@ -51,10 +51,12 @@ public class JavaBluetoothPluginFactory implements DuplexPluginFactory { @Override public DuplexPlugin createPlugin(DuplexPluginCallback callback) { + BluetoothConnectionLimiter connectionLimiter = + new BluetoothConnectionLimiterImpl(); Backoff backoff = backoffFactory.createBackoff(MIN_POLLING_INTERVAL, MAX_POLLING_INTERVAL, BACKOFF_BASE); - JavaBluetoothPlugin plugin = new JavaBluetoothPlugin(ioExecutor, - secureRandom, backoff, callback, MAX_LATENCY); + JavaBluetoothPlugin plugin = new JavaBluetoothPlugin(connectionLimiter, + ioExecutor, secureRandom, backoff, callback, MAX_LATENCY); eventBus.addListener(plugin); return plugin; } diff --git a/bramble-j2se/src/main/java/org/briarproject/bramble/plugin/bluetooth/JavaBluetoothTransportConnection.java b/bramble-j2se/src/main/java/org/briarproject/bramble/plugin/bluetooth/JavaBluetoothTransportConnection.java index ffb0f6d4f8..097c3c1915 100644 --- a/bramble-j2se/src/main/java/org/briarproject/bramble/plugin/bluetooth/JavaBluetoothTransportConnection.java +++ b/bramble-j2se/src/main/java/org/briarproject/bramble/plugin/bluetooth/JavaBluetoothTransportConnection.java @@ -14,11 +14,15 @@ import javax.microedition.io.StreamConnection; class JavaBluetoothTransportConnection extends AbstractDuplexTransportConnection { + private final BluetoothConnectionLimiter connectionManager; private final StreamConnection stream; - JavaBluetoothTransportConnection(Plugin plugin, StreamConnection stream) { + JavaBluetoothTransportConnection(Plugin plugin, + BluetoothConnectionLimiter connectionManager, + StreamConnection stream) { super(plugin); this.stream = stream; + this.connectionManager = connectionManager; } @Override @@ -33,6 +37,10 @@ class JavaBluetoothTransportConnection @Override protected void closeConnection(boolean exception) throws IOException { - stream.close(); + try { + stream.close(); + } finally { + connectionManager.connectionClosed(this); + } } } -- GitLab