MXS-2544 Add utility class for handling SQLite

Preparation and cleanup for adding more data to handle user roles in PAM
authenticator.

server/modules/authenticator/PAM/PAMAuth/pam_auth.cc

@@ -15,6 +15,7 @@
 #include <string>
 #include <maxscale/authenticator.hh>
 #include <maxscale/users.h>
+#include <maxbase/format.hh>
 
 #include "pam_instance.hh"
 #include "pam_client_session.hh"
@@ -32,6 +33,79 @@ const int NUM_FIELDS = 6;
 const char* SQLITE_OPEN_FAIL = "Failed to open SQLite3 handle for file '%s': '%s'";
 const char* SQLITE_OPEN_OOM = "Failed to allocate memory for SQLite3 handle for file '%s'.";
 
+using SSQLite = SQLite::SSQLite;
+
+SSQLite SQLite::create(const string& filename, int flags, string* error_out)
+{
+    SSQLite rval;
+    sqlite3* dbhandle = nullptr;
+    const char* zFilename = filename.c_str();
+    int ret = sqlite3_open_v2(zFilename, &dbhandle, flags, NULL);
+    string error_msg;
+    if (ret == SQLITE_OK)
+    {
+        rval.reset(new SQLite(dbhandle));
+    }
+    // Even if the open failed, the handle may exist and an error message can be read.
+    else if (dbhandle)
+    {
+        error_msg = mxb::string_printf(SQLITE_OPEN_FAIL, zFilename, sqlite3_errmsg(dbhandle));
+        sqlite3_close_v2(dbhandle);
+    }
+    else
+    {
+        error_msg = mxb::string_printf(SQLITE_OPEN_OOM, zFilename);
+    }
+
+    if (!error_msg.empty() && error_out)
+    {
+        *error_out = error_msg;
+    }
+    return rval;
+}
+
+SQLite::SQLite(sqlite3* handle)
+    : m_dbhandle(handle)
+{
+    mxb_assert(handle);
+}
+
+SQLite::~SQLite()
+{
+    sqlite3_close_v2(m_dbhandle);
+}
+
+bool SQLite::exec(const std::string& sql)
+{
+    return exec_impl(sql, nullptr, nullptr);
+}
+
+bool SQLite::exec_impl(const std::string& sql, CallbackVoid cb, void* cb_data)
+{
+    char* err = nullptr;
+    bool success = (sqlite3_exec(m_dbhandle, sql.c_str(), cb, cb_data, &err) == SQLITE_OK);
+    if (success)
+    {
+        m_errormsg.clear();
+    }
+    else
+    {
+        m_errormsg = err;
+        sqlite3_free(err);
+    }
+    return success;
+}
+
+void SQLite::set_timeout(int ms)
+{
+    sqlite3_busy_timeout(m_dbhandle, ms);
+}
+
+const char* SQLite::error() const
+{
+    return m_errormsg.c_str();
+}
+
 /**
  * Initialize PAM authenticator
  *

server/modules/authenticator/PAM/PAMAuth/pam_auth.hh

@@ -37,3 +37,77 @@ extern const int NUM_FIELDS;
 
 extern const char* SQLITE_OPEN_FAIL;
 extern const char* SQLITE_OPEN_OOM;
+
+struct sqlite3;
+
+class SQLite;
+
+/**
+ * Convenience class for working with SQLite.
+ */
+class SQLite
+{
+public:
+    SQLite(const SQLite& rhs) = delete;
+    SQLite& operator=(const SQLite& rhs) = delete;
+
+    using SSQLite = std::unique_ptr<SQLite>;
+
+    template<class T>
+    using Callback = int (*)(T* data, int n_columns, char** rows, char** field_names);
+
+    /**
+     * Create a new database handle.
+     *
+     * @param filename The filename/url given to sqlite3_open_v2
+     * @param flags Flags given to sqlite3_open_v2
+     * @return New handle if successful, null otherwise.
+     */
+    static SSQLite create(const std::string& filename, int flags, std::string* error_out);
+    ~SQLite();
+
+    /**
+     * Run a simple query which returns no data.
+     *
+     * @param sql SQL to run
+     * @return True on success
+     */
+    bool exec(const std::string& sql);
+
+    /**
+     * Run a query which may return data.
+     *
+     * @param sql SQL to run
+     * @param cb Callback given to sqlite3_exec
+     * @param cb_data Data pointer given to sqlite3_exec
+     * @return True on success
+     */
+    template <class T>
+    bool exec(const std::string& sql, Callback<T> cb, T* cb_data)
+    {
+        return exec_impl(sql, reinterpret_cast<CallbackVoid>(cb), cb_data);
+    }
+
+    /**
+     * Calls sqlite3_busy_timeout.
+     *
+     * @param ms The timeout in ms
+     */
+    void set_timeout(int ms);
+
+    /**
+     * Get latest error.
+     *
+     * @return Error string
+     */
+    const char* error() const;
+
+private:
+    using CallbackVoid = int (*)(void* data, int n_columns, char** rows, char** field_names);
+    bool exec_impl(const std::string& sql, CallbackVoid cb, void* cb_data);
+
+    SQLite(sqlite3* handle);
+
+    sqlite3* m_dbhandle {nullptr};
+    std::string m_errormsg;
+};

