[PATCH 1/3] selinux: Implement LSM notification system

[PATCH 1/3] selinux: Implement LSM notification system

[Sebastien Buisson]

From: Daniel Jurgens <danielj@mellanox.com>

Add a generic notification mechanism in the LSM. Interested consumers
can register a callback with the LSM and security modules can produce
events.

Add a call to the notification mechanism from SELinux when the AVC
cache changes.

Signed-off-by: Daniel Jurgens <danielj@mellanox.com>
Signed-off-by: Sebastien Buisson <sbuisson@ddn.com>
---
 include/linux/security.h | 23 +++++++++++++++++++++++
 security/security.c      | 20 ++++++++++++++++++++
 security/selinux/hooks.c | 12 ++++++++++++
 3 files changed, 55 insertions(+)

diff --git a/include/linux/security.h b/include/linux/security.h
index af675b5..73a9c93 100644
--- a/include/linux/security.h
+++ b/include/linux/security.h
@@ -68,6 +68,10 @@
 struct user_namespace;
 struct timezone;
 
+enum lsm_event {
+	LSM_POLICY_CHANGE,
+};
+
 /* These functions are in security/commoncap.c */
 extern int cap_capable(const struct cred *cred, struct user_namespace *ns,
 		       int cap, int audit);
@@ -163,6 +167,10 @@ struct security_mnt_opts {
 	int num_mnt_opts;
 };
 
