samba resetting permissions on extant files on NT_TRANSACT_CREATE

[Buck Huppmann]

one of our diligent users discovered this. i'm less diligent, so please
rebuke me gently

seems that if samba gets a request to create a file and the file already
exists, it will reset the perms on the file. this isn't what NT (4, SP6a)
seems to do, and it's not what UNIX does, so i'm having trouble grokking
it. at any rate, since it potentially impacts on security, i'm hoping
either that someone can hack on this or to be made to understand it

thanks

--- demo ---

(D: is mapped to an NT4SP6a server share; G: is mapped to a samba 2.2.2
share with ``inherit permissions'', though i think that's irrelevant to
the behavioral course exhibited)

	G:\>for %i in (d g) do ( cacls %i:\foo\bar & type nul > %i:\foo\bar
	More? cacls %i:\ foo\bar )

	G:\>(cacls d:\foo\bar   & type nul   1>d:\foo\bar  & cacls d:\foo\bar  )
	d:\foo\bar US\me:F

	d:\foo\bar US\me:F


	G:\>(cacls g:\foo\bar   & type nul   1>g:\foo\bar  & cacls g:\foo\bar  )
	g:\foo\bar Everyone:(special access:)
			    WRITE_OWNER

		   US\someof:(special access:)
			    WRITE_OWNER

		   US\me:F

	g:\foo\bar Everyone:(special access:)
			    WRITE_OWNER

		   US\me:F
		   US\someof:(special access:)
			    READ_CONTROL
			    SYNCHRONIZE
			    FILE_GENERIC_READ
			    FILE_READ_DATA
			    FILE_READ_EA
			    FILE_READ_ATTRIBUTES

--- example patch ---

the second hunk is from the 

	if(!VALID_STAT(*psbuf)) {
		if (vfs_fstat(fsp,fsp->fd,psbuf) == -1) {

which is the same logic that open_file_shared() uses higher up in the
stack to determine whether it's creating the file, using the same
*psbuf contents. i'm unsure if the same sort of check should also be made
in nttrans.c:call_nt_transact_create() before set_sd() is called also, since
i don't have any ready way to test this (so i have a hard time trying to
force myself to study that code, which is too close to the actual SMBs for
my comfort)

--- samba-2.2.2/source/smbd/open.c.orig	Sat Oct 13 17:09:39 2001
+++ samba-2.2.2/source/smbd/open.c	Fri Dec 14 19:47:53 2001
@@ -95,6 +95,10 @@
 	pstring fname;
 	int accmode = (flags & O_ACCMODE);
 	int local_flags = flags;
+	/* flag whether we create a new file so modes can be reset if so
+	 * (but not otherwise)
+	 */
+	BOOL created;
 
 	fsp->fd = -1;
 	fsp->oplock_type = NO_OPLOCK;
@@ -167,7 +171,17 @@
 			fd_close(conn, fsp);
 			return False;
 		}
+		created = True; /* maybe. if we're in a race and have owner-
+				 * ship, we'll end up resetting the perms, so
+				 * hope we like whatever we chose this time */
 	}
+	else
+		created = False; /* maybe--unless it got unlinked out from
+				  * under us. it'd be nice to do another
+				  * fstat() here and make sure we're still
+				  * looking at the same inode, but that's
+				  * probably not buying much security, and
+				  * i'm not sure how sure we could get */
 
 	/*
 	 * POSIX allows read-only opens of directories. We don't
@@ -218,9 +232,10 @@
 
 	/*
 	 * Take care of inherited ACLs on created files. JRA.
+	 * IF we just created the file
 	 */
 
-	if ((flags & O_CREAT) && (conn->vfs_ops.fchmod_acl != NULL)) {
+	if (created && (conn->vfs_ops.fchmod_acl != NULL)) {
 		int saved_errno = errno; /* We might get ENOSYS in the next call.. */
 		if (conn->vfs_ops.fchmod_acl(fsp, fsp->fd, mode) == -1 && errno == ENOSYS)
 			errno = saved_errno; /* Ignore ENOSYS */