Fix bug #8797 - Samba does not correctly handle DENY ACEs when privileges apply.

Signed-off-by: Jeremy Allison <jra@samba.org>
Autobuild-User: Jeremy Allison <jra@samba.org>
Autobuild-Date: Sat Mar 10 01:33:45 CET 2012 on sn-devel-104

diff --git a/libcli/security/access_check.c b/libcli/security/access_check.c
index a9b618f577b2a1ab1cd1f6a8a93cfde78f2ecdf1..d9f6293a460ea264364d78fbc0af4037bc20aa5d 100644 (file)
--- a/libcli/security/access_check.c
+++ b/libcli/security/access_check.c
@@ -178,38 +178,12 @@ NTSTATUS se_access_check(const struct security_descriptor *sd,
                        bits_remaining));
        }
 
-       /* s3 had this with #if 0 previously. To be sure the merge
-          doesn't change any behaviour, we have the above #if check
-          on _SAMBA_BUILD_. */
-       if (access_desired & SEC_FLAG_SYSTEM_SECURITY) {
-               if (security_token_has_privilege(token, SEC_PRIV_SECURITY)) {
-                       bits_remaining &= ~SEC_FLAG_SYSTEM_SECURITY;
-               } else {
-                       return NT_STATUS_PRIVILEGE_NOT_HELD;
-               }
-       }
-
        /* 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)) {
-               bits_remaining &= ~(SEC_RIGHTS_PRIV_RESTORE);
-       }
-       if ((bits_remaining & SEC_RIGHTS_PRIV_BACKUP) &&
-           security_token_has_privilege(token, SEC_PRIV_BACKUP)) {
-               bits_remaining &= ~(SEC_RIGHTS_PRIV_BACKUP);
-       }
-
-       if ((bits_remaining & SEC_STD_WRITE_OWNER) &&
-            security_token_has_privilege(token, SEC_PRIV_TAKE_OWNERSHIP)) {
-               bits_remaining &= ~(SEC_STD_WRITE_OWNER);
-       }
-
        /* a NULL dacl allows access */
        if ((sd->type & SEC_DESC_DACL_PRESENT) && sd->dacl == NULL) {
                *access_granted = access_desired;
@@ -247,6 +221,34 @@ NTSTATUS se_access_check(const struct security_descriptor *sd,
 
        bits_remaining |= explicitly_denied_bits;
 
+       /*
+        * We check privileges here because they override even DENY entries.
+        */
+
+       /* Does the user have the privilege to gain SEC_PRIV_SECURITY? */
+       if (bits_remaining & SEC_FLAG_SYSTEM_SECURITY) {
+               if (security_token_has_privilege(token, SEC_PRIV_SECURITY)) {
+                       bits_remaining &= ~SEC_FLAG_SYSTEM_SECURITY;
+               } else {
+                       return NT_STATUS_PRIVILEGE_NOT_HELD;
+               }
+       }
+
+       /* TODO: remove this, as it is file server specific */
+       if ((bits_remaining & SEC_RIGHTS_PRIV_RESTORE) &&
+           security_token_has_privilege(token, SEC_PRIV_RESTORE)) {
+               bits_remaining &= ~(SEC_RIGHTS_PRIV_RESTORE);
+       }
+       if ((bits_remaining & SEC_RIGHTS_PRIV_BACKUP) &&
+           security_token_has_privilege(token, SEC_PRIV_BACKUP)) {
+               bits_remaining &= ~(SEC_RIGHTS_PRIV_BACKUP);
+       }
+
+       if ((bits_remaining & SEC_STD_WRITE_OWNER) &&
+            security_token_has_privilege(token, SEC_PRIV_TAKE_OWNERSHIP)) {
+               bits_remaining &= ~(SEC_STD_WRITE_OWNER);
+       }
+
 done:
        if (bits_remaining != 0) {
                *access_granted = bits_remaining;