GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — crypto: clear OpenSSL error queue after calling X509_verify() by takuro-sato · Pull Request #45377 · nodejs/node

Conversation

@takuro-sato

@takuro-sato takuro-sato commented Nov 8, 2022 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Contributor

Prior to this commit, functions accessing the OpenSSL error queue did not work properly after x509.verify() returned false.

Example code to reproduce the bug

const crypto = require('crypto');

const cert0 = new crypto.X509Certificate(
    '-----BEGIN CERTIFICATE-----\n' +
    'MIICeTCCAWGgAwIBAgIBADANBgkqhkiG9w0BAQsFADAAMB4XDTIyMTEwNzExMzU0\n' +
    'NloXDTIyMTEwNzExMzU0NlowADCCASIwDQYJKoZIhvcNAQEBBQADggEPADCCAQoC\n' +
    'ggEBAKXDiNM5kCg3m7mBmLGhTWDLo3rEDf7nfI77V2rS9jXg0wV1LvnxQcYFeUud\n' +
    'zfvZpSc07TdmSDWJhE04/9G2fk86RwnMYj9KBLCgqzjk/wnrlcB8BuFPC5ZUKm+e\n' +
    'OmpfwzxRsqU0dlp/C6BmZV8cuvOKbxcq7MnACmS3lOvDUIdXjkEI1LEkzQudQNQi\n' +
    'S2PDPRUFm5KDkLmky2MACUkUA1eGMI8GLV60fzrsMnD6/lIGgDgRjcMdtv7D0F0K\n' +
    'D/AbCfXv4Z6SCKgp+0nLC5PpacNWu0yk1y4ihm9BHCRrxGcMzX5ce9pUfXVt6nUA\n' +
    'gwzlTcMno28yb16dBd9+XdQu/p8CAwEAATANBgkqhkiG9w0BAQsFAAOCAQEAVaRJ\n' +
    'CSOGycfz+bOGvSkLIOGWjtRoeaX/hMwCB/b0BuAVmcyWJuPsPcghwDnNUJAlxDnB\n' +
    'R7F6+J9P2kv7V+B4nlB/w0YHbidlSGeyd7AENsHqwYzOaoGCcr0B8pQrocj5Dm3g\n' +
    'JmPGMmMSa+MJfPD5j1///plvbLzBdG8nm9nG1yzOho+4lJf0VUwDnVEcRJp1w7kF\n' +
    'Ys19JJo78BEnUHJMOv1vE1KINY1k6wB69BJwZoW6tDjwHcHAtWzOA6324r+2tmnK\n' +
    'HCKAw78IZsDjLtC13LtrqFgWXzOh6WkLGmS7W01Coi3+uZcmWwJv/8m+2VlOXgrg\n' +
    'iQYf67llQlRAay0wnA==\n' +
    '-----END CERTIFICATE-----\n'
);

const cert1 = new crypto.X509Certificate(
    '-----BEGIN CERTIFICATE-----\n' +
    'MIICeTCCAWGgAwIBAgIBADANBgkqhkiG9w0BAQsFADAAMB4XDTIyMTEwNzExMzU0\n' +
    'N1oXDTIyMTEwNzExMzU0N1owADCCASIwDQYJKoZIhvcNAQEBBQADggEPADCCAQoC\n' +
    'ggEBAOc1bvNUvDCUxLhgNc9oQ8WwXsVSGHWaulf93eA1m9oB3kzvoCbEURN1k/dv\n' +
    'QoeLhovosZqnyz4W/2o+RHwCy/PNi3FGGaFGcpMTdwEGLn8TlGcUKv32h1Her+og\n' +
    'XnwGqZizD8xBbqfi5WRcX3JS02EKC8BVlDQiru2eToR+T6ZiOIGRZDNYYHUGvoSz\n' +
    '0nFiLzFLEwuMhEJhGmXcy0JyWx4BnYwEYYnwdKSAPakYFzvePMvpYyX2Xg4eD65j\n' +
    'kBRtKSYxDK39rXR6BfH7UpVjsY6H3TlVXYQdkxBPfzZlCPJBMG6TJRl2IzYJU1H4\n' +
    'JK84mEhDGnE9fxxvxykYX5a7YYMCAwEAATANBgkqhkiG9w0BAQsFAAOCAQEAAPcQ\n' +
    'cA6MFAGvbrACFYKHx7rMZ/HxjEps47Dq8zapqPWaTZVCSRI3si8lH99UW0DCRlRJ\n' +
    'VQnW2l7Ag7edp2RLQlCoH8gzEkCXvG3CNVE+U82WnlsUA+FTTV+X4W9Va3nNe2ZU\n' +
    'B8SO4V0374osYlM5UaUz9VWU3ulBupufQ9wdJTEVPcfEekDXVGeJIFeiyafhIYrB\n' +
    'H6OA1xhS0Yf2NtEIup2vexJ3wOxXh1ZyPR190Jr2b9ePylcUU2DWSCMvnzFdCPLt\n' +
    'aHR8RktrScK8BVXMag2Fvvgoco0XCIoWl/kPN3I0SnvIcTHa/uWA49ijMqwM1yDc\n' +
    'Timu77Xm/VYec/v1qg==\n' +
    '-----END CERTIFICATE-----\n'
)

