[PATCH 0/8] make newrole suid (take 3)

[PATCH 0/8] make newrole suid (take 3)

[Michael C Thompson]

This is the intro to a set of eight patches.

I finally took the time to break things down so that (I hope) more 
people read and give feedback (I can't believe only Stephen Smalley had 
comments, my code isn't that great!)

These patches are an attempt to make newrole be an acceptably secure 
suid root program, to provide it with the capabilities to generate audit 
records (existing) and handle polyinstatiation (new).

The format of the patches is different from previous sends.

The 8 patches are as follows:
1) Modifications to Makefile to support future patch needs
    Add newrole-lspp.pamd
2) New extract_pw_data function and use in main()
3) Add signal handler function
4) Update drop_capabilities() and use in main()
5) Update the authentication functions and use in main()
    Add cleanup since pam_start is now left till program end
6) Move relabeling tty actions into functions
7) Move command-line argument parsing into a function
    Clear the environment during execution
    Add support for preserving the environment (-p)
8) Shift to using new defines in the Makefile and in newrole.c
    Add support for namespaces
    Remove unused code, cleanup and documentation

It is now possible to apply a single patch and the code will compile; 
however, this is not recommended

The comments and discussions from the previous send (take 2) of these 
patches have been integrated.

Thanks,
Mike



--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

[PATCH 1/8] make newrole suid (take 3)

[Michael C Thompson]

[-- Attachment #1: Type: text/plain, Size: 557 bytes --]

Michael C Thompson wrote:
> The 8 patches are as follows:
> 1) Modifications to Makefile to support future patch needs
>    Add newrole-lspp.pamd

This is the 1st of 8 patches.
This patch applies against policycoreutils-1.30.30-1.

This patch adds the new lspp pam.d support file for namespaces,
and includes new compile-time options to the Makefile.

Changes:
  * Makefile now has AUDIT_LOG_PRIV and NAMESPACE_PRIV, as well as
    LSPP_PRIV (causes both previous to be on)
  * Adds newrole-lspp.pamd

Signed-off-by: Michael Thompson <thompsmc@us.ibm.com>


[-- Attachment #2: 01-prep_non_source.patch --]
[-- Type: text/x-diff, Size: 2457 bytes --]

diff -Naur policycoreutils-1.30.30/newrole/Makefile policycoreutils-1.30.30.suid/newrole/Makefile
--- policycoreutils-1.30.30/newrole/Makefile	2006-09-29 10:50:27.000000000 -0500
+++ policycoreutils-1.30.30.suid/newrole/Makefile	2006-11-02 12:13:15.000000000 -0600
@@ -10,6 +10,19 @@
 # This is so that we have the CAP_AUDIT_WRITE capability. newrole will
 # shed all privileges and change to the user's uid.
 LOG_AUDIT_PRIV ?= n
+
+# Enable capabilities to permit newrole to generate audit records.
+# This will make newrole a setuid root program.
+# The capabilities used are: CAP_AUDIT_WRITE.
+AUDIT_LOG_PRIV ?= n
+# Enable capabilities to permit newrole to utilitize the pam_namespace module.
+# This will make newrole a setuid root program.
+# The capabilities used are: CAP_SYS_ADMIN, CAP_CHOWN, CAP_FOWNER and
+# CAP_DAC_OVERRIDE. 
+NAMESPACE_PRIV ?= n
+# If LSPP_PRIV is y, then newrole will be made into setuid root program.
+# Enabling this option will force AUDIT_LOG_PRIV and NAMESPACE_PRIV to be y.
+LSPP_PRIV ?= n
 VERSION = $(shell cat ../VERSION)
 
 CFLAGS ?= -Werror -Wall -W
@@ -26,6 +39,26 @@
 	override CFLAGS += -DUSE_AUDIT
 	LDLIBS += -laudit
 endif
+
+ifeq (${LSPP_PRIV},y)
+	override AUDIT_LOG_PRIV=y
+	override NAMESPACE_PRIV=y
+endif
+ifeq (${AUDIT_LOG_PRIV},y)
+	override CFLAGS += -DAUDIT_LOG_PRIV
+	IS_SUID=y
+endif
+ifeq (${NAMESPACE_PRIV},y)
+	override CFLAGS += -DNAMESPACE_PRIV
+	IS_SUID=y
+endif
+ifeq (${IS_SUID},y)
+	MODE := 4555
+	LDLIBS += -lcap
+else
+	MODE := 0555
+endif
+
 ifeq (${LOG_AUDIT_PRIV},y)
 	override CFLAGS += -DLOG_AUDIT_PRIV
 	LDLIBS += -lcap
@@ -46,8 +79,12 @@
 	install -m 644 newrole.1 $(MANDIR)/man1/
 ifeq (${PAMH}, /usr/include/security/pam_appl.h)
 	test -d $(ETCDIR)/pam.d || install -m 755 -d $(ETCDIR)/pam.d
+ifeq (${LSPP_PRIV},y)
+	install -m 644 newrole-lspp.pamd $(ETCDIR)/pam.d/newrole
+else
 	install -m 644 newrole.pamd $(ETCDIR)/pam.d/newrole
 endif
+endif
 
 clean:
 	rm -f $(TARGETS) *.o 
diff -Naur policycoreutils-1.30.30/newrole/newrole-lspp.pamd policycoreutils-1.30.30.suid/newrole/newrole-lspp.pamd
--- policycoreutils-1.30.30/newrole/newrole-lspp.pamd	1969-12-31 18:00:00.000000000 -0600
+++ policycoreutils-1.30.30.suid/newrole/newrole-lspp.pamd	2006-11-02 12:11:19.000000000 -0600
@@ -0,0 +1,5 @@
+#%PAM-1.0
+auth       include	system-auth
+account    include	system-auth
+password   include	system-auth
+session    required	pam_namespace.so unmnt_remnt no_unmount_on_close

[PATCH 2/8] make newrole suid (take 3)

[Michael C Thompson]

[-- Attachment #1: Type: text/plain, Size: 509 bytes --]

Michael C Thompson wrote:
> The 8 patches are as follows:
> 1) Modifications to Makefile to support future patch needs
>    Add newrole-lspp.pamd
> 2) New extract_pw_data function and use in main()

This is the 2nd of 8 patches.
This patch applies against policycoreutils-1.30.30-1.

This patch moves the parse /etc/passwd functionality from
main() into a separate function.

Changes:
  * Introduces the extract_pw_data() function and uses it in main()

Signed-off-by: Michael Thompson <thompsmc@us.ibm.com>


[-- Attachment #2: 02-extract_passwd.patch --]
[-- Type: text/x-diff, Size: 3914 bytes --]

diff -Naur policycoreutils-1.30.30/newrole/newrole.c policycoreutils-1.30.30.suid/newrole/newrole.c
--- policycoreutils-1.30.30/newrole/newrole.c	2006-09-29 10:50:27.000000000 -0500
+++ policycoreutils-1.30.30.suid/newrole/newrole.c	2006-11-02 12:19:12.000000000 -0600
@@ -332,6 +332,61 @@
 	return found;
 }
 
+/**
+ * Determine the Linux user identity to re-authenticate.
+ * If supported and set, use the login uid, as this should be more stable.
+ * Otherwise, use the real uid.
+ *
+ * This function assigns malloc'd memory into the pw_copy struct.
+ * Returns zero on success, non-zero otherwise
+ */
+int extract_pw_data(struct passwd *pw_copy)
+{
+	uid_t uid;
+	struct passwd *pw;
+
+#ifdef USE_AUDIT
+	uid = audit_getloginuid();
+	if (uid == (uid_t) - 1)
+		uid = getuid();
+#else
+	uid = getuid();
+#endif
+
+	setpwent();
+	pw = getpwuid(uid);
+	endpwent();
+	if (!(pw && pw->pw_name && pw->pw_name[0] && pw->pw_shell
+	      && pw->pw_shell[0] && pw->pw_dir && pw->pw_dir[0])) {
+		fprintf(stderr,
+			_("cannot find valid entry in the passwd file.\n"));
+		return -1;
+	}
+
+	*pw_copy = *pw;
+	pw = pw_copy;
+	pw->pw_name = strdup(pw->pw_name);
+	pw->pw_dir = strdup(pw->pw_dir);
+	pw->pw_shell = strdup(pw->pw_shell);
+
+	if (! (pw->pw_name && pw->pw_dir && pw->pw_shell)) {
+		fprintf(stderr, _("Out of memory!\n"));
+		goto out_free;
+	}
+
+	if (verify_shell(pw->pw_shell) == 0) {
+		fprintf(stderr, _("Error!  Shell is not valid.\n"));
+		goto out_free;
+	}
+	return 0;
+
+out_free:
+	free(pw->pw_name);
+	free(pw->pw_dir);
+	free(pw->pw_shell);
+	return -1;
+}
+
 /*
  * This function will drop the capabilities so that we are left
  * only with access to the audit system. If the user is root, we leave
@@ -460,8 +515,7 @@
 
 	context_t context;	/* manipulatable form of new_context */
 
-	struct passwd *pw;	/* struct derived from passwd file line */
-	struct passwd pw_copy;
+	struct passwd pw;	/* struct derived from passwd file line */
 
 	int clflag;		/* holds codes for command line flags */
 	int flag_index;		/* flag index in argv[] */
@@ -639,22 +693,8 @@
 #endif
 
 	/* Get the passwd info for the Linux user identity. */
-	pw = getpwuid(uid);
-	if (!pw) {
-		fprintf(stderr,
-			_("cannot find your entry in the passwd file.\n"));
-		exit(-1);
-	}
-	pw_copy = *pw;
-	pw = &pw_copy;
-	pw->pw_name = xstrdup(pw->pw_name);
-	pw->pw_dir = xstrdup(pw->pw_dir);
-	pw->pw_shell = xstrdup(pw->pw_shell);
-
-	if (verify_shell(pw->pw_shell) == 0) {
-		fprintf(stderr, _("Error!  Shell is not valid.\n"));
-		exit(-1);
-	}
+	if (extract_pw_data(&pw))
+		return -1;
 
 	/* Get the tty name. Pam will need it. */
 	ttyn = ttyname(0);
@@ -664,7 +704,7 @@
 		exit(-1);
 	}
 
-	printf(_("Authenticating %s.\n"), pw->pw_name);
+	printf(_("Authenticating %s.\n"), pw.pw_name);
 
 	/* 
 	 * Re-authenticate the user running this program.
@@ -673,13 +713,13 @@
 	 * by policy).  Trusted path mechanism would be preferred.
 	 */
 #ifdef USE_PAM
-	if (!authenticate_via_pam(pw, ttyn))
+	if (!authenticate_via_pam(&pw, ttyn))
 #else				/* !USE_PAM */
-	if (!authenticate_via_shadow_passwd(pw))
+	if (!authenticate_via_shadow_passwd(&pw))
 #endif				/* if/else USE_PAM */
 	{
 		fprintf(stderr, _("newrole: incorrect password for %s\n"),
-			pw->pw_name);
+			pw.pw_name);
 		return (-1);
 	}
 	/* If we reach here, then we have authenticated the user. */
@@ -904,7 +944,7 @@
 
 	if (optind < 1)
 		optind = 1;
-	if (asprintf(&argv[optind - 1], "-%s", pw->pw_shell) < 0) {
+	if (asprintf(&argv[optind - 1], "-%s", pw.pw_shell) < 0) {
 		fprintf(stderr, _("Error allocating shell.\n"));
 		exit(-1);
 	}
@@ -925,7 +965,7 @@
 	if (send_audit_message(1, old_context, new_context, ttyn))
 		exit(-1);
 	freecon(old_context);
-	execv(pw->pw_shell, argv + optind - 1);
+	execv(pw.pw_shell, argv + optind - 1);
 
 	/* If we reach here, then we failed to exec the new shell. */
 	perror(_("failed to exec shell\n"));

Re: [PATCH 2/8] make newrole suid (take 3)

[Serge E. Hallyn]

Quoting Michael C Thompson (thompsmc@us.ibm.com):
> +	setpwent();
> +	pw = getpwuid(uid);
> +	endpwent();

Why the set/endpwent() calls?  The original code didn't have them,
and you aren't useing getpwent() so they don't seem to do anything.

> +	if (!(pw && pw->pw_name && pw->pw_name[0] && pw->pw_shell
> +	      && pw->pw_shell[0] && pw->pw_dir && pw->pw_dir[0])) {
> +		fprintf(stderr,
> +			_("cannot find valid entry in the passwd file.\n"));
> +		return -1;
> +	}
> +
> +	*pw_copy = *pw;
> +	pw = pw_copy;
> +	pw->pw_name = strdup(pw->pw_name);

You switched from xstrdup to strdup - don't know whether that means
anything...

> +	pw->pw_dir = strdup(pw->pw_dir);
> +	pw->pw_shell = strdup(pw->pw_shell);

-serge

--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

Re: [PATCH 2/8] make newrole suid (take 3)

[Michael C Thompson]

Serge E. Hallyn wrote:
> Quoting Michael C Thompson (thompsmc@us.ibm.com):
>> +	setpwent();
>> +	pw = getpwuid(uid);
>> +	endpwent();
> 
> Why the set/endpwent() calls?  The original code didn't have them,
> and you aren't useing getpwent() so they don't seem to do anything.

Good point, I'm sure I had some (flawed) reason for doing it, but it 
doesn't seem to have made sense. I'll remove these extra calls.

>> +	if (!(pw && pw->pw_name && pw->pw_name[0] && pw->pw_shell
>> +	      && pw->pw_shell[0] && pw->pw_dir && pw->pw_dir[0])) {
>> +		fprintf(stderr,
>> +			_("cannot find valid entry in the passwd file.\n"));
>> +		return -1;
>> +	}
>> +
>> +	*pw_copy = *pw;
>> +	pw = pw_copy;
>> +	pw->pw_name = strdup(pw->pw_name);
> 
> You switched from xstrdup to strdup - don't know whether that means
> anything...

Yes, I did this because of a misunderstanding I had with the naming 
scheme of xstrdup, which is supposed to suceeded or exit. However, since 
I've changed the code to do better cleanup on errors, I've changed the 
calls to strdup and then do checks to ensure it succeeded.

>> +	pw->pw_dir = strdup(pw->pw_dir);
>> +	pw->pw_shell = strdup(pw->pw_shell);
> 
> -serge



--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

[PATCH 3/8] make newrole suid (take 3)

[Michael C Thompson]

[-- Attachment #1: Type: text/plain, Size: 510 bytes --]

Michael C Thompson wrote:
> The 8 patches are as follows:
> 1) Modifications to Makefile to support future patch needs
>    Add newrole-lspp.pamd
> 2) New extract_pw_data function and use in main()
> 3) Add signal handler function

