From: Arran Cudbard-Bell Date: Thu, 3 Mar 2022 00:17:51 +0000 (-0600) Subject: Use the common submodule infrastructure for rlm_sql X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=5eb089c69aa38f8dbdca66c5926449aec674fc7e;p=thirdparty%2Ffreeradius-server.git Use the common submodule infrastructure for rlm_sql This gives us thread instantiation/detach for sql drivers --- diff --git a/doc/antora/modules/howto/pages/modules/sql/odbc.adoc b/doc/antora/modules/howto/pages/modules/sql/odbc.adoc index 59660368232..484fe36b597 100644 --- a/doc/antora/modules/howto/pages/modules/sql/odbc.adoc +++ b/doc/antora/modules/howto/pages/modules/sql/odbc.adoc @@ -265,7 +265,7 @@ The `rlm_sql` module can be configured as follows: sql { dialect = "mssql" - driver = "rlm_sql_unixodbc" + driver = "unixodbc" server = "MSSQLdb" # The exact "[DSN]" from odbc.ini login = "radius" password = "radPass_123" diff --git a/doc/antora/modules/raddb/pages/mods-available/abfab_psk_sql.adoc b/doc/antora/modules/raddb/pages/mods-available/abfab_psk_sql.adoc index 6a7830e43ca..9002ae32385 100644 --- a/doc/antora/modules/raddb/pages/mods-available/abfab_psk_sql.adoc +++ b/doc/antora/modules/raddb/pages/mods-available/abfab_psk_sql.adoc @@ -26,7 +26,7 @@ sqlite { ... }:: Database statement based on `driver` setting. ``` sql psksql { - driver = "rlm_sql_sqlite" + driver = "sqlite" sqlite { filename = "/var/lib/trust_router/keys" } diff --git a/doc/antora/modules/raddb/pages/mods-available/cui.adoc b/doc/antora/modules/raddb/pages/mods-available/cui.adoc index 6fc9be04246..e0e2b4881bf 100644 --- a/doc/antora/modules/raddb/pages/mods-available/cui.adoc +++ b/doc/antora/modules/raddb/pages/mods-available/cui.adoc @@ -79,7 +79,7 @@ set `SQL-User-Name` to the new value. ``` sql cuisql { dialect = "sqlite" - driver = "rlm_sql_${dialect}" + driver = "${dialect}" # server = "localhost" # port = 3306 # login = "radius" diff --git a/doc/antora/modules/raddb/pages/mods-available/sql.adoc b/doc/antora/modules/raddb/pages/mods-available/sql.adoc index 7e888079ee3..667e2ec3fda 100644 --- a/doc/antora/modules/raddb/pages/mods-available/sql.adoc +++ b/doc/antora/modules/raddb/pages/mods-available/sql.adoc @@ -293,7 +293,7 @@ please create them and contribute them back to the project. ``` sql { dialect = "sqlite" - driver = "rlm_sql_${dialect}" + driver = "${dialect}" $-INCLUDE ${modconfdir}/sql/driver/${dialect} # server = "localhost" # port = 3306 diff --git a/raddb/mods-available/abfab_psk_sql b/raddb/mods-available/abfab_psk_sql index 2b3cbf5a89f..71659468be7 100644 --- a/raddb/mods-available/abfab_psk_sql +++ b/raddb/mods-available/abfab_psk_sql @@ -21,7 +21,7 @@ sql psksql { # # driver:: Database driver. # - driver = "rlm_sql_sqlite" + driver = "sqlite" # # sqlite { ... }:: Database statement based on `driver` setting. diff --git a/raddb/mods-available/cui b/raddb/mods-available/cui index 89140217b5a..37a50f4d438 100644 --- a/raddb/mods-available/cui +++ b/raddb/mods-available/cui @@ -44,7 +44,7 @@ sql cuisql { # | Log to disk | rlm_sql_null # |=== # - driver = "rlm_sql_${dialect}" + driver = "${dialect}" # # [NOTE] diff --git a/raddb/mods-available/sql b/raddb/mods-available/sql index 1e99b081d24..cbc5efee708 100644 --- a/raddb/mods-available/sql +++ b/raddb/mods-available/sql @@ -63,14 +63,14 @@ sql { # [options="header,autowidth"] # |=== # | Driver | Dialect - # | rlm_sql_db2 | mssql - # | rlm_sql_firebird | mssql - # | rlm_sql_freetds | mssql - # | rlm_sql_null | any - # | rlm_sql_unixodbc | mssql + # | db2 | mssql + # | firebird | mssql + # | freetds | mssql + # | null | any + # | unixodbc | mssql # |=== # - driver = "rlm_sql_${dialect}" + driver = "${dialect}" # # Include driver specific configuration file if one diff --git a/src/modules/rlm_sql/drivers/rlm_sql_cassandra/rlm_sql_cassandra.c b/src/modules/rlm_sql/drivers/rlm_sql_cassandra/rlm_sql_cassandra.c index fd9364889cb..8bacaedceee 100644 --- a/src/modules/rlm_sql/drivers/rlm_sql_cassandra/rlm_sql_cassandra.c +++ b/src/modules/rlm_sql/drivers/rlm_sql_cassandra/rlm_sql_cassandra.c @@ -386,7 +386,7 @@ static int _sql_socket_destructor(rlm_sql_cassandra_conn_t *conn) static sql_rcode_t sql_socket_init(rlm_sql_handle_t *handle, rlm_sql_config_t const *config, fr_time_delta_t timeout) { rlm_sql_cassandra_conn_t *conn; - rlm_sql_cassandra_t *inst = config->driver; + rlm_sql_cassandra_t *inst = talloc_get_type_abort(handle->inst->driver_submodule->dl_inst->data, rlm_sql_cassandra_t); MEM(conn = handle->conn = talloc_zero(handle, rlm_sql_cassandra_conn_t)); talloc_set_destructor(conn, _sql_socket_destructor); @@ -435,7 +435,7 @@ static sql_rcode_t sql_socket_init(rlm_sql_handle_t *handle, rlm_sql_config_t co static sql_rcode_t sql_query(rlm_sql_handle_t *handle, rlm_sql_config_t const *config, char const *query) { rlm_sql_cassandra_conn_t *conn = handle->conn; - rlm_sql_cassandra_t *conf = config->driver; + rlm_sql_cassandra_t *inst = talloc_get_type_abort(handle->inst->driver_submodule->dl_inst->data, rlm_sql_cassandra_t); CassStatement *statement; CassFuture *future; CassError ret; diff --git a/src/modules/rlm_sql/drivers/rlm_sql_mysql/rlm_sql_mysql.c b/src/modules/rlm_sql/drivers/rlm_sql_mysql/rlm_sql_mysql.c index b75eee19b2b..fa127408fb5 100644 --- a/src/modules/rlm_sql/drivers/rlm_sql_mysql/rlm_sql_mysql.c +++ b/src/modules/rlm_sql/drivers/rlm_sql_mysql/rlm_sql_mysql.c @@ -219,8 +219,9 @@ static int mod_load(void) static sql_rcode_t sql_socket_init(rlm_sql_handle_t *handle, rlm_sql_config_t const *config, fr_time_delta_t timeout) { + rlm_sql_mysql_t *inst = talloc_get_type_abort(handle->inst->driver_submodule->dl_inst->data, rlm_sql_mysql_t); rlm_sql_mysql_conn_t *conn; - rlm_sql_mysql_t *inst = config->driver; + unsigned int connect_timeout = (unsigned int)fr_time_delta_to_sec(timeout); unsigned long sql_flags; @@ -704,8 +705,8 @@ static size_t sql_warnings(TALLOC_CTX *ctx, sql_log_entry_t out[], size_t outlen static size_t sql_error(TALLOC_CTX *ctx, sql_log_entry_t out[], size_t outlen, rlm_sql_handle_t *handle, rlm_sql_config_t const *config) { + rlm_sql_mysql_t *inst = talloc_get_type_abort(handle->inst->driver_submodule->dl_inst->data, rlm_sql_mysql_t); rlm_sql_mysql_conn_t *conn = handle->conn; - rlm_sql_mysql_t *inst = config->driver; char const *error; size_t i = 0; diff --git a/src/modules/rlm_sql/drivers/rlm_sql_oracle/rlm_sql_oracle.c b/src/modules/rlm_sql/drivers/rlm_sql_oracle/rlm_sql_oracle.c index 27d1762eccb..9e436edd709 100644 --- a/src/modules/rlm_sql/drivers/rlm_sql_oracle/rlm_sql_oracle.c +++ b/src/modules/rlm_sql/drivers/rlm_sql_oracle/rlm_sql_oracle.c @@ -263,8 +263,8 @@ static sql_rcode_t sql_socket_init(rlm_sql_handle_t *handle, rlm_sql_config_t co { char errbuff[512]; - rlm_sql_oracle_t *inst = config->driver; - rlm_sql_oracle_conn_t *conn; + rlm_sql_oracle_t *inst = talloc_get_type_abort(handle->inst->driver_submodule->dl_inst->data, rlm_sql_oracle_t); + rlm_sql_oracle_conn_t *conn; MEM(conn = handle->conn = talloc_zero(handle, rlm_sql_oracle_conn_t)); talloc_set_destructor(conn, _sql_socket_destructor); diff --git a/src/modules/rlm_sql/drivers/rlm_sql_postgresql/rlm_sql_postgresql.c b/src/modules/rlm_sql/drivers/rlm_sql_postgresql/rlm_sql_postgresql.c index 334f7d968a9..97c37c24f88 100644 --- a/src/modules/rlm_sql/drivers/rlm_sql_postgresql/rlm_sql_postgresql.c +++ b/src/modules/rlm_sql/drivers/rlm_sql_postgresql/rlm_sql_postgresql.c @@ -232,10 +232,10 @@ static int _sql_socket_destructor(rlm_sql_postgres_conn_t *conn) return 0; } -static int CC_HINT(nonnull) sql_socket_init(rlm_sql_handle_t *handle, rlm_sql_config_t const *config, +static int CC_HINT(nonnull) sql_socket_init(rlm_sql_handle_t *handle, UNUSED rlm_sql_config_t const *config, UNUSED fr_time_delta_t timeout) { - rlm_sql_postgresql_t *inst = config->driver; + rlm_sql_postgresql_t *inst = talloc_get_type_abort(handle->inst->driver_submodule->dl_inst->data, rlm_sql_postgresql_t); rlm_sql_postgres_conn_t *conn; MEM(conn = handle->conn = talloc_zero(handle, rlm_sql_postgres_conn_t)); @@ -265,7 +265,7 @@ static CC_HINT(nonnull) sql_rcode_t sql_query(rlm_sql_handle_t *handle, rlm_sql_ char const *query) { rlm_sql_postgres_conn_t *conn = handle->conn; - rlm_sql_postgresql_t *inst = config->driver; + rlm_sql_postgresql_t *inst = talloc_get_type_abort(handle->inst->driver_submodule->dl_inst->data, rlm_sql_postgresql_t); fr_time_delta_t timeout = config->query_timeout; fr_time_t start; int sockfd; diff --git a/src/modules/rlm_sql/drivers/rlm_sql_sqlite/rlm_sql_sqlite.c b/src/modules/rlm_sql/drivers/rlm_sql_sqlite/rlm_sql_sqlite.c index 78b3b1b033a..e6e81754bdb 100644 --- a/src/modules/rlm_sql/drivers/rlm_sql_sqlite/rlm_sql_sqlite.c +++ b/src/modules/rlm_sql/drivers/rlm_sql_sqlite/rlm_sql_sqlite.c @@ -411,8 +411,8 @@ static void _sql_greatest(sqlite3_context *ctx, int num_values, sqlite3_value ** static int CC_HINT(nonnull) sql_socket_init(rlm_sql_handle_t *handle, rlm_sql_config_t const *config, UNUSED fr_time_delta_t timeout) { - rlm_sql_sqlite_conn_t *conn; - rlm_sql_sqlite_t *inst = config->driver; + rlm_sql_sqlite_conn_t *conn; + rlm_sql_sqlite_t *inst = talloc_get_type_abort(handle->inst->driver_submodule->dl_inst->data, rlm_sql_sqlite_t); int status; @@ -687,7 +687,7 @@ static int mod_instantiate(module_inst_ctx_t const *mctx) { rlm_sql_t const *parent = talloc_get_type_abort(mctx->inst->parent->data, rlm_sql_t); rlm_sql_config_t const *config = &parent->config; - rlm_sql_sqlite_t *inst = talloc_get_type_abort(mctx->inst, rlm_sql_sqlite_t); + rlm_sql_sqlite_t *inst = talloc_get_type_abort(mctx->inst->data, rlm_sql_sqlite_t); bool exists; struct stat buf; diff --git a/src/modules/rlm_sql/rlm_sql.c b/src/modules/rlm_sql/rlm_sql.c index b93c429b6d8..1cd7cbecd17 100644 --- a/src/modules/rlm_sql/rlm_sql.c +++ b/src/modules/rlm_sql/rlm_sql.c @@ -26,7 +26,7 @@ */ RCSID("$Id$") -#define LOG_PREFIX inst->name +#define LOG_PREFIX mctx->inst->name #include @@ -44,6 +44,9 @@ RCSID("$Id$") extern module_rlm_t rlm_sql; +static int driver_parse(UNUSED TALLOC_CTX *ctx, void *out, void *parent, + CONF_ITEM *ci, UNUSED CONF_PARSER const *rule); + /* * So we can do pass2 xlat checks on the queries. */ @@ -82,7 +85,8 @@ static const CONF_PARSER postauth_config[] = { }; static const CONF_PARSER module_config[] = { - { FR_CONF_OFFSET("driver", FR_TYPE_STRING, rlm_sql_config_t, sql_driver_name), .dflt = "rlm_sql_null" }, + { FR_CONF_OFFSET("driver", FR_TYPE_VOID, rlm_sql_t, driver_submodule), .dflt = "rlm_sql_null", + .func = driver_parse }, { FR_CONF_OFFSET("server", FR_TYPE_STRING, rlm_sql_config_t, sql_server), .dflt = "" }, /* Must be zero length so drivers can determine if it was set */ { FR_CONF_OFFSET("port", FR_TYPE_UINT32, rlm_sql_config_t, sql_port), .dflt = "0" }, { FR_CONF_OFFSET("login", FR_TYPE_STRING, rlm_sql_config_t, sql_login), .dflt = "" }, @@ -139,6 +143,38 @@ fr_dict_attr_autoload_t rlm_sql_dict_attr[] = { { NULL } }; +static int driver_parse(UNUSED TALLOC_CTX *ctx, void *out, void *parent, + CONF_ITEM *ci, UNUSED CONF_PARSER const *rule) +{ + rlm_sql_t *inst = talloc_get_type_abort(parent, rlm_sql_t); + char const *name = cf_pair_value(cf_item_to_pair(ci)); + CONF_SECTION *cs = cf_item_to_section(cf_parent(ci)); + CONF_SECTION *driver_cs; + module_instance_t *mi; + + fr_assert(out == &inst->driver_submodule); /* Make sure we're being told to write in the right place */ + + driver_cs = cf_section_find(cs, name, NULL); + + /* + * Allocate an empty section if one doesn't exist + * this is so defaults get parsed. + */ + if (!driver_cs) driver_cs = cf_section_alloc(cs, cs, name, NULL); + + mi = module_bootstrap(DL_MODULE_TYPE_SUBMODULE, module_by_data(parent), driver_cs); + if (unlikely(mi == NULL)) { + cf_log_err(driver_cs, "Failed loading SQL driver"); + return -1; + } + + *((module_instance_t **)out) = mi; + + inst->driver = (rlm_sql_driver_t const *)inst->driver_submodule->module; /* Public symbol exported by the submodule */ + + return 0; +} + /* * Fall-Through checking function from rlm_files.c */ @@ -1014,61 +1050,12 @@ static int mod_detach(module_detach_ctx_t const *mctx) static int mod_bootstrap(module_inst_ctx_t const *mctx) { - rlm_sql_t *inst = talloc_get_type_abort(mctx->inst->data, rlm_sql_t); - CONF_SECTION *conf = mctx->inst->conf; - CONF_SECTION *driver_cs; - char const *name; - xlat_t *xlat; + rlm_sql_t *inst = talloc_get_type_abort(mctx->inst->data, rlm_sql_t); + CONF_SECTION *conf = mctx->inst->conf; + xlat_t *xlat; xlat_arg_parser_t *sql_xlat_arg; - inst->name = cf_section_name2(conf); - if (!inst->name) inst->name = cf_section_name1(conf); - - /* - * Accomodate full and partial driver names - */ - name = strrchr(inst->config.sql_driver_name, '_'); - if (!name) { - name = inst->config.sql_driver_name; - } else { - name++; - } - - /* - * Get the module's subsection or allocate one - */ - driver_cs = cf_section_find(conf, name, NULL); - if (!driver_cs) { - driver_cs = cf_section_alloc(conf, conf, name, NULL); - if (!driver_cs) return -1; - } - - /* - * Load the driver - */ - if (dl_module_instance(inst, &inst->driver_inst, driver_cs, dl_module_instance_by_data(inst), name, DL_MODULE_TYPE_SUBMODULE) < 0) { - return -1; - } - inst->driver = (rlm_sql_driver_t const *)inst->driver_inst->module->common; - - fr_assert(!inst->driver->common.inst_size || inst->driver_inst->data); - - /* - * Call the driver's instantiate function (if set) - */ - if (inst->driver->instantiate && (inst->driver->instantiate(&inst->config, - inst->driver_inst->data, - driver_cs)) < 0) { - error: - TALLOC_FREE(inst->driver_inst); - return -1; - } - - /* - * @fixme Inst should be passed to all driver callbacks - * instead of being stored here. - */ - inst->config.driver = inst->driver_inst->data; + inst->name = mctx->inst->name; /* Need this for functions in sql.c */ /* * Register the group comparison attribute @@ -1081,7 +1068,7 @@ static int mod_bootstrap(module_inst_ctx_t const *mctx) if (inst->config.group_attribute) { group_attribute = inst->config.group_attribute; } else if (cf_section_name2(conf)) { - snprintf(buffer, sizeof(buffer), "%s-SQL-Group", inst->name); + snprintf(buffer, sizeof(buffer), "%s-SQL-Group", mctx->inst->name); group_attribute = buffer; } else { group_attribute = "SQL-Group"; @@ -1093,7 +1080,8 @@ static int mod_bootstrap(module_inst_ctx_t const *mctx) if (paircmp_register_by_name(group_attribute, attr_user_name, false, sql_groupcmp, inst) < 0) { PERROR("Failed registering group comparison"); - goto error; + error: + return -1; } inst->group_da = fr_dict_attr_search_by_qualified_oid(NULL, dict_freeradius, group_attribute, @@ -1107,7 +1095,7 @@ static int mod_bootstrap(module_inst_ctx_t const *mctx) /* * Register the SQL xlat function */ - xlat = xlat_register_module(inst, mctx, inst->name, sql_xlat, NULL); + xlat = xlat_register_module(inst, mctx, mctx->inst->name, sql_xlat, NULL); /* * The xlat escape function needs access to inst - so @@ -1124,7 +1112,7 @@ static int mod_bootstrap(module_inst_ctx_t const *mctx) /* * Register the SQL map processor function */ - if (inst->driver->sql_fields) map_proc_register(inst, inst->name, mod_map_proc, sql_map_verify, 0); + if (inst->driver->sql_fields) map_proc_register(inst, mctx->inst->name, mod_map_proc, sql_map_verify, 0); return 0; } @@ -1135,14 +1123,6 @@ static int mod_instantiate(module_inst_ctx_t const *mctx) rlm_sql_t *inst = talloc_get_type_abort(mctx->inst->data, rlm_sql_t); CONF_SECTION *conf = mctx->inst->conf; - /* - * Sanity check for crazy people. - */ - if (strncmp(inst->config.sql_driver_name, "rlm_sql_", 8) != 0) { - ERROR("\"%s\" is NOT an SQL driver!", inst->config.sql_driver_name); - return -1; - } - /* * We need authorize_group_check_query or authorize_group_reply_query * if group_membership_query is set. diff --git a/src/modules/rlm_sql/rlm_sql.h b/src/modules/rlm_sql/rlm_sql.h index 427020f8513..a7379e70892 100644 --- a/src/modules/rlm_sql/rlm_sql.h +++ b/src/modules/rlm_sql/rlm_sql.h @@ -86,7 +86,6 @@ typedef struct { } sql_acct_section_t; typedef struct { - char const *sql_driver_name; //!< SQL driver module name e.g. rlm_sql_sqlite. char const *sql_server; //!< Server to connect to. uint32_t sql_port; //!< Port to connect to. char const *sql_login; //!< Login credentials to use. @@ -124,10 +123,6 @@ typedef struct { char const *connect_query; //!< Query executed after establishing //!< new connection. - - void *driver; //!< Where drivers should write a - //!< pointer to their configurations. - /* * @todo The rest of the queries should also be moved into * their own sections. @@ -221,7 +216,7 @@ struct sql_inst { //!< dictionary attribute. exfile_t *ef; - dl_module_inst_t *driver_inst; //!< Driver's instance data. + module_instance_t *driver_submodule; //!< Driver's submodule. rlm_sql_driver_t const *driver; //!< Driver's exported interface. int (*sql_set_user)(rlm_sql_t const *inst, request_t *request, char const *username); diff --git a/src/modules/rlm_sql/sql.c b/src/modules/rlm_sql/sql.c index 8e534d6a6f5..43309a4eb60 100644 --- a/src/modules/rlm_sql/sql.c +++ b/src/modules/rlm_sql/sql.c @@ -337,7 +337,7 @@ sql_rcode_t rlm_sql_fetch_row(rlm_sql_row_t *out, rlm_sql_t const *inst, request */ void rlm_sql_print_error(rlm_sql_t const *inst, request_t *request, rlm_sql_handle_t *handle, bool force_debug) { - char const *driver; + char const *driver = inst->driver_submodule->name; sql_log_entry_t log[20]; size_t num, i; @@ -347,8 +347,6 @@ void rlm_sql_print_error(rlm_sql_t const *inst, request_t *request, rlm_sql_hand return; } - driver = inst->config.sql_driver_name; - for (i = 0; i < num; i++) { if (force_debug) goto debug; diff --git a/src/tests/modules/sql_mysql/module.conf b/src/tests/modules/sql_mysql/module.conf index d94b6ec9257..7392c1ce824 100644 --- a/src/tests/modules/sql_mysql/module.conf +++ b/src/tests/modules/sql_mysql/module.conf @@ -1,5 +1,5 @@ sql { - driver = "rlm_sql_mysql" + driver = "mysql" dialect = "mysql" # Connection info: diff --git a/src/tests/modules/sql_postgresql/module.conf b/src/tests/modules/sql_postgresql/module.conf index 140e27d391c..e03bd7c47c0 100644 --- a/src/tests/modules/sql_postgresql/module.conf +++ b/src/tests/modules/sql_postgresql/module.conf @@ -1,5 +1,5 @@ sql { - driver = "rlm_sql_postgresql" + driver = "postgresql" dialect = "postgresql" # Connection info: diff --git a/src/tests/modules/sql_sqlite/module.conf b/src/tests/modules/sql_sqlite/module.conf index 6caaee69237..1bf24752825 100644 --- a/src/tests/modules/sql_sqlite/module.conf +++ b/src/tests/modules/sql_sqlite/module.conf @@ -1,5 +1,5 @@ sql { - driver = "rlm_sql_sqlite" + driver = "sqlite" dialect = "sqlite" sqlite { # Path to the sqlite database