MXS-862: Refactor backend authentication handling

The backend responses are now read in one place and the functions just
read the data. The protocol level will now handle the packet gathering
process and the authentication part just inspects the data.

Backend connections now load authenticators when they are being
connected. In the future, this enables the use of authentication modules
for backend connection.

server/core/config.c

@@ -196,6 +196,7 @@ static char *server_params[] =
     "protocol",
     "port",
     "address",
+    "authenticator",
     "monitoruser",
     "monitorpw",
     "persistpoolmax",
@@ -2504,6 +2505,7 @@ int create_new_server(CONFIG_CONTEXT *obj)
     char *protocol = config_get_value(obj->parameters, "protocol");
     char *monuser = config_get_value(obj->parameters, "monitoruser");
     char *monpw = config_get_value(obj->parameters, "monitorpw");
+    char *auth = config_get_value(obj->parameters, "authenticator");
 
     if (address && port && protocol)
     {
@@ -2540,6 +2542,11 @@ int create_new_server(CONFIG_CONTEXT *obj)
             error_count++;
         }
 
+        if (auth && (server->authenticator = MXS_STRDUP(auth)) == NULL)
+        {
+            error_count++;
+        }
+
         char *endptr;
         const char *poolmax = config_get_value_string(obj->parameters, "persistpoolmax");
         if (poolmax)

server/core/dcb.c

@@ -793,6 +793,22 @@ dcb_connect(SERVER *server, SESSION *session, const char *protocol)
     memcpy(&(dcb->func), funcs, sizeof(GWPROTOCOL));
     dcb->protoname = MXS_STRDUP_A(protocol);
 
+    const char *authenticator = server->authenticator ?
+        server->authenticator : dcb->func.auth_default ?
+        dcb->func.auth_default() : "NullAuthDeny";
+
+    GWAUTHENTICATOR *authfuncs = (GWAUTHENTICATOR*)load_module(authenticator,
+                                                               MODULE_AUTHENTICATOR);
+    if (authfuncs == NULL)
+    {
+
+        MXS_ERROR("Failed to load authenticator module '%s'.", authenticator);
+        dcb_close(dcb);
+        return NULL;
+    }
+
+    memcpy(&dcb->authfunc, authfuncs, sizeof(GWAUTHENTICATOR));
+
     /**
      * Link dcb to session. Unlink is called in dcb_final_free
      */
