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

Squirrel-packed executable not signed #449

Closed
ghost opened this issue May 30, 2016 · 15 comments · May be fixed by qcif/data-curator#563
Closed

Squirrel-packed executable not signed #449

ghost opened this issue May 30, 2016 · 15 comments · May be fixed by qcif/data-curator#563

Comments

@ghost
Copy link

ghost commented May 30, 2016

Previously, my "App.exe" that was installed by Squirrel (%APPDATA/Local/$myapp/$myapp-$version/$app.exe) was signed. Now, on 4.1.0, only the Squirrel installer is signed.

I believe this happened between 3.25.0 and 4.1.0.

@ghost
Copy link
Author

ghost commented May 30, 2016

Note: I am only building an Windows ia32 version.

@ghost
Copy link
Author

ghost commented May 30, 2016

Just did some testing:

3.16.1: works
3.19.0: works
3.20.0: broken
3.22.1: broken

Looks like it was actually 3.20.0 that broke it.

@ghost
Copy link
Author

ghost commented May 31, 2016

Running my build script with DEBUG=*, I see the following:

4.1.0:

Tue, 31 May 2016 13:56:55 GMT electron-windows-installer Executing C:\Users\user\Desktop\app\node_modules\electron-winstaller-fixed\vendor\Update.com --releasify C:\Users\user\Desktop\app\dist\win-ia32\app-0.1.0-full.nupkg --releaseDir C:\Users\user\Desktop\app\win-ia32

3.19.0:

Tue, 31 May 2016 13:59:49 GMT electron-windows-installer Spawning C:\Users\user\Desktop\app\node_modules\electron-winstaller-fixed\vendor\Update.com --releasify C:\Users\user\Desktop\app\dist\win\in.nupkg --releaseDir C:\Users\user\Desktop\app\dist\win --loadingGif C:\Users\user\Desktop\app\build\loading.gif --signWithParams /a /f "C:\Users\user\Desktop\app\build\cert.p12" /p "password" --setupIcon C:\Users\user\Desktop\app\build\icon.ico --no-msi

Is --signWithParams suppose to be missing? Are we expected to sign our binaries before the installer is built?

@develar
Copy link
Member

develar commented May 31, 2016

Reproduced Works for me.

Is --signWithParams suppose to be missing

Yes. We sign on our side.

@ghost
Copy link
Author

ghost commented May 31, 2016

Yes. We sign on our side.

Can you point me to where is this done? Perhaps my executable isn't being matched for some reason.

@ghost
Copy link
Author

ghost commented May 31, 2016

I just checked my dist/win-ia32-unpacked directory: App.exe is not signed, but Update.exe is.

@develar
Copy link
Member

develar commented May 31, 2016

@bontibon 4.2 will be released in 10-30 minutes. I hope your issue is fixed in this version.
screen shot 2016-05-31 at 17 00 44

@ghost
Copy link
Author

ghost commented May 31, 2016

@develar That screenshot shows that the setup is signed, which is not my issue. Can you confirm that dist/win-ia32-unpacked/TestApp.exe is signed?

@develar
Copy link
Member

develar commented May 31, 2016

Can you confirm that dist/win-ia32-unpacked/TestApp.exe is signed

Yes, please wait 4.2 and we will continue investigation.

@ghost ghost mentioned this issue May 31, 2016
@develar
Copy link
Member

develar commented Jun 2, 2016

Please try 4.2.0 version.

@ghost
Copy link
Author

ghost commented Jun 2, 2016

Just tested with 4.2.0:

  • win-ia32/App0.1.0-ia32.exe: Signed
  • win-ia32-unpacked/Update.exe: Signed
  • win-ia32-unpacked/App.exe: Not signed

However, I was using devMetadata.build.win.certificateFile and devMetadata.build.win.certificatePassword. I switched to cscLink and cscKeyPassword and now everything is signed correctly. Thanks!

It was very unexpected that certificateFile and certificatePassword still worked to sign the installer and updater, but not the application. Is there a reason for that?

@develar
Copy link
Member

develar commented Jun 2, 2016

Is there a reason for that?

Just bug. Thanks for investigation.

certificateFile

Please answer to "Please use CSC_LINK (you can set it to base64 data of file). If it is not suitable for you, we can add support for file param (but! on OS X we use keychain and, maybe, on Windows also there is such system store?)."

I am OS X user and it is unclear for me – should we support certificateFile or CSC_LINK env is enough? (no, I am not joking, just want to understand).

@develar develar added the bug label Jun 2, 2016
@ghost
Copy link
Author

ghost commented Jun 2, 2016

should we support certificateFile or CSC_LINK env is enough

I preferred certificateFile. Because I use the electron-builder API via a build script, I now need to first read my local certificate into a buffer, convert it to base64, then set cscLink in the build options.

Maybe support file://... in CSC_LINK?

@ghost
Copy link
Author

ghost commented Jun 2, 2016

For clarification, I mean that CSC_LINK and CSC_KEY_PASSWORD should be the ONLY way to sign applications. All code that uses devMetadata.build.win.certificateFile and devMetadata.build.win.certificatePassword should be removed.

@develar
Copy link
Member

develar commented Jun 5, 2016

I don't want to break existing projects — we must deprecate in one major release and remove in the next. For now certificateFile and certificatePassword will be fully supported.

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