server/modules/authenticator/PAM/PAMAuth/pam_client_session.cc

@@ -20,6 +20,8 @@
 using maxscale::Buffer;
 using std::string;
 
+using SSQLite = SQLite::SSQLite;
+
 namespace
 {
 /**
@@ -59,67 +61,44 @@ bool store_client_password(DCB* dcb, GWBUF* buffer)
  * @param column_names Column names
  * @return Always 0
  */
-int user_services_cb(void* data, int columns, char** column_vals, char** column_names)
+int user_services_cb(PamClientSession::StringVector* data, int columns, char** column_vals,
+                     char** column_names)
 {
     mxb_assert(columns == 1);
-    PamClientSession::StringVector* results = static_cast<PamClientSession::StringVector*>(data);
     if (column_vals[0])
     {
-        results->push_back(column_vals[0]);
+        data->push_back(column_vals[0]);
     }
     else
     {
         // Empty is a valid value.
-        results->push_back("");
+        data->push_back("");
     }
     return 0;
 }
 }
 
-PamClientSession::PamClientSession(sqlite3* dbhandle, const PamInstance& instance)
+PamClientSession::PamClientSession(const PamInstance& instance, SSQLite sqlite)
-    : m_dbhandle(dbhandle)
+    : m_instance(instance)
-    , m_instance(instance)
+    , m_sqlite(std::move(sqlite))
 {
 }
 
-PamClientSession::~PamClientSession()
-{
-    sqlite3_close_v2(m_dbhandle);
-}
-
 PamClientSession* PamClientSession::create(const PamInstance& inst)
 {
+    PamClientSession* rval = nullptr;
     // This handle is only used from one thread, can define no_mutex.
-    sqlite3* dbhandle = NULL;
-    bool error = false;
     int db_flags = SQLITE_OPEN_READONLY | SQLITE_OPEN_SHAREDCACHE | SQLITE_OPEN_NOMUTEX;
-    const char* filename = inst.m_dbname.c_str();
+    string sqlite_error;
-    if (sqlite3_open_v2(filename, &dbhandle, db_flags, NULL) == SQLITE_OK)
+    auto sqlite = SQLite::create(inst.m_dbname, db_flags, &sqlite_error);
+    if (sqlite)
     {
-        sqlite3_busy_timeout(dbhandle, 1000);
+        sqlite->set_timeout(1000);
+        rval = new(std::nothrow) PamClientSession(inst, std::move(sqlite));
     }
     else
     {
-        if (dbhandle)
+        MXB_ERROR("Could not create PAM authenticator session: %s", sqlite_error.c_str());
-        {
-            MXS_ERROR(SQLITE_OPEN_FAIL, filename, sqlite3_errmsg(dbhandle));
-        }
-        else
-        {
-            MXS_ERROR(SQLITE_OPEN_OOM, filename);
-        }
-        error = true;
-    }
-
-    PamClientSession* rval = NULL;
-    if (!error && ((rval = new (std::nothrow) PamClientSession(dbhandle, inst)) == NULL))
-    {
-        error = true;
-    }
-
-    if (error)
-    {
-        sqlite3_close_v2(dbhandle);
     }
     return rval;
 }
