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

Make Windows builds using a new ssl.com code signing certificate #3050

Merged
merged 9 commits into from
Apr 23, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
41 changes: 37 additions & 4 deletions .github/actions/build-zui/action.yml
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,13 @@ inputs:
required: true

# Windows Inputs
csc_key_password:
ssl_com_username:
required: true
csc_link:
ssl_com_password:
required: true
ssl_com_totp_secret:
required: true
ssl_com_credential_id:
required: true

# Mac Inputs
Expand Down Expand Up @@ -47,16 +51,45 @@ runs:
security find-identity -p codesigning -v
shell: bash

- name: Checkout esigner-codesign repository
if: runner.os == 'Windows'
uses: actions/checkout@v3
with:
repository: 'SSLcom/esigner-codesign'
path: esigner-codesign

- name: Make values from package.json available in Actions steps
id: zui-package
uses: RadovanPelka/github-action-json@v1.0.1
with:
path: apps/zui/package.json
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added this because one of the env variables to be passed to the esigner tool is the expected filename of what would be signed, and that filename includes both the app name (Zui or Zui Insiders) and the version string, both of which are in package.json.


- name: Build & Publish
run: ${{ inputs.cmd }}
shell: bash
env:
GH_TOKEN: ${{ inputs.gh_token }}
WIN_CSC_KEY_PASSWORD: ${{ inputs.csc_key_password }}
WIN_CSC_LINK: ${{ inputs.csc_link }}
APPLE_ID: ${{ inputs.apple_id }}
APPLE_ID_PASSWORD: ${{ inputs.apple_id_password }}
APPLE_TEAM_ID: ${{ inputs.apple_team_id }}
CODE_SIGN_SCRIPT_PATH: ${{ github.workspace }}/esigner-codesign/dist/index.js
INPUT_COMMAND: sign
INPUT_FILE_PATH: ${{ github.workspace }}/dist/apps/zui/${{ fromJSON(steps.zui-package.outputs.productName) }} Setup ${{ fromJSON(steps.zui-package.outputs.version) }}.exe
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The fromJSON() wrappers are needed because the Action I added above to pull values from the package.json delivers strings with quotes around them, and when these remained intact the esigner tool interpreted them as parts of the filename and choked.

INPUT_OVERRIDE: true
INPUT_MALWARE_BLOCK: false
INPUT_CLEAN_LOGS: false
INPUT_JVM_MAX_MEMORY: 1024M
INPUT_ENVIRONMENT_NAME: PROD
INPUT_USERNAME: ${{ inputs.ssl_com_username }}
INPUT_PASSWORD: ${{ inputs.ssl_com_password }}
INPUT_TOTP_SECRET: ${{ inputs.ssl_com_totp_secret }}
INPUT_CREDENTIAL_ID: ${{ inputs.ssl_com_credential_id }}

- name: Check for successful signing with SignTool
if: runner.os == 'Windows'
run: |
"C:\Program Files (x86)\Microsoft SDKs\ClickOnce\SignTool\signtool.exe" verify /pa "${{ github.workspace }}/dist/apps/zui/${{ fromJSON(steps.zui-package.outputs.productName) }} Setup ${{ fromJSON(steps.zui-package.outputs.version) }}.exe"
shell: cmd
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

While getting the esigner tool working with electron-builder, one buggy-seeming thing I encountered is that even if the custom signing code returns false due to an error, the electron-builder apparently still behaved as if it succeeded. Here's an example run where you can see what happens: At first it looks green/successful, but if you expand the "Build Zui" step and scroll to the bottom you can see the red failure message. Bad, right?

This would have made it likely that we could one day be producing unsigned builds and not be aware. I found via web searches that this signtool verify /pa approach adequately sniffs out if an executable is, indeed, signed. I tested it out and it does what we need (example run catching an unsigned build).


