[Samba] [PATCH] Re: Samba 4.1.17 classic update w/LDAP - parsing error

[Rowland Penny]

On 23/09/15 20:42, Andrew Bartlett wrote:
> On Thu, 2015-09-24 at 06:59 +1200, Andrew Bartlett wrote:
>> On Wed, 2015-09-23 at 10:42 +0200, Mgr. Peter Tuharsky wrote:
>>> Hallo,
>>>
>>> I'm trying Samba 4.1.17 (Debian Jessie packages) classic update
>>> process
>>> with previous LDAP backend. The process goes normal, occasional
>>> "group
>>> already exists" etc.
>>>
>>> But it ends up with an error:
>>>
>>> ERROR(<type 'exceptions.ValueError'>): uncaught exception - unable
>>> to
>>> parse dn string
>>>    File "/usr/lib/python2.7/dist-packages/samba/netcmd/__init__.py",
>>> line
>>> 175, in _run
>>>      return self.run(*args, **kwargs)
>>>    File "/usr/lib/python2.7/dist-packages/samba/netcmd/domain.py",
>>> line
>>> 1321, in run
>>>      useeadb=eadb, dns_backend=dns_backend, use_ntvfs=use_ntvfs)
>>>    File "/usr/lib/python2.7/dist-packages/samba/upgrade.py", line
>>> 903,
>>> in
>>> upgrade_from_samba3
>>>      add_group_from_mapping_entry(result.samdb, g, logger)
>>>    File "/usr/lib/python2.7/dist-packages/samba/upgrade.py", line
>>> 274,
>>> in
>>> add_group_from_mapping_entry
>>>      m.dn = ldb.Dn(samdb, "CN=%s,CN=Users,%s" % (groupmap.nt_name,
>>> samdb.get_default_basedn()))
>>>
>>>
>>> Does someone have a clue, please?
>> That looks like a bug.  My guess is that, as Roland suggested, the
>> group name isn't just normal characters.  We do support other chars
>> in
>> group names, but the bug here was not to escape the values.  You
>> could
>> expect a particular problem with any of these in particular: =,()
> Can you confirm this patch (against master, but should apply back to
> 4.1) works for you?
>
> If so, can I get a second team member to review/push?
>
> Thanks,
>
> Andrew Bartlett
>
>
>

Hi Andre, can I say as a mere user, I don't think you should do that. If 
you look here:

https://technet.microsoft.com/en-us/library/cc783256%28v=ws.10%29.aspx

You will find this:

A group name (pre–Windows 2000) (samAccountName object attribute) can 
contain up to 256 uppercase of lowercase characters except for the 
following:

" / \ [ ] : ; | = , + * ? <>

and as the next line down in upgrade.py is this:

         m['sAMAccountName'] = ldb.MessageElement(groupmap.nt_name, 
ldb.FLAG_MOD_ADD,
             'sAMAccountName')

It would seem that by allowing ',' & '=' in the 'CN' you are also going 
to allow it in the 'sAMAccountName', where according to microsoft it 
isn't allowed.

Rowland