[PATCH] New sys_chmod() hook for the LSM framework

[PATCH] New sys_chmod() hook for the LSM framework

[Lorenzo Hernández García-Hierro]

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

Hi,

As commented yesterday, I was going to release a few more hooks for some
*critical* syscalls, this one adds a hook to sys_chmod(), and makes us
able to apply checks and logics before releasing the operation to
sys_chmod().

The main goal is to provide a simple way to handle chmod() calls and
apply security restrictions & checks to them, and also add add auditing
capabilities (ie.: log chmod() calls in chroot()'ed environments, etc).

Patch attached and available at:
http://pearls.tuxedo-es.org/patches/sys_chmod_lsm-hook-2.6.11-rc3.patch

I would like to see this merged, Chris should decide :)

An user of this will be, as commented in my past emails, vSecurity 0.2
release, and any other LSM module that wants to have control over
chmod()'ing.

I will make available another hook for sys_fchmod() ASAP.

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

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

diff -Nur linux-2.6.11-rc3/fs/open.c linux-2.6.11-rc3.chm/fs/open.c
--- linux-2.6.11-rc3/fs/open.c	2005-02-06 21:40:40.000000000 +0100
+++ linux-2.6.11-rc3.chm/fs/open.c	2005-02-08 16:10:09.901293560 +0100
@@ -650,6 +650,11 @@
 	down(&inode->i_sem);
 	if (mode == (mode_t) -1)
 		mode = inode->i_mode;
+		
+	error = security_chmod(&nd, inode, mode);
+	if (error)
+		goto dput_and_out;	
+	
 	newattrs.ia_mode = (mode & S_IALLUGO) | (inode->i_mode & ~S_IALLUGO);
 	newattrs.ia_valid = ATTR_MODE | ATTR_CTIME;
 	error = notify_change(nd.dentry, &newattrs);
diff -Nur linux-2.6.11-rc3/include/linux/security.h linux-2.6.11-rc3.chm/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.chm/include/linux/security.h	2005-02-08 16:10:37.670072064 +0100
@@ -1008,6 +1008,12 @@
  *	@ts contains new time
  *	@tz contains new timezone
  *	Return 0 if permission is granted.
+ * @chmod:
+ *	Check permission before changing file modes by sys_chmod().
+ *	@nd contains the nameidata struct.
+ *	@inode contains the inode struct.
+ *	@mode contains the mode value.
+ *	Return 0 if permission is granted.
  * @vm_enough_memory:
  *	Check permissions for allocating a new virtual mapping.
  *      @pages contains the number of pages.
@@ -1044,6 +1050,7 @@
 	int (*quota_on) (struct dentry * dentry);
 	int (*syslog) (int type);
 	int (*settime) (struct timespec *ts, struct timezone *tz);
+	int (*chmod) (struct nameidata *nd, struct inode *inode, mode_t mode);
 	int (*vm_enough_memory) (long pages);
 
 	int (*bprm_alloc_security) (struct linux_binprm * bprm);
@@ -1304,6 +1311,10 @@
 	return security_ops->settime(ts, tz);
 }
 
+static inline int security_chmod(struct nameidata *nd, struct inode *inode, mode_t mode)
+{
+	return security_ops->chmod(nd, inode, mode);
+}
 
 static inline int security_vm_enough_memory(long pages)
 {
@@ -1986,6 +1997,11 @@
 	return cap_settime(ts, tz);
 }
 
+static inline int security_chmod(struct nameidata *nd, struct inode *inode, mode_t mode)
+{
+	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.chm/security/dummy.c
--- linux-2.6.11-rc3/security/dummy.c	2005-02-06 21:40:57.000000000 +0100
+++ linux-2.6.11-rc3.chm/security/dummy.c	2005-02-08 15:58:26.000000000 +0100
@@ -108,6 +108,11 @@
 	return 0;
 }
 
+static int dummy_chmod(struct nameidata *nd, struct inode *inode, mode_t mode)
+{
+	return 0;
+}
+
 static int dummy_vm_enough_memory(long pages)
 {
 	int cap_sys_admin = 0;
@@ -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, chmod);
 	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] New sys_chmod() hook for the LSM framework

[Chris Wright]

* Lorenzo Hernández García-Hierro (lorenzo@gnu.org) wrote:
> As commented yesterday, I was going to release a few more hooks for some
> *critical* syscalls, this one adds a hook to sys_chmod(), and makes us
> able to apply checks and logics before releasing the operation to
> sys_chmod().

This is exactly the kind of hook we've tried to avoid.  This is really
asking for permission to alter inode attribute data.  This type of
hook is incomplete because there are other ways to alter this data,
and this access is already controlled by the inode_setattr hook (as
Tony mentioned).  So this is a no go.

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

Re: [PATCH] New sys_chmod() hook for the LSM framework

[Lorenzo Hernández García-Hierro]

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

El mar, 08-02-2005 a las 16:15 -0800, Chris Wright escribió:
> * Lorenzo Hernández García-Hierro (lorenzo@gnu.org) wrote:
> > As commented yesterday, I was going to release a few more hooks for some
> > *critical* syscalls, this one adds a hook to sys_chmod(), and makes us
> > able to apply checks and logics before releasing the operation to
> > sys_chmod().
> 
> This is exactly the kind of hook we've tried to avoid.  This is really
> asking for permission to alter inode attribute data.  This type of
> hook is incomplete because there are other ways to alter this data,
> and this access is already controlled by the inode_setattr hook (as
> Tony mentioned).  So this is a no go.

Right, the patch is no longer available as notify_change grabs the
inode_setattr hook returned data.

Did you checked the other one on sys_chroot()? (I've updated it a day or
so ago).

Cheers, thanks for commenting on it.
-- 
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 --]