From: Nick Porter Date: Wed, 15 Sep 2021 15:15:40 +0000 (+0100) Subject: v4: Convert LDAP escape / unescape xlats to new API (#4227) X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=12d5afa57183803d4f4efa39b88c0ed8f13dddb9;p=thirdparty%2Ffreeradius-server.git v4: Convert LDAP escape / unescape xlats to new API (#4227) * Update %{ldap_escape } to new xlat api * Add tests for %{ldap_escape: } * Convert %{ldap_unescape: } to new xlat api * Add tests for %{ldap_unescape: } * Look for the current character in the "specials" list, not the first --- diff --git a/src/lib/ldap/util.c b/src/lib/ldap/util.c index 2c9c7bc689..ff3da18454 100644 --- a/src/lib/ldap/util.c +++ b/src/lib/ldap/util.c @@ -130,7 +130,7 @@ size_t fr_ldap_unescape_func(UNUSED request_t *request, char *out, size_t outlen p++; /* It's an escaped special, just remove the slash */ - if (memchr(specials, *in, sizeof(specials) - 1)) { + if (memchr(specials, *p, sizeof(specials) - 1)) { *out++ = *p++; continue; } diff --git a/src/modules/rlm_ldap/rlm_ldap.c b/src/modules/rlm_ldap/rlm_ldap.c index d4e13c5d75..f5049237ef 100644 --- a/src/modules/rlm_ldap/rlm_ldap.c +++ b/src/modules/rlm_ldap/rlm_ldap.c @@ -264,26 +264,81 @@ fr_dict_attr_autoload_t rlm_ldap_dict_attr[] = { }; +static xlat_arg_parser_t const ldap_escape_xlat_arg = { .required = true, .concat = true, .type = FR_TYPE_STRING }; + /** Escape LDAP string * * @ingroup xlat_functions */ -static ssize_t ldap_escape_xlat(UNUSED TALLOC_CTX *ctx, char **out, size_t outlen, - UNUSED void const *mod_inst, UNUSED void const *xlat_inst, - request_t *request, char const *fmt) +static xlat_action_t ldap_escape_xlat(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) { - return fr_ldap_escape_func(request, *out, outlen, fmt, NULL); + fr_value_box_t *vb, *in_vb = fr_dlist_head(in); + fr_sbuff_t sbuff; + fr_sbuff_uctx_talloc_t sbuff_ctx; + size_t len; + + MEM(vb = fr_value_box_alloc_null(ctx)); + /* + * Maximum space needed for output would be 3 times the input if every + * char needed escaping + */ + if (!fr_sbuff_init_talloc(vb, &sbuff, &sbuff_ctx, in_vb->length * 3, in_vb->length * 3)) { + REDEBUG("Failed to allocate buffer for escaped string"); + talloc_free(vb); + return XLAT_ACTION_FAIL; + } + + /* + * Call the escape function, including the space for the trailing NULL + */ + len = fr_ldap_escape_func(request, fr_sbuff_buff(&sbuff), in_vb->length * 3 + 1, in_vb->vb_strvalue, NULL); + + /* + * Trim buffer to fit used space and assign to box + */ + fr_sbuff_trim_talloc(&sbuff, len); + fr_value_box_strdup_shallow(vb, NULL, fr_sbuff_buff(&sbuff), in_vb->tainted); + fr_dcursor_append(out, vb); + return XLAT_ACTION_DONE; } /** Unescape LDAP string * * @ingroup xlat_functions */ -static ssize_t ldap_unescape_xlat(UNUSED TALLOC_CTX *ctx, char **out, size_t outlen, - UNUSED void const *mod_inst, UNUSED void const *xlat_inst, - request_t *request, char const *fmt) +static xlat_action_t ldap_unescape_xlat(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) { - return fr_ldap_unescape_func(request, *out, outlen, fmt, NULL); + fr_value_box_t *vb, *in_vb = fr_dlist_head(in); + fr_sbuff_t sbuff; + fr_sbuff_uctx_talloc_t sbuff_ctx; + size_t len; + + MEM(vb = fr_value_box_alloc_null(ctx)); + /* + * Maximum space needed for output will be the same as the input + */ + if (!fr_sbuff_init_talloc(vb, &sbuff, &sbuff_ctx, in_vb->length, in_vb->length)) { + REDEBUG("Failed to allocate buffer for unescaped string"); + talloc_free(vb); + return XLAT_ACTION_FAIL; + } + + /* + * Call the unescape function, including the space for the trailing NULL + */ + len = fr_ldap_unescape_func(request, fr_sbuff_buff(&sbuff), in_vb->length + 1, in_vb->vb_strvalue, NULL); + + /* + * Trim buffer to fit used space and assign to box + */ + fr_sbuff_trim_talloc(&sbuff, len); + fr_value_box_strdup_shallow(vb, NULL, fr_sbuff_buff(&sbuff), in_vb->tainted); + fr_dcursor_append(out, vb); + return XLAT_ACTION_DONE; } /** Expand an LDAP URL into a query, and return a string result from that query. @@ -1511,6 +1566,7 @@ static int mod_bootstrap(void *instance, CONF_SECTION *conf) rlm_ldap_t *inst = instance; char buffer[256]; char const *group_attribute; + xlat_t *xlat; inst->name = cf_section_name2(conf); if (!inst->name) inst->name = cf_section_name1(conf); @@ -1553,8 +1609,10 @@ static int mod_bootstrap(void *instance, CONF_SECTION *conf) } xlat_register_legacy(inst, inst->name, ldap_xlat, fr_ldap_escape_func, NULL, 0, XLAT_DEFAULT_BUF_LEN); - xlat_register_legacy(inst, "ldap_escape", ldap_escape_xlat, NULL, NULL, 0, XLAT_DEFAULT_BUF_LEN); - xlat_register_legacy(inst, "ldap_unescape", ldap_unescape_xlat, NULL, NULL, 0, XLAT_DEFAULT_BUF_LEN); + xlat = xlat_register(NULL, "ldap_escape", ldap_escape_xlat, false); + xlat_func_mono(xlat, &ldap_escape_xlat_arg); + xlat = xlat_register(NULL, "ldap_unescape", ldap_unescape_xlat, false); + xlat_func_mono(xlat, &ldap_escape_xlat_arg); map_proc_register(inst, inst->name, mod_map_proc, ldap_map_verify, 0); return 0; diff --git a/src/tests/modules/ldap/xlat.attrs b/src/tests/modules/ldap/xlat.attrs new file mode 100644 index 0000000000..d08077d093 --- /dev/null +++ b/src/tests/modules/ldap/xlat.attrs @@ -0,0 +1,12 @@ +# +# Input packet +# +Packet-Type = Access-Request +User-Name = "john" +User-Password = "password" +NAS-IP-Address = 1.2.3.5 + +# +# Expected answer +# +Packet-Type == Access-Accept diff --git a/src/tests/modules/ldap/xlat.unlang b/src/tests/modules/ldap/xlat.unlang new file mode 100644 index 0000000000..e72fbf97e2 --- /dev/null +++ b/src/tests/modules/ldap/xlat.unlang @@ -0,0 +1,62 @@ +# +# Test the "ldap" module xlat escape functions +# + +update request { + &Tmp-String-0 := "safe string" + &Tmp-String-1 := 'non safe,+"\<>;*=() string' + &Tmp-String-2 := ',+"\<>;*=()' +} + +# String with no escaping +update control { + &Tmp-String-0 := "%{ldap_escape:%{Tmp-String-0}}" +} + +if (&control.Tmp-String-0 != "safe string") { + test_fail +} + +# String with some characters to escape +update control { + &Tmp-String-1 := "%{ldap_escape:%{Tmp-String-1}}" +} + +if (&control.Tmp-String-1 != 'non safe\2c\2b\22\5c\3c\3e\3b\2a\3d\28\29 string') { + test_fail +} + +# String where all characters require escaping +update control { + &Tmp-String-2 := "%{ldap_escape:%{Tmp-String-2}}" +} + +if (&control.Tmp-String-2 != '\2c\2b\22\5c\3c\3e\3b\2a\3d\28\29') { + test_fail +} + +update request { + &Tmp-String-3 := "%{ldap_unescape:%{control.Tmp-String-0}}" +} + +if (&Tmp-String-3 != 'safe string') { + test_fail +} + +update request { + &Tmp-String-4 := "%{ldap_unescape:%{control.Tmp-String-1}}" +} + +if (&Tmp-String-4 != 'non safe,+"\<>;*=() string') { + test_fail +} + +update request { + &Tmp-String-5 := "%{ldap_unescape:%{control.Tmp-String-2}}" +} + +if (&Tmp-String-5 != ',+"\<>;*=()') { + test_fail +} + +test_pass