WL#15154 patch #2 Preliminary SocketAuthenticator refactoring

Move client_authenticate() out of SocketClient::connect() (which
returns void) into a separate SocketClient::authenticate() method
which can return a value.

In SocketAuthenticator, change the signature of the authentication
routines to return an int (which can represent a result code) rather
than a bool. Results less than AuthOk represent failure, and results
greater than or equal to AuthOk represent success.

Remove the username and password variables from SocketAuthSimple;
make them constant strings in the implementation.

There are no functional changes.

Change-Id: I4c25e99f1b9b692db39213dfa63352da8993a8fb

Author: jdduncan

storage/ndb/include/util/SocketAuthenticator.hpp

@@ -27,25 +27,31 @@
 
 #include "util/NdbSocket.h"
 
+/* client_authenticate() and server_authenticate() return a value
+   less than AuthOk on failure. They return a value greater than or
+   equal to AuthOk on success.
+*/
+
 class SocketAuthenticator
 {
 public:
   SocketAuthenticator() {}
   virtual ~SocketAuthenticator() {}
-  virtual bool client_authenticate(NdbSocket &) = 0;
-  virtual bool server_authenticate(NdbSocket &) = 0;
+  virtual int client_authenticate(NdbSocket &) = 0;
+  virtual int server_authenticate(NdbSocket &) = 0;
+
+  static constexpr int AuthOk = 0;
+  static const char * error(int);   // returns error message for code
 };
 
 
 class SocketAuthSimple : public SocketAuthenticator
 {
-  char *m_passwd;
-  char *m_username;
 public:
-  SocketAuthSimple(const char *username, const char *passwd);
-  ~SocketAuthSimple() override;
-  bool client_authenticate(NdbSocket &) override;
-  bool server_authenticate(NdbSocket &) override;
+  SocketAuthSimple() {}
+  ~SocketAuthSimple() override {}
+  int client_authenticate(NdbSocket &) override;
+  int server_authenticate(NdbSocket &) override;
 };
 
 

storage/ndb/include/util/SocketClient.hpp

@@ -46,6 +46,7 @@ class SocketClient
   int bind(ndb_sockaddr local);
   ndb_socket_t connect(ndb_sockaddr server_addr);
   void connect(NdbSocket &, ndb_sockaddr server_addr);
+  int authenticate(NdbSocket &);
 
   ndb_socket_t m_sockfd;
 };

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

@@ -145,9 +145,7 @@ Transporter::Transporter(TransporterRegistry &t_reg,
     m_socket_client= nullptr;
   else
   {
-    m_socket_client= new SocketClient(new SocketAuthSimple("ndbd",
-                                                           "ndbd passwd"));
-
+    m_socket_client= new SocketClient(new SocketAuthSimple());
     m_socket_client->set_connect_timeout(m_timeOutMillis);
   }
 
@@ -335,6 +333,14 @@ Transporter::connect_client()
       }
     }
     m_socket_client->connect(secureSocket, remote_addr);
+
+   /** Socket Authentication */
+    if(m_socket_client->authenticate(secureSocket) < SocketAuthSimple::AuthOk)
+    {
+      DEBUG_FPRINTF((stderr, "Socket Authenticator failed\n"));
+      DBUG_RETURN(false);
+    }
+
   }
 
   DBUG_RETURN(connect_client(secureSocket));

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