- name: Check notorization with gatekeeper
if: runner.os == 'macOS'
Expand Down
6 changes: 4 additions & 2 deletions .github/workflows/build-insiders.yml
Original file line number Diff line number Diff line change
Expand Up @@ -56,8 +56,10 @@ jobs:
cmd: yarn nx package-insiders zui
gh_token: ${{ secrets.PAT_TOKEN }}
# Windows
csc_key_password: ${{ secrets.WINDOWS_SIGNING_PASSPHRASE }}
csc_link: ${{ secrets.WINDOWS_SIGNING_PFX_BASE64 }}
ssl_com_username: ${{ secrets.WINDOWS_SIGNING_SSL_COM_USERNAME }}
ssl_com_password: ${{ secrets.WINDOWS_SIGNING_SSL_COM_PASSWORD }}
ssl_com_totp_secret: ${{ secrets.WINDOWS_SIGNING_SSL_COM_TOTP_SECRET }}
ssl_com_credential_id: ${{ secrets.WINDOWS_SIGNING_SSL_COM_CREDENTIAL_ID }}
# Mac
apple_id: ${{ secrets.APPLEID_USER }}
apple_id_password: ${{ secrets.APPLEID_PASSWORD }}
Expand Down
6 changes: 4 additions & 2 deletions .github/workflows/build.yml
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,10 @@ jobs:
cmd: yarn nx package-zui zui
gh_token: ${{ secrets.GITHUB_TOKEN }}
# Windows
csc_key_password: ${{ secrets.WINDOWS_SIGNING_PASSPHRASE }}
csc_link: ${{ secrets.WINDOWS_SIGNING_PFX_BASE64 }}
ssl_com_username: ${{ secrets.WINDOWS_SIGNING_SSL_COM_USERNAME }}
ssl_com_password: ${{ secrets.WINDOWS_SIGNING_SSL_COM_PASSWORD }}
ssl_com_totp_secret: ${{ secrets.WINDOWS_SIGNING_SSL_COM_TOTP_SECRET }}
ssl_com_credential_id: ${{ secrets.WINDOWS_SIGNING_SSL_COM_CREDENTIAL_ID }}
# Mac
apple_id: ${{ secrets.APPLEID_USER }}
apple_id_password: ${{ secrets.APPLEID_PASSWORD }}
Expand Down
6 changes: 4 additions & 2 deletions .github/workflows/release-insiders.yml
Original file line number Diff line number Diff line change
Expand Up @@ -56,8 +56,10 @@ jobs:
cmd: yarn nx release-insiders zui
gh_token: ${{ secrets.PAT_TOKEN }}
# Windows
csc_key_password: ${{ secrets.WINDOWS_SIGNING_PASSPHRASE }}
csc_link: ${{ secrets.WINDOWS_SIGNING_PFX_BASE64 }}
ssl_com_username: ${{ secrets.WINDOWS_SIGNING_SSL_COM_USERNAME }}
ssl_com_password: ${{ secrets.WINDOWS_SIGNING_SSL_COM_PASSWORD }}
ssl_com_totp_secret: ${{ secrets.WINDOWS_SIGNING_SSL_COM_TOTP_SECRET }}
ssl_com_credential_id: ${{ secrets.WINDOWS_SIGNING_SSL_COM_CREDENTIAL_ID }}
# Mac
apple_id: ${{ secrets.APPLEID_USER }}
apple_id_password: ${{ secrets.APPLEID_PASSWORD }}
Expand Down
6 changes: 4 additions & 2 deletions .github/workflows/release.yml
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,10 @@ jobs:
cmd: yarn nx release-zui zui
gh_token: ${{ secrets.GITHUB_TOKEN }}
# Windows
csc_key_password: ${{ secrets.WINDOWS_SIGNING_PASSPHRASE }}
csc_link: ${{ secrets.WINDOWS_SIGNING_PFX_BASE64 }}
ssl_com_username: ${{ secrets.WINDOWS_SIGNING_SSL_COM_USERNAME }}
ssl_com_password: ${{ secrets.WINDOWS_SIGNING_SSL_COM_PASSWORD }}
ssl_com_totp_secret: ${{ secrets.WINDOWS_SIGNING_SSL_COM_TOTP_SECRET }}
ssl_com_credential_id: ${{ secrets.WINDOWS_SIGNING_SSL_COM_CREDENTIAL_ID }}
# Mac
apple_id: ${{ secrets.APPLEID_USER }}
apple_id_password: ${{ secrets.APPLEID_PASSWORD }}
Expand Down
73 changes: 73 additions & 0 deletions apps/zui/electron-builder-config.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,73 @@
const { execSync } = require('child_process');
const zuiPackage = require('./package.json')

