improve state handling of obfsvpn; try to restart obfsvpn in on different proxy port in case the default one is already boudn

2 files changed, 103 insertions, 22 deletions

diff --git a/app/src/main/java/se/leap/bitmaskclient/eip/VpnConfigGenerator.java b/app/src/main/java/se/leap/bitmaskclient/eip/VpnConfigGenerator.java
index 0288ab25..76e32349 100644
--- a/app/src/main/java/se/leap/bitmaskclient/eip/VpnConfigGenerator.java
+++ b/app/src/main/java/se/leap/bitmaskclient/eip/VpnConfigGenerator.java
@@ -379,7 +379,7 @@ public class VpnConfigGenerator {
         }
 
         stringBuilder.append(getRouteString(ipAddress, transport));
-        String transparentProxyRemote = REMOTE + " " + ObfsvpnClient.IP + " " + ObfsvpnClient.PORT + " udp" + newLine;
+        String transparentProxyRemote = REMOTE + " " + ObfsvpnClient.IP + " " + ObfsvpnClient.DEFAULT_PORT + " udp" + newLine;
         stringBuilder.append(transparentProxyRemote);
     }
 
diff --git a/app/src/main/java/se/leap/bitmaskclient/pluggableTransports/ObfsvpnClient.java b/app/src/main/java/se/leap/bitmaskclient/pluggableTransports/ObfsvpnClient.java
index 625bbfd8..0691c826 100644
--- a/app/src/main/java/se/leap/bitmaskclient/pluggableTransports/ObfsvpnClient.java
+++ b/app/src/main/java/se/leap/bitmaskclient/pluggableTransports/ObfsvpnClient.java
@@ -5,12 +5,16 @@ import static se.leap.bitmaskclient.base.models.Constants.QUIC;
 
 import android.util.Log;
 
+import java.util.concurrent.CountDownLatch;
+import java.util.concurrent.TimeUnit;
+import java.util.concurrent.atomic.AtomicInteger;
+import java.util.concurrent.atomic.AtomicReference;
+
 import client.Client;
 import client.Client_;
 import client.EventLogger;
 import de.blinkt.openvpn.core.VpnStatus;
 import de.blinkt.openvpn.core.connection.Connection;
-import se.leap.bitmaskclient.base.models.Constants;
 import se.leap.bitmaskclient.pluggableTransports.models.HoppingConfig;
 import se.leap.bitmaskclient.pluggableTransports.models.KcpConfig;
 import se.leap.bitmaskclient.pluggableTransports.models.Obfs4Options;
