Merge branch 'plugin-toggle-settings' into 'plugin-toggles'

Add toggle settings for transport plugins

See merge request briar/briar!1211

bramble-core/src/main/java/org/briarproject/bramble/plugin/bluetooth/BluetoothPlugin.java

@@ -9,6 +9,7 @@ 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.lifecycle.IoExecutor;
 import org.briarproject.bramble.api.nullsafety.MethodsNotNullByDefault;
 import org.briarproject.bramble.api.nullsafety.NotNullByDefault;
 import org.briarproject.bramble.api.nullsafety.ParametersNotNullByDefault;
@@ -48,6 +49,7 @@ import static org.briarproject.bramble.api.plugin.BluetoothConstants.ID;
 import static org.briarproject.bramble.api.plugin.BluetoothConstants.PREF_BT_ENABLE;
 import static org.briarproject.bramble.api.plugin.BluetoothConstants.PROP_ADDRESS;
 import static org.briarproject.bramble.api.plugin.BluetoothConstants.PROP_UUID;
+import static org.briarproject.bramble.api.plugin.BluetoothConstants.REASON_NO_BT_ADAPTER;
 import static org.briarproject.bramble.api.plugin.BluetoothConstants.UUID_BYTES;
 import static org.briarproject.bramble.api.plugin.Plugin.State.ACTIVE;
 import static org.briarproject.bramble.api.plugin.Plugin.State.DISABLED;
@@ -76,7 +78,6 @@ abstract class BluetoothPlugin<SS> implements DuplexPlugin, EventListener {
 
 	protected final PluginState state = new PluginState();
 
-	private volatile boolean contactConnections = false;
 	private volatile String contactConnectionsUuid = null;
 
 	abstract void initialiseAdapter() throws IOException;
@@ -126,16 +127,18 @@ abstract class BluetoothPlugin<SS> implements DuplexPlugin, EventListener {
 		LOG.info("Bluetooth enabled");
 		// We may not have been able to get the local address before
 		ioExecutor.execute(this::updateProperties);
-		if (shouldAllowContactConnections()) bind();
-		callback.pluginStateChanged(getState());
+		if (getState() == INACTIVE) bind();
 	}
 
 	void onAdapterDisabled() {
 		LOG.info("Bluetooth disabled");
 		connectionLimiter.allConnectionsClosed();
 		// The server socket may not have been closed automatically
-		tryToClose(state.clearServerSocket());
-		callback.pluginStateChanged(getState());
+		SS ss = state.clearServerSocket();
+		if (ss != null) {
+			LOG.info("Closing server socket");
+			tryToClose(ss);
+		}
 	}
 
 	@Override
@@ -160,29 +163,22 @@ abstract class BluetoothPlugin<SS> implements DuplexPlugin, EventListener {
 		try {
 			initialiseAdapter();
 		} catch (IOException e) {
+			state.setNoAdapter();
 			throw new PluginException(e);
 		}
 		updateProperties();
-		state.setStarted();
-		loadSettings(callback.getSettings());
-		if (shouldAllowContactConnections()) {
+		Settings settings = callback.getSettings();
+		boolean enabledByUser = settings.getBoolean(PREF_BT_ENABLE, false);
+		state.setStarted(enabledByUser);
+		if (enabledByUser) {
 			if (isAdapterEnabled()) bind();
 			else enableAdapter();
 		}
 	}
 
-	private void loadSettings(Settings settings) {
-		contactConnections = settings.getBoolean(PREF_BT_ENABLE, false);
-	}
-
-	private boolean shouldAllowContactConnections() {
-		return contactConnections;
-	}
-
 	private void bind() {
 		ioExecutor.execute(() -> {
-			if (!shouldAllowContactConnections() || getState() != ACTIVE)
-				return;
+			if (getState() != INACTIVE) return;
 			// Bind a server socket to accept connections from contacts
 			SS ss;
 			try {
@@ -191,8 +187,7 @@ abstract class BluetoothPlugin<SS> implements DuplexPlugin, EventListener {
 				logException(LOG, WARNING, e);
 				return;
 			}
-			if (!shouldAllowContactConnections() ||
-					!state.setServerSocket(ss)) {
+			if (!state.setServerSocket(ss)) {
 				LOG.info("Closing redundant server socket");
 				tryToClose(ss);
 				return;
@@ -259,7 +254,7 @@ abstract class BluetoothPlugin<SS> implements DuplexPlugin, EventListener {
 
 	@Override
 	public int getReasonDisabled() {
-		return getState() == DISABLED ? REASON_STARTING_STOPPING : -1;
+		return state.getReasonDisabled();
 	}
 
 	@Override
@@ -275,7 +270,7 @@ abstract class BluetoothPlugin<SS> implements DuplexPlugin, EventListener {
 	@Override
 	public void poll(Collection<Pair<TransportProperties, ConnectionHandler>>
 			properties) {
-		if (!shouldAllowContactConnections() || getState() != ACTIVE) return;
+		if (getState() != ACTIVE) return;
 		backoff.increment();
 		for (Pair<TransportProperties, ConnectionHandler> p : properties) {
 			connect(p.getFirst(), p.getSecond());
@@ -288,8 +283,7 @@ abstract class BluetoothPlugin<SS> implements DuplexPlugin, EventListener {
 		String uuid = p.get(PROP_UUID);
 		if (isNullOrEmpty(uuid)) return;
 		ioExecutor.execute(() -> {
-			if (!shouldAllowContactConnections() || getState() != ACTIVE)
-				return;
+			if (getState() != ACTIVE) return;
 			if (!connectionLimiter.canOpenContactConnection()) return;
 			DuplexTransportConnection d = createConnection(p);
 			if (d != null) {
@@ -333,8 +327,7 @@ abstract class BluetoothPlugin<SS> implements DuplexPlugin, EventListener {
 
 	@Override
 	public DuplexTransportConnection createConnection(TransportProperties p) {
-		if (!shouldAllowContactConnections() || getState() != ACTIVE)
-			return null;
+		if (getState() != ACTIVE) return null;
 		if (!connectionLimiter.canOpenContactConnection()) return null;
 		String address = p.get(PROP_ADDRESS);
 		if (isNullOrEmpty(address)) return null;
@@ -439,16 +432,17 @@ abstract class BluetoothPlugin<SS> implements DuplexPlugin, EventListener {
 		}
 	}
 
+	@IoExecutor
 	private void onSettingsUpdated(Settings settings) {
-		boolean wasAllowed = shouldAllowContactConnections();
-		loadSettings(settings);
-		boolean isAllowed = shouldAllowContactConnections();
-		if (wasAllowed && !isAllowed) {
-			LOG.info("Contact connections disabled");
-			tryToClose(state.clearServerSocket());
+		boolean enabledByUser = settings.getBoolean(PREF_BT_ENABLE, false);
+		SS ss = state.setEnabledByUser(enabledByUser);
+		State s = getState();
+		if (ss != null) {
+			LOG.info("Disabled by user, closing server socket");
+			tryToClose(ss);
 			disableAdapterIfEnabledByUs();
-		} else if (!wasAllowed && isAllowed) {
-			LOG.info("Contact connections enabled");
+		} else if (s == INACTIVE) {
+			LOG.info("Enabled by user, opening server socket");
 			if (isAdapterEnabled()) bind();
 			else enableAdapter();
 		}
@@ -482,13 +476,18 @@ abstract class BluetoothPlugin<SS> implements DuplexPlugin, EventListener {
 	protected class PluginState {
 
 		@GuardedBy("this")
-		private boolean started = false, stopped = false;
+		private boolean started = false,
+				stopped = false,
+				noAdapter = false,
+				enabledByUser = false;
+
 		@GuardedBy("this")
 		@Nullable
 		private SS serverSocket = null;
 
-		synchronized void setStarted() {
+		synchronized void setStarted(boolean enabledByUser) {
 			started = true;
+			this.enabledByUser = enabledByUser;
 			callback.pluginStateChanged(getState());
 		}
 
@@ -501,6 +500,23 @@ abstract class BluetoothPlugin<SS> implements DuplexPlugin, EventListener {
 			return ss;
 		}
 
+		synchronized void setNoAdapter() {
+			noAdapter = true;
+			callback.pluginStateChanged(getState());
+		}
+
+		@Nullable
+		synchronized SS setEnabledByUser(boolean enabledByUser) {
+			this.enabledByUser = enabledByUser;
+			SS ss = null;
+			if (!enabledByUser) {
+				ss = serverSocket;
+				serverSocket = null;
+			}
+			callback.pluginStateChanged(getState());
+			return ss;
+		}
+
 		synchronized boolean setServerSocket(SS ss) {
 			if (stopped || serverSocket != null) return false;
 			serverSocket = ss;
@@ -512,12 +528,19 @@ abstract class BluetoothPlugin<SS> implements DuplexPlugin, EventListener {
 		synchronized SS clearServerSocket() {
 			SS ss = serverSocket;
 			serverSocket = null;
+			callback.pluginStateChanged(getState());
 			return ss;
 		}
 
 		synchronized State getState() {
-			if (!started || stopped) return DISABLED;
-			return isAdapterEnabled() ? ACTIVE : INACTIVE;
+			if (!started || stopped || !enabledByUser) return DISABLED;
+			return serverSocket == null ? INACTIVE : ACTIVE;
+		}
+
+		synchronized int getReasonDisabled() {
+			if (noAdapter && !stopped) return REASON_NO_BT_ADAPTER;
+			if (!started || stopped) return REASON_STARTING_STOPPING;
+			return enabledByUser ? -1 : REASON_USER;
 		}
 	}
 }

bramble-core/src/main/java/org/briarproject/bramble/plugin/tcp/LanTcpPluginFactory.java

@@ -1,5 +1,6 @@
 package org.briarproject.bramble.plugin.tcp;
 
+import org.briarproject.bramble.api.event.EventBus;
 import org.briarproject.bramble.api.nullsafety.NotNullByDefault;
 import org.briarproject.bramble.api.plugin.Backoff;
 import org.briarproject.bramble.api.plugin.BackoffFactory;
@@ -25,11 +26,13 @@ public class LanTcpPluginFactory implements DuplexPluginFactory {
 	private static final double BACKOFF_BASE = 1.2;
 
 	private final Executor ioExecutor;
+	private final EventBus eventBus;
 	private final BackoffFactory backoffFactory;
 
-	public LanTcpPluginFactory(Executor ioExecutor,
+	public LanTcpPluginFactory(Executor ioExecutor, EventBus eventBus,
 			BackoffFactory backoffFactory) {
 		this.ioExecutor = ioExecutor;
+		this.eventBus = eventBus;
 		this.backoffFactory = backoffFactory;
 	}
 
@@ -47,7 +50,9 @@ public class LanTcpPluginFactory implements DuplexPluginFactory {
 	public DuplexPlugin createPlugin(PluginCallback callback) {
 		Backoff backoff = backoffFactory.createBackoff(MIN_POLLING_INTERVAL,
 				MAX_POLLING_INTERVAL, BACKOFF_BASE);
-		return new LanTcpPlugin(ioExecutor, backoff, callback, MAX_LATENCY,
-				MAX_IDLE_TIME);
+		LanTcpPlugin plugin = new LanTcpPlugin(ioExecutor, backoff, callback,
+				MAX_LATENCY, MAX_IDLE_TIME);
+		eventBus.addListener(plugin);
+		return plugin;
 	}
 }

bramble-core/src/main/java/org/briarproject/bramble/plugin/tcp/TcpPlugin.java

@@ -3,7 +3,10 @@ package org.briarproject.bramble.plugin.tcp;
 import org.briarproject.bramble.PoliteExecutor;
 import org.briarproject.bramble.api.Pair;
 import org.briarproject.bramble.api.data.BdfList;
+import org.briarproject.bramble.api.event.Event;
+import org.briarproject.bramble.api.event.EventListener;
 import org.briarproject.bramble.api.keyagreement.KeyAgreementListener;
+import org.briarproject.bramble.api.lifecycle.IoExecutor;
 import org.briarproject.bramble.api.nullsafety.MethodsNotNullByDefault;
 import org.briarproject.bramble.api.nullsafety.NotNullByDefault;
 import org.briarproject.bramble.api.nullsafety.ParametersNotNullByDefault;
@@ -15,6 +18,8 @@ import org.briarproject.bramble.api.plugin.duplex.DuplexTransportConnection;
 import org.briarproject.bramble.api.properties.TransportProperties;
 import org.briarproject.bramble.api.rendezvous.KeyMaterialSource;
 import org.briarproject.bramble.api.rendezvous.RendezvousEndpoint;
+import org.briarproject.bramble.api.settings.Settings;
+import org.briarproject.bramble.api.settings.event.SettingsUpdatedEvent;
 
 import java.io.IOException;
 import java.net.InetAddress;
@@ -46,6 +51,7 @@ import static java.util.logging.Logger.getLogger;
 import static org.briarproject.bramble.api.plugin.Plugin.State.ACTIVE;
 import static org.briarproject.bramble.api.plugin.Plugin.State.DISABLED;
 import static org.briarproject.bramble.api.plugin.Plugin.State.INACTIVE;
+import static org.briarproject.bramble.api.plugin.TcpConstants.PREF_TCP_ENABLE;
 import static org.briarproject.bramble.util.IoUtils.tryToClose;
 import static org.briarproject.bramble.util.LogUtils.logException;
 import static org.briarproject.bramble.util.PrivacyUtils.scrubSocketAddress;
@@ -53,7 +59,7 @@ import static org.briarproject.bramble.util.StringUtils.isNullOrEmpty;
 
 @MethodsNotNullByDefault
 @ParametersNotNullByDefault
-abstract class TcpPlugin implements DuplexPlugin {
+abstract class TcpPlugin implements DuplexPlugin, EventListener {
 
 	private static final Logger LOG = getLogger(TcpPlugin.class.getName());
 
@@ -119,7 +125,8 @@ abstract class TcpPlugin implements DuplexPlugin {
 	@Override
 	public void start() {
 		if (used.getAndSet(true)) throw new IllegalStateException();
-		state.setStarted();
+		Settings settings = callback.getSettings();
+		state.setStarted(settings.getBoolean(PREF_TCP_ENABLE, false));
 		bind();
 	}
 
@@ -197,7 +204,7 @@ abstract class TcpPlugin implements DuplexPlugin {
 
 	@Override
 	public int getReasonDisabled() {
-		return getState() == DISABLED ? REASON_STARTING_STOPPING : -1;
+		return state.getReasonDisabled();
 	}
 
 	@Override
@@ -331,18 +338,43 @@ abstract class TcpPlugin implements DuplexPlugin {
 		}
 	}
 
+	@Override
+	public void eventOccurred(Event e) {
+		if (e instanceof SettingsUpdatedEvent) {
+			SettingsUpdatedEvent s = (SettingsUpdatedEvent) e;
+			if (s.getNamespace().equals(getId().getString()))
+				ioExecutor.execute(() -> onSettingsUpdated(s.getSettings()));
+		}
+	}
+
+	@IoExecutor
+	private void onSettingsUpdated(Settings settings) {
+		boolean enabledByUser = settings.getBoolean(PREF_TCP_ENABLE, false);
+		ServerSocket ss = state.setEnabledByUser(enabledByUser);
+		State s = getState();
+		if (ss != null) {
+			LOG.info("Disabled by user, closing server socket");
+			tryToClose(ss, LOG, WARNING);
+		} else if (s == INACTIVE) {
+			LOG.info("Enabled by user, opening server socket");
+			bind();
+		}
+	}
+
 	@ThreadSafe
 	@NotNullByDefault
 	protected class PluginState {
 
 		@GuardedBy("this")
-		private boolean started = false, stopped = false;
+		private boolean started = false, stopped = false, enabledByUser = false;
+
 		@GuardedBy("this")
 		@Nullable
 		private ServerSocket serverSocket = null;
 
-		synchronized void setStarted() {
+		synchronized void setStarted(boolean enabledByUser) {
 			started = true;
+			this.enabledByUser = enabledByUser;
 			callback.pluginStateChanged(getState());
 		}
 
@@ -355,6 +387,18 @@ abstract class TcpPlugin implements DuplexPlugin {
 			return ss;
 		}
 
+		@Nullable
+		synchronized ServerSocket setEnabledByUser(boolean enabledByUser) {
+			this.enabledByUser = enabledByUser;
+			ServerSocket ss = null;
+			if (!enabledByUser) {
+				ss = serverSocket;
+				serverSocket = null;
+			}
+			callback.pluginStateChanged(getState());
+			return ss;
+		}
+
 		@Nullable
 		synchronized ServerSocket getServerSocket() {
 			return serverSocket;
@@ -373,8 +417,13 @@ abstract class TcpPlugin implements DuplexPlugin {
 		}
 
 		synchronized State getState() {
-			if (!started || stopped) return DISABLED;
+			if (!started || stopped || !enabledByUser) return DISABLED;
 			return serverSocket == null ? INACTIVE : ACTIVE;
 		}
+
+		synchronized int getReasonDisabled() {
+			if (!started || stopped) return REASON_STARTING_STOPPING;
+			return enabledByUser ? -1 : REASON_USER;
+		}
 	}
 }

bramble-core/src/main/java/org/briarproject/bramble/plugin/tcp/WanTcpPluginFactory.java

@@ -1,5 +1,6 @@
 package org.briarproject.bramble.plugin.tcp;
 
+import org.briarproject.bramble.api.event.EventBus;
 import org.briarproject.bramble.api.lifecycle.ShutdownManager;
 import org.briarproject.bramble.api.nullsafety.NotNullByDefault;
 import org.briarproject.bramble.api.plugin.Backoff;
@@ -26,12 +27,14 @@ public class WanTcpPluginFactory implements DuplexPluginFactory {
 	private static final double BACKOFF_BASE = 1.2;
 
 	private final Executor ioExecutor;
+	private final EventBus eventBus;
 	private final BackoffFactory backoffFactory;
 	private final ShutdownManager shutdownManager;
 
-	public WanTcpPluginFactory(Executor ioExecutor,
+	public WanTcpPluginFactory(Executor ioExecutor, EventBus eventBus,
 			BackoffFactory backoffFactory, ShutdownManager shutdownManager) {
 		this.ioExecutor = ioExecutor;
+		this.eventBus = eventBus;
 		this.backoffFactory = backoffFactory;
 		this.shutdownManager = shutdownManager;
 	}
@@ -50,8 +53,10 @@ public class WanTcpPluginFactory implements DuplexPluginFactory {
 	public DuplexPlugin createPlugin(PluginCallback callback) {
 		Backoff backoff = backoffFactory.createBackoff(MIN_POLLING_INTERVAL,
 				MAX_POLLING_INTERVAL, BACKOFF_BASE);
-		return new WanTcpPlugin(ioExecutor, backoff,
+		WanTcpPlugin plugin = new WanTcpPlugin(ioExecutor, backoff,
 				new PortMapperImpl(shutdownManager), callback, MAX_LATENCY,
 				MAX_IDLE_TIME);
+		eventBus.addListener(plugin);
+		return plugin;
 	}
 }

bramble-core/src/main/java/org/briarproject/bramble/plugin/tor/TorPlugin.java

@@ -75,6 +75,7 @@ import static org.briarproject.bramble.api.plugin.Plugin.State.ENABLING;
 import static org.briarproject.bramble.api.plugin.Plugin.State.INACTIVE;
 import static org.briarproject.bramble.api.plugin.TorConstants.CONTROL_PORT;
 import static org.briarproject.bramble.api.plugin.TorConstants.ID;
+import static org.briarproject.bramble.api.plugin.TorConstants.PREF_TOR_ENABLE;
 import static org.briarproject.bramble.api.plugin.TorConstants.PREF_TOR_MOBILE;
 import static org.briarproject.bramble.api.plugin.TorConstants.PREF_TOR_NETWORK;
 import static org.briarproject.bramble.api.plugin.TorConstants.PREF_TOR_NETWORK_AUTOMATIC;
@@ -87,7 +88,6 @@ import static org.briarproject.bramble.api.plugin.TorConstants.PROP_ONION_V3;
 import static org.briarproject.bramble.api.plugin.TorConstants.REASON_BATTERY;
 import static org.briarproject.bramble.api.plugin.TorConstants.REASON_COUNTRY_BLOCKED;
 import static org.briarproject.bramble.api.plugin.TorConstants.REASON_MOBILE_DATA;
-import static org.briarproject.bramble.api.plugin.TorConstants.REASON_USER;
 import static org.briarproject.bramble.plugin.tor.TorRendezvousCrypto.SEED_BYTES;
 import static org.briarproject.bramble.util.IoUtils.copyAndClose;
 import static org.briarproject.bramble.util.IoUtils.tryToClose;
@@ -200,7 +200,7 @@ abstract class TorPlugin implements DuplexPlugin, EventHandler, EventListener {
 			}
 		}
 		// Load the settings
-		settings = callback.getSettings();
+		settings = migrateSettings(callback.getSettings());
 		// Install or update the assets if necessary
 		if (!assetsAreUpToDate()) installAssets();
 		if (cookieFile.exists() && !cookieFile.delete())
@@ -288,6 +288,18 @@ abstract class TorPlugin implements DuplexPlugin, EventHandler, EventListener {
 		bind();
 	}
 
+	// TODO: Remove after a reasonable migration period (added 2020-01-16)
+	private Settings migrateSettings(Settings settings) {
+		int network = settings.getInt(PREF_TOR_NETWORK,
+				PREF_TOR_NETWORK_AUTOMATIC);
+		if (network == PREF_TOR_NETWORK_NEVER) {
+			settings.putInt(PREF_TOR_NETWORK, PREF_TOR_NETWORK_AUTOMATIC);
+			settings.putBoolean(PREF_TOR_ENABLE, false);
+			callback.mergeSettings(settings);
+		}
+		return settings;
+	}
+
 	private boolean assetsAreUpToDate() {
 		return doneFile.lastModified() > getLastUpdateTime();
 	}
@@ -763,6 +775,7 @@ abstract class TorPlugin implements DuplexPlugin, EventHandler, EventListener {
 			String country = locationUtils.getCurrentCountry();
 			boolean blocked =
 					circumventionProvider.isTorProbablyBlocked(country);
+			boolean enabledByUser = settings.getBoolean(PREF_TOR_ENABLE, true);
 			int network = settings.getInt(PREF_TOR_NETWORK,
 					PREF_TOR_NETWORK_AUTOMATIC);
 			boolean useMobile = settings.getBoolean(PREF_TOR_MOBILE, true);
@@ -785,7 +798,7 @@ abstract class TorPlugin implements DuplexPlugin, EventHandler, EventListener {
 
 			if (!online) {
 				LOG.info("Disabling network, device is offline");
-			} else if (network == PREF_TOR_NETWORK_NEVER) {
+			} else if (!enabledByUser) {
 				LOG.info("Disabling network, user has disabled Tor");
 				disabledBySettings = true;
 				reasonDisabled = REASON_USER;
@@ -870,11 +883,11 @@ abstract class TorPlugin implements DuplexPlugin, EventHandler, EventListener {
 			callback.pluginStateChanged(getState());
 		}
 
+		// Doesn't affect getState()
 		synchronized void setTorStarted() {
 			torStarted = true;
 		}
 
-		@SuppressWarnings("BooleanMethodIsAlwaysInverted")
 		synchronized boolean isTorRunning() {
 			return torStarted && !stopped;
 		}
@@ -914,12 +927,14 @@ abstract class TorPlugin implements DuplexPlugin, EventHandler, EventListener {
 			callback.pluginStateChanged(getState());
 		}
 
+		// Doesn't affect getState()
 		synchronized boolean setServerSocket(ServerSocket ss) {
 			if (stopped || serverSocket != null) return false;
 			serverSocket = ss;
 			return true;
 		}
 
+		// Doesn't affect getState()
 		synchronized void clearServerSocket(ServerSocket ss) {
 			if (serverSocket == ss) serverSocket = null;
 		}