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

Crypto: X509Certificate.checkPrivateKey() used with a non-matching key makes the next call to createPrivateKey() fail. #45485

Closed
tkarls opened this issue Nov 16, 2022 · 5 comments · Fixed by #45495
Labels
confirmed-bug Issues with confirmed bugs. crypto Issues and PRs related to the crypto subsystem.

Comments

@tkarls
Copy link

tkarls commented Nov 16, 2022

Version

18.12.1

Platform

Microsoft Windows NT 10.0.19044.0 x64

Subsystem

crypto

What steps will reproduce the bug?

Run the following script using node
I have tested with node 16, 17 and 18 with the same behaviour.

The script contain a certificate and a private key that does NOT match.
I'm using X509Certificate.checkPrivateKey() to verify the combination.

The validation returns false (as expected).
Then the next line creates a new private key again using createPrivateKey

This should work as the key itself is OK (and it is the same indata that was used the first time createPrivateKey was used)

Instead an exception is thrown.

const { X509Certificate, createPrivateKey } = require('node:crypto');

const certificate = `-----BEGIN CERTIFICATE-----
MIICGjCCAb+gAwIBAgIQPBumDEVNCS1HVGGqmK2K5TAKBggqhkjOPQQDAjAfMR0w
GwYDVQQDExRTdGVwIENBIGludGVybWVkaWF0ZTAeFw0yMjExMTYxMjA3MDhaFw0y
MzExMTYxODA4MDhaMBgxFjAUBgNVBAMTDW15LmRvbWFpbi5leHQwWTATBgcqhkjO
PQIBBggqhkjOPQMBBwNCAAQx3ReMlvFMR6CvXth5cgdyvH5K1PVBLL6077wgn3KH
oZngq2Qdegx/uOiKv6zU2NWUyGQzq5aBZQAhgKYT3eaio4HjMIHgMA4GA1UdDwEB
/wQEAwIHgDAdBgNVHSUEFjAUBggrBgEFBQcDAQYIKwYBBQUHAwIwHQYDVR0OBBYE
FLg8zUDxW06swsdeZ/WNpK7bWMysMB8GA1UdIwQYMBaAFE0eDYW1uSFCr8tbOgbr
UpCwapXYMBgGA1UdEQQRMA+CDW15LmRvbWFpbi5leHQwVQYMKwYBBAGCpGTGKEAB
BEUwQwIBAQQRc3VwcG9ydEBwYW5lZGEuc2UEKzRZOUpvR1RfWWNiVlhfR09UX2Fk
Nmk1cGZ6S2hpVDdVckE4ZU05TFZuVXMwCgYIKoZIzj0EAwIDSQAwRgIhAO3U+GsY
fKbRZKIWypOaWcOjREg9xD3KnsTwS5TGfMprAiEArEZowGfEzr2v6VjWJTh1+UUi
M54YrQa7Vdvp0DblvCo=
-----END CERTIFICATE-----`;

const key = `-----BEGIN EC PRIVATE KEY-----
MHcCAQEEIKeq1Bx1/6rRuHbNze/UQV1JchO5QasI17Abf+Tb8QqeoAoGCCqGSM49
AwEHoUQDQgAEsCYBL0B7tjzdx0unix2qKy+Mv3/RCmelFY91pR3EGdQGDmBYpAGN
p9WxWgJIIkdLlEVju/kD1Q55+dYWOlR/Wg==
-----END EC PRIVATE KEY-----`;

const cryptoCrt = new X509Certificate(certificate);
const cryptoKey = createPrivateKey(key);

const isMatching = cryptoCrt.checkPrivateKey(cryptoKey);
console.log(isMatching);

createPrivateKey(key);
console.log('done');

How often does it reproduce? Is there a required condition?

Always

What is the expected behavior?

Console output should be:

false
done

What do you see instead?

console output is:

false
node:internal/crypto/keys:620
    handle.init(kKeyTypePrivate, data, format, type, passphrase);
           ^

Error: error:05800074:x509 certificate routines::key values mismatch
    at createPrivateKey (node:internal/crypto/keys:620:12)
    at Object.<anonymous> (C:\work\gitwork\monorepo\applications\orchestration_service\src\orchestrationHandlers\cryptotest.js:30:1)
    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:81:12)
    at node:internal/main/run_main_module:23:47 {
  library: 'x509 certificate routines',
  reason: 'key values mismatch',
  code: 'ERR_OSSL_X509_KEY_VALUES_MISMATCH'
}

Node.js v18.12.1

Additional information

It looks to me, that the exception thrown on the second createPrivateKey is the actual error that occured in the checkPrivateKey function, causing it to return false.
In fact receiving that info instead as a plain false would have been nice.

But now it looks like the error is "queued" and then thrown when using createPrivateKey again.

@tkarls
Copy link
Author

tkarls commented Nov 16, 2022

Just tested and I can reproduce this on linux too.
Node version: 18.12.1
uname -a: Linux 5.19.12-200.fc36.x86_64 #1 SMP PREEMPT_DYNAMIC Wed Sep 28 17:11:05 UTC 2022 x86_64 GNU/Linux

@mscdex
Copy link
Contributor

mscdex commented Nov 16, 2022

Probably the same solution as #45377.

@VoltrexKeyva VoltrexKeyva added the crypto Issues and PRs related to the crypto subsystem. label Nov 16, 2022
@tkarls
Copy link
Author

tkarls commented Nov 16, 2022

Probably the same solution as #45377.

Ah, thanks, that looks like exactly the same issue but for the verify() function instead.

That PR won't fix this function though. And this probably affects even more functions (like all that do not throw on error unkess they have been marked with that clear on return)

This is pretty serious flaw in the crypto lib I think. Is there any known workaround? Is it possible to clear the openssl error queue manually from node? I cannot find anything like that in the docs.

@panva
Copy link
Member

panva commented Nov 17, 2022

Fix in #45495

@tkarls
Copy link
Author

tkarls commented Nov 17, 2022

Fix in #45495

That was quick! Great stuff 👍

@panva panva added the confirmed-bug Issues with confirmed bugs. label Nov 17, 2022
nodejs-github-bot pushed a commit that referenced this issue Nov 19, 2022
Fixes: #45485
PR-URL: #45495
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
ruyadorno pushed a commit that referenced this issue Nov 21, 2022
Fixes: #45485
PR-URL: #45495
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
marco-ippolito pushed a commit to marco-ippolito/node that referenced this issue Nov 23, 2022
Fixes: nodejs#45485
PR-URL: nodejs#45495
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
danielleadams pushed a commit that referenced this issue Dec 30, 2022
Fixes: #45485
PR-URL: #45495
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
danielleadams pushed a commit that referenced this issue Dec 30, 2022
Fixes: #45485
PR-URL: #45495
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
danielleadams pushed a commit that referenced this issue Jan 3, 2023
Fixes: #45485
PR-URL: #45495
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
danielleadams pushed a commit that referenced this issue Jan 4, 2023
Fixes: #45485
PR-URL: #45495
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
danielleadams pushed a commit that referenced this issue Jan 5, 2023
Fixes: #45485
PR-URL: #45495
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
oraluben pushed a commit to oraluben/node that referenced this issue Mar 14, 2023
oraluben pushed a commit to oraluben/node that referenced this issue Mar 17, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
confirmed-bug Issues with confirmed bugs. crypto Issues and PRs related to the crypto subsystem.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants