Merge branch 'bug/overly-complex-certificate-verification-method' into develop

5 files changed, 209 insertions, 79 deletions

diff --git a/res/values/strings.xml b/res/values/strings.xml
index 39c82d40..1abd85cd 100755
--- a/res/values/strings.xml
+++ b/res/values/strings.xml
@@ -10,6 +10,7 @@
     <string name="cant_read_folder">Unable to read directory</string>
     <string name="select">Select</string>
     <string name="cancel">Cancel</string>
+    <string name="ok">OK</string>
     <string name="no_data">No Data</string>
     <string name="useLZO">LZO Compression</string>
     <string name="client_no_certificate">No Certificate</string>
diff --git a/src/se/leap/bitmaskclient/ConfigHelper.java b/src/se/leap/bitmaskclient/ConfigHelper.java
index b916a9ac..d2253dfe 100644
--- a/src/se/leap/bitmaskclient/ConfigHelper.java
+++ b/src/se/leap/bitmaskclient/ConfigHelper.java
@@ -178,6 +178,18 @@ public class ConfigHelper {
 		shared_preferences_editor.remove(shared_preferences_key);
 		return shared_preferences_editor.commit();
 	}
+	
+	public static boolean checkErroneousDownload(String downloaded_string) {
+		try {
+			if(new JSONObject(downloaded_string).has(ProviderAPI.ERRORS) || downloaded_string.isEmpty()) {
+				return true;
+			} else {
+				return false;
+			}
+		} catch(JSONException e) {
+			return false;
+		}
+	}
 
 	/**
 	 *  Treat the input as the MSB representation of a number,
diff --git a/src/se/leap/bitmaskclient/ConfigurationWizard.java b/src/se/leap/bitmaskclient/ConfigurationWizard.java
index a0ac1bc2..c03d7c9e 100644
--- a/src/se/leap/bitmaskclient/ConfigurationWizard.java
+++ b/src/se/leap/bitmaskclient/ConfigurationWizard.java
@@ -124,8 +124,10 @@ implements ProviderListFragment.Callbacks, NewProviderDialog.NewProviderDialogIn
 				//Toast.makeText(this, getResources().getString(R.string.config_error_parsing), Toast.LENGTH_LONG);

 				setResult(RESULT_CANCELED, mConfigState);

 			}

-		}

+		}
 		else if(resultCode == ProviderAPI.INCORRECTLY_UPDATED_PROVIDER_DOT_JSON) {

+			String reason_to_fail = resultData.getString(ProviderAPI.ERRORS);

+			showDownloadFailedDialog(getCurrentFocus(), reason_to_fail);
 			mProgressDialog.dismiss();

 			setResult(RESULT_CANCELED, mConfigState);

 		}

@@ -142,8 +144,9 @@ implements ProviderListFragment.Callbacks, NewProviderDialog.NewProviderDialogIn
 			}

 		}

 		else if(resultCode == ProviderAPI.INCORRECTLY_DOWNLOADED_JSON_FILES) {

-			//Toast.makeText(getApplicationContext(), R.string.incorrectly_downloaded_json_files_message, Toast.LENGTH_LONG).show();

-			mProgressDialog.dismiss();

+			//Toast.makeText(getApplicationContext(), R.string.incorrectly_downloaded_json_files_message, Toast.LENGTH_LONG).show();
+			String reason_to_fail = resultData.getString(ProviderAPI.ERRORS);

+			showDownloadFailedDialog(getCurrentFocus(), reason_to_fail);
 			setResult(RESULT_CANCELED, mConfigState);

 		}

 		else if(resultCode == ProviderAPI.CORRECTLY_DOWNLOADED_CERTIFICATE) {

@@ -319,6 +322,25 @@ implements ProviderListFragment.Callbacks, NewProviderDialog.NewProviderDialogIn
 	 * use it anonymously (if possible) 

 	 * or cancel his/her election pressing the back button.

 	 * @param view

+	 * @param reason_to_fail 

+	 */

