2974459,2879351,1951574,1852572,1661378,1613456 Revisions to the NativeAccess()

routine that queries file permissions on Windows native filesystems.  Meant to
fix numerous bugs where [file writable|readable|executable] "lies" about what
operations are possible, especially when the file resides on a Samba share.
Patch cherrypicked off the fix-win-native-access branch.

2 files changed, 56 insertions, 24 deletions

diff --git a/ChangeLog b/ChangeLog
index 692b3a5..3edec76 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,13 @@
+2012-02-02  Don Porter  <dgp@users.sourceforge.net>
+
+	* win/tclWinFile.c:	[Bugs 2974459,2879351,1951574,1852572,
+	1661378,1613456]: Revisions to the NativeAccess() routine that
+	queries file permissions on Windows native filesystems.  Meant to
+	fix numerous bugs where [file writable|readable|executable] "lies"
+	about what operations are possible, especially when the file resides
+	on a Samba share.  Patch cherrypicked off the fix-win-native-access
+	branch.
+
 2012-01-22  Jan Nijtmans  <nijtmans@users.sf.net>
 
 	* tools/uniClass.tcl:    [Frq 3473670]: Various Unicode-related 
diff --git a/win/tclWinFile.c b/win/tclWinFile.c
index 5a3f193..f145e23 100644
--- a/win/tclWinFile.c
+++ b/win/tclWinFile.c
@@ -1342,14 +1342,25 @@ NativeAccess(
 	return -1;
     }
 
-    if ((mode & W_OK) 
-      && (tclWinProcs->getFileSecurityProc == NULL)
-      && (attr & FILE_ATTRIBUTE_READONLY)) {
+    if (mode == F_OK) {
 	/*
-	 * We don't have the advanced 'getFileSecurityProc', and
-	 * our attributes say the file is not writable.  If we
-	 * do have 'getFileSecurityProc', we'll do a more
-	 * robust XP-related check below.
+	 * File exists, nothing else to check.
+	 */
+
+	return 0;
+    }
+
+    if ((mode & W_OK)
+	&& (attr & FILE_ATTRIBUTE_READONLY)
+	&& !(attr & FILE_ATTRIBUTE_DIRECTORY)) {
+	/*
+	 * The attributes say the file is not writable.	 If the file is a
+	 * regular file (i.e., not a directory), then the file is not
+	 * writable, full stop.	 For directories, the read-only bit is
+	 * (mostly) ignored by Windows, so we can't ascertain anything about
+	 * directory access from the attrib data.  However, if we have the
+	 * advanced 'getFileSecurityProc', then more robust ACL checks
+	 * will be done below.
 	 */
 
 	Tcl_SetErrno(EACCES);
@@ -1373,15 +1384,14 @@ NativeAccess(
      * we have a more complex permissions structure so we try to check that.
      * The code below is remarkably complex for such a simple thing as finding
      * what permissions the OS has set for a file.
-     *
-     * If we are simply checking for file existence, then we don't need all
-     * these complications (which are really quite slow: with this code 'file
-     * readable' is 5-6 times slower than 'file exists').
      */
 
-    if ((mode != F_OK) && (tclWinProcs->getFileSecurityProc != NULL)) {
+    if (tclWinProcs->getFileSecurityProc != NULL) {
 	SECURITY_DESCRIPTOR *sdPtr = NULL;
 	unsigned long size;
+	SID *pSid = 0;
+	BOOL SidDefaulted;
+	SID_IDENTIFIER_AUTHORITY samba_unmapped = { 0, 0, 0, 0, 0, 22 };
 	GENERIC_MAPPING genMap;
 	HANDLE hToken = NULL;
 	DWORD desiredAccess = 0;
@@ -1398,7 +1408,8 @@ NativeAccess(
 	size = 0;
 	(*tclWinProcs->getFileSecurityProc)(nativePath,
 		OWNER_SECURITY_INFORMATION | GROUP_SECURITY_INFORMATION
-		| DACL_SECURITY_INFORMATION, 0, 0, &size);
+		| DACL_SECURITY_INFORMATION | LABEL_SECURITY_INFORMATION,
+		0, 0, &size);
 
 	/*
 	 * Should have failed with ERROR_INSUFFICIENT_BUFFER
@@ -1431,7 +1442,8 @@ NativeAccess(
 
 	if (!(*tclWinProcs->getFileSecurityProc)(nativePath,
 		OWNER_SECURITY_INFORMATION | GROUP_SECURITY_INFORMATION
-		| DACL_SECURITY_INFORMATION, sdPtr, size, &size)) {
+		| DACL_SECURITY_INFORMATION | LABEL_SECURITY_INFORMATION,
+		sdPtr, size, &size)) {
 	    /*
 	     * Error getting owner SD
 	     */
@@ -1440,6 +1452,26 @@ NativeAccess(
 	}
 
 	/*
+	 * As of Samba 3.0.23 (10-Jul-2006), unmapped users and groups are
+	 * assigned to SID domains S-1-22-1 and S-1-22-2, where "22" is the
+	 * top-level authority.	 If the file owner and group is unmapped then
+	 * the ACL access check below will only test against world access,
+	 * which is likely to be more restrictive than the actual access
+	 * restrictions.  Since the ACL tests are more likely wrong than
+	 * right, skip them.  Moreover, the unix owner access permissions are
+	 * usually mapped to the Windows attributes, so if the user is the
+	 * file owner then the attrib checks above are correct (as far as they
+	 * go).
+	 */
+
+	if(!GetSecurityDescriptorOwner(sdPtr,&pSid,&SidDefaulted) ||
+	   memcmp(GetSidIdentifierAuthority(pSid),&samba_unmapped,
+		  sizeof(SID_IDENTIFIER_AUTHORITY))==0) {
+	    HeapFree(GetProcessHeap(), 0, sdPtr);
+	    return 0; /* Attrib tests say access allowed. */
+	}
+
+	/*
 	 * Perform security impersonation of the user and open the
 	 * resulting thread token.
 	 */
@@ -1515,16 +1547,6 @@ NativeAccess(
 	    Tcl_SetErrno(EACCES);
 	    return -1;
 	}
-	/*
-	 * For directories the above checks are ok.  For files, though,
-	 * we must still check the 'attr' value.
-	 */
-	if ((mode & W_OK)
-	  && !(attr & FILE_ATTRIBUTE_DIRECTORY)
-	  && (attr & FILE_ATTRIBUTE_READONLY)) {
-	    Tcl_SetErrno(EACCES);
-	    return -1;
-	}
     }
     return 0;
 }