[RFC] LSM/TOMOYO: LSM hooks for chmod/chown/chroot/mount/open/execve

[RFC] LSM/TOMOYO: LSM hooks for chmod/chown/chroot/mount/open/execve

[Tetsuo Handa]

Hello.

I released TOMOYO 1.7.0 (non-LSM version of TOMOYO Linux).
http://sourceforge.jp/projects/tomoyo/lists/archive/users-en/2009-September/000092.html

Before resuming TOMOYO 2.x (LSM version of TOMOYO Linux), I'd like to show you
what hooks I want to pick up from http://sourceforge.jp/projects/tomoyo/svn/view/trunk/1.7.x/ccs-patch/patches/ccs-patch-2.6.31.diff?view=markup&root=tomoyo .

(1) TOMOYO 1.7 checks chmod()/chown()/chgrp() operations, and I'm planning to
    check them in TOMOYO 2.x as well. Thus I'd like to add

    security_path_chmod() in SYSCALL_DEFINE2(fchmod, ...) and
    SYSCALL_DEFINE3(fchmodat, ...).

    security_path_chown() in SYSCALL_DEFINE3(chown, ...) and
    SYSCALL_DEFINE5(fchownat, ...) and SYSCALL_DEFINE3(lchown, ...) and
    SYSCALL_DEFINE3(fchown, ...).

(2) TOMOYO 1.x checks chroot() operations, and I'm planning to check it in
    TOMOYO 2.x as well. Thus I'd like to add

    security_path_chroot() in SYSCALL_DEFINE1(chroot, ...).

(3) TOMOYO 1.x checks mount() operations with original mount 'flags'.
    Currently, security_sb_mount() receives 'flags & ~(MS_NOSUID | MS_NOEXEC |
    MS_NODEV | MS_ACTIVE | MS_NOATIME | MS_NODIRATIME | MS_RELATIME |
    MS_KERNMOUNT | MS_STRICTATIME)'. I want to pass original mount 'flags' to
    LSM hook.

(4) TOMOYO has unique set of permissions different from read/write/execute.
    TOMOYO 1.7 doesn't check read/write permissions for open(pathname, 3)
    because TOMOYO 1.7 instead checks ioctl permission. I'm planning to
    implement it in TOMOYO 2.x as well. Currently, original flags passed to
    open() is not passed to LSM hooks. Thus, TOMOYO 2.x can't tell
    open(pathname, 3) from open(pathname, 2). I want to pass original lower
    2 bits of flags to LSM so that TOMOYO 2.x can distinguish open for
    read/write/ioctl and open for ioctl only.

(5) TOMOYO 1.6/1.7 have "execute handler". I'm planning to implement it in
    TOMOYO 2.x as well. The "execute handler" is a mechanism for executing
    a program which is different from a program passed to execve().

    What this mechanism is doing is similar to load_script() defined in
    fs/binfmt_script.c .
    http://tomoyo.sourceforge.jp/cgi-bin/lxr/source/security/ccsecurity/domain.c?v=linux-2.6.30.5-ccs-1.7.0#L922

    The idea of this mechanism comes from anti-shellcode technique.
    An old exploit program for Samba's trans2open vulnerability attempts to
    execute /bin/sh . If there is no MAC, /bin/sh is executed. If there is MAC
    and it rejects execution of /bin/sh from /usr/sbin/smbd , the exploit
    program cannot execute /bin/sh . However, the exploit code repeats execute
    request of /bin/sh until it succeeds. As a result, the victim process eats
    100% of CPU resources.
    If a process was hijacked by an exploit code, rejecting execve() request
    unlikely recovers the hijacked process's control. In other words, returning
    control back to the hijacked process is not useful. Then, I thought we have
    a chance to do something better (by executing a different program) than
    simply wasting CPU resource (by rejecting execve() requests forever).

    After I came up to "execute handler" idea, I found another usage.
    I found that we can use "execute handler" for validating (and manipulating)
    parameters passed to execve() (e.g. file descriptors/argv[]/envp[]).
    Although there is a disadvantage that there is no way to tell the caller
    of execve() that the requested program was not executed, I verified that
    "execute handler" is useful using TOMOYO 1.6 .

    In some environments, /bin/egrep is implemented as

      #!/bin/sh
      exec grep -E ${1+"$@"}

    which simply executes the requested program with modified command line.
    An "execute handler" is implemented something like

      #!/bin/sh
      check_whether_to_execute_originally_requested_program_or_not || exit 1
      exec originally_requested_program some_parameters

    Unlike load_script(), I can't call search_binary_handler() from LSM hooks
    because I can't let LSM hooks say "you don't need to call
    search_binary_handler() because I already called search_binary_handler()".
    This means that I can't use stack memory for remembering bprm->filename and
    bprm->interp . Thus, I need to use kmalloc() and a LSM hook for kfree().

    Which approach is better you think?
    (a) Add post-execve() hook which is called after search_binary_handler()
        finished. This approach can avoid addition of member to cred->security
        that is only valid during execve().
    (b) Hold the pointer to kmalloc()ed memory within bprm->cred->security and
        kfree() the pointer at security_bprm_committing_creds().
        This approach can avoid addition of new LSM hook.

