From e918810a4fc4b0f632708b439b4d8e0259af855f Mon Sep 17 00:00:00 2001 From: Esa Korhonen Date: Thu, 25 Jan 2018 11:29:04 +0200 Subject: [PATCH 1/3] MXS-1604: PAMAuth Use "mysql" as default service name, fix authentication data updating If a user has an empty service name, use "mysql" as default. Authentication data was only updated inside get_pam_user_services() if no service was found. It was possible that the PAM service changed but the old service would be used for authenticating, causing a false negative. Now, the auth data is updated outside the function if authentication fails for any reason. The new service data is compared to the old and if equal, password check is not attempted again. This gives a false negative only if user password has changed after the previous attempt. Also, fixed some comments. --- .../PAM/PAMAuth/pam_client_session.cc | 98 +++++++++++-------- 1 file changed, 58 insertions(+), 40 deletions(-) diff --git a/server/modules/authenticator/PAM/PAMAuth/pam_client_session.cc b/server/modules/authenticator/PAM/PAMAuth/pam_client_session.cc index 5e6431d01..20e02f248 100644 --- a/server/modules/authenticator/PAM/PAMAuth/pam_client_session.cc +++ b/server/modules/authenticator/PAM/PAMAuth/pam_client_session.cc @@ -138,15 +138,15 @@ int conversation_func(int num_msg, const struct pam_message **msg, } /** - * @brief Check if the client token is valid + * @brief Check if the client password is correct for the service * - * @param token Client token - * @param len Length of the token - * @param output Pointer where the client principal name is stored - * @return True if client token is valid + * @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 */ -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, DCB* client) { ConversationData appdata(client, 0, password); pam_conv conv_struct = {conversation_func, &appdata}; @@ -164,8 +164,7 @@ bool validate_pam_password(const string& user, const string& password, MXS_DEBUG("pam_authenticate returned success."); break; case PAM_AUTH_ERR: - MXS_DEBUG("pam_authenticate returned authentication failure" - " (wrong password)."); + MXS_DEBUG("pam_authenticate returned authentication failure (wrong password)."); // Normal failure break; default: @@ -223,13 +222,11 @@ PamClientSession* PamClientSession::create(const PamInstance& inst) } /** - * @brief Check which PAM services the session user has access to + * Check which PAM services the session user has access to. * - * @param auth Authenticator session * @param dcb Client DCB * @param session MySQL session - * - * @return An array of PAM service names for the session user + * @param services_out Output for services */ void PamClientSession::get_pam_user_services(const DCB* dcb, const MYSQL_session* session, StringVector* services_out) @@ -241,28 +238,14 @@ void PamClientSession::get_pam_user_services(const DCB* dcb, const MYSQL_session "' LIKE db) ORDER BY authentication_string"; MXS_DEBUG("PAM services search sql: '%s'.", services_query.c_str()); char *err; - /** - * Try search twice: first time with the current users, second - * time with fresh users. - */ - for (int i = 0; i < 2; i++) + if (sqlite3_exec(m_dbhandle, services_query.c_str(), user_services_cb, + services_out, &err) != SQLITE_OK) { - if (i == 0 || service_refresh_users(dcb->service) == 0) - { - if (sqlite3_exec(m_dbhandle, services_query.c_str(), user_services_cb, - services_out, &err) != SQLITE_OK) - { - MXS_ERROR("Failed to execute query: '%s'", err); - sqlite3_free(err); - } - else if (!services_out->empty()) - { - MXS_DEBUG("User '%s' matched %lu rows in %s db.", session->user, - services_out->size(), m_instance.m_tablename.c_str()); - break; - } - } + MXS_ERROR("Failed to execute query: '%s'", err); + sqlite3_free(err); } + MXS_DEBUG("User '%s' matched %lu rows in %s db.", session->user, + services_out->size(), m_instance.m_tablename.c_str()); } /** @@ -328,17 +311,52 @@ int PamClientSession::authenticate(DCB* dcb) * responded with the password. Try to continue authentication without more * messages to client. */ string password((char*)ses->auth_token, ses->auth_token_len); - StringVector services; - get_pam_user_services(dcb, ses, &services); - - for (StringVector::const_iterator i = services.begin(); i != services.end(); i++) + /* + * Authentication may be attempted twice: first with old user account info and then with + * updated info. Updating may fail if it has been attempted too often lately. The second password + * check is useless if the user services are same as on the first attempt. + */ + bool authenticated = false; + StringVector services_old; + for (int loop = 0; loop < 2 && !authenticated; loop++) { - if (validate_pam_password(ses->user, password, *i, dcb)) + if (loop == 0 || service_refresh_users(dcb->service) == 0) { - rval = MXS_AUTH_SUCCEEDED; - break; + bool try_validate = true; + StringVector services; + get_pam_user_services(dcb, ses, &services); + if (loop == 0) + { + services_old = services; + } + else if (services == services_old) + { + try_validate = false; + } + if (try_validate) + { + for (StringVector::iterator iter = services.begin(); + iter != services.end() && !authenticated; + iter++) + { + // The server PAM plugin uses "mysql" as the default service when authenticating + // a user with no service. + if (iter->empty()) + { + *iter = "mysql"; + } + if (validate_pam_password(ses->user, password, *iter, dcb)) + { + authenticated = true; + } + } + } } } + if (authenticated) + { + rval = MXS_AUTH_SUCCEEDED; + } } } return rval; From ef5c8d3114549b1f290e186ad80d9d3b340cda90 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Markus=20M=C3=A4kel=C3=A4?= Date: Fri, 26 Jan 2018 10:20:16 +0200 Subject: [PATCH 2/3] MXS-1631: Update handshake version string Updated handshake version string to 5.5.5-10.2.12. This will signal that MaxScale is capable of behaving like a fully-fledged 10.2 server. --- include/maxscale/protocol/mysql.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/maxscale/protocol/mysql.h b/include/maxscale/protocol/mysql.h index 022bcfa03..0e9388568 100644 --- a/include/maxscale/protocol/mysql.h +++ b/include/maxscale/protocol/mysql.h @@ -43,7 +43,7 @@ MXS_BEGIN_DECLS -#define GW_MYSQL_VERSION "5.5.5-10.0.0 " MAXSCALE_VERSION "-maxscale" +#define GW_MYSQL_VERSION "5.5.5-10.2.12 " MAXSCALE_VERSION "-maxscale" #define GW_MYSQL_LOOP_TIMEOUT 300000000 #define GW_MYSQL_READ 0 #define GW_MYSQL_WRITE 1 From 9b5d4d129e6a12f930faae4a24dd5ab00e794572 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Markus=20M=C3=A4kel=C3=A4?= Date: Fri, 26 Jan 2018 10:05:48 +0200 Subject: [PATCH 3/3] MXS-1630: Combine MaxCtrl into the main package Added MaxCtrl to the core package. Removed the old packages from build scripts. --- BUILD/build_deb_local.sh | 2 +- BUILD/build_rpm_local.sh | 2 +- cmake/defaults.cmake | 3 +++ maxctrl/CMakeLists.txt | 34 +++++++++++++++++++--------------- 4 files changed, 24 insertions(+), 17 deletions(-) diff --git a/BUILD/build_deb_local.sh b/BUILD/build_deb_local.sh index a063fc339..bd3f0f886 100755 --- a/BUILD/build_deb_local.sh +++ b/BUILD/build_deb_local.sh @@ -48,7 +48,7 @@ cp _build/*.gz . set -x if [ "$build_experimental" == "yes" ] then - for component in experimental devel client + for component in experimental devel do cd _build rm CMakeCache.txt diff --git a/BUILD/build_rpm_local.sh b/BUILD/build_rpm_local.sh index ba93f63d4..689616f54 100755 --- a/BUILD/build_rpm_local.sh +++ b/BUILD/build_rpm_local.sh @@ -43,7 +43,7 @@ cp _build/*.gz . if [ "$build_experimental" == "yes" ] then - for component in experimental devel client + for component in experimental devel do cd _build rm CMakeCache.txt diff --git a/cmake/defaults.cmake b/cmake/defaults.cmake index 3d65d5cc4..6002be775 100644 --- a/cmake/defaults.cmake +++ b/cmake/defaults.cmake @@ -27,6 +27,9 @@ set(BUILD_CDC TRUE CACHE BOOL "Build Avro router") # Build the multimaster monitor set(BUILD_MMMON TRUE CACHE BOOL "Build multimaster monitor") +# Build MaxCtrl +set(BUILD_MAXCTRL TRUE CACHE BOOL "Build MaxCtrl") + # Build Luafilter set(BUILD_LUAFILTER FALSE CACHE BOOL "Build Luafilter") diff --git a/maxctrl/CMakeLists.txt b/maxctrl/CMakeLists.txt index f61acf65d..6516a82eb 100644 --- a/maxctrl/CMakeLists.txt +++ b/maxctrl/CMakeLists.txt @@ -1,20 +1,24 @@ -find_package(NPM) -find_package(NodeJS) +if (BUILD_MAXCTRL) + find_package(NPM) + find_package(NodeJS) -if (NPM_FOUND AND NODEJS_FOUND AND NODEJS_VERSION VERSION_GREATER "6.0.0") - add_custom_command(OUTPUT ${CMAKE_BINARY_DIR}/maxctrl/maxctrl - COMMAND ${CMAKE_CURRENT_SOURCE_DIR}/build.sh ${CMAKE_SOURCE_DIR} - WORKING_DIRECTORY ${CMAKE_BINARY_DIR}) - add_custom_target(maxctrl ALL DEPENDS ${CMAKE_BINARY_DIR}/maxctrl/maxctrl) - install_script(${CMAKE_BINARY_DIR}/maxctrl/maxctrl client) + if (NPM_FOUND AND NODEJS_FOUND AND NODEJS_VERSION VERSION_GREATER "6.0.0") + add_custom_command(OUTPUT ${CMAKE_BINARY_DIR}/maxctrl/maxctrl + COMMAND ${CMAKE_CURRENT_SOURCE_DIR}/build.sh ${CMAKE_SOURCE_DIR} + WORKING_DIRECTORY ${CMAKE_BINARY_DIR}) + add_custom_target(maxctrl ALL DEPENDS ${CMAKE_BINARY_DIR}/maxctrl/maxctrl) + install_script(${CMAKE_BINARY_DIR}/maxctrl/maxctrl core) - add_custom_target(test_maxctrl - COMMAND ${CMAKE_SOURCE_DIR}/test/run_npm_test.sh - ${CMAKE_SOURCE_DIR} # Path to MaxScale sources - ${CMAKE_CURRENT_SOURCE_DIR} # Path to test sources - ${CMAKE_BINARY_DIR}/maxctrl-test/ # Location where tests are built and run - WORKING_DIRECTORY ${CMAKE_BINARY_DIR}) + add_custom_target(test_maxctrl + COMMAND ${CMAKE_SOURCE_DIR}/test/run_npm_test.sh + ${CMAKE_SOURCE_DIR} # Path to MaxScale sources + ${CMAKE_CURRENT_SOURCE_DIR} # Path to test sources + ${CMAKE_BINARY_DIR}/maxctrl-test/ # Location where tests are built and run + WORKING_DIRECTORY ${CMAKE_BINARY_DIR}) + else() + message(FATAL_ERROR "Not building MaxCtrl: npm or Node.js >= 6.0.0 not found. Add the following to skip MaxCtrl: -DBUILD_MAXCTRL=N") + endif() else() - message(STATUS "Not building MaxCtrl: npm or Node.js >= 6.0.0 not found") + messages(STATUS "Not building MaxCtrl: BUILD_MAXCTRL=N") endif()