From: Tobias Brunner <tobias@strongswan.org>
Date: Thu, 21 Jun 2018 09:17:22 +0000 (+0200)
Subject: android: Make fetching OCSP/CRL interruptible
X-Git-Tag: 5.7.0dr5~20^2~17
X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=ad2d20e5f0174ad629b942f080982fcfbf843d22;p=thirdparty%2Fstrongswan.git

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 2f402de5db..34e7938940 100644
--- 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 b7334cee1d..6eaccc4d8d 100644
--- 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);
+			}
 		}
 	}