Managing DNs in libads only in utf8

[simo]

Hello technical people,

after a report about a possible problem with how we manage DNs,
I discovered we currently may have some problems in case "unix charset"
is not set to UTF-8 and we are using security = ads. *

The problem is that we always convert everything coming out of ldap to
the local unix charset and then we convert** it back utf8 before using
it (see ads_get_dn()).

The problem in doing this is that we convert some DN this way:
utf8 -> local -> utf8

If the local unix charset is not able to represent one of the characters
of the DN, we actually corrupt the DN by doing the double conversion.

This affects primarily lookup_usergroups_member()*** and
lookup_usergroups_memberof() in nsswitch/winbindd_ads.c as they need an
intact DN.

Attached there is a preliminary _untested_ patch that should fix this
problem.
I am posting it in an early stage (I need to fix some assumptions like
user, group and container charset in libads/ldap_user.c) because of some
criticism issued on #samba-technical about doing this change.

As you can see from the patch the change is quite small and should not
impact anything outside functions that directly deal with DNs.
I deliberately ignored any implication about attributes conversion from
utf8 -> unix charset except in a few places where these attributes
contained DNs and for this cases I added a specific
ads_pull_string_utf8() function that skips the conversion.

There are still 2 lines in libgpo/gpo_ldap.c that need fixing as they
use strequal() to compare DNs where we need an utf8 case-insensitive
comparison function, not a unix charset one like strequal()

Attached also a file with the list of functions I have carefully
inspected to make sure the change was not affecting anything else but
DNs (some fns may be missing because so trivial to check I didn't take
their names). The ones marked with X are completely checked, the others
(basically all functions of bin/net that take input from the command
line) I need to decide what to do. My intention is to convert from unix
charset to utf8 all the arguments used to form a DN in these functions.


I'd like comments on this patch, and please keep in mind the scope of it
is to make DNs _only_ always utf8.


Later on I will go again through libads (and eventually smbldap) code
and see if we can push our unix charset vs utf8 barrier at the libads
library boundary instead of at the "wire" level (which actually means at
the ldap libraries boundary, so not really the wire anyway). I will make
a separate proposal for this if I think the task is doable in a safe way
(ie not risking leaking utf8 strings inside the unix charset regions of
samba).

It's very late here, I am sorry if I wrote any stupidity, I am sure you
will forgive me :-)


* See also https://bugzilla.samba.org/show_bug.cgi?id=3675

** I think I spotted a couple of places where this is not true, but more
investigation on this is needed..

*** While ther I also found that escape_ldap_string_alloc() is probably
used incorrectly against DNs or DNs components, but I plan to check that
in a later stage

-- 
Simo Sorce
Samba Team GPL Compliance Officer
email: idra at samba.org
http://samba.org
-------------- next part --------------
A non-text attachment was scrubbed...
Name: samba3_libads_utf8_dns.patch
Type: text/x-patch
Size: 19178 bytes
Desc: not available
Url : http://lists.samba.org/archive/samba-technical/attachments/20070227/2e2d34e1/samba3_libads_utf8_dns.bin
-------------- next part --------------

Input coming from command line:

net_ads_gpo_delete_link
net_ads_gpo_add_link
net_ads_dn
net_ads_user -> ads_user_add
net_ads_user -> ads_user_delete
net_ads_group -> ads_group_add
net_ads_group -> ads_group_delete
net_precreate_machine_acct
net_ads_gpo_get_gpo
net_ads_gpo_get_link

Internal:

X ads_get_dn

X ads_gen_mod
X ads_gen_add
X ads_del_dn

X ads_mod_printer_entry
X ads_add_printer_entry

X ads_search_dn
X ads_do_search
X ads_do_search_all_args
X ads_do_paged_search_args
X ads_do_search_all
X ads_do_search_all_fn
X ads_do_search_retry

X ads_search_retry_extended_dn

X ads_schema_path

X ads_default_ou_string
X ads_ou_string
X ads_user_add
X ads_group_add
X ads_add_user_acct
X ads_add_group_acct

X ads_site_dn
X ads_site_dn_for_machine
X ads_parent_dn

X ads_parse_gpo
X ads_get_gpo
X ads_get_gpo_list
X ads_get_gpo_link
X ads_parse_gplink
X add_gplink_to_gpo_list

X ads_create_machine_acct

X ads->config.bind_path
X gpo->ds_path
X gp_link->link_names


TODO: strequal for gpo DN comparisons