const cert2 = new crypto.X509Certificate(
    '-----BEGIN CERTIFICATE-----\n' +
    'MIICeTCCAWGgAwIBAgIBADANBgkqhkiG9w0BAQsFADAAMB4XDTIyMTEwNzExMzU0\n' +
    'N1oXDTIyMTEwNzExMzU0N1owADCCASIwDQYJKoZIhvcNAQEBBQADggEPADCCAQoC\n' +
    'ggEBAJwgfV4eP/rsHYVtuCBvahP7tWJ/Djs9V11NHuVB11zUng2NiwfT3qvRfR/G\n' +
    'kc+fEUB/8K9kV4kGiFwB+xOKZ/0vpiRVKINPhrDJYdehahlT5iwF3dkTqNARp9MK\n' +
    'Dk1z3iAa0aQXNXCwIUNhs+8GZmXHZn8AOjAcBz03alBwUaYINMQVfjERT+BOCr7E\n' +
    'wK793iY8knF0ZT07GKIzqRFJhueBA+VKfoXluSSa7SoAO3UwYt72k1UIdUjA5C73\n' +
    'I8ZnlmIAVldjpO4AZdhwcVrHth54G3SAtt/vjk3052iJXpICduPWCUCF+pZDkRSO\n' +
    'DkSNfzaxr40I2raP0hvveUiArBUCAwEAATANBgkqhkiG9w0BAQsFAAOCAQEAjkdh\n' +
    'B0fyPt2Gyt5KRoCqsy8dS3iEhT98YzULeBk+mtcJXsbH8aEBXQzqzSnYfP3LeDxK\n' +
    'xP87pcP+V9sruQixHGNYQqRar8jpBuAhQAX8aqgWe4rc++3RbotafYJ7ILRj3J6K\n' +
    '0JXB3YXxjk7vp0aO/eaodKFM3g5EnKtXHH37yDZMQ665RPglh+MoJOw4BVutLPh4\n' +
    'MpmTY1yxl/HbIvqv8uQw55ftjXSaRB/pCCh4IjcQ3LS9FXE4pdESwgKScy8JsiDt\n' +
    'z/BtAH3hG/AbVon2RWBF4d5exFl4aJj3CSNKvMlu7q/CSbDWxdCafO433QRsI6SV\n' +
    'fW0jkmnFsvaUpOAF0w==\n' +
    '-----END CERTIFICATE-----\n'
);

const privateKeyPem = '-----BEGIN PRIVATE KEY-----\n' +
'MIGHAgEAMBMGByqGSM49AgEGCCqGSM49AwEHBG0wawIBAQQgo5BAQydImlcOf1LS\n' +
'7FrRfoBgHHUqevagq/1nmv3thMChRANCAAQ/oNhs1q1GJ4OOats5kaARTOPBZgzb\n' +
'MpVnfwmmLDYGOJotCe0sVAUMYBCQ4DHko9DdOH6HWPSiGAzohs4nwDZz\n' +
'-----END PRIVATE KEY-----';

