WL#15524 Patch #2 TLS-safe upgrade of mgm socket to transporter

This changes TransporterRegistry::connect_ndb_mgmd() to return
NdbSocket rather than ndb_socket_t.

Back-ported from mysql-trunk.

Change-Id: Ic3b9ccf39ec78ed25705a4bbbdc5ac2953a35611

Author: jdduncan

storage/ndb/include/transporter/TransporterRegistry.hpp

@@ -261,26 +261,20 @@ class TransporterRegistry
                       bool& close_with_reset,
                       bool& log_failure);
 
-  bool connect_server(ndb_socket_t sockfd, BaseString & msg,
-                      bool & close_with_reset, bool & log_failure) {
-    NdbSocket sock(sockfd, NdbSocket::From::Existing);
-    return connect_server(sock, msg, close_with_reset, log_failure);
-  }
-
   bool connect_client(NdbMgmHandle *h);
 
   /**
    * Given a hostname and port, creates a NdbMgmHandle, turns it into
    * a transporter, and returns the socket.
    */
-  ndb_socket_t connect_ndb_mgmd(const char* server_name,
-                                unsigned short server_port);
+  NdbSocket connect_ndb_mgmd(const char* server_name,
+                             unsigned short server_port);
 
   /**
    * Given a connected NdbMgmHandle, turns it into a transporter
    * and returns the socket.
    */
-  ndb_socket_t connect_ndb_mgmd(NdbMgmHandle *h);
+  NdbSocket connect_ndb_mgmd(NdbMgmHandle *h);
 
   /**
    * Manage allTransporters and theNodeIdTransporters when using

storage/ndb/src/common/transporter/Transporter.cpp

@@ -259,11 +259,20 @@ Transporter::connect_server(NdbSocket & sockfd,
 }
 
 
+bool
+Transporter::connect_client_mgm(int port)
+{
+  require(!isPartOfMultiTransporter());
+  NdbSocket secureSocket =
+    m_transporter_registry.connect_ndb_mgmd(remoteHostName, port);
+  return connect_client(secureSocket);
+}
+
+
 bool
 Transporter::connect_client()
 {
   NdbSocket secureSocket;
-  ndb_socket_t sockfd;
   DBUG_ENTER("Transporter::connect_client");
 
   require(!isMultiTransporter());
@@ -284,9 +293,7 @@ Transporter::connect_client()
 
   if(isMgmConnection)
   {
-    require(!isPartOfMultiTransporter());
-    sockfd= m_transporter_registry.connect_ndb_mgmd(remoteHostName, port);
-    secureSocket.init_from_new(sockfd);
+    return connect_client_mgm(port);
   }
   else
   {

storage/ndb/src/common/transporter/Transporter.hpp

@@ -132,6 +132,7 @@ class Transporter {
     NdbSocket socket(fd, NdbSocket::From::Existing);
     return connect_client(socket);
   }
+  bool connect_client_mgm(int);
   bool connect_server(NdbSocket & socket, BaseString& errormsg);
 
   /**

storage/ndb/src/common/transporter/TransporterRegistry.cpp

@@ -3724,7 +3724,8 @@ bool TransporterRegistry::connect_client(NdbMgmHandle *h)
   }
   require(!t->isMultiTransporter());
   require(!t->isPartOfMultiTransporter());
-  bool res = t->connect_client(connect_ndb_mgmd(h));
+  NdbSocket secureSocket = connect_ndb_mgmd(h);
+  bool res = t->connect_client(secureSocket);
   if (res == true)
   {
     DEBUG_FPRINTF((stderr, "(%u)performStates[%u] = DISCONNECTING,"
@@ -3776,55 +3777,52 @@ bool TransporterRegistry::report_dynamic_ports(NdbMgmHandle h) const
  * Given a connected NdbMgmHandle, turns it into a transporter
  * and returns the socket.
  */