(6) TOMOYO 1.6/1.7 have "process state variables". I'm planning to implement it
    in TOMOYO 2.x as well. The "process state variables" is used for tracking
    current process's state (to avoid "execute handler" loop etc.) and for
    associating different permissions according to the process's behavior.

    Which approach is better you think?
    (a) Add per "struct task_struct" variable like
        http://tomoyo.sourceforge.jp/cgi-bin/lxr/source/include/linux/sched.h?v=linux-2.6.30.5-ccs-1.7.0#L1434 .
        This approach can simplify the code because we don't need to care about
        error paths (prepare_creds() failures).
    (b) Hold the variable within cred->security .
        This approach can avoid modification of "struct task_struct".

Regards.

[PATCH] LSM hooks for chmod/chown/chroot/mount

[Tetsuo Handa]

Hello.

Tetsuo Handa wrote:
> Before resuming TOMOYO 2.x (LSM version of TOMOYO Linux), I'd like to show you
> what hooks I want to pick up from http://sourceforge.jp/projects/tomoyo/svn/view/trunk/1.7.x/ccs-patch/patches/ccs-patch-2.6.31.diff?view=markup&root=tomoyo .
> 
I made draft version of TOMOYO 2.3.0 .

> (1) TOMOYO 1.7 checks chmod()/chown()/chgrp() operations, and I'm planning to
>     check them in TOMOYO 2.x as well. Thus I'd like to add
> 
>     security_path_chmod() in SYSCALL_DEFINE2(fchmod, ...) and
>     SYSCALL_DEFINE3(fchmodat, ...).
> 
>     security_path_chown() in SYSCALL_DEFINE3(chown, ...) and
>     SYSCALL_DEFINE5(fchownat, ...) and SYSCALL_DEFINE3(lchown, ...) and
>     SYSCALL_DEFINE3(fchown, ...).
>
This is the patch.

[LSM] Add security_path_chmod() and security_path_chown().

This patch allows pathname based LSM modules to check chmod()/chown()
operations. Since notify_change() does not receive "struct vfsmount *",
we add security_path_chmod() and security_path_chown() to the caller of
notify_change().

These hooks are used by TOMOYO.

Signed-off-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
---
 fs/open.c                |   24 ++++++++++++++++++++----
 include/linux/security.h |   30 ++++++++++++++++++++++++++++++
 security/capability.c    |   13 +++++++++++++
 security/security.c      |   15 +++++++++++++++
 4 files changed, 78 insertions(+), 4 deletions(-)

