Re: implement-file-posix-capabilities.patch

Re: implement-file-posix-capabilities.patch

[Serge E. Hallyn]

Quoting Chris Wright (chrisw@sous-sol.org):
> [folks, this is getting much too long-winded to stay a private thread]
> 
> * Serge E. Hallyn (serue@us.ibm.com) wrote:
> > Quoting Chris Wright (chrisw@sous-sol.org):
> > > * Andrew Morgan (morgan@kernel.org) wrote:
> > > > I share Casey's view that what's in the kernel now appears to have
> > > > evolved because of the absence of filesystem capabilities and not in
> > > > preparation for them. I'm not at all clear that they offer any real
> > > > improvement over the more macroscopic, but much simpler to understand,
> > > > superuser model.
> > > 
> > > Presently they offer next to nothing.  From the POV of kernel code,
> > > they've added an extremely coarse grained way to check privs for
> > > actions (CAP_SYS_ADMIN renders much of this useless for least priv).
> > > >From userspace POV, there's basically one well-known program that uses
> > > the existing crippled model to drop privs.  And from end-users' POV,
> > > there's no end of frustrated attempts to make something useful out
> > > of it.  From a security POV, there's missing functionality (which has
> > > been addressed in Andrew's older patches and Serge's current patchset).
> > 
> > I'm sorry I'm not sure what your conclusion is then.  Are you saying
> > that my patch does add functionality, or arguing that it shouldn't be
> > included?
> 
> It does add functionality, it is existing capabilities which are not
> particularly useful.

Ah, ok.

> > > > I think the implementation (the patch) adds support for storing
> > > > capabilities in the filesystem. But, I don't believe it is as a step
> > > > towards 'POSIX' capability support, but just adding another incrementa
> > > > twist to the Linux capability implementation - its hard to reason about
> > > > such a thing, in terms of security or otherwise. (Have you thought about
> > > > whether LD_LIBRARY_PATH is ignored by capability aware, but non-setuid-0
> > > > programs? To voice one example. There are many system consequences to
> > > > this model that will only become apparent when people start to seriously
> > > > implement and use it.)
> > > 
> > > The classic argument has been one of administration.  Tools know how to
> > > search for setuid programs,
> > 
> > Sure, but 'find' plus a 5-line program to check for security.capability
> > xattrs will find capability programs, right?
> 
> Yes.  I'm not 100% sold on the administration issue, but it is real, and
> unfortunately the extra 5-line program isn't a great solution.

It might not be :), but surely once people see what the actual
administration challenges turn out to be, they/we can write the tools to
satisfy those needs?  I don't believe they will be insurmountable.

You had in the past played with caps quite a bit, do you already have a
sense of what kinds of admin problems people will have?

> > Are there popular gui's in
> > widespread use which depend on programs granting extra privilege doing
> > so using setuid?
> > 
> > > and mainline MAC is not adding caps based on
> > > security domain during exec, etc.
> > 
> > I don't understand what you're saying - could you explain?  Which
> > 'mainline MAC', what sort of domains (TE?)?
> 
> mainline MAC meaning basically SELinux.  IOW, while LIDS and Apparmor
> had/have models for handling capabilities (don't recall if it was grant
> or restrict only), SELinux is just now talking about doing something
> like this, but nothing is upstream and in wide distribution.
> 
> > > My biggest concern is leaking caps to
> > > programs which are meant to be unprivileged.
> > 
> > Would it help if we place CONFIG_SECURITY_FILE_CAPABILITIES under
> > CONFIG_EXPERIMENTAl for a bit?
> 
> Would not hurt ;-)

Ok, here's a patch on top of 2.6.22-rc4-mm2 to do so,

thanks,
-serge

>From bf1566bb34ed47ffadfe9289ba2f1a85df5dc36f Mon Sep 17 00:00:00 2001
From: Serge E. Hallyn <serue@us.ibm.com>
Date: Thu, 21 Jun 2007 11:40:23 -0400
Subject: [PATCH 1/1] file capabilities: make file capabilities option EXPERIMENTAL

Make file capabilities depend upon CONFIG_EXPERIMENTAL, as few
people have used them to date.

Signed-off-by: Serge E. Hallyn <serue@us.ibm.com>
---
 security/Kconfig |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/security/Kconfig b/security/Kconfig
index 7c941d9..ac56c2c 100644
--- a/security/Kconfig
+++ b/security/Kconfig
@@ -81,8 +81,8 @@ config SECURITY_CAPABILITIES
 	  If you are unsure how to answer this question, answer Y.
 
 config SECURITY_FILE_CAPABILITIES
-	bool "File POSIX Capabilities"
-	depends on SECURITY=n || SECURITY_CAPABILITIES!=n
+	bool "File POSIX Capabilities (EXPERIMENTAL)"
+	depends on (SECURITY=n || SECURITY_CAPABILITIES!=n) && EXPERIMENTAL
 	default n
 	help
 	  This enables filesystem capabilities, allowing you to give
-- 
1.5.1.1.GIT

Re: implement-file-posix-capabilities.patch

[Andrew Morgan]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Serge,

[time passes]

I'm a little better up to speed on all the kernel now. I don't feel that
I conceptually object so much to this patch-series any more.... :-)

I do, however, think the patch needs some work:

1) As previously discussed, fE should be an all or nothing single bit:

How about?:

#define VFS_CAP_REVISION_MASK     0xFF000000
#define VFS_CAP_REVISION          0x01000000

#define VFS_CAP_FLAGS_MASK        ~VFS_CAP_REVISION_MASK
#define VFS_CAP_FLAGS_EFFECTIVE     0x000001

struct vfs_cap_data {
	__u32  magic_etc;
	struct {
        	__u32 permitted;     /* Little endian */
	        __u32 inheritable;   /* Little endian */
        } data[1];
};

2) Allocate capability bit-31 for CAP_SETFCAP, and use it to gate
whether the user can set this xattr on a file or not. CAP_SYS_ADMIN is
way too overloaded and this functionality is special.

3) The cap_from_disk() interface checking needs some work.... Most
notably, size must be greater than sizeof(u32) or the very first line
will do something nasty... I'd recommend you use code like this:

[...] cap_from_disk(...)
{
   if (size != sizeof(struct vfs_cap_data)) {
	printk(KERN_WARNING "%s: invalid cap size %d for file %s\n",
	     __FUNCTION__, size, bprm->filename);
	return -EINVAL;
   }

   switch ((version & VFS_CAP_REVISION_MASK)) {
   case VFS_CAP_REVISION:
        bprm->cap_effective = (version & VFS_CAP_FLAGS_EFFECTIVE)
		? CAP_FULL_SET : CAP_EMPTY_SET;
	bprm->cap_permitted =
		to_cap_t( le32_to_cpu(dcap->data[0].permitted) );
	bprm->cap_inheritable =
		to_cap_t( le32_to_cpu(dcap->data[0].inheritable) );
        return 0;
   default:
	return -EINVAL;
   }
}

Basically, I don't believe in designing a secure interface to be forward
compatible - things never work out that way and the legacy you are
implicitly committing to will haunt you in the future... FWIW I've known
a few x86 MSR designers over the years and each one has made this
mistake at least once... The future is uncertain, so don't trust it will
look the way you want it to. ;-)

5) I would rename 'set_file_caps' to 'get_file_caps' since this is what
the function actually does. If you must use 'set' then call the function
'set_bprm_caps'.

6) I also don't see the value of explicitly zero'ing the capabilities
(in cap_bprm_set_security()) only to override them elsewhere.

I'd move the 'cap_clear (bprm->cap_...)' code from
cap_bprm_set_security() into the 'out:' code at the end of
'get_file_caps()' (sic). Put rc=0 at the top of the function, and
replace the return 0; at the top of that function with a 'goto
clear_out;' then replace the out: code as follows:

   out:
       dput(dentry);
       if ((void *)dcaps != (void *)&v1caps)
		kfree(dcaps);
       if (rc) {
       clear_out:
  	        cap_clear (bprm->cap_inheritable);
 	        cap_clear (bprm->cap_permitted);
 	        cap_clear (bprm->cap_effective);
       }
       return rc;

7) This one is subtle, and to my mind not well appreciated. In
cap_bprm_apply_creds(), the wart of the global 'cap_bset' masking
permitted bits can lead to problems like the one we saw a few years back
with sendmail and capabilities. There is an assumption in setting
permitted (they are called 'forced' in some documents) capabilities on a
file that the file will execute with at least these. The inheritable
ones are optional.

The long and the short of it is there needs to be a check somewhere that:

   current->cap_permitted is a superset of file->cap_permitted

That is, what cap_bset takes away, current->cap_inheritable gives back.
If the above is not true, then the executable should fail to execute;
- -EPERM. On the surface I don't see how to do this with the LSM framework
because the relevant function is a 'void' one and can't return an error.

8) There are a number of (massive) cleanups that I would like to see
done, but they are more related to the non-file capabilities support in
the kernel and I won't pollute this present discussion any more with those.

I hope that was helpful. FWIW I did set up a git repostitory on
kernel.org to port my old patches, but in the process of porting them
better understood what you had done. If you do the above I think I'd be
happy to work from that...

Cheers

Andrew

PS. If anyone is touching file with my transmeta email in them, feel
free to replace them with the @kernel.org address.
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.2.6 (GNU/Linux)

iD8DBQFGfNYwQheEq9QabfIRAgQ2AJ9q3+BgOPlZvTboqEyM3O845xKZOQCcCLQm
zKVfemAw2F5h43rApDXuJ4o=
=OJWn
-----END PGP SIGNATURE-----

Re: implement-file-posix-capabilities.patch

[Serge E. Hallyn]

Quoting Andrew Morgan (morgan@kernel.org):
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
> 
> Serge,
> 
> [time passes]
> 
> I'm a little better up to speed on all the kernel now. I don't feel that
> I conceptually object so much to this patch-series any more.... :-)
> 
> I do, however, think the patch needs some work:
> 
> 1) As previously discussed, fE should be an all or nothing single bit:
> 
> How about?:
> 
> #define VFS_CAP_REVISION_MASK     0xFF000000
> #define VFS_CAP_REVISION          0x01000000
> 
> #define VFS_CAP_FLAGS_MASK        ~VFS_CAP_REVISION_MASK
> #define VFS_CAP_FLAGS_EFFECTIVE     0x000001
> 
> struct vfs_cap_data {
> 	__u32  magic_etc;
> 	struct {
>         	__u32 permitted;     /* Little endian */
> 	        __u32 inheritable;   /* Little endian */
>         } data[1];
> };

I don't particularly mind, but can you point out any case where
it is an advantage to have the one bit for f'E rather than just
drop f'E altogether?  Instead of having

	f'I=something
	f'P=something
	f'E=off

we can always just remove the security.capability xattr.  Right?

If there's a case where that does not suffice, then I have no objection
to doing it this way.

> 2) Allocate capability bit-31 for CAP_SETFCAP, and use it to gate
> whether the user can set this xattr on a file or not. CAP_SYS_ADMIN is
> way too overloaded and this functionality is special.

The functionality is special, but someone with CAP_SYS_ADMIN can always
unload the capability module and create the security.capability xattr
using the dummy module.

If we do add this cap, do we want to make it apply to all security.*
xattrs?

> 3) The cap_from_disk() interface checking needs some work.... Most
> notably, size must be greater than sizeof(u32) or the very first line
> will do something nasty... I'd recommend you use code like this:
> 
> [...] cap_from_disk(...)
> {
>    if (size != sizeof(struct vfs_cap_data)) {
> 	printk(KERN_WARNING "%s: invalid cap size %d for file %s\n",
> 	     __FUNCTION__, size, bprm->filename);
> 	return -EINVAL;
>    }
> 
>    switch ((version & VFS_CAP_REVISION_MASK)) {
>    case VFS_CAP_REVISION:
>         bprm->cap_effective = (version & VFS_CAP_FLAGS_EFFECTIVE)
> 		? CAP_FULL_SET : CAP_EMPTY_SET;
> 	bprm->cap_permitted =
> 		to_cap_t( le32_to_cpu(dcap->data[0].permitted) );
> 	bprm->cap_inheritable =
> 		to_cap_t( le32_to_cpu(dcap->data[0].inheritable) );
>         return 0;
>    default:
> 	return -EINVAL;
>    }
> }
> 
> Basically, I don't believe in designing a secure interface to be forward
> compatible - things never work out that way and the legacy you are
> implicitly committing to will haunt you in the future... FWIW I've known
> a few x86 MSR designers over the years and each one has made this
> mistake at least once... The future is uncertain, so don't trust it will
> look the way you want it to. ;-)

Ok, so you're saying that when we do switch to 64-bit caps or some other
evolution, we switch to completely separate logic based on the
VFS_CAP_REVISION?

That seems sane to me.

> 5) I would rename 'set_file_caps' to 'get_file_caps' since this is what
> the function actually does. If you must use 'set' then call the function
> 'set_bprm_caps'.

set_bprm_caps actually sounds best to me.

> 6) I also don't see the value of explicitly zero'ing the capabilities
> (in cap_bprm_set_security()) only to override them elsewhere.
> 
> I'd move the 'cap_clear (bprm->cap_...)' code from
> cap_bprm_set_security() into the 'out:' code at the end of
> 'get_file_caps()' (sic). Put rc=0 at the top of the function, and
> replace the return 0; at the top of that function with a 'goto
> clear_out;' then replace the out: code as follows:
> 
>    out:
>        dput(dentry);
>        if ((void *)dcaps != (void *)&v1caps)
> 		kfree(dcaps);
>        if (rc) {
>        clear_out:
>   	        cap_clear (bprm->cap_inheritable);
>  	        cap_clear (bprm->cap_permitted);
>  	        cap_clear (bprm->cap_effective);
>        }
>        return rc;

Sounds sane.  If it looks less sane when I try to write the patch I'll
get back to you  :)

> 7) This one is subtle, and to my mind not well appreciated. In
> cap_bprm_apply_creds(), the wart of the global 'cap_bset' masking
> permitted bits can lead to problems like the one we saw a few years back
> with sendmail and capabilities. There is an assumption in setting
> permitted (they are called 'forced' in some documents) capabilities on a
> file that the file will execute with at least these. The inheritable
> ones are optional.

Hmm, changing the behavior of the cap_bset is something that seems to
belong in 8), though I see what you're saying, it does affect the
behavior of vfs caps.

One gets a cozy feeling from the fact that cap_bset is set to
(~0 & ~~CAP_TO_MASK(CAP_SETPCAP)), but since it's sysctl controllable
that seems like it could present a real security problem.

So yeah, I think you're right - but the question is whose word do we
take here?  The admin who set the vfs caps on the binary, or the admin
who set cap_bset through sysctl?

I wonder whether anyone actually uses the cap_bset sysctl...

> The long and the short of it is there needs to be a check somewhere that:
> 
>    current->cap_permitted is a superset of file->cap_permitted
> 
> That is, what cap_bset takes away, current->cap_inheritable gives back.
> If the above is not true, then the executable should fail to execute;
> - -EPERM. On the surface I don't see how to do this with the LSM framework
> because the relevant function is a 'void' one and can't return an error.
> 
> 8) There are a number of (massive) cleanups that I would like to see
> done, but they are more related to the non-file capabilities support in
> the kernel and I won't pollute this present discussion any more with those.
> 
> I hope that was helpful. FWIW I did set up a git repostitory on
> kernel.org to port my old patches, but in the process of porting them
> better understood what you had done. If you do the above I think I'd be
> happy to work from that...

If you have a list of such cleanups you could send out, we can then
decide whether those all are safe to apply to the current capability
module, or whether it makes sense to fork off a shiny new capabiltyv2
module :)

many thanks for all the suggestions,
-serge

> Cheers
> 
> Andrew
> 
> PS. If anyone is touching file with my transmeta email in them, feel
> free to replace them with the @kernel.org address.
> -----BEGIN PGP SIGNATURE-----
> Version: GnuPG v1.2.6 (GNU/Linux)
> 
> iD8DBQFGfNYwQheEq9QabfIRAgQ2AJ9q3+BgOPlZvTboqEyM3O845xKZOQCcCLQm
> zKVfemAw2F5h43rApDXuJ4o=
> =OJWn
> -----END PGP SIGNATURE-----
> -
> To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: implement-file-posix-capabilities.patch

[James Morris]

On Sun, 24 Jun 2007, Serge E. Hallyn wrote:

> > 2) Allocate capability bit-31 for CAP_SETFCAP, and use it to gate
> > whether the user can set this xattr on a file or not. CAP_SYS_ADMIN is
> > way too overloaded and this functionality is special.
> 
> The functionality is special, but someone with CAP_SYS_ADMIN can always
> unload the capability module and create the security.capability xattr
> using the dummy module.
> 
> If we do add this cap, do we want to make it apply to all security.*
> xattrs?

The underlying issue here is the notion of security mechanisms which are 
built as loadable modules.  It's not useful for any in-tree users, and 
introduces several unnecessary problems which then need to be addressed.

A better approach would be to make LSM a statically linked interface.

This would also allow us to unexport the LSM symbols and reduce the API 
abuse by third-party modules.


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

[PATCH][RFC] security: Convert LSM into a static interface

[James Morris]

Convert LSM into a static interface, as the ability to unload a security 
module is not required by in-tree users and potentially complicates the 
overall security architecture.

Needlessly exported LSM symbols have been unexported, to help reduce API 
abuse.

Module parameters for the capability and root_plug modules have been
converted to kernel parameters.

The SECURITY_FRAMEWORK_VERSION macro has also been removed.

Signed-off-by: James Morris <jmorris@namei.org>
---

Please review & let me know if anything is broken.


 Documentation/kernel-parameters.txt |   17 +++++++++++
 security/Kconfig                    |    4 +-
 security/capability.c               |   32 ++++----------------
 security/commoncap.c                |    3 --
 security/dummy.c                    |    1 -
 security/root_plug.c                |   53 +++++++++++++---------------------
 security/security.c                 |    9 +----
 security/selinux/hooks.c            |    1 -
 security/selinux/xfrm.c             |    1 -
 9 files changed, 48 insertions(+), 73 deletions(-)

diff --git a/Documentation/kernel-parameters.txt b/Documentation/kernel-parameters.txt
index 5d0283c..4c406fb 100644
--- a/Documentation/kernel-parameters.txt
+++ b/Documentation/kernel-parameters.txt
@@ -74,10 +74,12 @@ parameter is applicable:
 	PPT	Parallel port support is enabled.
 	PS2	Appropriate PS/2 support is enabled.
 	RAM	RAM disk support is enabled.
+	ROOTPLUG The example Root Plug LSM is enabled.
 	S390	S390 architecture is enabled.
 	SCSI	Appropriate SCSI support is enabled.
 			A lot of drivers has their options described inside of
 			Documentation/scsi/.
+	SECURITY Different security models are enabled.
 	SELINUX SELinux support is enabled.
 	SERIAL	Serial support is enabled.
 	SH	SuperH architecture is enabled.
@@ -376,6 +378,12 @@ and is between 256 and 4096 characters. It is defined in the file
 			possible to determine what the correct size should be.
 			This option provides an override for these situations.
 
+	capability_disable=
+			[SECURITY] Disable capabilities.  This would normally
+			be used only if an alternative security model is to be
+			configured.  Potentially dangerous and should only be
+			used if you are entirely sure of the consequences.
+
 	cdu31a=		[HW,CD]
 			Format: <io>,<irq>[,PAS]
 			See header of drivers/cdrom/cdu31a.c.
@@ -1541,6 +1549,15 @@ and is between 256 and 4096 characters. It is defined in the file
 
 	rootfstype=	[KNL] Set root filesystem type
 
+	root_plug_vendor_id=
+			[ROOTPLUG] Override the default vendor ID
+
+	root_plug_product_id=
+			[ROOTPLUG] Override the default product ID
+
+	root_plug_debug=
+			[ROOTPLUG] Enable debugging output
+
 	rw		[KNL] Mount root device read-write on boot
 
 	S		[KNL] Run init in single mode
diff --git a/security/Kconfig b/security/Kconfig
index 460e5c9..8ae5490 100644
--- a/security/Kconfig
+++ b/security/Kconfig
@@ -74,14 +74,14 @@ config SECURITY_NETWORK_XFRM
 	  If you are unsure how to answer this question, answer N.
 
 config SECURITY_CAPABILITIES
-	tristate "Default Linux Capabilities"
+	bool "Default Linux Capabilities"
 	depends on SECURITY
 	help
 	  This enables the "default" Linux capabilities functionality.
 	  If you are unsure how to answer this question, answer Y.
 
 config SECURITY_ROOTPLUG
-	tristate "Root Plug Support"
+	bool "Root Plug Support"
 	depends on USB && SECURITY
 	help
 	  This is a sample LSM module that should only be used as such.
diff --git a/security/capability.c b/security/capability.c
index 38296a0..1c97953 100644
--- a/security/capability.c
+++ b/security/capability.c
@@ -8,7 +8,6 @@
  *
  */
 
-#include <linux/module.h>
 #include <linux/init.h>
 #include <linux/kernel.h>
 #include <linux/security.h>
