klibc - Mar 2020 - [klibc:update-dash] dash: builtin: Use test_access from NetBSD when faccessat is unavailable

Commit-ID:  5c8d9d4456c47d03d6a71db78488f208a9f8b084
Gitweb:    
http://git.kernel.org/?p=libs/klibc/klibc.git;a=commit;h=5c8d9d4456c47d03d6a71db78488f208a9f8b084
Author:     Herbert Xu <herbert at gondor.apana.org.au>
AuthorDate: Sun, 20 May 2018 01:30:49 +0800
Committer:  Ben Hutchings <ben at decadent.org.uk>
CommitDate: Sat, 28 Mar 2020 21:42:55 +0000

[klibc] dash: builtin: Use test_access from NetBSD when faccessat is unavailable

[ dash commit 03cbaba30d7f6e4f89b6e881b6b909cb45924025 ]

This patch adds the test_access code from NetBSD when faccess is
unavailable.  The code has been modified so that root can always
read/write any file.

Signed-off-by: Herbert Xu <herbert at gondor.apana.org.au>
Signed-off-by: Ben Hutchings <ben at decadent.org.uk>

---
 usr/dash/bltin/test.c | 203 ++++++++++++++++++++++++++++++++++++++------------
 1 file changed, 157 insertions(+), 46 deletions(-)

diff --git a/usr/dash/bltin/test.c b/usr/dash/bltin/test.c
index d1458df3..b7188df7 100644
--- a/usr/dash/bltin/test.c
+++ b/usr/dash/bltin/test.c
@@ -151,8 +151,7 @@ static int equalf(const char *, const char *);
 #ifdef HAVE_FACCESSAT
 static int test_file_access(const char *, int);
 #else
-static int test_st_mode(const struct stat64 *, int);
-static int bash_group_member(gid_t);
+static int test_access(const struct stat64 *, int);
 #endif
 
 #ifdef HAVE_FACCESSAT
@@ -397,11 +396,11 @@ filstat(char *nm, enum token mode)
 	switch (mode) {
 #ifndef HAVE_FACCESSAT
 	case FILRD:
-		return test_st_mode(&s, R_OK);
+		return test_access(&s, R_OK);
 	case FILWR:
-		return test_st_mode(&s, W_OK);
+		return test_access(&s, W_OK);
 	case FILEX:
-		return test_st_mode(&s, X_OK);
+		return test_access(&s, X_OK);
 #endif
 	case FILEXIST:
 		return 1;
@@ -537,53 +536,165 @@ static int test_file_access(const char *path, int mode)
 }
 #else	/* HAVE_FACCESSAT */
 /*
- * Similar to what access(2) does, but uses the effective uid and gid.
- * Doesn't make the mistake of telling root that any file is executable.
- * Returns non-zero if the file is accessible.
+ * The manual, and IEEE POSIX 1003.2, suggests this should check the mode bits,
+ * not use access():
+ *
+ *	True shall indicate only that the write flag is on.  The file is not
+ *	writable on a read-only file system even if this test indicates true.
+ *
+ * Unfortunately IEEE POSIX 1003.1-2001, as quoted in SuSv3, says only:
+ *
+ *	True shall indicate that permission to read from file will be granted,
+ *	as defined in "File Read, Write, and Creation".
+ *
+ * and that section says:
+ *
+ *	When a file is to be read or written, the file shall be opened with an
+ *	access mode corresponding to the operation to be performed.  If file
+ *	access permissions deny access, the requested operation shall fail.
+ *
+ * and of course access permissions are described as one might expect:
+ *
+ *     * If a process has the appropriate privilege:
+ *
+ *        * If read, write, or directory search permission is requested,
+ *          access shall be granted.
+ *
+ *        * If execute permission is requested, access shall be granted if
+ *          execute permission is granted to at least one user by the file
+ *          permission bits or by an alternate access control mechanism;
+ *          otherwise, access shall be denied.
+ *
+ *   * Otherwise:
+ *
+ *        * The file permission bits of a file contain read, write, and
+ *          execute/search permissions for the file owner class, file group
+ *          class, and file other class.
+ *
+ *        * Access shall be granted if an alternate access control mechanism
+ *          is not enabled and the requested access permission bit is set for
+ *          the class (file owner class, file group class, or file other class)
+ *          to which the process belongs, or if an alternate access control
+ *          mechanism is enabled and it allows the requested access; otherwise,
+ *          access shall be denied.
+ *
+ * and when I first read this I thought:  surely we can't go about using
+ * open(O_WRONLY) to try this test!  However the POSIX 1003.1-2001 Rationale
+ * section for test does in fact say:
+ *
+ *	On historical BSD systems, test -w directory always returned false
+ *	because test tried to open the directory for writing, which always
+ *	fails.
+ *
+ * and indeed this is in fact true for Seventh Edition UNIX, UNIX 32V, and UNIX
+ * System III, and thus presumably also for BSD up to and including 4.3.
+ *
+ * Secondly I remembered why using open() and/or access() are bogus.  They
+ * don't work right for detecting read and write permissions bits when
called
+ * by root.
+ *
+ * Interestingly the 'test' in 4.4BSD was closer to correct (as per
+ * 1003.2-1992) and it was implemented efficiently with stat() instead of
+ * open().
+ *
+ * This was apparently broken in NetBSD around about 1994/06/30 when the old
+ * 4.4BSD implementation was replaced with a (arguably much better coded)
+ * implementation derived from pdksh.
+ *
+ * Note that modern pdksh is yet different again, but still not correct, at
+ * least not w.r.t. 1003.2-1992.
+ *
+ * As I think more about it and read more of the related IEEE docs I don't
like
+ * that wording about 'test -r' and 'test -w' in 1003.1-2001 at
all.  I very
+ * much prefer the original wording in 1003.2-1992.  It is much more useful,
+ * and so that's what I've implemented.
+ *
+ * (Note that a strictly conforming implementation of 1003.1-2001 is in fact
+ * totally useless for the case in question since its 'test -w' and
'test -r'
+ * can never fail for root for any existing files, i.e. files for which
'test
+ * -e' succeeds.)
+ *
+ * The rationale for 1003.1-2001 suggests that the wording was
"clarified" in
+ * 1003.1-2001 to align with the 1003.2b draft.  1003.2b Draft 12 (July 1999),
+ * which is the latest copy I have, does carry the same suggested wording as is
+ * in 1003.1-2001, with its rationale saying:
+ *
+ * 	This change is a clarification and is the result of interpretation
+ * 	request PASC 1003.2-92 #23 submitted for IEEE Std 1003.2-1992.
+ *
+ * That interpretation can be found here:
+ *
+ *   http://www.pasc.org/interps/unofficial/db/p1003.2/pasc-1003.2-23.html
+ *
+ * Not terribly helpful, unfortunately.  I wonder who that fence sitter was.
+ *
+ * Worse, IMVNSHO, I think the authors of 1003.2b-D12 have mis-interpreted the
+ * PASC interpretation and appear to be gone against at least one widely used
+ * implementation (namely 4.4BSD).  The problem is that for file access by root
+ * this means that if test '-r' and '-w' are to behave as if
open() were called
+ * then there's no way for a shell script running as root to check if a
file
+ * has certain access bits set other than by the grotty means of interpreting
+ * the output of 'ls -l'.  This was widely considered to be a bug in
V7's
+ * "test" and is, I believe, one of the reasons why direct use of
access() was
+ * avoided in some more recent implementations!
+ *
+ * I have always interpreted '-r' to match '-w' and
'-x' as per the original
+ * wording in 1003.2-1992, not the other way around.  I think 1003.2b goes much
+ * too far the wrong way without any valid rationale and that it's best if
we
+ * stick with 1003.2-1992 and test the flags, and not mimic the behaviour of
+ * open() since we already know very well how it will work -- existance of the
+ * file is all that matters to open() for root.
+ *
+ * Unfortunately the SVID is no help at all (which is, I guess, partly why
+ * we're in this mess in the first place :-).
+ *
+ * The SysV implementation (at least in the 'test' builtin in /bin/sh)
does use
+ * access(name, 2) even though it also goes to much greater lengths for
'-x'
+ * matching the 1003.2-1992 definition (which is no doubt where that definition
+ * came from).
+ *
+ * The ksh93 implementation uses access() for '-r' and '-w' if
+ * (euid==uid&&egid==gid), but uses st_mode for '-x' iff
running as root.
+ * i.e. it does strictly conform to 1003.1-2001 (and presumably 1003.2b).
  */
