smbcacls support for automatic inheritance propagation

[David Disseldorp]

On Thu, 30 Jan 2014 15:41:00 +0000, Noel Power wrote:

> On 29/01/14 17:50, David Disseldorp wrote:
> > 1651                 d_printf("Inheritance enabled at %s, can't apply set operation\n",filename);
> > 1652                 result = NT_STATUS_UNSUCCESSFUL;
> >
> > - why is this considered an error, does icacls.exe behave similarly in
> >   such a case?
> No, but.. there is no real equivalent to set with iacacls. Also because
> smbcacls --set replaces the ACL with the
> ACL specified on the command-line (with no regard to any inherited
> parent acls) and we issue the --propagate-inheritance indicating we wish
> to honour inheritance we would more than likely be setting non-senisical
> values here. Actually probably what was intended was --add, for that
> reason I inhibit set via an error, if we wish we could remove that error
> and issue a warning instead and continue to propagate, I'm ok with that.
> I see in some of the previous email some confusing comments from me)
> about this (including a comment indicating I was no longer uncomfortable
> with '--set' in the context of --propagate-inheritance) I have no idea
> what I meant there (because I am uncomfortable with it :-)) So.. what do
> you think change this to a warning?

Given it would stomp on any inherited permissions in the tree below, I
tend to agree that bailing out on non-protected set is the best thing
to do for now.

> > 1526         /* find acl(s) that are inherited */
> > 1527         for (j = 0; old->dacl && j < old->dacl->num_aces; j++) {
> > 1528 
> > 1529                 if (old->dacl->aces[j].flags & SEC_ACE_FLAG_INHERITED_ACE) {
> > 1530                         if (!add_ace_with_ctx(aclctx, &acl_to_remove,
> > 1531                                               &old->dacl->aces[j])) {
> > 1532                                 status = NT_STATUS_NO_MEMORY;
> > 1533                                 goto out;
> > 1534                         }
> > 1535                 }
> > 1536         }
> > 1537 
> > 1538         /* remove any acl(s) that are inherited */
> > 1539         if (acl_to_remove) {
> > 1540                 for (i = 0; i < acl_to_remove->num_aces; i++) {
> > 1541                         struct security_ace ace = acl_to_remove->aces[i];
> > 1542                         for (j = 0; old->dacl && j < old->dacl->num_aces; j++) {
> > 1543 
> > 1544                                 if (sec_ace_equal(&ace,
> > 1545                                                   &old->dacl->aces[j])) {
> > 1546                                         uint32 k;
> > 1547                                         for (k = j; k < old->dacl->num_aces-1;
> > 1548                                                 k++) {
> > 1549                                                 old->dacl->aces[k] =
> > 1550                                                         old->dacl->aces[k+1];
> > 1551                                         }
> > 1552                                         old->dacl->num_aces--;
> > 1553                                         break;
> > 1554                                 }
> > 1555                         }
> > 1556                 }
> > 1557         }
> > - This code looks waaaaay too complex for what it's doing. IIUC it
> >   determines which ACEs can be removed from the existing ACL, and then
> >   removes them by collapsing the array in the subsequent loop.
> hmm, not sure I agree with the 'waaaay too complex' possibly I am tunnel
> visioned :-)
> > 	- sec_ace_equal() is used to check whether the ACE should be
> > 	  removed, but couldn't this also match an explicit
> > 	  (non-inherited) ACE?
> not sure I understand what you mean ?

I thought sec_ace_equal() was only doing a trustee SID comparison. Turns
out it does a full comparison, so your logic was okay :)

> > 	- Isn't it possible to build a new ACL in the first iteration,
> > 	  ignoring inherited ACEs. Then combine it with acl_to_add.
> sure, I guess, it would save a loop, will do that
> > - What happens in the following scenario:
> >   parent_dir
> > 	+---child_dir
> > 		+-----nested_file
> >   1) setacl(parent_dir, allow:ddiss:read, OI)
> >   2) setacl(child_dir, allow:noelp:write, OI)
> >   Does nested_file end up with both allow: ACEs, or would propagation
> >   during setacl(child_dir,...) remove the inherited allow:ddiss ACE?
> it would remove it (if set would work) but remember from above
> attempting to set is prohibited

Would it remain on --add, --modify and --delete?

Cheers, David