From: Alan T. DeKok Date: Sun, 27 Aug 2023 12:34:11 +0000 (-0400) Subject: move paircmp() to rlm_sql X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=82b273daf2bb7a06a333d63b803571de257b9f5f;p=thirdparty%2Ffreeradius-server.git move paircmp() to rlm_sql and drastically simplify it. The behavior is similar enough for most cases, except: * regular expression operators are no longer supported. It's not hard to re-add them. As they're not needed right now, they can be temporarily removed * virtual attributes like Packet-Src-IP-Address are not supported Again, this isn't terribly difficult to re-add. But once the Packet-* attributes are moved to Net.* attributes, then any virtual attribute comparisons become much less useful. The remainder are Virtual-Server, Request-Processing-Stage, and Module-Return-Code. Those could arguably all be moved to realized attributes in the control list. And be made immutable, so that "unlang" can't change them. --- diff --git a/raddb/mods-available/sql b/raddb/mods-available/sql index 8236b7f83b7..45361402878 100644 --- a/raddb/mods-available/sql +++ b/raddb/mods-available/sql @@ -126,6 +126,13 @@ sql { # postauth_table = "radpostauth" + # + # NOTE: Temporarily (2023-08-27), the SQL check items only support "real" + # attributes, and do not support regular expressions. This limitation will + # be removed when the SQL module is rewritten to support maps (for assignments) + # and xlat expressions (for conditions) + # + # # authcheck_table:: # groupcheck_table:: diff --git a/src/lib/server/paircmp.c b/src/lib/server/paircmp.c index 7edc48471c4..d415583d6ae 100644 --- a/src/lib/server/paircmp.c +++ b/src/lib/server/paircmp.c @@ -56,7 +56,6 @@ fr_dict_autoload_t paircmp_dict[] = { { NULL } }; -static fr_dict_attr_t const *attr_auth_type; static fr_dict_attr_t const *attr_crypt_password; static fr_dict_attr_t const *attr_packet_dst_ip_address; static fr_dict_attr_t const *attr_packet_dst_ipv6_address; @@ -261,50 +260,6 @@ int paircmp_pairs(UNUSED request_t *request, fr_pair_t const *check, fr_pair_t * return fr_value_box_cmp_op(check->op, &vp->data, &check->data); } -/** Compare check_item and vp. May call the attribute comparison function. - * - * Unlike paircmp_pairs() this function will call any attribute-specific - * comparison functions registered. vp to be matched is request_item or - * found in check_list or looked up from external sources depending on the - * comparison function called. - * - * @param[in] request Current request. - * @param[in] request_item item to compare. - * @param[in] check_item item to compare. - * @return - * - 0 if check_item and vp are equal. - * - -1 if vp value is less than check_item value. - * - 1 is vp value is more than check_item value. - */ -static int paircmp_func(request_t *request, - fr_pair_t *request_item, - fr_pair_t *check_item) -{ - paircmp_t *c; - - PAIR_VERIFY(check_item); - - /* - * Check for =* and !* and return appropriately - */ - if (check_item->op == T_OP_CMP_TRUE) return 0; - if (check_item->op == T_OP_CMP_FALSE) return 1; - - /* - * See if there is a special compare function. - */ - for (c = cmp; c; c = c->next) { - if (c->da == check_item->da) { - return generic_cmp(request, check_item); - } - } - - if (!request) return -1; /* doesn't exist, don't compare it */ - - return paircmp_pairs(request, check_item, request_item); -} - - /** Compare check_item and request * * Unlike paircmp_pairs() this function will call any @@ -346,151 +301,6 @@ int paircmp_virtual(request_t *request, fr_dict_attr_t const *da, fr_token_t op, } -/** Compare two pair lists except for the password information. - * - * For every element in "check_list" at least one matching copy must be present - * in "request_list". - * - * @param[in] request Current request. - * @param[in] request_list request valuepairs. - * @param[in] check_list Check/control valuepairs. - * @return 0 on match. - */ -int paircmp(request_t *request, - fr_pair_list_t *request_list, - fr_pair_list_t *check_list) -{ - fr_pair_t *auth_item; - - int result = 0; - int compare; - - fr_pair_list_foreach(check_list, check_item) { - /* - * If the user is setting a configuration value, - * then don't bother comparing it to any attributes - * sent to us by the user. It ALWAYS matches. - */ - if ((check_item->op == T_OP_SET) || - (check_item->op == T_OP_ADD_EQ)) { - continue; - } - - /* - * Attributes we skip during comparison. - * These are "server" check items. - */ - if ((check_item->da == attr_crypt_password) || - (check_item->da == attr_auth_type) || - (check_item->da == attr_strip_user_name)) { - continue; - } - - /* - * IF the password attribute exists, THEN - * we can do comparisons against it. If not, - * then the request did NOT contain a - * User-Password attribute, so we CANNOT do - * comparisons against it. - * - * This hack makes CHAP-Password work.. - */ - if (check_item->da == attr_user_password) { - if (check_item->op == T_OP_CMP_EQ) { - WARN("Found User-Password == \"...\""); - WARN("Are you sure you don't mean Password.Cleartext?"); - WARN("See \"man rlm_pap\" for more information"); - } - if (fr_pair_find_by_da(request_list, NULL, attr_user_password) == NULL) continue; - } - - auth_item = fr_pair_list_head(request_list); - - try_again: - /* - * Not found, it's not a match. - */ - if (!auth_item) { - /* - * Didn't find it. If we were *trying* - * to not find it, then we succeeded. - */ - if (check_item->op == T_OP_CMP_FALSE) { - continue; - } else { - return -1; - } - } - - /* - * Else we found it, but we were trying to not - * find it, so we failed. - */ - if (check_item->op == T_OP_CMP_FALSE) return -1; - - /* - * OK it is present now compare them. - */ - compare = paircmp_func(request, auth_item, check_item); - switch (check_item->op) { - case T_OP_EQ: - default: -#ifdef STATIC_ANALYZER - if (!check_item->da) return -1; -#endif - - RWDEBUG("Invalid operator '%s' for item %s: reverting to '=='", - fr_table_str_by_value(fr_tokens_table, check_item->op, ""), check_item->da->name); - FALL_THROUGH; - case T_OP_CMP_TRUE: - case T_OP_CMP_FALSE: - case T_OP_CMP_EQ: - if (compare != 0) result = -1; - break; - - case T_OP_NE: - if (compare == 0) result = -1; - break; - - case T_OP_LT: - if (compare >= 0) result = -1; - break; - - case T_OP_GT: - if (compare <= 0) result = -1; - break; - - case T_OP_LE: - if (compare > 0) result = -1; - break; - - case T_OP_GE: - if (compare < 0) result = -1; - break; - -#ifdef HAVE_REGEX - case T_OP_REG_EQ: - case T_OP_REG_NE: - if (compare != 0) result = -1; - break; -#endif - } /* switch over the operator of the check item */ - - /* - * This attribute didn't match, but maybe there's - * another of the same attribute, which DOES match. - */ - if (result != 0) { - auth_item = fr_pair_find_by_da(request_list, auth_item, check_item->da); - result = 0; - goto try_again; - } - - } /* for every entry in the check item list */ - - return result; -} - /** Find a comparison function for two attributes. * * @param[in] da to find comparison function for. diff --git a/src/lib/server/paircmp.h b/src/lib/server/paircmp.h index 23772552161..4319b9d17c5 100644 --- a/src/lib/server/paircmp.h +++ b/src/lib/server/paircmp.h @@ -36,8 +36,6 @@ extern "C" { int paircmp_pairs(request_t *request, fr_pair_t const *check, fr_pair_t *vp); -int paircmp(request_t *request, fr_pair_list_t *request_list, fr_pair_list_t *check_list); - int paircmp_virtual(request_t *request, fr_dict_attr_t const *da, fr_token_t op, fr_value_box_t const *value); int paircmp_find(fr_dict_attr_t const *da); diff --git a/src/modules/rlm_sql/rlm_sql.c b/src/modules/rlm_sql/rlm_sql.c index b874428307b..56461c2bd22 100644 --- a/src/modules/rlm_sql/rlm_sql.c +++ b/src/modules/rlm_sql/rlm_sql.c @@ -844,6 +844,61 @@ static xlat_action_t sql_group_xlat(TALLOC_CTX *ctx, fr_dcursor_t *out, return XLAT_ACTION_DONE; } +/* + * Simplified version of the old paircmp() + * + * It does not support virtual attributes like Packet-Src-IP-Address, etc. That limitation will be + * removed when this code is rewritten to support maps (for assignment) and xlat expressions (for + * conditions). + */ +static bool paircmp(request_t *request, fr_pair_list_t *check_list) +{ + fr_pair_t *vp = NULL; + + fr_pair_list_foreach(check_list, check) { + /* + * If the user is setting a configuration value, then don't bother comparing it to any + * attributes sent to us by the user. It ALWAYS matches. + */ + if (!fr_comparison_op(check_item->op)) continue; + + next_vp: + vp = fr_pair_find_by_da(&request->request_pairs, vp, check->da); + if (!vp) { + /* + * Matches if the VP doesn't exist. + */ + if (check->op == T_OP_CMP_FALSE) continue; + + /* + * Otherwise doesn't match. + */ + return false; + } + + /* + * We didn't want it, but it exists. + */ + if (check->op == T_OP_CMP_FALSE) return false; + + /* + * We want it, and it exists. We don't care what value it has. + */ + if (check_item->op == T_OP_CMP_TRUE) continue; + + /* + * This attribute doesn't match. Maybe there's another one which does match? + */ + if (fr_value_box_cmp_op(check->op, &vp->data, &check->data) != 0) goto next_vp; + } + + /* + * Everything matched, we're OK. + */ + return true; +} + + static unlang_action_t rlm_sql_process_groups(rlm_rcode_t *p_result, @@ -931,8 +986,7 @@ static unlang_action_t rlm_sql_process_groups(rlm_rcode_t *p_result, * If we got check rows we need to process them before we decide to * process the reply rows */ - if ((rows > 0) && - (paircmp(request, &request->request_pairs, &check_tmp) != 0)) { + if ((rows > 0) && !paircmp(request,&check_tmp)) { fr_pair_list_free(&check_tmp); entry = entry->next; @@ -1304,7 +1358,7 @@ static unlang_action_t CC_HINT(nonnull) mod_authorize(rlm_rcode_t *p_result, mod */ RDEBUG2("User found in radcheck table"); user_found = true; - if (paircmp(request, &request->request_pairs, &check_tmp) != 0) { + if (!paircmp(request, &check_tmp)) { fr_pair_list_free(&check_tmp); goto skip_reply; }