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

>26.0.1 gets stuck on Windows signing the squirrel exe #8854

Closed
t3chguy opened this issue Feb 7, 2025 · 28 comments · Fixed by #8855
Closed

>26.0.1 gets stuck on Windows signing the squirrel exe #8854

t3chguy opened this issue Feb 7, 2025 · 28 comments · Fixed by #8855

Comments

@t3chguy
Copy link
Contributor

t3chguy commented Feb 7, 2025

Before update (26.0.1) https://github.com/element-hq/element-desktop/actions/runs/13175306673/job/36773250686
After update to 26.0.6 https://github.com/element-hq/element-desktop/actions/runs/13196581336/job/36839196683 it errors when calling upon a vendored signtool.exe from electron-winstaller/electron-windows-sign - the signing we use via eSignerCKA requires a rather modern signtool so we prefer the system one by use of the SIGNTOOL_PATH envvar which electron-builder respects.

electron-win-sign however does not respect it, it instead reads WINDOWS_SIGNTOOL_PATH upon changing to which signing just seems to hang. https://github.com/element-hq/element-desktop/actions/runs/13197854827/job/36843095558

I'm surprised to see D:\a\element-desktop\element-desktop\node_modules\electron-winstaller\vendor\signtool.exe being called given e-b is passing a signing hookFunction so should be using its own signtool handling -

if (await (await packager.signingManager.value).cscInfo.value) {
options.windowsSign = {
hookFunction: async (file: string) => {
await packager.sign(file)
},
}
}

Maybe that if-statement isn't being entered due to the condition not doing the same thing as the old signing code - looks like other areas in the code just look at options.sign potentially https://github.com/electron-userland/electron-builder/blob/master/packages/app-builder-lib/src/platformPackager.ts#L370

@t3chguy
Copy link
Contributor Author

t3chguy commented Feb 7, 2025

On a retry with debug logging enabled it seems to hang directly after signing the MSI https://github.com/element-hq/element-desktop/actions/runs/13200748516/job/36851975119

@t3chguy
Copy link
Contributor Author

t3chguy commented Feb 7, 2025

electron-builder run with debugging and crash from electron-windows-sign due to it not using e-b's packager.sign: https://github.com/element-hq/element-desktop/actions/runs/13204321507/job/36863804553

the reason I believe WINDOWS_SIGNTOOL_PATH to not work is because the other parameters (subject name & certificate sha) are not being passed by e-b as its relying on handling signing itself but fails to do so.

@t3chguy
Copy link
Contributor Author

t3chguy commented Feb 7, 2025

I've had to roll back to get code signing working again which is a shame as it means I lost ARM64 support. I will set up a better environment to test including code signing before the next e-b upgrade -= element-hq/element-desktop#2131 should be that

@beyondkmp
Copy link
Collaborator

@t3chguy Do you use a custom sign function?

@beyondkmp
Copy link
Collaborator

if CSC_LINK is used, electron-builder does have unit tests (UT) to ensure that signing can pass.

test.ifAll.ifNotCiMac(
  "Squirrel.Windows",
  app(
    {
      targets: Platform.WINDOWS.createTarget(["squirrel"]),
      config: {
        win: {
          compression: "normal",
        },
        executableName: " test with spaces",
        electronFuses: {
          runAsNode: true,
          enableCookieEncryption: true,
          enableNodeOptionsEnvironmentVariable: true,
          enableNodeCliInspectArguments: true,
          enableEmbeddedAsarIntegrityValidation: true,
          onlyLoadAppFromAsar: true,
          loadBrowserProcessSpecificV8Snapshot: true,
          grantFileProtocolExtraPrivileges: undefined, // unsupported on current electron version in our tests
        },
      },
    },
    { signedWin: true }
  )
)

Image

@t3chguy
Copy link
Contributor Author

t3chguy commented Feb 8, 2025

@t3chguy Do you use a custom sign function?

No, we pass signtool options of subject Name and sha1. We also specify the signtool executable via environment as we need a modern one which is compatible with the eSignerCKA

@beyondkmp
Copy link
Collaborator

This issue can be reproduced in the GitHub Action. However, signing works fine locally on Windows 11, and the problem cannot be reproduced there. The machines in GitHub Actions run on Windows Server 2022. It's still unclear why it fails in GitHub Actions, and it will take some time to investigate the cause.

@t3chguy
Copy link
Contributor Author

t3chguy commented Feb 8, 2025