@@ -51,8 +50,13 @@ static struct security_operations capability_ops = {
 static int secondary;
 
 static int capability_disable;
-module_param_named(disable, capability_disable, int, 0);
-MODULE_PARM_DESC(disable, "To disable capabilities module set disable = 1");
+
+static int __init capability_disable_setup(char *str)
+{
+	capability_disable = simple_strtol(str, NULL, 0);
+	return 1;
+}
+__setup("capability_disable=", capability_disable_setup);
 
 static int __init capability_init (void)
 {
@@ -75,26 +79,4 @@ static int __init capability_init (void)
 	return 0;
 }
 
-static void __exit capability_exit (void)
-{
-	if (capability_disable)
-		return;
-	/* remove ourselves from the security framework */
-	if (secondary) {
-		if (mod_unreg_security (KBUILD_MODNAME, &capability_ops))
-			printk (KERN_INFO "Failure unregistering capabilities "
-				"with primary module.\n");
-		return;
-	}
-
-	if (unregister_security (&capability_ops)) {
-		printk (KERN_INFO
-			"Failure unregistering capabilities with the kernel\n");
-	}
-}
-
 security_initcall (capability_init);
-module_exit (capability_exit);
-
-MODULE_DESCRIPTION("Standard Linux Capabilities Security Module");
-MODULE_LICENSE("GPL");
diff --git a/security/commoncap.c b/security/commoncap.c
index 384379e..04bd44b 100644
--- a/security/commoncap.c
+++ b/security/commoncap.c
@@ -339,6 +339,3 @@ EXPORT_SYMBOL(cap_task_post_setuid);
 EXPORT_SYMBOL(cap_task_reparent_to_init);
 EXPORT_SYMBOL(cap_syslog);
 EXPORT_SYMBOL(cap_vm_enough_memory);
-
-MODULE_DESCRIPTION("Standard Linux Common Capabilities Security Module");
-MODULE_LICENSE("GPL");
diff --git a/security/dummy.c b/security/dummy.c
index 8ffd764..6d4e34b 100644
--- a/security/dummy.c
+++ b/security/dummy.c
@@ -15,7 +15,6 @@
 #undef DEBUG
 
 #include <linux/capability.h>
-#include <linux/module.h>
 #include <linux/kernel.h>
 #include <linux/mman.h>
 #include <linux/pagemap.h>
diff --git a/security/root_plug.c b/security/root_plug.c
index 38dd4f3..3125e25 100644
--- a/security/root_plug.c
+++ b/security/root_plug.c
@@ -22,7 +22,6 @@
  *	License.
  */
 
-#include <linux/module.h>
 #include <linux/kernel.h>
 #include <linux/init.h>
 #include <linux/security.h>
@@ -33,25 +32,34 @@ static int secondary;
 
 /* default is a generic type of usb to serial converter */
 static int vendor_id = 0x0557;
-static int product_id = 0x2008;
 
-module_param(vendor_id, uint, 0400);
-MODULE_PARM_DESC(vendor_id, "USB Vendor ID of device to look for");
+static int __init root_plug_vendor_id(char *str)
+{
+	vendor_id = simple_strtol(str, NULL, 0);
+	return 1;
+}
+__setup("root_plug_vendor_id=", root_plug_vendor_id);
+
+static int product_id = 0x2008;
 
-module_param(product_id, uint, 0400);
-MODULE_PARM_DESC(product_id, "USB Product ID of device to look for");
+static int __init root_plug_product_id(char *str)
+{
+	product_id = simple_strtol(str, NULL, 0);
+	return 1;
+}
+__setup("root_plug_product_id=", root_plug_product_id);
 
 /* should we print out debug messages */
 static int debug = 0;
 
-module_param(debug, bool, 0600);
-MODULE_PARM_DESC(debug, "Debug enabled or not");
+static int __init root_plug_debug(char *str)
+{
+	debug = simple_strtol(str, NULL, 0);
+	return 1;
+}
+__setup("root_plug_debug=", root_plug_debug);
 
-#if defined(CONFIG_SECURITY_ROOTPLUG_MODULE)
-#define MY_NAME THIS_MODULE->name
-#else
 #define MY_NAME "root_plug"
-#endif
 
 #define root_dbg(fmt, arg...)					\
 	do {							\
@@ -117,25 +125,4 @@ static int __init rootplug_init (void)
 	return 0;
 }
 
-static void __exit rootplug_exit (void)
-{
-	/* remove ourselves from the security framework */
-	if (secondary) {
-		if (mod_unreg_security (MY_NAME, &rootplug_security_ops))
-			printk (KERN_INFO "Failure unregistering Root Plug "
-				" module with primary module.\n");
-	} else { 
-		if (unregister_security (&rootplug_security_ops)) {
-			printk (KERN_INFO "Failure unregistering Root Plug "
-				"module with the kernel\n");
-		}
-	}
-	printk (KERN_INFO "Root Plug module removed\n");
-}
-
 security_initcall (rootplug_init);
-module_exit (rootplug_exit);
-
-MODULE_DESCRIPTION("Root Plug sample LSM module, written for Linux Journal article");
-MODULE_LICENSE("GPL");
-
diff --git a/security/security.c b/security/security.c
index fc8601b..e49dae2 100644
--- a/security/security.c
+++ b/security/security.c
@@ -17,7 +17,6 @@
 #include <linux/kernel.h>
 #include <linux/security.h>
 
-#define SECURITY_FRAMEWORK_VERSION	"1.0.0"
 
 /* things that live in dummy.c */
 extern struct security_operations dummy_security_ops;
@@ -51,8 +50,7 @@ static void __init do_security_initcalls(void)
  */
 int __init security_init(void)
 {
-	printk(KERN_INFO "Security Framework v" SECURITY_FRAMEWORK_VERSION
-	       " initialized\n");
+	printk(KERN_INFO "Security Framework initialized\n");
 
 	if (verify(&dummy_security_ops)) {
 		printk(KERN_ERR "%s could not verify "
@@ -172,8 +170,5 @@ int mod_unreg_security(const char *name, struct security_operations *ops)
 	return security_ops->unregister_security(name, ops);
 }
 
-EXPORT_SYMBOL_GPL(register_security);
-EXPORT_SYMBOL_GPL(unregister_security);
-EXPORT_SYMBOL_GPL(mod_reg_security);
-EXPORT_SYMBOL_GPL(mod_unreg_security);
+/* Export required for modular use of security.h inlines */
 EXPORT_SYMBOL(security_ops);
diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
index ad8dd4e..105319a 100644
--- a/security/selinux/hooks.c
+++ b/security/selinux/hooks.c
@@ -20,7 +20,6 @@
  *      as published by the Free Software Foundation.
  */
 
-#include <linux/module.h>
 #include <linux/init.h>
 #include <linux/kernel.h>
 #include <linux/ptrace.h>
diff --git a/security/selinux/xfrm.c b/security/selinux/xfrm.c
index bd8d1ef..b589238 100644
--- a/security/selinux/xfrm.c
+++ b/security/selinux/xfrm.c
@@ -31,7 +31,6 @@
  *   2. Emulating a reasonable SO_PEERSEC across machines
  *   3. Testing addition of sk_policy's with security context via setsockopt
  */
-#include <linux/module.h>
 #include <linux/kernel.h>
 #include <linux/init.h>
 #include <linux/security.h>
-- 
1.5.2.1

Re: [PATCH][RFC] security: Convert LSM into a static interface

[Chris Wright]

* James Morris (jmorris@namei.org) wrote:
> -module_param_named(disable, capability_disable, int, 0);
> -MODULE_PARM_DESC(disable, "To disable capabilities module set disable = 1");
> +
> +static int __init capability_disable_setup(char *str)
> +{
> +	capability_disable = simple_strtol(str, NULL, 0);
> +	return 1;
> +}
> +__setup("capability_disable=", capability_disable_setup);

This changes the command line argument.  Other than that and a some
sanity testing I'll take this.

thanks,
-chris

Re: [PATCH][RFC] security: Convert LSM into a static interface

[James Morris]

On Sun, 24 Jun 2007, Chris Wright wrote:

> * James Morris (jmorris@namei.org) wrote:
> > -module_param_named(disable, capability_disable, int, 0);
> > -MODULE_PARM_DESC(disable, "To disable capabilities module set disable = 1");
> > +
> > +static int __init capability_disable_setup(char *str)
> > +{
> > +	capability_disable = simple_strtol(str, NULL, 0);
> > +	return 1;
> > +}
> > +__setup("capability_disable=", capability_disable_setup);
> 
> This changes the command line argument.  Other than that and a some
> sanity testing I'll take this.

Do you mean the name change, or the logic?

The name needs to change because kernel parameters are in a global 
namespace.

If the logic has changed, I'm missing something :-)


-- 
James Morris
<jmorris@namei.org>

Re: [PATCH][RFC] security: Convert LSM into a static interface

[Chris Wright]

* James Morris (jmorris@namei.org) wrote:
> On Sun, 24 Jun 2007, Chris Wright wrote:
> 
> > * James Morris (jmorris@namei.org) wrote:
> > > -module_param_named(disable, capability_disable, int, 0);
> > > -MODULE_PARM_DESC(disable, "To disable capabilities module set disable = 1");
> > > +
> > > +static int __init capability_disable_setup(char *str)
> > > +{
> > > +	capability_disable = simple_strtol(str, NULL, 0);
> > > +	return 1;
> > > +}
> > > +__setup("capability_disable=", capability_disable_setup);
> > 
> > This changes the command line argument.  Other than that and a some
> > sanity testing I'll take this.
> 
> Do you mean the name change, or the logic?
> 
> The name needs to change because kernel parameters are in a global 
> namespace.

Sorry, I mean it's currently capability.disable (and the only reason it
matters is that at one point I know there were users of it).

thanks,
-chris

Re: [PATCH][RFC] security: Convert LSM into a static interface

[Casey Schaufler]

--- Chris Wright <chrisw@sous-sol.org> wrote:

> * James Morris (jmorris@namei.org) wrote:
> > -module_param_named(disable, capability_disable, int, 0);
> > -MODULE_PARM_DESC(disable, "To disable capabilities module set disable =
> 1");
> > +
> > +static int __init capability_disable_setup(char *str)
> > +{
> > +	capability_disable = simple_strtol(str, NULL, 0);
> > +	return 1;
> > +}
> > +__setup("capability_disable=", capability_disable_setup);
> 
> This changes the command line argument.  Other than that and a some
> sanity testing I'll take this.

Sigh. So much for my schedule.

So, for planning purposes, when ought I expect to have to start
dealing with this?



Casey Schaufler
casey@schaufler-ca.com

Re: [PATCH][RFC] security: Convert LSM into a static interface

[Chris Wright]

* Casey Schaufler (casey@schaufler-ca.com) wrote:
> So, for planning purposes, when ought I expect to have to start
> dealing with this?

What is your specific concern or use case?

Re: [PATCH][RFC] security: Convert LSM into a static interface

[Casey Schaufler]

--- Chris Wright <chrisw@sous-sol.org> wrote:

> * Casey Schaufler (casey@schaufler-ca.com) wrote:
> > So, for planning purposes, when ought I expect to have to start
> > dealing with this?
> 
> What is your specific concern or use case?

Just hoping to avoid a change collision. If I have to deal
with this today it's easy, if it doesn't show up anywhere
until 2.6.28 I'm breezing, but if it all hits in two weeks I
have some scrambling and yet another delay to deal with. Not
your problem, a little information would be helpful though.

BTW, I reviewed my notes from the early days of LSM and it
turns out that I agree with the notion that loadable modules
don't make a whole lot of sense. So long as I can choose
security models as easily as I can change file systems, I'm
reasonably happy. That, and that the "default" "regular"
policy isn't too terribly different from the traditional
Unix policy.


Casey Schaufler
casey@schaufler-ca.com

Re: [PATCH][RFC] security: Convert LSM into a static interface

[Chris Wright]

* Casey Schaufler (casey@schaufler-ca.com) wrote:
> Just hoping to avoid a change collision. If I have to deal
> with this today it's easy, if it doesn't show up anywhere
> until 2.6.28 I'm breezing, but if it all hits in two weeks I
> have some scrambling and yet another delay to deal with. Not
> your problem, a little information would be helpful though.

Assuming no issues, 2.6.23 (which is months away, although
merge window isn't).

> BTW, I reviewed my notes from the early days of LSM and it
> turns out that I agree with the notion that loadable modules
> don't make a whole lot of sense. So long as I can choose
> security models as easily as I can change file systems, I'm
> reasonably happy. That, and that the "default" "regular"
> policy isn't too terribly different from the traditional
> Unix policy.

The default will continue to be capabilities.

thanks,
-chris

Re: [PATCH][RFC] security: Convert LSM into a static interface

[Casey Schaufler]

--- Chris Wright <chrisw@sous-sol.org> wrote:

> * Casey Schaufler (casey@schaufler-ca.com) wrote:
> > Just hoping to avoid a change collision. If I have to deal
> > with this today it's easy, if it doesn't show up anywhere
> > until 2.6.28 I'm breezing, but if it all hits in two weeks I
> > have some scrambling and yet another delay to deal with. Not
> > your problem, a little information would be helpful though.
> 
> Assuming no issues, 2.6.23 (which is months away, although
> merge window isn't).

Thank you.

> > BTW, I reviewed my notes from the early days of LSM and it
> > turns out that I agree with the notion that loadable modules
> > don't make a whole lot of sense. So long as I can choose
> > security models as easily as I can change file systems, I'm
> > reasonably happy. That, and that the "default" "regular"
> > policy isn't too terribly different from the traditional
> > Unix policy.
> 
> The default will continue to be capabilities.

Good. Thank you again.


Casey Schaufler
casey@schaufler-ca.com

Re: [PATCH][RFC] security: Convert LSM into a static interface

[Roberto De Ioris]

Il giorno dom, 24/06/2007 alle 20.37 -0700, Casey Schaufler ha scritto:
> --- Chris Wright <chrisw@sous-sol.org> wrote:
> 
> > * Casey Schaufler (casey@schaufler-ca.com) wrote:
> > > So, for planning purposes, when ought I expect to have to start
> > > dealing with this?
> > 
> > What is your specific concern or use case?
> 
> Just hoping to avoid a change collision. If I have to deal
> with this today it's easy, if it doesn't show up anywhere
> until 2.6.28 I'm breezing, but if it all hits in two weeks I
> have some scrambling and yet another delay to deal with. Not
> your problem, a little information would be helpful though.

I think you will not have too much problem with this patch.
After all, you will only need to remove a couple of functions (for the
unloading) and take rid of 'old' module parameters (if you use it)



-- 
Roberto De Ioris
http://unbit.it
JID: roberto@jabber.unbit.it

[PATCH try #2] security: Convert LSM into a static interface

[James Morris]

Convert LSM into a static interface, as the ability to unload a security 
module is not required by in-tree users and potentially complicates the 
overall security architecture.

Needlessly exported LSM symbols have been unexported, to help reduce API
abuse.

Parameters for the capability and root_plug modules are now specified at 
boot.

The SECURITY_FRAMEWORK_VERSION macro has also been removed.

Signed-off-by: James Morris <jmorris@namei.org>
---

Changes:
 - retain capability.disable kernel param name

 

 Documentation/kernel-parameters.txt |   17 +++++++++++
 security/Kconfig                    |    4 +-
 security/capability.c               |   24 ----------------
 security/commoncap.c                |    3 --
 security/dummy.c                    |    1 -
 security/root_plug.c                |   53 +++++++++++++---------------------
 security/security.c                 |    9 +----
 security/selinux/hooks.c            |    1 -
 security/selinux/xfrm.c             |    1 -
 9 files changed, 41 insertions(+), 72 deletions(-)

diff --git a/Documentation/kernel-parameters.txt b/Documentation/kernel-parameters.txt
index 5d0283c..35e1202 100644
--- a/Documentation/kernel-parameters.txt
+++ b/Documentation/kernel-parameters.txt
@@ -74,10 +74,12 @@ parameter is applicable:
 	PPT	Parallel port support is enabled.
 	PS2	Appropriate PS/2 support is enabled.
 	RAM	RAM disk support is enabled.
+	ROOTPLUG The example Root Plug LSM is enabled.
 	S390	S390 architecture is enabled.
 	SCSI	Appropriate SCSI support is enabled.
 			A lot of drivers has their options described inside of
 			Documentation/scsi/.
+	SECURITY Different security models are enabled.
 	SELINUX SELinux support is enabled.
 	SERIAL	Serial support is enabled.
 	SH	SuperH architecture is enabled.
@@ -376,6 +378,12 @@ and is between 256 and 4096 characters. It is defined in the file
 			possible to determine what the correct size should be.
 			This option provides an override for these situations.
 
+	capability.disable=
+			[SECURITY] Disable capabilities.  This would normally
+			be used only if an alternative security model is to be
+			configured.  Potentially dangerous and should only be
+			used if you are entirely sure of the consequences.
+
 	cdu31a=		[HW,CD]
 			Format: <io>,<irq>[,PAS]
 			See header of drivers/cdrom/cdu31a.c.
@@ -1541,6 +1549,15 @@ and is between 256 and 4096 characters. It is defined in the file
 
 	rootfstype=	[KNL] Set root filesystem type
 
+	root_plug_vendor_id=
+			[ROOTPLUG] Override the default vendor ID
+
+	root_plug_product_id=
+			[ROOTPLUG] Override the default product ID
+
+	root_plug_debug=
+			[ROOTPLUG] Enable debugging output
+
 	rw		[KNL] Mount root device read-write on boot
 
 	S		[KNL] Run init in single mode
diff --git a/security/Kconfig b/security/Kconfig
index 460e5c9..8ae5490 100644
--- a/security/Kconfig
+++ b/security/Kconfig
@@ -74,14 +74,14 @@ config SECURITY_NETWORK_XFRM
 	  If you are unsure how to answer this question, answer N.
 
 config SECURITY_CAPABILITIES
-	tristate "Default Linux Capabilities"
+	bool "Default Linux Capabilities"
 	depends on SECURITY
 	help
 	  This enables the "default" Linux capabilities functionality.
 	  If you are unsure how to answer this question, answer Y.
 
 config SECURITY_ROOTPLUG
-	tristate "Root Plug Support"
+	bool "Root Plug Support"
 	depends on USB && SECURITY
 	help
 	  This is a sample LSM module that should only be used as such.
diff --git a/security/capability.c b/security/capability.c
index 38296a0..fda6a14 100644
--- a/security/capability.c
+++ b/security/capability.c
@@ -8,7 +8,6 @@
  *
  */
 
-#include <linux/module.h>
 #include <linux/init.h>
 #include <linux/kernel.h>
 #include <linux/security.h>
@@ -52,7 +51,6 @@ static int secondary;
 
 static int capability_disable;
 module_param_named(disable, capability_disable, int, 0);
-MODULE_PARM_DESC(disable, "To disable capabilities module set disable = 1");
 
 static int __init capability_init (void)
 {
@@ -75,26 +73,4 @@ static int __init capability_init (void)
 	return 0;
 }
 
-static void __exit capability_exit (void)
-{
-	if (capability_disable)
-		return;
-	/* remove ourselves from the security framework */
-	if (secondary) {
-		if (mod_unreg_security (KBUILD_MODNAME, &capability_ops))
-			printk (KERN_INFO "Failure unregistering capabilities "
-				"with primary module.\n");
-		return;
-	}
-
-	if (unregister_security (&capability_ops)) {
-		printk (KERN_INFO
-			"Failure unregistering capabilities with the kernel\n");
-	}
-}
-
 security_initcall (capability_init);
-module_exit (capability_exit);
-
-MODULE_DESCRIPTION("Standard Linux Capabilities Security Module");
-MODULE_LICENSE("GPL");
diff --git a/security/commoncap.c b/security/commoncap.c
index 384379e..04bd44b 100644
--- a/security/commoncap.c
+++ b/security/commoncap.c
@@ -339,6 +339,3 @@ EXPORT_SYMBOL(cap_task_post_setuid);
 EXPORT_SYMBOL(cap_task_reparent_to_init);
 EXPORT_SYMBOL(cap_syslog);
 EXPORT_SYMBOL(cap_vm_enough_memory);
-
-MODULE_DESCRIPTION("Standard Linux Common Capabilities Security Module");
-MODULE_LICENSE("GPL");
diff --git a/security/dummy.c b/security/dummy.c
index 8ffd764..6d4e34b 100644
--- a/security/dummy.c
+++ b/security/dummy.c
@@ -15,7 +15,6 @@
 #undef DEBUG
 
 #include <linux/capability.h>
-#include <linux/module.h>
 #include <linux/kernel.h>
 #include <linux/mman.h>
 #include <linux/pagemap.h>
diff --git a/security/root_plug.c b/security/root_plug.c
index 38dd4f3..3125e25 100644
--- a/security/root_plug.c
+++ b/security/root_plug.c
@@ -22,7 +22,6 @@
  *	License.
  */
 
-#include <linux/module.h>
 #include <linux/kernel.h>
 #include <linux/init.h>
 #include <linux/security.h>
@@ -33,25 +32,34 @@ static int secondary;
 
 /* default is a generic type of usb to serial converter */
 static int vendor_id = 0x0557;
-static int product_id = 0x2008;
 
-module_param(vendor_id, uint, 0400);
-MODULE_PARM_DESC(vendor_id, "USB Vendor ID of device to look for");
+static int __init root_plug_vendor_id(char *str)
+{
+	vendor_id = simple_strtol(str, NULL, 0);
+	return 1;
+}
+__setup("root_plug_vendor_id=", root_plug_vendor_id);
+
+static int product_id = 0x2008;
 
-module_param(product_id, uint, 0400);
-MODULE_PARM_DESC(product_id, "USB Product ID of device to look for");
+static int __init root_plug_product_id(char *str)
+{
+	product_id = simple_strtol(str, NULL, 0);
+	return 1;
+}
+__setup("root_plug_product_id=", root_plug_product_id);
 
 /* should we print out debug messages */
 static int debug = 0;
 
-module_param(debug, bool, 0600);
-MODULE_PARM_DESC(debug, "Debug enabled or not");
+static int __init root_plug_debug(char *str)
+{
+	debug = simple_strtol(str, NULL, 0);
+	return 1;
+}
+__setup("root_plug_debug=", root_plug_debug);
 
-#if defined(CONFIG_SECURITY_ROOTPLUG_MODULE)
-#define MY_NAME THIS_MODULE->name
-#else
 #define MY_NAME "root_plug"
-#endif
 
 #define root_dbg(fmt, arg...)					\
 	do {							\
@@ -117,25 +125,4 @@ static int __init rootplug_init (void)
 	return 0;
 }
 
-static void __exit rootplug_exit (void)
-{
-	/* remove ourselves from the security framework */
-	if (secondary) {
-		if (mod_unreg_security (MY_NAME, &rootplug_security_ops))
-			printk (KERN_INFO "Failure unregistering Root Plug "
-				" module with primary module.\n");
-	} else { 
-		if (unregister_security (&rootplug_security_ops)) {
-			printk (KERN_INFO "Failure unregistering Root Plug "
-				"module with the kernel\n");
-		}
-	}
-	printk (KERN_INFO "Root Plug module removed\n");
-}
-
 security_initcall (rootplug_init);
-module_exit (rootplug_exit);
-
-MODULE_DESCRIPTION("Root Plug sample LSM module, written for Linux Journal article");
-MODULE_LICENSE("GPL");
-
diff --git a/security/security.c b/security/security.c
index fc8601b..e49dae2 100644
--- a/security/security.c
+++ b/security/security.c
@@ -17,7 +17,6 @@
 #include <linux/kernel.h>
 #include <linux/security.h>
 
-#define SECURITY_FRAMEWORK_VERSION	"1.0.0"
 
 /* things that live in dummy.c */
 extern struct security_operations dummy_security_ops;
@@ -51,8 +50,7 @@ static void __init do_security_initcalls(void)
  */
 int __init security_init(void)
 {
-	printk(KERN_INFO "Security Framework v" SECURITY_FRAMEWORK_VERSION
-	       " initialized\n");
+	printk(KERN_INFO "Security Framework initialized\n");
 
 	if (verify(&dummy_security_ops)) {
 		printk(KERN_ERR "%s could not verify "
@@ -172,8 +170,5 @@ int mod_unreg_security(const char *name, struct security_operations *ops)
 	return security_ops->unregister_security(name, ops);
 }
 
-EXPORT_SYMBOL_GPL(register_security);
-EXPORT_SYMBOL_GPL(unregister_security);
-EXPORT_SYMBOL_GPL(mod_reg_security);
-EXPORT_SYMBOL_GPL(mod_unreg_security);
+/* Export required for modular use of security.h inlines */
 EXPORT_SYMBOL(security_ops);
diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
index ad8dd4e..105319a 100644
--- a/security/selinux/hooks.c
+++ b/security/selinux/hooks.c
@@ -20,7 +20,6 @@
  *      as published by the Free Software Foundation.
  */
 
-#include <linux/module.h>
 #include <linux/init.h>
 #include <linux/kernel.h>
 #include <linux/ptrace.h>
diff --git a/security/selinux/xfrm.c b/security/selinux/xfrm.c
index bd8d1ef..b589238 100644
--- a/security/selinux/xfrm.c
+++ b/security/selinux/xfrm.c
@@ -31,7 +31,6 @@
  *   2. Emulating a reasonable SO_PEERSEC across machines
  *   3. Testing addition of sk_policy's with security context via setsockopt
  */
-#include <linux/module.h>
 #include <linux/kernel.h>
 #include <linux/init.h>
 #include <linux/security.h>
-- 
1.5.2.1

Re: [PATCH try #2] security: Convert LSM into a static interface

[Petr Vandrovec]

James Morris wrote:
> Convert LSM into a static interface, as the ability to unload a security 
> module is not required by in-tree users and potentially complicates the 
> overall security architecture.

Hello,

> -module_param(debug, bool, 0600);
> -MODULE_PARM_DESC(debug, "Debug enabled or not");
> +static int __init root_plug_debug(char *str)
> +{
> +	debug = simple_strtol(str, NULL, 0);
> +	return 1;
> +}
> +__setup("root_plug_debug=", root_plug_debug);

is this necessary?  What about just documenting 
root_plug.{vendor_id,product_id,debug}, so it won't break existing 
root_plug users (if there are any) ?  I thought that typed 
module_param() is prefered over untyped __setup()...

						Thanks,
							Petr

Re: [PATCH try #2] security: Convert LSM into a static interface

[James Morris]

On Sun, 24 Jun 2007, Petr Vandrovec wrote:

> > -module_param(debug, bool, 0600);
> > -MODULE_PARM_DESC(debug, "Debug enabled or not");
> > +static int __init root_plug_debug(char *str)
> > +{
> > +	debug = simple_strtol(str, NULL, 0);
> > +	return 1;
> > +}
> > +__setup("root_plug_debug=", root_plug_debug);
> 
> is this necessary?  What about just documenting
> root_plug.{vendor_id,product_id,debug}, so it won't break existing root_plug
> users (if there are any) ?  I thought that typed module_param() is prefered
> over untyped __setup()...

I didn't know module_param was preferred.

The idea was that root_plug is example code, and should do the typical 
thing, which I thought would be __setup.

I can easily change it if needed.


-- 
James Morris
<jmorris@namei.org>

Re: [PATCH try #2] security: Convert LSM into a static interface

[Stephen Smalley]

On Mon, 2007-06-25 at 00:33 -0400, James Morris wrote:
> Convert LSM into a static interface, as the ability to unload a security 
> module is not required by in-tree users and potentially complicates the 
> overall security architecture.
> 
> Needlessly exported LSM symbols have been unexported, to help reduce API
> abuse.
> 
> Parameters for the capability and root_plug modules are now specified at 
> boot.
> 
> The SECURITY_FRAMEWORK_VERSION macro has also been removed.
> 
> Signed-off-by: James Morris <jmorris@namei.org>
> ---

> diff --git a/security/security.c b/security/security.c
> index fc8601b..e49dae2 100644
> --- a/security/security.c
> +++ b/security/security.c
> @@ -172,8 +170,5 @@ int mod_unreg_security(const char *name, struct security_operations *ops)
>  	return security_ops->unregister_security(name, ops);
>  }
>  
> -EXPORT_SYMBOL_GPL(register_security);
> -EXPORT_SYMBOL_GPL(unregister_security);
> -EXPORT_SYMBOL_GPL(mod_reg_security);
> -EXPORT_SYMBOL_GPL(mod_unreg_security);
> +/* Export required for modular use of security.h inlines */
>  EXPORT_SYMBOL(security_ops);

I think you want to eliminate that last export too, by taking the
security hooks that are called by modules into out-of-line wrapper
functions in security.c rather than directly referencing security_ops.

-- 
Stephen Smalley
National Security Agency

[PATCH try #3] security: Convert LSM into a static interface

[James Morris]

Convert LSM into a static interface, as the ability to unload a security
module is not required by in-tree users and potentially complicates the
overall security architecture.

Needlessly exported LSM symbols have been unexported, to help reduce API
abuse.

Parameters for the capability and root_plug modules are now specified
at boot.

The SECURITY_FRAMEWORK_VERSION macro has also been removed.

Signed-off-by: James Morris <jmorris@namei.org>
---

Changes in this version:
- retained module params for root_plug
- unexorted security_ops, moved inlines to security.c, exported some 
  needed security hooks

Please review, I've done quite a bit of testing, but it's a large change 
now.


 Documentation/kernel-parameters.txt |   17 +
 include/linux/security.h            | 1171 ++++++-----------------------------
 security/Kconfig                    |    4 +-
 security/capability.c               |   24 -
 security/commoncap.c                |    3 -
 security/dummy.c                    |    1 -
 security/root_plug.c                |   31 +-
 security/security.c                 |  954 ++++++++++++++++++++++++++++-
 security/selinux/hooks.c            |    1 -
 security/selinux/xfrm.c             |    1 -
 10 files changed, 1168 insertions(+), 1039 deletions(-)

diff --git a/Documentation/kernel-parameters.txt b/Documentation/kernel-parameters.txt
index 5d0283c..cd65510 100644
--- a/Documentation/kernel-parameters.txt
+++ b/Documentation/kernel-parameters.txt
@@ -74,10 +74,12 @@ parameter is applicable:
 	PPT	Parallel port support is enabled.
 	PS2	Appropriate PS/2 support is enabled.
 	RAM	RAM disk support is enabled.
+	ROOTPLUG The example Root Plug LSM is enabled.
 	S390	S390 architecture is enabled.
 	SCSI	Appropriate SCSI support is enabled.
 			A lot of drivers has their options described inside of
 			Documentation/scsi/.
+	SECURITY Different security models are enabled.
 	SELINUX SELinux support is enabled.
 	SERIAL	Serial support is enabled.
 	SH	SuperH architecture is enabled.
@@ -376,6 +378,12 @@ and is between 256 and 4096 characters. It is defined in the file
 			possible to determine what the correct size should be.
 			This option provides an override for these situations.
 
+	capability.disable=
+			[SECURITY] Disable capabilities.  This would normally
+			be used only if an alternative security model is to be
+			configured.  Potentially dangerous and should only be
+			used if you are entirely sure of the consequences.
+
 	cdu31a=		[HW,CD]
 			Format: <io>,<irq>[,PAS]
 			See header of drivers/cdrom/cdu31a.c.
@@ -1541,6 +1549,15 @@ and is between 256 and 4096 characters. It is defined in the file
 
 	rootfstype=	[KNL] Set root filesystem type
 
+	root_plug.vendor_id=
+			[ROOTPLUG] Override the default vendor ID
+
+	root_plug.product_id=
+			[ROOTPLUG] Override the default product ID
+
+	root_plug.debug=
+			[ROOTPLUG] Enable debugging output
+
 	rw		[KNL] Mount root device read-write on boot
 
 	S		[KNL] Run init in single mode
diff --git a/include/linux/security.h b/include/linux/security.h
index 9eb9e0f..5f58808 100644
--- a/include/linux/security.h
+++ b/include/linux/security.h
@@ -1398,730 +1398,8 @@ struct security_operations {
 
 };
 
-/* global variables */
 extern struct security_operations *security_ops;
 
-/* inline stuff */
-static inline int security_ptrace (struct task_struct * parent, struct task_struct * child)
-{
-	return security_ops->ptrace (parent, child);
-}
-
-static inline int security_capget (struct task_struct *target,
-				   kernel_cap_t *effective,
-				   kernel_cap_t *inheritable,
-				   kernel_cap_t *permitted)
-{
-	return security_ops->capget (target, effective, inheritable, permitted);
-}
-
-static inline int security_capset_check (struct task_struct *target,
-					 kernel_cap_t *effective,
-					 kernel_cap_t *inheritable,
-					 kernel_cap_t *permitted)
-{
-	return security_ops->capset_check (target, effective, inheritable, permitted);
-}
-
-static inline void security_capset_set (struct task_struct *target,
-					kernel_cap_t *effective,
-					kernel_cap_t *inheritable,
-					kernel_cap_t *permitted)
-{
-	security_ops->capset_set (target, effective, inheritable, permitted);
-}
-
-static inline int security_capable(struct task_struct *tsk, int cap)
-{
-	return security_ops->capable(tsk, cap);
-}
-
-static inline int security_acct (struct file *file)
-{
-	return security_ops->acct (file);
-}
-
-static inline int security_sysctl(struct ctl_table *table, int op)
-{
-	return security_ops->sysctl(table, op);
-}
-
-static inline int security_quotactl (int cmds, int type, int id,
-				     struct super_block *sb)
-{
-	return security_ops->quotactl (cmds, type, id, sb);
-}
-
-static inline int security_quota_on (struct dentry * dentry)
-{
-	return security_ops->quota_on (dentry);
-}
-
-static inline int security_syslog(int type)
-{
-	return security_ops->syslog(type);
-}
-
-static inline int security_settime(struct timespec *ts, struct timezone *tz)
-{
-	return security_ops->settime(ts, tz);
-}
-
-
-static inline int security_vm_enough_memory(long pages)
-{
-	return security_ops->vm_enough_memory(pages);
-}
-
-static inline int security_bprm_alloc (struct linux_binprm *bprm)
-{
-	return security_ops->bprm_alloc_security (bprm);
-}
-static inline void security_bprm_free (struct linux_binprm *bprm)
-{
-	security_ops->bprm_free_security (bprm);
-}
-static inline void security_bprm_apply_creds (struct linux_binprm *bprm, int unsafe)
-{
-	security_ops->bprm_apply_creds (bprm, unsafe);
-}
-static inline void security_bprm_post_apply_creds (struct linux_binprm *bprm)
-{
-	security_ops->bprm_post_apply_creds (bprm);
-}
-static inline int security_bprm_set (struct linux_binprm *bprm)
-{
-	return security_ops->bprm_set_security (bprm);
-}
-
-static inline int security_bprm_check (struct linux_binprm *bprm)
-{
-	return security_ops->bprm_check_security (bprm);
-}
-
-static inline int security_bprm_secureexec (struct linux_binprm *bprm)
-{
-	return security_ops->bprm_secureexec (bprm);
-}
-
-static inline int security_sb_alloc (struct super_block *sb)
-{
-	return security_ops->sb_alloc_security (sb);
-}
-
-static inline void security_sb_free (struct super_block *sb)
-{
-	security_ops->sb_free_security (sb);
-}
-
-static inline int security_sb_copy_data (struct file_system_type *type,
-					 void *orig, void *copy)
-{
-	return security_ops->sb_copy_data (type, orig, copy);
-}
-
-static inline int security_sb_kern_mount (struct super_block *sb, void *data)
-{
-	return security_ops->sb_kern_mount (sb, data);
-}
-
-static inline int security_sb_statfs (struct dentry *dentry)
-{
-	return security_ops->sb_statfs (dentry);
-}
-
-static inline int security_sb_mount (char *dev_name, struct nameidata *nd,
-				    char *type, unsigned long flags,
-				    void *data)
-{
-	return security_ops->sb_mount (dev_name, nd, type, flags, data);
-}
-
-static inline int security_sb_check_sb (struct vfsmount *mnt,
-					struct nameidata *nd)
-{
-	return security_ops->sb_check_sb (mnt, nd);
-}
-
-static inline int security_sb_umount (struct vfsmount *mnt, int flags)
-{
-	return security_ops->sb_umount (mnt, flags);
-}
-
-static inline void security_sb_umount_close (struct vfsmount *mnt)
-{
-	security_ops->sb_umount_close (mnt);
-}
-
-static inline void security_sb_umount_busy (struct vfsmount *mnt)
-{
-	security_ops->sb_umount_busy (mnt);
-}
-
-static inline void security_sb_post_remount (struct vfsmount *mnt,
-					     unsigned long flags, void *data)
-{
-	security_ops->sb_post_remount (mnt, flags, data);
-}
-
-static inline void security_sb_post_mountroot (void)
-{
-	security_ops->sb_post_mountroot ();
-}
-
-static inline void security_sb_post_addmount (struct vfsmount *mnt,
-					      struct nameidata *mountpoint_nd)
-{
-	security_ops->sb_post_addmount (mnt, mountpoint_nd);
-}
-
-static inline int security_sb_pivotroot (struct nameidata *old_nd,
-					 struct nameidata *new_nd)
-{
-	return security_ops->sb_pivotroot (old_nd, new_nd);
-}
-
-static inline void security_sb_post_pivotroot (struct nameidata *old_nd,
-					       struct nameidata *new_nd)
-{
-	security_ops->sb_post_pivotroot (old_nd, new_nd);
-}
-
-static inline int security_inode_alloc (struct inode *inode)
-{
-	inode->i_security = NULL;
-	return security_ops->inode_alloc_security (inode);
-}
-
-static inline void security_inode_free (struct inode *inode)
-{
-	security_ops->inode_free_security (inode);
-}
-
-static inline int security_inode_init_security (struct inode *inode,
-						struct inode *dir,
-						char **name,
-						void **value,
-						size_t *len)
-{
-	if (unlikely (IS_PRIVATE (inode)))
-		return -EOPNOTSUPP;
-	return security_ops->inode_init_security (inode, dir, name, value, len);
-}
-	
-static inline int security_inode_create (struct inode *dir,
-					 struct dentry *dentry,
-					 int mode)
-{
-	if (unlikely (IS_PRIVATE (dir)))
-		return 0;
-	return security_ops->inode_create (dir, dentry, mode);
-}
-
-static inline int security_inode_link (struct dentry *old_dentry,
-				       struct inode *dir,
-				       struct dentry *new_dentry)
-{
-	if (unlikely (IS_PRIVATE (old_dentry->d_inode)))
-		return 0;
-	return security_ops->inode_link (old_dentry, dir, new_dentry);
-}
-
-static inline int security_inode_unlink (struct inode *dir,
-					 struct dentry *dentry)
-{
-	if (unlikely (IS_PRIVATE (dentry->d_inode)))
-		return 0;
-	return security_ops->inode_unlink (dir, dentry);
-}
-
-static inline int security_inode_symlink (struct inode *dir,
-					  struct dentry *dentry,
-					  const char *old_name)
-{
-	if (unlikely (IS_PRIVATE (dir)))
-		return 0;
-	return security_ops->inode_symlink (dir, dentry, old_name);
-}
-
-static inline int security_inode_mkdir (struct inode *dir,
-					struct dentry *dentry,
-					int mode)
-{
-	if (unlikely (IS_PRIVATE (dir)))
-		return 0;
-	return security_ops->inode_mkdir (dir, dentry, mode);
-}
-
-static inline int security_inode_rmdir (struct inode *dir,
-					struct dentry *dentry)
-{
-	if (unlikely (IS_PRIVATE (dentry->d_inode)))
-		return 0;
-	return security_ops->inode_rmdir (dir, dentry);
-}
-
-static inline int security_inode_mknod (struct inode *dir,
-					struct dentry *dentry,
-					int mode, dev_t dev)
-{
-	if (unlikely (IS_PRIVATE (dir)))
-		return 0;
-	return security_ops->inode_mknod (dir, dentry, mode, dev);
-}
-
-static inline int security_inode_rename (struct inode *old_dir,
-					 struct dentry *old_dentry,
-					 struct inode *new_dir,
-					 struct dentry *new_dentry)
-{
-        if (unlikely (IS_PRIVATE (old_dentry->d_inode) ||
-            (new_dentry->d_inode && IS_PRIVATE (new_dentry->d_inode))))
-		return 0;
-	return security_ops->inode_rename (old_dir, old_dentry,
-					   new_dir, new_dentry);
-}
-
-static inline int security_inode_readlink (struct dentry *dentry)
-{
-	if (unlikely (IS_PRIVATE (dentry->d_inode)))
-		return 0;
-	return security_ops->inode_readlink (dentry);
-}
-
-static inline int security_inode_follow_link (struct dentry *dentry,
-					      struct nameidata *nd)
-{
-	if (unlikely (IS_PRIVATE (dentry->d_inode)))
-		return 0;
-	return security_ops->inode_follow_link (dentry, nd);
-}
-
-static inline int security_inode_permission (struct inode *inode, int mask,
-					     struct nameidata *nd)
-{
-	if (unlikely (IS_PRIVATE (inode)))
-		return 0;
-	return security_ops->inode_permission (inode, mask, nd);
-}
-
-static inline int security_inode_setattr (struct dentry *dentry,
-					  struct iattr *attr)
-{
-	if (unlikely (IS_PRIVATE (dentry->d_inode)))
-		return 0;
-	return security_ops->inode_setattr (dentry, attr);
-}
-
-static inline int security_inode_getattr (struct vfsmount *mnt,
-					  struct dentry *dentry)
-{
-	if (unlikely (IS_PRIVATE (dentry->d_inode)))
-		return 0;
-	return security_ops->inode_getattr (mnt, dentry);
-}
-
-static inline void security_inode_delete (struct inode *inode)
-{
-	if (unlikely (IS_PRIVATE (inode)))
-		return;
-	security_ops->inode_delete (inode);
-}
-
-static inline int security_inode_setxattr (struct dentry *dentry, char *name,
-					   void *value, size_t size, int flags)
-{
-	if (unlikely (IS_PRIVATE (dentry->d_inode)))
-		return 0;
-	return security_ops->inode_setxattr (dentry, name, value, size, flags);
-}
-
-static inline void security_inode_post_setxattr (struct dentry *dentry, char *name,
-						void *value, size_t size, int flags)
-{
-	if (unlikely (IS_PRIVATE (dentry->d_inode)))
-		return;
-	security_ops->inode_post_setxattr (dentry, name, value, size, flags);
-}
-
-static inline int security_inode_getxattr (struct dentry *dentry, char *name)
-{
-	if (unlikely (IS_PRIVATE (dentry->d_inode)))
-		return 0;
-	return security_ops->inode_getxattr (dentry, name);
-}
-
-static inline int security_inode_listxattr (struct dentry *dentry)
-{
-	if (unlikely (IS_PRIVATE (dentry->d_inode)))
-		return 0;
-	return security_ops->inode_listxattr (dentry);
-}
-
-static inline int security_inode_removexattr (struct dentry *dentry, char *name)
-{
-	if (unlikely (IS_PRIVATE (dentry->d_inode)))
-		return 0;
-	return security_ops->inode_removexattr (dentry, name);
-}
-
-static inline const char *security_inode_xattr_getsuffix(void)
-{
-	return security_ops->inode_xattr_getsuffix();
-}
-
-static inline int security_inode_getsecurity(const struct inode *inode, const char *name, void *buffer, size_t size, int err)
-{
-	if (unlikely (IS_PRIVATE (inode)))
-		return 0;
-	return security_ops->inode_getsecurity(inode, name, buffer, size, err);
-}
-
-static inline int security_inode_setsecurity(struct inode *inode, const char *name, const void *value, size_t size, int flags)
-{
-	if (unlikely (IS_PRIVATE (inode)))
-		return 0;
-	return security_ops->inode_setsecurity(inode, name, value, size, flags);
-}
-
-static inline int security_inode_listsecurity(struct inode *inode, char *buffer, size_t buffer_size)
-{
-	if (unlikely (IS_PRIVATE (inode)))
-		return 0;
-	return security_ops->inode_listsecurity(inode, buffer, buffer_size);
-}
-
-static inline int security_file_permission (struct file *file, int mask)
-{
-	return security_ops->file_permission (file, mask);
-}
-
-static inline int security_file_alloc (struct file *file)
-{
-	return security_ops->file_alloc_security (file);
-}
-
-static inline void security_file_free (struct file *file)
-{
-	security_ops->file_free_security (file);
-}
-
-static inline int security_file_ioctl (struct file *file, unsigned int cmd,
-				       unsigned long arg)
-{
-	return security_ops->file_ioctl (file, cmd, arg);
-}
-
-static inline int security_file_mmap (struct file *file, unsigned long reqprot,
-				      unsigned long prot,
-				      unsigned long flags)
-{
-	return security_ops->file_mmap (file, reqprot, prot, flags);
-}
-
-static inline int security_file_mprotect (struct vm_area_struct *vma,
-					  unsigned long reqprot,
-					  unsigned long prot)
-{
-	return security_ops->file_mprotect (vma, reqprot, prot);
-}
-
-static inline int security_file_lock (struct file *file, unsigned int cmd)
-{
-	return security_ops->file_lock (file, cmd);
-}
-
-static inline int security_file_fcntl (struct file *file, unsigned int cmd,
-				       unsigned long arg)
-{
-	return security_ops->file_fcntl (file, cmd, arg);
-}
-
-static inline int security_file_set_fowner (struct file *file)
-{
-	return security_ops->file_set_fowner (file);
-}
-
-static inline int security_file_send_sigiotask (struct task_struct *tsk,
-						struct fown_struct *fown,
-						int sig)
-{
-	return security_ops->file_send_sigiotask (tsk, fown, sig);
-}
-
-static inline int security_file_receive (struct file *file)
-{
-	return security_ops->file_receive (file);
-}
-
-static inline int security_task_create (unsigned long clone_flags)
-{
-	return security_ops->task_create (clone_flags);
-}
-
-static inline int security_task_alloc (struct task_struct *p)
-{
-	return security_ops->task_alloc_security (p);
-}
-
-static inline void security_task_free (struct task_struct *p)
-{
-	security_ops->task_free_security (p);
-}
-
-static inline int security_task_setuid (uid_t id0, uid_t id1, uid_t id2,
-					int flags)
-{
-	return security_ops->task_setuid (id0, id1, id2, flags);
-}
-
-static inline int security_task_post_setuid (uid_t old_ruid, uid_t old_euid,
-					     uid_t old_suid, int flags)
-{
-	return security_ops->task_post_setuid (old_ruid, old_euid, old_suid, flags);
-}
-
-static inline int security_task_setgid (gid_t id0, gid_t id1, gid_t id2,
-					int flags)
-{
-	return security_ops->task_setgid (id0, id1, id2, flags);
-}
-
-static inline int security_task_setpgid (struct task_struct *p, pid_t pgid)
-{
-	return security_ops->task_setpgid (p, pgid);
-}
-
-static inline int security_task_getpgid (struct task_struct *p)
-{
-	return security_ops->task_getpgid (p);
-}
-
-static inline int security_task_getsid (struct task_struct *p)
-{
-	return security_ops->task_getsid (p);
-}
-
-static inline void security_task_getsecid (struct task_struct *p, u32 *secid)
-{
-	security_ops->task_getsecid (p, secid);
-}
-
-static inline int security_task_setgroups (struct group_info *group_info)
-{
-	return security_ops->task_setgroups (group_info);
-}
-
-static inline int security_task_setnice (struct task_struct *p, int nice)
-{
-	return security_ops->task_setnice (p, nice);
-}
-
-static inline int security_task_setioprio (struct task_struct *p, int ioprio)
-{
-	return security_ops->task_setioprio (p, ioprio);
-}
-
-static inline int security_task_getioprio (struct task_struct *p)
-{
-	return security_ops->task_getioprio (p);
-}
-
-static inline int security_task_setrlimit (unsigned int resource,
-					   struct rlimit *new_rlim)
-{
-	return security_ops->task_setrlimit (resource, new_rlim);
-}
-
-static inline int security_task_setscheduler (struct task_struct *p,
-					      int policy,
-					      struct sched_param *lp)
-{
-	return security_ops->task_setscheduler (p, policy, lp);
-}
-
-static inline int security_task_getscheduler (struct task_struct *p)
-{
-	return security_ops->task_getscheduler (p);
-}
-
-static inline int security_task_movememory (struct task_struct *p)
-{
-	return security_ops->task_movememory (p);
-}
-
-static inline int security_task_kill (struct task_struct *p,
-				      struct siginfo *info, int sig,
-				      u32 secid)
-{
-	return security_ops->task_kill (p, info, sig, secid);
-}
-
-static inline int security_task_wait (struct task_struct *p)
-{
-	return security_ops->task_wait (p);
-}
-
-static inline int security_task_prctl (int option, unsigned long arg2,
-				       unsigned long arg3,
-				       unsigned long arg4,
-				       unsigned long arg5)
-{
-	return security_ops->task_prctl (option, arg2, arg3, arg4, arg5);
-}
-
-static inline void security_task_reparent_to_init (struct task_struct *p)
-{
-	security_ops->task_reparent_to_init (p);
-}
-
-static inline void security_task_to_inode(struct task_struct *p, struct inode *inode)
-{
-	security_ops->task_to_inode(p, inode);
-}
-
-static inline int security_ipc_permission (struct kern_ipc_perm *ipcp,
-					   short flag)
-{
-	return security_ops->ipc_permission (ipcp, flag);
-}
-
-static inline int security_msg_msg_alloc (struct msg_msg * msg)
-{
-	return security_ops->msg_msg_alloc_security (msg);
-}
-
-static inline void security_msg_msg_free (struct msg_msg * msg)
-{
-	security_ops->msg_msg_free_security(msg);
-}
-
-static inline int security_msg_queue_alloc (struct msg_queue *msq)
-{
-	return security_ops->msg_queue_alloc_security (msq);
-}
-
-static inline void security_msg_queue_free (struct msg_queue *msq)
-{
-	security_ops->msg_queue_free_security (msq);
-}
-
-static inline int security_msg_queue_associate (struct msg_queue * msq, 
-						int msqflg)
-{
-	return security_ops->msg_queue_associate (msq, msqflg);
-}
-
-static inline int security_msg_queue_msgctl (struct msg_queue * msq, int cmd)
-{
-	return security_ops->msg_queue_msgctl (msq, cmd);
-}
-
-static inline int security_msg_queue_msgsnd (struct msg_queue * msq,
-					     struct msg_msg * msg, int msqflg)
-{
-	return security_ops->msg_queue_msgsnd (msq, msg, msqflg);
-}
-
-static inline int security_msg_queue_msgrcv (struct msg_queue * msq,
-					     struct msg_msg * msg,
-					     struct task_struct * target,
-					     long type, int mode)
-{
-	return security_ops->msg_queue_msgrcv (msq, msg, target, type, mode);
-}
-
-static inline int security_shm_alloc (struct shmid_kernel *shp)
-{
-	return security_ops->shm_alloc_security (shp);
-}
-
-static inline void security_shm_free (struct shmid_kernel *shp)
-{
-	security_ops->shm_free_security (shp);
-}
-
-static inline int security_shm_associate (struct shmid_kernel * shp, 
-					  int shmflg)
-{
-	return security_ops->shm_associate(shp, shmflg);
-}
-
-static inline int security_shm_shmctl (struct shmid_kernel * shp, int cmd)
-{
-	return security_ops->shm_shmctl (shp, cmd);
-}
-
-static inline int security_shm_shmat (struct shmid_kernel * shp, 
-				      char __user *shmaddr, int shmflg)
-{
-	return security_ops->shm_shmat(shp, shmaddr, shmflg);
-}
-
-static inline int security_sem_alloc (struct sem_array *sma)
-{
-	return security_ops->sem_alloc_security (sma);
-}
-
-static inline void security_sem_free (struct sem_array *sma)
-{
-	security_ops->sem_free_security (sma);
-}
-
-static inline int security_sem_associate (struct sem_array * sma, int semflg)
-{
-	return security_ops->sem_associate (sma, semflg);
-}
-
-static inline int security_sem_semctl (struct sem_array * sma, int cmd)
-{
-	return security_ops->sem_semctl(sma, cmd);
-}
-
-static inline int security_sem_semop (struct sem_array * sma, 
-				      struct sembuf * sops, unsigned nsops, 
-				      int alter)
-{
-	return security_ops->sem_semop(sma, sops, nsops, alter);
-}
-
-static inline void security_d_instantiate (struct dentry *dentry, struct inode *inode)
-{
-	if (unlikely (inode && IS_PRIVATE (inode)))
-		return;
-	security_ops->d_instantiate (dentry, inode);
-}
-
-static inline int security_getprocattr(struct task_struct *p, char *name, char **value)
-{
-	return security_ops->getprocattr(p, name, value);
-}
-
-static inline int security_setprocattr(struct task_struct *p, char *name, void *value, size_t size)
-{
-	return security_ops->setprocattr(p, name, value, size);
-}
-
-static inline int security_netlink_send(struct sock *sk, struct sk_buff * skb)
-{
-	return security_ops->netlink_send(sk, skb);
-}
-
-static inline int security_netlink_recv(struct sk_buff * skb, int cap)
-{
-	return security_ops->netlink_recv(skb, cap);
-}
-
-static inline int security_secid_to_secctx(u32 secid, char **secdata, u32 *seclen)
-{
-	return security_ops->secid_to_secctx(secid, secdata, seclen);
-}
-
-static inline void security_release_secctx(char *secdata, u32 seclen)
-{
-	return security_ops->release_secctx(secdata, seclen);
-}
-
 /* prototypes */
 extern int security_init	(void);
 extern int register_security	(struct security_operations *ops);
@@ -2135,6 +1413,154 @@ extern struct dentry *securityfs_create_dir(const char *name, struct dentry *par
 extern void securityfs_remove(struct dentry *dentry);
 
 
+/* Security operations */
+int security_ptrace (struct task_struct * parent, struct task_struct * child);
+int security_capget (struct task_struct *target,
+		     kernel_cap_t *effective,
+		     kernel_cap_t *inheritable,
+		     kernel_cap_t *permitted);
+int security_capset_check (struct task_struct *target,
+			   kernel_cap_t *effective,
+			   kernel_cap_t *inheritable,
+			   kernel_cap_t *permitted);
+void security_capset_set (struct task_struct *target,
+			  kernel_cap_t *effective,
+			  kernel_cap_t *inheritable,
+			  kernel_cap_t *permitted);
+int security_capable(struct task_struct *tsk, int cap);
+int security_acct (struct file *file);
+int security_sysctl(struct ctl_table *table, int op);
+int security_quotactl (int cmds, int type, int id, struct super_block *sb);
+int security_quota_on (struct dentry * dentry);
+int security_syslog(int type);
+int security_settime(struct timespec *ts, struct timezone *tz);
+int security_vm_enough_memory(long pages);
+int security_bprm_alloc (struct linux_binprm *bprm);
+void security_bprm_free (struct linux_binprm *bprm);
+void security_bprm_apply_creds (struct linux_binprm *bprm, int unsafe);
+void security_bprm_post_apply_creds (struct linux_binprm *bprm);
+int security_bprm_set (struct linux_binprm *bprm);
+int security_bprm_check (struct linux_binprm *bprm);
+int security_bprm_secureexec (struct linux_binprm *bprm);
+int security_sb_alloc (struct super_block *sb);
+void security_sb_free (struct super_block *sb);
+int security_sb_copy_data (struct file_system_type *type, void *orig, void *copy);
+int security_sb_kern_mount (struct super_block *sb, void *data);
+int security_sb_statfs (struct dentry *dentry);
+int security_sb_mount (char *dev_name, struct nameidata *nd,
+                       char *type, unsigned long flags, void *data);
+int security_sb_check_sb (struct vfsmount *mnt, struct nameidata *nd);
+int security_sb_umount (struct vfsmount *mnt, int flags);
+void security_sb_umount_close (struct vfsmount *mnt);
+void security_sb_umount_busy (struct vfsmount *mnt);
+void security_sb_post_remount (struct vfsmount *mnt, unsigned long flags, void *data);
+void security_sb_post_mountroot (void);
+void security_sb_post_addmount (struct vfsmount *mnt, struct nameidata *mountpoint_nd);
+int security_sb_pivotroot (struct nameidata *old_nd, struct nameidata *new_nd);
+void security_sb_post_pivotroot (struct nameidata *old_nd, struct nameidata *new_nd);
+int security_inode_alloc (struct inode *inode);
+void security_inode_free (struct inode *inode);
+int security_inode_init_security (struct inode *inode, struct inode *dir,
+				  char **name, void **value, size_t *len);
+int security_inode_create (struct inode *dir, struct dentry *dentry, int mode);
+int security_inode_link (struct dentry *old_dentry, struct inode *dir,
+			 struct dentry *new_dentry);
+int security_inode_unlink (struct inode *dir, struct dentry *dentry);
+int security_inode_symlink (struct inode *dir, struct dentry *dentry,
+			    const char *old_name);
+int security_inode_mkdir (struct inode *dir, struct dentry *dentry, int mode);
+int security_inode_rmdir (struct inode *dir, struct dentry *dentry);
+int security_inode_mknod (struct inode *dir, struct dentry *dentry, int mode, dev_t dev);
+int security_inode_rename (struct inode *old_dir, struct dentry *old_dentry,
+			   struct inode *new_dir, struct dentry *new_dentry);
+int security_inode_readlink (struct dentry *dentry);
+int security_inode_follow_link (struct dentry *dentry, struct nameidata *nd);
+int security_inode_permission (struct inode *inode, int mask, struct nameidata *nd);
+int security_inode_setattr (struct dentry *dentry, struct iattr *attr);
+int security_inode_getattr (struct vfsmount *mnt, struct dentry *dentry);
+void security_inode_delete (struct inode *inode);
+int security_inode_setxattr (struct dentry *dentry, char *name,
+			     void *value, size_t size, int flags);
+void security_inode_post_setxattr (struct dentry *dentry, char *name,
+				   void *value, size_t size, int flags);
+int security_inode_getxattr (struct dentry *dentry, char *name);
+int security_inode_listxattr (struct dentry *dentry);
+int security_inode_removexattr (struct dentry *dentry, char *name);
+const char *security_inode_xattr_getsuffix(void);
+int security_inode_getsecurity(const struct inode *inode, const char *name, void *buffer, size_t size, int err);
+int security_inode_setsecurity(struct inode *inode, const char *name, const void *value, size_t size, int flags);
+int security_inode_listsecurity(struct inode *inode, char *buffer, size_t buffer_size);
+int security_file_permission (struct file *file, int mask);
+int security_file_alloc (struct file *file);
+void security_file_free (struct file *file);
+int security_file_ioctl (struct file *file, unsigned int cmd, unsigned long arg);
+int security_file_mmap (struct file *file, unsigned long reqprot,
+			unsigned long prot, unsigned long flags);
+int security_file_mprotect (struct vm_area_struct *vma, unsigned long reqprot,
+			    unsigned long prot);
+int security_file_lock (struct file *file, unsigned int cmd);
+int security_file_fcntl (struct file *file, unsigned int cmd, unsigned long arg);
+int security_file_set_fowner (struct file *file);
+int security_file_send_sigiotask (struct task_struct *tsk,
+				  struct fown_struct *fown, int sig);
+int security_file_receive (struct file *file);
+int security_task_create (unsigned long clone_flags);
+int security_task_alloc (struct task_struct *p);
+void security_task_free (struct task_struct *p);
+int security_task_setuid (uid_t id0, uid_t id1, uid_t id2, int flags);
+int security_task_post_setuid (uid_t old_ruid, uid_t old_euid,
+			       uid_t old_suid, int flags);
+int security_task_setgid (gid_t id0, gid_t id1, gid_t id2, int flags);
+int security_task_setpgid (struct task_struct *p, pid_t pgid);
+int security_task_getpgid (struct task_struct *p);
+int security_task_getsid (struct task_struct *p);
+void security_task_getsecid (struct task_struct *p, u32 *secid);
+int security_task_setgroups (struct group_info *group_info);
+int security_task_setnice (struct task_struct *p, int nice);
+int security_task_setioprio (struct task_struct *p, int ioprio);
+int security_task_getioprio (struct task_struct *p);
+int security_task_setrlimit (unsigned int resource, struct rlimit *new_rlim);
+int security_task_setscheduler (struct task_struct *p,
+				int policy, struct sched_param *lp);
+int security_task_getscheduler (struct task_struct *p);
+int security_task_movememory (struct task_struct *p);
+int security_task_kill (struct task_struct *p, struct siginfo *info,
+			int sig, u32 secid);
+int security_task_wait (struct task_struct *p);
+int security_task_prctl (int option, unsigned long arg2, unsigned long arg3,
+			 unsigned long arg4, unsigned long arg5);
+void security_task_reparent_to_init (struct task_struct *p);
+void security_task_to_inode(struct task_struct *p, struct inode *inode);
+int security_ipc_permission (struct kern_ipc_perm *ipcp, short flag);
+int security_msg_msg_alloc (struct msg_msg * msg);
+void security_msg_msg_free (struct msg_msg * msg);
+int security_msg_queue_alloc (struct msg_queue *msq);
+void security_msg_queue_free (struct msg_queue *msq);
+int security_msg_queue_associate (struct msg_queue * msq, int msqflg);
+int security_msg_queue_msgctl (struct msg_queue * msq, int cmd);
+int security_msg_queue_msgsnd (struct msg_queue * msq,
+			       struct msg_msg * msg, int msqflg);
+int security_msg_queue_msgrcv (struct msg_queue * msq, struct msg_msg * msg,
+			       struct task_struct * target, long type, int mode);
+int security_shm_alloc (struct shmid_kernel *shp);
+void security_shm_free (struct shmid_kernel *shp);
+int security_shm_associate (struct shmid_kernel * shp, int shmflg);
+int security_shm_shmctl (struct shmid_kernel * shp, int cmd);
+int security_shm_shmat (struct shmid_kernel * shp, char __user *shmaddr, int shmflg);
+int security_sem_alloc (struct sem_array *sma);
+void security_sem_free (struct sem_array *sma);
+int security_sem_associate (struct sem_array * sma, int semflg);
+int security_sem_semctl (struct sem_array * sma, int cmd);
+int security_sem_semop (struct sem_array * sma, struct sembuf * sops,
+			unsigned nsops, int alter);
+void security_d_instantiate (struct dentry *dentry, struct inode *inode);
+int security_getprocattr(struct task_struct *p, char *name, char **value);
+int security_setprocattr(struct task_struct *p, char *name, void *value, size_t size);
+int security_netlink_send(struct sock *sk, struct sk_buff * skb);
+int security_netlink_recv(struct sk_buff * skb, int cap);
+int security_secid_to_secctx(u32 secid, char **secdata, u32 *seclen);
+void security_release_secctx(char *secdata, u32 seclen);
+
 #else /* CONFIG_SECURITY */
 
 /*
@@ -2799,170 +2225,43 @@ static inline void security_release_secctx(char *secdata, u32 seclen)
 #endif	/* CONFIG_SECURITY */
 
 #ifdef CONFIG_SECURITY_NETWORK
-static inline int security_unix_stream_connect(struct socket * sock,
-					       struct socket * other, 
-					       struct sock * newsk)
-{
-	return security_ops->unix_stream_connect(sock, other, newsk);
-}
 
+int security_unix_stream_connect(struct socket * sock, struct socket * other,
+				 struct sock * newsk);
+int security_unix_may_send(struct socket * sock,  struct socket * other);
+int security_socket_create (int family, int type, int protocol, int kern);
+int security_socket_post_create(struct socket * sock, int family,
+				int type, int protocol, int kern);
+int security_socket_bind(struct socket * sock, struct sockaddr * address, int addrlen);
+int security_socket_connect(struct socket * sock, struct sockaddr * address, int addrlen);
+int security_socket_listen(struct socket * sock, int backlog);
+int security_socket_accept(struct socket * sock, struct socket * newsock);
+void security_socket_post_accept(struct socket * sock, struct socket * newsock);
+int security_socket_sendmsg(struct socket * sock, struct msghdr * msg, int size);
+int security_socket_recvmsg(struct socket * sock, struct msghdr * msg,
+			    int size, int flags);
+int security_socket_getsockname(struct socket * sock);
+int security_socket_getpeername(struct socket * sock);
+int security_socket_getsockopt(struct socket * sock, int level, int optname);
+int security_socket_setsockopt(struct socket * sock, int level, int optname);
+int security_socket_shutdown(struct socket * sock, int how);
+int security_sock_rcv_skb (struct sock * sk, struct sk_buff * skb);
+int security_socket_getpeersec_stream(struct socket *sock, char __user *optval,
+				      int __user *optlen, unsigned len);
+int security_socket_getpeersec_dgram(struct socket *sock, struct sk_buff *skb, u32 *secid);
+int security_sk_alloc(struct sock *sk, int family, gfp_t priority);
+void security_sk_free(struct sock *sk);
+void security_sk_clone(const struct sock *sk, struct sock *newsk);
+void security_sk_classify_flow(struct sock *sk, struct flowi *fl);
+void security_req_classify_flow(const struct request_sock *req, struct flowi *fl);
+void security_sock_graft(struct sock* sk, struct socket *parent);
+int security_inet_conn_request(struct sock *sk,
+			struct sk_buff *skb, struct request_sock *req);
+void security_inet_csk_clone(struct sock *newsk,
+			const struct request_sock *req);
+void security_inet_conn_established(struct sock *sk,
+			struct sk_buff *skb);
 
-static inline int security_unix_may_send(struct socket * sock, 
-					 struct socket * other)
-{
-	return security_ops->unix_may_send(sock, other);
-}
-
-static inline int security_socket_create (int family, int type,
-					  int protocol, int kern)
-{
-	return security_ops->socket_create(family, type, protocol, kern);
-}
-
-static inline int security_socket_post_create(struct socket * sock,
-					      int family,
-					      int type,
-					      int protocol, int kern)
-{
-	return security_ops->socket_post_create(sock, family, type,
-						protocol, kern);
-}
-
-static inline int security_socket_bind(struct socket * sock, 
-				       struct sockaddr * address, 
-				       int addrlen)
-{
-	return security_ops->socket_bind(sock, address, addrlen);
-}
-
-static inline int security_socket_connect(struct socket * sock, 
-					  struct sockaddr * address, 
-					  int addrlen)
-{
-	return security_ops->socket_connect(sock, address, addrlen);
-}
-
-static inline int security_socket_listen(struct socket * sock, int backlog)
-{
-	return security_ops->socket_listen(sock, backlog);
-}
-
-static inline int security_socket_accept(struct socket * sock, 
-					 struct socket * newsock)
-{
-	return security_ops->socket_accept(sock, newsock);
-}
-
-static inline void security_socket_post_accept(struct socket * sock, 
-					       struct socket * newsock)
-{
-	security_ops->socket_post_accept(sock, newsock);
-}
-
-static inline int security_socket_sendmsg(struct socket * sock, 
-					  struct msghdr * msg, int size)
-{
-	return security_ops->socket_sendmsg(sock, msg, size);
-}
-
-static inline int security_socket_recvmsg(struct socket * sock, 
-					  struct msghdr * msg, int size, 
-					  int flags)
-{
-	return security_ops->socket_recvmsg(sock, msg, size, flags);
-}
-
-static inline int security_socket_getsockname(struct socket * sock)
-{
-	return security_ops->socket_getsockname(sock);
-}
-
-static inline int security_socket_getpeername(struct socket * sock)
-{
-	return security_ops->socket_getpeername(sock);
-}
-
-static inline int security_socket_getsockopt(struct socket * sock, 
-					     int level, int optname)
-{
-	return security_ops->socket_getsockopt(sock, level, optname);
-}
-
-static inline int security_socket_setsockopt(struct socket * sock, 
-					     int level, int optname)
-{
-	return security_ops->socket_setsockopt(sock, level, optname);
-}
-
-static inline int security_socket_shutdown(struct socket * sock, int how)
-{
-	return security_ops->socket_shutdown(sock, how);
-}
-
-static inline int security_sock_rcv_skb (struct sock * sk, 
-					 struct sk_buff * skb)
-{
-	return security_ops->socket_sock_rcv_skb (sk, skb);
-}
-
-static inline int security_socket_getpeersec_stream(struct socket *sock, char __user *optval,
-						    int __user *optlen, unsigned len)
-{
-	return security_ops->socket_getpeersec_stream(sock, optval, optlen, len);
-}
-
-static inline int security_socket_getpeersec_dgram(struct socket *sock, struct sk_buff *skb, u32 *secid)
-{
-	return security_ops->socket_getpeersec_dgram(sock, skb, secid);
-}
-
-static inline int security_sk_alloc(struct sock *sk, int family, gfp_t priority)
-{
-	return security_ops->sk_alloc_security(sk, family, priority);
-}
-
-static inline void security_sk_free(struct sock *sk)
-{
-	return security_ops->sk_free_security(sk);
-}
-
-static inline void security_sk_clone(const struct sock *sk, struct sock *newsk)
-{
-	return security_ops->sk_clone_security(sk, newsk);
-}
-
-static inline void security_sk_classify_flow(struct sock *sk, struct flowi *fl)
-{
-	security_ops->sk_getsecid(sk, &fl->secid);
-}
-
-static inline void security_req_classify_flow(const struct request_sock *req, struct flowi *fl)
-{
-	security_ops->req_classify_flow(req, fl);
-}
-
-static inline void security_sock_graft(struct sock* sk, struct socket *parent)
-{
-	security_ops->sock_graft(sk, parent);
-}
-
-static inline int security_inet_conn_request(struct sock *sk,
-			struct sk_buff *skb, struct request_sock *req)
-{
-	return security_ops->inet_conn_request(sk, skb, req);
-}
-
-static inline void security_inet_csk_clone(struct sock *newsk,
-			const struct request_sock *req)
-{
-	security_ops->inet_csk_clone(newsk, req);
-}
-
-static inline void security_inet_conn_established(struct sock *sk,
-			struct sk_buff *skb)
-{
-	security_ops->inet_conn_established(sk, skb);
-}
 #else	/* CONFIG_SECURITY_NETWORK */
 static inline int security_unix_stream_connect(struct socket * sock,
 					       struct socket * other,
@@ -3120,77 +2419,24 @@ static inline void security_inet_conn_established(struct sock *sk,
 #endif	/* CONFIG_SECURITY_NETWORK */
 
 #ifdef CONFIG_SECURITY_NETWORK_XFRM
-static inline int security_xfrm_policy_alloc(struct xfrm_policy *xp, struct xfrm_user_sec_ctx *sec_ctx)
-{
-	return security_ops->xfrm_policy_alloc_security(xp, sec_ctx);
-}
-
-static inline int security_xfrm_policy_clone(struct xfrm_policy *old, struct xfrm_policy *new)
-{
-	return security_ops->xfrm_policy_clone_security(old, new);
-}
-
-static inline void security_xfrm_policy_free(struct xfrm_policy *xp)
-{
-	security_ops->xfrm_policy_free_security(xp);
-}
-
-static inline int security_xfrm_policy_delete(struct xfrm_policy *xp)
-{
-	return security_ops->xfrm_policy_delete_security(xp);
-}
-
-static inline int security_xfrm_state_alloc(struct xfrm_state *x,
-			struct xfrm_user_sec_ctx *sec_ctx)
-{
-	return security_ops->xfrm_state_alloc_security(x, sec_ctx, 0);
-}
-
-static inline int security_xfrm_state_alloc_acquire(struct xfrm_state *x,
-				struct xfrm_sec_ctx *polsec, u32 secid)
-{
-	if (!polsec)
-		return 0;
-	/*
-	 * We want the context to be taken from secid which is usually
-	 * from the sock.
-	 */
-	return security_ops->xfrm_state_alloc_security(x, NULL, secid);
-}
-
-static inline int security_xfrm_state_delete(struct xfrm_state *x)
-{
-	return security_ops->xfrm_state_delete_security(x);
-}
-
-static inline void security_xfrm_state_free(struct xfrm_state *x)
-{
-	security_ops->xfrm_state_free_security(x);
-}
-
-static inline int security_xfrm_policy_lookup(struct xfrm_policy *xp, u32 fl_secid, u8 dir)
-{
-	return security_ops->xfrm_policy_lookup(xp, fl_secid, dir);
-}
-
-static inline int security_xfrm_state_pol_flow_match(struct xfrm_state *x,
-			struct xfrm_policy *xp, struct flowi *fl)
-{
-	return security_ops->xfrm_state_pol_flow_match(x, xp, fl);
-}
-
-static inline int security_xfrm_decode_session(struct sk_buff *skb, u32 *secid)
-{
-	return security_ops->xfrm_decode_session(skb, secid, 1);
-}
 
-static inline void security_skb_classify_flow(struct sk_buff *skb, struct flowi *fl)
-{
-	int rc = security_ops->xfrm_decode_session(skb, &fl->secid, 0);
+int security_xfrm_policy_alloc(struct xfrm_policy *xp, struct xfrm_user_sec_ctx *sec_ctx);
+int security_xfrm_policy_clone(struct xfrm_policy *old, struct xfrm_policy *new);
+void security_xfrm_policy_free(struct xfrm_policy *xp);
+int security_xfrm_policy_delete(struct xfrm_policy *xp);
+int security_xfrm_state_alloc(struct xfrm_state *x, struct xfrm_user_sec_ctx *sec_ctx);
+int security_xfrm_state_alloc_acquire(struct xfrm_state *x,
+				      struct xfrm_sec_ctx *polsec, u32 secid);
+int security_xfrm_state_delete(struct xfrm_state *x);
+void security_xfrm_state_free(struct xfrm_state *x);
+int security_xfrm_policy_lookup(struct xfrm_policy *xp, u32 fl_secid, u8 dir);
+int security_xfrm_state_pol_flow_match(struct xfrm_state *x,
+				       struct xfrm_policy *xp, struct flowi *fl);
+int security_xfrm_decode_session(struct sk_buff *skb, u32 *secid);
+void security_skb_classify_flow(struct sk_buff *skb, struct flowi *fl);
 
-	BUG_ON(rc);
-}
 #else	/* CONFIG_SECURITY_NETWORK_XFRM */
+
 static inline int security_xfrm_policy_alloc(struct xfrm_policy *xp, struct xfrm_user_sec_ctx *sec_ctx)
 {
 	return 0;
@@ -3255,24 +2501,11 @@ static inline void security_skb_classify_flow(struct sk_buff *skb, struct flowi
 
 #ifdef CONFIG_KEYS
 #ifdef CONFIG_SECURITY
-static inline int security_key_alloc(struct key *key,
-				     struct task_struct *tsk,
-				     unsigned long flags)
-{
-	return security_ops->key_alloc(key, tsk, flags);
-}
-
-static inline void security_key_free(struct key *key)
-{
-	security_ops->key_free(key);
-}
 
-static inline int security_key_permission(key_ref_t key_ref,
-					  struct task_struct *context,
-					  key_perm_t perm)
-{
-	return security_ops->key_permission(key_ref, context, perm);
-}
+int security_key_alloc(struct key *key, struct task_struct *tsk, unsigned long flags);
+void security_key_free(struct key *key);
+int security_key_permission(key_ref_t key_ref,
+			    struct task_struct *context, key_perm_t perm);
 
 #else
 
diff --git a/security/Kconfig b/security/Kconfig
index 460e5c9..8ae5490 100644
--- a/security/Kconfig
+++ b/security/Kconfig
@@ -74,14 +74,14 @@ config SECURITY_NETWORK_XFRM
 	  If you are unsure how to answer this question, answer N.
 
 config SECURITY_CAPABILITIES
-	tristate "Default Linux Capabilities"
+	bool "Default Linux Capabilities"
 	depends on SECURITY
 	help
 	  This enables the "default" Linux capabilities functionality.
 	  If you are unsure how to answer this question, answer Y.
 
 config SECURITY_ROOTPLUG
-	tristate "Root Plug Support"
+	bool "Root Plug Support"
 	depends on USB && SECURITY
 	help
 	  This is a sample LSM module that should only be used as such.
diff --git a/security/capability.c b/security/capability.c
index 38296a0..fda6a14 100644
--- a/security/capability.c
+++ b/security/capability.c
@@ -8,7 +8,6 @@
  *
  */
 
-#include <linux/module.h>
 #include <linux/init.h>
 #include <linux/kernel.h>
 #include <linux/security.h>
@@ -52,7 +51,6 @@ static int secondary;
 
 static int capability_disable;
 module_param_named(disable, capability_disable, int, 0);
-MODULE_PARM_DESC(disable, "To disable capabilities module set disable = 1");
 
 static int __init capability_init (void)
 {
@@ -75,26 +73,4 @@ static int __init capability_init (void)
 	return 0;
 }
 
-static void __exit capability_exit (void)
-{
-	if (capability_disable)
-		return;
-	/* remove ourselves from the security framework */
-	if (secondary) {
-		if (mod_unreg_security (KBUILD_MODNAME, &capability_ops))
-			printk (KERN_INFO "Failure unregistering capabilities "
-				"with primary module.\n");
-		return;
-	}
-
-	if (unregister_security (&capability_ops)) {
-		printk (KERN_INFO
-			"Failure unregistering capabilities with the kernel\n");
-	}
-}
-
 security_initcall (capability_init);
-module_exit (capability_exit);
-
-MODULE_DESCRIPTION("Standard Linux Capabilities Security Module");
-MODULE_LICENSE("GPL");
diff --git a/security/commoncap.c b/security/commoncap.c
index 384379e..04bd44b 100644
--- a/security/commoncap.c
+++ b/security/commoncap.c
@@ -339,6 +339,3 @@ EXPORT_SYMBOL(cap_task_post_setuid);
 EXPORT_SYMBOL(cap_task_reparent_to_init);
 EXPORT_SYMBOL(cap_syslog);
 EXPORT_SYMBOL(cap_vm_enough_memory);
-
-MODULE_DESCRIPTION("Standard Linux Common Capabilities Security Module");
-MODULE_LICENSE("GPL");
diff --git a/security/dummy.c b/security/dummy.c
index 8ffd764..6d4e34b 100644
--- a/security/dummy.c
+++ b/security/dummy.c
@@ -15,7 +15,6 @@
 #undef DEBUG
 
 #include <linux/capability.h>
-#include <linux/module.h>
 #include <linux/kernel.h>
 #include <linux/mman.h>
 #include <linux/pagemap.h>
diff --git a/security/root_plug.c b/security/root_plug.c
index 38dd4f3..870f130 100644
--- a/security/root_plug.c
+++ b/security/root_plug.c
@@ -22,11 +22,11 @@
  *	License.
  */
 
-#include <linux/module.h>
 #include <linux/kernel.h>
 #include <linux/init.h>
 #include <linux/security.h>
 #include <linux/usb.h>
+#include <linux/moduleparam.h>
 
 /* flag to keep track of how we were registered */
 static int secondary;
@@ -36,22 +36,14 @@ static int vendor_id = 0x0557;
 static int product_id = 0x2008;
 
 module_param(vendor_id, uint, 0400);
-MODULE_PARM_DESC(vendor_id, "USB Vendor ID of device to look for");
-
 module_param(product_id, uint, 0400);
-MODULE_PARM_DESC(product_id, "USB Product ID of device to look for");
 
 /* should we print out debug messages */
 static int debug = 0;
 
 module_param(debug, bool, 0600);
-MODULE_PARM_DESC(debug, "Debug enabled or not");
 
-#if defined(CONFIG_SECURITY_ROOTPLUG_MODULE)
-#define MY_NAME THIS_MODULE->name
-#else
 #define MY_NAME "root_plug"
-#endif
 
 #define root_dbg(fmt, arg...)					\
 	do {							\
@@ -117,25 +109,4 @@ static int __init rootplug_init (void)
 	return 0;
 }
 
-static void __exit rootplug_exit (void)
-{
-	/* remove ourselves from the security framework */
-	if (secondary) {
-		if (mod_unreg_security (MY_NAME, &rootplug_security_ops))
-			printk (KERN_INFO "Failure unregistering Root Plug "
-				" module with primary module.\n");
-	} else { 
-		if (unregister_security (&rootplug_security_ops)) {
-			printk (KERN_INFO "Failure unregistering Root Plug "
-				"module with the kernel\n");
-		}
-	}
-	printk (KERN_INFO "Root Plug module removed\n");
-}
-
 security_initcall (rootplug_init);
-module_exit (rootplug_exit);
-
-MODULE_DESCRIPTION("Root Plug sample LSM module, written for Linux Journal article");
-MODULE_LICENSE("GPL");
-
diff --git a/security/security.c b/security/security.c
index fc8601b..3889cc3 100644
--- a/security/security.c
+++ b/security/security.c
@@ -17,7 +17,6 @@
 #include <linux/kernel.h>
 #include <linux/security.h>
 
-#define SECURITY_FRAMEWORK_VERSION	"1.0.0"
 
 /* things that live in dummy.c */
 extern struct security_operations dummy_security_ops;
@@ -51,8 +50,7 @@ static void __init do_security_initcalls(void)
  */
 int __init security_init(void)
 {
-	printk(KERN_INFO "Security Framework v" SECURITY_FRAMEWORK_VERSION
-	       " initialized\n");
+	printk(KERN_INFO "Security Framework initialized\n");
 
 	if (verify(&dummy_security_ops)) {
 		printk(KERN_ERR "%s could not verify "
@@ -172,8 +170,948 @@ int mod_unreg_security(const char *name, struct security_operations *ops)
 	return security_ops->unregister_security(name, ops);
 }
 
-EXPORT_SYMBOL_GPL(register_security);
-EXPORT_SYMBOL_GPL(unregister_security);
-EXPORT_SYMBOL_GPL(mod_reg_security);
-EXPORT_SYMBOL_GPL(mod_unreg_security);
-EXPORT_SYMBOL(security_ops);
+/* Security operations */
+
+int security_ptrace (struct task_struct * parent, struct task_struct * child)
+{
+	return security_ops->ptrace (parent, child);
+}
+
+int security_capget (struct task_struct *target,
+		     kernel_cap_t *effective,
+		     kernel_cap_t *inheritable,
+		     kernel_cap_t *permitted)
+{
+	return security_ops->capget (target, effective, inheritable, permitted);
+}
+
+int security_capset_check (struct task_struct *target,
+			   kernel_cap_t *effective,
+			   kernel_cap_t *inheritable,
+			   kernel_cap_t *permitted)
+{
+	return security_ops->capset_check (target, effective, inheritable, permitted);
+}
+
+void security_capset_set (struct task_struct *target,
+			  kernel_cap_t *effective,
+			  kernel_cap_t *inheritable,
+			  kernel_cap_t *permitted)
+{
+	security_ops->capset_set (target, effective, inheritable, permitted);
+}
+
+int security_capable(struct task_struct *tsk, int cap)
+{
+	return security_ops->capable(tsk, cap);
+}
+
+int security_acct (struct file *file)
+{
+	return security_ops->acct (file);
+}
+
+int security_sysctl(struct ctl_table *table, int op)
+{
+	return security_ops->sysctl(table, op);
+}
+
+int security_quotactl (int cmds, int type, int id, struct super_block *sb)
+{
+	return security_ops->quotactl (cmds, type, id, sb);
+}
+
+int security_quota_on (struct dentry * dentry)
+{
+	return security_ops->quota_on (dentry);
+}
+
+int security_syslog(int type)
+{
+	return security_ops->syslog(type);
+}
+
+int security_settime(struct timespec *ts, struct timezone *tz)
+{
+	return security_ops->settime(ts, tz);
+}
+
+
+int security_vm_enough_memory(long pages)
+{
+	return security_ops->vm_enough_memory(pages);
+}
+
+int security_bprm_alloc (struct linux_binprm *bprm)
+{
+	return security_ops->bprm_alloc_security (bprm);
+}
+
+void security_bprm_free (struct linux_binprm *bprm)
+{
+	security_ops->bprm_free_security (bprm);
+}
+
+void security_bprm_apply_creds (struct linux_binprm *bprm, int unsafe)
+{
+	security_ops->bprm_apply_creds (bprm, unsafe);
+}
+
+void security_bprm_post_apply_creds (struct linux_binprm *bprm)
+{
+	security_ops->bprm_post_apply_creds (bprm);
+}
+
+int security_bprm_set (struct linux_binprm *bprm)
+{
+	return security_ops->bprm_set_security (bprm);
+}
+
+int security_bprm_check (struct linux_binprm *bprm)
+{
+	return security_ops->bprm_check_security (bprm);
+}
+
+int security_bprm_secureexec (struct linux_binprm *bprm)
+{
+	return security_ops->bprm_secureexec (bprm);
+}
+
+int security_sb_alloc (struct super_block *sb)
+{
+	return security_ops->sb_alloc_security (sb);
+}
+
+void security_sb_free (struct super_block *sb)
+{
+	security_ops->sb_free_security (sb);
+}
+
+int security_sb_copy_data (struct file_system_type *type, void *orig, void *copy)
+{
+	return security_ops->sb_copy_data (type, orig, copy);
+}
+
+int security_sb_kern_mount (struct super_block *sb, void *data)
+{
+	return security_ops->sb_kern_mount (sb, data);
+}
+
+int security_sb_statfs (struct dentry *dentry)
+{
+	return security_ops->sb_statfs (dentry);
+}
+
+int security_sb_mount (char *dev_name, struct nameidata *nd,
+                       char *type, unsigned long flags, void *data)
+{
+	return security_ops->sb_mount (dev_name, nd, type, flags, data);
+}
+
+int security_sb_check_sb (struct vfsmount *mnt, struct nameidata *nd)
+{
+	return security_ops->sb_check_sb (mnt, nd);
+}
+
+int security_sb_umount (struct vfsmount *mnt, int flags)
+{
+	return security_ops->sb_umount (mnt, flags);
+}
+
+void security_sb_umount_close (struct vfsmount *mnt)
+{
+	security_ops->sb_umount_close (mnt);
+}
+
+void security_sb_umount_busy (struct vfsmount *mnt)
+{
+	security_ops->sb_umount_busy (mnt);
+}
+
+void security_sb_post_remount (struct vfsmount *mnt, unsigned long flags, void *data)
+{
+	security_ops->sb_post_remount (mnt, flags, data);
+}
+
+void security_sb_post_mountroot (void)
+{
+	security_ops->sb_post_mountroot ();
+}
+
+void security_sb_post_addmount (struct vfsmount *mnt, struct nameidata *mountpoint_nd)
+{
+	security_ops->sb_post_addmount (mnt, mountpoint_nd);
+}
+
+int security_sb_pivotroot (struct nameidata *old_nd, struct nameidata *new_nd)
+{
+	return security_ops->sb_pivotroot (old_nd, new_nd);
+}
+
+void security_sb_post_pivotroot (struct nameidata *old_nd, struct nameidata *new_nd)
+{
+	security_ops->sb_post_pivotroot (old_nd, new_nd);
+}
+
+int security_inode_alloc (struct inode *inode)
+{
+	inode->i_security = NULL;
+	return security_ops->inode_alloc_security (inode);
+}
+
+void security_inode_free (struct inode *inode)
+{
+	security_ops->inode_free_security (inode);
+}
+
+int security_inode_init_security (struct inode *inode, struct inode *dir,
+				  char **name, void **value, size_t *len)
+{
+	if (unlikely (IS_PRIVATE (inode)))
+		return -EOPNOTSUPP;
+	return security_ops->inode_init_security (inode, dir, name, value, len);
+}
+EXPORT_SYMBOL(security_inode_init_security);
+
+int security_inode_create (struct inode *dir, struct dentry *dentry, int mode)
+{
+	if (unlikely (IS_PRIVATE (dir)))
+		return 0;
+	return security_ops->inode_create (dir, dentry, mode);
+}
+
+int security_inode_link (struct dentry *old_dentry, struct inode *dir,
+			 struct dentry *new_dentry)
+{
+	if (unlikely (IS_PRIVATE (old_dentry->d_inode)))
+		return 0;
+	return security_ops->inode_link (old_dentry, dir, new_dentry);
+}
+
+int security_inode_unlink (struct inode *dir, struct dentry *dentry)
+{
+	if (unlikely (IS_PRIVATE (dentry->d_inode)))
+		return 0;
+	return security_ops->inode_unlink (dir, dentry);
+}
+
+int security_inode_symlink (struct inode *dir, struct dentry *dentry,
+			    const char *old_name)
+{
+	if (unlikely (IS_PRIVATE (dir)))
+		return 0;
+	return security_ops->inode_symlink (dir, dentry, old_name);
+}
+
+int security_inode_mkdir (struct inode *dir, struct dentry *dentry, int mode)
+{
+	if (unlikely (IS_PRIVATE (dir)))
+		return 0;
+	return security_ops->inode_mkdir (dir, dentry, mode);
+}
+
+int security_inode_rmdir (struct inode *dir, struct dentry *dentry)
+{
+	if (unlikely (IS_PRIVATE (dentry->d_inode)))
+		return 0;
+	return security_ops->inode_rmdir (dir, dentry);
+}
+
+int security_inode_mknod (struct inode *dir, struct dentry *dentry, int mode, dev_t dev)
+{
+	if (unlikely (IS_PRIVATE (dir)))
+		return 0;
+	return security_ops->inode_mknod (dir, dentry, mode, dev);
+}
+
+int security_inode_rename (struct inode *old_dir, struct dentry *old_dentry,
+			   struct inode *new_dir, struct dentry *new_dentry)
+{
+        if (unlikely (IS_PRIVATE (old_dentry->d_inode) ||
+            (new_dentry->d_inode && IS_PRIVATE (new_dentry->d_inode))))
+		return 0;
+	return security_ops->inode_rename (old_dir, old_dentry,
+					   new_dir, new_dentry);
+}
+
+int security_inode_readlink (struct dentry *dentry)
+{
+	if (unlikely (IS_PRIVATE (dentry->d_inode)))
+		return 0;
+	return security_ops->inode_readlink (dentry);
+}
+
+int security_inode_follow_link (struct dentry *dentry, struct nameidata *nd)
+{
+	if (unlikely (IS_PRIVATE (dentry->d_inode)))
+		return 0;
+	return security_ops->inode_follow_link (dentry, nd);
+}
+
+int security_inode_permission (struct inode *inode, int mask, struct nameidata *nd)
+{
+	if (unlikely (IS_PRIVATE (inode)))
+		return 0;
+	return security_ops->inode_permission (inode, mask, nd);
+}
+
+int security_inode_setattr (struct dentry *dentry, struct iattr *attr)
+{
+	if (unlikely (IS_PRIVATE (dentry->d_inode)))
+		return 0;
+	return security_ops->inode_setattr (dentry, attr);
+}
+
+int security_inode_getattr (struct vfsmount *mnt, struct dentry *dentry)
+{
+	if (unlikely (IS_PRIVATE (dentry->d_inode)))
+		return 0;
+	return security_ops->inode_getattr (mnt, dentry);
+}
+
+void security_inode_delete (struct inode *inode)
+{
+	if (unlikely (IS_PRIVATE (inode)))
+		return;
+	security_ops->inode_delete (inode);
+}
+
+int security_inode_setxattr (struct dentry *dentry, char *name,
+			     void *value, size_t size, int flags)
+{
+	if (unlikely (IS_PRIVATE (dentry->d_inode)))
+		return 0;
+	return security_ops->inode_setxattr (dentry, name, value, size, flags);
+}
+
+void security_inode_post_setxattr (struct dentry *dentry, char *name,
+				   void *value, size_t size, int flags)
+{
+	if (unlikely (IS_PRIVATE (dentry->d_inode)))
+		return;
+	security_ops->inode_post_setxattr (dentry, name, value, size, flags);
+}
+
+int security_inode_getxattr (struct dentry *dentry, char *name)
+{
+	if (unlikely (IS_PRIVATE (dentry->d_inode)))
+		return 0;
+	return security_ops->inode_getxattr (dentry, name);
+}
+
+int security_inode_listxattr (struct dentry *dentry)
+{
+	if (unlikely (IS_PRIVATE (dentry->d_inode)))
+		return 0;
+	return security_ops->inode_listxattr (dentry);
+}
+
+int security_inode_removexattr (struct dentry *dentry, char *name)
+{
+	if (unlikely (IS_PRIVATE (dentry->d_inode)))
+		return 0;
+	return security_ops->inode_removexattr (dentry, name);
+}
+
+const char *security_inode_xattr_getsuffix(void)
+{
+	return security_ops->inode_xattr_getsuffix();
+}
+
+int security_inode_getsecurity(const struct inode *inode, const char *name, void *buffer, size_t size, int err)
+{
+	if (unlikely (IS_PRIVATE (inode)))
+		return 0;
+	return security_ops->inode_getsecurity(inode, name, buffer, size, err);
+}
+
+int security_inode_setsecurity(struct inode *inode, const char *name, const void *value, size_t size, int flags)
+{
+	if (unlikely (IS_PRIVATE (inode)))
+		return 0;
+	return security_ops->inode_setsecurity(inode, name, value, size, flags);
+}
+
+int security_inode_listsecurity(struct inode *inode, char *buffer, size_t buffer_size)
+{
+	if (unlikely (IS_PRIVATE (inode)))
+		return 0;
+	return security_ops->inode_listsecurity(inode, buffer, buffer_size);
+}
+
+int security_file_permission (struct file *file, int mask)
+{
+	return security_ops->file_permission (file, mask);
+}
+
+int security_file_alloc (struct file *file)
+{
+	return security_ops->file_alloc_security (file);
+}
+
+void security_file_free (struct file *file)
+{
+	security_ops->file_free_security (file);
+}
+
+int security_file_ioctl (struct file *file, unsigned int cmd, unsigned long arg)
+{
+	return security_ops->file_ioctl (file, cmd, arg);
+}
+
+int security_file_mmap (struct file *file, unsigned long reqprot,
+			unsigned long prot, unsigned long flags)
+{
+	return security_ops->file_mmap (file, reqprot, prot, flags);
+}
+
+int security_file_mprotect (struct vm_area_struct *vma, unsigned long reqprot,
+			    unsigned long prot)
+{
+	return security_ops->file_mprotect (vma, reqprot, prot);
+}
+
+int security_file_lock (struct file *file, unsigned int cmd)
+{
+	return security_ops->file_lock (file, cmd);
+}
+
+int security_file_fcntl (struct file *file, unsigned int cmd, unsigned long arg)
+{
+	return security_ops->file_fcntl (file, cmd, arg);
+}
+
+int security_file_set_fowner (struct file *file)
+{
+	return security_ops->file_set_fowner (file);
+}
+
+int security_file_send_sigiotask (struct task_struct *tsk,
+				  struct fown_struct *fown, int sig)
+{
+	return security_ops->file_send_sigiotask (tsk, fown, sig);
+}
+
+int security_file_receive (struct file *file)
+{
+	return security_ops->file_receive (file);
+}
+
+int security_task_create (unsigned long clone_flags)
+{
+	return security_ops->task_create (clone_flags);
+}
+
+int security_task_alloc (struct task_struct *p)
+{
+	return security_ops->task_alloc_security (p);
+}
+
+void security_task_free (struct task_struct *p)
+{
+	security_ops->task_free_security (p);
+}
+
+int security_task_setuid (uid_t id0, uid_t id1, uid_t id2, int flags)
+{
+	return security_ops->task_setuid (id0, id1, id2, flags);
+}
+
+int security_task_post_setuid (uid_t old_ruid, uid_t old_euid,
+			       uid_t old_suid, int flags)
+{
+	return security_ops->task_post_setuid (old_ruid, old_euid, old_suid, flags);
+}
+
+int security_task_setgid (gid_t id0, gid_t id1, gid_t id2, int flags)
+{
+	return security_ops->task_setgid (id0, id1, id2, flags);
+}
+
+int security_task_setpgid (struct task_struct *p, pid_t pgid)
+{
+	return security_ops->task_setpgid (p, pgid);
+}
+
+int security_task_getpgid (struct task_struct *p)
+{
+	return security_ops->task_getpgid (p);
+}
+
+int security_task_getsid (struct task_struct *p)
+{
+	return security_ops->task_getsid (p);
+}
+
+void security_task_getsecid (struct task_struct *p, u32 *secid)
+{
+	security_ops->task_getsecid (p, secid);
+}
+EXPORT_SYMBOL(security_task_getsecid);
+
+int security_task_setgroups (struct group_info *group_info)
+{
+	return security_ops->task_setgroups (group_info);
+}
+
+int security_task_setnice (struct task_struct *p, int nice)
+{
+	return security_ops->task_setnice (p, nice);
+}
+
+int security_task_setioprio (struct task_struct *p, int ioprio)
+{
+	return security_ops->task_setioprio (p, ioprio);
+}
+
+int security_task_getioprio (struct task_struct *p)
+{
+	return security_ops->task_getioprio (p);
+}
+
+int security_task_setrlimit (unsigned int resource, struct rlimit *new_rlim)
+{
+	return security_ops->task_setrlimit (resource, new_rlim);
+}
+
+int security_task_setscheduler (struct task_struct *p,
+				int policy, struct sched_param *lp)
+{
+	return security_ops->task_setscheduler (p, policy, lp);
+}
+
+int security_task_getscheduler (struct task_struct *p)
+{
+	return security_ops->task_getscheduler (p);
+}
+
+int security_task_movememory (struct task_struct *p)
+{
+	return security_ops->task_movememory (p);
+}
+
+int security_task_kill (struct task_struct *p, struct siginfo *info,
+			int sig, u32 secid)
+{
+	return security_ops->task_kill (p, info, sig, secid);
+}
+
+int security_task_wait (struct task_struct *p)
+{
+	return security_ops->task_wait (p);
+}
+
+int security_task_prctl (int option, unsigned long arg2, unsigned long arg3,
+			 unsigned long arg4, unsigned long arg5)
+{
+	return security_ops->task_prctl (option, arg2, arg3, arg4, arg5);
+}
+
+void security_task_reparent_to_init (struct task_struct *p)
+{
+	security_ops->task_reparent_to_init (p);
+}
+
+void security_task_to_inode(struct task_struct *p, struct inode *inode)
+{
+	security_ops->task_to_inode(p, inode);
+}
+
+int security_ipc_permission (struct kern_ipc_perm *ipcp, short flag)
+{
+	return security_ops->ipc_permission (ipcp, flag);
+}
+
+int security_msg_msg_alloc (struct msg_msg * msg)
+{
+	return security_ops->msg_msg_alloc_security (msg);
+}
+
+void security_msg_msg_free (struct msg_msg * msg)
+{
+	security_ops->msg_msg_free_security(msg);
+}
+
+int security_msg_queue_alloc (struct msg_queue *msq)
+{
+	return security_ops->msg_queue_alloc_security (msq);
+}
+
+void security_msg_queue_free (struct msg_queue *msq)
+{
+	security_ops->msg_queue_free_security (msq);
+}
+
+int security_msg_queue_associate (struct msg_queue * msq, int msqflg)
+{
+	return security_ops->msg_queue_associate (msq, msqflg);
+}
+
+int security_msg_queue_msgctl (struct msg_queue * msq, int cmd)
+{
+	return security_ops->msg_queue_msgctl (msq, cmd);
+}
+
+int security_msg_queue_msgsnd (struct msg_queue * msq,
+			       struct msg_msg * msg, int msqflg)
+{
+	return security_ops->msg_queue_msgsnd (msq, msg, msqflg);
+}
+
+int security_msg_queue_msgrcv (struct msg_queue * msq, struct msg_msg * msg,
+			       struct task_struct * target, long type, int mode)
+{
+	return security_ops->msg_queue_msgrcv (msq, msg, target, type, mode);
+}
+
+int security_shm_alloc (struct shmid_kernel *shp)
+{
+	return security_ops->shm_alloc_security (shp);
+}
+
+void security_shm_free (struct shmid_kernel *shp)
+{
+	security_ops->shm_free_security (shp);
+}
+
+int security_shm_associate (struct shmid_kernel * shp, int shmflg)
+{
+	return security_ops->shm_associate(shp, shmflg);
+}
+
+int security_shm_shmctl (struct shmid_kernel * shp, int cmd)
+{
+	return security_ops->shm_shmctl (shp, cmd);
+}
+
+int security_shm_shmat (struct shmid_kernel * shp, char __user *shmaddr, int shmflg)
+{
+	return security_ops->shm_shmat(shp, shmaddr, shmflg);
+}
+
+int security_sem_alloc (struct sem_array *sma)
+{
+	return security_ops->sem_alloc_security (sma);
+}
+
+void security_sem_free (struct sem_array *sma)
+{
+	security_ops->sem_free_security (sma);
+}
+
+int security_sem_associate (struct sem_array * sma, int semflg)
+{
+	return security_ops->sem_associate (sma, semflg);
+}
+
+int security_sem_semctl (struct sem_array * sma, int cmd)
+{
+	return security_ops->sem_semctl(sma, cmd);
+}
+
+int security_sem_semop (struct sem_array * sma, struct sembuf * sops,
+			unsigned nsops, int alter)
+{
+	return security_ops->sem_semop(sma, sops, nsops, alter);
+}
+
+void security_d_instantiate (struct dentry *dentry, struct inode *inode)
+{
+	if (unlikely (inode && IS_PRIVATE (inode)))
+		return;
+	security_ops->d_instantiate (dentry, inode);
+}
+EXPORT_SYMBOL(security_d_instantiate);
+
+int security_getprocattr(struct task_struct *p, char *name, char **value)
+{
+	return security_ops->getprocattr(p, name, value);
+}
+
+int security_setprocattr(struct task_struct *p, char *name, void *value, size_t size)
+{
+	return security_ops->setprocattr(p, name, value, size);
+}
+
+int security_netlink_send(struct sock *sk, struct sk_buff * skb)
+{
+	return security_ops->netlink_send(sk, skb);
+}
+EXPORT_SYMBOL(security_netlink_send);
+
+int security_netlink_recv(struct sk_buff * skb, int cap)
+{
+	return security_ops->netlink_recv(skb, cap);
+}
+EXPORT_SYMBOL(security_netlink_recv);
+
+int security_secid_to_secctx(u32 secid, char **secdata, u32 *seclen)
+{
+	return security_ops->secid_to_secctx(secid, secdata, seclen);
+}
+EXPORT_SYMBOL(security_secid_to_secctx);
+
+void security_release_secctx(char *secdata, u32 seclen)
+{
+	return security_ops->release_secctx(secdata, seclen);
+}
+EXPORT_SYMBOL(security_release_secctx);
+
+#ifdef CONFIG_SECURITY_NETWORK
+
+int security_unix_stream_connect(struct socket * sock, struct socket * other,
+				 struct sock * newsk)
+{
+	return security_ops->unix_stream_connect(sock, other, newsk);
+}
+EXPORT_SYMBOL(security_unix_stream_connect);
+
+int security_unix_may_send(struct socket * sock,  struct socket * other)
+{
+	return security_ops->unix_may_send(sock, other);
+}
+EXPORT_SYMBOL(security_unix_may_send);
+
+int security_socket_create (int family, int type, int protocol, int kern)
+{
+	return security_ops->socket_create(family, type, protocol, kern);
+}
+
+int security_socket_post_create(struct socket * sock, int family,
+				int type, int protocol, int kern)
+{
+	return security_ops->socket_post_create(sock, family, type,
+						protocol, kern);
+}
+
+int security_socket_bind(struct socket * sock, struct sockaddr * address, int addrlen)
+{
+	return security_ops->socket_bind(sock, address, addrlen);
+}
+
+int security_socket_connect(struct socket * sock, struct sockaddr * address, int addrlen)
+{
+	return security_ops->socket_connect(sock, address, addrlen);
+}
+
+int security_socket_listen(struct socket * sock, int backlog)
+{
+	return security_ops->socket_listen(sock, backlog);
+}
+
+int security_socket_accept(struct socket * sock, struct socket * newsock)
+{
+	return security_ops->socket_accept(sock, newsock);
+}
+
+void security_socket_post_accept(struct socket * sock, struct socket * newsock)
+{
+	security_ops->socket_post_accept(sock, newsock);
+}
+
+int security_socket_sendmsg(struct socket * sock, struct msghdr * msg, int size)
+{
+	return security_ops->socket_sendmsg(sock, msg, size);
+}
+
+int security_socket_recvmsg(struct socket * sock, struct msghdr * msg,
+			    int size, int flags)
+{
+	return security_ops->socket_recvmsg(sock, msg, size, flags);
+}
+
+int security_socket_getsockname(struct socket * sock)
+{
+	return security_ops->socket_getsockname(sock);
+}
+
+int security_socket_getpeername(struct socket * sock)
+{
+	return security_ops->socket_getpeername(sock);
+}
+
+int security_socket_getsockopt(struct socket * sock, int level, int optname)
+{
+	return security_ops->socket_getsockopt(sock, level, optname);
+}
+
+int security_socket_setsockopt(struct socket * sock, int level, int optname)
+{
+	return security_ops->socket_setsockopt(sock, level, optname);
+}
+
+int security_socket_shutdown(struct socket * sock, int how)
+{
+	return security_ops->socket_shutdown(sock, how);
+}
+
+int security_sock_rcv_skb (struct sock * sk, struct sk_buff * skb)
+{
+	return security_ops->socket_sock_rcv_skb (sk, skb);
+}
+EXPORT_SYMBOL(security_sock_rcv_skb);
+
+int security_socket_getpeersec_stream(struct socket *sock, char __user *optval,
+				      int __user *optlen, unsigned len)
+{
+	return security_ops->socket_getpeersec_stream(sock, optval, optlen, len);
+}
+
+int security_socket_getpeersec_dgram(struct socket *sock, struct sk_buff *skb, u32 *secid)
+{
+	return security_ops->socket_getpeersec_dgram(sock, skb, secid);
+}
+EXPORT_SYMBOL(security_socket_getpeersec_dgram);
+
+int security_sk_alloc(struct sock *sk, int family, gfp_t priority)
+{
+	return security_ops->sk_alloc_security(sk, family, priority);
+}
+
+void security_sk_free(struct sock *sk)
+{
+	return security_ops->sk_free_security(sk);
+}
+
+void security_sk_clone(const struct sock *sk, struct sock *newsk)
+{
+	return security_ops->sk_clone_security(sk, newsk);
+}
+
+void security_sk_classify_flow(struct sock *sk, struct flowi *fl)
+{
+	security_ops->sk_getsecid(sk, &fl->secid);
+}
+EXPORT_SYMBOL(security_sk_classify_flow);
+
+void security_req_classify_flow(const struct request_sock *req, struct flowi *fl)
+{
+	security_ops->req_classify_flow(req, fl);
+}
+EXPORT_SYMBOL(security_req_classify_flow);
+
+void security_sock_graft(struct sock* sk, struct socket *parent)
+{
+	security_ops->sock_graft(sk, parent);
+}
+EXPORT_SYMBOL(security_sock_graft);
+
+int security_inet_conn_request(struct sock *sk,
+			struct sk_buff *skb, struct request_sock *req)
+{
+	return security_ops->inet_conn_request(sk, skb, req);
+}
+EXPORT_SYMBOL(security_inet_conn_request);
+
+void security_inet_csk_clone(struct sock *newsk,
+			const struct request_sock *req)
+{
+	security_ops->inet_csk_clone(newsk, req);
+}
+
+void security_inet_conn_established(struct sock *sk,
+			struct sk_buff *skb)
+{
+	security_ops->inet_conn_established(sk, skb);
+}
+
+#endif	/* CONFIG_SECURITY_NETWORK */
+
+#ifdef CONFIG_SECURITY_NETWORK_XFRM
+
+int security_xfrm_policy_alloc(struct xfrm_policy *xp, struct xfrm_user_sec_ctx *sec_ctx)
+{
+	return security_ops->xfrm_policy_alloc_security(xp, sec_ctx);
+}
+EXPORT_SYMBOL(security_xfrm_policy_alloc);
+
+int security_xfrm_policy_clone(struct xfrm_policy *old, struct xfrm_policy *new)
+{
+	return security_ops->xfrm_policy_clone_security(old, new);
+}
+
+void security_xfrm_policy_free(struct xfrm_policy *xp)
+{
+	security_ops->xfrm_policy_free_security(xp);
+}
+EXPORT_SYMBOL(security_xfrm_policy_free);
+
+int security_xfrm_policy_delete(struct xfrm_policy *xp)
+{
+	return security_ops->xfrm_policy_delete_security(xp);
+}
+
+int security_xfrm_state_alloc(struct xfrm_state *x, struct xfrm_user_sec_ctx *sec_ctx)
+{
+	return security_ops->xfrm_state_alloc_security(x, sec_ctx, 0);
+}
+EXPORT_SYMBOL(security_xfrm_state_alloc);
+
+int security_xfrm_state_alloc_acquire(struct xfrm_state *x,
+				      struct xfrm_sec_ctx *polsec, u32 secid)
+{
+	if (!polsec)
+		return 0;
+	/*
+	 * We want the context to be taken from secid which is usually
+	 * from the sock.
+	 */
+	return security_ops->xfrm_state_alloc_security(x, NULL, secid);
+}
+
+int security_xfrm_state_delete(struct xfrm_state *x)
+{
+	return security_ops->xfrm_state_delete_security(x);
+}
+EXPORT_SYMBOL(security_xfrm_state_delete);
+
+void security_xfrm_state_free(struct xfrm_state *x)
+{
+	security_ops->xfrm_state_free_security(x);
+}
+
+int security_xfrm_policy_lookup(struct xfrm_policy *xp, u32 fl_secid, u8 dir)
+{
+	return security_ops->xfrm_policy_lookup(xp, fl_secid, dir);
+}
+
+int security_xfrm_state_pol_flow_match(struct xfrm_state *x,
+				       struct xfrm_policy *xp, struct flowi *fl)
+{
+	return security_ops->xfrm_state_pol_flow_match(x, xp, fl);
+}
+
+int security_xfrm_decode_session(struct sk_buff *skb, u32 *secid)
+{
+	return security_ops->xfrm_decode_session(skb, secid, 1);
+}
+
+void security_skb_classify_flow(struct sk_buff *skb, struct flowi *fl)
+{
+	int rc = security_ops->xfrm_decode_session(skb, &fl->secid, 0);
+
+	BUG_ON(rc);
+}
+EXPORT_SYMBOL(security_skb_classify_flow);
+
+#endif	/* CONFIG_SECURITY_NETWORK_XFRM */
+
+#ifdef CONFIG_KEYS
+
+int security_key_alloc(struct key *key, struct task_struct *tsk, unsigned long flags)
+{
+	return security_ops->key_alloc(key, tsk, flags);
+}
+
+void security_key_free(struct key *key)
+{
+	security_ops->key_free(key);
+}
+
+int security_key_permission(key_ref_t key_ref,
+			    struct task_struct *context, key_perm_t perm)
+{
+	return security_ops->key_permission(key_ref, context, perm);
+}
+
+#endif	/* CONFIG_KEYS */
diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
index ad8dd4e..105319a 100644
--- a/security/selinux/hooks.c
+++ b/security/selinux/hooks.c
@@ -20,7 +20,6 @@
  *      as published by the Free Software Foundation.
  */
 
-#include <linux/module.h>
 #include <linux/init.h>
 #include <linux/kernel.h>
 #include <linux/ptrace.h>
diff --git a/security/selinux/xfrm.c b/security/selinux/xfrm.c
index bd8d1ef..b589238 100644
--- a/security/selinux/xfrm.c
+++ b/security/selinux/xfrm.c
@@ -31,7 +31,6 @@
  *   2. Emulating a reasonable SO_PEERSEC across machines
  *   3. Testing addition of sk_policy's with security context via setsockopt
  */
-#include <linux/module.h>
 #include <linux/kernel.h>
 #include <linux/init.h>
 #include <linux/security.h>
-- 
1.5.2.1

Re: [PATCH try #2] security: Convert LSM into a static interface

[Andreas Gruenbacher]

On Monday 25 June 2007 06:33, James Morris wrote:
> Convert LSM into a static interface, as the ability to unload a security 
> module is not required by in-tree users and potentially complicates the 
> overall security architecture.

It's useful for some LSMs to be modular, and LSMs which are y/n options won't 
have any security architecture issues with unloading at all. The mere fact 
that SELinux cannot be built as a module is a rather weak argument for 
disabling LSM modules as a whole, so  please don't.

Thanks,
Andreas

Re: [PATCH try #2] security: Convert LSM into a static interface

[James Morris]

On Mon, 25 Jun 2007, Andreas Gruenbacher wrote:

> It's useful for some LSMs to be modular, and LSMs which are y/n options won't 
> have any security architecture issues with unloading at all. 

Which LSMs?  Upstream, there are SELinux and capabilty, and they're not 
safe as loadable modules.

> The mere fact 
> that SELinux cannot be built as a module is a rather weak argument for 
> disabling LSM modules as a whole, so  please don't.

That's not the argument.  Please review the thread.


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

Re: [PATCH try #2] security: Convert LSM into a static interface

[Serge E. Hallyn]

Quoting James Morris (jmorris@namei.org):
> On Mon, 25 Jun 2007, Andreas Gruenbacher wrote:
> 
> > It's useful for some LSMs to be modular, and LSMs which are y/n options won't 
> > have any security architecture issues with unloading at all. 
> 
> Which LSMs?  Upstream, there are SELinux and capabilty, and they're not 
> safe as loadable modules.
> 
> > The mere fact 
> > that SELinux cannot be built as a module is a rather weak argument for 
> > disabling LSM modules as a whole, so  please don't.
> 
> That's not the argument.  Please review the thread.

The argument is 'abuse', right?

Abuse is defined as using the LSM hooks for non-security applications,
right?

It seems to me that the community is doing a good job of discouraging
such abuse - by redirecting the "wrong-doers" to implement proper
upstream solutions, i.e. taskstats, the audit subsystem, etc.

Such encouragement seems a far better response than taking away freedoms
and flexibility from everyone.

-serge

Re: [PATCH try #2] security: Convert LSM into a static interface

[Adrian Bunk]

On Mon, Jun 25, 2007 at 10:57:31PM -0500, Serge E. Hallyn wrote:
> Quoting James Morris (jmorris@namei.org):
> > On Mon, 25 Jun 2007, Andreas Gruenbacher wrote:
> > 
> > > It's useful for some LSMs to be modular, and LSMs which are y/n options won't 
> > > have any security architecture issues with unloading at all. 
> > 
> > Which LSMs?  Upstream, there are SELinux and capabilty, and they're not 
> > safe as loadable modules.
> > 
> > > The mere fact 
> > > that SELinux cannot be built as a module is a rather weak argument for 
> > > disabling LSM modules as a whole, so  please don't.
> > 
> > That's not the argument.  Please review the thread.
> 
> The argument is 'abuse', right?
> 
> Abuse is defined as using the LSM hooks for non-security applications,
> right?
> 
> It seems to me that the community is doing a good job of discouraging
> such abuse - by redirecting the "wrong-doers" to implement proper
> upstream solutions, i.e. taskstats, the audit subsystem, etc.
> 
> Such encouragement seems a far better response than taking away freedoms
> and flexibility from everyone.

We are not living in a world where everyone had good intentions...

For _some_ "wrong-doers" your approach works.

But how do you convince the "wrong-doers" who do things like putting 
MODULE_LICENSE("GPL") into their binary-only modules and who ignore you 
and get away because noone sues them?

The spirit of the GPLv2 is to defend the freedom of the software 
(different from the spirit of the BSD licence), and considering that 
there aren't many people defending the GPLv2 copyright of the Linux 
kernel at court against abusers, making it harder for people to do the 
abuse might not be the worst choice...

> -serge

cu
Adrian

-- 

       "Is there not promise of rain?" Ling Tan asked suddenly out
        of the darkness. There had been need of rain for many days.
       "Only a promise," Lao Er said.
                                       Pearl S. Buck - Dragon Seed

Re: [PATCH try #2] security: Convert LSM into a static interface

[Serge E. Hallyn]

Quoting Adrian Bunk (bunk@stusta.de):
> On Mon, Jun 25, 2007 at 10:57:31PM -0500, Serge E. Hallyn wrote:
> > Quoting James Morris (jmorris@namei.org):
> > > On Mon, 25 Jun 2007, Andreas Gruenbacher wrote:
> > > 
> > > > It's useful for some LSMs to be modular, and LSMs which are y/n options won't 
> > > > have any security architecture issues with unloading at all. 
> > > 
> > > Which LSMs?  Upstream, there are SELinux and capabilty, and they're not 
> > > safe as loadable modules.
> > > 
> > > > The mere fact 
> > > > that SELinux cannot be built as a module is a rather weak argument for 
> > > > disabling LSM modules as a whole, so  please don't.
> > > 
> > > That's not the argument.  Please review the thread.
> > 
> > The argument is 'abuse', right?
> > 
> > Abuse is defined as using the LSM hooks for non-security applications,
> > right?
> > 
> > It seems to me that the community is doing a good job of discouraging
> > such abuse - by redirecting the "wrong-doers" to implement proper
> > upstream solutions, i.e. taskstats, the audit subsystem, etc.
> > 
> > Such encouragement seems a far better response than taking away freedoms
> > and flexibility from everyone.
> 
> We are not living in a world where everyone had good intentions...

Oh no, i took a wrong turn somewhere  :)

> For _some_ "wrong-doers" your approach works.
> 
> But how do you convince the "wrong-doers" who do things like putting 
> MODULE_LICENSE("GPL") into their binary-only modules and who ignore you 
> and get away because noone sues them?

Do these really exist?  Maybe noone sues them because noone knows who
they are...

But - note that you've changed completely the meaning of 'abuse'.
So mine was wrong?

> The spirit of the GPLv2 is to defend the freedom of the software 
> (different from the spirit of the BSD licence), and considering that 
> there aren't many people defending the GPLv2 copyright of the Linux 
> kernel at court against abusers, making it harder for people to do the 
> abuse might not be the worst choice...

Well, but you seem to be saying that the license means squat, and
resorting to making things inconvenient rather than illegal.

Now I guess if it really is accepted that that's the way it should be,
then this patch will go in.

-serge

Re: [PATCH try #2] security: Convert LSM into a static interface

[Adrian Bunk]

On Tue, Jun 26, 2007 at 09:06:44AM -0500, Serge E. Hallyn wrote:
> Quoting Adrian Bunk (bunk@stusta.de):
> > On Mon, Jun 25, 2007 at 10:57:31PM -0500, Serge E. Hallyn wrote:
> > > Quoting James Morris (jmorris@namei.org):
> > > > On Mon, 25 Jun 2007, Andreas Gruenbacher wrote:
> > > > 
> > > > > It's useful for some LSMs to be modular, and LSMs which are y/n options won't 
> > > > > have any security architecture issues with unloading at all. 
> > > > 
> > > > Which LSMs?  Upstream, there are SELinux and capabilty, and they're not 
> > > > safe as loadable modules.
> > > > 
> > > > > The mere fact 
> > > > > that SELinux cannot be built as a module is a rather weak argument for 
> > > > > disabling LSM modules as a whole, so  please don't.
> > > > 
> > > > That's not the argument.  Please review the thread.
> > > 
> > > The argument is 'abuse', right?
> > > 
> > > Abuse is defined as using the LSM hooks for non-security applications,
> > > right?
> > > 
> > > It seems to me that the community is doing a good job of discouraging
> > > such abuse - by redirecting the "wrong-doers" to implement proper
> > > upstream solutions, i.e. taskstats, the audit subsystem, etc.
> > > 
> > > Such encouragement seems a far better response than taking away freedoms
> > > and flexibility from everyone.
> > 
> > We are not living in a world where everyone had good intentions...
> 
> Oh no, i took a wrong turn somewhere  :)
> 
> > For _some_ "wrong-doers" your approach works.
> > 
> > But how do you convince the "wrong-doers" who do things like putting 
> > MODULE_LICENSE("GPL") into their binary-only modules and who ignore you 
> > and get away because noone sues them?
> 
> Do these really exist?  Maybe noone sues them because noone knows who
> they are...

   http://lwn.net/Articles/82306/

> But - note that you've changed completely the meaning of 'abuse'.
> So mine was wrong?

Technical and legal abuse are related.

For GPL'ed modules you might assume good faith and get the authors to do 
things in a proper way. Authors of legally questionable modules that 
cheat in many ways are quite a different issue.

> > The spirit of the GPLv2 is to defend the freedom of the software 
> > (different from the spirit of the BSD licence), and considering that 
> > there aren't many people defending the GPLv2 copyright of the Linux 
> > kernel at court against abusers, making it harder for people to do the 
> > abuse might not be the worst choice...
> 
> Well, but you seem to be saying that the license means squat, and
> resorting to making things inconvenient rather than illegal.

No, the point is that there's no reason for making illegal things 
convenient.

I'm not talking about removing things that are used inside the kernel, 
but what you call "freedom" can also be called "hooks for possible abuse".

Additionally, it both makes the kernel bigger for everyone and requires 
proper handling of loading/unloading in the security architecture.

> Now I guess if it really is accepted that that's the way it should be,
> then this patch will go in.
> 
> -serge

cu
Adrian

-- 

       "Is there not promise of rain?" Ling Tan asked suddenly out
        of the darkness. There had been need of rain for many days.
       "Only a promise," Lao Er said.
                                       Pearl S. Buck - Dragon Seed

Re: [PATCH try #2] security: Convert LSM into a static interface

[Serge E. Hallyn]

Quoting Adrian Bunk (bunk@stusta.de):
> On Tue, Jun 26, 2007 at 09:06:44AM -0500, Serge E. Hallyn wrote:
> > Quoting Adrian Bunk (bunk@stusta.de):
> > > On Mon, Jun 25, 2007 at 10:57:31PM -0500, Serge E. Hallyn wrote:
> > > > Quoting James Morris (jmorris@namei.org):
> > > > > On Mon, 25 Jun 2007, Andreas Gruenbacher wrote:
> > > > > 
> > > > > > It's useful for some LSMs to be modular, and LSMs which are y/n options won't 
> > > > > > have any security architecture issues with unloading at all. 
> > > > > 
> > > > > Which LSMs?  Upstream, there are SELinux and capabilty, and they're not 
> > > > > safe as loadable modules.
> > > > > 
> > > > > > The mere fact 
> > > > > > that SELinux cannot be built as a module is a rather weak argument for 
> > > > > > disabling LSM modules as a whole, so  please don't.
> > > > > 
> > > > > That's not the argument.  Please review the thread.
> > > > 
> > > > The argument is 'abuse', right?
> > > > 
> > > > Abuse is defined as using the LSM hooks for non-security applications,
> > > > right?
> > > > 
> > > > It seems to me that the community is doing a good job of discouraging
> > > > such abuse - by redirecting the "wrong-doers" to implement proper
> > > > upstream solutions, i.e. taskstats, the audit subsystem, etc.
> > > > 
> > > > Such encouragement seems a far better response than taking away freedoms
> > > > and flexibility from everyone.
> > > 
> > > We are not living in a world where everyone had good intentions...
> > 
> > Oh no, i took a wrong turn somewhere  :)
> > 
> > > For _some_ "wrong-doers" your approach works.
> > > 
> > > But how do you convince the "wrong-doers" who do things like putting 
> > > MODULE_LICENSE("GPL") into their binary-only modules and who ignore you 
> > > and get away because noone sues them?
> > 
> > Do these really exist?  Maybe noone sues them because noone knows who
> > they are...
> 
>    http://lwn.net/Articles/82306/

LinuxAnt?  Are they using LSM?

It looks to me like this patch will do nothing about them.

> > But - note that you've changed completely the meaning of 'abuse'.
> > So mine was wrong?
> 
> Technical and legal abuse are related.

True but going by your logic we could remove support for modules period
to prevent legal abuse by non-gpl modules.

> For GPL'ed modules you might assume good faith and get the authors to do 
> things in a proper way. Authors of legally questionable modules that 
> cheat in many ways are quite a different issue.
> 
> > > The spirit of the GPLv2 is to defend the freedom of the software 
> > > (different from the spirit of the BSD licence), and considering that 
> > > there aren't many people defending the GPLv2 copyright of the Linux 
> > > kernel at court against abusers, making it harder for people to do the 
> > > abuse might not be the worst choice...
> > 
> > Well, but you seem to be saying that the license means squat, and
> > resorting to making things inconvenient rather than illegal.
> 
> No, the point is that there's no reason for making illegal things 
> convenient.

But no, the point is that that you are making legal things very
inconvenient.

> I'm not talking about removing things that are used inside the kernel, 

Since capabilities can currently be compiled as a module, you are.
(Though that sounds weird, so maybe I'm misreading what you are saying)

> but what you call "freedom" can also be called "hooks for possible abuse".

Yup, that is true.

> Additionally, it both makes the kernel bigger for everyone and requires 
> proper handling of loading/unloading in the security architecture.
> 
> > Now I guess if it really is accepted that that's the way it should be,
> > then this patch will go in.
> > 
> > -serge
> 
> cu
> Adrian

thanks,
-serge

Re: [PATCH try #2] security: Convert LSM into a static interface

[Adrian Bunk]

On Tue, Jun 26, 2007 at 10:53:29AM -0500, Serge E. Hallyn wrote:
> Quoting Adrian Bunk (bunk@stusta.de):
> > On Tue, Jun 26, 2007 at 09:06:44AM -0500, Serge E. Hallyn wrote:
> > > Quoting Adrian Bunk (bunk@stusta.de):
> > > > On Mon, Jun 25, 2007 at 10:57:31PM -0500, Serge E. Hallyn wrote:
> > > > > Quoting James Morris (jmorris@namei.org):
> > > > > > On Mon, 25 Jun 2007, Andreas Gruenbacher wrote:
> > > > > > 
> > > > > > > It's useful for some LSMs to be modular, and LSMs which are y/n options won't 
> > > > > > > have any security architecture issues with unloading at all. 
> > > > > > 
> > > > > > Which LSMs?  Upstream, there are SELinux and capabilty, and they're not 
> > > > > > safe as loadable modules.
> > > > > > 
> > > > > > > The mere fact 
> > > > > > > that SELinux cannot be built as a module is a rather weak argument for 
> > > > > > > disabling LSM modules as a whole, so  please don't.
> > > > > > 
> > > > > > That's not the argument.  Please review the thread.
> > > > > 
> > > > > The argument is 'abuse', right?
> > > > > 
> > > > > Abuse is defined as using the LSM hooks for non-security applications,
> > > > > right?
> > > > > 
> > > > > It seems to me that the community is doing a good job of discouraging
> > > > > such abuse - by redirecting the "wrong-doers" to implement proper
> > > > > upstream solutions, i.e. taskstats, the audit subsystem, etc.
> > > > > 
> > > > > Such encouragement seems a far better response than taking away freedoms
> > > > > and flexibility from everyone.
> > > > 
> > > > We are not living in a world where everyone had good intentions...
> > > 
> > > Oh no, i took a wrong turn somewhere  :)
> > > 
> > > > For _some_ "wrong-doers" your approach works.
> > > > 
> > > > But how do you convince the "wrong-doers" who do things like putting 
> > > > MODULE_LICENSE("GPL") into their binary-only modules and who ignore you 
> > > > and get away because noone sues them?
> > > 
> > > Do these really exist?  Maybe noone sues them because noone knows who
> > > they are...
> > 
> >    http://lwn.net/Articles/82306/
> 
> LinuxAnt?  Are they using LSM?
> 
> It looks to me like this patch will do nothing about them.

It was an example how creative "wrong-doers" sometimes are.

> > > But - note that you've changed completely the meaning of 'abuse'.
> > > So mine was wrong?
> > 
> > Technical and legal abuse are related.
> 
> True but going by your logic we could remove support for modules period
> to prevent legal abuse by non-gpl modules.

The problem is that this would result in distributions having to ship
50 MB kernel images.

> > For GPL'ed modules you might assume good faith and get the authors to do 
> > things in a proper way. Authors of legally questionable modules that 
> > cheat in many ways are quite a different issue.
> > 
> > > > The spirit of the GPLv2 is to defend the freedom of the software 
> > > > (different from the spirit of the BSD licence), and considering that 
> > > > there aren't many people defending the GPLv2 copyright of the Linux 
> > > > kernel at court against abusers, making it harder for people to do the 
> > > > abuse might not be the worst choice...
> > > 
> > > Well, but you seem to be saying that the license means squat, and
> > > resorting to making things inconvenient rather than illegal.
> > 
> > No, the point is that there's no reason for making illegal things 
> > convenient.
> 
> But no, the point is that that you are making legal things very
> inconvenient.
> 
> > I'm not talking about removing things that are used inside the kernel, 
> 
> Since capabilities can currently be compiled as a module, you are.
> (Though that sounds weird, so maybe I'm misreading what you are saying)

If the LSM maintainer says non-modular capabilities is the way to go
then there's no user left.

> > but what you call "freedom" can also be called "hooks for possible abuse".
> 
> Yup, that is true.
> 
> > Additionally, it both makes the kernel bigger for everyone and requires 
> > proper handling of loading/unloading in the security architecture.
> > 
> > > Now I guess if it really is accepted that that's the way it should be,
> > > then this patch will go in.
> 
> thanks,
> -serge

cu
Adrian

-- 

       "Is there not promise of rain?" Ling Tan asked suddenly out
        of the darkness. There had been need of rain for many days.
       "Only a promise," Lao Er said.
                                       Pearl S. Buck - Dragon Seed

Re: [PATCH try #2] security: Convert LSM into a static interface

[Greg KH]

On Tue, Jun 26, 2007 at 09:06:44AM -0500, Serge E. Hallyn wrote:
> Quoting Adrian Bunk (bunk@stusta.de):
> > On Mon, Jun 25, 2007 at 10:57:31PM -0500, Serge E. Hallyn wrote:
> > > Quoting James Morris (jmorris@namei.org):
> > > > On Mon, 25 Jun 2007, Andreas Gruenbacher wrote:
> > > > 
> > > > > It's useful for some LSMs to be modular, and LSMs which are y/n options won't 
> > > > > have any security architecture issues with unloading at all. 
> > > > 
> > > > Which LSMs?  Upstream, there are SELinux and capabilty, and they're not 
> > > > safe as loadable modules.
> > > > 
> > > > > The mere fact 
> > > > > that SELinux cannot be built as a module is a rather weak argument for 
> > > > > disabling LSM modules as a whole, so  please don't.
> > > > 
> > > > That's not the argument.  Please review the thread.
> > > 
> > > The argument is 'abuse', right?
> > > 
> > > Abuse is defined as using the LSM hooks for non-security applications,
> > > right?
> > > 
> > > It seems to me that the community is doing a good job of discouraging
> > > such abuse - by redirecting the "wrong-doers" to implement proper
> > > upstream solutions, i.e. taskstats, the audit subsystem, etc.
> > > 
> > > Such encouragement seems a far better response than taking away freedoms
> > > and flexibility from everyone.
> > 
> > We are not living in a world where everyone had good intentions...
> 
> Oh no, i took a wrong turn somewhere  :)
> 
> > For _some_ "wrong-doers" your approach works.
> > 
> > But how do you convince the "wrong-doers" who do things like putting 
> > MODULE_LICENSE("GPL") into their binary-only modules and who ignore you 
> > and get away because noone sues them?
> 
> Do these really exist?

Yes they do.

> Maybe noone sues them because noone knows who they are...

Maybe no one knows because the people doing the legal action against
them are trying to be nice and do it quietly.

And legal action takes time, it is quite slow going unfortunatly.

Heck, I've seen code that is even properly licensed under the GPL abuse
this security layer for things it was not ment to do at all, and that
stuff comes from _very_ big companies that really should know better...

So I agree that we should unexport it.  It will make people who want to
abuse the interface at least think twice about it.

thanks,

greg "I want to mark structures read-only" k-h

Re: [PATCH try #2] security: Convert LSM into a static interface

[Serge E. Hallyn]

Quoting Greg KH (greg@kroah.com):
> On Tue, Jun 26, 2007 at 09:06:44AM -0500, Serge E. Hallyn wrote:
> > Quoting Adrian Bunk (bunk@stusta.de):
> > > On Mon, Jun 25, 2007 at 10:57:31PM -0500, Serge E. Hallyn wrote:
> > > > Quoting James Morris (jmorris@namei.org):
> > > > > On Mon, 25 Jun 2007, Andreas Gruenbacher wrote:
> > > > > 
> > > > > > It's useful for some LSMs to be modular, and LSMs which are y/n options won't 
> > > > > > have any security architecture issues with unloading at all. 
> > > > > 
> > > > > Which LSMs?  Upstream, there are SELinux and capabilty, and they're not 
> > > > > safe as loadable modules.
> > > > > 
> > > > > > The mere fact 
> > > > > > that SELinux cannot be built as a module is a rather weak argument for 
> > > > > > disabling LSM modules as a whole, so  please don't.
> > > > > 
> > > > > That's not the argument.  Please review the thread.
> > > > 
> > > > The argument is 'abuse', right?
> > > > 
> > > > Abuse is defined as using the LSM hooks for non-security applications,
> > > > right?
> > > > 
> > > > It seems to me that the community is doing a good job of discouraging
> > > > such abuse - by redirecting the "wrong-doers" to implement proper
> > > > upstream solutions, i.e. taskstats, the audit subsystem, etc.
> > > > 
> > > > Such encouragement seems a far better response than taking away freedoms
> > > > and flexibility from everyone.
> > > 
> > > We are not living in a world where everyone had good intentions...
> > 
> > Oh no, i took a wrong turn somewhere  :)
> > 
> > > For _some_ "wrong-doers" your approach works.
> > > 
> > > But how do you convince the "wrong-doers" who do things like putting 
> > > MODULE_LICENSE("GPL") into their binary-only modules and who ignore you 
> > > and get away because noone sues them?
> > 
> > Do these really exist?
> 
> Yes they do.
> 
> > Maybe noone sues them because noone knows who they are...
> 
> Maybe no one knows because the people doing the legal action against
> them are trying to be nice and do it quietly.

So they're being nice to the violaters, and then clamping down on
everyone...

> And legal action takes time, it is quite slow going unfortunatly.
> 
> Heck, I've seen code that is even properly licensed under the GPL abuse
> this security layer for things it was not ment to do at all, and that
> stuff comes from _very_ big companies that really should know better...

But that's back to the other type of 'abuse' which i was originally
talking about, and which IMO is being well addressed through education.

As for the others, I have no better suggestions.  I wish I did.

> So I agree that we should unexport it.  It will make people who want to
> abuse the interface at least think twice about it.

And those who don't abuse it too.

> thanks,
> 
> greg "I want to mark structures read-only" k-h

And I know I'm not the one who's going to stop you...

-serge

Re: [PATCH try #2] security: Convert LSM into a static interface

[Kyle Moffett]

On Jun 25, 2007, at 16:37:58, Andreas Gruenbacher wrote:
> On Monday 25 June 2007 06:33, James Morris wrote:
>> Convert LSM into a static interface, as the ability to unload a  
>> security module is not required by in-tree users and potentially  
>> complicates the overall security architecture.
>
> It's useful for some LSMs to be modular, and LSMs which are y/n  
> options won't have any security architecture issues with unloading  
> at all. The mere fact that SELinux cannot be built as a module is a  
> rather weak argument for disabling LSM modules as a whole, so   
> please don't.

Here are a few questions for you:

   1)  What do you expect to happen to all the megs of security data  
when you "rmmod selinux"?  Do you maintain a massive linked list of  
security data (with all the locking and performance problems) so that  
you can iterate over it calling kfree()? What synchronization  
primitive do we have right now which could safely stop all CPUs  
outside of security calls while we NULL out and free security data  
and disable security operations?  Don't say "software suspend" and  
"process freezer", since those have whole order-of-magnitude- 
complexity problems of their own (and don't always work right either).

   2)  When you "modprobe my_custom_security_module", how exactly do  
you expect that all the processes, files, shared memory segments,  
file descriptors, sockets, SYSV mutexes, packets, etc will get  
appropriate security pointers?  This isn't even solvable the same way  
the "rmmod" problem is, since most of that isn't even accessible  
without iterating over the ENTIRE dcache, icache, every process,  
every process' file-descriptors, every socket, every unix socket,  
every anonymous socket, every SYSV shm object, every currently-in- 
process packet.

   3)  This sounds suspiciously like "The mere fact that the  
Linux-2.6-VM cannot be built as a module is a rather weak argument  
for disabling VFS modules as a whole".  We don't do "pluggable  
fundamental infrastructure" in Linux.  If it's fundamental  
infrastructure then you eliminate as many differences as possible and  
leave the rest to CONFIG options (or delete it entirely).


So... Do you have a proposal for solving those rather fundamental  
design gotchas?  If so, I'm sure everybody here would love to see  
your patch; though maybe not if it's a 32MB patch-zilla-of-doom (AKPM  
beware, the merge-conflict-from-hell is on its way).  On the other  
hand, if you accept that these problems basically can't be solved and  
we make things static and rip out a bunch of code, we can probably  
improve our performance under larger security models (like SELinux/ 
AppArmor/TOMOYO/MagicSecurityFlavorOfTheWeek(TM)) by a percent or two.

Cheers,
Kyle Moffett

Re: [PATCH try #2] security: Convert LSM into a static interface

[Kyle Moffett]

Oops, typo:

On Jun 26, 2007, at 00:09:24, Kyle Moffett wrote:
> This sounds suspiciously like "The mere fact that the Linux-2.6-VM  
> cannot be built as a module is a rather weak argument for disabling  
> VFS modules as a whole"

Meant to say: "...disabling VM modules as a whole."

Cheers,
Kyle Moffett

Re: [PATCH try #2] security: Convert LSM into a static interface

[Serge E. Hallyn]

Quoting Kyle Moffett (mrmacman_g4@mac.com):
> On Jun 25, 2007, at 16:37:58, Andreas Gruenbacher wrote:
> >On Monday 25 June 2007 06:33, James Morris wrote:
> >>Convert LSM into a static interface, as the ability to unload a  
> >>security module is not required by in-tree users and potentially  
> >>complicates the overall security architecture.
> >
> >It's useful for some LSMs to be modular, and LSMs which are y/n  
> >options won't have any security architecture issues with unloading  
> >at all. The mere fact that SELinux cannot be built as a module is a  
> >rather weak argument for disabling LSM modules as a whole, so   
> >please don't.
> 
> Here are a few questions for you:
> 
>   1)  What do you expect to happen to all the megs of security data  
> when you "rmmod selinux"?

Read the sentence right above yours again.

Noone is saying we should be able to rmmod selinux.

> Do you maintain a massive linked list of  
> security data (with all the locking and performance problems) so that  
> you can iterate over it calling kfree()? What synchronization  
> primitive do we have right now which could safely stop all CPUs  
> outside of security calls while we NULL out and free security data  
> and disable security operations?  Don't say "software suspend" and  
> "process freezer", since those have whole order-of-magnitude- 
> complexity problems of their own (and don't always work right either).
> 
>   2)  When you "modprobe my_custom_security_module", how exactly do  
> you expect that all the processes, files, shared memory segments,  
> file descriptors, sockets, SYSV mutexes, packets, etc will get  
> appropriate security pointers?>

Those don't all need labels for capabilities, for instance.  This
question is as wrong as the last one.

> This isn't even solvable the same way  
> the "rmmod" problem is, since most of that isn't even accessible  
> without iterating over the ENTIRE dcache, icache, every process,  
> every process' file-descriptors, every socket, every unix socket,  
> every anonymous socket, every SYSV shm object, every currently-in- 
> process packet.
> 
>   3)  This sounds suspiciously like "The mere fact that the  
> Linux-2.6-VM cannot be built as a module is a rather weak argument  
> for disabling VFS modules as a whole".  We don't do "pluggable  

No, your argument sounds like "my fs can't be a module so neither should
any."

> fundamental infrastructure" in Linux.  If it's fundamental  
> infrastructure then you eliminate as many differences as possible and  
> leave the rest to CONFIG options (or delete it entirely).
> 
> 
> So... Do you have a proposal for solving those rather fundamental  
> design gotchas?  If so, I'm sure everybody here would love to see  
> your patch; though maybe not if it's a 32MB patch-zilla-of-doom (AKPM  
> beware, the merge-conflict-from-hell is on its way).  On the other  
> hand, if you accept that these problems basically can't be solved and  
> we make things static and rip out a bunch of code, we can probably  
> improve our performance under larger security models (like SELinux/ 
> AppArmor/TOMOYO/MagicSecurityFlavorOfTheWeek(TM)) by a percent or two.
> 
> Cheers,
> Kyle Moffett
> 
> -
> To unsubscribe from this list: send the line "unsubscribe 
> linux-security-module" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH try #2] security: Convert LSM into a static interface

[Kyle Moffett]

On Jun 26, 2007, at 09:47:12, Serge E. Hallyn wrote:
> Quoting Kyle Moffett (mrmacman_g4@mac.com):
>> On Jun 25, 2007, at 16:37:58, Andreas Gruenbacher wrote:
>>> It's useful for some LSMs to be modular, and LSMs which are y/n  
>>> options won't have any security architecture issues with  
>>> unloading at all. The mere fact that SELinux cannot be built as a  
>>> module is a rather weak argument for disabling LSM modules as a  
>>> whole, so please don't.
>>
>> Here are a few questions for you:
>>
>>   1)  What do you expect to happen to all the megs of security  
>> data when you "rmmod selinux"?
>
> Read the sentence right above yours again.
>
> Noone is saying we should be able to rmmod selinux.

Ok, so say we extend LSM to do what AppArmor or TOMOYO need, what do  
you expect to happen when you "rmmod tomoyo", "rmmod apparmor", or  
whatever?  Each of those is also going to stick lots of context on  
various objects during the course of running, the same way that the  
VM subsystem sticks lots of context on filesystem pages while  
running.  Besides, even the standard "capabilities" module wants to  
attach a list of capabilities to every process and defines  
inheritance rules for them.  Ergo you have the problems described below:

>> Do you maintain a massive linked list of security data (with all  
>> the locking and performance problems) so that you can iterate over  
>> it calling kfree()? What synchronization primitive do we have  
>> right now which could safely stop all CPUs outside of security  
>> calls while we NULL out and free security data and disable  
>> security operations?  Don't say "software suspend" and "process  
>> freezer", since those have whole order-of-magnitude-complexity  
>> problems of their own (and don't always work right either).
>
>>   2)  When you "modprobe my_custom_security_module", how exactly  
>> do you expect that all the processes, files, shared memory  
>> segments, file descriptors, sockets, SYSV mutexes, packets, etc  
>> will get appropriate security pointers?
>
> Those don't all need labels for capabilities, for instance.  This  
> question is as wrong as the last one.

Ok, so let's just restrict ourselves to the simple dumb-as-dirt  
capabilities module.  Every process is "labeled" with capabilities  
while running under that LSM, right?  What happens when you "rmmod  
capabilities"?  Do you iterate over all the processes to remove their  
security data even while they may be using it?  Or do you just let it  
leak?  Some daemons test if capabilities are supported, and if so  
they modify their capability set instead of forking a high-priv and a  
low-priv process and doing IPC.  When you remove the capabilities  
module, suddenly all those programs will lose that critical "low- 
privilege" data and become full root.  What happens later when you  
"modprobe capabilities"?  Do you suddenly have to stop the system  
while you iterate over EVERY process to set capabilities based on  
whether it's root or not?  It's also impossible to determine from a  
given state in time what processes should have capabilities, as the  
model includes inheritance, which includes processes that don't even  
exist anymore.

>> 3)  This sounds suspiciously like "The mere fact that the  
>> Linux-2.6-VM cannot be built as a module is a rather weak argument  
>> for disabling VFS modules as a whole".
>
> No, your argument sounds like "my fs can't be a module so neither  
> should any."

Let's go over the differences between "my fs" and "my LSM", and the  
similarities between "my VM" and "my LSM":  Filesystems don't get  
hooked from virtually every userspace-initiated operation, whereas  
both VMs and LSMs do.  VMs and LSMs attach anonymous state data to a  
large percentage of the allocated objects in the system, whereas  
filesystems allocate their own independent datastructure and use  
that.  Would you want to "rmmod ext3" and then "modprobe ext2" while  
you have an ext2-as-ext3 filesystem *mounted*???  If you want a good  
analogy, that's a better one than the "my fs can't be a module" crap.

This whole discussion boils down to 2 points:
   1) As currently implemented, no LSM may be safely rmmod-ed
   2) Someone has submitted a patch which fixes that problem (you  
can't rmmod them at all, so no crashes)

If you really want to do modular LSMs, then you need to submit a  
patch which fixes all the race conditions in LSM removal *without*  
adding much extra overhead.  I'm sure if your solutions works then  
everyone will be much more open to modular LSMs.  I said this before:

>> So... Do you have a proposal for solving those rather fundamental  
>> design gotchas?  If so, I'm sure everybody here would love to see  
>> your patch

Cheers,
Kyle Moffett

Re: [PATCH try #2] security: Convert LSM into a static interface

[Crispin Cowan]

Kyle Moffett wrote:
> Let's go over the differences between "my fs" and "my LSM", and the
> similarities between "my VM" and "my LSM":  Filesystems don't get
> hooked from virtually every userspace-initiated operation, whereas
> both VMs and LSMs do.  VMs and LSMs attach anonymous state data to a
> large percentage of the allocated objects in the system, whereas
> filesystems allocate their own independent datastructure and use
> that.  Would you want to "rmmod ext3" and then "modprobe ext2" while
> you have an ext2-as-ext3 filesystem *mounted*???  If you want a good
> analogy, that's a better one than the "my fs can't be a module" crap.
>
> This whole discussion boils down to 2 points:
>   1) As currently implemented, no LSM may be safely rmmod-ed
>   2) Someone has submitted a patch which fixes that problem (you can't
> rmmod them at all, so no crashes)
>
> If you really want to do modular LSMs, then you need to submit a patch
> which fixes all the race conditions in LSM removal *without* adding
> much extra overhead.  I'm sure if your solutions works then everyone
> will be much more open to modular LSMs.  I said this before:
Hmmm. You seem to be mostly concerned with safely rmmod'ing modules. In
contrast, my main concern with the proposed patch is that it removes the
ability to *insert* a module.

Consider the use case of joe admin who is running enterprise-supported
RHEL or SLES, and wants to try some newfangled LSM FooSecureMod thingie.
So he grabs a machine, config's selinux=0 or apparmor=0 and loads his
own module on boot, and plays with it. He even likes FooSecure, better
than SELinux or AppArmor, and wants to roll it out across his data center.

Without James's patch, he can do that, and at worst has a tainted
kernel. RH or Novell or his favorite distro vendor can fix that with a
wave of the hand and bless FooSecure as a module. With James's patch, he
has to patch his kernels, and then enterprise support is hopeless, to
say nothing of the barrier to entry that "patch and rebuild kernel" is
more than many admins are willing to do.

So to solve the problem James & Kyle are concerned with, and preserve
user choice, how about we *only* remove the ability to rmmod, and leave
in place the ability to modprobe? Or even easier, LSMs that don't want
to be unloaded can just block rmmod, and simple LSMs that can be
unloaded safely can permit it.

Crispin

-- 
Crispin Cowan, Ph.D.               http://crispincowan.com/~crispin/
Director of Software Engineering   http://novell.com
	AppArmor Chat: irc.oftc.net/#apparmor

Re: [PATCH try #2] security: Convert LSM into a static interface

[Kyle Moffett]

On Jun 26, 2007, at 20:57:53, Crispin Cowan wrote:
> Kyle Moffett wrote:
>> Let's go over the differences between "my fs" and "my LSM", and  
>> the similarities between "my VM" and "my LSM":  Filesystems don't  
>> get hooked from virtually every userspace-initiated operation,  
>> whereas both VMs and LSMs do.  VMs and LSMs attach anonymous state  
>> data to a large percentage of the allocated objects in the system,  
>> whereas filesystems allocate their own independent datastructure  
>> and use that.  Would you want to "rmmod ext3" and then "modprobe  
>> ext2" while you have an ext2-as-ext3 filesystem *mounted*???  If  
>> you want a good analogy, that's a better one than the "my fs can't  
>> be a module" crap.
>>
>> This whole discussion boils down to 2 points:
>>   1) As currently implemented, no LSM may be safely rmmod-ed
>>   2) Someone has submitted a patch which fixes that problem (you  
>> can't
>> rmmod them at all, so no crashes)
>>
>> If you really want to do modular LSMs, then you need to submit a  
>> patch which fixes all the race conditions in LSM removal *without*  
>> adding much extra overhead.  I'm sure if your solutions works then  
>> everyone will be much more open to modular LSMs.
>
> Hmmm. You seem to be mostly concerned with safely rmmod'ing  
> modules. In contrast, my main concern with the proposed patch is  
> that it removes the ability to *insert* a module.

You must have missed this in my emails:
>>   2) When you "modprobe my_custom_security_module", how exactly do  
>> you expect that all the processes, files, shared memory segments,  
>> file descriptors, sockets, SYSV mutexes, packets, etc will get  
>> appropriate security pointers?  This isn't even solvable the same  
>> way the "rmmod" problem is, since most of that isn't even  
>> accessible without iterating over the ENTIRE dcache, icache, every  
>> process, every process' file-descriptors, every socket, every unix  
>> socket, every anonymous socket, every SYSV shm object, every  
>> currently-in-process packet.

I'd argue that security-module-insertion is actually MORE complicated  
than removal.  Here's one example:  TOMOYO cares about the process  
execution tree, but you can't penalize the no-LSM case by a percent  
or two to add that kind of data.  When TOMOYO is loaded, it wants to  
do access control based on process execution trees for which data  
DOES NOT EXIST!!!  Not only that, but the processes which originally  
ran the one you care about (and which you'd need to recreate that  
data) may have exited anywhere from seconds to years before.  It is  
fundamentally IMPOSSIBLE to recreate that data, even if you could  
solve the problems of how to do it while the system is running  
without racing with existing process operations.  Imagine a process  
which hasn't had security data tagged to it yet which opens thousands  
of FIFOs per second, waits for your tagging code to assign security  
data to them in the filesystem, and then removes them; if you did it  
right you could prevent the code from EVER completely tagging every  
object (even assuming you could recreate enough information).

Such a need to add extra security data to multiple classes of objects  
is *fundamental* to any security module (isn't that the whole  
point?)  As such, you can't just "modprobe" one and expect it to  
work.  That's like mounting an ext2 filesystem, and then later trying  
to "modprobe ext3" and dynamically switch to the ext3 code and enable  
journalling all at once ON THE MOUNTED FILESYSTEM!!!

Sure, theoretically it *could* be done, but the code complexity is  
hardly worth it (plus nobody has yet even tried posting patches to  
make it happen).

> Consider the use case of joe admin who is running enterprise- 
> supported RHEL or SLES, and wants to try some newfangled LSM  
> FooSecureMod thingie.  So he grabs a machine, config's selinux=0 or  
> apparmor=0 and loads his own module on boot, and plays with it. He  
> even likes FooSecure, better than SELinux or AppArmor, and wants to  
> roll it out across his data center.

Flatly impossible.  You simply cannot "load" a security module and  
hope to provide any useful information about the system's present  
state.  If you want comprehensive security it has to be there before  
a single byte of userspace code is executed.  SELinux sort-of handles  
unlabelled objects by treating them with a small set of initial  
"types", but that's only enough to get the system up enough to  
actually relabel objects with type-transitions (after init loads the  
selinux policy it reexecs itself, before doing anything else).

> So to solve the problem James & Kyle are concerned with, and  
> preserve user choice, how about we *only* remove the ability to  
> rmmod, and leave in place the ability to modprobe? Or even easier,  
> LSMs that don't want to be unloaded can just block rmmod, and  
> simple LSMs that can be unloaded safely can permit it.

An LSM simple enough to unload would be too simple for anybody to  
want to load in the first place (even capabilities can have this  
problem).  What kind of security module would you load that doesn't  
need to add security data to objects?

Then of course there's also the -ENOPATCH problem.

Cheers,
Kyle Moffett

Re: [PATCH try #2] security: Convert LSM into a static interface

[Chris Wright]

* Crispin Cowan (crispin@novell.com) wrote:
> and simple LSMs that can be
> unloaded safely can permit it.

there are none, and making the above possible is prohibitively
expensive.

Re: [PATCH try #2] security: Convert LSM into a static interface

[Serge E. Hallyn]

Quoting Kyle Moffett (mrmacman_g4@mac.com):
> This whole discussion boils down to 2 points:

Yes it can, but not the two you list.

>   1) As currently implemented, no LSM may be safely rmmod-ed

That's not the rationale for the patch, it's just some talking point you
picked up.  The rationale for the patch is to prevent abuse.  So point 1
is

	1) Is the LSM infrastructure being abused, and how detrimental
	   is that abuse

As has come up, the abuse comes in two forms, and people seem to want to
blur the two forms to make it seem especially relevant and heinous...

>   2) Someone has submitted a patch which fixes that problem (you  
> can't rmmod them at all, so no crashes)

	2) Is the loss of flexibility in the LSM framework a worthwhile
	   tradoff against the abuse prevention.

Clearly I and a very few others feel no, and a very vocal set (which
sure sounds like a majority) says yes.

Now quit trying to give technical justifications for something which is
technical only insofar as it is a technical roadblock to prevent a legal
problem.

> If you really want to do modular LSMs, then you need to submit a  
> patch which fixes all the race conditions in LSM removal *without*  

LSM is an infrastructure.  It's up to the modules to provide that, and
it can be done.  DTE used to do it.  Dirjail used to do it.  Capability
does it.

And since LSM won't be modular anymore it doesn't matter.

> adding much extra overhead.  I'm sure if your solutions works then  
> everyone will be much more open to modular LSMs.  I said this before:

Another blatant lie, not unlike "come to the table to upstream your LSM,
and we'll help you, honest."

(The funny thing about that is, I actually like SELinux, more than the
alternatives in general.  I just can't stand the attitudes voice by much
of its camp.)

-serge

PS - should we rename 'LSM' to 'LSI' - linux security infrastructure?
Calling it LSM now is kind of moronic.

Re: [PATCH try #2] security: Convert LSM into a static interface

[James Morris]

On Wed, 27 Jun 2007, Serge E. Hallyn wrote:

> Quoting Kyle Moffett (mrmacman_g4@mac.com):
> > This whole discussion boils down to 2 points:
> 
> Yes it can, but not the two you list.
> 
> >   1) As currently implemented, no LSM may be safely rmmod-ed
> 
> That's not the rationale for the patch, it's just some talking point you
> picked up.  The rationale for the patch is to prevent abuse.

This is not correct.  Reducing API abuse is simply a bonus.

The rationale for the patch is to remove unneeded infrastructure which 
complicates security by introducing the idea that the security module can 
be removed at all.

It was in response to your very own posting about the new capabilities 
code which would need to take this into account.


Recall:

On Sun, 24 Jun 2007, Serge E. Hallyn wrote:

> > 2) Allocate capability bit-31 for CAP_SETFCAP, and use it to gate
> > whether the user can set this xattr on a file or not. CAP_SYS_ADMIN is
> > way too overloaded and this functionality is special.
> 
> The functionality is special, but someone with CAP_SYS_ADMIN can always
> unload the capability module and create the security.capability xattr
> using the dummy module.
> 
> If we do add this cap, do we want to make it apply to all security.*
> xattrs?

The underlying issue here is the notion of security mechanisms which are 
built as loadable modules.  It's not useful for any in-tree users, and 
introduces several unnecessary problems which then need to be addressed.

A better approach would be to make LSM a statically linked interface.

This would also allow us to unexport the LSM symbols and reduce the API 
abuse by third-party modules.



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

Re: [PATCH try #2] security: Convert LSM into a static interface

[Serge E. Hallyn]

Quoting James Morris (jmorris@namei.org):
> On Wed, 27 Jun 2007, Serge E. Hallyn wrote:
> 
> > Quoting Kyle Moffett (mrmacman_g4@mac.com):
> > > This whole discussion boils down to 2 points:
> > 
> > Yes it can, but not the two you list.
> > 
> > >   1) As currently implemented, no LSM may be safely rmmod-ed
> > 
> > That's not the rationale for the patch, it's just some talking point you
> > picked up.  The rationale for the patch is to prevent abuse.
> 
> This is not correct.  Reducing API abuse is simply a bonus.
> 
> The rationale for the patch is to remove unneeded infrastructure which 
> complicates security by introducing the idea that the security module can 
> be removed at all.
> 
> It was in response to your very own posting about the new capabilities 
> code which would need to take this into account.

It's (IMO) by far not the optimal "solution"  :)  If it is felt a
solution is really needed, re-introduction of a
security_ops->module_exit hook and introduction of CAP_SYS_CAPDISABLE
would be better.

But I'm well aware there are far too many (separate and not so separate)
agendas driving this, and have no expectations of being able to stop it.

James, FWIW, I'm sorry I haven't had a chance to actually test the
patch.  I'll try to get around to that today or at least this week.

-serge

> Recall:
> 
> On Sun, 24 Jun 2007, Serge E. Hallyn wrote:
> 
> > > 2) Allocate capability bit-31 for CAP_SETFCAP, and use it to gate
> > > whether the user can set this xattr on a file or not. CAP_SYS_ADMIN is
> > > way too overloaded and this functionality is special.
> > 
> > The functionality is special, but someone with CAP_SYS_ADMIN can always
> > unload the capability module and create the security.capability xattr
> > using the dummy module.
> > 
> > If we do add this cap, do we want to make it apply to all security.*
> > xattrs?
> 
> The underlying issue here is the notion of security mechanisms which are 
> built as loadable modules.  It's not useful for any in-tree users, and 
> introduces several unnecessary problems which then need to be addressed.
> 
> A better approach would be to make LSM a statically linked interface.
> 
> This would also allow us to unexport the LSM symbols and reduce the API 
> abuse by third-party modules.
> 
> 
> 
> - James
> -- 
> James Morris
> <jmorris@namei.org>
> -
> To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH try #2] security: Convert LSM into a static interface

[Serge E. Hallyn]

Quoting Serge E. Hallyn (serue@us.ibm.com):
> Quoting James Morris (jmorris@namei.org):
> > On Wed, 27 Jun 2007, Serge E. Hallyn wrote:
> > 
> > > Quoting Kyle Moffett (mrmacman_g4@mac.com):
> > > > This whole discussion boils down to 2 points:
> > > 
> > > Yes it can, but not the two you list.
> > > 
> > > >   1) As currently implemented, no LSM may be safely rmmod-ed
> > > 
> > > That's not the rationale for the patch, it's just some talking point you
> > > picked up.  The rationale for the patch is to prevent abuse.
> > 
> > This is not correct.  Reducing API abuse is simply a bonus.
> > 
> > The rationale for the patch is to remove unneeded infrastructure which 
> > complicates security by introducing the idea that the security module can 
> > be removed at all.
> > 
> > It was in response to your very own posting about the new capabilities 
> > code which would need to take this into account.
> 
> It's (IMO) by far not the optimal "solution"  :)  If it is felt a
> solution is really needed, re-introduction of a
> security_ops->module_exit hook and introduction of CAP_SYS_CAPDISABLE
> would be better.
> 
> But I'm well aware there are far too many (separate and not so separate)
> agendas driving this, and have no expectations of being able to stop it.
> 
> James, FWIW, I'm sorry I haven't had a chance to actually test the
> patch.  I'll try to get around to that today or at least this week.

Patch tests fine for me for expected capability behavior with lsm=n,
lsm=y, lsm=y+capability=y, lsm=y+selinux=y, and lsm=y+caps=y+selinux=y.

So while I'm opposed to the patch, it appears to be safe.

thanks,
-serge

Re: [PATCH try #2] security: Convert LSM into a static interface

[James Morris]

On Wed, 27 Jun 2007, Serge E. Hallyn wrote:

> Patch tests fine for me for expected capability behavior with lsm=n,
> lsm=y, lsm=y+capability=y, lsm=y+selinux=y, and lsm=y+caps=y+selinux=y.
> 
> So while I'm opposed to the patch, it appears to be safe.

I've also tested a bunch of scenarios: allmodconfig, lsm=y,cap=n, 
selinux=y,cap=n  etc.


-- 
James Morris
<jmorris@namei.org>

Re: [PATCH try #2] security: Convert LSM into a static interface

[Serge E. Hallyn]

Quoting James Morris (jmorris@namei.org):
> On Wed, 27 Jun 2007, Serge E. Hallyn wrote:
> 
> > Patch tests fine for me for expected capability behavior with lsm=n,
> > lsm=y, lsm=y+capability=y, lsm=y+selinux=y, and lsm=y+caps=y+selinux=y.
> > 
> > So while I'm opposed to the patch, it appears to be safe.
> 
> I've also tested a bunch of scenarios: allmodconfig, lsm=y,cap=n, 
> selinux=y,cap=n  etc.

I was wondering about the uninlining of all those functions, so did a
set of performance runs.  Found no statistically relevant change in
dbench, tbench, or reaim.  (tried to run kernbench too but the benchmark
failed somewhere and i didn't care enough to look into it)

-serge

Re: [PATCH][RFC] security: Convert LSM into a static interface

[Serge E. Hallyn]

Quoting James Morris (jmorris@namei.org):
> Convert LSM into a static interface, as the ability to unload a security 
> module is not required by in-tree users and potentially complicates the 
> overall security architecture.
> 
> Needlessly exported LSM symbols have been unexported, to help reduce API 
> abuse.
> 
> Module parameters for the capability and root_plug modules have been
> converted to kernel parameters.
> 
> The SECURITY_FRAMEWORK_VERSION macro has also been removed.

Sigh, as much as I would *like* to stay out of this (I don't
use modules at all on any system where I can avoid it), won't
it make development - and especially testing - of new lsms
much more painful and therefore less likely?

I realize there has been a dearth of new LSMs to date, but if
for instance a new solaris 10 based capability module were written,
well, people would want to be able to

	rmmod capability
	modprobe cap_prm

thanks,
-serge

> Signed-off-by: James Morris <jmorris@namei.org>
> ---
> 
> Please review & let me know if anything is broken.
> 
> 
>  Documentation/kernel-parameters.txt |   17 +++++++++++
>  security/Kconfig                    |    4 +-
>  security/capability.c               |   32 ++++----------------
>  security/commoncap.c                |    3 --
>  security/dummy.c                    |    1 -
>  security/root_plug.c                |   53 +++++++++++++---------------------
>  security/security.c                 |    9 +----
>  security/selinux/hooks.c            |    1 -
>  security/selinux/xfrm.c             |    1 -
>  9 files changed, 48 insertions(+), 73 deletions(-)
> 
> diff --git a/Documentation/kernel-parameters.txt b/Documentation/kernel-parameters.txt
> index 5d0283c..4c406fb 100644
> --- a/Documentation/kernel-parameters.txt
> +++ b/Documentation/kernel-parameters.txt
> @@ -74,10 +74,12 @@ parameter is applicable:
>  	PPT	Parallel port support is enabled.
>  	PS2	Appropriate PS/2 support is enabled.
>  	RAM	RAM disk support is enabled.
> +	ROOTPLUG The example Root Plug LSM is enabled.
>  	S390	S390 architecture is enabled.
>  	SCSI	Appropriate SCSI support is enabled.
>  			A lot of drivers has their options described inside of
>  			Documentation/scsi/.
> +	SECURITY Different security models are enabled.
>  	SELINUX SELinux support is enabled.
>  	SERIAL	Serial support is enabled.
>  	SH	SuperH architecture is enabled.
> @@ -376,6 +378,12 @@ and is between 256 and 4096 characters. It is defined in the file
>  			possible to determine what the correct size should be.
>  			This option provides an override for these situations.
>  
> +	capability_disable=
> +			[SECURITY] Disable capabilities.  This would normally
> +			be used only if an alternative security model is to be
> +			configured.  Potentially dangerous and should only be
> +			used if you are entirely sure of the consequences.
> +
>  	cdu31a=		[HW,CD]
>  			Format: <io>,<irq>[,PAS]
>  			See header of drivers/cdrom/cdu31a.c.
> @@ -1541,6 +1549,15 @@ and is between 256 and 4096 characters. It is defined in the file
>  
>  	rootfstype=	[KNL] Set root filesystem type
>  
> +	root_plug_vendor_id=
> +			[ROOTPLUG] Override the default vendor ID
> +
> +	root_plug_product_id=
> +			[ROOTPLUG] Override the default product ID
> +
> +	root_plug_debug=
> +			[ROOTPLUG] Enable debugging output
> +
>  	rw		[KNL] Mount root device read-write on boot
>  
>  	S		[KNL] Run init in single mode
> diff --git a/security/Kconfig b/security/Kconfig
> index 460e5c9..8ae5490 100644
> --- a/security/Kconfig
> +++ b/security/Kconfig
> @@ -74,14 +74,14 @@ config SECURITY_NETWORK_XFRM
>  	  If you are unsure how to answer this question, answer N.
>  
>  config SECURITY_CAPABILITIES
> -	tristate "Default Linux Capabilities"
> +	bool "Default Linux Capabilities"
>  	depends on SECURITY
>  	help
>  	  This enables the "default" Linux capabilities functionality.
>  	  If you are unsure how to answer this question, answer Y.
>  
>  config SECURITY_ROOTPLUG
> -	tristate "Root Plug Support"
> +	bool "Root Plug Support"
>  	depends on USB && SECURITY
>  	help
>  	  This is a sample LSM module that should only be used as such.
> diff --git a/security/capability.c b/security/capability.c
> index 38296a0..1c97953 100644
> --- a/security/capability.c
> +++ b/security/capability.c
> @@ -8,7 +8,6 @@
>   *
>   */
>  
> -#include <linux/module.h>
>  #include <linux/init.h>
>  #include <linux/kernel.h>
>  #include <linux/security.h>
> @@ -51,8 +50,13 @@ static struct security_operations capability_ops = {
>  static int secondary;
>  
>  static int capability_disable;
> -module_param_named(disable, capability_disable, int, 0);
> -MODULE_PARM_DESC(disable, "To disable capabilities module set disable = 1");
> +
> +static int __init capability_disable_setup(char *str)
> +{
> +	capability_disable = simple_strtol(str, NULL, 0);
> +	return 1;
> +}
> +__setup("capability_disable=", capability_disable_setup);
>  
>  static int __init capability_init (void)
>  {
> @@ -75,26 +79,4 @@ static int __init capability_init (void)
>  	return 0;
>  }
>  
> -static void __exit capability_exit (void)
> -{
> -	if (capability_disable)
> -		return;
> -	/* remove ourselves from the security framework */
> -	if (secondary) {
> -		if (mod_unreg_security (KBUILD_MODNAME, &capability_ops))
> -			printk (KERN_INFO "Failure unregistering capabilities "
> -				"with primary module.\n");
> -		return;
> -	}
> -
> -	if (unregister_security (&capability_ops)) {
> -		printk (KERN_INFO
> -			"Failure unregistering capabilities with the kernel\n");
> -	}
> -}
> -
>  security_initcall (capability_init);
> -module_exit (capability_exit);
> -
> -MODULE_DESCRIPTION("Standard Linux Capabilities Security Module");
> -MODULE_LICENSE("GPL");
> diff --git a/security/commoncap.c b/security/commoncap.c
> index 384379e..04bd44b 100644
> --- a/security/commoncap.c
> +++ b/security/commoncap.c
> @@ -339,6 +339,3 @@ EXPORT_SYMBOL(cap_task_post_setuid);
>  EXPORT_SYMBOL(cap_task_reparent_to_init);
>  EXPORT_SYMBOL(cap_syslog);
>  EXPORT_SYMBOL(cap_vm_enough_memory);
> -
> -MODULE_DESCRIPTION("Standard Linux Common Capabilities Security Module");
> -MODULE_LICENSE("GPL");
> diff --git a/security/dummy.c b/security/dummy.c
> index 8ffd764..6d4e34b 100644
> --- a/security/dummy.c
> +++ b/security/dummy.c
> @@ -15,7 +15,6 @@
>  #undef DEBUG
>  
>  #include <linux/capability.h>
> -#include <linux/module.h>
>  #include <linux/kernel.h>
>  #include <linux/mman.h>
>  #include <linux/pagemap.h>
> diff --git a/security/root_plug.c b/security/root_plug.c
> index 38dd4f3..3125e25 100644
> --- a/security/root_plug.c
> +++ b/security/root_plug.c
> @@ -22,7 +22,6 @@
>   *	License.
>   */
>  
> -#include <linux/module.h>
>  #include <linux/kernel.h>
>  #include <linux/init.h>
>  #include <linux/security.h>
> @@ -33,25 +32,34 @@ static int secondary;
>  
>  /* default is a generic type of usb to serial converter */
>  static int vendor_id = 0x0557;
> -static int product_id = 0x2008;
>  
> -module_param(vendor_id, uint, 0400);
> -MODULE_PARM_DESC(vendor_id, "USB Vendor ID of device to look for");
> +static int __init root_plug_vendor_id(char *str)
> +{
> +	vendor_id = simple_strtol(str, NULL, 0);
> +	return 1;
> +}
> +__setup("root_plug_vendor_id=", root_plug_vendor_id);
> +
> +static int product_id = 0x2008;
>  
> -module_param(product_id, uint, 0400);
> -MODULE_PARM_DESC(product_id, "USB Product ID of device to look for");
> +static int __init root_plug_product_id(char *str)
> +{
> +	product_id = simple_strtol(str, NULL, 0);
> +	return 1;
> +}
> +__setup("root_plug_product_id=", root_plug_product_id);
>  
>  /* should we print out debug messages */
>  static int debug = 0;
>  
> -module_param(debug, bool, 0600);
> -MODULE_PARM_DESC(debug, "Debug enabled or not");
> +static int __init root_plug_debug(char *str)
> +{
> +	debug = simple_strtol(str, NULL, 0);
> +	return 1;
> +}
> +__setup("root_plug_debug=", root_plug_debug);
>  
> -#if defined(CONFIG_SECURITY_ROOTPLUG_MODULE)
> -#define MY_NAME THIS_MODULE->name
> -#else
>  #define MY_NAME "root_plug"
> -#endif
>  
>  #define root_dbg(fmt, arg...)					\
>  	do {							\
> @@ -117,25 +125,4 @@ static int __init rootplug_init (void)
>  	return 0;
>  }
>  
> -static void __exit rootplug_exit (void)
> -{
> -	/* remove ourselves from the security framework */
> -	if (secondary) {
> -		if (mod_unreg_security (MY_NAME, &rootplug_security_ops))
> -			printk (KERN_INFO "Failure unregistering Root Plug "
> -				" module with primary module.\n");
> -	} else { 
> -		if (unregister_security (&rootplug_security_ops)) {
> -			printk (KERN_INFO "Failure unregistering Root Plug "
> -				"module with the kernel\n");
> -		}
> -	}
> -	printk (KERN_INFO "Root Plug module removed\n");
> -}
> -
>  security_initcall (rootplug_init);
> -module_exit (rootplug_exit);
> -
> -MODULE_DESCRIPTION("Root Plug sample LSM module, written for Linux Journal article");
> -MODULE_LICENSE("GPL");
> -
> diff --git a/security/security.c b/security/security.c
> index fc8601b..e49dae2 100644
> --- a/security/security.c
> +++ b/security/security.c
> @@ -17,7 +17,6 @@
>  #include <linux/kernel.h>
>  #include <linux/security.h>
>  
> -#define SECURITY_FRAMEWORK_VERSION	"1.0.0"
>  
>  /* things that live in dummy.c */
>  extern struct security_operations dummy_security_ops;
> @@ -51,8 +50,7 @@ static void __init do_security_initcalls(void)
>   */
>  int __init security_init(void)
>  {
> -	printk(KERN_INFO "Security Framework v" SECURITY_FRAMEWORK_VERSION
> -	       " initialized\n");
> +	printk(KERN_INFO "Security Framework initialized\n");
>  
>  	if (verify(&dummy_security_ops)) {
>  		printk(KERN_ERR "%s could not verify "
> @@ -172,8 +170,5 @@ int mod_unreg_security(const char *name, struct security_operations *ops)
>  	return security_ops->unregister_security(name, ops);
>  }
>  
> -EXPORT_SYMBOL_GPL(register_security);
> -EXPORT_SYMBOL_GPL(unregister_security);
> -EXPORT_SYMBOL_GPL(mod_reg_security);
> -EXPORT_SYMBOL_GPL(mod_unreg_security);
> +/* Export required for modular use of security.h inlines */
>  EXPORT_SYMBOL(security_ops);
> diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
> index ad8dd4e..105319a 100644
> --- a/security/selinux/hooks.c
> +++ b/security/selinux/hooks.c
> @@ -20,7 +20,6 @@
>   *      as published by the Free Software Foundation.
>   */
>  
> -#include <linux/module.h>
>  #include <linux/init.h>
>  #include <linux/kernel.h>
>  #include <linux/ptrace.h>
> diff --git a/security/selinux/xfrm.c b/security/selinux/xfrm.c
> index bd8d1ef..b589238 100644
> --- a/security/selinux/xfrm.c
> +++ b/security/selinux/xfrm.c
> @@ -31,7 +31,6 @@
>   *   2. Emulating a reasonable SO_PEERSEC across machines
>   *   3. Testing addition of sk_policy's with security context via setsockopt
>   */
> -#include <linux/module.h>
>  #include <linux/kernel.h>
>  #include <linux/init.h>
>  #include <linux/security.h>
> -- 
> 1.5.2.1
> 
> -
> To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH][RFC] security: Convert LSM into a static interface

[Chris Wright]

* Serge E. Hallyn (serge@hallyn.com) wrote:
> Sigh, as much as I would *like* to stay out of this (I don't
> use modules at all on any system where I can avoid it), won't
> it make development - and especially testing - of new lsms
> much more painful and therefore less likely?

Dev, hopefully not.  Testing, well, perhaps.

> I realize there has been a dearth of new LSMs to date, but if
> for instance a new solaris 10 based capability module were written,
> well, people would want to be able to
> 
> 	rmmod capability
> 	modprobe cap_prm

The problem is it's not necessarily even safe to do rmmod at all.
And modprobe may require extra labelling, or extra checks for
unlabelled objects (perhaps not so much for your example).

thanks,
-chris

Re: [PATCH][RFC] security: Convert LSM into a static interface

[Serge E. Hallyn]

Quoting Chris Wright (chrisw@sous-sol.org):
> * Serge E. Hallyn (serge@hallyn.com) wrote:
> > Sigh, as much as I would *like* to stay out of this (I don't
> > use modules at all on any system where I can avoid it), won't
> > it make development - and especially testing - of new lsms
> > much more painful and therefore less likely?
> 
> Dev, hopefully not.  Testing, well, perhaps.
> 
> > I realize there has been a dearth of new LSMs to date, but if
> > for instance a new solaris 10 based capability module were written,
> > well, people would want to be able to
> > 
> > 	rmmod capability
> > 	modprobe cap_prm
> 
> The problem is it's not necessarily even safe to do rmmod at all.
> And modprobe may require extra labelling, or extra checks for
> unlabelled objects (perhaps not so much for your example).

Right, and given that it's trivial for the author of an LSM which
shouldn't be modular to make the LSM a boolean config rather than
tristate, it doesn't seem like a good reason to take away the
ability to have LSM modules.

-serge

Re: [PATCH][RFC] security: Convert LSM into a static interface

[Casey Schaufler]

--- "Serge E. Hallyn" <serge@hallyn.com> wrote:

> Quoting James Morris (jmorris@namei.org):
> > Convert LSM into a static interface, as the ability to unload a security 
> > module is not required by in-tree users and potentially complicates the 
> > overall security architecture.
> > 
> > Needlessly exported LSM symbols have been unexported, to help reduce API 
> > abuse.
> > 
> > Module parameters for the capability and root_plug modules have been
> > converted to kernel parameters.
> > 
> > The SECURITY_FRAMEWORK_VERSION macro has also been removed.
> 
> Sigh, as much as I would *like* to stay out of this (I don't
> use modules at all on any system where I can avoid it), won't
> it make development - and especially testing - of new lsms
> much more painful and therefore less likely?

While there's lots of pain involved in developing an LSM
modern development environments (e.g. virtual machines)
have reduced the value of loadable modules for debug purposes.

> I realize there has been a dearth of new LSMs to date,

but so much excitment over those proposed!

> but if
> for instance a new solaris 10 based capability module were written,
> well, people would want to be able to
> 
> 	rmmod capability
> 	modprobe cap_prm

I think the value is overrated. You would never want to do that
in a production environment, and in a debug environment you could
just as easily reboot and get some start-up testing out of the way.


Casey Schaufler
casey@schaufler-ca.com

Re: [PATCH][RFC] security: Convert LSM into a static interface

[James Morris]

On Mon, 25 Jun 2007, Casey Schaufler wrote:

> While there's lots of pain involved in developing an LSM
> modern development environments (e.g. virtual machines)
> have reduced the value of loadable modules for debug purposes.

lguest is pretty good for this.  You can boot a kernel in approximately 
the same time as loading a module.

-- 
James Morris
<jmorris@namei.org>

Re: [PATCH][RFC] security: Convert LSM into a static interface

[Serge E. Hallyn]

Quoting Casey Schaufler (casey@schaufler-ca.com):
> 
> --- "Serge E. Hallyn" <serge@hallyn.com> wrote:
> 
> > Quoting James Morris (jmorris@namei.org):
> > > Convert LSM into a static interface, as the ability to unload a security 
> > > module is not required by in-tree users and potentially complicates the 
> > > overall security architecture.
> > > 
> > > Needlessly exported LSM symbols have been unexported, to help reduce API 
> > > abuse.
> > > 
> > > Module parameters for the capability and root_plug modules have been
> > > converted to kernel parameters.
> > > 
> > > The SECURITY_FRAMEWORK_VERSION macro has also been removed.
> > 
> > Sigh, as much as I would *like* to stay out of this (I don't
> > use modules at all on any system where I can avoid it), won't
> > it make development - and especially testing - of new lsms
> > much more painful and therefore less likely?
> 
> While there's lots of pain involved in developing an LSM
> modern development environments (e.g. virtual machines)
> have reduced the value of loadable modules for debug purposes.
> 
> > I realize there has been a dearth of new LSMs to date,
> 
> but so much excitment over those proposed!
> 
> > but if
> > for instance a new solaris 10 based capability module were written,
> > well, people would want to be able to
> > 
> > 	rmmod capability
> > 	modprobe cap_prm
> 
> I think the value is overrated. You would never want to do that
> in a production environment, and in a debug environment you could
> just as easily reboot and get some start-up testing out of the way.

And in a development environment you can just as easily select
CONFIG_XYZ=y, no?

So we have two options, one which provides greater choice and
flexibility.

-serge

Re: [PATCH][RFC] security: Convert LSM into a static interface

[Casey Schaufler]

--- "Serge E. Hallyn" <serge@hallyn.com> wrote:


> > I think the value is overrated. You would never want to do that
> > in a production environment, and in a debug environment you could
> > just as easily reboot and get some start-up testing out of the way.
> 
> And in a development environment you can just as easily select
> CONFIG_XYZ=y, no?
> 
> So we have two options, one which provides greater choice and
> flexibility.

I have no problem with leaving things the way they are today,
and none with the proposed change. Either way works for me. It
would be a little bit better for me if it stayed as is, but that's
not a coding issue.


Casey Schaufler
casey@schaufler-ca.com

Re: implement-file-posix-capabilities.patch

[Andrew Morgan]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Serge E. Hallyn wrote:
> 
>> I don't particularly mind, but can you point out any case where
>> it is an advantage to have the one bit for f'E rather than just
>> drop f'E altogether?  Instead of having
> 
>> 	f'I=something
>> 	f'P=something
>> 	f'E=off
> 
>> we can always just remove the security.capability xattr.  Right?

No. Bear in mind that capabilities are all about trusting specific
applications with privilege as opposed to trusting the superuser to not
run dangerous applications.

There are three situations, we'll take them in turn:

  - no capabilities (fP=fI=fE=0): this is for applications that are not
intended to operate with privilege. Because of the way the capability
convolution rules work, such a program can't execute with privilege. Period.

  - with capabilities (fP and/or fI != 0), but fE=0 (off): this is for
applications that are intended to operate with privilege, but they were
designed to know about capabilities and they manipulate (raise and
lower) capabilities as needed to do the things they do.

  - with capabilities, but fE=1 (on): this is a class of applications
loosely called 'legacy'. They can use privilege to operate, but don't
strictly need to know about it. For example, /bin/chown . Such a program
will have fP=0,fI=CAP_CHOWN. Since the administrator sets
fP=0,fI=CAP_CHOWN,fE=1 on the /bin/chown file, any process with
CAP_CHOWN in its inheritable set (pI) can "exec /bin/chown" and have it
do the thing historically reserved for the superuser...

In some future world, the legacy fE bit may become unnecessary because
every application will be rewritten to be careful about exercising
privilege explicitly. In the meantime, the fE bit can be used to drop
the setuid-0 bits from things like ping and traceroute.

>> If there's a case where that does not suffice, then I have no objection
>> to doing it this way.

Does that explain it?

> 2) Allocate capability bit-31 for CAP_SETFCAP, and use it to gate
> whether the user can set this xattr on a file or not. CAP_SYS_ADMIN is
> way too overloaded and this functionality is special.
> 
>> The functionality is special, but someone with CAP_SYS_ADMIN can always
>> unload the capability module and create the security.capability xattr
>> using the dummy module.

This argument leads down a rat hole. (As appears to have happened with
the non-modularization LSM thread elsewhere...)

The simple fact is that CAP_SYS_ADMIN is equivalent to every other
capability in the system if it can be used to load any flavor of kernel
module. Arguing that you don't need a capability for something because
you can do it with CAP_SYS_ADMIN is very close to admitting that you
prefer the superuser (uid=0) model.

>> If we do add this cap, do we want to make it apply to all security.*
>> xattrs?

I recommend limiting it to just capabilities.

For now, you can leave the other security attributes with the
CAP_SYS_ADMIN ("misc") capability. In the original 'POSIX' drafts, there
is a separate notion of advisory and mandatory access control;
leveraging different capabilities to override.

> 3) The cap_from_disk() interface checking needs some work.... Most
> notably, size must be greater than sizeof(u32) or the very first line
> will do something nasty... I'd recommend you use code like this:
> 
> [...] cap_from_disk(...)
> {
>    if (size != sizeof(struct vfs_cap_data)) {
[...]
> mistake at least once... The future is uncertain, so don't trust it will
> look the way you want it to. ;-)
> 
>> Ok, so you're saying that when we do switch to 64-bit caps or some other
>> evolution, we switch to completely separate logic based on the
>> VFS_CAP_REVISION?

Yes.

>> That seems sane to me.

You might also want to verify that unallocated bits hold the value
'zero'. Its funny what people do when they realize they can silently
store bits in obscure places like this. That really messes up allocating
bits in the future. Add a check that is something like:

  if (version & ~(VFS_CAP_REVISION_MASK|VFS_CAP_FLAGS_EFFECTIVE)) {
      return -EINVAL;
  }

> 7) This one is subtle, and to my mind not well appreciated. In
> cap_bprm_apply_creds(), the wart of the global 'cap_bset' masking
> permitted bits can lead to problems like the one we saw a few years back
> with sendmail and capabilities. There is an assumption in setting
> permitted (they are called 'forced' in some documents) capabilities on a
> file that the file will execute with at least these. The inheritable
> ones are optional.
> 
>> Hmm, changing the behavior of the cap_bset is something that seems to
>> belong in 8), though I see what you're saying, it does affect the
>> behavior of vfs caps.

I'm not really changing the behavior of cap_bset. I'm specifying the
behavior of fP. ;-)

[Your comments on cap_bset and CAP_SETPCAP are exactly where my
ax^H^H^Hscalpel will fall after all this VFS support is stable. But
*that* is a subject for a different thread... aka item 8.]

>> So yeah, I think you're right - but the question is whose word do we
>> take here?  The admin who set the vfs caps on the binary, or the admin
>> who set cap_bset through sysctl?

The former.

Cheers

Andrew
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.2.6 (GNU/Linux)

iD8DBQFGge7WQheEq9QabfIRAo5oAJwLmR40gW/McHxGTFukm/PCRyLvegCgglpD
tf7NgFcQvFNnMVDhNfslBGY=
=2tDD
-----END PGP SIGNATURE-----

Re: implement-file-posix-capabilities.patch

[Serge E. Hallyn]

Quoting Andrew Morgan (morgan@kernel.org):
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
> 
> Serge E. Hallyn wrote:
> > 
> >> I don't particularly mind, but can you point out any case where
> >> it is an advantage to have the one bit for f'E rather than just
> >> drop f'E altogether?  Instead of having
> > 
> >> 	f'I=something
> >> 	f'P=something
> >> 	f'E=off
> > 
> >> we can always just remove the security.capability xattr.  Right?
> 
> No. Bear in mind that capabilities are all about trusting specific
> applications with privilege as opposed to trusting the superuser to not
> run dangerous applications.
> 
> There are three situations, we'll take them in turn:
> 
>   - no capabilities (fP=fI=fE=0): this is for applications that are not
> intended to operate with privilege. Because of the way the capability
> convolution rules work, such a program can't execute with privilege. Period.

(Except that in Linux, with SECURE_NOROOT=0, root will be able to.  With
SECURE_NOROOT=y it won't.)

>   - with capabilities (fP and/or fI != 0), but fE=0 (off): this is for
> applications that are intended to operate with privilege, but they were
> designed to know about capabilities and they manipulate (raise and
> lower) capabilities as needed to do the things they do.
> 
>   - with capabilities, but fE=1 (on): this is a class of applications
> loosely called 'legacy'. They can use privilege to operate, but don't
> strictly need to know about it. For example, /bin/chown . Such a program
> will have fP=0,fI=CAP_CHOWN. Since the administrator sets
> fP=0,fI=CAP_CHOWN,fE=1 on the /bin/chown file, any process with
> CAP_CHOWN in its inheritable set (pI) can "exec /bin/chown" and have it
> do the thing historically reserved for the superuser...
> 
> In some future world, the legacy fE bit may become unnecessary because
> every application will be rewritten to be careful about exercising
> privilege explicitly. In the meantime, the fE bit can be used to drop
> the setuid-0 bits from things like ping and traceroute.
> 
> >> If there's a case where that does not suffice, then I have no objection
> >> to doing it this way.
> 
> Does that explain it?

Yes, thanks, but then it still could come in handy to have fE be a full
bitset, so the application gets some eff caps automatically, while
others it has to manually set...

> > 2) Allocate capability bit-31 for CAP_SETFCAP, and use it to gate
> > whether the user can set this xattr on a file or not. CAP_SYS_ADMIN is
> > way too overloaded and this functionality is special.
> > 
> >> The functionality is special, but someone with CAP_SYS_ADMIN can always
> >> unload the capability module and create the security.capability xattr
> >> using the dummy module.
> 
> This argument leads down a rat hole. (As appears to have happened with
> the non-modularization LSM thread elsewhere...)
> 
> The simple fact is that CAP_SYS_ADMIN is equivalent to every other
> capability in the system if it can be used to load any flavor of kernel
> module. Arguing that you don't need a capability for something because
> you can do it with CAP_SYS_ADMIN is very close to admitting that you
> prefer the superuser (uid=0) model.

Well no, it could also just be acknowledging that at some point some
better cap breakup should be attempted, though of course some amount of
"these caps can subvert that cap" will always be there.

But I'm not actually objecting  :)  I expect this will be the first of
the patches I whip up.

> >> If we do add this cap, do we want to make it apply to all security.*
> >> xattrs?
> 
> I recommend limiting it to just capabilities.
> 
> For now, you can leave the other security attributes with the
> CAP_SYS_ADMIN ("misc") capability. In the original 'POSIX' drafts, there
> is a separate notion of advisory and mandatory access control;
> leveraging different capabilities to override.
> 
> > 3) The cap_from_disk() interface checking needs some work.... Most
> > notably, size must be greater than sizeof(u32) or the very first line
> > will do something nasty... I'd recommend you use code like this:
> > 
> > [...] cap_from_disk(...)
> > {
> >    if (size != sizeof(struct vfs_cap_data)) {
> [...]
> > mistake at least once... The future is uncertain, so don't trust it will
> > look the way you want it to. ;-)
> > 
> >> Ok, so you're saying that when we do switch to 64-bit caps or some other
> >> evolution, we switch to completely separate logic based on the
> >> VFS_CAP_REVISION?
> 
> Yes.
> 
> >> That seems sane to me.
> 
> You might also want to verify that unallocated bits hold the value
> 'zero'. Its funny what people do when they realize they can silently
> store bits in obscure places like this. That really messes up allocating
> bits in the future. Add a check that is something like:
> 
>   if (version & ~(VFS_CAP_REVISION_MASK|VFS_CAP_FLAGS_EFFECTIVE)) {
>       return -EINVAL;
>   }

I used to do that (except with a stupid loop), but let's say you update
your distro, but the updated kernel has a problem.  So you boot into an
older kernel.  But the distro update added a new capability.  Now
nothing runs with privilege.

Or are you saying that we would name the xattrs
'security.capability.vN', and with each capability we add bump N, so
that after we've added 5 capabilities, each binary carries with it 5
xattrs, security.capability.v1 through security.capability.v5?

> > 7) This one is subtle, and to my mind not well appreciated. In
> > cap_bprm_apply_creds(), the wart of the global 'cap_bset' masking
> > permitted bits can lead to problems like the one we saw a few years back
> > with sendmail and capabilities. There is an assumption in setting
> > permitted (they are called 'forced' in some documents) capabilities on a
> > file that the file will execute with at least these. The inheritable
> > ones are optional.
> > 
> >> Hmm, changing the behavior of the cap_bset is something that seems to
> >> belong in 8), though I see what you're saying, it does affect the
> >> behavior of vfs caps.
> 
> I'm not really changing the behavior of cap_bset. I'm specifying the
> behavior of fP. ;-)
> 
> [Your comments on cap_bset and CAP_SETPCAP are exactly where my
> ax^H^H^Hscalpel will fall after all this VFS support is stable. But
> *that* is a subject for a different thread... aka item 8.]
> 
> >> So yeah, I think you're right - but the question is whose word do we
> >> take here?  The admin who set the vfs caps on the binary, or the admin
> >> who set cap_bset through sysctl?
> 
> The former.

Well how about I whip up the patch (when time permits) and see if anyone
complains  :)

thanks,
-serge

Re: implement-file-posix-capabilities.patch

[Andrew Morgan]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Serge E. Hallyn wrote:
>> Does that explain it?
> 
> Yes, thanks, but then it still could come in handy to have fE be a full
> bitset, so the application gets some eff caps automatically, while
> others it has to manually set...

[We touched on this a number of emails back.]

If an application is capability aware, it can manipulate its own
capabilities and should have fE=0.

If an application is not capability aware, it needs to have *all* of its
capabilities enabled at exec() time. Otherwise, it won't work.

The only reason for having an fE bitmap is to allow a capability-aware
program (you really trust to do its privileged operations carefully) to
be lazy and get some of its capabilities raised for free. Perhaps you
can clarify why this is a desirable thing? :-)

Cheers

Andrew
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.2.6 (GNU/Linux)

iD8DBQFGg1LqQheEq9QabfIRAo3BAKCO8QrfcKBNqhfnn2BHp8O/qDkgXgCgleEl
xP7LZPU9Qn6AjqI3ZM3FZ+4=
=urmz
-----END PGP SIGNATURE-----

Re: implement-file-posix-capabilities.patch

[Serge E. Hallyn]

Quoting Andrew Morgan (morgan@kernel.org):
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
> 
> Serge E. Hallyn wrote:
> >> Does that explain it?
> > 
> > Yes, thanks, but then it still could come in handy to have fE be a full
> > bitset, so the application gets some eff caps automatically, while
> > others it has to manually set...
> 
> [We touched on this a number of emails back.]
> 
> If an application is capability aware, it can manipulate its own
> capabilities and should have fE=0.
> 
> If an application is not capability aware, it needs to have *all* of its
> capabilities enabled at exec() time. Otherwise, it won't work.
> 
> The only reason for having an fE bitmap is to allow a capability-aware
> program (you really trust to do its privileged operations carefully) to
> be lazy and get some of its capabilities raised for free. Perhaps you
> can clarify why this is a desirable thing? :-)

Sure - because it doesn't hurt anything, someone just *might* find it
useful one day, and mostly the three bitmaps just look a lot cleaner to
me than hiding a bit inside the version field.  There are a *few* people
using this, and so a complete switch in format for no actual net gain
seems wrong.  If we want to fake fE to the user as being one bit we can
do that through the setfcaps/getfcaps programs.

There also are prior examples of doing it this way (i.e. Olaf Dietsche's
implementation)

OTOH I don't deny implementing it fully as you describe seems to make
the intent of the code clearer to readers and maintainers.

I guess maybe I'll give it a go and see what turns out.

thanks,
-serge

Re: implement-file-posix-capabilities.patch

[Casey Schaufler]

--- Andrew Morgan <morgan@kernel.org> wrote:

> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
> 
> Serge E. Hallyn wrote:
> >> Does that explain it?
> > 
> > Yes, thanks, but then it still could come in handy to have fE be a full
> > bitset, so the application gets some eff caps automatically, while
> > others it has to manually set...
> 
> [We touched on this a number of emails back.]
> 
> If an application is capability aware, it can manipulate its own
> capabilities and should have fE=0.
> 
> If an application is not capability aware, it needs to have *all* of its
> capabilities enabled at exec() time. Otherwise, it won't work.

The intent of the fE vector in the POSIX draft is that those capabilities
are set on exec (lower vectors permitting). There are cases where it
does make sense to raise just some (e.g. ping). 

> The only reason for having an fE bitmap is to allow a capability-aware
> program (you really trust to do its privileged operations carefully) to
> be lazy and get some of its capabilities raised for free. Perhaps you
> can clarify why this is a desirable thing? :-)

No, it's to allow you to grant a subset of the available capabilities
to a program that is not aware of capabilities. You can give "date"
the capability to reset the clock without giving it the capability
to remove other people's files without changing the code or running
it setuid.


Casey Schaufler
casey@schaufler-ca.com

Re: implement-file-posix-capabilities.patch

[Serge E. Hallyn]

Quoting Casey Schaufler (casey@schaufler-ca.com):
> 
> --- Andrew Morgan <morgan@kernel.org> wrote:
> 
> > -----BEGIN PGP SIGNED MESSAGE-----
> > Hash: SHA1
> > 
> > Serge E. Hallyn wrote:
> > >> Does that explain it?
> > > 
> > > Yes, thanks, but then it still could come in handy to have fE be a full
> > > bitset, so the application gets some eff caps automatically, while
> > > others it has to manually set...
> > 
> > [We touched on this a number of emails back.]
> > 
> > If an application is capability aware, it can manipulate its own
> > capabilities and should have fE=0.
> > 
> > If an application is not capability aware, it needs to have *all* of its
> > capabilities enabled at exec() time. Otherwise, it won't work.
> 
> The intent of the fE vector in the POSIX draft is that those capabilities
> are set on exec (lower vectors permitting). There are cases where it
> does make sense to raise just some (e.g. ping). 
> 
> > The only reason for having an fE bitmap is to allow a capability-aware
> > program (you really trust to do its privileged operations carefully) to
> > be lazy and get some of its capabilities raised for free. Perhaps you
> > can clarify why this is a desirable thing? :-)
> 
> No, it's to allow you to grant a subset of the available capabilities
> to a program that is not aware of capabilities. You can give "date"
> the capability to reset the clock without giving it the capability
> to remove other people's files without changing the code or running
> it setuid.

Would there be a difference between that and setting either fI or fP
(depending on your intent) to those caps, and setting fE=1 in Andrew's
scheme?

thanks,
-serge

Re: implement-file-posix-capabilities.patch

[Casey Schaufler]

--- "Serge E. Hallyn" <serue@us.ibm.com> wrote:

> Quoting Casey Schaufler (casey@schaufler-ca.com):
> > 
> > --- Andrew Morgan <morgan@kernel.org> wrote:
> > 
> > > -----BEGIN PGP SIGNED MESSAGE-----
> > > Hash: SHA1
> > > 
> > > Serge E. Hallyn wrote:
> > > >> Does that explain it?
> > > > 
> > > > Yes, thanks, but then it still could come in handy to have fE be a full
> > > > bitset, so the application gets some eff caps automatically, while
> > > > others it has to manually set...
> > > 
> > > [We touched on this a number of emails back.]
> > > 
> > > If an application is capability aware, it can manipulate its own
> > > capabilities and should have fE=0.
> > > 
> > > If an application is not capability aware, it needs to have *all* of its
> > > capabilities enabled at exec() time. Otherwise, it won't work.
> > 
> > The intent of the fE vector in the POSIX draft is that those capabilities
> > are set on exec (lower vectors permitting). There are cases where it
> > does make sense to raise just some (e.g. ping). 
> > 
> > > The only reason for having an fE bitmap is to allow a capability-aware
> > > program (you really trust to do its privileged operations carefully) to
> > > be lazy and get some of its capabilities raised for free. Perhaps you
> > > can clarify why this is a desirable thing? :-)
> > 
> > No, it's to allow you to grant a subset of the available capabilities
> > to a program that is not aware of capabilities. You can give "date"
> > the capability to reset the clock without giving it the capability
> > to remove other people's files without changing the code or running
> > it setuid.
> 
> Would there be a difference between that and setting either fI or fP
> (depending on your intent) to those caps, and setting fE=1 in Andrew's
> scheme?

Arg, you're making me think. The POSIX group went through this,
let me see if I can reconstruct the logic.

The main issue is one if there being a possible case where you
have a capability ignorant program that you want to exec with
a different fP and fE. On first glance it seems that since the
program is capability ignorant it can't matter. But what if your
capability ignorant program exec's a capability aware program
to perform a helper function? You may well want the first program
to have a capability that it does not use in fP (but not fE)
to pass along to the helper program. True, you could probably
come up with a way to set the capabilities on the helper program
to account for this use, but there may be design and security
constraints that make doing so complicated. 



Casey Schaufler
casey@schaufler-ca.com

Re: implement-file-posix-capabilities.patch

[Andrew Morgan]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Casey Schaufler wrote:
>> Would there be a difference between that and setting either fI or fP
>> (depending on your intent) to those caps, and setting fE=1 in Andrew's
>> scheme?
> 
> Arg, you're making me think. The POSIX group went through this,
> let me see if I can reconstruct the logic.
> 
> The main issue is one if there being a possible case where you
> have a capability ignorant program that you want to exec with
> a different fP and fE. On first glance it seems that since the
> program is capability ignorant it can't matter. But what if your
> capability ignorant program exec's a capability aware program
> to perform a helper function? You may well want the first program
> to have a capability that it does not use in fP (but not fE)
> to pass along to the helper program. True, you could probably

I'm not sure I've quite flogged this horse to death yet.. :-)

In my other reply, I quoted the rules. Here they are again:

    pI' = pI
    pP' = (X & fP) | (pI & fI)
    pE' = pP' & fE

If program A exec()utes helper program B, then the only capabilities
(p*') that B can get from A are a subset of A's pI set.

If A doesn't know about capabilities, then nothing about the fE value
associated with the A program file can alter A's pI set and thus affect
B. That is, nothing about the fE or fP value used to exec()ute A gets
propagated through a subsequent exec() to B.

So far as I can see, to achieve the helper program support you are
describing, the value of pI that program A (and thus program B) inherits
will have to contain the relevant capabilities, and B will have to have
a sufficient fI value to pick them up...

Incidentally, this is also where my request that we require (pP' >= fP)
be true comes in. If a helper program (which may also be a legacy
program) is used in a way that it is configured (via fP) to have powers
that are denied to it (via X=cap_bset etc.,) then it should simply not
be permitted to run (-EPERM). It should not have the opportunity to
silently confuse itself (as was the case with sendmail when we tried to
emulate setuid-0 behavior with capabilities a few years back).

> come up with a way to set the capabilities on the helper program
> to account for this use, but there may be design and security
> constraints that make doing so complicated. 

I've not seen anything yet to make be believe there is a case for a
non-single bit fE value... Its a little ironic that I read all of the
rationale I've been espousing in POSIX drafts - so far as I'm aware the
only detail I'm mixing in there is the (pP' >= fP), -EPERM, thing.

If you or anyone can cite some counter examples, please do!

Cheers

Andrew
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.2.6 (GNU/Linux)

iD8DBQFGhJjxQheEq9QabfIRAmyLAKCUxirmAuS4VM0U+9HloeOF6cKt2gCgi/fh
ElhM1CISM4a+e0umBjK9GV0=
=Vrqj
-----END PGP SIGNATURE-----

Re: implement-file-posix-capabilities.patch

[Serge E. Hallyn]

Quoting Andrew Morgan (morgan@kernel.org):
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
> 
> Casey Schaufler wrote:
> >> Would there be a difference between that and setting either fI or fP
> >> (depending on your intent) to those caps, and setting fE=1 in Andrew's
> >> scheme?
> > 
> > Arg, you're making me think. The POSIX group went through this,
> > let me see if I can reconstruct the logic.
> > 
> > The main issue is one if there being a possible case where you
> > have a capability ignorant program that you want to exec with
> > a different fP and fE. On first glance it seems that since the
> > program is capability ignorant it can't matter. But what if your
> > capability ignorant program exec's a capability aware program
> > to perform a helper function? You may well want the first program
> > to have a capability that it does not use in fP (but not fE)
> > to pass along to the helper program. True, you could probably
> 
> I'm not sure I've quite flogged this horse to death yet.. :-)
> 
> In my other reply, I quoted the rules. Here they are again:
> 
>     pI' = pI
>     pP' = (X & fP) | (pI & fI)
>     pE' = pP' & fE
>
> If program A exec()utes helper program B, then the only capabilities
> (p*') that B can get from A are a subset of A's pI set.
> 
> If A doesn't know about capabilities, then nothing about the fE value
> associated with the A program file can alter A's pI set and thus affect
> B. That is, nothing about the fE or fP value used to exec()ute A gets
> propagated through a subsequent exec() to B.
> 
> So far as I can see, to achieve the helper program support you are
> describing, the value of pI that program A (and thus program B) inherits
> will have to contain the relevant capabilities, and B will have to have
> a sufficient fI value to pick them up...
> 
> Incidentally, this is also where my request that we require (pP' >= fP)
> be true comes in. If a helper program (which may also be a legacy

That was going to be my next thing to look at, btw.

> program) is used in a way that it is configured (via fP) to have powers
> that are denied to it (via X=cap_bset etc.,) then it should simply not
> be permitted to run (-EPERM). It should not have the opportunity to
> silently confuse itself (as was the case with sendmail when we tried to
> emulate setuid-0 behavior with capabilities a few years back).
> 
> > come up with a way to set the capabilities on the helper program
> > to account for this use, but there may be design and security
> > constraints that make doing so complicated. 
> 
> I've not seen anything yet to make be believe there is a case for a
> non-single bit fE value... Its a little ironic that I read all of the
> rationale I've been espousing in POSIX drafts - so far as I'm aware the
> only detail I'm mixing in there is the (pP' >= fP), -EPERM, thing.
> 
> If you or anyone can cite some counter examples, please do!

Not really.

But, your argument is:

fE can only be used to create a different initial pE' than pP'.

If the binary doesn't know about capabilities, then you would never want
to do that since it won't change it's pE', and therefore it's pP' is
meaningless once pE' is set.

If the binary does know about capabilities, then it can change it's pE'
within the bounds of it's pP' whenever it wants, so we may want to set
pE' to 0 at start, but having pE' be just a subset of pP' has no
advantage since the binary can set pE' to the subset when it wants.

Now, *my* argument :) is that:

While there is no particular advantage (that I can think of) to having
the bitmap pE', there is also no disadvantage, and it produces kernel
code closer to the actual equations usually seen.

In the course of this discussion, the two advantages I've seen which
I'll concede to you are:

	1. the resulting equations may be easier to read/understand,
	   just bc it's one less bitmask operation whose purpose to try
	   to understand.  But I'm not certain.
	2. Improper setting of fE *could* in theory cause another
	   hard to diagnose sendmail-type vulnerability, by having fP
	   set to the right value, but fE to the wrong value.  Though
	   that is stretching things a bit.

The change in xattr format offsets those, which is why I like the idea of
just having the setfcaps program understand 'on=e' and 'off=e' (where on
is of course just an alias for 'all'), but keeping the kernel code the
same.

-serge

Re: implement-file-posix-capabilities.patch

[Casey Schaufler]

--- Andrew Morgan <morgan@kernel.org> wrote:

> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
> 
> Casey Schaufler wrote:
> >> Would there be a difference between that and setting either fI or fP
> >> (depending on your intent) to those caps, and setting fE=1 in Andrew's
> >> scheme?
> > 
> > Arg, you're making me think. The POSIX group went through this,
> > let me see if I can reconstruct the logic.
> > 
> > The main issue is one if there being a possible case where you
> > have a capability ignorant program that you want to exec with
> > a different fP and fE. On first glance it seems that since the
> > program is capability ignorant it can't matter. But what if your
> > capability ignorant program exec's a capability aware program
> > to perform a helper function? You may well want the first program
> > to have a capability that it does not use in fP (but not fE)
> > to pass along to the helper program. True, you could probably
> 
> I'm not sure I've quite flogged this horse to death yet.. :-)
> 
> In my other reply, I quoted the rules. Here they are again:
> 
>     pI' = pI
>     pP' = (X & fP) | (pI & fI)
>     pE' = pP' & fE
> 
> If program A exec()utes helper program B, then the only capabilities
> (p*') that B can get from A are a subset of A's pI set.
> 
> If A doesn't know about capabilities, then nothing about the fE value
> associated with the A program file can alter A's pI set and thus affect
> B. That is, nothing about the fE or fP value used to exec()ute A gets
> propagated through a subsequent exec() to B.
> 
> So far as I can see, to achieve the helper program support you are
> describing, the value of pI that program A (and thus program B) inherits
> will have to contain the relevant capabilities, and B will have to have
> a sufficient fI value to pick them up...
> 
> Incidentally, this is also where my request that we require (pP' >= fP)
> be true comes in. If a helper program (which may also be a legacy
> program) is used in a way that it is configured (via fP) to have powers
> that are denied to it (via X=cap_bset etc.,) then it should simply not
> be permitted to run (-EPERM). It should not have the opportunity to
> silently confuse itself (as was the case with sendmail when we tried to
> emulate setuid-0 behavior with capabilities a few years back).
> 
> > come up with a way to set the capabilities on the helper program
> > to account for this use, but there may be design and security
> > constraints that make doing so complicated. 
> 
> I've not seen anything yet to make be believe there is a case for a
> non-single bit fE value... Its a little ironic that I read all of the
> rationale I've been espousing in POSIX drafts - so far as I'm aware the
> only detail I'm mixing in there is the (pP' >= fP), -EPERM, thing.
> 
> If you or anyone can cite some counter examples, please do!

Nope, I'm fresh out. If the reality is that you get no added value
with a vector over a scalar I'm good with either scheme. Looks like
you've done the dilegance. I see no flaws in your logic. I suppose
I could argue for the vector in terms of compatability with Irix,
but I'll leave that to those who might care.

Thank you for both the work and the clear explainations.


Casey Schaufler
casey@schaufler-ca.com

Re: implement-file-posix-capabilities.patch

[Andrew Morgan]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Casey Schaufler wrote:
>> The only reason for having an fE bitmap is to allow a capability-aware
>> program (you really trust to do its privileged operations carefully) to
>> be lazy and get some of its capabilities raised for free. Perhaps you
>> can clarify why this is a desirable thing? :-)
> 
> No, it's to allow you to grant a subset of the available capabilities
> to a program that is not aware of capabilities. You can give "date"
> the capability to reset the clock without giving it the capability
> to remove other people's files without changing the code or running
> it setuid.

This is precisely what fE = 0 or ~0 provides.

Recall, an exec()'d program gets its p*' capabilities from a convolution
of its exec()er's inheritable set (pI) *and* the file's capabilities
(fI,fP,fE):

    pI' = pI
    pP' = (X & fP) | (pI & fI)
    pE' = pP' & fE

[Linux essentially has cap_bset for X.]

The fine-grain ability for ping to do its thing without becoming
powerful enough to load a kernel module, for example, is facilitated by
the "pP' &" part of the derivation of pE' (and not simply the unfiltered
value of fE!).

