tls: do not free cert in .getCertificate()
The documentation of `SSL_get_certificate` states that it returns an internal pointer that must not be freed by the caller. Therefore, using a smart pointer to take ownership is incorrect. Refs: https://man.openbsd.org/SSL_get_certificate.3 Refs: https://github.com/nodejs/node/pull/24261 Fixes: https://github.com/nodejs-private/security/issues/217 PR-URL: https://github.com/nodejs/node/pull/25490 Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Sam Roberts <vieuxtech@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
This commit is contained in:
parent
9e9890a8ff
commit
e888f667f5
@ -1962,10 +1962,10 @@ void SSLWrap<Base>::GetCertificate(
|
|||||||
|
|
||||||
Local<Object> result;
|
Local<Object> result;
|
||||||
|
|
||||||
X509Pointer cert(SSL_get_certificate(w->ssl_.get()));
|
X509* cert = SSL_get_certificate(w->ssl_.get());
|
||||||
|
|
||||||
if (cert)
|
if (cert != nullptr)
|
||||||
result = X509ToObject(env, cert.get());
|
result = X509ToObject(env, cert);
|
||||||
|
|
||||||
args.GetReturnValue().Set(result);
|
args.GetReturnValue().Set(result);
|
||||||
}
|
}
|
||||||
|
@ -37,8 +37,13 @@ const server = tls
|
|||||||
rejectUnauthorized: false
|
rejectUnauthorized: false
|
||||||
},
|
},
|
||||||
function() {
|
function() {
|
||||||
assert.strictEqual(client.getCertificate().serialNumber,
|
for (let i = 0; i < 10; ++i) {
|
||||||
'ECC9B856270DA9A8');
|
// Calling this repeatedly is a regression test that verifies
|
||||||
|
// that .getCertificate() does not accidentally decrease the
|
||||||
|
// reference count of the X509* certificate on the native side.
|
||||||
|
assert.strictEqual(client.getCertificate().serialNumber,
|
||||||
|
'ECC9B856270DA9A8');
|
||||||
|
}
|
||||||
client.end();
|
client.end();
|
||||||
server.close();
|
server.close();
|
||||||
}
|
}
|
||||||
|
Loading…
x
Reference in New Issue
Block a user