Windows 2008 and the handling of Owner Rights permissions

[Richard Sharpe]

2012/3/4 Richard Sharpe <realrichardsharpe at gmail.com>:
> Hi,
>
> Here http://technet.microsoft.com/en-us/library/dd125370%28v=WS.10%29.aspx
> it suggests that if an ACL on an object contains the Owner Rights
> principal (S-1-3-4) and the permissions do not contain WRITE_DAC and
> READ_CONTROL then the current handling of se_access_check
> (libcli/security/access_check.c) is incorrect.
>
> The solution seems simple. Defer the check for SEC_STD_WRITE_DAC and
> SEC_STD_READ_CONTROL until after we have scanned the ACL and save the
> permissions associated with S-1-3-4 in a variable that starts out as
> ~0 and is used with SEC_STD_WRITE_DAC and SEC_STD_READ_CONTROL to
> determine the default permissions that should apply and therefore
> those bits that should be removed ...
>
> Thoughts? I guess I need to fire up a Windows Server 2008 VM to see if
> this applies to file objects, but I suspect it does.

Here is a patch that looks like it will implement this. If there are
no objections I will at least create a bug in bugzilla.

The following patch is also attached for those who want to apply it
and play with it. It is against master.

diff --git a/libcli/security/access_check.c b/libcli/security/access_check.c
index a9b618f..8d32c95 100644
--- a/libcli/security/access_check.c
+++ b/libcli/security/access_check.c
@@ -159,6 +159,11 @@ NTSTATUS se_access_check(const struct security_descriptor *
sd,
 	uint32_t i;
 	uint32_t bits_remaining;
 	uint32_t explicitly_denied_bits = 0;
+	/*
+	 * Up until Windows Server 2008, owner always had these rights. Now
+	 * we have to use Owner Rights perms if they are on the file.
+	 */
+	uint32_t owner_rights = SEC_STD_WRITE_DAC | SEC_STD_READ_CONTROL;

 	*access_granted = access_desired;
 	bits_remaining = access_desired;
@@ -189,12 +194,6 @@ NTSTATUS se_access_check(const struct security_descriptor *
sd,
 		}
 	}

-	/* the owner always gets SEC_STD_WRITE_DAC and SEC_STD_READ_CONTROL */
-	if ((bits_remaining & (SEC_STD_WRITE_DAC|SEC_STD_READ_CONTROL)) &&
-	    security_token_has_sid(token, sd->owner_sid)) {
-		bits_remaining &= ~(SEC_STD_WRITE_DAC|SEC_STD_READ_CONTROL);
-	}
-
 	/* TODO: remove this, as it is file server specific */
 	if ((bits_remaining & SEC_RIGHTS_PRIV_RESTORE) &&
 	    security_token_has_privilege(token, SEC_PRIV_RESTORE)) {
@@ -228,6 +227,19 @@ NTSTATUS se_access_check(const struct security_descriptor *
sd,
 			continue;
 		}

+		/*
+		 * We need the Owner Rights permissions to ensure we
+		 * give or deny the correct permissions to the owner. Replace
+		 * owner_rights with the perms here if it is present.
+		 *
+		 * We don't care if we are not the owner because that is taken
+		 * care of below when we check if our token has the owner SID.
+		 */
+		if (dom_sid_equal(&ace->trustee, &global_sid_Owner_Rights)) {
+			owner_rights = ace->access_mask;
+			continue;
+		}
+
 		if (!security_token_has_sid(token, &ace->trustee)) {
 			continue;
 		}
@@ -245,6 +257,12 @@ NTSTATUS se_access_check(const struct security_descriptor *
sd,
 		}
 	}

+	/* the owner always gets owner rights as defined above */
+	if ((bits_remaining & owner_rights) &&
+	    security_token_has_sid(token, sd->owner_sid)) {
+		bits_remaining &= ~(SEC_STD_WRITE_DAC|SEC_STD_READ_CONTROL);
+	}
+
 	bits_remaining |= explicitly_denied_bits;

 done:
diff --git a/libcli/security/dom_sid.h b/libcli/security/dom_sid.h
index df57bd1..c4a417b 100644
--- a/libcli/security/dom_sid.h
+++ b/libcli/security/dom_sid.h
@@ -38,6 +38,7 @@ extern const struct dom_sid global_sid_Authenticated_Users;
 extern const struct dom_sid global_sid_Network;
 extern const struct dom_sid global_sid_Creator_Owner;
 extern const struct dom_sid global_sid_Creator_Group;
+extern const struct dom_sid global_sid_Owner_Rights;
 extern const struct dom_sid global_sid_Anonymous;
 extern const struct dom_sid global_sid_Builtin;
 extern const struct dom_sid global_sid_Builtin_Administrators;
diff --git a/libcli/security/util_sid.c b/libcli/security/util_sid.c
index f87d3eb..9a24a4a 100644
--- a/libcli/security/util_sid.c
+++ b/libcli/security/util_sid.c
@@ -62,6 +62,8 @@ const struct dom_sid global_sid_Creator_Owner =		
/* Creator Owner */
 { 1, 1, {0,0,0,0,0,3}, {0,0,0,0,0,0,0,0,0,0,0,0,0,0,0}};
 const struct dom_sid global_sid_Creator_Group =		/* Creator Group
 */
 { 1, 1, {0,0,0,0,0,3}, {1,0,0,0,0,0,0,0,0,0,0,0,0,0,0}};
+const struct dom_sid global_sid_Owner_Rights =		/* Owner Rights */
+{ 1, 1, {0,0,0,0,0,3}, {4,0,0,0,0,0,0,0,0,0,0,0,0,0,0}};
 const struct dom_sid global_sid_Anonymous =			/* Anonymous log
in */
 { 1, 1, {0,0,0,0,0,5}, {7,0,0,0,0,0,0,0,0,0,0,0,0,0,0}};
 const struct dom_sid global_sid_Enterprise_DCs =		/* Enterprise DC
s */



-- 
Regards,
Richard Sharpe
(何以解憂？唯有杜康。--曹操)
-------------- next part --------------
A non-text attachment was scrubbed...
Name: owner-rights.patch
Type: application/octet-stream
Size: 3548 bytes
Desc: not available
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20120304/987e98b1/attachment.obj>