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

Update Windows code signing certificate #628

Closed
rgaudin opened this issue Aug 1, 2024 · 28 comments · Fixed by #641
Closed

Update Windows code signing certificate #628

rgaudin opened this issue Aug 1, 2024 · 28 comments · Fixed by #641
Assignees
Labels
build dependencies not a bug It's not a bug, it's a feature! security Vulnerabilities or other issues affecting security
Milestone

Comments

@rgaudin
Copy link
Member

rgaudin commented Aug 1, 2024

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:

  • Kiwix JS Electron.exe (inside kiwix-js-electron_3.3.8_x64.nsis.zip)
  • Kiwix JS Electron.exe (inside kiwix-js-electron_3.3.8.zip)
  • kiwix-js-electron_x86-64_3.3.8.appx
  • kiwix-js-windows_3.3.8.appx
  • kiwix-js-windows_3.3.8.0.appxbundle
  • kiwix-js-electron_win_setup_3.3.8.exe
  • kiwix-js-electron_win_portable_3.3.8.exe
  • kiwix-js-electron_win7_setup_3.3.8.exe
  • kiwix-js-electron_win_web_setup_3.3.8.exe

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
Copy link
Member

Jaifroid commented Aug 1, 2024

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 Jaifroid transferred this issue from kiwix/kiwix-js Aug 1, 2024
@Jaifroid
Copy link
Member

Jaifroid commented Aug 1, 2024

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
Copy link
Member Author

rgaudin commented Aug 1, 2024

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
Copy link
Member

Jaifroid commented Aug 3, 2024

@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:

  • win-ia32 (one for Win7/8/8.1; one for Win10/11)
  • win-arm64 (Win10/11 only)
  • win-x64 (Win10/11 only)
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

  • 18 unique signings per official release of each of Kiwix JS Electron, Wikivoyage by Kiwix, and WikiMed by Kiwix
  • Maximum number of unique signings per month (excluding nightlies): 18 * 3 = 54 monthly

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

@Jaifroid
Copy link
Member

Jaifroid commented Aug 3, 2024

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?

@Jaifroid Jaifroid added not a bug It's not a bug, it's a feature! build security Vulnerabilities or other issues affecting security dependencies and removed bug labels Aug 3, 2024
@Jaifroid Jaifroid added this to the Release 3.4.0 milestone Aug 3, 2024
@rgaudin
Copy link
Member Author

rgaudin commented Aug 3, 2024

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
Copy link
Member

Jaifroid commented Aug 3, 2024

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

@rgaudin
Copy link
Member Author

rgaudin commented Aug 5, 2024

@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
Copy link
Member

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
Copy link
Member

Jaifroid commented Aug 5, 2024

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
Copy link
Member Author

rgaudin commented Aug 5, 2024

It works!

Screenshot 2024-08-05 at 18 41 40

@Jaifroid
Copy link
Member

@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
Copy link
Member Author

rgaudin commented Aug 14, 2024

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
Copy link
Member

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
Copy link
Member

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

@Jaifroid
Copy link
Member

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. electron-userland/electron-builder#8276 (comment)

@Jaifroid
Copy link
Member

@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
Copy link
Member

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

@rgaudin
Copy link
Member Author

rgaudin commented Aug 16, 2024

Indeed! I just added them.

@Jaifroid
Copy link
Member

Great! Many thanks!

@Jaifroid
Copy link
Member

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

Issues encountered

  • I could not get anything to work with the org credentials you provided. I get the error seen here and specifically this message on line 126 of the workflow: Signing credentials not configured. Make sure certificate is issued before signing. I suspect those credentials have not been correctly initialized?
  • Instead, I added my own credentials to this Repo's secrets (with a USER qualification to distinguish them), and this partially works. It signs all the exe files, but chokes on the appx files. I'm guessing it's an incompatible version of signtool, although I'm using the latest installed on GA's "windows-latest", because it makes no sense that these sign fine on my own machine (see below) using the exact same procedure. I'll experiment with downloading a different kit.

Success on local PC:

  • I can build all Windows apps and sign them all locally (on my PC) as seen in screenshot below
    image

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
Copy link
Member

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

That was hard work...🥵

@rgaudin
Copy link
Member Author

rgaudin commented Aug 18, 2024

Good news! Mind sharing what the difficulty was?

@Jaifroid
Copy link
Member

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:

  • If signing on Windows, I found I had to use a powershell runner, not the newer pwsh (PowerShell Core), though this may be related specifically to Electron Builder;
  • There are multiple SDKs and multiple SignTools already installed on GitHub. Getting their paths is guesswork. Although you'd have thought that a 64bit runner would work best with a 64bit SignTool, in fact only the x86 version in the latest SDK would sign anything at all with this procedure;
  • I've added this info to the Wiki right at the bottom, but for signing appx or msix, then it's necessary for the subject of the Certificate to match exactly the subject in the appxmanifest. I don't think this is a problem for exe files.

@Jaifroid
Copy link
Member

Jaifroid commented Aug 18, 2024

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
Copy link
Member

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.

@Jaifroid Jaifroid modified the milestones: Release 3.4.0, Release 3.5.0 Aug 19, 2024
@rgaudin
Copy link
Member Author

rgaudin commented Aug 19, 2024

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
Copy link
Member

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
build dependencies not a bug It's not a bug, it's a feature! security Vulnerabilities or other issues affecting security
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants