LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
From: Kentaro Takeda <takedakn@nttdata.co.jp>
To: Miklos Szeredi <miklos@szeredi.hu>
Cc: sds@tycho.nsa.gov, jmorris@namei.org, chrisw@sous-sol.org,
	serue@us.ibm.com, linux-security-module@vger.kernel.org,
	linux-kernel@vger.kernel.org, haradats@nttdata.co.jp,
	akpm@linux-foundation.org, penguin-kernel@I-love.SAKURA.ne.jp,
	viro@zeniv.linux.org.uk, hch@lst.de, crispin@crispincowan.com,
	casey@schaufler-ca.com
Subject: Re: [TOMOYO #11 (linux-next) 01/11] Introduce new LSM hooks where vfsmount is available.
Date: Tue, 21 Oct 2008 14:09:47 +0900	[thread overview]
Message-ID: <48FD641B.80406@nttdata.co.jp> (raw)
In-Reply-To: <E1Krxrj-0001r8-7t@pomaz-ex.szeredi.hu>

Miklos Szeredi wrote:
>> (6) Introducing new LSM hooks.
>>  (this patch)
>>
>>  We understand that adding new LSM hooks which receive "struct
>>  vfsmount" outside VFS helper functions is the most straightforward
>>  approach. This approach has less impact to existing LSM module and
>>  no impact to VFS helper functions.
> 
> AppArmor will need a few additional hooks, but the ones added by this
> patch look OK.  One thing I'd prefer is if there were two different
> hooks for truncate and ftruncate:
> 
>    int (*path_truncate) (struct path *path, ...);
> 
> and
> 
>    int (*file_truncate) (struct file *file, ...);
When you submit AppArmor, you can introduce security_file_truncate() 
and replace security_path_truncate() with security_file_truncate() 
since TOMOYO can get "struct path *path" from 
"struct file *file"->f_path .

> security_path_truncate() is missing from do_coredump() in exec.c.  Is
> this intentional?
Yes.
TOMOYO checks only requests from userspace, not from kernel (e.g. 
filesystem). Since do_coredump() performs request from kernel, we 
don't insert security_path_truncate() in do_coredump() .

> Also seems to be missing:
> 
>  - security_path_mknod() from do_create() in ipc/mqueue.c
TOMOYO doesn't check IPC for now.

>  - security_path_mknod() from unix_bind() in net/unix/af_unix.c
There is security_path_mknod() call in unix_bind(). Please see below.
;-)

--- linux-next.orig/net/unix/af_unix.c
+++ linux-next/net/unix/af_unix.c
@@ -828,7 +828,12 @@ static int unix_bind(struct socket *sock
 		err = mnt_want_write(nd.path.mnt);
 		if (err)
 			goto out_mknod_dput;
+		err = security_path_mknod(&nd.path, dentry, mode, 0);
+		if (err)
+			goto out_mknod_drop_write;
 		err = vfs_mknod(nd.path.dentry->d_inode, dentry, mode, 0);
+		security_path_clear();
+out_mknod_drop_write:
 		mnt_drop_write(nd.path.mnt);
 		if (err)
 			goto out_mknod_dput;

>  - security_path_unlink() from sys_mq_unlink() in ipc/mqueue.c
Same reason as security_path_mknod() from do_create() in ipc/mqueue.c .

> The hooks are also not called from nfsd, I presume that's intentional?
Same reason as do_coredump() . Requests from nfsd are not from 
userspace.

Since AppArmor checks both reqests from userspace and kernel, 
AppArmor will need to insert security_path_*() to more locations. 
Then TOMOYO will want a mechanism for dinstinguishing requests from 
userspace and ones from kernel.

>> (6.1) Introducing security_path_clear() hook.
>>  (this patch)
>>
>>  To perform DAC performed in vfs_foo() before MAC, we let
>>  security_path_foo() save a result into our own hash table and
>>  return 0, and let security_inode_foo() return the saved
>>  result. Since security_inode_foo() is not always called after
>>  security_path_foo(), we need security_path_clear() to clear the
>>  hash table.
> 
> This is not a good interface, IMO.  It's easy to forget (e.g. two
> places in open.c), and hard to detect.
> 
> And is it necessary at all?  Saving the result in the per-task
> security context and destroying it at exit should be an equivalent
> solution.
Though current kernel has current->security, CRED patchset by David moves 
security field from current to current->cred . Since current->cred is shared by 
multiple processes, we'll not be able to implement per-task security. Please see 
http://lkml.org/lkml/2008/10/2/7 in detail.

Regards,


  reply	other threads:[~2008-10-21  5:10 UTC|newest]

Thread overview: 21+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-10-20  7:34 [TOMOYO #11 (linux-next) 00/11] TOMOYO Linux Kentaro Takeda
2008-10-20  7:34 ` [TOMOYO #11 (linux-next) 01/11] Introduce new LSM hooks where vfsmount is available Kentaro Takeda
2008-10-20 12:27   ` Shaya Potter
2008-10-20 19:34     ` crispin
2008-10-20 21:23       ` Shaya Potter
2008-10-23 17:57         ` Shaya Potter
2008-10-20 16:44   ` Miklos Szeredi
2008-10-21  5:09     ` Kentaro Takeda [this message]
2008-10-20  7:34 ` [TOMOYO #11 (linux-next) 02/11] Add in_execve flag into task_struct Kentaro Takeda
2008-10-20  7:34 ` [TOMOYO #11 (linux-next) 03/11] Singly linked list implementation Kentaro Takeda
2008-10-20  7:34 ` [TOMOYO #11 (linux-next) 04/11] Introduce d_realpath() Kentaro Takeda
2008-10-20  7:34 ` [TOMOYO #11 (linux-next) 05/11] Memory and pathname management functions Kentaro Takeda
2008-10-20  7:34 ` [TOMOYO #11 (linux-next) 06/11] Common functions for TOMOYO Linux Kentaro Takeda
2008-10-20  7:34 ` [TOMOYO #11 (linux-next) 07/11] File operation restriction part Kentaro Takeda
2008-10-20  7:34 ` [TOMOYO #11 (linux-next) 08/11] Domain transition handler Kentaro Takeda
2008-10-20  7:34 ` [TOMOYO #11 (linux-next) 09/11] LSM adapter functions Kentaro Takeda
2008-10-20  7:34 ` [TOMOYO #11 (linux-next) 10/11] Kconfig and Makefile Kentaro Takeda
2008-10-20  7:34 ` [TOMOYO #11 (linux-next) 11/11] MAINTAINERS info Kentaro Takeda
2008-10-27  2:18 ` [TOMOYO #11 (linux-next) 00/11] TOMOYO Linux Kentaro Takeda
2008-10-29 19:18   ` Serge E. Hallyn
2008-10-30  5:27     ` Toshiharu Harada

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=48FD641B.80406@nttdata.co.jp \
    --to=takedakn@nttdata.co.jp \
    --cc=akpm@linux-foundation.org \
    --cc=casey@schaufler-ca.com \
    --cc=chrisw@sous-sol.org \
    --cc=crispin@crispincowan.com \
    --cc=haradats@nttdata.co.jp \
    --cc=hch@lst.de \
    --cc=jmorris@namei.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-security-module@vger.kernel.org \
    --cc=miklos@szeredi.hu \
    --cc=penguin-kernel@I-love.SAKURA.ne.jp \
    --cc=sds@tycho.nsa.gov \
    --cc=serue@us.ibm.com \
    --cc=viro@zeniv.linux.org.uk \
    --subject='Re: [TOMOYO #11 (linux-next) 01/11] Introduce new LSM hooks where vfsmount is available.' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).