zathras / jrpn

JRPN - A Calculator Simulator Inspired by the HP-16C "Computer Scientist" Calculator and the HP-15C Scientific Calculator
https://jrpn.jovial.com/
Other
65 stars 8 forks source link

signing key changed? #88

Closed IzzySoft closed 8 months ago

IzzySoft commented 8 months ago

My updater today complained:

WARNING: "com.jovial.jrpn15_25.apk" is signed by a key that is not allowed:
33973c67254b45cc504dfb28aa793b5ac3302e1119600809f177d80610b9f2f8

which means your update was rejected automatically, as id did not match the key pinned here (and thus would be rejected on-device as well). Previous key was:

Signer #1 certificate DN: CN=Bill Foote, OU=Unknown, O=Jovial Software, L=Los Angeles, ST=CA, C=US
Signer #1 certificate SHA-256 digest: 669d5a153142f98edf9d369259a448f8a836a58433914db435a9e76da4663f9f
Signer #1 certificate SHA-1 digest: d4e0697d86b14b9af51a770fb6ca744031d1897d
Signer #1 certificate MD5 digest: 75bbea2100d560f6ca369a1b6aa01a4a
Signer #1 key algorithm: RSA
Signer #1 key size (bits): 2048

but the recent release is signed by:

Signer #1 certificate DN: CN=Jovial Software, OU=Unknown, O=Unknown, L=Unknown, ST=Unknown, C=Unknown
Signer #1 certificate SHA-256 digest: 33973c67254b45cc504dfb28aa793b5ac3302e1119600809f177d80610b9f2f8
Signer #1 certificate SHA-1 digest: 0e026ba7ade069bdcc5801806a9480ab57768c16
Signer #1 certificate MD5 digest: a63a92255325650433800b4470059111
Signer #1 key algorithm: RSA
Signer #1 key size (bits): 2048

So I had to disable updates for now until this is clarified. I've checked your release notes, but could not find any hint. Can you please state what happened? Maybe this was unintentional? Thanks in advance!

zathras commented 8 months ago

Yes, the signing key has changed, because I automated the APK build using github actions. I did set it up so it will consistently use this new key going forward. See also https://github.com/zathras/jrpn/issues/71

IzzySoft commented 8 months ago

OK, I see. Just for verification (as this could very well be "not you" but someone having gained access to your repo): can you provide the latest APK signed with the "old key" as well (i.e. an APK built from the very same commit, just signed by the previous key)? That would confirm the switch is "legit" – and I could perform the "migration" in my repo.

For some background, see e.g. How to keep your key safe and what measures to take for the event of loss? (speaking about which, it would be a good idea to have your commits signed :wink:)

Thanks in advance!

zathras commented 8 months ago

Well, like many things in security, make sure you understand what threat you're protecting against.

Like it or not, building on a cloud computing platform, like GitHub actions, inevitably means that if my github account is hacked, the attacker gets the private key. If I were less careful about security, I'd just have put my old private key up on the build server, and you'd never have noticed. But because I'm a little more paranoid than that, I went with a new keystore on the github build server. The net security impact to you is zero, but my upload to the Google app store is perhaps slightly more secure.

Now, I use 2FA on github and am pretty secure, so a github account compromise is pretty unlikely. Whether it's more or less unlikely than an attacker gaining control of my development machine via some other exploit is a matter for debate.

In order to upload something to the releases section of my github account, they'd need to gain access to my github account. Which gives them the signing key. So, me "proving" that I'm me this one time doesn't really increase your overall security, if the thing you're worried about is a github account compromise.

Frankly, a much bigger vulnerability than all this is the fact that all software these days pulls in a bazillion dependencies from all over the place, and nobody really audits the transitive closure of everything they include. (I'm probably better than most -- I at least tried to minimize my dependencies, and I pay attention to the authors. But the Flutter platform I'm using surely pulls in God-knows-what from God-knows-where.) But I digress.

Anyway, FWIW, here's a JRPN 16 APK from the development machine. app-release.apk.zip

IzzySoft commented 8 months ago

And now guess why I suggested you to sign your commits. It's good that you use 2FA (Github gives me no way to verify that, but I believe you do). And you already guessed I'd say it's not a good idea to put private stuff in the cloud (while 2FA offers some protection here, the cloud provider would still have access to it).

all software these days pulls in a bazillion dependencies

Indeed it does, and especially with Flutter maybe (I had multiple cases already where we needed to find the culprit – a well-known candidate there is Flutter's geolocation pulling in GMS). And as we are frank, let me tell you I have a scanner in place for such things, too (see What about security? on my repo's info page – and for more details the article that will go live tomorrow in my blog). So just because there are other vulns does not mean to ignore this one :wink:

here's a JRPN 16 APK from the development machine.

Which doesn't match the one at the latest release: plenty of differences in DEX, resources and assets. Was that built from the same commit?

