diff --git a/deps/ncrypto/ncrypto.cc b/deps/ncrypto/ncrypto.cc index ae7a343fe49767..b21a0e88f0f99b 100644 --- a/deps/ncrypto/ncrypto.cc +++ b/deps/ncrypto/ncrypto.cc @@ -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 {}; @@ -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 {}; diff --git a/src/crypto/crypto_x509.cc b/src/crypto/crypto_x509.cc index 991c4ca6bfb404..a82d923e0a7fc7 100644 --- a/src/crypto/crypto_x509.cc +++ b/src/crypto/crypto_x509.cc @@ -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; @@ -182,19 +183,35 @@ MaybeLocal GetDer(Environment* env, const X509View& view) { MaybeLocal GetSubjectAltNameString(Environment* env, const X509View& view) { - Local 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 ret; if (!ToV8Value(env->context(), bio).ToLocal(&ret)) return {}; return ret; } MaybeLocal GetInfoAccessString(Environment* env, const X509View& view) { - Local 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 ret; if (!ToV8Value(env->context(), bio).ToLocal(&ret)) { return {}; } diff --git a/test/parallel/test-crypto-x509.js b/test/parallel/test-crypto-x509.js index a122ee9e300f30..e84ff3e2942540 100644 --- a/test/parallel/test-crypto-x509.js +++ b/test/parallel/test-crypto-x509.js @@ -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); } {