diff --git a/server/modules/authenticator/PAM/PAMAuth/pam_client_session.cc b/server/modules/authenticator/PAM/PAMAuth/pam_client_session.cc index 62f5d59cf..b02323458 100644 --- a/server/modules/authenticator/PAM/PAMAuth/pam_client_session.cc +++ b/server/modules/authenticator/PAM/PAMAuth/pam_client_session.cc @@ -243,9 +243,7 @@ bool validate_pam_password(const string& user, const string& password, const str } PamClientSession::PamClientSession(sqlite3* dbhandle, const PamInstance& instance) - : m_state(PAM_AUTH_INIT) - , m_sequence(0) - , m_dbhandle(dbhandle) + : m_dbhandle(dbhandle) , m_instance(instance) { } @@ -371,7 +369,7 @@ Buffer PamClientSession::create_auth_change_packet() const gw_mysql_set_byte3(pData, plen); pData += 3; *pData++ = m_sequence; - *pData++ = 0xfe; // AuthSwitchRequest command + *pData++ = MYSQL_REPLY_AUTHSWITCHREQUEST; memcpy(pData, DIALOG.c_str(), DIALOG_SIZE); // Plugin name pData += DIALOG_SIZE; *pData++ = DIALOG_ECHO_DISABLED; @@ -388,7 +386,7 @@ int PamClientSession::authenticate(DCB* dcb) if (*ses->user) { rval = MXS_AUTH_FAILED; - if (m_state == PAM_AUTH_INIT) + if (m_state == State::INIT) { /** We need to send the authentication switch packet to change the * authentication to something other than the 'mysql_native_password' @@ -396,11 +394,11 @@ int PamClientSession::authenticate(DCB* dcb) Buffer authbuf = create_auth_change_packet(); if (authbuf.length() && dcb->func.write(dcb, authbuf.release())) { - m_state = PAM_AUTH_DATA_SENT; + m_state = State::ASKED_FOR_PW; rval = MXS_AUTH_INCOMPLETE; } } - else if (m_state == PAM_AUTH_DATA_SENT) + else if (m_state == State::PW_RECEIVED) { /** We sent the authentication change packet + plugin name and the client * responded with the password. Try to continue authentication without more @@ -452,6 +450,7 @@ int PamClientSession::authenticate(DCB* dcb) { rval = MXS_AUTH_SUCCEEDED; } + m_state = State::DONE; } } return rval; @@ -465,20 +464,22 @@ bool PamClientSession::extract(DCB* dcb, GWBUF* buffer) switch (m_state) { - case PAM_AUTH_INIT: - // The buffer doesn't have any PAM-specific data yet + case State::INIT: + // The buffer doesn't have any PAM-specific data yet, as it's the normal HandShakeResponse. rval = true; break; - case PAM_AUTH_DATA_SENT: + case State::ASKED_FOR_PW: + // Client should have responses with password. if (store_client_password(dcb, buffer)) { + m_state = State::PW_RECEIVED; rval = true; } break; default: - MXS_ERROR("Unexpected authentication state: %d", m_state); + MXS_ERROR("Unexpected authentication state: %d", static_cast(m_state)); mxb_assert(!true); break; } diff --git a/server/modules/authenticator/PAM/PAMAuth/pam_client_session.hh b/server/modules/authenticator/PAM/PAMAuth/pam_client_session.hh index 26875ce4c..6abb4b6fa 100644 --- a/server/modules/authenticator/PAM/PAMAuth/pam_client_session.hh +++ b/server/modules/authenticator/PAM/PAMAuth/pam_client_session.hh @@ -36,8 +36,16 @@ private: void get_pam_user_services(const DCB* dcb, const MYSQL_session* session, StringVector* services_out); maxscale::Buffer create_auth_change_packet() const; - pam_auth_state m_state; /**< Authentication state*/ - uint8_t m_sequence; /**< The next packet seqence number */ - sqlite3* const m_dbhandle; /**< SQLite3 database handle */ - const PamInstance& m_instance; /**< Authenticator instance */ + enum class State + { + INIT, + ASKED_FOR_PW, + PW_RECEIVED, + DONE + }; + + State m_state {State::INIT}; /**< Authentication state*/ + uint8_t m_sequence {0}; /**< The next packet seqence number */ + sqlite3* const m_dbhandle; /**< SQLite3 database handle */ + const PamInstance& m_instance; /**< Authenticator instance */ }; diff --git a/server/modules/authenticator/PAM/PAMBackendAuth/pam_backend_auth.cc b/server/modules/authenticator/PAM/PAMBackendAuth/pam_backend_auth.cc index de6dd51e5..a418eb952 100644 --- a/server/modules/authenticator/PAM/PAMBackendAuth/pam_backend_auth.cc +++ b/server/modules/authenticator/PAM/PAMBackendAuth/pam_backend_auth.cc @@ -21,8 +21,7 @@ static void* pam_backend_auth_alloc(void* instance) { - PamBackendSession* pses = new(std::nothrow) PamBackendSession(); - return pses; + return new(std::nothrow) PamBackendSession(); } static void pam_backend_auth_free(void* data) diff --git a/server/modules/authenticator/PAM/PAMBackendAuth/pam_backend_session.cc b/server/modules/authenticator/PAM/PAMBackendAuth/pam_backend_session.cc index c70eed55e..e1ed65e7d 100644 --- a/server/modules/authenticator/PAM/PAMBackendAuth/pam_backend_session.cc +++ b/server/modules/authenticator/PAM/PAMBackendAuth/pam_backend_session.cc @@ -16,148 +16,129 @@ namespace { + /** - * Check that the AuthSwitchRequest packet is as expected. Partially an inverse of - * create_auth_change_packet() in pam_auth.cc. + * Parse packet type and plugin name from packet data. Advances pointer. * - * To deal with arbitrary messages in the packet, this needs to mirror also the server - * plugin code. - * - * @param dcb Backend DCB - * @param buffer Buffer containing an AuthSwitchRequest packet - * @return True on success, false on error + * @param data Data from server. The pointer is advanced. + * @param end Pointer to after the end of data + * @param server_name Server name for logging + * @return True if all expected fields were parsed */ -bool check_auth_switch_request(DCB* dcb, GWBUF* buffer) +bool parse_authswitchreq(const uint8_t** data, const uint8_t* end, const char* server_name) { - /** - * The server PAM plugin sends data usually once, at the moment it gets a prompt-type message - * from the api. The "message"-segment may contain multiple messages from the api separated by \n. - * MaxScale should ignore this text and search for "Password: " near the end of the message. If - * server sends more data, authentication ends in error, but this should only happen if the server - * asks multiple questions. - * - * The AuthSwitchRequest packet: - * 4 bytes - Header - * 0xfe - Command byte - * string[NUL] - Auth plugin name, should be "dialog" - * byte - Message type, 2 or 4 - * string[EOF] - Message(s) - * - * Authenticators receive complete packets from protocol. - */ - - // Smallest buffer that is parsed, header + cmd byte. - int min_readable_buflen = MYSQL_HEADER_LEN + 1; - // The buffer should be small, don't accept big messages since it most likely means a complicated auth - // scheme. - const int MAX_BUFLEN = 2000; - - // Smallest buffer with every packet component, although the components may be wrong. - int min_msg_buflen = min_readable_buflen + 3; - // Smallest buffer with all expected data. - int min_acceptable_buflen = MYSQL_HEADER_LEN + 1 + DIALOG_SIZE + 1 + PASSWORD.length(); - - bool rval = false; - const char* srv_name = dcb->server->name; - int buflen = gwbuf_length(buffer); - if (buflen <= min_readable_buflen || buflen > MAX_BUFLEN) + const uint8_t* ptr = *data; + if (ptr >= end) { - MXB_ERROR("Authentication start packet from '%s' is %i bytes. Expected length of packet is " - "between %i and %i.", srv_name, buflen, min_acceptable_buflen, MAX_BUFLEN); - // Lengths between min_readable_buflen and min_acceptable_buflen are checked below. + return false; } - else - { - uint8_t data[buflen + 1]; // + 1 to ensure that the end has a zero. - data[buflen] = 0; - gwbuf_copy_data(buffer, 0, buflen, data); - uint8_t cmdbyte = data[MYSQL_HEADER_LEN]; - if (cmdbyte == MYSQL_REPLY_AUTHSWITCHREQUEST) - { - bool malformed_packet = false; - // Correct packet type. - if (buflen >= min_msg_buflen) - { - // Buffer is long enough to contain at least some of the fields. Try to read plugin name. - const char* ptr = reinterpret_cast(data) + min_readable_buflen; - const char* end = reinterpret_cast(data) + buflen; - if (strcmp(ptr, DIALOG.c_str()) == 0) - { - // Correct plugin. - ptr += DIALOG_SIZE; - if (end - ptr >= 2) // message type + message - { - int msg_type = *ptr++; - if (msg_type == DIALOG_ECHO_ENABLED || msg_type == DIALOG_ECHO_DISABLED) - { - // The rest of the buffer contains a message. - // The server separates messages with linebreaks. Search for the last. - const char* linebrk_pos = strrchr(ptr, '\n'); - if (linebrk_pos) - { - int msg_len = linebrk_pos - ptr; - MXS_INFO("Server '%s' PAM plugin sent messages: '%.*s'", - srv_name, msg_len, ptr); - ptr = linebrk_pos + 1; - } - if (ptr == PASSWORD) - { - rval = true; - } - else - { - MXB_ERROR("'%s' asked for '%s' when '%s was expected.", - srv_name, ptr, PASSWORD.c_str()); - } - } - else - { - malformed_packet = true; - } - } - else - { - malformed_packet = true; - } - } - else - { - MXB_ERROR("'%s' asked for authentication plugin '%s' when '%s' was expected.", - srv_name, ptr, DIALOG.c_str()); - } + bool success = false; + uint8_t cmdbyte = *ptr++; + if (cmdbyte == MYSQL_REPLY_AUTHSWITCHREQUEST) + { + // Correct packet type. + if (ptr < end) + { + const char* plugin_name = reinterpret_cast(ptr); + if (strcmp(plugin_name, DIALOG.c_str()) == 0) + { + // Correct plugin. + ptr += DIALOG_SIZE; + success = true; } else { - malformed_packet = true; + MXB_ERROR("'%s' asked for authentication plugin '%s' when '%s' was expected.", + server_name, plugin_name, DIALOG.c_str()); } - - if (malformed_packet) - { - MXB_ERROR("Received malformed AuthSwitchRequest-packet from '%s'.", srv_name); - } - } - else if (cmdbyte == MYSQL_REPLY_OK) - { - // Authentication is already done? Maybe the server authenticated us as the anonymous user. This - // is quite insecure. */ - MXB_ERROR("Authentication to '%s' was complete before it even started, " - "anonymous users may be enabled.", srv_name); } else { - MXB_ERROR("Expected AuthSwitchRequest-packet from '%s' but received %#x.", srv_name, cmdbyte); + MXB_ERROR("Received malformed AuthSwitchRequest-packet from '%s'.", server_name); } } - return rval; + else if (cmdbyte == MYSQL_REPLY_OK) + { + // Authentication is already done? Maybe the server authenticated us as the anonymous user. This + // is quite insecure. */ + MXB_ERROR("Authentication to '%s' was complete before it even started, anonymous users may " + "be enabled.", server_name); + } + else + { + MXB_ERROR("Expected AuthSwitchRequest-packet from '%s' but received %#x.", server_name, cmdbyte); + } + + if (success) + { + *data = ptr; + } + return success; } -} -PamBackendSession::PamBackendSession() - : m_state(PAM_AUTH_INIT) - , m_sequence(0) + +/** + * Parse prompt type and message text from packet data. Advances pointer. + * + * @param data Data from server. The pointer is advanced. + * @param end Pointer to after the end of data + * @param server_name Server name for logging + * @return True if all expected fields were parsed + */ +bool parse_password_prompt(const uint8_t** data, const uint8_t* end, const char* server_name) { + const uint8_t* ptr = *data; + if (end - ptr < 2) // Need at least message type + message + { + return false; + } + + bool success = false; + int msg_type = *ptr++; + if (msg_type == DIALOG_ECHO_ENABLED || msg_type == DIALOG_ECHO_DISABLED) + { + const char* messages = reinterpret_cast(ptr); + // The rest of the buffer contains a message. + // The server separates messages with linebreaks. Search for the last. + const char* linebrk_pos = strrchr(messages, '\n'); + const char* prompt; + if (linebrk_pos) + { + int msg_len = linebrk_pos - messages; + MXS_INFO("PAM plugin of '%s' sent message: '%.*s'", server_name, msg_len, messages); + prompt = linebrk_pos + 1; + } + else + { + prompt = messages; // No additional messages. + } + + if (prompt == PASSWORD) + { + success = true; + } + else + { + MXB_ERROR("'%s' asked for '%s' when '%s was expected.", server_name, prompt, PASSWORD.c_str()); + } + } + else + { + MXB_ERROR("'%s' sent an unknown message type %i.", server_name, msg_type); + } + + if (success) + { + *data = ptr; + } + return success; } +} + +PamBackendSession::PamBackendSession() +{} + /** * Send password to server * @@ -177,53 +158,134 @@ bool PamBackendSession::send_client_password(DCB* dcb) bool PamBackendSession::extract(DCB* dcb, GWBUF* buffer) { - gwbuf_copy_data(buffer, MYSQL_SEQ_OFFSET, 1, &m_sequence); - m_sequence++; - bool rval = false; + /** + * The server PAM plugin sends data usually once, at the moment it gets a prompt-type message + * from the api. The "message"-segment may contain multiple messages from the api separated by \n. + * MaxScale should ignore this text and search for "Password: " near the end of the message. See + * https://github.com/MariaDB/server/blob/10.3/plugin/auth_pam/auth_pam.c + * for how communication is handled on the other side. + * + * The AuthSwitchRequest packet: + * 4 bytes - Header + * 0xfe - Command byte + * string[NUL] - Auth plugin name, should be "dialog" + * byte - Message type, 2 or 4 + * string[EOF] - Message(s) + * + * Additional prompts after AuthSwitchRequest: + * 4 bytes - Header + * byte - Message type, 2 or 4 + * string[EOF] - Message(s) + * + * Authenticators receive complete packets from protocol. + */ - if (m_state == PAM_AUTH_INIT && check_auth_switch_request(dcb, buffer)) + // Smallest buffer that is parsed, header + (cmd-byte/msg-type + message). + const int min_readable_buflen = MYSQL_HEADER_LEN + 1 + 1; + // The buffer should be reasonable size. Large buffers likely mean that the auth scheme is complicated. + const int MAX_BUFLEN = 2000; + const char* srv_name = dcb->server->name; + const int buflen = gwbuf_length(buffer); + if (buflen <= min_readable_buflen || buflen > MAX_BUFLEN) { - rval = true; + MXB_ERROR("Received packet of size %i from '%s' during authentication. Expected packet size is " + "between %i and %i.", buflen, srv_name, min_readable_buflen, MAX_BUFLEN); + return false; } - else if (m_state == PAM_AUTH_DATA_SENT) + + uint8_t data[buflen + 1]; // + 1 to ensure that the end has a zero. + data[buflen] = 0; + gwbuf_copy_data(buffer, 0, buflen, data); + m_sequence = data[MYSQL_SEQ_OFFSET] + 1; + const uint8_t* data_ptr = data + MYSQL_COM_OFFSET; + const uint8_t* end_ptr = data + buflen; + bool success = false; + bool unexpected_data = false; + + switch (m_state) { - /** Read authentication response */ - if (mxs_mysql_is_ok_packet(buffer)) + case State::INIT: + // Server should have sent the AuthSwitchRequest + 1st prompt + if (parse_authswitchreq(&data_ptr, end_ptr, srv_name) + && parse_password_prompt(&data_ptr, end_ptr, srv_name)) { - MXS_DEBUG("pam_backend_auth_extract received ok packet from '%s'.", dcb->server->name); - m_state = PAM_AUTH_OK; - rval = true; + m_state = State::RECEIVED_PROMT; + success = true; } else { - MXS_ERROR("Expected ok from server but got something else. Authentication failed."); + unexpected_data = true; } + break; + + case State::PW_SENT: + { + /** Read authentication response. This is typically either OK packet or ERROR, but can be another + * prompt. */ + uint8_t cmdbyte = data[MYSQL_COM_OFFSET]; + if (cmdbyte == MYSQL_REPLY_OK) + { + MXS_DEBUG("pam_backend_auth_extract received ok packet from '%s'.", srv_name); + m_state = State::DONE; + success = true; + } + else if (cmdbyte == MYSQL_REPLY_ERR) + { + MXS_DEBUG("pam_backend_auth_extract received error packet from '%s'.", srv_name); + m_state = State::DONE; + } + else + { + // The packet may contain another prompt, try parse it. Currently, it's expected to be + // another "Password: ", in the future other setups may be supported. + if (parse_password_prompt(&data_ptr, end_ptr, srv_name)) + { + m_state = State::RECEIVED_PROMT; + success = true; + } + else + { + MXS_ERROR("Expected OK, ERR or PAM prompt from '%s' but received something else. ", + srv_name); + unexpected_data = true; + } + } + } + break; + + + default: + // This implicates an error in either PAM authenticator or backend protocol. + mxb_assert(!true); + unexpected_data = true; + break; } - if (!rval) + if (unexpected_data) { - MXS_DEBUG("pam_backend_auth_extract to backend '%s' failed for user '%s'.", - dcb->server->name, - dcb->user); + MXS_ERROR("Failed to read data from '%s' when authenticating user '%s'.", srv_name, dcb->user); } - return rval; + return success; } int PamBackendSession::authenticate(DCB* dcb) { int rval = MXS_AUTH_FAILED; - if (m_state == PAM_AUTH_INIT) + if (m_state == State::RECEIVED_PROMT) { - MXS_DEBUG("pam_backend_auth_authenticate sending password to '%s'.", - dcb->server->name); + MXS_DEBUG("pam_backend_auth_authenticate sending password to '%s'.", dcb->server->name); if (send_client_password(dcb)) { + m_state = State::PW_SENT; rval = MXS_AUTH_INCOMPLETE; - m_state = PAM_AUTH_DATA_SENT; + } + else + { + m_state = State::DONE; } } - else if (m_state == PAM_AUTH_OK) + else if (m_state == State::DONE) { rval = MXS_AUTH_SUCCEEDED; } diff --git a/server/modules/authenticator/PAM/PAMBackendAuth/pam_backend_session.hh b/server/modules/authenticator/PAM/PAMBackendAuth/pam_backend_session.hh index 264db09d5..c7f5decb8 100644 --- a/server/modules/authenticator/PAM/PAMBackendAuth/pam_backend_session.hh +++ b/server/modules/authenticator/PAM/PAMBackendAuth/pam_backend_session.hh @@ -18,9 +18,10 @@ class PamBackendSession { - PamBackendSession(const PamBackendSession& orig); - PamBackendSession& operator=(const PamBackendSession&); public: + PamBackendSession(const PamBackendSession& orig) = delete; + PamBackendSession& operator=(const PamBackendSession&) = delete; + PamBackendSession(); bool extract(DCB* dcb, GWBUF* buffer); int authenticate(DCB* dcb); @@ -28,6 +29,14 @@ public: private: bool send_client_password(DCB* dcb); - pam_auth_state m_state; /**< Authentication state*/ - uint8_t m_sequence; /**< The next packet seqence number */ + enum class State + { + INIT, + RECEIVED_PROMT, + PW_SENT, + DONE + }; + + State m_state {State::INIT}; /**< Authentication state*/ + uint8_t m_sequence {0}; /**< The next packet sequence number */ };