--- linux-2.6.31.orig/fs/open.c
+++ linux-2.6.31/fs/open.c
@@ -615,6 +615,9 @@ SYSCALL_DEFINE2(fchmod, unsigned int, fd
 	err = mnt_want_write_file(file);
 	if (err)
 		goto out_putf;
+	err = security_path_chmod(dentry, file->f_vfsmnt, mode);
+	if (err)
+		goto out_drop_write;
 	mutex_lock(&inode->i_mutex);
 	if (mode == (mode_t) -1)
 		mode = inode->i_mode;
@@ -622,6 +625,7 @@ SYSCALL_DEFINE2(fchmod, unsigned int, fd
 	newattrs.ia_valid = ATTR_MODE | ATTR_CTIME;
 	err = notify_change(dentry, &newattrs);
 	mutex_unlock(&inode->i_mutex);
+out_drop_write:
 	mnt_drop_write(file->f_path.mnt);
 out_putf:
 	fput(file);
@@ -644,6 +648,9 @@ SYSCALL_DEFINE3(fchmodat, int, dfd, cons
 	error = mnt_want_write(path.mnt);
 	if (error)
 		goto dput_and_out;
+	error = security_path_chmod(path.dentry, path.mnt, mode);
+	if (error)
+		goto out_drop_write;
 	mutex_lock(&inode->i_mutex);
 	if (mode == (mode_t) -1)
 		mode = inode->i_mode;
@@ -651,6 +658,7 @@ SYSCALL_DEFINE3(fchmodat, int, dfd, cons
 	newattrs.ia_valid = ATTR_MODE | ATTR_CTIME;
 	error = notify_change(path.dentry, &newattrs);
 	mutex_unlock(&inode->i_mutex);
+out_drop_write:
 	mnt_drop_write(path.mnt);
 dput_and_out:
 	path_put(&path);
@@ -699,7 +707,9 @@ SYSCALL_DEFINE3(chown, const char __user
 	error = mnt_want_write(path.mnt);
 	if (error)
 		goto out_release;
-	error = chown_common(path.dentry, user, group);
+	error = security_path_chown(&path, user, group);
+	if (!error)
+		error = chown_common(path.dentry, user, group);
 	mnt_drop_write(path.mnt);
 out_release:
 	path_put(&path);
@@ -724,7 +734,9 @@ SYSCALL_DEFINE5(fchownat, int, dfd, cons
 	error = mnt_want_write(path.mnt);
 	if (error)
 		goto out_release;
-	error = chown_common(path.dentry, user, group);
+	error = security_path_chown(&path, user, group);
+	if (!error)
+		error = chown_common(path.dentry, user, group);
 	mnt_drop_write(path.mnt);
 out_release:
 	path_put(&path);
@@ -743,7 +755,9 @@ SYSCALL_DEFINE3(lchown, const char __use
 	error = mnt_want_write(path.mnt);
 	if (error)
 		goto out_release;
-	error = chown_common(path.dentry, user, group);
+	error = security_path_chown(&path, user, group);
+	if (!error)
+		error = chown_common(path.dentry, user, group);
 	mnt_drop_write(path.mnt);
 out_release:
 	path_put(&path);
@@ -766,7 +780,9 @@ SYSCALL_DEFINE3(fchown, unsigned int, fd
 		goto out_fput;
 	dentry = file->f_path.dentry;
 	audit_inode(NULL, dentry);
-	error = chown_common(dentry, user, group);
+	error = security_path_chown(&file->f_path, user, group);
+	if (!error)
+		error = chown_common(dentry, user, group);
 	mnt_drop_write(file->f_path.mnt);
 out_fput:
 	fput(file);
--- linux-2.6.31.orig/include/linux/security.h
+++ linux-2.6.31/include/linux/security.h
@@ -447,6 +447,18 @@ static inline void security_free_mnt_opt
  *	@new_dir contains the path structure for parent of the new link.
  *	@new_dentry contains the dentry structure of the new link.
  *	Return 0 if permission is granted.
+ * @path_chmod:
+ *	Check for permission to change DAC's permission of a file or directory.
+ *	@dentry contains the dentry structure.
+ *	@mnt contains the vfsmnt structure.
+ *	@mode contains DAC's mode.
+ *	Return 0 if permission is granted.
+ * @path_chown:
+ *	Check for permission to change owner/group of a file or directory.
+ *	@path contains the path structure.
+ *	@uid contains new owner's ID.
+ *	@gid contains new group's ID.
+ *	Return 0 if permission is granted.
  * @inode_readlink:
  *	Check the permission to read the symbolic link.
  *	@dentry contains the dentry structure for the file link.
@@ -1422,6 +1434,9 @@ struct security_operations {
 			  struct dentry *new_dentry);
 	int (*path_rename) (struct path *old_dir, struct dentry *old_dentry,
 			    struct path *new_dir, struct dentry *new_dentry);
+	int (*path_chmod) (struct dentry *dentry, struct vfsmount *mnt,
+			   mode_t mode);
+	int (*path_chown) (struct path *path, uid_t uid, gid_t gid);
 #endif
 
 	int (*inode_alloc_security) (struct inode *inode);
@@ -2822,6 +2837,9 @@ int security_path_link(struct dentry *ol
 		       struct dentry *new_dentry);
 int security_path_rename(struct path *old_dir, struct dentry *old_dentry,
 			 struct path *new_dir, struct dentry *new_dentry);
+int security_path_chmod(struct dentry *dentry, struct vfsmount *mnt,
+			mode_t mode);
+int security_path_chown(struct path *path, uid_t uid, gid_t gid);
 #else	/* CONFIG_SECURITY_PATH */
 static inline int security_path_unlink(struct path *dir, struct dentry *dentry)
 {
@@ -2871,6 +2889,18 @@ static inline int security_path_rename(s
 {
 	return 0;
 }
+
+static inline int security_path_chmod(struct dentry *dentry,
+				      struct vfsmount *mnt,
+				      mode_t mode)
+{
+	return 0;
+}
+
+static inline int security_path_chown(struct path *path, uid_t uid, gid_t gid)
+{
+	return 0;
+}
 #endif	/* CONFIG_SECURITY_PATH */
 
 #ifdef CONFIG_KEYS
--- linux-2.6.31.orig/security/capability.c
+++ linux-2.6.31/security/capability.c
@@ -308,6 +308,17 @@ static int cap_path_truncate(struct path
 {
 	return 0;
 }
+
+static int cap_path_chmod(struct dentry *dentry, struct vfsmount *mnt,
+			  mode_t mode)
+{
+	return 0;
+}
+
+static int cap_path_chown(struct path *path, uid_t uid, gid_t gid)
+{
+	return 0;
+}
 #endif
 
 static int cap_file_permission(struct file *file, int mask)
@@ -926,6 +937,8 @@ void security_fixup_ops(struct security_
 	set_to_cap_if_null(ops, path_link);
 	set_to_cap_if_null(ops, path_rename);
 	set_to_cap_if_null(ops, path_truncate);
+	set_to_cap_if_null(ops, path_chmod);
+	set_to_cap_if_null(ops, path_chown);
 #endif
 	set_to_cap_if_null(ops, file_permission);
 	set_to_cap_if_null(ops, file_alloc_security);
--- linux-2.6.31.orig/security/security.c
+++ linux-2.6.31/security/security.c
@@ -434,6 +434,21 @@ int security_path_truncate(struct path *
 		return 0;
 	return security_ops->path_truncate(path, length, time_attrs);
 }
+
+int security_path_chmod(struct dentry *dentry, struct vfsmount *mnt,
+			mode_t mode)
+{
+	if (unlikely(IS_PRIVATE(dentry->d_inode)))
+		return 0;
+	return security_ops->path_chmod(dentry, mnt, mode);
+}
+
+int security_path_chown(struct path *path, uid_t uid, gid_t gid)
+{
+	if (unlikely(IS_PRIVATE(path->dentry->d_inode)))
+		return 0;
+	return security_ops->path_chown(path, uid, gid);
+}
 #endif
 
 int security_inode_create(struct inode *dir, struct dentry *dentry, int mode)

> (2) TOMOYO 1.x checks chroot() operations, and I'm planning to check it in
>     TOMOYO 2.x as well. Thus I'd like to add
> 
>     security_path_chroot() in SYSCALL_DEFINE1(chroot, ...).
> 
This is the patch.

[LSM] Add security_path_chroot().

This patch allows pathname based LSM modules to check chroot() operations.

This hook is used by TOMOYO.

Signed-off-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
---
 fs/open.c                |    3 +++
 include/linux/security.h |   11 +++++++++++
 security/capability.c    |    6 ++++++
 security/security.c      |    5 +++++
 4 files changed, 25 insertions(+)

--- linux-2.6.31.orig/fs/open.c
+++ linux-2.6.31/fs/open.c
@@ -586,6 +586,9 @@ SYSCALL_DEFINE1(chroot, const char __use
 	error = -EPERM;
 	if (!capable(CAP_SYS_CHROOT))
 		goto dput_and_out;
+	error = security_path_chroot(&path);
+	if (error)
+		goto dput_and_out;
 
 	set_fs_root(current->fs, &path);
 	error = 0;
--- linux-2.6.31.orig/include/linux/security.h
+++ linux-2.6.31/include/linux/security.h
@@ -459,6 +459,10 @@ static inline void security_free_mnt_opt
  *	@uid contains new owner's ID.
  *	@gid contains new group's ID.
  *	Return 0 if permission is granted.
+ * @path_chroot:
+ *	Check for permission to change root directory.
+ *	@path contains the path structure.
+ *	Return 0 if permission is granted.
  * @inode_readlink:
  *	Check the permission to read the symbolic link.
  *	@dentry contains the dentry structure for the file link.
@@ -1437,6 +1441,7 @@ struct security_operations {
 	int (*path_chmod) (struct dentry *dentry, struct vfsmount *mnt,
 			   mode_t mode);
 	int (*path_chown) (struct path *path, uid_t uid, gid_t gid);
+	int (*path_chroot) (struct path *path);
 #endif
 
 	int (*inode_alloc_security) (struct inode *inode);
@@ -2840,6 +2845,7 @@ int security_path_rename(struct path *ol
 int security_path_chmod(struct dentry *dentry, struct vfsmount *mnt,
 			mode_t mode);
 int security_path_chown(struct path *path, uid_t uid, gid_t gid);
+int security_path_chroot(struct path *path);
 #else	/* CONFIG_SECURITY_PATH */
 static inline int security_path_unlink(struct path *dir, struct dentry *dentry)
 {
@@ -2901,6 +2907,11 @@ static inline int security_path_chown(st
 {
 	return 0;
 }
+
+static inline int security_path_chroot(struct path *path)
+{
+	return 0;
+}
 #endif	/* CONFIG_SECURITY_PATH */
 
 #ifdef CONFIG_KEYS
--- linux-2.6.31.orig/security/capability.c
+++ linux-2.6.31/security/capability.c
@@ -319,6 +319,11 @@ static int cap_path_chown(struct path *p
 {
 	return 0;
 }
+
+static int cap_path_chroot(struct path *root)
+{
+	return 0;
+}
 #endif
 
 static int cap_file_permission(struct file *file, int mask)
@@ -939,6 +944,7 @@ void security_fixup_ops(struct security_
 	set_to_cap_if_null(ops, path_truncate);
 	set_to_cap_if_null(ops, path_chmod);
 	set_to_cap_if_null(ops, path_chown);
+	set_to_cap_if_null(ops, path_chroot);
 #endif
 	set_to_cap_if_null(ops, file_permission);
 	set_to_cap_if_null(ops, file_alloc_security);
--- linux-2.6.31.orig/security/security.c
+++ linux-2.6.31/security/security.c
@@ -449,6 +449,11 @@ int security_path_chown(struct path *pat
 		return 0;
 	return security_ops->path_chown(path, uid, gid);
 }
+
+int security_path_chroot(struct path *path)
+{
+	return security_ops->path_chroot(path);
+}
 #endif
 
 int security_inode_create(struct inode *dir, struct dentry *dentry, int mode)

> (3) TOMOYO 1.x checks mount() operations with original mount 'flags'.
>     Currently, security_sb_mount() receives 'flags & ~(MS_NOSUID | MS_NOEXEC |
>     MS_NODEV | MS_ACTIVE | MS_NOATIME | MS_NODIRATIME | MS_RELATIME |
>     MS_KERNMOUNT | MS_STRICTATIME)'. I want to pass original mount 'flags' to
>     LSM hook.
> 
This is the patch.

[LSM] Pass original mount flags to security_sb_mount()

This patch allows LSM modules to determine based on original mount flags
passed to mount(). A LSM module can get masked mount flags (if needed) by

	flags &= ~(MS_NOSUID | MS_NOEXEC | MS_NODEV | MS_ACTIVE |
		   MS_NOATIME | MS_NODIRATIME | MS_RELATIME| MS_KERNMOUNT |
		   MS_STRICTATIME);

Signed-off-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
---
 fs/namespace.c |   20 ++++++++++----------
 1 file changed, 10 insertions(+), 10 deletions(-)

--- linux-2.6.31.orig/fs/namespace.c
+++ linux-2.6.31/fs/namespace.c
@@ -1906,6 +1906,16 @@ long do_mount(char *dev_name, char *dir_
 	if (data_page)
 		((char *)data_page)[PAGE_SIZE - 1] = 0;
 
+	/* ... and get the mountpoint */
+	retval = kern_path(dir_name, LOOKUP_FOLLOW, &path);
+	if (retval)
+		return retval;
+
+	retval = security_sb_mount(dev_name, &path,
+				   type_page, flags, data_page);
+	if (retval)
+		goto dput_out;
+
 	/* Default to relatime unless overriden */
 	if (!(flags & MS_NOATIME))
 		mnt_flags |= MNT_RELATIME;
@@ -1930,16 +1940,6 @@ long do_mount(char *dev_name, char *dir_
 		   MS_NOATIME | MS_NODIRATIME | MS_RELATIME| MS_KERNMOUNT |
 		   MS_STRICTATIME);
 
-	/* ... and get the mountpoint */
-	retval = kern_path(dir_name, LOOKUP_FOLLOW, &path);
-	if (retval)
-		return retval;
-
-	retval = security_sb_mount(dev_name, &path,
-				   type_page, flags, data_page);
-	if (retval)
-		goto dput_out;
-
 	if (flags & MS_REMOUNT)
 		retval = do_remount(&path, flags & ~MS_REMOUNT, mnt_flags,
 				    data_page);

> (4) TOMOYO has unique set of permissions different from read/write/execute.
>     TOMOYO 1.7 doesn't check read/write permissions for open(pathname, 3)
>     because TOMOYO 1.7 instead checks ioctl permission. I'm planning to
>     implement it in TOMOYO 2.x as well. Currently, original flags passed to
>     open() is not passed to LSM hooks. Thus, TOMOYO 2.x can't tell
>     open(pathname, 3) from open(pathname, 2). I want to pass original lower
>     2 bits of flags to LSM so that TOMOYO 2.x can distinguish open for
>     read/write/ioctl and open for ioctl only.
> 
This was my misunderstanding. security_dentry_open() can get original flags
passed to open().

I'll refresh these patches if they are OK.

Regards.

Re: [RFC] LSM/TOMOYO: LSM hooks for chmod/chown/chroot/mount/open/execve

[James Morris]

On Sun, 6 Sep 2009, Tetsuo Handa wrote:

> Hello.
> 
> I released TOMOYO 1.7.0 (non-LSM version of TOMOYO Linux).
> http://sourceforge.jp/projects/tomoyo/lists/archive/users-en/2009-September/000092.html
> 
> Before resuming TOMOYO 2.x (LSM version of TOMOYO Linux), I'd like to show you
> what hooks I want to pick up from http://sourceforge.jp/projects/tomoyo/svn/view/trunk/1.7.x/ccs-patch/patches/ccs-patch-2.6.31.diff?view=markup&root=tomoyo .

I'd be interested to know what other pathname folk think about these 
hooks.  Are they useful for AppArmor?

Also, any comment from fs/vfs folk?


-- 
James Morris
<jmorris@namei.org>

Re: [RFC] LSM/TOMOYO: LSM hooks for chmod/chown/chroot/mount/open/execve

[John Johansen]

James Morris wrote:
> On Sun, 6 Sep 2009, Tetsuo Handa wrote:
> 
>> Hello.
>>
>> I released TOMOYO 1.7.0 (non-LSM version of TOMOYO Linux).
>> http://sourceforge.jp/projects/tomoyo/lists/archive/users-en/2009-September/000092.html
>>
>> Before resuming TOMOYO 2.x (LSM version of TOMOYO Linux), I'd like to show you
>> what hooks I want to pick up from http://sourceforge.jp/projects/tomoyo/svn/view/trunk/1.7.x/ccs-patch/patches/ccs-patch-2.6.31.diff?view=markup&root=tomoyo .
> 
> I'd be interested to know what other pathname folk think about these 
> hooks.  Are they useful for AppArmor?
> 
The LSM hooks Tetsuo post http://lkml.org/lkml/2009/10/4/36 ,
http://lkml.org/lkml/2009/10/4/37 , http://lkml.org/lkml/2009/10/4/34 are all
very useful to AppArmor.

john