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

Certificate issue #244

Closed
IzzySoft opened this issue Feb 8, 2024 · 42 comments
Closed

Certificate issue #244

IzzySoft opened this issue Feb 8, 2024 · 42 comments

Comments

@IzzySoft
Copy link

IzzySoft commented Feb 8, 2024

A scan (see here for details and background) just revealed the APKs at your releases are signed using a debug key. As that has security implications, may I ask you to please switch to a proper release key, and provide the corresponding APK signed with it? Thanks in advance!

@sgrimault
Copy link
Collaborator

For reasons of compatibility between the very first versions of the application that were distributed within the national parks, and between the RC versions and the final versions, we use the same keystore.
This avoids users having to uninstall everything locally if the keystore ever changes.
The application is also able to check for any updates and suggest them to the user. If we change the signature, we break this functionality.

@IzzySoft
Copy link
Author

IzzySoft commented Feb 8, 2024

Oof. No offense meant, but that somehow reminds me of cars that come with a long pole attached to their fronts to keep them compatible with horse carts 🙊 But yes, I'm aware that switching signing keys (as well as changing package names) is nothing one should do lightly. But maybe we can find a workable solution to the problem.

The application is also able to check for any updates and suggest them to the user.

Now, wouldn't that solve the issue? The update could be properly signed, and the suggestion include a hint to uninstall and reinstall (with proper data export/import where needed). The new versions can still share the same keystore, just the new one with release keys.

If that for some reason beyond my understanding would not work (or not with a "hard cut"): what is affected is only the signing, right? So you could maybe have a separate APK signed with the debug key (as you have now), and the one signed with the release key named *release.apk – both only differing in the signing? Then have some "download counter" to see if the debug version falls below some threshold and can be dropped one day, maybe.

I cannot outline what I don't fully understand, not being an Android dev, but there's also a thing called "key rotation" (supported from Android 9 upwards) in signing scheme v3.

Maybe one of those options can be applied? Please note that once the above linked issue has been completed in processing, apps using debug keys will no longer be allowed in my repo. So the ones remaining after the deadline (in March) will have to be removed. With apps which are still actively maintained (like yours) that would be really sad – so I hope we can find a way to keep Occtax in.

@sgrimault
Copy link
Collaborator

Yes, sorry about that,
I'll have to study the latest possibilities a little, but it was already quite complicated to maintain a certain compatibility between the different versions of the application, in particular the fact that updates must be applied without problem.
When a park has a fleet of terminals for its agents, it would be a real shame if the fleet had to be repatriated if the last update went wrong..
Example of a possible problem:
If the signature key is updated to make the new version incompatible with the current one, implying a complete uninstall of the application, all observation records that have not been synchronized and all observation records in progress are lost. This is unacceptable, especially for users.
We'll have to see if it's possible to switch to another key without breaking compatibility.
I'll see what I can do with apk-key-rotation.

@IzzySoft
Copy link
Author

IzzySoft commented Feb 9, 2024

Thanks, @sgrimault! Yes, I see the problem (and thanks to your details, now also understand it a lot better). As you're looking into it, I will definitely mark an exception for Occtax should the "deadline" arrive and you're not yet ready. I'm confident we'll find a solution there.

I hope no park runs pre-Android-9 devices, so key rotation is an option. Though I've never seen a key rotation in practice (and thus have neither experience with it nor can provide help/details for it), it sounds like that would be the most transparent approach.

@IzzySoft
Copy link
Author

IzzySoft commented Mar 2, 2024

Sorry to complain again, but the scanner in my repo just reported for today's update:

! repo/fr.geonature.occtax2_3220.apk declares sensitive permission(s):
  android.permission.REQUEST_INSTALL_PACKAGES android.permission.ACCESS_COARSE_LOCATION
  android.permission.ACCESS_FINE_LOCATION android.permission.READ_EXTERNAL_STORAGE
  android.permission.MANAGE_EXTERNAL_STORAGE
! repo/fr.geonature.occtax2_3220.apk contains signature block blobs: 0x504b4453 (DEPENDENCY_INFO_BLOCK; GOOGLE)

Location permissions are clear by the app's purpose, so I just added them to your app's "allow list":

    android.permission.ACCESS_COARSE_LOCATION: needed to record the location of observance
    android.permission.ACCESS_FINE_LOCATION: needed to record the location of observance

But could you please clarify storage and especially REQUEST_INSTALL_PACKAGES? Thanks in advance!

Btw, that DEPENDENCY_INFO_BLOCK is easy to get rid of:

android {
    dependenciesInfo {
        // Disables dependency metadata when building APKs.
        includeInApk = false
        // Disables dependency metadata when building Android App Bundles.
        includeInBundle = false
    }
}

For some background: that BLOB is supposed to be just a binary representation of your app's dependency tree. But as it's encrypted with a public key belonging to Google, only Google can read it – and nobody else can even verify what it really contains.

@sgrimault
Copy link
Collaborator

Hello @IzzySoft,
I'll be making these changes for the next version to come (2.7).
In particular, about the certificate used to sign the application and to remove the dependency info block.
Concerning the REQUEST_INSTALL_PACKAGES permission, it's necessary because the application is able to update itself by retrieving the latest APK information from a GeoNature instance.

@IzzySoft
Copy link
Author

IzzySoft commented Mar 3, 2024

I'll be making these changes for the next version to come (2.7).
In particular, about the certificate used to sign the application and to remove the dependency info block.

Thanks a lot!

Concerning the REQUEST_INSTALL_PACKAGES permission, it's necessary because the application is able to update itself by retrieving the latest APK information from a GeoNature instance.

Thanks! Can we go without the self-updater for the variant intended for my repo? Self-updaters are not in accordance with the inclusion criteria there (see IzzyOnDroid App Inclusion Criteria for details) as they bypass all checks performed in the repo (same with F-Droid.org by the way – though they rarely even notice the presence of a self-updater unless reported to them as they lack the proper scans).

The relevant part of the inclusion criteria (wording is almost identical to F-Droid's):

[the app] must not download additional executable binary files (e.g. addons, auto-updates, etc.) without explicit user consent. Consent means it needs to be opt-in (it must not be harder to decline than to accept or presented in a way users are likely to press accept without reading) and structured in a way that clearly explains to users that they’re choosing to bypass the checks performed in this repo if they activate it.

An example for "explicit user consent", if you prefer that path (I'm aware that your app is a bit special in this concern, including instance version and app version needing to match each other, so I'll give you that option right ahead and not keep it back as "last resort" as I'd usually do):

01_enable_update_check 02_update_action

First screen is before the update check itself is enabled. Who declines there won't see the second one ever as no checks means no updates found. Second screen is when the check was enabled and an update was found.

Screenshots are from the RiMusic app if you wonder. Your phrasing will most likely differ, concerning "instance specialties". But if you could at least ask the consent, it would be much appreciated.

@sgrimault
Copy link
Collaborator

In this case, we could consider the following approach, using Android flavors:
Currently, the "generic" flavor is used by default when the application is built. In particular, it relies on the shared certificate between RC and "official" versions.
We could therefore create another flavor (e.g. "fdroid"?) on which we would disable certain functionalities (such as the ability to update the application directly from a GeoNature instance), using its own certificate to sign the APK.
This approach implies that it will not be possible to make these two versions cohabit. A person relying on his GeoNature instance will not be able to install a version of "Occtax" from F-Droid. The same applies if his version comes from F-Droid.

@IzzySoft
Copy link
Author

IzzySoft commented Mar 5, 2024

A person relying on his GeoNature instance will not be able to install a version of "Occtax" from F-Droid.

This is the part where I miss some pieces. If there's a general "binding" between app version and instance, does this make sense? For whom would this build be useful?

it will not be possible to make these two versions cohabit

That would depend. If both share the very same applicationId it's certainly true. If not, both can co-exist on the device. Though again this would raise the question on whom it would be useful for. I have to leave that to your expertise – but maybe the approach of opt-in to self-updates would be more viable?

@sgrimault
Copy link
Collaborator

The idea I've come up with is to have two distinct build phases, one dedicated to deploying the application via GeoNature, the second via F-Droid.

Actually, the application relies heavily on the APIs exposed by GeoNature, and since GeoNature is also evolving on its side, it's possible that a slightly older version will be deployed. So we're going to make available a version of the application that's not necessarily up to date, but compatible with this instance of GeoNature. Hence a deployment controlled by GeoNature.
A version of the application published on F-Droid will certainly be more up to date, and therefore won't necessarily work with older instances of GeoNature.

What's currently missing is a mapping table between compatible versions of GeoNature and versions of the application. This will enable the application to notify the user if it is configured with an instance of GeoNature that is too old.

@IzzySoft
Copy link
Author

IzzySoft commented Mar 5, 2024

So the basic problem exists already now: if a newer version is available via my repo, the F-Droid client will offer the upgrade. The app itself cannot control that and intervene.

Hence a deployment controlled by GeoNature.

This means there's a message from the server to the app saying when and what version is available for update? My repo currently keeps the latest 3 versions. Would that match the needs? Or might there even older versions be required? If so, maybe the opt-in self-updater would make more sense as with that it's possible to choose which update strategy should be followed.

@sgrimault
Copy link
Collaborator

On start-up, the application can perform a few small operations if it has network access:

  • update the parameter file from GeoNature (settings_occtax.json)
  • check for a new version of the application from GeoNature

It is up to the administrator of the GeoNature instance to provide a more recent version of the application.

There's no way of knowing how national parks configure their GeoNature instances. Generally speaking, we always push for the most recent versions. But nothing is guaranteed!
Ideally, there should be a compatibility table between GeoNature versions and the application (as dedicated API). In this way, the application will be able to perform this check as soon as we configure which instance of GeoNature we wish to connect to. And we can notify the user that this instance of GeoNature is not compatible.

What do you mean by "opt-in self-updater" ? Something like that ?
https://developer.android.com/guide/playcore/in-app-updates

Somewhere, this is already the case. Once the application has done its startup check and detected a new version, it simply notifies the user that a new version is available. It's up to the user to perform the update at any time.

@IzzySoft
Copy link
Author

IzzySoft commented Mar 6, 2024

I'm inclined to say your app is a very specific use-case, and we should keep things simple. So as long as it's clear to users where those updates are coming from when they're announced, and they are free to choose to accept them or rather update via their F-Droid client, that should be fine. I don't want to put too much burden onto your shoulders for this – it's not that there is much choice. And two different versions here would just cause confusion and add complications. What do you think? That a way we can go?

Something like that ?

That's a little different if I understand it correctly. It would be bound to PlayStore and just trigger updates from there. Depending on how far back you'd need to go, that might not be working with my repo, as I only keep up to 3 versions here. With your app currently being around 7 MB per APK, I could of course add an exception and keep 4. If your self-updater uses my repo as source, all checks would be applied so there cannot be any objections from my end.

Somewhere, this is already the case. Once the application has done its startup check and detected a new version, it simply notifies the user that a new version is available. It's up to the user to perform the update at any time.

I'd count that as opt-in then. Remaining question then would just be where the update is taken from. The notification is sent by the instance, so the part of "checking a potentially non-free source" is not there.

So how would the install process be then? I get a notification from the instance, and then look where? Download from inside the app and self-install (which would need that permission) – or check the app manager of my choice (the F-Droid app, or some other app store) to install the version the notification recommended?

@sgrimault
Copy link
Collaborator

No, the application currently relies on the Geonature instance to which it points.
On startup, it checks whether a more recent version is present. If so, it notifies the user that a more recent version of the application is available. The user can then choose whether or not to install it now or later. If the user decides to update, the application will simply download the APK from GeoNature and proceed with the update. Hence the REQUEST_INSTALL_PACKAGES permission required by the application.

If we don't distinguish between the current version of the application (managed by GeoNature) and the one coming from F-Droid, we must :

  • have the same applicationId
  • be signed in the same way with the same certificate

In this way, the application can be updated transparently, either from GeoNature (if the version on GeoNature is more recent) or from F-Droid (if the version on F-Droid is more recent).
But, with the risk of the application version being too recent for the GeoNature instance on which it has been configured...

@IzzySoft
Copy link
Author

IzzySoft commented Mar 9, 2024

And with the new version becoming available in my repo within 24h of your making it available here, it won't be "older" in my repo – just the same or newer than what the instance offers.

OK, got it. So what do you propose how we shall proceed here? I'm open to compromises, but debug builds would be a bit critical. Having a releasekey-signed APK here with the same applicationId would make cross-updates impossible (signature mismatch) but might enable a slow move over to this one by the instances, once they become aware of that. Same would of course work with a different applicationId which then would also make it possible to install the two alongside each other. As for the updater, guess we could at least include a hint with the app description (also with a few words explaining the background).

@IzzySoft
Copy link
Author

So what did we decide on now, @sgrimault? End of the month I wanted to have that issue closed.

@sgrimault
Copy link
Collaborator

Hello @IzzySoft ,

I suggest you to start with the simple solution of combining the two certificates (the current one, shared between the different versions of the application, and the new one, which will meet the F-DRoid requirements).
And if you want, I can make you an "unofficial" version of the future version 2.7.0 so that you can have a look/test on your own.
Future version 2.7.0 is not yet ready, so I'm still waiting for any feedback from the national parks.

@IzzySoft
Copy link
Author

I suggest you to start with the simple solution of combining the two certificates

How would I do that? Are you planning to dual-sign? I know there can be multiple signatures on a single APK file, but I've never seen such an APK – nor do I know how fdroidserver would handle that. I can give that a try if you have such an APK of course. And then sit in the conflict how I should deal with the mix of a "release signature" and a "debug signature" on the same APK file 🙈

Thanks for the offer of the "unofficial" version – but I unfortunately won't find time for testing (drowning in multiple other tasks currently).

Future version 2.7.0 is not yet ready, so I'm still waiting for any feedback from the national parks.

Thanks for letting me know! So what will 2.7.0 then be signed with? Do you plan switching certs for that?

@sgrimault
Copy link
Collaborator

It's the simplest solution I can implement quickly, and we can also test it to see if combining the two certificates works.
As I explained earlier, I can't delete the current certificate without breaking compatibility with the application update via GeoNature.

If the two-certificate approach works, then future version 2.7.0 will carry the current certificate plus the new one, and subsequent versions can then do without the current certificate (replaced by the new one). The only constraint is that this approach only works on Android 9-based terminals.

@sgrimault
Copy link
Collaborator

Hello @IzzySoft,

So I did a few tests by signing the APK with the current certificate and then making a "new version" that uses the new certificate (with key rotation) and then successfully updated the currently installed application:

creating rotation lineage file:

apksigner rotate --out SigningCertificateLineage.debug.release --old-signer --ks debug.keystore --new-signer --ks release.keystore

signing APK with rotation data:

apksigner sign --ks debug.keystore --next-signer --ks release.keystore --lineage SigningCertificateLineage.debug.release --in occtax-2.7.0-rc8-generic-debug.apk --out occtax-2.7.0-rc8-generic-debug.rotate.apk

The following APK with new certificat with rotation data:
occtax-2.7.0-rc8-generic-debug.rotate.tar.gz

@IzzySoft
Copy link
Author

IzzySoft commented Apr 5, 2024

Sounds good! I took a look at the APK, but it shows only a single signer and has the debug flag active. Do you want to provide me a release APK with key rotation active, so I add that to the repo and we can see if a) it's accepted by fdroidserver and b) by fdroidclient, so c) it can be updated to via fdroid?

(apologies for my delayed answer, I had a short "vacation break")

@sgrimault
Copy link
Collaborator

Yes, no worries :)

