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