Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

(back) Filter updates by bundleId #559

Closed
4 tasks
gmaclennan opened this issue Apr 21, 2021 · 4 comments
Closed
4 tasks

(back) Filter updates by bundleId #559

gmaclennan opened this issue Apr 21, 2021 · 4 comments
Assignees

Comments

@gmaclennan
Copy link
Member

gmaclennan commented Apr 21, 2021

We release different "variants" of Mapeo, that are distinguished by different "bundle id"s or "application id"s. Each variant with a different bundle id installs as a separate app on the phone, so you can have two variants of Mapeo running alongside each other with their own data, e.g. the QA variant and the main variant.

If we do not check that the bundle id of the update matches the running application, then we could end up upgrading / installing a different app to the one that is currently running. E.g. if someone is using the QA variant, and they get an update from someone running the main variant, then the QA variant would install the update over the main variant on their phone, which is not what the user would be expecting (the QA variant that they are running would not update).

To avoid this, we need to only accept updates that share the same bundle id. I think that it makes sense to avoid discovery of different variants altogether, e.g. QA variants should only discover other QA variants for finding updates. Could we incorporate the bundleId into the discovery key?

Current variants we build:

  • com.mapeo: the "main" variant of the app - blue icon
  • com.mapeo.qa: used for QA testing without overwriting the main variant - red icon
  • com.mapeo.debug: used for development when running locally - yellow icon
  • com.mapeo.icca: The "Mapeo for ICCAs" variant created for WCMC - green icon

There are also .debug variants of the QA and ICCA builds. These are only built as internal builds used for debugging errors with release candidates.


Kira's work checklist:

@hackergrrl
Copy link
Contributor

This makes sense -- I think it's a good solution. The alternative would be to store the bundleId in the upgrade metadata, but I don't see a benefit to doing it that way vs the discovery key. Right now we use the key mapeo-upgrade. We could instead use mapeo-upgrade-${bundleId}.

The main work is to pass the bundleId into the UpgradeManager, where it'd be passed down into the UpgradeDownloader and UpgradeServer so they can incorporate it into the discovery key they listen/search on.

@hackergrrl hackergrrl removed their assignment Apr 21, 2021
@gmaclennan
Copy link
Member Author

This makes sense -- I think it's a good solution. The alternative would be to store the bundleId in the upgrade metadata, but I don't see a benefit to doing it that way vs the discovery key. Right now we use the key mapeo-upgrade. We could instead use mapeo-upgrade-${bundleId}.

Yes that makes sense. Or ${bundleId}-upgrade e.g. com.mapeo-upgrade.

The main work is to pass the bundleId into the UpgradeManager, where it'd be passed down into the UpgradeDownloader and UpgradeServer so they can incorporate it into the discovery key they listen/search on.

The PR I made against your branch from share-apk-p2p-gregor makes bundleId available in scope for when you create the UpgradeManager instance.

@hackergrrl hackergrrl self-assigned this Apr 22, 2021
@gmaclennan
Copy link
Member Author

I think that this is important enough that in addition to limiting it by discover key, we should also check the bundleId of incoming APKs. Otherwise it carries a small security risk:

  1. Hacker creates a malicious fork of Mapeo with a different bundleId
  2. Hacker gets Mapeo to share that update via the com.mapeo bundleId
  3. Malicious Mapeo will be installed alongside regular Mapeo (same icon and name).
  4. User accidentally opens malicious Mapeo

I think we should read the following from incoming APKs before installing:

  • Bundle ID
  • Version Name
  • Build Number
  • minSdkVersion

Andre Staltz wrote a small native module to read similar information from an APK: https://github.com/staltz/react-native-android-packagemanager/blob/master/android/src/main/java/com/reactlibrary/RNAndroidPackagemanagerModule.java I think we should vendorize that code for now (e.g. copy it into Mapeo codebase) and modify it to also read these values. Bundle ID is the most important in my opinion, since if the other metadata is incorrect install will just fail, but bundleId being different will cause unexpected things to happen.

I can look at the java code for this tomorrow

@hackergrrl
Copy link
Contributor

I think that this is important enough that in addition to limiting it by discover key, we should also check the bundleId of incoming APKs. Otherwise it carries a small security risk: [...]

Agreed.

I think we should read the following from incoming APKs before installing: [...] Version Name, Build Number, minSdkVersion

What would you want to do with these values?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants