All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] newrole: use pam session management services -v3
@ 2006-08-28  2:14 Janak Desai
  2006-08-29 21:08 ` Stephen Smalley
  0 siblings, 1 reply; 6+ messages in thread
From: Janak Desai @ 2006-08-28  2:14 UTC (permalink / raw)
  To: sds, sgrubb; +Cc: klaus, chanson, kmacmillan, dwalsh, ltcgcw, selinux


This patch updates newrole to allow it to use pam session mangement
services. Currently newrole only uses the authenticaion services of
pam. Session management services are needed to allow pam_namespace
to appropriately reconfigure polyinstantiated namespace for the
new session being established with nerole.

Changes since v2 of this patch, posted on 8/16/06:
    1. Restructured to use pam_start and pam_end only once
    2. Updated to use pam_open_session and pam_close_session appropriately
    3. Removed system-auth and pam_xauth session services from newrole.pamd.
       These were not used since newrole did not have session management
       support, and could result in unintended behavior now that session
       management support is enabled with this patch.
    4. Forward ported to the latest selinux subversion tree (version 2000)
       on sourceforge. 

Signed-off-by: Janak Desai <janak@us.ibm.com>

---

 newrole.c    |   95 +++++++++++++++++++++++++++++++++++------------------------
 newrole.pamd |    2 -
 2 files changed, 57 insertions(+), 40 deletions(-)

diff -Naurp policycoreutils/newrole/newrole.c policycoreutils+newrolepatch/newrole/newrole.c
--- policycoreutils/newrole/newrole.c	2006-08-27 21:22:13.000000000 +0000
+++ policycoreutils+newrolepatch/newrole/newrole.c	2006-08-27 21:22:56.000000000 +0000
@@ -84,6 +84,17 @@
 #define PACKAGE "policycoreutils"	/* the name of this package lang translation */
 #endif
 
+#ifdef USE_PAM
+#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 char *, pam_handle_t *);
+#endif
+
 /* USAGE_STRING describes the command-line args of this program. */
 #define USAGE_STRING "USAGE: newrole [ -r role ] [ -t type ] [ -l level ] [ -V ] [ -- args ]"
 
@@ -144,22 +155,6 @@ static char *build_new_range(char *newle
 }
 
 #ifdef USE_PAM
-
-/************************************************************************
- *
- * 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 
@@ -176,30 +171,13 @@ int authenticate_via_pam(const struct pa
  *
  */
 
-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 */
 	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;
@@ -224,10 +202,7 @@ int authenticate_via_pam(const struct pa
 		result = 1;	/* user authenticated OK! */
 	}
 
-	/* We're done with PAM.  Free `pam_handle'. */
       out:
-	pam_end(pam_handle, rc);
-
 	return (result);
 
 }				/* authenticate_via_pam() */
@@ -434,6 +409,18 @@ int main(int argc, char *argv[])
 	int fd;
 	int enforcing;
 	sigset_t empty;
+#ifdef USE_PAM
+	int rc;			/* 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
 
 #ifdef LOG_AUDIT_PRIV
 	drop_capabilities();
@@ -624,8 +611,18 @@ int main(int argc, char *argv[])
 	 * malicious software), not to authorize the operation (which is covered
 	 * by policy).  Trusted path mechanism would be preferred.
 	 */
+
 #ifdef USE_PAM
-	if (!authenticate_via_pam(pw, ttyn))
+	/* 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 (!authenticate_via_pam(ttyn, pam_handle))
 #else				/* !USE_PAM */
 	if (!authenticate_via_shadow_passwd(pw))
 #endif				/* if/else USE_PAM */
@@ -797,6 +794,18 @@ int main(int argc, char *argv[])
 			rc = wait(NULL);
 		} while (rc < 0 && errno == EINTR);
 
+#ifdef USE_PAM
+		rc = pam_close_session(pam_handle,0);
+		if(rc != PAM_SUCCESS) {
+			fprintf(stderr, "pam_close_session failed\n");
+			pam_end(pam_handle, rc);
+			exit(-1);
+		}
+
+		/* We're done with PAM.  Free `pam_handle'. */
+		pam_end(pam_handle, rc);
+#endif
+
 		if (!new_tty_context || !tty_context)
 			exit(0);
 
@@ -900,6 +909,16 @@ int main(int argc, char *argv[])
 	}
 #endif
 	freecon(old_context);
