From 5f58cff6b25c74fee5df4cd4f00b263f7428c2ba Mon Sep 17 00:00:00 2001 From: Fedor Indutny Date: Sat, 2 Jan 2016 23:05:40 -0500 Subject: [PATCH] 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: https://github.com/nodejs/node/issues/4485 PR-URL: https://github.com/nodejs/node/pull/4515 Reviewed-By: Ben Noordhuis --- src/node_crypto.cc | 8 -------- src/node_crypto.h | 15 +++++++++++++++ src/tls_wrap.cc | 10 +++++++++- 3 files changed, 24 insertions(+), 9 deletions(-) diff --git a/src/node_crypto.cc b/src/node_crypto.cc index b03e6444ff8a3b..6a3ad5d16d4b2d 100644 --- a/src/node_crypto.cc +++ b/src/node_crypto.cc @@ -116,14 +116,6 @@ static X509_NAME *cnnic_ev_name = d2i_X509_NAME(nullptr, &cnnic_ev_p, sizeof(CNNIC_EV_ROOT_CA_SUBJECT_DATA)-1); -// Forcibly clear OpenSSL's error stack on return. This stops stale errors -// from popping up later in the lifecycle of crypto operations where they -// would cause spurious failures. It's a rather blunt method, though. -// ERR_clear_error() isn't necessarily cheap either. -struct ClearErrorOnReturn { - ~ClearErrorOnReturn() { ERR_clear_error(); } -}; - static uv_mutex_t* locks; const char* const root_certs[] = { diff --git a/src/node_crypto.h b/src/node_crypto.h index 3bec02c38ebdec..e009fc1da63b01 100644 --- a/src/node_crypto.h +++ b/src/node_crypto.h @@ -41,6 +41,21 @@ namespace node { namespace crypto { +// Forcibly clear OpenSSL's error stack on return. This stops stale errors +// from popping up later in the lifecycle of crypto operations where they +// would cause spurious failures. It's a rather blunt method, though. +// ERR_clear_error() isn't necessarily cheap either. +struct ClearErrorOnReturn { + ~ClearErrorOnReturn() { ERR_clear_error(); } +}; + +// Pop errors from OpenSSL's error stack that were added +// between when this was constructed and destructed. +struct MarkPopErrorOnReturn { + MarkPopErrorOnReturn() { ERR_set_mark(); } + ~MarkPopErrorOnReturn() { ERR_pop_to_mark(); } +}; + enum CheckResult { CHECK_CERT_REVOKED = 0, CHECK_OK = 1 diff --git a/src/tls_wrap.cc b/src/tls_wrap.cc index fc00893b77f84f..06d78ffdaa4d8e 100644 --- a/src/tls_wrap.cc +++ b/src/tls_wrap.cc @@ -31,7 +31,6 @@ using v8::Object; using v8::String; using v8::Value; - TLSWrap::TLSWrap(Environment* env, Kind kind, StreamBase* stream, @@ -401,6 +400,8 @@ void TLSWrap::ClearOut() { if (ssl_ == nullptr) return; + crypto::MarkPopErrorOnReturn mark_pop_error_on_return; + char out[kClearOutChunkSize]; int read; for (;;) { @@ -460,6 +461,8 @@ bool TLSWrap::ClearIn() { if (ssl_ == nullptr) return false; + crypto::MarkPopErrorOnReturn mark_pop_error_on_return; + int written = 0; while (clear_in_->Length() > 0) { size_t avail = 0; @@ -587,6 +590,8 @@ int TLSWrap::DoWrite(WriteWrap* w, if (ssl_ == nullptr) return UV_EPROTO; + crypto::MarkPopErrorOnReturn mark_pop_error_on_return; + int written = 0; for (i = 0; i < count; i++) { written = SSL_write(ssl_, bufs[i].base, bufs[i].len); @@ -702,8 +707,11 @@ void TLSWrap::DoRead(ssize_t nread, int TLSWrap::DoShutdown(ShutdownWrap* req_wrap) { + crypto::MarkPopErrorOnReturn mark_pop_error_on_return; + if (ssl_ != nullptr && SSL_shutdown(ssl_) == 0) SSL_shutdown(ssl_); + shutdown_ = true; EncOut(); return stream_->DoShutdown(req_wrap);