[RFC][PATCH][CVS] chroot jailing support

Henrique de Moraes Holschuh hmh at debian.org
Sun Dec 29 21:52:20 EST 2002


The attached patch adds chroot() jailing support to Cyrus master.

Here's the catch:
 1. If no jail= parameters are set, cyrus behaves exactly as it always did
 2. If jail= parameters are set, we NEED to keep superuser privileges
    so that we can chroot(). That means:
 2a.  If your system has seteuid, Cyrus uses that to be able to
      go forth from user Cyrus to user root, and back.
 2b.  If your system does not have seteuid, master must run as root.

This thing has not been heavily tested, but it seems to work, and it should
not diminish the security of Cyrus at all if it is not being used.

OTOH, a system with jails (and seteuid) has a slightly less secure cyrus
master process (because an attacker that compromises it can seteuid(0) to
become root again, depending on the type of the exploit), and more secure
services (those things master starts inside jails ;-) ).

The codepaths in master are MUCH easier to audit, so I think it overall
enhances the security of Cyrus to run services inside chroot jails. IF it is
done right.

Any comments?  Should I submit this to CMU for inclusion on Cyrus eventually
(if they like it)?

-- 
  "One disk to rule them all, One disk to find them. One disk to bring
  them all and in the darkness grind them. In the Land of Redmond
  where the shadows lie." -- The Silicon Valley Tarot
  Henrique Holschuh
-------------- next part --------------
diff -ru cyrus-imapd.orig/configure.in cyrus-imapd/configure.in
--- cyrus-imapd.orig/configure.in	2002-12-19 15:41:17.000000000 -0200
+++ cyrus-imapd/configure.in	2002-12-30 00:35:32.000000000 -0200
@@ -241,6 +241,10 @@
 dnl for detaching terminal
 AC_CHECK_FUNCS(daemon setsid)
 
+dnl for improved chroot jailing
+AC_CHECK_FUNCS(seteuid)
+AC_CHECK_FUNCS(setegid)
+
 AC_EGREP_HEADER(socklen_t, sys/socket.h, AC_DEFINE(HAVE_SOCKLEN_T))
 AC_EGREP_HEADER(rlim_t, sys/resource.h, AC_DEFINE(HAVE_RLIM_T))
 
diff -ru cyrus-imapd.orig/man/cyrus.conf.5 cyrus-imapd/man/cyrus.conf.5
--- cyrus-imapd.orig/man/cyrus.conf.5	2002-10-04 10:26:50.000000000 -0300
+++ cyrus-imapd/man/cyrus.conf.5	2002-12-30 00:35:32.000000000 -0200
@@ -119,6 +119,11 @@
 .IP "\fBmaxchild=\fR-1" 5
 The maximum number of instances of this service to spawn.  A value of
 -1 means unlimited.  This integer value is optional.
+.IP "\fBjail=\fR<no default>" 5
+Runs the service caged inside a chroot jail at the specified directory.  This
+means we cannot fully drop root privileges early, since they are needed for
+entering the jail.  All jailed services run with root priviledges fully
+dropped.
 .SS EVENTS
 This section lists processes that should be run at specific intervals,
 similar to cron jobs.  This section is typically used to perform
diff -ru cyrus-imapd.orig/master/master.c cyrus-imapd/master/master.c
--- cyrus-imapd.orig/master/master.c	2002-12-26 20:20:49.000000000 -0200
+++ cyrus-imapd/master/master.c	2002-12-30 00:40:19.000000000 -0200
@@ -81,6 +81,13 @@
 #define INADDR_ANY 0x00000000
 #endif
 
+#ifndef uid_t
+#define uid_t int
+#endif
+#ifndef gid_t
+#define gid_t int
+#endif
+
 #ifdef HAVE_UCDSNMP
 #include <ucd-snmp/ucd-snmp-config.h>
 #include <ucd-snmp/ucd-snmp-includes.h>