-ndb_socket_t TransporterRegistry::connect_ndb_mgmd(NdbMgmHandle *h)
+NdbSocket TransporterRegistry::connect_ndb_mgmd(NdbMgmHandle *h)
 {
-  ndb_socket_t sockfd;
-
   DBUG_ENTER("TransporterRegistry::connect_ndb_mgmd(NdbMgmHandle)");
 
   if ( h==nullptr || *h == nullptr )
   {
     g_eventLogger->error("Mgm handle is NULL (%s:%d)", __FILE__, __LINE__);
-    DBUG_RETURN(sockfd);
+    DBUG_RETURN(NdbSocket{});  // an invalid socket, newly created on the stack
   }
 
   if (!report_dynamic_ports(*h))
   {
     ndb_mgm_destroy_handle(h);
-    DBUG_RETURN(sockfd);
+    DBUG_RETURN(NdbSocket{});  // an invalid socket, newly created on the stack
   }
 
   /**
    * convert_to_transporter also disposes of the handle (i.e. we don't leak
-   * memory here.
+   * memory here).
    */
   DBUG_PRINT("info", ("Converting handle to transporter"));
-  sockfd= ndb_mgm_convert_to_transporter(h);
-  if (!ndb_socket_valid(sockfd))
+  NdbSocket socket = ndb_mgm_convert_to_transporter(h);
+  if (! socket.is_valid())
   {
     g_eventLogger->error("Failed to convert to transporter (%s: %d)",
                          __FILE__, __LINE__);
     ndb_mgm_destroy_handle(h);
   }
-  DBUG_RETURN(sockfd);
+  DBUG_RETURN(socket);
 }
 
 /**
  * Given a SocketClient, creates a NdbMgmHandle, turns it into a transporter
  * and returns the socket.
  */
