[PATCH] Improve provision acl handling

[Andrew Bartlett]

On Mon, 2012-11-05 at 21:25 +0100, Jelmer Vernooij wrote:
> On Mon, Nov 05, 2012 at 10:08:12PM +1100, Andrew Bartlett wrote:
> > This patch avoids going to and from a string in dsacl2fsacl and catches
> > a missing case (directory checking) in the sysvolcheck code.
> > 
> > The directory walk was missed due to a cut-and-paste error.
> ... This would be really nice to unit-test I think. :-)
> 
> Either way, it's an improvement over the current situation so +1 with
> one minor note:
> 
> > @@ -1071,7 +1071,6 @@ class cmd_del(Command):
> >  
> >          self.outf.write("GPO %s deleted.\n" % gpo)
> >  
> > -
> >  class cmd_gpo(SuperCommand):
> >      """Group Policy Object (GPO) management."""
> >  
> The Python coding standard (PEP8) prescribes two empty lines between
> top-level classes. We might not be sticking to that everywhere, but
> please don't remove existing whitespace like that. :-)

Thanks, I'll fix that up.

> > @@ -1395,7 +1395,7 @@ def set_gpos_acl(sysvol, dnsdomain, domainsid, domaindn, samdb, lp, use_ntvfs, p
> >          acl = ndr_unpack(security.descriptor,
> >                           str(policy["nTSecurityDescriptor"])).as_sddl()
> >          policy_path = getpolicypath(sysvol, dnsdomain, str(policy["cn"]))
> > -        set_dir_acl(policy_path, dsacl2fsacl(acl, str(domainsid)), lp,
> > +        set_dir_acl(policy_path, dsacl2fsacl(acl, domainsid), lp,
> >                      str(domainsid), use_ntvfs,
> >                      passdb=passdb)
> >  
> > @@ -1522,7 +1522,7 @@ def check_gpos_acl(sysvol, dnsdomain, domainsid, domaindn, samdb, lp,
> >          acl = ndr_unpack(security.descriptor,
> >                           str(policy["nTSecurityDescriptor"])).as_sddl()
> >          policy_path = getpolicypath(sysvol, dnsdomain, str(policy["cn"]))
> > -        check_dir_acl(policy_path, dsacl2fsacl(acl, str(domainsid)), lp,
> > +        check_dir_acl(policy_path, dsacl2fsacl(acl, domainsid), lp,
> >                        domainsid, direct_db_access)
> 
> As a sidethought, I wonder if we need to add a convenience function in
> the C bindings that allows us to convert a Python object to a dom_sid,
> whether it is a Python dom_sid object or a Python string. Having to
> cast sids everywhere in the Python code is annoying and prone to bugs.

The biggest issue is that the samdb bindings created a string for
samdb.domain_sid(), when it always should have been a dom_sid.  But I
agree, having more of the functions be flexible in their input would
make this less annoying.

Andrew Bartlett

-- 
Andrew Bartlett                                http://samba.org/~abartlet/
Authentication Developer, Samba Team           http://samba.org