From: Arran Cudbard-Bell Date: Sun, 3 May 2026 04:38:10 +0000 (-0400) Subject: LDAP requires _two_ safety schemes, one for DNs one for filters X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=c30b2eede776dc8e0fde095ef140f1cd0a891791;p=thirdparty%2Ffreeradius-server.git LDAP requires _two_ safety schemes, one for DNs one for filters - The DN safety scheme would escape '+', which is the RDN separator char. This would break instances where usernames were extracted directly from certificates, as '+' would become \2c and would not correctly be broken into its constituent RDN values. - The existing filter schemes were not correctly applied in a number of places, meaning that if the administrator did not escape values with %ldap.uri.escape(), content from unsafe attributes could become structural elements of filters or DNs. --- diff --git a/doc/antora/modules/reference/pages/raddb/mods-available/ldap.adoc b/doc/antora/modules/reference/pages/raddb/mods-available/ldap.adoc index 3ce0cf77df0..59d240f5691 100644 --- a/doc/antora/modules/reference/pages/raddb/mods-available/ldap.adoc +++ b/doc/antora/modules/reference/pages/raddb/mods-available/ldap.adoc @@ -823,11 +823,18 @@ Retrieve a value from an LDAP directory using an LDAP uri. If the LDAP uri starts `ldap:///`, i.e. no host is specified, then the server configured for the module will be used. -.Example +When embedding user-controlled values in the filter part of the URI, wrap them with +`%ldap.filter.escape(...)`. When embedding values in the DN part (base DN, or a DN +being looked up), wrap them with `%ldap.dn.escape(...)`. Inserting unescaped user +input allows LDAP injection attacks. + +.Example - safe filter embedding [source,unlang] ---- -reply.Reply-Message := "Welcome %ldap("ldap:///ou=people,dc=example,dc=com?displayName?sub?(uid=%{User-Name})")" +# User-Name is filter-escaped before being embedded in the search filter. +# Without escaping, a User-Name of '*' would produce (uid=*) and match all users. +reply.Reply-Message := "Welcome %ldap("ldap:///ou=people,dc=example,dc=com?displayName?sub?(uid=%ldap.filter.escape(%{User-Name}))")" ---- .Output @@ -835,10 +842,21 @@ reply.Reply-Message := "Welcome %ldap("ldap:///ou=people,dc=example,dc=com?displ "Welcome Example User" ``` -=== %ldap.uri.escape(...) +.Example - safe DN embedding + +[source,unlang] +---- +# User-Name is DN-escaped before being used in the base DN. +# Without escaping, a value containing ',' could add extra DN components. +result := %ldap("ldap:///ou=%ldap.dn.escape(%{User-Name}),dc=example,dc=com?cn?base") +---- + +=== %ldap.dn.escape(...) -Escape a string for use in an LDAP filter or DN. The value will then be marked as safe for use -in LDAP URIs and DNs, and will not be escaped or modified. +Escape a string for use in an LDAP distinguished name (RFC 4514). Characters that are +special in a DN component (`,`, `+`, `"`, `\`, `<`, `>`, `;`, `*`, `=`, `(`, `)`) are +converted to `\HH` hex sequences. The result is marked safe for DN positions and will not +be re-escaped. .Return: _string_ @@ -846,21 +864,21 @@ in LDAP URIs and DNs, and will not be escaped or modified. [source,unlang] ---- -my-string := "ldap:///ou=profiles,dc=example,dc=com??sub?(objectClass=radiusprofile)" -reply.Reply-Message := "The LDAP url is %ldap.uri.escape(%{my-string}}" +my-string := "cn=admin,dc=example,dc=com" +reply.Reply-Message := "Escaped: %ldap.dn.escape(%{my-string})" ---- .Output ``` -"The LDAP url is ldap:///ou=profiles,dc=example,dc=com??sub?\28objectClass=radiusprofile\29" +"Escaped: cn\3dadmin\2cdc\3dexample\2cdc\3dcom" ``` -=== %ldap.uri.safe(...) +=== %ldap.dn.safe(...) -Mark a string as safe for use in an LDAP filter or DN. Values marked as safe for use in LDAP -URIs will not be escaped or modified, and will be allowed in places where dynamic values are -usually prohibited. +Mark a string as already safe for use in an LDAP DN. The value will not be escaped or +modified, and will be allowed in places where dynamic values are usually prohibited. +Use this only for strings you have constructed or validated yourself. .Return: _string_ @@ -871,9 +889,9 @@ usually prohibited. my-int := "%ldap.profile(ldap://%ldap.uri.safe(%{LDAP-Host}):%ldap.uri.safe(%{LDAP-Port})/ou=profiles,dc=example,dc=com??sub?(objectClass=radiusprofile)" ---- -=== %ldap.uri.unescape(...) +=== %ldap.dn.unescape(...) -Unescape a string for use in an LDAP filter or DN. +Decode `\HH` hex sequences in an LDAP DN string back to their original characters. .Return: _string_ @@ -881,16 +899,59 @@ Unescape a string for use in an LDAP filter or DN. [source,unlang] ---- -my-string := "ldap:///ou=profiles,dc=example,dc=com??sub?\28objectClass=radiusprofile\29" -reply.Reply-Message := "The LDAP url is %ldap.uri.unescape(%{my-string})" +my-string := "cn\3dadmin\2cdc\3dexample\2cdc\3dcom" +reply.Reply-Message := "Unescaped: %ldap.dn.unescape(%{my-string})" ---- .Output ``` -"The LDAP url is ldap:///ou=profiles,dc=example,dc=com??sub?(objectClass=radiusprofile)" +"Unescaped: cn=admin,dc=example,dc=com" ``` +=== %ldap.filter.escape(...) + +Escape a string for use as an assertion value in an LDAP search filter (RFC 4515). +Only the characters that are special in a filter assertion value are escaped: +`*`, `(`, `)`, `\`, and NUL. Characters such as `=`, `+`, and `,` are intentionally +left unescaped because OpenLDAP does not decode non-required `\HH` sequences, so +escaping them would cause silent match failures for usernames that legitimately contain +those characters. + +Use this function -- not `%ldap.dn.escape` -- when inserting user-controlled values +into the filter part of an LDAP URI or search string. + +.Return: _string_ + +.Example + +[source,unlang] +---- +# Safely embed User-Name in a search filter. +# A payload like '*' would otherwise produce (uid=*), matching every user. +result := %ldap("ldap:///ou=people,dc=example,dc=com?cn?sub?(uid=%ldap.filter.escape(%{User-Name}))") +---- + +=== %ldap.filter.safe(...) + +Mark a string as already safe for use in an LDAP filter assertion value. The value will +not be escaped or modified. Use this only for strings you have constructed or validated +yourself. + +.Return: _string_ + +=== %ldap.filter.unescape(...) + +Decode `\HH` hex sequences in an LDAP filter assertion value back to their original +characters. + +.Return: _string_ + +=== %ldap.uri.escape(...), %ldap.uri.safe(...), %ldap.uri.unescape(...) + +Aliases for `%ldap.dn.escape`, `%ldap.dn.safe`, and `%ldap.dn.unescape` respectively. +Retained for backwards compatibility. Prefer the `ldap.dn.*` names in new configs. + === %ldap.uri.attr_option(...) Add an option to all attribute referenced in an LDAP URI. diff --git a/raddb/mods-available/ldap b/raddb/mods-available/ldap index 405af396713..3756e83d33e 100644 --- a/raddb/mods-available/ldap +++ b/raddb/mods-available/ldap @@ -975,11 +975,18 @@ ldap { # If the LDAP uri starts `ldap:///`, i.e. no host is specified, then # the server configured for the module will be used. # -# .Example +# When embedding user-controlled values in the filter part of the URI, wrap +# them with `%ldap.filter.escape(...)`. When embedding values in the DN part +# (base DN, or in a DN being looked up), wrap them with `%ldap.dn.escape(...)`. +# Inserting unescaped user input allows LDAP injection attacks. +# +# .Example - safe filter embedding # # [source,unlang] # ---- -# reply.Reply-Message := "Welcome %ldap("ldap:///ou=people,dc=example,dc=com?displayName?sub?(uid=%{User-Name})")" +# # User-Name is filter-escaped before being embedded in the search filter. +# # Without escaping, a User-Name of '*' would produce (uid=*) and match all users. +# reply.Reply-Message := "Welcome %ldap("ldap:///ou=people,dc=example,dc=com?displayName?sub?(uid=%ldap.filter.escape(%{User-Name}))")" # ---- # # .Output @@ -987,10 +994,21 @@ ldap { # "Welcome Example User" # ``` # -# === %ldap.uri.escape(...) +# .Example - safe DN embedding +# +# [source,unlang] +# ---- +# # User-Name is DN-escaped before being used in the base DN. +# # Without escaping, a value containing ',' could add extra DN components. +# result := %ldap("ldap:///ou=%ldap.dn.escape(%{User-Name}),dc=example,dc=com?cn?base") +# ---- +# +# === %ldap.dn.escape(...) # -# Escape a string for use in an LDAP filter or DN. The value will then be marked as safe for use -# in LDAP URIs and DNs, and will not be escaped or modified. +# Escape a string for use in an LDAP distinguished name (RFC 4514). Characters +# that are special in a DN component (`,`, `+`, `"`, `\`, `<`, `>`, `;`, `*`, `=`, `(`, `)`) +# are converted to `\HH` hex sequences. The result is marked safe for use in DN positions +# and will not be re-escaped. # # .Return: _string_ # @@ -998,21 +1016,21 @@ ldap { # # [source,unlang] # ---- -# my-string := "ldap:///ou=profiles,dc=example,dc=com??sub?(objectClass=radiusprofile)" -# reply.Reply-Message := "The LDAP url is %ldap.uri.escape(%{my-string}}" +# my-string := "cn=admin,dc=example,dc=com" +# reply.Reply-Message := "Escaped: %ldap.dn.escape(%{my-string})" # ---- # # .Output # # ``` -# "The LDAP url is ldap:///ou=profiles,dc=example,dc=com??sub?\28objectClass=radiusprofile\29" +# "Escaped: cn\3dadmin\2cdc\3dexample\2cdc\3dcom" # ``` # -# === %ldap.uri.safe(...) +# === %ldap.dn.safe(...) # -# Mark a string as safe for use in an LDAP filter or DN. Values marked as safe for use in LDAP -# URIs will not be escaped or modified, and will be allowed in places where dynamic values are -# usually prohibited. +# Mark a string as already safe for use in an LDAP DN. The value will not be escaped or +# modified, and will be allowed in places where dynamic values are usually prohibited. +# Use this only for strings you have constructed or validated yourself. # # .Return: _string_ # @@ -1023,9 +1041,9 @@ ldap { # my-int := "%ldap.profile(ldap://%ldap.uri.safe(%{LDAP-Host}):%ldap.uri.safe(%{LDAP-Port})/ou=profiles,dc=example,dc=com??sub?(objectClass=radiusprofile)" # ---- # -# === %ldap.uri.unescape(...) +# === %ldap.dn.unescape(...) # -# Unescape a string for use in an LDAP filter or DN. +# Decode `\HH` hex sequences in an LDAP DN string back to their original characters. # # .Return: _string_ # @@ -1033,16 +1051,59 @@ ldap { # # [source,unlang] # ---- -# my-string := "ldap:///ou=profiles,dc=example,dc=com??sub?\28objectClass=radiusprofile\29" -# reply.Reply-Message := "The LDAP url is %ldap.uri.unescape(%{my-string})" +# my-string := "cn\3dadmin\2cdc\3dexample\2cdc\3dcom" +# reply.Reply-Message := "Unescaped: %ldap.dn.unescape(%{my-string})" # ---- # # .Output # # ``` -# "The LDAP url is ldap:///ou=profiles,dc=example,dc=com??sub?(objectClass=radiusprofile)" +# "Unescaped: cn=admin,dc=example,dc=com" # ``` # +# === %ldap.filter.escape(...) +# +# Escape a string for use as an assertion value in an LDAP search filter (RFC 4515). +# Only the characters that are special in a filter assertion value are escaped: +# `*`, `(`, `)`, `\`, and NUL. Characters such as `=`, `+`, and `,` are intentionally +# left unescaped because OpenLDAP does not decode non-required `\HH` sequences, so +# escaping them would cause silent match failures for usernames that legitimately +# contain those characters. +# +# Use this function -- not `%ldap.dn.escape` -- when inserting user-controlled values +# into the filter part of an LDAP URI or search string. +# +# .Return: _string_ +# +# .Example +# +# [source,unlang] +# ---- +# # Safely embed User-Name in a search filter. +# # A payload like '*' would otherwise produce (uid=*), matching every user. +# result := %ldap("ldap:///ou=people,dc=example,dc=com?cn?sub?(uid=%ldap.filter.escape(%{User-Name}))") +# ---- +# +# === %ldap.filter.safe(...) +# +# Mark a string as already safe for use in an LDAP filter assertion value. The value will +# not be escaped or modified. Use this only for strings you have constructed or validated +# yourself. +# +# .Return: _string_ +# +# === %ldap.filter.unescape(...) +# +# Decode `\HH` hex sequences in an LDAP filter assertion value back to their original +# characters. +# +# .Return: _string_ +# +# === %ldap.uri.escape(...), %ldap.uri.safe(...), %ldap.uri.unescape(...) +# +# Aliases for `%ldap.dn.escape`, `%ldap.dn.safe`, and `%ldap.dn.unescape` respectively. +# Retained for backwards compatibility. Prefer the `ldap.dn.*` names in new configs. +# # === %ldap.uri.attr_option(...) # # Add an option to all attribute referenced in an LDAP URI. diff --git a/src/lib/ldap/base.h b/src/lib/ldap/base.h index 6adb84e9dbe..1bd34526332 100644 --- a/src/lib/ldap/base.h +++ b/src/lib/ldap/base.h @@ -761,7 +761,10 @@ unlang_action_t fr_ldap_trunk_extended(TALLOC_CTX *ctx, void fr_ldap_timeout_debug(request_t *request, fr_ldap_connection_t const *conn, fr_time_delta_t timeout, char const *prefix); -size_t fr_ldap_uri_escape_func(UNUSED request_t *request, char *out, size_t outlen, char const *in, UNUSED void *arg) +size_t fr_ldap_dn_escape_func(UNUSED request_t *request, char *out, size_t outlen, char const *in, UNUSED void *arg) + CC_HINT(nonnull(2,4)); + +size_t fr_ldap_filter_escape_func(UNUSED request_t *request, char *out, size_t outlen, char const *in, UNUSED void *arg) CC_HINT(nonnull(2,4)); size_t fr_ldap_uri_unescape_func(UNUSED request_t *request, char *out, size_t outlen, char const *in, UNUSED void *arg) @@ -958,7 +961,9 @@ char const *fr_ldap_url_err_to_str(int ldap_url_err); void fr_ldap_entry_dump(LDAPMessage *entry); -int fr_ldap_box_escape(fr_value_box_t *vb, UNUSED void *uctx); +int fr_ldap_dn_box_escape(fr_value_box_t *vb, UNUSED void *uctx); + +int fr_ldap_filter_box_escape(fr_value_box_t *vb, UNUSED void *uctx); int fr_ldap_filter_to_tmpl(TALLOC_CTX *ctx, tmpl_rules_t const *t_rules, char const **sub, size_t sublen, tmpl_t **out) CC_HINT(nonnull()); diff --git a/src/lib/ldap/util.c b/src/lib/ldap/util.c index 698a9a2bd3b..276aad16dea 100644 --- a/src/lib/ldap/util.c +++ b/src/lib/ldap/util.c @@ -34,7 +34,8 @@ USES_APPLE_DEPRECATED_API #include -static const char specials[] = ",+\"\\<>;*=()"; +/* RFC 4514 DN attribute value special characters */ +static const char dn_specials[] = ",+\"\\<>;*=()"; static const char hextab[] = "0123456789abcdef"; static const bool escapes[SBUFF_CHAR_CLASS] = { [' '] = true, @@ -49,18 +50,15 @@ static const bool escapes[SBUFF_CHAR_CLASS] = { ['\''] = true }; -/** Converts "bad" strings into ones which are safe for LDAP - * - * @note RFC 4515 says filter strings can only use the @verbatim \ @endverbatim - * format, whereas RFC 4514 indicates that some chars in DNs, may be escaped simply - * with a backslash. For simplicity, we always use the hex escape sequences. - * In other areas where we're doing DN comparison, the DNs need to be normalised first - * so that they both use only hex escape sequences. - * - * @note This is a callback for xlat operations. +/* RFC 4515 filter assertion value special characters */ +static const char filter_specials[] = "*()\\"; +; + +/** Escape a string for use as an RFC 4514 DN attribute value * - * Will escape any characters in input strings that would cause the string to be interpreted - * as part of a DN and or filter. Escape sequence is @verbatim \ @endverbatim. + * Escapes characters that have special meaning in DNs. Leading space and + * '#' are also escaped as required by RFC 4514. + * Escape sequence is @verbatim \ @endverbatim. * * @param request The current request. * @param out Pointer to output buffer. @@ -68,7 +66,7 @@ static const bool escapes[SBUFF_CHAR_CLASS] = { * @param in Raw unescaped string. * @param arg Any additional arguments (unused). */ -size_t fr_ldap_uri_escape_func(UNUSED request_t *request, char *out, size_t outlen, char const *in, UNUSED void *arg) +size_t fr_ldap_dn_escape_func(UNUSED request_t *request, char *out, size_t outlen, char const *in, UNUSED void *arg) { size_t left = outlen; @@ -78,7 +76,7 @@ size_t fr_ldap_uri_escape_func(UNUSED request_t *request, char *out, size_t outl /* * Encode unsafe characters. */ - if (memchr(specials, *in, sizeof(specials) - 1)) { + if (memchr(dn_specials, *in, sizeof(dn_specials) - 1)) { encode: /* * Only 3 or less bytes available. @@ -108,24 +106,24 @@ size_t fr_ldap_uri_escape_func(UNUSED request_t *request, char *out, size_t outl return outlen - left; } -int fr_ldap_box_escape(fr_value_box_t *vb, UNUSED void *uctx) +int fr_ldap_dn_box_escape(fr_value_box_t *vb, UNUSED void *uctx) { fr_sbuff_t sbuff; fr_sbuff_uctx_talloc_t sbuff_ctx; size_t len; - fr_assert(!fr_value_box_is_safe_for(vb, fr_ldap_box_escape)); + fr_assert(!fr_value_box_is_safe_for(vb, fr_ldap_dn_box_escape)); if ((vb->type != FR_TYPE_STRING) && (fr_value_box_cast_in_place(vb, vb, FR_TYPE_STRING, NULL) < 0)) { return -1; } if (!fr_sbuff_init_talloc(vb, &sbuff, &sbuff_ctx, vb->vb_length * 3, vb->vb_length * 3)) { - fr_strerror_printf_push("Failed to allocate buffer for escaped filter"); + fr_strerror_printf_push("Failed to allocate buffer for escaped DN"); return -1; } - len = fr_ldap_uri_escape_func(NULL, fr_sbuff_buff(&sbuff), vb->vb_length * 3 + 1, vb->vb_strvalue, NULL); + len = fr_ldap_dn_escape_func(NULL, fr_sbuff_buff(&sbuff), vb->vb_length * 3 + 1, vb->vb_strvalue, NULL); /* * If the returned length is unchanged, the value was already safe @@ -140,6 +138,77 @@ int fr_ldap_box_escape(fr_value_box_t *vb, UNUSED void *uctx) return 0; } +/** Escape a string for use as an RFC 4515 filter assertion value + * + * Escapes only the characters that MUST be escaped in filter assertion values + * per RFC 4515: '*', '(', ')', '\'. Other characters (including ',', '+', + * '=') must NOT be escaped -- some LDAP implementations do not decode + * non-required \HH sequences in assertion values and will fail to match. + * Escape sequence is @verbatim \ @endverbatim. + * + * @param request The current request. + * @param out Pointer to output buffer. + * @param outlen Size of the output buffer. + * @param in Raw unescaped string. + * @param arg Any additional arguments (unused). + */ +size_t fr_ldap_filter_escape_func(UNUSED request_t *request, char *out, size_t outlen, char const *in, UNUSED void *arg) +{ + size_t left = outlen; + + while (*in) { + if (memchr(filter_specials, *in, sizeof(filter_specials) - 1)) { + if (left <= 3) break; + + *out++ = '\\'; + *out++ = hextab[(*in >> 4) & 0x0f]; + *out++ = hextab[*in & 0x0f]; + in++; + left -= 3; + + continue; + } + + if (left <= 1) break; + + *out++ = *in++; + left--; + } + + *out = '\0'; + + return outlen - left; +} + +int fr_ldap_filter_box_escape(fr_value_box_t *vb, UNUSED void *uctx) +{ + fr_sbuff_t sbuff; + fr_sbuff_uctx_talloc_t sbuff_ctx; + size_t len; + + fr_assert(!fr_value_box_is_safe_for(vb, fr_ldap_filter_box_escape)); + + if ((vb->type != FR_TYPE_STRING) && (fr_value_box_cast_in_place(vb, vb, FR_TYPE_STRING, NULL) < 0)) { + return -1; + } + + if (!fr_sbuff_init_talloc(vb, &sbuff, &sbuff_ctx, vb->vb_length * 3, vb->vb_length * 3)) { + fr_strerror_printf_push("Failed to allocate buffer for escaped filter"); + return -1; + } + + len = fr_ldap_filter_escape_func(NULL, fr_sbuff_buff(&sbuff), vb->vb_length * 3 + 1, vb->vb_strvalue, NULL); + + if (len == vb->vb_length) { + talloc_free(fr_sbuff_buff(&sbuff)); + } else { + fr_sbuff_trim_talloc(&sbuff, len); + fr_value_box_strdup_shallow_replace(vb, fr_sbuff_buff(&sbuff), len); + } + + return 0; +} + /** Converts escaped DNs and filter strings into normal * * @note RFC 4515 says filter strings can only use the @verbatim \ @endverbatim @@ -174,7 +243,7 @@ size_t fr_ldap_uri_unescape_func(UNUSED request_t *request, char *out, size_t ou p++; /* It's an escaped special, just remove the slash */ - if (memchr(specials, *p, sizeof(specials) - 1)) { + if (memchr(dn_specials, *p, sizeof(dn_specials) - 1)) { *out++ = *p++; continue; } diff --git a/src/modules/rlm_ldap/groups.c b/src/modules/rlm_ldap/groups.c index 8119017fd72..4288accde46 100644 --- a/src/modules/rlm_ldap/groups.c +++ b/src/modules/rlm_ldap/groups.c @@ -136,7 +136,7 @@ static unlang_action_t ldap_group_name2dn_start(unlang_result_t *p_result, reque inst->group.obj_filter ? inst->group.obj_filter : "", group_ctx->group_name[0] && group_ctx->group_name[1] ? "(|" : ""); while (*name) { - fr_ldap_uri_escape_func(request, buffer, sizeof(buffer), *name++, NULL); + fr_ldap_filter_escape_func(request, buffer, sizeof(buffer), *name++, NULL); filter = talloc_asprintf_append_buffer(filter, "(%s=%s)", inst->group.obj_name_attr, buffer); group_ctx->name_cnt++; @@ -823,12 +823,12 @@ unlang_action_t rlm_ldap_check_groupobj_dynamic(unlang_result_t *p_result, reque }, .at_runtime = true, .escape.box_escape = (fr_value_box_escape_t) { - .func = fr_ldap_box_escape, - .safe_for = (fr_value_box_safe_for_t)fr_ldap_box_escape, + .func = fr_ldap_filter_box_escape, + .safe_for = (fr_value_box_safe_for_t)fr_ldap_filter_box_escape, .always_escape = false, }, .escape.mode = TMPL_ESCAPE_PRE_CONCAT, - .literals_safe_for = (fr_value_box_safe_for_t)fr_ldap_box_escape, + .literals_safe_for = (fr_value_box_safe_for_t)fr_ldap_filter_box_escape, .cast = FR_TYPE_STRING, }; diff --git a/src/modules/rlm_ldap/rlm_ldap.c b/src/modules/rlm_ldap/rlm_ldap.c index fb3deee6d42..dcdd09c02a1 100644 --- a/src/modules/rlm_ldap/rlm_ldap.c +++ b/src/modules/rlm_ldap/rlm_ldap.c @@ -181,9 +181,33 @@ static const conf_parser_t module_config[] = { CONF_PARSER_TERMINATOR }; +#define LDAP_DN_CALL_ENV_ESCAPE \ + .pair.escape = { \ + .box_escape = { \ + .func = fr_ldap_dn_box_escape, \ + .safe_for = (fr_value_box_safe_for_t)fr_ldap_dn_box_escape, \ + .always_escape = false, \ + }, \ + .mode = TMPL_ESCAPE_PRE_CONCAT \ + }, \ + .pair.literals_safe_for = (fr_value_box_safe_for_t)fr_ldap_dn_box_escape + +#define LDAP_FILTER_CALL_ENV_ESCAPE \ + .pair.escape = { \ + .box_escape = { \ + .func = fr_ldap_filter_box_escape, \ + .safe_for = (fr_value_box_safe_for_t)fr_ldap_filter_box_escape, \ + .always_escape = false, \ + }, \ + .mode = TMPL_ESCAPE_PRE_CONCAT \ + }, \ + .pair.literals_safe_for = (fr_value_box_safe_for_t)fr_ldap_filter_box_escape + #define USER_CALL_ENV_COMMON(_struct) \ - { FR_CALL_ENV_OFFSET("base_dn", FR_TYPE_STRING, CALL_ENV_FLAG_REQUIRED | CALL_ENV_FLAG_CONCAT, _struct, user_base), .pair.dflt = "", .pair.dflt_quote = T_SINGLE_QUOTED_STRING }, \ - { FR_CALL_ENV_OFFSET("filter", FR_TYPE_STRING, CALL_ENV_FLAG_NULLABLE | CALL_ENV_FLAG_CONCAT, _struct, user_filter), .pair.dflt = "(&)", .pair.dflt_quote = T_SINGLE_QUOTED_STRING } + { FR_CALL_ENV_OFFSET("base_dn", FR_TYPE_STRING, CALL_ENV_FLAG_REQUIRED | CALL_ENV_FLAG_CONCAT, _struct, user_base), \ + .pair.dflt = "", .pair.dflt_quote = T_SINGLE_QUOTED_STRING, LDAP_DN_CALL_ENV_ESCAPE }, \ + { FR_CALL_ENV_OFFSET("filter", FR_TYPE_STRING, CALL_ENV_FLAG_NULLABLE | CALL_ENV_FLAG_CONCAT, _struct, user_filter), \ + .pair.dflt = "(&)", .pair.dflt_quote = T_SINGLE_QUOTED_STRING, LDAP_FILTER_CALL_ENV_ESCAPE } static const call_env_method_t authenticate_method_env = { FR_CALL_ENV_METHOD_OUT(ldap_auth_call_env_t), @@ -223,26 +247,21 @@ static const call_env_method_t authorize_method_env = { })) }, { FR_CALL_ENV_SUBSECTION("group", NULL, CALL_ENV_FLAG_NONE, ((call_env_parser_t[]) { - { FR_CALL_ENV_OFFSET("base_dn", FR_TYPE_STRING, CALL_ENV_FLAG_CONCAT, ldap_autz_call_env_t, group_base) }, + { FR_CALL_ENV_OFFSET("base_dn", FR_TYPE_STRING, CALL_ENV_FLAG_CONCAT, ldap_autz_call_env_t, group_base), + LDAP_DN_CALL_ENV_ESCAPE }, { FR_CALL_ENV_PARSE_ONLY_OFFSET("membership_filter", FR_TYPE_STRING, CALL_ENV_FLAG_CONCAT, ldap_autz_call_env_t, group_filter), .pair.func = ldap_group_filter_parse, - .pair.escape = { - .box_escape = { - .func = fr_ldap_box_escape, - .safe_for = (fr_value_box_safe_for_t)fr_ldap_box_escape, - .always_escape = false, - }, - .mode = TMPL_ESCAPE_PRE_CONCAT - }, - .pair.literals_safe_for = (fr_value_box_safe_for_t)fr_ldap_box_escape, + LDAP_FILTER_CALL_ENV_ESCAPE }, CALL_ENV_TERMINATOR })) }, { FR_CALL_ENV_SUBSECTION("profile", NULL, CALL_ENV_FLAG_NONE, ((call_env_parser_t[]) { - { FR_CALL_ENV_OFFSET("default", FR_TYPE_STRING, CALL_ENV_FLAG_CONCAT, ldap_autz_call_env_t, default_profile) }, + { FR_CALL_ENV_OFFSET("default", FR_TYPE_STRING, CALL_ENV_FLAG_CONCAT, ldap_autz_call_env_t, default_profile), + LDAP_DN_CALL_ENV_ESCAPE }, { FR_CALL_ENV_OFFSET("filter", FR_TYPE_STRING, CALL_ENV_FLAG_CONCAT, ldap_autz_call_env_t, profile_filter), - .pair.dflt = "(&)", .pair.dflt_quote = T_SINGLE_QUOTED_STRING }, //!< Correct filter for when the DN is known. + .pair.dflt = "(&)", .pair.dflt_quote = T_SINGLE_QUOTED_STRING, + LDAP_FILTER_CALL_ENV_ESCAPE }, CALL_ENV_TERMINATOR } )) }, CALL_ENV_TERMINATOR @@ -274,18 +293,11 @@ static const call_env_method_t xlat_memberof_method_env = { })) }, { FR_CALL_ENV_SUBSECTION("group", NULL, CALL_ENV_FLAG_NONE, ((call_env_parser_t[]) { - { FR_CALL_ENV_OFFSET("base_dn", FR_TYPE_STRING, CALL_ENV_FLAG_CONCAT, ldap_xlat_memberof_call_env_t, group_base) }, + { FR_CALL_ENV_OFFSET("base_dn", FR_TYPE_STRING, CALL_ENV_FLAG_CONCAT, ldap_xlat_memberof_call_env_t, group_base), + LDAP_DN_CALL_ENV_ESCAPE }, { FR_CALL_ENV_PARSE_ONLY_OFFSET("membership_filter", FR_TYPE_STRING, CALL_ENV_FLAG_CONCAT, ldap_xlat_memberof_call_env_t, group_filter), .pair.func = ldap_group_filter_parse, - .pair.escape = { - .box_escape = { - .func = fr_ldap_box_escape, - .safe_for = (fr_value_box_safe_for_t)fr_ldap_box_escape, - .always_escape = false, - }, - .mode = TMPL_ESCAPE_PRE_CONCAT - }, - .pair.literals_safe_for = (fr_value_box_safe_for_t)fr_ldap_box_escape, + LDAP_FILTER_CALL_ENV_ESCAPE }, CALL_ENV_TERMINATOR })) }, @@ -304,7 +316,8 @@ static const call_env_method_t xlat_profile_method_env = { { FR_CALL_ENV_SUBSECTION("profile", NULL, CALL_ENV_FLAG_NONE, ((call_env_parser_t[]) { { FR_CALL_ENV_OFFSET("filter", FR_TYPE_STRING, CALL_ENV_FLAG_CONCAT, ldap_xlat_profile_call_env_t, profile_filter), - .pair.dflt = "(&)", .pair.dflt_quote = T_SINGLE_QUOTED_STRING }, //!< Correct filter for when the DN is known. + .pair.dflt = "(&)", .pair.dflt_quote = T_SINGLE_QUOTED_STRING, + LDAP_FILTER_CALL_ENV_ESCAPE }, //!< Correct filter for when the DN is known. CALL_ENV_TERMINATOR })) }, CALL_ENV_TERMINATOR @@ -398,9 +411,10 @@ static size_t ldap_uri_scheme_table_len = NUM_ELEMENTS(ldap_uri_scheme_table); /** This is the common function that actually ends up doing all the URI escaping */ -#define LDAP_URI_SAFE_FOR (fr_value_box_safe_for_t)fr_ldap_uri_escape_func +#define LDAP_DN_SAFE_FOR (fr_value_box_safe_for_t)fr_ldap_dn_escape_func +#define LDAP_FILTER_SAFE_FOR (fr_value_box_safe_for_t)fr_ldap_filter_escape_func -static xlat_arg_parser_t const ldap_uri_escape_xlat_arg[] = { +static xlat_arg_parser_t const ldap_escape_xlat_arg[] = { { .required=true, .type = FR_TYPE_STRING }, XLAT_ARG_PARSER_TERMINATOR }; @@ -410,11 +424,11 @@ static xlat_arg_parser_t const ldap_safe_xlat_arg[] = { XLAT_ARG_PARSER_TERMINATOR }; -/** Escape LDAP string +/** Escape a string for use in an RFC 4514 DN attribute value * * @ingroup xlat_functions */ -static xlat_action_t ldap_uri_escape_xlat(TALLOC_CTX *ctx, fr_dcursor_t *out, +static xlat_action_t ldap_dn_escape_xlat(TALLOC_CTX *ctx, fr_dcursor_t *out, UNUSED xlat_ctx_t const *xctx, request_t *request, fr_value_box_list_t *in) { @@ -426,34 +440,61 @@ static xlat_action_t ldap_uri_escape_xlat(TALLOC_CTX *ctx, fr_dcursor_t *out, fr_assert(in_group->type == FR_TYPE_GROUP); while ((in_vb = fr_value_box_list_pop_head(&in_group->vb_group))) { - /* - * If it's already safe, just move it over. - */ - if (fr_value_box_is_safe_for_only(in_vb, LDAP_URI_SAFE_FOR)) { + if (fr_value_box_is_safe_for_only(in_vb, LDAP_DN_SAFE_FOR)) { fr_dcursor_append(out, in_vb); continue; } 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->vb_length * 3, in_vb->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_uri_escape_func(request, fr_sbuff_buff(&sbuff), in_vb->vb_length * 3 + 1, in_vb->vb_strvalue, NULL); + len = fr_ldap_dn_escape_func(request, fr_sbuff_buff(&sbuff), in_vb->vb_length * 3 + 1, in_vb->vb_strvalue, NULL); + + fr_sbuff_trim_talloc(&sbuff, len); + fr_value_box_strdup_shallow(vb, NULL, fr_sbuff_buff(&sbuff), in_vb->tainted); + talloc_free(in_vb); + + fr_dcursor_append(out, vb); + } + return XLAT_ACTION_DONE; +} + +/** Escape a string for use as an RFC 4515 filter assertion value + * + * @ingroup xlat_functions + */ +static xlat_action_t ldap_filter_escape_xlat(TALLOC_CTX *ctx, fr_dcursor_t *out, + UNUSED xlat_ctx_t const *xctx, + request_t *request, fr_value_box_list_t *in) +{ + fr_value_box_t *vb, *in_vb, *in_group = fr_value_box_list_head(in); + fr_sbuff_t sbuff; + fr_sbuff_uctx_talloc_t sbuff_ctx; + size_t len; + + fr_assert(in_group->type == FR_TYPE_GROUP); + + while ((in_vb = fr_value_box_list_pop_head(&in_group->vb_group))) { + if (fr_value_box_is_safe_for_only(in_vb, LDAP_FILTER_SAFE_FOR)) { + fr_dcursor_append(out, in_vb); + continue; + } + + MEM(vb = fr_value_box_alloc_null(ctx)); + + if (!fr_sbuff_init_talloc(vb, &sbuff, &sbuff_ctx, in_vb->vb_length * 3, in_vb->vb_length * 3)) { + REDEBUG("Failed to allocate buffer for escaped string"); + talloc_free(vb); + return XLAT_ACTION_FAIL; + } + + len = fr_ldap_filter_escape_func(request, fr_sbuff_buff(&sbuff), in_vb->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); talloc_free(in_vb); @@ -532,7 +573,7 @@ static int ldap_uri_part_escape(fr_value_box_t *vb, UNUSED void *uctx) /* * Call the escape function, including the space for the trailing NULL */ - len = fr_ldap_uri_escape_func(NULL, fr_sbuff_buff(&sbuff), vb->vb_length * 3 + 1, vb->vb_strvalue, NULL); + len = fr_ldap_dn_escape_func(NULL, fr_sbuff_buff(&sbuff), vb->vb_length * 3 + 1, vb->vb_strvalue, NULL); fr_sbuff_trim_talloc(&sbuff, len); fr_value_box_strdup_shallow_replace(vb, fr_sbuff_buff(&sbuff), len); @@ -723,24 +764,24 @@ static void ldap_xlat_signal(xlat_ctx_t const *xctx, request_t *request, UNUSED * This is equivalent to the old "tainted_allowed" flag. */ static fr_uri_part_t const ldap_uri_parts[] = { - { .name = "scheme", .safe_for = LDAP_URI_SAFE_FOR, .terminals = &FR_SBUFF_TERMS(L(":")), .part_adv = { [':'] = 1 }, .extra_skip = 2 }, - { .name = "host", .safe_for = LDAP_URI_SAFE_FOR, .terminals = &FR_SBUFF_TERMS(L(":"), L("/")), .part_adv = { [':'] = 1, ['/'] = 2 } }, - { .name = "port", .safe_for = LDAP_URI_SAFE_FOR, .terminals = &FR_SBUFF_TERMS(L("/")), .part_adv = { ['/'] = 1 } }, - { .name = "dn", .safe_for = LDAP_URI_SAFE_FOR, .terminals = &FR_SBUFF_TERMS(L("?")), .part_adv = { ['?'] = 1 }, .func = ldap_uri_part_escape }, - { .name = "attrs", .safe_for = LDAP_URI_SAFE_FOR, .terminals = &FR_SBUFF_TERMS(L("?")), .part_adv = { ['?'] = 1 }}, - { .name = "scope", .safe_for = LDAP_URI_SAFE_FOR, .terminals = &FR_SBUFF_TERMS(L("?")), .part_adv = { ['?'] = 1 }, .func = ldap_uri_part_escape }, - { .name = "filter", .safe_for = LDAP_URI_SAFE_FOR, .terminals = &FR_SBUFF_TERMS(L("?")), .part_adv = { ['?'] = 1}, .func = ldap_uri_part_escape }, - { .name = "exts", .safe_for = LDAP_URI_SAFE_FOR, .func = ldap_uri_part_escape }, + { .name = "scheme", .safe_for = LDAP_DN_SAFE_FOR, .terminals = &FR_SBUFF_TERMS(L(":")), .part_adv = { [':'] = 1 }, .extra_skip = 2 }, + { .name = "host", .safe_for = LDAP_DN_SAFE_FOR, .terminals = &FR_SBUFF_TERMS(L(":"), L("/")), .part_adv = { [':'] = 1, ['/'] = 2 } }, + { .name = "port", .safe_for = LDAP_DN_SAFE_FOR, .terminals = &FR_SBUFF_TERMS(L("/")), .part_adv = { ['/'] = 1 } }, + { .name = "dn", .safe_for = LDAP_DN_SAFE_FOR, .terminals = &FR_SBUFF_TERMS(L("?")), .part_adv = { ['?'] = 1 }, .func = ldap_uri_part_escape }, + { .name = "attrs", .safe_for = LDAP_DN_SAFE_FOR, .terminals = &FR_SBUFF_TERMS(L("?")), .part_adv = { ['?'] = 1 }}, + { .name = "scope", .safe_for = LDAP_DN_SAFE_FOR, .terminals = &FR_SBUFF_TERMS(L("?")), .part_adv = { ['?'] = 1 }, .func = ldap_uri_part_escape }, + { .name = "filter", .safe_for = LDAP_DN_SAFE_FOR, .terminals = &FR_SBUFF_TERMS(L("?")), .part_adv = { ['?'] = 1}, .func = ldap_uri_part_escape }, + { .name = "exts", .safe_for = LDAP_DN_SAFE_FOR, .func = ldap_uri_part_escape }, XLAT_URI_PART_TERMINATOR }; static fr_uri_part_t const ldap_dn_parts[] = { - { .name = "dn", .safe_for = LDAP_URI_SAFE_FOR , .func = ldap_uri_part_escape }, + { .name = "dn", .safe_for = LDAP_DN_SAFE_FOR , .func = ldap_uri_part_escape }, XLAT_URI_PART_TERMINATOR }; static xlat_arg_parser_t const ldap_xlat_arg[] = { - { .required = true, .type = FR_TYPE_STRING, .safe_for = LDAP_URI_SAFE_FOR, .will_escape = true, }, + { .required = true, .type = FR_TYPE_STRING, .safe_for = LDAP_DN_SAFE_FOR, .will_escape = true, }, XLAT_ARG_PARSER_TERMINATOR }; @@ -1037,7 +1078,7 @@ static xlat_action_t ldap_group_xlat_resume(TALLOC_CTX *ctx, fr_dcursor_t *out, } static xlat_arg_parser_t const ldap_group_xlat_arg[] = { - { .required = true, .concat = true, .type = FR_TYPE_STRING, .safe_for = LDAP_URI_SAFE_FOR }, + { .required = true, .concat = true, .type = FR_TYPE_STRING, .safe_for = LDAP_DN_SAFE_FOR }, XLAT_ARG_PARSER_TERMINATOR }; @@ -2869,7 +2910,7 @@ static int mod_bootstrap(module_inst_ctx_t const *mctx) xlat_func_args_set(xlat, ldap_xlat_arg); xlat_func_call_env_set(xlat, &xlat_profile_method_env); - map_proc_register(mctx->mi->boot, inst, mctx->mi->name, mod_map_proc, ldap_map_verify, 0, LDAP_URI_SAFE_FOR); + map_proc_register(mctx->mi->boot, inst, mctx->mi->name, mod_map_proc, ldap_map_verify, 0, LDAP_DN_SAFE_FOR); return 0; } @@ -2878,17 +2919,31 @@ static int mod_load(void) { xlat_t *xlat; - if (unlikely(!(xlat = xlat_func_register(NULL, "ldap.uri.escape", ldap_uri_escape_xlat, FR_TYPE_STRING)))) return -1; - xlat_func_args_set(xlat, ldap_uri_escape_xlat_arg); + if (unlikely(!(xlat = xlat_func_register(NULL, "ldap.dn.escape", ldap_dn_escape_xlat, FR_TYPE_STRING)))) return -1; + xlat_func_args_set(xlat, ldap_escape_xlat_arg); xlat_func_flags_set(xlat, XLAT_FUNC_FLAG_PURE); - xlat_func_safe_for_set(xlat, LDAP_URI_SAFE_FOR); /* Used for all LDAP escaping */ + xlat_func_safe_for_set(xlat, LDAP_DN_SAFE_FOR); - if (unlikely(!(xlat = xlat_func_register(NULL, "ldap.uri.safe", xlat_transparent, FR_TYPE_STRING)))) return -1; + if (unlikely(!(xlat = xlat_func_register(NULL, "ldap.dn.safe", xlat_transparent, FR_TYPE_STRING)))) return -1; xlat_func_args_set(xlat, ldap_safe_xlat_arg); xlat_func_flags_set(xlat, XLAT_FUNC_FLAG_PURE); - xlat_func_safe_for_set(xlat, LDAP_URI_SAFE_FOR); + xlat_func_safe_for_set(xlat, LDAP_DN_SAFE_FOR); - if (unlikely(!(xlat = xlat_func_register(NULL, "ldap.uri.unescape", ldap_uri_unescape_xlat, FR_TYPE_STRING)))) return -1; + if (unlikely(!(xlat = xlat_func_register(NULL, "ldap.dn.unescape", ldap_uri_unescape_xlat, FR_TYPE_STRING)))) return -1; + xlat_func_args_set(xlat, ldap_uri_unescape_xlat_arg); + xlat_func_flags_set(xlat, XLAT_FUNC_FLAG_PURE); + + if (unlikely(!(xlat = xlat_func_register(NULL, "ldap.filter.escape", ldap_filter_escape_xlat, FR_TYPE_STRING)))) return -1; + xlat_func_args_set(xlat, ldap_escape_xlat_arg); + xlat_func_flags_set(xlat, XLAT_FUNC_FLAG_PURE); + xlat_func_safe_for_set(xlat, LDAP_FILTER_SAFE_FOR); + + if (unlikely(!(xlat = xlat_func_register(NULL, "ldap.filter.safe", xlat_transparent, FR_TYPE_STRING)))) return -1; + xlat_func_args_set(xlat, ldap_safe_xlat_arg); + xlat_func_flags_set(xlat, XLAT_FUNC_FLAG_PURE); + xlat_func_safe_for_set(xlat, LDAP_FILTER_SAFE_FOR); + + if (unlikely(!(xlat = xlat_func_register(NULL, "ldap.filter.unescape", ldap_uri_unescape_xlat, FR_TYPE_STRING)))) return -1; xlat_func_args_set(xlat, ldap_uri_unescape_xlat_arg); xlat_func_flags_set(xlat, XLAT_FUNC_FLAG_PURE); @@ -2896,11 +2951,36 @@ static int mod_load(void) xlat_func_args_set(xlat, ldap_uri_attr_option_xlat_arg); xlat_func_flags_set(xlat, XLAT_FUNC_FLAG_PURE); + /* + * ldap.uri.* are kept as aliases for ldap.dn.* so that existing configs + * continue to work. They use the same safe_for token for now; if the URI + * context ever needs its own rules, a separate token can be introduced. + */ + if (unlikely(!(xlat = xlat_func_register(NULL, "ldap.uri.escape", ldap_dn_escape_xlat, FR_TYPE_STRING)))) return -1; + xlat_func_args_set(xlat, ldap_escape_xlat_arg); + xlat_func_flags_set(xlat, XLAT_FUNC_FLAG_PURE); + xlat_func_safe_for_set(xlat, LDAP_DN_SAFE_FOR); + + if (unlikely(!(xlat = xlat_func_register(NULL, "ldap.uri.safe", xlat_transparent, FR_TYPE_STRING)))) return -1; + xlat_func_args_set(xlat, ldap_safe_xlat_arg); + xlat_func_flags_set(xlat, XLAT_FUNC_FLAG_PURE); + xlat_func_safe_for_set(xlat, LDAP_DN_SAFE_FOR); + + if (unlikely(!(xlat = xlat_func_register(NULL, "ldap.uri.unescape", ldap_uri_unescape_xlat, FR_TYPE_STRING)))) return -1; + xlat_func_args_set(xlat, ldap_uri_unescape_xlat_arg); + xlat_func_flags_set(xlat, XLAT_FUNC_FLAG_PURE); + return 0; } static void mod_unload(void) { + xlat_func_unregister("ldap.dn.escape"); + xlat_func_unregister("ldap.dn.safe"); + xlat_func_unregister("ldap.dn.unescape"); + xlat_func_unregister("ldap.filter.escape"); + xlat_func_unregister("ldap.filter.safe"); + xlat_func_unregister("ldap.filter.unescape"); xlat_func_unregister("ldap.uri.escape"); xlat_func_unregister("ldap.uri.safe"); xlat_func_unregister("ldap.uri.unescape"); diff --git a/src/tests/ldap_sync/rfc4533/config/radiusd.conf b/src/tests/ldap_sync/rfc4533/config/radiusd.conf index a4f18af274f..68dbbf05bef 100644 --- a/src/tests/ldap_sync/rfc4533/config/radiusd.conf +++ b/src/tests/ldap_sync/rfc4533/config/radiusd.conf @@ -202,7 +202,7 @@ server test { load Cookie { string csn - csn := %str.concat(%ldap("ldap:///%ldap.uri.safe(%{LDAP-Sync.Directory-Root-DN})?contextCSN?base"), ';') + csn := %str.concat(%ldap("ldap:///%ldap.dn.safe(%{LDAP-Sync.Directory-Root-DN})?contextCSN?base"), ';') reply.LDAP-Sync.Cookie := "rid=000,csn=%{csn}" } diff --git a/src/tests/modules/ldap/dynamic_dn.unlang b/src/tests/modules/ldap/dynamic_dn.unlang index 82e63f7619d..8aa5ff61049 100644 --- a/src/tests/modules/ldap/dynamic_dn.unlang +++ b/src/tests/modules/ldap/dynamic_dn.unlang @@ -1,14 +1,14 @@ string base_dn -base_dn=%ldap.uri.safe('dc=example,dc=com') +base_dn=%ldap.dn.safe('dc=example,dc=com') ldap_dynamic_dn if (!ok) { test_fail } # Bad DN -base_dn := %ldap.uri.safe('dc=example,dc=foo,dc=com') +base_dn := %ldap.dn.safe('dc=example,dc=foo,dc=com') ldap_dynamic_dn if (!notfound) { test_fail diff --git a/src/tests/modules/ldap/filter_injection.attrs b/src/tests/modules/ldap/filter_injection.attrs new file mode 100644 index 00000000000..92e148fdd7d --- /dev/null +++ b/src/tests/modules/ldap/filter_injection.attrs @@ -0,0 +1,14 @@ +# +# Input packet - User-Name is a wildcard filter injection payload. +# Without call_env escaping on user.filter, (uid=*) matches every user +# in the directory. With escaping it becomes (uid=\2a) and matches nobody. +# +Packet-Type = Access-Request +User-Name = "*" +User-Password = "password" +NAS-IP-Address = 1.2.3.5 + +# +# Expected answer +# +Packet-Type == Access-Accept diff --git a/src/tests/modules/ldap/filter_injection.unlang b/src/tests/modules/ldap/filter_injection.unlang new file mode 100644 index 00000000000..c8d362ff285 --- /dev/null +++ b/src/tests/modules/ldap/filter_injection.unlang @@ -0,0 +1,13 @@ +# +# Verify that user.filter call_env escaping prevents LDAP filter injection. +# +# User-Name = "*" would produce (uid=*) without escaping, matching every +# user in the directory. With LDAP_FILTER_CALL_ENV_ESCAPE applied to +# user.filter, the wildcard is escaped to \2a and the search returns nothing. +# +ldap +if (!notfound) { + test_fail +} + +test_pass diff --git a/src/tests/modules/ldap/profile_injection.attrs b/src/tests/modules/ldap/profile_injection.attrs new file mode 100644 index 00000000000..d319290b6c4 --- /dev/null +++ b/src/tests/modules/ldap/profile_injection.attrs @@ -0,0 +1,17 @@ +# +# Input packet - User-Profile contains a DN injection payload. +# The payload attempts to apply the "suspended" profile by injecting +# filter characters into the profile DN. With DN escaping on the +# default_profile call_env, the crafted DN is never found. +# +Packet-Type = Access-Request +User-Name = "john" +User-Password = "password" +NAS-IP-Address = 1.2.3.5 + +# +# Expected answer - john is still authorised via the default (radprofile) profile, +# the injected profile reference simply returns notfound. +# +Packet-Type == Access-Accept +Idle-Timeout == 3600 diff --git a/src/tests/modules/ldap/profile_injection.unlang b/src/tests/modules/ldap/profile_injection.unlang new file mode 100644 index 00000000000..1fb23667a7b --- /dev/null +++ b/src/tests/modules/ldap/profile_injection.unlang @@ -0,0 +1,28 @@ +# +# Verify that User-Profile DN injection is blocked by call_env escaping. +# +# Set a crafted User-Profile value before running ldap. Without DN escaping +# on the profile lookup, filter-special chars in a DN reference can be used +# to manipulate the search. With LDAP_DN_CALL_ENV_ESCAPE the crafted DN is +# escaped and simply returns notfound, leaving the user authorised via the +# default (radprofile) profile only. +# +control.User-Profile := 'cn=suspended)(|(cn=*,ou=profiles,dc=example,dc=com' + +ldap + +if (!(ok || updated)) { + test_fail +} + +# radprofile should still have been applied. +if (!(reply.Idle-Timeout == 3600)) { + test_fail +} + +# The injected profile must not have been applied. +if (reply.Reply-Message == 'User-Suspended') { + test_fail +} + +test_pass diff --git a/src/tests/modules/ldap/xlat.unlang b/src/tests/modules/ldap/xlat.unlang index 1506aa90f7b..1a8d9b8db60 100644 --- a/src/tests/modules/ldap/xlat.unlang +++ b/src/tests/modules/ldap/xlat.unlang @@ -8,13 +8,13 @@ string result_string test_string := "safe string" # String with no escaping -result_string := %ldap.uri.escape(%{test_string}) +result_string := %ldap.dn.escape(%{test_string}) if (!(result_string == "safe string")) { test_fail } -result_string := %ldap.uri.unescape(%{result_string}) +result_string := %ldap.dn.unescape(%{result_string}) if (!(result_string == 'safe string')) { test_fail @@ -22,13 +22,13 @@ if (!(result_string == 'safe string')) { # String with some characters to escape test_string := 'non safe,+"\<>;*=() string' -result_string := %ldap.uri.escape(%{test_string}) +result_string := %ldap.dn.escape(%{test_string}) if (!(result_string == 'non safe\2c\2b\22\5c\3c\3e\3b\2a\3d\28\29 string')) { test_fail } -result_string := %ldap.uri.unescape(%{result_string}) +result_string := %ldap.dn.unescape(%{result_string}) if (!(result_string == 'non safe,+"\<>;*=() string')) { test_fail @@ -36,7 +36,7 @@ if (!(result_string == 'non safe,+"\<>;*=() string')) { # String where all characters require escaping test_string := ',+"\<>;*=()' -result_string := %ldap.uri.escape(%{test_string}) +result_string := %ldap.dn.escape(%{test_string}) if (!(result_string == '\2c\2b\22\5c\3c\3e\3b\2a\3d\28\29')) { test_fail @@ -47,22 +47,78 @@ control += { Filter-Id = 'safe' Filter-Id = 'non safe,+' } -control.Filter-Id := %ldap.uri.escape(control.Filter-Id[*]) +control.Filter-Id := %ldap.dn.escape(control.Filter-Id[*]) if ((control.Filter-Id[0] != 'safe') || (control.Filter-Id[1] != 'non safe\2c\2b')) { test_fail } -control.Filter-Id := %ldap.uri.unescape(control.Filter-Id[*]) +control.Filter-Id := %ldap.dn.unescape(control.Filter-Id[*]) if ((control.Filter-Id[0] != 'safe') || (control.Filter-Id[1] != 'non safe,+')) { test_fail } -result_string := %ldap.uri.unescape(%{result_string}) +result_string := %ldap.dn.unescape(%{result_string}) if (!(result_string == ',+"\<>;*=()')) { test_fail } +# Filter escape: only *()\ are escaped; , + = and others are left as-is +test_string := '*()\,+=' +result_string := %ldap.filter.escape(%{test_string}) +if (!(result_string == '\2a\28\29\5c,+=')) { + test_fail +} + +result_string := %ldap.filter.unescape(%{result_string}) +if (!(result_string == '*()\,+=')) { + test_fail +} + +# +# Injection scenario tests: verify that dn.escape and filter.escape +# neutralize attack payloads differently, reflecting the distinct RFC rules. +# + +# DN injection: a crafted value containing , and = would add extra DN components +# if left unescaped. dn.escape converts them to \HH. +test_string := 'john,ou=evil,dc=attacker,dc=com' +result_string := %ldap.dn.escape(%{test_string}) +if (!(result_string == 'john\2cou\3devil\2cdc\3dattacker\2cdc\3dcom')) { + test_fail +} + +# filter.escape leaves , and = alone (OpenLDAP does not decode non-required \HH +# sequences, so escaping = as \3d would cause silent match failures). +result_string := %ldap.filter.escape(%{test_string}) +if (!(result_string == 'john,ou=evil,dc=attacker,dc=com')) { + test_fail +} + +# Filter injection: parens and asterisk must be escaped to prevent filter manipulation. +# Pipe and equals are NOT in filter_specials. +test_string := 'john)(|(uid=*)' +result_string := %ldap.filter.escape(%{test_string}) +if (!(result_string == 'john\29\28|\28uid=\2a\29')) { + test_fail +} + +# The same payload through dn.escape also escapes = (to \3d). +result_string := %ldap.dn.escape(%{test_string}) +if (!(result_string == 'john\29\28|\28uid\3d\2a\29')) { + test_fail +} + +# Wildcard injection via the %ldap() URI xlat: User-Name='*' becomes (uid=*) +# without escaping, matching every user. Wrapping with filter.escape makes it +# (uid=\2a) which matches nobody. +User-Name := '*' +result_string := %ldap("ldap://$ENV{LDAP_TEST_SERVER}:$ENV{LDAP_TEST_SERVER_PORT}/ou=people,dc=example,dc=com?displayName?sub?(uid=%ldap.filter.escape(%{User-Name}))") +if (result_string) { + test_fail +} +User-Name := 'john' + result_string := %ldap("ldap://$ENV{LDAP_TEST_SERVER}:$ENV{LDAP_TEST_SERVER_PORT}/ou=people,dc=example,dc=com?displayName?sub?(uid=john)") if (!(result_string == "John Doe")) { @@ -105,7 +161,7 @@ if (!(result_string == "Fred Jones")) { } # Reference an alternative LDAP server in the xlat -result_string := %ldap("ldap://$ENV{LDAP_TEST_SERVER}:%ldap.uri.escape(%{$ENV{LDAP_TEST_SERVER_PORT} + 1})/dc=subdept,dc=example,dc=com?displayName?sub?(uid=fred)") +result_string := %ldap("ldap://$ENV{LDAP_TEST_SERVER}:%ldap.dn.escape(%{$ENV{LDAP_TEST_SERVER_PORT} + 1})/dc=subdept,dc=example,dc=com?displayName?sub?(uid=fred)") if (!(result_string == "Fred Jones")) { test_fail diff --git a/src/tests/modules/ldap/xlat_profile.unlang b/src/tests/modules/ldap/xlat_profile.unlang index b56c5f14a5d..91823b8beca 100644 --- a/src/tests/modules/ldap/xlat_profile.unlang +++ b/src/tests/modules/ldap/xlat_profile.unlang @@ -45,6 +45,23 @@ group { control := {} reply := {} + + # Profile DN injection: injection chars in %{user} are DN-escaped before use. + # Without escaping, 'suspended)(|(cn=*' would be injected into the filter, + # matching all profiles. With DN escaping the crafted DN simply doesn't exist. + user := 'suspended)(|(cn=*' + + if (%ldap.profile("ldap:///cn=%{user},ou=profiles,dc=example,dc=com")) { + test_fail + } + + # Should be notfound - injected DN doesn't exist in the directory. + if (!notfound) { + test_fail + } + + control := {} + reply := {} } if (!%ldap.profile('cn=profile3,ou=profiles,dc=example,dc=com')) {