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]> v4.x-staging Commit Metadata: PR-URL: #4709 Reviewed-By: James M Snell <[email protected]>
nodejs · Jan 15, 2016 · cfb0945 · cfb0945
1 parent 5608267
commit cfb0945
Show file tree

Hide file tree

Showing 3 changed files with 24 additions and 9 deletions.
diff --git 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
@@ -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
@@ -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 (;;) {
@@ -462,6 +463,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;
@@ -589,6 +592,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);
@@ -704,8 +709,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);