From b65799c9e4eb1e56ca8ff8279acdc2c70a0ab496 Mon Sep 17 00:00:00 2001 From: Takuro Sato Date: Wed, 9 Nov 2022 00:41:19 +0900 Subject: [PATCH 1/4] crypto: clear OpenSSL error queue after calling X509_verify() Prior to this commit, functions accessing the OpenSSL error queue did not work properly after x509.verify() returned false. --- src/crypto/crypto_x509.cc | 1 + test/parallel/test-crypto-x509.js | 5 +++++ 2 files changed, 6 insertions(+) diff --git a/src/crypto/crypto_x509.cc b/src/crypto/crypto_x509.cc index 3c30749c394655..cb3595fcb23215 100644 --- a/src/crypto/crypto_x509.cc +++ b/src/crypto/crypto_x509.cc @@ -472,6 +472,7 @@ void X509Certificate::Verify(const FunctionCallbackInfo& args) { X509_verify( cert->get(), key->Data()->GetAsymmetricKey().get()) > 0); + ERR_clear_error(); } void X509Certificate::ToLegacy(const FunctionCallbackInfo& args) { diff --git a/test/parallel/test-crypto-x509.js b/test/parallel/test-crypto-x509.js index 510e3183cf3ce3..a2d23c304cd636 100644 --- a/test/parallel/test-crypto-x509.js +++ b/test/parallel/test-crypto-x509.js @@ -186,6 +186,11 @@ const der = Buffer.from( code: 'ERR_INVALID_ARG_VALUE' }); + // Confirm failure of X509Certificate:verify() doesn't affect other functions that use OpenSSL + assert(!x509.verify(x509.publicKey)); + // It should not throw + createPrivateKey(key); + // X509Certificate can be cloned via MessageChannel/MessagePort const mc = new MessageChannel(); mc.port1.onmessage = common.mustCall(({ data }) => { From 2775ffc4f338fc8c567d8391402c126d8a20a24e Mon Sep 17 00:00:00 2001 From: Takuro Sato Date: Wed, 9 Nov 2022 01:26:01 +0900 Subject: [PATCH 2/4] use ClearErrorOnReturn instead of ERR_clear_error() --- src/crypto/crypto_x509.cc | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/crypto/crypto_x509.cc b/src/crypto/crypto_x509.cc index cb3595fcb23215..b3c888d16fece3 100644 --- a/src/crypto/crypto_x509.cc +++ b/src/crypto/crypto_x509.cc @@ -468,11 +468,12 @@ void X509Certificate::Verify(const FunctionCallbackInfo& args) { ASSIGN_OR_RETURN_UNWRAP(&key, args[0]); CHECK_EQ(key->Data()->GetKeyType(), kKeyTypePublic); + ClearErrorOnReturn clear_error_on_return; + args.GetReturnValue().Set( X509_verify( cert->get(), key->Data()->GetAsymmetricKey().get()) > 0); - ERR_clear_error(); } void X509Certificate::ToLegacy(const FunctionCallbackInfo& args) { From 5003cc93790183cb43c97173db0e783e7a0633b2 Mon Sep 17 00:00:00 2001 From: Takuro Sato <79583855+takuro-sato@users.noreply.github.com> Date: Wed, 9 Nov 2022 03:47:26 +0900 Subject: [PATCH 3/4] improve comment MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Tobias Nießen --- test/parallel/test-crypto-x509.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/parallel/test-crypto-x509.js b/test/parallel/test-crypto-x509.js index a2d23c304cd636..90f46a43aa8738 100644 --- a/test/parallel/test-crypto-x509.js +++ b/test/parallel/test-crypto-x509.js @@ -186,7 +186,7 @@ const der = Buffer.from( code: 'ERR_INVALID_ARG_VALUE' }); - // Confirm failure of X509Certificate:verify() doesn't affect other functions that use OpenSSL + // Confirm failure of X509Certificate:verify() doesn't affect other functions that use OpenSSL. assert(!x509.verify(x509.publicKey)); // It should not throw createPrivateKey(key); From f26f710858afbed259b9b6a5cfeedea1529c481f Mon Sep 17 00:00:00 2001 From: Takuro Sato <79583855+takuro-sato@users.noreply.github.com> Date: Wed, 9 Nov 2022 03:47:34 +0900 Subject: [PATCH 4/4] improve comment MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Tobias Nießen --- test/parallel/test-crypto-x509.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/parallel/test-crypto-x509.js b/test/parallel/test-crypto-x509.js index 90f46a43aa8738..0c628285f785a8 100644 --- a/test/parallel/test-crypto-x509.js +++ b/test/parallel/test-crypto-x509.js @@ -188,7 +188,7 @@ const der = Buffer.from( // Confirm failure of X509Certificate:verify() doesn't affect other functions that use OpenSSL. assert(!x509.verify(x509.publicKey)); - // It should not throw + // This call should not throw. createPrivateKey(key); // X509Certificate can be cloned via MessageChannel/MessagePort