]> git.ipfire.org Git - thirdparty/open-vm-tools.git/commitdiff
Fix the SSL/AsyncSocket "SSL_shutdown:shutdown while in init" problem
authorOliver Kurth <okurth@vmware.com>
Fri, 5 Jan 2018 22:47:15 +0000 (14:47 -0800)
committerOliver Kurth <okurth@vmware.com>
Fri, 5 Jan 2018 22:47:15 +0000 (14:47 -0800)
QE regression tests have caught a bug in the bora SSL/AsyncSocket
layer. A normal SSL connection could be abnormally terminated by some
other random connections.

Seems a common bug of the SSL usage according to the link below.
VMware is not the first one to hit it. :-)
https://marc.info/?l=openssl-users&w=4&r=1&s=shutdown+while+in+init&q=b

The cause is that the SSL error queue was not cleared before
calling the SSL IO functions like SSL_write,read,connect,accept.
Refer to the man page of SSL_get_error, Paragraph 2 of the Description.
Fixed the problem by adding addtional call to clear the error queue.

Found another problem that the system error is not preserved between
where it is set after the SSL IO call and the use of it to determine
whether to retry the SSL IO. This is probably due to tools has its
own logging which does not preserve the system error number.
Fixed the logging.

open-vm-tools/lib/asyncsocket/asyncsocket.c
open-vm-tools/lib/sslDirect/sslDirect.c

index 54c61d269cc2e22e0e04764142ab3586e8deb9e6..f5310d8a502a7b89fda0cb22d264ac83ef397925 100644 (file)
@@ -3309,10 +3309,13 @@ AsyncTCPSocketFillRecvBuffer(AsyncTCPSocket *s)         // IN
                           s->base.recvPos,
                           needed);
       }
-      TCPSOCKLOG(3, s, ("need\t%d\trecv\t%d\tremain\t%d\n", needed, recvd,
-                        needed - recvd));
-
+      /*
+       * Do NOT make any system call directly or indirectly here
+       * unless you can preserve the system error number
+       */
       if (recvd > 0) {
+         TCPSOCKLOG(3, s, ("need\t%d\trecv\t%d\tremain\t%d\n", needed, recvd,
+                           needed - recvd));
          s->sslConnected = TRUE;
          s->base.recvPos += recvd;
          if (AsyncSocketCheckAndDispatchRecv(&s->base, &result)) {
@@ -3477,10 +3480,13 @@ AsyncTCPSocketWriteBuffers(AsyncTCPSocket *s)         // IN
 
       sent = SSL_Write(s->sslSock,
                        (uint8 *) head->buf + s->sendPos, left);
-
-      TCPSOCKLOG(3, s, ("left\t%d\tsent\t%d\tremain\t%d\n",
-                      left, sent, left - sent));
+      /*
+       * Do NOT make any system call directly or indirectly here
+       * unless you can preserve the system error number
+       */
       if (sent > 0) {
+         TCPSOCKLOG(3, s, ("left\t%d\tsent\t%d\tremain\t%d\n",
+                        left, sent, left - sent));
          s->sendBufFull = FALSE;
          s->sslConnected = TRUE;
          if ((s->sendPos += sent) == sizeToSend) {
index b110987c8c2ecdcf1cd6bce12b3fc9d0555bc69b..c5e6d2942b2b8795ba3d0e12ab33053c8837ec4a 100644 (file)
@@ -52,7 +52,7 @@
 #include <openssl/rand.h>
 #include <openssl/engine.h>
 
-#define SSL_LOG(x) Debug x
+#define SSL_LOG(x) WITH_ERRNO(_e, Debug x)
 
 struct SSLSockStruct {
    SSL *sslCnx;
@@ -401,7 +401,14 @@ SSL_Read(SSLSock ssl,   // IN
    }
 
    if (ssl->encrypted) {
-      int result = SSL_read(ssl->sslCnx, buf, (int)num);
+      int result;
+
+      /*
+       * Need to clear the thread error queue before calling SSL_xxx function
+       * See bug 1982292 and man SSL_get_error(3) for details.
+       */
+      ERR_clear_error();
+      result = SSL_read(ssl->sslCnx, buf, (int)num);
 
       ssl->sslIOError = SSLSetErrorState(ssl->sslCnx, result);
       if (ssl->sslIOError != SSL_ERROR_NONE) {
@@ -500,7 +507,10 @@ SSL_RecvDataAndFd(SSLSock ssl,    // IN/OUT: socket
    return SSL_Read(ssl, buf, num);
 #else
    if (ssl->encrypted) {
-      int result = SSL_read(ssl->sslCnx, buf, (int)num);
+      int result;
+
+      ERR_clear_error();
+      result = SSL_read(ssl->sslCnx, buf, (int)num);
 
       ssl->sslIOError = SSLSetErrorState(ssl->sslCnx, result);
       if (ssl->sslIOError != SSL_ERROR_NONE) {
@@ -572,7 +582,10 @@ SSL_Write(SSLSock ssl,   // IN
       goto end;
    }
    if (ssl->encrypted) {
-      int result = SSL_write(ssl->sslCnx, buf, (int)num);
+      int result;
+
+      ERR_clear_error();
+      result = SSL_write(ssl->sslCnx, buf, (int)num);
 
       ssl->sslIOError = SSLSetErrorState(ssl->sslCnx, result);
       if (ssl->sslIOError != SSL_ERROR_NONE) {