+
+#ifdef USE_PAM
+	/* Ask PAM to setup session for user running this program */
+	rc = pam_open_session(pam_handle,0);
+	if(rc != PAM_SUCCESS) {
+		fprintf(stderr, _("Namespace setup failed.\n"));
+		exit(-1);
+	}
+#endif
+
 	execv(pw->pw_shell, argv + optind - 1);
 
 	/* If we reach here, then we failed to exec the new shell. */
diff -Naurp policycoreutils/newrole/newrole.pamd policycoreutils+newrolepatch/newrole/newrole.pamd
--- policycoreutils/newrole/newrole.pamd	2006-08-27 21:22:13.000000000 +0000
+++ policycoreutils+newrolepatch/newrole/newrole.pamd	2006-08-27 21:22:56.000000000 +0000
@@ -2,5 +2,3 @@
 auth       include	system-auth
 account    include	system-auth
 password   include	system-auth
-session    include	system-auth
-session    optional	pam_xauth.so



--
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.

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

* Re: [PATCH] newrole: use pam session management services -v3
  2006-08-28  2:14 [PATCH] newrole: use pam session management services -v3 Janak Desai
@ 2006-08-29 21:08 ` Stephen Smalley
  2006-08-30 11:38   ` Stephen Smalley
  0 siblings, 1 reply; 6+ messages in thread
From: Stephen Smalley @ 2006-08-29 21:08 UTC (permalink / raw)
  To: janak; +Cc: sgrubb, klaus, chanson, kmacmillan, dwalsh, ltcgcw, selinux

On Sun, 2006-08-27 at 22:14 -0400, Janak Desai wrote:
> This patch updates newrole to allow it to use pam session mangement
> services. Currently newrole only uses the authenticaion services of
> pam. Session management services are needed to allow pam_namespace
> to appropriately reconfigure polyinstantiated namespace for the
> new session being established with nerole.
> 
> Changes since v2 of this patch, posted on 8/16/06:
>     1. Restructured to use pam_start and pam_end only once
>     2. Updated to use pam_open_session and pam_close_session appropriately
>     3. Removed system-auth and pam_xauth session services from newrole.pamd.
>        These were not used since newrole did not have session management
>        support, and could result in unintended behavior now that session
>        management support is enabled with this patch.
>     4. Forward ported to the latest selinux subversion tree (version 2000)
>        on sourceforge. 
> 
> Signed-off-by: Janak Desai <janak@us.ibm.com>
> 
> ---
> 
>  newrole.c    |   95 +++++++++++++++++++++++++++++++++++------------------------
>  newrole.pamd |    2 -
>  2 files changed, 57 insertions(+), 40 deletions(-)

You didn't introduce this, but it appears that newrole no longer builds
properly with make LOG_AUDIT_PRIV=y (duplicate _GNU_SOURCE definitions)
when building on rawhide.  Aren't you encountering this?

> @@ -797,6 +794,18 @@ int main(int argc, char *argv[])
>  			rc = wait(NULL);
>  		} while (rc < 0 && errno == EINTR);
>  
> +#ifdef USE_PAM
> +		rc = pam_close_session(pam_handle,0);
> +		if(rc != PAM_SUCCESS) {
> +			fprintf(stderr, "pam_close_session failed\n");
> +			pam_end(pam_handle, rc);
> +			exit(-1);
> +		}

Hmm...so a pam_close_session failure bails before we restore the tty
label?  Possibly the error message should include the pam_strerror()
string as well?

> +
> +		/* We're done with PAM.  Free `pam_handle'. */
> +		pam_end(pam_handle, rc);
> +#endif
> +
>  		if (!new_tty_context || !tty_context)
>  			exit(0);
>  
> @@ -900,6 +909,16 @@ int main(int argc, char *argv[])
>  	}
>  #endif
>  	freecon(old_context);
> +
> +#ifdef USE_PAM
> +	/* Ask PAM to setup session for user running this program */
> +	rc = pam_open_session(pam_handle,0);
> +	if(rc != PAM_SUCCESS) {
> +		fprintf(stderr, _("Namespace setup failed.\n"));
> +		exit(-1);
> +	}
> +#endif

I understand that you need this to happen after the setexeccon, but do
you really want it to happen after the audit message has already been
generated?  The error message also should likely be both more general
(since you only know that pam_open_session failed, not that it was
specifically namespace related) and more specific (including
pam_strerror result).