This is the 3rd of 8 patches.
This patch applies against policycoreutils-1.30.30-1.

This patch moves the signal handler setup from main() into a new
function.

Changes:
  * Adds set_signal_handles() and uses it in main()

Signed-off-by: Michael Thompson <thompsmc@us.ibm.com>


[-- Attachment #2: 03-signal_func.patch --]
[-- Type: text/x-diff, Size: 1367 bytes --]

diff -Naur policycoreutils-1.30.30/newrole/newrole.c policycoreutils-1.30.30.suid/newrole/newrole.c
--- policycoreutils-1.30.30/newrole/newrole.c	2006-11-02 12:20:18.000000000 -0600
+++ policycoreutils-1.30.30.suid/newrole/newrole.c	2006-11-02 12:22:27.000000000 -0600
@@ -498,6 +498,30 @@
 }
 #endif
 
+/**
+ * Take care of any signal setup
+ */
+static int set_signal_handles()
+{
+	sigset_t empty;
+
+	/* Empty the signal mask in case someone is blocking a signal */
+	if (sigemptyset(&empty)) {
+		fprintf(stderr, _("Unable to obtain empty signal set\n"));
+		return -1;
+	}
+
+	(void)sigprocmask(SIG_SETMASK, &empty, NULL);
+
+	/* Terminate on SIGHUP. */
+	if (signal(SIGHUP, SIG_DFL) == SIG_ERR) {
+		fprintf(stderr, _("Unable to set SIGHUP handler\n"));
+		return -1;
+	}
+
+	return 0;
+}
+
 /************************************************************************
  *
  * All code used for both PAM and shadow passwd goes in this section.
@@ -534,18 +558,13 @@
 	uid_t uid;
 	int fd;
 	int enforcing;
-	sigset_t empty;
 
 #ifdef LOG_AUDIT_PRIV
 	drop_capabilities();
 #endif
 
-	/* Empty the signal mask in case someone is blocking a signal */
-	sigemptyset(&empty);
-	(void)sigprocmask(SIG_SETMASK, &empty, NULL);
-
-	/* Terminate on SIGHUP. */
-	signal(SIGHUP, SIG_DFL);
+	if (set_signal_handles())
+		return -1;
 
 #ifdef USE_NLS
 	setlocale(LC_ALL, "");

[PATCH 4/8] make newrole suid (take 3)

[Michael C Thompson]

[-- Attachment #1: Type: text/plain, Size: 976 bytes --]

Michael C Thompson wrote:
> The 8 patches are as follows:
> 1) Modifications to Makefile to support future patch needs
>    Add newrole-lspp.pamd
> 2) New extract_pw_data function and use in main()
> 3) Add signal handler function
> 4) Update drop_capabilities() and use in main()

This is the 4th of 8 patches.
This patch applies against policycoreutils-1.30.30-1.

This patch adds expands the drop_capabilities functionality
to support various compile-time options (with audit, with
namespace, or neither).

Changes:
  * Splits drop_capabilities into three versions (compile time option):
    - 'No-cap' version, returns true
    - 'audit-only' version, retains only CAP_AUDIT_WRITE
       Enable with AUDIT_LOG_PRIV=y
    - 'namespace+' version, retains CAP_AUDIT_WRITE, CAP_SYS_ADMIN and
       more to allow namespace actions
       Enable with NAMESPACE_PRIV = y
  * main() calls drop_capabilities unconditionally

Signed-off-by: Michael Thompson <thompsmc@us.ibm.com>


[-- Attachment #2: 04-drop_capabilties.patch --]
[-- Type: text/x-diff, Size: 6289 bytes --]

diff -Naur policycoreutils-1.30.30/newrole/newrole.c policycoreutils-1.30.30.suid/newrole/newrole.c
--- policycoreutils-1.30.30/newrole/newrole.c	2006-11-02 12:24:57.000000000 -0600
+++ policycoreutils-1.30.30.suid/newrole/newrole.c	2006-11-02 12:24:19.000000000 -0600
@@ -387,67 +387,138 @@
 	return -1;
 }
 
-/*
+/**
  * This function will drop the capabilities so that we are left
  * only with access to the audit system. If the user is root, we leave
  * the capabilities alone since they already should have access to the
  * audit netlink socket.
+ *
+ * Returns zero on success, non-zero otherwise
  */
-#ifdef LOG_AUDIT_PRIV
-static void drop_capabilities(void)
+#if defined(AUDIT_LOG_PRIV) && !defined(NAMESPACE_PRIV)
+static int drop_capabilities(void)
 {
+	int rc = 0;
+	cap_t new_caps, tmp_caps;
+	cap_value_t cap_list[] = { CAP_AUDIT_WRITE };
+	cap_value_t tmp_cap_list[] = { CAP_AUDIT_WRITE, CAP_SETUID };
 	uid_t uid = getuid();
 
-	if (uid) {		/* Non-root path */
-		cap_t new_caps, tmp_caps;
-		cap_value_t cap_list[] = { CAP_AUDIT_WRITE };
-		cap_value_t tmp_cap_list[] = { CAP_AUDIT_WRITE, CAP_SETUID };
-
-		new_caps = cap_init();
-		tmp_caps = cap_init();
-		if (!new_caps || !tmp_caps) {
-			fprintf(stderr,
-				_("Error initing capabilities, aborting.\n"));
-			exit(-1);
-		}
-		cap_set_flag(new_caps, CAP_PERMITTED, 1, cap_list, CAP_SET);
-		cap_set_flag(new_caps, CAP_EFFECTIVE, 1, cap_list, CAP_SET);
-		cap_set_flag(tmp_caps, CAP_PERMITTED, 2, tmp_cap_list, CAP_SET);
-		cap_set_flag(tmp_caps, CAP_EFFECTIVE, 2, tmp_cap_list, CAP_SET);
+	if (!uid)
+		return 0;
 
-		/* Keep capabilities across uid change */
-		prctl(PR_SET_KEEPCAPS, 1, 0, 0, 0);
+	/* Non-root caller, suid root path */
+	new_caps = cap_init();
+	tmp_caps = cap_init();
+	if (!new_caps || !tmp_caps) {
+		fprintf(stderr, _("Error initing capabilities, aborting.\n"));
+		return -1;
+	}
+	rc |= cap_set_flag(new_caps, CAP_PERMITTED, 1, cap_list, CAP_SET);
+	rc |= cap_set_flag(new_caps, CAP_EFFECTIVE, 1, cap_list, CAP_SET);
+	rc |= cap_set_flag(tmp_caps, CAP_PERMITTED, 2, tmp_cap_list, CAP_SET);
+	rc |= cap_set_flag(tmp_caps, CAP_EFFECTIVE, 2, tmp_cap_list, CAP_SET);
+	if (rc) {
+		fprintf(stderr, _("Error setting capabilities, aborting\n"));
+		goto out;
+	}
 
-		/* We should still have root's caps, so drop most capabilities now */
-		if (cap_set_proc(tmp_caps)) {
-			fprintf(stderr,
-				_("Error dropping capabilities, aborting\n"));
-			exit(-1);
-		}
-		cap_free(tmp_caps);
+	/* Keep capabilities across uid change */
+	if (prctl(PR_SET_KEEPCAPS, 1, 0, 0, 0)) {
+		fprintf(stderr, _("Error setting KEEPCAPS, aborting\n"));
+		rc = -1;
+		goto out;
+	}
 
-		/* Change uid */
-		if (setresuid(uid, uid, uid)) {
-			fprintf(stderr, _("Error changing uid, aborting.\n"));
-			exit(-1);
-		}
+	/* Does this temporary change really buy us much? */
+	/* We should still have root's caps, so drop most capabilities now */
+	if ((rc = cap_set_proc(tmp_caps))) {
+		fprintf(stderr, _("Error dropping capabilities, aborting\n"));
+		goto out;
+	}
 
-		/* Now get rid of this ability */
-		if (prctl(PR_SET_KEEPCAPS, 0, 0, 0, 0) < 0) {
-			fprintf(stderr,
-				_("Error resetting KEEPCAPS, aborting\n"));
-			exit(-1);
-		}
+	/* Change uid */
+	if ((rc = setresuid(uid, uid, uid))) {
+		fprintf(stderr, _("Error changing uid, aborting.\n"));
+		goto out;
+	}
 
-		/* Finish dropping capabilities. */
-		if (cap_set_proc(new_caps)) {
-			fprintf(stderr,
-				_
-				("Error dropping SETUID capability, aborting\n"));
-			exit(-1);
-		}
-		cap_free(new_caps);
+	/* Now get rid of this ability */
+	if ((rc = prctl(PR_SET_KEEPCAPS, 0, 0, 0, 0) < 0)) {
+		fprintf(stderr, _("Error resetting KEEPCAPS, aborting\n"));
+		goto out;
+	}
+
+	/* Finish dropping capabilities. */
+	if ((rc = cap_set_proc(new_caps))) {
+		fprintf(stderr,
+			_("Error dropping SETUID capability, aborting\n"));
+		goto out;
+	}
+out:
+	if (cap_free(tmp_caps) || cap_free(new_caps))
+		fprintf(stderr, _("Error freeing caps\n"));
+	return rc;
+}
+#elif defined(NAMESPACE_PRIV)
+/**
+ * This function will drop the capabilities so that we are left
+ * only with access to the audit system and the ability to raise
+ * CAP_SYS_ADMIN, CAP_DAC_OVERRIDE, CAP_FOWNER and CAP_CHOWN,
+ * before invoking pam_namespace.  These capabilities are needed
+ * for performing bind mounts/unmounts and to create potential new
+ * instance directories with appropriate DAC attributes. If the
+ * user is root, we leave the capabilities alone since they already
+ * should have access to the audit netlink socket and should have
+ * the ability to create/mount/unmount instance directories.
+ *
+ * Returns zero on success, non-zero otherwise
+ */
+static int drop_capabilities(void)
+{
+	int rc = 0;
+	cap_t new_caps;
+	cap_value_t cap_list[] = { CAP_AUDIT_WRITE, CAP_SETUID,
+				   CAP_SYS_ADMIN, CAP_FOWNER, CAP_CHOWN,
+				   CAP_DAC_OVERRIDE };
+
+	if (!getuid())
+		return 0;
+
+	/* Non-root caller, suid root path */
+	new_caps = cap_init();
+	if (!new_caps) {
+		fprintf(stderr, _("Error initing capabilities, aborting.\n"));
+		return -1;
+	}
+	rc |= cap_set_flag(new_caps, CAP_PERMITTED, 6, cap_list, CAP_SET);
+	rc |= cap_set_flag(new_caps, CAP_EFFECTIVE, 6, cap_list, CAP_SET);
+	if (rc) {
+		fprintf(stderr, _("Error setting capabilities, aborting\n"));
+		goto out;
+	}
+
+	/* Ensure that caps are dropped after setuid call */
+	if ((rc = prctl(PR_SET_KEEPCAPS, 0, 0, 0, 0) < 0)) {
+		fprintf(stderr, _("Error resetting KEEPCAPS, aborting\n"));
+		goto out;
 	}
+
+	/* We should still have root's caps, so drop most capabilities now */
+	if ((rc = cap_set_proc(new_caps))) {
+		fprintf(stderr, _("Error dropping capabilities, aborting\n"));
+		goto out;
+	}
+out:
+	if (cap_free(new_caps))
+		fprintf(stderr, _("Error freeing caps\n"));
+	return rc;
+}
+
+#else
+static inline int drop_capabilities(void)
+{
+	return 0;
 }
 #endif
 
@@ -559,10 +630,15 @@
 	int fd;
 	int enforcing;
 
-#ifdef LOG_AUDIT_PRIV
-	drop_capabilities();
-#endif
-
+	/*
+	 * Step 0: Setup
+	 *
+	 * Do some intial setup, including dropping capabilities, checking
+	 * if it makes sense to continue to run newrole, and setting up
+	 * a scrubbed environment.
+	 */
+	if (drop_capabilities())
+		return -1;
 	if (set_signal_handles())
 		return -1;
 

Re: [PATCH 4/8] make newrole suid (take 3)

[Serge E. Hallyn]

Quoting Michael C Thompson (thompsmc@us.ibm.com):
> Michael C Thompson wrote:
> >The 8 patches are as follows:
> >1) Modifications to Makefile to support future patch needs
> >   Add newrole-lspp.pamd
> >2) New extract_pw_data function and use in main()
> >3) Add signal handler function
> >4) Update drop_capabilities() and use in main()
> 
> This is the 4th of 8 patches.
> This patch applies against policycoreutils-1.30.30-1.
> 
> This patch adds expands the drop_capabilities functionality
> to support various compile-time options (with audit, with
> namespace, or neither).
> 
> Changes:
>  * Splits drop_capabilities into three versions (compile time option):
>    - 'No-cap' version, returns true
>    - 'audit-only' version, retains only CAP_AUDIT_WRITE
>       Enable with AUDIT_LOG_PRIV=y
>    - 'namespace+' version, retains CAP_AUDIT_WRITE, CAP_SYS_ADMIN and
>       more to allow namespace actions
>       Enable with NAMESPACE_PRIV = y
>  * main() calls drop_capabilities unconditionally

It looks like your two later versions of drop_capabilities() don't call
setresuid?  Your comment doesn't explain why, and a very quick scan of
your later patches didn't show you adding them.

thanks,
-serge

--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

Re: [PATCH 4/8] make newrole suid (take 3)

[Michael C Thompson]

Serge E. Hallyn wrote:
> Quoting Michael C Thompson (thompsmc@us.ibm.com):
>> Michael C Thompson wrote:
>>> The 8 patches are as follows:
>>> 1) Modifications to Makefile to support future patch needs
>>>   Add newrole-lspp.pamd
>>> 2) New extract_pw_data function and use in main()
>>> 3) Add signal handler function
>>> 4) Update drop_capabilities() and use in main()
>> This is the 4th of 8 patches.
>> This patch applies against policycoreutils-1.30.30-1.
>>
>> This patch adds expands the drop_capabilities functionality
>> to support various compile-time options (with audit, with
>> namespace, or neither).
>>
>> Changes:
>>  * Splits drop_capabilities into three versions (compile time option):
>>    - 'No-cap' version, returns true
>>    - 'audit-only' version, retains only CAP_AUDIT_WRITE
>>       Enable with AUDIT_LOG_PRIV=y
>>    - 'namespace+' version, retains CAP_AUDIT_WRITE, CAP_SYS_ADMIN and
>>       more to allow namespace actions
>>       Enable with NAMESPACE_PRIV = y
>>  * main() calls drop_capabilities unconditionally
> 
> It looks like your two later versions of drop_capabilities() don't call
> setresuid?  Your comment doesn't explain why, and a very quick scan of
> your later patches didn't show you adding them.

OK, thanks for pointing this out, its a clear deficiency in the code's 
documentation. Here is my explanation of the code:

There are 4 versions of compiling this code:
1. No auditing or namespace support
2. Auditing but not namespace
3. Namespace but not auditing
4. Both

For the 3rd version of drop_capabilities, that is the return 0; inline, 
it is used when compiled without auditing or namespace capabilities. 
Therefore, there is no need to make newrole suid in this case, and 
drop_capabilities is a 'no-op'.

In the case where you compile with only auditing support the original 
version of drop_capabilities (which does do setresuid) is sufficient 
because as long as we maintain the CAP_AUDIT_WRITE, we can get rid of 
the other capabilities and switch to the user's uid.

However, in order to support namespaces, we need to retain more 
capabilities and Stephen Smalley suggested that the euid of the process 
not be changed to the caller's uid until after the pam_open_session() 
call, so that any actions taken by the pam_namespace module are done as 
euid 0. The reasoning was that the calling user would not be able to 
take advantage of the directories that get created by the pam_namespace 
module before it could set their correct permissions and security 
contexts. The transition_to_caller_uid() function is what handles this 
setresuid() change when compiled to support namespaces, and is called 
after the pam_open_session() call returns.

So, in summary, the 1st version of drop_capabilities() does setresuid 
immediately because we don't need to retain euid 0, the 2nd version of 
drop_capabilities() defers the setresuid to transition_to_call_uid() 
function in order to have a more secure pam_open_session() call, and the 
3rd version is for when newrole is not suid, and therefore doesn't need 
to make that call.

Let me know if any of the above explanations are not sufficient because 
I plan on adding this to the code.

Mike


--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

Re: [PATCH 4/8] make newrole suid (take 3)

[Serge E. Hallyn]

Quoting Michael C Thompson (thompsmc@us.ibm.com):
> Serge E. Hallyn wrote:
> >Quoting Michael C Thompson (thompsmc@us.ibm.com):
> >>Michael C Thompson wrote:
> >>>The 8 patches are as follows:
> >>>1) Modifications to Makefile to support future patch needs
> >>>  Add newrole-lspp.pamd
> >>>2) New extract_pw_data function and use in main()
> >>>3) Add signal handler function
> >>>4) Update drop_capabilities() and use in main()
> >>This is the 4th of 8 patches.
> >>This patch applies against policycoreutils-1.30.30-1.
> >>
> >>This patch adds expands the drop_capabilities functionality
> >>to support various compile-time options (with audit, with
> >>namespace, or neither).
> >>
> >>Changes:
> >> * Splits drop_capabilities into three versions (compile time option):
> >>   - 'No-cap' version, returns true
> >>   - 'audit-only' version, retains only CAP_AUDIT_WRITE
> >>      Enable with AUDIT_LOG_PRIV=y
> >>   - 'namespace+' version, retains CAP_AUDIT_WRITE, CAP_SYS_ADMIN and
> >>      more to allow namespace actions
> >>      Enable with NAMESPACE_PRIV = y
> >> * main() calls drop_capabilities unconditionally
> >
> >It looks like your two later versions of drop_capabilities() don't call
> >setresuid?  Your comment doesn't explain why, and a very quick scan of
> >your later patches didn't show you adding them.
> 
> OK, thanks for pointing this out, its a clear deficiency in the code's 
> documentation. Here is my explanation of the code:
> 
> There are 4 versions of compiling this code:
> 1. No auditing or namespace support
> 2. Auditing but not namespace
> 3. Namespace but not auditing
> 4. Both
> 
> For the 3rd version of drop_capabilities, that is the return 0; inline, 
> it is used when compiled without auditing or namespace capabilities. 
> Therefore, there is no need to make newrole suid in this case, and 
> drop_capabilities is a 'no-op'.

Ok - so will the Makefile in that case automatically not install it
suid?

> In the case where you compile with only auditing support the original 
> version of drop_capabilities (which does do setresuid) is sufficient 
> because as long as we maintain the CAP_AUDIT_WRITE, we can get rid of 
> the other capabilities and switch to the user's uid.
> 
> However, in order to support namespaces, we need to retain more 
> capabilities and Stephen Smalley suggested that the euid of the process 
> not be changed to the caller's uid until after the pam_open_session() 
> call, so that any actions taken by the pam_namespace module are done as 
> euid 0. The reasoning was that the calling user would not be able to 
> take advantage of the directories that get created by the pam_namespace 
> module before it could set their correct permissions and security 
> contexts. The transition_to_caller_uid() function is what handles this 
> setresuid() change when compiled to support namespaces, and is called 
> after the pam_open_session() call returns.

Ok - sorry for not following the thread on that.

So the short answer for this case is "it will get set in
pam_open_session".  I trust we know exactly what else will get called
before pam_open_session happens, and trust it to be safe as root?

thanks,
-serge

--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

Re: [PATCH 4/8] make newrole suid (take 3)

[Michael C Thompson]

Serge E. Hallyn wrote:
> Quoting Michael C Thompson (thompsmc@us.ibm.com):
>> For the 3rd version of drop_capabilities, that is the return 0; inline, 
>> it is used when compiled without auditing or namespace capabilities. 
>> Therefore, there is no need to make newrole suid in this case, and 
>> drop_capabilities is a 'no-op'.
> 
> Ok - so will the Makefile in that case automatically not install it
> suid?

Correct. It will only be made suid if either AUDIT_LOG_PRIV or 
NAMESPACE_PRIV are flagged yes.

>> In the case where you compile with only auditing support the original 
>> version of drop_capabilities (which does do setresuid) is sufficient 
>> because as long as we maintain the CAP_AUDIT_WRITE, we can get rid of 
>> the other capabilities and switch to the user's uid.
>>
>> However, in order to support namespaces, we need to retain more 
>> capabilities and Stephen Smalley suggested that the euid of the process 
>> not be changed to the caller's uid until after the pam_open_session() 
>> call, so that any actions taken by the pam_namespace module are done as 
>> euid 0. The reasoning was that the calling user would not be able to 
>> take advantage of the directories that get created by the pam_namespace 
>> module before it could set their correct permissions and security 
>> contexts. The transition_to_caller_uid() function is what handles this 
>> setresuid() change when compiled to support namespaces, and is called 
>> after the pam_open_session() call returns.
> 
> Ok - sorry for not following the thread on that.

No worries.

> So the short answer for this case is "it will get set in
> pam_open_session".  I trust we know exactly what else will get called
> before pam_open_session happens, and trust it to be safe as root?

Not sure what you mean by "it will get set in pam_open_session". The uid 
gets set after the pam_open_session call.

This is a short list of what we do prior to the uid change:
- locale actions
- calls into libselinux, libaudit and libc
- calls to pam

In short, for the case where we want namespace support, we execute as 
root for nearly the entire lifetime of the application, and the parent 
process retains euid 0 as well while the shell has been exec'd and is 
waiting for the child to die.

We don't actually *need* to do this, but my reason was that if we retain 
the ability to setuid and retain the capabilities on the permitted (but 
not effective) list, then if an exploit would be discovered for the 
program, then all the attacker would need to do is re-raise those 
capabilities and game over.

I've only briefly consider the effects of maintaining euid=0 during the 
life-time of the application, but it seems to me the only impact it will 
have is during the pam_open_session call.

Does that make sense?

Thanks,
Mike


--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

Re: [PATCH 4/8] make newrole suid (take 3)

[Serge E. Hallyn]

Quoting Michael C Thompson (thompsmc@us.ibm.com):
> This is a short list of what we do prior to the uid change:
> - locale actions
> - calls into libselinux, libaudit and libc
> - calls to pam
> 
> In short, for the case where we want namespace support, we execute as 
> root for nearly the entire lifetime of the application, and the parent 
> process retains euid 0 as well while the shell has been exec'd and is 
> waiting for the child to die.
> 
> We don't actually *need* to do this, but my reason was that if we retain 
> the ability to setuid and retain the capabilities on the permitted (but 
> not effective) list, then if an exploit would be discovered for the 
> program, then all the attacker would need to do is re-raise those 
> capabilities and game over.
> 
> I've only briefly consider the effects of maintaining euid=0 during the 
> life-time of the application, but it seems to me the only impact it will 
> have is during the pam_open_session call.
> 
> Does that make sense?

If I'm thinking straight, the main advantage of still dropping euid=0
is that if the attacker has limited space for shellcode, or for some
other reason can only do exec(/bin/sh), the caps will be dropped on
exec, but euid=0 will not.

-serge

--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

Re: [PATCH 4/8] make newrole suid (take 3)

[Stephen Smalley]

On Wed, 2006-11-08 at 23:15 -0600, Serge E. Hallyn wrote:
> Quoting Michael C Thompson (thompsmc@us.ibm.com):
> > This is a short list of what we do prior to the uid change:
> > - locale actions
> > - calls into libselinux, libaudit and libc
> > - calls to pam
> > 
> > In short, for the case where we want namespace support, we execute as 
> > root for nearly the entire lifetime of the application, and the parent 
> > process retains euid 0 as well while the shell has been exec'd and is 
> > waiting for the child to die.
> > 
> > We don't actually *need* to do this, but my reason was that if we retain 
> > the ability to setuid and retain the capabilities on the permitted (but 
> > not effective) list, then if an exploit would be discovered for the 
> > program, then all the attacker would need to do is re-raise those 
> > capabilities and game over.
> > 
> > I've only briefly consider the effects of maintaining euid=0 during the 
> > life-time of the application, but it seems to me the only impact it will 
> > have is during the pam_open_session call.
> > 
> > Does that make sense?
> 
> If I'm thinking straight, the main advantage of still dropping euid=0
> is that if the attacker has limited space for shellcode, or for some
> other reason can only do exec(/bin/sh), the caps will be dropped on
> exec, but euid=0 will not.

pam_namespace needs to run with fsuid 0 (so that member directories it
creates are immediately assigned a uid that is not accessible to the
unprivileged caller).  Further, in the absence of a kernel with the
fscaps patch, reverting uid while retaining capabilities (which
pam_namespace also requires) is potentially dangerous, as it exposes the
process to e.g. signals and other manipulation by the original uid.

-- 
Stephen Smalley
National Security Agency


--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

Re: [PATCH 4/8] make newrole suid (take 3)

[Serge E. Hallyn]

Quoting Stephen Smalley (sds@tycho.nsa.gov):
> On Wed, 2006-11-08 at 23:15 -0600, Serge E. Hallyn wrote:
> > Quoting Michael C Thompson (thompsmc@us.ibm.com):
> > > This is a short list of what we do prior to the uid change:
> > > - locale actions
> > > - calls into libselinux, libaudit and libc
> > > - calls to pam
> > > 
> > > In short, for the case where we want namespace support, we execute as 
> > > root for nearly the entire lifetime of the application, and the parent 
> > > process retains euid 0 as well while the shell has been exec'd and is 
> > > waiting for the child to die.
> > > 
> > > We don't actually *need* to do this, but my reason was that if we retain 
> > > the ability to setuid and retain the capabilities on the permitted (but 
> > > not effective) list, then if an exploit would be discovered for the 
> > > program, then all the attacker would need to do is re-raise those 
> > > capabilities and game over.
> > > 
> > > I've only briefly consider the effects of maintaining euid=0 during the 
> > > life-time of the application, but it seems to me the only impact it will 
> > > have is during the pam_open_session call.
> > > 
> > > Does that make sense?
> > 
> > If I'm thinking straight, the main advantage of still dropping euid=0
> > is that if the attacker has limited space for shellcode, or for some
> > other reason can only do exec(/bin/sh), the caps will be dropped on
> > exec, but euid=0 will not.
> 
> pam_namespace needs to run with fsuid 0 (so that member directories it
> creates are immediately assigned a uid that is not accessible to the

Actually a quick test (which I may be doing wrong) and the setfsuid
manpage suggests files are created as the euid, not fsuid.

I suppose a reserved non-root uid could be used, which would address
both the created file and malicious signals you worry about, as well
as the retention of uid across a shellcoded exec.

But I really don't mean to beat a dead horse.  Thank you both for the
explanations, the patch does look sane.  (On my first reading I'd
actually misread the prctl and thought it was going to keep caps
across the final setuid).

thanks,
-serge

> unprivileged caller).  Further, in the absence of a kernel with the
> fscaps patch, reverting uid while retaining capabilities (which
> pam_namespace also requires) is potentially dangerous, as it exposes the
> process to e.g. signals and other manipulation by the original uid.


--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

Re: [PATCH 4/8] make newrole suid (take 3)

[Stephen Smalley]

On Thu, 2006-11-09 at 10:37 -0600, Serge E. Hallyn wrote:
> Quoting Stephen Smalley (sds@tycho.nsa.gov):
> > On Wed, 2006-11-08 at 23:15 -0600, Serge E. Hallyn wrote:
> > > Quoting Michael C Thompson (thompsmc@us.ibm.com):
> > > > This is a short list of what we do prior to the uid change:
> > > > - locale actions
> > > > - calls into libselinux, libaudit and libc
> > > > - calls to pam
> > > > 
> > > > In short, for the case where we want namespace support, we execute as 
> > > > root for nearly the entire lifetime of the application, and the parent 
> > > > process retains euid 0 as well while the shell has been exec'd and is 
> > > > waiting for the child to die.
> > > > 
> > > > We don't actually *need* to do this, but my reason was that if we retain 
> > > > the ability to setuid and retain the capabilities on the permitted (but 
> > > > not effective) list, then if an exploit would be discovered for the 
> > > > program, then all the attacker would need to do is re-raise those 
> > > > capabilities and game over.
> > > > 
> > > > I've only briefly consider the effects of maintaining euid=0 during the 
> > > > life-time of the application, but it seems to me the only impact it will 
> > > > have is during the pam_open_session call.
> > > > 
> > > > Does that make sense?
> > > 
> > > If I'm thinking straight, the main advantage of still dropping euid=0
> > > is that if the attacker has limited space for shellcode, or for some
> > > other reason can only do exec(/bin/sh), the caps will be dropped on
> > > exec, but euid=0 will not.
> > 
> > pam_namespace needs to run with fsuid 0 (so that member directories it
> > creates are immediately assigned a uid that is not accessible to the
> 
> Actually a quick test (which I may be doing wrong) and the setfsuid
> manpage suggests files are created as the euid, not fsuid.

Look at ext3_new_inode().  The fsuid is assigned to newly created
inodes.  However, it is true that the fsuid shadows the euid
automatically, so if you set the euid, the fsuid will match.  But you
can independently set the fsuid without affecting the euid.

> I suppose a reserved non-root uid could be used, which would address
> both the created file and malicious signals you worry about, as well
> as the retention of uid across a shellcoded exec.
>
> But I really don't mean to beat a dead horse.  Thank you both for the
> explanations, the patch does look sane.  (On my first reading I'd
> actually misread the prctl and thought it was going to keep caps
> across the final setuid).

-- 
Stephen Smalley
National Security Agency


--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

Re: [PATCH 4/8] make newrole suid (take 3)

[Serge E. Hallyn]

Quoting Stephen Smalley (sds@tycho.nsa.gov):
> On Thu, 2006-11-09 at 10:37 -0600, Serge E. Hallyn wrote:
> > Quoting Stephen Smalley (sds@tycho.nsa.gov):
> > > On Wed, 2006-11-08 at 23:15 -0600, Serge E. Hallyn wrote:
> > > > Quoting Michael C Thompson (thompsmc@us.ibm.com):
> > > > > This is a short list of what we do prior to the uid change:
> > > > > - locale actions
> > > > > - calls into libselinux, libaudit and libc
> > > > > - calls to pam
> > > > > 
> > > > > In short, for the case where we want namespace support, we execute as 
> > > > > root for nearly the entire lifetime of the application, and the parent 
> > > > > process retains euid 0 as well while the shell has been exec'd and is 
> > > > > waiting for the child to die.
> > > > > 
> > > > > We don't actually *need* to do this, but my reason was that if we retain 
> > > > > the ability to setuid and retain the capabilities on the permitted (but 
> > > > > not effective) list, then if an exploit would be discovered for the 
> > > > > program, then all the attacker would need to do is re-raise those 
> > > > > capabilities and game over.
> > > > > 
> > > > > I've only briefly consider the effects of maintaining euid=0 during the 
> > > > > life-time of the application, but it seems to me the only impact it will 
> > > > > have is during the pam_open_session call.
> > > > > 
> > > > > Does that make sense?
> > > > 
> > > > If I'm thinking straight, the main advantage of still dropping euid=0
> > > > is that if the attacker has limited space for shellcode, or for some
> > > > other reason can only do exec(/bin/sh), the caps will be dropped on
> > > > exec, but euid=0 will not.
> > > 
> > > pam_namespace needs to run with fsuid 0 (so that member directories it
> > > creates are immediately assigned a uid that is not accessible to the
> > 
> > Actually a quick test (which I may be doing wrong) and the setfsuid
> > manpage suggests files are created as the euid, not fsuid.
> 
> Look at ext3_new_inode().  The fsuid is assigned to newly created

Yup, I see it.  I don't see why my testcase didn't do it.

In any case the open(2) manpage still says that the owner is set to the
euid.  I suppose that should have a linux-specific note...

> inodes.  However, it is true that the fsuid shadows the euid
> automatically, so if you set the euid, the fsuid will match.  But you
> can independently set the fsuid without affecting the euid.

Yes, I hung onto cap_setuid while setting euid, then set fsuid back to
0, then did creat(), but the owner remained the euid.

I'll need to figure out why sometime later.

thanks,
-serge

--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

Re: [PATCH 4/8] make newrole suid (take 3)

[Michael C Thompson]

Serge E. Hallyn wrote:
> But I really don't mean to beat a dead horse.  Thank you both for the
> explanations, the patch does look sane.  (On my first reading I'd
> actually misread the prctl and thought it was going to keep caps
> across the final setuid).

So the fallout from this is: no changes need to be made to the patches?

Thanks,
Mike


--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

Re: [PATCH 4/8] make newrole suid (take 3)

[Stephen Smalley]

On Thu, 2006-11-09 at 14:22 -0600, Michael C Thompson wrote:
> Serge E. Hallyn wrote:
> > But I really don't mean to beat a dead horse.  Thank you both for the
> > explanations, the patch does look sane.  (On my first reading I'd
> > actually misread the prctl and thought it was going to keep caps
> > across the final setuid).
> 
> So the fallout from this is: no changes need to be made to the patches?

Yes.

-- 
Stephen Smalley
National Security Agency


--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

[PATCH 5/8] make newrole suid (take 3)

[Michael C Thompson]

[-- Attachment #1: Type: text/plain, Size: 968 bytes --]

Michael C Thompson wrote:
> The 8 patches are as follows:
> 1) Modifications to Makefile to support future patch needs
>    Add newrole-lspp.pamd
> 2) New extract_pw_data function and use in main()
> 3) Add signal handler function
> 4) Update drop_capabilities() and use in main()
> 5) Update the authentication functions and use in main()
>    Add cleanup since pam_start is now left till program end

This is the 5th of 8 patches.
This patch applies against policycoreutils-1.30.30-1.

This patch updates the authentication functions so that they are
cleaner, and moves pam_start() into main() in prep for namespace
support.

Changes:
  * Updates the authentication functions
    - Noteably, pam_start is now done in main()
    - Removed a lot of very obvious comments and clutter
  * Adds pam variables to main()
  * Adds pam_start() to main()
  * Adds cleanup gotos to main() for pam_end() and missing frees

Signed-off-by: Michael Thompson <thompsmc@us.ibm.com>


[-- Attachment #2: 05-update_authentication.patch --]
[-- Type: text/x-diff, Size: 9312 bytes --]

diff -Naur policycoreutils-1.30.30/newrole/newrole.c policycoreutils-1.30.30.suid/newrole/newrole.c
--- policycoreutils-1.30.30/newrole/newrole.c	2006-11-02 12:28:46.000000000 -0600
+++ policycoreutils-1.30.30.suid/newrole/newrole.c	2006-11-02 12:37:17.000000000 -0600
@@ -150,16 +150,11 @@
  * All PAM code goes in this section.
  *
  ************************************************************************/
-
-#include <unistd.h>		/* for getuid(), exit(), getopt() */
-
 #include <security/pam_appl.h>	/* for PAM functions */
 #include <security/pam_misc.h>	/* for misc_conv PAM utility function */
 
 #define SERVICE_NAME "newrole"	/* the name of this program for PAM */
 
-int authenticate_via_pam(const struct passwd *, const char *);
-
 /* authenticate_via_pam()
  *
  * in:     pw - struct containing data from our user's line in 
@@ -173,63 +168,39 @@
  * This function uses PAM to authenticate the user running this
  * program.  This is the only function in this program that makes PAM
  * calls.
- *
  */
-
-int authenticate_via_pam(const struct passwd *pw, const char *ttyn)
+int authenticate_via_pam(const char *ttyn, pam_handle_t *pam_handle)
 {
 
-	int result = 0;		/* our result, set to 0 (not authenticated) by default */
-	int rc;			/* pam return code */
-	pam_handle_t *pam_handle;	/* opaque handle used by all PAM functions */
+	int result = 0;		/* set to 0 (not authenticated) by default */
+	int pam_rc;		/* pam return code */
 	const char *tty_name;
 
-	/* This is a jump table of functions for PAM to use when it wants to *
-	 * communicate with the user.  We'll be using misc_conv(), which is  *
-	 * provided for us via pam_misc.h.                                   */
-	struct pam_conv pam_conversation = {
-		misc_conv,
-		NULL
-	};
-
-	/* Make `p_pam_handle' a valid PAM handle so we can use it when *
-	 * calling PAM functions.                                       */
-	rc = pam_start(SERVICE_NAME,
-		       pw->pw_name, &pam_conversation, &pam_handle);
-	if (rc != PAM_SUCCESS) {
-		fprintf(stderr, _("failed to initialize PAM\n"));
-		exit(-1);
-	}
-
 	if (strncmp(ttyn, "/dev/", 5) == 0)
 		tty_name = ttyn + 5;
 	else
 		tty_name = ttyn;
 
-	rc = pam_set_item(pam_handle, PAM_TTY, tty_name);
-	if (rc != PAM_SUCCESS) {
+	pam_rc = pam_set_item(pam_handle, PAM_TTY, tty_name);
+	if (pam_rc != PAM_SUCCESS) {
 		fprintf(stderr, _("failed to set PAM_TTY\n"));
 		goto out;
 	}
 
 	/* Ask PAM to authenticate the user running this program */
-	rc = pam_authenticate(pam_handle, 0);
-	if (rc != PAM_SUCCESS) {
+	pam_rc = pam_authenticate(pam_handle, 0);
+	if (pam_rc != PAM_SUCCESS) {
 		goto out;
 	}
 
 	/* Ask PAM to verify acct_mgmt */
-	rc = pam_acct_mgmt(pam_handle, 0);
-	if (rc == PAM_SUCCESS) {
+	pam_rc = pam_acct_mgmt(pam_handle, 0);
+	if (pam_rc == PAM_SUCCESS) {
 		result = 1;	/* user authenticated OK! */
 	}
 
-	/* We're done with PAM.  Free `pam_handle'. */
       out:
-	pam_end(pam_handle, rc);
-
-	return (result);
-
+	return result;
 }				/* authenticate_via_pam() */
 
 #else				/* else !USE_PAM */
@@ -239,19 +210,14 @@
  * All shadow passwd code goes in this section.
  *
  ************************************************************************/
-
-#include <unistd.h>		/* for getuid(), exit(), crypt() */
 #include <shadow.h>		/* for shadow passwd functions */
 #include <string.h>		/* for strlen(), memset() */
 
 #define PASSWORD_PROMPT _("Password:")	/* prompt for getpass() */
 
-int authenticate_via_shadow_passwd(const struct passwd *);
-
 /* authenticate_via_shadow_passwd()
  *
- * in:     pw - struct containing data from our user's line in 
- *                         the passwd file.
+ * in:     uname - the calling user's user name
  * out:    nothing
  * return: value   condition
  *         -----   ---------
@@ -261,48 +227,34 @@
  *
  * This function uses the shadow passwd file to thenticate the user running
  * this program.
- *
  */
-
-int authenticate_via_shadow_passwd(const struct passwd *pw)
+int authenticate_via_shadow_passwd(const char *uname)
 {
-
-	struct spwd *p_shadow_line;	/* struct derived from shadow passwd file line */
-	char *unencrypted_password_s;	/* unencrypted password input by user */
-	char *encrypted_password_s;	/* user's password input after being crypt()ed */
-
-	/* Make `p_shadow_line' point to the data from the current user's *
-	 * line in the shadow passwd file.                                */
-	setspent();		/* Begin access to the shadow passwd file. */
-	p_shadow_line = getspnam(pw->pw_name);
-	endspent();		/* End access to the shadow passwd file. */
+	struct spwd *p_shadow_line;
+	char *unencrypted_password_s;
+	char *encrypted_password_s;
+
+	setspent();
+	p_shadow_line = getspnam(uname);
+	endspent();
 	if (!(p_shadow_line)) {
-		fprintf(stderr,
-			_
-			("Cannot find your entry in the shadow passwd file.\n"));
-		exit(-1);
+		fprintf(stderr, _("Cannot find your entry in the shadow "
+			"passwd file.\n"));
+		return 0;
 	}
 
 	/* Ask user to input unencrypted password */
 	if (!(unencrypted_password_s = getpass(PASSWORD_PROMPT))) {
 		fprintf(stderr, _("getpass cannot open /dev/tty\n"));
-		exit(-1);
+		return 0;
 	}
 
-	/* Use crypt() to encrypt user's input password.  Clear the *
-	 * unencrypted password as soon as we're done, so it is not * 
-	 * visible to memory snoopers.                              */
+	/* Use crypt() to encrypt user's input password. */
 	encrypted_password_s = crypt(unencrypted_password_s,
 				     p_shadow_line->sp_pwdp);
 	memset(unencrypted_password_s, 0, strlen(unencrypted_password_s));
-
-	/* Return 1 (authenticated) iff the encrypted version of the user's *
-	 * input password matches the encrypted password stored in the      *
-	 * shadow password file.                                            */
 	return (!strcmp(encrypted_password_s, p_shadow_line->sp_pwdp));
-
-}				/* authenticate_via_shadow_passwd() */
-
+}
 #endif				/* if/else USE_PAM */
 
 /*
@@ -626,10 +578,22 @@
 	char *level_s = NULL;	/* level spec'd by user in argv[] */
 	char *ttyn = NULL;	/* tty path */
 	pid_t childPid = 0;
-	uid_t uid;
-	int fd;
+	int fd, rc;
 	int enforcing;
 
+#ifdef USE_PAM
+	int pam_status;		   /* pam return code */
+	pam_handle_t *pam_handle;  /* opaque handle used by all PAM functions */
+
+	/* This is a jump table of functions for PAM to use when it wants to *
+	 * communicate with the user.  We'll be using misc_conv(), which is  *
+	 * provided for us via pam_misc.h.                                   */
+	struct pam_conv pam_conversation = {
+		misc_conv,
+		NULL
+	};
+#endif
+
 	/*
 	 * Step 0: Setup
 	 *
@@ -771,26 +735,6 @@
 		exit(-1);
 	}
 
-	/*
-	 * Determine the Linux user identity to re-authenticate.
-	 * If supported and set, use the login uid, as this should be more stable.
-	 * Otherwise, use the real uid.
-	 * The SELinux user identity is no longer used, as Linux users are now
-	 * mapped to SELinux users via seusers and the SELinux user identity space
-	 * is separate.
-	 */
-#ifdef USE_AUDIT
-	uid = audit_getloginuid();
-	if (uid == (uid_t) - 1)
-		uid = getuid();
-#else
-	uid = getuid();
-#endif
-
-	/* Get the passwd info for the Linux user identity. */
-	if (extract_pw_data(&pw))
-		return -1;
-
 	/* Get the tty name. Pam will need it. */
 	ttyn = ttyname(0);
 	if (!ttyn || *ttyn == '\0') {
@@ -799,28 +743,36 @@
 		exit(-1);
 	}
 
-	printf(_("Authenticating %s.\n"), pw.pw_name);
+	/* Get the passwd info for the Linux user identity. */
+	if (extract_pw_data(&pw))
+		return -1;
 
-	/* 
+	/*
+	 * Step 2:  Authenticate the user.
+	 *
 	 * Re-authenticate the user running this program.
 	 * This is just to help confirm user intent (vs. invocation by
 	 * malicious software), not to authorize the operation (which is covered
 	 * by policy).  Trusted path mechanism would be preferred.
 	 */
+	printf(_("Authenticating %s.\n"), pw.pw_name);
 #ifdef USE_PAM
-	if (!authenticate_via_pam(&pw, ttyn))
-#else				/* !USE_PAM */
-	if (!authenticate_via_shadow_passwd(&pw))
-#endif				/* if/else USE_PAM */
+	pam_status = pam_start(SERVICE_NAME, pw.pw_name, &pam_conversation,
+			       &pam_handle);
+	if (pam_status != PAM_SUCCESS) {
+		fprintf(stderr, _("failed to initialize PAM\n"));
+		goto err_free;
+	}
+
+	if (!authenticate_via_pam(ttyn, pam_handle))
+#else
+	if (!authenticate_via_shadow_passwd(pw.pw_name))
+#endif
 	{
 		fprintf(stderr, _("newrole: incorrect password for %s\n"),
 			pw.pw_name);
-		return (-1);
+		goto err_close_pam;
 	}
-	/* If we reach here, then we have authenticated the user. */
-#ifdef CANTSPELLGDB
-	printf("You are authenticated!\n");
-#endif
 
 	/*
 	 *
@@ -1062,7 +1014,26 @@
 	freecon(old_context);
 	execv(pw.pw_shell, argv + optind - 1);
 
-	/* If we reach here, then we failed to exec the new shell. */
+	/*
+	 * Error path cleanup
+	 *
+	 * If we reach here, then we failed to exec the new shell.
+	 */
 	perror(_("failed to exec shell\n"));
-	return (-1);
+err_close_pam:
+#ifdef USE_PAM
+	rc = pam_end(pam_handle, pam_status);
+	if (rc != PAM_SUCCESS)
+		fprintf(stderr, "pam_end failed with %s\n",
+			pam_strerror(pam_handle, rc));
+#endif
+err_free:
+	freecon(tty_context);
+	freecon(new_tty_context);
+	freecon(old_context);
+	freecon(new_context);
+	free(pw.pw_name);
+	free(pw.pw_dir);
+	free(pw.pw_shell);
+	return -1;
 }				/* main() */

[PATCH 6/8] make newrole suid (take 3)

[Michael C Thompson]

[-- Attachment #1: Type: text/plain, Size: 921 bytes --]

Michael C Thompson wrote:
> The 8 patches are as follows:
> 1) Modifications to Makefile to support future patch needs
>    Add newrole-lspp.pamd
> 2) New extract_pw_data function and use in main()
> 3) Add signal handler function
> 4) Update drop_capabilities() and use in main()
> 5) Update the authentication functions and use in main()
>    Add cleanup since pam_start is now left till program end
> 6) Move relabeling tty actions into functions

This is the 6th of 8 patches.
This patch applies against policycoreutils-1.30.30-1.

This patch moves the tty relabeling actions into their own
functions and adds better cleanup to main on error paths.

Changes:
  * Introduces relabel_tty() and restore_tty_label()
    - Move functionality from main() into functions
  * Uses the above new functions in main()
  * Updates the parent process to have better cleanup

Signed-off-by: Michael Thompson <thompsmc@us.ibm.com>


[-- Attachment #2: 06-update_relabeling.patch --]
[-- Type: text/x-diff, Size: 8301 bytes --]

diff -Naur policycoreutils-1.30.30/newrole/newrole.c policycoreutils-1.30.30.suid/newrole/newrole.c
--- policycoreutils-1.30.30/newrole/newrole.c	2006-11-02 12:38:27.000000000 -0600
+++ policycoreutils-1.30.30.suid/newrole/newrole.c	2006-11-02 12:48:46.000000000 -0600
@@ -522,6 +522,113 @@
 #endif
 
 /**
+ * This function attempts to relabel the tty. If this function fails, then
+ * the fd is closed, the contexts are free'd and -1 is returned. On success,
+ * a valid fd is returned and tty_context and new_tty_context are set.
+ *
+ * This function will not fail if it can not relabel the tty when selinux is
+ * in permissive mode.
+ */
+static int relabel_tty(const char *ttyn, security_context_t new_context,
+			security_context_t *tty_context,
+			security_context_t *new_tty_context)
+{
+	int fd;
+	int enforcing = security_getenforce();
+	security_context_t tty_con = NULL;
+	security_context_t new_tty_con = NULL;
+
+	if (enforcing < 0) {
+		fprintf(stderr, _("Could not determine enforcing mode.\n"));
+		return -1;
+	}
+
+	/* Re-open TTY descriptor */
+	fd = open(ttyn, O_RDWR);
+	if (fd < 0) {
+		fprintf(stderr, _("Error!  Could not open %s.\n"), ttyn);
+		return fd;
+	}
+
+	if (fgetfilecon(fd, &tty_con) < 0) {
+		fprintf(stderr, _("%s!  Could not get current context "
+			"for %s, not relabeling tty.\n"),
+			enforcing ? "Error" : "Warning", ttyn);
+		if (enforcing)
+			goto close_fd;
+	}
+
+	if (tty_con &&
+	    (security_compute_relabel(new_context, tty_con,
+				      SECCLASS_CHR_FILE, &new_tty_con) < 0)) {
+		fprintf(stderr,	_("%s!  Could not get new context for %s, "
+			"not relabeling tty.\n"),
+			enforcing ? "Error" : "Warning", ttyn);
+		if (enforcing)
+			goto close_fd;
+	}
+
+	if (new_tty_con)
+		if (fsetfilecon(fd, new_tty_con) < 0) {
+			fprintf(stderr,
+				_("%s!  Could not set new context for %s\n"),
+				enforcing ? "Error" : "Warning", ttyn);
+			freecon(new_tty_con);
+			new_tty_con = NULL;
+			if (enforcing)
+				goto close_fd;
+		}
+
+	*tty_context = tty_con;
+	*new_tty_context = new_tty_con;
+	return fd;
+
+close_fd:
+	freecon(tty_con);
+	close(fd);
+	return -1;
+}
+
+/**
+ * This function attempts to revert the relabeling done to the tty.
+ * fd   - referencing the opened ttyn
+ * ttyn - name of tty to restore
+ * tty_context     - original context of the tty
+ * new_tty_context - context tty was relabeled to
+ *
+ * Returns zero on success, non-zero otherwise
+ */
+static int restore_tty_label(int fd, const char *ttyn,
+			     security_context_t tty_context,
+			     security_context_t new_tty_context)
+{
+	int rc = 0;
+	security_context_t chk_tty_context = NULL;
+
+	if (!new_tty_context)
+		goto skip_relabel;
+
+	/* Verify that the tty still has the context set by newrole. */
+	if ((rc = fgetfilecon(fd, &chk_tty_context)) < 0) {
+		fprintf(stderr, "Could not fgetfilecon %s.\n", ttyn);
+		goto skip_relabel;
+	}
+
+	if ((rc = strcmp(chk_tty_context, new_tty_context))) {
+		fprintf(stderr, _("%s changed labels.\n"), ttyn);
+		goto skip_relabel;
+	}
+
+	if ((rc = fsetfilecon(fd, tty_context)) < 0)
+		fprintf(stderr,
+			_("Warning! Could not restore context for %s\n"), ttyn);
+skip_relabel:
+	freecon(chk_tty_context);
+	return rc;
+}
+
+
+/**
  * Take care of any signal setup
  */
 static int set_signal_handles()
@@ -558,7 +665,6 @@
 	security_context_t old_context = NULL;	/* our original securiy context */
 	security_context_t tty_context = NULL;	/* The current context of tty file */
 	security_context_t new_tty_context = NULL;	/* The new context of tty file */
-	security_context_t chk_tty_context = NULL;
 
 	context_t context;	/* manipulatable form of new_context */
 
@@ -580,6 +686,7 @@
 	pid_t childPid = 0;
 	int fd, rc;
 	int enforcing;
+	char *shell_argv0 = NULL;
 
 #ifdef USE_PAM
 	int pam_status;		   /* pam return code */
@@ -855,110 +962,77 @@
 	}
 
 	/*
+	 * Step 3:  Handle relabeling of the tty.
 	 *
-	 * Step 4:  Handle relabeling of the tty.
-	 *
+	 * Once we authenticate the user, we know that we want to proceed with
+	 * the action. Prior to this point, no changes are made the to system.
 	 */
+	fd = relabel_tty(ttyn, new_context, &tty_context, &new_tty_context);
+	if (fd < 0)
+		goto err_close_pam;
 
-	/* Re-open TTY descriptor */
-	fd = open(ttyn, O_RDWR);
-	if (fd < 0) {
-		fprintf(stderr, _("Error!  Could not open %s.\n"), ttyn);
-		exit(-1);
-	}
-
-	tty_context = NULL;
-	if (fgetfilecon(fd, &tty_context) < 0) {
-		fprintf(stderr,
-			_
-			("%s!  Could not get current context for %s, not relabeling tty.\n"),
-			enforcing ? "Error" : "Warning", ttyn);
-		if (enforcing)
-			exit(-1);
-	}
-#ifdef CANTSPELLGDB
-	if (tty_context)
-		printf("Your tty %s was labeled with context %s\n", ttyn,
-		       tty_context);
-#endif
-
-	new_tty_context = NULL;
-	if (tty_context
-	    &&
-	    (security_compute_relabel
-	     (new_context, tty_context, SECCLASS_CHR_FILE,
-	      &new_tty_context) < 0)) {
-		fprintf(stderr,
-			_
-			("%s!  Could not get new context for %s, not relabeling tty.\n"),
-			enforcing ? "Error" : "Warning", ttyn);
-		if (enforcing)
-			exit(-1);
-	}
-#ifdef CANTSPELLGDB
-	if (new_tty_context)
-		printf("Relabeling tty %s to context %s\n", ttyn,
-		       new_tty_context);
-#endif
-
-	if (new_tty_context) {
-		if (fsetfilecon(fd, new_tty_context) < 0) {
-			fprintf(stderr,
-				_("%s!  Could not set new context for %s\n"),
-				enforcing ? "Error" : "Warning", ttyn);
-			freecon(new_tty_context);
-			new_tty_context = NULL;
-			if (enforcing)
-				exit(-1);
-		}
-	}
-
-	/* Fork, allowing parent to clean up after shell has executed */
+	/*
+	 * Step 4: Fork
+	 *
+	 * Fork, allowing parent to clean up after shell has executed.
+	 * Child: reopen stdin, stdout, stderr and exec shell
+	 * Parnet: wait for child to die and restore tty's context
+	 */
 	childPid = fork();
 	if (childPid < 0) {
+		/* fork failed, no child to worry about */
 		int errsv = errno;
 		fprintf(stderr, _("newrole: failure forking: %s"),
 			strerror(errsv));
-		if (fsetfilecon(fd, tty_context) < 0)
-			fprintf(stderr,
-				_
-				("Warning!  Could not restore context for %s\n"),
-				ttyn);
-		freecon(tty_context);
-		exit(-1);
+		if (restore_tty_label(fd, ttyn, tty_context, new_tty_context))
+			fprintf(stderr, _("Unable to restore tty label...\n"));
+		if (close(fd))
+			fprintf(stderr, _("Failed to close tty properly\n"));
+		goto err_close_pam;
 	} else if (childPid) {
-		/* PARENT */
+		/* PARENT
+		 * It doesn't make senes to exit early on errors at this point,
+		 * since we are doing cleanup which needs to be done.
+		 * We can exit with a bad rc though
+		 */
 		int rc;
+		int exit_code = 0;
+
 		do {
 			rc = wait(NULL);
 		} while (rc < 0 && errno == EINTR);
 
-		if (!new_tty_context || !tty_context)
-			exit(0);
-
-		/* Verify that the tty still has the context set by newrole. */
-		if (fgetfilecon(fd, &chk_tty_context) < 0) {
-			fprintf(stderr, "Could not fgetfilecon %s.\n", ttyn);
-			exit(-1);
-		}
-
-		if (strcmp(chk_tty_context, new_tty_context)) {
-			fprintf(stderr, _("%s changed labels.\n"), ttyn);
-			exit(-1);
+		if (restore_tty_label(fd, ttyn, tty_context, new_tty_context)) {
+			fprintf(stderr, _("Unable to restore tty label...\n"));
+			exit_code = -1;
 		}
-
+		freecon(tty_context);
 		freecon(new_tty_context);
-
-#ifdef CANTSPELLGDB
-		printf("Restoring tty %s back to context %s\n", ttyn,
-		       tty_context);
+		if (close(fd)) {
+			fprintf(stderr, _("Failed to close tty properly\n"));
+			exit_code = -1;
+		}
+#ifdef USE_PAM
+#ifdef NAMESPACE_PRIV
+		pam_status = pam_close_session(pam_handle,0);
+		if (pam_status != PAM_SUCCESS) {
+			fprintf(stderr, "pam_close_session failed with %s\n",
+				pam_strerror(pam_handle, pam_status));
+			exit_code = -1;
+		}
 #endif
-
-		fsetfilecon(fd, tty_context);
-		freecon(tty_context);
-
-		/* Done! */
-		exit(0);
+		rc = pam_end(pam_handle, pam_status);
+		if (rc != PAM_SUCCESS) {
+			fprintf(stderr, "pam_end failed with %s\n",
+				pam_strerror(pam_handle, rc));
+			exit_code = -1;
+		}
+#endif
+		free(pw.pw_name);
+		free(pw.pw_dir);
+		free(pw.pw_shell);
+		free(shell_argv0);
+		return exit_code;
 	}
 
 	/* CHILD */
@@ -1035,5 +1109,6 @@
 	free(pw.pw_name);
 	free(pw.pw_dir);
 	free(pw.pw_shell);
+	free(shell_argv0);
 	return -1;
 }				/* main() */

[PATCH 7/8] make newrole suid (take 3)

[Michael C Thompson]

[-- Attachment #1: Type: text/plain, Size: 1229 bytes --]

Michael C Thompson wrote:
> The 8 patches are as follows:
> 1) Modifications to Makefile to support future patch needs
>    Add newrole-lspp.pamd
> 2) New extract_pw_data function and use in main()
> 3) Add signal handler function
> 4) Update drop_capabilities() and use in main()
> 5) Update the authentication functions and use in main()
>    Add cleanup since pam_start is now left till program end
> 6) Move relabeling tty actions into functions
> 7) Move command-line argument parsing into a function
>    Clear the environment during execution
>    Add support for preserving the environment (-p)

This is the 7th of 8 patches.
This patch applies against policycoreutils-1.30.30-1.

This function introduces a sanitized environment during the life-time
of newrole's execution, and sets the environment to either the preserved
environment or a minimal environment before shell execution.

Changes:
  * Introduces restore_environment()
    - New functionality, for preserving the environment or sanitizing it
  * Introduces parse_command_line_arguments()
    - Move functionality from main() into parse_command_line_arguments
  * Uses the above new functions in main()

Signed-off-by: Michael Thompson <thompsmc@us.ibm.com>


