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

win,tools: upgrade Windows signing to smctl #50956

Merged
merged 1 commit into from
Dec 8, 2023

Conversation

StefanStojanovic
Copy link
Contributor

@StefanStojanovic StefanStojanovic commented Nov 28, 2023

This PR introduces a new signing process on Windows based on new requirements for it to be valid. It relies on DigiCert's cloud HSM service called KeyLocker and the new certificate stored there.

The signing tool is changed (in a way) from signtool to smctl. That is a DigiCert client-side tool for various operations, one of which is signing files. Under the hood, smctl calls signtool with the key from KeyLocker, so in its essence, the signing tool is still the same. The decision to use smctl instead of signtool directly came down to it being part of the mandatory DigiCert client-side tools and needing less configuring and simpler command to run.

There is a release CI job testing these changes and all 6 Windows release machines are prepared for signing with the new certificate. These changes should also be landed on all LTS versions (v18 and v20) for future releases. I will also update the docs, and write new ones where needed to describe the entire process after the process is over.

cc @nodejs/build @nodejs/platform-windows @nodejs/releasers
Fixes: nodejs/build#3491

@nodejs-github-bot nodejs-github-bot added needs-ci PRs that need a full CI run. tools Issues and PRs related to the tools directory. windows Issues and PRs related to the Windows platform. labels Nov 28, 2023
@StefanStojanovic StefanStojanovic force-pushed the mefi-new-cert branch 3 times, most recently from 5258981 to f318d9b Compare November 30, 2023 14:22
As a part of the new signing requrements for Windows change approach to
use the DigiCert cloud HSM service KeyLocker.
@StefanStojanovic
Copy link
Contributor Author

Pinging @mhdawson and @richardlau since they have the biggest context on this. The current certificate expires in ~2 weeks, so I think we should try and push the PR this week, and hopefully land changes in both LTS branches by the end of this week or early next week.

@richardlau richardlau added lts-watch-v18.x PRs that may need to be released in v18.x. lts-watch-v20.x PRs that may need to be released in v20.x labels Dec 5, 2023
@StefanStojanovic
Copy link
Contributor Author

I see this PR has the needs-ci label, but the change it introduces only runs in the release CI. Since from the test CI point of view, nothing has changed, does it make sense to request CI for this, or just land it as it is? Also worth mentioning that I tested in the release CI here.

@StefanStojanovic
Copy link
Contributor Author

Hey, @richardlau (pinging you because you approved the PR). Is there something else I should do before we land this?

Copy link
Member

@mhdawson mhdawson left a comment

Choose a reason for hiding this comment

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

LGTM

@mhdawson mhdawson added the fast-track PRs that do not need to wait for 48 hours to land. label Dec 8, 2023
Copy link
Contributor

github-actions bot commented Dec 8, 2023

Fast-track has been requested by @mhdawson. Please 👍 to approve.

@mhdawson mhdawson added the request-ci Add this label to start a Jenkins CI on a PR. label Dec 8, 2023
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Dec 8, 2023
@nodejs-github-bot
Copy link
Collaborator

@richardlau richardlau removed the fast-track PRs that do not need to wait for 48 hours to land. label Dec 8, 2023
@richardlau
Copy link
Member

PR has been opened for over a week so a fast-track is unnecessary (it can land as soon as the CI passes).

@nodejs-github-bot
Copy link
Collaborator

@nodejs-github-bot
Copy link
Collaborator

@nodejs-github-bot
Copy link
Collaborator

@richardlau richardlau added the commit-queue Add this label to land a pull request using GitHub Actions. label Dec 8, 2023
@nodejs-github-bot nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Dec 8, 2023
@nodejs-github-bot nodejs-github-bot merged commit 1ba508d into nodejs:main Dec 8, 2023
76 checks passed
@nodejs-github-bot
Copy link
Collaborator

Landed in 1ba508d

StefanStojanovic added a commit to JaneaSystems/node that referenced this pull request Dec 11, 2023
As a part of the new signing requrements for Windows change approach to
use the DigiCert cloud HSM service KeyLocker.

