Re: [PATCH] Introduce new LSM hooks where vfsmount is available.

Previous thread: [PATCH 0/5] loglevel=pci:8,acpi:8 support v4 by Yinghai Lu on Tuesday, September 16, 2008 - 6:14 pm. (6 messages)

Next thread: Re: [Bugme-new] [Bug 11543] New: kernel panic: softlockup in tick_periodic() ??? by j_kernel on Tuesday, September 16, 2008 - 7:43 pm. (10 messages)
From: Kentaro Takeda
Date: Tuesday, September 16, 2008 - 7:16 pm

TOMOYO Linux needs method for calculating pathname in LSM module.
However, we have received comment from Al Viro, the vfs maintainer,
that adding vfsmount parameter to vfs helper functions (and LSM hooks)
is not preferable. We have asked some people (including Al), and we
came back to the most straightforward approach; adding new LSM hooks
where vfsmount is available.

The attached patch introduces several new LSM hooks TOMOYO Linux
needs. It has less impact to existing LSM module and no impact to vfs
helper functions. Please review it.

Regards,

---
Subject: Introduce new LSM hooks.

This patch allows LSM to check permission using "struct vfsmount"
without passing "struct vfsmount" to VFS helper functions.

Signed-off-by: Kentaro Takeda <takedakn@nttdata.co.jp>
Signed-off-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
Signed-off-by: Toshiharu Harada <haradats@nttdata.co.jp>
---
 fs/namei.c               |   37 ++++++++++++
 fs/open.c                |    5 +
 include/linux/security.h |  135 +++++++++++++++++++++++++++++++++++++++++++++++
 net/unix/af_unix.c       |    4 +
 security/capability.c    |   53 ++++++++++++++++++
 security/security.c      |   63 +++++++++++++++++++++
 6 files changed, 297 insertions(+)

--- linux-2.6.27-rc6.orig/fs/namei.c
+++ linux-2.6.27-rc6/fs/namei.c
@@ -1585,6 +1585,10 @@ int may_open(struct nameidata *nd, int a
 		 * Refuse to truncate files with mandatory locks held on them.
 		 */
 		error = locks_verify_locked(inode);
+		if (!error)
+			error = security_path_truncate(&nd->path, 0,
+					       ATTR_MTIME|ATTR_CTIME|ATTR_OPEN,
+						       NULL);
 		if (!error) {
 			DQUOT_INIT(inode);
 
@@ -1615,7 +1619,11 @@ static int __open_namei_create(struct na
 
 	if (!IS_POSIXACL(dir->d_inode))
 		mode &= ~current->fs->umask;
+	error = security_path_mknod(&nd->path, path->dentry, mode, 0);
+	if (error)
+		goto out_unlock;
 	error = vfs_create(dir->d_inode, path->dentry, mode, nd);
+out_unlock:
 ...
From: Casey Schaufler
Date: Tuesday, September 16, 2008 - 9:00 pm

As always, the community will be eager to see the code that makes

--

From: Kentaro Takeda
Date: Wednesday, September 17, 2008 - 1:35 am

We are ready for submitting body of TOMOYO Linux as always. :-)
However, the largest stumbling block of merging TOMOYO Linux is the
method for calculating pathname in LSM module. This patch contains
all modifications to the existing kernel code by TOMOYO Linux (the
rest modification is within security/tomoyo).

We'd like to ask the community's approval of this approach before
review of TOMOYO Linux main module.

Regards,

--

From: david
Date: Wednesday, September 17, 2008 - 10:46 am

you can bet that if these hooks are accepted, AppArmor and the various 
other LSMs that have not been accepted becouse of the problems they have 
dealing with paths will switch to using the 'approved' hooks.

so there are several bodies of code that have already been published that 
would want to use hooks like these if they are available.

David Lang
--

From: James Morris
Date: Monday, September 22, 2008 - 11:06 pm

I don't see any technical errors in this patch.

If it is going to be merged, please make a new config option for 
path-based hooks (similar to that for the network hooks), so they can be 
compiled out.


- James
-- 
James Morris
<jmorris@namei.org>
--

From: david
Date: Monday, September 22, 2008 - 11:12 pm

one question about these new hook locations.

it is possible to gather all the info that was gathered at the old hook 
locations from the new ones? I realize that you are not eliminating the 
old hooks (and possibly can't for backwards compatibility), but possibly 
they should be depriciated in favor of the new locations if they can 
satisfy both the old uses and new use cases.

David Lang
--

From: Stephen Smalley
Date: Tuesday, September 23, 2008 - 8:02 am

They can't.

-- 
Stephen Smalley
National Security Agency

--

From: Alexey Dobriyan
Date: Tuesday, September 23, 2008 - 8:20 am

Another pointless config option.

It's actually pretty surprising that SECURITY_NETWORK is
a) user-visible and b) is not SECURITY && NET.

Same for SECURITY_NETWORK_XFRM.
--

From: James Morris
Date: Tuesday, September 23, 2008 - 2:20 pm

Why is it pointless?  If distros don't want to use these hooks, they 

IIRC, this option was a perquisite for merge.  Networking hooks have a 
relatively high overhead and not everyone wants to use them, even if they 
have networking enabled.


- James
-- 
James Morris
<jmorris@namei.org>
--

From: Kentaro Takeda
Date: Wednesday, September 24, 2008 - 1:00 am

I see, here it is.


---
Subject: introduce new LSM hooks where vfsmount is available.

----- What is this patch for? -----

There are security_inode_*() LSM hooks for attribute-based MAC, but they are not
suitable for pathname-based MAC because they don't receive "struct vfsmount"
information.

----- How this patch was developed? -----

Two pathname-based MACs, AppArmor and TOMOYO Linux, are trying to merge
upstream. But because of "struct vfsmount" problem, they have been unable to
merge upstream.

Here are the list of approaches and the reasons of denial.

(1) Not using LSM
 http://lwn.net/Articles/277833/

 This approach was rejected because security modules should use LSM because the
 whole idea behind LSM was to have a single set of hooks for all security
 modules; if every module now adds its own set of hooks, that purpose will have
 been defeated and the kernel will turn into a big mess of security hooks.

(2) Retrieving "struct vfsmount" from "struct task_struct".
 http://lkml.org/lkml/2007/11/5/388

 Since "struct task_struct" contains list of "struct vfsmount",
 "struct vfsmount" which corresponds to "struct dentry" can be retrieved from
 the list unless "mount --bind" is used.

 This approach turned out to cause a critical problem that getting namespace_sem
 lock from security_inode_*() triggers AB-BA deadlock.

(3) Adding "struct vfsmount" parameter to VFS helper functions.
 http://lkml.org/lkml/2008/5/29/207

 This approach adds "struct vfsmount" to VFS helper functions (e.g. vfs_mkdir()
 and vfs_symlink()) and LSM hooks inside VFS helper functions. This approach is
 helpful for not only AppArmor and TOMOYO Linux 2.x but also SELinux and
 auditing purpose, for this approach allows existent LSM users to use pathnames
 in their access control and audit logs.

 This approach was rejected by Al Viro, the VFS maintainer, because he thinks
 individual filesystem should remain "struct vfsmount"-unaware and VFS helper
 functions should not receive "struct ...
From: Serge E. Hallyn
Date: Wednesday, September 24, 2008 - 6:28 am

Is there any reason why users should have to deal with this option?
Would it make sense in this case to make the option and have tomoyo and
--

From: James Morris
Date: Wednesday, September 24, 2008 - 2:18 pm

Who are users in this case?  It's probably useful as a separate option for 
testing purposes and for people doing development.  LSMs which depend on 
it should probably still use SELECT in any case.

-- 
James Morris
<jmorris@namei.org>
--

Previous thread: [PATCH 0/5] loglevel=pci:8,acpi:8 support v4 by Yinghai Lu on Tuesday, September 16, 2008 - 6:14 pm. (6 messages)

Next thread: Re: [Bugme-new] [Bug 11543] New: kernel panic: softlockup in tick_periodic() ??? by j_kernel on Tuesday, September 16, 2008 - 7:43 pm. (10 messages)