Akylas / alpimaps

Offline map app iOS/Android
https://www.akylas.fr
MIT License
71 stars 1 forks source link

signing key #17

Closed IzzySoft closed 3 months ago

IzzySoft commented 3 months ago

Something happened to your signing key it seems. Today's update shipped signed with this one:

Number of signers: 1
Signer #1 certificate DN: CN=Unknown, OU=Unknown, O=Unknown, L=Unknown, ST=Unknown, C=Unknown
Signer #1 certificate SHA-256 digest: 706a362910a6b49205c09dab9092d1e923be631b0e9327cfe0a93c5ae30c9763
Signer #1 certificate SHA-1 digest: b2f9230c49c2541d72751de78f603d55cf9d0c30
Signer #1 certificate MD5 digest: cb90cd211082cdae128a0245e6d1a22f
Signer #1 key algorithm: RSA
Signer #1 key size (bits): 2048

which was blocked as it's not listed as AllowedAPKSigningKey. 254 had this one:

Number of signers: 1
Signer #1 certificate DN: CN=Appcelerator Titanium, OU=Nolan's Gang, O=Appcelerator, L=Mountain View, ST=CA, C=US
Signer #1 certificate SHA-256 digest: d6d29621132d4af29ba142acf0a6e587daafb38a61d515ca528ad6d2b05ba485
Signer #1 certificate SHA-1 digest: cce37f08fa039c8807bccbab7b8861f4759d479f
Signer #1 certificate MD5 digest: 665ed17fc42cb973ad7f4ddd29125656
Signer #1 key algorithm: RSA
Signer #1 key size (bits): 1024

Can you please check what went wrong, and replace the APK with one signed by the correct key? Thanks in advance!

farfromrefug commented 3 months ago

@IzzySoft thanks. Actually was on purpose as you told me before the old one was gonna expire. I made the move to use a new one. Is that OK?

IzzySoft commented 3 months ago

I was searching for that issue as I remembered something like that! Unfortunately, other than I usually do, I seem to have left no note in the YAML. Let me look again then… No notes in my YAML. But right:

The certificate uses the MD5withRSA signature algorithm which is considered a security risk and is disabled.
The certificate uses a 1024-bit RSA key which is considered a security risk. This key size will be disabled in a future update.

But where was the related issue? Maybe over at IoD? I also vaguely remember talking about that "Unknown" thingy. So did we say this should be the new key nevertheless?

farfromrefug commented 3 months ago

@IzzySoft here it is ahttps://github.com/Akylas/alpimaps/issues/15#issuecomment-2054219743 Thought I could use any key to replace it. Was I wrong ?

IzzySoft commented 3 months ago

Ah, hidden in the permission issue… strange. I must have missed some search term then, apologies!

Thought I could use any key to replace it. Was I wrong ?

Yes and no. The signing key for an app is pinned here (to prevent malicious updates by imposters: someone might get control over a repo, but getting control over the signing key at the same time is rather unlikely unless it was placed with the CI config here). So while a signing key can be changed, it means two things:

  1. It needs to be coordinated (i.e. involves both sides, the app dev and the repo maintainer)
  2. It means everyone having the previous version installed needs to uninstall and re-install

As we already figured, the latter cannot be helped here. But to make folks aware of the action needed, it should be mentioned in the corresponding per-release changelog, ideally prominently enough to not be missed. May I suggest preceding those lines by something like

### Breaking
The app's signing key had to be changed as the old one used an algorithm meanwhile considered unsafe. To update, you will have to uninstall and reinstall the app.

That adds about 180 chars. There are already 369 in the file, so this would be ~50 chars too long which means we'd need to optimize there. Let me see… hm, markdown isn't supported there anyhow, so those URLs could be simplified (or the commits simply stripped). There, clearly inside the 500-char-limit again:

### Breaking
The app's signing key had to be changed as it used an algorithm meanwhile considered unsafe. To update, you will have to uninstall and reinstall the app.

### Features
- get local time from anywhere in the world. astronomy view will show local data + current time+ more daylight related infos (commit 3e54c5ec)

### Bug fixes
- prevent adding existing source (commit 0f12e191)
- item info fix (commit bb1c3d39)

Then, a short suggestion: your new signing key looks a bit… well, strange. I mean, its DN:

Signer #1 certificate DN: CN=Unknown, OU=Unknown, O=Unknown, L=Unknown, ST=Unknown, C=Unknown

That DN is a human readable part one usually takes to associate with the signer (app developer or vendor). One especially doesn't set all fields to "Unknown", but simply leaves those empty which one needs to skip. Well, the original one obviously wasn't you either:

Signer #1 certificate DN: CN=Appcelerator Titanium, OU=Nolan's Gang, O=Appcelerator, L=Mountain View, ST=CA, C=US

Maybe going with

CN=farfromrefuge O=Akylas L=Grenoble C=FR

That's all publicly available information anyway (your Github user and details from the org). Looks much more trust-able than "Unknown Unknown Unknown", wouldn't you agree? So when changing for security reasons, maybe better not losing trust along the path? :wink:

farfromrefug commented 3 months ago

@IzzySoft about the key s data. It is actually the same key I use for my other apps. And yes it is always unknown cause I never take the time to fill those fields. :s I can use a real new one with filled data if you think it is better. And yes I will put it in the change log

IzzySoft commented 3 months ago

I can use a real new one with filled data if you think it is better.

It will definitely look more trustworthy, so I'd highly recommend it. I won't force you to however. But as you have to change it here anyway, it might be a good chance.

And yes I will put it in the change log

Great, thanks! The updater will come along again around 5 pm UTC. It will yell at me again as the key was changed. I'll look then if its the new one and, if it is, take the proper actions and report back here again. Of course you can ping me here anytime should there be questions open – or should you want me to check earlier to be sure :wink:

farfromrefug commented 3 months ago

@IzzySoft not sure when I ll have the Time To do it. But i ll ping you

farfromrefug commented 3 months ago

@IzzySoft i did update the key, the latest github release apks and the changelogs

IzzySoft commented 3 months ago

Thanks, cert looks fine now! Update in place, new key pinned. Just the changelog will be truncated at exactly char 500 (Fastlane limit) as you've left the Markdown links in (which don't work anyway as changelogs are defined to be plain text). Which means the last line or so will be missing (changelog has 546 chars currently, as I pointed out above with the suggestion to "un-markdown" the commit hashes :wink:) Nothing crucial, though. I can fix that here locally this time (just not regularly each time :stuck_out_tongue_winking_eye:) – done.

farfromrefug commented 3 months ago

@IzzySoft thanks! no big deal for the changelog ;)