From: Alan T. DeKok Date: Sat, 22 Feb 2025 13:27:42 +0000 (-0500) Subject: move validation checks from run-time to load-time X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=84cc8bb0286f789d067dbfad6f28a40693f6d8a0;p=thirdparty%2Ffreeradius-server.git move validation checks from run-time to load-time and tighten them up a little bit --- diff --git a/raddb/mods-available/radius b/raddb/mods-available/radius index 4923950e7b7..7d917122e5c 100644 --- a/raddb/mods-available/radius +++ b/raddb/mods-available/radius @@ -311,6 +311,9 @@ radius { # If the home server does not respond to proxied packets, the # module starts pinging the home server with these packets. # + # Disable status checks by deleting this section, or by + # commenting it out. + # status_check { # # type:: You can specify any type of request packet here, @@ -328,37 +331,32 @@ radius { type = Status-Server # - # `Status-Server` packet contents are fixed and cannot - # be edited. - # - # For other packet types, you can set the contents - # here. The section MUST be set over - # "&request. = value", and anything else - # will cause a parse error. + # The packet contents can be set here. # # We RECOMMEND that you use packet contents which # lets the other end easily tell that they are not # "real" packets from a NAS. # - # The example here is for Access-Request. The - # contents will vary by other packet types. - # - # The module will automatically update the contents - # of the Event-Timestamp attribute to be the time - # when the packet is sent. The module will also - # automatically add a Proxy-State attribute. - # - # WARNING: Do NOT do SQL queries, LDAP queries, dynamic - # expansions, etc. in this section. The contents are - # created when a connection is opened, and are not - # changeable after that. - # -# update request { -# &User-Name := "test-user" -# &User-Password := "this-is-not-a-real-password" -# &NAS-Identifier := "Status check. Are you alive?" -# &Event-Timestamp = 0 -# } + # The example here is for Status-Server. The + # contents will vary by other packet types. The + # Message-Authenticator attribute will be added + # automatically, and does not need to be specified + # here. + # + # If the Event-Timestamp attribute is added, it will + # be updated each time the packet is sent. + # + # WARNING: Do NOT do SQL queries, LDAP queries, + # dynamic expansions, etc. in this section. The + # contents of the packet are created when a + # connection is opened, and are not changeable after + # that. + # + update { + User-Name := "test-user" + NAS-Identifier := "Status check. Are you alive?" + Event-Timestamp = 0 + } } # diff --git a/src/modules/rlm_radius/bio.c b/src/modules/rlm_radius/bio.c index f22e9df96cd..3b277c379fb 100644 --- a/src/modules/rlm_radius/bio.c +++ b/src/modules/rlm_radius/bio.c @@ -285,28 +285,6 @@ static void CC_HINT(nonnull) status_check_alloc(bio_handle_t *h) * creating them. */ while ((map = map_list_next(&inst->status_check_map, map))) { - /* - * Skip things which aren't attributes. - */ - if (!tmpl_is_attr(map->lhs)) continue; - - /* - * Ignore internal attributes. - */ - if (tmpl_attr_tail_da(map->lhs)->flags.internal) continue; - - /* - * Ignore signalling attributes. They shouldn't exist. - */ - if ((tmpl_attr_tail_da(map->lhs) == attr_proxy_state) || - (tmpl_attr_tail_da(map->lhs) == attr_message_authenticator)) continue; - - /* - * Allow passwords only in Access-Request packets. - */ - if ((inst->status_check != FR_RADIUS_CODE_ACCESS_REQUEST) && - (tmpl_attr_tail_da(map->lhs) == attr_user_password)) continue; - (void) map_to_request(request, map, map_to_vp, NULL); } diff --git a/src/modules/rlm_radius/rlm_radius.c b/src/modules/rlm_radius/rlm_radius.c index f7eadc37bf5..8b3ac054c02 100644 --- a/src/modules/rlm_radius/rlm_radius.c +++ b/src/modules/rlm_radius/rlm_radius.c @@ -406,6 +406,58 @@ static int status_check_type_parse(UNUSED TALLOC_CTX *ctx, void *out, UNUSED voi return 0; } +static int status_check_verify(map_t *map, void *ctx) +{ + rlm_radius_t const *inst = talloc_get_type_abort_const(ctx, rlm_radius_t); + fr_dict_attr_t const *da; + + fr_assert(tmpl_is_attr(map->lhs)); + + if (unlang_fixup_update(map, NULL) < 0) return -1; + + if (!map->rhs) return 0; + + if (tmpl_is_xlat(map->rhs)) { + if (xlat_impure_func(tmpl_xlat(map->rhs))) { + invalid_xlat: + cf_log_err(map->ci, "Cannot assign dynamic values here"); + return -1; + } + } else if (!tmpl_is_data(map->rhs)) { + goto invalid_xlat; + } + + da = tmpl_attr_tail_da(map->lhs); + + /* + * Ignore internal attributes. + */ + if (da->flags.internal) { + cf_log_err(map->ci, "Cannot use internal attributes"); + return -1; + } + + /* + * Ignore signalling attributes. They shouldn't exist. + */ + if ((da == attr_proxy_state) || + (da == attr_message_authenticator)) { + cannot_use: + cf_log_err(map->ci, "Cannot use %s here.", da->name); + return -1; + } + + /* + * Allow passwords only in Access-Request packets. + */ + if ((inst->status_check != FR_RADIUS_CODE_ACCESS_REQUEST) && + ((da == attr_user_password) || (da == attr_chap_password) || (da == attr_eap_message))) { + goto cannot_use; + } + + return 0; +} + /** Allow the admin to set packet contents for Status-Server ping checks * * @param[in] ctx to allocate data in (instance of proto_radius). @@ -417,7 +469,7 @@ static int status_check_type_parse(UNUSED TALLOC_CTX *ctx, void *out, UNUSED voi * - 0 on success. * - -1 on failure. */ -static int status_check_update_parse(TALLOC_CTX *ctx, void *out, UNUSED void *parent, +static int status_check_update_parse(TALLOC_CTX *ctx, void *out, void *parent, CONF_ITEM *ci, UNUSED conf_parser_t const *rule) { int rcode; @@ -442,13 +494,17 @@ static int status_check_update_parse(TALLOC_CTX *ctx, void *out, UNUSED void *pa tmpl_rules_t parse_rules = { .attr = { .dict_def = dict_radius, + .namespace = fr_dict_root(dict_radius), + .list_def = request_attr_request, + .list_presence = TMPL_ATTR_LIST_FORBID, + .prefix = TMPL_ATTR_REF_PREFIX_AUTO, } }; - rcode = map_afrom_cs(ctx, head, cs, &parse_rules, &parse_rules, unlang_fixup_update, NULL, 128); + rcode = map_afrom_cs(ctx, head, cs, &parse_rules, &parse_rules, status_check_verify, parent, 128); if (rcode < 0) return -1; /* message already printed */ if (map_list_empty(head)) { - cf_log_err(cs, "'update' sections cannot be empty"); + cf_log_err(cs, "Invalid configuration - status check packets cannot be empty"); return -1; } }