@@ -19,20 +23,19 @@ import se.leap.bitmaskclient.pluggableTransports.models.QuicConfig;
 
 public class ObfsvpnClient implements EventLogger {
 
-    public static final int PORT = 8080;
+    public static final int DEFAULT_PORT = 8080;
     public static final String IP = "127.0.0.1";
+    private static final String ERROR_BIND = "bind: address already in use";
     private final Object LOCK = new Object();
-
+    private final AtomicInteger currentPort = new AtomicInteger(DEFAULT_PORT);
+    private CountDownLatch startCallback = null;
 
     private static final String TAG = ObfsvpnClient.class.getSimpleName();
 
     public final Client_ client;
 
     public ObfsvpnClient(Obfs4Options options) throws IllegalStateException {
-
-        //FIXME: use a different strategy here
-        //Basically we would want to track if the more performant transport protocol (KCP?/TCP?) usage was successful
-        //if so, we stick to it, otherwise we flip the flag
+        // each obfuscation transport has only 1 protocol
         String protocol = options.transport.getProtocols()[0];
         boolean kcpEnabled = KCP.equals(protocol);
         boolean quicEnabled = QUIC.equals(protocol);
@@ -42,57 +45,135 @@ public class ObfsvpnClient implements EventLogger {
         }
         KcpConfig kcpConfig = new KcpConfig(kcpEnabled);
         QuicConfig quicConfig = new QuicConfig(quicEnabled);
-        HoppingConfig hoppingConfig = new HoppingConfig(hoppingEnabled,IP+":"+PORT, options);
-        ObfsvpnConfig obfsvpnConfig = new ObfsvpnConfig(IP+":"+PORT, hoppingConfig, kcpConfig, quicConfig, options.bridgeIP, options.transport.getPorts()[0], options.transport.getOptions().getCert() );
+        HoppingConfig hoppingConfig = new HoppingConfig(hoppingEnabled,IP+":"+ DEFAULT_PORT, options);
+        ObfsvpnConfig obfsvpnConfig = new ObfsvpnConfig(IP+":"+ DEFAULT_PORT, hoppingConfig, kcpConfig, quicConfig, options.bridgeIP, options.transport.getPorts()[0], options.transport.getOptions().getCert() );
         try {
-            Log.d(TAG, obfsvpnConfig.toString());
+            Log.d(TAG, "create new obfsvpn client: " + obfsvpnConfig);
             client = Client.newFFIClient(obfsvpnConfig.toString());
         } catch (Exception e) {
             throw new IllegalStateException(e);
         }
     }
 
-    public int start() {
+    public void start() throws RuntimeException {
         synchronized (LOCK) {
+            client.setEventLogger(this);
+
+            // this CountDownLatch stops blocking if:
+            // a) obfsvpn changed its state to RUNNING
+            // b) an unrecoverable error happened
+            final CountDownLatch callback = new CountDownLatch(1);
+            this.startCallback = callback;
+            AtomicReference<Exception> err = new AtomicReference<>();
             new Thread(() -> {
                 try {
-                    if (client.isStarted()) {
-                        return;
-                    }
-                    client.setEventLogger(this);
-                    client.start();
-                } catch (Exception e) {
+                    start(0);
+                } catch (RuntimeException e) {
+                    // save exception and stop blocking
                     e.printStackTrace();
+                    err.set(e);
+                    callback.countDown();
                 }
             }).start();
-            return PORT;
+
+            try {
+                boolean completedBeforeTimeout = callback.await(35, TimeUnit.SECONDS);
+                Exception startException =  err.get();
+                this.startCallback = null;
+                if (!completedBeforeTimeout) {
+                    client.setEventLogger(null);
+                    throw new RuntimeException("failed to start obfsvpn: timeout error");
+                } else if (startException != null) {
+                    client.setEventLogger(null);
+                    throw new RuntimeException("failed to start obfsvpn: ", startException);
+                }
+            } catch (InterruptedException e) {
+                this.startCallback = null;
+                client.setEventLogger(null);
+                throw new RuntimeException("failed to start obfsvpn: ", e);
+            }
+        }
+    }
+
+    private void start(int portOffset) throws RuntimeException {
+        currentPort.set(DEFAULT_PORT + portOffset);
+        Log.d(TAG, "listen to 127.0.0.1:"+ (currentPort.get()));
+        final CountDownLatch errOnStartCDL = new CountDownLatch(1);
+        AtomicReference<Exception> err = new AtomicReference<>();
+        new Thread(() -> {
+            try {
+                client.setProxyAddr(IP + ":" + (DEFAULT_PORT+portOffset));
+                client.start();
+            } catch (Exception e) {
+                err.set(e);
+                errOnStartCDL.countDown();
+            }
+        }).start();
+
+        try {
+            // wait for 250 ms, in case there is an immediate error due to misconfiguration
+            // or bound ports the CountDownLatch is set to 0 and thus the return value of await is true
+            boolean receivedErr = errOnStartCDL.await(250, TimeUnit.MILLISECONDS);
+            if (receivedErr) {
+                Exception e = err.get();
+                // attempt to restart the client with a different local proxy port in case
+                // there's a port binding error
+                if (e != null &&
+                        e.getMessage() != null &&
+                        e.getMessage().contains(ERROR_BIND) &&
+                        portOffset < 10) {
+                    start(portOffset + 1);
+                    return;
+                } else {
+                    resetAndThrow(new RuntimeException("Failed to start obfsvpn: " + e));
+                }
+            }
+        } catch (InterruptedException e) {
+            resetAndThrow(new RuntimeException(e));
         }
     }
 
-    public void stop() {
+    private void resetAndThrow(RuntimeException e) throws RuntimeException{
+        startCallback.countDown();
+        startCallback = null;
+        client.setEventLogger(null);
+        throw e;
+    }
+
+    public boolean stop() {
         synchronized (LOCK) {
             try {
                 client.stop();
             } catch (Exception e) {
                 e.printStackTrace();
+                return false;
             } finally {
                 client.setEventLogger(null);
             }
+            return true;
         }
     }
 
+    public int getPort() {
+        return currentPort.get();
+    }
+
     public boolean isStarted() {
         return client.isStarted();
     }
 
     @Override
     public void error(String s) {
-        VpnStatus.logError("[obfs4-client] " + s);
-
+        VpnStatus.logError("[obfs4-client] error: " + s);
     }
 
     @Override
     public void log(String state, String message) {
         VpnStatus.logDebug("[obfs4-client] " + state + ": " + message);
+        CountDownLatch startCallback = this.startCallback;
+        if (startCallback != null && "RUNNING".equals(state)) {
+            startCallback.countDown();
+            this.startCallback = null;
+        }
     }
 }