> +
>  	execv(pw->pw_shell, argv + optind - 1);
>  
>  	/* If we reach here, then we failed to exec the new shell. */
> diff -Naurp policycoreutils/newrole/newrole.pamd policycoreutils+newrolepatch/newrole/newrole.pamd
> --- policycoreutils/newrole/newrole.pamd	2006-08-27 21:22:13.000000000 +0000
> +++ policycoreutils+newrolepatch/newrole/newrole.pamd	2006-08-27 21:22:56.000000000 +0000
> @@ -2,5 +2,3 @@
>  auth       include	system-auth
>  account    include	system-auth
>  password   include	system-auth
> -session    include	system-auth
> -session    optional	pam_xauth.so

Unfortunately, this seems to yield a non-working newrole, as
pam_open_session() then always fails due to the absence of any session
modules.  What is the graceful way of handling this?

-- 
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.

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

* Re: [PATCH] newrole: use pam session management services -v3
  2006-08-29 21:08 ` Stephen Smalley
@ 2006-08-30 11:38   ` Stephen Smalley
  2006-08-30 17:38     ` Janak Desai
  0 siblings, 1 reply; 6+ messages in thread
From: Stephen Smalley @ 2006-08-30 11:38 UTC (permalink / raw)
  To: janak; +Cc: sgrubb, klaus, chanson, kmacmillan, dwalsh, ltcgcw, selinux

On Tue, 2006-08-29 at 17:08 -0400, Stephen Smalley wrote:
> On Sun, 2006-08-27 at 22:14 -0400, Janak Desai wrote:
> > This patch updates newrole to allow it to use pam session mangement
> > services. Currently newrole only uses the authenticaion services of
> > pam. Session management services are needed to allow pam_namespace
> > to appropriately reconfigure polyinstantiated namespace for the
> > new session being established with nerole.
> > 
> > Changes since v2 of this patch, posted on 8/16/06:
> >     1. Restructured to use pam_start and pam_end only once
> >     2. Updated to use pam_open_session and pam_close_session appropriately
> >     3. Removed system-auth and pam_xauth session services from newrole.pamd.
> >        These were not used since newrole did not have session management
> >        support, and could result in unintended behavior now that session
> >        management support is enabled with this patch.
> >     4. Forward ported to the latest selinux subversion tree (version 2000)
> >        on sourceforge. 
> > 
> > Signed-off-by: Janak Desai <janak@us.ibm.com>
> > 
> > ---
> > 
> >  newrole.c    |   95 +++++++++++++++++++++++++++++++++++------------------------
> >  newrole.pamd |    2 -
> >  2 files changed, 57 insertions(+), 40 deletions(-)
> 
> You didn't introduce this, but it appears that newrole no longer builds
> properly with make LOG_AUDIT_PRIV=y (duplicate _GNU_SOURCE definitions)
> when building on rawhide.  Aren't you encountering this?

Actually, on further thought, I don't understand how newrole can work
with pam_namespace without further changes.  If you build newrole
without LOG_AUDIT_PRIV=y, then it is non-suid and will lack the
necessary capability to mount (which sadly is CAP_SYS_ADMIN, the gateway
to all power).  If you build newrole with LOG_AUDIT_PRIV=y, then it is
suid but drops all capabilities except for CAP_AUDIT_WRITE upon startup.
So if you are running newrole as any non-root user, pam_namespace
shouldn't be able to mount.  To do so, it would need to retain
CAP_SYS_ADMIN all the way up until the point it calls pam_open_session,
at which point the entire code needs much tighter review.  Am I missing
something?

> > @@ -797,6 +794,18 @@ int main(int argc, char *argv[])
> >  			rc = wait(NULL);
> >  		} while (rc < 0 && errno == EINTR);
> >  
> > +#ifdef USE_PAM
> > +		rc = pam_close_session(pam_handle,0);
> > +		if(rc != PAM_SUCCESS) {
> > +			fprintf(stderr, "pam_close_session failed\n");
> > +			pam_end(pam_handle, rc);
> > +			exit(-1);
> > +		}
> 
> Hmm...so a pam_close_session failure bails before we restore the tty
> label?  Possibly the error message should include the pam_strerror()
> string as well?
> 
> > +
> > +		/* We're done with PAM.  Free `pam_handle'. */
> > +		pam_end(pam_handle, rc);
> > +#endif
> > +
> >  		if (!new_tty_context || !tty_context)
> >  			exit(0);
> >  
> > @@ -900,6 +909,16 @@ int main(int argc, char *argv[])
> >  	}
> >  #endif
> >  	freecon(old_context);
> > +
> > +#ifdef USE_PAM
> > +	/* Ask PAM to setup session for user running this program */
> > +	rc = pam_open_session(pam_handle,0);
> > +	if(rc != PAM_SUCCESS) {
> > +		fprintf(stderr, _("Namespace setup failed.\n"));
> > +		exit(-1);
> > +	}
> > +#endif
> 
> I understand that you need this to happen after the setexeccon, but do
> you really want it to happen after the audit message has already been
> generated?  The error message also should likely be both more general
> (since you only know that pam_open_session failed, not that it was
> specifically namespace related) and more specific (including
> pam_strerror result).
> 
> > +
> >  	execv(pw->pw_shell, argv + optind - 1);
> >  
> >  	/* If we reach here, then we failed to exec the new shell. */
> > diff -Naurp policycoreutils/newrole/newrole.pamd policycoreutils+newrolepatch/newrole/newrole.pamd
> > --- policycoreutils/newrole/newrole.pamd	2006-08-27 21:22:13.000000000 +0000
> > +++ policycoreutils+newrolepatch/newrole/newrole.pamd	2006-08-27 21:22:56.000000000 +0000
> > @@ -2,5 +2,3 @@
> >  auth       include	system-auth
> >  account    include	system-auth
> >  password   include	system-auth
> > -session    include	system-auth
> > -session    optional	pam_xauth.so
> 
> Unfortunately, this seems to yield a non-working newrole, as
> pam_open_session() then always fails due to the absence of any session
> modules.  What is the graceful way of handling this?
> 
-- 
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.

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

* Re: [PATCH] newrole: use pam session management services -v3
  2006-08-30 11:38   ` Stephen Smalley
@ 2006-08-30 17:38     ` Janak Desai
  2006-08-30 18:57       ` Stephen Smalley
  0 siblings, 1 reply; 6+ messages in thread
From: Janak Desai @ 2006-08-30 17:38 UTC (permalink / raw)
  To: Stephen Smalley
  Cc: sgrubb, klaus, chanson, kmacmillan, dwalsh, ltcgcw, selinux

On Wed, 2006-08-30 at 07:38 -0400, Stephen Smalley wrote:
> On Tue, 2006-08-29 at 17:08 -0400, Stephen Smalley wrote:
> > On Sun, 2006-08-27 at 22:14 -0400, Janak Desai wrote:
> > > This patch updates newrole to allow it to use pam session mangement
> > > services. Currently newrole only uses the authenticaion services of
> > > pam. Session management services are needed to allow pam_namespace
> > > to appropriately reconfigure polyinstantiated namespace for the
> > > new session being established with nerole.
> > > 
> > > Changes since v2 of this patch, posted on 8/16/06:
> > >     1. Restructured to use pam_start and pam_end only once
> > >     2. Updated to use pam_open_session and pam_close_session appropriately
> > >     3. Removed system-auth and pam_xauth session services from newrole.pamd.
> > >        These were not used since newrole did not have session management
> > >        support, and could result in unintended behavior now that session
> > >        management support is enabled with this patch.
> > >     4. Forward ported to the latest selinux subversion tree (version 2000)
> > >        on sourceforge. 
> > > 
> > > Signed-off-by: Janak Desai <janak@us.ibm.com>
> > > 
> > > ---
> > > 
> > >  newrole.c    |   95 +++++++++++++++++++++++++++++++++++------------------------
> > >  newrole.pamd |    2 -
> > >  2 files changed, 57 insertions(+), 40 deletions(-)
> > 
> > You didn't introduce this, but it appears that newrole no longer builds
> > properly with make LOG_AUDIT_PRIV=y (duplicate _GNU_SOURCE definitions)
> > when building on rawhide.  Aren't you encountering this?
> 
> Actually, on further thought, I don't understand how newrole can work
> with pam_namespace without further changes.  If you build newrole
> without LOG_AUDIT_PRIV=y, then it is non-suid and will lack the
> necessary capability to mount (which sadly is CAP_SYS_ADMIN, the gateway
> to all power).  If you build newrole with LOG_AUDIT_PRIV=y, then it is
> suid but drops all capabilities except for CAP_AUDIT_WRITE upon startup.
> So if you are running newrole as any non-root user, pam_namespace
> shouldn't be able to mount.  To do so, it would need to retain
> CAP_SYS_ADMIN all the way up until the point it calls pam_open_session,
> at which point the entire code needs much tighter review.  Am I missing
> something?
> 

No you are not. I just checked my build environment. I was using fedora
policycoreutils-1.30.26 which was built without LOG_AUDIT_PRIV. I think
I had just used rpmbuild --recompile, but I am not 100% sure. The 
makefile turns off LOG_AUDIT_PRIV. For my unit tests I am copying the
binary manually and making it suid because I wanted to keep my 
/etc/pam.d/newrole file. 

What is best way to proceed from here? I haven't played with
capability manipulation but is it possible to drop syadmin from
effective but not from permitted and raise it again just before
pam_open_session? 



> > > @@ -797,6 +794,18 @@ int main(int argc, char *argv[])
> > >  			rc = wait(NULL);
> > >  		} while (rc < 0 && errno == EINTR);
> > >  
> > > +#ifdef USE_PAM
> > > +		rc = pam_close_session(pam_handle,0);
> > > +		if(rc != PAM_SUCCESS) {
> > > +			fprintf(stderr, "pam_close_session failed\n");
> > > +			pam_end(pam_handle, rc);
> > > +			exit(-1);
> > > +		}
> > 
> > Hmm...so a pam_close_session failure bails before we restore the tty
> > label?  Possibly the error message should include the pam_strerror()
> > string as well?
> > 

ok. I will move the pam_close_session() call after the tty
context is restored and just before the parent exits.

> > > +
> > > +		/* We're done with PAM.  Free `pam_handle'. */
> > > +		pam_end(pam_handle, rc);
> > > +#endif
> > > +
> > >  		if (!new_tty_context || !tty_context)
> > >  			exit(0);
> > >  
> > > @@ -900,6 +909,16 @@ int main(int argc, char *argv[])
> > >  	}
> > >  #endif
> > >  	freecon(old_context);
> > > +
> > > +#ifdef USE_PAM
> > > +	/* Ask PAM to setup session for user running this program */
> > > +	rc = pam_open_session(pam_handle,0);
> > > +	if(rc != PAM_SUCCESS) {
> > > +		fprintf(stderr, _("Namespace setup failed.\n"));
> > > +		exit(-1);
> > > +	}
> > > +#endif
> > 
> > I understand that you need this to happen after the setexeccon, but do
> > you really want it to happen after the audit message has already been
> > generated?  The error message also should likely be both more general
> > (since you only know that pam_open_session failed, not that it was
> > specifically namespace related) and more specific (including
> > pam_strerror result).
> > 

