Skip to content
This repository was archived by the owner on Apr 22, 2023. It is now read-only.

Commit 3a2b503

Browse files
committed
crypto: Clear error after DiffieHellman key errors
Fixes #5499
1 parent d5d5170 commit 3a2b503

File tree

2 files changed

+21
-12
lines changed

2 files changed

+21
-12
lines changed

src/node_crypto.cc

+10-7
Original file line numberDiff line numberDiff line change
@@ -67,6 +67,14 @@ namespace crypto {
6767

6868
using namespace v8;
6969

70+
// Forcibly clear OpenSSL's error stack on return. This stops stale errors
71+
// from popping up later in the lifecycle of crypto operations where they
72+
// would cause spurious failures. It's a rather blunt method, though.
73+
// ERR_clear_error() isn't necessarily cheap either.
74+
struct ClearErrorOnReturn {
75+
~ClearErrorOnReturn() { ERR_clear_error(); }
76+
};
77+
7078
static Persistent<String> errno_symbol;
7179
static Persistent<String> syscall_symbol;
7280
static Persistent<String> subject_symbol;
@@ -908,13 +916,6 @@ int Connection::HandleBIOError(BIO *bio, const char* func, int rv) {
908916

909917

910918
int Connection::HandleSSLError(const char* func, int rv, ZeroStatus zs) {
911-
// Forcibly clear OpenSSL's error stack on return. This stops stale errors
912-
// from popping up later in the lifecycle of the SSL connection where they
913-
// would cause spurious failures. It's a rather blunt method, though.
914-
// ERR_clear_error() isn't necessarily cheap either.
915-
struct ClearErrorOnReturn {
916-
~ClearErrorOnReturn() { ERR_clear_error(); }
917-
};
918919
ClearErrorOnReturn clear_error_on_return;
919920
(void) &clear_error_on_return; // Silence unused variable warning.
920921

@@ -3603,6 +3604,8 @@ class DiffieHellman : public ObjectWrap {
36033604
return ThrowException(Exception::Error(String::New("Not initialized")));
36043605
}
36053606

3607+
ClearErrorOnReturn clear_error_on_return;
3608+
(void) &clear_error_on_return; // Silence compiler warning.
36063609
BIGNUM* key = 0;
36073610

36083611
if (args.Length() == 0) {

test/simple/test-crypto.js

+11-5
Original file line numberDiff line numberDiff line change
@@ -698,10 +698,21 @@ var secret3 = dh3.computeSecret(key2, 'hex', 'base64');
698698

699699
assert.equal(secret1, secret3);
700700

701+
// Run this one twice to make sure that the dh3 clears its error properly
702+
(function() {
703+
var c = crypto.createDecipher('aes-128-ecb', '');
704+
assert.throws(function() { c.final('utf8') }, /wrong final block length/);
705+
})();
706+
701707
assert.throws(function() {
702708
dh3.computeSecret('');
703709
}, /key is too small/i);
704710

711+
(function() {
712+
var c = crypto.createDecipher('aes-128-ecb', '');
713+
assert.throws(function() { c.final('utf8') }, /wrong final block length/);
714+
})();
715+
705716
// Create a shared using a DH group.
706717
var alice = crypto.createDiffieHellmanGroup('modp5');
707718
var bob = crypto.createDiffieHellmanGroup('modp5');
@@ -858,11 +869,6 @@ assert.equal(-1, crypto.getHashes().indexOf('SHA1'));
858869
assert.equal(-1, crypto.getHashes().indexOf('SHA'));
859870
assertSorted(crypto.getHashes());
860871

861-
(function() {
862-
var c = crypto.createDecipher('aes-128-ecb', '');
863-
assert.throws(function() { c.final('utf8') }, /invalid public key/);
864-
})();
865-
866872
// Base64 padding regression test, see #4837.
867873
(function() {
868874
var c = crypto.createCipher('aes-256-cbc', 'secret');

0 commit comments

Comments
 (0)