-ndb_socket_t
+NdbSocket
 TransporterRegistry::connect_ndb_mgmd(const char* server_name,
                                       unsigned short server_port)
 {
   NdbMgmHandle h= ndb_mgm_create_handle();
-  ndb_socket_t s;
 
   DBUG_ENTER("TransporterRegistry::connect_ndb_mgmd(SocketClient)");
 
   if ( h == nullptr )
   {
-    DBUG_RETURN(s);
+    DBUG_RETURN(NdbSocket());
   }
 
   /**
@@ -3845,7 +3843,7 @@ TransporterRegistry::connect_ndb_mgmd(const char* server_name,
   {
     DBUG_PRINT("info", ("connection to mgmd failed"));
     ndb_mgm_destroy_handle(&h);
-    DBUG_RETURN(s);
+    DBUG_RETURN(NdbSocket());
   }
 
   DBUG_RETURN(connect_ndb_mgmd(&h));

storage/ndb/src/mgmapi/mgmapi.cpp

@@ -3631,43 +3631,36 @@ ndb_mgm_get_connection_int_parameter(NdbMgmHandle handle,
   DBUG_RETURN(res);
 }
 
-ndb_socket_t
+NdbSocket
 ndb_mgm_convert_to_transporter(NdbMgmHandle *handle)
 {
-  DBUG_ENTER("ndb_mgm_convert_to_transporter");
-  ndb_socket_t s;
-
   if(handle == nullptr)
   {
     SET_ERROR(*handle, NDB_MGM_ILLEGAL_SERVER_HANDLE, "");
-    ndb_socket_invalidate(&s);
-    DBUG_RETURN(s);
+    return {};
   }
 
   if ((*handle)->connected != 1)
   {
     SET_ERROR(*handle, NDB_MGM_SERVER_NOT_CONNECTED , "");
-    ndb_socket_invalidate(&s);
-    DBUG_RETURN(s);
+    return {};
   }
 
+  // FIXME: Decide whether to keep this
   if ((*handle)->socket.has_tls())
   {
     SET_ERROR(*handle, NDB_MGM_CANNOT_CONVERT_TO_TRANSPORTER, "");
-    ndb_socket_invalidate(&s);
-    DBUG_RETURN(s);
+    return {};
   }
 
-  (*handle)->connected= 0;   // we pretend we're disconnected
-  s= (*handle)->socket.ndb_socket();
-
-  SocketOutputStream s_output(s, (*handle)->timeout);
+  NdbSocket s = std::move((*handle)->socket);
+  SecureSocketOutputStream s_output(s, (*handle)->timeout);
   s_output.println("transporter connect");
   s_output.println("%s", "");
 
+  (*handle)->connected= 0;   // The handle no longer owns the connection
   ndb_mgm_destroy_handle(handle); // set connected=0, so won't disconnect
-
-  DBUG_RETURN(s);
+  return s;
 }
 
 extern "C"

storage/ndb/src/mgmapi/mgmapi_internal.h

@@ -26,6 +26,7 @@
 #define MGMAPI_INTERNAL_H
 
 #include "portlib/ndb_socket.h"
+#include "util/NdbSocket.h"
 
 
 /**
@@ -86,11 +87,10 @@ int ndb_mgm_get_connection_int_parameter(NdbMgmHandle handle,
  * Convert connection to transporter
  * @param   handle    NDB management handle.
  *
- * @return socket
- *
  * @note the socket is now able to be used as a transporter connection
+ * @note the management handle is no longer valid after this call
  */
-ndb_socket_t ndb_mgm_convert_to_transporter(NdbMgmHandle *handle);
+NdbSocket ndb_mgm_convert_to_transporter(NdbMgmHandle *handle);
 
 int ndb_mgm_disconnect_quiet(NdbMgmHandle handle);
 

storage/ndb/src/mgmsrv/MgmtSrvr.cpp

@@ -5123,14 +5123,14 @@ MgmtSrvr::getConnectionDbParameter(int node1, int node2,
 
 
 bool
-MgmtSrvr::transporter_connect(ndb_socket_t sockfd,
+MgmtSrvr::transporter_connect(NdbSocket & socket,
                               BaseString& msg,
                               bool& close_with_reset,
                               bool& log_failure)
 {
   DBUG_ENTER("MgmtSrvr::transporter_connect");
   TransporterRegistry* tr= theFacade->get_registry();
-  if (!tr->connect_server(sockfd, msg, close_with_reset, log_failure))
+  if (!tr->connect_server(socket, msg, close_with_reset, log_failure))
     DBUG_RETURN(false);
 
   /**

storage/ndb/src/mgmsrv/MgmtSrvr.hpp

@@ -356,7 +356,7 @@ class MgmtSrvr : private ConfigSubscriber, public trp_client {
   int getConnectionDbParameter(int node1, int node2, int param,
 			       int *value, BaseString& msg);
 
-  bool transporter_connect(ndb_socket_t sockfd,
+  bool transporter_connect(NdbSocket & socket,
                            BaseString& errormsg,
                            bool& close_with_reset,
                            bool& log_failure);

storage/ndb/src/mgmsrv/Services.cpp

@@ -1895,8 +1895,8 @@ MgmApiSession::transporter_connect(Parser_t::Context &ctx,
   bool close_with_reset = true;
   bool log_failure = false;
   BaseString errormsg;
-  if (!m_mgmsrv.transporter_connect(m_secure_socket.ndb_socket(), errormsg,
-                                    close_with_reset, log_failure))
+  if (!m_mgmsrv.transporter_connect(m_secure_socket,
+                                    errormsg, close_with_reset, log_failure))
   {
     // Connection not allowed or failed
     if (log_failure)
@@ -1918,7 +1918,7 @@ MgmApiSession::transporter_connect(Parser_t::Context &ctx,
       but don't close the socket, it's been taken over
       by the transporter
     */
-    m_secure_socket.invalidate();  // so nobody closes it
+    NdbSocket s = std::move(m_secure_socket);
   }
 
   m_stop= true; // Stop the session

storage/ndb/test/include/NdbMgmd.hpp

@@ -105,6 +105,10 @@ class NdbMgmd {
     return m_handle;
   }
 
+  NdbSocket convert_to_transporter() {
+    return ndb_mgm_convert_to_transporter(& m_handle);
+  }
+
   ndb_socket_t socket(void) const {
     return _ndb_mgm_get_socket(m_handle);
   }