diff --git a/server/modules/authenticator/PAM/PAMAuth/pam_client_session.cc b/server/modules/authenticator/PAM/PAMAuth/pam_client_session.cc index 28a37d787..62f5d59cf 100644 --- a/server/modules/authenticator/PAM/PAMAuth/pam_client_session.cc +++ b/server/modules/authenticator/PAM/PAMAuth/pam_client_session.cc @@ -78,14 +78,15 @@ int user_services_cb(void* data, int columns, char** column_vals, char** column_ /** Used by the PAM conversation function */ struct ConversationData { - DCB* m_client; - int m_counter; - string m_password; + int m_calls {0}; // How many times the conversation function has been called? + string m_client; // Client username + string m_password; // Password to give to first password prompt + string m_client_remote; // Client address - ConversationData(DCB* client, int counter, const string& password) + ConversationData(const string& client, const string& password, const string& client_remote) : m_client(client) - , m_counter(counter) , m_password(password) + , m_client_remote(client_remote) { } }; @@ -97,50 +98,74 @@ struct ConversationData * http://www.linux-pam.org/Linux-PAM-html/adg-interface-of-app-expected.html#adg-pam_conv * for more information. */ -int conversation_func(int num_msg, - const struct pam_message** msg, - struct pam_response** resp_out, +int conversation_func(int num_msg, const struct pam_message** messages, struct pam_response** responses_out, void* appdata_ptr) { MXS_DEBUG("Entering PAM conversation function."); - int rval = PAM_CONV_ERR; ConversationData* data = static_cast(appdata_ptr); - if (data->m_counter > 1) + + // The responses are saved as an array of structures. This is unlike the input messages, which is an + // array of pointers to struct. Each message should have an answer, even if empty. + auto responses = static_cast(MXS_CALLOC(num_msg, sizeof(pam_response))); + if (!responses) { - MXS_ERROR("Multiple calls to conversation function for client '%s'. %s", - data->m_client->user, - GENERAL_ERRMSG); + return PAM_BUF_ERR; } - else if (num_msg == 1) + + bool conv_error = false; + for (int i = 0; i < num_msg; i++) { - pam_message first = *msg[0]; - if ((first.msg_style != PAM_PROMPT_ECHO_OFF && first.msg_style != PAM_PROMPT_ECHO_ON) - || PASSWORD != first.msg) + const pam_message* message = messages[i]; // This may crash on Solaris, see PAM documentation. + pam_response* response = &responses[i]; + int msg_type = message->msg_style; + // In an ideal world, these messages would be sent to the client instead of the log. The problem + // is that the messages should be sent with the AuthSwitchRequest-packet, requiring the blocking + // PAM api to work with worker-threads. Not worth the trouble unless really required. + if (msg_type == PAM_ERROR_MSG) { - MXS_ERROR("Unexpected PAM message: type='%d', contents='%s'", - first.msg_style, - first.msg); + MXB_WARNING("Error message from PAM api: %s", message->msg); + } + else if (msg_type == PAM_TEXT_INFO) + { + MXB_NOTICE("Message from PAM api: '%s'", message->msg); + } + else if (msg_type == PAM_PROMPT_ECHO_ON || msg_type == PAM_PROMPT_ECHO_OFF) + { + // PAM system is asking for something. We only know how to answer the password question, + // anything else is an error. + if (message->msg == PASSWORD) + { + response->resp = MXS_STRDUP(data->m_password.c_str()); + // retcode should be already 0. + } + else + { + MXB_ERROR("Unexpected prompt from PAM api: '%s'. Only '%s' is allowed.", + message->msg, PASSWORD.c_str()); + conv_error = true; + } } else { - pam_response* response = static_cast(MXS_MALLOC(sizeof(pam_response))); - if (response) - { - response->resp_retcode = 0; - response->resp = MXS_STRDUP(data->m_password.c_str()); - *resp_out = response; - rval = PAM_SUCCESS; - } + // Faulty PAM system or perhaps different api version. + MXB_ERROR("Unknown PAM message type '%i'.", msg_type); + conv_error = true; + mxb_assert(!true); } } + + data->m_calls++; + if (conv_error) + { + // On error, the response output should not be set. + MXS_FREE(responses); + return PAM_CONV_ERR; + } else { - MXS_ERROR("Conversation function received '%d' messages from API. Only " - "singular messages are supported.", - num_msg); + *responses_out = responses; + return PAM_SUCCESS; } - data->m_counter++; - return rval; } /** @@ -149,15 +174,16 @@ int conversation_func(int num_msg, * @param user Username * @param password Password * @param service Which PAM service is the user logging to - * @param client Client DCB - * @return True if username & password are ok + * @param client_remote Client address. Used for log messages. + * @return True if username & password are ok and account is valid. */ -bool validate_pam_password(const string& user, const string& password, const string& service, DCB* client) +bool validate_pam_password(const string& user, const string& password, const string& service, + const string& client_remote) { const char PAM_START_ERR_MSG[] = "Failed to start PAM authentication for user '%s': '%s'."; const char PAM_AUTH_ERR_MSG[] = "Pam authentication for user '%s' failed: '%s'."; const char PAM_ACC_ERR_MSG[] = "Pam account check for user '%s' failed: '%s'."; - ConversationData appdata(client, 0, password); + ConversationData appdata(user, password, client_remote); pam_conv conv_struct = {conversation_func, &appdata}; bool authenticated = false; bool account_ok = false; @@ -414,7 +440,7 @@ int PamClientSession::authenticate(DCB* dcb) { *iter = "mysql"; } - if (validate_pam_password(ses->user, password, *iter, dcb)) + if (validate_pam_password(ses->user, password, *iter, dcb->remote)) { authenticated = true; } diff --git a/server/modules/authenticator/PAM/PAMBackendAuth/pam_backend_session.cc b/server/modules/authenticator/PAM/PAMBackendAuth/pam_backend_session.cc index 5cef02baa..c70eed55e 100644 --- a/server/modules/authenticator/PAM/PAMBackendAuth/pam_backend_session.cc +++ b/server/modules/authenticator/PAM/PAMBackendAuth/pam_backend_session.cc @@ -17,9 +17,12 @@ namespace { /** - * Check that the AuthSwitchRequest packet is as expected. Inverse of + * Check that the AuthSwitchRequest packet is as expected. Partially an inverse of * create_auth_change_packet() in pam_auth.cc. * + * 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 @@ -27,59 +30,124 @@ namespace bool check_auth_switch_request(DCB* dcb, GWBUF* buffer) { /** + * 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 - * byte - Message type - * string[EOF] - Message + * string[NUL] - Auth plugin name, should be "dialog" + * byte - Message type, 2 or 4 + * string[EOF] - Message(s) + * + * Authenticators receive complete packets from protocol. */ - /** We know how long the packet should be in the simple case. */ - unsigned int expected_buflen = MYSQL_HEADER_LEN + 1 + DIALOG_SIZE + 1 + PASSWORD.length(); - uint8_t data[expected_buflen]; - size_t copied = gwbuf_copy_data(buffer, 0, expected_buflen, data); - /* Check that this is an AuthSwitchRequest. */ - if ((copied <= MYSQL_HEADER_LEN) || (data[MYSQL_HEADER_LEN] != MYSQL_REPLY_AUTHSWITCHREQUEST)) - { - /** Server responded with something we did not expect. If it's an OK packet, - * it's possible that the server authenticated us as the anonymous user. This - * means that the server is not secure. */ - bool was_ok_packet = copied > MYSQL_HEADER_LEN - && data[MYSQL_HEADER_LEN + 1] == MYSQL_REPLY_OK; - MXS_ERROR("Server '%s' returned an unexpected authentication response.%s", - dcb->server->name, - was_ok_packet ? - " Authentication was complete before it even started, " - "anonymous users might not be disabled." : ""); - return false; - } - unsigned int buflen = gwbuf_length(buffer); - if (buflen != expected_buflen) - { - MXS_ERROR("Length of server AuthSwitchRequest packet was '%u', expected '%u'. %s", - buflen, - expected_buflen, - GENERAL_ERRMSG); - return false; - } + // 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; - /* Check that the server is using the "dialog" plugin and asking for the password. */ - uint8_t* plugin_name_loc = data + MYSQL_HEADER_LEN + 1; - uint8_t* msg_type_loc = plugin_name_loc + DIALOG_SIZE; - uint8_t msg_type = *msg_type_loc; - uint8_t* msg_loc = msg_type_loc + 1; + // 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; - if ((DIALOG == (char*)plugin_name_loc) - && (msg_type == DIALOG_ECHO_ENABLED || msg_type == DIALOG_ECHO_DISABLED) - && PASSWORD.compare(0, PASSWORD.length(), (char*)msg_loc, PASSWORD.length()) == 0) + const char* srv_name = dcb->server->name; + int buflen = gwbuf_length(buffer); + if (buflen <= min_readable_buflen || buflen > MAX_BUFLEN) { - rval = true; + 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. } else { - MXS_ERROR("AuthSwitchRequest packet contents unexpected. %s", GENERAL_ERRMSG); + 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()); + } + } + else + { + malformed_packet = true; + } + + 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); + } } return rval; } @@ -98,7 +166,6 @@ PamBackendSession::PamBackendSession() */ bool PamBackendSession::send_client_password(DCB* dcb) { - bool rval = false; MYSQL_session* ses = (MYSQL_session*)dcb->session->client_dcb->data; size_t buflen = MYSQL_HEADER_LEN + ses->auth_token_len; uint8_t bufferdata[buflen]; @@ -123,15 +190,13 @@ bool PamBackendSession::extract(DCB* dcb, GWBUF* buffer) /** Read authentication response */ if (mxs_mysql_is_ok_packet(buffer)) { - MXS_DEBUG("pam_backend_auth_extract received ok packet from '%s'.", - dcb->server->name); + MXS_DEBUG("pam_backend_auth_extract received ok packet from '%s'.", dcb->server->name); m_state = PAM_AUTH_OK; rval = true; } else { - MXS_ERROR("Expected ok from server but got something else. Authentication" - " failed."); + MXS_ERROR("Expected ok from server but got something else. Authentication failed."); } } diff --git a/server/modules/authenticator/PAM/pam_auth_common.cc b/server/modules/authenticator/PAM/pam_auth_common.cc index 6c362ebf8..3dcbfced0 100644 --- a/server/modules/authenticator/PAM/pam_auth_common.cc +++ b/server/modules/authenticator/PAM/pam_auth_common.cc @@ -22,5 +22,4 @@ const std::string DIALOG = "dialog"; const int DIALOG_SIZE = DIALOG.length() + 1; /* First query from server */ const std::string PASSWORD = "Password: "; -const char GENERAL_ERRMSG[] = "Only simple password-based PAM authentication with one call " - "to the conversation function is supported."; + diff --git a/server/modules/authenticator/PAM/pam_auth_common.hh b/server/modules/authenticator/PAM/pam_auth_common.hh index 134c0c62d..d74da0384 100644 --- a/server/modules/authenticator/PAM/pam_auth_common.hh +++ b/server/modules/authenticator/PAM/pam_auth_common.hh @@ -21,7 +21,6 @@ extern const std::string DIALOG; extern const std::string PASSWORD; extern const int DIALOG_SIZE; -extern const char GENERAL_ERRMSG[]; /** PAM authentication states */ enum pam_auth_state @@ -33,7 +32,7 @@ enum pam_auth_state }; /* Magic numbers from server source - * https://github.com/MariaDB/server/blob/10.2/plugin/auth_pam/auth_pam.c */ + * https://github.com/MariaDB/server/blob/10.2/plugin/auth_pam/auth_pam.c */ enum dialog_plugin_msg_types { DIALOG_ECHO_ENABLED = 2,