Security: Introduce security= boot parameter

PATCH In review Ahmed S. Darwish 2 participants 6 messages Mar 6, 2008 – Mar 7, 2008

Subsystems (4)

Documentation SELinux security module Security (LSM) Smack security module

lore

Share & tools

lore

Ahmed S. Darwish

author

2008-03-06 12:22 UTC #0 lore raw

PATCH 1/1 Security: Introduce security= boot parameter

Hi!,

[
Handle Andrew's concerns:
- Use __init and __initdata in appropriate places.
- Do not rely upon dummy_ops layout, use C99 initializations.
- Use DEFINE_SPINLOCK instead of dynamic initialization.

Create a new thread cause the original became hard to follow.
]

-->

Add the security= boot parameter. This is done to avoid LSM registration clashes in case of more than one bult-in module.

User can choose a security module to enable at boot. If no security= boot parameter is specified, only the first LSM asking for registration will be loaded. An invalid security module name will be treated as if no module has been chosen.

LSM modules must check now if they are allowed to register by calling security_module_enable(ops) first. Modify SELinux and SMACK to do so.

Do not let SMACK register smackfs if it was not chosen on boot. Smackfs assumes that smack hooks are registered and the initial task security setup (swapper->security) is done.

Signed-off-by Ahmed S. Darwish <[email protected]>

---

 Documentation/kernel-parameters.txt |    6 ++++
 include/linux/security.h            |   12 +++++++++
 security/dummy.c                    |    4 ++-
 security/security.c                 |   45 +++++++++++++++++++++++++++++++++++-
 security/selinux/hooks.c            |    7 +++++
 security/smack/smack.h              |    2 +
 security/smack/smack_lsm.c          |    7 ++++-
 security/smack/smackfs.c            |   11 ++++++++

 8 files changed, 90 insertions(+), 4 deletions(-)

Documentation/kernel-parameters.txt +6 -0

index 9a5b665..64efbdc 100644
@@ -374,6 +374,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.
 
+	security=	[SECURITY] Choose a security module to enable at boot. 
+			If this boot parameter is not specified, only the first 
+			security module asking for security registration will be
+			loaded. An invalid security module name will be treated
+			as if no module has been chosen.
+
 	capability.disable=
 			[SECURITY] Disable capabilities.  This would normally
 			be used only if an alternative security model is to be

include/linux/security.h +12 -0

index fe52cde..801c9ad 100644
@@ -42,6 +42,9 @@
 
 extern unsigned securebits;
 