PR-URL: nodejs#50956
Fixes: nodejs/build#3491
Reviewed-By: Richard Lau <rlau@redhat.com>
Reviewed-By: Michael Dawson <midawson@redhat.com>
@richardlau richardlau mentioned this pull request Dec 14, 2023
UlisesGascon pushed a commit that referenced this pull request Dec 15, 2023
As a part of the new signing requrements for Windows change approach to
use the DigiCert cloud HSM service KeyLocker.

PR-URL: #50956
Fixes: nodejs/build#3491
Reviewed-By: Richard Lau <rlau@redhat.com>
Reviewed-By: Michael Dawson <midawson@redhat.com>
RafaelGSS pushed a commit that referenced this pull request Dec 15, 2023
As a part of the new signing requrements for Windows change approach to
use the DigiCert cloud HSM service KeyLocker.

PR-URL: #50956
Fixes: nodejs/build#3491
Reviewed-By: Richard Lau <rlau@redhat.com>
Reviewed-By: Michael Dawson <midawson@redhat.com>
UlisesGascon pushed a commit that referenced this pull request Dec 15, 2023
As a part of the new signing requrements for Windows change approach to
use the DigiCert cloud HSM service KeyLocker.

PR-URL: #50956
Fixes: nodejs/build#3491
Reviewed-By: Richard Lau <rlau@redhat.com>
Reviewed-By: Michael Dawson <midawson@redhat.com>
@RafaelGSS RafaelGSS mentioned this pull request Dec 15, 2023
UlisesGascon pushed a commit that referenced this pull request Dec 19, 2023
As a part of the new signing requrements for Windows change approach to
use the DigiCert cloud HSM service KeyLocker.

PR-URL: #50956
Fixes: nodejs/build#3491
Reviewed-By: Richard Lau <rlau@redhat.com>
Reviewed-By: Michael Dawson <midawson@redhat.com>
UlisesGascon pushed a commit that referenced this pull request Jan 9, 2024
As a part of the new signing requrements for Windows change approach to
use the DigiCert cloud HSM service KeyLocker.

PR-URL: #50956
Fixes: nodejs/build#3491
Reviewed-By: Richard Lau <rlau@redhat.com>
Reviewed-By: Michael Dawson <midawson@redhat.com>
richardlau pushed a commit that referenced this pull request Jan 12, 2024
As a part of the new signing requrements for Windows change approach to
use the DigiCert cloud HSM service KeyLocker.

PR-URL: #50956
Fixes: nodejs/build#3491
Reviewed-By: Richard Lau <rlau@redhat.com>
Reviewed-By: Michael Dawson <midawson@redhat.com>
@richardlau richardlau added backported-to-v18.x PRs backported to the v18.x-staging branch. backported-to-v20.x PRs backported to the v20.x-staging branch. and removed lts-watch-v18.x PRs that may need to be released in v18.x. lts-watch-v20.x PRs that may need to be released in v20.x labels Jan 16, 2024
RafaelGSS pushed a commit that referenced this pull request Feb 14, 2024
This is a security release.

Notable changes:

crypto:
  * update root certificates to NSS 3.95 (Node.js GitHub Bot) #50805
  * disable PKCS#1 padding for privateDecrypt (Michael Dawson) nodejs-private/node-private#525
deps:
  * upgrade npm to 10.2.4 (npm team) #50751
  * update archs files for openssl-3.0.13+quic1 (Node.js GitHub Bot) #51614
  * upgrade openssl sources to quictls/openssl-3.0.13+quic1 (Node.js GitHub Bot) ://github.com//pull/51614
  * fix GHSA-f74f-cvh7-c6q6/CVE-2024-24806 (Santiago Gimeno) #51614
http:
  * add maximum chunk extension size (Paolo Insogna) nodejs-private/node-private#520
lib:
  * update undici to v5.28.3 (Matteo Collina) nodejs-private/node-private#536
src:
  * fix HasOnly(capability) in node::credentials (Tobias Nießen) nodejs-private/node-private#505
test:
  * skip test-child-process-stdio-reuse-readable-stdio on Windows (Joyee Cheung) #49621
tools:
  * add macOS notarization verification step (Ulises Gascón) #50833
  * use macOS keychain to notarize the releases (Ulises Gascón) #50715
  * remove unused file (Ulises Gascon) #50622
  * add macOS notarization stapler (Ulises Gascón) #50625
  * improve macOS notarization process output readability (Ulises Gascón) #50389
  * remove unused `version` function (Ulises Gascón) #50390
