[PATCH] sys_chroot() hook for additional chroot() jails enforcing

[PATCH] sys_chroot() hook for additional chroot() jails enforcing

[Lorenzo Hernández García-Hierro]

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

Hi,

Attached you can find a patch which adds a new hook for the sys_chroot()
syscall, and makes us able to add additional enforcing and security
checks by using the Linux Security Modules framework (ie. chdir
enforcing, etc).

Current user of the hook is the forthcoming 0.2 revision of vSecurity.

With it, and used within an LSM module, we can achieve the goal of
enforcing and apply some hardening to the sys_chroot() syscall.
Even if chroot jails are broken by design, in terms of security, with a
few changes to their base and some syscalls that it relies with, we can
achieve the goal of preventing some of the already known attacks against
them.

I will make available some patches for other syscalls as well
(sys_fchmod(), sys_chmod(), ...), that will add a few more hooks to the
LSM framework, in the hope that they will be useful.

The patch can be retrieved too from:
http://pearls.tuxedo-es.org/patches/sys_chroot_lsm-hook-2.6.11-rc3.patch

Thanks in advance, and, again, I will appreciate any suggestions on
which hooks are good candidates to be added.
Feel free to edit tuxedo-es.org wiki at http://wiki.tuxedo-es.org/LSM
and put suggestions & comments there.

