1 files changed, 44 insertions, 49 deletions

diff --git a/source3/printing/nt_printing.c b/source3/printing/nt_printing.c
index 48b61ca781..1c69467c20 100644
--- a/source3/printing/nt_printing.c
+++ b/source3/printing/nt_printing.c
@@ -2176,66 +2176,63 @@ uint32 nt_printing_setsec(char *printername, SEC_DESC_BUF *secdesc_ctr)
 {
 	SEC_DESC_BUF *new_secdesc_ctr = NULL;
 	SEC_DESC_BUF *old_secdesc_ctr = NULL;
+	DOM_SID *owner_sid, *group_sid;
+	SEC_ACL *dacl, *sacl;
+	SEC_DESC *psd = NULL;
+	size_t secdesc_size;
 	prs_struct ps;
 	TALLOC_CTX *mem_ctx = NULL;
 	fstring key;
+	uint16 secdesc_type;
 	uint32 status;
 
 	mem_ctx = talloc_init();
-	if (mem_ctx == NULL)
-		return False;
+	if (mem_ctx == NULL) return False;
 
-        /* The old owner and group sids of the security descriptor are not
-	   present when new ACEs are added or removed by changing printer
-	   permissions through NT.  If they are NULL in the new security
-	   descriptor then copy them over from the old one. */
+	/* Make a copy of the security descriptor so we can fill in bits
+	   that NT doesn't pass to us.  Any pieces of the security descriptor
+	   that are NULL are not meant to change. */
 
-	if (!secdesc_ctr->sec->owner_sid || !secdesc_ctr->sec->grp_sid) {
-		DOM_SID *owner_sid, *group_sid;
-		SEC_ACL *dacl, *sacl;
-		SEC_DESC *psd = NULL;
-		size_t size;
-
-		nt_printing_getsec(printername, &old_secdesc_ctr);
+	nt_printing_getsec(printername, &old_secdesc_ctr);
+	
+	secdesc_type = secdesc_ctr->sec->type;
 
-		/* Pick out correct owner and group sids */
+	/* Copy over owner and group sids.  There seems to be no flag for
+	   this so just check the pointer values. */
 
-		owner_sid = secdesc_ctr->sec->owner_sid ?
-			secdesc_ctr->sec->owner_sid :
-			old_secdesc_ctr->sec->owner_sid;
+	owner_sid = secdesc_ctr->sec->owner_sid ?
+		secdesc_ctr->sec->owner_sid :
+		old_secdesc_ctr->sec->owner_sid;
 
-		group_sid = secdesc_ctr->sec->grp_sid ?
-			secdesc_ctr->sec->grp_sid :
-			old_secdesc_ctr->sec->grp_sid;
+	group_sid = secdesc_ctr->sec->grp_sid ?
+		secdesc_ctr->sec->grp_sid :
+		old_secdesc_ctr->sec->grp_sid;
 
-		dacl = secdesc_ctr->sec->dacl ?
-			secdesc_ctr->sec->dacl :
-			old_secdesc_ctr->sec->dacl;
+	/* Ignore changes to the system ACL.  This has the effect of making
+	   changes through the security tab audit button not sticking. 
+	   Perhaps in future Samba could implement these settings somehow. */
 
-		sacl = secdesc_ctr->sec->sacl ?
-			secdesc_ctr->sec->sacl :
-			old_secdesc_ctr->sec->sacl;
+	sacl = NULL;
+	secdesc_type &= ~SEC_DESC_SACL_PRESENT;
 
-		/* Make a deep copy of the security descriptor */
+	/* Copy across discretionary ACL */
 
-		psd = make_sec_desc(secdesc_ctr->sec->revision,
-				    secdesc_ctr->sec->type,
-				    owner_sid, group_sid,
-				    sacl,
-				    dacl,
-				    &size);
+	if (secdesc_type & SEC_DESC_DACL_PRESENT) {
+		dacl = secdesc_ctr->sec->dacl;
+	} else {
+		dacl = old_secdesc_ctr->sec->dacl;
+		secdesc_type |= SEC_DESC_DACL_PRESENT;
+	}
 
-		new_secdesc_ctr = make_sec_desc_buf(size, psd);
+	/* Create new security descriptor from bits */
 
-		/* Free up memory */
+	psd = make_sec_desc(secdesc_ctr->sec->revision, secdesc_type,
+			    owner_sid, group_sid, sacl, dacl, &secdesc_size);
 
-		free_sec_desc(&psd);
-		free_sec_desc_buf(&old_secdesc_ctr);
-	}
+	new_secdesc_ctr = make_sec_desc_buf(secdesc_size, psd);
 
-	if (!new_secdesc_ctr) {
-		new_secdesc_ctr = secdesc_ctr;
-	}
+	free_sec_desc(&psd);
+	free_sec_desc_buf(&old_secdesc_ctr);
 
 	/* Store the security descriptor in a tdb */
 
@@ -2245,7 +2242,7 @@ uint32 nt_printing_setsec(char *printername, SEC_DESC_BUF *secdesc_ctr)
 	if (!sec_io_desc_buf("nt_printing_setsec", &new_secdesc_ctr, 
 			     &ps, 1)) {
 		status = ERROR_INVALID_FUNCTION;
-		goto out;
+		goto done;
 	}
 
 	slprintf(key, sizeof(key), "SECDESC/%s", printername);
@@ -2259,16 +2256,14 @@ uint32 nt_printing_setsec(char *printername, SEC_DESC_BUF *secdesc_ctr)
 
 	/* Free mallocated memory */
 
- out:
+ done:
 	free_sec_desc_buf(&old_secdesc_ctr);
-
-	if (new_secdesc_ctr != secdesc_ctr) {
-		free_sec_desc_buf(&new_secdesc_ctr);
-	}
+	free_sec_desc_buf(&new_secdesc_ctr);
 
 	prs_mem_free(&ps);
-	if (mem_ctx)
-		talloc_destroy(mem_ctx);
+
+	if (mem_ctx) talloc_destroy(mem_ctx);
+
 	return status;
 }