Skip to content

Commit 69343d6

Browse files
indutnyFishrock123
authored andcommitted
tls_wrap: clear errors on return
Adopt `MarkPopErrorOnReturn` from `node_crypto.cc`, and use it to clear errors after `SSL_read`/`SSL_write`/`SSL_shutdown` functions. See: #4485 PR-URL: #4515 Reviewed-By: Ben Noordhuis <[email protected]>
1 parent e57fd51 commit 69343d6

File tree

3 files changed

+24
-16
lines changed

3 files changed

+24
-16
lines changed

src/node_crypto.cc

-15
Original file line numberDiff line numberDiff line change
@@ -116,21 +116,6 @@ static X509_NAME *cnnic_ev_name =
116116
d2i_X509_NAME(nullptr, &cnnic_ev_p,
117117
sizeof(CNNIC_EV_ROOT_CA_SUBJECT_DATA)-1);
118118

119-
// Forcibly clear OpenSSL's error stack on return. This stops stale errors
120-
// from popping up later in the lifecycle of crypto operations where they
121-
// would cause spurious failures. It's a rather blunt method, though.
122-
// ERR_clear_error() isn't necessarily cheap either.
123-
struct ClearErrorOnReturn {
124-
~ClearErrorOnReturn() { ERR_clear_error(); }
125-
};
126-
127-
// Pop errors from OpenSSL's error stack that were added
128-
// between when this was constructed and destructed.
129-
struct MarkPopErrorOnReturn {
130-
MarkPopErrorOnReturn() { ERR_set_mark(); }
131-
~MarkPopErrorOnReturn() { ERR_pop_to_mark(); }
132-
};
133-
134119
static uv_mutex_t* locks;
135120

136121
const char* const root_certs[] = {

src/node_crypto.h

+15
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,21 @@
3939
namespace node {
4040
namespace crypto {
4141

42+
// Forcibly clear OpenSSL's error stack on return. This stops stale errors
43+
// from popping up later in the lifecycle of crypto operations where they
44+
// would cause spurious failures. It's a rather blunt method, though.
45+
// ERR_clear_error() isn't necessarily cheap either.
46+
struct ClearErrorOnReturn {
47+
~ClearErrorOnReturn() { ERR_clear_error(); }
48+
};
49+
50+
// Pop errors from OpenSSL's error stack that were added
51+
// between when this was constructed and destructed.
52+
struct MarkPopErrorOnReturn {
53+
MarkPopErrorOnReturn() { ERR_set_mark(); }
54+
~MarkPopErrorOnReturn() { ERR_pop_to_mark(); }
55+
};
56+
4257
enum CheckResult {
4358
CHECK_CERT_REVOKED = 0,
4459
CHECK_OK = 1

src/tls_wrap.cc

+9-1
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,6 @@ using v8::Object;
3131
using v8::String;
3232
using v8::Value;
3333

34-
3534
TLSWrap::TLSWrap(Environment* env,
3635
Kind kind,
3736
StreamBase* stream,
@@ -401,6 +400,8 @@ void TLSWrap::ClearOut() {
401400
if (ssl_ == nullptr)
402401
return;
403402

403+
crypto::MarkPopErrorOnReturn mark_pop_error_on_return;
404+
404405
char out[kClearOutChunkSize];
405406
int read;
406407
for (;;) {
@@ -462,6 +463,8 @@ bool TLSWrap::ClearIn() {
462463
if (ssl_ == nullptr)
463464
return false;
464465

466+
crypto::MarkPopErrorOnReturn mark_pop_error_on_return;
467+
465468
int written = 0;
466469
while (clear_in_->Length() > 0) {
467470
size_t avail = 0;
@@ -589,6 +592,8 @@ int TLSWrap::DoWrite(WriteWrap* w,
589592
if (ssl_ == nullptr)
590593
return UV_EPROTO;
591594

595+
crypto::MarkPopErrorOnReturn mark_pop_error_on_return;
596+
592597
int written = 0;
593598
for (i = 0; i < count; i++) {
594599
written = SSL_write(ssl_, bufs[i].base, bufs[i].len);
@@ -704,8 +709,11 @@ void TLSWrap::DoRead(ssize_t nread,
704709

705710

706711
int TLSWrap::DoShutdown(ShutdownWrap* req_wrap) {
712+
crypto::MarkPopErrorOnReturn mark_pop_error_on_return;
713+
707714
if (ssl_ != nullptr && SSL_shutdown(ssl_) == 0)
708715
SSL_shutdown(ssl_);
716+
709717
shutdown_ = true;
710718
EncOut();
711719
return stream_->DoShutdown(req_wrap);

0 commit comments

Comments
 (0)