[redhat-lspp] [RFC] [PATCH] extending cron subsys

Stephen Smalley sds at tycho.nsa.gov
Fri Dec 16 18:11:04 UTC 2005


On Tue, 2005-11-08 at 12:06 -0500, Janak Desai wrote:
> These patches (fc4 and fc5 versions) extend the cron subsystem
> to allow a user to submit cron jobs from different security contexts,
> and have the cron daemon execute them in the context from which they
> were submitted.

Hi,

Not sure if you've made further changes to these patches, but FWIW...

> Currently, a user's crontab file is stored in the /var/spool/cron
> directory as the user's name. With this patch a user can execute
> crontab with a new option '-c' to append the current security
> context to the crontab file name. The cron daemon, as part of
> processing cron jobs, will attempt to set the security context
> of the job to the context appended to the user name (if -c was
> used with crontab). If the crontab file does not contain a
> security context in its name (crontab without -c), the cron daemon
> will continue to operate as it does now (use get_default_context_*
> to obtain cron job's security context) for that perticular job.

So to be clear: we are still using the existing SELinux entrypoint
permission check by crond to ensure that commands are not run from a
maliciously fabricated crontab file (which may now also include a
fabricated context in as its name suffix) in a privileged context,
right?

> Similarly, admins can append security contexts to files in
> {hourly,daily,monthly} directories if they would like those jobs
> to execute with a certain security contexts.

Is that really true?  It seems like your patch is based on crontab file
name, which would be /etc/crontab in the system case, not the
individual /etc/cron.{hourly,daily,monthly}/* scripts that are run as
jobs by that crontab file.

> So far, I haven't made any changes to cron "allow/deny" logic.
> Which means an admin will have to explicity provide security
> context in addition to user name to allow/deny cron capability.
> Does it make more sense to keep allow/deny logic granular to just
> a user or to a user+context? That is, if users are allowed/denied
> to submit cron jobs, they can/can't do that from any contexts
> to which they have access.

Not certain myself, but you could provide both capabilities as config
options.

> I haven't made changes to man pages, but will do so once I get
> feedback on the patch itself.

A few comments below.  However, I strongly recommend that you get this
patch reviewed by actual cron maintainers.  We received a lot of good
feedback on the original vixie cron SELinux patch from the Debian cron
maintainer that led to a number of changes in it, IIRC.

<snip> 
diff -Naurp vixie-cron-4.1/crontab.c mult/crontab.c
--- vixie-cron-4.1/crontab.c	2005-11-08 15:58:56.000000000 +0000
+++ mult/crontab.c	2005-11-07 22:31:35.000000000 +0000,
@@ -157,6 +184,29 @@ parse_args(int argc, char *argv[]) {
 				usage("bad debug option");
 			break;
 #endif
+#ifdef WITH_SELINUX
+		case 'c':
+			if (is_selinux_enabled() > 0) {
+				if (getcon(&context)) {
+					fprintf(stderr,
+					   "Cannot obtain process context\n");
+					exit(ERROR_EXIT);
+				}

This needs to be getprevcon(), not getcon(), IIUC.  Under strict policy,
crontab runs in its own domain separate from the user's domain, whereas
you want to capture the caller's security context (not crontab's own
security context). 

diff -Naurp vixie-cron-4.1/database.c mult/database.c
--- vixie-cron-4.1/database.c	2005-11-08 15:58:57.000000000 +0000
+++ mult/database.c	2005-11-07 22:53:40.000000000 +0000
@@ -246,46 +262,46 @@ process_crontab(const char *uname, const
 	} else if ((pw = getpwnam(uname)) == NULL) {
 		/* file doesn't have a user in passwd file.
 		 */
-		log_it(fname, getpid(), "ORPHAN", "no passwd entry");
+		log_it(uname, getpid(), "ORPHAN", "no passwd entry");
 		goto next_crontab;
 	}

Why not leave the log_it() calls alone and thus get the full context
information included in the log entries? 
 
diff -Naurp vixie-cron-4.1/user.c mult/user.c
--- vixie-cron-4.1/user.c	2005-11-08 15:58:57.000000000 +0000
+++ mult/user.c	2005-11-08 14:48:46.000000000 +0000
@@ -47,8 +47,14 @@ static	int get_security_context(const ch
 	char *seuser=NULL;
 	char *level=NULL;
 	*rcontext = NULL;
+	char *tptr;
 
-	if (getseuserbyname(name, &seuser, &level) == 0) {
+	tptr = strchr(tabname, ':');
+	if (tptr) {
+		tptr++;
+		scontext = (security_context_t) strdup(tptr);

strdup can fail.

-- 
Stephen Smalley
National Security Agency




More information about the redhat-lspp mailing list