$ $ apkdiff app-release.apk jrpn16_app-release.apk
Binary files /tmp/tmp.ksHBuq61qv/a/assets/flutter_assets/NOTICES.Z and /tmp/tmp.ksHBuq61qv/b/assets/flutter_assets/NOTICES.Z differ
Binary files /tmp/tmp.ksHBuq61qv/a/classes.dex and /tmp/tmp.ksHBuq61qv/b/classes.dex differ
Binary files /tmp/tmp.ksHBuq61qv/a/lib/arm64-v8a/libapp.so and /tmp/tmp.ksHBuq61qv/b/lib/arm64-v8a/libapp.so differ
Binary files /tmp/tmp.ksHBuq61qv/a/lib/arm64-v8a/libflutter.so and /tmp/tmp.ksHBuq61qv/b/lib/arm64-v8a/libflutter.so differ
Binary files /tmp/tmp.ksHBuq61qv/a/lib/armeabi-v7a/libapp.so and /tmp/tmp.ksHBuq61qv/b/lib/armeabi-v7a/libapp.so differ
Binary files /tmp/tmp.ksHBuq61qv/a/lib/armeabi-v7a/libflutter.so and /tmp/tmp.ksHBuq61qv/b/lib/armeabi-v7a/libflutter.so differ
Binary files /tmp/tmp.ksHBuq61qv/a/lib/x86_64/libapp.so and /tmp/tmp.ksHBuq61qv/b/lib/x86_64/libapp.so differ
Binary files /tmp/tmp.ksHBuq61qv/a/lib/x86_64/libflutter.so and /tmp/tmp.ksHBuq61qv/b/lib/x86_64/libflutter.so differ
…
Binary files /tmp/tmp.ksHBuq61qv/a/res/_C.png and /tmp/tmp.ksHBuq61qv/b/res/_C.png differ

EDIT: The differences in the .so files may be due to the fact that Flutter embeds the full path into them, and the APKs having been built at different environments with different paths. That still leaves the classes.dex and (a possible pointer to different commits) the NOTICES.Z file. Plus that PNG. Maybe you simply build an APK on your development machine, then create a copy of the unsigned APK, and sign one copy with the original and the other with the new certificate? Then the APKs should be identical except for the signature. It would of course be better if you'd sign a copy of the APK attached to the latest release here with the old key, so it's clear what it is compared against.

IzzySoft commented 7 months ago

So where do we stand here now? I just got this again (with the update checks set to monthly):

2024-04-01 03:38:35,500 WARNING: "com.jovial.jrpn15_25.apk" is signed by a key that is not allowed:
33973c67254b45cc504dfb28aa793b5ac3302e1119600809f177d80610b9f2f8
2024-04-01 03:38:35,500 WARNING: "com.jovial.jrpn2_25.apk" is signed by a key that is not allowed:
33973c67254b45cc504dfb28aa793b5ac3302e1119600809f177d80610b9f2f8

so next step would be disabling updates completely here as I don't want to have this pop up every months :wink:

zathras commented 7 months ago

That should have been signed with the same key as the last build. Builds are consistently signed with the same key, starting one release ago.

IzzySoft commented 7 months ago

Yes, but as the key change was not verified (see my comment above: the APKs didn't match), it's still the "old key" which is pinned here.

May I suggest you take the already signed APK jrpn15_app-release.apk, manually sign it with the "old key" e.g. using apksigner sign --ks keystore.jks --in jrpn15_app-release.apk --out jrpn15_app-release_oldkey.apk (which would overwrite the signature already there), and provide the resulting jrpn15_app-release_oldkey.apk here? I'd then check those two (the problems outlined in my previous comment should not be there then), which should confirm you're in possession of both keys and thus the switch-over legit, and enable me to perform the migration here.

Thanks in advance!

zathras commented 7 months ago

What about JRPN 16?

IzzySoft commented 7 months ago

That's the one identifying as com.jovial.jrpn2. Let me check… Apart from the logs stating

! repo/com.jovial.jrpn2_25.apk contains signature block blobs: 0x504b4453 (DEPENDENCY_INFO_BLOCK; GOOGLE)

same issue as with 15, and would be solved along the path: if 15 is verified, that can be applied to 16 as well – so to keep things simple, I concentrated on one of the two.

Currently, updates are set to "monthly" instead of "daily" for both (originally in the hope the issue would resolve quickly) – so the above log line was from the last monthly check, with the APK not being accepted as the certificate did not match.

So, can you please provide me with an APK as outlined above – so I can verify and then hopefully switch to the new key, thus solving this issue?

zathras commented 7 months ago

OK. I don't have an easy way of verifying this, but maybe it worked. jrpn15_app-release_oldkey.apk.zip

IzzySoft commented 7 months ago

Thanks! That worked out. Only difference shown are the signing keys/certs, which are expected to be different. So I've pinned the new key (the old one will be removed in a month) and triggered an update – which will become available with the next sync around 6 pm UTC. Btw, updater reported:

! repo/com.jovial.jrpn15_26.apk contains signature block blobs: 0x504b4453 (DEPENDENCY_INFO_BLOCK; GOOGLE)

which can be easily avoided:

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.


Performed the transition for com.jovial.jrpn2 as well, as that uses the same key(s). Same DEPENDENCY_INFO_BLOCK message btw :wink:

So thanks for your help! Updates are set to daily check again, with the latest releases in place.