The fact that the vendored signtool is being executed rather than the one specified in SIGNTOOL_PATH is issue enough. Based on the debug logs looks like the sign hook isn't being passed due to the if-statement not triggering.

@beyondkmp
Copy link
Collaborator

However, when I tested it locally, I was able to use the electron-builder sign function without any issues. From the error logs, it seems that the process did enter the electron-builder sign function, but the failure was caused by an issue with the win option parameter at the end.

@t3chguy
Copy link
Contributor Author

t3chguy commented Feb 8, 2025

@beyondkmp if you look at https://github.com/element-hq/element-desktop/actions/runs/13204321507/job/36863804553#step:17:599

...
• signing with signtool.exe  path=dist\Element Nightly 0.0.1-nightly.2025020701.msi
  • signing         file=dist\Element Nightly 0.0.1-nightly.2025020701.msi subject=OID.1.3.6.1.4.1.311.60.2.1.3=GB, OID.2.5.4.15=Private Organization, CN=NEW VECTOR LTD, SERIALNUMBER=10873661, O=NEW VECTOR LTD, L=London, C=GB thumbprint=ED8CD4DF9BD2ED1B68CD98F28333B33E755BB911 store=My user=current user
  • executing       file=C:/Program Files (x86)/Windows Kits/10/bin/10.0.22000.0/x86/signtool.exe args=sign /tr http://timestamp.digicert.com/ /sha1 ED8CD4DF9BD2ED1B68CD98F28333B33E755BB911 /s My /fd sha256 /td sha256 /d Element Nightly /du https://element.io/ /debug D:\a\element-desktop\element-desktop\dist\Element Nightly 0.0.1-nightly.2025020701.msi env={}
  • executed        file=C:/Program Files (x86)/Windows Kits/10/bin/10.0.22000.0/x86/signtool.exe stdout=
The following certificates were considered:
    Issued to: NEW VECTOR LTD

    Issued by: SSL.com EV Code Signing Intermediate CA RSA R3

    Expires:   Mon Mar 16 15:47:01 2026

    SHA1 hash: ED8CD4DF9BD2ED1B68CD98F28333B33E755BB911

                      After EKU filter, 1 certs were left.
After expiry filter, 1 certs were left.
After Hash filter, 1 certs were left.
After Private Key filter, 1 certs were left.
The following certificate was selected:
    Issued to: NEW VECTOR LTD

    Issued by: SSL.com EV Code Signing Intermediate CA RSA R3

    Expires:   Mon Mar 16 15:47:01 2026

    SHA1 hash: ED8CD4DF9BD2ED1B68CD98F28333B33E755BB911

                      Done Adding Additional Store
Successfully signed: D:\a\element-desktop\element-desktop\dist\Element Nightly 0.0.1-nightly.2025020701.msi

                      Number of files successfully Signed: 1

Number of warnings: 0

Number of errors: 0


  ⨯ Failed with exit code: 4294967295
Output:
System.AggregateException: One or more errors occurred. ---> System.Exception: Failed to sign, command invoked was: 'D:\a\element-desktop\element-desktop\node_modules\electron-winstaller\vendor\signtool.exe sign windows-sign C:\Users\runneradmin\AppData\Local\SquirrelTemp\tempa\lib\net45\Element Nightly_ExecutionStub.exe'
   at Squirrel.Update.Program.<signPEFile>d__17.MoveNext()
--- End of stack trace from previous location where exception was thrown ---
...

You can see a successful electron-builder sign using C:/Program Files (x86)/Windows Kits/10/bin/10.0.22000.0/x86/signtool.exe on dist\Element Nightly 0.0.1-nightly.2025020701.msi
Followed by a failure using D:\a\element-desktop\element-desktop\node_modules\electron-winstaller\vendor\signtool.exe on C:\Users\runneradmin\AppData\Local\SquirrelTemp\tempa\lib\net45\Element Nightly_ExecutionStub.exe

From the error logs, it seems that the process did enter the electron-builder sign function

It looks like for squirrel signing this function is not entered as there's no signing log block for it.

@beyondkmp
Copy link
Collaborator

https://github.com/electron/windows-installer/blob/e722a519f8625c939087de2602b2b5b594886bc4/src/sign.ts#L13

Based on the source code of the Windows installer, if the windowsSign parameter is set, it will replace the signtool.exe in the vendor directory with a fake sign tool to support this parameter. Currently, it seems that the replacement did not succeed on GitHub Actions, and the real signtool.exe is still being used, which does not support the windowsSign parameter, leading to the signing failure.

