Bug 14073 - ? characters in DN's are wrongly escaped
Summary: ? characters in DN's are wrongly escaped
Status: NEW
Alias: None
Product: Samba 4.1 and newer
Classification: Unclassified
Component: AD: LDB/DSDB/SAMDB (show other bugs)
Version: 4.13.3
Hardware: All All
: P5 normal (vote)
Target Milestone: ---
Assignee: Andrew Bartlett
QA Contact: Samba QA Contact
URL: https://tools.ietf.org/html/rfc4514
Depends on:
Reported: 2019-08-05 13:09 UTC by Florian Best
Modified: 2020-12-30 09:32 UTC (History)
2 users (show)

See Also:

possible patch (734 bytes, patch)
2020-07-07 21:53 UTC, Douglas Bagnall
no flags Details
additional patch fixing ';' and '=' (1.02 KB, patch)
2020-07-07 22:10 UTC, Douglas Bagnall
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Florian Best 2019-08-05 13:09:57 UTC
The Samba LDAP server escapes "?" characters in the DN in search results.

According to RFC 4514 the "?" is not a character meant to be escaped:

String Representation of Distinguished Names: Converting an AttributeValue from ASN.1 to a String:

Therefor e.g. the openldap tools ldap_bv2dn() and ldap_str2dn() fail to parse the DN (DECODING_ERROR).
This makes these objects unusable with tools like e.g. python-ldap.

To reproduce this:
1. Create container "foo?bar"
> $ ldbedit -H /var/lib/samba/private/sam.ldb
> dn: CN=foo?bar,DC=school,DC=dev
> objectClass: container
2. Search for container:
> $ ldbsearch --debug-stderr -H ldaps://localhost -Umaster100$%password 'CN=foo*bar'
> # record 1
> dn: CN=foo\?bar,DC=school,DC=dev
> objectClass: top
> objectClass: container
> cn: foo?bar
> instanceType: 4
> whenCreated: 20190801201812.0Z
> whenChanged: 20190801201812.0Z
> uSNCreated: 40925
> uSNChanged: 40925
> showInAdvancedViewOnly: TRUE
> name: foo?bar
> objectGUID: 29087ce0-cae5-43ba-bf27-3a8bd3c59ea0
> objectCategory: CN=Container,CN=Schema,CN=Configuration,DC=school,DC=dev
> distinguishedName: CN=foo\?bar,DC=school,DC=dev

See the printed DN, which contains "\?".
Comment 1 Björn Jacke 2020-05-22 15:53:30 UTC
indeed. If you add a "?" to a user's DN, then MMC also start to show funny effects and crashes.
Comment 2 Florian Best 2020-07-06 10:38:22 UTC
(In reply to Björn Jacke from comment #1)
So this is also a security issue.
Comment 3 Douglas Bagnall 2020-07-07 21:53:41 UTC
Created attachment 16119 [details]
possible patch

Are you able to compile with this patch?

Looking at the file, it says we follow the older RFC2253, but my reading is that there is no difference there ("Implementations MAY escape other characters", but in that case "the character to be escaped is replaced by a backslash and two hex digits").

Also, I note we don't escape ';' in the proper way.

We follow MS Active Directory over the RFC, but there appears to be no difference in this case.

Comment 4 Douglas Bagnall 2020-07-07 22:10:47 UTC
Created attachment 16120 [details]
additional patch fixing ';' and '='

An additional patch for '=' and ';'.

The RFCs say these characters { ' ', '"', '#', '+', ',', ';', '<', '=', '>', '\' } must be escaped in the "\c" form.

Other characters can be escaped in the two hex digit "\hh" form, but characters in that list can't be.
Comment 5 Florian Best 2020-07-07 23:13:02 UTC
Thank you for your patches. I built samba with them. Unfortionately the ? character is still escaped.
Comment 6 Douglas Bagnall 2020-07-08 04:25:57 UTC
(In reply to Florian Best from comment #5)

OK. The patches work for me in a testenv.

My guess is if you add a new 'CN=foo?baz,DC=school,DC=dev', it will not be escaped. 

In other words, the escaping is being done on the way in.

The simple fix will not fix existing question marks. For that we would need a repack.
Comment 7 Douglas Bagnall 2020-07-08 05:16:38 UTC
(In reply to Douglas Bagnall from comment #6)
> In other words, the escaping is being done on the way in.

This looks to be the case with v2 ldap packing format, introduced in 4.11.
Comment 8 Björn Jacke 2020-12-29 16:24:54 UTC
Douglas: can you make those patches into a merge request. so that we can get this fixed?
Comment 9 Douglas Bagnall 2020-12-29 23:48:58 UTC
Björn, unfortunately it is way more complicated than that.

We save a canonicalised version of the DN in the database (for fast comparisons), so if we fix the escaping we need to make an upgrade step that goes through and repacks all the objects.

Also we need to separate canonicalName escaping from DN escaping.
(see https://lists.samba.org/archive/cifs-protocol/2020-August/003511.html)
Comment 10 Björn Jacke 2020-12-30 09:32:54 UTC
wouldn't the upcoming 4.14 release be a good chance to get this fixed along with a forced repack?