Crypho / cordova-plugin-secure-storage

Secure storage plugin for Apache Cordova
MIT License
278 stars 269 forks source link

Can multiple Android apps from a Company access the same shared data? #151

Closed ajayambre closed 6 years ago

ajayambre commented 6 years ago

I apologize for posting a question here, but I could not find a detailed documentation explaining how multiple apps from the same Company can use this plugin to share data between them.

We were able to make this work on iOS by using the same string value in the KeyChain preferences settings in XCode and when creating the SecureStorage in our code. But could not find something similar for our Android app.

Use Case: We have multiple Enterprise apps built using Ionic 3 and all of them connect to the same server. The device needs to be authorized using a code before the users are allowed to login. The code is generated by an administrator on the server and shared with the user via an email.

We are trying to avoid a situation where the Admin needs to generate a separate code for each app that the user installs on his device.

When the app starts it looks for a key in the SecureStorage and if present uses that as the device ID. If the key is not present in the SecureStorage, we get the uuid using the Device plugin and store it using the SecureStorage plugin.

We were able to make this work on iOS as the apps were able to access the shared data from the KeyChain.

On Android, the apps are not getting the key from the SecureStorage and so each app tries to generate using the Device plugin. Starting Android 8, the Device plugin returns a different value for each app even if it is published by the same Company.

Apologies again for posting a question here. Any help/guidance on this would be greatly appreciated Thanks!

demetris-manikas commented 6 years ago

Hi @ajayambre Not sure if this is going to work but check

https://developer.android.com/reference/android/content/Context.html#MODE_PRIVATE and https://stackoverflow.com/questions/6354035/two-android-applications-with-the-same-user-id

Beware that the SharedPreferences instance is generated using getContext().getPackageName() + "." + service.

Please report back if you manage to make it work.

ajayambre commented 6 years ago

Thank you @demetris-manikas

I added the same value for android:sharedUserId in AndroidManifest.xml for both my apps. But it was not enough.

I was able to make this work but had to do the below code changes.

Change 1: As you rightly said, the SharedPreferences instance had to be generated in a different way as every app will have a unique package name. I changed this and stripped the last part of the package name. So now apps from com.mycompany.app1 and com.mycompany.app2 will be able to access each others SharedPreferences but app with com.othercompany.app will not be able to access it.

private String service2alias(String service) {     String packageName = getContext().getPackageName();     packageName = packageName.substring(0, packageName.lastIndexOf("."));     String res = packageName + "." + service;     return res; }

Change 2: As mentioned in this Stackoverflow link, an app can read other apps SharedPreferences by using createPackageContext. So I added another parameter packageName to the init action and used the createPackageContext method to create the Context and passed that to the SharedPreferencesHandler constructor.

context.createPackageContext(packageName, Context.CONTEXT_IGNORE_SECURITY | Context.CONTEXT_INCLUDE_CODE);

Result: I used the same packageName parameter com.example.app1 for creating the SecureStorage from both my apps. App1 inserted a value in the SecureStorage. App2 was able to read that value from the SecureStorage.

I tested by changing the android:sharedUserId to a different value in my App2 and then it could not read the value stored by App1.

The additional parameter is required specifically for Android and can be ignored for iOS and Windows. Please let me know what you think of this solution.

Thank you for this plugin. I would love to contibute to it! :)

demetris-manikas commented 6 years ago

I would love to see a PR on this. Lets see some code!

ggozad commented 6 years ago

This looks to me as a bad idea. How do you handle the shared secret in this case?

ggozad commented 6 years ago

So in brief. As I mentioned before context.createPackageContext(packageName, Context.CONTEXT_IGNORE_SECURITY | Context.CONTEXT_INCLUDE_CODE); is a bad idea as it essentially allows other contexts to read from shared preferences. In iOS this is possible due to the keychain which we do not have for android. I am gonna close this, if you have more input feel free to reopen.

ajayambre commented 6 years ago

Hi @ggozad

I am not sure if I understand your concern correctly.

Is it because of the flags Context.CONTEXT_IGNORE_SECURITY | Context.CONTEXT_INCLUDE_CODE OR with the idea of using the createPackageContext altogether.

If it is just because of the flags, then I tried doing context.createPackageContext(packageName, 0) and that still works if both the applications use the same android:sharedUserId.

ggozad commented 6 years ago

Hey @ajayambre, The flags are indeed the problem. They would potentially allow other apps to also read the preferences and compromise the key. When passing 0 instead of Context.CONTEXT_IGNORE_SECURITY | Context.CONTEXT_INCLUDE_CODE you are essentially passing all the flags together, it's a bit mask, see https://www.learncpp.com/cpp-tutorial/3-8a-bit-flags-and-bit-masks/ for example.

ajayambre commented 6 years ago

Thanks for the detailed explanation @ggozad Sorry for the delayed response.

Request you to please have a look at the below code.

Consider App1 with com.example.app1 as package name. It uses com.example.app1 as packageName param when creating the SecureStorage. The SharedPreferencesHandler would be created with the same default Context as it is being done with the current implementation as context.getPackageName().equals(packageName) is true.

Consider another App2 with com.example.app2 as package name. It uses com.example.app1 as packageName param when creating the SecureStorage. It is able to gain access and read the data from the SecureStorage created by App1.

Does this mean we have a potential security problem with the current implementation?

SharedPreferencesHandler PREFS = new SharedPreferencesHandler(alias + "_SS",        getPackageContext(getContext(), packageName));

private Context getPackageContext(Context context, String packageName) {     Context pkgContext = null;     if (context.getPackageName().equals(packageName)) {         pkgContext = context;     } else {         try {         pkgContext = context.createPackageContext(packageName, 0);         } catch (Exception e) {         e.printStackTrace();         pkgContext = context;         }      }      return pkgContext; }

ggozad commented 6 years ago

Hey, I had a closer look at this. As long as you pass 0 to createPackageContext you should be fine, you are merely passing a different package name to create your package context. So it should be ok to share the storage across apps this way.

ajayambre commented 6 years ago

Thank you @ggozad Can I submit a PR for this?

ggozad commented 6 years ago

@ajayambre Feel free to do so. It should not be a default behavior, perhaps a config option is appropriate.

ggozad commented 6 years ago

@ajayambre gonna close this as there is no issue to resolve. Waiting for your PR.

ajayambre commented 6 years ago

@ggozad As suggested, have added an optional packageName option for Android. There are no breaking changes for existing apps and I expect that they will be able to fetch values for previously saved keys.