kiwix / kiwix-js-pwa

Kiwix JS Offline Browser implemented as a Progressive Web App (PWA), and packaged as Electron, NWJS and UWP apps for Windows and Linux.
https://pwa.kiwix.org
GNU General Public License v3.0
177 stars 29 forks source link

Update Windows code signing certificate #628

Closed rgaudin closed 3 weeks ago

rgaudin commented 1 month ago

It's not a bug obviously but it's time sensitive as the current certificate expires in 3 weeks. It's not just a certificate replacement as it's not possible anymore to get a Code Signing certificate with a private key bundled. Private keys now have to be stored in an HSM*. Our new certificate is thus now using a Cloud Signing solution (we dont have the private key).

All the details are available at https://github.com/kiwix/overview/wiki/Cloud-Code-Signing-(Windows)

⚠️ IMPORTANT ⚠️: We have a quota of cloud signs (240 per year for all kiwix binaries) and we decided that we'd only sign releases . Signing of nightlies must thus be removed.

Looking at the download folder, it looks like you have 9 files signed per release:

Is this correct? I see that you also publish a lot of variations for Custom Apps. I also see that the release frequency is higher than for other kiwix softwares. @Jaifroid could you provide an estimation of number of signatures needed, per year, for all the ones you are in charge of? We'll update our signing subscription based on your number.

A quick glimpse at your Github Actions workflow makes me think you are not releasing from there. Can you confirm? Can you explain this choice and whether this is subject to change or not. If not, I think we'll provide you with a signing account.

Please get in touch here or on Slack to discuss next steps

Jaifroid commented 1 month ago

Thanks for this issue. JFYI, all of the listed apps are published from the kiwix-js-pwa repo (and built/signed mostly on GitHub actions there, except the UWP versions), not from this one. We don't release any Windows executables for the Browser Extension currently. I'll transfer the issue if possible.

Jaifroid commented 1 month ago

One issue is that Electron Builder takes care of the signing, with the secret being provided via GitHub actions. I'll need to check if it works with the solution mentioned in https://github.com/kiwix/overview/wiki/Cloud-Code-Signing-(Windows), but that solution is for Linux, and Electron Builder can only build appx and appxbundle files on Windows. Since the setup is Java-based, maybe that's no big deal, I'll just need to adapt the code provided.

rgaudin commented 1 month ago

The solution works on Windows as well ; you just need to download the windows version ; but that's still one executable to run. If electron takes care of the signing, you might have to use the Cloud Key Adapter method so that the default (microsoft) tools are used. Let me know once you get there so I can assist you.

Jaifroid commented 1 month ago

@rgaudin Here are the number of unique signings for an official release of Kiwix JS Electron which I've extracted from the logs of the last GitHub Actions release. By way of explanation, each Electron release includes a package built for:

Windows 7/8/8.1

  • signing         file=dist\bld\Electron\win-ia32-unpacked\Kiwix JS Electron.exe certificateFile=C:\Users\RUNNER~1\AppData\Local\Temp\t-DVa83Q\0.p12
  • signing         file=dist\bld\Electron\win-ia32-unpacked\resources\elevate.exe certificateFile=C:\Users\RUNNER~1\AppData\Local\Temp\t-DVa83Q\0.p12
  • signing NSIS uninstaller  file=dist\bld\Electron\__uninstaller-nsis-kiwix-js-electron.exe certificateFile=C:\Users\RUNNER~1\AppData\Local\Temp\t-DVa83Q\0.p12
  • signing         file=dist\bld\Electron\Kiwix JS Electron Setup 3.3.8-E.exe certificateFile=C:\Users\RUNNER~1\AppData\Local\Temp\t-DVa83Q\0.p12

