From: Nick Porter Date: Mon, 29 Mar 2021 14:26:48 +0000 (+0100) Subject: v4: Convert %(client: ) to new xlat api (#4038) X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=9a85827967e23ed0d995ced66db3b9bcb8d149ba;p=thirdparty%2Ffreeradius-server.git v4: Convert %(client: ) to new xlat api (#4038) * Convert %(client: ) to new xlat api * Update documentation for %(client: ) xlat * Update existing uses of %(client: ) xlat in raddb * Update existing reference to %(client: ) in tests * Add specific test for %(client: ) xlat * Tidy client map test * Fix docs They were very wrong... * Fix examples in config file too Co-authored-by: Arran Cudbard-Bell --- diff --git a/doc/antora/modules/raddb/pages/mods-available/client.adoc b/doc/antora/modules/raddb/pages/mods-available/client.adoc index 6d4d675d294..2e78555f4b1 100644 --- a/doc/antora/modules/raddb/pages/mods-available/client.adoc +++ b/doc/antora/modules/raddb/pages/mods-available/client.adoc @@ -73,9 +73,9 @@ map client [] { // <1> [options="header,autowidth"] |=== | XLAT | Description -| `%{client:nas_type}` | Expands to client's `nas_type` (or "" if nas_type not set). -| `%{client:.nas_type}` | Expands to the `nas_type` of the client specified - by `` (or "" if `nas_type` not set). +| `%(client:)` | Expands to a field in the client definition. +| `%(client: )` | Expands to a field in the client definition specified + by ``. |=== diff --git a/doc/antora/modules/reference/pages/xlat/builtin.adoc b/doc/antora/modules/reference/pages/xlat/builtin.adoc index f4912202b96..1205e52a6c3 100644 --- a/doc/antora/modules/reference/pages/xlat/builtin.adoc +++ b/doc/antora/modules/reference/pages/xlat/builtin.adoc @@ -188,7 +188,7 @@ Server installed in /opt/freeradius Module rlm_exec.shell_escape = yes ``` -=== %{client:} +=== %(client:) Refers to a variable that was defined in the client section for the current client. See the sections `client { ... }` in `clients.conf`. @@ -199,7 +199,7 @@ current client. See the sections `client { ... }` in `clients.conf`. [source,unlang] ---- -"The client ipaddr is %{client:ipaddr}" +"The client ipaddr is %(client:ipaddr)" ---- .Output @@ -922,8 +922,8 @@ The packet type (`Access-Request`, etc.) === +%{Packet-SRC-IP-Address} and %{Packet-SRC-IPv6-Address}+ The source IPv4 or IPv6 address of the packet. See also the expansions -`%{client:ipaddr}` and `%{client:ipv6addr}`. The two expansions -should be identical, unless `%{client:ipaddr}` contains a DNS hostname. +`%(client:ipaddr)` and `%(client:ipv6addr)`. The two expansions +should be identical, unless `%(client:ipaddr)` contains a DNS hostname. === +%{Packet-DST-IP-Address} and %{Packet-DST-IPv6-Address}+ diff --git a/raddb/mods-available/client b/raddb/mods-available/client index 5cf8b2a9b00..efff6539c2c 100644 --- a/raddb/mods-available/client +++ b/raddb/mods-available/client @@ -76,9 +76,9 @@ # [options="header,autowidth"] # |=== # | XLAT | Description -# | `%{client:nas_type}` | Expands to client's `nas_type` (or "" if nas_type not set). -# | `%{client:.nas_type}` | Expands to the `nas_type` of the client specified -# by `` (or "" if `nas_type` not set). +# | `%(client:)` | Expands to a field in the client definition. +# | `%(client: )` | Expands to a field in the client definition specified + by ``. # |=== # diff --git a/raddb/mods-config/perl/example.pl b/raddb/mods-config/perl/example.pl index 0dc1693dfc1..d712e5e1c51 100644 --- a/raddb/mods-config/perl/example.pl +++ b/raddb/mods-config/perl/example.pl @@ -126,7 +126,7 @@ sub authenticate { return RLM_MODULE_REJECT; } else { # Accept user and set some attribute - if (&radiusd::xlat("%{client:group}") eq 'UltraAllInclusive') { + if (&radiusd::xlat("%(client:group)") eq 'UltraAllInclusive') { # User called from NAS with unlim plan set, set higher limits $RAD_REPLY{'h323-credit-amount'} = "1000000"; } else { diff --git a/raddb/policy.d/abfab-tr b/raddb/policy.d/abfab-tr index e84867e274a..37c001340fa 100644 --- a/raddb/policy.d/abfab-tr +++ b/raddb/policy.d/abfab-tr @@ -25,8 +25,8 @@ abfab_psk_authorize { abfab_client_check { # check that the acceptor host name is correct - if ("%{client:gss_acceptor_host_name}" && &GSS-acceptor-host-name) { - if ("%{client:gss_acceptor_host_name}" != "%{gss-acceptor-host-name}") { + if ("%(client:gss_acceptor_host_name)" && &GSS-acceptor-host-name) { + if ("%(client:gss_acceptor_host_name)" != "%{gss-acceptor-host-name}") { update reply { &Reply-Message = "GSS-Acceptor-Host-Name incorrect" } @@ -35,16 +35,16 @@ abfab_client_check { } # set trust-router-coi attribute from the client configuration - if ("%{client:trust_router_coi}") { + if ("%(client:trust_router_coi)") { update request { - &Trust-Router-COI := "%{client:trust_router_coi}" + &Trust-Router-COI := "%(client:trust_router_coi)" } } # set gss-acceptor-realm-name attribute from the client configuration - if ("%{client:gss_acceptor_realm_name}") { + if ("%(client:gss_acceptor_realm_name)") { update request { - &GSS-Acceptor-Realm-Name := "%{client:gss_acceptor_realm_name}" + &GSS-Acceptor-Realm-Name := "%(client:gss_acceptor_realm_name)" } } } diff --git a/raddb/policy.d/cui b/raddb/policy.d/cui index ea834f403e9..b26821e9eb3 100644 --- a/raddb/policy.d/cui +++ b/raddb/policy.d/cui @@ -39,7 +39,7 @@ cui_require_operator_name = "no" # } # cui.authorize { - if ("%{client:add_cui}" == 'yes') { + if ("%(client:add_cui)" == 'yes') { update request { &Chargeable-User-Identity := 0x00 } diff --git a/raddb/policy.d/operator-name b/raddb/policy.d/operator-name index 47f519aecb4..279e2932dc2 100644 --- a/raddb/policy.d/operator-name +++ b/raddb/policy.d/operator-name @@ -26,9 +26,9 @@ # an Operator-Name attribute # operator-name.authorize { - if ("%{client:Operator-Name}") { + if ("%(client:Operator-Name)") { update request { - &Operator-Name = "%{client:Operator-Name}" + &Operator-Name = "%(client:Operator-Name)" } } } diff --git a/src/modules/rlm_client/rlm_client.c b/src/modules/rlm_client/rlm_client.c index 07c39d10904..dd040b1c435 100644 --- a/src/modules/rlm_client/rlm_client.c +++ b/src/modules/rlm_client/rlm_client.c @@ -207,63 +207,72 @@ finish: return rcode; } +static xlat_arg_parser_t const xlat_client_args[] = { + { .required = true, .single = true, .type = FR_TYPE_STRING }, + { .single = true, .type = FR_TYPE_STRING }, + XLAT_ARG_PARSER_TERMINATOR +}; + /** xlat to get client config data * * Example: @verbatim -%{client:[.]foo} +%(client:foo []) @endverbatim * * @ingroup xlat_functions */ -static ssize_t xlat_client(TALLOC_CTX *ctx, char **out, UNUSED size_t outlen, - UNUSED void const *mod_inst, UNUSED void const *xlat_inst, - request_t *request, char const *fmt) +static xlat_action_t xlat_client(TALLOC_CTX *ctx, fr_dcursor_t *out, request_t *request, + UNUSED void const *xlat_inst, UNUSED void *xlat_thread_inst, + fr_value_box_list_t *in) { char const *value = NULL; - char buffer[INET6_ADDRSTRLEN], *q; - char const *p = fmt; fr_ipaddr_t ip; CONF_PAIR *cp; RADCLIENT *client = NULL; - - *out = NULL; - - q = strrchr(p, '.'); - if (q) { - strlcpy(buffer, p, (q + 1) - p); - if (fr_inet_pton(&ip, buffer, -1, AF_UNSPEC, false, true) < 0) goto request_client; - - p = q + 1; + fr_value_box_t *field = fr_dlist_head(in); + fr_value_box_t *client_ip = fr_dlist_next(in, field); + fr_value_box_t *vb; + + if (client_ip) { + if (fr_inet_pton(&ip, client_ip->vb_strvalue, -1, AF_UNSPEC, false, true) < 0) { + RDEBUG("Invalid client IP address \"%s\"", client_ip->vb_strvalue); + return XLAT_ACTION_FAIL; + } client = client_find(NULL, &ip, IPPROTO_IP); if (!client) { - RDEBUG("No client found with IP \"%s\"", buffer); - return 0; + RDEBUG("No client found with IP \"%s\"", client_ip->vb_strvalue); + return XLAT_ACTION_FAIL; } } else { - request_client: client = request->client; if (!client) { RERROR("No client associated with this request"); - - return -1; + return XLAT_ACTION_FAIL; } } - cp = cf_pair_find(client->cs, p); + cp = cf_pair_find(client->cs, field->vb_strvalue); if (!cp || !(value = cf_pair_value(cp))) { - if (strcmp(fmt, "shortname") == 0 && request->client->shortname) { + if (strcmp(field->vb_strvalue, "shortname") == 0 && request->client->shortname) { value = request->client->shortname; } - else if (strcmp(fmt, "nas_type") == 0 && request->client->nas_type) { + else if (strcmp(field->vb_strvalue, "nas_type") == 0 && request->client->nas_type) { value = request->client->nas_type; } - if (!value) return 0; + if (!value) return XLAT_ACTION_DONE; + } + + MEM(vb = fr_value_box_alloc_null(ctx)); + + if (fr_value_box_strdup(ctx, vb, NULL, value, false) < 0) { + talloc_free(vb); + return XLAT_ACTION_FAIL; } - *out = talloc_typed_strdup(ctx, value); - return talloc_array_length(*out) - 1; + fr_dcursor_append(out, vb); + return XLAT_ACTION_DONE; } @@ -343,7 +352,9 @@ static unlang_action_t CC_HINT(nonnull) mod_authorize(rlm_rcode_t *p_result, UNU */ static int mod_bootstrap(void *instance, UNUSED CONF_SECTION *conf) { - xlat_register_legacy(instance, "client", xlat_client, NULL, NULL, 0, 0); + xlat_t *xlat; + xlat = xlat_register(instance, "client", xlat_client, false); + xlat_func_args(xlat, xlat_client_args); map_proc_register(instance, "client", map_proc_client, NULL, 0); return 0; diff --git a/src/modules/rlm_perl/rlm_perl.c b/src/modules/rlm_perl/rlm_perl.c index 10783e4b19b..2d7363a4492 100644 --- a/src/modules/rlm_perl/rlm_perl.c +++ b/src/modules/rlm_perl/rlm_perl.c @@ -311,7 +311,7 @@ static XS(XS_radiusd_log) /* * This is a wraper for xlat_aeval * Now users are able to get data that is accessible only via xlat - * e.g. %{client:...} + * e.g. %(client:...) * Call syntax is radiusd::xlat(string), string will be handled the * same way it is described in EXPANSIONS section of man unlang */ diff --git a/src/tests/modules/client/map.unlang b/src/tests/modules/client/map.unlang index 0990589be3f..fc9c52344c7 100644 --- a/src/tests/modules/client/map.unlang +++ b/src/tests/modules/client/map.unlang @@ -2,10 +2,7 @@ map client { &Tmp-String-0 := 'nas_type' } -if (&Tmp-String-0 == 'a_type') { - test_pass -} -else { +if (&Tmp-String-0 != 'a_type') { test_fail } @@ -13,10 +10,7 @@ map client 127.0.0.1 { &Tmp-String-0 := 'nas_type' } -if (&Tmp-String-0 == 'a_type') { - test_pass -} -else { +if (&Tmp-String-0 != 'a_type') { test_fail } @@ -24,10 +18,7 @@ map client 127.0.0.2 { &Tmp-String-0 := 'nas_type' } -if (&Tmp-String-0 == 'b_type') { - test_pass -} -else { +if (&Tmp-String-0 != 'b_type') { test_fail } @@ -35,10 +26,7 @@ map client 127.0.0.5 { &Tmp-String-0 := 'nas_type' } -if (&Tmp-String-0 == 'b_type') { - test_pass -} -else { +if (&Tmp-String-0 != 'b_type') { test_fail } @@ -49,24 +37,15 @@ map client { &Tmp-String-1 += 'group' } -if (&Tmp-String-1[0] == 'a') { - test_pass -} -else { +if (&Tmp-String-1[0] != 'a') { test_fail } -if (&Tmp-String-1[1] == 'b') { - test_pass -} -else { +if (&Tmp-String-1[1] != 'b') { test_fail } -if (&Tmp-String-1[2] == 'c') { - test_pass -} -else { +if (&Tmp-String-1[2] != 'c') { test_fail } @@ -74,24 +53,15 @@ map client 127.0.0.2 { &Tmp-String-2 += 'group' } -if (&Tmp-String-2[0] == 'd') { - test_pass -} -else { +if (&Tmp-String-2[0] != 'd') { test_fail } -if (&Tmp-String-2[1] == 'e') { - test_pass -} -else { +if (&Tmp-String-2[1] != 'e') { test_fail } -if (&Tmp-String-2[2] == 'f') { - test_pass -} -else { +if (&Tmp-String-2[2] != 'f') { test_fail } @@ -106,14 +76,9 @@ map client { if (&Tmp-String-3) { test_fail } -else { - test_pass -} if (&Tmp-String-4) { test_fail } -else { - test_pass -} +test_pass \ No newline at end of file diff --git a/src/tests/modules/client/xlat.unlang b/src/tests/modules/client/xlat.unlang new file mode 100644 index 00000000000..d705c4ab801 --- /dev/null +++ b/src/tests/modules/client/xlat.unlang @@ -0,0 +1,53 @@ +update request { + &Tmp-String-0 := "%(client:nas_type)" +} + +if (&Tmp-String-0 != 'a_type') { + test_fail +} + +update request { + &Tmp-String-0 := "%(client:nas_type 127.0.0.1)" +} + +if (&Tmp-String-0 != 'a_type') { + test_fail +} + +update request { + &Tmp-String-0 := "%(client:nas_type 127.0.0.2)" +} + +if (&Tmp-String-0 != 'b_type') { + test_fail +} + +update request { + &Tmp-String-0 := "%(client:nas_type 127.0.0.5)" +} + +if (&Tmp-String-0 != 'b_type') { + test_fail +} + +# +# Test non-existent client properties +# +update request { + &Tmp-String-3 := "%(client:non-existent-attr)" + &Tmp-String-4 += "%(client:non-existing-attr2)" +} + +if (&Tmp-String-3 != "") { + test_fail +} + +if (&Tmp-String-4 != "") { + test_fail +} + +# +# Tests for multi-valued keys to be added when update takes multiple values +# + +test_pass \ No newline at end of file diff --git a/src/tests/modules/perl/test.pl b/src/tests/modules/perl/test.pl index 40274fdb69b..9eecf91dcb4 100644 --- a/src/tests/modules/perl/test.pl +++ b/src/tests/modules/perl/test.pl @@ -70,7 +70,7 @@ sub authenticate { return RLM_MODULE_REJECT; } else { # Accept user and set some attribute - if (&radiusd::xlat("%{client:group}") eq 'UltraAllInclusive') { + if (&radiusd::xlat("%(client:group)") eq 'UltraAllInclusive') { # User called from NAS with unlim plan set, set higher limits $RAD_REPLY{'h323-credit-amount'} = "1000000"; } else {