[-- Attachment #2: 07-add_preserve_env.patch --]
[-- Type: text/x-diff, Size: 16184 bytes --]

diff -Naur policycoreutils-1.30.30/newrole/newrole.c policycoreutils-1.30.30.suid/newrole/newrole.c
--- policycoreutils-1.30.30/newrole/newrole.c	2006-11-02 12:48:59.000000000 -0600
+++ policycoreutils-1.30.30.suid/newrole/newrole.c	2006-11-02 12:58:52.000000000 -0600
@@ -85,10 +85,13 @@
 #endif
 
 /* USAGE_STRING describes the command-line args of this program. */
-#define USAGE_STRING "USAGE: newrole [ -r role ] [ -t type ] [ -l level ] [ -V ] [ -- args ]"
+#define USAGE_STRING "USAGE: newrole [ -r role ] [ -t type ] [ -l level ] [ -p ] [ -V ] [ -- args ]"
 
+#define DEFAULT_PATH "/usr/bin:/bin"
 #define DEFAULT_CONTEXT_SIZE 255	/* first guess at context size */
 
+extern char **environ;
+
 char *xstrdup(const char *s)
 {
 	char *s2;
@@ -340,6 +343,74 @@
 }
 
 /**
+ * Either restore the original environment, or set up a minimal one.
+ *
+ * The minimal environment contains:
+ * TERM, DISPLAY and XAUTHORITY - if they are set, preserve values
+ * HOME, SHELL, USER and LOGNAME - set to contents of /etc/passwd
+ * PATH - set to default value DEFAULT_PATH
+ *
+ * Returns zero on success, non-zero otherwise
+ */
+static int restore_environment(int preserve_environment,
+			       char **old_environ, const struct passwd *pw)
+{
+	char const *term_env;
+	char const *display_env;
+	char const *xauthority_env;
+	char *term = NULL;		/* temporary container */
+	char *display = NULL;		/* temporary container */
+	char *xauthority = NULL;	/* temporary container */
+	int rc;
+
+	environ = old_environ;
+
+	if (preserve_environment)
+		return 0;
+
+	term_env = getenv("TERM");
+	display_env = getenv("DISPLAY");
+	xauthority_env = getenv("XAUTHORITY");
+
+	/* Save the variable values we want */
+	if (term_env)
+		term = strdup(term_env);
+	if (display_env)
+		display = strdup(display_env);
+	if (xauthority_env)
+		xauthority = strdup(xauthority_env);
+	if ((term_env && !term) || (display_env && !display) ||
+	     (xauthority_env && !xauthority)) {
+		rc = -1;
+		goto out;
+	}
+
+	/* Construct a new environment */
+	if ((rc = clearenv())) {
+		fprintf(stderr, _("Unable to clear environment\n"));
+		goto out;
+	}
+
+	/* Restore that which we saved */
+	if (term)
+		rc |= setenv("TERM", term, 1);
+	if (display)
+		rc |= setenv("DISPLAY", display, 1);
+	if (xauthority)
+		rc |= setenv("XAUTHORITY", xauthority, 1);
+	rc |= setenv("HOME", pw->pw_dir, 1);
+	rc |= setenv("SHELL", pw->pw_shell, 1);
+	rc |= setenv("USER", pw->pw_name, 1);
+	rc |= setenv("LOGNAME", pw->pw_name, 1);
+	rc |= setenv("PATH", DEFAULT_PATH, 1);
+out:
+	free(term);
+	free(display);
+	free(xauthority);
+	return rc;
+}
+
+/**
  * This function will drop the capabilities so that we are left
  * only with access to the audit system. If the user is root, we leave
  * the capabilities alone since they already should have access to the
@@ -627,6 +698,172 @@
 	return rc;
 }
 
+/**
+ * Parses and validates the provided command line options and
+ * constructs a new context based on our old context and the
+ * arguments specified on the command line. On success
+ * new_context will be set to valid values, otherwise its value
+ * is left unchanged.
+ *
+ * Returns zero on success, non-zero otherwise.
+ */
+static int parse_command_line_arguments(int argc, char **argv, char *ttyn,
+					security_context_t old_context,
+					security_context_t *new_context,
+					int *preserve_environment)
+{
+	int flag_index;		/* flag index in argv[] */
+	int clflag;		/* holds codes for command line flags */
+	char *role_s = NULL;	/* role spec'd by user in argv[] */
+	char *type_s = NULL;	/* type spec'd by user in argv[] */
+	char *type_ptr = NULL;	/* stores malloc'd data from get_default_type */
+	char *level_s = NULL;	/* level spec'd by user in argv[] */
+	char *range_ptr = NULL;
+	security_context_t new_con = NULL;
+	context_t context = NULL; /* manipulatable form of new_context */
+	const struct option long_options[] = {
+		{"role", 1, 0, 'r'},
+		{"type", 1, 0, 't'},
+		{"level", 1, 0, 'l'},
+		{"preserve-environment", 0, 0, 'p'},
+		{"version", 0, 0, 'V'},
+		{NULL, 0, 0, 0}
+	};
+
+	*preserve_environment = 0;
+	while (1) {
+		clflag = getopt_long(argc, argv, "r:t:l:pV", long_options,
+				     &flag_index);
+		if (clflag == -1)
+			break;
+
+		switch (clflag) {
+		case 'V':
+			printf("newrole: %s version %s\n", PACKAGE, VERSION);
+			exit(0);
+			break;
+		case 'p':
+			*preserve_environment = 1;
+			break;
+		case 'r':
+			if (role_s) {
+				fprintf(stderr,
+					_("Error: multiple roles specified\n"));
+				return -1;
+			}
+			role_s = optarg;
+			break;
+		case 't':
+			if (type_s) {
+				fprintf(stderr,
+					_("Error: multiple types specified\n"));
+				return -1;
+			}
+			type_s = optarg;
+			break;
+		case 'l':
+			if (!is_selinux_mls_enabled()) {
+				fprintf(stderr, _("Sorry, -l may be used with "
+					"SELinux MLS support.\n"));
+				return -1;
+			}
+			if (level_s) {
+				fprintf(stderr, _("Error: multiple levels "
+					"specified\n"));
+				return -1;
+			}
+			level_s = optarg;
+			break;
+		default:
+			fprintf(stderr, "%s\n", USAGE_STRING);
+			return -1;
+		}
+	}
+
+	/* Verify that the combination of command-line arguments are viable */
+	if (!(role_s || type_s || level_s)) {
+		fprintf(stderr, "%s\n", USAGE_STRING);
+		return -1;
+	}
+
+	/* Fill in a default type if one hasn't been specified. */
+	if (role_s && !type_s) {
+	 	/* get_default_type() returns malloc'd memory */
+		if (get_default_type(role_s, &type_ptr)) {
+			fprintf(stderr, _("Couldn't get default type.\n"));
+			send_audit_message(0, old_context, new_con, ttyn);
+			return -1;
+		}
+		type_s = type_ptr;
+	}
+
+	/* Create a temporary new context structure we extract and modify */
+	context = context_new(old_context);
+	if (!context) {
+		fprintf(stderr, _("failed to get new context.\n"));
+		goto err_free;
+	}
+
+	/* Modify the temporary new context */
+	if (role_s)
+		if (context_role_set(context, role_s)) {
+			fprintf(stderr, _("failed to set new role %s\n"),
+				role_s);
+			goto err_free;
+		}
+
+	if (type_s)
+		if (context_type_set(context, type_s)) {
+			fprintf(stderr, _("failed to set new type %s\n"),
+				type_s);
+			goto err_free;
+		}
+
+	if (level_s) {
+		range_ptr = build_new_range(level_s,context_range_get(context));
+		if (!range_ptr) {
+			fprintf(stderr,
+				_("failed to build new range with level %s\n"),
+				level_s);
+			goto err_free;
+		}
+		if (context_range_set(context, range_ptr)) {
+			fprintf(stderr, _("failed to set new range %s\n"),
+				range_ptr);
+			goto err_free;
+		}
+	}
+
+	/* Construct the final new context */
+	if (!(new_con = context_str(context))) {
+		fprintf(stderr, _("failed to convert new context to string\n"));
+		goto err_free;
+	}
+
+	if (security_check_context(new_con) < 0) {
+		fprintf(stderr, _("%s is not a valid context\n"), new_con);
+		send_audit_message(0, old_context, new_con, ttyn);
+		goto err_free;
+	}
+
+	*new_context = strdup(new_con);
+	if (!*new_context) {
+		fprintf(stderr, _("Unable to allocate memory for new_context"));
+		goto err_free;
+	}
+
+	free(type_ptr);
+	free(range_ptr);
+	context_free(context);
+	return 0;
+
+err_free:
+	free(type_ptr);
+	free(range_ptr);
+	/* Don't free new_con, context_free(context) handles this */
+	context_free(context);
+	return -1;
+}
 
 /**
  * Take care of any signal setup
@@ -660,32 +897,20 @@
 
 int main(int argc, char *argv[])
 {
-
-	security_context_t new_context = NULL;	/* our target security context */
-	security_context_t old_context = NULL;	/* our original securiy context */
-	security_context_t tty_context = NULL;	/* The current context of tty file */
-	security_context_t new_tty_context = NULL;	/* The new context of tty file */
-
-	context_t context;	/* manipulatable form of new_context */
+	security_context_t new_context = NULL;	/* target security context */
+	security_context_t old_context = NULL;	/* original securiy context */
+	security_context_t tty_context = NULL;	/* current context of tty */
+	security_context_t new_tty_context = NULL; /* new context of tty */
 
 	struct passwd pw;	/* struct derived from passwd file line */
-
-	int clflag;		/* holds codes for command line flags */
-	int flag_index;		/* flag index in argv[] */
-	const struct option long_options[] = {	/* long option flags for getopt() */
-		{"role", 1, 0, 'r'},
-		{"type", 1, 0, 't'},
-		{"level", 1, 0, 'l'},
-		{"version", 0, 0, 'V'},
-		{NULL, 0, 0, 0}
-	};
-	char *role_s = NULL;	/* role spec'd by user in argv[] */
-	char *type_s = NULL;	/* type spec'd by user in argv[] */
-	char *level_s = NULL;	/* level spec'd by user in argv[] */
 	char *ttyn = NULL;	/* tty path */
+
+	char **old_environ;
+	int preserve_environment;
+
+	int fd;
+	int rc;
 	pid_t childPid = 0;
-	int fd, rc;
-	int enforcing;
 	char *shell_argv0 = NULL;
 
 #ifdef USE_PAM
@@ -719,139 +944,40 @@
 	textdomain(PACKAGE);
 #endif
 
-	/*
-	 *
-	 * Step 1:  Handle command-line arguments.
-	 *
-	 */
+	old_environ = environ;
+	environ = NULL;
 
 	if (!is_selinux_enabled()) {
-		fprintf(stderr,
-			_
-			("Sorry, newrole may be used only on a SELinux kernel.\n"));
-		exit(-1);
-	}
-	enforcing = security_getenforce();
-	if (enforcing < 0) {
-		fprintf(stderr, _("Could not determine enforcing mode.\n"));
-		exit(-1);
-	}
-
-	while (1) {
-		clflag =
-		    getopt_long(argc, argv, "r:t:l:V", long_options,
-				&flag_index);
-		if (clflag == -1)
-			break;
-
-		switch (clflag) {
-		case 'V':
-			printf("newrole: %s version %s\n", PACKAGE, VERSION);
-			exit(0);
-			break;
-		case 'r':
-			/* If role_s is already set, the user spec'd multiple roles - bad. */
-			if (role_s) {
-				fprintf(stderr,
-					_("Error: multiple roles specified\n"));
-				exit(-1);
-			}
-			role_s = optarg;	/* save the role string spec'd by user */
-			break;
-
-		case 't':
-			/* If type_s is already set, the user spec'd multiple types - bad. */
-			if (type_s) {
-				fprintf(stderr,
-					_("Error: multiple types specified\n"));
-				exit(-1);
-			}
-			type_s = optarg;	/* save the type string spec'd by user */
-			break;
-
-		case 'l':
-			if (!is_selinux_mls_enabled()) {
-				fprintf(stderr,
-					_
-					("Sorry, -l may be used with SELinux MLS support.\n"));
-				exit(-1);
-			}
-			/* If level_s is already set, the user spec'd multiple levels - bad. */
-			if (level_s) {
-				fprintf(stderr,
-					_
-					("Error: multiple levels specified\n"));
-				exit(-1);
-			}
-			level_s = optarg;	/* save the level string spec'd by user */
-			break;
-
-		default:
-			fprintf(stderr, "%s\n", USAGE_STRING);
-			exit(-1);
-		}		/* switch( clflag ) */
-	}			/* while command-line flags remain for newrole */
-
-	/* Verify that the combination of command-line arguments we were *
-	 * given is a viable one.                                        */
-	if (!(role_s || type_s || level_s)) {
-		fprintf(stderr, "%s\n", USAGE_STRING);
-		exit(-1);
+		fprintf(stderr, _("Sorry, newrole may be used only on "
+			"a SELinux kernel.\n"));
+		return -1;
 	}
 
-	/* Fill in a default type if one hasn't been specified */
-	if (role_s && !type_s) {
-		if (get_default_type(role_s, &type_s)) {
-			fprintf(stderr, _("Couldn't get default type.\n"));
-			send_audit_message(0, old_context, new_context, ttyn);
-			exit(-1);
-		}
-#ifdef CANTSPELLGDB
-		printf("Your type will be %s.\n", type_s);
-#endif
+	if (security_getenforce() < 0) {
+		fprintf(stderr, _("Could not determine enforcing mode.\n"));
+		return -1;
 	}
 
 	/*
+	 * Step 1: Parse command line and valid arguments
 	 *
-	 * Step 2:  Authenticate the user.
-	 *
+	 * old_context and ttyn are required for audit logging,
+	 * context validation and pam
 	 */
-
-	/*
-	 * Get the context of the caller, and extract
-	 * the username from the context.  Don't rely on the Linux
-	 * uid information - it isn't trustworthy.
-	 */
-
-	/* Put the caller's context into `old_context'. */
-	if (0 != (getprevcon(&old_context))) {
+	if (getprevcon(&old_context)) {
 		fprintf(stderr, _("failed to get old_context.\n"));
-		exit(-1);
-	}
-#ifdef CANTSPELLGDB
-	printf("Your old context was %s\n", old_context);
-#endif
-
-	/* 
-	 * Create a context structure so that we extract and modify 
-	 * components easily. 
-	 */
-	context = context_new(old_context);
-	if (context == 0) {
-		fprintf(stderr, _("failed to get new context.\n"));
-		exit(-1);
+		return -1;
 	}
 
-	/* Get the tty name. Pam will need it. */
 	ttyn = ttyname(0);
 	if (!ttyn || *ttyn == '\0') {
 		fprintf(stderr,
 			_("Error!  Could not retrieve tty information.\n"));
-		exit(-1);
+		return -1;
 	}
 
-	/* Get the passwd info for the Linux user identity. */
-	if (extract_pw_data(&pw))
+	if (parse_command_line_arguments(argc, argv, ttyn, old_context,
+					 &new_context, &preserve_environment))
 		return -1;
 
 	/*
@@ -862,6 +988,9 @@
 	 * malicious software), not to authorize the operation (which is covered
 	 * by policy).  Trusted path mechanism would be preferred.
 	 */
+	if (extract_pw_data(&pw))
+		goto err_free;
+
 	printf(_("Authenticating %s.\n"), pw.pw_name);
 #ifdef USE_PAM
 	pam_status = pam_start(SERVICE_NAME, pw.pw_name, &pam_conversation,
@@ -882,86 +1011,6 @@
 	}
 
 	/*
-	 *
-	 * Step 3:  Construct a new context based on our old context and the
-	 *          arguments specified on the command line.
-	 *
-	 */
-
-	/* The first step in constructing a new context for the new shell we  *
-	 * plan to exec is to take our old context in `context' as a   *
-	 * starting point, and modify it according to the options the user *
-	 * specified on the command line.                                  */
-
-	/* If the user specified a new role on the command line (if `role_s'   *
-	 * is set), then replace the old role in `context' with this new role. */
-	if (role_s) {
-		if (context_role_set(context, role_s)) {
-			fprintf(stderr, _("failed to set new role %s\n"),
-				role_s);
-			exit(-1);
-		}
-#ifdef CANTSPELLGDB
-		printf("Your new role is %s\n", context_role_get(context));
-#endif
-	}
-
-	/* if user specified new role */
-	/* If the user specified a new type on the command line (if `type_s'   *
-	 * is set), then replace the old type in `context' with this new type. */
-	if (type_s) {
-		if (context_type_set(context, type_s)) {
-			fprintf(stderr, _("failed to set new type %s\n"),
-				type_s);
-			exit(-1);
-		}
-#ifdef CANTSPELLGDB
-		printf("Your new type is %s\n", context_type_get(context));
-#endif
-	}
-
-	/* if user specified new type */
-	/* If the user specified a new level on the command line (if `level_s'   *
-	 * is set), then replace the old level in `context' with this new level. */
-	if (level_s) {
-		char *range_s =
-		    build_new_range(level_s, context_range_get(context));
-		if (!range_s) {
-			fprintf(stderr,
-				_("failed to build new range with level %s\n"),
-				level_s);
-			exit(-1);
-		}
-		if (context_range_set(context, range_s)) {
-			fprintf(stderr, _("failed to set new range %s\n"),
-				range_s);
-			free(range_s);
-			exit(-1);
-		}
-		free(range_s);
-#ifdef CANTSPELLGDB
-		printf("Your new range is %s\n", context_range_get(context));
-#endif
-	}
-
-	/* if user specified new level */
-	/* The second step in creating the new context is to convert our modified *
-	 * `context' structure back to a context string and then to a Context.    */
-	if (!(new_context = context_str(context))) {
-		fprintf(stderr, _("failed to convert new context to string\n"));
-		exit(-1);
-	}
-#ifdef CANTSPELLGDB
-	printf("Your new context is %s\n", new_context);
-#endif
-
-	if (security_check_context(new_context) < 0) {
-		fprintf(stderr, _("%s is not a valid context\n"), new_context);
-		send_audit_message(0, old_context, new_context, ttyn);
-		exit(-1);
-	}
-
-	/*
 	 * Step 3:  Handle relabeling of the tty.
 	 *
 	 * Once we authenticate the user, we know that we want to proceed with
@@ -1086,6 +1135,13 @@
 	if (send_audit_message(1, old_context, new_context, ttyn))
 		exit(-1);
 	freecon(old_context);
+
+	/* Handle environment changes */
+	if (restore_environment(preserve_environment, old_environ, &pw)) {
+		fprintf(stderr, _("Unable to restore the environment, "
+			"aborting\n"));
+		goto err_close_pam;
+	}
 	execv(pw.pw_shell, argv + optind - 1);
 
 	/*

[PATCH 8/8] make newrole suid (take 3)

[Michael C Thompson]

[-- Attachment #1: Type: text/plain, Size: 1188 bytes --]

Michael C Thompson wrote:
> The 8 patches are as follows:
> 1) Modifications to Makefile to support future patch needs
>    Add newrole-lspp.pamd
> 2) New extract_pw_data function and use in main()
> 3) Add signal handler function
> 4) Update drop_capabilities() and use in main()
> 5) Update the authentication functions and use in main()
>    Add cleanup since pam_start is now left till program end
> 6) Move relabeling tty actions into functions
> 7) Move command-line argument parsing into a function
>    Clear the environment during execution
>    Add support for preserving the environment (-p)
> 8) Shift to using new defines in the Makefile and in newrole.c
>    Add support for namespaces
>    Remove unused code, cleanup and documentation

This is the 8th of 8 patches.
This patch applies against policycoreutils-1.30.30-1.

This function finalizes all of the changes made by the previous 7 
patches and introduces the namespace support.

Changes:
  * Introduces namespace support and transition_to_caller_uid()
    - New functionality, for polyinstantitation
  * Various bits of cleanup remaining from previous patches

Signed-off-by: Michael Thompson <thompsmc@us.ibm.com>


[-- Attachment #2: 08-enable_namespace.patch --]
[-- Type: text/x-diff, Size: 7712 bytes --]

diff -Naur policycoreutils-1.30.30/newrole/Makefile policycoreutils-1.30.30.suid/newrole/Makefile
--- policycoreutils-1.30.30/newrole/Makefile	2006-11-02 12:13:45.000000000 -0600
+++ policycoreutils-1.30.30.suid/newrole/Makefile	2006-11-02 13:06:52.000000000 -0600
@@ -6,11 +6,6 @@
 LOCALEDIR = /usr/share/locale
 PAMH = $(shell ls /usr/include/security/pam_appl.h 2>/dev/null)
 AUDITH = $(shell ls /usr/include/libaudit.h 2>/dev/null)
-# If LOG_AUDIT_PRIV is y, then newrole will be made into setuid root program.
-# This is so that we have the CAP_AUDIT_WRITE capability. newrole will
-# shed all privileges and change to the user's uid.
-LOG_AUDIT_PRIV ?= n
-
 # Enable capabilities to permit newrole to generate audit records.
 # This will make newrole a setuid root program.
 # The capabilities used are: CAP_AUDIT_WRITE.
@@ -39,7 +34,6 @@
 	override CFLAGS += -DUSE_AUDIT
 	LDLIBS += -laudit
 endif
-
 ifeq (${LSPP_PRIV},y)
 	override AUDIT_LOG_PRIV=y
 	override NAMESPACE_PRIV=y
@@ -59,14 +53,6 @@
 	MODE := 0555
 endif
 
-ifeq (${LOG_AUDIT_PRIV},y)
-	override CFLAGS += -DLOG_AUDIT_PRIV
-	LDLIBS += -lcap
-	MODE := 4555
-else
-	MODE := 555
-endif
-
 TARGETS=$(patsubst %.c,%,$(wildcard *.c))
 
 all: $(TARGETS)
diff -Naur policycoreutils-1.30.30/newrole/newrole.c policycoreutils-1.30.30.suid/newrole/newrole.c
--- policycoreutils-1.30.30/newrole/newrole.c	2006-11-02 13:00:29.000000000 -0600
+++ policycoreutils-1.30.30.suid/newrole/newrole.c	2006-11-02 13:05:23.000000000 -0600
@@ -36,11 +36,6 @@
  * setuid root, so that it can read the shadow passwd file.
  * 
  *
- * option CANTSPELLGDB:
- *
- * If you set CANTSPELLGDB you will turn on some debugging printfs.
- *
- *
  * Authors:  Tim Fraser , 
  *           Anthony Colatrella <amcolat@epoch.ncsc.mil>
  * Various bug fixes by Stephen Smalley <sds@epoch.ncsc.mil>
@@ -48,6 +43,14 @@
  *************************************************************************/
 
 #define _GNU_SOURCE
+
+#if defined(AUDIT_LOG_PRIV) && !defined(USE_AUDIT)
+#error AUDIT_LOG_PRIV needs the USE_AUDIT option
+#endif
+#if defined(NAMESPACE_PRIV) && !defined(USE_PAM)
+#error NAMESPACE_PRIV needs the USE_PAM option
+#endif
+
 #include <stdio.h>
 #include <stdlib.h>		/* for malloc(), realloc(), free() */
 #include <pwd.h>		/* for getpwuid() */
@@ -63,13 +66,11 @@
 #include <selinux/get_default_type.h>
 #include <selinux/get_context_list.h>	/* for SELINUX_DEFAULTUSER */
 #include <signal.h>
+#include <unistd.h>		/* for getuid(), exit(), getopt() */
 #ifdef USE_AUDIT
 #include <libaudit.h>
 #endif
-#ifdef LOG_AUDIT_PRIV
-#ifndef USE_AUDIT
-#error LOG_AUDIT_PRIV needs the USE_AUDIT option
-#endif
+#if defined(AUDIT_LOG_PRIV) || (NAMESPACE_PRIV)
 #include <sys/prctl.h>
 #include <sys/capability.h>
 #endif
@@ -92,18 +93,17 @@
 
 extern char **environ;
 
-char *xstrdup(const char *s)
-{
-	char *s2;
-
-	s2 = strdup(s);
-	if (!s2) {
-		fprintf(stderr, _("Out of memory!\n"));
-		exit(1);
-	}
-	return s2;
-}
-
+/**
+ * Construct from the current range and specified desired level a resulting
+ * range. If the specified level is a range, return that. If it is not, then
+ * construct a range with level as the sensitivity and clearance of the current
+ * context.
+ *
+ * newlevel - the level specified on the command line
+ * range    - the range in the current context
+ *
+ * Returns malloc'd memory
+ */
 static char *build_new_range(char *newlevel, const char *range)
 {
 	char *newrangep = NULL;
@@ -120,9 +120,8 @@
 		return newrangep;
 	}
 
-	/* look for MLS range */
+	/* look for MLS range in current context */
 	tmpptr = strchr(range, '-');
-
 	if (tmpptr) {
 		/* we are inserting into a ranged MLS context */
 		len = strlen(newlevel) + 1 + strlen(tmpptr + 1) + 1;
@@ -260,7 +259,7 @@
 }
 #endif				/* if/else USE_PAM */
 
-/*
+/**
  * This function checks to see if the shell is known in /etc/shells.
  * If so, it returns 1. On error or illegal shell, it returns 0.
  */
@@ -269,7 +268,7 @@
 	int found = 0;
 	const char *buf;
 
-	if (!shell_name)
+	if (! (shell_name && shell_name[0]))
 		return found;
 
 	while ((buf = getusershell()) != NULL) {
@@ -545,7 +544,29 @@
 }
 #endif
 
-#ifdef LOG_AUDIT_PRIV
+#ifdef NAMESPACE_PRIV
+/**
+ * This function will set the uid values to be that of caller's uid, and
+ * will drop any privilages which maybe have been raised.
+ */
+static int transition_to_caller_uid()
+{
+	uid_t uid = getuid();
+
+	if (prctl(PR_SET_KEEPCAPS, 0, 0, 0, 0) < 0) {
+		fprintf(stderr, _("Error resetting KEEPCAPS, aborting\n"));
+		return -1;
+	}
+
+	if (setresuid(uid, uid, uid)) {
+		fprintf(stderr, _("Error changing uid, aborting.\n"));
+		return -1;
+	}
+	return 0;
+}
+#endif
+
+#ifdef AUDIT_LOG_PRIV
 /* Send audit message */
 static
 int send_audit_message(int success, security_context_t old_context,
@@ -1085,62 +1106,67 @@
 	}
 
 	/* CHILD */
-
-	close(fd);
-
-	/* Close and reopen descriptors 0 through 2 */
-	if (close(0) || close(1) || close(2)) {
+	/* Close the tty and reopen descriptors 0 through 2 */
+	if (close(fd) || close(0) || close(1) || close(2)) {
 		fprintf(stderr, _("Could not close descriptors.\n"));
-		exit(-1);
+		goto err_close_pam;
 	}
 	fd = open(ttyn, O_RDONLY);
-	if (fd != 0) {
-		exit(-1);
-	}
+	if (fd != 0)
+		goto err_close_pam;
 	fd = open(ttyn, O_WRONLY);
-	if (fd != 1) {
-		exit(-1);
-	}
+	if (fd != 1)
+		goto err_close_pam;
 	fd = open(ttyn, O_WRONLY);
-	if (fd != 2) {
-		exit(-1);
-	}
+	if (fd != 2)
+		goto err_close_pam;
 
 	/*
-	 *
 	 * Step 5:  Execute a new shell with the new context in `new_context'. 
 	 *
+	 * Establish context, namesapce and any options for the new shell
 	 */
-
 	if (optind < 1)
 		optind = 1;
-	if (asprintf(&argv[optind - 1], "-%s", pw.pw_shell) < 0) {
-		fprintf(stderr, _("Error allocating shell.\n"));
-		exit(-1);
-	}
-#ifdef CANTSPELLGDB
-	{
-		int i;
-		printf("Executing ");
-		for (i = optind - 1; i < argc; i++)
-			printf("%s ", argv[i]);
-		printf("with context %s\n", new_context);
+
+	/* This is ugly, but use newrole's argv for the exec'd shells argv */
+	if (asprintf(&shell_argv0, "-%s", pw.pw_shell) < 0) {
+		fprintf(stderr, _("Error allocating shell's argv0.\n"));
+		shell_argv0 = NULL;
+		goto err_close_pam;
 	}
-#endif
-	if (setexeccon(new_context) < 0) {
+	argv[optind-1] = shell_argv0;
+
+	if (setexeccon(new_context)) {
 		fprintf(stderr, _("Could not set exec context to %s.\n"),
 			new_context);
-		exit(-1);
+		goto err_close_pam;
+	}
+
+#ifdef NAMESPACE_PRIV
+	/* Ask PAM to setup session for user running this program */
+	pam_status = pam_open_session(pam_handle,0);
+	if (pam_status != PAM_SUCCESS) {
+		fprintf(stderr, "pam_open_session failed with %s\n",
+			pam_strerror(pam_handle, pam_status));
+		goto err_close_pam;
 	}
+#endif
+
 	if (send_audit_message(1, old_context, new_context, ttyn))
-		exit(-1);
+		goto err_close_pam_session;
+#ifdef NAMESPACE_PRIV
+	if (transition_to_caller_uid())
+		goto err_close_pam_session;
+#endif
 	freecon(old_context);
+	freecon(new_context);
 
 	/* Handle environment changes */
 	if (restore_environment(preserve_environment, old_environ, &pw)) {
 		fprintf(stderr, _("Unable to restore the environment, "
 			"aborting\n"));
-		goto err_close_pam;
+		goto err_close_pam_session;
 	}
 	execv(pw.pw_shell, argv + optind - 1);
 
@@ -1150,6 +1176,13 @@
 	 * If we reach here, then we failed to exec the new shell.
 	 */
 	perror(_("failed to exec shell\n"));
+err_close_pam_session:
+#ifdef NAMESPACE_PRIV
+	pam_status = pam_close_session(pam_handle,0);
+	if(pam_status != PAM_SUCCESS)
+		fprintf(stderr, "pam_close_session failed with %s\n",
+			pam_strerror(pam_handle, pam_status));
+#endif
 err_close_pam:
 #ifdef USE_PAM
 	rc = pam_end(pam_handle, pam_status);

Re: [PATCH 0/8] make newrole suid (take 3)

[Stephen Smalley]

On Thu, 2006-11-02 at 18:37 -0600, Michael C Thompson wrote:
> This is the intro to a set of eight patches.
> 
> I finally took the time to break things down so that (I hope) more 
> people read and give feedback (I can't believe only Stephen Smalley had 
> comments, my code isn't that great!)
> 
> These patches are an attempt to make newrole be an acceptably secure 
> suid root program, to provide it with the capabilities to generate audit 
> records (existing) and handle polyinstatiation (new).
> 
> The format of the patches is different from previous sends.
> 
> The 8 patches are as follows:
> 1) Modifications to Makefile to support future patch needs
>     Add newrole-lspp.pamd
> 2) New extract_pw_data function and use in main()
> 3) Add signal handler function
> 4) Update drop_capabilities() and use in main()
> 5) Update the authentication functions and use in main()
>     Add cleanup since pam_start is now left till program end
> 6) Move relabeling tty actions into functions
> 7) Move command-line argument parsing into a function
>     Clear the environment during execution
>     Add support for preserving the environment (-p)
> 8) Shift to using new defines in the Makefile and in newrole.c
>     Add support for namespaces
>     Remove unused code, cleanup and documentation
> 
> It is now possible to apply a single patch and the code will compile; 
> however, this is not recommended
> 
> The comments and discussions from the previous send (take 2) of these 
> patches have been integrated.

Thanks, merged into policycoreutils 1.33.1.

-- 
Stephen Smalley
National Security Agency


--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.