@@ -97,11 +104,11 @@
 #include "lock.h"
 
 enum {
-    become_cyrus_early = 1,
     child_table_size = 10000,
     child_table_inc = 100
 };
 
+static int become_cyrus_early = 1;
 static int verbose = 0;
 static int listen_queue_backlog = 32;
 static int pidfd = -1;
@@ -148,38 +155,75 @@
     exit(code);
 }
 
-int become_cyrus(void)
+int become_cyrus(const char* jail, int reversible)
 {
+    /* Encapsulates all chroot and set*id mechanics
+         jail - request non-reversible setuid AND chroot
+         reversible - request reversible seteuid
+       Non-reversible setgid is performed, in any case */
     struct passwd *p;
-    int newuid, newgid;
+    uid_t newuid;
+    gid_t newgid;
     int result;
-    static int uid = 0;
+    static uid_t uid = 0;
 
-    if (uid) return setuid(uid);
+    if (jail) reversible = 0;
 
-    p = getpwnam(CYRUS_USER);
-    if (p == NULL) {
-	syslog(LOG_ERR, "no entry in /etc/passwd for user %s", CYRUS_USER);
-	return -1;
+    if (uid && !jail) {
+#ifdef HAVE_SETEUID
+	if (reversible) return seteuid(uid);
+	if (seteuid(0)) return -1;
+#endif
+	return setuid(uid);
     }
 
-    /* Save these in case initgroups does a getpw*() */
-    newuid = p->pw_uid;
-    newgid = p->pw_gid;
+    if (uid) newuid = uid;
+    else {
+	p = getpwnam(CYRUS_USER);
+	if (p == NULL) {
+	    syslog(LOG_ERR, "no entry in /etc/passwd for user %s", CYRUS_USER);
+	    return -1;
+	}
+
+	/* Save these in case initgroups does a getpw*() */
+	newuid = p->pw_uid;
+	newgid = p->pw_gid;
 
-    if (initgroups(CYRUS_USER, newgid)) {
-        syslog(LOG_ERR, "unable to initialize groups for user %s: %s",
-	       CYRUS_USER, strerror(errno));
-        return -1;
+	if (initgroups(CYRUS_USER, newgid)) {
+            syslog(LOG_ERR, "unable to initialize groups for user %s: %m",
+		   CYRUS_USER);
+	    return -1;
+	}
+
+	if (setgid(newgid)) {
+	    syslog(LOG_ERR, "unable to set group id to %d for user %s: %m",
+	           newgid, CYRUS_USER);
+	    return -1;
+	}
     }
 
-    if (setgid(newgid)) {
-        syslog(LOG_ERR, "unable to set group id to %d for user %s: %s",
-              newgid, CYRUS_USER, strerror(errno));
-        return -1;
+#ifdef HAVE_SETEUID
+    if (!reversible && !geteuid()) seteuid(0);
+#endif
+
+    if (jail) {
+	/* enter chroot jail. We must be the superuser to do this */
+	if (chroot(jail)) {
+	    syslog(LOG_ERR, "unable to enter chroot jail %s: %m", jail);
+	    return -1;
+	}
+	if (chdir("/")) {
+	    syslog(LOG_ERR, "unable to chdir / inside jail: %m");
+	    return -1;
+	}
     }
 
+#ifdef HAVE_SETEUID
+    if (reversible) result = seteuid(newuid);
+    else result = setuid(newuid);
+#else
     result = setuid(newuid);
+#endif
 
     /* Only set static uid if successful, else future calls won't reset gid */
     if (result == 0)
@@ -447,7 +491,7 @@
 	/* Child - Release our pidfile lock. */
 	if(pidfd != -1) close(pidfd);
 
-	if (become_cyrus() != 0) {
+	if (become_cyrus(NULL, 0) != 0) {
 	    syslog(LOG_ERR, "can't change to the cyrus user");
 	    exit(1);
 	}
@@ -550,7 +594,7 @@
 	/* Child - Release our pidfile lock. */
 	if(pidfd != -1) close(pidfd);
 
-	if (become_cyrus() != 0) {
+	if (become_cyrus(s->jail, 0) != 0) {
 	    syslog(LOG_ERR, "can't change to the cyrus user");
 	    exit(1);
 	}
@@ -661,7 +705,7 @@
 		/* Child - Release our pidfile lock. */
 		if(pidfd != -1) close(pidfd);
 
-		if (become_cyrus() != 0) {
+		if (become_cyrus(NULL, 0) != 0) {
 		    syslog(LOG_ERR, "can't change to the cyrus user");
 		    exit(1);
 		}
@@ -913,6 +957,7 @@
 
 void add_service(const char *name, struct entry *e, void *rock)
 {
+    /* FIXME: this thing leaks memory if it fails to add a service, and during reconfig */
     int ignore_err = (int) rock;
     char *cmd = mystrdup(masterconf_getstring(e, "cmd", NULL));
     int prefork = masterconf_getint(e, "prefork", 0);
@@ -921,6 +966,7 @@
     char *listen = mystrdup(masterconf_getstring(e, "listen", NULL));
     char *proto = mystrdup(masterconf_getstring(e, "proto", "tcp"));
     char *max = mystrdup(masterconf_getstring(e, "maxchild", "-1"));
+    char *jail = mystrdup(masterconf_getstring(e, "jail", NULL));
     int i;
 
     if(babysit && prefork == 0) prefork = 1;
@@ -967,6 +1013,8 @@
 
 	Services[i].maxforkrate = maxforkrate;
 
+	Services[i].jail = jail;
+
 	if (!strcmp(Services[i].proto, "tcp")) {
 	    Services[i].desired_workers = prefork;
 	    Services[i].babysit = babysit;
@@ -1011,6 +1059,8 @@
 
 	Services[nservices].maxforkrate = maxforkrate;
 
+	Services[nservices].jail = jail;
+
 	if(!strcmp(Services[nservices].proto, "tcp")) {
 	    Services[nservices].desired_workers = prefork;
 	    Services[nservices].babysit = babysit;
@@ -1155,6 +1205,12 @@
     masterconf_getsection("SERVICES", &add_service, (void*) 1);
 
     for (i = 0; i < nservices; i++) {
+	if (become_cyrus_early && Services[i].jail) {
+		syslog(LOG_WARNING, "WARNING: too late to jail service %s",
+		       Services[i].name);
+		free(Services[i].jail);
+		Services[i].jail = NULL;
+	}
 	if (!Services[i].exec && Services[i].socket) {
 	    /* cleanup newly disabled services */
 
@@ -1448,7 +1504,22 @@
 		   Services[i].stat[0], Services[i].stat[1]);
     }
 
-    if (become_cyrus_early) become_cyrus();
+    if (become_cyrus_early) {
+	/* Will we need to chroot later? */
+	for (i = 0; i < nservices; i++)
+	if (Services[i].jail) willjail = 1;
+#ifndef HAVE_SETEUID
+	if (willjail) become_cyrus_early = 0;
+#endif
+	if (become_cyrus_early && become_cyrus(NULL, willjail) != 0) {
+	    syslog(LOG_ERR, "can't change to the cyrus user: %m");
+	    exit(1);
+	}
+#ifdef HAVE_SETEUID
+	/* We can still chroot, tell rest of the code so */
+	if (willjail) become_cyrus_early = 0;
+#endif
+    }
 
     /* ok, we're going to start spawning like mad now */
     syslog(LOG_NOTICE, "ready for work");
diff -ru cyrus-imapd.orig/master/master.h cyrus-imapd/master/master.h
--- cyrus-imapd.orig/master/master.h	2002-05-26 11:30:51.000000000 -0300
+++ cyrus-imapd/master/master.h	2002-12-30 00:35:32.000000000 -0200
@@ -11,6 +11,7 @@
     char *const *exec;
     int babysit;
     unsigned int maxforkrate;
+    char *jail;
     
     int socket;
     struct sockaddr *saddr;


More information about the Info-cyrus mailing list