always provide private VPN key over management interface, avoid exposing it in persisted openvpn config. The private key is stored encrypted instead

8 files changed, 58 insertions, 48 deletions

diff --git a/app/src/main/java/de/blinkt/openvpn/VpnProfile.java b/app/src/main/java/de/blinkt/openvpn/VpnProfile.java
index 9f722dfe..780ac9d8 100644
--- a/app/src/main/java/de/blinkt/openvpn/VpnProfile.java
+++ b/app/src/main/java/de/blinkt/openvpn/VpnProfile.java
@@ -76,6 +76,7 @@ import de.blinkt.openvpn.core.connection.Connection;
 import de.blinkt.openvpn.core.connection.ConnectionAdapter;
 import se.leap.bitmaskclient.BuildConfig;
 import se.leap.bitmaskclient.R;
+import se.leap.bitmaskclient.base.models.ProviderObservable;
 
 public class VpnProfile implements Serializable, Cloneable {
     // Note that this class cannot be moved to core where it belongs since
@@ -442,8 +443,9 @@ public class VpnProfile implements Serializable, Cloneable {
                 cfg.append(insertFileData("ca", mCaFilename));
 
                 // Client Cert + Key
-                cfg.append(insertFileData("key", mClientKeyFilename));
                 cfg.append(insertFileData("cert", mClientCertFilename));
+                mPrivateKey = ProviderObservable.getInstance().getCurrentProvider().getRSAPrivateKey();
+                cfg.append("management-external-key nopadding pkcs1 pss digest\n");
 
                 break;
             case VpnProfile.TYPE_USERPASS_PKCS12:
@@ -761,7 +763,7 @@ public class VpnProfile implements Serializable, Cloneable {
     public Intent prepareStartService(Context context) {
         Intent intent = getStartServiceIntent(context);
 
-        // TODO: Handle this?!
+        // This can remain outcommented for now, Bitmask uses VpnProfile.TYPE_CERTIFICATE
 //        if (mAuthenticationType == VpnProfile.TYPE_KEYSTORE || mAuthenticationType == VpnProfile.TYPE_USERPASS_KEYSTORE) {
 //            if (getKeyStoreCertificates(context) == null)
 //                return null;
@@ -843,6 +845,14 @@ public class VpnProfile implements Serializable, Cloneable {
         return ExtAuthHelper.getCertificateChain(context, mExternalAuthenticator, mAlias);
     }
 
+    /**
+     * returns an array certificates, depending on the profile type either from the keychain or an external cert provider
+     * @param context
+     * @return pem encoded certificates, where:
+     *  [0] is the ca cert
+     *  [1] is an optional extra cert
+     *  [2] is the vpn certificate
+     */
     public String[] getExternalCertificates(Context context) {
         return getExternalCertificates(context, 5);
     }
@@ -977,8 +987,9 @@ public class VpnProfile implements Serializable, Cloneable {
         if (mUseTLSAuth && TextUtils.isEmpty(mTLSAuthFilename))
             return R.string.missing_tlsauth;
 
-        if ((mAuthenticationType == TYPE_USERPASS_CERTIFICATES || mAuthenticationType == TYPE_CERTIFICATES)
-                && (TextUtils.isEmpty(mClientCertFilename) || TextUtils.isEmpty(mClientKeyFilename)))
+        if ((mAuthenticationType == TYPE_USERPASS_CERTIFICATES &&
+                (TextUtils.isEmpty(mClientCertFilename) || (TextUtils.isEmpty(mClientKeyFilename)))) ||
+                mAuthenticationType == TYPE_CERTIFICATES && TextUtils.isEmpty(mClientCertFilename))
             return R.string.missing_certificates;
 
         if ((mAuthenticationType == TYPE_CERTIFICATES || mAuthenticationType == TYPE_USERPASS_CERTIFICATES)
diff --git a/app/src/main/java/de/blinkt/openvpn/core/ConfigParser.java b/app/src/main/java/de/blinkt/openvpn/core/ConfigParser.java
index e8d333e3..ff27a5a2 100644
--- a/app/src/main/java/de/blinkt/openvpn/core/ConfigParser.java
+++ b/app/src/main/java/de/blinkt/openvpn/core/ConfigParser.java
@@ -70,9 +70,7 @@ public class ConfigParser {
             "management",
             "management-client",
             "management-query-remote",
-            "management-query-passwords",
             "management-query-proxy",
-            "management-external-key",
             "management-forget-disconnect",
             "management-signal",
             "management-log-cache",
diff --git a/app/src/main/java/de/blinkt/openvpn/core/VPNLaunchHelper.java b/app/src/main/java/de/blinkt/openvpn/core/VPNLaunchHelper.java
index 80427a03..67636762 100644
--- a/app/src/main/java/de/blinkt/openvpn/core/VPNLaunchHelper.java
+++ b/app/src/main/java/de/blinkt/openvpn/core/VPNLaunchHelper.java
@@ -5,7 +5,6 @@
 
 package de.blinkt.openvpn.core;
 
-import android.annotation.TargetApi;
 import android.content.Context;
 import android.os.Build;
 
diff --git a/app/src/main/java/se/leap/bitmaskclient/base/models/Provider.java b/app/src/main/java/se/leap/bitmaskclient/base/models/Provider.java
index 08e13cf6..14c78cc3 100644
--- a/app/src/main/java/se/leap/bitmaskclient/base/models/Provider.java
+++ b/app/src/main/java/se/leap/bitmaskclient/base/models/Provider.java
@@ -29,6 +29,7 @@ import static se.leap.bitmaskclient.base.models.Constants.PROVIDER_ALLOW_ANONYMO
 import static se.leap.bitmaskclient.base.models.Constants.TRANSPORT;
 import static se.leap.bitmaskclient.base.models.Constants.TYPE;
 import static se.leap.bitmaskclient.base.utils.ConfigHelper.ObfsVpnHelper.useObfsVpn;
+import static se.leap.bitmaskclient.base.utils.ConfigHelper.RSAHelper.parseRsaKeyFromString;
 import static se.leap.bitmaskclient.providersetup.ProviderAPI.ERRORS;
 
 import android.os.Parcel;
@@ -44,6 +45,7 @@ import org.json.JSONObject;
 
 import java.net.MalformedURLException;
 import java.net.URL;
+import java.security.interfaces.RSAPrivateKey;
 import java.util.ArrayList;
 import java.util.HashSet;
 import java.util.Locale;
@@ -79,6 +81,8 @@ public final class Provider implements Parcelable {
     private String caCert = "";
     private String apiVersion = "";
     private String privateKey = "";
+
+    private transient RSAPrivateKey rsaPrivateKey = null;
     private String vpnCertificate = "";
     private long lastEipServiceUpdate = 0L;
     private long lastGeoIpUpdate = 0L;
@@ -701,6 +705,13 @@ public final class Provider implements Parcelable {
         return privateKey;
     }
 
+    public RSAPrivateKey getRSAPrivateKey() {
+        if (rsaPrivateKey == null) {
+            rsaPrivateKey = parseRsaKeyFromString(privateKey);
+        }
+        return rsaPrivateKey;
+    }
+
     public void setPrivateKey(String privateKey) {
         this.privateKey = privateKey;
     }
diff --git a/app/src/main/java/se/leap/bitmaskclient/base/utils/ConfigHelper.java b/app/src/main/java/se/leap/bitmaskclient/base/utils/ConfigHelper.java
index 2412efdd..9289738a 100644
--- a/app/src/main/java/se/leap/bitmaskclient/base/utils/ConfigHelper.java
+++ b/app/src/main/java/se/leap/bitmaskclient/base/utils/ConfigHelper.java
@@ -131,35 +131,37 @@ public class ConfigHelper {
         return null;
     }
 
-    public static RSAPrivateKey parseRsaKeyFromString(String rsaKeyString) {
-        RSAPrivateKey key;
-        try {
-            KeyFactory kf;
-            if (Build.VERSION.SDK_INT < Build.VERSION_CODES.P) {
-                kf = KeyFactory.getInstance("RSA", "BC");
-            } else {
-                kf = KeyFactory.getInstance("RSA");
+    public static class RSAHelper {
+        public static RSAPrivateKey parseRsaKeyFromString(String rsaKeyString) {
+            RSAPrivateKey key;
+            try {
+                KeyFactory kf;
+                if (Build.VERSION.SDK_INT < Build.VERSION_CODES.P) {
+                    kf = KeyFactory.getInstance("RSA", "BC");
+                } else {
+                    kf = KeyFactory.getInstance("RSA");
+                }
+                rsaKeyString = rsaKeyString.replaceFirst("-----BEGIN RSA PRIVATE KEY-----", "").replaceFirst("-----END RSA PRIVATE KEY-----", "");
+                PKCS8EncodedKeySpec keySpec = new PKCS8EncodedKeySpec(Base64.decode(rsaKeyString));
+                key = (RSAPrivateKey) kf.generatePrivate(keySpec);
+            } catch (InvalidKeySpecException e) {
+                // TODO Auto-generated catch block
+                e.printStackTrace();
+                return null;
+            } catch (NoSuchAlgorithmException e) {
+                // TODO Auto-generated catch block
+                e.printStackTrace();
+                return null;
+            } catch (NullPointerException e) {
+                e.printStackTrace();
+                return null;
+            } catch (NoSuchProviderException e) {
+                e.printStackTrace();
+                return null;
             }
-            rsaKeyString = rsaKeyString.replaceFirst("-----BEGIN RSA PRIVATE KEY-----", "").replaceFirst("-----END RSA PRIVATE KEY-----", "");
-            PKCS8EncodedKeySpec keySpec = new PKCS8EncodedKeySpec(Base64.decode(rsaKeyString));
-            key = (RSAPrivateKey) kf.generatePrivate(keySpec);
-        } catch (InvalidKeySpecException e) {
-            // TODO Auto-generated catch block
-            e.printStackTrace();
-            return null;
-        } catch (NoSuchAlgorithmException e) {
-            // TODO Auto-generated catch block
-            e.printStackTrace();
-            return null;
-        } catch (NullPointerException e) {
-            e.printStackTrace();
-            return null;
-        } catch (NoSuchProviderException e) {
-            e.printStackTrace();
-            return null;
-        }
 
-        return key;
+            return key;
+        }
     }
 
     private static String byteArrayToHex(byte[] input) {
diff --git a/app/src/main/java/se/leap/bitmaskclient/eip/GatewaysManager.java b/app/src/main/java/se/leap/bitmaskclient/eip/GatewaysManager.java
index d114665b..5e05b7c1 100644
--- a/app/src/main/java/se/leap/bitmaskclient/eip/GatewaysManager.java
+++ b/app/src/main/java/se/leap/bitmaskclient/eip/GatewaysManager.java
@@ -22,7 +22,6 @@ import static de.blinkt.openvpn.core.connection.Connection.TransportType.OPENVPN
 import static de.blinkt.openvpn.core.connection.Connection.TransportType.PT;
 import static se.leap.bitmaskclient.base.models.Constants.GATEWAYS;
 import static se.leap.bitmaskclient.base.models.Constants.HOST;
-import static se.leap.bitmaskclient.base.models.Constants.PROVIDER_PRIVATE_KEY;
 import static se.leap.bitmaskclient.base.models.Constants.PROVIDER_VPN_CERTIFICATE;
 import static se.leap.bitmaskclient.base.models.Constants.SORTED_GATEWAYS;
 import static se.leap.bitmaskclient.base.utils.PreferenceHelper.getObfuscationPinningCert;
@@ -124,6 +123,7 @@ public class GatewaysManager {
 
     GatewaySelector gatewaySelector;
 
+
     public GatewaysManager(Context context) {
         this.context = context;
         configureFromCurrentProvider();
@@ -392,7 +392,6 @@ public class GatewaysManager {
          try {
              JSONObject eipDefinition = provider.getEipServiceJson();
              JSONObject secrets = secretsConfigurationFromCurrentProvider();
-
              JSONArray gatewaysDefined = new JSONArray();
              try {
                  gatewaysDefined = eipDefinition.getJSONArray(GATEWAYS);
@@ -488,10 +487,8 @@ public class GatewaysManager {
     private JSONObject secretsConfigurationFromCurrentProvider() {
         JSONObject result = new JSONObject();
         Provider provider = ProviderObservable.getInstance().getCurrentProvider();
-
         try {
             result.put(Provider.CA_CERT, provider.getCaCert());
-            result.put(PROVIDER_PRIVATE_KEY, provider.getPrivateKey());
             result.put(PROVIDER_VPN_CERTIFICATE, provider.getVpnCertificate());
         } catch (JSONException e) {
             e.printStackTrace();
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 2c22d4f7..fa2ab352 100644
--- a/app/src/main/java/se/leap/bitmaskclient/eip/VpnConfigGenerator.java
+++ b/app/src/main/java/se/leap/bitmaskclient/eip/VpnConfigGenerator.java
@@ -490,13 +490,6 @@ public class VpnConfigGenerator {
                             + newLine
                             + "</ca>";
 
-            String key =
-                    "<key>"
-                            + newLine
-                            + secrets.getString(PROVIDER_PRIVATE_KEY)
-                            + newLine
-                            + "</key>";
-
             String openvpnCert =
                     "<cert>"
                             + newLine
@@ -504,7 +497,7 @@ public class VpnConfigGenerator {
                             + newLine
                             + "</cert>";
 
-            return ca + newLine + key + newLine + openvpnCert;
+            return ca + newLine + openvpnCert;
         } catch (JSONException e) {
             e.printStackTrace();
             return "";
diff --git a/app/src/main/java/se/leap/bitmaskclient/providersetup/ProviderApiManagerBase.java b/app/src/main/java/se/leap/bitmaskclient/providersetup/ProviderApiManagerBase.java
index 14308875..fdaef28b 100644
--- a/app/src/main/java/se/leap/bitmaskclient/providersetup/ProviderApiManagerBase.java
+++ b/app/src/main/java/se/leap/bitmaskclient/providersetup/ProviderApiManagerBase.java
@@ -43,13 +43,12 @@ import static se.leap.bitmaskclient.base.models.Constants.PROVIDER_PRIVATE_KEY;
 import static se.leap.bitmaskclient.base.models.Constants.PROVIDER_VPN_CERTIFICATE;
 import static se.leap.bitmaskclient.base.models.Provider.CA_CERT;
 import static se.leap.bitmaskclient.base.models.Provider.GEOIP_URL;
-import static se.leap.bitmaskclient.base.models.Provider.MOTD_URL;
 import static se.leap.bitmaskclient.base.models.Provider.PROVIDER_API_IP;
 import static se.leap.bitmaskclient.base.models.Provider.PROVIDER_IP;
+import static se.leap.bitmaskclient.base.utils.ConfigHelper.RSAHelper.parseRsaKeyFromString;
 import static se.leap.bitmaskclient.base.utils.ConfigHelper.getDomainFromMainURL;
 import static se.leap.bitmaskclient.base.utils.ConfigHelper.getFingerprintFromCertificate;
 import static se.leap.bitmaskclient.base.utils.ConfigHelper.getProviderFormattedString;
-import static se.leap.bitmaskclient.base.utils.ConfigHelper.parseRsaKeyFromString;
 import static se.leap.bitmaskclient.base.utils.PreferenceHelper.deleteProviderDetailsFromPreferences;
 import static se.leap.bitmaskclient.base.utils.PreferenceHelper.getFromPersistedProvider;
 import static se.leap.bitmaskclient.base.utils.PreferenceHelper.getLongFromPersistedProvider;