Skip to content

crypto: surface OpenSSL errors in Certificate.exportPublicKey #63272

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 3 commits into
base: main
Choose a base branch
from
+14 −2
Open

crypto: surface OpenSSL errors in Certificate.exportPublicKey #63272

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
455c83f
crypto: surface OpenSSL errors in Certificate.exportPublicKey
fru1tworld May 13, 2026
adf8f83
Update test/parallel/test-crypto-certificate.js
fru1tworld May 13, 2026
e753ba5
fixup! crypto: surface OpenSSL errors in Certificate.exportPublicKey
fru1tworld May 14, 2026
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
8 changes: 7 additions & 1 deletion src/crypto/crypto_spkac.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
namespace node {

using ncrypto::BIOPointer;
using ncrypto::MarkPopErrorOnReturn;
using v8::Context;
using v8::FunctionCallbackInfo;
using v8::Local;
Expand Down Expand Up @@ -40,8 +41,13 @@ void ExportPublicKey(const FunctionCallbackInfo<Value>& args) {
if (!input.CheckSizeInt32()) [[unlikely]]
return THROW_ERR_OUT_OF_RANGE(env, "spkac is too large");

MarkPopErrorOnReturn mark_pop_error_on_return;
BIOPointer bio = ncrypto::ExportPublicKey(input.data(), input.size());
if (!bio) return args.GetReturnValue().SetEmptyString();
if (!bio) {
return ThrowCryptoError(env,
mark_pop_error_on_return.peekError(),
"Failed to export public key from SPKAC");
}
Comment on lines +44 to +50
Copy link
Copy Markdown
Member

@Renegade334 Renegade334 May 13, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is only wrapping a single operation, should it not just be using ERR_get_error() instead?


auto pkey = ByteSource::FromBIO(bio);
args.GetReturnValue().Set(pkey.ToBuffer(env).FromMaybe(Local<Value>()));
Expand Down
8 changes: 7 additions & 1 deletion test/parallel/test-crypto-certificate.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,13 @@ function checkMethods(certificate) {
stripLineEndings(certificate.exportPublicKey(spkacValid).toString('utf8')),
stripLineEndings(spkacPublicPem.toString('utf8'))
);
assert.strictEqual(certificate.exportPublicKey(spkacFail), '');
// Refs: https://github.com/nodejs/node/issues/63264
// exportPublicKey surfaces internal OpenSSL failures (including invalid
// SPKAC input) via an exception, rather than silently returning ''.
assert.throws(
() => certificate.exportPublicKey(spkacFail),
{ code: 'ERR_OSSL_ASN1_WRONG_TAG' },
);
Comment thread
fru1tworld marked this conversation as resolved.

assert.strictEqual(
certificate.exportChallenge(spkacValid).toString('utf8'),
Expand Down