@@ -142,11 +121,9 @@ void PamClientSession::get_pam_user_services(const DCB* dcb, const MYSQL_session
         + " AND " + FIELD_PROXY + " = '0' ORDER BY authentication_string;";
     MXS_DEBUG("PAM services search sql: '%s'.", services_query.c_str());
 
-    char* err;
+    if (!m_sqlite->exec(services_query, user_services_cb, services_out))
-    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);
+        MXS_ERROR("Failed to execute query: '%s'", m_sqlite->error());
-        sqlite3_free(err);
     }
 
     auto word_entry = [](size_t num) -> const char* {
@@ -169,24 +146,23 @@ void PamClientSession::get_pam_user_services(const DCB* dcb, const MYSQL_session
             + " AND " + FIELD_PROXY + " = '1' ORDER BY authentication_string;";
         MXS_DEBUG("PAM proxy user services search sql: '%s'.", anon_query.c_str());
 
-        if (sqlite3_exec(m_dbhandle, anon_query.c_str(), user_services_cb, services_out, &err) != SQLITE_OK)
+        if (m_sqlite->exec(anon_query, user_services_cb, services_out))
-        {
-            MXS_ERROR("Failed to execute query: '%s'", err);
-            sqlite3_free(err);
-        }
-        else
         {
             auto num_services = services_out->size();
             if (num_services == 0)
             {
-                MXS_INFO("Found no PAM user entries for '%s'@'%s'.", session->user, dcb->remote);
+                MXB_INFO("Found no PAM user entries for '%s'@'%s'.", session->user, dcb->remote);
             }
             else
             {
-                MXS_INFO("Found %lu matching anonymous PAM user %s for '%s'@'%s'.",
+                MXB_INFO("Found %lu matching anonymous PAM user %s for '%s'@'%s'.",
                          num_services, word_entry(num_services), session->user, dcb->remote);
             }
         }
+        else
+        {
+            MXB_ERROR("Failed to execute query: '%s'", m_sqlite->error());
+        }
     }
 }
 

server/modules/authenticator/PAM/PAMAuth/pam_client_session.hh

@@ -23,16 +23,18 @@
 /** Client authenticator PAM-specific session data */
 class PamClientSession
 {
-    PamClientSession(const PamClientSession& orig);
-    PamClientSession& operator=(const PamClientSession&);
 public:
+    PamClientSession(const PamClientSession& orig) = delete;
+    PamClientSession& operator=(const PamClientSession&) = delete;
+
     typedef std::vector<std::string> StringVector;
     static PamClientSession* create(const PamInstance& inst);
-    ~PamClientSession();
+
     int  authenticate(DCB* client);
     bool extract(DCB* dcb, GWBUF* read_buffer);
+
 private:
-    PamClientSession(sqlite3* dbhandle, const PamInstance& instance);
+    PamClientSession(const PamInstance& instance, SQLite::SSQLite sqlite);
     void get_pam_user_services(const DCB* dcb,
                                const MYSQL_session* session,
                                StringVector* services_out);
@@ -46,8 +48,9 @@ private:
         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 */
+    SQLite::SSQLite const m_sqlite;                /**< SQLite3 database handle */
+
+    State    m_state {State::INIT};   /**< Authentication state */
+    uint8_t  m_sequence {0};          /**< The next packet seqence number */
 };

server/modules/authenticator/PAM/PAMAuth/pam_instance.cc

@@ -49,57 +49,30 @@ PamInstance* PamInstance::create(char** options)
 
     if (sqlite3_threadsafe() == 0)
     {
-        MXS_WARNING("SQLite3 was compiled with thread safety off. May cause "
+        MXB_WARNING("SQLite3 was compiled with thread safety off. May cause corruption of "
-                    "corruption of in-memory database.");
+                    "in-memory database.");
     }
 
-    bool error = false;
     /* This handle may be used from multiple threads, set full mutex. */
-    sqlite3* dbhandle = NULL;
     int db_flags = SQLITE_OPEN_READWRITE | SQLITE_OPEN_CREATE |
                    SQLITE_OPEN_SHAREDCACHE | SQLITE_OPEN_FULLMUTEX;
-    const char* filename = pam_db_fname.c_str();
+    string sqlite_error;
-    if (sqlite3_open_v2(filename, &dbhandle, db_flags, NULL) != SQLITE_OK)
+    PamInstance* instance = nullptr;
+    auto sqlite = SQLite::create(pam_db_fname, db_flags, &sqlite_error);
+    if (sqlite)
     {
-        // Even if the open failed, the handle may exist and an error message can be read.
+        instance = new PamInstance(std::move(sqlite), pam_db_fname);
-        if (dbhandle)
+        if (!instance->prepare_tables())
         {
-            MXS_ERROR(SQLITE_OPEN_FAIL, filename, sqlite3_errmsg(dbhandle));
+            delete instance;
+            instance = nullptr;
+        }
     }
     else
     {
-            // This means memory allocation failed.
+        MXB_ERROR("Could not create PAM authenticator: %s", sqlite_error.c_str());
-            MXS_ERROR(SQLITE_OPEN_OOM, filename);
-        }
-        error = true;
     }
 
