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 d26b16c7470294269714359a6c87da5c70cb3d68..5436c3cacd41799d91a9314940da35a1a80aa660 100644 (file)
--- 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 7e336734280168f168b6ade354d6d1ddf6bd9469..eee7c23a7a8d1036be5c78bc7a710ba5dd68ca80 100644 (file)
--- 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 76420d3e7ab26d7aac32bcafccf9a816dd50ce79..144e8ce7ac7de08199c1d7efd37f72f38974040a 100644 (file)
--- 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 8a6e256def32517f8b93000e19c37e5d487cbdd0..ef6427f8b1b4b9ccce11ef31633f639ca48abe08 100644 (file)
--- 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 8fb8f5051f3a151e26d0c033c7f72cd05cc00116..52d62ed52171893b69326a7ce2f5cc3e640a40fc 100644 (file)
--- 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 2a3f0df5f1f520af52e9bc9330a3cef906464afc..7f76cb6286d31d1c62acdb37c31a556a3586a580 100644 (file)
--- 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 18d21b3a8650e76b3eabe03c6e81da6c05641f14..83d8893f99092b8a5018a3ca5b96db00900dd358 100644 (file)
--- 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 1f720ccaa92d5831fe22a414de901c4378c3b6c5..50ce95079878b2f6d1ae91d90e4ce79d3ee9e995 100644 (file)
--- 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 d35f29867ec16cbc11e0c548adab8935cc3fd65b..79a3cc7e26641fc5233d2941ef0aac7a15d9eb2c 100644 (file)
--- 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 5c11ce639ace4453937fe41ae95b53c8057b48e9..e2e7f772c404cbc01d08f82c6b41805b11945ede 100644 (file)
--- 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 e95340de881b8a1cd74e5104d4ba47c35f86b7ee..2772c5d3c73ae8bcfcb40ff139af547d9830a4c9 100644 (file)
--- 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 29fce303d156b98033acc7a5430e5c1ca206bcaf..f7ccb44439a9c5450d6b53bf1de13d39b87b86ce 100644 (file)
--- 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 65535a243e8c084043fb68951b73e22e37de4694..c9f53cdb0e6536a379f64257c6be50bacf2b2d52 100644 (file)
--- 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 02a05352b7c5fef3bb6da43a2dad518a369d4664..dd842bb5c11f4d08c29d2099302cb98387dc1993 100644 (file)
--- 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!