android: Make fetching OCSP/CRL interruptible

This allows cancelling connecting if e.g. the OCSP server is not
reachable. Previously this caused some delay in disconnecting state but
even worse it cause an ANR if the user tried reconnecting during that
time as the main thread would get struck in setNextProfile() (we could
probably find a better solution there too in the future).

diff --git a/src/frontends/android/app/src/main/java/org/strongswan/android/logic/CharonVpnService.java b/src/frontends/android/app/src/main/java/org/strongswan/android/logic/CharonVpnService.java
index 2f402de5dbcc551b6972e60fc84c6d5ef47b8fec..34e7938940bd3ae0e9c13d68675b913b57e97f96 100644 (file)
--- a/src/frontends/android/app/src/main/java/org/strongswan/android/logic/CharonVpnService.java
+++ b/src/frontends/android/app/src/main/java/org/strongswan/android/logic/CharonVpnService.java
@@ -283,6 +283,7 @@ public class CharonVpnService extends VpnService implements Runnable, VpnStateSe
                                                startConnection(mCurrentProfile);
                                                mIsDisconnecting = false;
 
+                                               SimpleFetcher.enable();
                                                addNotification();
                                                mBuilderAdapter.setProfile(mCurrentProfile);
                                                if (initializeCharon(mBuilderAdapter, mLogFile, mAppDir, mCurrentProfile.getVpnType().has(VpnTypeFeature.BYOD)))
@@ -350,6 +351,7 @@ public class CharonVpnService extends VpnService implements Runnable, VpnStateSe
                        {
                                setState(State.DISCONNECTING);
                                mIsDisconnecting = true;
+                               SimpleFetcher.disable();
                                deinitializeCharon();
                                Log.i(TAG, "charon stopped");
                                mCurrentProfile = null;

diff --git a/src/frontends/android/app/src/main/java/org/strongswan/android/logic/SimpleFetcher.java b/src/frontends/android/app/src/main/java/org/strongswan/android/logic/SimpleFetcher.java
index b7334cee1df8f7298f591f9d8b13d6134181390b..6eaccc4d8d8ef3314d40f07aec47a394a9bdb3ae 100644 (file)
--- a/src/frontends/android/app/src/main/java/org/strongswan/android/logic/SimpleFetcher.java
+++ b/src/frontends/android/app/src/main/java/org/strongswan/android/logic/SimpleFetcher.java
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2017 Tobias Brunner
+ * Copyright (C) 2017-2018 Tobias Brunner
  * HSR Hochschule fuer Technik Rapperswil
  *
  * This program is free software; you can redistribute it and/or modify it
@@ -23,36 +23,118 @@ import java.io.IOException;
 import java.io.InputStream;
 import java.io.OutputStream;
 import java.net.HttpURLConnection;
+import java.net.SocketTimeoutException;
 import java.net.URL;
+import java.util.ArrayList;
+import java.util.concurrent.CancellationException;
+import java.util.concurrent.ExecutionException;
+import java.util.concurrent.ExecutorService;
+import java.util.concurrent.Executors;
+import java.util.concurrent.Future;
+import java.util.concurrent.TimeUnit;
+import java.util.concurrent.TimeoutException;
 
 @Keep
 public class SimpleFetcher
 {
-       public static byte[] fetch(String uri, byte[] data, String contentType) throws IOException
+       private static ExecutorService mExecutor = Executors.newCachedThreadPool();
+       private static Object mLock = new Object();
+       private static ArrayList<Future> mFutures = new ArrayList<>();
+       private static boolean mDisabled;
+
+       public static byte[] fetch(String uri, byte[] data, String contentType)
        {
-               URL url = new URL(uri);
-               HttpURLConnection conn = (HttpURLConnection)url.openConnection();
-               conn.setConnectTimeout(10000);
-               conn.setReadTimeout(10000);
-               try
+               Future<byte[]> future;
+
+               synchronized (mLock)
                {
-                       if (contentType != null)
+                       if (mDisabled)
                        {
-                               conn.setRequestProperty("Content-Type", contentType);
+                               return null;
                        }
-                       if (data != null)
+                       future = mExecutor.submit(() -> {
+                               URL url = new URL(uri);
+                               HttpURLConnection conn = (HttpURLConnection) url.openConnection();
+                               conn.setConnectTimeout(10000);
+                               conn.setReadTimeout(10000);
+                               try
+                               {
+                                       if (contentType != null)
+                                       {
+                                               conn.setRequestProperty("Content-Type", contentType);
+                                       }
+                                       if (data != null)
+                                       {
+                                               conn.setDoOutput(true);
+                                               conn.setFixedLengthStreamingMode(data.length);
+                                               OutputStream out = new BufferedOutputStream(conn.getOutputStream());
+                                               out.write(data);
+                                               out.close();
+                                       }
+                                       return streamToArray(conn.getInputStream());
+                               }
+                               catch (SocketTimeoutException e)
+                               {
+                                       return null;
+                               }
+                               finally
+                               {
+                                       conn.disconnect();
+                               }
+                       });
+
+                       mFutures.add(future);
+               }
+
+               try
+               {
+                       /* this enforces a timeout as the ones set on HttpURLConnection might not work reliably */
+                       return future.get(10000, TimeUnit.MILLISECONDS);
+               }
+               catch (InterruptedException|ExecutionException|TimeoutException|CancellationException e)
+               {
+                       return null;
+               }
+               finally
+               {
+                       synchronized (mLock)
                        {
-                               conn.setDoOutput(true);
-                               conn.setFixedLengthStreamingMode(data.length);
-                               OutputStream out = new BufferedOutputStream(conn.getOutputStream());
-                               out.write(data);
-                               out.close();
+                               mFutures.remove(future);
                        }
-                       return streamToArray(conn.getInputStream());
                }
-               finally
+       }
+
+       /**
+        * Enable fetching after it has been disabled.
+        */
+       public static void enable()
+       {
+               synchronized (mLock)
+               {
+                       mDisabled = false;
+               }
+       }
+
+       /**
+        * Disable the fetcher and abort any future requests.
+        *
+        * The native thread is not cancelable as it is working on an IKE_SA (cancelling the methods of
+        * HttpURLConnection is not reliably possible anyway), so to abort while fetching we cancel the
+        * Future (causing a return from fetch() immediately) and let the executor thread continue its
+        * thing in the background.
+        *
+        * Also prevents future fetches until enabled again (e.g. if we aborted OCSP but would then
+        * block in the subsequent fetch for a CRL).
+        */
+       public static void disable()
+       {
+               synchronized (mLock)
                {
-                       conn.disconnect();
+                       mDisabled = true;
+                       for (Future future : mFutures)
+                       {
+                               future.cancel(true);
+                       }
                }
        }