Don't call string_to_sid after cli_lsa_lookup_names() fails.

Can't use space as a ACL separator as it breaks NT user/group names that
contain spaces (i.e most of the default ones).

The MSDN is contradictory over the exact ordering of ACE entries in an ACL.
However NT4 gives a "The information may have been modified by a computer
running Windows NT 5.0" if denied ACEs do not appear before allowed ACEs.
Sort ACE list before calling cli_set_secdesc().
(This used to be commit 01a89978ff4b9fcc813c4f251ddfe23c0485a696)

diff --git a/source3/utils/smbcacls.c b/source3/utils/smbcacls.c
index d73699264f562ece39a387d79194f8b1c16147d3..a193a1404307322493ef97b25acb6ee9029a23e3 100644 (file)
--- a/source3/utils/smbcacls.c
+++ b/source3/utils/smbcacls.c
@@ -138,7 +138,7 @@ static BOOL StringToSid(DOM_SID *sid, char *str)
        if (!open_policy_hnd() ||
            cli_lsa_lookup_names(&lsa_cli, &pol, 1, &str, &sids, &types, 
                                 &num_sids) != NT_STATUS_NOPROBLEMO) {
-               result = string_to_sid(sid, str);
+               result = False;
                goto done;
        }
 
@@ -345,7 +345,7 @@ static SEC_DESC *sec_desc_parse(char *str)
        SEC_ACL *dacl=NULL;
        int revision=1;
 
-       while (next_token(&p, tok, " \t,\r\n", sizeof(tok))) {
+       while (next_token(&p, tok, "\t,\r\n", sizeof(tok))) {
 
                if (strncmp(tok,"REVISION:", 9) == 0) {
                        revision = strtol(tok+9, NULL, 16);
@@ -531,6 +531,51 @@ static void owner_set(struct cli_state *cli, enum chown_mode change_mode, char *
        cli_close(cli, fnum);
 }
 
+/* The MSDN is contradictory over the ordering of ACE entries in an ACL.
+   However NT4 gives a "The information may have been modified by a
+   computer running Windows NT 5.0" if denied ACEs do not appear before
+   allowed ACEs. */
+
+static void sort_acl(SEC_ACL *the_acl)
+{
+       SEC_ACE *tmp_ace;
+       int i, ace_ndx = 0;
+       BOOL do_denied = True;
+
+       tmp_ace = (SEC_ACE *)malloc(sizeof(SEC_ACE) * the_acl->num_aces);
+
+       if (!tmp_ace) return;
+
+ copy_aces:
+       
+       for (i = 0; i < the_acl->num_aces; i++) {
+
+               /* Copy denied ACEs */
+
+               if (do_denied &&
+                   the_acl->ace[i].type == SEC_ACE_TYPE_ACCESS_DENIED) {
+                       tmp_ace[ace_ndx] = the_acl->ace[i];
+                       ace_ndx++;
+               }
+
+               /* Copy other ACEs */
+
+               if (!do_denied &&
+                   the_acl->ace[i].type != SEC_ACE_TYPE_ACCESS_DENIED) {
+                       tmp_ace[ace_ndx] = the_acl->ace[i];
+                       ace_ndx++;
+               }
+       }
+
+       if (do_denied) {
+               do_denied = False;
+               goto copy_aces;
+       }
+
+       free(the_acl->ace);
+       the_acl->ace = tmp_ace;
+}
+
 /***************************************************** 
 set the ACLs on a file given an ascii description
 *******************************************************/
@@ -632,6 +677,12 @@ static void cacl_set(struct cli_state *cli, char *filename,
                free_sec_desc(&sd);
        }
 
+       /* Denied ACE entries must come before allowed ones */
+
+       sort_acl(old->dacl);
+
+       /* Create new security descriptor and set it */
+
        sd = make_sec_desc(old->revision, old->owner_sid, old->grp_sid, 
                           NULL, old->dacl, &sd_size);
 
@@ -639,6 +690,8 @@ static void cacl_set(struct cli_state *cli, char *filename,
                printf("ERROR: secdesc set failed: %s\n", cli_errstr(cli));
        }
 
+       /* Clean up */
+
        free_sec_desc(&sd);
        free_sec_desc(&old);