From: Alan T. DeKok Date: Mon, 27 Jan 2025 17:46:59 +0000 (-0500) Subject: disable expansion in SQL modules for RHS values of check queries X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=feedbb7988faeb7444ac0bcf4455ed013c0b0a9c;p=thirdparty%2Ffreeradius-server.git disable expansion in SQL modules for RHS values of check queries it turns out to be not particularly useful, and has some corner cases we're going to avoid for a bit. As a result, disabled the "attrref" test. --- diff --git a/src/lib/server/map.c b/src/lib/server/map.c index d592280d06a..8066ebad5ba 100644 --- a/src/lib/server/map.c +++ b/src/lib/server/map.c @@ -2487,13 +2487,14 @@ void map_debug_log(request_t *request, map_t const *map, fr_pair_t const *vp) * @param[in] rhs of map * @param[in] lhs_rules for parsing the LHS * @param[in] rhs_rules for parsing the RHS + * @param[in] bare_word_only RHS is bare words, and nothing else. * @return * - 0 on success. * - -1 on failure. */ int map_afrom_fields(TALLOC_CTX *ctx, map_t **out, map_t **parent_p, request_t *request, char const *lhs, char const *op_str, char const *rhs, - tmpl_rules_t const *lhs_rules, tmpl_rules_t const *rhs_rules) + tmpl_rules_t const *lhs_rules, tmpl_rules_t const *rhs_rules, bool bare_word_only) { ssize_t slen; fr_token_t quote, op; @@ -2610,14 +2611,48 @@ int map_afrom_fields(TALLOC_CTX *ctx, map_t **out, map_t **parent_p, request_t * } /* - * If we have a string, where the *entire* string is - * quoted, then tokenize it that way, - * - * @todo - if the string starts with '(' OR '%' OR - * doesn't begin with a quote, BUT contains spaces, then - * parse it as an xlat expression! + * Try to figure out what we should do with the RHS. */ - if (rhs[0] == '"') { + if ((map->op == T_OP_CMP_TRUE) || (map->op == T_OP_CMP_FALSE)) { + /* + * These operators require a hard-coded string on the RHS. + */ + if (strcmp(rhs, "ANY") != 0) { + fr_strerror_printf("Invalid value %s for operator %s", rhs, fr_tokens[map->op]); + goto error; + } + + if (tmpl_afrom_value_box(map, &map->rhs, fr_box_strvalue("ANY"), false) < 0) goto error; + + } else if (bare_word_only) { + fr_value_box_t *vb; + + /* + * No value, or no enum, parse it as a bare-word string. + */ + if (!rhs[0] || !my_rules.enumv) goto do_bare_word; + + MEM(vb = fr_value_box_alloc(map, my_rules.enumv->type, my_rules.enumv)); + + /* + * It MUST be the given data type. + */ + slen = fr_value_box_from_str(map, vb, my_rules.enumv->type, my_rules.enumv, + rhs, strlen(rhs), NULL, false); + if (slen <= 0) goto error; + + if (tmpl_afrom_value_box(map, &map->rhs, vb, true) < 0) { + goto error; + } + + } else if (rhs[0] == '"') { + /* + * We've been asked to expand the RHS. Passwords like + * + * "%{Calling-Station-ID}" + * + * might not do what you want. + */ quote = T_DOUBLE_QUOTED_STRING; goto parse_quoted; @@ -2654,19 +2689,10 @@ int map_afrom_fields(TALLOC_CTX *ctx, map_t **out, map_t **parent_p, request_t * /* * Ignore any extra data after the string. + * + * @todo - this should likely be a parse error: we didn't parse the entire string! */ - } else if ((map->op == T_OP_CMP_TRUE) || (map->op == T_OP_CMP_FALSE)) { - /* - * These operators require a hard-coded string on the RHS. - */ - if (strcmp(rhs, "ANY") != 0) { - fr_strerror_printf("Invalid value %s for operator %s", rhs, fr_tokens[map->op]); - goto error; - } - - if (tmpl_afrom_value_box(map, &map->rhs, fr_box_strvalue("ANY"), false) < 0) goto error; - } else if (rhs[0] == '&') { /* * No enums here. @@ -2684,6 +2710,7 @@ int map_afrom_fields(TALLOC_CTX *ctx, map_t **out, map_t **parent_p, request_t * } } else if (!rhs[0] || !my_rules.enumv || (my_rules.enumv->type == FR_TYPE_STRING)) { + do_bare_word: quote = T_BARE_WORD; if (tmpl_attr_tail_da_is_structural(map->lhs) && !*rhs) goto done; diff --git a/src/lib/server/map.h b/src/lib/server/map.h index cb19772b01d..e18f9c633cc 100644 --- a/src/lib/server/map.h +++ b/src/lib/server/map.h @@ -151,7 +151,8 @@ ssize_t map_afrom_substr(TALLOC_CTX *ctx, map_t **out, map_t **parent_p, fr_sbu int map_afrom_fields(TALLOC_CTX *ctx, map_t **out, map_t **parent_p, request_t *request, char const *lhs, char const *op, char const *rhs, - tmpl_rules_t const *lhs_rules, tmpl_rules_t const *rhs_rules) CC_HINT(nonnull); + tmpl_rules_t const *lhs_rules, tmpl_rules_t const *rhs_rules, + bool bare_word_only) CC_HINT(nonnull); int map_to_vp(TALLOC_CTX *ctx, fr_pair_list_t *out, request_t *request, map_t const *map, void *uctx) CC_HINT(nonnull (2,3,4)); diff --git a/src/modules/rlm_sql/rlm_sql.c b/src/modules/rlm_sql/rlm_sql.c index c78cc8b3d10..b041e92acf8 100644 --- a/src/modules/rlm_sql/rlm_sql.c +++ b/src/modules/rlm_sql/rlm_sql.c @@ -80,6 +80,8 @@ static const conf_parser_t module_config[] = { */ { FR_CONF_OFFSET_SUBSECTION("pool", 0, rlm_sql_config_t, trunk_conf, trunk_config) }, + { FR_CONF_OFFSET_FLAGS("expand_rhs", CONF_FLAG_HIDDEN, rlm_sql_config_t, expand_rhs) }, + CONF_PARSER_TERMINATOR }; @@ -1426,6 +1428,7 @@ static unlang_action_t mod_autz_group_resume(rlm_rcode_t *p_result, UNUSED int * .out = &autz_ctx->reply_tmp, .list = request_attr_reply, .query = query, + .expand_rhs = true, }; if (unlang_function_repeat_set(request, mod_autz_group_resume) < 0) RETURN_MODULE_FAIL; @@ -1582,6 +1585,7 @@ static unlang_action_t mod_authorize_resume(rlm_rcode_t *p_result, int *priority .out = &autz_ctx->reply_tmp, .list = request_attr_reply, .query = query, + .expand_rhs = true, }; if (unlang_function_repeat_set(request, mod_authorize_resume) < 0) RETURN_MODULE_FAIL; diff --git a/src/modules/rlm_sql/rlm_sql.h b/src/modules/rlm_sql/rlm_sql.h index ef2cf6c2891..c961e8f0a41 100644 --- a/src/modules/rlm_sql/rlm_sql.h +++ b/src/modules/rlm_sql/rlm_sql.h @@ -90,6 +90,8 @@ typedef struct { //!< in the previous reply list to process //!< profiles. + bool expand_rhs; //!< expand the RHS for check / reply tables + char const *allowed_chars; //!< Chars which done need escaping.. fr_time_delta_t query_timeout; //!< How long to allow queries to run for. @@ -151,6 +153,7 @@ typedef struct { fr_dict_attr_t const *list; //!< Default list for pair evaluation. map_list_t *out; //!< List to append entries to. int rows; //!< How many rows the query returned. + bool expand_rhs; //!< for reply items } fr_sql_map_ctx_t; extern fr_table_num_sorted_t const sql_rcode_description_table[]; diff --git a/src/modules/rlm_sql/sql.c b/src/modules/rlm_sql/sql.c index 11ccd1490ed..cbb61ba856e 100644 --- a/src/modules/rlm_sql/sql.c +++ b/src/modules/rlm_sql/sql.c @@ -323,7 +323,9 @@ static unlang_action_t sql_get_map_list_resume(rlm_rcode_t *p_result, UNUSED int RPERROR("SQL query returned NULL values"); RETURN_MODULE_FAIL; } - if (map_afrom_fields(map_ctx->ctx, &map, &parent, request, row[2], row[4], row[3], &lhs_rules, &rhs_rules) < 0) { + if (map_afrom_fields(map_ctx->ctx, &map, &parent, request, row[2], row[4], row[3], + &lhs_rules, &rhs_rules, + !(inst->config.expand_rhs || map_ctx->expand_rhs)) < 0) { RPEDEBUG("Data read from SQL cannot be parsed."); REDEBUG(" %s", row[2]); REDEBUG(" %s", row[4]); diff --git a/src/tests/modules/sql_mysql/attrref.attrs b/src/tests/modules/sql_mysql/attrref.attrs deleted file mode 120000 index a8a5c1bb227..00000000000 --- a/src/tests/modules/sql_mysql/attrref.attrs +++ /dev/null @@ -1 +0,0 @@ -../sql/attrref.attrs \ No newline at end of file diff --git a/src/tests/modules/sql_mysql/attrref.unlang b/src/tests/modules/sql_mysql/attrref.unlang deleted file mode 120000 index f001171bccf..00000000000 --- a/src/tests/modules/sql_mysql/attrref.unlang +++ /dev/null @@ -1 +0,0 @@ -../sql/attrref.unlang \ No newline at end of file diff --git a/src/tests/modules/sql_postgresql/attrref.attrs b/src/tests/modules/sql_postgresql/attrref.attrs deleted file mode 120000 index a8a5c1bb227..00000000000 --- a/src/tests/modules/sql_postgresql/attrref.attrs +++ /dev/null @@ -1 +0,0 @@ -../sql/attrref.attrs \ No newline at end of file diff --git a/src/tests/modules/sql_postgresql/attrref.unlang b/src/tests/modules/sql_postgresql/attrref.unlang deleted file mode 120000 index f001171bccf..00000000000 --- a/src/tests/modules/sql_postgresql/attrref.unlang +++ /dev/null @@ -1 +0,0 @@ -../sql/attrref.unlang \ No newline at end of file diff --git a/src/tests/modules/sql_sqlite/attrref.attrs b/src/tests/modules/sql_sqlite/attrref.attrs deleted file mode 120000 index a8a5c1bb227..00000000000 --- a/src/tests/modules/sql_sqlite/attrref.attrs +++ /dev/null @@ -1 +0,0 @@ -../sql/attrref.attrs \ No newline at end of file diff --git a/src/tests/modules/sql_sqlite/attrref.unlang b/src/tests/modules/sql_sqlite/attrref.unlang deleted file mode 120000 index f001171bccf..00000000000 --- a/src/tests/modules/sql_sqlite/attrref.unlang +++ /dev/null @@ -1 +0,0 @@ -../sql/attrref.unlang \ No newline at end of file