@@ -3096,7 +3112,7 @@ dcb_accept(DCB *listener, GWPROTOCOL *protocol_funcs)
         }
         else
         {
-            const char *authenticator_name = "NullAuth";
+            const char *authenticator_name = "NullAuthDeny";
             GWAUTHENTICATOR *authfuncs;
 
             client_dcb->service = listener->session->service;
@@ -3140,7 +3156,7 @@ dcb_accept(DCB *listener, GWPROTOCOL *protocol_funcs)
             if ((authfuncs = (GWAUTHENTICATOR *)load_module(authenticator_name,
                                                             MODULE_AUTHENTICATOR)) == NULL)
             {
-                if ((authfuncs = (GWAUTHENTICATOR *)load_module("NullAuth",
+                if ((authfuncs = (GWAUTHENTICATOR *)load_module("NullAuthDeny",
                                                                 MODULE_AUTHENTICATOR)) == NULL)
                 {
                     MXS_ERROR("Failed to load authenticator module for %s, free dcb %p\n",

server/core/server.c

@@ -83,6 +83,7 @@ server_alloc(char *servname, char *protocol, unsigned short port)
 #endif
     server->name = servname;
     server->protocol = protocol;
+    server->authenticator = NULL;
     server->port = port;
     server->status = SERVER_RUNNING;
     server->node_id = -1;

server/include/gw_authenticator.h

@@ -77,6 +77,29 @@ typedef struct gw_authenticator
 #define MXS_AUTH_LOADUSERS_OK    0 /**< Users loaded successfully */
 #define MXS_AUTH_LOADUSERS_ERROR 1 /**< Failed to load users */
 
+/**
+ * Authentication states
+ *
+ * The state usually goes from INIT to CONNECTED and alternates between
+ * MESSAGE_READ and RESPONSE_SENT until ending up in either FAILED or COMPLETE.
+ *
+ * If the server immediately rejects the connection, the state ends up in
+ * HANDSHAKE_FAILED. If the connection creation would block, instead of going to
+ * the CONNECTED state, the connection will be in PENDING_CONNECT state until
+ * the connection can be created.
+ */
+typedef enum
+{
+    MXS_AUTH_STATE_INIT, /**< Initial authentication state */
+    MXS_AUTH_STATE_PENDING_CONNECT,/**< Connection creation is underway */
+    MXS_AUTH_STATE_CONNECTED, /**< Network connection to server created */
+    MXS_AUTH_STATE_MESSAGE_READ, /**< Read a authentication message from the server */
+    MXS_AUTH_STATE_RESPONSE_SENT, /**< Responded to the read authentication message */
+    MXS_AUTH_STATE_FAILED, /**< Authentication failed */
+    MXS_AUTH_STATE_HANDSHAKE_FAILED, /**< Authentication failed immediately */
+    MXS_AUTH_STATE_COMPLETE /**< Authentication is complete */
+} mxs_auth_state_t;
+
 /**
  * The GWAUTHENTICATOR version data. The following should be updated whenever
  * the GWAUTHENTICATOR structure is changed. See the rules defined in modinfo.h

server/include/server.h

@@ -86,6 +86,7 @@ typedef struct server
     char           *name;          /**< Server name/IP address*/
     unsigned short port;           /**< Port to listen on */
     char           *protocol;      /**< Protocol module to use */
+    char           *authenticator; /**< Authenticator module name */
     SSL_LISTENER   *server_ssl;    /**< SSL data structure for server, if any */
     unsigned int   status;         /**< Status flag bitmap for the server */
     char           *monuser;       /**< User name to use to monitor the db */

server/include/skygw_debug.h

@@ -213,14 +213,14 @@ typedef enum skygw_chk_t
                                  (s) == SESSION_STATE_STOPPING ? "SESSION_STATE_STOPPING":\
                                   "SESSION_STATE_UNKNOWN"))))))
 
-#define STRPROTOCOLSTATE(s) ((s) == MYSQL_ALLOC ? "MYSQL_ALLOC" :       \
+#define STRPROTOCOLSTATE(s) ((s) == MXS_AUTH_STATE_INIT ? "MXS_AUTH_STATE_INIT" : \
-        ((s) == MYSQL_PENDING_CONNECT ? "MYSQL_PENDING_CONNECT" :       \
+                            ((s) == MXS_AUTH_STATE_PENDING_CONNECT ? "MXS_AUTH_STATE_PENDING_CONNECT" : \
-        ((s) == MYSQL_CONNECTED ? "MYSQL_CONNECTED" :                   \
+                            ((s) == MXS_AUTH_STATE_CONNECTED ? "MXS_AUTH_STATE_CONNECTED" : \
-        ((s) == MYSQL_AUTH_SENT ? "MYSQL_AUTH_SENT" :                   \
+                            ((s) == MXS_AUTH_STATE_MESSAGE_READ ? "MXS_AUTH_STATE_MESSAGE_READ" : \
-        ((s) == MYSQL_AUTH_RECV ? "MYSQL_AUTH_RECV" :                   \
+                            ((s) == MXS_AUTH_STATE_RESPONSE_SENT ? "MXS_AUTH_STATE_RESPONSE_SENT" : \
-        ((s) == MYSQL_AUTH_FAILED ? "MYSQL_AUTH_FAILED" :               \
+                            ((s) == MXS_AUTH_STATE_FAILED ? "MXS_AUTH_STATE_FAILED" : \
-        ((s) == MYSQL_IDLE ? "MYSQL_IDLE" :                             \
+                            ((s) == MXS_AUTH_STATE_COMPLETE ? "MXS_AUTH_STATE_COMPLETE" : \
-        "UNKNOWN MYSQL STATE")))))))
+                            "UNKNOWN AUTH STATE")))))))
 
 #define STRITEMTYPE(t) ((t) == Item::FIELD_ITEM ? "FIELD_ITEM" :      \
         ((t) == Item::FUNC_ITEM ? "FUNC_ITEM" :                       \

server/modules/include/mysql_client_server_protocol.h

@@ -79,7 +79,7 @@
 #define GW_MYSQL_SCRAMBLE_SIZE 20
 #define GW_SCRAMBLE_LENGTH_323 8
 
-#define DEFAULT_AUTH_PLUGIN_NAME "mysql_native_password"
+#define DEFAULT_MYSQL_AUTH_PLUGIN "mysql_native_password"
 
 /** Maximum length of a MySQL packet */
 #define MYSQL_PACKET_LENGTH_MAX 0x00ffffff
@@ -97,28 +97,6 @@
 #define COM_QUIT_PACKET_SIZE (4+1)
 struct dcb;
 
-typedef enum
-{
-    MYSQL_ALLOC,                        /* Initial state of protocol auth state */
-    /* The following are used only for backend connections */
-    MYSQL_PENDING_CONNECT,
-    MYSQL_CONNECTED,
-    /* The following can be used for either client or backend */
-    /* The comments have only been checked for client use at present */
-    MYSQL_AUTH_SENT,
-    MYSQL_AUTH_RECV,                    /* This is only ever a transient value */
-    MYSQL_AUTH_FAILED,                  /* Once this is set, the connection */
-    /* will be ended, so this is transient */
-    /* The following is used only for backend connections */
-    MYSQL_HANDSHAKE_FAILED,
-    /* The following are obsolete and will be removed */
-    MYSQL_AUTH_SSL_REQ, /*< client requested SSL but SSL_accept hasn't beed called */
-    MYSQL_AUTH_SSL_HANDSHAKE_DONE, /*< SSL handshake has been fully completed */
-    MYSQL_AUTH_SSL_HANDSHAKE_FAILED, /*< SSL handshake failed for any reason */
-    MYSQL_AUTH_SSL_HANDSHAKE_ONGOING, /*< SSL_accept has been called but the
-                                           * SSL handshake hasn't been completed */
-    MYSQL_IDLE
-} mysql_auth_state_t;
 
 typedef enum
 {
@@ -272,7 +250,7 @@ typedef struct
     mysql_server_cmd_t  current_command;              /**< Current command being executed */
     server_command_t    protocol_command;             /*< session command list */
     server_command_t*   protocol_cmd_history;         /*< session command history */
-    mysql_auth_state_t  protocol_auth_state;          /*< Authentication status */
+    mxs_auth_state_t  protocol_auth_state;            /*< Authentication status */
     mysql_protocol_state_t protocol_state;            /*< Protocol struct status */
     uint8_t             scramble[MYSQL_SCRAMBLE_LEN]; /*< server scramble,
         * created or received */
@@ -288,6 +266,9 @@ typedef struct
 #endif
 } MySQLProtocol;
 
+/** Defines for response codes */
+#define MYSQL_REPLY_ERR 0xff
+#define MYSQL_REPLY_OK  0x00
 
 /*
  * Let's try this with proper enums instead of numbers
@@ -307,7 +288,7 @@ typedef struct
 #define MYSQL_GET_ERRCODE(payload)              (gw_mysql_get_byte2(&payload[5]))
 #define MYSQL_GET_STMTOK_NPARAM(payload)        (gw_mysql_get_byte2(&payload[9]))
 #define MYSQL_GET_STMTOK_NATTR(payload)         (gw_mysql_get_byte2(&payload[11]))
-#define MYSQL_IS_ERROR_PACKET(payload)          ((int)MYSQL_GET_COMMAND(payload)==0xff)
+#define MYSQL_IS_ERROR_PACKET(payload)          ((int)MYSQL_GET_COMMAND(payload)==MYSQL_REPLY_ERR)
 #define MYSQL_IS_COM_QUIT(payload)              (MYSQL_GET_COMMAND(payload)==MYSQL_COM_QUIT)
 #define MYSQL_IS_COM_INIT_DB(payload)           (MYSQL_GET_COMMAND(payload)==MYSQL_COM_INIT_DB)
 #define MYSQL_IS_CHANGE_USER(payload)       (MYSQL_GET_COMMAND(payload)==MYSQL_COM_CHANGE_USER)
@@ -383,5 +364,6 @@ void init_response_status (
     mysql_server_cmd_t cmd,
     int* npackets,
     ssize_t* nbytes);
+bool read_complete_packet(DCB *dcb, GWBUF **readbuf);
 
 #endif /** _MYSQL_PROTOCOL_H */

server/modules/protocol/MySQLClient/mysql_client.c

@@ -312,7 +312,7 @@ int MySQLSendHandshake(DCB* dcb)
     memcpy(mysql_plugin_data, server_scramble + 8, 12);
 
     const char* plugin_name = dcb->authfunc.plugin_name ?
-        dcb->authfunc.plugin_name : DEFAULT_AUTH_PLUGIN_NAME;
+        dcb->authfunc.plugin_name : DEFAULT_MYSQL_AUTH_PLUGIN;
     int plugin_name_len = strlen(plugin_name);
 
     mysql_payload_size =
@@ -512,7 +512,7 @@ int gw_read_client_event(DCB* dcb)
          * will be changed to MYSQL_IDLE (see below).
          *
          */
-    case MYSQL_AUTH_SENT:
+    case MXS_AUTH_STATE_MESSAGE_READ:
         /* After this call read_buffer will point to freed data */
         if (nbytes_read < 3 || (0 == max_bytes && nbytes_read <
             (MYSQL_GET_PACKET_LEN((uint8_t *) GWBUF_DATA(read_buffer)) + 4)) ||
@@ -533,12 +533,12 @@ int gw_read_client_event(DCB* dcb)
          * result in a call that comes to this section of code.
          *
          */
-    case MYSQL_IDLE:
+    case MXS_AUTH_STATE_COMPLETE:
         /* After this call read_buffer will point to freed data */
         return_code = gw_read_normal_data(dcb, read_buffer, nbytes_read);
         break;
 
-    case MYSQL_AUTH_FAILED:
+    case MXS_AUTH_STATE_FAILED:
         gwbuf_free(read_buffer);
         return_code = 1;
         break;
@@ -601,7 +601,7 @@ gw_read_do_authentication(DCB *dcb, GWBUF *read_buffer, int nbytes_read)
     {
         SESSION *session;
 
-        protocol->protocol_auth_state = MYSQL_AUTH_RECV;
+        protocol->protocol_auth_state = MXS_AUTH_STATE_RESPONSE_SENT;
         /**
          * Create session, and a router session for it.
          * If successful, there will be backend connection(s)
@@ -619,7 +619,7 @@ gw_read_do_authentication(DCB *dcb, GWBUF *read_buffer, int nbytes_read)
             ss_dassert(session->state != SESSION_STATE_ALLOC &&
                 session->state != SESSION_STATE_DUMMY);
 
-            protocol->protocol_auth_state = MYSQL_IDLE;
+            protocol->protocol_auth_state = MXS_AUTH_STATE_COMPLETE;
             /**
              * Send an AUTH_OK packet to the client,
              * packet sequence is # packet_number
@@ -640,7 +640,7 @@ gw_read_do_authentication(DCB *dcb, GWBUF *read_buffer, int nbytes_read)
         MXS_AUTH_INCOMPLETE != auth_val &&
         MXS_AUTH_SSL_INCOMPLETE != auth_val)
     {
-        protocol->protocol_auth_state = MYSQL_AUTH_FAILED;
+        protocol->protocol_auth_state = MXS_AUTH_STATE_FAILED;
         mysql_client_auth_error_handling(dcb, auth_val);
         /**
          * Close DCB and which will release MYSQL_session
@@ -1106,7 +1106,7 @@ int gw_write_client_event(DCB *dcb)
     protocol = (MySQLProtocol *)dcb->protocol;
     CHK_PROTOCOL(protocol);
 
-    if (protocol->protocol_auth_state == MYSQL_IDLE)
+    if (protocol->protocol_auth_state == MXS_AUTH_STATE_COMPLETE)
     {
         dcb_drain_writeq(dcb);
         goto return_1;
@@ -1207,7 +1207,7 @@ static void gw_process_one_new_client(DCB *client_dcb)
     MySQLSendHandshake(client_dcb);
 
     // client protocol state change
-    protocol->protocol_auth_state = MYSQL_AUTH_SENT;
+    protocol->protocol_auth_state = MXS_AUTH_STATE_MESSAGE_READ;
 
     /**
      * Set new descriptor to event set. At the same time,

server/modules/protocol/mysql_common.c

@@ -50,6 +50,7 @@
 #include <skygw_utils.h>
 #include <log_manager.h>
 #include <netinet/tcp.h>
+#include <modutil.h>
 
 static server_command_t* server_command_init(server_command_t* srvcmd, mysql_server_cmd_t cmd);
 
@@ -78,7 +79,7 @@ MySQLProtocol* mysql_protocol_init(DCB* dcb, int fd)
         goto return_p;
     }
     p->protocol_state = MYSQL_PROTOCOL_ALLOC;
-    p->protocol_auth_state = MYSQL_ALLOC;
+    p->protocol_auth_state = MXS_AUTH_STATE_INIT;
     p->current_command = MYSQL_COM_UNDEFINED;
     p->protocol_command.scom_cmd = MYSQL_COM_UNDEFINED;
     p->protocol_command.scom_nresponse_packets = 0;
@@ -143,28 +144,20 @@ const char* gw_mysql_protocol_state2string (int state)
 {
     switch(state)
     {
-    case MYSQL_ALLOC:
+    case MXS_AUTH_STATE_INIT:
-        return "MySQL Protocl struct allocated";
+        return "Authentication initialized";
-    case MYSQL_PENDING_CONNECT:
+    case MXS_AUTH_STATE_PENDING_CONNECT:
-        return "MySQL Backend socket PENDING connect";
+        return "Network connection pending";
-    case MYSQL_CONNECTED:
+    case MXS_AUTH_STATE_CONNECTED:
-        return "MySQL Backend socket CONNECTED";
+        return "Network connection created";
-    case MYSQL_AUTH_SENT:
+    case MXS_AUTH_STATE_MESSAGE_READ:
-        return "MySQL Authentication handshake has been sent";
+        return "Read server handshake";
-    case MYSQL_AUTH_RECV:
+    case MXS_AUTH_STATE_RESPONSE_SENT:
-        return "MySQL Received user, password, db and capabilities";
+        return "Response to handshake sent";
-    case MYSQL_AUTH_FAILED:
+    case MXS_AUTH_STATE_FAILED:
-        return "MySQL Authentication failed";
+        return "Authentication failed";
-    case MYSQL_IDLE:
+    case MXS_AUTH_STATE_COMPLETE:
-        return "MySQL authentication is succesfully done.";
+        return "Authentication is complete.";
-    case MYSQL_AUTH_SSL_REQ:
-        return "MYSQL_AUTH_SSL_REQ";
-    case MYSQL_AUTH_SSL_HANDSHAKE_DONE:
-        return "MYSQL_AUTH_SSL_HANDSHAKE_DONE";
-    case MYSQL_AUTH_SSL_HANDSHAKE_FAILED:
-        return "MYSQL_AUTH_SSL_HANDSHAKE_FAILED";
-    case MYSQL_AUTH_SSL_HANDSHAKE_ONGOING:
-        return "MYSQL_AUTH_SSL_HANDSHAKE_ONGOING";
     default:
         return "MySQL (unknown protocol state)";
     }
@@ -992,3 +985,43 @@ char *create_auth_fail_str(char *username,
 retblock:
     return errstr;
 }
+
+/**
+ * @brief Read a complete packet from a DCB
+ *
+ * Read a complete packet from a connected DCB. If data was read, @c readbuf
+ * will point to the head of the read data. If no data was read, @c readbuf will
+ * be set to NULL.
+ *
+ * @param dcb DCB to read from
+ * @param readbuf Pointer to a buffer where the data is stored
+ * @return True on success, false if an error occurred while data was being read
+ */
+bool read_complete_packet(DCB *dcb, GWBUF **readbuf)
+{
+    bool rval = false;
+    GWBUF *localbuf = NULL;
+
+    if (dcb_read(dcb, &localbuf, 0) >= 0)
+    {
+        rval = true;
+        dcb->last_read = hkheartbeat;
+        GWBUF *packets = modutil_get_complete_packets(&localbuf);
+
+        if (packets)
+        {
+            /** A complete packet was read */
+            *readbuf = packets;
+        }
+
+        if (localbuf)
+        {
+            /** Store any extra data in the DCB's readqueue */
+            spinlock_acquire(&dcb->authlock);
+            dcb->dcb_readqueue = gwbuf_append(dcb->dcb_readqueue, localbuf);
+            spinlock_release(&dcb->authlock);
+        }
+    }
+
+    return rval;
+}