Bluetooth debugging and code cleanup.

Generate a random UUID instead of using a fixed UUID. Close sockets when
exceptions are thrown (not doing so can cause problems with subsequent
sockets on Android). Use a semaphore with tryAcquire() instead of a lock
when making alien calls, to avoid possible deadlocks.

briar-core/src/net/sf/briar/plugins/PluginsModule.java

@@ -6,6 +6,7 @@ import java.util.concurrent.ExecutorService;
 import java.util.concurrent.Executors;
 
 import net.sf.briar.api.android.AndroidExecutor;
+import net.sf.briar.api.crypto.CryptoComponent;
 import net.sf.briar.api.lifecycle.ShutdownManager;
 import net.sf.briar.api.plugins.PluginExecutor;
 import net.sf.briar.api.plugins.PluginManager;
@@ -63,14 +64,15 @@ public class PluginsModule extends AbstractModule {
 			@PluginExecutor ExecutorService pluginExecutor,
 			AndroidExecutor androidExecutor, Context appContext,
 			ReliabilityLayerFactory reliabilityFactory,
-			ShutdownManager shutdownManager) {
+			ShutdownManager shutdownManager, CryptoComponent crypto) {
 		final Collection<DuplexPluginFactory> factories =
 				new ArrayList<DuplexPluginFactory>();
 		if(OsUtils.isAndroid()) {
 			factories.add(new DroidtoothPluginFactory(pluginExecutor,
-					androidExecutor, appContext));
+					androidExecutor, appContext, crypto.getSecureRandom()));
 		} else {
-			factories.add(new BluetoothPluginFactory(pluginExecutor));
+			factories.add(new BluetoothPluginFactory(pluginExecutor,
+					crypto.getSecureRandom()));
 			factories.add(new ModemPluginFactory(pluginExecutor,
 					reliabilityFactory));
 		}

briar-core/src/net/sf/briar/plugins/bluetooth/BluetoothPlugin.java

@@ -6,6 +6,7 @@ import static java.util.logging.Level.WARNING;
 import static javax.bluetooth.DiscoveryAgent.GIAC;
 
 import java.io.IOException;
+import java.security.SecureRandom;
 import java.util.Collection;
 import java.util.Map;
 import java.util.Map.Entry;
@@ -14,6 +15,7 @@ import java.util.concurrent.Executor;
 import java.util.concurrent.Executors;
 import java.util.concurrent.ScheduledExecutorService;
 import java.util.concurrent.ScheduledFuture;
+import java.util.concurrent.Semaphore;
 import java.util.logging.Logger;
 
 import javax.bluetooth.BluetoothStateException;
