From: Nick Porter Date: Wed, 3 Nov 2021 19:52:10 +0000 (+0000) Subject: v4: Convert SQL xlat to new API (#4302) X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=8908eee31ed8a9a33065301cfa39f67e4861e875;p=thirdparty%2Ffreeradius-server.git v4: Convert SQL xlat to new API (#4302) * Define sql_xlat_escape() * Define mox_xlat_instantiate() for rlm_sql * Amend SQL xlat to use new API * Register new sql xlat * Remove old sql xlat escape function --- diff --git a/src/modules/rlm_sql/rlm_sql.c b/src/modules/rlm_sql/rlm_sql.c index 0a477a3c44b..3921b429afc 100644 --- a/src/modules/rlm_sql/rlm_sql.c +++ b/src/modules/rlm_sql/rlm_sql.c @@ -140,8 +140,6 @@ fr_dict_attr_autoload_t rlm_sql_dict_attr[] = { { NULL } }; -static size_t sql_escape_for_xlat_func(request_t *request, char *out, size_t outlen, char const *in, void *arg); - /* * Fall-Through checking function from rlm_files.c */ @@ -158,9 +156,51 @@ static sql_fall_through_t fall_through(fr_pair_list_t *vps) */ static size_t sql_escape_func(request_t *, char *out, size_t outlen, char const *in, void *arg); +/** Escape a tainted VB used as an xlat argument + * + */ +static int sql_xlat_escape(request_t *request, fr_value_box_t *vb, void *uctx) +{ + fr_sbuff_t sbuff; + fr_sbuff_uctx_talloc_t sbuff_ctx; + + size_t len; + rlm_sql_handle_t *handle; + rlm_sql_t *inst = talloc_get_type_abort(uctx, rlm_sql_t); + fr_dlist_t entry; + + handle = fr_pool_connection_get(inst->pool, request); + if (!handle) { + fr_value_box_clear_value(vb); + return -1; + } + + /* + * Maximum escaped length is 3 * original - if every character needs escaping + */ + if (!fr_sbuff_init_talloc(vb, &sbuff, &sbuff_ctx, vb->length * 3, vb->length * 3)) { + fr_strerror_printf_push("Failed to allocate buffer for escaped sql argument"); + return -1; + } + + len = inst->sql_escape_func(request, fr_sbuff_buff(&sbuff), vb->length * 3 + 1, vb->vb_strvalue, handle); + + /* + * fr_value_box_strdup_shallow resets the dlist entries - take a copy + */ + entry = vb->entry; + fr_sbuff_trim_talloc(&sbuff, len); + fr_value_box_clear_value(vb); + fr_value_box_strdup_shallow(vb, NULL, fr_sbuff_buff(&sbuff), vb->tainted); + vb->entry = entry; + + fr_pool_connection_release(inst->pool, request, handle); + return 0; +} + /** Execute an arbitrary SQL query * - * For SELECTs, the first value of the first column will be returned. + * For SELECTs, the values of the first column will be returned. * For INSERTS, UPDATEs and DELETEs, the number of rows affected will * be returned instead. * @@ -170,23 +210,30 @@ static size_t sql_escape_func(request_t *, char *out, size_t outlen, char const * * @ingroup xlat_functions */ -static ssize_t sql_xlat(UNUSED TALLOC_CTX *ctx, char **out, UNUSED size_t outlen, - void const *mod_inst, UNUSED void const *xlat_inst, - request_t *request, char const *fmt) +static xlat_action_t sql_xlat(TALLOC_CTX *ctx, fr_dcursor_t *out, request_t *request, + void const *xlat_inst, UNUSED void *xlat_thread_inst, + fr_value_box_list_t *in) { rlm_sql_handle_t *handle = NULL; rlm_sql_row_t row; - rlm_sql_t const *inst = mod_inst; + rlm_sql_t const *inst; + void *instance; sql_rcode_t rcode; - ssize_t ret = 0; + xlat_action_t ret = XLAT_ACTION_DONE; char const *p; + fr_value_box_t *arg = fr_dlist_head(in); + fr_value_box_t *vb = NULL; + bool fetched = false; + + memcpy(&instance, xlat_inst, sizeof(instance)); + inst = talloc_get_type_abort(instance, rlm_sql_t); handle = fr_pool_connection_get(inst->pool, request); /* connection pool should produce error */ - if (!handle) return 0; + if (!handle) return XLAT_ACTION_FAIL; - rlm_sql_query_log(inst, request, NULL, fmt); + rlm_sql_query_log(inst, request, NULL, arg->vb_strvalue); - p = fmt; + p = arg->vb_strvalue; /* * Trim whitespace for the prefix check @@ -202,12 +249,12 @@ static ssize_t sql_xlat(UNUSED TALLOC_CTX *ctx, char **out, UNUSED size_t outlen (strncasecmp(p, "delete", 6) == 0)) { int numaffected; - rcode = rlm_sql_query(inst, request, &handle, fmt); + rcode = rlm_sql_query(inst, request, &handle, arg->vb_strvalue); if (rcode != RLM_SQL_OK) { query_error: RERROR("SQL query failed: %s", fr_table_str_by_value(sql_rcode_description_table, rcode, "")); - ret = -1; + ret = XLAT_ACTION_FAIL; goto finish; } @@ -219,43 +266,51 @@ static ssize_t sql_xlat(UNUSED TALLOC_CTX *ctx, char **out, UNUSED size_t outlen goto finish; } - MEM(*out = talloc_typed_asprintf(request, "%d", numaffected)); - ret = talloc_array_length(*out) - 1; + MEM(vb = fr_value_box_alloc_null(ctx)); + fr_value_box_uint32(vb, NULL, (uint32_t)numaffected, false); + fr_dcursor_append(out, vb); (inst->driver->sql_finish_query)(handle, inst->config); goto finish; } /* else it's a SELECT statement */ - rcode = rlm_sql_select_query(inst, request, &handle, fmt); + rcode = rlm_sql_select_query(inst, request, &handle, arg->vb_strvalue); if (rcode != RLM_SQL_OK) goto query_error; - rcode = rlm_sql_fetch_row(&row, inst, request, &handle); - switch (rcode) { - case RLM_SQL_OK: - if (row[0]) break; + do { + rcode = rlm_sql_fetch_row(&row, inst, request, &handle); + switch (rcode) { + case RLM_SQL_OK: + if (row[0]) break; - RDEBUG2("NULL value in first column of result"); - (inst->driver->sql_finish_select_query)(handle, inst->config); - ret = -1; + RDEBUG2("NULL value in first column of result"); + ret = XLAT_ACTION_FAIL; - goto finish; + goto finish_query; - case RLM_SQL_NO_MORE_ROWS: - RDEBUG2("SQL query returned no results"); - (inst->driver->sql_finish_select_query)(handle, inst->config); - ret = -1; + case RLM_SQL_NO_MORE_ROWS: + if (!fetched) { + RDEBUG2("SQL query returned no results"); + ret = XLAT_ACTION_FAIL; + } - goto finish; + goto finish_query; - default: - (inst->driver->sql_finish_select_query)(handle, inst->config); - goto query_error; - } + default: + (inst->driver->sql_finish_select_query)(handle, inst->config); + goto query_error; + } + + fetched = true; - *out = talloc_bstrndup(request, row[0], strlen(row[0])); - ret = talloc_array_length(*out) - 1; + MEM(vb = fr_value_box_alloc_null(ctx)); + fr_value_box_strdup(vb, vb, NULL, row[0], false); + fr_dcursor_append(out, vb); + } while (1); + +finish_query: (inst->driver->sql_finish_select_query)(handle, inst->config); finish: @@ -609,28 +664,6 @@ static size_t sql_escape_func(UNUSED request_t *request, char *out, size_t outle return len; } -/** Passed as the escape function to map_proc and sql xlat methods - * - * The variant reserves a connection for the escape functions to use, and releases it after - * escaping is complete. - */ -static size_t sql_escape_for_xlat_func(request_t *request, char *out, size_t outlen, char const *in, void *arg) -{ - size_t ret; - rlm_sql_t *inst = talloc_get_type_abort(arg, rlm_sql_t); - rlm_sql_handle_t *handle; - - handle = fr_pool_connection_get(inst->pool, request); - if (!handle) { - out[0] = '\0'; - return 0; - } - ret = inst->sql_escape_func(request, out, outlen, in, handle); - fr_pool_connection_release(inst->pool, request, handle); - - return ret; -} - /* * Set the SQL user name. * @@ -965,6 +998,14 @@ finish: RETURN_MODULE_RCODE(rcode); } +/** Make module instance available to xlats + * + */ +static int mod_xlat_instantiate(void *xlat_inst, UNUSED xlat_exp_t const *exp, void *uctx) +{ + *((void **)xlat_inst) = talloc_get_type_abort(uctx, rlm_sql_t); + return 0; +} static int mod_detach(void *instance) { @@ -990,6 +1031,8 @@ static int mod_bootstrap(void *instance, CONF_SECTION *conf) rlm_sql_t *inst = talloc_get_type_abort(instance, rlm_sql_t); CONF_SECTION *driver_cs; char const *name; + xlat_t *xlat; + xlat_arg_parser_t *sql_xlat_arg; /* * Hack... @@ -1083,7 +1126,21 @@ static int mod_bootstrap(void *instance, CONF_SECTION *conf) /* * Register the SQL xlat function */ - xlat_register_legacy(inst, inst->name, sql_xlat, sql_escape_for_xlat_func, NULL, 0, 0); + xlat = xlat_register(inst, inst->name, sql_xlat, false); + + /* + * The xlat escape function needs access to inst - so + * argument parser details need to be defined here + */ + sql_xlat_arg = talloc_zero(inst, xlat_arg_parser_t); + sql_xlat_arg->type = FR_TYPE_STRING; + sql_xlat_arg->required = true; + sql_xlat_arg->concat = true; + sql_xlat_arg->func = sql_xlat_escape; + sql_xlat_arg->uctx = inst; + xlat_func_mono(xlat, sql_xlat_arg); + + xlat_async_instantiate_set(xlat, mod_xlat_instantiate, rlm_sql_t *, NULL, inst); /* * Register the SQL map processor function