Re: [HTCondor-users] bug in how condor_submit checks initialdir permissions?

[David Champion <dgc@xxxxxxxxxxxx>]

* On 11 Mar 2013, Jaime Frey wrote: 
> On Mar 5, 2013, at 9:29 AM, Jason Ferrara <jason.ferrara@xxxxxxxxxxxxx> wrote:
> 
> > We've been having issues where a user would submit a job, and condor_submit would respond with
> > 
> > ERROR: No such directory: <path to initialdir>
> > 
> > even though the directory pointed to by initialdir does exist and the user has full read/write/execute permission for it.
> > 
> > To check for access to initialdir, condor_submit calls check_iwd, which calls access_euid, which calls access_euid_dir. access_euid_dir checks if the effrective uid or gid has access by manually checking permission bits, but it doesn't check secondary groups or ACLs, so the access check can fail even if the user really does have access. Also, check_iwd always prints "No such directory", even if the failure is caused by lack of write permission to the directory.
> > 
> > Replacing all calls to access_euid with the system provided euidaccess seems to fix the problem. Is this the right thing to do?
> 
> That does sound like the right thing to do, where euidaccess() is available. I'll make an entry in our bug tracking system, though I can't say when we'll get a chance to make the change and test it.

This is catching users in OSG Connect, so I'd like to see a fix.
euidaccess() (eaccess()) is a GNU extension, and pretty unportable.

access_euid_dir() amounts to this rule:

* if testing R_OK, attempt to open the directory to read
* if testing W_OK, attempt to create a file in the directory (i.e. write the directory)
* if testing X_OK, do a bunch of tests against euid and egid, neglecting supplementary groups and POSIX ACLs.

That is, R_OK and W_OK use empirical tests, while X_OK attempts to
emulate the logic of the kernel.  Why not have the X_OK test simply
attempt chdir() to the directory?  For example:


diff --git a/src/condor_utils/access_euid.unix.cpp b/src/condor_utils/access_euid.unix.cpp
--- a/src/condor_utils/access_euid.unix.cpp
+++ b/src/condor_utils/access_euid.unix.cpp
@@ -21,6 +21,7 @@
 #define _FILE_OFFSET_BITS 64
 #include "condor_common.h"
 #include "condor_debug.h"
+#include <fcntl.h>
 #include <dirent.h>
 
 
@@ -69,33 +70,13 @@
 	}
 
 	if ((X_OK == 0) || (mode & X_OK)) {
-		struct stat st;
-		if (!statbuf) {
-			/* stats are expensive, so only do it if I have to */
-			if (stat(path, &st) < 0) {
-				if( ! errno ) {
-					dprintf( D_ALWAYS, "WARNING: stat() failed, but "
-							 "errno is still 0!  Beware of misleading "
-							 "error messages\n" );
-				}
-				return -1;
-			}
-			statbuf = &st;
-		}
-		int bit = 0;
-		if( statbuf->st_uid == geteuid() ) {
-			bit |= S_IXUSR;
-		}
-		else if( statbuf->st_gid == getegid() ) {
-			bit |= S_IXGRP;
-		}
-		else {
-			bit |= S_IXOTH;
-		}
-		if (!(statbuf->st_mode & bit)) {
-			errno = EACCES;
-			return -1;
-		}
+		int back;
+		back = open('.', O_RDONLY);
+		if (chdir(path) != 0)
+			return -1;     /* errno was set by chdir() */
+
+		/* chdir succeeded, so this effective user has access */
+		fchdir(back);
 	}
 
 	return 0;


-- 
       David Champion â dgc@xxxxxxxxxxxx â University of Chicago
Enrico Fermi Institute â Computation Institute â USATLAS Midwest Tier 2
                      OSG Connect â CI Connect