win,tools:
  * upgrade Windows signing to smctl (Stefan Stojanovic) #50956
zlib:
  * pause stream if outgoing buffer is full (Matteo Collina) nodejs-private/node-private#542

PR-URL: nodejs-private/node-private#545
rdw-msft pushed a commit to rdw-msft/node that referenced this pull request Mar 20, 2024
This is a security release.

Notable changes:

crypto:
  * update root certificates to NSS 3.95 (Node.js GitHub Bot) nodejs#50805
  * disable PKCS#1 padding for privateDecrypt (Michael Dawson) https://github.com/nodejs-private/node-private/pull/525
deps:
  * upgrade npm to 10.2.4 (npm team) nodejs#50751
  * update archs files for openssl-3.0.13+quic1 (Node.js GitHub Bot) nodejs#51614
  * upgrade openssl sources to quictls/openssl-3.0.13+quic1 (Node.js GitHub Bot) ://github.com/nodejs/pull/51614
  * fix GHSA-f74f-cvh7-c6q6/CVE-2024-24806 (Santiago Gimeno) nodejs#51614
http:
  * add maximum chunk extension size (Paolo Insogna) https://github.com/nodejs-private/node-private/pull/520
lib:
  * update undici to v5.28.3 (Matteo Collina) https://github.com/nodejs-private/node-private/pull/536
src:
  * fix HasOnly(capability) in node::credentials (Tobias Nießen) https://github.com/nodejs-private/node-private/pull/505
test:
  * skip test-child-process-stdio-reuse-readable-stdio on Windows (Joyee Cheung) nodejs#49621
tools:
  * add macOS notarization verification step (Ulises Gascón) nodejs#50833
  * use macOS keychain to notarize the releases (Ulises Gascón) nodejs#50715
  * remove unused file (Ulises Gascon) nodejs#50622
  * add macOS notarization stapler (Ulises Gascón) nodejs#50625
  * improve macOS notarization process output readability (Ulises Gascón) nodejs#50389
  * remove unused `version` function (Ulises Gascón) nodejs#50390
win,tools:
  * upgrade Windows signing to smctl (Stefan Stojanovic) nodejs#50956
zlib:
  * pause stream if outgoing buffer is full (Matteo Collina) https://github.com/nodejs-private/node-private/pull/542

PR-URL: https://github.com/nodejs-private/node-private/pull/545
sercher added a commit to sercher/graaljs that referenced this pull request Apr 25, 2024
As a part of the new signing requrements for Windows change approach to
use the DigiCert cloud HSM service KeyLocker.

PR-URL: nodejs/node#50956
Fixes: nodejs/build#3491
Reviewed-By: Richard Lau <rlau@redhat.com>
Reviewed-By: Michael Dawson <midawson@redhat.com>
sercher added a commit to sercher/graaljs that referenced this pull request Apr 25, 2024
This is a security release.

Notable changes:

crypto:
  * update root certificates to NSS 3.95 (Node.js GitHub Bot) nodejs/node#50805
  * disable PKCS#1 padding for privateDecrypt (Michael Dawson) https://github.com/nodejs-private/node-private/pull/525
deps:
  * upgrade npm to 10.2.4 (npm team) nodejs/node#50751
  * update archs files for openssl-3.0.13+quic1 (Node.js GitHub Bot) nodejs/node#51614
  * upgrade openssl sources to quictls/openssl-3.0.13+quic1 (Node.js GitHub Bot) ://github.com/nodejs/node/pull/51614
  * fix GHSA-f74f-cvh7-c6q6/CVE-2024-24806 (Santiago Gimeno) nodejs/node#51614
http:
  * add maximum chunk extension size (Paolo Insogna) https://github.com/nodejs-private/node-private/pull/520
lib:
  * update undici to v5.28.3 (Matteo Collina) https://github.com/nodejs-private/node-private/pull/536
src:
  * fix HasOnly(capability) in node::credentials (Tobias Nießen) https://github.com/nodejs-private/node-private/pull/505
