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
commented
Nov 8, 2022
•
edited
Loading
Uh oh!
There was an error while loading. Please reload this page.
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
Prior to this commit, functions accessing the OpenSSL error queue did
not work properly after x509.verify() returned false.
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
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.
Uh oh!
There was an error while loading. Please reload this page.
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.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
|
Thanks for the recommendation. I added the email to my GitHub account.
|
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
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
This comment was marked as outdated.
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
|
@takuro-sato Thank you for your contribution! I noticed it is your first. Congratulations 🎉
|
takuro-sato reacted with heart emoji
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
|
#45495 follows this up, fixing the same problem with x509.checkPrivateKey().
|
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
This was referenced Nov 19, 2022
|
Hi there,
Is there any chance that this (as well as the other issue with private keys) may be backported to Node 18?
|
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
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.
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.
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.
|