ok. I will move pam_open_session to just after setexeccon() and update
the message appropriately. 

> > > +
> > >  	execv(pw->pw_shell, argv + optind - 1);
> > >  
> > >  	/* If we reach here, then we failed to exec the new shell. */
> > > diff -Naurp policycoreutils/newrole/newrole.pamd policycoreutils+newrolepatch/newrole/newrole.pamd
> > > --- policycoreutils/newrole/newrole.pamd	2006-08-27 21:22:13.000000000 +0000
> > > +++ policycoreutils+newrolepatch/newrole/newrole.pamd	2006-08-27 21:22:56.000000000 +0000
> > > @@ -2,5 +2,3 @@
> > >  auth       include	system-auth
> > >  account    include	system-auth
> > >  password   include	system-auth
> > > -session    include	system-auth
> > > -session    optional	pam_xauth.so
> > 
> > Unfortunately, this seems to yield a non-working newrole, as
> > pam_open_session() then always fails due to the absence of any session
> > modules.  What is the graceful way of handling this?
> > 

hmm. Not sure. Would it work to keep pam_namespace in here?
The /etc/security/namespace.conf file is installed by default
with no polyinstantiation so those that do not care to 
polyinstantiate any directories will still see the expected
behavior from their system. 

-Janak


--
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.

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