Windows 10/11

  • signing         file=dist\bld\Electron\win-ia32-unpacked\Kiwix JS Electron.exe certificateFile=C:\Users\RUNNER~1\AppData\Local\Temp\t-AgyBl5\0.p12
  • signing         file=dist\bld\Electron\win-unpacked\Kiwix JS Electron.exe certificateFile=C:\Users\RUNNER~1\AppData\Local\Temp\t-AgyBl5\0.p12
  • signing         file=dist\bld\Electron\win-arm64-unpacked\Kiwix JS Electron.exe certificateFile=C:\Users\RUNNER~1\AppData\Local\Temp\t-AgyBl5\0.p12
  • signing         file=dist\bld\Electron\Kiwix JS Electron 3.3.8-E.appx certificateFile=C:\Users\RUNNER~1\AppData\Local\Temp\t-AgyBl5\0.p12
  • signing         file=dist\bld\Electron\win-ia32-unpacked\resources\elevate.exe certificateFile=C:\Users\RUNNER~1\AppData\Local\Temp\t-AgyBl5\0.p12
  • signing NSIS uninstaller  file=dist\bld\Electron\__uninstaller-nsis-kiwix-js-electron.exe certificateFile=C:\Users\RUNNER~1\AppData\Local\Temp\t-AgyBl5\0.p12
  • signing         file=dist\bld\Electron\Kiwix JS Electron Setup 3.3.8-E.exe certificateFile=C:\Users\RUNNER~1\AppData\Local\Temp\t-AgyBl5\0.p12
  • signing         file=dist\bld\Electron\win-arm64-unpacked\resources\elevate.exe certificateFile=C:\Users\RUNNER~1\AppData\Local\Temp\t-AgyBl5\0.p12
  • signing         file=dist\bld\Electron\win-unpacked\resources\elevate.exe certificateFile=C:\Users\RUNNER~1\AppData\Local\Temp\t-AgyBl5\0.p12
  • signing NSIS uninstaller  file=dist\bld\Electron\nsis-web\__uninstaller-nsis-web-kiwix-js-electron.exe certificateFile=C:\Users\RUNNER~1\AppData\Local\Temp\t-AgyBl5\0.p12
  • signing         file=dist\bld\Electron\nsis-web\Kiwix JS Electron Web Setup 3.3.8-E.exe certificateFile=C:\Users\RUNNER~1\AppData\Local\Temp\t-AgyBl5\0.p12
  • signing         file=dist\bld\Electron\Kiwix JS Electron 3.3.8-E.exe certificateFile=C:\Users\RUNNER~1\AppData\Local\Temp\t-AgyBl5\0.p12

Additionally, I normally sign on my own PC (a clean VM) the UWP version built with Visual Studio 2017. I believe this involves two signings: one to code-sign the appxbundle, and one to add the timestamp.

I normally update Wikivoyage and WikiMed whenever there is a new ZIM to build with, i.e. monthly. For the base app, I also usually release monthly, but this could be reduced to bi-monthly.


Summary

You will no-doubt ask me for strategies to reduce the number of signings, and I can have a think about that.

Jaifroid commented 1 month ago

Perhaps some of the signing methods, like using the default MS Tools, might allow a number of signings per session, rather than counting each individual signing within a run? It seems extraordinary if we have to pay for each sub-signing of code, installer, uninstaller, etc., to produce a single package. Is there no other solution?

rgaudin commented 1 month ago

Thank you @Jaifroid that's very informative. That sums it to around 650 signings per year. I advise we switch to the next signing tier (1,200) which should be plentiful. We'll discuss it on Monday with @Popolechien and come back to you.

Jaifroid commented 1 month ago

Thanks! I was thinking 240 signings looked very difficult to achieve for a large operation like Kiwix!

rgaudin commented 1 month ago

@Jaifroid we'll get the Tier2! @Popolechien is a little bit behind at the moment so give him a couple days to order it. I'll come back to you once it's OK. You can already migrate though.

Popolechien commented 1 month ago

This is one of the worst websites ever but I poked around and it asked if I wanted to upgrade, I said yes, so I reckon you guys should be good.

Jaifroid commented 1 month ago

This is one of the worst websites ever but I poked around and it asked if I wanted to upgrade, I said yes, so I reckon you guys should be good.

Thank you very much! 🥳

rgaudin commented 1 month ago

It works!

Screenshot 2024-08-05 at 18 41 40
Jaifroid commented 3 weeks ago

@rgaudin I'm going to start working on this at the end of the week and weekend (I'm aware current certificate expires on Sunday, and I'm currently completing a new release of the main app, which needed some critical updates anyhow, in case it takes me a while to get the new setup working).

Just to be clear, I should disable code signing in all nightly builds as a first step, right? I think even the expanded limit would not cover those. Other than that, will I need any credentials to use the new cloud-based signing, or will it be enough with the secrets provided in the Repo? My use case is very likely to be setting up the GitHub VM (Windows) in such a way that the installed signtool used by Electron-builder under the hood will "just work" (fingers crossed 🤞).

rgaudin commented 3 weeks ago

Just to be clear, I should disable code signing in all nightly builds as a first step, right?

Yes.

Other than that, will I need any credentials to use the new cloud-based signing, or will it be enough with the secrets provided in the Repo

I just sent an invite to you as I understand you're still doing some signings outside of Github Actions. Quoting the linked doc

  • Signing users must first enroll in Cloud Signing by creating an OTP passcode in the web UI.
  • Under the Signing credentials expandable section at the bottom of the certificate page, make sur to disable malware detection. It's too suspicious and prevents legitimate binaries from being signed.
Jaifroid commented 3 weeks ago

I just sent an invite to you as I understand you're still doing some signings outside of Github Actions. Quoting the linked doc

Great, thank you, I've received the invite! 🙏