@@ -46,12 +48,14 @@ class BluetoothPlugin implements DuplexPlugin {
 
 	private static final Logger LOG =
 			Logger.getLogger(BluetoothPlugin.class.getName());
+	private static final int UUID_BYTES = 16;
 
 	private final Executor pluginExecutor;
 	private final Clock clock;
+	private final SecureRandom secureRandom;
 	private final DuplexPluginCallback callback;
 	private final long maxLatency, pollingInterval;
-	private final Object discoveryLock = new Object();
+	private final Semaphore discoverySemaphore = new Semaphore(1);
 	private final ScheduledExecutorService scheduler;
 
 	private volatile boolean running = false;
@@ -61,10 +65,11 @@ class BluetoothPlugin implements DuplexPlugin {
 	private volatile LocalDevice localDevice = null;
 
 	BluetoothPlugin(@PluginExecutor Executor pluginExecutor, Clock clock,
-			DuplexPluginCallback callback, long maxLatency,
-			long pollingInterval) {
+			SecureRandom secureRandom, DuplexPluginCallback callback,
+			long maxLatency, long pollingInterval) {
 		this.pluginExecutor = pluginExecutor;
 		this.clock = clock;
+		this.secureRandom = secureRandom;
 		this.callback = callback;
 		this.maxLatency = maxLatency;
 		this.pollingInterval = pollingInterval;
@@ -130,9 +135,17 @@ class BluetoothPlugin implements DuplexPlugin {
 		return "btspp://" + address + ":" + uuid + ";name=RFCOMM";
 	}
 
-	// FIXME: Get the UUID from the local transport properties
 	private String getUuid() {
-		return UUID.nameUUIDFromBytes(new byte[0]).toString();
+		String uuid = callback.getLocalProperties().get("uuid");
+		if(uuid == null) {
+			byte[] random = new byte[UUID_BYTES];
+			secureRandom.nextBytes(random);
+			uuid = UUID.nameUUIDFromBytes(random).toString();
+			TransportProperties p = new TransportProperties();
+			p.put("uuid", uuid);
+			callback.mergeLocalProperties(p);
+		}
+		return uuid;
 	}
 
 	private void tryToClose(StreamConnectionNotifier scn) {
@@ -229,29 +242,33 @@ class BluetoothPlugin implements DuplexPlugin {
 			long timeout) {
 		if(!running) return null;
 		// Use the same pseudo-random UUID as the contact
-		String uuid = generateUuid(r.nextBytes(16));
+		byte[] b = r.nextBytes(UUID_BYTES);
+		String uuid = UUID.nameUUIDFromBytes(b).toString();
 		// Discover nearby devices and connect to any with the right UUID
 		DiscoveryAgent discoveryAgent = localDevice.getDiscoveryAgent();
 		long end = clock.currentTimeMillis() + timeout;
 		String url = null;
 		while(url == null && clock.currentTimeMillis() < end) {
-			InvitationListener listener =
-					new InvitationListener(discoveryAgent, uuid);
-			// FIXME: Avoid making alien calls with a lock held
-			synchronized(discoveryLock) {
-				try {
-					discoveryAgent.startInquiry(GIAC, listener);
-					url = listener.waitForUrl();
-				} catch(BluetoothStateException e) {
-					if(LOG.isLoggable(WARNING))
-						LOG.log(WARNING, e.toString(), e);
-					return null;
-				} catch(InterruptedException e) {
-					if(LOG.isLoggable(INFO))
-						LOG.info("Interrupted while waiting for URL");
-					Thread.currentThread().interrupt();
-					return null;
-				}
+			if(!discoverySemaphore.tryAcquire()) {
+				if(LOG.isLoggable(INFO))
+					LOG.info("Another device discovery is in progress");
+				return null;
+			}
+			try {
+				InvitationListener listener =
+						new InvitationListener(discoveryAgent, uuid);
+				discoveryAgent.startInquiry(GIAC, listener);
+				url = listener.waitForUrl();
+			} catch(BluetoothStateException e) {
+				if(LOG.isLoggable(WARNING)) LOG.log(WARNING, e.toString(), e);
+				return null;
+			} catch(InterruptedException e) {
+				if(LOG.isLoggable(INFO))
+					LOG.info("Interrupted while waiting for URL");
+				Thread.currentThread().interrupt();
+				return null;
+			} finally {
+				discoverySemaphore.release();
 			}
 			if(!running) return null;
 		}
@@ -259,16 +276,12 @@ class BluetoothPlugin implements DuplexPlugin {
 		return connect(url);
 	}
 
-	private String generateUuid(byte[] b) {
-		UUID uuid = UUID.nameUUIDFromBytes(b);
-		return uuid.toString().replaceAll("-", "");
-	}
-
 	public DuplexTransportConnection acceptInvitation(PseudoRandom r,
 			long timeout) {
 		if(!running) return null;
 		// Use the same pseudo-random UUID as the contact
-		String uuid = generateUuid(r.nextBytes(16));
+		byte[] b = r.nextBytes(UUID_BYTES);
+		String uuid = UUID.nameUUIDFromBytes(b).toString();
 		String url = makeUrl("localhost", uuid);
 		// Make the device discoverable if possible
 		makeDeviceDiscoverable();

briar-core/src/net/sf/briar/plugins/bluetooth/BluetoothPluginFactory.java

@@ -1,5 +1,6 @@
 package net.sf.briar.plugins.bluetooth;
 
+import java.security.SecureRandom;
 import java.util.concurrent.Executor;
 
 import net.sf.briar.api.clock.Clock;
@@ -16,10 +17,13 @@ public class BluetoothPluginFactory implements DuplexPluginFactory {
 	private static final long POLLING_INTERVAL = 3 * 60 * 1000; // 3 minutes
 
 	private final Executor pluginExecutor;
+	private final SecureRandom secureRandom;
 	private final Clock clock;
 
-	public BluetoothPluginFactory(@PluginExecutor Executor pluginExecutor) {
+	public BluetoothPluginFactory(@PluginExecutor Executor pluginExecutor,
+			SecureRandom secureRandom) {
 		this.pluginExecutor = pluginExecutor;
+		this.secureRandom = secureRandom;
 		clock = new SystemClock();
 	}
 
@@ -28,7 +32,7 @@ public class BluetoothPluginFactory implements DuplexPluginFactory {
 	}
 
 	public DuplexPlugin createPlugin(DuplexPluginCallback callback) {
-		return new BluetoothPlugin(pluginExecutor, clock, callback,
-				MAX_LATENCY, POLLING_INTERVAL);
+		return new BluetoothPlugin(pluginExecutor, clock, secureRandom,
+				callback, MAX_LATENCY, POLLING_INTERVAL);
 	}
 }

briar-core/src/net/sf/briar/plugins/droidtooth/DroidtoothPlugin.java

@@ -11,6 +11,7 @@ import static java.util.logging.Level.WARNING;
 
 import java.io.IOException;
 import java.net.SocketTimeoutException;
+import java.security.SecureRandom;
 import java.util.ArrayList;
 import java.util.Collection;
 import java.util.Map;
@@ -52,6 +53,7 @@ class DroidtoothPlugin implements DuplexPlugin {
 
 	private static final Logger LOG =
 			Logger.getLogger(DroidtoothPlugin.class.getName());
+	private static final int UUID_BYTES = 16;
 	private static final String FOUND = "android.bluetooth.device.action.FOUND";
 	private static final String DISCOVERY_FINISHED =
 			"android.bluetooth.adapter.action.DISCOVERY_FINISHED";
@@ -59,6 +61,7 @@ class DroidtoothPlugin implements DuplexPlugin {
 	private final Executor pluginExecutor;
 	private final AndroidExecutor androidExecutor;
 	private final Context appContext;
+	private final SecureRandom secureRandom;
 	private final DuplexPluginCallback callback;
 	private final long maxLatency, pollingInterval;
 
@@ -69,11 +72,12 @@ class DroidtoothPlugin implements DuplexPlugin {
 
 	DroidtoothPlugin(@PluginExecutor Executor pluginExecutor,
 			AndroidExecutor androidExecutor, Context appContext,
-			DuplexPluginCallback callback, long maxLatency,
-			long pollingInterval) {
+			SecureRandom secureRandom, DuplexPluginCallback callback,
+			long maxLatency, long pollingInterval) {
 		this.pluginExecutor = pluginExecutor;
 		this.androidExecutor = androidExecutor;
 		this.appContext = appContext;
+		this.secureRandom = secureRandom;
 		this.callback = callback;
 		this.maxLatency = maxLatency;
 		this.pollingInterval = pollingInterval;
@@ -129,11 +133,12 @@ class DroidtoothPlugin implements DuplexPlugin {
 		p.put("address", adapter.getAddress());
 		callback.mergeLocalProperties(p);
 		// Bind a server socket to accept connections from contacts
-		BluetoothServerSocket ss;
+		BluetoothServerSocket ss = null;
 		try {
 			ss = InsecureBluetooth.listen(adapter, "RFCOMM", getUuid());
 		} catch(IOException e) {
 			if(LOG.isLoggable(WARNING)) LOG.log(WARNING, e.toString(), e);
+			tryToClose(ss);
 			return;
 		}
 		if(!running) {
@@ -161,14 +166,22 @@ class DroidtoothPlugin implements DuplexPlugin {
 		}
 	}
 
-	// FIXME: Get the UUID from the local transport properties
 	private UUID getUuid() {
-		return UUID.nameUUIDFromBytes(new byte[0]);
+		String uuid = callback.getLocalProperties().get("uuid");
+		if(uuid == null) {
+			byte[] random = new byte[UUID_BYTES];
+			secureRandom.nextBytes(random);
+			uuid = UUID.nameUUIDFromBytes(random).toString();
+			TransportProperties p = new TransportProperties();
+			p.put("uuid", uuid);
+			callback.mergeLocalProperties(p);
+		}
+		return UUID.fromString(uuid);
 	}
 
 	private void tryToClose(BluetoothServerSocket ss) {
 		try {
-			ss.close();
+			if(ss != null) ss.close();
 		} catch(IOException e) {
 			if(LOG.isLoggable(WARNING)) LOG.log(WARNING, e.toString(), e);
 		}
@@ -244,16 +257,28 @@ class DroidtoothPlugin implements DuplexPlugin {
 		}
 		// Try to connect
 		BluetoothDevice d = adapter.getRemoteDevice(address);
+		BluetoothSocket s = null;
 		try {
-			BluetoothSocket s = InsecureBluetooth.createSocket(d, u);
+			s = InsecureBluetooth.createSocket(d, u);
+			if(LOG.isLoggable(INFO)) LOG.info("Connecting to " + address);
 			s.connect();
+			if(LOG.isLoggable(INFO)) LOG.info("Connected to " + address);
 			return new DroidtoothTransportConnection(s, maxLatency);
 		} catch(IOException e) {
 			if(LOG.isLoggable(WARNING)) LOG.log(WARNING, e.toString(), e);
+			tryToClose(s);
 			return null;
 		}
 	}
 
+	private void tryToClose(BluetoothSocket s) {
+		try {
+			if(s != null) s.close();
+		} catch(IOException e) {
+			if(LOG.isLoggable(WARNING)) LOG.log(WARNING, e.toString(), e);
+		}
+	}
+
 	public DuplexTransportConnection createConnection(ContactId c) {
 		if(!running) return null;
 		TransportProperties p = callback.getRemoteProperties().get(c);
@@ -273,7 +298,9 @@ class DroidtoothPlugin implements DuplexPlugin {
 			long timeout) {
 		if(!running) return null;
 		// Use the same pseudo-random UUID as the contact
-		String uuid = UUID.nameUUIDFromBytes(r.nextBytes(16)).toString();
+		byte[] b = r.nextBytes(UUID_BYTES);
+		String uuid = UUID.nameUUIDFromBytes(b).toString();
+		if(LOG.isLoggable(INFO)) LOG.info("Sending invitation, UUID " + uuid);
 		// Register to receive Bluetooth discovery intents
 		IntentFilter filter = new IntentFilter();
 		filter.addAction(FOUND);
@@ -296,18 +323,25 @@ class DroidtoothPlugin implements DuplexPlugin {
 			long timeout) {
 		if(!running) return null;
 		// Use the same pseudo-random UUID as the contact
-		UUID uuid = UUID.nameUUIDFromBytes(r.nextBytes(16));
+		byte[] b = r.nextBytes(UUID_BYTES);
+		UUID uuid = UUID.nameUUIDFromBytes(b);
+		if(LOG.isLoggable(INFO)) LOG.info("Accepting invitation, UUID " + uuid);
 		// Bind a new server socket to accept the invitation connection
-		final BluetoothServerSocket ss;
+		BluetoothServerSocket ss = null;
 		try {
 			ss = InsecureBluetooth.listen(adapter, "RFCOMM", uuid);
 		} catch(IOException e) {
 			if(LOG.isLoggable(WARNING)) LOG.log(WARNING, e.toString(), e);
+			tryToClose(ss);
 			return null;
 		}
 		// Return the first connection received by the socket, if any
 		try {
 			BluetoothSocket s = ss.accept((int) timeout);
+			if(LOG.isLoggable(INFO)) {
+				String address = s.getRemoteDevice().getAddress();
+				LOG.info("Incoming connection from " + address);
+			}
 			return new DroidtoothTransportConnection(s, maxLatency);
 		} catch(SocketTimeoutException e) {
 			if(LOG.isLoggable(INFO)) LOG.info("Invitation timed out");
@@ -368,7 +402,8 @@ class DroidtoothPlugin implements DuplexPlugin {
 				connectToDiscoveredDevices();
 			} else if(action.equals(FOUND)) {
 				BluetoothDevice d = intent.getParcelableExtra(EXTRA_DEVICE);
-				addresses.add(d.getAddress());
+				String address = d.getAddress();
+				addresses.add(address);
 			}
 		}
 

briar-core/src/net/sf/briar/plugins/droidtooth/DroidtoothPluginFactory.java

@@ -1,5 +1,6 @@
 package net.sf.briar.plugins.droidtooth;
 
+import java.security.SecureRandom;
 import java.util.concurrent.Executor;
 
 import net.sf.briar.api.android.AndroidExecutor;
@@ -18,12 +19,15 @@ public class DroidtoothPluginFactory implements DuplexPluginFactory {
 	private final Executor pluginExecutor;
 	private final AndroidExecutor androidExecutor;
 	private final Context appContext;
+	private final SecureRandom secureRandom;
 
 	public DroidtoothPluginFactory(@PluginExecutor Executor pluginExecutor,
-			AndroidExecutor androidExecutor, Context appContext) {
+			AndroidExecutor androidExecutor, Context appContext,
+			SecureRandom secureRandom) {
 		this.pluginExecutor = pluginExecutor;
 		this.androidExecutor = androidExecutor;
 		this.appContext = appContext;
+		this.secureRandom = secureRandom;
 	}
 
 	public TransportId getId() {
@@ -32,6 +36,6 @@ public class DroidtoothPluginFactory implements DuplexPluginFactory {
 
 	public DuplexPlugin createPlugin(DuplexPluginCallback callback) {
 		return new DroidtoothPlugin(pluginExecutor, androidExecutor, appContext,
-				callback, MAX_LATENCY, POLLING_INTERVAL);
+				secureRandom, callback, MAX_LATENCY, POLLING_INTERVAL);
 	}
 }

briar-tests/src/net/sf/briar/plugins/bluetooth/BluetoothClientTest.java

@@ -1,5 +1,6 @@
 package net.sf.briar.plugins.bluetooth;
 
+import java.security.SecureRandom;
 import java.util.Collections;
 import java.util.Map;
 import java.util.concurrent.Executor;
@@ -20,14 +21,14 @@ public class BluetoothClientTest extends DuplexClientTest {
 		// Store the server's Bluetooth address and UUID
 		TransportProperties p = new TransportProperties();
 		p.put("address", serverAddress);
-		p.put("uuid", BluetoothTest.getUuid());
+		p.put("uuid", BluetoothTest.EMPTY_UUID);
 		Map<ContactId, TransportProperties> remote =
 			Collections.singletonMap(contactId, p);
 		// Create the plugin
 		callback = new ClientCallback(new TransportConfig(),
 				new TransportProperties(), remote);
-		plugin = new BluetoothPlugin(executor, new SystemClock(), callback, 0,
-				0);
+		plugin = new BluetoothPlugin(executor, new SystemClock(),
+				new SecureRandom(), callback, 0, 0);
 	}
 
 	public static void main(String[] args) throws Exception {

briar-tests/src/net/sf/briar/plugins/bluetooth/BluetoothServerTest.java

@@ -1,5 +1,6 @@
 package net.sf.briar.plugins.bluetooth;
 
+import java.security.SecureRandom;
 import java.util.Collections;
 import java.util.concurrent.Executor;
 import java.util.concurrent.ExecutorService;
@@ -17,12 +18,12 @@ public class BluetoothServerTest extends DuplexServerTest {
 	private BluetoothServerTest(Executor executor) {
 		// Store the UUID
 		TransportProperties local = new TransportProperties();
-		local.put("uuid", BluetoothTest.getUuid());
+		local.put("uuid", BluetoothTest.EMPTY_UUID);
 		// Create the plugin
 		callback = new ServerCallback(new TransportConfig(), local,
 				Collections.singletonMap(contactId, new TransportProperties()));
-		plugin = new BluetoothPlugin(executor, new SystemClock(), callback, 0,
-				0);
+		plugin = new BluetoothPlugin(executor, new SystemClock(),
+				new SecureRandom(), callback, 0, 0);
 	}
 
 	public static void main(String[] args) throws Exception {

briar-tests/src/net/sf/briar/plugins/bluetooth/BluetoothTest.java

@@ -4,10 +4,6 @@ import java.util.UUID;
 
 class BluetoothTest {
 
-	private static final String EMPTY_UUID =
-			UUID.nameUUIDFromBytes(new byte[0]).toString().replaceAll("-", "");
-
-	static String getUuid() {
-		return EMPTY_UUID;
-	}
+	static final String EMPTY_UUID =
+			UUID.nameUUIDFromBytes(new byte[0]).toString();
 }