fixed a problem with group policy writes causing policy corruption

This bug was caused by two things:

  1) in the unix ACL mapping, we were not taking into account group
  write permssions for the SEC_STD_DELETE flag

  2) when a file is created using OVERWRITE mode, a fchmod() would
  fail if the user is not the file owner. We resolve that by only
  doing the fchmod() if the mapped file attribute does not match the
  desired file attribute

diff --git a/source4/ntvfs/posix/pvfs_acl.c b/source4/ntvfs/posix/pvfs_acl.c
index 203b6b11c081f300b9f8aa2e0d924f584a2cc0e1..ad7ac5a7494a8f9dc9a8997cfc3a40a0613f103f 100644 (file)
--- a/source4/ntvfs/posix/pvfs_acl.c
+++ b/source4/ntvfs/posix/pvfs_acl.c
@@ -448,6 +448,35 @@ static bool pvfs_read_only(struct pvfs_state *pvfs, uint32_t access_mask)
        return false;
 }
 
+/*
+  see if we are a member of the appropriate unix group
+ */
+static bool pvfs_group_member(struct pvfs_state *pvfs, gid_t gid)
+{
+       int i, ngroups;
+       gid_t *groups;
+       if (getegid() == gid) {
+               return true;
+       }
+       ngroups = getgroups(0, NULL);
+       if (ngroups == 0) {
+               return false;
+       }
+       groups = talloc_array(pvfs, gid_t, ngroups);
+       if (groups == NULL) {
+               return false;
+       }
+       if (getgroups(ngroups, groups) != ngroups) {
+               talloc_free(groups);
+               return false;
+       }
+       for (i=0; i<ngroups; i++) {
+               if (groups[i] == gid) break;
+       }
+       talloc_free(groups);
+       return i < ngroups;
+}
+
 /*
   default access check function based on unix permissions
   doing this saves on building a full security descriptor
@@ -473,6 +502,12 @@ NTSTATUS pvfs_access_check_unix(struct pvfs_state *pvfs,
                max_bits |= SEC_STD_ALL;
        }
 
+       if ((name->st.st_mode & S_IWOTH) ||
+           ((name->st.st_mode & S_IWGRP) && 
+            pvfs_group_member(pvfs, name->st.st_gid))) {
+               max_bits |= SEC_STD_ALL;
+       }
+
        if (uwrap_enabled()) {
                /* when running with the uid wrapper, files will be created
                   owned by the ruid, but we may have a different simulated 
@@ -491,6 +526,8 @@ NTSTATUS pvfs_access_check_unix(struct pvfs_state *pvfs,
        }
 
        if (*access_mask & ~max_bits) {
+               DEBUG(0,(__location__ " denied access to '%s' - wanted 0x%08x but got 0x%08x (missing 0x%08x)\n",
+                        name->full_name, *access_mask, max_bits, *access_mask & ~max_bits));
                return NT_STATUS_ACCESS_DENIED;
        }
 

diff --git a/source4/ntvfs/posix/pvfs_open.c b/source4/ntvfs/posix/pvfs_open.c
index 12f50fcc97eb71154978bd43b95588557ba23c38..46e39a00dd46c51716ce81f8922cf77e81aa5d35 100644 (file)
--- a/source4/ntvfs/posix/pvfs_open.c
+++ b/source4/ntvfs/posix/pvfs_open.c
@@ -534,7 +534,7 @@ static int pvfs_handle_destructor(struct pvfs_file_handle *h)
 
                if (!timeval_is_zero(&tv[0]) || !timeval_is_zero(&tv[1])) {
                        if (utimes(h->name->full_name, tv) == -1) {
-                               DEBUG(0,("pvfs_handle_destructor: utimes() failed '%s' - %s\n",
+                               DEBUG(3,("pvfs_handle_destructor: utimes() failed '%s' - %s\n",
                                         h->name->full_name, strerror(errno)));
                        }
                }
@@ -1516,6 +1516,8 @@ NTSTATUS pvfs_open(struct ntvfs_module_context *ntvfs,
        if (fd == -1) {
                status = pvfs_map_errno(f->pvfs, errno);
 
+               DEBUG(0,(__location__ " mapped errno %s for %s (was %d)\n", 
+                        nt_errstr(status), f->handle->name->full_name, errno));
                /*
                 * STATUS_MORE_ENTRIES is EAGAIN or EWOULDBLOCK
                 */
@@ -1581,10 +1583,12 @@ NTSTATUS pvfs_open(struct ntvfs_module_context *ntvfs,
        if (f->handle->name->stream_id == 0 &&
            (io->generic.in.open_disposition == NTCREATEX_DISP_OVERWRITE ||
             io->generic.in.open_disposition == NTCREATEX_DISP_OVERWRITE_IF)) {
-               /* for overwrite we need to replace file permissions */
+               /* for overwrite we may need to replace file permissions */
                uint32_t attrib = io->ntcreatex.in.file_attr | FILE_ATTRIBUTE_ARCHIVE;
                mode_t mode = pvfs_fileperms(pvfs, attrib);
-               if (fchmod(fd, mode) == -1) {
+               if (f->handle->name->st.st_mode != mode &&
+                   f->handle->name->dos.attrib != attrib &&
+                   fchmod(fd, mode) == -1) {
                        talloc_free(lck);
                        return pvfs_map_errno(pvfs, errno);
                }

diff --git a/source4/ntvfs/posix/pvfs_util.c b/source4/ntvfs/posix/pvfs_util.c
index 81ff20a60833ca7c5dc717f6170ea984702e3e7d..b1b0a647892d74722beb8f254bf76f654cf6e7b4 100644 (file)
--- a/source4/ntvfs/posix/pvfs_util.c
+++ b/source4/ntvfs/posix/pvfs_util.c
@@ -39,7 +39,10 @@ bool pvfs_has_wildcard(const char *str)
 */
 NTSTATUS pvfs_map_errno(struct pvfs_state *pvfs, int unix_errno)
 {
-       return map_nt_error_from_unix(unix_errno);
+       NTSTATUS status;
+       status = map_nt_error_from_unix(unix_errno);
+       DEBUG(10,(__location__ " mapped unix errno %d -> %s\n", unix_errno, nt_errstr(status)));
+       return status;
 }