unlock unlimited code runs for $79, limited time →
NOSSLETTER.techLIVE
back to OSSpedia
rabbitmq/rabbitmq-server
mid8/10
securityerror-handlingtesting-strategy

LDAP: escape DN template values, not full bind values (backport #17278)

Project: rabbitmq/rabbitmq-server | Language: Erlang | Concepts: security, error-handling, testing-strategy | Difficulty: mid
Link: https://github.com/rabbitmq/rabbitmq-server/pull/17279

The problem

The LDAP authentication backend was escaping entire bind values according to RFC 4514, which prevented certain path traversal attacks but also broke legitimate Active Directory (AD) authentication scenarios. In particular, user-supplied DNs or non-DN patterns (like UPNs or down-level logon names) were being over-escaped, causing failed logins and incompatibility with common AD setups.

Why it's non-trivial

A naive fix would simply remove or relax escaping, but this would reintroduce security vulnerabilities (e.g., DN injection). The challenge is to apply escaping only where it's strictly needed (i.e., when interpolating into a DN template), and to avoid corrupting user-supplied DNs or non-DN bind patterns. The solution must also handle edge cases like AD down-level names (DOMAIN\user), and maintain compatibility with a variety of LDAP/AD environments.

The approach

1. Only escape values when interpolating into DN templates

Previously, escaping was applied broadly. The new approach distinguishes between DN patterns (which contain attr=value pairs) and non-DN patterns, and only escapes when necessary.

fill_bind_dn(Pattern, Username) ->
    case is_dn_pattern(Pattern) of
        true  -> fill_dn_pattern(Pattern, Username, []);
        false -> fill_raw(Pattern, Username)
    end.

Annotation:
This function checks if the pattern is a DN (contains =). If so, it escapes the value; otherwise, it fills the pattern raw, avoiding unnecessary escaping for non-DN patterns.

2. Correctly identify DN patterns

is_dn_pattern(Pattern) ->
    lists:member($=, rabbit_data_coercion:to_list(Pattern)).

Annotation:
A DN pattern is identified by the presence of an equals sign (=), which is standard in LDAP DNs (e.g., cn=username,...). This check ensures escaping is only applied to DN templates.

3. Properly fill and escape DN templates

fill_dn_pattern(Pattern, Username, OtherArgs) ->
    ADArgs = safe_ad_args(
               rabbit_auth_backend_ldap_util:get_active_directory_args(Username)),
    fill_dn(Pattern, [{username, Username}] ++ ADArgs ++ OtherArgs).

Annotation:
When filling a DN pattern, the username and any AD-specific arguments are inserted, and escaping is applied only to the relevant template values.

4. Remove over-escaping for non-DN patterns

fill_raw(Pattern, Username) ->
    ADArgs = rabbit_auth_backend_ldap_util:get_active_directory_args(Username),
    fill(Pattern, [{username, Username}] ++ ADArgs).

Annotation:
For non-DN patterns (like UPNs or raw DNs), the username is inserted without escaping, preserving the intended value.

5. Extensive test coverage for all cases

New and updated tests ensure that:

  • DN templates escape values correctly.
  • Non-DN patterns (including raw DNs, UPNs, and down-level logon names) are not corrupted.
  • Edge cases (special characters, unicode, empty values) are handled safely.

Example test for non-DN pattern (no escaping):

?assertEqual("O'Brien,Sean@example.com",
             rabbit_auth_backend_ldap:escaped_user_dn(<<"O'Brien,Sean">>)),

Example test for DN pattern (escaping applied):

?assertEqual("cn=foo\\\\bar,ou=People,dc=example,dc=com",
             rabbit_auth_backend_ldap:escaped_user_dn(<<"foo\\bar">>)),

From the review thread

michaelklishin: I was forced to investigate and fix a bunch of unrelated test flakes on v4.3.x, which culminated in a drive-by update to this PR and a separate one https://github.com/rabbitmq/rabbitmq-server/pull/17280 for main.

michaelklishin: @chandrimad008 if that would help, you can get an alpha build of 4.3.6 from a separate repo, and give it a shot in an actual AD, LDAP environment.
We'd really appreciate the early feedback.

Takeaways

  • Apply escaping only where it is semantically necessary: Escaping should be context-aware—applied to template values interpolated into DNs, but not to entire bind values or user-supplied DNs.
  • Use pattern recognition to guide security logic: Simple checks (like the presence of =) can distinguish between DNs and non-DN patterns, enabling safer and more flexible handling.
  • Comprehensive tests are essential for security fixes: Edge cases (special characters, unicode, empty strings) and real-world scenarios (AD logon names, UPNs) must be covered to avoid regressions and ensure compatibility.