Jaifroid commented 3 weeks ago

(And yes, some signing will take place outside Actions.)

Jaifroid commented 3 weeks ago

Note to self - some code on how to patch Electron-builder's internal signing process by using the sign: and afterSign: keys in package.json. https://github.com/electron-userland/electron-builder/issues/8276#issuecomment-2256220661

Jaifroid commented 3 weeks ago

@rgaudin I've just noticed that the secrets mentioned in https://github.com/kiwix/overview/wiki/Cloud-Code-Signing-(Windows) are not available in this Repo, i.e.:

ESIGNER_USERNAME
ESIGNER_PASSWORD
ESIGNER_CREDENTIAL_ID
ESIGNER_TOTP_SECRET

Would you be able to add them? (I need these here in kiwix-js-pwa, rather than kiwix-js).

Many thanks in advance. 🙏

Jaifroid commented 3 weeks ago

(I presume these are Org secrets, rather than my own info that I should add?)

rgaudin commented 3 weeks ago

Indeed! I just added them.

Jaifroid commented 3 weeks ago

Great! Many thanks!

Jaifroid commented 3 weeks ago

@rgaudin I've had partial success converting to the new eSigner certificate and integrating this with Electron-Builder (in #641).

Issues encountered

Success on local PC:

Just to emphasize: the success on my own PC uses the same procedure for signing but uses a different signtool version.

It took a ridiculous amount of time, tinkering, trying alternative signing procedures, even writing my own sign.js (which turns out not to be needed, though it's still in the PR), testing locally, then testing ever-so-slowly on GH Actions 😶. I've also burned through a lot of signings (it says 61 used this month, but I don't think that was all me) to get this far... If most of that usage is me, then it seems to be "charging" failed signings as well as successful ones.

In any case, once it's all fixed, signings will only happen on full releases, so I'm not worried about the usage, just a necessary part of getting this right.

Jaifroid commented 3 weeks ago

Finally managed to get a good run with the appx and appxbundle properly signed!

That was hard work...🥵

rgaudin commented 3 weeks ago

Good news! Mind sharing what the difficulty was?

Jaifroid commented 3 weeks ago

Good news! Mind sharing what the difficulty was?

Yes, I wrote briefly about the difficulties in the PR. But there were lots of silly things that didn't work and because debugging actions is so difficult, each time I'd made a minor change hoping it would work, a long-running action had to complete or fail to provide often very limited feedback. The frustrating thing was that I got it working locally pretty quickly, but just couldn't get it to work consistently in cloud.

Main issues were related to Electron-Builder, PowerShell, a change in the "Subject" of the Certificate, and myriad different versions of SignTool.exe. Also, lack of documentation for my use-case in a runner. Most of that won't be relevant for other devs of Kiwix, as I think they'll only use the simple CodeSigner utility to sign a binary distributed inside a ZIP, with no installer, uninstaller, NSIS, Web installer, etc. There was also the issue that the org credentials kept failing for any kind of file where my credentials would work for exe files (commented on above). This would need to be corroborated.

These are other points that could be of relevance if Kiwix Desktop ever changes installer type or has to be signed on Windows as opposed to Linux:

Jaifroid commented 3 weeks ago

One other thing to be aware of. This certificate is an OV type instead of an EV type (as our previous one was). OV types are cheaper, but they don't establish as much reputation. So the first few times someone downloads on Windows, they're going to get screens looking like this, even though the app is signed. It's not too hard to work around, but some users may get scared off. This never happened with the previous certificate. This is a known issue with OV certs -- it only appears to affect exe files, not msix/appx (because these run in a secure container). Each specific download of an exe has to "establish reputation" by a user overriding the warning or reporting it as safe. Anyway, it is what it is. The EV pricing at SSL.com looked horrendous.

image image

Jaifroid commented 3 weeks ago

PR merged. It should skip signing of apps for nightly if I haven't made a mistake. Will double-check tomorrow and adjust if necessary. Nightly chron jobs only run on main.

rgaudin commented 3 weeks ago

Thank you @Jaifroid for the detailed explanation.

Regarding the credentials issue, it should eventually be investigated ; probably the next time someone needs to sign using the kiwixbot credentials.

Regarding your last comment, no the previous certificate was not EV, it also was an OV one. That warning is normal and would also appear with an EV certificate (that's new W11 behavior though).

As can be confirmed from other sources, Microsoft needs to build trust for apps and/or certificates and since this certificate is new, windows shows this warning. It will fade away with certificate usage.

Jaifroid commented 3 weeks ago

As can be confirmed from other sources, Microsoft needs to build trust for apps and/or certificates and since this certificate is new, windows shows this warning. It will fade away with certificate usage.

Ah, that's good to know. I didn't realize it's the certificate itself that accrues trust. Makes sense now.