As I said before, either the program knows how to raise and lower bits
in its pE set, or it doesn't. In the former case, fE=0. In the latter
case, fE=~0 will ensure that it gets all of the capabilities it is
permitted to exercise at time of execution.

Could you cite some examples of where this position is unreasonable?

Thanks

Andrew

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.7 (Darwin)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org

iD8DBQFGg9i1+bHCR3gb8jsRAvViAJ9T5x1fHrLGF4niRq7VhRqg4sej3wCgxkom
oAFQEQwLkd/D6J5gi7Fb3Ww=
=+ZLb
-----END PGP SIGNATURE-----

Re: implement-file-posix-capabilities.patch

[Serge E. Hallyn]

All,

Regarding future/backward compatibility of file capabilities:

Quoting Andrew Morgan (morgan@kernel.org):
> -----BEGIN PGP SIGNED MESSAGE-----

...

> #define VFS_CAP_REVISION_MASK     0xFF000000
> #define VFS_CAP_REVISION          0x01000000
> 
> #define VFS_CAP_FLAGS_MASK        ~VFS_CAP_REVISION_MASK
> #define VFS_CAP_FLAGS_EFFECTIVE     0x000001
> 
> struct vfs_cap_data {
> 	__u32  magic_etc;
> 	struct {
>         	__u32 permitted;     /* Little endian */
> 	        __u32 inheritable;   /* Little endian */
>         } data[1];
> };

