Skip to content

Commit 004e34d

Browse files
RafaelGSSjuanarbol
authored andcommitted
crypto: clear OpenSSL error on invalid ca cert
CVE-ID: CVE-2023-23919 PR-URL: nodejs-private/node-private#370 Refs: https://hackerone.com/bugs?report_id=1808596 Reviewed-By: Michael Dawson <midawson@redhat.com> Signed-off-by: RafaelGSS <rafael.nunu@hotmail.com>
1 parent e393d8a commit 004e34d

File tree

2 files changed

+49
-1
lines changed

2 files changed

+49
-1
lines changed

src/crypto/crypto_x509.cc

+4
Original file line numberDiff line numberDiff line change
@@ -340,6 +340,7 @@ void X509Certificate::Pem(const FunctionCallbackInfo<Value>& args) {
340340

341341
void X509Certificate::CheckCA(const FunctionCallbackInfo<Value>& args) {
342342
X509Certificate* cert;
343+
ClearErrorOnReturn clear_error_on_return;
343344
ASSIGN_OR_RETURN_UNWRAP(&cert, args.Holder());
344345
args.GetReturnValue().Set(X509_check_ca(cert->get()) == 1);
345346
}
@@ -440,6 +441,8 @@ void X509Certificate::CheckIssued(const FunctionCallbackInfo<Value>& args) {
440441
X509Certificate* issuer;
441442
ASSIGN_OR_RETURN_UNWRAP(&issuer, args[0]);
442443

444+
ClearErrorOnReturn clear_error_on_return;
445+
443446
args.GetReturnValue().Set(
444447
X509_check_issued(issuer->get(), cert->get()) == X509_V_OK);
445448
}
@@ -482,6 +485,7 @@ void X509Certificate::ToLegacy(const FunctionCallbackInfo<Value>& args) {
482485
Environment* env = Environment::GetCurrent(args);
483486
X509Certificate* cert;
484487
ASSIGN_OR_RETURN_UNWRAP(&cert, args.Holder());
488+
ClearErrorOnReturn clear_error_on_return;
485489
Local<Value> ret;
486490
if (X509ToObject(env, cert->get()).ToLocal(&ret))
487491
args.GetReturnValue().Set(ret);

test/parallel/test-crypto-x509.js

+45-1
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ const {
99
X509Certificate,
1010
createPrivateKey,
1111
generateKeyPairSync,
12+
createSign,
1213
} = require('crypto');
1314

1415
const {
@@ -190,14 +191,57 @@ const der = Buffer.from(
190191
{
191192
// https://github.com/nodejs/node/issues/45377
192193
// https://github.com/nodejs/node/issues/45485
193-
// Confirm failures of X509Certificate:verify() and X509Certificate:CheckPrivateKey()
194+
// Confirm failures of
195+
// X509Certificate:verify()
196+
// X509Certificate:CheckPrivateKey()
197+
// X509Certificate:CheckCA()
198+
// X509Certificate:CheckIssued()
199+
// X509Certificate:ToLegacy()
194200
// do not affect other functions that use OpenSSL.
195201
// Subsequent calls to e.g. createPrivateKey should not throw.
196202
const keyPair = generateKeyPairSync('ed25519');
197203
assert(!x509.verify(keyPair.publicKey));
198204
createPrivateKey(key);
199205
assert(!x509.checkPrivateKey(keyPair.privateKey));
200206
createPrivateKey(key);
207+
const certPem = `
208+
-----BEGIN CERTIFICATE-----
209+
MIID6zCCAtOgAwIBAgIUTUREAaNcNL0zPkxAlMX0GJtJ/FcwDQYJKoZIhvcNAQEN
210+
BQAwgYkxCzAJBgNVBAYTAlVTMRMwEQYDVQQIDApDYWxpZm9ybmlhMREwDwYDVQQH
211+
DAhDYXJsc2JhZDEPMA0GA1UECgwGVmlhc2F0MR0wGwYDVQQLDBRWaWFzYXQgU2Vj
212+
dXJlIE1vYmlsZTEiMCAGA1UEAwwZSGFja2VyT25lIHJlcG9ydCAjMTgwODU5NjAi
213+
GA8yMDIyMTIxNjAwMDAwMFoYDzIwMjMxMjE1MjM1OTU5WjCBiTELMAkGA1UEBhMC
214+
VVMxEzARBgNVBAgMCkNhbGlmb3JuaWExETAPBgNVBAcMCENhcmxzYmFkMQ8wDQYD
215+
VQQKDAZWaWFzYXQxHTAbBgNVBAsMFFZpYXNhdCBTZWN1cmUgTW9iaWxlMSIwIAYD
216+
VQQDDBlIYWNrZXJPbmUgcmVwb3J0ICMxODA4NTk2MIIBIjANBgkqhkiG9w0BAQEF
217+
AAOCAQ8AMIIBCgKCAQEA6I7RBPm4E/9rIrCHV5lfsHI/yYzXtACJmoyP8OMkjbeB
218+
h21oSJJF9FEnbivk6bYaHZIPasa+lSAydRM2rbbmfhF+jQoWYCIbV2ztrbFR70S1
219+
wAuJrlYYm+8u+1HUru5UBZWUr/p1gFtv3QjpA8+43iwE4pXytTBKPXFo1f5iZwGI
220+
D5Bz6DohT7Tyb8cpQ1uMCMCT0EJJ4n8wUrvfBgwBO94O4qlhs9vYgnDKepJDjptc
221+
uSuEpvHALO8+EYkQ7nkM4Xzl/WK1yFtxxE93Jvd1OvViDGVrRVfsq+xYTKknGLX0
222+
QIeoDDnIr0OjlYPd/cqyEgMcFyFxwDSzSc1esxdCpQIDAQABo0UwQzAdBgNVHQ4E
223+
FgQUurygsEKdtQk0T+sjM0gEURdveRUwEgYDVR0TAQH/BAgwBgEB/wIB/zAOBgNV
224+
HQ8BAf8EBAMCB4AwDQYJKoZIhvcNAQENBQADggEBAH7mIIXiQsQ4/QGNNFOQzTgP
225+
/bUbMSZJsY5TPAvS9rF9yQVzs4dJZnQk5kEb/qrDQSe27oP0L0hfFm1wTGy+aKfa
226+
BVGHdRmmvHtDUPLA9URCFShqKuS+GXp+6zt7dyZPRrPmiZaciiCMPHOnx59xSdPm
227+
AZG8cD3fmK2ThC4FAMyvRb0qeobka3s22xTQ2kjwJO5gykTkZ+BR6SzRHQTjYMuT
228+
iry9Bu8Kvbzu3r5n+/bmNz+xRNmEeehgT2qsHjA5b2YBVTr9MdN9Ro3H3saA3upr
229+
oans248kpal88CGqsN2so/wZKxVnpiXlPHMdiNL7hRSUqlHkUi07FrP2Htg8kjI=
230+
-----END CERTIFICATE-----`.trim();
231+
const c = new X509Certificate(certPem);
232+
assert(!c.ca);
233+
const signer = createSign('SHA256');
234+
assert(signer.sign(key, 'hex'));
235+
236+
const c1 = new X509Certificate(certPem);
237+
assert(!c1.checkIssued(c1));
238+
const signer1 = createSign('SHA256');
239+
assert(signer1.sign(key, 'hex'));
240+
241+
const c2 = new X509Certificate(certPem);
242+
assert(c2.toLegacyObject());
243+
const signer2 = createSign('SHA256');
244+
assert(signer2.sign(key, 'hex'));
201245
}
202246

203247
// X509Certificate can be cloned via MessageChannel/MessagePort

0 commit comments

Comments
 (0)