From 442d8bed9a12dc721859f30f09fd943f6a4d4048 Mon Sep 17 00:00:00 2001 From: Esa Korhonen Date: Mon, 27 May 2019 14:12:02 +0300 Subject: [PATCH] MXS-2479 Add username and host to PAM authenticator log messages --- .../PAM/PAMAuth/pam_client_session.cc | 11 ++-- .../PAM/PAMBackendAuth/pam_backend_session.cc | 52 +++++++++++-------- .../PAM/PAMBackendAuth/pam_backend_session.hh | 12 +++-- 3 files changed, 43 insertions(+), 32 deletions(-) diff --git a/server/modules/authenticator/PAM/PAMAuth/pam_client_session.cc b/server/modules/authenticator/PAM/PAMAuth/pam_client_session.cc index b02323458..3532a10a9 100644 --- a/server/modules/authenticator/PAM/PAMAuth/pam_client_session.cc +++ b/server/modules/authenticator/PAM/PAMAuth/pam_client_session.cc @@ -113,6 +113,7 @@ int conversation_func(int num_msg, const struct pam_message** messages, struct p } bool conv_error = false; + string userhost = data->m_client + "@" + data->m_client_remote; for (int i = 0; i < num_msg; i++) { const pam_message* message = messages[i]; // This may crash on Solaris, see PAM documentation. @@ -123,11 +124,13 @@ int conversation_func(int num_msg, const struct pam_message** messages, struct p // PAM api to work with worker-threads. Not worth the trouble unless really required. if (msg_type == PAM_ERROR_MSG) { - MXB_WARNING("Error message from PAM api: %s", message->msg); + MXB_WARNING("Error message from PAM api when authenticating '%s': '%s'", + userhost.c_str(), message->msg); } else if (msg_type == PAM_TEXT_INFO) { - MXB_NOTICE("Message from PAM api: '%s'", message->msg); + MXB_NOTICE("Message from PAM api when authenticating '%s': '%s'", + userhost.c_str(), message->msg); } else if (msg_type == PAM_PROMPT_ECHO_ON || msg_type == PAM_PROMPT_ECHO_OFF) { @@ -140,8 +143,8 @@ int conversation_func(int num_msg, const struct pam_message** messages, struct p } else { - MXB_ERROR("Unexpected prompt from PAM api: '%s'. Only '%s' is allowed.", - message->msg, PASSWORD.c_str()); + MXB_ERROR("Unexpected prompt from PAM api when authenticating '%s': '%s'. " + "Only '%s' is allowed.", userhost.c_str(), message->msg, PASSWORD.c_str()); conv_error = true; } } diff --git a/server/modules/authenticator/PAM/PAMBackendAuth/pam_backend_session.cc b/server/modules/authenticator/PAM/PAMBackendAuth/pam_backend_session.cc index e1ed65e7d..e714d257d 100644 --- a/server/modules/authenticator/PAM/PAMBackendAuth/pam_backend_session.cc +++ b/server/modules/authenticator/PAM/PAMBackendAuth/pam_backend_session.cc @@ -14,18 +14,14 @@ #include "pam_backend_session.hh" #include -namespace -{ - /** * Parse packet type and plugin name 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_authswitchreq(const uint8_t** data, const uint8_t* end, const char* server_name) +bool PamBackendSession::parse_authswitchreq(const uint8_t** data, const uint8_t* end) { const uint8_t* ptr = *data; if (ptr >= end) @@ -33,6 +29,7 @@ bool parse_authswitchreq(const uint8_t** data, const uint8_t* end, const char* s return false; } + const char* server_name = m_servername.c_str(); bool success = false; uint8_t cmdbyte = *ptr++; if (cmdbyte == MYSQL_REPLY_AUTHSWITCHREQUEST) @@ -49,8 +46,9 @@ bool parse_authswitchreq(const uint8_t** data, const uint8_t* end, const char* s } else { - MXB_ERROR("'%s' asked for authentication plugin '%s' when '%s' was expected.", - server_name, plugin_name, DIALOG.c_str()); + MXB_ERROR("'%s' asked for authentication plugin '%s' when authenticating '%s'. " + "Only '%s' is supported.", + server_name, plugin_name, m_clienthost.c_str(), DIALOG.c_str()); } } else @@ -62,8 +60,8 @@ bool parse_authswitchreq(const uint8_t** data, const uint8_t* end, const char* s { // 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); + MXB_ERROR("Authentication of '%s' to '%s' was complete before it even started, anonymous users may " + "be enabled.", m_clienthost.c_str(), server_name); } else { @@ -82,10 +80,9 @@ bool parse_authswitchreq(const uint8_t** data, const uint8_t* end, const char* s * * @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) +bool PamBackendSession::parse_password_prompt(const uint8_t** data, const uint8_t* end) { const uint8_t* ptr = *data; if (end - ptr < 2) // Need at least message type + message @@ -93,6 +90,7 @@ bool parse_password_prompt(const uint8_t** data, const uint8_t* end, const char* return false; } + const char* server_name = m_servername.c_str(); bool success = false; int msg_type = *ptr++; if (msg_type == DIALOG_ECHO_ENABLED || msg_type == DIALOG_ECHO_DISABLED) @@ -105,7 +103,8 @@ bool parse_password_prompt(const uint8_t** data, const uint8_t* end, const char* if (linebrk_pos) { int msg_len = linebrk_pos - messages; - MXS_INFO("PAM plugin of '%s' sent message: '%.*s'", server_name, msg_len, messages); + MXS_INFO("'%s' sent message when authenticating '%s': '%.*s'", + server_name, m_clienthost.c_str(), msg_len, messages); prompt = linebrk_pos + 1; } else @@ -119,12 +118,14 @@ bool parse_password_prompt(const uint8_t** data, const uint8_t* end, const char* } else { - MXB_ERROR("'%s' asked for '%s' when '%s was expected.", server_name, prompt, PASSWORD.c_str()); + MXB_ERROR("'%s' asked for '%s' when authenticating '%s'. '%s' was expected.", + server_name, prompt, m_clienthost.c_str(), PASSWORD.c_str()); } } else { - MXB_ERROR("'%s' sent an unknown message type %i.", server_name, msg_type); + MXB_ERROR("'%s' sent an unknown message type %i when authenticating '%s'.", + server_name, msg_type, m_clienthost.c_str()); } if (success) @@ -134,8 +135,6 @@ bool parse_password_prompt(const uint8_t** data, const uint8_t* end, const char* return success; } -} - PamBackendSession::PamBackendSession() {} @@ -180,11 +179,18 @@ bool PamBackendSession::extract(DCB* dcb, GWBUF* buffer) * Authenticators receive complete packets from protocol. */ + const char* srv_name = dcb->server->name; + if (m_servername.empty()) + { + m_servername = srv_name; + auto client_dcb = dcb->session->client_dcb; + m_clienthost = client_dcb->user + (std::string)"@" + client_dcb->remote; + } + // 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) { @@ -206,10 +212,10 @@ bool PamBackendSession::extract(DCB* dcb, GWBUF* 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)) + if (parse_authswitchreq(&data_ptr, end_ptr) + && parse_password_prompt(&data_ptr, end_ptr)) { - m_state = State::RECEIVED_PROMT; + m_state = State::RECEIVED_PROMPT; success = true; } else @@ -238,9 +244,9 @@ bool PamBackendSession::extract(DCB* dcb, GWBUF* buffer) { // 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)) + if (parse_password_prompt(&data_ptr, end_ptr)) { - m_state = State::RECEIVED_PROMT; + m_state = State::RECEIVED_PROMPT; success = true; } else @@ -272,7 +278,7 @@ int PamBackendSession::authenticate(DCB* dcb) { int rval = MXS_AUTH_FAILED; - if (m_state == State::RECEIVED_PROMT) + if (m_state == State::RECEIVED_PROMPT) { MXS_DEBUG("pam_backend_auth_authenticate sending password to '%s'.", dcb->server->name); if (send_client_password(dcb)) diff --git a/server/modules/authenticator/PAM/PAMBackendAuth/pam_backend_session.hh b/server/modules/authenticator/PAM/PAMBackendAuth/pam_backend_session.hh index c7f5decb8..690d12e74 100644 --- a/server/modules/authenticator/PAM/PAMBackendAuth/pam_backend_session.hh +++ b/server/modules/authenticator/PAM/PAMBackendAuth/pam_backend_session.hh @@ -12,8 +12,6 @@ */ #pragma once #include "pam_backend_auth.hh" - -#include #include "../pam_auth_common.hh" class PamBackendSession @@ -28,15 +26,19 @@ public: private: bool send_client_password(DCB* dcb); + bool parse_authswitchreq(const uint8_t** data, const uint8_t* end); + bool parse_password_prompt(const uint8_t** data, const uint8_t* end); enum class State { INIT, - RECEIVED_PROMT, + RECEIVED_PROMPT, PW_SENT, DONE }; - State m_state {State::INIT}; /**< Authentication state*/ - uint8_t m_sequence {0}; /**< The next packet sequence number */ + State m_state {State::INIT}; /**< Authentication state */ + uint8_t m_sequence {0}; /**< The next packet sequence number */ + std::string m_servername; /**< Backend name, for logging */ + std::string m_clienthost; /**< Client name & host, for logging */ };