const privateKey = crypto.createPrivateKey({key: privateKeyPem})
console.log(privateKey.asymmetricKeyDetails) // { namedCurve: 'prime256v1' }

crypto.createPrivateKey({key: privateKeyPem})
console.log(cert1.verify(cert2.publicKey)) // true

crypto.createPrivateKey({key: privateKeyPem})
console.log(cert0.verify(cert2.publicKey))  // false
crypto.createPrivateKey({key: privateKeyPem}) // It throws and error is about RSA key even though `key` is prime256v1

output:

{ namedCurve: 'prime256v1' }
true
false
node:internal/crypto/keys:620
    handle.init(kKeyTypePrivate, data, format, type, passphrase);
           ^

Error: error:0200008A:rsa routines::invalid padding
    at Object.createPrivateKey (node:internal/crypto/keys:620:12)
    at Object.<anonymous> (/home/takuro/Projects/dev_node/bug.js:76:8)
    at Module._compile (node:internal/modules/cjs/loader:1159:14)
    at Module._extensions..js (node:internal/modules/cjs/loader:1213:10)
    at Module.load (node:internal/modules/cjs/loader:1037:32)
    at Module._load (node:internal/modules/cjs/loader:878:12)
    at Function.executeUserEntryPoint [as runMain] (node:internal/modules/run_main:82:12)
    at node:internal/main/run_main_module:23:47 {
  opensslErrorStack: [
    'error:06880006:asn1 encoding routines::EVP lib',
    'error:1C880004:Provider routines::RSA lib',
    'error:02000072:rsa routines::padding check failed'
  ],
  library: 'rsa routines',
  reason: 'invalid padding',
  code: 'ERR_OSSL_RSA_INVALID_PADDING'
}

Node.js v19.0.1

Note

I got an error when I ran make -j4 test locally, but I believe it's not related to this PR because I got the same one with the latest main and v19.0.1.