-    char *err = NULL;
-    if (!error && sqlite3_exec(dbhandle, drop_sql.c_str(), NULL, NULL, &err) != SQLITE_OK)
-    {
-        MXS_ERROR("Failed to drop table: '%s'", err);
-        sqlite3_free(err);
-        error = true;
-    }
-    if (!error && sqlite3_exec(dbhandle, create_sql.c_str(), NULL, NULL, &err) != SQLITE_OK)
-    {
-        MXS_ERROR("Failed to create table: '%s'", err);
-        sqlite3_free(err);
-        error = true;
-    }
-
-    PamInstance *instance = NULL;
-    if (!error &&
-        ((instance = new (std::nothrow) PamInstance(dbhandle, pam_db_fname, pam_table_name)) == NULL))
-    {
-        error = true;
-    }
-
-    if (error)
-    {
-        // Close the handle even if never opened.
-        sqlite3_close_v2(dbhandle);
-    }
     return instance;
 }
 
@@ -110,13 +83,81 @@ PamInstance* PamInstance::create(char** options)
  * @param dbname Text-form name of @c dbhandle
  * @param tablename Name of table where authentication data is saved
  */
-PamInstance::PamInstance(sqlite3* dbhandle, const string& dbname, const string& tablename)
+PamInstance::PamInstance(SQLite::SSQLite dbhandle, const string& dbname)
     : m_dbname(dbname)
-    , m_tablename(tablename)
+    , m_tablename("pam_users")
-    , m_dbhandle(dbhandle)
+    , m_sqlite(std::move(dbhandle))
 {
 }
 
+bool PamInstance::prepare_tables()
+{
+    struct ColDef
+    {
+        enum class ColType {
+            BOOL,
+            TEXT,
+        };
+        string name;
+        ColType type;
+    };
+    using ColDefArray = std::vector<ColDef>;
+    using Type = ColDef::ColType;
+
+    /** Deletion statement for the in-memory table */
+    auto gen_drop_sql = [](const string& tblname) {
+        return "DROP TABLE IF EXISTS " + tblname + ";";
+    };
+
+    /** CREATE TABLE statement for the in-memory table */
+    auto gen_create_sql = [](const string& tblname, const ColDefArray& coldefs) {
+        string rval = "CREATE TABLE " + tblname + " (";
+        string sep;
+        for (const auto& coldef : coldefs)
+        {
+            string column_type;
+            switch (coldef.type)
+            {
+                case Type::BOOL:
+                    column_type = "BOOLEAN";
+                    break;
+                case Type::TEXT:
+                    column_type = "TINYTEXT";
+                    break;
+            }
+            rval += sep + coldef.name + " " + column_type;
+            sep = ",";
+        }
+        rval += "\n);";
+        return rval;
+    };
+
+    /** Table names and columns. The tables mostly match the ones in the server, but have only
+     *  a subset of columns. */
+
+    const string users = m_tablename;
+    const string drop_users_sql = gen_drop_sql(users);
+    // Sqlite3 doesn't require datatypes in the create-statement but it's good to have for clarity.
+    const ColDefArray users_coldef = {{FIELD_USER, Type::TEXT},
+                                      {FIELD_HOST, Type::TEXT},
+                                      {FIELD_DB, Type::TEXT},
+                                      {FIELD_ANYDB, Type::BOOL},
+                                      {FIELD_AUTHSTR, Type::TEXT},
+                                      {FIELD_PROXY, Type::BOOL}};
+    const string create_users_sql = gen_create_sql(users, users_coldef);
+
+    bool rval = false;
+    if (m_sqlite->exec(drop_users_sql) && m_sqlite->exec(create_users_sql))
+    {
+        rval = true;
+    }
+    else
+    {
+        MXB_ERROR("Failed to create sqlite3 table: %s", m_sqlite->error());
+    }
+    return rval;
+}
+
 /**
  * @brief Add new PAM user entry to the internal user database
  *
@@ -173,24 +214,22 @@ void PamInstance::add_pam_user(const char* user, const char* host, const char* d
             service_str.c_str(),
             proxy ? "1" : "0");
 
-    char* err;
+    if (m_sqlite->exec(insert_sql))
-    if (sqlite3_exec(m_dbhandle, insert_sql, NULL, NULL, &err) != SQLITE_OK)
-    {
-        MXS_ERROR("Failed to insert user: %s", err);
-        sqlite3_free(err);
-    }
-    else
     {
         if (proxy)
         {
-            MXS_INFO("Added anonymous PAM user ''@'%s' with proxy grants using service %s.",
+            MXB_INFO("Added anonymous PAM user ''@'%s' with proxy grants using service %s.",
                      host, service_str.c_str());
         }
         else
         {
-            MXS_INFO("Added normal PAM user '%s'@'%s' using service %s.", user, host, service_str.c_str());
+            MXB_INFO("Added normal PAM user '%s'@'%s' using service %s.", user, host, service_str.c_str());
         }
     }
+    else
+    {
+        MXB_ERROR("Failed to insert user: %s", m_sqlite->error());
+    }
 }
 
 /**
@@ -200,11 +239,9 @@ void PamInstance::delete_old_users()
 {
     /** Delete query used to clean up the database before loading new users */
     const string delete_query = "DELETE FROM " + m_tablename;