+int call_lsm_notifier(enum lsm_event event, void *data);
+int register_lsm_notifier(struct notifier_block *nb);
+int unregister_lsm_notifier(struct notifier_block *nb);
+
 static inline void security_init_mnt_opts(struct security_mnt_opts *opts)
 {
 	opts->mnt_opts = NULL;
@@ -381,6 +389,21 @@ int security_sem_semop(struct sem_array *sma, struct sembuf *sops,
 struct security_mnt_opts {
 };
 
+static inline int call_lsm_notifier(enum lsm_event event, void *data)
+{
+	return 0;
+}
+
+static inline int register_lsm_notifier(struct notifier_block *nb)
+{
+	return 0;
+}
+
+static inline  int unregister_lsm_notifier(struct notifier_block *nb)
+{
+	return 0;
+}
+
 static inline void security_init_mnt_opts(struct security_mnt_opts *opts)
 {
 }
diff --git a/security/security.c b/security/security.c
index b9fea39..ef9d9e1 100644
--- a/security/security.c
+++ b/security/security.c
@@ -32,6 +32,8 @@
 /* Maximum number of letters for an LSM name string */
 #define SECURITY_NAME_MAX	10
 
+static ATOMIC_NOTIFIER_HEAD(lsm_notifier_chain);
+
 struct security_hook_heads security_hook_heads __lsm_ro_after_init;
 char *lsm_names;
 /* Boot-time LSM user choice */
@@ -146,6 +148,24 @@ void __init security_add_hooks(struct security_hook_list *hooks, int count,
 		panic("%s - Cannot get early memory.\n", __func__);
 }
 
+int call_lsm_notifier(enum lsm_event event, void *data)
+{
+	return atomic_notifier_call_chain(&lsm_notifier_chain, event, data);
+}
+EXPORT_SYMBOL(call_lsm_notifier);
+
+int register_lsm_notifier(struct notifier_block *nb)
+{
+	return atomic_notifier_chain_register(&lsm_notifier_chain, nb);
+}
+EXPORT_SYMBOL(register_lsm_notifier);
+
+int unregister_lsm_notifier(struct notifier_block *nb)
+{
+	return atomic_notifier_chain_unregister(&lsm_notifier_chain, nb);
+}
+EXPORT_SYMBOL(unregister_lsm_notifier);
+
 /*
  * Hook list operation macros.
  *
diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
index e67a526..a4d36f8 100644
--- a/security/selinux/hooks.c
+++ b/security/selinux/hooks.c
@@ -171,6 +171,14 @@ static int selinux_netcache_avc_callback(u32 event)
 	return 0;
 }
 
+static int selinux_lsm_notifier_avc_callback(u32 event)
+{
+	if (event == AVC_CALLBACK_RESET)
+		call_lsm_notifier(LSM_POLICY_CHANGE, NULL);
+
+	return 0;
+}
+
 /*
  * initialise the security for the init task
  */
@@ -6379,6 +6387,10 @@ static __init int selinux_init(void)
 	if (avc_add_callback(selinux_netcache_avc_callback, AVC_CALLBACK_RESET))
 		panic("SELinux: Unable to register AVC netcache callback\n");
 
+	if (avc_add_callback(selinux_lsm_notifier_avc_callback,
+			     AVC_CALLBACK_RESET))
+		panic("SELinux: Unable to register AVC LSM notifier callback\n");
+
 	if (selinux_enforcing)
 		printk(KERN_DEBUG "SELinux:  Starting in enforcing mode\n");
 	else
-- 
1.8.3.1

[PATCH 2/3] selinux: add checksum to policydb

[Sebastien Buisson]

Add policycksum field to struct policydb. It holds the sha256
checksum computed on the binary policy every time the notifier is
called after a policy change.
Add security_policy_cksum hook to give access to policy checksum to
the rest of the kernel. Lustre client makes use of this information.

Signed-off-by: Sebastien Buisson <sbuisson@ddn.com>
---
 include/linux/lsm_hooks.h           |  2 +
 include/linux/security.h            |  7 +++
 security/security.c                 |  6 +++
 security/selinux/hooks.c            | 12 ++++-
 security/selinux/include/security.h |  2 +
 security/selinux/ss/policydb.h      |  4 ++
 security/selinux/ss/services.c      | 91 +++++++++++++++++++++++++++++++++++++
 7 files changed, 123 insertions(+), 1 deletion(-)

diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h
index 080f34e..7aada73 100644
--- a/include/linux/lsm_hooks.h
+++ b/include/linux/lsm_hooks.h
@@ -1568,6 +1568,7 @@
 	int (*inode_setsecctx)(struct dentry *dentry, void *ctx, u32 ctxlen);
 	int (*inode_getsecctx)(struct inode *inode, void **ctx, u32 *ctxlen);
 
+	ssize_t (*policy_cksum)(char *cksum, size_t len);
 #ifdef CONFIG_SECURITY_NETWORK
 	int (*unix_stream_connect)(struct sock *sock, struct sock *other,
 					struct sock *newsk);
@@ -1813,6 +1814,7 @@ struct security_hook_heads {
 	struct list_head inode_notifysecctx;
 	struct list_head inode_setsecctx;
 	struct list_head inode_getsecctx;
+	struct list_head policy_cksum;
 #ifdef CONFIG_SECURITY_NETWORK
 	struct list_head unix_stream_connect;
 	struct list_head unix_may_send;
diff --git a/include/linux/security.h b/include/linux/security.h
index 73a9c93..8461d31 100644
--- a/include/linux/security.h
+++ b/include/linux/security.h
@@ -385,6 +385,8 @@ int security_sem_semop(struct sem_array *sma, struct sembuf *sops,
 int security_inode_notifysecctx(struct inode *inode, void *ctx, u32 ctxlen);
 int security_inode_setsecctx(struct dentry *dentry, void *ctx, u32 ctxlen);
 int security_inode_getsecctx(struct inode *inode, void **ctx, u32 *ctxlen);
+
+ssize_t security_policy_cksum(char *cksum, size_t len);
 #else /* CONFIG_SECURITY */
 struct security_mnt_opts {
 };
@@ -1189,6 +1191,11 @@ static inline int security_inode_getsecctx(struct inode *inode, void **ctx, u32
 {
 	return -EOPNOTSUPP;
 }
+
+static inline ssize_t security_policy_cksum(char *cksum, size_t len)
+{
+	return -EOPNOTSUPP;
+}
 #endif	/* CONFIG_SECURITY */
 
 #ifdef CONFIG_SECURITY_NETWORK
diff --git a/security/security.c b/security/security.c
index ef9d9e1..a82c08c 100644
--- a/security/security.c
+++ b/security/security.c
@@ -1305,6 +1305,12 @@ int security_inode_getsecctx(struct inode *inode, void **ctx, u32 *ctxlen)
 }
 EXPORT_SYMBOL(security_inode_getsecctx);
 
+ssize_t security_policy_cksum(char *cksum, size_t len)
+{
+	return call_int_hook(policy_cksum, -EOPNOTSUPP, cksum, len);
+}
+EXPORT_SYMBOL(security_policy_cksum);
+
 #ifdef CONFIG_SECURITY_NETWORK
 
 int security_unix_stream_connect(struct sock *sock, struct sock *other, struct sock *newsk)
diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
index a4d36f8..3759198 100644
--- a/security/selinux/hooks.c
+++ b/security/selinux/hooks.c
@@ -173,8 +173,11 @@ static int selinux_netcache_avc_callback(u32 event)
 
 static int selinux_lsm_notifier_avc_callback(u32 event)
 {
-	if (event == AVC_CALLBACK_RESET)
+	if (event == AVC_CALLBACK_RESET) {
+		if (security_policydb_compute_cksum() != 0)
+			printk(KERN_ERR "Failed to compute policydb cksum\n");
 		call_lsm_notifier(LSM_POLICY_CHANGE, NULL);
+	}
 
 	return 0;
 }
@@ -6071,6 +6074,11 @@ static int selinux_inode_getsecctx(struct inode *inode, void **ctx, u32 *ctxlen)
 	*ctxlen = len;
 	return 0;
 }
+
+static ssize_t selinux_policy_cksum(char *cksum, size_t len)
+{
+	return security_policydb_cksum(cksum, len);
+}
 #ifdef CONFIG_KEYS
 
 static int selinux_key_alloc(struct key *k, const struct cred *cred,
@@ -6285,6 +6293,8 @@ static int selinux_key_getsecurity(struct key *key, char **_buffer)
 	LSM_HOOK_INIT(inode_setsecctx, selinux_inode_setsecctx),
 	LSM_HOOK_INIT(inode_getsecctx, selinux_inode_getsecctx),
 
+	LSM_HOOK_INIT(policy_cksum, selinux_policy_cksum),
+
 	LSM_HOOK_INIT(unix_stream_connect, selinux_socket_unix_stream_connect),
 	LSM_HOOK_INIT(unix_may_send, selinux_socket_unix_may_send),
 
diff --git a/security/selinux/include/security.h b/security/selinux/include/security.h
index f979c35..a329571 100644
--- a/security/selinux/include/security.h
+++ b/security/selinux/include/security.h
@@ -97,6 +97,8 @@ enum {
 int security_load_policy(void *data, size_t len);
 int security_read_policy(void **data, size_t *len);
 size_t security_policydb_len(void);
+ssize_t security_policydb_cksum(char *cksum, size_t len);
+int security_policydb_compute_cksum(void);
 
 int security_policycap_supported(unsigned int req_cap);
 
diff --git a/security/selinux/ss/policydb.h b/security/selinux/ss/policydb.h
index 725d594..dc29492 100644
--- a/security/selinux/ss/policydb.h
+++ b/security/selinux/ss/policydb.h
@@ -25,6 +25,7 @@
 #define _SS_POLICYDB_H_
 
 #include <linux/flex_array.h>
+#include <crypto/sha.h>
 
 #include "symtab.h"
 #include "avtab.h"
@@ -293,6 +294,9 @@ struct policydb {
 	size_t len;
 
 	unsigned int policyvers;
+	/* checksum computed on the policy */
+	unsigned char policycksum[SHA256_DIGEST_SIZE*2 + 1];
+	size_t policycksum_len;
 
 	unsigned int reject_unknown : 1;
 	unsigned int allow_unknown : 1;
diff --git a/security/selinux/ss/services.c b/security/selinux/ss/services.c
index 60d9b02..a35d294 100644
--- a/security/selinux/ss/services.c
+++ b/security/selinux/ss/services.c
@@ -53,6 +53,8 @@
 #include <linux/flex_array.h>
 #include <linux/vmalloc.h>
 #include <net/netlabel.h>
+#include <crypto/hash.h>
+#include <crypto/sha.h>
 
 #include "flask.h"
 #include "avc.h"
@@ -2170,6 +2172,95 @@ size_t security_policydb_len(void)
 }
 
 /**
+ * security_policydb_cksum - Get policydb checksum.
+ * @cksum: string to store checksum to
+ * @len: length of checksum
+ */
+ssize_t security_policydb_cksum(char *cksum, size_t len)
+{
+	int rc;
+
+	read_lock(&policy_rwlock);
+	if (strlcpy(cksum, policydb.policycksum, len) >= len)
+		rc = -ENAMETOOLONG;
+	rc = policydb.policycksum_len;
+	read_unlock(&policy_rwlock);
+
+	return rc;
+}
+
+/**
+ * security_policydb_compute_cksum - Compute checksum of a policy database.
+ */
+int security_policydb_compute_cksum(void)
+{
+	struct crypto_ahash *tfm;
+	struct ahash_request *req;
+	struct scatterlist sl;
+	char hashval[SHA256_DIGEST_SIZE];
+	int idx;
+	unsigned char *p;
+	size_t len;
+	void *data;
+	int rc;
+
+	rc = security_read_policy(&data, &len);
+	if (rc) {
+		printk(KERN_ERR "Failed to read security policy\n");
+		return rc;
+	}
+
+	tfm = crypto_alloc_ahash("sha256", 0, CRYPTO_ALG_ASYNC);
+	if (IS_ERR(tfm)) {
+		printk(KERN_ERR "Failed to alloc crypto hash sha256\n");
+		vfree(data);
+		rc = PTR_ERR(tfm);
+		return rc;
+	}
+
+	req = ahash_request_alloc(tfm, GFP_KERNEL);
+	if (!req) {
+		printk(KERN_ERR "Failed to alloc ahash_request for sha256\n");
+		crypto_free_ahash(tfm);
+		vfree(data);
+		rc = -ENOMEM;
+		return rc;
+	}
+
+	ahash_request_set_callback(req, 0, NULL, NULL);
+
+	rc = crypto_ahash_init(req);
+	if (rc) {
+		printk(KERN_ERR "Failed to init ahash\n");
+		ahash_request_free(req);
+		crypto_free_ahash(tfm);
+		vfree(data);
+		return rc;
+	}
+
+	sg_init_one(&sl, (void *)data, len);
+	ahash_request_set_crypt(req, &sl, hashval, sl.length);
+	rc = crypto_ahash_digest(req);
+
+	crypto_free_ahash(tfm);
+	ahash_request_free(req);
+	vfree(data);
+	if (rc) {
+		printk(KERN_ERR "Failed to compute digest\n");
+		return rc;
+	}
+
+	p = policydb.policycksum;
+	for (idx = 0; idx < SHA256_DIGEST_SIZE; idx++) {
+		snprintf(p, 3, "%02x", (unsigned char)(hashval[idx]));
+		p += 2;
+	}
+	policydb.policycksum_len = (size_t)(p - policydb.policycksum);
+
+	return 0;
+}
+
+/**
  * security_port_sid - Obtain the SID for a port.
  * @protocol: protocol number
  * @port: port number
-- 
1.8.3.1

[PATCH 3/3] selinux: expose policy SHA256 checksum via selinuxfs

[Sebastien Buisson]

Expose policy SHA256 checksum via selinuxfs.

Signed-off-by: Sebastien Buisson <sbuisson@ddn.com>
---
 security/selinux/selinuxfs.c | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/security/selinux/selinuxfs.c b/security/selinux/selinuxfs.c
index ce71718..b2d5deb 100644
--- a/security/selinux/selinuxfs.c
+++ b/security/selinux/selinuxfs.c
@@ -30,6 +30,7 @@
 #include <linux/uaccess.h>
 #include <linux/kobject.h>
 #include <linux/ctype.h>
+#include <crypto/sha.h>
 
 /* selinuxfs pseudo filesystem for exporting the security policy API.
    Based on the proc code and the fs/nfsd/nfsctl.c code. */
@@ -99,6 +100,7 @@ enum sel_inos {
 	SEL_STATUS,	/* export current status using mmap() */
 	SEL_POLICY,	/* allow userspace to read the in kernel policy */
 	SEL_VALIDATE_TRANS, /* compute validatetrans decision */
+	SEL_POLICYCKSUM,/* return policy SHA256 checkum */
 	SEL_INO_NEXT,	/* The next inode number to use */
 };
 
@@ -313,6 +315,22 @@ static ssize_t sel_read_policyvers(struct file *filp, char __user *buf,
 	.llseek		= generic_file_llseek,
 };
 
+static ssize_t sel_read_policycksum(struct file *filp, char __user *buf,
+				    size_t count, loff_t *ppos)
+{
+	size_t tmpbuflen = SHA256_DIGEST_SIZE*2 + 1;
+	char tmpbuf[tmpbuflen];
+	ssize_t length;
+
+	length = security_policydb_cksum(tmpbuf, tmpbuflen);
+	return simple_read_from_buffer(buf, count, ppos, tmpbuf, length);
+}
+
+static const struct file_operations sel_policycksum_ops = {
+	.read		= sel_read_policycksum,
+	.llseek		= generic_file_llseek,
+};
+
 /* declaration for sel_write_load */
 static int sel_make_bools(void);
 static int sel_make_classes(void);
@@ -1825,6 +1843,8 @@ static int sel_fill_super(struct super_block *sb, void *data, int silent)
 		[SEL_POLICY] = {"policy", &sel_policy_ops, S_IRUGO},
 		[SEL_VALIDATE_TRANS] = {"validatetrans", &sel_transition_ops,
 					S_IWUGO},
+		[SEL_POLICYCKSUM] = {"policycksum", &sel_policycksum_ops,
+				     S_IRUGO},
 		/* last one */ {""}
 	};
 	ret = simple_fill_super(sb, SELINUX_MAGIC, selinux_files);
-- 
1.8.3.1

Re: [PATCH 1/3] selinux: Implement LSM notification system

[Casey Schaufler]

On 4/26/2017 8:02 AM, Sebastien Buisson wrote:
> From: Daniel Jurgens <danielj@mellanox.com>
>
> Add a generic notification mechanism in the LSM. Interested consumers
> can register a callback with the LSM and security modules can produce
> events.

Why is this a generic mechanism? Do you ever see anyone
other than SELinux using it?

> Add a call to the notification mechanism from SELinux when the AVC
> cache changes.

This seems like a whole lot of mechanism for
something you could accomplish with a log message.
What am I missing?

>
> Signed-off-by: Daniel Jurgens <danielj@mellanox.com>
> Signed-off-by: Sebastien Buisson <sbuisson@ddn.com>
> ---
>  include/linux/security.h | 23 +++++++++++++++++++++++
>  security/security.c      | 20 ++++++++++++++++++++
>  security/selinux/hooks.c | 12 ++++++++++++
>  3 files changed, 55 insertions(+)
>
> diff --git a/include/linux/security.h b/include/linux/security.h
> index af675b5..73a9c93 100644
> --- a/include/linux/security.h
> +++ b/include/linux/security.h
> @@ -68,6 +68,10 @@
>  struct user_namespace;
>  struct timezone;
>  
> +enum lsm_event {
> +	LSM_POLICY_CHANGE,
> +};
> +
>  /* These functions are in security/commoncap.c */
>  extern int cap_capable(const struct cred *cred, struct user_namespace *ns,
>  		       int cap, int audit);
> @@ -163,6 +167,10 @@ struct security_mnt_opts {
>  	int num_mnt_opts;
>  };
>  
> +int call_lsm_notifier(enum lsm_event event, void *data);
> +int register_lsm_notifier(struct notifier_block *nb);
> +int unregister_lsm_notifier(struct notifier_block *nb);
> +
>  static inline void security_init_mnt_opts(struct security_mnt_opts *opts)
>  {
>  	opts->mnt_opts = NULL;
> @@ -381,6 +389,21 @@ int security_sem_semop(struct sem_array *sma, struct sembuf *sops,
>  struct security_mnt_opts {
>  };
>  
> +static inline int call_lsm_notifier(enum lsm_event event, void *data)
> +{
> +	return 0;
> +}
> +
> +static inline int register_lsm_notifier(struct notifier_block *nb)
> +{
> +	return 0;
> +}
> +
> +static inline  int unregister_lsm_notifier(struct notifier_block *nb)
> +{
> +	return 0;
> +}
> +
>  static inline void security_init_mnt_opts(struct security_mnt_opts *opts)
>  {
>  }
> diff --git a/security/security.c b/security/security.c
> index b9fea39..ef9d9e1 100644
> --- a/security/security.c
> +++ b/security/security.c
> @@ -32,6 +32,8 @@
>  /* Maximum number of letters for an LSM name string */
>  #define SECURITY_NAME_MAX	10
>  
> +static ATOMIC_NOTIFIER_HEAD(lsm_notifier_chain);
> +
>  struct security_hook_heads security_hook_heads __lsm_ro_after_init;
>  char *lsm_names;
>  /* Boot-time LSM user choice */
> @@ -146,6 +148,24 @@ void __init security_add_hooks(struct security_hook_list *hooks, int count,
>  		panic("%s - Cannot get early memory.\n", __func__);
>  }
>  
> +int call_lsm_notifier(enum lsm_event event, void *data)
> +{
> +	return atomic_notifier_call_chain(&lsm_notifier_chain, event, data);
> +}
> +EXPORT_SYMBOL(call_lsm_notifier);
> +
> +int register_lsm_notifier(struct notifier_block *nb)
> +{
> +	return atomic_notifier_chain_register(&lsm_notifier_chain, nb);
> +}
> +EXPORT_SYMBOL(register_lsm_notifier);
> +
> +int unregister_lsm_notifier(struct notifier_block *nb)
> +{
> +	return atomic_notifier_chain_unregister(&lsm_notifier_chain, nb);
> +}
> +EXPORT_SYMBOL(unregister_lsm_notifier);
> +
>  /*
>   * Hook list operation macros.
>   *
> diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
> index e67a526..a4d36f8 100644
> --- a/security/selinux/hooks.c
> +++ b/security/selinux/hooks.c
> @@ -171,6 +171,14 @@ static int selinux_netcache_avc_callback(u32 event)
>  	return 0;
>  }
>  
> +static int selinux_lsm_notifier_avc_callback(u32 event)
> +{
> +	if (event == AVC_CALLBACK_RESET)
> +		call_lsm_notifier(LSM_POLICY_CHANGE, NULL);
> +
> +	return 0;
> +}
> +
>  /*
>   * initialise the security for the init task
>   */
> @@ -6379,6 +6387,10 @@ static __init int selinux_init(void)
>  	if (avc_add_callback(selinux_netcache_avc_callback, AVC_CALLBACK_RESET))
>  		panic("SELinux: Unable to register AVC netcache callback\n");
>  
> +	if (avc_add_callback(selinux_lsm_notifier_avc_callback,
> +			     AVC_CALLBACK_RESET))
> +		panic("SELinux: Unable to register AVC LSM notifier callback\n");
> +
>  	if (selinux_enforcing)
>  		printk(KERN_DEBUG "SELinux:  Starting in enforcing mode\n");
>  	else

Re: [PATCH 1/3] selinux: Implement LSM notification system

[Daniel Jurgens]

On 4/26/2017 10:38 AM, Casey Schaufler wrote:
> On 4/26/2017 8:02 AM, Sebastien Buisson wrote:
>> From: Daniel Jurgens <danielj@mellanox.com>
>>
>> Add a generic notification mechanism in the LSM. Interested consumers
>> can register a callback with the LSM and security modules can produce
>> events.
> Why is this a generic mechanism? Do you ever see anyone
> other than SELinux using it?
I had created an SELinux specific mechanism, Paul Moore requested I make it generic.
>> Add a call to the notification mechanism from SELinux when the AVC
>> cache changes.
> This seems like a whole lot of mechanism for
> something you could accomplish with a log message.
> What am I missing?
This was part of a larger patch set that hasn't been accepted yet.  SELinux support for Inifiniband.  Subsequent patches in that patch set will use it as well.
>> Signed-off-by: Daniel Jurgens <danielj@mellanox.com>
>> Signed-off-by: Sebastien Buisson <sbuisson@ddn.com>
>> ---
>>  include/linux/security.h | 23 +++++++++++++++++++++++
>>  security/security.c      | 20 ++++++++++++++++++++
>>  security/selinux/hooks.c | 12 ++++++++++++
>>  3 files changed, 55 insertions(+)
>>
>> diff --git a/include/linux/security.h b/include/linux/security.h
>> index af675b5..73a9c93 100644
>> --- a/include/linux/security.h
>> +++ b/include/linux/security.h
>> @@ -68,6 +68,10 @@
>>  struct user_namespace;
>>  struct timezone;
>>  
>> +enum lsm_event {
>> +	LSM_POLICY_CHANGE,
>> +};
>> +
>>  /* These functions are in security/commoncap.c */
>>  extern int cap_capable(const struct cred *cred, struct user_namespace *ns,
>>  		       int cap, int audit);
>> @@ -163,6 +167,10 @@ struct security_mnt_opts {
>>  	int num_mnt_opts;
>>  };
>>  
>> +int call_lsm_notifier(enum lsm_event event, void *data);
>> +int register_lsm_notifier(struct notifier_block *nb);
>> +int unregister_lsm_notifier(struct notifier_block *nb);
>> +
>>  static inline void security_init_mnt_opts(struct security_mnt_opts *opts)
>>  {
>>  	opts->mnt_opts = NULL;
>> @@ -381,6 +389,21 @@ int security_sem_semop(struct sem_array *sma, struct sembuf *sops,
>>  struct security_mnt_opts {
>>  };
>>  
>> +static inline int call_lsm_notifier(enum lsm_event event, void *data)
>> +{
>> +	return 0;
>> +}
>> +
>> +static inline int register_lsm_notifier(struct notifier_block *nb)
>> +{
>> +	return 0;
>> +}
>> +
>> +static inline  int unregister_lsm_notifier(struct notifier_block *nb)
>> +{
>> +	return 0;
>> +}
>> +
>>  static inline void security_init_mnt_opts(struct security_mnt_opts *opts)
>>  {
>>  }
>> diff --git a/security/security.c b/security/security.c
>> index b9fea39..ef9d9e1 100644
>> --- a/security/security.c
>> +++ b/security/security.c
>> @@ -32,6 +32,8 @@
>>  /* Maximum number of letters for an LSM name string */
>>  #define SECURITY_NAME_MAX	10
>>  
>> +static ATOMIC_NOTIFIER_HEAD(lsm_notifier_chain);
>> +
>>  struct security_hook_heads security_hook_heads __lsm_ro_after_init;
>>  char *lsm_names;
>>  /* Boot-time LSM user choice */
>> @@ -146,6 +148,24 @@ void __init security_add_hooks(struct security_hook_list *hooks, int count,
>>  		panic("%s - Cannot get early memory.\n", __func__);
>>  }
>>  
>> +int call_lsm_notifier(enum lsm_event event, void *data)
>> +{
>> +	return atomic_notifier_call_chain(&lsm_notifier_chain, event, data);
>> +}
>> +EXPORT_SYMBOL(call_lsm_notifier);
>> +
>> +int register_lsm_notifier(struct notifier_block *nb)
>> +{
>> +	return atomic_notifier_chain_register(&lsm_notifier_chain, nb);
>> +}
>> +EXPORT_SYMBOL(register_lsm_notifier);
>> +
>> +int unregister_lsm_notifier(struct notifier_block *nb)
>> +{
>> +	return atomic_notifier_chain_unregister(&lsm_notifier_chain, nb);
>> +}
>> +EXPORT_SYMBOL(unregister_lsm_notifier);
>> +
>>  /*
>>   * Hook list operation macros.
>>   *
>> diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
>> index e67a526..a4d36f8 100644
>> --- a/security/selinux/hooks.c
>> +++ b/security/selinux/hooks.c
>> @@ -171,6 +171,14 @@ static int selinux_netcache_avc_callback(u32 event)
>>  	return 0;
>>  }
>>  
>> +static int selinux_lsm_notifier_avc_callback(u32 event)
>> +{
>> +	if (event == AVC_CALLBACK_RESET)
>> +		call_lsm_notifier(LSM_POLICY_CHANGE, NULL);
>> +
>> +	return 0;
>> +}
>> +
>>  /*
>>   * initialise the security for the init task
>>   */
>> @@ -6379,6 +6387,10 @@ static __init int selinux_init(void)
>>  	if (avc_add_callback(selinux_netcache_avc_callback, AVC_CALLBACK_RESET))
>>  		panic("SELinux: Unable to register AVC netcache callback\n");
>>  
>> +	if (avc_add_callback(selinux_lsm_notifier_avc_callback,
>> +			     AVC_CALLBACK_RESET))
>> +		panic("SELinux: Unable to register AVC LSM notifier callback\n");
>> +
>>  	if (selinux_enforcing)
>>  		printk(KERN_DEBUG "SELinux:  Starting in enforcing mode\n");
>>  	else
>

Re: [PATCH 1/3] selinux: Implement LSM notification system

[Sebastien Buisson]

2017-04-26 17:48 GMT+02:00 Daniel Jurgens <danielj@mellanox.com>:
> This was part of a larger patch set that hasn't been accepted yet.  SELinux support for Inifiniband.  Subsequent patches in that patch set will use it as well.

I revived this patch following Stephen Smalley's suggestion to base
the policy checksum computation and change notification on the work
you had done on LSM notification callbacks.

Re: [PATCH 1/3] selinux: Implement LSM notification system

[Casey Schaufler]

On 4/26/2017 8:48 AM, Daniel Jurgens wrote:
> On 4/26/2017 10:38 AM, Casey Schaufler wrote:
>> On 4/26/2017 8:02 AM, Sebastien Buisson wrote:
>>> From: Daniel Jurgens <danielj@mellanox.com>
>>>
>>> Add a generic notification mechanism in the LSM. Interested consumers
>>> can register a callback with the LSM and security modules can produce
>>> events.
>> Why is this a generic mechanism? Do you ever see anyone
>> other than SELinux using it?
> I had created an SELinux specific mechanism, Paul Moore requested I make it generic.
>>> Add a call to the notification mechanism from SELinux when the AVC
>>> cache changes.
>> This seems like a whole lot of mechanism for
>> something you could accomplish with a log message.
>> What am I missing?
> This was part of a larger patch set that hasn't been accepted yet.  SELinux support for Inifiniband.  Subsequent patches in that patch set will use it as well.

You should include this information in a 0/3 message.
Without the context the patch set is curious, but not
compelling.

>>> Signed-off-by: Daniel Jurgens <danielj@mellanox.com>
>>> Signed-off-by: Sebastien Buisson <sbuisson@ddn.com>
>>> ---
>>>  include/linux/security.h | 23 +++++++++++++++++++++++
>>>  security/security.c      | 20 ++++++++++++++++++++
>>>  security/selinux/hooks.c | 12 ++++++++++++
>>>  3 files changed, 55 insertions(+)
>>>
>>> diff --git a/include/linux/security.h b/include/linux/security.h
>>> index af675b5..73a9c93 100644
>>> --- a/include/linux/security.h
>>> +++ b/include/linux/security.h
>>> @@ -68,6 +68,10 @@
>>>  struct user_namespace;
>>>  struct timezone;
>>>  
>>> +enum lsm_event {
>>> +	LSM_POLICY_CHANGE,
>>> +};
>>> +
>>>  /* These functions are in security/commoncap.c */
>>>  extern int cap_capable(const struct cred *cred, struct user_namespace *ns,
>>>  		       int cap, int audit);
>>> @@ -163,6 +167,10 @@ struct security_mnt_opts {
>>>  	int num_mnt_opts;
>>>  };
>>>  
>>> +int call_lsm_notifier(enum lsm_event event, void *data);
>>> +int register_lsm_notifier(struct notifier_block *nb);
>>> +int unregister_lsm_notifier(struct notifier_block *nb);
>>> +
>>>  static inline void security_init_mnt_opts(struct security_mnt_opts *opts)
>>>  {
>>>  	opts->mnt_opts = NULL;
>>> @@ -381,6 +389,21 @@ int security_sem_semop(struct sem_array *sma, struct sembuf *sops,
>>>  struct security_mnt_opts {
>>>  };
>>>  
>>> +static inline int call_lsm_notifier(enum lsm_event event, void *data)
>>> +{
>>> +	return 0;
>>> +}
>>> +
>>> +static inline int register_lsm_notifier(struct notifier_block *nb)
>>> +{
>>> +	return 0;
>>> +}
>>> +
>>> +static inline  int unregister_lsm_notifier(struct notifier_block *nb)
>>> +{
>>> +	return 0;
>>> +}
>>> +
>>>  static inline void security_init_mnt_opts(struct security_mnt_opts *opts)
>>>  {
>>>  }
>>> diff --git a/security/security.c b/security/security.c
>>> index b9fea39..ef9d9e1 100644
>>> --- a/security/security.c
>>> +++ b/security/security.c
>>> @@ -32,6 +32,8 @@
>>>  /* Maximum number of letters for an LSM name string */
>>>  #define SECURITY_NAME_MAX	10
>>>  
>>> +static ATOMIC_NOTIFIER_HEAD(lsm_notifier_chain);
>>> +
>>>  struct security_hook_heads security_hook_heads __lsm_ro_after_init;
>>>  char *lsm_names;
>>>  /* Boot-time LSM user choice */
>>> @@ -146,6 +148,24 @@ void __init security_add_hooks(struct security_hook_list *hooks, int count,
>>>  		panic("%s - Cannot get early memory.\n", __func__);
>>>  }
>>>  
>>> +int call_lsm_notifier(enum lsm_event event, void *data)
>>> +{
>>> +	return atomic_notifier_call_chain(&lsm_notifier_chain, event, data);
>>> +}
>>> +EXPORT_SYMBOL(call_lsm_notifier);
>>> +
>>> +int register_lsm_notifier(struct notifier_block *nb)
>>> +{
>>> +	return atomic_notifier_chain_register(&lsm_notifier_chain, nb);
>>> +}
>>> +EXPORT_SYMBOL(register_lsm_notifier);
>>> +
>>> +int unregister_lsm_notifier(struct notifier_block *nb)
>>> +{
>>> +	return atomic_notifier_chain_unregister(&lsm_notifier_chain, nb);
>>> +}
>>> +EXPORT_SYMBOL(unregister_lsm_notifier);
>>> +
>>>  /*
>>>   * Hook list operation macros.
>>>   *
>>> diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
>>> index e67a526..a4d36f8 100644
>>> --- a/security/selinux/hooks.c
>>> +++ b/security/selinux/hooks.c
>>> @@ -171,6 +171,14 @@ static int selinux_netcache_avc_callback(u32 event)
>>>  	return 0;
>>>  }
>>>  
>>> +static int selinux_lsm_notifier_avc_callback(u32 event)
>>> +{
>>> +	if (event == AVC_CALLBACK_RESET)
>>> +		call_lsm_notifier(LSM_POLICY_CHANGE, NULL);
>>> +
>>> +	return 0;
>>> +}
>>> +
>>>  /*
>>>   * initialise the security for the init task
>>>   */
>>> @@ -6379,6 +6387,10 @@ static __init int selinux_init(void)
>>>  	if (avc_add_callback(selinux_netcache_avc_callback, AVC_CALLBACK_RESET))
>>>  		panic("SELinux: Unable to register AVC netcache callback\n");
>>>  
>>> +	if (avc_add_callback(selinux_lsm_notifier_avc_callback,
>>> +			     AVC_CALLBACK_RESET))
>>> +		panic("SELinux: Unable to register AVC LSM notifier callback\n");
>>> +
>>>  	if (selinux_enforcing)
>>>  		printk(KERN_DEBUG "SELinux:  Starting in enforcing mode\n");
>>>  	else
>

Re: [PATCH 1/3] selinux: Implement LSM notification system

[Stephen Smalley]

On Wed, 2017-04-26 at 08:38 -0700, Casey Schaufler wrote:
> On 4/26/2017 8:02 AM, Sebastien Buisson wrote:
> > From: Daniel Jurgens <danielj@mellanox.com>
> > 
> > Add a generic notification mechanism in the LSM. Interested
> > consumers
> > can register a callback with the LSM and security modules can
> > produce
> > events.
> 
> Why is this a generic mechanism? Do you ever see anyone
> other than SELinux using it?

I do - any security module that wants to support access control over
Lustre filesystems or Infiniband. Seems ironic for you to be arguing
for a SELinux-specific interface rather than a LSM interface.

> 
> > Add a call to the notification mechanism from SELinux when the AVC
> > cache changes.
> 
> This seems like a whole lot of mechanism for
> something you could accomplish with a log message.
> What am I missing?

It's a notification to a kernel subsystem that policy has changed so
that the subsystem can update any cached state.

> 
> > 
> > Signed-off-by: Daniel Jurgens <danielj@mellanox.com>
> > Signed-off-by: Sebastien Buisson <sbuisson@ddn.com>
> > ---
> >  include/linux/security.h | 23 +++++++++++++++++++++++
> >  security/security.c      | 20 ++++++++++++++++++++
> >  security/selinux/hooks.c | 12 ++++++++++++
> >  3 files changed, 55 insertions(+)
> > 
> > diff --git a/include/linux/security.h b/include/linux/security.h
> > index af675b5..73a9c93 100644
> > --- a/include/linux/security.h
> > +++ b/include/linux/security.h
> > @@ -68,6 +68,10 @@
> >  struct user_namespace;
> >  struct timezone;
> >  
> > +enum lsm_event {
> > +	LSM_POLICY_CHANGE,
> > +};
> > +
> >  /* These functions are in security/commoncap.c */
> >  extern int cap_capable(const struct cred *cred, struct
> > user_namespace *ns,
> >  		       int cap, int audit);
> > @@ -163,6 +167,10 @@ struct security_mnt_opts {
> >  	int num_mnt_opts;
> >  };
> >  
> > +int call_lsm_notifier(enum lsm_event event, void *data);
> > +int register_lsm_notifier(struct notifier_block *nb);
> > +int unregister_lsm_notifier(struct notifier_block *nb);
> > +
> >  static inline void security_init_mnt_opts(struct security_mnt_opts
> > *opts)
> >  {
> >  	opts->mnt_opts = NULL;
> > @@ -381,6 +389,21 @@ int security_sem_semop(struct sem_array *sma,
> > struct sembuf *sops,
> >  struct security_mnt_opts {
> >  };
> >  
> > +static inline int call_lsm_notifier(enum lsm_event event, void
> > *data)
> > +{
> > +	return 0;
> > +}
> > +
> > +static inline int register_lsm_notifier(struct notifier_block *nb)
> > +{
> > +	return 0;
> > +}
> > +
> > +static inline  int unregister_lsm_notifier(struct notifier_block
> > *nb)
> > +{
> > +	return 0;
> > +}
> > +
> >  static inline void security_init_mnt_opts(struct security_mnt_opts
> > *opts)
> >  {
> >  }
> > diff --git a/security/security.c b/security/security.c
> > index b9fea39..ef9d9e1 100644
> > --- a/security/security.c
> > +++ b/security/security.c
> > @@ -32,6 +32,8 @@
> >  /* Maximum number of letters for an LSM name string */
> >  #define SECURITY_NAME_MAX	10
> >  
> > +static ATOMIC_NOTIFIER_HEAD(lsm_notifier_chain);
> > +
> >  struct security_hook_heads security_hook_heads
> > __lsm_ro_after_init;
> >  char *lsm_names;
> >  /* Boot-time LSM user choice */
> > @@ -146,6 +148,24 @@ void __init security_add_hooks(struct
> > security_hook_list *hooks, int count,
> >  		panic("%s - Cannot get early memory.\n",
> > __func__);
> >  }
> >  
> > +int call_lsm_notifier(enum lsm_event event, void *data)
> > +{
> > +	return atomic_notifier_call_chain(&lsm_notifier_chain,
> > event, data);
> > +}
> > +EXPORT_SYMBOL(call_lsm_notifier);
> > +
> > +int register_lsm_notifier(struct notifier_block *nb)
> > +{
> > +	return atomic_notifier_chain_register(&lsm_notifier_chain,
> > nb);
> > +}
> > +EXPORT_SYMBOL(register_lsm_notifier);
> > +
> > +int unregister_lsm_notifier(struct notifier_block *nb)
> > +{
> > +	return
> > atomic_notifier_chain_unregister(&lsm_notifier_chain, nb);
> > +}
> > +EXPORT_SYMBOL(unregister_lsm_notifier);
> > +
> >  /*
> >   * Hook list operation macros.
> >   *
> > diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
> > index e67a526..a4d36f8 100644
> > --- a/security/selinux/hooks.c
> > +++ b/security/selinux/hooks.c
> > @@ -171,6 +171,14 @@ static int selinux_netcache_avc_callback(u32
> > event)
> >  	return 0;
> >  }
> >  
> > +static int selinux_lsm_notifier_avc_callback(u32 event)
> > +{
> > +	if (event == AVC_CALLBACK_RESET)
> > +		call_lsm_notifier(LSM_POLICY_CHANGE, NULL);
> > +
> > +	return 0;
> > +}
> > +
> >  /*
> >   * initialise the security for the init task
> >   */
> > @@ -6379,6 +6387,10 @@ static __init int selinux_init(void)
> >  	if (avc_add_callback(selinux_netcache_avc_callback,
> > AVC_CALLBACK_RESET))
> >  		panic("SELinux: Unable to register AVC netcache
> > callback\n");
> >  
> > +	if (avc_add_callback(selinux_lsm_notifier_avc_callback,
> > +			     AVC_CALLBACK_RESET))
> > +		panic("SELinux: Unable to register AVC LSM
> > notifier callback\n");
> > +
> >  	if (selinux_enforcing)
> >  		printk(KERN_DEBUG "SELinux:  Starting in enforcing
> > mode\n");
> >  	else

Re: [PATCH 1/3] selinux: Implement LSM notification system

[Casey Schaufler]

On 4/26/2017 10:36 AM, Stephen Smalley wrote:
> On Wed, 2017-04-26 at 08:38 -0700, Casey Schaufler wrote:
>> On 4/26/2017 8:02 AM, Sebastien Buisson wrote:
>>> From: Daniel Jurgens <danielj@mellanox.com>
>>>
>>> Add a generic notification mechanism in the LSM. Interested
>>> consumers
>>> can register a callback with the LSM and security modules can
>>> produce
>>> events.
>> Why is this a generic mechanism? Do you ever see anyone
>> other than SELinux using it?
> I do - any security module that wants to support access control over
> Lustre filesystems or Infiniband. Seems ironic for you to be arguing
> for a SELinux-specific interface rather than a LSM interface.

I lost the connection between this mechanism and
Lustre and Infiniband, neither of which are mentioned
in the patchset. And I *did* pose it as a question.
I am all in favor of general mechanisms where they
are generally useful. I am also adverse to special
purpose clutter.

>
>>> Add a call to the notification mechanism from SELinux when the AVC
>>> cache changes.
>> This seems like a whole lot of mechanism for
>> something you could accomplish with a log message.
>> What am I missing?
> It's a notification to a kernel subsystem that policy has changed so
> that the subsystem can update any cached state.
>
>>> Signed-off-by: Daniel Jurgens <danielj@mellanox.com>
>>> Signed-off-by: Sebastien Buisson <sbuisson@ddn.com>
>>> ---
>>>  include/linux/security.h | 23 +++++++++++++++++++++++
>>>  security/security.c      | 20 ++++++++++++++++++++
>>>  security/selinux/hooks.c | 12 ++++++++++++
>>>  3 files changed, 55 insertions(+)
>>>
>>> diff --git a/include/linux/security.h b/include/linux/security.h
>>> index af675b5..73a9c93 100644
>>> --- a/include/linux/security.h
>>> +++ b/include/linux/security.h
>>> @@ -68,6 +68,10 @@
>>>  struct user_namespace;
>>>  struct timezone;
>>>  
>>> +enum lsm_event {
>>> +	LSM_POLICY_CHANGE,
>>> +};
>>> +
>>>  /* These functions are in security/commoncap.c */
>>>  extern int cap_capable(const struct cred *cred, struct
>>> user_namespace *ns,
>>>  		       int cap, int audit);
>>> @@ -163,6 +167,10 @@ struct security_mnt_opts {
>>>  	int num_mnt_opts;
>>>  };
>>>  
>>> +int call_lsm_notifier(enum lsm_event event, void *data);
>>> +int register_lsm_notifier(struct notifier_block *nb);
>>> +int unregister_lsm_notifier(struct notifier_block *nb);
>>> +
>>>  static inline void security_init_mnt_opts(struct security_mnt_opts
>>> *opts)
>>>  {
>>>  	opts->mnt_opts = NULL;
>>> @@ -381,6 +389,21 @@ int security_sem_semop(struct sem_array *sma,
>>> struct sembuf *sops,
>>>  struct security_mnt_opts {
>>>  };
>>>  
>>> +static inline int call_lsm_notifier(enum lsm_event event, void
>>> *data)
>>> +{
>>> +	return 0;
>>> +}
>>> +
>>> +static inline int register_lsm_notifier(struct notifier_block *nb)
>>> +{
>>> +	return 0;
>>> +}
>>> +
>>> +static inline  int unregister_lsm_notifier(struct notifier_block
>>> *nb)
>>> +{
>>> +	return 0;
>>> +}
>>> +
>>>  static inline void security_init_mnt_opts(struct security_mnt_opts
>>> *opts)
>>>  {
>>>  }
>>> diff --git a/security/security.c b/security/security.c
>>> index b9fea39..ef9d9e1 100644
>>> --- a/security/security.c
>>> +++ b/security/security.c
>>> @@ -32,6 +32,8 @@
>>>  /* Maximum number of letters for an LSM name string */
>>>  #define SECURITY_NAME_MAX	10
>>>  
>>> +static ATOMIC_NOTIFIER_HEAD(lsm_notifier_chain);
>>> +
>>>  struct security_hook_heads security_hook_heads
>>> __lsm_ro_after_init;
>>>  char *lsm_names;
>>>  /* Boot-time LSM user choice */
>>> @@ -146,6 +148,24 @@ void __init security_add_hooks(struct
>>> security_hook_list *hooks, int count,
>>>  		panic("%s - Cannot get early memory.\n",
>>> __func__);
>>>  }
>>>  
>>> +int call_lsm_notifier(enum lsm_event event, void *data)
>>> +{
>>> +	return atomic_notifier_call_chain(&lsm_notifier_chain,
>>> event, data);
>>> +}
>>> +EXPORT_SYMBOL(call_lsm_notifier);
>>> +
>>> +int register_lsm_notifier(struct notifier_block *nb)
>>> +{
>>> +	return atomic_notifier_chain_register(&lsm_notifier_chain,
>>> nb);
>>> +}
>>> +EXPORT_SYMBOL(register_lsm_notifier);
>>> +
>>> +int unregister_lsm_notifier(struct notifier_block *nb)
>>> +{
>>> +	return
>>> atomic_notifier_chain_unregister(&lsm_notifier_chain, nb);
>>> +}
>>> +EXPORT_SYMBOL(unregister_lsm_notifier);
>>> +
>>>  /*
>>>   * Hook list operation macros.
>>>   *
>>> diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
>>> index e67a526..a4d36f8 100644
>>> --- a/security/selinux/hooks.c
>>> +++ b/security/selinux/hooks.c
>>> @@ -171,6 +171,14 @@ static int selinux_netcache_avc_callback(u32
>>> event)
>>>  	return 0;
>>>  }
>>>  
>>> +static int selinux_lsm_notifier_avc_callback(u32 event)
>>> +{
>>> +	if (event == AVC_CALLBACK_RESET)
>>> +		call_lsm_notifier(LSM_POLICY_CHANGE, NULL);
>>> +
>>> +	return 0;
>>> +}
>>> +
>>>  /*
>>>   * initialise the security for the init task
>>>   */
>>> @@ -6379,6 +6387,10 @@ static __init int selinux_init(void)
>>>  	if (avc_add_callback(selinux_netcache_avc_callback,
>>> AVC_CALLBACK_RESET))
>>>  		panic("SELinux: Unable to register AVC netcache
>>> callback\n");
>>>  
>>> +	if (avc_add_callback(selinux_lsm_notifier_avc_callback,
>>> +			     AVC_CALLBACK_RESET))
>>> +		panic("SELinux: Unable to register AVC LSM
>>> notifier callback\n");
>>> +
>>>  	if (selinux_enforcing)
>>>  		printk(KERN_DEBUG "SELinux:  Starting in enforcing
>>> mode\n");
>>>  	else

Re: [PATCH 2/3] selinux: add checksum to policydb

[Stephen Smalley]

On Thu, 2017-04-27 at 00:02 +0900, Sebastien Buisson wrote:
> Add policycksum field to struct policydb. It holds the sha256
> checksum computed on the binary policy every time the notifier is
> called after a policy change.
> Add security_policy_cksum hook to give access to policy checksum to
> the rest of the kernel. Lustre client makes use of this information.
> 
> Signed-off-by: Sebastien Buisson <sbuisson@ddn.com>
> ---
>  include/linux/lsm_hooks.h           |  2 +
>  include/linux/security.h            |  7 +++
>  security/security.c                 |  6 +++
>  security/selinux/hooks.c            | 12 ++++-
>  security/selinux/include/security.h |  2 +
>  security/selinux/ss/policydb.h      |  4 ++
>  security/selinux/ss/services.c      | 91
> +++++++++++++++++++++++++++++++++++++
>  7 files changed, 123 insertions(+), 1 deletion(-)
> 

> diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
> index a4d36f8..3759198 100644
> --- a/security/selinux/hooks.c
> +++ b/security/selinux/hooks.c
> @@ -173,8 +173,11 @@ static int selinux_netcache_avc_callback(u32
> event)
>  
>  static int selinux_lsm_notifier_avc_callback(u32 event)
>  {
> -	if (event == AVC_CALLBACK_RESET)
> +	if (event == AVC_CALLBACK_RESET) {
> +		if (security_policydb_compute_cksum() != 0)
> +			printk(KERN_ERR "Failed to compute policydb
> cksum\n");

This seems like an odd place to trigger the computation. Why aren't you
just computing it when the policy is loaded directly in
security_load_policy()?  You already have the (data, len) on entry to
that function.  Just compute it at load time, save it, and be done.  No
need for a notifier then for your use case unless I am missing
something.

I suppose the question is which checksum do you want - the hash of the
policy file that was written to /sys/fs/selinux/load by userspace, or
the hash of the policy file that the kernel generates on demand if you
open /sys/fs/selinux/policy.  Those can differ in non-semantic ways due
to ordering differences, for example.  I think the former is more
likely to be of interest to userspace (e.g. to compare the hash value
against the hash of the policy file), and is cheaper since you already
have a (data, len) pair on entry to security_load_policy() that you can
hash immediately rather than requiring the kernel to regenerate the
image from the policydb.

>  		call_lsm_notifier(LSM_POLICY_CHANGE, NULL);
> +	}
>  
>  	return 0;
>  }
> 

> diff --git a/security/selinux/ss/services.c
> b/security/selinux/ss/services.c
> index 60d9b02..a35d294 100644
> --- a/security/selinux/ss/services.c
> +++ b/security/selinux/ss/services.c
> @@ -53,6 +53,8 @@
>  #include <linux/flex_array.h>
>  #include <linux/vmalloc.h>
>  #include <net/netlabel.h>
> +#include <crypto/hash.h>
> +#include <crypto/sha.h>
>  
>  #include "flask.h"
>  #include "avc.h"
> @@ -2170,6 +2172,95 @@ size_t security_policydb_len(void)
>  }
>  
>  /**
> + * security_policydb_cksum - Get policydb checksum.
> + * @cksum: string to store checksum to
> + * @len: length of checksum
> + */
> +ssize_t security_policydb_cksum(char *cksum, size_t len)
> +{
> +	int rc;
> +
> +	read_lock(&policy_rwlock);
> +	if (strlcpy(cksum, policydb.policycksum, len) >= len)
> +		rc = -ENAMETOOLONG;
> +	rc = policydb.policycksum_len;

Obviously you'll clobber -ENAMETOOLONG here.

> +	read_unlock(&policy_rwlock);
> +
> +	return rc;
> +}

You are requiring all callers to know that they are dealing with a
sha256 hash string in order to provide an adequately sized buffer.
So we either ought to make that evident in the interface, or make the
interface more flexible/general.  The latter is imho preferable.  We
could simply allocate a buffer of the right length and return it, like
selinux_inode_getsecurity() does.

> +
> +/**
> + * security_policydb_compute_cksum - Compute checksum of a policy
> database.
> + */
> +int security_policydb_compute_cksum(void)
> +{
> +	struct crypto_ahash *tfm;
> +	struct ahash_request *req;
> +	struct scatterlist sl;
> +	char hashval[SHA256_DIGEST_SIZE];
> +	int idx;
> +	unsigned char *p;
> +	size_t len;
> +	void *data;
> +	int rc;
> +
> +	rc = security_read_policy(&data, &len);
> +	if (rc) {
> +		printk(KERN_ERR "Failed to read security policy\n");
> +		return rc;
> +	}

This requires regenerating the policy image from the policydb; simpler
if we can just hash what we were given in security_load_policy() and
save it at that time.

> +
> +	tfm = crypto_alloc_ahash("sha256", 0, CRYPTO_ALG_ASYNC);

Why are you using the async interface?

> +	if (IS_ERR(tfm)) {
> +		printk(KERN_ERR "Failed to alloc crypto hash
> sha256\n");
> +		vfree(data);
> +		rc = PTR_ERR(tfm);
> +		return rc;
> +	}
> +
> +	req = ahash_request_alloc(tfm, GFP_KERNEL);
> +	if (!req) {
> +		printk(KERN_ERR "Failed to alloc ahash_request for
> sha256\n");
> +		crypto_free_ahash(tfm);
> +		vfree(data);
> +		rc = -ENOMEM;
> +		return rc;
> +	}
> +
> +	ahash_request_set_callback(req, 0, NULL, NULL);
> +
> +	rc = crypto_ahash_init(req);
> +	if (rc) {
> +		printk(KERN_ERR "Failed to init ahash\n");
> +		ahash_request_free(req);
> +		crypto_free_ahash(tfm);
> +		vfree(data);
> +		return rc;
> +	}
> +
> +	sg_init_one(&sl, (void *)data, len);
> +	ahash_request_set_crypt(req, &sl, hashval, sl.length);
> +	rc = crypto_ahash_digest(req);
> +
> +	crypto_free_ahash(tfm);
> +	ahash_request_free(req);
> +	vfree(data);
> +	if (rc) {
> +		printk(KERN_ERR "Failed to compute digest\n");
> +		return rc;
> +	}
> +
> +	p = policydb.policycksum;
> +	for (idx = 0; idx < SHA256_DIGEST_SIZE; idx++) {
> +		snprintf(p, 3, "%02x", (unsigned
> char)(hashval[idx]));
> +		p += 2;
> +	}
> +	policydb.policycksum_len = (size_t)(p -
> policydb.policycksum);
> +
> +	return 0;
> +}
> +
> +/**
>   * security_port_sid - Obtain the SID for a port.
>   * @protocol: protocol number
>   * @port: port number

Re: [PATCH 3/3] selinux: expose policy SHA256 checksum via selinuxfs

[Stephen Smalley]

On Thu, 2017-04-27 at 00:02 +0900, Sebastien Buisson wrote:
> Expose policy SHA256 checksum via selinuxfs.
> 
> Signed-off-by: Sebastien Buisson <sbuisson@ddn.com>
> ---
>  security/selinux/selinuxfs.c | 20 ++++++++++++++++++++
>  1 file changed, 20 insertions(+)
> 
> diff --git a/security/selinux/selinuxfs.c
> b/security/selinux/selinuxfs.c
> index ce71718..b2d5deb 100644
> --- a/security/selinux/selinuxfs.c
> +++ b/security/selinux/selinuxfs.c
> @@ -30,6 +30,7 @@
>  #include <linux/uaccess.h>
>  #include <linux/kobject.h>
>  #include <linux/ctype.h>
> +#include <crypto/sha.h>
>  
>  /* selinuxfs pseudo filesystem for exporting the security policy
> API.
>     Based on the proc code and the fs/nfsd/nfsctl.c code. */
> @@ -99,6 +100,7 @@ enum sel_inos {
>  	SEL_STATUS,	/* export current status using mmap() */
>  	SEL_POLICY,	/* allow userspace to read the in kernel
> policy */
>  	SEL_VALIDATE_TRANS, /* compute validatetrans decision */
> +	SEL_POLICYCKSUM,/* return policy SHA256 checkum */
>  	SEL_INO_NEXT,	/* The next inode number to use */
>  };
>  
> @@ -313,6 +315,22 @@ static ssize_t sel_read_policyvers(struct file
> *filp, char __user *buf,
>  	.llseek		= generic_file_llseek,
>  };
>  
> +static ssize_t sel_read_policycksum(struct file *filp, char __user
> *buf,
> +				    size_t count, loff_t *ppos)
> +{
> +	size_t tmpbuflen = SHA256_DIGEST_SIZE*2 + 1;
> +	char tmpbuf[tmpbuflen];
> +	ssize_t length;
> +
> +	length = security_policydb_cksum(tmpbuf, tmpbuflen);
> +	return simple_read_from_buffer(buf, count, ppos, tmpbuf,
> length);
> +}

Should we also include information about the hash used, in case it
changes in the future?

> +
> +static const struct file_operations sel_policycksum_ops = {
> +	.read		= sel_read_policycksum,
> +	.llseek		= generic_file_llseek,
> +};
> +
>  /* declaration for sel_write_load */
>  static int sel_make_bools(void);
>  static int sel_make_classes(void);
> @@ -1825,6 +1843,8 @@ static int sel_fill_super(struct super_block
> *sb, void *data, int silent)
>  		[SEL_POLICY] = {"policy", &sel_policy_ops, S_IRUGO},
>  		[SEL_VALIDATE_TRANS] = {"validatetrans",
> &sel_transition_ops,
>  					S_IWUGO},
> +		[SEL_POLICYCKSUM] = {"policycksum",
> &sel_policycksum_ops,
> +				     S_IRUGO},
>  		/* last one */ {""}
>  	};
>  	ret = simple_fill_super(sb, SELINUX_MAGIC, selinux_files);

Re: [PATCH 3/3] selinux: expose policy SHA256 checksum via selinuxfs

[James Morris]

On Wed, 26 Apr 2017, Stephen Smalley wrote:

> > +	return simple_read_from_buffer(buf, count, ppos, tmpbuf,
> > length);
> > +}
> 
> Should we also include information about the hash used, in case it
> changes in the future?
> 

Good idea, yes.


-- 
James Morris
<jmorris@namei.org>

Re: [PATCH 2/3] selinux: add checksum to policydb

[Sebastien Buisson]

2017-04-26 20:30 GMT+02:00 Stephen Smalley <sds@tycho.nsa.gov>:
> This seems like an odd place to trigger the computation.

I noticed that the policy as exposed via /sys/fs/selinux/policy can
also be modified in security_set_bools(). So in order to limit the
places from where to compute the policy checksum, I moved the call to
checksum computation to selinux_lsm_notifier_avc_callback().
That being said, maybe the hash of /sys/fs/selinux/policy is not the
checksum we want. See your comments and my answers below.

> Why aren't you
> just computing it when the policy is loaded directly in
> security_load_policy()?  You already have the (data, len) on entry to
> that function.  Just compute it at load time, save it, and be done.  No
> need for a notifier then for your use case unless I am missing
> something.

You are right. Getting from the Lustre client code the SELinux
internally computed checksum is cheap, so no need to be notified every
time the policy changes, and no need to store the checksum in Lustre
at that time.
I will drop the "Implement LSM notification system" patch from this
series, as I cannot justify its usefulness from a Lustre client
standpoint anymore.

> I suppose the question is which checksum do you want - the hash of the
> policy file that was written to /sys/fs/selinux/load by userspace, or
> the hash of the policy file that the kernel generates on demand if you
> open /sys/fs/selinux/policy.  Those can differ in non-semantic ways due
> to ordering differences, for example.  I think the former is more
> likely to be of interest to userspace (e.g. to compare the hash value
> against the hash of the policy file), and is cheaper since you already
> have a (data, len) pair on entry to security_load_policy() that you can
> hash immediately rather than requiring the kernel to regenerate the
> image from the policydb.

OK, I understand now why I was seeing differences between the checksum
computed on a (data, len) pair on entry to security_load_policy(), and
the checksum computed on a (data, len) pair got from
security_read_policy().
I thought it was a problem to have a difference between the internally
computed checksum and the one a user can get by calling sha256sum on
/sys/fs/selinux/policy. But now I see it makes sense to reflect what
was loaded by userspace. So I will simplify this patch accordingly.

Re: [PATCH 2/3] selinux: add checksum to policydb

[Stephen Smalley]

On Thu, 2017-04-27 at 10:41 +0200, Sebastien Buisson wrote:
> 2017-04-26 20:30 GMT+02:00 Stephen Smalley <sds@tycho.nsa.gov>:
> > This seems like an odd place to trigger the computation.
> 
> I noticed that the policy as exposed via /sys/fs/selinux/policy can
> also be modified in security_set_bools().

That's true, but does that matter for your use case?  Do you care about
non-persistent boolean changes? What is the property you want to
ensure?

>  So in order to limit the
> places from where to compute the policy checksum, I moved the call to
> checksum computation to selinux_lsm_notifier_avc_callback().
> That being said, maybe the hash of /sys/fs/selinux/policy is not the
> checksum we want. See your comments and my answers below.
> 
> > Why aren't you
> > just computing it when the policy is loaded directly in
> > security_load_policy()?  You already have the (data, len) on entry
> > to
> > that function.  Just compute it at load time, save it, and be
> > done.  No
> > need for a notifier then for your use case unless I am missing
> > something.
> 
> You are right. Getting from the Lustre client code the SELinux
> internally computed checksum is cheap, so no need to be notified
> every
> time the policy changes, and no need to store the checksum in Lustre
> at that time.
> I will drop the "Implement LSM notification system" patch from this
> series, as I cannot justify its usefulness from a Lustre client
> standpoint anymore.
> 
> > I suppose the question is which checksum do you want - the hash of
> > the
> > policy file that was written to /sys/fs/selinux/load by userspace,
> > or
> > the hash of the policy file that the kernel generates on demand if
> > you
> > open /sys/fs/selinux/policy.  Those can differ in non-semantic ways
> > due
> > to ordering differences, for example.  I think the former is more
> > likely to be of interest to userspace (e.g. to compare the hash
> > value
> > against the hash of the policy file), and is cheaper since you
> > already
> > have a (data, len) pair on entry to security_load_policy() that you
> > can
> > hash immediately rather than requiring the kernel to regenerate the
> > image from the policydb.
> 
> OK, I understand now why I was seeing differences between the
> checksum
> computed on a (data, len) pair on entry to security_load_policy(),
> and
> the checksum computed on a (data, len) pair got from
> security_read_policy().
> I thought it was a problem to have a difference between the
> internally
> computed checksum and the one a user can get by calling sha256sum on
> /sys/fs/selinux/policy. But now I see it makes sense to reflect what
> was loaded by userspace. So I will simplify this patch accordingly.

Ok, that should work as long as you just want to validate that all the
clients loaded the same policy file, and aren't concerned about non-
persistent policy boolean changes.

You needed to get (global) enforcing mode too, didn't you?  That's
separate from the policy.

Make sure you make the hash algorithm explicit in both what is returned
by the hook to lustre and by what is exported via selinuxfs.  Can
likely just encode the hash algorithm name in the string when you
generate it.

Re: [PATCH 2/3] selinux: add checksum to policydb

[Sebastien Buisson]

2017-04-27 17:18 GMT+02:00 Stephen Smalley <sds@tycho.nsa.gov>:
> Ok, that should work as long as you just want to validate that all the
> clients loaded the same policy file, and aren't concerned about non-
> persistent policy boolean changes.

As far as I understand, non-persistent policy boolean changes can
affect the way the policy is enforced. So that is a problem if the
checksum does not reflect it. We want to protect against someone
tampering the policy locally on a Lustre client, even if it does not
survive a reboot.
I just checked, with the method of computing the checksum on a (data,
len) pair on entry to security_load_policy() the checksum does not
change after using setsebool. So it seems I would need to call
security_read_policy() to retrieve the binary representation of the
policy as currently enforced by the kernel. Unless you can see another
way?

> You needed to get (global) enforcing mode too, didn't you?  That's
> separate from the policy.

Exactly, I also need to rework the patch I proposed about this, in
light of the comments I received.

> Make sure you make the hash algorithm explicit in both what is returned
> by the hook to lustre and by what is exported via selinuxfs.  Can
> likely just encode the hash algorithm name in the string when you
> generate it.

Sure, I will add "sha256:" at the beginning of the string.

Re: [PATCH 2/3] selinux: add checksum to policydb

[Stephen Smalley]

On Thu, 2017-04-27 at 19:12 +0200, Sebastien Buisson wrote:
> 2017-04-27 17:18 GMT+02:00 Stephen Smalley <sds@tycho.nsa.gov>:
> > Ok, that should work as long as you just want to validate that all
> > the
> > clients loaded the same policy file, and aren't concerned about
> > non-
> > persistent policy boolean changes.
> 
> As far as I understand, non-persistent policy boolean changes can
> affect the way the policy is enforced. So that is a problem if the
> checksum does not reflect it. We want to protect against someone
> tampering the policy locally on a Lustre client, even if it does not
> survive a reboot.

A boolean change can affect which TE rules in the policy are
enabled/disabled, but only in ways that are defined by the original
policy.  You can't add arbitrary TE rules that way, just enable/disable
blocks that were defined conditionally in the policy.  It also has no
effect on MLS enforcement, for example.  So it depends on your goals.

> I just checked, with the method of computing the checksum on a (data,
> len) pair on entry to security_load_policy() the checksum does not
> change after using setsebool. So it seems I would need to call
> security_read_policy() to retrieve the binary representation of the
> policy as currently enforced by the kernel. Unless you can see
> another
> way?

I don't think that's a viable option, since security_read_policy() is
going to be expensive in order to generate a full policy image, while
security_set_bools() is supposed to be substantially cheaper than a
full policy load.

Also, the advantage of taking the hash of the original input file is
that you can independently compute a reference hash offline or on the
server from the same policy file and compare them and you can identify
which policy file was loaded based on the hash.

If you care about the active boolean state, then I'd suggest hashing
the active boolean state separately and storing that after the policy
hash.  You can do that in both security_load_policy() and
security_set_bools().  Just iterate through the bools like
security_set_bools() does, write the ->state of each boolean into a
buffer, and then hash that buffer.

> > You needed to get (global) enforcing mode too, didn't you?  That's
> > separate from the policy.
> 
> Exactly, I also need to rework the patch I proposed about this, in
> light of the comments I received.

So perhaps what you really want is a hook interface and a selinuxfs
interface that returns a single string that encodes all of the policy
properties that you care about?  Rather than separate hooks and
interfaces?  You could embed the enforcing status in the string too. 
Should probably include checkreqprot as well since that affects
enforcement of mmap/mprotect checks.

> > Make sure you make the hash algorithm explicit in both what is
> > returned
> > by the hook to lustre and by what is exported via selinuxfs.  Can
> > likely just encode the hash algorithm name in the string when you
> > generate it.
> 
> Sure, I will add "sha256:" at the beginning of the string.

Re: [PATCH 2/3] selinux: add checksum to policydb

[Sebastien Buisson]

2017-04-27 20:47 GMT+02:00 Stephen Smalley <sds@tycho.nsa.gov>:
>> I just checked, with the method of computing the checksum on a (data,
>> len) pair on entry to security_load_policy() the checksum does not
>> change after using setsebool. So it seems I would need to call
>> security_read_policy() to retrieve the binary representation of the
>> policy as currently enforced by the kernel. Unless you can see
>> another
>> way?
>
> I don't think that's a viable option, since security_read_policy() is
> going to be expensive in order to generate a full policy image, while
> security_set_bools() is supposed to be substantially cheaper than a
> full policy load.
>
> Also, the advantage of taking the hash of the original input file is
> that you can independently compute a reference hash offline or on the
> server from the same policy file and compare them and you can identify
> which policy file was loaded based on the hash.
>
> If you care about the active boolean state, then I'd suggest hashing
> the active boolean state separately and storing that after the policy
> hash.  You can do that in both security_load_policy() and
> security_set_bools().  Just iterate through the bools like
> security_set_bools() does, write the ->state of each boolean into a
> buffer, and then hash that buffer.

I just noticed another issue: with the method of computing the
checksum on a (data, len) pair on entry to security_load_policy(), the
checksum does not change after inserting a new module with semodule.
It is a problem as a module can allow actions by certain users on some
file contexts. So not detecting that kind of policy tampering defeats
the purpose of the checksum as I imagine it.

To address this I propose to come back to the idea of the notifier.
The checksum would not be stored inside the struct policydb. The
checksum would be computed on a (data, len) pair got from
security_read_policy() every time someone is asking for it through the
security_policy_cksum() hook. The ones that would potentially call
security_policy_cksum() are those that would register a callback on
lsm_notifier, and the userspace processes reading
/sys/fs/selinux/policycksum. So no matter if computing the checksum
gets expensive, that would be the caller's responsibility to use it
with care. Just like with /sys/fs/selinux/policy today in fact.

>> > You needed to get (global) enforcing mode too, didn't you?  That's
>> > separate from the policy.
>>
>> Exactly, I also need to rework the patch I proposed about this, in
>> light of the comments I received.
>
> So perhaps what you really want is a hook interface and a selinuxfs
> interface that returns a single string that encodes all of the policy
> properties that you care about?  Rather than separate hooks and
> interfaces?  You could embed the enforcing status in the string too.
> Should probably include checkreqprot as well since that affects
> enforcement of mmap/mprotect checks.

True, I should build a string of the form:
<0 or 1 for enforce>:<0 or 1 for checkreqprot>:<hashalg>=<global checksum>
I should probably rename it 'policybrief' instead of 'policycksum'.

I realize that the 'SELinux user to UNIX user' assignments are
important as well. If for instance a regular user on a given cluster
node is mapped to unconfined_u instead of user_u, this user would
erroneously have major privileges. I do not know where I should look
for this information, and possibly compute another checksum.

Re: [PATCH 2/3] selinux: add checksum to policydb

[Stephen Smalley]

On Fri, 2017-04-28 at 17:16 +0200, Sebastien Buisson wrote:
> 2017-04-27 20:47 GMT+02:00 Stephen Smalley <sds@tycho.nsa.gov>:
> > > I just checked, with the method of computing the checksum on a
> > > (data,
> > > len) pair on entry to security_load_policy() the checksum does
> > > not
> > > change after using setsebool. So it seems I would need to call
> > > security_read_policy() to retrieve the binary representation of
> > > the
> > > policy as currently enforced by the kernel. Unless you can see
> > > another
> > > way?
> > 
> > I don't think that's a viable option, since security_read_policy()
> > is
> > going to be expensive in order to generate a full policy image,
> > while
> > security_set_bools() is supposed to be substantially cheaper than a
> > full policy load.
> > 
> > Also, the advantage of taking the hash of the original input file
> > is
> > that you can independently compute a reference hash offline or on
> > the
> > server from the same policy file and compare them and you can
> > identify
> > which policy file was loaded based on the hash.
> > 
> > If you care about the active boolean state, then I'd suggest
> > hashing
> > the active boolean state separately and storing that after the
> > policy
> > hash.  You can do that in both security_load_policy() and
> > security_set_bools().  Just iterate through the bools like
> > security_set_bools() does, write the ->state of each boolean into a
> > buffer, and then hash that buffer.
> 
> I just noticed another issue: with the method of computing the
> checksum on a (data, len) pair on entry to security_load_policy(),
> the
> checksum does not change after inserting a new module with semodule.
> It is a problem as a module can allow actions by certain users on
> some
> file contexts. So not detecting that kind of policy tampering defeats
> the purpose of the checksum as I imagine it.

You seem to be conflating kernel policy with userspace policy. 
security_load_policy() is provided with the kernel policy image, which
is the result of linking the kernel-relevant portions of all policy
modules together. A hash of that image will change if you insert a
policy module that affects the kernel policy in any way.  But a change
that only affects userspace policy isn't ever going to be reflected in
the kernel.  It doesn't matter where or when you compute your checksum
within the kernel; it isn't ever going to reflect those userspace
policy changes.

> To address this I propose to come back to the idea of the notifier.
> The checksum would not be stored inside the struct policydb. The
> checksum would be computed on a (data, len) pair got from
> security_read_policy() every time someone is asking for it through
> the
> security_policy_cksum() hook. The ones that would potentially call
> security_policy_cksum() are those that would register a callback on
> lsm_notifier, and the userspace processes reading
> /sys/fs/selinux/policycksum. So no matter if computing the checksum
> gets expensive, that would be the caller's responsibility to use it
> with care. Just like with /sys/fs/selinux/policy today in fact.

This won't detect changes to userspace policy configurations either,
and it is less efficient than just computing/updating the checksum in
security_load_policy() and security_set_bools().  Also, if all you want
is a hash of /sys/fs/selinux/policy, then userspace can already read
and hash that itself at any time.  You aren't really providing any
additional information that way.  In contrast, saving and providing a
hash of the policy image that was loaded is not something that is
currently available, and could be useful in checking against a
reference hash of the policy file or in identifying which policy file
was loaded.

> > > > You needed to get (global) enforcing mode too, didn't
> > > > you?  That's
> > > > separate from the policy.
> > > 
> > > Exactly, I also need to rework the patch I proposed about this,
> > > in
> > > light of the comments I received.
> > 
> > So perhaps what you really want is a hook interface and a selinuxfs
> > interface that returns a single string that encodes all of the
> > policy
> > properties that you care about?  Rather than separate hooks and
> > interfaces?  You could embed the enforcing status in the string
> > too.
> > Should probably include checkreqprot as well since that affects
> > enforcement of mmap/mprotect checks.
> 
> True, I should build a string of the form:
> <0 or 1 for enforce>:<0 or 1 for checkreqprot>:<hashalg>=<global
> checksum>
> I should probably rename it 'policybrief' instead of 'policycksum'.
>
> I realize that the 'SELinux user to UNIX user' assignments are
> important as well. If for instance a regular user on a given cluster
> node is mapped to unconfined_u instead of user_u, this user would
> erroneously have major privileges. I do not know where I should look
> for this information, and possibly compute another checksum.

As above, that's userspace policy configuration, and not something that
kernel can or should deal with.

Re: [PATCH 2/3] selinux: add checksum to policydb

[Sebastien Buisson]

2017-04-28 17:50 GMT+02:00 Stephen Smalley <sds@tycho.nsa.gov>:
> You seem to be conflating kernel policy with userspace policy.
> security_load_policy() is provided with the kernel policy image, which
> is the result of linking the kernel-relevant portions of all policy
> modules together. A hash of that image will change if you insert a
> policy module that affects the kernel policy in any way.  But a change
> that only affects userspace policy isn't ever going to be reflected in
> the kernel.  It doesn't matter where or when you compute your checksum
> within the kernel; it isn't ever going to reflect those userspace
> policy changes.

Here is the content of the module is used for my tests:

#============= user_t ==============
allow user_t mnt_t:dir { write add_name };
allow user_t mnt_t:file { write create };

After loading the .pp corresponding to it, I can see that with the
method of computing the checksum on the (data, len) pair on entry to
security_load_policy(), the checksum does not change. However, when
using the (data, len) pair got from
security_read_policy(), the checksum changes. And when I remove the
module, the checksum is back to its previous value.
So this is what makes me think there is a difference. Am I missing something?

Re: [PATCH 2/3] selinux: add checksum to policydb

[Stephen Smalley]

On Fri, 2017-04-28 at 18:08 +0200, Sebastien Buisson wrote:
> 2017-04-28 17:50 GMT+02:00 Stephen Smalley <sds@tycho.nsa.gov>:
> > You seem to be conflating kernel policy with userspace policy.
> > security_load_policy() is provided with the kernel policy image,
> > which
> > is the result of linking the kernel-relevant portions of all policy
> > modules together. A hash of that image will change if you insert a
> > policy module that affects the kernel policy in any way.  But a
> > change
> > that only affects userspace policy isn't ever going to be reflected
> > in
> > the kernel.  It doesn't matter where or when you compute your
> > checksum
> > within the kernel; it isn't ever going to reflect those userspace
> > policy changes.
> 
> Here is the content of the module is used for my tests:
> 
> #============= user_t ==============
> allow user_t mnt_t:dir { write add_name };
> allow user_t mnt_t:file { write create };
> 
> After loading the .pp corresponding to it, I can see that with the
> method of computing the checksum on the (data, len) pair on entry to
> security_load_policy(), the checksum does not change. However, when
> using the (data, len) pair got from
> security_read_policy(), the checksum changes. And when I remove the
> module, the checksum is back to its previous value.
> So this is what makes me think there is a difference. Am I missing
> something?

Policy is loaded via security_load_policy(), so the policy image has to
go through it in the first place to be loaded (ignoring kernel exploits
or direct /dev/mem access).  You couldn't have loaded the modified
policy with your new rules without the modified policy getting
processed by security_load_policy().  So I'm assuming there is a bug in
your code or your testing.