...

> 3) The cap_from_disk() interface checking needs some work.... Most
> notably, size must be greater than sizeof(u32) or the very first line
> will do something nasty... I'd recommend you use code like this:
> 
> [...] cap_from_disk(...)
> {
>    if (size != sizeof(struct vfs_cap_data)) {
> 	printk(KERN_WARNING "%s: invalid cap size %d for file %s\n",
> 	     __FUNCTION__, size, bprm->filename);
> 	return -EINVAL;
>    }
> 
>    switch ((version & VFS_CAP_REVISION_MASK)) {
>    case VFS_CAP_REVISION:
>         bprm->cap_effective = (version & VFS_CAP_FLAGS_EFFECTIVE)
> 		? CAP_FULL_SET : CAP_EMPTY_SET;
> 	bprm->cap_permitted =
> 		to_cap_t( le32_to_cpu(dcap->data[0].permitted) );
> 	bprm->cap_inheritable =
> 		to_cap_t( le32_to_cpu(dcap->data[0].inheritable) );
>         return 0;
>    default:
> 	return -EINVAL;
>    }
> }
> 
> Basically, I don't believe in designing a secure interface to be forward
> compatible - things never work out that way and the legacy you are
> implicitly committing to will haunt you in the future... FWIW I've known
> a few x86 MSR designers over the years and each one has made this
> mistake at least once... The future is uncertain, so don't trust it will
> look the way you want it to. ;-)

