tls: ignore .shutdown() syscall error

Quote from SSL_shutdown man page: The output of SSL_get_error(3) may be misleading, as an erroneous SSL_ERROR_SYSCALL may be flagged even though no error occurred. Also, handle all other errors to prevent assertion in `ClearError()`.
nodejs · May 28, 2013 · fa170dd · fa170dd
1 parent 28f4c15
commit fa170dd
Show file tree

Hide file tree

Showing 3 changed files with 47 additions and 11 deletions.
diff --git a/lib/tls.js b/lib/tls.js
@@ -282,8 +282,14 @@ function onCryptoStreamFinish() {
       // NOTE: first call checks if client has sent us shutdown,
       // second call enqueues shutdown into the BIO.
       if (this.pair.ssl.shutdown() !== 1) {
+        if (this.pair.ssl && this.pair.ssl.error)
+          return this.pair.error();
+
         this.pair.ssl.shutdown();
       }
+
+      if (this.pair.ssl && this.pair.ssl.error)
+        return this.pair.error();
     }
   } else {
     debug('encrypted.onfinish');

diff --git a/src/node_crypto.cc b/src/node_crypto.cc
@@ -915,7 +915,10 @@ int Connection::HandleBIOError(BIO *bio, const char* func, int rv) {
 }
 
 
-int Connection::HandleSSLError(const char* func, int rv, ZeroStatus zs) {
+int Connection::HandleSSLError(const char* func,
+                               int rv,
+                               ZeroStatus zs,
+                               SyscallStatus ss) {
   ClearErrorOnReturn clear_error_on_return;
   (void) &clear_error_on_return;  // Silence unused variable warning.
 
@@ -940,6 +943,9 @@ int Connection::HandleSSLError(const char* func, int rv, ZeroStatus zs) {
                  Exception::Error(String::New("ZERO_RETURN")));
     return rv;
 
+  } else if ((err == SSL_ERROR_SYSCALL) && (ss == kIgnoreSyscall)) {
+    return 0;
+
   } else {
     HandleScope scope;
     BUF_MEM* mem;
@@ -1372,17 +1378,26 @@ Handle<Value> Connection::ClearOut(const Arguments& args) {
 
     if (ss->is_server_) {
       rv = SSL_accept(ss->ssl_);
-      ss->HandleSSLError("SSL_accept:ClearOut", rv, kZeroIsAnError);
+      ss->HandleSSLError("SSL_accept:ClearOut",
+                         rv,
+                         kZeroIsAnError,
+                         kSyscallError);
     } else {
       rv = SSL_connect(ss->ssl_);
-      ss->HandleSSLError("SSL_connect:ClearOut", rv, kZeroIsAnError);
+      ss->HandleSSLError("SSL_connect:ClearOut",
+                         rv,
+                         kZeroIsAnError,
+                         kSyscallError);
     }
 
     if (rv < 0) return scope.Close(Integer::New(rv));
   }
 
   int bytes_read = SSL_read(ss->ssl_, buffer_data + off, len);
-  ss->HandleSSLError("SSL_read:ClearOut", bytes_read, kZeroIsNotAnError);
+  ss->HandleSSLError("SSL_read:ClearOut",
+                     bytes_read,
+                     kZeroIsNotAnError,
+                     kSyscallError);
   ss->SetShutdownFlags();
 
   return scope.Close(Integer::New(bytes_read));
@@ -1472,10 +1487,16 @@ Handle<Value> Connection::ClearIn(const Arguments& args) {
     int rv;
     if (ss->is_server_) {
       rv = SSL_accept(ss->ssl_);
-      ss->HandleSSLError("SSL_accept:ClearIn", rv, kZeroIsAnError);
+      ss->HandleSSLError("SSL_accept:ClearIn",
+                         rv,
+                         kZeroIsAnError,
+                         kSyscallError);
     } else {
       rv = SSL_connect(ss->ssl_);
-      ss->HandleSSLError("SSL_connect:ClearIn", rv, kZeroIsAnError);
+      ss->HandleSSLError("SSL_connect:ClearIn",
+                         rv,
+                         kZeroIsAnError,
+                         kSyscallError);
     }
 
     if (rv < 0) return scope.Close(Integer::New(rv));
@@ -1485,7 +1506,8 @@ Handle<Value> Connection::ClearIn(const Arguments& args) {
 
   ss->HandleSSLError("SSL_write:ClearIn",
                      bytes_written,
-                     len == 0 ? kZeroIsNotAnError : kZeroIsAnError);
+                     len == 0 ? kZeroIsNotAnError : kZeroIsAnError,
+                     kSyscallError);
   ss->SetShutdownFlags();
 
   return scope.Close(Integer::New(bytes_written));
@@ -1725,10 +1747,13 @@ Handle<Value> Connection::Start(const Arguments& args) {
     int rv;
     if (ss->is_server_) {
       rv = SSL_accept(ss->ssl_);
-      ss->HandleSSLError("SSL_accept:Start", rv, kZeroIsAnError);
+      ss->HandleSSLError("SSL_accept:Start", rv, kZeroIsAnError, kSyscallError);
     } else {
       rv = SSL_connect(ss->ssl_);
-      ss->HandleSSLError("SSL_connect:Start", rv, kZeroIsAnError);
+      ss->HandleSSLError("SSL_connect:Start",
+                         rv,
+                         kZeroIsAnError,
+                         kSyscallError);
     }
 
     return scope.Close(Integer::New(rv));
@@ -1745,7 +1770,7 @@ Handle<Value> Connection::Shutdown(const Arguments& args) {
 
   if (ss->ssl_ == NULL) return False();
   int rv = SSL_shutdown(ss->ssl_);
-  ss->HandleSSLError("SSL_shutdown", rv, kZeroIsNotAnError);
+  ss->HandleSSLError("SSL_shutdown", rv, kZeroIsNotAnError, kIgnoreSyscall);
   ss->SetShutdownFlags();
 
   return scope.Close(Integer::New(rv));

diff --git a/src/node_crypto.h b/src/node_crypto.h
@@ -218,7 +218,12 @@ class Connection : ObjectWrap {
     kZeroIsAnError
   };
 
-  int HandleSSLError(const char* func, int rv, ZeroStatus zs);
+  enum SyscallStatus {
+    kIgnoreSyscall,
+    kSyscallError
+  };
+
+  int HandleSSLError(const char* func, int rv, ZeroStatus zs, SyscallStatus ss);
 
   void ClearError();
   void SetShutdownFlags();