* Re: [PATCH] newrole: use pam session management services -v3
  2006-08-30 17:38     ` Janak Desai
@ 2006-08-30 18:57       ` Stephen Smalley
  2006-08-30 19:29         ` Janak Desai
  0 siblings, 1 reply; 6+ messages in thread
From: Stephen Smalley @ 2006-08-30 18:57 UTC (permalink / raw)
  To: janak; +Cc: sgrubb, klaus, chanson, kmacmillan, dwalsh, ltcgcw, selinux

On Wed, 2006-08-30 at 13:38 -0400, Janak Desai wrote:
> On Wed, 2006-08-30 at 07:38 -0400, Stephen Smalley wrote:
> > On Tue, 2006-08-29 at 17:08 -0400, Stephen Smalley wrote:
> > > On Sun, 2006-08-27 at 22:14 -0400, Janak Desai wrote:
> > > > This patch updates newrole to allow it to use pam session mangement
> > > > services. Currently newrole only uses the authenticaion services of
> > > > pam. Session management services are needed to allow pam_namespace
> > > > to appropriately reconfigure polyinstantiated namespace for the
> > > > new session being established with nerole.
> > > > 
> > > > Changes since v2 of this patch, posted on 8/16/06:
> > > >     1. Restructured to use pam_start and pam_end only once
> > > >     2. Updated to use pam_open_session and pam_close_session appropriately
> > > >     3. Removed system-auth and pam_xauth session services from newrole.pamd.
> > > >        These were not used since newrole did not have session management
> > > >        support, and could result in unintended behavior now that session
> > > >        management support is enabled with this patch.
> > > >     4. Forward ported to the latest selinux subversion tree (version 2000)
> > > >        on sourceforge. 
> > > > 
> > > > Signed-off-by: Janak Desai <janak@us.ibm.com>
> > > > 
> > > > ---
> > > > 
> > > >  newrole.c    |   95 +++++++++++++++++++++++++++++++++++------------------------
> > > >  newrole.pamd |    2 -
> > > >  2 files changed, 57 insertions(+), 40 deletions(-)
> > > 
> > > You didn't introduce this, but it appears that newrole no longer builds
> > > properly with make LOG_AUDIT_PRIV=y (duplicate _GNU_SOURCE definitions)
> > > when building on rawhide.  Aren't you encountering this?
> > 
> > Actually, on further thought, I don't understand how newrole can work
> > with pam_namespace without further changes.  If you build newrole
> > without LOG_AUDIT_PRIV=y, then it is non-suid and will lack the
> > necessary capability to mount (which sadly is CAP_SYS_ADMIN, the gateway
> > to all power).  If you build newrole with LOG_AUDIT_PRIV=y, then it is
> > suid but drops all capabilities except for CAP_AUDIT_WRITE upon startup.
> > So if you are running newrole as any non-root user, pam_namespace
> > shouldn't be able to mount.  To do so, it would need to retain
> > CAP_SYS_ADMIN all the way up until the point it calls pam_open_session,
> > at which point the entire code needs much tighter review.  Am I missing
> > something?
> > 
> 
> No you are not. I just checked my build environment. I was using fedora
> policycoreutils-1.30.26 which was built without LOG_AUDIT_PRIV. I think
> I had just used rpmbuild --recompile, but I am not 100% sure. The 
> makefile turns off LOG_AUDIT_PRIV. For my unit tests I am copying the
> binary manually and making it suid because I wanted to keep my 
> /etc/pam.d/newrole file.

Hmmm...AFAICS, that rpm does build with LOG_AUDIT_PRIV (=y), and
installing it does yield a suid newrole binary that drops all caps
except CAP_AUDIT_WRITE when executed unless the caller is root already.
So you must have disabled LOG_AUDIT_PRIV yourself, or only tested it as
a root user (make sure you test as both root and non-root, as there is a
difference there).

> What is best way to proceed from here? I haven't played with
> capability manipulation but is it possible to drop syadmin from
> effective but not from permitted and raise it again just before
> pam_open_session?

That appears to be possible.  It isn't a very strong guarantee though.

There is also the question of when newrole should retain that
capability, since the newrole code itself doesn't directly use it ever.
Ideally, newrole would only retain it if pam_namespace is actually
present in the pam config.  Possibly we should have two separate
newrole.pamd files, one installed by default w/o pam_namespace and one
installed if LOG_AUDIT_PRIV=y that includes pam_namespace, and then you
make your code changes conditional on LOG_AUDIT_PRIV as well.  Likely
should rename LOG_AUDIT_PRIV at that point to something more general,
like LSPP_PRIV, to connote the fact that it is needed for LSPP
functionality (both audit and namespace) and that it requires extra
privileges for newrole.

You'd also need a policy change for newrole, or handle that via a
loadable policy module.

> > > > diff -Naurp policycoreutils/newrole/newrole.pamd policycoreutils+newrolepatch/newrole/newrole.pamd
> > > > --- policycoreutils/newrole/newrole.pamd	2006-08-27 21:22:13.000000000 +0000
> > > > +++ policycoreutils+newrolepatch/newrole/newrole.pamd	2006-08-27 21:22:56.000000000 +0000
> > > > @@ -2,5 +2,3 @@
> > > >  auth       include	system-auth
> > > >  account    include	system-auth
> > > >  password   include	system-auth
> > > > -session    include	system-auth
> > > > -session    optional	pam_xauth.so
> > > 
> > > Unfortunately, this seems to yield a non-working newrole, as
> > > pam_open_session() then always fails due to the absence of any session
> > > modules.  What is the graceful way of handling this?
> > > 
> 
> hmm. Not sure. Would it work to keep pam_namespace in here?
> The /etc/security/namespace.conf file is installed by default
> with no polyinstantiation so those that do not care to 
> polyinstantiate any directories will still see the expected
> behavior from their system. 

The problem is that not everyone has pam_namespace (at least outside of
Fedora), and I assume that newrole would likewise fail fatally upon
pam_open_session if we put pam_namespace as the only session module and
it isn't present on the system.  But if we have two separate pam configs
as above, then that would likely work.

-- 
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.

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

* Re: [PATCH] newrole: use pam session management services -v3
  2006-08-30 18:57       ` Stephen Smalley