make -s jstest
ninja: Entering directory `out/Release'
ninja: no work to do.
=== release test-tls-dhe ===                                                  
Path: parallel/test-tls-dhe
Command: out/Release/node --no-warnings /home/takuro/Projects/node/test/parallel/test-tls-dhe.js
--- TIMEOUT ---
[05:04|% 100|+ 3769|-   1]: Done                                              
make[1]: *** [Makefile:307: jstest] Error 1
make: *** [Makefile:333: test] Error 2

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

bricss reacted with thumbs up emoji
All reactions
  • 👍 1 reaction
@takuro-sato
crypto: clear OpenSSL error queue after calling X509_verify()
b65799c
Prior to this commit, functions accessing the OpenSSL error queue did
not work properly after x509.verify() returned false.
@nodejs-github-bot

nodejs-github-bot commented Nov 8, 2022

Copy link
Copy Markdown
Collaborator

Review requested:

  • @nodejs/crypto
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@nodejs-github-bot nodejs-github-bot added c++ Issues and PRs that require attention from people who are familiar with C++. crypto Issues and PRs related to the crypto subsystem. needs-ci PRs that need a full CI run. labels Nov 8, 2022
@takuro-sato
takuro-sato marked this pull request as ready for review November 8, 2022 16:17
tniessen
tniessen reviewed Nov 8, 2022
View reviewed changes

@tniessen tniessen left a comment

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

As this function is currently implemented, it ignores the difference between errors and verification failure. As such, this change appears logical. (Whether the current behavior is the best choice is a separate question.)

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
Comment thread src/crypto/crypto_x509.cc Resolved

Uh oh!

There was an error while loading. Please reload this page.

@takuro-sato
use ClearErrorOnReturn instead of ERR_clear_error()
2775ffc
tniessen
tniessen reviewed Nov 8, 2022
View reviewed changes

@tniessen tniessen left a comment

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Thank you for your contribution.

It seems that the email address you are using to commit your changes is not associated with your GitHub account; we highly recommend that you either add the commit email address to your GitHub account (not to your public GitHub profile!) or that you re-commit your changes with an email address of your choice that is connected to your GitHub account. Note that you can add multiple email addresses to the same GitHub account.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
Comment thread test/parallel/test-crypto-x509.js Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread test/parallel/test-crypto-x509.js Resolved

Uh oh!

There was an error while loading. Please reload this page.

takuro-sato and others added 2 commits November 9, 2022 03:47
@takuro-sato @tniessen
improve comment
5003cc9
Co-authored-by: Tobias Nießen <[email protected]>
@takuro-sato @tniessen
improve comment
f26f710
Co-authored-by: Tobias Nießen <[email protected]>
@takuro-sato

takuro-sato commented Nov 8, 2022

Copy link
Copy Markdown
Contributor Author

Thanks for the recommendation. I added the email to my GitHub account.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@Trott
Trott force-pushed the main branch from 2d76238 to ca3ed36 Compare November 12, 2022 01:49
@mscdex mscdex mentioned this pull request Nov 16, 2022
Crypto: X509Certificate.checkPrivateKey() used with a non-matching key makes the next call to createPrivateKey() fail. #45485
Closed
jasnell
jasnell approved these changes Nov 16, 2022
View reviewed changes
@panva panva added the request-ci Add this label to start a Jenkins CI on a PR. Only starts once the PR has an approving review. label Nov 17, 2022
@github-actions github-actions Bot removed the request-ci Add this label to start a Jenkins CI on a PR. Only starts once the PR has an approving review. label Nov 17, 2022
@nodejs-github-bot

This comment was marked as outdated.

Sign in to view
tniessen
tniessen approved these changes Nov 17, 2022
View reviewed changes
@tniessen tniessen added author ready PRs with CI started, the required approvals, and no outstanding review comments. commit-queue-squash PRs the Commit Queue should land as one squashed commit. labels Nov 17, 2022
@nodejs-github-bot

nodejs-github-bot commented Nov 17, 2022

Copy link
Copy Markdown
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/47984/

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

panva
panva approved these changes Nov 17, 2022
View reviewed changes
@panva panva added the commit-queue PRs queued for automated landing through the Commit Queue. label Nov 17, 2022
@panva

panva commented Nov 17, 2022

Copy link
Copy Markdown
Member

@takuro-sato Thank you for your contribution! I noticed it is your first. Congratulations 🎉

takuro-sato reacted with heart emoji
All reactions
  • ❤️ 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@nodejs-github-bot nodejs-github-bot removed the commit-queue PRs queued for automated landing through the Commit Queue. label Nov 17, 2022
@nodejs-github-bot
nodejs-github-bot merged commit 405ea2a into nodejs:main Nov 17, 2022
@nodejs-github-bot

nodejs-github-bot commented Nov 17, 2022

Copy link
Copy Markdown
Collaborator

Landed in 405ea2a

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@panva

panva commented Nov 17, 2022

Copy link
Copy Markdown
Member

#45495 follows this up, fixing the same problem with x509.checkPrivateKey().

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@github-actions github-actions Bot mentioned this pull request Nov 18, 2022
CI Reliability 2022-11-18 nodejs/reliability#432
Open
12 tasks
This was referenced Nov 19, 2022
CI Reliability 2022-11-19 nodejs/reliability#433
Open
CI Reliability 2022-11-20 nodejs/reliability#434
Open
CI Reliability 2022-11-21 nodejs/reliability#435
Open
ruyadorno pushed a commit that referenced this pull request Nov 21, 2022
@takuro-sato @ruyadorno
crypto: clear OpenSSL error queue after calling X509_verify()
efe19eb
Prior to this commit, functions accessing the OpenSSL error queue did
not work properly after x509.verify() returned false.

PR-URL: #45377
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Tobias Nießen <[email protected]>
Reviewed-By: Filip Skokan <[email protected]>
@github-actions github-actions Bot mentioned this pull request Nov 22, 2022
CI Reliability 2022-11-22 nodejs/reliability#436
Open
15 tasks
@ruyadorno ruyadorno mentioned this pull request Nov 24, 2022
v19.2.0 proposal #45615
Merged
@mjones-vsat

mjones-vsat commented Dec 16, 2022

Copy link
Copy Markdown

Hi there,

Is there any chance that this (as well as the other issue with private keys) may be backported to Node 18?

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@mjones-vsat mjones-vsat mentioned this pull request Dec 16, 2022
Release plan - v18.x Maintenance LTS nodejs/Release#737
Closed
@danielleadams danielleadams mentioned this pull request Dec 30, 2022
v18.13.0 release proposal #46025
Merged
danielleadams pushed a commit that referenced this pull request Dec 30, 2022
@takuro-sato @danielleadams
crypto: clear OpenSSL error queue after calling X509_verify()
3b11c84
Prior to this commit, functions accessing the OpenSSL error queue did
not work properly after x509.verify() returned false.

PR-URL: #45377
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Tobias Nießen <[email protected]>
Reviewed-By: Filip Skokan <[email protected]>
danielleadams pushed a commit that referenced this pull request Jan 3, 2023
@takuro-sato @danielleadams
crypto: clear OpenSSL error queue after calling X509_verify()
dae92e7
Prior to this commit, functions accessing the OpenSSL error queue did
not work properly after x509.verify() returned false.

PR-URL: #45377
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Tobias Nießen <[email protected]>
Reviewed-By: Filip Skokan <[email protected]>
oraluben pushed a commit to oraluben/node that referenced this pull request Mar 14, 2023
@takuro-sato @oraluben
crypto: clear OpenSSL error queue after calling X509_verify()
388d58e
Prior to this commit, functions accessing the OpenSSL error queue did
not work properly after x509.verify() returned false.

Backport-PR-URL: https://github.com/nodejs-private/node-private/pull/375
CVE-ID: CVE-2023-23919
PR-URL: nodejs/node#45377
Refs: https://hackerone.com/bugs?subject=nodejs&report_id=1808596
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Tobias Nießen <[email protected]>
Reviewed-By: Filip Skokan <[email protected]>
oraluben pushed a commit to oraluben/node that referenced this pull request Mar 17, 2023
@takuro-sato @oraluben
crypto: clear OpenSSL error queue after calling X509_verify()
3d76c7a
Prior to this commit, functions accessing the OpenSSL error queue did
not work properly after x509.verify() returned false.

Backport-PR-URL: https://github.com/nodejs-private/node-private/pull/375
CVE-ID: CVE-2023-23919
PR-URL: nodejs/node#45377
Refs: https://hackerone.com/bugs?subject=nodejs&report_id=1808596
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Tobias Nießen <[email protected]>
Reviewed-By: Filip Skokan <[email protected]>
m0th3rch1p pushed a commit to m0th3rch1p/node that referenced this pull request Jul 20, 2025
@takuro-sato @richardlau
crypto: clear OpenSSL error queue after calling X509_verify()
d0ece30
Prior to this commit, functions accessing the OpenSSL error queue did
not work properly after x509.verify() returned false.

Backport-PR-URL: nodejs-private/node-private#375
CVE-ID: CVE-2023-23919
PR-URL: nodejs#45377
Refs: https://hackerone.com/bugs?subject=nodejs&report_id=1808596
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Tobias Nießen <[email protected]>
Reviewed-By: Filip Skokan <[email protected]>
@achamayou achamayou mentioned this pull request May 6, 2026
Flaky: modules_test polyfill > isValidX509CertChain times out at 2s microsoft/CCF#7862
Closed
Copilot AI mentioned this pull request May 6, 2026
fix: generate X509 cert chain once in before() to prevent test timeout microsoft/CCF#7864
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@panva panva panva approved these changes
@jasnell jasnell jasnell approved these changes
@tniessen tniessen tniessen approved these changes

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

author ready PRs with CI started, the required approvals, and no outstanding review comments. c++ Issues and PRs that require attention from people who are familiar with C++. commit-queue-squash PRs the Commit Queue should land as one squashed commit. crypto Issues and PRs related to the crypto subsystem. needs-ci PRs that need a full CI run.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

6 participants

@takuro-sato @nodejs-github-bot @panva @mjones-vsat @jasnell @tniessen
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.