[PATCH] cron changes needed for MLS range checking (requires at least the libselinux patches)

[PATCH] cron changes needed for MLS range checking (requires at least the libselinux patches)

[James Antill]

[-- Attachment #1.1: Type: text/plain, Size: 520 bytes --]


 I think this is what we want for the cron patch. It's basically doing
the same checks as the PAM patches. It also limits what the user can
change to just the MLS range.
 At the moment I've just copied the original functions that need to be
replaced, so you can see the old vs. the new. As the final commit the
old ones should probably just die.
 I've also kept the name SELINUX_ROLE_TYPE, I'm not sure if it should be
changed to SELINUX_ROLE_RANGE or something else?

-- 
James Antill <jantill@redhat.com>

[-- Attachment #1.2: vixie-cron-4.1-_60-SELinux-contains-range.patch --]
[-- Type: text/x-patch, Size: 7775 bytes --]

Only in vixie-cron-4.1: crond.pam.pamd_crond
diff -rup vixie-cron-4.1-orig/security.c vixie-cron-4.1/security.c
--- vixie-cron-4.1-orig/security.c	2006-11-02 22:28:04.000000000 -0500
+++ vixie-cron-4.1/security.c	2006-11-06 18:04:21.000000000 -0500
@@ -23,12 +23,16 @@
 
 #ifdef WITH_SELINUX
 #include <selinux/selinux.h>
+#include <selinux/context.h>
 #include <selinux/flask.h>
 #include <selinux/av_permissions.h>
 #include <selinux/get_context_list.h>
 #endif
 
 static char ** build_env(char **cronenv);
+static int cron_get_job_range( user *u, void *scontextp, void *file_contextp, char **jobenv );
+static int cron_change_selinux_range( user *u,
+                                      void *scontext, void *file_context );
 
 int cron_set_job_security_context( entry *e, user *u, char ***jobenv )
 {
@@ -60,7 +64,8 @@ int cron_set_job_security_context( entry
 
     security_context_t scontext=0, file_context=0; 
 
-    if ( cron_get_job_context(u, &scontext, &file_context, *jobenv) < OK )
+    /* if ( cron_get_job_context(u, &scontext, &file_context, *jobenv) < OK ) */
+    if ( cron_get_job_range(u, &scontext, &file_context, *jobenv) < OK )
     {
 	syslog(LOG_ERR, "CRON (%s) ERROR: failed to get selinux context: %s", 
 	       e->pwd->pw_name, strerror(errno)
@@ -79,7 +84,8 @@ int cron_set_job_security_context( entry
     }	
 
 #if WITH_SELINUX
-    if ( cron_change_selinux_context( u, scontext, file_context ) != 0 )
+    /* if ( cron_change_selinux_context( u, scontext, file_context ) != 0 ) */
+    if ( cron_change_selinux_range( u, scontext, file_context ) != 0 )
     {
         syslog(LOG_INFO,"CRON (%s) ERROR: failed to change SELinux context", 
 	       e->pwd->pw_name);
@@ -201,6 +207,7 @@ cron_authorize_context
 #ifdef WITH_SELINUX
 	struct av_decision avd;
 	int retval;
+        unsigned int bit = FILE__ENTRYPOINT;
 	/*
 	 * Since crontab files are not directly executed,
 	 * crond must ensure that the crontab file has
@@ -208,13 +215,37 @@ cron_authorize_context
 	 * the user cron job.  It performs an entrypoint
 	 * permission check for this purpose.
 	 */
-	retval = security_compute_av(scontext,
-				     file_context,
-				     SECCLASS_FILE,
-				     FILE__ENTRYPOINT,
-				     &avd);
+	retval = security_compute_av(scontext, file_context,
+				     SECCLASS_FILE, bit, &avd);
 
-	if (retval || ((FILE__ENTRYPOINT & avd.allowed) != FILE__ENTRYPOINT))
+	if (retval || ((bit & avd.allowed) != bit))
+		return 0;
+#endif
+	return 1;
+}
+
+static int 
+cron_authorize_range
+( 
+	security_context_t scontext,
+	security_context_t file_context
+)	
+{
+#ifdef WITH_SELINUX
+	struct av_decision avd;
+	int retval;
+        unsigned int bit = CONTEXT__CONTAINS;
+	/*
+	 * Since crontab files are not directly executed,
+	 * crond must ensure that the crontab range has
+	 * a context that is appropriate for the context of
+	 * the user cron job.  It performs an entrypoint
+	 * permission check for this purpose.
+	 */
+	retval = security_compute_av(scontext, file_context,
+				     SECCLASS_CONTEXT, bit, &avd);
+
+	if (retval || ((bit & avd.allowed) != bit))
 		return 0;
 #endif
 	return 1;
@@ -265,6 +296,98 @@ int cron_get_job_context( user *u, void 
 	return 0;
 }
 
+static int cron_get_job_range( user *u, void *scontextp, void *file_contextp, char **jobenv )
+{
+#if WITH_SELINUX
+	char *sroletype;
+
+	if ( is_selinux_enabled() <= 0 )
+		return 0;
+	if ( (file_contextp == 0) || (scontextp == 0L) )
+		return -1;
+
+	*((security_context_t*)scontextp) = u->scontext;
+	*((void **)file_contextp) = 0L;
+
+	if ( (sroletype = env_get("SELINUX_ROLE_TYPE",jobenv)) != 0L )
+	{
+		char crontab[MAX_FNAME];
+                context_t ccon;
+
+		if ( strcmp(u->name,"*system*") == 0 )
+			strncpy(crontab, u->tabname, MAX_FNAME);
+		else
+			snprintf(crontab, MAX_FNAME, "%s/%s", CRONDIR, u->tabname);
+
+		if ( getfilecon( crontab, file_contextp ) == -1 )
+		{		
+			if ( security_getenforce() > 0 ) 
+			{
+				log_it(u->name, 
+				       getpid(), "getfilecon FAILED for SELINUX_ROLE_TYPE", 
+				       sroletype
+				      );
+				return -1;
+			} else
+			if ( access( crontab, F_OK ) == 0 )
+                        {
+				log_it(u->name,
+				       getpid(), 
+				       "getfilecon FAILED but SELinux in permissive mode, continuing "
+				       "- SELINUX_ROLE_TYPE=", sroletype
+				       );
+				return 0;
+                        }
+		}
+                
+                if (!(ccon = context_new(file_contextp)))
+                {
+			if ( security_getenforce() > 0 ) 
+			{
+				log_it(u->name, 
+				       getpid(), "context_new FAILED for SELINUX_ROLE_TYPE", 
+				       sroletype
+				      );
+				return -1;
+			} else
+                        {
+				log_it(u->name,
+				       getpid(), 
+				       "context_new FAILED but SELinux in permissive mode, continuing "
+				       "- SELINUX_ROLE_TYPE=", sroletype
+				       );
+				return 0;
+                        }
+                }                  
+
+                if (context_range_set(ccon, sroletype))
+                {
+			if ( security_getenforce() > 0 ) 
+			{
+				log_it(u->name, 
+				       getpid(), "context_range_set FAILED for SELINUX_ROLE_TYPE", 
+				       sroletype
+				      );
+				return -1;
+			} else
+                        {
+				log_it(u->name,
+				       getpid(), 
+				       "context_range_set FAILED but SELinux in permissive mode, continuing "
+				       "- SELINUX_ROLE_TYPE=", sroletype
+				       );
+				return 0;
+                        }
+                }
+
+	        *((security_context_t*)scontextp) = context_str(ccon);
+
+                context_free(ccon);
+	}
+#endif
+	return 0;
+}
+
 int cron_change_selinux_context( user *u, void *scontext, void *file_context )
 {
 #ifdef WITH_SELINUX
@@ -332,6 +455,74 @@ int cron_change_selinux_context( user *u
 	return 0;
 }
 
+static int cron_change_selinux_range( user *u,
+                                      void *scontext, void *file_context )
+{
+#ifdef WITH_SELINUX
+	if ( is_selinux_enabled() <= 0 )
+		return 0;
+
+	if ( scontext == 0L )
+	{
+		if (security_getenforce() > 0) 
+		{
+			log_it( u->name, getpid(), 
+				"NULL security context for user", 
+				""
+			      );
+			return -1;
+		}else
+		{
+			log_it( u->name, getpid(), 
+				"NULL security context for user, "
+				"but SELinux in permissive mode, continuing",
+				""
+				);
+			return 0;
+		}
+	}
+	
+	if ( file_context )
+	{		
+		if ( ! cron_authorize_range( scontext, file_context ) )
+		{
+			if ( security_getenforce() > 0 ) 
+			{
+				syslog(LOG_ERR,
+				       "CRON (%s) ERROR:"
+				       "Unauthorized exec context to SELINUX_ROLE_TYPE %s for user", 
+				       u->name, (char*)scontext
+				      );
+				return -1;
+			} else
+			{
+				syslog(LOG_INFO,
+				       "CRON (%s) WARNING:"
+				       "Unauthorized exec context to SELINUX_ROLE_TYPE %s for user,"
+				       " but SELinux in permissive mode, continuing", 
+				       u->name, (char*)scontext
+				      );
+			}
+		}
+	} 
+
+	if ( setexeccon(scontext) < 0 ) 
+	{
+		if (security_getenforce() > 0) 
+		{
+			syslog(LOG_ERR,
+			       "CRON (%s) ERROR:"
+			       "Could not set exec context to %s for user", 
+			       u->name, (char*)scontext
+			      );
+
+			return -1;
+		}
+	}
+#endif
+	return 0;
+}
+
 int get_security_context( const char *name, 
 			  int crontab_fd, 
 			  security_context_t *rcontext, 
Only in vixie-cron-4.1: security.c.security
Only in vixie-cron-4.1: security.c.selinux-contains-range

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

Re: [PATCH] cron changes needed for MLS range checking (requires at least the libselinux patches)

[Stephen Smalley]

On Tue, 2006-11-07 at 17:02 -0500, James Antill wrote:
>  I think this is what we want for the cron patch. It's basically doing
> the same checks as the PAM patches. It also limits what the user can
> change to just the MLS range.
>  At the moment I've just copied the original functions that need to be
> replaced, so you can see the old vs. the new. As the final commit the
> old ones should probably just die.
>  I've also kept the name SELINUX_ROLE_TYPE, I'm not sure if it should be
> changed to SELINUX_ROLE_RANGE or something else?

As I understood it, you were only going to allow level specification,
not user/role/domain, so it would just be SELINUX_LEVEL or MLS_LEVEL or
similar.

As in the pam case, you should be checking between a context for the
user with the seusers-specified range and a context for the user with
the user-specified level.  Your patch doesn't seem to match that
description - it refers to a file context as the target.

Also, the function that performs the setexeccon (which you call
cron_change_selinux_range) is more general - it is supposed to set the
entire user context appropriately for the user on whose behalf cron is
running a job.  

-- 
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] cron changes needed for MLS range checking (requires at least the libselinux patches)

[James Antill]

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

On Wed, 2006-11-08 at 09:04 -0500, Stephen Smalley wrote:
> On Tue, 2006-11-07 at 17:02 -0500, James Antill wrote:
> >  I think this is what we want for the cron patch. It's basically doing
> > the same checks as the PAM patches. It also limits what the user can
> > change to just the MLS range.
> >  At the moment I've just copied the original functions that need to be
> > replaced, so you can see the old vs. the new. As the final commit the
> > old ones should probably just die.
> >  I've also kept the name SELINUX_ROLE_TYPE, I'm not sure if it should be
> > changed to SELINUX_ROLE_RANGE or something else?
> 
> As I understood it, you were only going to allow level specification,
> not user/role/domain, so it would just be SELINUX_LEVEL or MLS_LEVEL or
> similar.

 Right, that's what I meant by "if it should be changed...". After
speaking with Dan today, I've changed it to MLS_LEVEL.

> As in the pam case, you should be checking between a context for the
> user with the seusers-specified range and a context for the user with
> the user-specified level.  Your patch doesn't seem to match that
> description - it refers to a file context as the target.

 One context comes from the cron file, and one from that plus the level
change as requested by the user. See cron_get_job_range().
 Changing that to be the result of getseuserbyname(), matching PAM,
instead of the file context would be possible ... although I'm not sure
if using "root" when u->name is "*system*" is the right thing to do.

> Also, the function that performs the setexeccon (which you call
> cron_change_selinux_range) is more general - it is supposed to set the
> entire user context appropriately for the user on whose behalf cron is
> running a job.  

 Right. Are you saying I need to call, cron_authorize_context() as well
as cron_authorize_range()?
 I decided this wasn't required because that function is called from
within get_security_context(), and instead of being able to change
everything now ... they can only change the level. So we don't need to
re-auth the entire security context, just the level.
 I'm certainly open to just checking it anyway, if you see any holes in
my reasoning or if everyone would just prefer to check it twice.

 If that isn't what you meant, could you explain further what the
problem is?

-- 
James Antill <jantill@redhat.com>

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

Re: [PATCH] cron changes needed for MLS range checking (requires at least the libselinux patches)

[Stephen Smalley]

On Wed, 2006-11-08 at 15:32 -0500, James Antill wrote:
> > As in the pam case, you should be checking between a context for the
> > user with the seusers-specified range and a context for the user with
> > the user-specified level.  Your patch doesn't seem to match that
> > description - it refers to a file context as the target.
> 
>  One context comes from the cron file, and one from that plus the level
> change as requested by the user. See cron_get_job_range().
>  Changing that to be the result of getseuserbyname(), matching PAM,
> instead of the file context would be possible ... although I'm not sure
> if using "root" when u->name is "*system*" is the right thing to do.

The scontext is supposed to be a process context in which to run the
cron job, not a file context.  You are presently replacing the default
scontext (extracted from u->scontext that was previously computed) with
a strange mixture of the crontab file context and the user-specified
range.  What you want to do is to take the default scontext value,
create a new context that is identical except for its range (from the
environment), and apply a check between those two contexts (and the
check is only needed when using a user-supplied range).  BTW, you cannot
continue to refer to the string returned by context_str() after
performing a context_free() on the structure; you'd have to dup it
first.

> > Also, the function that performs the setexeccon (which you call
> > cron_change_selinux_range) is more general - it is supposed to set the
> > entire user context appropriately for the user on whose behalf cron is
> > running a job.  
> 
>  Right. Are you saying I need to call, cron_authorize_context() as well
> as cron_authorize_range()?
>  I decided this wasn't required because that function is called from
> within get_security_context(), and instead of being able to change
> everything now ... they can only change the level. So we don't need to
> re-auth the entire security context, just the level.
>  I'm certainly open to just checking it anyway, if you see any holes in
> my reasoning or if everyone would just prefer to check it twice.
> 
>  If that isn't what you meant, could you explain further what the
> problem is?

That is what I meant, but if it is being checked earlier and the crontab
file cannot be replaced in the interim without going through the check
again, it may be ok to not recheck in your patch.

-- 
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] cron changes needed for MLS range checking (requires at least the libselinux patches)

[James Antill]

[-- Attachment #1.1: Type: text/plain, Size: 1048 bytes --]

On Wed, 2006-11-08 at 15:53 -0500, Stephen Smalley wrote:

> The scontext is supposed to be a process context in which to run the
> cron job, not a file context.  You are presently replacing the default
> scontext (extracted from u->scontext that was previously computed) with
> a strange mixture of the crontab file context and the user-specified
> range.  What you want to do is to take the default scontext value,
> create a new context that is identical except for its range (from the
> environment), and apply a check between those two contexts (and the
> check is only needed when using a user-supplied range).

 Ok, I've used u->scontext instead of the file context now. I've also
renamed the variables. And the check should only happen if they specify
a different level.

>   BTW, you cannot
> continue to refer to the string returned by context_str() after
> performing a context_free() on the structure; you'd have to dup it
> first.

 Right, stupid mistake. Fixed that too.

-- 
James Antill <jantill@redhat.com>

[-- Attachment #1.2: vixie-cron-4.1-_60-SELinux-contains-range.patch --]
[-- Type: text/x-patch, Size: 7514 bytes --]

Only in vixie-cron-4.1: crond.pam.pamd_crond
diff -rup vixie-cron-4.1-orig/security.c vixie-cron-4.1/security.c
--- vixie-cron-4.1-orig/security.c	2006-11-02 22:28:04.000000000 -0500
+++ vixie-cron-4.1/security.c	2006-11-08 16:54:03.000000000 -0500
@@ -23,6 +23,7 @@
 
 #ifdef WITH_SELINUX
 #include <selinux/selinux.h>
+#include <selinux/context.h>
 #include <selinux/flask.h>
 #include <selinux/av_permissions.h>
 #include <selinux/get_context_list.h>
@@ -30,6 +31,12 @@
 
 static char ** build_env(char **cronenv);
 
+#ifdef WITH_SELINUX
+static int cron_change_selinux_range( user *u,
+                                      security_context_t ucontext );
+static int cron_get_job_range( user *u, security_context_t *ucontextp, char **jobenv );
+#endif
+
 int cron_set_job_security_context( entry *e, user *u, char ***jobenv )
 {
     time_t minutely_time = 0;
@@ -58,9 +65,9 @@ int cron_set_job_security_context( entry
      * we'll not be permitted to read the cron spool directory :-)
      */
 
-    security_context_t scontext=0, file_context=0; 
+    security_context_t ucontext=0; 
 
-    if ( cron_get_job_context(u, &scontext, &file_context, *jobenv) < OK )
+    if ( cron_get_job_range(u, &ucontext, *jobenv) < OK )
     {
 	syslog(LOG_ERR, "CRON (%s) ERROR: failed to get selinux context: %s", 
 	       e->pwd->pw_name, strerror(errno)
@@ -79,16 +86,16 @@ int cron_set_job_security_context( entry
     }	
 
 #if WITH_SELINUX
-    if ( cron_change_selinux_context( u, scontext, file_context ) != 0 )
+    if (cron_change_selinux_range(u, ucontext) != 0)
     {
         syslog(LOG_INFO,"CRON (%s) ERROR: failed to change SELinux context", 
 	       e->pwd->pw_name);
-	if ( file_context )
-		freecon(file_context);
+	if ( ucontext )
+		freecon(ucontext);
 	return -1;
     }
-    if ( file_context )
-	freecon(file_context);
+    if ( ucontext )
+	freecon(ucontext);
 #endif
 
     log_close();
@@ -201,6 +208,7 @@ cron_authorize_context
 #ifdef WITH_SELINUX
 	struct av_decision avd;
 	int retval;
+        unsigned int bit = FILE__ENTRYPOINT;
 	/*
 	 * Since crontab files are not directly executed,
 	 * crond must ensure that the crontab file has
@@ -208,13 +216,37 @@ cron_authorize_context
 	 * the user cron job.  It performs an entrypoint
 	 * permission check for this purpose.
 	 */
-	retval = security_compute_av(scontext,
-				     file_context,
-				     SECCLASS_FILE,
-				     FILE__ENTRYPOINT,
-				     &avd);
+	retval = security_compute_av(scontext, file_context,
+				     SECCLASS_FILE, bit, &avd);
+
+	if (retval || ((bit & avd.allowed) != bit))
+		return 0;
+#endif
+	return 1;
+}
+
+static int 
+cron_authorize_range
+( 
+	security_context_t scontext,
+	security_context_t file_context
+)	
+{
+#ifdef WITH_SELINUX
+	struct av_decision avd;
+	int retval;
+        unsigned int bit = CONTEXT__CONTAINS;
+	/*
+	 * Since crontab files are not directly executed,
+	 * crond must ensure that the crontab range has
+	 * a context that is appropriate for the context of
+	 * the user cron job.  It performs an entrypoint
+	 * permission check for this purpose.
+	 */
+	retval = security_compute_av(scontext, file_context,
+				     SECCLASS_CONTEXT, bit, &avd);
 
-	if (retval || ((FILE__ENTRYPOINT & avd.allowed) != FILE__ENTRYPOINT))
+	if (retval || ((bit & avd.allowed) != bit))
 		return 0;
 #endif
 	return 1;
@@ -265,6 +297,80 @@ int cron_get_job_context( user *u, void 
 	return 0;
 }
 
+#if WITH_SELINUX
+/* always uses u->scontext as the default process context, then changes the
+   level, and retuns it in ucontextp (or NULL otherwise) */
+static int cron_get_job_range( user *u, security_context_t *ucontextp,
+                               char **jobenv )
+{
+	char *sroletype;
+
+	if ( is_selinux_enabled() <= 0 )
+		return 0;
+	if ( ucontextp == 0L )
+		return -1;
+
+	*ucontextp = 0L;
+
+	if ( (sroletype = env_get("MLS_LEVEL",jobenv)) != 0L )
+	{
+		char crontab[MAX_FNAME];
+                context_t ccon;
+
+		if ( strcmp(u->name,"*system*") == 0 )
+			strncpy(crontab, u->tabname, MAX_FNAME);
+		else
+			snprintf(crontab, MAX_FNAME, "%s/%s", CRONDIR, u->tabname);
+                
+                if (!(ccon = context_new(u->scontext)))
+                {
+			if ( security_getenforce() > 0 ) 
+			{
+				log_it(u->name, 
+				       getpid(), "context_new FAILED for SELINUX_ROLE_TYPE", 
+				       sroletype
+				      );
+				return -1;
+			} else
+                        {
+				log_it(u->name,
+				       getpid(), 
+				       "context_new FAILED but SELinux in permissive mode, continuing "
+				       "- SELINUX_ROLE_TYPE=", sroletype
+				       );
+				return 0;
+                        }
+                }                  
+
+                if (context_range_set(ccon, sroletype))
+                {
+			if ( security_getenforce() > 0 ) 
+			{
+				log_it(u->name, 
+				       getpid(), "context_range_set FAILED for SELINUX_ROLE_TYPE", 
+				       sroletype
+				      );
+				return -1;
+			} else
+                        {
+				log_it(u->name,
+				       getpid(), 
+				       "context_range_set FAILED but SELinux in permissive mode, continuing "
+				       "- SELINUX_ROLE_TYPE=", sroletype
+				       );
+				return 0;
+                        }
+                }
+
+	        *ucontextp = strdup(context_str(ccon));
+
+                context_free(ccon);
+	}
+
+	return 0;
+}
+#endif
+
 int cron_change_selinux_context( user *u, void *scontext, void *file_context )
 {
 #ifdef WITH_SELINUX
@@ -332,6 +438,74 @@ int cron_change_selinux_context( user *u
 	return 0;
 }
 
+#ifdef WITH_SELINUX
+static int cron_change_selinux_range( user *u,
+                                      security_context_t ucontext )
+{
+	if ( is_selinux_enabled() <= 0 )
+		return 0;
+
+	if ( u->scontext == 0L )
+	{
+		if (security_getenforce() > 0) 
+		{
+			log_it( u->name, getpid(), 
+				"NULL security context for user", 
+				""
+			      );
+			return -1;
+		}else
+		{
+			log_it( u->name, getpid(), 
+				"NULL security context for user, "
+				"but SELinux in permissive mode, continuing",
+				""
+				);
+			return 0;
+		}
+	}
+	
+	if ( ucontext && strcmp(u->scontext, ucontext) )
+	{		
+                if ( ! cron_authorize_range( u->scontext, ucontext ))
+		{
+			if ( security_getenforce() > 0 ) 
+			{
+				syslog(LOG_ERR,
+				       "CRON (%s) ERROR:"
+				       "Unauthorized exec context to SELINUX_ROLE_TYPE %s for user", 
+				       u->name, (char*)ucontext
+				      );
+				return -1;
+			} else
+			{
+				syslog(LOG_INFO,
+				       "CRON (%s) WARNING:"
+				       "Unauthorized exec context to SELINUX_ROLE_TYPE %s for user,"
+				       " but SELinux in permissive mode, continuing", 
+				       u->name, (char*)ucontext
+				      );
+			}
+		}
+	} 
+
+	if ( setexeccon(ucontext) < 0 ) 
+	{
+		if (security_getenforce() > 0) 
+		{
+			syslog(LOG_ERR,
+			       "CRON (%s) ERROR:"
+			       "Could not set exec context to %s for user", 
+			       u->name, (char*)ucontext
+			      );
+
+			return -1;
+		}
+	}
+	return 0;
+}
+#endif
+
 int get_security_context( const char *name, 
 			  int crontab_fd, 
 			  security_context_t *rcontext, 
Only in vixie-cron-4.1: security.c.security
Only in vixie-cron-4.1: security.c.selinux-contains-range

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

Re: [PATCH] cron changes needed for MLS range checking (requires at least the libselinux patches)

[Stephen Smalley]

On Wed, 2006-11-08 at 16:57 -0500, James Antill wrote:
> On Wed, 2006-11-08 at 15:53 -0500, Stephen Smalley wrote:
> 
> > The scontext is supposed to be a process context in which to run the
> > cron job, not a file context.  You are presently replacing the default
> > scontext (extracted from u->scontext that was previously computed) with
> > a strange mixture of the crontab file context and the user-specified
> > range.  What you want to do is to take the default scontext value,
> > create a new context that is identical except for its range (from the
> > environment), and apply a check between those two contexts (and the
> > check is only needed when using a user-supplied range).
> 
>  Ok, I've used u->scontext instead of the file context now. I've also
> renamed the variables. And the check should only happen if they specify
> a different level.
> 
> >   BTW, you cannot
> > continue to refer to the string returned by context_str() after
> > performing a context_free() on the structure; you'd have to dup it
> > first.
> 
>  Right, stupid mistake. Fixed that too.

Looks better.  A few nits:

+static int 
+cron_authorize_range
+( 
+	security_context_t scontext,
+	security_context_t file_context

s/file_context/ucontext/

+)	
+{
+#ifdef WITH_SELINUX
+	struct av_decision avd;
+	int retval;
+        unsigned int bit = CONTEXT__CONTAINS;
+	/*
+	 * Since crontab files are not directly executed,
+	 * crond must ensure that the crontab range has
+	 * a context that is appropriate for the context of
+	 * the user cron job.  It performs an entrypoint
+	 * permission check for this purpose.

cut-and-paste

+	 */
+	retval = security_compute_av(scontext, file_context,

s/file_context/ucontext/

+static int cron_get_job_range( user *u, security_context_t *ucontextp,
+                               char **jobenv )
+{
+	char *sroletype;

s/sroletype/range/

+	if ( (sroletype = env_get("MLS_LEVEL",jobenv)) != 0L )
+	{
+		char crontab[MAX_FNAME];
+                context_t ccon;
+
+		if ( strcmp(u->name,"*system*") == 0 )
+			strncpy(crontab, u->tabname, MAX_FNAME);
+		else
+			snprintf(crontab, MAX_FNAME, "%s/%s", CRONDIR, u->tabname);
+                
+                if (!(ccon = context_new(u->scontext)))
+                {
+			if ( security_getenforce() > 0 ) 
+			{
+				log_it(u->name, 
+				       getpid(), "context_new FAILED for SELINUX_ROLE_TYPE",

s/SELINUX_ROLE_TYPE/MLS_LEVEL/
 
+				       sroletype
+				      );
+				return -1;
+			} else
+                        {
+				log_it(u->name,
+				       getpid(), 
+				       "context_new FAILED but SELinux in permissive mode, continuing "
+				       "- SELINUX_ROLE_TYPE=", sroletype
+				       );
+				return 0;
+                        }

I wouldn't put tests of security_getenforce() on anything other than
permission denials (which avc_has_perm would do internally for you if
you were to use it instead of security_compute_av).  If context_new()
fails, it means you are out of memory, so permissive mode or not, you
are going to die.

+
+	        *ucontextp = strdup(context_str(ccon));

Needs checking of both the intermediate result (context_str return
value) and strdup to avoid seg faulting on NULL.

-- 
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] cron changes needed for MLS range checking (requires at least the libselinux patches)

[James Antill]

[-- Attachment #1.1: Type: text/plain, Size: 958 bytes --]

On Wed, 2006-11-08 at 17:13 -0500, Stephen Smalley wrote:

> Looks better.  A few nits:
> +	/*
> +	 * Since crontab files are not directly executed,
> +	 * crond must ensure that the crontab range has
> +	 * a context that is appropriate for the context of
> +	 * the user cron job.  It performs an entrypoint
> +	 * permission check for this purpose.
> 
> cut-and-paste

 I did alter it a little, but I've altered it more now :).

> I wouldn't put tests of security_getenforce() on anything other than
> permission denials

 Done.

> +
> +	        *ucontextp = strdup(context_str(ccon));
> 
> Needs checking of both the intermediate result (context_str return
> value) and strdup to avoid seg faulting on NULL.

 Ahh, I had copied the assumption that context_x() doesn't fail from
PAM ... I assumed it preallocated in context_new(). I'll fix PAM too.
 Attached is the latest cron patch.

-- 
James Antill <jantill@redhat.com>

[-- Attachment #1.2: vixie-cron-4.1-_60-SELinux-contains-range.patch --]
[-- Type: text/x-patch, Size: 7464 bytes --]

Only in vixie-cron-4.1: crond.pam.pamd_crond
diff -rup vixie-cron-4.1-orig/security.c vixie-cron-4.1/security.c
--- vixie-cron-4.1-orig/security.c	2006-11-02 22:28:04.000000000 -0500
+++ vixie-cron-4.1/security.c	2006-11-08 17:35:27.000000000 -0500
@@ -23,6 +23,7 @@
 
 #ifdef WITH_SELINUX
 #include <selinux/selinux.h>
+#include <selinux/context.h>
 #include <selinux/flask.h>
 #include <selinux/av_permissions.h>
 #include <selinux/get_context_list.h>
@@ -30,6 +31,12 @@
 
 static char ** build_env(char **cronenv);
 
+#ifdef WITH_SELINUX
+static int cron_change_selinux_range( user *u,
+                                      security_context_t ucontext );
+static int cron_get_job_range( user *u, security_context_t *ucontextp, char **jobenv );
+#endif
+
 int cron_set_job_security_context( entry *e, user *u, char ***jobenv )
 {
     time_t minutely_time = 0;
@@ -58,9 +65,9 @@ int cron_set_job_security_context( entry
      * we'll not be permitted to read the cron spool directory :-)
      */
 
-    security_context_t scontext=0, file_context=0; 
+    security_context_t ucontext=0; 
 
-    if ( cron_get_job_context(u, &scontext, &file_context, *jobenv) < OK )
+    if ( cron_get_job_range(u, &ucontext, *jobenv) < OK )
     {
 	syslog(LOG_ERR, "CRON (%s) ERROR: failed to get selinux context: %s", 
 	       e->pwd->pw_name, strerror(errno)
@@ -79,16 +86,16 @@ int cron_set_job_security_context( entry
     }	
 
 #if WITH_SELINUX
-    if ( cron_change_selinux_context( u, scontext, file_context ) != 0 )
+    if (cron_change_selinux_range(u, ucontext) != 0)
     {
         syslog(LOG_INFO,"CRON (%s) ERROR: failed to change SELinux context", 
 	       e->pwd->pw_name);
-	if ( file_context )
-		freecon(file_context);
+	if ( ucontext )
+		freecon(ucontext);
 	return -1;
     }
-    if ( file_context )
-	freecon(file_context);
+    if ( ucontext )
+	freecon(ucontext);
 #endif
 
     log_close();
@@ -201,6 +208,7 @@ cron_authorize_context
 #ifdef WITH_SELINUX
 	struct av_decision avd;
 	int retval;
+        unsigned int bit = FILE__ENTRYPOINT;
 	/*
 	 * Since crontab files are not directly executed,
 	 * crond must ensure that the crontab file has
@@ -208,13 +216,36 @@ cron_authorize_context
 	 * the user cron job.  It performs an entrypoint
 	 * permission check for this purpose.
 	 */
-	retval = security_compute_av(scontext,
-				     file_context,
-				     SECCLASS_FILE,
-				     FILE__ENTRYPOINT,
-				     &avd);
+	retval = security_compute_av(scontext, file_context,
+				     SECCLASS_FILE, bit, &avd);
+
+	if (retval || ((bit & avd.allowed) != bit))
+		return 0;
+#endif
+	return 1;
+}
+
+static int 
+cron_authorize_range
+( 
+	security_context_t scontext,
+	security_context_t ucontext
+)	
+{
+#ifdef WITH_SELINUX
+	struct av_decision avd;
+	int retval;
+        unsigned int bit = CONTEXT__CONTAINS;
+	/*
+	 * Since crontab files are not directly executed,
+	 * so crond must ensure that any user specified range
+	 * is allowed by the default users range.  It performs
+         * an entrypoint permission check for this purpose.
+	 */
+	retval = security_compute_av(scontext, ucontext,
+				     SECCLASS_CONTEXT, bit, &avd);
 
-	if (retval || ((FILE__ENTRYPOINT & avd.allowed) != FILE__ENTRYPOINT))
+	if (retval || ((bit & avd.allowed) != bit))
 		return 0;
 #endif
 	return 1;
@@ -265,6 +296,70 @@ int cron_get_job_context( user *u, void 
 	return 0;
 }
 
+#if WITH_SELINUX
+/* always uses u->scontext as the default process context, then changes the
+   level, and retuns it in ucontextp (or NULL otherwise) */
+static int cron_get_job_range( user *u, security_context_t *ucontextp,
+                               char **jobenv )
+{
+	char *range;
+
+	if ( is_selinux_enabled() <= 0 )
+		return 0;
+	if ( ucontextp == 0L )
+		return -1;
+
+	*ucontextp = 0L;
+
+	if ( (range = env_get("MLS_LEVEL",jobenv)) != 0L )
+	{
+		char crontab[MAX_FNAME];
+                context_t ccon;
+
+		if ( strcmp(u->name,"*system*") == 0 )
+			strncpy(crontab, u->tabname, MAX_FNAME);
+		else
+			snprintf(crontab, MAX_FNAME, "%s/%s", CRONDIR, u->tabname);
+                
+                if (!(ccon = context_new(u->scontext)))
+                {
+			log_it(u->name, 
+                               getpid(), "context_new FAILED for MLS_RANGE", 
+                               range);
+                        return -1;
+                }                  
+
+                if (context_range_set(ccon, range))
+                {
+                        log_it(u->name, 
+                               getpid(), "context_range_set FAILED for MLS_RANGE", 
+                               range);
+                        return -1;
+                }
+
+                if (!(*ucontext = context_str(ccon)))
+                {
+                        log_it(u->name, 
+                               getpid(), "context_str FAILED for MLS_RANGE", 
+                               range);
+                        return -1;
+                }
+
+                if (!(*ucontextp = strdup(*ucontextp)))
+                {
+                        log_it(u->name, 
+                               getpid(), "strdup FAILED for MLS_RANGE", 
+                               range);
+                        return -1;
+                }
+
+                context_free(ccon);
+	}
+
+	return 0;
+}
+#endif
+
 int cron_change_selinux_context( user *u, void *scontext, void *file_context )
 {
 #ifdef WITH_SELINUX
@@ -332,6 +427,74 @@ int cron_change_selinux_context( user *u
 	return 0;
 }
 
+#ifdef WITH_SELINUX
+static int cron_change_selinux_range( user *u,
+                                      security_context_t ucontext )
+{
+	if ( is_selinux_enabled() <= 0 )
+		return 0;
+
+	if ( u->scontext == 0L )
+	{
+		if (security_getenforce() > 0) 
+		{
+			log_it( u->name, getpid(), 
+				"NULL security context for user", 
+				""
+			      );
+			return -1;
+		}else
+		{
+			log_it( u->name, getpid(), 
+				"NULL security context for user, "
+				"but SELinux in permissive mode, continuing",
+				""
+				);
+			return 0;
+		}
+	}
+	
+	if ( ucontext && strcmp(u->scontext, ucontext) )
+	{		
+                if ( ! cron_authorize_range( u->scontext, ucontext ))
+		{
+			if ( security_getenforce() > 0 ) 
+			{
+				syslog(LOG_ERR,
+				       "CRON (%s) ERROR:"
+				       "Unauthorized exec context to SELINUX_ROLE_TYPE %s for user", 
+				       u->name, (char*)ucontext
+				      );
+				return -1;
+			} else
+			{
+				syslog(LOG_INFO,
+				       "CRON (%s) WARNING:"
+				       "Unauthorized exec context to SELINUX_ROLE_TYPE %s for user,"
+				       " but SELinux in permissive mode, continuing", 
+				       u->name, (char*)ucontext
+				      );
+			}
+		}
+	} 
+
+	if ( setexeccon(ucontext) < 0 ) 
+	{
+		if (security_getenforce() > 0) 
+		{
+			syslog(LOG_ERR,
+			       "CRON (%s) ERROR:"
+			       "Could not set exec context to %s for user", 
+			       u->name, (char*)ucontext
+			      );
+
+			return -1;
+		}
+	}
+	return 0;
+}
+#endif
+
 int get_security_context( const char *name, 
 			  int crontab_fd, 
 			  security_context_t *rcontext, 
Only in vixie-cron-4.1: security.c.security
Only in vixie-cron-4.1: security.c.selinux-contains-range

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

Re: [PATCH] cron changes needed for MLS range checking (requires at least the libselinux patches)

[Stephen Smalley]

On Wed, 2006-11-08 at 18:47 -0500, James Antill wrote:
>  Attached is the latest cron patch.

diff -rup vixie-cron-4.1-orig/security.c vixie-cron-4.1/security.c
--- vixie-cron-4.1-orig/security.c	2006-11-02 22:28:04.000000000 -0500
+++ vixie-cron-4.1/security.c	2006-11-08 17:35:27.000000000 -0500
+static int 
+cron_authorize_range
+( 
+	security_context_t scontext,
+	security_context_t ucontext
+)	
+{
+#ifdef WITH_SELINUX
+	struct av_decision avd;
+	int retval;
+        unsigned int bit = CONTEXT__CONTAINS;
+	/*
+	 * Since crontab files are not directly executed,
+	 * so crond must ensure that any user specified range
+	 * is allowed by the default users range.  It performs
+         * an entrypoint permission check for this purpose.
+	 */

Still not accurate.  This check is quite different in purpose and
rationale than the entrypoint check; it has nothing to do with the fact
that crontab files are not directly executed.  It is just a check of
whether the user-specified level falls within the seusers-specified
range for that Linux user.

+static int cron_change_selinux_range( user *u,
+                                      security_context_t ucontext )
+{
+	if ( is_selinux_enabled() <= 0 )
+		return 0;
+
+	if ( u->scontext == 0L )
+	{
+		if (security_getenforce() > 0) 
+		{
+			log_it( u->name, getpid(), 
+				"NULL security context for user", 
+				""
+			      );
+			return -1;
+		}else
+		{
+			log_it( u->name, getpid(), 
+				"NULL security context for user, "
+				"but SELinux in permissive mode, continuing",
+				""
+				);
+			return 0;
+		}

Another case where I don't understand why enforcing/permissive makes any
difference.

+	}
+	
+	if ( ucontext && strcmp(u->scontext, ucontext) )
+	{		
+                if ( ! cron_authorize_range( u->scontext, ucontext ))
+		{
+			if ( security_getenforce() > 0 ) 
+			{
+				syslog(LOG_ERR,
+				       "CRON (%s) ERROR:"
+				       "Unauthorized exec context to SELINUX_ROLE_TYPE %s for user", 
+				       u->name, (char*)ucontext
+				      );

Still refers to SELINUX_ROLE_TYPE in the log message.

+				return -1;
+			} else
+			{
+				syslog(LOG_INFO,
+				       "CRON (%s) WARNING:"
+				       "Unauthorized exec context to SELINUX_ROLE_TYPE %s for user,"
+				       " but SELinux in permissive mode, continuing", 
+				       u->name, (char*)ucontext
+				      );

Ditto.

+			}
+		}
+	} 
+
+	if ( setexeccon(ucontext) < 0 ) 
+	{
+		if (security_getenforce() > 0) 
+		{
+			syslog(LOG_ERR,
+			       "CRON (%s) ERROR:"
+			       "Could not set exec context to %s for user", 
+			       u->name, (char*)ucontext
+			      );
+
+			return -1;
+		}

Likely want to log something in the else case too so you don't just
silently proceed under crond's own context.

-- 
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] cron changes needed for MLS range checking (requires at least the libselinux patches)

[James Antill]

[-- Attachment #1.1: Type: text/plain, Size: 2811 bytes --]

On Thu, 2006-11-09 at 10:07 -0500, Stephen Smalley wrote:
> On Wed, 2006-11-08 at 18:47 -0500, James Antill wrote:
> >  Attached is the latest cron patch.
> 
> diff -rup vixie-cron-4.1-orig/security.c vixie-cron-4.1/security.c
> --- vixie-cron-4.1-orig/security.c	2006-11-02 22:28:04.000000000 -0500
> +++ vixie-cron-4.1/security.c	2006-11-08 17:35:27.000000000 -0500
> +static int 
> +cron_authorize_range
> +( 
> +	security_context_t scontext,
> +	security_context_t ucontext
> +)	
> +{
> +#ifdef WITH_SELINUX
> +	struct av_decision avd;
> +	int retval;
> +        unsigned int bit = CONTEXT__CONTAINS;
> +	/*
> +	 * Since crontab files are not directly executed,
> +	 * so crond must ensure that any user specified range
> +	 * is allowed by the default users range.  It performs
> +         * an entrypoint permission check for this purpose.
> +	 */
> 
> Still not accurate.  This check is quite different in purpose and
> rationale than the entrypoint check; it has nothing to do with the fact
> that crontab files are not directly executed.  It is just a check of
> whether the user-specified level falls within the seusers-specified
> range for that Linux user.

 Ok. I've changed the comment again.

> +static int cron_change_selinux_range( user *u,
> +                                      security_context_t ucontext )
> +{
> +	if ( is_selinux_enabled() <= 0 )
> +		return 0;
> +
> +	if ( u->scontext == 0L )
> +	{
> +		if (security_getenforce() > 0) 
> +		{
> +			log_it( u->name, getpid(), 
> +				"NULL security context for user", 
> +				""
> +			      );
> +			return -1;
> +		}else
> +		{
> +			log_it( u->name, getpid(), 
> +				"NULL security context for user, "
> +				"but SELinux in permissive mode, continuing",
> +				""
> +				);
> +			return 0;
> +		}
> 
> Another case where I don't understand why enforcing/permissive makes any
> difference.

 Because without enforcing mode we just ignore the problem and continue,
with it we error out. I think this is more of a theoretical assert type
problem anyway, but still.

> Still refers to SELINUX_ROLE_TYPE in the log message.

 Fixed.

> +	if ( setexeccon(ucontext) < 0 ) 
> +	{
> +		if (security_getenforce() > 0) 
> +		{
> +			syslog(LOG_ERR,
> +			       "CRON (%s) ERROR:"
> +			       "Could not set exec context to %s for user", 
> +			       u->name, (char*)ucontext
> +			      );
> +
> +			return -1;
> +		}
> 
> Likely want to log something in the else case too so you don't just
> silently proceed under crond's own context.

 Done.

-- 
James Antill - <james.antill@redhat.com>
setsockopt(fd, IPPROTO_TCP, TCP_CONGESTION, ...);
setsockopt(fd, IPPROTO_TCP, TCP_DEFER_ACCEPT, ...);
setsockopt(fd, SOL_SOCKET,  SO_ATTACH_FILTER, ...);


[-- Attachment #1.2: vixie-cron-4.1-_60-SELinux-contains-range.patch --]
[-- Type: text/x-patch, Size: 7696 bytes --]

Only in vixie-cron-4.1: crond.pam.pamd_crond
diff -rup vixie-cron-4.1-orig/security.c vixie-cron-4.1/security.c
--- vixie-cron-4.1-orig/security.c	2006-11-02 22:28:04.000000000 -0500
+++ vixie-cron-4.1/security.c	2006-11-09 10:38:08.000000000 -0500
@@ -23,6 +23,7 @@
 
 #ifdef WITH_SELINUX
 #include <selinux/selinux.h>
+#include <selinux/context.h>
 #include <selinux/flask.h>
 #include <selinux/av_permissions.h>
 #include <selinux/get_context_list.h>
@@ -30,6 +31,12 @@
 
 static char ** build_env(char **cronenv);
 
+#ifdef WITH_SELINUX
+static int cron_change_selinux_range( user *u,
+                                      security_context_t ucontext );
+static int cron_get_job_range( user *u, security_context_t *ucontextp, char **jobenv );
+#endif
+
 int cron_set_job_security_context( entry *e, user *u, char ***jobenv )
 {
     time_t minutely_time = 0;
@@ -58,9 +65,9 @@ int cron_set_job_security_context( entry
      * we'll not be permitted to read the cron spool directory :-)
      */
 
-    security_context_t scontext=0, file_context=0; 
+    security_context_t ucontext=0; 
 
-    if ( cron_get_job_context(u, &scontext, &file_context, *jobenv) < OK )
+    if ( cron_get_job_range(u, &ucontext, *jobenv) < OK )
     {
 	syslog(LOG_ERR, "CRON (%s) ERROR: failed to get selinux context: %s", 
 	       e->pwd->pw_name, strerror(errno)
@@ -79,16 +86,16 @@ int cron_set_job_security_context( entry
     }	
 
 #if WITH_SELINUX
-    if ( cron_change_selinux_context( u, scontext, file_context ) != 0 )
+    if (cron_change_selinux_range(u, ucontext) != 0)
     {
         syslog(LOG_INFO,"CRON (%s) ERROR: failed to change SELinux context", 
 	       e->pwd->pw_name);
-	if ( file_context )
-		freecon(file_context);
+	if ( ucontext )
+		freecon(ucontext);
 	return -1;
     }
-    if ( file_context )
-	freecon(file_context);
+    if ( ucontext )
+	freecon(ucontext);
 #endif
 
     log_close();
@@ -201,6 +208,7 @@ cron_authorize_context
 #ifdef WITH_SELINUX
 	struct av_decision avd;
 	int retval;
+        unsigned int bit = FILE__ENTRYPOINT;
 	/*
 	 * Since crontab files are not directly executed,
 	 * crond must ensure that the crontab file has
@@ -208,13 +216,35 @@ cron_authorize_context
 	 * the user cron job.  It performs an entrypoint
 	 * permission check for this purpose.
 	 */
-	retval = security_compute_av(scontext,
-				     file_context,
-				     SECCLASS_FILE,
-				     FILE__ENTRYPOINT,
-				     &avd);
+	retval = security_compute_av(scontext, file_context,
+				     SECCLASS_FILE, bit, &avd);
+
+	if (retval || ((bit & avd.allowed) != bit))
+		return 0;
+#endif
+	return 1;
+}
+
+static int 
+cron_authorize_range
+( 
+	security_context_t scontext,
+	security_context_t ucontext
+)	
+{
+#ifdef WITH_SELINUX
+	struct av_decision avd;
+	int retval;
+        unsigned int bit = CONTEXT__CONTAINS;
+	/*
+	 * Since crontab files are not directly executed,
+	 * so crond must ensure that any user specified range
+	 * falls within the seusers-specified range for that Linux user.
+	 */
+	retval = security_compute_av(scontext, ucontext,
+				     SECCLASS_CONTEXT, bit, &avd);
 
-	if (retval || ((FILE__ENTRYPOINT & avd.allowed) != FILE__ENTRYPOINT))
+	if (retval || ((bit & avd.allowed) != bit))
 		return 0;
 #endif
 	return 1;
@@ -265,6 +295,70 @@ int cron_get_job_context( user *u, void 
 	return 0;
 }
 
+#if WITH_SELINUX
+/* always uses u->scontext as the default process context, then changes the
+   level, and retuns it in ucontextp (or NULL otherwise) */
+static int cron_get_job_range( user *u, security_context_t *ucontextp,
+                               char **jobenv )
+{
+	char *range;
+
+	if ( is_selinux_enabled() <= 0 )
+		return 0;
+	if ( ucontextp == 0L )
+		return -1;
+
+	*ucontextp = 0L;
+
+	if ( (range = env_get("MLS_LEVEL",jobenv)) != 0L )
+	{
+		char crontab[MAX_FNAME];
+                context_t ccon;
+
+		if ( strcmp(u->name,"*system*") == 0 )
+			strncpy(crontab, u->tabname, MAX_FNAME);
+		else
+			snprintf(crontab, MAX_FNAME, "%s/%s", CRONDIR, u->tabname);
+                
+                if (!(ccon = context_new(u->scontext)))
+                {
+			log_it(u->name, 
+                               getpid(), "context_new FAILED for MLS_LEVEL", 
+                               range);
+                        return -1;
+                }                  
+
+                if (context_range_set(ccon, range))
+                {
+                        log_it(u->name, 
+                               getpid(), "context_range_set FAILED for MLS_LEVEL", 
+                               range);
+                        return -1;
+                }
+
+                if (!(*ucontext = context_str(ccon)))
+                {
+                        log_it(u->name, 
+                               getpid(), "context_str FAILED for MLS_LEVEL", 
+                               range);
+                        return -1;
+                }
+
+                if (!(*ucontextp = strdup(*ucontextp)))
+                {
+                        log_it(u->name, 
+                               getpid(), "strdup FAILED for MLS_LEVEL", 
+                               range);
+                        return -1;
+                }
+
+                context_free(ccon);
+	}
+
+	return 0;
+}
+#endif
+
 int cron_change_selinux_context( user *u, void *scontext, void *file_context )
 {
 #ifdef WITH_SELINUX
@@ -332,6 +426,84 @@ int cron_change_selinux_context( user *u
 	return 0;
 }
 
+#ifdef WITH_SELINUX
+static int cron_change_selinux_range( user *u,
+                                      security_context_t ucontext )
+{
+	if ( is_selinux_enabled() <= 0 )
+		return 0;
+
+	if ( u->scontext == 0L )
+	{
+		if (security_getenforce() > 0) 
+		{
+			log_it( u->name, getpid(), 
+				"NULL security context for user", 
+				""
+			      );
+			return -1;
+		}else
+		{
+			log_it( u->name, getpid(), 
+				"NULL security context for user, "
+				"but SELinux in permissive mode, continuing",
+				""
+				);
+			return 0;
+		}
+	}
+	
+	if ( ucontext && strcmp(u->scontext, ucontext) )
+	{		
+                if ( ! cron_authorize_range( u->scontext, ucontext ))
+		{
+			if ( security_getenforce() > 0 ) 
+			{
+				syslog(LOG_ERR,
+				       "CRON (%s) ERROR:"
+				       "Unauthorized range in MLS_LEVEL %s for user", 
+				       u->name, (char*)ucontext
+				      );
+				return -1;
+			} else
+			{
+				syslog(LOG_INFO,
+				       "CRON (%s) WARNING:"
+				       "Unauthorized range in MLS_LEVEL %s for user,"
+				       " but SELinux in permissive mode, continuing", 
+				       u->name, (char*)ucontext
+				      );
+			}
+		}
+	} 
+
+	if ( setexeccon(ucontext) < 0 ) 
+	{
+		if (security_getenforce() > 0) 
+		{
+			syslog(LOG_ERR,
+			       "CRON (%s) ERROR:"
+			       "Could not set exec context to %s for user", 
+			       u->name, (char*)ucontext
+			      );
+
+			return -1;
+		} else
+		{
+			syslog(LOG_ERR,
+			       "CRON (%s) ERROR:"
+			       "Could not set exec context to %s for user, "
+                               " but SELinux in permissive mode, continuing", 
+			       u->name, (char*)ucontext
+			      );
+
+			return 0;
+		}
+	}
+	return 0;
+}
+#endif
+
 int get_security_context( const char *name, 
 			  int crontab_fd, 
 			  security_context_t *rcontext, 
Only in vixie-cron-4.1: security.c~
Only in vixie-cron-4.1: security.c.security
Only in vixie-cron-4.1: security.c.selinux-contains-range

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

Re: [PATCH] cron changes needed for MLS range checking (requires at least the libselinux patches)

[Stephen Smalley]

On Thu, 2006-11-09 at 10:40 -0500, James Antill wrote:
>  Because without enforcing mode we just ignore the problem and continue,
> with it we error out. I think this is more of a theoretical assert type
> problem anyway, but still.

That's my point - it seems like it is a bug regardless of whether we are
permissive or enforcing, and should thus always return -1.  I'd only
expect security_getenforce() to make a difference for error handling on
permission checks.

Anyway, the patch looks sane at this point, although I'm not completely
clear how it integrates into the existing pile of selinux-related
patches in vixie-cron (it would help to consolidate them).

What is your plan on the client (crontab program) side?  The old patch
instrumented it to automatically insert a SELINUX_ROLE_TYPE= definition
with the caller's context if a certain option was used to crontab; will
you replace that with your new MLS_LEVEL= definition and the caller's
current range or just drop it altogether and require the user to
manually specify it in the crontab file?  Am I correct in understanding
that there can only be one MLS_LEVEL= definition per crontab file (for
all cron jobs in that crontab)?  Can it go anywhere in the crontab file?

-- 
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] cron changes needed for MLS range checking (requires at least the libselinux patches)

[James Antill]

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

On Thu, 2006-11-09 at 10:57 -0500, Stephen Smalley wrote:
> On Thu, 2006-11-09 at 10:40 -0500, James Antill wrote:
> >  Because without enforcing mode we just ignore the problem and continue,
> > with it we error out. I think this is more of a theoretical assert type
> > problem anyway, but still.
> 
> That's my point - it seems like it is a bug regardless of whether we are
> permissive or enforcing, and should thus always return -1.  I'd only
> expect security_getenforce() to make a difference for error handling on
> permission checks.

 Well get_security_context() does the same thing if fgetfilecon(),
getseuserbyname()/get_default_context_with_level() or
cron_authorize_context() fail (which would lead to u->scontext being
NULL, AIUI), so I really wouldn't want to change it unless all those
changed in some way.

> Anyway, the patch looks sane at this point, although I'm not completely
> clear how it integrates into the existing pile of selinux-related
> patches in vixie-cron (it would help to consolidate them).

 I can't really do that, easily.

> What is your plan on the client (crontab program) side?  The old patch
> instrumented it to automatically insert a SELINUX_ROLE_TYPE= definition
> with the caller's context if a certain option was used to crontab; will
> you replace that with your new MLS_LEVEL= definition and the caller's
> current range or just drop it altogether and require the user to
> manually specify it in the crontab file?

 Atm. I've got a patch which changes the crontab command to only add the
level when -s is specified.

>   Am I correct in understanding
> that there can only be one MLS_LEVEL= definition per crontab file (for
> all cron jobs in that crontab)?

 Yes.

>   Can it go anywhere in the crontab file?

 Yes.

-- 
James Antill - <james.antill@redhat.com>
setsockopt(fd, IPPROTO_TCP, TCP_CONGESTION, ...);
setsockopt(fd, IPPROTO_TCP, TCP_DEFER_ACCEPT, ...);
setsockopt(fd, SOL_SOCKET,  SO_ATTACH_FILTER, ...);


[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 189 bytes --]