Skip to content

Commit feedbb7

Browse files
committed
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.
1 parent a28fdbe commit feedbb7

File tree

11 files changed

+58
-27
lines changed

11 files changed

+58
-27
lines changed

src/lib/server/map.c

Lines changed: 46 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -2487,13 +2487,14 @@ void map_debug_log(request_t *request, map_t const *map, fr_pair_t const *vp)
24872487
* @param[in] rhs of map
24882488
* @param[in] lhs_rules for parsing the LHS
24892489
* @param[in] rhs_rules for parsing the RHS
2490+
* @param[in] bare_word_only RHS is bare words, and nothing else.
24902491
* @return
24912492
* - 0 on success.
24922493
* - -1 on failure.
24932494
*/
24942495
int map_afrom_fields(TALLOC_CTX *ctx, map_t **out, map_t **parent_p, request_t *request,
24952496
char const *lhs, char const *op_str, char const *rhs,
2496-
tmpl_rules_t const *lhs_rules, tmpl_rules_t const *rhs_rules)
2497+
tmpl_rules_t const *lhs_rules, tmpl_rules_t const *rhs_rules, bool bare_word_only)
24972498
{
24982499
ssize_t slen;
24992500
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 *
26102611
}
26112612

26122613
/*
2613-
* If we have a string, where the *entire* string is
2614-
* quoted, then tokenize it that way,
2615-
*
2616-
* @todo - if the string starts with '(' OR '%' OR
2617-
* doesn't begin with a quote, BUT contains spaces, then
2618-
* parse it as an xlat expression!
2614+
* Try to figure out what we should do with the RHS.
26192615
*/
2620-
if (rhs[0] == '"') {
2616+
if ((map->op == T_OP_CMP_TRUE) || (map->op == T_OP_CMP_FALSE)) {
2617+
/*
2618+
* These operators require a hard-coded string on the RHS.
2619+
*/
2620+
if (strcmp(rhs, "ANY") != 0) {
2621+
fr_strerror_printf("Invalid value %s for operator %s", rhs, fr_tokens[map->op]);
2622+
goto error;
2623+
}
2624+
2625+
if (tmpl_afrom_value_box(map, &map->rhs, fr_box_strvalue("ANY"), false) < 0) goto error;
2626+
2627+
} else if (bare_word_only) {
2628+
fr_value_box_t *vb;
2629+
2630+
/*
2631+
* No value, or no enum, parse it as a bare-word string.
2632+
*/
2633+
if (!rhs[0] || !my_rules.enumv) goto do_bare_word;
2634+
2635+
MEM(vb = fr_value_box_alloc(map, my_rules.enumv->type, my_rules.enumv));
2636+
2637+
/*
2638+
* It MUST be the given data type.
2639+
*/
2640+
slen = fr_value_box_from_str(map, vb, my_rules.enumv->type, my_rules.enumv,
2641+
rhs, strlen(rhs), NULL, false);
2642+
if (slen <= 0) goto error;
2643+
2644+
if (tmpl_afrom_value_box(map, &map->rhs, vb, true) < 0) {
2645+
goto error;
2646+
}
2647+
2648+
} else if (rhs[0] == '"') {
2649+
/*
2650+
* We've been asked to expand the RHS. Passwords like
2651+
*
2652+
* "%{Calling-Station-ID}"
2653+
*
2654+
* might not do what you want.
2655+
*/
26212656
quote = T_DOUBLE_QUOTED_STRING;
26222657
goto parse_quoted;
26232658

@@ -2654,19 +2689,10 @@ int map_afrom_fields(TALLOC_CTX *ctx, map_t **out, map_t **parent_p, request_t *
26542689

26552690
/*
26562691
* Ignore any extra data after the string.
2692+
*
2693+
* @todo - this should likely be a parse error: we didn't parse the entire string!
26572694
*/
26582695

2659-
} else if ((map->op == T_OP_CMP_TRUE) || (map->op == T_OP_CMP_FALSE)) {
2660-
/*
2661-
* These operators require a hard-coded string on the RHS.
2662-
*/
2663-
if (strcmp(rhs, "ANY") != 0) {
2664-
fr_strerror_printf("Invalid value %s for operator %s", rhs, fr_tokens[map->op]);
2665-
goto error;
2666-
}
2667-
2668-
if (tmpl_afrom_value_box(map, &map->rhs, fr_box_strvalue("ANY"), false) < 0) goto error;
2669-
26702696
} else if (rhs[0] == '&') {
26712697
/*
26722698
* No enums here.
@@ -2684,6 +2710,7 @@ int map_afrom_fields(TALLOC_CTX *ctx, map_t **out, map_t **parent_p, request_t *
26842710
}
26852711

26862712
} else if (!rhs[0] || !my_rules.enumv || (my_rules.enumv->type == FR_TYPE_STRING)) {
2713+
do_bare_word:
26872714
quote = T_BARE_WORD;
26882715

26892716
if (tmpl_attr_tail_da_is_structural(map->lhs) && !*rhs) goto done;

src/lib/server/map.h

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -151,7 +151,8 @@ ssize_t map_afrom_substr(TALLOC_CTX *ctx, map_t **out, map_t **parent_p, fr_sbu
151151

152152
int map_afrom_fields(TALLOC_CTX *ctx, map_t **out, map_t **parent_p, request_t *request,
153153
char const *lhs, char const *op, char const *rhs,
154-
tmpl_rules_t const *lhs_rules, tmpl_rules_t const *rhs_rules) CC_HINT(nonnull);
154+
tmpl_rules_t const *lhs_rules, tmpl_rules_t const *rhs_rules,
155+
bool bare_word_only) CC_HINT(nonnull);
155156

156157
int map_to_vp(TALLOC_CTX *ctx, fr_pair_list_t *out, request_t *request,
157158
map_t const *map, void *uctx) CC_HINT(nonnull (2,3,4));

src/modules/rlm_sql/rlm_sql.c

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -80,6 +80,8 @@ static const conf_parser_t module_config[] = {
8080
*/
8181
{ FR_CONF_OFFSET_SUBSECTION("pool", 0, rlm_sql_config_t, trunk_conf, trunk_config) },
8282

83+
{ FR_CONF_OFFSET_FLAGS("expand_rhs", CONF_FLAG_HIDDEN, rlm_sql_config_t, expand_rhs) },
84+
8385
CONF_PARSER_TERMINATOR
8486
};
8587

@@ -1426,6 +1428,7 @@ static unlang_action_t mod_autz_group_resume(rlm_rcode_t *p_result, UNUSED int *
14261428
.out = &autz_ctx->reply_tmp,
14271429
.list = request_attr_reply,
14281430
.query = query,
1431+
.expand_rhs = true,
14291432
};
14301433

14311434
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
15821585
.out = &autz_ctx->reply_tmp,
15831586
.list = request_attr_reply,
15841587
.query = query,
1588+
.expand_rhs = true,
15851589
};
15861590

15871591
if (unlang_function_repeat_set(request, mod_authorize_resume) < 0) RETURN_MODULE_FAIL;

src/modules/rlm_sql/rlm_sql.h

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -90,6 +90,8 @@ typedef struct {
9090
//!< in the previous reply list to process
9191
//!< profiles.
9292

93+
bool expand_rhs; //!< expand the RHS for check / reply tables
94+
9395
char const *allowed_chars; //!< Chars which done need escaping..
9496
fr_time_delta_t query_timeout; //!< How long to allow queries to run for.
9597

@@ -151,6 +153,7 @@ typedef struct {
151153
fr_dict_attr_t const *list; //!< Default list for pair evaluation.
152154
map_list_t *out; //!< List to append entries to.
153155
int rows; //!< How many rows the query returned.
156+
bool expand_rhs; //!< for reply items
154157
} fr_sql_map_ctx_t;
155158

156159
extern fr_table_num_sorted_t const sql_rcode_description_table[];

src/modules/rlm_sql/sql.c

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -323,7 +323,9 @@ static unlang_action_t sql_get_map_list_resume(rlm_rcode_t *p_result, UNUSED int
323323
RPERROR("SQL query returned NULL values");
324324
RETURN_MODULE_FAIL;
325325
}
326-
if (map_afrom_fields(map_ctx->ctx, &map, &parent, request, row[2], row[4], row[3], &lhs_rules, &rhs_rules) < 0) {
326+
if (map_afrom_fields(map_ctx->ctx, &map, &parent, request, row[2], row[4], row[3],
327+
&lhs_rules, &rhs_rules,
328+
!(inst->config.expand_rhs || map_ctx->expand_rhs)) < 0) {
327329
RPEDEBUG("Data read from SQL cannot be parsed.");
328330
REDEBUG(" %s", row[2]);
329331
REDEBUG(" %s", row[4]);

src/tests/modules/sql_mysql/attrref.attrs

Lines changed: 0 additions & 1 deletion
This file was deleted.

src/tests/modules/sql_mysql/attrref.unlang

Lines changed: 0 additions & 1 deletion
This file was deleted.

src/tests/modules/sql_postgresql/attrref.attrs

Lines changed: 0 additions & 1 deletion
This file was deleted.

src/tests/modules/sql_postgresql/attrref.unlang

Lines changed: 0 additions & 1 deletion
This file was deleted.

src/tests/modules/sql_sqlite/attrref.attrs

Lines changed: 0 additions & 1 deletion
This file was deleted.

0 commit comments

Comments
 (0)