const config = {
appId: "io.brimdata.zui",
asar: true,
asarUnpack: ["zdeps", "LICENSE.txt", "acknowledgments.txt", "**/*.node"],
directories: {output: "../../dist/apps/zui"},
protocols: [{name: "zui", "schemes": ["zui"]}],
win: {target: ["nsis"]},
linux: {target: ["deb", "rpm"]},
rpm: {depends: ["openssl"]},
deb: {depends: ["openssl"]},
nsis: {oneClick: false, perMachine: false},
forceCodeSigning: true,
afterSign: "electron-builder-notarize",
publish: {
provider: "github"
},
files: [
"dist/**",
"out/**",
"build/**",
"zdeps/**",
"LICENSE.txt",
"acknowledgments.txt",
"package.json"
],
}

// Code below for code signing with SSL.com cert in electron-builder via GitHub
// Actions taken from:
// https://github.com/electron-userland/electron-builder/issues/6158#issuecomment-1994110062
if (process.env.CODE_SIGN_SCRIPT_PATH) {
const version = zuiPackage.version;
Copy link
Contributor Author

@philrz philrz Apr 17, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The code I borrowed from the GitHub issue was doing this via:

const version = execSync('node -p "require(\'./package.json\').version"').toString().trim();

I don't know why he didn't opt to just require the package.json and access the value directly like I'm doing there. My approach feels cleaner, so I'm going with it. But I figured I'd point it out in case I'm missing something subtle.

const productName = zuiPackage.productName;
const versionedExe = `${productName} Setup ${version}.exe`;

config.win.sign = (configuration) => {
console.log("Requested signing for ", configuration.path);

// Only proceed if the versioned exe file is in the configuration path - skip signing everything else
if (!configuration.path.includes(versionedExe)) {
console.log("Configuration path does not include the versioned exe, signing skipped.");
return true;
}

const scriptPath = process.env.CODE_SIGN_SCRIPT_PATH;

try {
// Execute the sign script synchronously
const output = execSync(`node "${scriptPath}"`).toString();
console.log(`Script output: ${output}`);
} catch (error) {
console.error(`Error executing script: ${error.message}`);
if (error.stdout) {
console.log(`Script stdout: ${error.stdout.toString()}`);
}
if (error.stderr) {
console.error(`Script stderr: ${error.stderr.toString()}`);
}
return false;
}

return true; // Return true at the end of successful signing
};

// Sign only for Windows 10 and above
config.win.signingHashAlgorithms = ["sha256"];

}

module.exports = config;
16 changes: 16 additions & 0 deletions apps/zui/electron-builder-insiders-config.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
const config = {
extends: "./electron-builder-config.js",
appId: "io.brimdata.zui-insiders",
mac: {
icon: "build/insiders/icon.icns"
},
win: {
icon: "build/insiders/icon.ico"
},
publish: {
provider: "github",
releaseType: "release"
}
}

module.exports = config;
14 changes: 0 additions & 14 deletions apps/zui/electron-builder-insiders.json

This file was deleted.

25 changes: 0 additions & 25 deletions apps/zui/electron-builder.json

This file was deleted.

8 changes: 4 additions & 4 deletions apps/zui/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -27,10 +27,10 @@
"tsc": "tsc",
"postinstall": "node scripts/post-install",
"prepare": "husky install",
"package-zui": "electron-builder --publish never",
"release-zui": "electron-builder",
"package-insiders": "electron-builder --config electron-builder-insiders.json --publish never",
"release-insiders": "electron-builder --config electron-builder-insiders.json --publish always"
"package-zui": "electron-builder --config electron-builder-config.js --publish never",
"release-zui": "electron-builder --config electron-builder-config.js",
"package-insiders": "electron-builder --config electron-builder-insiders-config.js --publish never",
"release-insiders": "electron-builder --config electron-builder-insiders-config.js --publish always"
},
"dependencies": {
"keytar": "^7.7.0",
Expand Down
Loading