test:
  * skip test-child-process-stdio-reuse-readable-stdio on Windows (Joyee Cheung) nodejs/node#49621
tools:
  * add macOS notarization verification step (Ulises Gascón) nodejs/node#50833
  * use macOS keychain to notarize the releases (Ulises Gascón) nodejs/node#50715
  * remove unused file (Ulises Gascon) nodejs/node#50622
  * add macOS notarization stapler (Ulises Gascón) nodejs/node#50625
  * improve macOS notarization process output readability (Ulises Gascón) nodejs/node#50389
  * remove unused `version` function (Ulises Gascón) nodejs/node#50390
win,tools:
  * upgrade Windows signing to smctl (Stefan Stojanovic) nodejs/node#50956
zlib:
  * pause stream if outgoing buffer is full (Matteo Collina) https://github.com/nodejs-private/node-private/pull/542

PR-URL: https://github.com/nodejs-private/node-private/pull/545
sercher added a commit to sercher/graaljs that referenced this pull request Apr 25, 2024
As a part of the new signing requrements for Windows change approach to
use the DigiCert cloud HSM service KeyLocker.

PR-URL: nodejs/node#50956
Fixes: nodejs/build#3491
Reviewed-By: Richard Lau <rlau@redhat.com>
Reviewed-By: Michael Dawson <midawson@redhat.com>
sercher added a commit to sercher/graaljs that referenced this pull request Apr 25, 2024
This is a security release.

Notable changes:

crypto:
  * update root certificates to NSS 3.95 (Node.js GitHub Bot) nodejs/node#50805
  * disable PKCS#1 padding for privateDecrypt (Michael Dawson) https://github.com/nodejs-private/node-private/pull/525
deps:
  * upgrade npm to 10.2.4 (npm team) nodejs/node#50751
  * update archs files for openssl-3.0.13+quic1 (Node.js GitHub Bot) nodejs/node#51614
  * upgrade openssl sources to quictls/openssl-3.0.13+quic1 (Node.js GitHub Bot) ://github.com/nodejs/node/pull/51614
  * fix GHSA-f74f-cvh7-c6q6/CVE-2024-24806 (Santiago Gimeno) nodejs/node#51614
http:
  * add maximum chunk extension size (Paolo Insogna) https://github.com/nodejs-private/node-private/pull/520
lib:
  * update undici to v5.28.3 (Matteo Collina) https://github.com/nodejs-private/node-private/pull/536
src:
  * fix HasOnly(capability) in node::credentials (Tobias Nießen) https://github.com/nodejs-private/node-private/pull/505
test:
  * skip test-child-process-stdio-reuse-readable-stdio on Windows (Joyee Cheung) nodejs/node#49621
tools:
  * add macOS notarization verification step (Ulises Gascón) nodejs/node#50833
  * use macOS keychain to notarize the releases (Ulises Gascón) nodejs/node#50715
  * remove unused file (Ulises Gascon) nodejs/node#50622
  * add macOS notarization stapler (Ulises Gascón) nodejs/node#50625
  * improve macOS notarization process output readability (Ulises Gascón) nodejs/node#50389
  * remove unused `version` function (Ulises Gascón) nodejs/node#50390
win,tools:
  * upgrade Windows signing to smctl (Stefan Stojanovic) nodejs/node#50956
zlib:
  * pause stream if outgoing buffer is full (Matteo Collina) https://github.com/nodejs-private/node-private/pull/542

PR-URL: https://github.com/nodejs-private/node-private/pull/545
abhishekumar-tyagi pushed a commit to abhishekumar-tyagi/node that referenced this pull request May 5, 2024
As a part of the new signing requrements for Windows change approach to
use the DigiCert cloud HSM service KeyLocker.

PR-URL: nodejs/node#50956
Fixes: nodejs/build#3491
Reviewed-By: Richard Lau <rlau@redhat.com>
Reviewed-By: Michael Dawson <midawson@redhat.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backported-to-v18.x PRs backported to the v18.x-staging branch. backported-to-v20.x PRs backported to the v20.x-staging branch. needs-ci PRs that need a full CI run. tools Issues and PRs related to the tools directory. windows Issues and PRs related to the Windows platform.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Your DigiCert Code Signing certificate expires in 90 days
4 participants