@@ -117,19 +117,22 @@ TransporterRegistry::get_bytes_received(NodeId node_id) const
 SocketServer::Session * TransporterService::newSession(ndb_socket_t sockfd)
 {
   /* The connection is currently running over a plain network socket.
-     If m_auth is a TlsAuthenticator, it might get upgraded to a TLS socket
-     in server_authenticate().
+     If m_auth is a SocketAuthTls, it might get upgraded to a TLS socket.
   */
   NdbSocket secureSocket;
   secureSocket.init_from_new(sockfd);
 
   DBUG_ENTER("SocketServer::Session * TransporterService::newSession");
   DEBUG_FPRINTF((stderr, "New session created\n"));
-  if (m_auth && !m_auth->server_authenticate(secureSocket))
+  if(m_auth)
   {
-    DEBUG_FPRINTF((stderr, "Failed to authenticate new session\n"));
-    secureSocket.close_with_reset(true); // Close with reset
-    DBUG_RETURN(0);
+    int r = m_auth->server_authenticate(secureSocket);
+    if(r < SocketAuthenticator::AuthOk)
+    {
+      DEBUG_FPRINTF((stderr, "Failed to authenticate new session\n"));
+      secureSocket.close_with_reset(true); // Close with reset
+      DBUG_RETURN(0);
+    }
   }
 
   BaseString msg;
@@ -3595,7 +3598,7 @@ TransporterRegistry::start_service(SocketServer& socket_server)
     if(t.m_s_service_port<0)
       port= -t.m_s_service_port; // is a dynamic port
     TransporterService *transporter_service =
-      new TransporterService(new SocketAuthSimple("ndbd", "ndbd passwd"));
+      new TransporterService(new SocketAuthSimple());
     ndb_sockaddr addr;
     if (t.m_interface && Ndb_getAddr(&addr, t.m_interface))
     {

storage/ndb/src/common/util/SocketAuthenticator.cpp

@@ -26,49 +26,36 @@
 #include <SocketAuthenticator.hpp>
 #include <InputStream.hpp>
 #include <OutputStream.hpp>
-SocketAuthSimple::SocketAuthSimple(const char *username, const char *passwd) {
-  if (username)
-    m_username= strdup(username);
-  else
-    m_username= nullptr;
-  if (passwd)
-    m_passwd= strdup(passwd);
-  else
-    m_passwd= nullptr;
-}
 
-SocketAuthSimple::~SocketAuthSimple()
+const char * SocketAuthenticator::error(int result)
 {
-  if (m_passwd)
-    free(m_passwd);
-  if (m_username)
-    free(m_username);
+  return (result < AuthOk) ? "Socket Auth failure" : "Success";
 }
 
-bool SocketAuthSimple::client_authenticate(NdbSocket & sockfd)
+int SocketAuthSimple::client_authenticate(NdbSocket & sockfd)
 {
   SecureSocketOutputStream s_output(sockfd);
   SecureSocketInputStream  s_input(sockfd);
 
   // Write username and password
-  s_output.println("%s", m_username ? m_username : "");
-  s_output.println("%s", m_passwd ? m_passwd : "");
+  s_output.println("ndbd");
+  s_output.println("ndbd passwd");
 
   char buf[16];
 
   // Read authentication result
   if (s_input.gets(buf, sizeof(buf)) == nullptr)
-    return false;
+    return -1;
   buf[sizeof(buf)-1]= 0;
 
   // Verify authentication result
   if (strncmp("ok", buf, 2) == 0)
-    return true;
+    return 0;
 
-  return false;
+  return -1;
 }
 
-bool SocketAuthSimple::server_authenticate(NdbSocket & sockfd)
+int SocketAuthSimple::server_authenticate(NdbSocket & sockfd)
 {
   SecureSocketOutputStream s_output(sockfd);
   SecureSocketInputStream  s_input(sockfd);
@@ -77,17 +64,17 @@ bool SocketAuthSimple::server_authenticate(NdbSocket & sockfd)
 
   // Read username
   if (s_input.gets(buf, sizeof(buf)) == nullptr)
-    return false;
+    return -1;
   buf[sizeof(buf)-1]= 0;
 
   // Read password
   if (s_input.gets(buf, sizeof(buf)) == nullptr)
-    return false;
+    return -1;
   buf[sizeof(buf)-1]= 0;
 
   // Write authentication result
   s_output.println("ok");
 
-  return true;
+  return 0;
 }
 

storage/ndb/src/common/util/SocketClient.cpp

@@ -26,6 +26,8 @@
 #include <ndb_global.h>
 
 #include <cassert>
+
+#include "EventLogger.hpp"
 #include "SocketClient.hpp"
 #include "SocketAuthenticator.hpp"
 #include "portlib/ndb_socket_poller.h"
@@ -209,16 +211,22 @@ SocketClient::connect(NdbSocket & secureSocket,
   assert(m_last_used_port == 0);
   ndb_socket_get_port(m_sockfd, &m_last_used_port);
 
+  // Transfer the fd to the NdbSocket
   secureSocket.init_from_new(m_sockfd);
+  ndb_socket_invalidate(&m_sockfd);
+}
 
-  if (m_auth) {
-    if (!m_auth->client_authenticate(secureSocket))
-    {
-      DEBUG_FPRINTF((stderr, "authenticate failed in connect\n"));
-      secureSocket.close();
-      secureSocket.invalidate();
-    }
+int
+SocketClient::authenticate(NdbSocket & secureSocket)
+{
+  assert(m_auth);
+  int r = m_auth->client_authenticate(secureSocket);
+  if (r < SocketAuthenticator::AuthOk)
+  {
+    g_eventLogger->error("Socket authentication failed: %s\n",
+                         m_auth->error(r));
+    secureSocket.close();
+    secureSocket.invalidate();
   }
-
-  ndb_socket_invalidate(&m_sockfd);
+  return r;
 }