Re: [PATCH] split security_key_alloc into two functions

[Serge E. Hallyn]

The security_key_alloc() function acted as both an authorizer and
security structure allocation function.  These roles should be
separated.  There are two reasons for this.

First, if two modules are stacked, the first module might grant
permission and allocate security data, after which the second
module refuses permission.

Second, by adding a security_post_alloc() function after the
serial number has been assigned, security modules can append
useful info.

Note that currently there is no LSM using these hooks, so the
question of whether an LSM needs to recored the serial number
can't really be answered.

An alternative to this patch, supported by the historical approach
to LSM hooks, would be to remove all these hooks.  However as
the keystore starts being used - in particular by, eg, ecryptfs -
one might expect LSMs to be more interested in key activity.

Changelog:
Moved the security_key_post_alloc() hook under the
key_serial_lock to ensure that is not accessed before
an LSM has a chance to tag it.

:100644 100644 aaa0a5c... 569e874... M	include/linux/security.h
:100644 100644 fd99429... 1eff777... M	security/dummy.c
:100644 100644 a057e33... b72ffe2... M	security/keys/key.c

Signed-off-by: Serge Hallyn <serue@xxxxxxxxxx>

--- a/include/linux/security.h
+++ b/include/linux/security.h
@@ -844,10 +844,15 @@ struct swap_info_struct;
  * Security hooks affecting all Key Management operations
  *
  * @key_alloc:
- *	Permit allocation of a key and assign security data. Note that key does
- *	not have a serial number assigned at this point.
+ *	Check permission to allocate a key and assign security data. Note
+ *	that key does not have a serial number assigned at this point.
  *	@key points to the key.
  *	Return 0 if permission is granted, -ve error otherwise.
+ * @key_post_alloc:
+ * 	Allocate and attach a security structure to a key structure.
+ * 	This is called under the key_serial_lock spinlock, after a
+ * 	serial number is assigned and the key is inserted into a keyring.
+ *	@key points to the key.
  * @key_free:
  *	Notification of destruction; free security data.
  *	@key points to the key.
@@ -1312,6 +1317,7 @@ struct security_operations {
 	/* key management security hooks */
 #ifdef CONFIG_KEYS
 	int (*key_alloc)(struct key *key);
+	void (*key_post_alloc)(struct key *key);
 	void (*key_free)(struct key *key);
 	int (*key_permission)(key_ref_t key_ref,
 			      struct task_struct *context,
@@ -3001,6 +3007,11 @@ static inline int security_key_alloc(str
 	return security_ops->key_alloc(key);
 }
 
+static inline void security_key_post_alloc(struct key *key)
+{
+	security_ops->key_post_alloc(key);
+}
+
 static inline void security_key_free(struct key *key)
 {
 	security_ops->key_free(key);
@@ -3020,6 +3031,10 @@ static inline int security_key_alloc(str
 	return 0;
 }
 
+static inline void security_key_post_alloc(struct key *key)
+{
+}
+
 static inline void security_key_free(struct key *key)
 {
 }
diff --git a/security/dummy.c b/security/dummy.c
index fd99429..1eff777 100644
--- a/security/dummy.c
+++ b/security/dummy.c
@@ -860,6 +860,10 @@ static inline int dummy_key_alloc(struct
 	return 0;
 }
 
+static inline void dummy_key_post_alloc(struct key *key)
+{
+}
+
 static inline void dummy_key_free(struct key *key)
 {
 }
@@ -1036,6 +1040,7 @@ void security_fixup_ops (struct security
 #endif	/* CONFIG_SECURITY_NETWORK_XFRM */
 #ifdef CONFIG_KEYS
 	set_to_dummy_if_null(ops, key_alloc);
+	set_to_dummy_if_null(ops, key_post_alloc);
 	set_to_dummy_if_null(ops, key_free);
 	set_to_dummy_if_null(ops, key_permission);
 #endif	/* CONFIG_KEYS */
diff --git a/security/keys/key.c b/security/keys/key.c
index a057e33..b72ffe2 100644
--- a/security/keys/key.c
+++ b/security/keys/key.c
@@ -231,6 +231,8 @@ static inline void key_alloc_serial(stru
  insert_here:
 	rb_link_node(&key->serial_node, parent, p);
 	rb_insert_color(&key->serial_node, &key_serial_tree);
+	/* let the security module know the key has been published */
+	security_key_post_alloc(key);
 
 	spin_unlock(&key_serial_lock);
 
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/