MXS-2642 Do not re-test a pam-service for a given user

Because of how the user-data was read, the same service name could be
found multiple times if the user-search query matched multiple rows. Now
the service names are read to a set, which ignores duplicates. The same
service may be attempted again if the authentication fails and user-data
is fetched again.
This commit is contained in:
Esa Korhonen
2019-08-27 15:34:44 +03:00
parent 7a1abc26d8
commit 6edbd52324
2 changed files with 14 additions and 15 deletions

View File

@ -62,15 +62,15 @@ bool store_client_password(DCB* dcb, GWBUF* buffer)
int user_services_cb(void* data, int columns, char** column_vals, char** column_names) int user_services_cb(void* data, int columns, char** column_vals, char** column_names)
{ {
mxb_assert(columns == 1); mxb_assert(columns == 1);
PamClientSession::StringVector* results = static_cast<PamClientSession::StringVector*>(data); auto results = static_cast<PamClientSession::StringSet*>(data);
if (column_vals[0]) if (column_vals[0])
{ {
results->push_back(column_vals[0]); results->insert(column_vals[0]);
} }
else else
{ {
// Empty is a valid value. // Empty is a valid value.
results->push_back(""); results->insert("");
} }
return 0; return 0;
} }
@ -302,7 +302,7 @@ PamClientSession* PamClientSession::create(const PamInstance& inst)
* @param services_out Output for services * @param services_out Output for services
*/ */
void PamClientSession::get_pam_user_services(const DCB* dcb, const MYSQL_session* session, void PamClientSession::get_pam_user_services(const DCB* dcb, const MYSQL_session* session,
StringVector* services_out) StringSet* services_out)
{ {
string services_query = string("SELECT authentication_string FROM ") + m_instance.m_tablename + " WHERE " string services_query = string("SELECT authentication_string FROM ") + m_instance.m_tablename + " WHERE "
+ FIELD_USER + " = '" + session->user + "'" + FIELD_USER + " = '" + session->user + "'"
@ -430,13 +430,13 @@ int PamClientSession::authenticate(DCB* dcb)
* check is useless if the user services are same as on the first attempt. * check is useless if the user services are same as on the first attempt.
*/ */
bool authenticated = false; bool authenticated = false;
StringVector services_old; StringSet services_old;
for (int loop = 0; loop < 2 && !authenticated; loop++) for (int loop = 0; loop < 2 && !authenticated; loop++)
{ {
if (loop == 0 || service_refresh_users(dcb->service) == 0) if (loop == 0 || service_refresh_users(dcb->service) == 0)
{ {
bool try_validate = true; bool try_validate = true;
StringVector services; StringSet services;
get_pam_user_services(dcb, ses, &services); get_pam_user_services(dcb, ses, &services);
if (loop == 0) if (loop == 0)
{ {
@ -448,17 +448,16 @@ int PamClientSession::authenticate(DCB* dcb)
} }
if (try_validate) if (try_validate)
{ {
for (StringVector::iterator iter = services.begin(); for (auto iter = services.begin(); iter != services.end() && !authenticated; ++iter)
iter != services.end() && !authenticated;
iter++)
{ {
string pam_service = *iter;
// The server PAM plugin uses "mysql" as the default service when authenticating // The server PAM plugin uses "mysql" as the default service when authenticating
// a user with no service. // a user with no service.
if (iter->empty()) if (pam_service.empty())
{ {
*iter = "mysql"; pam_service = "mysql";
} }
if (validate_pam_password(ses->user, password, *iter, dcb->remote)) if (validate_pam_password(ses->user, password, pam_service, dcb->remote))
{ {
authenticated = true; authenticated = true;
} }

View File

@ -15,7 +15,7 @@
#include <stdint.h> #include <stdint.h>
#include <string> #include <string>
#include <vector> #include <set>
#include <maxscale/sqlite3.h> #include <maxscale/sqlite3.h>
#include "pam_instance.hh" #include "pam_instance.hh"
#include "../pam_auth_common.hh" #include "../pam_auth_common.hh"
@ -26,14 +26,14 @@ class PamClientSession
PamClientSession(const PamClientSession& orig); PamClientSession(const PamClientSession& orig);
PamClientSession& operator=(const PamClientSession&); PamClientSession& operator=(const PamClientSession&);
public: public:
typedef std::vector<std::string> StringVector; using StringSet = std::set<std::string>;
static PamClientSession* create(const PamInstance& inst); static PamClientSession* create(const PamInstance& inst);
~PamClientSession(); ~PamClientSession();
int authenticate(DCB* client); int authenticate(DCB* client);
bool extract(DCB* dcb, GWBUF* read_buffer); bool extract(DCB* dcb, GWBUF* read_buffer);
private: private:
PamClientSession(sqlite3* dbhandle, const PamInstance& instance); PamClientSession(sqlite3* dbhandle, const PamInstance& instance);
void get_pam_user_services(const DCB* dcb, const MYSQL_session* session, StringVector* services_out); void get_pam_user_services(const DCB* dcb, const MYSQL_session* session, StringSet* services_out);
maxscale::Buffer create_auth_change_packet() const; maxscale::Buffer create_auth_change_packet() const;
enum class State enum class State