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

3 files changed, 48 insertions, 4 deletions

diff --git a/source4/ntvfs/posix/pvfs_acl.c b/source4/ntvfs/posix/pvfs_acl.c
index 203b6b11c0..ad7ac5a749 100644
--- a/source4/ntvfs/posix/pvfs_acl.c
+++ b/source4/ntvfs/posix/pvfs_acl.c
@@ -449,6 +449,35 @@ static bool pvfs_read_only(struct pvfs_state *pvfs, uint32_t access_mask)
 }
 
 /*
+  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
   for the common case of access check on files with no 
@@ -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 12f50fcc97..46e39a00dd 100644
--- 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 81ff20a608..b1b0a64789 100644
--- 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;
 }