@ 2006-08-30 19:29         ` Janak Desai
  0 siblings, 0 replies; 6+ messages in thread
From: Janak Desai @ 2006-08-30 19:29 UTC (permalink / raw)
  To: Stephen Smalley
  Cc: sgrubb, klaus, chanson, kmacmillan, dwalsh, ltcgcw, selinux

On Wed, 2006-08-30 at 14:57 -0400, Stephen Smalley wrote:
> On Wed, 2006-08-30 at 13:38 -0400, Janak Desai wrote:
> > On Wed, 2006-08-30 at 07:38 -0400, Stephen Smalley wrote:
> > > On Tue, 2006-08-29 at 17:08 -0400, Stephen Smalley wrote:
> > > > On Sun, 2006-08-27 at 22:14 -0400, Janak Desai wrote:
> > > > > This patch updates newrole to allow it to use pam session mangement
> > > > > services. Currently newrole only uses the authenticaion services of
> > > > > pam. Session management services are needed to allow pam_namespace
> > > > > to appropriately reconfigure polyinstantiated namespace for the
> > > > > new session being established with nerole.
> > > > > 
> > > > > Changes since v2 of this patch, posted on 8/16/06:
> > > > >     1. Restructured to use pam_start and pam_end only once
> > > > >     2. Updated to use pam_open_session and pam_close_session appropriately
> > > > >     3. Removed system-auth and pam_xauth session services from newrole.pamd.
> > > > >        These were not used since newrole did not have session management
> > > > >        support, and could result in unintended behavior now that session
> > > > >        management support is enabled with this patch.
> > > > >     4. Forward ported to the latest selinux subversion tree (version 2000)
> > > > >        on sourceforge. 
> > > > > 
> > > > > Signed-off-by: Janak Desai <janak@us.ibm.com>
> > > > > 
> > > > > ---
> > > > > 
> > > > >  newrole.c    |   95 +++++++++++++++++++++++++++++++++++------------------------
> > > > >  newrole.pamd |    2 -
> > > > >  2 files changed, 57 insertions(+), 40 deletions(-)
> > > > 
> > > > You didn't introduce this, but it appears that newrole no longer builds
> > > > properly with make LOG_AUDIT_PRIV=y (duplicate _GNU_SOURCE definitions)
> > > > when building on rawhide.  Aren't you encountering this?
> > > 
> > > Actually, on further thought, I don't understand how newrole can work
> > > with pam_namespace without further changes.  If you build newrole
> > > without LOG_AUDIT_PRIV=y, then it is non-suid and will lack the
> > > necessary capability to mount (which sadly is CAP_SYS_ADMIN, the gateway
> > > to all power).  If you build newrole with LOG_AUDIT_PRIV=y, then it is
> > > suid but drops all capabilities except for CAP_AUDIT_WRITE upon startup.
> > > So if you are running newrole as any non-root user, pam_namespace
> > > shouldn't be able to mount.  To do so, it would need to retain
> > > CAP_SYS_ADMIN all the way up until the point it calls pam_open_session,
> > > at which point the entire code needs much tighter review.  Am I missing
> > > something?
> > > 
> > 
> > No you are not. I just checked my build environment. I was using fedora
> > policycoreutils-1.30.26 which was built without LOG_AUDIT_PRIV. I think
> > I had just used rpmbuild --recompile, but I am not 100% sure. The 
> > makefile turns off LOG_AUDIT_PRIV. For my unit tests I am copying the
> > binary manually and making it suid because I wanted to keep my 
> > /etc/pam.d/newrole file.
> 
> Hmmm...AFAICS, that rpm does build with LOG_AUDIT_PRIV (=y), and
> installing it does yield a suid newrole binary that drops all caps
> except CAP_AUDIT_WRITE when executed unless the caller is root already.
> So you must have disabled LOG_AUDIT_PRIV yourself, or only tested it as
> a root user (make sure you test as both root and non-root, as there is a
> difference there).
> 
> > What is best way to proceed from here? I haven't played with
> > capability manipulation but is it possible to drop syadmin from
> > effective but not from permitted and raise it again just before
> > pam_open_session?
> 
> That appears to be possible.  It isn't a very strong guarantee though.
> 
> There is also the question of when newrole should retain that
> capability, since the newrole code itself doesn't directly use it ever.
> Ideally, newrole would only retain it if pam_namespace is actually
> present in the pam config.  Possibly we should have two separate
> newrole.pamd files, one installed by default w/o pam_namespace and one
> installed if LOG_AUDIT_PRIV=y that includes pam_namespace, and then you
> make your code changes conditional on LOG_AUDIT_PRIV as well.  Likely
> should rename LOG_AUDIT_PRIV at that point to something more general,
> like LSPP_PRIV, to connote the fact that it is needed for LSPP
> functionality (both audit and namespace) and that it requires extra
> privileges for newrole.
> 
> You'd also need a policy change for newrole, or handle that via a
> loadable policy module.
> 
> > > > > diff -Naurp policycoreutils/newrole/newrole.pamd policycoreutils+newrolepatch/newrole/newrole.pamd
> > > > > --- policycoreutils/newrole/newrole.pamd	2006-08-27 21:22:13.000000000 +0000
> > > > > +++ policycoreutils+newrolepatch/newrole/newrole.pamd	2006-08-27 21:22:56.000000000 +0000
> > > > > @@ -2,5 +2,3 @@
> > > > >  auth       include	system-auth
> > > > >  account    include	system-auth
> > > > >  password   include	system-auth
> > > > > -session    include	system-auth
> > > > > -session    optional	pam_xauth.so
> > > > 
> > > > Unfortunately, this seems to yield a non-working newrole, as
> > > > pam_open_session() then always fails due to the absence of any session
> > > > modules.  What is the graceful way of handling this?
> > > > 
> > 
> > hmm. Not sure. Would it work to keep pam_namespace in here?
> > The /etc/security/namespace.conf file is installed by default
> > with no polyinstantiation so those that do not care to 
> > polyinstantiate any directories will still see the expected
> > behavior from their system. 
> 
> The problem is that not everyone has pam_namespace (at least outside of
> Fedora), and I assume that newrole would likewise fail fatally upon
> pam_open_session if we put pam_namespace as the only session module and
> it isn't present on the system.  But if we have two separate pam configs
> as above, then that would likely work.
> 

ok, thanks. I will incorporate these changes and repost.

-Janak


--
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.

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

end of thread, other threads:[~2006-08-30 19:29 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2006-08-28  2:14 [PATCH] newrole: use pam session management services -v3 Janak Desai
2006-08-29 21:08 ` Stephen Smalley
2006-08-30 11:38   ` Stephen Smalley
2006-08-30 17:38     ` Janak Desai
2006-08-30 18:57       ` Stephen Smalley
2006-08-30 19:29         ` Janak Desai

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