There are a few obvious approaches we can take:

1. Exactly Andrew describes.  Once userspace switches to a new cap
format, an older kernel simply won't support them

2. As Andrew describes, but also encode the version number into the
capability name, i.e. security.capability.v3.  Now userspace can
optionally tack on more than one capability version to be backward
compatible.

3. Somewhat different than Andrew describes.  We mandate that any
capability version N+1 consist of

struct vfs_cap_data {
	__u32 magic;
	capability_version_1;
	capability_version_2;
	...
	capability_version_N;
	capability_version_N+1;
};

Or, for brevity,

struct vfs_cap_data {
	__u32 first_magic;
	__u32 last_magic;
	capability_version_first;
	...
	capability_version_last;
};

4. Stick to the current plan, where switching to 64-bit caps will be
done as

struct vfs_cap_data_disk {
	__le32 version;
	__le32 data[]; /* eff[0], perm[0], inh[0], eff[1], ... */
};

What would people prefer?

thanks,
-serge

Re: implement-file-posix-capabilities.patch

[Andrew Morgan]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Serge E. Hallyn wrote:
> 1. Exactly Andrew describes.  Once userspace switches to a new cap
> format, an older kernel simply won't support them

Mmm. Let me see. I think I prefer this one! :-)

> 2. As Andrew describes, but also encode the version number into the
> capability name, i.e. security.capability.v3.  Now userspace can
> optionally tack on more than one capability version to be backward
> compatible.

