Skip to content

crypto: surface OpenSSL errors in X509Certificate.toLegacyObject #63301

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.

Sign up for GitHub

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

Open
fru1tworld wants to merge 1 commit into
base: main
Choose a base branch
from
+28 −6
Open

crypto: surface OpenSSL errors in X509Certificate.toLegacyObject #63301

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions deps/ncrypto/ncrypto.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1084,7 +1084,7 @@ BIOPointer X509View::getSubject() const {
}

BIOPointer X509View::getSubjectAltName() const {
ClearErrorOnReturn clearErrorOnReturn;
MarkPopErrorOnReturn markPopErrorOnReturn;
if (cert_ == nullptr) return {};
BIOPointer bio(BIO_new(BIO_s_mem()));
if (!bio) return {};
Expand All @@ -1110,7 +1110,7 @@ BIOPointer X509View::getIssuer() const {
}

BIOPointer X509View::getInfoAccess() const {
ClearErrorOnReturn clearErrorOnReturn;
MarkPopErrorOnReturn markPopErrorOnReturn;
if (cert_ == nullptr) return {};
BIOPointer bio(BIO_new(BIO_s_mem()));
if (!bio) return {};
Expand Down
25 changes: 21 additions & 4 deletions src/crypto/crypto_x509.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ using ncrypto::ClearErrorOnReturn;
using ncrypto::DataPointer;
using ncrypto::Digest;
using ncrypto::ECKeyPointer;
using ncrypto::MarkPopErrorOnReturn;
using ncrypto::SSLPointer;
using ncrypto::X509Name;
using ncrypto::X509Pointer;
Expand Down Expand Up @@ -182,19 +183,35 @@ MaybeLocal<Value> GetDer(Environment* env, const X509View& view) {

MaybeLocal<Value> GetSubjectAltNameString(Environment* env,
const X509View& view) {
Local<Value> ret;
MarkPopErrorOnReturn mark_pop_error_on_return;
auto bio = view.getSubjectAltName();
if (!bio) [[unlikely]]
if (!bio) [[unlikely]] {
// Distinguish "extension absent" (empty OpenSSL error queue) from an
// internal OpenSSL failure (e.g. allocation or extension parsing).
auto err = mark_pop_error_on_return.peekError();
if (err != 0) {
ThrowCryptoError(env, err, "Failed to get subjectAltName");
return {};
}
return Undefined(env->isolate());
}
Local<Value> ret;
if (!ToV8Value(env->context(), bio).ToLocal(&ret)) return {};
return ret;
}

MaybeLocal<Value> GetInfoAccessString(Environment* env, const X509View& view) {
Local<Value> ret;
MarkPopErrorOnReturn mark_pop_error_on_return;
auto bio = view.getInfoAccess();
if (!bio) [[unlikely]]
if (!bio) [[unlikely]] {
auto err = mark_pop_error_on_return.peekError();
if (err != 0) {
ThrowCryptoError(env, err, "Failed to get infoAccess");
return {};
}
return Undefined(env->isolate());
}
Local<Value> ret;
if (!ToV8Value(env->context(), bio).ToLocal(&ret)) {
return {};
}
Expand Down
5 changes: 5 additions & 0 deletions test/parallel/test-crypto-x509.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -330,6 +330,11 @@ oans248kpal88CGqsN2so/wZKxVnpiXlPHMdiNL7hRSUqlHkUi07FrP2Htg8kjI=
assert.match(
legacyObject.serialNumber,
legacyObjectCheck.serialNumberPattern);
// Refs: https://github.com/nodejs/node/issues/63265
// agent1-cert.pem has no subjectAltName extension; the legacy object should
// still report this as `undefined` (extension absent), not throw, while
// internal OpenSSL failures are now surfaced as exceptions.
assert.strictEqual(legacyObject.subjectaltname, undefined);
}

{
Expand Down
Loading