The "rc" version I provided you with contains the elements for debugging, but not the "release" version.
I've made you a new sample version as if it were a release:
occtax-2.7.0-rc8-generic-release.rotate.tar.gz

After checking we can see that v3 scheme is being used:

$ apksigner verify -v ../occtax-2.7.0-rc8-generic-release.rotate.apk
Verifies
Verified using v1 scheme (JAR signing): false
Verified using v2 scheme (APK Signature Scheme v2): true
Verified using v3 scheme (APK Signature Scheme v3): true
Verified using v4 scheme (APK Signature Scheme v4): false
Verified for SourceStamp: false
Number of signers: 1

@IzzySoft
Copy link
Author

IzzySoft commented Apr 5, 2024

OK, you're using a newer version of apksigner there. Did you test if that APK installs on top of a previous one with the old key?

Some notes:

  • key rotation details are stored with v3 only
  • apksigner only shows me the new key
  • my security expert tells me fdroidserver will only see the old key
  • sec exp also tells me fdroidclient will only check the new key, as the old one is only in v2

so adding this APK to my repo should work straight away, but I'm not sure if the APK would install over an existing older one. Android should accept it I guess, but fdroidclient might not offer to:

image

Going by that graph, it would only check using v3 and thus think the signature does not match. But we can give it a try if you wish. So if you want to, here's what we could try:

  1. I add the APK to my repo here. If it's not accepted as-is, I'd skip and abort to discuss further – or add the new key to the allow-list if you say "go for it".
  2. new index etc will be pushed around 6 pm UTC
  3. as soon as it's there, you validate if the update works as expected. If it does, we keep it – should it fail I'd immediately roll back and manually trigger a sync, so with some luck nobody except the two of us would notice.

Depending on where we end up, we'd decide how to continue. If all works well I'd suggest keeping this configuration for a month or so (if possible), then drop the old key and play the game again. If that works out fine as well, I'd remove the old key (and any APKs left signed with only that) on my end, and we call our case closed successful.

Waiting for your signal before doing anything here. But looking forward to the experiment eagerly 🤪

@sgrimault
Copy link
Collaborator

yes, I've done a few tests and the update goes smoothly from a previous version to the new one signed with the new certificate.
Then we can consider deleting the old certificate in future versions (i.e. n+2).

@IzzySoft
Copy link
Author

IzzySoft commented Apr 7, 2024

Yupp, that matches what I thought. Shall I take that rc8 APK from above, or is there a "final release" one coming?