If you have a significant legacy of use of earlier versions, I guess
this makes sense. However, given the experimental nature of this support
(it will be a while before the user space support for this is
secure/robust), I'm not all that concerned about legacy support.

> 3. Somewhat different than Andrew describes.  We mandate that any
> capability version N+1 consist of
> 
> struct vfs_cap_data {
> 	__u32 magic;
> 	capability_version_1;
> 	capability_version_2;
> 	...
> 	capability_version_N;
> 	capability_version_N+1;
> };

Ugh. I don't like this. It presumes that the kernel will get more and
more complicated over time. Please don't do this one.

> Or, for brevity,
> 
> struct vfs_cap_data {
> 	__u32 first_magic;
> 	__u32 last_magic;
> 	capability_version_first;
> 	...
> 	capability_version_last;
> };
> 
> 4. Stick to the current plan, where switching to 64-bit caps will be
> done as
> 
> struct vfs_cap_data_disk {
> 	__le32 version;
> 	__le32 data[]; /* eff[0], perm[0], inh[0], eff[1], ... */
> };

While asserting that it is more flexible etc., no one has yet actually
given an example of where fE being richer than a simple binary helps
anything. Until I see an example, I'm going to hold the position that
this is needless "complexity".

Cheers

Andrew

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.7 (Darwin)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org

iD8DBQFGjBFXmwytjiwfWMwRAofJAKCXX2GkN39o45fCQmxpNpZIEVH8EgCeLaDy
AoWZNj/1MqT7oayabxUhIn8=
=OSBu
-----END PGP SIGNATURE-----

Re: implement-file-posix-capabilities.patch

[Casey Schaufler]

--- Andrew Morgan <morgan@kernel.org> wrote:

> no one has yet actually
> given an example of where fE being richer than a simple binary helps
> anything. Until I see an example, I'm going to hold the position that
> this is needless "complexity".

The only counter to this argument is that you now have a different
structure on files than on processes. Not a major issue, but one 
structure to describe capability sets is less complex than two.
That way you can have one function to print a capset, regardless
of its coming off a file or a process.

Just a thought.


Casey Schaufler
casey@schaufler-ca.com