In my local tests, it succeeded because everything was on the same C drive. However, I noticed that on GitHub Actions, the data and node_modules are on different drives — one on the C drive and the other on the D drive. This difference might be the cause of the issue.

@t3chguy
Copy link
Contributor Author

t3chguy commented Feb 8, 2025

@beyondkmp where are you seeing the fake sign tool? I see it calling an earlier code path if hookFunction is passed: https://github.com/electron/windows-sign/blob/main/src/sign.ts#L37-L41

@t3chguy
Copy link
Contributor Author

t3chguy commented Feb 8, 2025

This looks to be similar if not the same as electron/windows-installer#404 & more concretely electron/windows-installer#508

Definitely seems like the bug is upstream of e-b but it is a monumental bug to be shipping either way. I'm going to look into a patch-package fix workaround in the coming week, as I do want the other benefits of the major upgrade.

@beyondkmp
Copy link
Collaborator

@t3chguy electron/windows-sign#12 (comment)

It seems that hookFunction cannot be used, and we can only use hookModulePath, which is more troublesome.

@t3chguy
Copy link
Contributor Author

t3chguy commented Feb 10, 2025

@beyondkmp what's the plan then? to switch to hookModulePath or maybe to try fix hookFunction upstream to work correctly given its more versatile?

@mmaietta
Copy link
Collaborator

I personally we should fix upstream, as I'm getting more involved with working with the electron wg-ecosystem group so I just need to understand how to set up an env for this issue to debug further locally.

Is there a minimum repro repo for this or do I need to use the full project you're using, the latter will prob be more difficult to set up on my arm64 Windows VM, so just wondering if we can consolidate the work to a minimum repro. Seems like your signing env might be something we'll need to repro locally with though?

@t3chguy
Copy link
Contributor Author

t3chguy commented Feb 10, 2025

I have eSignerCKA working with demo credentials so it should be easier to spin up a minimum repro, but I'm currently sick and have severe brain fog so don't think I'll be able to get you a minimum repro.

  1. electron-builder 26.0.6 project with modern electron
  2. Workflow like https://github.com/element-hq/element-desktop/blob/t3chguy/test-windows-signing/.github/workflows/build_windows.yaml#L120-L161
  3. ENV ED_SIGNTOOL_THUMBPRINT & ED_SIGNTOOL_SUBJECT_NAME need passing to electron-builder via something like https://github.com/element-hq/element-desktop/blob/develop/electron-builder.ts#L185-L186

Hope that helps @mmaietta

@mmaietta
Copy link
Collaborator

@t3chguy I'll try to test that out locally. Best wishes on getting better and feeling healthy soon!

@t3chguy
Copy link
Contributor Author

t3chguy commented Feb 10, 2025

@mmaietta oh almost forgot, we tell electron-builder to use the system signtool explicitly https://github.com/element-hq/element-desktop/blob/t3chguy/test-windows-signing/.github/workflows/build_windows.yaml#L39 as eSignerCKA needs a fairly modern signtool

@mmaietta
Copy link
Collaborator

Weird, I get 404 - page not found. Cannot find a valid ref in <path> with the links

@mmaietta
Copy link
Collaborator

It seems that hookFunction cannot be used, and we can only use hookModulePath, which is more troublesome.

Wait, I don't understand this. Looking at the code, it should just execute our hook, no additional logic
https://github.com/electron/windows-sign/blob/2419f57fb9ce432078855e3de6f610400e8588fd/src/sign-with-hook.ts#L37-L47

@beyondkmp
Copy link
Collaborator

beyondkmp commented Feb 12, 2025

It seems that hookFunction cannot be used, and we can only use hookModulePath, which is more troublesome.

Wait, I don't understand this. Looking at the code, it should just execute our hook, no additional logic https://github.com/electron/windows-sign/blob/2419f57fb9ce432078855e3de6f610400e8588fd/src/sign-with-hook.ts#L37-L47

