From ca82cdf77ee4d30b820a1f936315c6c5be78359d Mon Sep 17 00:00:00 2001 From: Fup Duck Date: Sun, 11 Feb 2018 13:25:24 +0100 Subject: 8827 - discussion * validate urls before changing anything in Provider.define() * save private key and vpn cert after login/signup --- .../se/leap/bitmaskclient/ProviderApiManager.java | 8 +++++- .../java/se/leap/bitmaskclient/ConfigHelper.java | 5 ++++ .../java/se/leap/bitmaskclient/EipFragment.java | 5 ++-- .../main/java/se/leap/bitmaskclient/Provider.java | 24 +++++++++++++++--- .../leap/bitmaskclient/ProviderApiManagerBase.java | 29 ++++++++++++++++++---- .../ProviderCredentialsBaseActivity.java | 14 ++++++++--- .../se/leap/bitmaskclient/ProviderApiManager.java | 9 +++++-- 7 files changed, 77 insertions(+), 17 deletions(-) (limited to 'app') diff --git a/app/src/insecure/java/se/leap/bitmaskclient/ProviderApiManager.java b/app/src/insecure/java/se/leap/bitmaskclient/ProviderApiManager.java index d57dfe6d..1c5247c0 100644 --- a/app/src/insecure/java/se/leap/bitmaskclient/ProviderApiManager.java +++ b/app/src/insecure/java/se/leap/bitmaskclient/ProviderApiManager.java @@ -52,10 +52,12 @@ import static se.leap.bitmaskclient.Constants.BROADCAST_RESULT_KEY; import static se.leap.bitmaskclient.Constants.PROVIDER_KEY; import static se.leap.bitmaskclient.Constants.PROVIDER_VPN_CERTIFICATE; import static se.leap.bitmaskclient.DownloadFailedDialog.DOWNLOAD_ERRORS.ERROR_CERTIFICATE_PINNING; +import static se.leap.bitmaskclient.DownloadFailedDialog.DOWNLOAD_ERRORS.ERROR_CORRUPTED_PROVIDER_JSON; import static se.leap.bitmaskclient.ProviderAPI.ERRORS; import static se.leap.bitmaskclient.R.string.certificate_error; import static se.leap.bitmaskclient.R.string.malformed_url; import static se.leap.bitmaskclient.R.string.warning_corrupted_provider_cert; +import static se.leap.bitmaskclient.R.string.warning_corrupted_provider_details; /** * Created by cyberta on 04.01.18. @@ -145,7 +147,11 @@ public class ProviderApiManager extends ProviderApiManagerBase { try { JSONObject providerJson = new JSONObject(providerDotJsonString); - provider.define(providerJson); + if (provider.define(providerJson)) { + result.putBoolean(BROADCAST_RESULT_KEY, true); + } else { + return setErrorResult(result, warning_corrupted_provider_details, ERROR_CORRUPTED_PROVIDER_JSON.toString()); + } result.putBoolean(BROADCAST_RESULT_KEY, true); } catch (JSONException e) { diff --git a/app/src/main/java/se/leap/bitmaskclient/ConfigHelper.java b/app/src/main/java/se/leap/bitmaskclient/ConfigHelper.java index 329fd543..f8204b20 100644 --- a/app/src/main/java/se/leap/bitmaskclient/ConfigHelper.java +++ b/app/src/main/java/se/leap/bitmaskclient/ConfigHelper.java @@ -281,6 +281,7 @@ public class ConfigHelper { provider.setMainUrl(new URL(preferences.getString(Provider.MAIN_URL, ""))); provider.define(new JSONObject(preferences.getString(Provider.KEY, ""))); provider.setCaCert(preferences.getString(Provider.CA_CERT, "")); + provider.setCaCertFingerprint(preferences.getString(Provider.CA_CERT_FINGERPRINT, "")); provider.setVpnCertificate(preferences.getString(PROVIDER_VPN_CERTIFICATE, "")); provider.setPrivateKey(preferences.getString(PROVIDER_PRIVATE_KEY, "")); } catch (MalformedURLException | JSONException e) { @@ -290,6 +291,10 @@ public class ConfigHelper { return provider; } + public String getFromPersistedProvider(String toFetch, String providerDomain, SharedPreferences preferences) { + return preferences.getString(toFetch + "." + providerDomain, ""); + } + public static String getProviderName(String provider) { return getProviderName(null, provider); } diff --git a/app/src/main/java/se/leap/bitmaskclient/EipFragment.java b/app/src/main/java/se/leap/bitmaskclient/EipFragment.java index b4c7a7de..41d9ff04 100644 --- a/app/src/main/java/se/leap/bitmaskclient/EipFragment.java +++ b/app/src/main/java/se/leap/bitmaskclient/EipFragment.java @@ -63,6 +63,7 @@ import static android.view.View.VISIBLE; import static de.blinkt.openvpn.core.ConnectionStatus.LEVEL_NONETWORK; import static se.leap.bitmaskclient.Constants.BROADCAST_EIP_EVENT; import static se.leap.bitmaskclient.Constants.BROADCAST_PROVIDER_API_EVENT; +import static se.leap.bitmaskclient.Constants.BROADCAST_RESULT_CODE; import static se.leap.bitmaskclient.Constants.BROADCAST_RESULT_KEY; import static se.leap.bitmaskclient.Constants.EIP_ACTION_CHECK_CERT_VALIDITY; import static se.leap.bitmaskclient.Constants.EIP_ACTION_START; @@ -266,7 +267,7 @@ public class EipFragment extends Fragment implements Observer { } private boolean canStartEIP() { - boolean certificateExists = !provider.hasVpnCertificate(); + boolean certificateExists = provider.hasVpnCertificate(); boolean isAllowedAnon = provider.allowsAnonymous(); return (isAllowedAnon || certificateExists) && !eipStatus.isConnected() && !eipStatus.isConnecting(); } @@ -463,7 +464,7 @@ public class EipFragment extends Fragment implements Observer { return; } - int resultCode = intent.getIntExtra(BROADCAST_RESULT_KEY, -1); + int resultCode = intent.getIntExtra(BROADCAST_RESULT_CODE, -1); Bundle resultData = intent.getParcelableExtra(BROADCAST_RESULT_KEY); switch (action) { case BROADCAST_EIP_EVENT: diff --git a/app/src/main/java/se/leap/bitmaskclient/Provider.java b/app/src/main/java/se/leap/bitmaskclient/Provider.java index 4d608222..e53dd4fb 100644 --- a/app/src/main/java/se/leap/bitmaskclient/Provider.java +++ b/app/src/main/java/se/leap/bitmaskclient/Provider.java @@ -141,9 +141,27 @@ public final class Provider implements Parcelable { } } - public void define(JSONObject providerJson) { - definition = providerJson; - parseDefinition(definition); + public boolean define(JSONObject providerJson) { + /* + * fix against "api_uri": "https://calyx.net.malicious.url.net:4430", + * This method aims to prevent attacks where the provider.json file got manipulated by a third party. + * The main url should not change. + */ + + try { + String providerApiUrl = providerJson.getString(Provider.API_URL); + String providerDomain = providerJson.getString(Provider.DOMAIN); + if (getMainUrlString().contains(providerDomain) && providerApiUrl.contains(providerDomain + ":")) { + definition = providerJson; + parseDefinition(definition); + return true; + } else { + return false; + } + } catch (JSONException e) { + e.printStackTrace(); + return false; + } } protected JSONObject getDefinition() { diff --git a/app/src/main/java/se/leap/bitmaskclient/ProviderApiManagerBase.java b/app/src/main/java/se/leap/bitmaskclient/ProviderApiManagerBase.java index a9321a9c..5fe6ed05 100644 --- a/app/src/main/java/se/leap/bitmaskclient/ProviderApiManagerBase.java +++ b/app/src/main/java/se/leap/bitmaskclient/ProviderApiManagerBase.java @@ -87,7 +87,6 @@ import static se.leap.bitmaskclient.ProviderAPI.SIGN_UP; import static se.leap.bitmaskclient.ProviderAPI.SUCCESSFUL_LOGIN; import static se.leap.bitmaskclient.ProviderAPI.SUCCESSFUL_LOGOUT; import static se.leap.bitmaskclient.ProviderAPI.SUCCESSFUL_SIGNUP; -import static se.leap.bitmaskclient.ProviderAPI.UPDATE_PROGRESSBAR; import static se.leap.bitmaskclient.ProviderAPI.UPDATE_PROVIDER_DETAILS; import static se.leap.bitmaskclient.R.string.certificate_error; import static se.leap.bitmaskclient.R.string.error_io_exception_user_message; @@ -665,6 +664,8 @@ public abstract class ProviderApiManagerBase { provider.setCaCert(getPersistedProviderCA(providerDomain)); provider.define(getPersistedProviderDefinition(providerDomain)); provider.setCaCertFingerprint(getPersistedCaCertFingerprint(providerDomain)); + provider.setPrivateKey(getPersistedPrivateKey(providerDomain)); + provider.setVpnCertificate(getPersistedVPNCertificate(providerDomain)); } } @@ -707,10 +708,6 @@ public abstract class ProviderApiManagerBase { return setErrorResult(result, warning_corrupted_provider_cert, ERROR_CERTIFICATE_PINNING.toString()); } - if (!hasApiUrlExpectedDomain(providerDefinition, mainUrl)){ - return setErrorResult(result, warning_corrupted_provider_details, ERROR_CORRUPTED_PROVIDER_JSON.toString()); - } - if (!canConnect(caCert, providerDefinition, result)) { return result; } @@ -797,6 +794,24 @@ public abstract class ProviderApiManagerBase { return ""; } + protected String getPersistedPrivateKey(String providerDomain) { + try { + return getPersistedProviderDefinition(providerDomain).getString(PROVIDER_PRIVATE_KEY); + } catch (JSONException e) { + e.printStackTrace(); + } + return ""; + } + + protected String getPersistedVPNCertificate(String providerDomain) { + try { + return getPersistedProviderDefinition(providerDomain).getString(PROVIDER_VPN_CERTIFICATE); + } catch (JSONException e) { + e.printStackTrace(); + } + return ""; + } + protected JSONObject getPersistedProviderDefinition(String providerDomain) { try { return new JSONObject(preferences.getString(Provider.KEY + "." + providerDomain, "")); @@ -806,6 +821,10 @@ public abstract class ProviderApiManagerBase { } } + protected String getFromPersistedProvider(String toFetch, String providerDomain) { + return preferences.getString(toFetch + "." + providerDomain, ""); + } + protected String getPersistedProviderCA(String providerDomain) { return preferences.getString(Provider.CA_CERT + "." + providerDomain, ""); } diff --git a/app/src/main/java/se/leap/bitmaskclient/ProviderCredentialsBaseActivity.java b/app/src/main/java/se/leap/bitmaskclient/ProviderCredentialsBaseActivity.java index 47a45a74..7714e979 100644 --- a/app/src/main/java/se/leap/bitmaskclient/ProviderCredentialsBaseActivity.java +++ b/app/src/main/java/se/leap/bitmaskclient/ProviderCredentialsBaseActivity.java @@ -40,6 +40,7 @@ import static se.leap.bitmaskclient.Constants.BROADCAST_RESULT_CODE; import static se.leap.bitmaskclient.Constants.BROADCAST_RESULT_KEY; import static se.leap.bitmaskclient.Constants.CREDENTIALS_PASSWORD; import static se.leap.bitmaskclient.Constants.CREDENTIALS_USERNAME; +import static se.leap.bitmaskclient.Constants.PROVIDER_KEY; import static se.leap.bitmaskclient.ProviderAPI.DOWNLOAD_CERTIFICATE; import static se.leap.bitmaskclient.ProviderAPI.LOG_IN; import static se.leap.bitmaskclient.ProviderAPI.SIGN_UP; @@ -201,7 +202,8 @@ public abstract class ProviderCredentialsBaseActivity extends ConfigWizardBaseAc ProviderAPICommand.execute(this, SIGN_UP, parameters, provider); } - void downloadVpnCertificate() { + void downloadVpnCertificate(Provider handledProvider) { + provider = handledProvider; ProviderAPICommand.execute(this, DOWNLOAD_CERTIFICATE, provider); } @@ -361,7 +363,8 @@ public abstract class ProviderCredentialsBaseActivity extends ConfigWizardBaseAc hideProgressBar(); } - private void successfullyFinished() { + private void successfullyFinished(Provider handledProvider) { + provider = handledProvider; Intent resultData = new Intent(); resultData.putExtra(Provider.KEY, provider); setResult(RESULT_OK, resultData); @@ -379,10 +382,13 @@ public abstract class ProviderCredentialsBaseActivity extends ConfigWizardBaseAc } int resultCode = intent.getIntExtra(BROADCAST_RESULT_CODE, -1); + Bundle resultData = intent.getParcelableExtra(BROADCAST_RESULT_KEY); + Provider handledProvider = resultData.getParcelable(PROVIDER_KEY); + switch (resultCode) { case ProviderAPI.SUCCESSFUL_SIGNUP: case ProviderAPI.SUCCESSFUL_LOGIN: - downloadVpnCertificate(); + downloadVpnCertificate(handledProvider); break; case ProviderAPI.FAILED_LOGIN: case ProviderAPI.FAILED_SIGNUP: @@ -390,7 +396,7 @@ public abstract class ProviderCredentialsBaseActivity extends ConfigWizardBaseAc break; case ProviderAPI.CORRECTLY_DOWNLOADED_CERTIFICATE: - successfullyFinished(); + successfullyFinished(handledProvider); //activity.eip_fragment.updateEipService(); break; case ProviderAPI.INCORRECTLY_DOWNLOADED_CERTIFICATE: diff --git a/app/src/production/java/se/leap/bitmaskclient/ProviderApiManager.java b/app/src/production/java/se/leap/bitmaskclient/ProviderApiManager.java index 839c5a5d..5317118b 100644 --- a/app/src/production/java/se/leap/bitmaskclient/ProviderApiManager.java +++ b/app/src/production/java/se/leap/bitmaskclient/ProviderApiManager.java @@ -36,9 +36,11 @@ import static android.text.TextUtils.isEmpty; import static se.leap.bitmaskclient.Constants.BROADCAST_RESULT_KEY; import static se.leap.bitmaskclient.Constants.PROVIDER_VPN_CERTIFICATE; import static se.leap.bitmaskclient.DownloadFailedDialog.DOWNLOAD_ERRORS.ERROR_CERTIFICATE_PINNING; +import static se.leap.bitmaskclient.DownloadFailedDialog.DOWNLOAD_ERRORS.ERROR_CORRUPTED_PROVIDER_JSON; import static se.leap.bitmaskclient.ProviderAPI.ERRORS; import static se.leap.bitmaskclient.R.string.malformed_url; import static se.leap.bitmaskclient.R.string.warning_corrupted_provider_cert; +import static se.leap.bitmaskclient.R.string.warning_corrupted_provider_details; /** * Implements the logic of the provider api http requests. The methods of this class need to be called from @@ -124,9 +126,12 @@ public class ProviderApiManager extends ProviderApiManagerBase { try { JSONObject providerJson = new JSONObject(providerDotJsonString); - provider.define(providerJson); + if (provider.define(providerJson)) { + result.putBoolean(BROADCAST_RESULT_KEY, true); + } else { + return setErrorResult(result, warning_corrupted_provider_details, ERROR_CORRUPTED_PROVIDER_JSON.toString()); + } - result.putBoolean(BROADCAST_RESULT_KEY, true); } catch (JSONException e) { String reason_to_fail = pickErrorMessage(providerDotJsonString); result.putString(ERRORS, reason_to_fail); -- cgit v1.2.3