From: Alan T. DeKok Date: Sat, 8 Mar 2025 13:36:43 +0000 (-0500) Subject: check and use new xlat_func_bare_words X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=4b6e30d2dc70d99b8cb2988c05cbb1aa37d9bdcd;p=thirdparty%2Ffreeradius-server.git check and use new xlat_func_bare_words which changes the parsing of function arguments from * all unquoted arguments must be single-token bare words to * all unquoted arguments are expressions The default is to enable the flag, as there are a large number of configuration files to update. for now, the compatibility flag is disabled for src/tests/unit and src/tests/xlat, and enabled for src/tests/keywords and src/tests/modules --- diff --git a/doc/antora/modules/reference/pages/raddb/mods-available/eap.adoc b/doc/antora/modules/reference/pages/raddb/mods-available/eap.adoc index d26b16c747..5436c3cacd 100644 --- a/doc/antora/modules/reference/pages/raddb/mods-available/eap.adoc +++ b/doc/antora/modules/reference/pages/raddb/mods-available/eap.adoc @@ -98,7 +98,7 @@ keys or WPA enterprise. ### EAP-PWD (Secure password-based authentication) -In v4, the "known good" password is taken from the `&request.control.Password.Cleartext` list, +In v4, the "known good" password is taken from the `request.control.Password.Cleartext` list, as is done by other modules. The change from v3 is that the `inner-tunnel` virtual server is no not used. @@ -409,7 +409,7 @@ password. The file should be provided as the attribute: - &control.TLS-Session-Cert-File + control.TLS-Session-Cert-File If there are any errors loading or verifying the certificate, then authentication will fail. @@ -626,7 +626,7 @@ be converted to attributes for use in the `virtual_server`. Attributes created during certificate processing -will be placed in the `&session-state` list. +will be placed in the `session-state` list. This is to simplify session-resumption, as the contents of this list also contains session data for stateful resumption, and this list is encoded @@ -661,7 +661,7 @@ in the session-ticket for stateless resumption. ==== Attribute generation is only performed on full handshake, or where we detect that attributes -are missing from the &session-state list during +are missing from the `session-state` list during stateful session-resumption. Certificate attributes will usually be retrieved @@ -730,8 +730,8 @@ unable to connect when the configurations are enabled. You must ensure that any attributes required for policy decisions are cached along with the TLS session data. This is usually done by placing policy attributes in the -`&session-state` list, or in the case of EAP-PEAP, EAP-TTLS and -EAP-FAST, the `&parent.session-state` list (i.e. in the request +`session-state` list, or in the case of EAP-PEAP, EAP-TTLS and +EAP-FAST, the `parent.session-state` list (i.e. in the request which sets up the TLS part of the authentication attempt). Caching this data means that the policies are cached at the @@ -904,7 +904,7 @@ This is to secure the SSID used to provide connectivity to the OSU You can override this configuration item at run-time by setting: - &control.EAP-TLS-Require-Client-Cert = Yes/No + control.EAP-TLS-Require-Client-Cert = Yes/No @@ -983,7 +983,7 @@ However, you can require one by setting the following option. You can also override this option by setting: - &control.EAP-TLS-Require-Client-Cert = Yes + control.EAP-TLS-Require-Client-Cert = Yes NOTE: The majority of supplicants do not support using a client certificate with `EAP-TTLS`, so this option is unlikely @@ -1071,7 +1071,7 @@ Unlike `EAP-TLS`, `PEAP` does not require a client certificate. However, you can require one by setting the following option. You can also override this option by setting -&control.EAP-TLS-Require-Client-Cert = Yes +control.EAP-TLS-Require-Client-Cert = Yes NOTE: The majority of supplicants do not support using a client certificate with `PEAP`, so this option is unlikely to @@ -1416,7 +1416,7 @@ eap { } session { # mode = auto -# name = "%{EAP-Type}%interpreter(server)" +# name = "%{EAP-Type}%interpreter('server')" # lifetime = 86400 # require_extended_master_secret = yes # require_perfect_forward_secrecy = no diff --git a/raddb/mods-available/eap b/raddb/mods-available/eap index 7e33673428..eee7c23a7a 100644 --- a/raddb/mods-available/eap +++ b/raddb/mods-available/eap @@ -870,7 +870,7 @@ eap { # the value provided here is first hashed with SHA256 # before being passed to OpenSSL. # -# name = "%{EAP-Type}%interpreter(server)" +# name = "%{EAP-Type}%interpreter('server')" # # lifetime:: The period for which a resumable session remains vali.d diff --git a/raddb/policy.d/debug b/raddb/policy.d/debug index 76420d3e7a..144e8ce7ac 100644 --- a/raddb/policy.d/debug +++ b/raddb/policy.d/debug @@ -2,7 +2,7 @@ # Outputs the contents of the control list in debugging (-X) mode # debug_control { - if (%debug_attr(control)) { + if (%debug_attr('control')) { noop } } @@ -11,7 +11,7 @@ debug_control { # Outputs the contents of the request list in debugging (-X) mode # debug_request { - if (%debug_attr(request)) { + if (%debug_attr('request')) { noop } } @@ -20,7 +20,7 @@ debug_request { # Outputs the contents of the reply list in debugging (-X) mode # debug_reply { - if (%debug_attr(reply)) { + if (%debug_attr('reply')) { noop } } @@ -29,7 +29,7 @@ debug_reply { # Outputs the contents of the session state list in debugging (-X) mode # debug_session_state { - if (%debug_attr(session-state)) { + if (%debug_attr('session-state')) { noop } } diff --git a/src/bin/unit_test_attribute.c b/src/bin/unit_test_attribute.c index 8a6e256def..ef6427f8b1 100644 --- a/src/bin/unit_test_attribute.c +++ b/src/bin/unit_test_attribute.c @@ -2386,6 +2386,7 @@ static size_t command_max_buffer_size(command_result_t *result, command_file_ctx } extern bool tmpl_require_enum_prefix; +extern bool xlat_func_bare_words; /** Set or clear migration flags. * @@ -3786,6 +3787,14 @@ int main(int argc, char *argv[]) default_log.fd = STDOUT_FILENO; default_log.print_level = false; + /* + * Migration option - it's enabled by default in + * src/lib/server/main_config.c, until we have time to + * update all of the default configuration files and + * tests. + */ + xlat_func_bare_words = false; + while ((c = getopt(argc, argv, "cd:D:F:fxMhpr:S:w:")) != -1) switch (c) { case 'c': do_commands = true; diff --git a/src/lib/server/main_config.c b/src/lib/server/main_config.c index 8fb8f5051f..52d62ed521 100644 --- a/src/lib/server/main_config.c +++ b/src/lib/server/main_config.c @@ -188,7 +188,7 @@ static const conf_parser_t thread_config[] = { * Migration configuration. */ extern bool tmpl_require_enum_prefix; -bool xlat_func_bare_words = false; +bool xlat_func_bare_words = true; static const conf_parser_t migrate_config[] = { { FR_CONF_OFFSET_FLAGS("rewrite_update", CONF_FLAG_HIDDEN, main_config_t, rewrite_update) }, diff --git a/src/lib/tls/conf.c b/src/lib/tls/conf.c index 2a3f0df5f1..7f76cb6286 100644 --- a/src/lib/tls/conf.c +++ b/src/lib/tls/conf.c @@ -90,7 +90,7 @@ static conf_parser_t tls_cache_config[] = { }, .dflt = "auto" }, { FR_CONF_OFFSET_HINT_TYPE("name", FR_TYPE_STRING, fr_tls_cache_conf_t, id_name), - .dflt = "%{EAP-Type}%interpreter(server)", .quote = T_DOUBLE_QUOTED_STRING }, + .dflt = "%{EAP-Type}%interpreter('server')", .quote = T_DOUBLE_QUOTED_STRING }, { FR_CONF_OFFSET("lifetime", fr_tls_cache_conf_t, lifetime), .dflt = "1d" }, { FR_CONF_OFFSET("require_extended_master_secret", fr_tls_cache_conf_t, require_extms), .dflt = "yes" }, diff --git a/src/lib/unlang/xlat_tokenize.c b/src/lib/unlang/xlat_tokenize.c index 18d21b3a86..83d8893f99 100644 --- a/src/lib/unlang/xlat_tokenize.c +++ b/src/lib/unlang/xlat_tokenize.c @@ -45,7 +45,8 @@ RCSID("$Id$") # define XLAT_HEXDUMP(...) #endif -extern bool tmpl_require_enum_prefix; +extern const bool tmpl_require_enum_prefix; +extern const bool xlat_func_bare_words; /** These rules apply to literal values and function arguments inside of an expansion * @@ -1333,7 +1334,7 @@ fr_slen_t xlat_tokenize_argv(TALLOC_CTX *ctx, xlat_exp_head_t **out, fr_sbuff_t xlat_t const *xlat, fr_sbuff_parse_rules_t const *p_rules, tmpl_rules_t const *t_rules, bool spaces) { - int argc = 0; + int argc; fr_sbuff_t our_in = FR_SBUFF(in); ssize_t slen; fr_sbuff_marker_t m; @@ -1379,6 +1380,7 @@ fr_slen_t xlat_tokenize_argv(TALLOC_CTX *ctx, xlat_exp_head_t **out, fr_sbuff_t */ fr_sbuff_adv_past_whitespace(in, SIZE_MAX, NULL); fr_sbuff_marker(&m, &our_in); + argc = 1; while (fr_sbuff_extend(&our_in)) { xlat_exp_t *node = NULL; @@ -1387,7 +1389,6 @@ fr_slen_t xlat_tokenize_argv(TALLOC_CTX *ctx, xlat_exp_head_t **out, fr_sbuff_t size_t len; fr_sbuff_set(&m, &our_in); /* Record start of argument */ - argc++; /* * Whitespace isn't significant for comma-separated argvs @@ -1421,19 +1422,41 @@ fr_slen_t xlat_tokenize_argv(TALLOC_CTX *ctx, xlat_exp_head_t **out, fr_sbuff_t * tmpl. And update the function arguments to say "we want a tmpl, not a * string". */ - if (fr_sbuff_is_char(&our_in, '&')) { - if (xlat_tokenize_attribute(node->group, &our_in, our_p_rules, t_rules, TMPL_ATTR_REF_PREFIX_YES) < 0) goto error; - break; - } + if (spaces && !tmpl_require_enum_prefix && fr_sbuff_is_char(&our_in, '&')) { + slen = xlat_tokenize_attribute(node->group, &our_in, our_p_rules, t_rules, TMPL_ATTR_REF_PREFIX_YES); - if (xlat_tokenize_input(node->group, &our_in, - our_p_rules, t_rules, arg->safe_for) < 0) { + } else if (spaces || xlat_func_bare_words) { + /* + * Spaces - each argument is a bare word all by itself, OR an xlat thing all by itself. + * + * No spaces - each arugment is an expression, which can have embedded spaces. + */ + slen = xlat_tokenize_input(node->group, &our_in, our_p_rules, t_rules, arg->safe_for); + + } else if (fr_sbuff_is_char(&our_in, ')')) { + /* + * %foo() + */ + slen = 0; + + } else { + slen = xlat_tokenize_expression(node, &node->group, &our_in, our_p_rules, t_rules); + } + if (slen < 0) { error: if (our_p_rules == &tmp_p_rules) talloc_const_free(our_p_rules->terminals); talloc_free(head); FR_SBUFF_ERROR_RETURN(&our_in); /* error */ } + + /* + * No data, but the argument was required. Complain. + */ + if (!slen && arg->required) { + fr_strerror_printf("Missing required arg %u", argc); + goto error; + } break; /* @@ -1548,9 +1571,11 @@ fr_slen_t xlat_tokenize_argv(TALLOC_CTX *ctx, xlat_exp_head_t **out, fr_sbuff_t next: if (!arg->variadic) { arg++; + argc++; + if (arg->type == FR_TYPE_NULL) { fr_strerror_printf("Too many arguments, expected %zu, got %d", - (size_t) (arg - arg_start), argc - 1); + (size_t) (arg - arg_start), argc); goto error; } } diff --git a/src/tests/keywords/md5-error b/src/tests/keywords/md5-error index 1f720ccaa9..50ce950798 100644 --- a/src/tests/keywords/md5-error +++ b/src/tests/keywords/md5-error @@ -9,7 +9,7 @@ test_string := "hello" # # MD5 HMAC with missing key should fail # -result_octets := %hmacmd5(%{test_string}, ) +result_octets := %hmacmd5(%{test_string}, ) # ERROR if result_octets { test_fail } diff --git a/src/tests/keywords/policy.conf b/src/tests/keywords/policy.conf index d35f29867e..79a3cc7e26 100644 --- a/src/tests/keywords/policy.conf +++ b/src/tests/keywords/policy.conf @@ -6,21 +6,21 @@ # Outputs the contents of the control list in debugging (-X) mode # debug_control { - %debug_attr(control.[*]) + %debug_attr('control.[*]') } # # Outputs the contents of the request list in debugging (-X) mode # debug_request { - %debug_attr(request.[*]) + %debug_attr('request.[*]') } # # Outputs the contents of the reply list in debugging (-X) mode # debug_reply { - %debug_attr(reply.[*]) + %debug_attr('reply.[*]') } # @@ -57,12 +57,12 @@ success { test_fail { reply += { - Result-Status = "Failure in test file %interpreter(...filename)[%interpreter(...line)]" + Result-Status = "Failure in test file %interpreter('...filename')[%interpreter('...line')]" } if (parent.request) { parent.reply += { - Result-Status = "Failure in test file %interpreter(...filename)[%interpreter(...line)]" + Result-Status = "Failure in test file %interpreter('...filename')[%interpreter('...line')]" } } } diff --git a/src/tests/modules/all.mk b/src/tests/modules/all.mk index 5c11ce639a..e2e7f772c4 100644 --- a/src/tests/modules/all.mk +++ b/src/tests/modules/all.mk @@ -103,7 +103,7 @@ $(OUTPUT)/%: $(DIR)/%.unlang $(TEST_BIN_DIR)/unit_test_module | build.raddb @echo "MODULE-TEST $(TEST)" ${Q}mkdir -p $(dir $@) ${Q}cp $(if $(wildcard $(basename $<).attrs),$(basename $<).attrs,src/tests/modules/default-input.attrs) $@.attrs - ${Q}if ! MODULE_TEST_DIR=$(dir $<) MODULE_TEST_UNLANG=$< TEST="$(TEST)" $(TEST_BIN)/unit_test_module -D share/dictionary -d src/tests/modules/ -i "$@.attrs" -f "$@.attrs" -r "$@" -xxx > "$@.log" 2>&1 || ! test -f "$@"; then \ + ${Q}if ! MODULE_TEST_DIR=$(dir $<) MODULE_TEST_UNLANG=$< TEST="$(TEST)" $(TEST_BIN)/unit_test_module -D share/dictionary -d src/tests/modules/ -i "$@.attrs" -f "$@.attrs" -r "$@" -S xlat_func_bare_words=yes -xxx > "$@.log" 2>&1 || ! test -f "$@"; then \ if ! grep ERROR $< 2>&1 > /dev/null; then \ if grep 'LeakSanitizer has encountered a fatal error' $@.log 2>&1 > /dev/null; then \ echo "MODULE-TEST $(TEST) - ignoring LeakSanitizer fatal error."; \ @@ -119,7 +119,7 @@ $(OUTPUT)/%: $(DIR)/%.unlang $(TEST_BIN_DIR)/unit_test_module | build.raddb if [ "$$EXPECTED" != "$$FOUND" ]; then \ cat "$@.log"; \ echo "# $@.log"; \ - echo "MODULE_TEST_DIR=$(dir $<) MODULE_TEST_UNLANG=$< $(TEST_BIN)/unit_test_module -D share/dictionary -d src/tests/modules/ -i \"$@.attrs\" -f \"$@.attrs\" -r \"$@\" -xx"; \ + echo "MODULE_TEST_DIR=$(dir $<) MODULE_TEST_UNLANG=$< $(TEST_BIN)/unit_test_module -D share/dictionary -d src/tests/modules/ -i \"$@.attrs\" -f \"$@.attrs\" -r \"$@\" -S xlat_func_bare_words=yes -xx"; \ exit 1; \ else \ touch "$@"; \ diff --git a/src/tests/unit/xlat/alternation.txt b/src/tests/unit/xlat/alternation.txt index e95340de88..2772c5d3c7 100644 --- a/src/tests/unit/xlat/alternation.txt +++ b/src/tests/unit/xlat/alternation.txt @@ -14,11 +14,11 @@ match %{(%{User-Name} || "bar")} xlat foo %{%{User-Name} || 'bar'} baz match foo %{(%{User-Name} || 'bar')} baz -xlat %{%test(bar) || %{User-Name}} -match %{(%test(bar) || %{User-Name})} +xlat %{%test('bar') || %{User-Name}} +match %{(%test('bar') || %{User-Name})} -xlat %{%test(bar) || %{%{User-Name} || 'bar'}} -match %{(%test(bar) || %{(%{User-Name} || 'bar')})} +xlat %{%test('bar') || %{%{User-Name} || 'bar'}} +match %{(%test('bar') || %{(%{User-Name} || 'bar')})} xlat %{%{User-Name} || } match ERROR offset 19: Zero length attribute name: Unresolved attributes are not allowed here @@ -29,8 +29,8 @@ matchERROR offset 23: Zero length attribute name: Unresolved attributes are not xlat %{%{%{User-Name} || 'foo'} || 'bar'} match %{(%{(%{User-Name} || 'foo')} || 'bar')} -xlat %{%{%{User-Name} || 'foo'} || %{%test(bar) || %{User-Name}}} -match %{(%{(%{User-Name} || 'foo')} || %{(%test(bar) || %{User-Name})})} +xlat %{%{%{User-Name} || 'foo'} || %{%test('bar') || %{User-Name}}} +match %{(%{(%{User-Name} || 'foo')} || %{(%test('bar') || %{User-Name})})} xlat %{ || } match ERROR offset 4: Zero length attribute name: Unresolved attributes are not allowed here diff --git a/src/tests/unit/xlat/base.txt b/src/tests/unit/xlat/base.txt index 29fce303d1..f7ccb44439 100644 --- a/src/tests/unit/xlat/base.txt +++ b/src/tests/unit/xlat/base.txt @@ -260,10 +260,10 @@ match %rpad(User-Name, 5, 'x') # The second argument should be an integer. # # @todo - parsing - we don't currently track string offsets for intermediate nodes, -# so the "offset 23" is wrong. It also doesn't say *which* string is wrong. We'll fix that later. +# so the "offset" is wrong. It also doesn't say *which* string is wrong. We'll fix that later. # -xlat %rpad(User-Name, foo, 'x') -match ERROR offset 27: Failed parsing argument 1 as type 'uint64' +xlat %rpad(User-Name, 'foo', 'x') +match ERROR offset 29: Failed parsing argument 1 as type 'uint64' # # Argument quoting diff --git a/src/tests/unit/xlat/purify.txt b/src/tests/unit/xlat/purify.txt index 65535a243e..c9f53cdb0e 100644 --- a/src/tests/unit/xlat/purify.txt +++ b/src/tests/unit/xlat/purify.txt @@ -244,10 +244,10 @@ match "a""b""c""d" # # @todo - bare words should probably be disallowed # -xlat_purify %md5(foo) +xlat_purify %md5('foo') match 0xacbd18db4cc2f85cedef654fccc4a4d8 -xlat_purify %md5(%md5(foo)) +xlat_purify %md5(%md5('foo')) match 0x47847ae721df523d6388aebc9c94d656 xlat_purify %md5('%md5(foo)') diff --git a/src/tests/xlat/expr.txt b/src/tests/xlat/expr.txt index 02a05352b7..dd842bb5c1 100644 --- a/src/tests/xlat/expr.txt +++ b/src/tests/xlat/expr.txt @@ -1,5 +1,18 @@ # this is "foo" + PRINTABLE version of the packet authentication vector -xlat_expr "foo%bin(000000)" + +# +# 1 argument +# +xlat_expr %file.exists() +match ERROR offset 24 'Missing required arg 1' + +# +# Only one argument +# +xlat_expr %file.exists("foo", "bar") +match ERROR offset 30 'Too many arguments, expected 1, got 2' + +xlat_expr "foo%bin('000000')" match foo\000\000\000 xlat_expr 1 && 2 @@ -90,12 +103,12 @@ match 0x666f6f7f000001 # This just casts the octets to 'string', without # any escaping. # -xlat_expr "foo" + (string)%bin(0x0001020304) +xlat_expr "foo" + (string)%bin('0x0001020304') match foo\000\001\002\003\004 # string + octets gets promoted to octets -xlat_expr "foo" + %bin(0x0001020304) +xlat_expr "foo" + %bin('0x0001020304') match 0x666f6f0001020304 # no escaping!