+	public void showDownloadFailedDialog(View view, String reason_to_fail) {

+		FragmentTransaction fragment_transaction = getFragmentManager().beginTransaction();

+		Fragment previous_provider_details_dialog = getFragmentManager().findFragmentByTag(DownloadFailedDialog.TAG);

+		if (previous_provider_details_dialog != null) {

+			fragment_transaction.remove(previous_provider_details_dialog);

+		}

+		fragment_transaction.addToBackStack(null);

+		

+		DialogFragment newFragment = DownloadFailedDialog.newInstance(reason_to_fail);

+		newFragment.show(fragment_transaction, DownloadFailedDialog.TAG);

+	}

+	

+	/**

+	 * Once selected a provider, this fragment offers the user to log in, 

+	 * use it anonymously (if possible) 

+	 * or cancel his/her election pressing the back button.

+	 * @param view

 	 */

 	public void showProviderDetails(View view) {

 		FragmentTransaction fragment_transaction = getFragmentManager().beginTransaction();

diff --git a/src/se/leap/bitmaskclient/DownloadFailedDialog.java b/src/se/leap/bitmaskclient/DownloadFailedDialog.java
new file mode 100644
index 00000000..3ce101a6
--- /dev/null
+++ b/src/se/leap/bitmaskclient/DownloadFailedDialog.java
@@ -0,0 +1,59 @@
+/**
+ * Copyright (c) 2013 LEAP Encryption Access Project and contributers
+ * 
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation, either version 3 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program. If not, see <http://www.gnu.org/licenses/>.
+ */
+ package se.leap.bitmaskclient;
+
+import se.leap.bitmaskclient.R;
+import android.app.AlertDialog;
+import android.app.Dialog;
+import android.app.DialogFragment;
+import android.content.DialogInterface;
+import android.os.Bundle;
+
+/**
+ * Implements a dialog to show why a download failed.
+ * 
+ * @author parmegv
+ *
+ */
+public class DownloadFailedDialog extends DialogFragment {
+
+	public static String TAG = "downloaded_failed_dialog";
+	private String reason_to_fail;
+	/**
+	 * @return a new instance of this DialogFragment.
+	 */
+	public static DialogFragment newInstance(String reason_to_fail) {
+		DownloadFailedDialog dialog_fragment = new DownloadFailedDialog();
+		dialog_fragment.reason_to_fail = reason_to_fail;
+		return dialog_fragment;
+	}
+
+    @Override
+	public Dialog onCreateDialog(Bundle savedInstanceState) {
+		AlertDialog.Builder builder = new AlertDialog.Builder(getActivity());
+		
+		builder.setMessage(reason_to_fail)
+		.setPositiveButton(R.string.ok, new DialogInterface.OnClickListener() {
+			public void onClick(DialogInterface dialog, int id) {
+				dialog.dismiss();
+			}
+		});
+			
+		// Create the AlertDialog object and return it
+		return builder.create();
+	}
+}
diff --git a/src/se/leap/bitmaskclient/ProviderAPI.java b/src/se/leap/bitmaskclient/ProviderAPI.java
index 39b44e24..1408efc8 100644
--- a/src/se/leap/bitmaskclient/ProviderAPI.java
+++ b/src/se/leap/bitmaskclient/ProviderAPI.java
@@ -57,6 +57,7 @@ import org.apache.http.client.ClientProtocolException;
 import org.jboss.security.srp.SRPParameters;
 import org.json.JSONException;
 import org.json.JSONObject;
+import org.json.JSONStringer;
 
 import se.leap.bitmaskclient.R;
 import se.leap.bitmaskclient.ProviderListContent.ProviderItem;
@@ -128,14 +129,8 @@ public class ProviderAPI extends IntentService {
 		CookieHandler.setDefault(new CookieManager(null, CookiePolicy.ACCEPT_ORIGINAL_SERVER) );
 	}
 	
-	private void displayToast(final int toast_string_id) {
-		mHandler.post(new Runnable() {
-			
-			@Override
-			public void run() {
-	            Toast.makeText(ProviderAPI.this, toast_string_id, Toast.LENGTH_LONG).show();                
-			}
-		});
+	private String formatErrorMessage(final int toast_string_id) {
+		return "{ \"" + ERRORS + "\" : \""+getResources().getString(toast_string_id)+"\" }";
 	}
 
 	@Override
@@ -145,10 +140,11 @@ public class ProviderAPI extends IntentService {
 		Bundle parameters = command.getBundleExtra(PARAMETERS);
 		
 		if(action.equalsIgnoreCase(DOWNLOAD_JSON_FILES_BUNDLE_EXTRA)) {
-			if(!downloadJsonFiles(parameters)) {
-				receiver.send(INCORRECTLY_DOWNLOADED_JSON_FILES, Bundle.EMPTY);
-			} else { 
+			Bundle result = downloadJsonFiles(parameters);
+			if(result.getBoolean(RESULT_KEY)) {
 				receiver.send(CORRECTLY_DOWNLOADED_JSON_FILES, Bundle.EMPTY);
+			} else {
+				receiver.send(INCORRECTLY_DOWNLOADED_JSON_FILES, result);
 			}
 		} else if(action.equalsIgnoreCase(UPDATE_PROVIDER_DOTJSON)) {
 			Bundle result = updateProviderDotJSON(parameters);
@@ -162,7 +158,7 @@ public class ProviderAPI extends IntentService {
 			if(result.getBoolean(RESULT_KEY)) {
 				receiver.send(CORRECTLY_UPDATED_PROVIDER_DOT_JSON, result);
 			} else {
-				receiver.send(INCORRECTLY_DOWNLOADED_JSON_FILES, Bundle.EMPTY);
+				receiver.send(INCORRECTLY_DOWNLOADED_JSON_FILES, result);
 			}
 		} else if (action.equalsIgnoreCase(SRP_AUTH)) {
 			Bundle session_id_bundle = authenticateBySRP(parameters);
@@ -191,28 +187,53 @@ public class ProviderAPI extends IntentService {
 	 * @param task
 	 * @return true if eip-service.json was parsed as a JSON object correctly.
 	 */
-	private boolean downloadJsonFiles(Bundle task) {
+	private Bundle downloadJsonFiles(Bundle task) {
+		Bundle result = new Bundle();
 		String cert_url = task.getString(Provider.CA_CERT);
 		String eip_service_json_url = task.getString(EIP.KEY);
 		boolean danger_on = task.getBoolean(ProviderItem.DANGER_ON);
 		try {
 			String cert_string = downloadWithCommercialCA(cert_url, danger_on);
-			if(cert_string.isEmpty()) return false;
-			X509Certificate certCert = ConfigHelper.parseX509CertificateFromString(cert_string);
-			cert_string = Base64.encodeToString( certCert.getEncoded(), Base64.DEFAULT);
-			ConfigHelper.saveSharedPref(Provider.CA_CERT, "-----BEGIN CERTIFICATE-----\n"+cert_string+"-----END CERTIFICATE-----");
-			
-			String eip_service_string = downloadWithCommercialCA(eip_service_json_url, danger_on);
-			ConfigHelper.saveSharedPref(EIP.KEY, new JSONObject(eip_service_string));
-			
-			return true;
+
+			if(ConfigHelper.checkErroneousDownload(cert_string)) {
+				JSONObject possible_errors = new JSONObject(cert_string);
+				String reason_to_fail = "";
+				if(cert_string.isEmpty())
+					reason_to_fail = "Empty certificate downloaded";
+				else
+				reason_to_fail = possible_errors.getString(ERRORS);
+				result.putString(ERRORS, reason_to_fail);
+				result.putBoolean(RESULT_KEY, false);
+			} else {
+				X509Certificate certCert = ConfigHelper.parseX509CertificateFromString(cert_string);
+				cert_string = Base64.encodeToString( certCert.getEncoded(), Base64.DEFAULT);
+				ConfigHelper.saveSharedPref(Provider.CA_CERT, "-----BEGIN CERTIFICATE-----\n"+cert_string+"-----END CERTIFICATE-----");
+			}
 		} catch (JSONException e) {
-			return false;
+			e.printStackTrace();
+			result.putBoolean(RESULT_KEY, false);
 		} catch (CertificateException e) {
 			// TODO Auto-generated catch block
 			e.printStackTrace();
-			return false;
+			result.putBoolean(RESULT_KEY, false);
+		}
+
+		try {
+			String eip_service_string = downloadWithCommercialCA(eip_service_json_url, danger_on);
+			JSONObject eip_service_json = new JSONObject(eip_service_string);
+			if(eip_service_json.has(ERRORS)) {
+				String reason_to_fail = eip_service_json.getString(ERRORS);
+				result.putString(ERRORS, reason_to_fail);
+				result.putBoolean(RESULT_KEY, false);
+			}
+			else ConfigHelper.saveSharedPref(EIP.KEY, eip_service_json);
+
+			result.putBoolean(RESULT_KEY, true);
+		} catch (JSONException e) {
+			result.putBoolean(RESULT_KEY, false);
 		}
+		
+		return result;
 	}
 	
 	/**
@@ -452,12 +473,18 @@ public class ProviderAPI extends IntentService {
 				result.putBoolean(RESULT_KEY, false);
 			} else {
 				JSONObject provider_json = new JSONObject(provider_dot_json_string);
-				ConfigHelper.saveSharedPref(EIP.ALLOWED_ANON, provider_json.getJSONObject(Provider.SERVICE).getBoolean(EIP.ALLOWED_ANON));
+				if(provider_json.has(ERRORS)) {
+					String reason_to_fail = provider_json.getString(ERRORS);
+					result.putString(ERRORS, reason_to_fail);
+					result.putBoolean(RESULT_KEY, false);
+				} else {
+					ConfigHelper.saveSharedPref(EIP.ALLOWED_ANON, provider_json.getJSONObject(Provider.SERVICE).getBoolean(EIP.ALLOWED_ANON));
 
-				//ProviderListContent.addItem(new ProviderItem(provider_name, provider_json_url, provider_json, custom, danger_on));
-				result.putBoolean(RESULT_KEY, true);
-				result.putString(Provider.KEY, provider_json.toString());
-				result.putBoolean(ProviderItem.DANGER_ON, danger_on);
+					//ProviderListContent.addItem(new ProviderItem(provider_name, provider_json_url, provider_json, custom, danger_on));
+					result.putBoolean(RESULT_KEY, true);
+					result.putString(Provider.KEY, provider_json.toString());
+					result.putBoolean(ProviderItem.DANGER_ON, danger_on);
+				}
 			}
 		} catch (JSONException e) {
 			result.putBoolean(RESULT_KEY, false);
@@ -487,19 +514,26 @@ public class ProviderAPI extends IntentService {
 			} else {
 				JSONObject provider_json = new JSONObject(provider_json_string);
 
-				ConfigHelper.saveSharedPref(Provider.KEY, provider_json);
-				ConfigHelper.saveSharedPref(ProviderItem.DANGER_ON, danger_on);
-				ConfigHelper.saveSharedPref(EIP.ALLOWED_ANON, provider_json.getJSONObject(Provider.SERVICE).getBoolean(EIP.ALLOWED_ANON));
-				ProviderItem added_provider = new ProviderItem(provider_name, provider_json_url, provider_json, custom, danger_on);
-				ProviderListContent.addItem(added_provider);
-
-				result.putString(Provider.NAME, added_provider.getName());
-				result.putBoolean(RESULT_KEY, true);
-				result.putString(Provider.KEY, provider_json.toString());
-				result.putBoolean(ProviderItem.DANGER_ON, danger_on);
+				if(provider_json.has(ERRORS)) {
+					String reason_to_fail = provider_json.getString(ERRORS);
+					result.putString(ERRORS, reason_to_fail);
+					result.putBoolean(RESULT_KEY, false);
+				} else {
+					ConfigHelper.saveSharedPref(Provider.KEY, provider_json);
+					ConfigHelper.saveSharedPref(ProviderItem.DANGER_ON, danger_on);
+					ConfigHelper.saveSharedPref(EIP.ALLOWED_ANON, provider_json.getJSONObject(Provider.SERVICE).getBoolean(EIP.ALLOWED_ANON));
+					ProviderItem added_provider = new ProviderItem(provider_name, provider_json_url, provider_json, custom, danger_on);
+					ProviderListContent.addItem(added_provider);
+
+					result.putString(Provider.NAME, added_provider.getName());
+					result.putBoolean(RESULT_KEY, true);
+					result.putString(Provider.KEY, provider_json.toString());
+					result.putBoolean(ProviderItem.DANGER_ON, danger_on);
+				}
 			}
 		} catch (JSONException e) {
 			result.putBoolean(RESULT_KEY, false);
+			result.putString(ERRORS, "Corrupt download");
 		}
 		
 		return result;
@@ -525,17 +559,14 @@ public class ProviderAPI extends IntentService {
 			url_connection.setConnectTimeout(seconds_of_timeout*1000);
 			json_file_content = new Scanner(url_connection.getInputStream()).useDelimiter("\\A").next();
 		} catch (MalformedURLException e) {
-			displayToast(R.string.malformed_url);
+			json_file_content = formatErrorMessage(R.string.malformed_url);
 		} catch(SocketTimeoutException e) {
-			displayToast(R.string.server_is_down_message);
-		} catch (FileNotFoundException e) {
-			e.printStackTrace();
-			displayToast(R.string.server_is_down_message);
+			json_file_content = formatErrorMessage(R.string.server_is_down_message);
 		} catch (IOException e) {
 			if(provider_url != null) {
 				json_file_content = downloadWithProviderCA(provider_url, danger_on);
 			} else {
-				displayToast(R.string.certificate_error);
+				json_file_content = formatErrorMessage(R.string.certificate_error);
 			}
 		} catch (Exception e) {
 			if(provider_url != null && danger_on) {
@@ -547,7 +578,7 @@ public class ProviderAPI extends IntentService {
 	}
 
 	/**
-	 * Tries to download the contents of the provided url using not commercially validated CA certificate from chosen provider. 
+	 * Tries to download the contents of the provided url using not commercially validated CA certificate from chosen provider.
 	 * @param url
 	 * @param danger_on true to download CA certificate in case it has not been downloaded.
 	 * @return an empty string if it fails, the url content if not. 
@@ -565,16 +596,13 @@ public class ProviderAPI extends IntentService {
 			// TODO Auto-generated catch block
 			e.printStackTrace();
 		} catch (UnknownHostException e) {
-			displayToast(R.string.server_is_down_message);
-		} catch (FileNotFoundException e) {
-			e.printStackTrace();
-			displayToast(R.string.server_is_down_message);
+			json_file_content = formatErrorMessage(R.string.server_is_down_message);
 		} catch (IOException e) {
 			// The downloaded certificate doesn't validate our https connection.
 			if(danger_on) {
 				json_file_content = downloadWithoutCA(url);
 			} else {
-				displayToast(R.string.certificate_error);
+				json_file_content = formatErrorMessage(R.string.certificate_error);
 			}
 		} catch (KeyStoreException e) {
 			// TODO Auto-generated catch block
@@ -650,11 +678,11 @@ public class ProviderAPI extends IntentService {
 			System.out.println("String ignoring certificate = " + string);
 		} catch (FileNotFoundException e) {
 			e.printStackTrace();
-			displayToast(R.string.server_is_down_message);
+			string = formatErrorMessage(R.string.server_is_down_message);
 		} catch (IOException e) {
 			// The downloaded certificate doesn't validate our https connection.
 			e.printStackTrace();
-			displayToast(R.string.certificate_error);
+			string = formatErrorMessage(R.string.certificate_error);
 		} catch (NoSuchAlgorithmException e) {
 			// TODO Auto-generated catch block
 			e.printStackTrace();
@@ -734,32 +762,40 @@ public class ProviderAPI extends IntentService {
 			boolean danger_on = ConfigHelper.getBoolFromSharedPref(ProviderItem.DANGER_ON);
 			String cert_string = downloadWithCommercialCA(new_cert_string_url, danger_on);
 			if(!cert_string.isEmpty()) {
-				// API returns concatenated cert & key.  Split them for OpenVPN options
-				String certificateString = null, keyString = null;
-				String[] certAndKey = cert_string.split("(?<=-\n)");
-				for (int i=0; i < certAndKey.length-1; i++){
-					if ( certAndKey[i].contains("KEY") ) {
-						keyString = certAndKey[i++] + certAndKey[i];
+				if(ConfigHelper.checkErroneousDownload(cert_string)) {
+					String reason_to_fail = provider_json.getString(ERRORS);
+					//result.putString(ConfigHelper.ERRORS_KEY, reason_to_fail);
+					//result.putBoolean(ConfigHelper.RESULT_KEY, false);
+					return false;
+				} else {
+					
+					// API returns concatenated cert & key.  Split them for OpenVPN options
+					String certificateString = null, keyString = null;
+					String[] certAndKey = cert_string.split("(?<=-\n)");
+					for (int i=0; i < certAndKey.length-1; i++){
+						if ( certAndKey[i].contains("KEY") ) {
+							keyString = certAndKey[i++] + certAndKey[i];
+						}
+						else if ( certAndKey[i].contains("CERTIFICATE") ) {
+							certificateString = certAndKey[i++] + certAndKey[i];
+						}
 					}
-					else if ( certAndKey[i].contains("CERTIFICATE") ) {
-						certificateString = certAndKey[i++] + certAndKey[i];
+					try {
+						RSAPrivateKey keyCert = ConfigHelper.parseRsaKeyFromString(keyString);
+						keyString = Base64.encodeToString( keyCert.getEncoded(), Base64.DEFAULT );
+						ConfigHelper.saveSharedPref(EIP.PRIVATE_KEY, "-----BEGIN RSA PRIVATE KEY-----\n"+keyString+"-----END RSA PRIVATE KEY-----");
+
+						X509Certificate certCert = ConfigHelper.parseX509CertificateFromString(certificateString);
+						certificateString = Base64.encodeToString( certCert.getEncoded(), Base64.DEFAULT);
+						ConfigHelper.saveSharedPref(EIP.CERTIFICATE, "-----BEGIN CERTIFICATE-----\n"+certificateString+"-----END CERTIFICATE-----");
+
+						return true;
+					} catch (CertificateException e) {
+						// TODO Auto-generated catch block
+						e.printStackTrace();
+						return false;
 					}
 				}
-				try {
-					RSAPrivateKey keyCert = ConfigHelper.parseRsaKeyFromString(keyString);
-					keyString = Base64.encodeToString( keyCert.getEncoded(), Base64.DEFAULT );
-					ConfigHelper.saveSharedPref(EIP.PRIVATE_KEY, "-----BEGIN RSA PRIVATE KEY-----\n"+keyString+"-----END RSA PRIVATE KEY-----");
-					
-					X509Certificate certCert = ConfigHelper.parseX509CertificateFromString(certificateString);
-					certificateString = Base64.encodeToString( certCert.getEncoded(), Base64.DEFAULT);
-					ConfigHelper.saveSharedPref(EIP.CERTIFICATE, "-----BEGIN CERTIFICATE-----\n"+certificateString+"-----END CERTIFICATE-----");
-					
-					return true;
-				} catch (CertificateException e) {
-					// TODO Auto-generated catch block
-					e.printStackTrace();
-					return false;
-				}
 			} else {
 				return false;
 			}