Cheers,
-- 
Lorenzo Hernández García-Hierro <lorenzo@gnu.org> 
[1024D/6F2B2DEC] & [2048g/9AE91A22][http://tuxedo-es.org]

[-- Attachment #1.2: sys_chroot_lsm-hook-2.6.11-rc3.patch --]
[-- Type: text/x-patch, Size: 2828 bytes --]

diff -Nur linux-2.6.11-rc3/fs/open.c linux-2.6.11-rc3.chroot-lsm/fs/open.c
--- linux-2.6.11-rc3/fs/open.c	2005-02-06 21:40:40.000000000 +0100
+++ linux-2.6.11-rc3.chroot-lsm/fs/open.c	2005-02-07 21:42:45.000000000 +0100
@@ -582,6 +582,10 @@
 	error = -EPERM;
 	if (!capable(CAP_SYS_CHROOT))
 		goto dput_and_out;
+		
+	error = security_chroot(&nd);
+	if (error)
+		goto dput_and_out;
 
 	set_fs_root(current->fs, nd.mnt, nd.dentry);
 	set_fs_altroot();
diff -Nur linux-2.6.11-rc3/include/linux/security.h linux-2.6.11-rc3.chroot-lsm/include/linux/security.h
--- linux-2.6.11-rc3/include/linux/security.h	2005-02-06 21:40:27.000000000 +0100
+++ linux-2.6.11-rc3.chroot-lsm/include/linux/security.h	2005-02-07 21:10:05.000000000 +0100
@@ -1008,6 +1008,10 @@
  *	@ts contains new time
  *	@tz contains new timezone
  *	Return 0 if permission is granted.
+ * @chroot:
+ *	Check permission to change the current root by sys_chroot() syscall.
+ *	@nd contains the nameidata struct passed by sys_chroot()
+ *	Return 0 if permission is granted.
  * @vm_enough_memory:
  *	Check permissions for allocating a new virtual mapping.
  *      @pages contains the number of pages.
@@ -1040,6 +1044,7 @@
 	int (*acct) (struct file * file);
 	int (*sysctl) (struct ctl_table * table, int op);
 	int (*capable) (struct task_struct * tsk, int cap);
+	int (*chroot) (struct nameidata * nd);
 	int (*quotactl) (int cmds, int type, int id, struct super_block * sb);
 	int (*quota_on) (struct dentry * dentry);
 	int (*syslog) (int type);
@@ -1304,6 +1309,10 @@
 	return security_ops->settime(ts, tz);
 }
 
+static inline int security_chroot(struct nameidata *nd)
+{
+	return security_ops->chroot(nd);
+}
 
 static inline int security_vm_enough_memory(long pages)
 {
@@ -1986,6 +1995,11 @@
 	return cap_settime(ts, tz);
 }
 
+static inline int security_chroot(struct nameidata *nd)
+{
+	return 0;
+}
+
 static inline int security_vm_enough_memory(long pages)
 {
 	return cap_vm_enough_memory(pages);
diff -Nur linux-2.6.11-rc3/security/dummy.c linux-2.6.11-rc3.chroot-lsm/security/dummy.c
--- linux-2.6.11-rc3/security/dummy.c	2005-02-06 21:40:57.000000000 +0100
+++ linux-2.6.11-rc3.chroot-lsm/security/dummy.c	2005-02-07 21:12:01.000000000 +0100
@@ -101,6 +101,11 @@
 	return 0;
 }
 
+static int dummy_chroot(struct nameidata *nd)
+{
+	return 0;
+}
+
 static int dummy_settime(struct timespec *ts, struct timezone *tz)
 {
 	if (!capable(CAP_SYS_TIME))
@@ -858,6 +863,7 @@
 	set_to_dummy_if_null(ops, sysctl);
 	set_to_dummy_if_null(ops, syslog);
 	set_to_dummy_if_null(ops, settime);
+	set_to_dummy_if_null(ops, chroot);
 	set_to_dummy_if_null(ops, vm_enough_memory);
 	set_to_dummy_if_null(ops, bprm_alloc_security);
 	set_to_dummy_if_null(ops, bprm_free_security);

[-- Attachment #2: Esta parte del mensaje está firmada digitalmente --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

Re: [PATCH] sys_chroot() hook for additional chroot() jails enforcing

[Chris Wright]

* Lorenzo Hernández García-Hierro (lorenzo@gnu.org) wrote:
> Attached you can find a patch which adds a new hook for the sys_chroot()
> syscall, and makes us able to add additional enforcing and security
> checks by using the Linux Security Modules framework (ie. chdir
> enforcing, etc).

If you want to make a change like this, collapse the
capable(CAP_SYS_CHROOT) check behind this hook, no point having two
outcalls from same call site.  What logic do you expect to put behind
the chroot() hook?

thanks,
-chris
-- 
Linux Security Modules     http://lsm.immunix.org     http://lsm.bkbits.net

Re: [PATCH] sys_chroot() hook for additional chroot() jails enforcing

[Serge E. Hallyn]

Hi,

If I understood you correct earlier, the only policy you needed to
enforce was to prevent double-chrooting.  If that is the case, why is it
not sufficient to keep a "process-has-used-chroot" flag in
current->security which is set on the first call to
capable(CAP_SYS_CHROOT) and inherited by forked children, after which
calls to capable(CAP_SYS_CHROOT) are refused?

Of course if you need to do more, then a hook might be necessary.

-serge

Re: [PATCH] sys_chroot() hook for additional chroot() jails enforcing

[Lorenzo Hernández García-Hierro]

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

El lun, 07-02-2005 a las 16:50 -0600, Serge E. Hallyn escribió:
> Hi,
> 
> If I understood you correct earlier, the only policy you needed to
> enforce was to prevent double-chrooting.  If that is the case, why is it
> not sufficient to keep a "process-has-used-chroot" flag in
> current->security which is set on the first call to
> capable(CAP_SYS_CHROOT) and inherited by forked children, after which
> calls to capable(CAP_SYS_CHROOT) are refused?
> 
> Of course if you need to do more, then a hook might be necessary.

Yeah, checking that process is chrooted using the current macro and
denying if capable() gets it trying to access CAP_SYS_CHROOT it's the
way that vSecurity currently does it.

But the hook will have to handle some chdir enforcing that can't be done
with current hooks, I will explain it further tomorrow.

It's too late here ;)

Cheers,
-- 
Lorenzo Hernández García-Hierro <lorenzo@gnu.org> 
[1024D/6F2B2DEC] & [2048g/9AE91A22][http://tuxedo-es.org]

[-- Attachment #2: Esta parte del mensaje está firmada digitalmente --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

Re: [PATCH] sys_chroot() hook for additional chroot() jails enforcing

[Lorenzo Hernández García-Hierro]

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

El lun, 07-02-2005 a las 14:34 -0800, Chris Wright escribió:
> * Lorenzo Hernández García-Hierro (lorenzo@gnu.org) wrote:
> > Attached you can find a patch which adds a new hook for the sys_chroot()
> > syscall, and makes us able to add additional enforcing and security
> > checks by using the Linux Security Modules framework (ie. chdir
> > enforcing, etc).
> 
> If you want to make a change like this, collapse the
> capable(CAP_SYS_CHROOT) check behind this hook, no point having two
> outcalls from same call site.

Right, did it.
New patch attached and also available at:
http://pearls.tuxedo-es.org/patches/sys_chroot_lsm-hook-2.6.11-rc3.patch

>   What logic do you expect to put behind
> the chroot() hook?

For example a chdir() handling function as grsec does, and also any
other check that comes up to mind.

Cheers and again thanks for the comments,
-- 
Lorenzo Hernández García-Hierro <lorenzo@gnu.org> 
[1024D/6F2B2DEC] & [2048g/9AE91A22][http://tuxedo-es.org]

[-- Attachment #1.2: sys_chroot_lsm-hook-2.6.11-rc3.patch --]
[-- Type: text/x-patch, Size: 2991 bytes --]

diff -Nur linux-2.6.11-rc3/fs/open.c linux-2.6.11-rc3.chroot-lsm/fs/open.c
--- linux-2.6.11-rc3/fs/open.c	2005-02-06 21:40:40.000000000 +0100
+++ linux-2.6.11-rc3.chroot-lsm/fs/open.c	2005-02-08 15:29:40.544611912 +0100
@@ -578,9 +578,9 @@
 	error = permission(nd.dentry->d_inode,MAY_EXEC,&nd);
 	if (error)
 		goto dput_and_out;
-
-	error = -EPERM;
-	if (!capable(CAP_SYS_CHROOT))
+		
+	error = security_chroot(&nd);
+	if (error)
 		goto dput_and_out;
 
 	set_fs_root(current->fs, nd.mnt, nd.dentry);
diff -Nur linux-2.6.11-rc3/include/linux/security.h linux-2.6.11-rc3.chroot-lsm/include/linux/security.h
--- linux-2.6.11-rc3/include/linux/security.h	2005-02-06 21:40:27.000000000 +0100
+++ linux-2.6.11-rc3.chroot-lsm/include/linux/security.h	2005-02-08 15:30:54.434378960 +0100
@@ -1008,6 +1008,10 @@
  *	@ts contains new time
  *	@tz contains new timezone
  *	Return 0 if permission is granted.
+ * @chroot:
+ *	Check permission to change the current root by sys_chroot() syscall.
+ *	@nd contains the nameidata struct passed by sys_chroot()
+ *	Return 0 if permission is granted.
  * @vm_enough_memory:
  *	Check permissions for allocating a new virtual mapping.
  *      @pages contains the number of pages.
@@ -1040,6 +1044,7 @@
 	int (*acct) (struct file * file);
 	int (*sysctl) (struct ctl_table * table, int op);
 	int (*capable) (struct task_struct * tsk, int cap);
+	int (*chroot) (struct nameidata * nd);
 	int (*quotactl) (int cmds, int type, int id, struct super_block * sb);
 	int (*quota_on) (struct dentry * dentry);
 	int (*syslog) (int type);
@@ -1304,6 +1309,10 @@
 	return security_ops->settime(ts, tz);
 }
 
+static inline int security_chroot(struct nameidata *nd)
+{
+	return security_ops->chroot(nd);
+}
 
 static inline int security_vm_enough_memory(long pages)
 {
@@ -1986,6 +1995,14 @@
 	return cap_settime(ts, tz);
 }
 
+static inline int security_chroot(struct nameidata *nd)
+{
+	if (!capable(CAP_SYS_CHROOT))
+		return -EPERM;
+	
+	return 0;
+}
+
 static inline int security_vm_enough_memory(long pages)
 {
 	return cap_vm_enough_memory(pages);
diff -Nur linux-2.6.11-rc3/security/dummy.c linux-2.6.11-rc3.chroot-lsm/security/dummy.c
--- linux-2.6.11-rc3/security/dummy.c	2005-02-06 21:40:57.000000000 +0100
+++ linux-2.6.11-rc3.chroot-lsm/security/dummy.c	2005-02-08 15:29:55.034409128 +0100
@@ -101,6 +101,14 @@
 	return 0;
 }
 
+static int dummy_chroot(struct nameidata *nd)
+{
+	if (!capable(CAP_SYS_CHROOT))
+		return -EPERM;
+	
+	return 0;
+}
+
 static int dummy_settime(struct timespec *ts, struct timezone *tz)
 {
 	if (!capable(CAP_SYS_TIME))
@@ -858,6 +866,7 @@
 	set_to_dummy_if_null(ops, sysctl);
 	set_to_dummy_if_null(ops, syslog);
 	set_to_dummy_if_null(ops, settime);
+	set_to_dummy_if_null(ops, chroot);
 	set_to_dummy_if_null(ops, vm_enough_memory);
 	set_to_dummy_if_null(ops, bprm_alloc_security);
 	set_to_dummy_if_null(ops, bprm_free_security);

[-- Attachment #2: Esta parte del mensaje está firmada digitalmente --]
[-- Type: application/pgp-signature, Size: 189 bytes --]