@sgrimault
Copy link
Collaborator

I'd rather wait a little longer and get an official "GO" from the national parks. There are still some small issues to be addressed for the future 2.7.
cc. @camillemonchicourt, @DonovanMaillard

@IzzySoft
Copy link
Author

Just give me a ping when ready. I somehow got eager to see how fdroidserver (and client) handle that 😜

@IzzySoft
Copy link
Author

IzzySoft commented Jun 3, 2024

Did the rangers report back meanwhile? 😉

@sgrimault
Copy link
Collaborator

I've released version 2.7.0-rc8 which contains the latest corrections based on feedback. I'm waiting for new feedback on this version.
For your information, this version is signed with the new certificate.

@IzzySoft
Copy link
Author

IzzySoft commented Jun 4, 2024

I'm waiting for new feedback on this version.

From the parks, or from me? By the file name it's a debug build. Will that become a release build when ready? IIRC, the other ones (at least when not marked pre-release) were release builds.

@sgrimault
Copy link
Collaborator

From the parks.
And yes this is still a RC version (and thus with debug flag)

@IzzySoft
Copy link
Author

Sorry to knock again: any ETA? I need to close that issue on my end, which I just kept open to give you more time here – but I cannot do that for much longer…

@sgrimault
Copy link
Collaborator

Hello @IzzySoft,

I'm currently working on the official version of the application based on the latest feedback. There are still a few things to work out, but it could be the subject of a future release.
I'll let you know as soon as it's ready (today or tomorrow).

@IzzySoft
Copy link
Author

Thanks – and 🤞

@sgrimault
Copy link
Collaborator

Hello @IzzySoft,

New 2.7.0 release : https://github.com/PnX-SI/gn_mobile_occtax/releases/tag/2.7.0 🙂

@IzzySoft
Copy link
Author

Yay! Thanks, went in smoothly. Added the new signing key to be permitted – and fdroidserver (with our patches) accepted it fine (fdroidserver with the official version will most likely fail as they knowingly broke support for key rotation). Marked the old (debug) key and the APKs signed with it to be removed in about a months.

Congrats! So we could close this issue now I guess – except there's one question still open:

! repo/fr.geonature.occtax2_3310.apk declares sensitive permission(s):
  android.permission.REQUEST_INSTALL_PACKAGES
  android.permission.READ_EXTERNAL_STORAGE
  android.permission.MANAGE_EXTERNAL_STORAGE

I guess the storage permissions we could add to the app's "green list" if you tell me what they are needed for (so I can add a reason). But what will we do with that self-updater? Something like outlined above – or maybe you already have done that?

@sgrimault
Copy link
Collaborator

For now, we're keeping things as they are, with the application updating itself according to the GeoNature configuration. Hence the permission android.permission.REQUEST_INSTALL_PACKAGES.
The other two permissions are used by the maps module to search for map data. This data can be located anywhere on the device, depending on the GeoNature configuration.

@IzzySoft
Copy link
Author

Thanks! Added the other two to the "green list". Still unsure how to deal with the self-updater, as per se it violates the inclusion criteria – though I see why an exception is needed here. It does not update automatically, right, but requires a confirmation? Then we could say its (dark) gray area. Might need to add NonFreeNet then, unless the updater is opt-in.

Btw, just got confirmation: not only the key-rotation APK was accepted by the repo builder (we use our own fork of fdroiddata here – the official one got broken in this regard back in January), but there's also currently 1 F-Droid client that can deal with it: Neo Store. A second one has support for that already implemented and will support it with its next release: Droid-ify. Just pointing this out should someone ask – though in your case the self-updater would take care for that.

@IzzySoft
Copy link
Author

@sgrimault as the REQUEST_INSTALL_PACKAGES warning popped up with the new release again, let me repeat the unanswered parts from my previous comment (you probably missed the notification back then):

  • It does not update automatically, right, but requires a confirmation?
  • Might need to add NonFreeNet then, unless the updater is opt-in.

Could you please clarify so we can get this off the table (and most likely also close this issue)? Thanks in advance!

@sgrimault
Copy link
Collaborator

@IzzySoft ,
Yes, that hasn't changed. The application can update itself via the GeoNature instance, and therefore needs this permission.

@IzzySoft
Copy link
Author

Thanks Sebastian! Gray area, but as said this is a rather special case. Added it to the app's "green list" as "needed for updates by some GeoNature instances" then.

Guess we can close this issue now? Feel free to hit the button 😉

@sgrimault
Copy link
Collaborator

okay, fine, I'll close it.

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

No branches or pull requests

2 participants