-static int
-test_st_mode(const struct stat64 *st, int mode)
+static int test_access(const struct stat64 *sp, int stmode)
 {
-	int euid = geteuid();
+	gid_t *groups;
+	register int n;
+	uid_t euid;
+	int maxgroups;
 
+	/*
+	 * I suppose we could use access() if not running as root and if we are
+	 * running with ((euid == uid) && (egid == gid)), but we've
already
+	 * done the stat() so we might as well just test the permissions
+	 * directly instead of asking the kernel to do it....
+	 */
+	euid = geteuid();
 	if (euid == 0) {
-		/* Root can read or write any file. */
-		if (mode != X_OK)
+		if (stmode != X_OK)
 			return 1;
 
-		/* Root can execute any file that has any one of the execute
-		   bits set. */
-		mode = S_IXUSR | S_IXGRP | S_IXOTH;
-	} else if (st->st_uid == euid)
-		mode <<= 6;
-	else if (bash_group_member(st->st_gid))
-		mode <<= 3;
-
-	return st->st_mode & mode;
-}
-
-/* Return non-zero if GID is one that we have in our groups list. */
-static int
-bash_group_member(gid_t gid)
-{
-	register int i;
-	gid_t *group_array;
-	int ngroups;
-
-	/* Short-circuit if possible, maybe saving a call to getgroups(). */
-	if (gid == getgid() || gid == getegid())
-		return (1);
-
-	ngroups = getgroups(0, NULL);
-	group_array = stalloc(ngroups * sizeof(gid_t));
-	if ((getgroups(ngroups, group_array)) != ngroups)
-		return (0);
-
-	/* Search through the list looking for GID. */
-	for (i = 0; i < ngroups; i++)
-		if (gid == group_array[i])
-			return (1);
+		/* any bit is good enough */
+		stmode = (stmode << 6) | (stmode << 3) | stmode;
+	} else if (sp->st_uid == euid)
+		stmode <<= 6;
+	else if (sp->st_gid == getegid())
+		stmode <<= 3;
+	else {
+		/* XXX stolen almost verbatim from ksh93.... */
+		/* on some systems you can be in several groups */
+		maxgroups = getgroups(0, NULL);
+		groups = stalloc(maxgroups * sizeof(*groups));
+		n = getgroups(maxgroups, groups);
+		while (--n >= 0) {
+			if (groups[n] == sp->st_gid) {
+				stmode <<= 3;
+				break;
+			}
+		}
+	}
 
-	return (0);
+	return sp->st_mode & stmode;
 }
 #endif	/* HAVE_FACCESSAT */