[PATCH] Failure to modify nTSecurityDescriptor attribute ussing ldb.modify_ldif()

[Zahari Zahariev]

Hello Andrew & Samba4,

I have updated my self to the latest Samba and I verified your work. Your changes indeed work and it now produces a parsing error if you try to use BASE64 value for nTSecurityDescriptor attribute in LDIF.

The new patch I am including is the way I understood Ldb is intelligent enough to read SDDL sting right off from the LDIF. I tested this against Windows 2003 and Samba and it produced different results (nothing unusual so far).

Test results
-------------

1. When I ran this test against Windows 2003 it resulted to this error:

error: Testing self.ldb.modify_ldif() for nTSecurityDescriptor [
Traceback (most recent call last):
  File "lib/ldb/tests/python/ldap.py", line 1064, in test_modify_descriptor
    self.ldb.modify_ldif(mod)
  File "bin/python/samba/__init__.py", line 200, in modify_ldif
    self.modify(msg)
LdbError: (53, 'LDAP error 53 LDAP_UNWILLING_TO_PERFORM -  <00002077: SvcErr: DSID-03150813, problem 5003 (WILL_NOT_PERFORM), data 0\n> <>')
]
  
2. When I ran this test against Samba4 it resulted to this error:

error: Testing self.ldb.modify_ldif() for nTSecurityDescriptor [
Traceback (most recent call last):
  File "lib/ldb/tests/python/ldap.py", line 1067, in test_modify_descriptor
    desc2 = res[0]["nTSecurityDescriptor"][0]
KeyError: 'No such element'
]

Looking further into the first result in "winerror.h" (hex:00002077 is dec:8311 ) so error 8311L is ERROR_DS_ILLEGAL_MOD_OPERATION (Illegal modify operation. Some aspect of the modification is not permitted.). My best guess is that Ldb successfully has parsed the SDDL but has sent some garbage on the wire that AD won't accept.

No less interesting is how Samba handled this. Due to no additional checks like there are in AD, Samba4 accepted the operation silently with NO ERROR what so ever. Real outcome is that nTSecurityDescriptor attribute disappeared from the user object that's why the error is that we cannot find the descriptor after the modify operation.

Best regards,
Zahari

---
Zahari Zahariev
Software Engineer, PPD & Installer team

----- Original Message -----
> From: Andrew Bartlett <abartlet at samba.org>
> To: Zahari Zahariev <zahari.zahariev at postpath.com>
> Cc: samba-technical at lists.samba.org <samba-technical at lists.samba.org>
> Sent: Monday, July 6, 2009 2:54:35 AM GMT+0200 Europe;Athens
> Subject: Re: [PATCH] Failure to modify nTSecurityDescriptor attribute ussing ldb.modify_ldif()

> > On Tue, 2009-06-30 at 15:34 +0300, Zahari Zahariev wrote:
> > Hello Samba4,
> > 
> > Method ldb.modify_ldif() does not work at all if you try to use it 
> for 
> > nTSecurityDescriptor modification.
> 
> The problem is, you must provide the nTSecurityDescriptor as SDDL, not
> as an encoded lump of DNR.
> 
> (The correctness of this behaviour is debatable)
> 
> > The patch below implements a simple unittest for this behavior. 
> First 
> > step is to create a regular user then save its nTSecurityDescriptor 
> in 
> > SDDL format. Next we create a "samba.security.descriptor" python 
> object 
> > which is ndr_packed() and included in ldb.modify_ldif() request 
> changing 
> > our previously created user's descriptor. After this we look up the 
> same 
> > user nTSecurityDescriptor then transform it into SDDL format and 
> > assertNotEqual() both this and the initial value. If 
> ldb.modify_ldif() 
> > operation is successful then the the two SDDL representations must 
> be 
> > different but as this functionality fails in our case they are the 
> same!
> > 
> > Another interesting observation is that ldb.modify_ldif() fails to 
> > change a security descriptor attribute with absolutely no warning or 
> 
> > error in other words if you do not look it up afterwards you would 
> have 
> > no clue that this operation fails.
> 
> This bug has been fixed, and is now in the master tree.  In short, it
> could not tell the difference between invalid LDIF and the end of file,
> 
> so assumed you submitted a NULL op.
> 
> The attached patch helped me debug this.
> 
> Andrew Bartlett
> 
> -- 
> Andrew Bartlett
> http://samba.org/~abartlet/
> Authentication Developer, Samba Team           http://samba.org
> Samba Developer, Cisco Inc.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-Test-that-tries-to-modify-nTSecurityDescriptor-using.patch
Type: text/x-patch
Size: 2657 bytes
Desc: not available
Url : http://lists.samba.org/archive/samba-technical/attachments/20090706/824e4284/0001-Test-that-tries-to-modify-nTSecurityDescriptor-using.bin