The windows-installer does not directly use the window-sign sign method for signing. Instead, it uses the getCreateSeaSignTool method(https://github.com/electron/windows-installer/blob/e722a519f8625c939087de2602b2b5b594886bc4/src/sign.ts#L74-L89), which first replaces the original signtool.exe with node.exe. This allows the execution of JavaScript code, and then it uses Squirrel.Windows to run node.exe for signing.

Inside sea.ts, the windowsSign is directly serialized into JSON(https://github.com/electron/windows-sign/blob/2419f57fb9ce432078855e3de6f610400e8588fd/src/sea.ts#L107). If a hookFunction is passed, this JSON serialization will fail, causing the issue. However, if a hookModulePath is passed, it can be serialized into JSON, and the code inside it can be executed.

Image

@mmaietta
Copy link
Collaborator

@t3chguy can you try out this patch-package and see if it resolves your issue?
electron-builder-squirrel-windows+26.0.6.patch

diff --git a/node_modules/electron-builder-squirrel-windows/out/SquirrelWindowsTarget.js b/node_modules/electron-builder-squirrel-windows/out/SquirrelWindowsTarget.js
index 15312f4..07e8c52 100644
--- a/node_modules/electron-builder-squirrel-windows/out/SquirrelWindowsTarget.js
+++ b/node_modules/electron-builder-squirrel-windows/out/SquirrelWindowsTarget.js
@@ -93,24 +93,26 @@ class SquirrelWindowsTarget extends app_builder_lib_1.Target {
         const appInfo = packager.appInfo;
         // If not specified will use the Squirrel.Windows that is shipped with electron-installer(https://github.com/electron/windows-installer/tree/main/vendor)
         // After https://github.com/electron-userland/electron-builder-binaries/pull/56 merged, will add `electron-builder-binaries` to get the latest version of squirrel.
-        let vendorDirectory = this.options.customSquirrelVendorDir;
+        let vendorDirectory = this.options.customSquirrelVendorDir || path.join(require.resolve("electron-winstaller/package.json"), "..", "vendor");
         if ((0, builder_util_1.isEmptyOrSpaces)(vendorDirectory) || !fs.existsSync(vendorDirectory)) {
             builder_util_1.log.warn({ vendorDirectory }, "unable to access Squirrel.Windows vendor directory, falling back to default electron-winstaller");
-            vendorDirectory = undefined;
+            vendorDirectory = path.join(require.resolve("electron-winstaller/package.json"), "..", "vendor");
         }
         const options = {
             appDirectory: this.appDirectory,
             outputDirectory: this.outputDirectory,
             name: this.options.useAppIdAsId ? appInfo.id : this.appName,
+            title: appInfo.productName || appInfo.name,
             version: appInfo.version,
             description: appInfo.description,
-            exe: `${this.packager.platformSpecificBuildOptions.executableName || this.options.name || appInfo.productName}.exe`,
+            exe: `${appInfo.productFilename || this.options.name || appInfo.productName}.exe`,
             authors: appInfo.companyName || "",
             iconUrl,
             copyright: appInfo.copyright,
             vendorDirectory,
             nuspecTemplate: path.join(__dirname, "..", "template.nuspectemplate"),
             noMsi: !this.options.msi,
+            usePackageJson: false,
         };
         const projectUrl = await appInfo.computePackageUrl();
         if (projectUrl != null) {
@@ -121,13 +123,20 @@ class SquirrelWindowsTarget extends app_builder_lib_1.Target {
             await (0, promises_1.writeFile)(nuspecTemplate, templateContent);
             options.nuspecTemplate = nuspecTemplate;
         }
-        if (await (await packager.signingManager.value).cscInfo.value) {
-            options.windowsSign = {
-                hookFunction: async (file) => {
-                    await packager.sign(file);
-                },
-            };
+        const tmpVendorDirectory = await packager.info.tempDirManager.createTempDir({ prefix: "squirrel-windows-vendor" });
+        // Copy entire vendor directory to temp directory
+        await fs.promises.cp(vendorDirectory, tmpVendorDirectory, { recursive: true });
+        builder_util_1.log.debug({ from: vendorDirectory, to: tmpVendorDirectory }, "copied vendor directory");
+        // Find and sign all executables in the temp vendor directory
+        const files = await fs.promises.readdir(tmpVendorDirectory);
+        for (const file of files) {
+            if (file.endsWith(".exe") || file.endsWith(".dll")) {
+                const filePath = path.join(tmpVendorDirectory, file);
+                builder_util_1.log.debug({ file: filePath }, "signing vendor executable");
+                await packager.sign(filePath);
+            }
         }
+        options.vendorDirectory = tmpVendorDirectory;
         if ((0, builder_util_1.isEmptyOrSpaces)(options.description)) {
             options.description = this.options.name || appInfo.productName;
         }

@t3chguy
Copy link
Contributor Author

t3chguy commented Feb 12, 2025

@mmaietta https://github.com/element-hq/element-desktop/actions/runs/13290420431/job/37109610542?pr=2132#step:17:1456

...
The following certificate was selected:
    Issued to: Esigner LLC

    Issued by: SSL.com EV Code Signing Intermediate CA RSA R2

    Expires:   Sat Jun 28 17:00:31 2025

    SHA1 hash: 247D3EE6F29D11968B33B29A23063F3721ADC3E4

                      Done Adding Additional Store
Successfully signed: C:\Users\RUNNER~1\AppData\Local\Temp\t-7bl6yJ\squirrel-windows-vendor-3\WriteZipToSetup.exe

                      Number of files successfully Signed: 1

Number of warnings: 0

Number of errors: 0


  • async task error  error=ENOENT: no such file or directory, copyfile 'C:\Users\RUNNER~1\AppData\Local\Temp\t-7bl6yJ\squirrel-windows-vendor-3\7z--ia32.exe' -> 'C:\Users\RUNNER~1\AppData\Local\Temp\t-7bl6yJ\squirrel-windows-vendor-3\7z.exe'
  ⨯ ENOENT: no such file or directory, copyfile 'C:\Users\RUNNER~1\AppData\Local\Temp\t-7bl6yJ\squirrel-windows-vendor-3\7z--ia32.exe' -> 'C:\Users\RUNNER~1\AppData\Local\Temp\t-7bl6yJ\squirrel-windows-vendor-3\7z.exe'  failedTask=build stackTrace=Error: ENOENT: no such file or directory, copyfile 'C:\Users\RUNNER~1\AppData\Local\Temp\t-7bl6yJ\squirrel-windows-vendor-3\7z--ia32.exe' -> 'C:\Users\RUNNER~1\AppData\Local\Temp\t-7bl6yJ\squirrel-windows-vendor-3\7z.exe'
    at Object.copyFileSync (node:fs:3086:11)
    at SquirrelWindowsTarget.select7zipArch (D:\a\element-desktop\element-desktop\node_modules\electron-builder-squirrel-windows\src\SquirrelWindowsTarget.ts:90:8)
    at SquirrelWindowsTarget.build (D:\a\element-desktop\element-desktop\node_modules\electron-builder-squirrel-windows\src\SquirrelWindowsTarget.ts:46:12)
    at async Promise.all (index 0)
    at AsyncTaskManager.awaitTasks (D:\a\element-desktop\element-desktop\node_modules\builder-util\src\asyncTaskManager.ts:65:25)
    at Packager.doBuild (D:\a\element-desktop\element-desktop\node_modules\app-builder-lib\src\packager.ts:475:5)
    at executeFinally (D:\a\element-desktop\element-desktop\node_modules\builder-util\src\promise.ts:12:14)
    at Packager.build (D:\a\element-desktop\element-desktop\node_modules\app-builder-lib\src\packager.ts:393:31)
    at executeFinally (D:\a\element-desktop\element-desktop\node_modules\builder-util\src\promise.ts:12:14)
error Command failed with exit code 1.
info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command.
Error: Process completed with exit code 1.

@t3chguy
Copy link
Contributor Author

t3chguy commented Feb 12, 2025

Though it did seemingly pass on the x64 build https://github.com/element-hq/element-desktop/actions/runs/13290932109/job/37111358677?pr=2132 - so the other error may be yet another major issue 🙈

I loaded up the x64 zip from github actions into a Windows box and:

Image

Image

Looks like the Squirrel Setup file is missing signature

Image

vs one made using older e-b

Image

Also looks like the icon of the Setup file is incorrect, and the name is just Setup rather than Element Setup <version>.exe which is also concerning.

Concerningly, I also see a bunch of things getting signed which are not part of the package but rather look like build/packaging utilities:

Image

eSigner costs per-signature so signing additional files would increase expenditure significantly given we do at least 1 build a day across 2 architectures and want to add arm64.

Image

@mmaietta
Copy link
Collaborator

Oomph, yeah, we definitely can't cause that impact on signing costs. Thanks for calling it out!

CC @beyondkmp looks like we need an alternative way to handle this signing logic and/or open a PR upstream for allowing more custom logic flows (e.g. Azure signing) that can't leverage electron/windows-sign package

@beyondkmp
Copy link
Collaborator

@mmaietta electron/windows-installer#548

However, based on my investigation, under the current architecture, implementing hookFunction in the upstream is not feasible. Moreover, Electron Forge also only supports hookModulePath.

@t3chguy
Copy link
Contributor Author

t3chguy commented Feb 18, 2025

@beyondkmp & @mmaietta thank you for persevering with this, great work.

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