-    char* err;
+    if (!m_sqlite->exec(delete_query))
-    if (sqlite3_exec(m_dbhandle, delete_query.c_str(), NULL, NULL, &err) != SQLITE_OK)
     {
-        MXS_ERROR("Failed to delete old users: %s", err);
+        MXB_ERROR("Failed to delete old users: %s", m_sqlite->error());
-        sqlite3_free(err);
     }
 }
 
@@ -313,7 +350,7 @@ void PamInstance::diagnostic(DCB* dcb)
     json_decref(array);
 }
 
-static int diag_cb_json(void* data, int columns, char** row, char** field_names)
+static int diag_cb_json(json_t* data, int columns, char** row, char** field_names)
 {
     mxb_assert(columns == NUM_FIELDS);
     json_t* obj = json_object();
@@ -321,22 +358,18 @@ static int diag_cb_json(void* data, int columns, char** row, char** field_names)
     {
         json_object_set_new(obj, field_names[i], json_string(row[i]));
     }
-    json_t* arr = static_cast<json_t*>(data);
+    json_array_append_new(data, obj);
-    json_array_append_new(arr, obj);
     return 0;
 }
 
 json_t* PamInstance::diagnostic_json()
 {
     json_t* rval = json_array();
-    char* err;
     string select = "SELECT * FROM " + m_tablename + ";";
-    if (sqlite3_exec(m_dbhandle, select.c_str(), diag_cb_json, rval, &err) != SQLITE_OK)
+    if (!m_sqlite->exec(select, diag_cb_json, rval))
     {
-        MXS_ERROR("Failed to print users: %s", err);
+        MXS_ERROR("Failed to print users: %s", m_sqlite->error());
-        sqlite3_free(err);
     }
-
     return rval;
 }
 

server/modules/authenticator/PAM/PAMAuth/pam_instance.hh

@@ -20,23 +20,27 @@
 /** The instance class for the client side PAM authenticator, created in pam_auth_init() */
 class PamInstance
 {
-    PamInstance(const PamInstance& orig);
-    PamInstance& operator=(const PamInstance&);
 public:
+    PamInstance(const PamInstance& orig) = delete;
+    PamInstance& operator=(const PamInstance&) = delete;
+
     static PamInstance* create(char** options);
-    ~PamInstance();
+
     int     load_users(SERVICE* service);
     void    diagnostic(DCB* dcb);
     json_t* diagnostic_json();
 
     const std::string m_dbname;     /**< Name of the in-memory database */
     const std::string m_tablename;  /**< The table where users are stored */
+
 private:
-    PamInstance(sqlite3* dbhandle, const std::string& m_dbname, const std::string& tablename);
+    PamInstance(SQLite::SSQLite dbhandle, const std::string& dbname);
+    bool prepare_tables();
+
     void add_pam_user(const char* user, const char* host, const char* db, bool anydb,
                       const char* pam_service, bool proxy);
     void delete_old_users();
     bool fetch_anon_proxy_users(SERVER* server, MYSQL* conn);
 
-    sqlite3* const m_dbhandle;      /**< SQLite3 database handle */
+    SQLite::SSQLite const m_sqlite;      /**< SQLite3 database handle */
 };