- fixed the sort_acl bug, sorting now works right

- don't allow setting of duplicate ACEs
- fixed a ACE delete bug
(This used to be commit 61293979ce2aded58a5ef2a54b3b05d1d278f7cf)

2 files changed, 52 insertions, 54 deletions

diff --git a/source3/lib/util_sid.c b/source3/lib/util_sid.c
index 98d4e77712..e888c1cbcb 100644
--- a/source3/lib/util_sid.c
+++ b/source3/lib/util_sid.c
@@ -442,29 +442,40 @@ BOOL sid_linearize(char *outbuf, size_t len, DOM_SID *sid)
 /*****************************************************************
  Compare two sids.
 *****************************************************************/  
-
-BOOL sid_equal(const DOM_SID *sid1, const DOM_SID *sid2)
+int sid_compare(const DOM_SID *sid1, const DOM_SID *sid2)
 {
 	int i;
 
-	if (sid1 == sid2) return True;
-	if (!sid1 || !sid2) return False;
+	if (sid1 == sid2) return 0;
+	if (!sid1) return -1;
+	if (!sid2) return 1;
 
 	/* compare most likely different rids, first: i.e start at end */
 	for (i = sid1->num_auths-1; i >= 0; --i)
 		if (sid1->sub_auths[i] != sid2->sub_auths[i])
-			return False;
+			return sid1->sub_auths[i] - sid2->sub_auths[i];
 
 	if (sid1->num_auths != sid2->num_auths)
-		return False;
+		return sid1->num_auths - sid2->num_auths;
+
 	if (sid1->sid_rev_num != sid2->sid_rev_num)
-		return False;
+		return sid1->sid_rev_num - sid2->sid_rev_num;
 
 	for (i = 0; i < 6; i++)
 		if (sid1->id_auth[i] != sid2->id_auth[i])
-			return False;
+			return sid1->id_auth[i] - sid2->id_auth[i];
 
-	return True;
+	return 0;
+}
+
+
+/*****************************************************************
+ Compare two sids.
+*****************************************************************/  
+
+BOOL sid_equal(const DOM_SID *sid1, const DOM_SID *sid2)
+{
+	return sid_compare(sid1, sid2) == 0;
 }
 
 
diff --git a/source3/utils/smbcacls.c b/source3/utils/smbcacls.c
index 5ea595c4ea..3e71f39077 100644
--- a/source3/utils/smbcacls.c
+++ b/source3/utils/smbcacls.c
@@ -175,7 +175,7 @@ static void print_ace(FILE *f, SEC_ACE *ace)
 	fprintf(f, "%s:", sidstr);
 
 	if (numeric) {
-		fprintf(f, "%d/%d/0x%08x\n", 
+		fprintf(f, "%d/%d/0x%08x", 
 			ace->type, ace->flags, ace->info.mask);
 		return;
 	}
@@ -198,7 +198,7 @@ static void print_ace(FILE *f, SEC_ACE *ace)
 
 	for (v = standard_values; v->perm; v++) {
 		if (ace->info.mask == v->mask) {
-			fprintf(f, "%s\n", v->perm);
+			fprintf(f, "%s", v->perm);
 			return;
 		}
 	}
@@ -226,8 +226,6 @@ static void print_ace(FILE *f, SEC_ACE *ace)
 			goto again;
 		}
 	}
-
-	fprintf(f, "\n");
 }
 
 
@@ -441,6 +439,7 @@ static void sec_desc_print(FILE *f, SEC_DESC *sd)
 		SEC_ACE *ace = &sd->dacl->ace[i];
 		fprintf(f, "ACL:");
 		print_ace(f, ace);
+		fprintf(f, "\n");
 	}
 
 }
@@ -533,51 +532,41 @@ static int owner_set(struct cli_state *cli, enum chown_mode change_mode,
 	return EXIT_OK;
 }
 
+
 /* 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)
+static int ace_compare(SEC_ACE *ace1, SEC_ACE *ace2)
 {
-	SEC_ACE *tmp_ace;
-	int i, ace_ndx = 0;
-	BOOL do_denied = True;
+	if (sec_ace_equal(ace1, ace2)) return 0;
+	if (ace1->type != ace2->type) return ace1->type - ace2->type;
+	if (sid_compare(&ace1->sid, &ace2->sid)) return sid_compare(&ace1->sid, &ace2->sid);
+	if (ace1->flags != ace2->flags) return ace1->flags - ace2->flags;
+	if (ace1->info.mask != ace2->info.mask) return ace1->info.mask - ace2->info.mask;
+	if (ace1->size != ace2->size) return ace1->size - ace2->size;
+	return memcmp(ace1, ace2, sizeof(SEC_ACE));
+}
 
+static void sort_acl(SEC_ACL *the_acl)
+{
+	int i;
 	if (!the_acl) return;
 
-	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++;
-		}
+	qsort(the_acl->ace, the_acl->num_aces, sizeof(the_acl->ace[0]), ace_compare);
 
-		/* 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++;
+	for (i=1;i<the_acl->num_aces;) {
+		if (sec_ace_equal(&the_acl->ace[i-1], &the_acl->ace[i])) {
+			int j;
+			for (j=i; j<the_acl->num_aces-1; j++) {
+				the_acl->ace[j] = the_acl->ace[j+1];
+			}
+			the_acl->num_aces--;
+		} else {
+			i++;
 		}
 	}
-
-	if (do_denied) {
-		do_denied = False;
-		goto copy_aces;
-	}
-
-	free(the_acl->ace);
-	the_acl->ace = tmp_ace;
 }
 
 /***************************************************** 
@@ -625,8 +614,9 @@ static int cacl_set(struct cli_state *cli, char *filename,
 			for (j=0;old->dacl && j<old->dacl->num_aces;j++) {
 				if (sec_ace_equal(&sd->dacl->ace[i],
 						  &old->dacl->ace[j])) {
-					if (j != old->dacl->num_aces-1) {
-						old->dacl->ace[j] = old->dacl->ace[j+1];
+					int k;
+					for (k=j; k<old->dacl->num_aces-1;k++) {
+						old->dacl->ace[k] = old->dacl->ace[k+1];
 					}
 					old->dacl->num_aces--;
 					if (old->dacl->num_aces == 0) {
@@ -642,10 +632,9 @@ static int cacl_set(struct cli_state *cli, char *filename,
 			}
 
 			if (!found) {
-				fstring str;
-
-				SidToString(str, &sd->dacl->ace[i].sid);
-				printf("ACL for SID %s not found\n", str);
+				printf("ACL for ACE:"); 
+				print_ace(stdout, &sd->dacl->ace[i]);
+				printf(" not found\n");
 			}
 		}
 		break;
@@ -689,11 +678,9 @@ static int cacl_set(struct cli_state *cli, char *filename,
 	}
 
 	/* 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);