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

[James Antill <jantill@redhat.com>]

[-- 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 --]