+/* Maximum number of letters for an LSM name string */
+#define SECURITY_NAME_MAX	10
+
 struct ctl_table;
 
 /*

@@ -117,6 +120,12 @@ struct request_sock;
 /**
  * struct security_operations - main security structure
  *
+ * Security module identifier.
+ *
+ * @name:
+ *	A string that acts as a unique identifeir for the LSM with max number
+ *	of characters = SECURITY_NAME_MAX.
+ *
  * Security hooks for program execution operations.
  *
  * @bprm_alloc_security:

@@ -1207,6 +1216,8 @@ struct request_sock;
  * This is the main security structure.
  */
 struct security_operations {
+	char name[SECURITY_NAME_MAX + 1];
+
 	int (*ptrace) (struct task_struct * parent, struct task_struct * child);
 	int (*capget) (struct task_struct * target,
 		       kernel_cap_t * effective,

@@ -1466,6 +1477,7 @@ struct security_operations {
 
 /* prototypes */
 extern int security_init	(void);
+extern int security_module_enable(struct security_operations *ops);
 extern int register_security	(struct security_operations *ops);
 extern int mod_reg_security	(const char *name, struct security_operations *ops);
 extern struct dentry *securityfs_create_file(const char *name, mode_t mode,

security/dummy.c +3 -1

index 649326b..b668fda 100644
@@ -979,7 +979,9 @@ static inline int dummy_key_permission(key_ref_t key_ref,
 }
 #endif /* CONFIG_KEYS */
 
-struct security_operations dummy_security_ops;
+struct security_operations dummy_security_ops = { 
+	.name = "dummy", 
+};
 
 #define set_to_dummy_if_null(ops, function)				\
 	do {								\

security/security.c +44 -1

63 lines not on this page · raw message

security/selinux/hooks.c +7 -0

index 75c2e99..49709a4 100644
@@ -5219,6 +5219,8 @@ static int selinux_key_permission(key_ref_t key_ref,
 #endif
 
 static struct security_operations selinux_ops = {
+	.name =				"selinux",
+
 	.ptrace =			selinux_ptrace,
 	.capget =			selinux_capget,
 	.capset_check =			selinux_capset_check,

@@ -5405,6 +5407,11 @@ static __init int selinux_init(void)
 {
 	struct task_security_struct *tsec;
 
+	if (!security_module_enable(&selinux_ops)) {
+		selinux_enabled = 0;
+		return 0;
+	}
+
 	if (!selinux_enabled) {
 		printk(KERN_INFO "SELinux:  Disabled at boot.\n");
 		return 0;

security/smack/smack.h +2 -0

index a21a0e9..c444f48 100644
@@ -15,6 +15,7 @@
 
 #include <linux/capability.h>
 #include <linux/spinlock.h>
+#include <linux/security.h>
 #include <net/netlabel.h>
 
 /*

@@ -195,6 +196,7 @@ extern struct smack_known smack_known_star;
 extern struct smack_known smack_known_unset;
 
 extern struct smk_list_entry *smack_list;
+extern struct security_operations smack_ops;
 
 /*
  * Stricly for CIPSO level manipulation.

security/smack/smack_lsm.c +6 -1

index 770eb06..afa7967 100644
@@ -2433,7 +2433,9 @@ static void smack_release_secctx(char *secdata, u32 seclen)
 {
 }
 
-static struct security_operations smack_ops = {
+struct security_operations smack_ops = {
+	.name =				"smack",
+
 	.ptrace = 			smack_ptrace,
 	.capget = 			cap_capget,
 	.capset_check = 		cap_capset_check,

@@ -2566,6 +2568,9 @@ static struct security_operations smack_ops = {
  */
 static __init int smack_init(void)
 {
+	if (!security_module_enable(&smack_ops))
+		return 0;
+
 	printk(KERN_INFO "Smack:  Initializing.\n");
 
 	/*

security/smack/smackfs.c +10 -1

index 358c92c..effd3de 100644
@@ -986,12 +986,21 @@ static struct vfsmount *smackfs_mount;
  *
  * register the smackfs
  *
- * Returns 0 unless the registration fails.
+ * Do not register smackfs if Smack wasn't enabled
+ * on boot. We can not put this method normally under the
+ * smack_init() code path since the security subsystem get
+ * initialized before the vfs caches.
+ *
+ * Returns true if we were not chosen on boot or if
+ * we were chosen and filesystem registration succeeded.
  */
 static int __init init_smk_fs(void)
 {
 	int err;
 
+	if (!security_module_enable(&smack_ops))
+		return 0;
+
 	err = register_filesystem(&smk_fs_type);
 	if (!err) {
 		smackfs_mount = kern_mount(&smk_fs_type);

Signature · 5 lines

"Better to light a candle, than curse the darkness"

Ahmed S. Darwish
Homepage: http://darwish.07.googlepages.com
Blog: http://darwish-07.blogspot.com

James Morris

maintainer

2008-03-06 13:33 UTC #1 lore raw

Quoted 4 lines · Ahmed S. Darwish

Handle Andrew's concerns:
- Use __init and __initdata in appropriate places.
- Do not rely upon dummy_ops layout, use C99 initializations.
- Use DEFINE_SPINLOCK instead of dynamic initialization.

The spinlock is not needed now, if security_module_enable() can only be called during boot via an initcall.

- James

Signature · 2 lines

James Morris
<[email protected]>

Ahmed S. Darwish

author

2008-03-06 14:33 UTC #2 lore raw

Hi James,

Quoted 10 lines · James Morris

Quoted 4 lines · Ahmed S. Darwish
Handle Andrew's concerns:
- Use __init and __initdata in appropriate places.
- Do not rely upon dummy_ops layout, use C99 initializations.
- Use DEFINE_SPINLOCK instead of dynamic initialization.
The spinlock is not needed now, if security_module_enable() can only be called during boot via an initcall.

Will do.

Would you mind answering my confusions below so I can do the change with good understanding ?

I see preempt_disable() before calling security and vfs_caches init, but what will prevent two processors/cores from executing security_module_enable() concurrently (thus possibly corrupting chosen_lsm) ?
security_module_enable() is also now used in __init init_smk_fs().

Or the init path got executed serially ?

Thank you,

Signature · 3 lines

Ahmed S. Darwish
Homepage: http://darwish.07.googlepages.com
Blog: http://darwish-07.blogspot.com

James Morris

maintainer

2008-03-06 15:04 UTC #3 lore raw

Quoted 10 lines · Ahmed S. Darwish

Would you mind answering my confusions below so I can do the change with good understanding ?
I see preempt_disable() before calling security and vfs_caches init, but what will prevent two processors/cores from executing security_module_enable() concurrently (thus possibly corrupting chosen_lsm) ?
security_module_enable() is also now used in __init init_smk_fs().
Or the init path got executed serially ?

The init phase only runs on one cpu, see the lock_kernel() in kernel_init(), from which context the initcalls are eventually run in sequence.

- James

Signature · 2 lines

James Morris
<[email protected]>

Ahmed S. Darwish

author

2008-03-06 16:12 UTC #4 lore raw

PATCH 1/1 Security: Introduce security= boot parameter

Hi,

[
Thanks James for all those helpful comments since patch-v1!.

- Remove spinlocks since we've dictated to use security_module_enable() under kernel init path only (__init).
]

-->

Add the security= boot parameter. This is done to avoid LSM registration clashes in case of more than one bult-in module.

LSM modules must check now if they are allowed to register by calling security_module_enable(ops) first. Modify SELinux and SMACK to do so.

Do not let SMACK register smackfs if it was not chosen on boot. Smackfs assumes that smack hooks are registered and the initial task security setup (swapper->security) is done.

Signed-off-by Ahmed S. Darwish <[email protected]>

---

 Documentation/kernel-parameters.txt |    6 +++++
 include/linux/security.h            |   12 +++++++++++
 security/dummy.c                    |    4 ++-
 security/security.c                 |   38 +++++++++++++++++++++++++++++++++++-
 security/selinux/hooks.c            |    7 ++++++
 security/smack/smack.h              |    2 +
 security/smack/smack_lsm.c          |    7 +++++-
 security/smack/smackfs.c            |   11 +++++++++-

 8 files changed, 83 insertions(+), 4 deletions(-)

Documentation/kernel-parameters.txt +6 -0

index 9a5b665..64efbdc 100644
@@ -374,6 +374,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.
 
+	security=	[SECURITY] Choose a security module to enable at boot. 
+			If this boot parameter is not specified, only the first 
+			security module asking for security registration will be
+			loaded. An invalid security module name will be treated
+			as if no module has been chosen.
+
 	capability.disable=
 			[SECURITY] Disable capabilities.  This would normally
 			be used only if an alternative security model is to be

include/linux/security.h +12 -0

index fe52cde..801c9ad 100644
@@ -42,6 +42,9 @@
 
 extern unsigned securebits;
 
+/* Maximum number of letters for an LSM name string */
+#define SECURITY_NAME_MAX	10
+
 struct ctl_table;
 
 /*

@@ -117,6 +120,12 @@ struct request_sock;
 /**
  * struct security_operations - main security structure
  *
+ * Security module identifier.
+ *
+ * @name:
+ *	A string that acts as a unique identifeir for the LSM with max number
+ *	of characters = SECURITY_NAME_MAX.
+ *
  * Security hooks for program execution operations.
  *
  * @bprm_alloc_security:

@@ -1207,6 +1216,8 @@ struct request_sock;
  * This is the main security structure.
  */
 struct security_operations {
+	char name[SECURITY_NAME_MAX + 1];
+
 	int (*ptrace) (struct task_struct * parent, struct task_struct * child);
 	int (*capget) (struct task_struct * target,
 		       kernel_cap_t * effective,

@@ -1466,6 +1477,7 @@ struct security_operations {
 
 /* prototypes */
 extern int security_init	(void);
+extern int security_module_enable(struct security_operations *ops);
 extern int register_security	(struct security_operations *ops);
 extern int mod_reg_security	(const char *name, struct security_operations *ops);
 extern struct dentry *securityfs_create_file(const char *name, mode_t mode,

security/dummy.c +3 -1

index 649326b..b668fda 100644
@@ -979,7 +979,9 @@ static inline int dummy_key_permission(key_ref_t key_ref,
 }
 #endif /* CONFIG_KEYS */
 
-struct security_operations dummy_security_ops;
+struct security_operations dummy_security_ops = { 
+	.name = "dummy", 
+};
 
 #define set_to_dummy_if_null(ops, function)				\
 	do {								\

security/security.c +37 -1

index d15e56c..8d67e73 100644
@@ -17,6 +17,8 @@
 #include <linux/kernel.h>
 #include <linux/security.h>
 
+/* Boot-time LSM user choice */
+static __initdata char chosen_lsm[SECURITY_NAME_MAX + 1];
 
 /* things that live in dummy.c */
 extern struct security_operations dummy_security_ops;

@@ -67,13 +69,47 @@ int __init security_init(void)
 	return 0;
 }
 
+/* Save user chosen LSM */
+static int __init choose_lsm(char *str)
+{
+	strncpy(chosen_lsm, str, SECURITY_NAME_MAX);
+	return 1;
+}
+__setup("security=", choose_lsm);
+
+/**
+ * security_module_enable - Load given security module on boot ?
+ * @ops: a pointer to the struct security_operations that is to be checked.
+ *
+ * Each LSM must pass this method before registering its own operations
+ * to avoid security registration races. This method may also be used
+ * to check if your LSM is currently loaded.
+ *
+ * Return true if:
+ *	-The passed LSM is the one chosen by user at boot time,
+ *	-or user didsn't specify a specific LSM and we're the first to ask
+ *	 for registeration permissoin,
+ *	-or the passed LSM is currently loaded.
+ * Otherwise, return false.
+ */
+int __init security_module_enable(struct security_operations *ops)
+{
+	if (!*chosen_lsm)
+		strncpy(chosen_lsm, ops->name, SECURITY_NAME_MAX);
+	else if (strncmp(ops->name, chosen_lsm, SECURITY_NAME_MAX))
+		return 0;
+
+	return 1;
+}
+
 /**
  * register_security - registers a security framework with the kernel
  * @ops: a pointer to the struct security_options that is to be registered
  *
  * This function is to allow a security module to register itself with the
  * kernel security subsystem.  Some rudimentary checking is done on the @ops
- * value passed to this function.
+ * value passed to this function. You'll need to check first if your LSM
+ * is allowed to register its @ops by calling security_module_enable(@ops).
  *
  * If there is already a security module registered with the kernel,
  * an error will be returned.  Otherwise 0 is returned on success.

security/selinux/hooks.c +7 -0

index 75c2e99..49709a4 100644
@@ -5219,6 +5219,8 @@ static int selinux_key_permission(key_ref_t key_ref,
 #endif
 
 static struct security_operations selinux_ops = {
+	.name =				"selinux",
+
 	.ptrace =			selinux_ptrace,
 	.capget =			selinux_capget,
 	.capset_check =			selinux_capset_check,

@@ -5405,6 +5407,11 @@ static __init int selinux_init(void)
 {
 	struct task_security_struct *tsec;
 
+	if (!security_module_enable(&selinux_ops)) {
+		selinux_enabled = 0;
+		return 0;
+	}
+
 	if (!selinux_enabled) {
 		printk(KERN_INFO "SELinux:  Disabled at boot.\n");
 		return 0;

security/smack/smack.h +2 -0

index a21a0e9..c444f48 100644
@@ -15,6 +15,7 @@
 
 #include <linux/capability.h>
 #include <linux/spinlock.h>
+#include <linux/security.h>
 #include <net/netlabel.h>
 
 /*

@@ -195,6 +196,7 @@ extern struct smack_known smack_known_star;
 extern struct smack_known smack_known_unset;
 
 extern struct smk_list_entry *smack_list;
+extern struct security_operations smack_ops;
 
 /*
  * Stricly for CIPSO level manipulation.

security/smack/smack_lsm.c +6 -1

index 770eb06..afa7967 100644
@@ -2433,7 +2433,9 @@ static void smack_release_secctx(char *secdata, u32 seclen)
 {
 }
 
-static struct security_operations smack_ops = {
+struct security_operations smack_ops = {
+	.name =				"smack",
+
 	.ptrace = 			smack_ptrace,
 	.capget = 			cap_capget,
 	.capset_check = 		cap_capset_check,

@@ -2566,6 +2568,9 @@ static struct security_operations smack_ops = {
  */
 static __init int smack_init(void)
 {
+	if (!security_module_enable(&smack_ops))
+		return 0;
+
 	printk(KERN_INFO "Smack:  Initializing.\n");
 
 	/*

security/smack/smackfs.c +10 -1

index 358c92c..effd3de 100644
@@ -986,12 +986,21 @@ static struct vfsmount *smackfs_mount;
  *
  * register the smackfs
  *
- * Returns 0 unless the registration fails.
+ * Do not register smackfs if Smack wasn't enabled
+ * on boot. We can not put this method normally under the
+ * smack_init() code path since the security subsystem get
+ * initialized before the vfs caches.
+ *
+ * Returns true if we were not chosen on boot or if
+ * we were chosen and filesystem registration succeeded.
  */
 static int __init init_smk_fs(void)
 {
 	int err;
 
+	if (!security_module_enable(&smack_ops))
+		return 0;
+
 	err = register_filesystem(&smk_fs_type);
 	if (!err) {
 		smackfs_mount = kern_mount(&smk_fs_type);

Signature · 5 lines

"Better to light a candle, than curse the darkness"

Ahmed S. Darwish
Homepage: http://darwish.07.googlepages.com
Blog: http://darwish-07.blogspot.com

James Morris

maintainer Acked-by on v1 1/1

2008-03-07 01:43 UTC #5 lore raw

Quoted 3 lines · Ahmed S. Darwish

+ * Each LSM must pass this method before registering its own operations
+ * to avoid security registration races. This method may also be used
+ * to check if your LSM is currently loaded.

The 2nd sentence is no longer correct in general, as the function can only be called during init. See a fixup patch below.

Otherwise, looks good!

Acked-by James Morris <[email protected]>

I've applied the patch and a comment fixup to
git://git.kernel.org/pub/scm/linux/kernel/git/jmorris/security-testing-2.6.git#next

commit d6bb3bacedb68aa41ea7598e0cf1bef33cebbcc9

Author: James Morris <[email protected]>
Date:   Fri Mar 7 12:23:49 2008 +1100

security: fix up documentation for security_module_enable

security_module_enable() can only be called during kernel init.

Signed-off-by James Morris <[email protected]>

security/security.c +1 -1

index 4377b00..8aa2f54 100644
@@ -83,7 +83,7 @@ __setup("security=", choose_lsm);
  *
  * Each LSM must pass this method before registering its own operations
  * to avoid security registration races. This method may also be used
- * to check if your LSM is currently loaded.
+ * to check if your LSM is currently loaded during kernel initialization.
  *
  * Return true if:
  *     -The passed LSM is the one chosen by user at boot time,

Signature · 2 lines

James Morris
<[email protected]>