All of lore.kernel.org
 help / color / mirror / Atom feed
* [LTP] [PATCH 1/3] syscalls/creat08: Convert to new API
@ 2021-08-06 15:45 Martin Doucha
  2021-08-06 15:45 ` [LTP] [PATCH 2/3] syscalls/open10: " Martin Doucha
                   ` (2 more replies)
  0 siblings, 3 replies; 13+ messages in thread
From: Martin Doucha @ 2021-08-06 15:45 UTC (permalink / raw)
  To: ltp

Signed-off-by: Martin Doucha <mdoucha@suse.cz>
---

creat08 and open10 are nearly identical tests except for validating creat() and
open() respectively. I've decided to keep them separate and add a new CVE test
which was supposed to replace one removed check in both tests.

 testcases/kernel/syscalls/creat/creat08.c | 536 +++++-----------------
 1 file changed, 111 insertions(+), 425 deletions(-)

diff --git a/testcases/kernel/syscalls/creat/creat08.c b/testcases/kernel/syscalls/creat/creat08.c
index d22558ac3..1ae531f23 100644
--- a/testcases/kernel/syscalls/creat/creat08.c
+++ b/testcases/kernel/syscalls/creat/creat08.c
@@ -1,456 +1,142 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
 /*
+ * Copyright (c) International Business Machines  Corp., 2002
+ *     Ported from SPIE by Airong Zhang <zhanga@us.ibm.com>
+ * Copyright (c) 2021 SUSE LLC <mdoucha@suse.cz>
  *
- *   Copyright (c) International Business Machines  Corp., 2002
- *
- *   This program is free software;  you can redistribute it and/or modify
- *   it under the terms of the GNU General Public License as published by
- *   the Free Software Foundation; either version 2 of the License, or
- *   (at your option) any later version.
- *
- *   This program is distributed in the hope that it will be useful,
- *   but WITHOUT ANY WARRANTY;  without even the implied warranty of
- *   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See
- *   the GNU General Public License for more details.
- *
- *   You should have received a copy of the GNU General Public License
- *   along with this program;  if not, write to the Free Software
- *   Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
- */
-
-/*
- * NAME
- *	creat08.c - Verifies that the group ID and setgid bit are
- *		   set correctly when a new file is created.
- *		   (ported from SPIE, section2/iosuite/creat5.c,
- *		    by Airong Zhang <zhanga@us.ibm.com>)
- * CALLS
- *	creat
- *
- * ALGORITHM
- *	Create two directories, one with the group ID of this process
- *	and the setgid bit not set, and the other with a group ID
- *	other than that of this process and with the setgid bit set.
- *	In each directory, create a file with and without the setgid
- *	bit set in the creation modes. Verify that the modes and group
- *	ID are correct on each of the 4 files.
- *	As root, create a file with the setgid bit on in the
- *	directory with the setgid bit.
- *	This tests the SVID3 create group semantics.
- *
- * USAGE
- *	creat08
- *
- * RESTRICTIONS
- *
+ * Verify that the group ID and setgid bit are set correctly when a new file
+ * is created.
  */
 
-#include <stdio.h>		/* needed by testhead.h         */
+#include <stdlib.h>
 #include <sys/types.h>
-#include <sys/stat.h>
-#include <fcntl.h>
-#include <errno.h>
-#include <grp.h>
 #include <pwd.h>
-#include "test.h"
-#include "safe_macros.h"
-
-char *TCID = "creat08";
-int TST_TOTAL = 1;
-int local_flag;
+#include "tst_test.h"
 
-#define PASSED 1
-#define FAILED 0
+#define MODE_RWX        0777
+#define MODE_SGID       (S_ISGID|0777)
 
-#define MODE_RWX        (S_IRWXU|S_IRWXG|S_IRWXO)
-#define MODE_SGID       (S_ISGID|S_IRWXU|S_IRWXG|S_IRWXO)
-#define DIR_A_TEMP	"testdir.A.%d"
-#define DIR_B_TEMP	"testdir.B.%d"
-#define SETGID		"setgid"
-#define NOSETGID	"nosetgid"
-#define ROOT_SETGID	"root_setgid"
-#define	MSGSIZE		150
+#define DIR_A		"dir_a"
+#define DIR_B		"dir_b"
+#define SETGID_A	DIR_A "/setgid"
+#define NOSETGID_A	DIR_A "/nosetgid"
+#define SETGID_B	DIR_B "/setgid"
+#define NOSETGID_B	DIR_B "/nosetgid"
+#define ROOT_SETGID	DIR_B "/root_setgid"
 
-static void tst_cleanup(void);
-static void cleanup(void);
-static void setup(void);
+static char *tmpdir;
+static uid_t orig_uid, nobody_uid;
+static gid_t nobody_gid, bin_gid;
+static int fd = -1;
 
-static char DIR_A[MSGSIZE], DIR_B[MSGSIZE];
-static char setgid_A[MSGSIZE], nosetgid_A[MSGSIZE];
-static char setgid_B[MSGSIZE], nosetgid_B[MSGSIZE], root_setgid_B[MSGSIZE];
+static void setup(void)
+{
+	struct passwd *ltpuser = SAFE_GETPWNAM("nobody");
+	struct group *ltpgroup = SAFE_GETGRNAM_FALLBACK("nobody", "nogroup");
+
+	orig_uid = getuid();
+	nobody_uid = ltpuser->pw_uid;
+	nobody_gid = ltpgroup->gr_gid;
+	ltpgroup = SAFE_GETGRNAM("bin");
+	bin_gid = ltpgroup->gr_gid;
+	tmpdir = tst_get_tmpdir();
+}
 
-int main(int ac, char **av)
+static void file_test(const char *name, mode_t mode, int sgid, gid_t gid)
 {
 	struct stat buf;
-	struct group *group;
-	struct passwd *user1;
-	gid_t group1_gid, group2_gid, mygid;
-	uid_t save_myuid, user1_uid;
-	pid_t mypid;
-
-	int fd;
-	int lc;
-
-	tst_parse_opts(ac, av, NULL, NULL);
-
-	setup();
-
-	for (lc = 0; TEST_LOOPING(lc); lc++) {
-
-		local_flag = PASSED;
-
-		save_myuid = getuid();
-		mypid = getpid();
-		sprintf(DIR_A, DIR_A_TEMP, mypid);
-		sprintf(DIR_B, DIR_B_TEMP, mypid);
-		sprintf(setgid_A, "%s/%s", DIR_A, SETGID);
-		sprintf(nosetgid_A, "%s/%s", DIR_A, NOSETGID);
-		sprintf(setgid_B, "%s/%s", DIR_B, SETGID);
-		sprintf(nosetgid_B, "%s/%s", DIR_B, NOSETGID);
-		sprintf(root_setgid_B, "%s/%s", DIR_B, ROOT_SETGID);
-
-		/* Get the uid of user1 */
-		if ((user1 = getpwnam("nobody")) == NULL) {
-			tst_brkm(TBROK | TERRNO, NULL,
-				 "getpwnam(\"nobody\") failed");
-		}
-
-		user1_uid = user1->pw_uid;
-
-		/*
-		 * Get the group IDs of group1 and group2.
-		 */
-		if ((group = getgrnam("nobody")) == NULL) {
-			if ((group = getgrnam("nogroup")) == NULL) {
-				tst_brkm(TBROK | TERRNO, cleanup,
-					 "getgrnam(\"nobody\") and "
-					 "getgrnam(\"nogroup\") failed");
-			}
-		}
-		group1_gid = group->gr_gid;
-		if ((group = getgrnam("bin")) == NULL) {
-			tst_brkm(TBROK | TERRNO, cleanup,
-				 "getgrnam(\"bin\") failed");
-		}
-		group2_gid = group->gr_gid;
-
-/*--------------------------------------------------------------*/
-/* Block0: Set up the parent directories			*/
-/*--------------------------------------------------------------*/
-		/*
-		 * Create a directory with group id the same as this process
-		 * and with no setgid bit.
-		 */
-		if (mkdir(DIR_A, MODE_RWX) == -1) {
-			tst_resm(TFAIL, "Creation of %s failed", DIR_A);
-			local_flag = FAILED;
-		}
-
-		if (chown(DIR_A, user1_uid, group2_gid) == -1) {
-			tst_resm(TFAIL, "Chown of %s failed", DIR_A);
-			local_flag = FAILED;
-		}
-
-		if (stat(DIR_A, &buf) == -1) {
-			tst_resm(TFAIL, "Stat of %s failed", DIR_A);
-			local_flag = FAILED;
-		}
-
-		/* Verify modes */
-		if (buf.st_mode & S_ISGID) {
-			tst_resm(TFAIL, "%s: Incorrect modes, setgid bit set",
-				 DIR_A);
-			local_flag = FAILED;
-		}
-
-		/* Verify group ID */
-		if (buf.st_gid != group2_gid) {
-			tst_resm(TFAIL, "%s: Incorrect group", DIR_A);
-			tst_resm(TINFO, "got %u and %u", buf.st_gid,
-				 group2_gid);
-			local_flag = FAILED;
-		}
-
-		/*
-		 * Create a directory with group id different from that of
-		 * this process and with the setgid bit set.
-		 */
-		if (mkdir(DIR_B, MODE_RWX) == -1) {
-			tst_resm(TFAIL, "Creation of %s failed", DIR_B);
-			local_flag = FAILED;
-		}
-
-		if (chown(DIR_B, user1_uid, group2_gid) == -1) {
-			tst_resm(TFAIL, "Chown of %s failed", DIR_B);
-			local_flag = FAILED;
-		}
-
-		if (chmod(DIR_B, MODE_SGID) == -1) {
-			tst_resm(TFAIL, "Chmod of %s failed", DIR_B);
-			local_flag = FAILED;
-		}
-
-		if (stat(DIR_B, &buf) == -1) {
-			tst_resm(TFAIL, "Stat of %s failed", DIR_B);
-			local_flag = FAILED;
-		}
-
-		/* Verify modes */
-		if (!(buf.st_mode & S_ISGID)) {
-			tst_resm(TFAIL,
-				 "%s: Incorrect modes, setgid bit not set",
-				 DIR_B);
-			local_flag = FAILED;
-		}
-
-		/* Verify group ID */
-		if (buf.st_gid != group2_gid) {
-			tst_resm(TFAIL, "%s: Incorrect group", DIR_B);
-			tst_resm(TINFO, "got %u and %u", buf.st_gid,
-				 group2_gid);
-			local_flag = FAILED;
-		}
-
-		if (local_flag == PASSED) {
-			tst_resm(TPASS, "Test passed in block0.");
-		} else {
-			tst_resm(TFAIL, "Test failed in block0.");
-		}
 
-		local_flag = PASSED;
+	fd = SAFE_CREAT(name, mode);
+	SAFE_STAT(name, &buf);
+	SAFE_CLOSE(fd);
 
-/*--------------------------------------------------------------*/
-/* Block1: Create two files in testdir.A, one with the setgid   */
-/*         bit set in the creation modes and the other without. */
-/*	   Both should inherit the group ID of the process and  */
-/*	   maintain the setgid bit as specified in the creation */
-/*	   modes.                                               */
-/*--------------------------------------------------------------*/
-		/*
-		 * Now become user1, group1
-		 */
-		if (setgid(group1_gid) == -1) {
-			tst_resm(TINFO,
-				 "Unable to set process group ID to group1");
-		}
-
-		if (setreuid(-1, user1_uid) == -1) {
-			tst_resm(TINFO, "Unable to set process uid to user1");
-		}
-		mygid = getgid();
-
-		/*
-		 * Create the file with setgid not set
-		 */
-		fd = open(nosetgid_A, O_CREAT | O_EXCL | O_RDWR, MODE_RWX);
-		if (fd == -1) {
-			tst_resm(TFAIL, "Creation of %s failed", nosetgid_A);
-			local_flag = FAILED;
-		}
-
-		if (stat(nosetgid_A, &buf) == -1) {
-			tst_resm(TFAIL, "Stat of %s failed", nosetgid_A);
-			local_flag = FAILED;
-		}
-
-		/* Verify modes */
-		if (buf.st_mode & S_ISGID) {
-			tst_resm(TFAIL, "%s: Incorrect modes, setgid bit set",
-				 nosetgid_A);
-			local_flag = FAILED;
-		}
-
-		/* Verify group ID */
-		if (buf.st_gid != mygid) {
-			tst_resm(TFAIL, "%s: Incorrect group", nosetgid_A);
-			local_flag = FAILED;
-		}
-		close(fd);
-
-		/*
-		 * Create the file with setgid set
-		 */
-		fd = open(setgid_A, O_CREAT | O_EXCL | O_RDWR, MODE_SGID);
-		if (fd == -1) {
-			tst_resm(TFAIL, "Creation of %s failed", setgid_A);
-			local_flag = FAILED;
-		}
-
-		if (stat(setgid_A, &buf) == -1) {
-			tst_resm(TFAIL, "Stat of %s failed", setgid_A);
-			local_flag = FAILED;
-		}
-
-		/* Verify modes */
-		if (!(buf.st_mode & S_ISGID)) {
-			tst_resm(TFAIL,
-				 "%s: Incorrect modes, setgid bit not set",
-				 setgid_A);
-			local_flag = FAILED;
-		}
-
-		/* Verify group ID */
-		if (buf.st_gid != mygid) {
-			tst_resm(TFAIL, "%s: Incorrect group", setgid_A);
-			tst_resm(TINFO, "got %u and %u", buf.st_gid, mygid);
-			local_flag = FAILED;
-		}
-		close(fd);
-
-		if (local_flag == PASSED) {
-			tst_resm(TPASS, "Test passed in block1.");
-		} else {
-			tst_resm(TFAIL, "Test failed in block1.");
-		}
-
-		local_flag = PASSED;
-
-/*--------------------------------------------------------------*/
-/* Block2: Create two files in testdir.B, one with the setgid   */
-/*         bit set in the creation modes and the other without. */
-/*	   Both should inherit the group ID of the parent       */
-/*	   directory, group2.                                   */
-/*--------------------------------------------------------------*/
-		/*
-		 * Create the file with setgid not set
-		 */
-		fd = creat(nosetgid_B, MODE_RWX);
-		if (fd == -1) {
-			tst_resm(TFAIL, "Creation of %s failed", nosetgid_B);
-			local_flag = FAILED;
-		}
-
-		if (stat(nosetgid_B, &buf) == -1) {
-			tst_resm(TFAIL, "Stat of %s failed", nosetgid_B);
-			local_flag = FAILED;
-		}
-
-		/* Verify modes */
-		if (buf.st_mode & S_ISGID) {
-			tst_resm(TFAIL,
-				 "%s: Incorrect modes, setgid bit should not be set",
-				 nosetgid_B);
-			local_flag = FAILED;
-		}
-
-		/* Verify group ID */
-		if (buf.st_gid != group2_gid) {
-			tst_resm(TFAIL, "%s: Incorrect group", nosetgid_B);
-			local_flag = FAILED;
-		}
-		close(fd);
-
-		/*
-		 * Create the file with setgid set
-		 */
-		fd = creat(setgid_B, MODE_SGID);
-		if (fd == -1) {
-			tst_resm(TFAIL, "Creation of %s failed", setgid_B);
-			local_flag = FAILED;
-		}
-
-		if (stat(setgid_B, &buf) == -1) {
-			tst_resm(TFAIL, "Stat of %s failed", setgid_B);
-			local_flag = FAILED;
-		}
-
-		/* Verify group ID */
-		if (buf.st_gid != group2_gid) {
-			tst_resm(TFAIL, "%s: Incorrect group", setgid_B);
-			tst_resm(TFAIL, "got %u and %u", buf.st_gid,
-				 group2_gid);
-			local_flag = FAILED;
-		}
+	if (buf.st_gid != gid) {
+		tst_res(TFAIL, "%s: Incorrect group, %u != %u", name,
+			buf.st_gid, gid);
+	} else {
+		tst_res(TPASS, "%s: Owned by correct group", name);
+	}
 
-		/*
-		 * Skip S_ISGID check
-		 * 0fa3ecd87848 ("Fix up non-directory creation in SGID directories")
-		 * clears S_ISGID for files created by non-group members
-		 */
+	if (sgid < 0) {
+		tst_res(TINFO, "%s: Skipping setgid bit check", name);
+		return;
+	}
 
-		close(fd);
+	if (buf.st_mode & S_ISGID)
+		tst_res(sgid ? TPASS : TFAIL, "%s: Setgid bit is set", name);
+	else
+		tst_res(sgid ? TFAIL : TPASS, "%s: Setgid bit not set", name);
+}
 
-		if (local_flag == PASSED) {
-			tst_resm(TPASS, "Test passed in block2.");
-		} else {
-			tst_resm(TFAIL, "Test failed in block2.");
-		}
+static void run(void)
+{
+	struct stat buf;
 
-		local_flag = PASSED;
-/*--------------------------------------------------------------*/
-/* Block3: Create a file in testdir.B, with the setgid bit set  */
-/*	   in the creation modes and do so as root. The file    */
-/*	   should inherit the group ID of the parent directory, */
-/*	   group2 and should have the setgid bit set.		*/
-/*--------------------------------------------------------------*/
-		/* Become root again */
-		if (setreuid(-1, save_myuid) == -1) {
-			tst_resm(TFAIL | TERRNO,
-				 "Changing back to root failed");
-			local_flag = FAILED;
-		}
+	/* Create directories and set permissions */
+	SAFE_MKDIR(DIR_A, MODE_RWX);
+	SAFE_CHOWN(DIR_A, nobody_uid, bin_gid);
+	SAFE_STAT(DIR_A, &buf);
 
-		/* Create the file with setgid set */
-		fd = creat(root_setgid_B, MODE_SGID);
-		if (fd == -1) {
-			tst_resm(TFAIL, "Creation of %s failed", root_setgid_B);
-			local_flag = FAILED;
-		}
+	if (buf.st_mode & S_ISGID)
+		tst_brk(TBROK, "%s: Setgid bit is set", DIR_A);
 
-		if (stat(root_setgid_B, &buf) == -1) {
-			tst_resm(TFAIL, "Stat of %s failed", root_setgid_B);
-			local_flag = FAILED;
-		}
+	if (buf.st_gid != bin_gid) {
+		tst_brk(TBROK, "%s: Incorrect group, %u != %u", DIR_A,
+			buf.st_gid, bin_gid);
+	}
 
-		/* Verify modes */
-		if (!(buf.st_mode & S_ISGID)) {
-			tst_resm(TFAIL,
-				 "%s: Incorrect modes, setgid bit not set",
-				 root_setgid_B);
-			local_flag = FAILED;
-		}
+	SAFE_MKDIR(DIR_B, MODE_RWX);
+	SAFE_CHOWN(DIR_B, nobody_uid, bin_gid);
+	SAFE_CHMOD(DIR_B, MODE_SGID);
+	SAFE_STAT(DIR_B, &buf);
 
-		/* Verify group ID */
-		if (buf.st_gid != group2_gid) {
-			tst_resm(TFAIL, "%s: Incorrect group", root_setgid_B);
-			tst_resm(TINFO, "got %u and %u", buf.st_gid,
-				 group2_gid);
-			local_flag = FAILED;
-		}
-		close(fd);
+	if (!(buf.st_mode & S_ISGID))
+		tst_brk(TBROK, "%s: Setgid bit not set", DIR_B);
 
-		if (local_flag == PASSED) {
-			tst_resm(TPASS, "Test passed in block3");
-		} else {
-			tst_resm(TFAIL, "Test failed in block3");
-		}
-		tst_cleanup();
+	if (buf.st_gid != bin_gid) {
+		tst_brk(TBROK, "%s: Incorrect group, %u != %u", DIR_B,
+			buf.st_gid, bin_gid);
 	}
-	cleanup();
-	tst_exit();
-}
 
-static void setup(void)
-{
-	tst_require_root();
-	tst_tmpdir();
-}
-
-static void tst_cleanup(void)
-{
-	if (unlink(setgid_A) == -1) {
-		tst_resm(TBROK, "%s failed", setgid_A);
-	}
-	if (unlink(nosetgid_A) == -1) {
-		tst_resm(TBROK, "unlink %s failed", nosetgid_A);
-	}
-	SAFE_RMDIR(NULL, DIR_A);
-	SAFE_UNLINK(NULL, setgid_B);
-	SAFE_UNLINK(NULL, root_setgid_B);
-	SAFE_UNLINK(NULL, nosetgid_B);
-	SAFE_RMDIR(NULL, DIR_B);
+	/* Switch to user nobody and create two files in DIR_A */
+	/* Both files should inherit GID from the process */
+	SAFE_SETGID(nobody_gid);
+	SAFE_SETREUID(-1, nobody_uid);
+	file_test(NOSETGID_A, MODE_RWX, 0, nobody_gid);
+	file_test(SETGID_A, MODE_SGID, 1, nobody_gid);
+
+	/* Create two files in DIR_B and validate owner and permissions */
+	/* Both files should inherit GID from the parent directory */
+	file_test(NOSETGID_B, MODE_RWX, 0, bin_gid);
+	/*
+	 * CVE 2018-13405 (privilege escalation using setgid bit) has its
+	 * own test, skip setgid check here
+	 */
+	file_test(SETGID_B, MODE_SGID, -1, bin_gid);
+
+	/* Switch back to root UID and create a file in DIR_B */
+	/* The file should inherid GID from parent directory */
+	SAFE_SETREUID(-1, orig_uid);
+	file_test(ROOT_SETGID, MODE_SGID, 1, bin_gid);
+
+	/* Cleanup between loops */
+	tst_purge_dir(tmpdir);
 }
 
 static void cleanup(void)
 {
-	tst_rmdir();
+	SAFE_SETREUID(-1, orig_uid);
+
+	if (fd >= 0)
+		SAFE_CLOSE(fd);
+
+	free(tmpdir);
 }
+
+static struct tst_test test = {
+	.test_all = run,
+	.setup = setup,
+	.cleanup = cleanup,
+	.needs_root = 1,
+	.needs_tmpdir = 1,
+};
-- 
2.32.0


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [LTP] [PATCH 2/3] syscalls/open10: Convert to new API
  2021-08-06 15:45 [LTP] [PATCH 1/3] syscalls/creat08: Convert to new API Martin Doucha
@ 2021-08-06 15:45 ` Martin Doucha
  2021-08-06 15:45 ` [LTP] [PATCH 3/3] Add test for CVE 2018-13405 Martin Doucha
  2021-08-13 13:49 ` [LTP] [PATCH 1/3] syscalls/creat08: Convert to new API Cyril Hrubis
  2 siblings, 0 replies; 13+ messages in thread
From: Martin Doucha @ 2021-08-06 15:45 UTC (permalink / raw)
  To: ltp

Signed-off-by: Martin Doucha <mdoucha@suse.cz>
---
 testcases/kernel/syscalls/open/open10.c | 540 +++++-------------------
 1 file changed, 110 insertions(+), 430 deletions(-)

diff --git a/testcases/kernel/syscalls/open/open10.c b/testcases/kernel/syscalls/open/open10.c
index 14feec9e1..2a0d21dd2 100644
--- a/testcases/kernel/syscalls/open/open10.c
+++ b/testcases/kernel/syscalls/open/open10.c
@@ -1,461 +1,141 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
 /*
+ * Copyright (c) International Business Machines  Corp., 2002
+ * Copyright (c) 2021 SUSE LLC <mdoucha@suse.cz>
  *
- *   Copyright (c) International Business Machines  Corp., 2002
- *
- *   This program is free software;  you can redistribute it and/or modify
- *   it under the terms of the GNU General Public License as published by
- *   the Free Software Foundation; either version 2 of the License, or
- *   (at your option) any later version.
- *
- *   This program is distributed in the hope that it will be useful,
- *   but WITHOUT ANY WARRANTY;  without even the implied warranty of
- *   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See
- *   the GNU General Public License for more details.
- *
- *   You should have received a copy of the GNU General Public License
- *   along with this program;  if not, write to the Free Software
- *   Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
+ * Verify that the group ID and setgid bit are set correctly when a new file
+ * is created.
  */
 
-/*
- * Description:
- *	Verifies that the group ID and setgid bit are
- *	set correctly when a new file is created using open.
- *
- * ALGORITHM
- *	Create two directories, one with the group ID of this process
- *	and the setgid bit not set, and the other with a group ID
- *	other than that of this process and with the setgid bit set.
- *	In each directory, create a file with and without the setgid
- *	bit set in the creation modes. Verify that the modes and group
- *	ID are correct on each of the 4 files.
- *	As root, create a file with the setgid bit on in the
- *	directory with the setgid bit.
- *	This tests the SVID3 create group semantics.
- */
-
-#include <stdio.h>
+#include <stdlib.h>
 #include <sys/types.h>
-#include <sys/stat.h>
-#include <fcntl.h>
-#include <errno.h>
-#include <grp.h>
 #include <pwd.h>
-#include "test.h"
+#include "tst_test.h"
 
-char *TCID = "open10";
-int TST_TOTAL = 1;
-static int local_flag;
+#define MODE_RWX        0777
+#define MODE_SGID       (S_ISGID|0777)
 
-#define PASSED 1
-#define FAILED 0
+#define DIR_A		"dir_a"
+#define DIR_B		"dir_b"
+#define SETGID_A	DIR_A "/setgid"
+#define NOSETGID_A	DIR_A "/nosetgid"
+#define SETGID_B	DIR_B "/setgid"
+#define NOSETGID_B	DIR_B "/nosetgid"
+#define ROOT_SETGID	DIR_B "/root_setgid"
 
-#define MODE_RWX        (S_IRWXU | S_IRWXG | S_IRWXO)
-#define MODE_SGID       (S_ISGID | S_IRWXU | S_IRWXG | S_IRWXO)
-#define DIR_A_TEMP	"open10.testdir.A.%d"
-#define DIR_B_TEMP	"open10.testdir.B.%d"
-#define SETGID		"setgid"
-#define NOSETGID	"nosetgid"
-#define ROOT_SETGID	"root_setgid"
-#define	MSGSIZE		150
+static char *tmpdir;
+static uid_t orig_uid, nobody_uid;
+static gid_t nobody_gid, bin_gid;
+static int fd = -1;
 
-static void setup(void);
-static void cleanup(void);
+static void setup(void)
+{
+	struct passwd *ltpuser = SAFE_GETPWNAM("nobody");
+	struct group *ltpgroup = SAFE_GETGRNAM_FALLBACK("nobody", "nogroup");
+
+	orig_uid = getuid();
+	nobody_uid = ltpuser->pw_uid;
+	nobody_gid = ltpgroup->gr_gid;
+	ltpgroup = SAFE_GETGRNAM("bin");
+	bin_gid = ltpgroup->gr_gid;
+	tmpdir = tst_get_tmpdir();
+}
 
-int main(int ac, char *av[])
+static void file_test(const char *name, mode_t mode, int sgid, gid_t gid)
 {
-	int ret;
 	struct stat buf;
-	struct group *group;
-	struct passwd *user1;
-	char DIR_A[MSGSIZE], DIR_B[MSGSIZE];
-	char setgid_A[MSGSIZE], nosetgid_A[MSGSIZE];
-	char setgid_B[MSGSIZE], nosetgid_B[MSGSIZE], root_setgid_B[MSGSIZE];
-	gid_t group1_gid, group2_gid, mygid;
-	uid_t save_myuid, user1_uid;
-	pid_t mypid;
-
-	int lc;
-	int fail_count = 0;
-
-	tst_parse_opts(ac, av, NULL, NULL);
-
-	setup();
-
-	for (lc = 0; TEST_LOOPING(lc); lc++) {
-		local_flag = PASSED;
-
-		save_myuid = getuid();
-		mypid = getpid();
-		sprintf(DIR_A, DIR_A_TEMP, mypid);
-		sprintf(DIR_B, DIR_B_TEMP, mypid);
-		sprintf(setgid_A, "%s/%s", DIR_A, SETGID);
-		sprintf(nosetgid_A, "%s/%s", DIR_A, NOSETGID);
-		sprintf(setgid_B, "%s/%s", DIR_B, SETGID);
-		sprintf(nosetgid_B, "%s/%s", DIR_B, NOSETGID);
-		sprintf(root_setgid_B, "%s/%s", DIR_B, ROOT_SETGID);
-
-		/* Get the uid of user1 */
-		user1 = getpwnam("nobody");
-		if (user1 == NULL)
-			tst_brkm(TBROK, cleanup, "nobody not in /etc/passwd");
-
-		user1_uid = user1->pw_uid;
-
-		/*
-		 * Get the group IDs of group1 and group2.
-		 */
-		group = getgrnam("nobody");
-		if (group == NULL) {
-			group = getgrnam("nogroup");
-			if (group == NULL) {
-				tst_brkm(TBROK, cleanup,
-					 "nobody/nogroup not in /etc/group");
-			}
-		}
-		group1_gid = group->gr_gid;
-		group = getgrnam("bin");
-		if (group == NULL)
-			tst_brkm(TBROK, cleanup, "bin not in /etc/group");
-
-		group2_gid = group->gr_gid;
-
-		/*
-		 * Create a directory with group id the same as this process
-		 * and with no setgid bit.
-		 */
-		if (mkdir(DIR_A, MODE_RWX) < 0) {
-			tst_resm(TFAIL | TERRNO, "mkdir(%s) failed", DIR_A);
-			local_flag = FAILED;
-		}
-
-		if (chown(DIR_A, user1_uid, group2_gid) < 0) {
-			tst_resm(TFAIL | TERRNO, "chown(%s) failed", DIR_A);
-			local_flag = FAILED;
-		}
-
-		if (stat(DIR_A, &buf) < 0) {
-			tst_resm(TFAIL | TERRNO, "stat(%s) failed", DIR_A);
-			local_flag = FAILED;
-		}
-
-		/* Verify modes */
-		if (buf.st_mode & S_ISGID) {
-			tst_resm(TFAIL, "%s: Incorrect modes, setgid bit set",
-				 DIR_A);
-			local_flag = FAILED;
-		}
-
-		/* Verify group ID */
-		if (buf.st_gid != group2_gid) {
-			tst_resm(TFAIL, "%s: Incorrect group (got %d and %d)",
-				 DIR_A, buf.st_gid, group2_gid);
-			local_flag = FAILED;
-		}
-
-		/*
-		 * Create a directory with group id different from that of
-		 * this process and with the setgid bit set.
-		 */
-		if (mkdir(DIR_B, MODE_RWX) < 0) {
-			tst_resm(TFAIL | TERRNO, "mkdir(%s) failed", DIR_B);
-			local_flag = FAILED;
-		}
-
-		if (chown(DIR_B, user1_uid, group2_gid) < 0) {
-			tst_resm(TFAIL | TERRNO, "chown(%s) failed", DIR_B);
-			local_flag = FAILED;
-		}
-
-		if (chmod(DIR_B, MODE_SGID) < 0) {
-			tst_resm(TFAIL | TERRNO, "chmod(%s) failed", DIR_B);
-			local_flag = FAILED;
-		}
-
-		if (stat(DIR_B, &buf) < 0) {
-			tst_resm(TFAIL | TERRNO, "stat(%s) failed", DIR_B);
-			local_flag = FAILED;
-		}
-
-		/* Verify modes */
-		if (!(buf.st_mode & S_ISGID)) {
-			tst_resm(TFAIL,
-				 "%s: Incorrect modes, setgid bit not set",
-				 DIR_B);
-			local_flag = FAILED;
-		}
-
-		/* Verify group ID */
-		if (buf.st_gid != group2_gid) {
-			tst_resm(TFAIL, "%s: Incorrect group (got %d and %d)",
-				 DIR_B, buf.st_gid, group2_gid);
-			local_flag = FAILED;
-		}
-
-		if (local_flag == PASSED) {
-			tst_resm(TPASS, "Test passed in block0.");
-		} else {
-			tst_resm(TFAIL, "Test failed in block0.");
-			fail_count++;
-		}
-
-		local_flag = PASSED;
-
-		/*
-		 * Create two files in testdir.A, one with the setgid
-		 * bit set in the creation modes and the other without.
-		 * Both should inherit the group ID of the process and
-		 * maintain the setgid bit as specified in the creation
-		 * modes.
-		 */
-		if (setgid(group1_gid) < 0) {
-			tst_resm(TINFO,
-				 "Unable to set process group ID to group1");
-		}
-
-		if (setreuid(-1, user1_uid) < 0)
-			tst_resm(TINFO, "Unable to set process uid to user1");
 
-		mygid = getgid();
+	fd = SAFE_OPEN(name, O_CREAT | O_EXCL | O_RDWR, mode);
+	SAFE_CLOSE(fd);
+	SAFE_STAT(name, &buf);
 
-		/*
-		 * Create the file with setgid not set
-		 */
-		ret = open(nosetgid_A, O_CREAT | O_EXCL | O_RDWR, MODE_RWX);
-		if (ret < 0) {
-			tst_resm(TFAIL | TERRNO, "open(%s) failed", nosetgid_A);
-			local_flag = FAILED;
-		}
-		close(ret);
-
-		if (stat(nosetgid_A, &buf) < 0) {
-			tst_resm(TFAIL | TERRNO, "stat(%s) failed", nosetgid_A);
-			local_flag = FAILED;
-		}
-
-		/* Verify modes */
-		if (buf.st_mode & S_ISGID) {
-			tst_resm(TFAIL, "%s: Incorrect modes, setgid bit set",
-				 nosetgid_A);
-			local_flag = FAILED;
-		}
-
-		/* Verify group ID */
-		if (buf.st_gid != mygid) {
-			tst_resm(TFAIL, "%s: Incorrect group (got %d and %d)",
-				 nosetgid_A, buf.st_gid, mygid);
-			local_flag = FAILED;
-		}
-
-		/*
-		 * Create the file with setgid set
-		 */
-		ret = open(setgid_A, O_CREAT | O_EXCL | O_RDWR, MODE_SGID);
-		if (ret < 0) {
-			tst_resm(TFAIL | TERRNO, "open(%s) failed", setgid_A);
-			local_flag = FAILED;
-		}
-		close(ret);
-
-		if (stat(setgid_A, &buf) < 0) {
-			tst_resm(TFAIL | TERRNO, "stat(%s) failed", setgid_A);
-			local_flag = FAILED;
-		}
-
-		/* Verify modes */
-		if (!(buf.st_mode & S_ISGID)) {
-			tst_resm(TFAIL,
-				 "%s: Incorrect modes, setgid bit not set",
-				 setgid_A);
-			local_flag = FAILED;
-		}
-
-		/* Verify group ID */
-		if (buf.st_gid != mygid) {
-			tst_resm(TFAIL, "%s: Incorrect group (%d and %d)",
-				 setgid_A, buf.st_gid, mygid);
-			local_flag = FAILED;
-		}
-
-		if (local_flag == PASSED) {
-			tst_resm(TPASS, "Test passed in block1.");
-		} else {
-			tst_resm(TFAIL, "Test failed in block1.");
-			fail_count++;
-		}
-
-		local_flag = PASSED;
-
-		/*
-		 * Create two files in testdir.B, one with the setgid
-		 * bit set in the creation modes and the other without.
-		 * Both should inherit the group ID of the parent
-		 * directory, group2. Either file should have the segid
-		 * bit set in the modes.
-		 */
-		/*
-		 * Create the file with setgid not set
-		 */
-		ret = open(nosetgid_B, O_CREAT | O_EXCL | O_RDWR, MODE_RWX);
-		if (ret < 0) {
-			tst_resm(TFAIL | TERRNO, "open(%s) failed", nosetgid_B);
-			local_flag = FAILED;
-		}
-		close(ret);
-
-		if (stat(nosetgid_B, &buf) < 0) {
-			tst_resm(TFAIL | TERRNO, "stat(%s) failed", nosetgid_B);
-			local_flag = FAILED;
-		}
-
-		/* Verify modes */
-		if (buf.st_mode & S_ISGID) {
-			tst_resm(TFAIL,
-				 "%s: Incorrect modes, setgid bit should be set",
-				 nosetgid_B);
-			local_flag = FAILED;
-		}
-
-		/* Verify group ID */
-		if (buf.st_gid != group2_gid) {
-			tst_resm(TFAIL, "%s: Incorrect group (got %d and %d)",
-				 nosetgid_B, buf.st_gid, group2_gid);
-			local_flag = FAILED;
-		}
-
-		/*
-		 * Create the file with setgid set
-		 */
-		ret = open(setgid_B, O_CREAT | O_EXCL | O_RDWR, MODE_SGID);
-		if (ret < 0) {
-			tst_resm(TFAIL | TERRNO, "open(%s) failed", setgid_B);
-			local_flag = FAILED;
-		}
-		close(ret);
-
-		if (stat(setgid_B, &buf) < 0) {
-			tst_resm(TFAIL | TERRNO, "stat(%s) failed", setgid_B);
-			local_flag = FAILED;
-		}
-
-		/* Verify group ID */
-		if (buf.st_gid != group2_gid) {
-			tst_resm(TFAIL, "%s: Incorrect group (got %d and %d)",
-				 setgid_B, buf.st_gid, group2_gid);
-			local_flag = FAILED;
-		}
-
-		/*
-		 * Skip S_ISGID check
-		 * 0fa3ecd87848 ("Fix up non-directory creation in SGID directories")
-		 * clears S_ISGID for files created by non-group members
-		 */
-
-		if (local_flag == PASSED) {
-			tst_resm(TPASS, "Test passed in block2.");
-		} else {
-			tst_resm(TFAIL, "Test failed in block2.");
-			fail_count++;
-		}
-
-		local_flag = PASSED;
-
-		/*
-		 * Create a file in testdir.B, with the setgid bit set
-		 * in the creation modes and do so as root. The file
-		 * should inherit the group ID of the parent directory,
-		 * group2 and should have the setgid bit set.
-		 */
-
-		/* Become root again */
-		if (setreuid(-1, save_myuid) < 0) {
-			tst_resm(TFAIL | TERRNO,
-				 "Changing back to root failed");
-			local_flag = FAILED;
-		}
+	if (buf.st_gid != gid) {
+		tst_res(TFAIL, "%s: Incorrect group, %u != %u", name,
+			buf.st_gid, gid);
+	} else {
+		tst_res(TPASS, "%s: Owned by correct group", name);
+	}
 
-		/* Create the file with setgid set */
-		ret = open(root_setgid_B, O_CREAT | O_EXCL | O_RDWR, MODE_SGID);
-		if (ret < 0) {
-			tst_resm(TFAIL | TERRNO, "open(%s) failed",
-				 root_setgid_B);
-			local_flag = FAILED;
-		}
-		close(ret);
+	if (sgid < 0) {
+		tst_res(TINFO, "%s: Skipping setgid bit check", name);
+		return;
+	}
 
-		if (stat(root_setgid_B, &buf) < 0) {
-			tst_resm(TFAIL | TERRNO, "stat(%s) failed",
-				 root_setgid_B);
-			local_flag = FAILED;
-		}
+	if (buf.st_mode & S_ISGID)
+		tst_res(sgid ? TPASS : TFAIL, "%s: Setgid bit is set", name);
+	else
+		tst_res(sgid ? TFAIL : TPASS, "%s: Setgid bit not set", name);
+}
 
-		/* Verify modes */
-		if (!(buf.st_mode & S_ISGID)) {
-			tst_resm(TFAIL,
-				 "%s: Incorrect modes, setgid bit not set",
-				 root_setgid_B);
-			local_flag = FAILED;
-		}
+static void run(void)
+{
+	struct stat buf;
 
-		/* Verify group ID */
-		if (buf.st_gid != group2_gid) {
-			tst_resm(TFAIL, "%s: Incorrect group (got %d and %d)",
-				 root_setgid_B, buf.st_gid, group2_gid);
-			local_flag = FAILED;
-		}
+	/* Create directories and set permissions */
+	SAFE_MKDIR(DIR_A, MODE_RWX);
+	SAFE_CHOWN(DIR_A, nobody_uid, bin_gid);
+	SAFE_STAT(DIR_A, &buf);
 
-		if (local_flag == PASSED) {
-			tst_resm(TPASS, "Test passed in block3.");
-		} else {
-			tst_resm(TFAIL, "Test failed in block3.");
-			fail_count++;
-		}
+	if (buf.st_mode & S_ISGID)
+		tst_brk(TBROK, "%s: Setgid bit is set", DIR_A);
 
-		/*
-		 * Clean up any files created by test before call to anyfail.
-		 * Remove the directories.
-		 */
-		if (unlink(setgid_A) < 0)
-			tst_resm(TWARN | TERRNO, "unlink(%s) failed", setgid_A);
-		if (unlink(nosetgid_A) < 0)
-			tst_resm(TWARN | TERRNO, "unlink(%s) failed",
-				 nosetgid_A);
-		if (rmdir(DIR_A) < 0)
-			tst_resm(TWARN | TERRNO, "rmdir(%s) failed", DIR_A);
+	if (buf.st_gid != bin_gid) {
+		tst_brk(TBROK, "%s: Incorrect group, %u != %u", DIR_A,
+			buf.st_gid, bin_gid);
+	}
 
-		if (unlink(setgid_B) < 0)
-			tst_resm(TWARN | TERRNO, "unlink(%s) failed", setgid_B);
-		if (unlink(root_setgid_B) < 0)
-			tst_resm(TWARN | TERRNO, "unlink(%s) failed",
-				 root_setgid_B);
-		if (unlink(nosetgid_B) < 0)
-			tst_resm(TWARN | TERRNO, "unlink(%s) failed",
-				 nosetgid_B);
-		if (rmdir(DIR_B) < 0)
-			tst_resm(TWARN | TERRNO, "rmdir(%s) failed", DIR_B);
+	SAFE_MKDIR(DIR_B, MODE_RWX);
+	SAFE_CHOWN(DIR_B, nobody_uid, bin_gid);
+	SAFE_CHMOD(DIR_B, MODE_SGID);
+	SAFE_STAT(DIR_B, &buf);
 
-		if (fail_count == 0) {
-			tst_resm(TPASS, "Test passed.");
-		} else {
-			tst_resm(TFAIL,
-				 "Test failed because of above failures.");
-		}
+	if (!(buf.st_mode & S_ISGID))
+		tst_brk(TBROK, "%s: Setgid bit not set", DIR_B);
 
+	if (buf.st_gid != bin_gid) {
+		tst_brk(TBROK, "%s: Incorrect group, %u != %u", DIR_B,
+			buf.st_gid, bin_gid);
 	}
 
-	cleanup();
-	tst_exit();
-}
-
-static void setup(void)
-{
-	tst_require_root();
-	tst_sig(NOFORK, DEF_HANDLER, cleanup);
-	TEST_PAUSE;
-	tst_tmpdir();
+	/* Switch to user nobody and create two files in DIR_A */
+	/* Both files should inherit GID from the process */
+	SAFE_SETGID(nobody_gid);
+	SAFE_SETREUID(-1, nobody_uid);
+	file_test(NOSETGID_A, MODE_RWX, 0, nobody_gid);
+	file_test(SETGID_A, MODE_SGID, 1, nobody_gid);
+
+	/* Create two files in DIR_B and validate owner and permissions */
+	/* Both files should inherit GID from the parent directory */
+	file_test(NOSETGID_B, MODE_RWX, 0, bin_gid);
+	/*
+	 * CVE 2018-13405 (privilege escalation using setgid bit) has its
+	 * own test, skip setgid check here
+	 */
+	file_test(SETGID_B, MODE_SGID, -1, bin_gid);
+
+	/* Switch back to root UID and create a file in DIR_B */
+	/* The file should inherid GID from parent directory */
+	SAFE_SETREUID(-1, orig_uid);
+	file_test(ROOT_SETGID, MODE_SGID, 1, bin_gid);
+
+	/* Cleanup between loops */
+	tst_purge_dir(tmpdir);
 }
 
 static void cleanup(void)
 {
-	tst_rmdir();
+	SAFE_SETREUID(-1, orig_uid);
+
+	if (fd >= 0)
+		SAFE_CLOSE(fd);
+
+	free(tmpdir);
 }
+
+static struct tst_test test = {
+	.test_all = run,
+	.setup = setup,
+	.cleanup = cleanup,
+	.needs_root = 1,
+	.needs_tmpdir = 1,
+};
-- 
2.32.0


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [LTP] [PATCH 3/3] Add test for CVE 2018-13405
  2021-08-06 15:45 [LTP] [PATCH 1/3] syscalls/creat08: Convert to new API Martin Doucha
  2021-08-06 15:45 ` [LTP] [PATCH 2/3] syscalls/open10: " Martin Doucha
@ 2021-08-06 15:45 ` Martin Doucha
  2021-08-17 10:23   ` Richard Palethorpe
  2021-08-13 13:49 ` [LTP] [PATCH 1/3] syscalls/creat08: Convert to new API Cyril Hrubis
  2 siblings, 1 reply; 13+ messages in thread
From: Martin Doucha @ 2021-08-06 15:45 UTC (permalink / raw)
  To: ltp

Signed-off-by: Martin Doucha <mdoucha@suse.cz>
---
 runtest/cve                                |   1 +
 runtest/syscalls                           |   1 +
 testcases/kernel/syscalls/creat/.gitignore |   1 +
 testcases/kernel/syscalls/creat/creat09.c  | 115 +++++++++++++++++++++
 4 files changed, 118 insertions(+)
 create mode 100644 testcases/kernel/syscalls/creat/creat09.c

diff --git a/runtest/cve b/runtest/cve
index c27f58d8d..42c8eedbe 100644
--- a/runtest/cve
+++ b/runtest/cve
@@ -55,6 +55,7 @@ cve-2018-1000001 realpath01
 cve-2018-1000199 ptrace08
 cve-2018-1000204 ioctl_sg01
 cve-2018-12896 timer_settime03
+cve-2018-13405 creat09
 cve-2018-18445 bpf_prog04
 cve-2018-18559 bind06
 cve-2018-18955 userns08
diff --git a/runtest/syscalls b/runtest/syscalls
index 9af5aa5c0..161794f2a 100644
--- a/runtest/syscalls
+++ b/runtest/syscalls
@@ -136,6 +136,7 @@ creat05 creat05
 creat06 creat06
 creat07 creat07
 creat08 creat08
+creat09 creat09
 
 delete_module01 delete_module01
 delete_module02 delete_module02
diff --git a/testcases/kernel/syscalls/creat/.gitignore b/testcases/kernel/syscalls/creat/.gitignore
index a39e63590..caafc02b6 100644
--- a/testcases/kernel/syscalls/creat/.gitignore
+++ b/testcases/kernel/syscalls/creat/.gitignore
@@ -6,3 +6,4 @@
 /creat07
 /creat07_child
 /creat08
+/creat09
diff --git a/testcases/kernel/syscalls/creat/creat09.c b/testcases/kernel/syscalls/creat/creat09.c
new file mode 100644
index 000000000..6255d8784
--- /dev/null
+++ b/testcases/kernel/syscalls/creat/creat09.c
@@ -0,0 +1,115 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Copyright (c) 2021 SUSE LLC <mdoucha@suse.cz>
+ *
+ * CVE-2018-13405
+ *
+ * Check for possible privilege escalation through creating files with setgid
+ * bit set inside a setgid directory owned by a group which the user does not
+ * belong to. Fixed in:
+ *
+ *  commit 0fa3ecd87848c9c93c2c828ef4c3a8ca36ce46c7
+ *  Author: Linus Torvalds <torvalds@linux-foundation.org>
+ *  Date:   Tue Jul 3 17:10:19 2018 -0700
+ *
+ *  Fix up non-directory creation in SGID directories
+ */
+
+#include <stdlib.h>
+#include <sys/types.h>
+#include <pwd.h>
+#include "tst_test.h"
+
+#define MODE_RWX        0777
+#define MODE_SGID       (S_ISGID|0777)
+
+#define WORKDIR		"testdir"
+#define CREAT_FILE	WORKDIR "/creat.tmp"
+#define OPEN_FILE	WORKDIR "/open.tmp"
+
+static uid_t orig_uid;
+static gid_t bin_gid;
+static int fd = -1;
+
+static void setup(void)
+{
+	struct stat buf;
+	struct passwd *ltpuser = SAFE_GETPWNAM("nobody");
+	struct group *ltpgroup = SAFE_GETGRNAM("bin");
+
+	orig_uid = getuid();
+	bin_gid = ltpgroup->gr_gid;
+
+	/* Create directories and set permissions */
+	SAFE_MKDIR(WORKDIR, MODE_RWX);
+	SAFE_CHOWN(WORKDIR, ltpuser->pw_uid, bin_gid);
+	SAFE_CHMOD(WORKDIR, MODE_SGID);
+	SAFE_STAT(WORKDIR, &buf);
+
+	if (!(buf.st_mode & S_ISGID))
+		tst_brk(TBROK, "%s: Setgid bit not set", WORKDIR);
+
+	if (buf.st_gid != bin_gid) {
+		tst_brk(TBROK, "%s: Incorrect group, %u != %u", WORKDIR,
+			buf.st_gid, bin_gid);
+	}
+
+	/* Switch user */
+	ltpgroup = SAFE_GETGRNAM_FALLBACK("nobody", "nogroup");
+	SAFE_SETGID(ltpgroup->gr_gid);
+	SAFE_SETREUID(-1, ltpuser->pw_uid);
+}
+
+static void file_test(const char *name, gid_t gid)
+{
+	struct stat buf;
+
+	SAFE_STAT(name, &buf);
+
+	if (buf.st_gid != gid) {
+		tst_res(TFAIL, "%s: Incorrect group, %u != %u", name,
+			buf.st_gid, gid);
+	} else {
+		tst_res(TPASS, "%s: Owned by correct group", name);
+	}
+
+	if (buf.st_mode & S_ISGID)
+		tst_res(TFAIL, "%s: Setgid bit is set", name);
+	else
+		tst_res(TPASS, "%s: Setgid bit not set", name);
+}
+
+static void run(void)
+{
+	fd = SAFE_CREAT(CREAT_FILE, MODE_SGID);
+	SAFE_CLOSE(fd);
+	file_test(CREAT_FILE, bin_gid);
+
+	fd = SAFE_OPEN(OPEN_FILE, O_CREAT | O_EXCL | O_RDWR, MODE_SGID);
+	file_test(OPEN_FILE, bin_gid);
+	SAFE_CLOSE(fd);
+
+	/* Cleanup between loops */
+	tst_purge_dir(WORKDIR);
+}
+
+static void cleanup(void)
+{
+	SAFE_SETREUID(-1, orig_uid);
+
+	if (fd >= 0)
+		SAFE_CLOSE(fd);
+}
+
+static struct tst_test test = {
+	.test_all = run,
+	.setup = setup,
+	.cleanup = cleanup,
+	.needs_root = 1,
+	.needs_tmpdir = 1,
+	.tags = (const struct tst_tag[]) {
+		{"linux-git", "0fa3ecd87848"},
+		{"CVE", "2018-13405"},
+		{}
+	},
+};
-- 
2.32.0


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [LTP] [PATCH 1/3] syscalls/creat08: Convert to new API
  2021-08-06 15:45 [LTP] [PATCH 1/3] syscalls/creat08: Convert to new API Martin Doucha
  2021-08-06 15:45 ` [LTP] [PATCH 2/3] syscalls/open10: " Martin Doucha
  2021-08-06 15:45 ` [LTP] [PATCH 3/3] Add test for CVE 2018-13405 Martin Doucha
@ 2021-08-13 13:49 ` Cyril Hrubis
  2021-08-13 14:15   ` Martin Doucha
  2 siblings, 1 reply; 13+ messages in thread
From: Cyril Hrubis @ 2021-08-13 13:49 UTC (permalink / raw)
  To: ltp

Hi!
Btw, it looks that this is one of the test failing on tubmbleweed, it
looks like the "bin" group is no longer present on the system.

Can't we just pick two non-zero numbers for the groupids instead of
trying to resolve whatever groups are supposed to be on the system?

We may as well add an library API to request one or two group ids to the
test library so that they are hardcoded only in a single place.

See:
https://progress.opensuse.org/issues/96644
https://openqa.opensuse.org/tests/1872454

-- 
Cyril Hrubis
chrubis@suse.cz

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [LTP] [PATCH 1/3] syscalls/creat08: Convert to new API
  2021-08-13 13:49 ` [LTP] [PATCH 1/3] syscalls/creat08: Convert to new API Cyril Hrubis
@ 2021-08-13 14:15   ` Martin Doucha
  2021-08-13 14:27     ` Cyril Hrubis
  0 siblings, 1 reply; 13+ messages in thread
From: Martin Doucha @ 2021-08-13 14:15 UTC (permalink / raw)
  To: ltp

On 13. 08. 21 15:49, Cyril Hrubis wrote:
> Hi!
> Btw, it looks that this is one of the test failing on tubmbleweed, it
> looks like the "bin" group is no longer present on the system.
> 
> Can't we just pick two non-zero numbers for the groupids instead of
> trying to resolve whatever groups are supposed to be on the system?
> 
> We may as well add an library API to request one or two group ids to the
> test library so that they are hardcoded only in a single place.
> 
> See:
> https://progress.opensuse.org/issues/96644
> https://openqa.opensuse.org/tests/1872454

The test passes on regular Tumbleweed, the ticket you've linked is about
JeOS. Yes, this needs to be fixed but I'd leave it to a separate
patchset. The rewrite isn't breaking anything that isn't already broken.
But you can drop the CVE test patch and I'll resubmit it with the new
patchset.

Latest Tumbleweed tests:
https://openqa.opensuse.org/tests/1873051#step/open10/8
https://openqa.opensuse.org/tests/1873529#step/open10/8

We can't pick just any group numbers. The test user (nobody) must be a
member of the first group but not the other. Using nobody/nogroup is
probably safe so for the second group we can just scan all existing
groups and pick for example max(gid)+1.

-- 
Martin Doucha   mdoucha@suse.cz
QA Engineer for Software Maintenance
SUSE LINUX, s.r.o.
CORSO IIa
Krizikova 148/34
186 00 Prague 8
Czech Republic

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [LTP] [PATCH 1/3] syscalls/creat08: Convert to new API
  2021-08-13 14:15   ` Martin Doucha
@ 2021-08-13 14:27     ` Cyril Hrubis
  2021-08-13 14:30       ` Martin Doucha
  0 siblings, 1 reply; 13+ messages in thread
From: Cyril Hrubis @ 2021-08-13 14:27 UTC (permalink / raw)
  To: ltp

Hi!
> > Btw, it looks that this is one of the test failing on tubmbleweed, it
> > looks like the "bin" group is no longer present on the system.
> > 
> > Can't we just pick two non-zero numbers for the groupids instead of
> > trying to resolve whatever groups are supposed to be on the system?
> > 
> > We may as well add an library API to request one or two group ids to the
> > test library so that they are hardcoded only in a single place.
> > 
> > See:
> > https://progress.opensuse.org/issues/96644
> > https://openqa.opensuse.org/tests/1872454
> 
> The test passes on regular Tumbleweed, the ticket you've linked is about
> JeOS. Yes, this needs to be fixed but I'd leave it to a separate
> patchset. The rewrite isn't breaking anything that isn't already broken.
> But you can drop the CVE test patch and I'll resubmit it with the new
> patchset.

Fair enough.

> Latest Tumbleweed tests:
> https://openqa.opensuse.org/tests/1873051#step/open10/8
> https://openqa.opensuse.org/tests/1873529#step/open10/8
> 
> We can't pick just any group numbers. The test user (nobody) must be a
> member of the first group but not the other. Using nobody/nogroup is
> probably safe so for the second group we can just scan all existing
> groups and pick for example max(gid)+1.

I guess that simply git for nobody/nogroup + 1 should work well then.

-- 
Cyril Hrubis
chrubis@suse.cz

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [LTP] [PATCH 1/3] syscalls/creat08: Convert to new API
  2021-08-13 14:27     ` Cyril Hrubis
@ 2021-08-13 14:30       ` Martin Doucha
  2021-08-13 15:18         ` Cyril Hrubis
  0 siblings, 1 reply; 13+ messages in thread
From: Martin Doucha @ 2021-08-13 14:30 UTC (permalink / raw)
  To: ltp

On 13. 08. 21 16:27, Cyril Hrubis wrote:
>> We can't pick just any group numbers. The test user (nobody) must be a
>> member of the first group but not the other. Using nobody/nogroup is
>> probably safe so for the second group we can just scan all existing
>> groups and pick for example max(gid)+1.
> 
> I guess that simply git for nobody/nogroup + 1 should work well then.

If nobody/nogroup + 1 turns out to be root gid (or any group where root
is an explicit member), then we may end up with false negatives in the
last subtest.

-- 
Martin Doucha   mdoucha@suse.cz
QA Engineer for Software Maintenance
SUSE LINUX, s.r.o.
CORSO IIa
Krizikova 148/34
186 00 Prague 8
Czech Republic

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [LTP] [PATCH 1/3] syscalls/creat08: Convert to new API
  2021-08-13 14:30       ` Martin Doucha
@ 2021-08-13 15:18         ` Cyril Hrubis
  2021-08-13 15:33           ` Martin Doucha
  0 siblings, 1 reply; 13+ messages in thread
From: Cyril Hrubis @ 2021-08-13 15:18 UTC (permalink / raw)
  To: ltp

Hi!
> >> We can't pick just any group numbers. The test user (nobody) must be a
> >> member of the first group but not the other. Using nobody/nogroup is
> >> probably safe so for the second group we can just scan all existing
> >> groups and pick for example max(gid)+1.
> > 
> > I guess that simply git for nobody/nogroup + 1 should work well then.
> 
> If nobody/nogroup + 1 turns out to be root gid (or any group where root
> is an explicit member), then we may end up with false negatives in the
> last subtest.

The root GID is 0 by definition and on my machine root is a member of
bin group yet the test seems to work fine. I do not get how root having
the bin group (or nobody+1) in the list of supplementary groups will
interfere with the test.

-- 
Cyril Hrubis
chrubis@suse.cz

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [LTP] [PATCH 1/3] syscalls/creat08: Convert to new API
  2021-08-13 15:18         ` Cyril Hrubis
@ 2021-08-13 15:33           ` Martin Doucha
  2021-08-13 17:19             ` Cyril Hrubis
  0 siblings, 1 reply; 13+ messages in thread
From: Martin Doucha @ 2021-08-13 15:33 UTC (permalink / raw)
  To: ltp

On 13. 08. 21 17:18, Cyril Hrubis wrote:
>> If nobody/nogroup + 1 turns out to be root gid (or any group where root
>> is an explicit member), then we may end up with false negatives in the
>> last subtest.
> 
> The root GID is 0 by definition and on my machine root is a member of
> bin group yet the test seems to work fine. I do not get how root having
> the bin group (or nobody+1) in the list of supplementary groups will
> interfere with the test.

Simple: The last test case is checking whether root has an exception
from the setgid bit removal logic that fixed the CVE. This logic is not
applied when the file is being created by a member of the group which
owns the parent directory. If root happens to be an explicit member of
the second group, the last subtest will pass even when the kernel
doesn't apply the root exception properly.

-- 
Martin Doucha   mdoucha@suse.cz
QA Engineer for Software Maintenance
SUSE LINUX, s.r.o.
CORSO IIa
Krizikova 148/34
186 00 Prague 8
Czech Republic

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [LTP] [PATCH 1/3] syscalls/creat08: Convert to new API
  2021-08-13 15:33           ` Martin Doucha
@ 2021-08-13 17:19             ` Cyril Hrubis
  0 siblings, 0 replies; 13+ messages in thread
From: Cyril Hrubis @ 2021-08-13 17:19 UTC (permalink / raw)
  To: ltp

Hi!
> > The root GID is 0 by definition and on my machine root is a member of
> > bin group yet the test seems to work fine. I do not get how root having
> > the bin group (or nobody+1) in the list of supplementary groups will
> > interfere with the test.
> 
> Simple: The last test case is checking whether root has an exception
> from the setgid bit removal logic that fixed the CVE. This logic is not
> applied when the file is being created by a member of the group which
> owns the parent directory. If root happens to be an explicit member of
> the second group, the last subtest will pass even when the kernel
> doesn't apply the root exception properly.

Then I guess the easiest and safest option would be to call
setgroups(0, NULL) in the test setup.

-- 
Cyril Hrubis
chrubis@suse.cz

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [LTP] [PATCH 3/3] Add test for CVE 2018-13405
  2021-08-06 15:45 ` [LTP] [PATCH 3/3] Add test for CVE 2018-13405 Martin Doucha
@ 2021-08-17 10:23   ` Richard Palethorpe
  2021-08-17 10:33     ` Martin Doucha
  0 siblings, 1 reply; 13+ messages in thread
From: Richard Palethorpe @ 2021-08-17 10:23 UTC (permalink / raw)
  To: ltp

Hello Martin,

Martin Doucha <mdoucha@suse.cz> writes:

> Signed-off-by: Martin Doucha <mdoucha@suse.cz>
> ---
>  runtest/cve                                |   1 +
>  runtest/syscalls                           |   1 +
>  testcases/kernel/syscalls/creat/.gitignore |   1 +
>  testcases/kernel/syscalls/creat/creat09.c  | 115 +++++++++++++++++++++
>  4 files changed, 118 insertions(+)
>  create mode 100644 testcases/kernel/syscalls/creat/creat09.c
>
> diff --git a/runtest/cve b/runtest/cve
> index c27f58d8d..42c8eedbe 100644
> --- a/runtest/cve
> +++ b/runtest/cve
> @@ -55,6 +55,7 @@ cve-2018-1000001 realpath01
>  cve-2018-1000199 ptrace08
>  cve-2018-1000204 ioctl_sg01
>  cve-2018-12896 timer_settime03
> +cve-2018-13405 creat09
>  cve-2018-18445 bpf_prog04
>  cve-2018-18559 bind06
>  cve-2018-18955 userns08
> diff --git a/runtest/syscalls b/runtest/syscalls
> index 9af5aa5c0..161794f2a 100644
> --- a/runtest/syscalls
> +++ b/runtest/syscalls
> @@ -136,6 +136,7 @@ creat05 creat05
>  creat06 creat06
>  creat07 creat07
>  creat08 creat08
> +creat09 creat09
>  
>  delete_module01 delete_module01
>  delete_module02 delete_module02
> diff --git a/testcases/kernel/syscalls/creat/.gitignore b/testcases/kernel/syscalls/creat/.gitignore
> index a39e63590..caafc02b6 100644
> --- a/testcases/kernel/syscalls/creat/.gitignore
> +++ b/testcases/kernel/syscalls/creat/.gitignore
> @@ -6,3 +6,4 @@
>  /creat07
>  /creat07_child
>  /creat08
> +/creat09
> diff --git a/testcases/kernel/syscalls/creat/creat09.c b/testcases/kernel/syscalls/creat/creat09.c
> new file mode 100644
> index 000000000..6255d8784
> --- /dev/null
> +++ b/testcases/kernel/syscalls/creat/creat09.c
> @@ -0,0 +1,115 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * Copyright (c) 2021 SUSE LLC <mdoucha@suse.cz>
> + *
> + * CVE-2018-13405
> + *
> + * Check for possible privilege escalation through creating files with setgid
> + * bit set inside a setgid directory owned by a group which the user does not
> + * belong to. Fixed in:
> + *
> + *  commit 0fa3ecd87848c9c93c2c828ef4c3a8ca36ce46c7
> + *  Author: Linus Torvalds <torvalds@linux-foundation.org>
> + *  Date:   Tue Jul 3 17:10:19 2018 -0700
> + *
> + *  Fix up non-directory creation in SGID directories
> + */

Very interesting bug! FYI, I noticed this code was changed again
recently to make it mount/user namespace aware. Which then reminds me of
userns08.

> +
> +#include <stdlib.h>
> +#include <sys/types.h>
> +#include <pwd.h>
> +#include "tst_test.h"
> +
> +#define MODE_RWX        0777
> +#define MODE_SGID       (S_ISGID|0777)
> +
> +#define WORKDIR		"testdir"
> +#define CREAT_FILE	WORKDIR "/creat.tmp"
> +#define OPEN_FILE	WORKDIR "/open.tmp"
> +
> +static uid_t orig_uid;
> +static gid_t bin_gid;
> +static int fd = -1;
> +
> +static void setup(void)
> +{
> +	struct stat buf;
> +	struct passwd *ltpuser = SAFE_GETPWNAM("nobody");
> +	struct group *ltpgroup = SAFE_GETGRNAM("bin");

These might not exist on some systems. I think you can just pick
arbitrary UID/GID numbers instead. No need to check the user/group
databases.

> +
> +	orig_uid = getuid();
> +	bin_gid = ltpgroup->gr_gid;
> +
> +	/* Create directories and set permissions */
> +	SAFE_MKDIR(WORKDIR, MODE_RWX);
> +	SAFE_CHOWN(WORKDIR, ltpuser->pw_uid, bin_gid);
> +	SAFE_CHMOD(WORKDIR, MODE_SGID);
> +	SAFE_STAT(WORKDIR, &buf);
> +
> +	if (!(buf.st_mode & S_ISGID))
> +		tst_brk(TBROK, "%s: Setgid bit not set", WORKDIR);
> +
> +	if (buf.st_gid != bin_gid) {
> +		tst_brk(TBROK, "%s: Incorrect group, %u != %u", WORKDIR,
> +			buf.st_gid, bin_gid);
> +	}
> +
> +	/* Switch user */
> +	ltpgroup = SAFE_GETGRNAM_FALLBACK("nobody", "nogroup");
> +	SAFE_SETGID(ltpgroup->gr_gid);
> +	SAFE_SETREUID(-1, ltpuser->pw_uid);
> +}
> +
> +static void file_test(const char *name, gid_t gid)

gid is always set to bin_gid.

> +{
> +	struct stat buf;
> +
> +	SAFE_STAT(name, &buf);
> +
> +	if (buf.st_gid != gid) {
> +		tst_res(TFAIL, "%s: Incorrect group, %u != %u", name,
> +			buf.st_gid, gid);
> +	} else {
> +		tst_res(TPASS, "%s: Owned by correct group", name);
> +	}
> +
> +	if (buf.st_mode & S_ISGID)
> +		tst_res(TFAIL, "%s: Setgid bit is set", name);
> +	else
> +		tst_res(TPASS, "%s: Setgid bit not set", name);
> +}
> +
> +static void run(void)
> +{
> +	fd = SAFE_CREAT(CREAT_FILE, MODE_SGID);
> +	SAFE_CLOSE(fd);
> +	file_test(CREAT_FILE, bin_gid);
> +
> +	fd = SAFE_OPEN(OPEN_FILE, O_CREAT | O_EXCL | O_RDWR, MODE_SGID);
> +	file_test(OPEN_FILE, bin_gid);
> +	SAFE_CLOSE(fd);
> +
> +	/* Cleanup between loops */
> +	tst_purge_dir(WORKDIR);
> +}
> +
> +static void cleanup(void)
> +{
> +	SAFE_SETREUID(-1, orig_uid);

Why are you doing this? I am assuming the temp dir will be deleted by
the parent process.

> +
> +	if (fd >= 0)
> +		SAFE_CLOSE(fd);
> +}
> +
> +static struct tst_test test = {
> +	.test_all = run,
> +	.setup = setup,
> +	.cleanup = cleanup,
> +	.needs_root = 1,
> +	.needs_tmpdir = 1,
> +	.tags = (const struct tst_tag[]) {
> +		{"linux-git", "0fa3ecd87848"},
> +		{"CVE", "2018-13405"},
> +		{}
> +	},
> +};
> -- 
> 2.32.0


-- 
Thank you,
Richard.

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [LTP] [PATCH 3/3] Add test for CVE 2018-13405
  2021-08-17 10:23   ` Richard Palethorpe
@ 2021-08-17 10:33     ` Martin Doucha
  2021-08-17 11:53       ` Richard Palethorpe
  0 siblings, 1 reply; 13+ messages in thread
From: Martin Doucha @ 2021-08-17 10:33 UTC (permalink / raw)
  To: ltp

On 17. 08. 21 12:23, Richard Palethorpe wrote:
> Hello Martin,
> 
> Martin Doucha <mdoucha@suse.cz> writes:
>> +static void setup(void)
>> +{
>> +	struct stat buf;
>> +	struct passwd *ltpuser = SAFE_GETPWNAM("nobody");
>> +	struct group *ltpgroup = SAFE_GETGRNAM("bin");
> 
> These might not exist on some systems. I think you can just pick
> arbitrary UID/GID numbers instead. No need to check the user/group
> databases.

I'm planning to rewrite this test after the first two patches get
merged. See previous discussion under the creat08 patch.


>> +static void cleanup(void)
>> +{
>> +	SAFE_SETREUID(-1, orig_uid);
> 
> Why are you doing this? I am assuming the temp dir will be deleted by
> the parent process.

That assumption is incorrect.

https://github.com/linux-test-project/ltp/commit/3833d44a2ba3773359d3b35a2108af691d75b4f9

-- 
Martin Doucha   mdoucha@suse.cz
QA Engineer for Software Maintenance
SUSE LINUX, s.r.o.
CORSO IIa
Krizikova 148/34
186 00 Prague 8
Czech Republic

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [LTP] [PATCH 3/3] Add test for CVE 2018-13405
  2021-08-17 10:33     ` Martin Doucha
@ 2021-08-17 11:53       ` Richard Palethorpe
  0 siblings, 0 replies; 13+ messages in thread
From: Richard Palethorpe @ 2021-08-17 11:53 UTC (permalink / raw)
  To: ltp

Hello Martin,

Martin Doucha <mdoucha@suse.cz> writes:

> On 17. 08. 21 12:23, Richard Palethorpe wrote:
>> Hello Martin,
>> 
>> Martin Doucha <mdoucha@suse.cz> writes:
>>> +static void setup(void)
>>> +{
>>> +	struct stat buf;
>>> +	struct passwd *ltpuser = SAFE_GETPWNAM("nobody");
>>> +	struct group *ltpgroup = SAFE_GETGRNAM("bin");
>> 
>> These might not exist on some systems. I think you can just pick
>> arbitrary UID/GID numbers instead. No need to check the user/group
>> databases.
>
> I'm planning to rewrite this test after the first two patches get
> merged. See previous discussion under the creat08 patch.

Ah, yes, sorry.

>
>
>>> +static void cleanup(void)
>>> +{
>>> +	SAFE_SETREUID(-1, orig_uid);
>> 
>> Why are you doing this? I am assuming the temp dir will be deleted by
>> the parent process.
>
> That assumption is incorrect.
>
> https://github.com/linux-test-project/ltp/commit/3833d44a2ba3773359d3b35a2108af691d75b4f9

This looks different as we call semctl in the cleanup callback. It
appears the testdir/tempdir cleanup is done from the parent
process. i.e. from do_exit() which is only called if pid == lib_pid.

-- 
Thank you,
Richard.

^ permalink raw reply	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2021-08-17 11:53 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-08-06 15:45 [LTP] [PATCH 1/3] syscalls/creat08: Convert to new API Martin Doucha
2021-08-06 15:45 ` [LTP] [PATCH 2/3] syscalls/open10: " Martin Doucha
2021-08-06 15:45 ` [LTP] [PATCH 3/3] Add test for CVE 2018-13405 Martin Doucha
2021-08-17 10:23   ` Richard Palethorpe
2021-08-17 10:33     ` Martin Doucha
2021-08-17 11:53       ` Richard Palethorpe
2021-08-13 13:49 ` [LTP] [PATCH 1/3] syscalls/creat08: Convert to new API Cyril Hrubis
2021-08-13 14:15   ` Martin Doucha
2021-08-13 14:27     ` Cyril Hrubis
2021-08-13 14:30       ` Martin Doucha
2021-08-13 15:18         ` Cyril Hrubis
2021-08-13 15:33           ` Martin Doucha
2021-08-13 17:19             ` Cyril Hrubis

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.