[PATCH v5 1/2] selinux: add brief info to policydb

[PATCH v5 1/2] selinux: add brief info to policydb

[Sebastien Buisson]

Add policybrief field to struct policydb. It holds a brief info
of the policydb, made of colon separated name and value pairs
that give information about how the policy is applied in the
security module(s).
Note that the ordering of the fields in the string may change.

Policy brief is computed every time the policy is loaded, and when
enforce or checkreqprot are changed.

Add security_policy_brief hook to give access to policy brief to
the rest of the kernel. It is useful for any network or
distributed file system that cares about how SELinux is enforced
on its client nodes. This information is used to detect changes
to the policy on file system client nodes, and can be forwarded
to file system server nodes. Depending on how the policy is
enforced on client side, server can refuse connection.

Signed-off-by: Sebastien Buisson <sbuisson@ddn.com>
---
 include/linux/lsm_hooks.h           | 20 +++++++++
 include/linux/security.h            |  7 +++
 security/security.c                 |  8 ++++
 security/selinux/hooks.c            |  7 +++
 security/selinux/include/security.h |  2 +
 security/selinux/selinuxfs.c        |  2 +
 security/selinux/ss/policydb.c      | 85 +++++++++++++++++++++++++++++++++++++
 security/selinux/ss/policydb.h      |  3 ++
 security/selinux/ss/services.c      | 67 +++++++++++++++++++++++++++++
 9 files changed, 201 insertions(+)

diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h
index 1aa6333..e4dc2b4 100644
--- a/include/linux/lsm_hooks.h
+++ b/include/linux/lsm_hooks.h
@@ -1331,6 +1331,24 @@
  *	@inode we wish to get the security context of.
  *	@ctx is a pointer in which to place the allocated security context.
  *	@ctxlen points to the place to put the length of @ctx.
+ *
+ * Security hooks for policy brief
+ *
+ * @policy_brief:
+ *
+ *	Returns a string containing a brief info of the policydb. The string
+ *	contains colon separated name and value pairs that give information
+ *	about how the policy is applied in the security module(s).
+ *	Note that the ordering of the fields in the string may change.
+ *
+ *	@brief: pointer to buffer holding brief
+ *	@len: in: brief buffer length if no alloc, out: brief string len
+ *	@alloc: whether to allocate buffer for brief or not
+ *	If @alloc, *brief must be kfreed by caller.
+ *	If not @alloc, caller must pass a buffer that can hold policy brief
+ *	info (including terminating NUL).
+ *	On success 0 is returned , or negative value on error.
+ *
  * This is the main security structure.
  */
 
@@ -1562,6 +1580,7 @@
 	int (*inode_setsecctx)(struct dentry *dentry, void *ctx, u32 ctxlen);
 	int (*inode_getsecctx)(struct inode *inode, void **ctx, u32 *ctxlen);
 
+	int (*policy_brief)(char **brief, size_t *len, bool alloc);
 #ifdef CONFIG_SECURITY_NETWORK
 	int (*unix_stream_connect)(struct sock *sock, struct sock *other,
 					struct sock *newsk);
@@ -1806,6 +1825,7 @@ struct security_hook_heads {
 	struct list_head inode_notifysecctx;
 	struct list_head inode_setsecctx;
 	struct list_head inode_getsecctx;
+	struct list_head policy_brief;
 #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 97df7ba..35ac97f 100644
--- a/include/linux/security.h
+++ b/include/linux/security.h
@@ -376,6 +376,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);
+
+int security_policy_brief(char **brief, size_t *len, bool alloc);
 #else /* CONFIG_SECURITY */
 struct security_mnt_opts {
 };
@@ -1159,6 +1161,11 @@ static inline int security_inode_getsecctx(struct inode *inode, void **ctx, u32
 {
 	return -EOPNOTSUPP;
 }
+
+static inline int security_policy_brief(char **brief, size_t *len, bool alloc)
+{
+	return -EOPNOTSUPP;
+}
 #endif	/* CONFIG_SECURITY */
 
 #ifdef CONFIG_SECURITY_NETWORK
diff --git a/security/security.c b/security/security.c
index d6d18a3..46052d3 100644
--- a/security/security.c
+++ b/security/security.c
@@ -1269,6 +1269,12 @@ int security_inode_getsecctx(struct inode *inode, void **ctx, u32 *ctxlen)
 }
 EXPORT_SYMBOL(security_inode_getsecctx);
 
+int security_policy_brief(char **brief, size_t *len, bool alloc)
+{
+	return call_int_hook(policy_brief, -EOPNOTSUPP, brief, len, alloc);
+}
+EXPORT_SYMBOL(security_policy_brief);
+
 #ifdef CONFIG_SECURITY_NETWORK
 
 int security_unix_stream_connect(struct sock *sock, struct sock *other, struct sock *newsk)
@@ -1868,6 +1874,8 @@ struct security_hook_heads security_hook_heads __lsm_ro_after_init = {
 		LIST_HEAD_INIT(security_hook_heads.inode_setsecctx),
 	.inode_getsecctx =
 		LIST_HEAD_INIT(security_hook_heads.inode_getsecctx),
+	.policy_brief =
+		LIST_HEAD_INIT(security_hook_heads.policy_brief),
 #ifdef CONFIG_SECURITY_NETWORK
 	.unix_stream_connect =
 		LIST_HEAD_INIT(security_hook_heads.unix_stream_connect),
diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
index e67a526..da245e8 100644
--- a/security/selinux/hooks.c
+++ b/security/selinux/hooks.c
@@ -6063,6 +6063,11 @@ static int selinux_inode_getsecctx(struct inode *inode, void **ctx, u32 *ctxlen)
 	*ctxlen = len;
 	return 0;
 }
+
+static int selinux_policy_brief(char **brief, size_t *len, bool alloc)
+{
+	return security_policydb_brief(brief, len, alloc);
+}
 #ifdef CONFIG_KEYS
 
 static int selinux_key_alloc(struct key *k, const struct cred *cred,
@@ -6277,6 +6282,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_brief, selinux_policy_brief),
+
 	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..a0d4d7d 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);
+int security_policydb_brief(char **brief, size_t *len, bool alloc);
+void security_policydb_update_info(u32 requested);
 
 int security_policycap_supported(unsigned int req_cap);
 
diff --git a/security/selinux/selinuxfs.c b/security/selinux/selinuxfs.c
index ce71718..e8fe914 100644
--- a/security/selinux/selinuxfs.c
+++ b/security/selinux/selinuxfs.c
@@ -159,6 +159,7 @@ static ssize_t sel_write_enforce(struct file *file, const char __user *buf,
 			from_kuid(&init_user_ns, audit_get_loginuid(current)),
 			audit_get_sessionid(current));
 		selinux_enforcing = new_value;
+		security_policydb_update_info(SECURITY__SETENFORCE);
 		if (selinux_enforcing)
 			avc_ss_reset(0);
 		selnl_notify_setenforce(selinux_enforcing);
@@ -621,6 +622,7 @@ static ssize_t sel_write_checkreqprot(struct file *file, const char __user *buf,
 		goto out;
 
 	selinux_checkreqprot = new_value ? 1 : 0;
+	security_policydb_update_info(SECURITY__SETCHECKREQPROT);
 	length = count;
 out:
 	kfree(page);
diff --git a/security/selinux/ss/policydb.c b/security/selinux/ss/policydb.c
index 0080122..83309a4 100644
--- a/security/selinux/ss/policydb.c
+++ b/security/selinux/ss/policydb.c
@@ -32,13 +32,20 @@
 #include <linux/errno.h>
 #include <linux/audit.h>
 #include <linux/flex_array.h>
+#include <crypto/hash.h>
 #include "security.h"
 
 #include "policydb.h"
 #include "conditional.h"
 #include "mls.h"
+#include "objsec.h"
 #include "services.h"
 
+static unsigned int policybrief_hash_size;
+static struct crypto_shash *policybrief_tfm;
+static const char policybrief_hash_alg[] = "sha256";
+unsigned int policybrief_len;
+
 #define _DEBUG_HASHES
 
 #ifdef DEBUG_HASHES
@@ -879,6 +886,8 @@ void policydb_destroy(struct policydb *p)
 	ebitmap_destroy(&p->filename_trans_ttypes);
 	ebitmap_destroy(&p->policycaps);
 	ebitmap_destroy(&p->permissive_map);
+
+	kfree(p->policybrief);
 }
 
 /*
@@ -2220,6 +2229,49 @@ static int ocontext_read(struct policydb *p, struct policydb_compat_info *info,
 }
 
 /*
+ * Compute summary of a policy database binary representation file,
+ * and store it into a policy database structure.
+ */
+static int policydb_brief(struct policydb *policydb, void *ptr)
+{
+	SHASH_DESC_ON_STACK(desc, policybrief_tfm);
+	struct policy_file *fp = ptr;
+	u8 *hashval;
+	int rc = 0;
+
+	BUG_ON(policydb->policybrief);
+
+	hashval = kmalloc(policybrief_hash_size, GFP_KERNEL);
+	if (hashval == NULL)
+		return -ENOMEM;
+
+	desc->tfm = policybrief_tfm;
+	desc->flags = 0;
+	rc = crypto_shash_digest(desc, fp->data, fp->len, hashval);
+	if (rc) {
+		printk(KERN_ERR "Failed crypto_shash_digest: %d\n", rc);
+		goto out_free;
+	}
+
+	/* policy brief is in the form:
+	 * selinux(enforce=<0 or 1>:checkreqprot=<0 or 1>:<hashalg>=<checksum>)
+	 */
+	policydb->policybrief = kasprintf(GFP_KERNEL,
+					  "selinux(enforce=%d:checkreqprot=%d:%s=%*phN)",
+					  selinux_enforcing,
+					  selinux_checkreqprot,
+					  policybrief_hash_alg,
+					  policybrief_hash_size, hashval);
+	if (policydb->policybrief == NULL)
+		rc = -ENOMEM;
+
+out_free:
+	kfree(hashval);
+
+	return rc;
+}
+
+/*
  * Read the configuration data from a policy database binary
  * representation file into a policy database structure.
  */
@@ -2238,6 +2290,11 @@ int policydb_read(struct policydb *p, void *fp)
 	if (rc)
 		return rc;
 
+	/* Compute summary of policy, and store it in policydb */
+	rc = policydb_brief(p, fp);
+	if (rc)
+		goto bad;
+
 	/* Read the magic number and string length. */
 	rc = next_entry(buf, fp, sizeof(u32) * 2);
 	if (rc)
@@ -3456,3 +3513,31 @@ int policydb_write(struct policydb *p, void *fp)
 
 	return 0;
 }
+
+static int __init init_policybrief_hash(void)
+{
+	struct crypto_shash *tfm;
+
+	if (!selinux_enabled)
+		return 0;
+
+	tfm = crypto_alloc_shash(policybrief_hash_alg, 0, 0);
+	if (IS_ERR(tfm)) {
+		printk(KERN_ERR "Failed to alloc crypto hash %s\n",
+		       policybrief_hash_alg);
+		return PTR_ERR(tfm);
+	}
+
+	policybrief_tfm = tfm;
+	policybrief_hash_size = crypto_shash_digestsize(policybrief_tfm);
+
+	/* policy brief is in the form:
+	 * selinux(enforce=<0 or 1>:checkreqprot=<0 or 1>:<hashalg>=<checksum>)
+	 */
+	policybrief_len = 35 + strlen(policybrief_hash_alg) +
+		2*policybrief_hash_size + 1;
+
+	return 0;
+}
+
+late_initcall(init_policybrief_hash);
diff --git a/security/selinux/ss/policydb.h b/security/selinux/ss/policydb.h
index 725d594..2e5048c 100644
--- a/security/selinux/ss/policydb.h
+++ b/security/selinux/ss/policydb.h
@@ -293,6 +293,8 @@ struct policydb {
 	size_t len;
 
 	unsigned int policyvers;
+	/* summary computed on the policy */
+	unsigned char *policybrief;
 
 	unsigned int reject_unknown : 1;
 	unsigned int allow_unknown : 1;
@@ -309,6 +311,7 @@ struct policydb {
 extern int policydb_role_isvalid(struct policydb *p, unsigned int role);
 extern int policydb_read(struct policydb *p, void *fp);
 extern int policydb_write(struct policydb *p, void *fp);
+extern unsigned int policybrief_len;
 
 #define PERM_SYMTAB_SIZE 32
 
diff --git a/security/selinux/ss/services.c b/security/selinux/ss/services.c
index 60d9b02..67eb80d 100644
--- a/security/selinux/ss/services.c
+++ b/security/selinux/ss/services.c
@@ -2170,6 +2170,73 @@ size_t security_policydb_len(void)
 }
 
 /**
+ * security_policydb_brief - Get policydb brief
+ * @brief: pointer to buffer holding brief
+ * @len: in: brief buffer length if no alloc, out: brief string len
+ * @alloc: whether to allocate buffer for brief or not
+ *
+ * If @alloc, *brief must be kfreed by caller.
+ * If not @alloc, caller must pass a buffer that can hold policybrief_len
+ * chars (including terminating NUL).
+ * On success 0 is returned , or negative value on error.
+ **/
+int security_policydb_brief(char **brief, size_t *len, bool alloc)
+{
+	if (!ss_initialized || brief == NULL)
+		return -EINVAL;
+
+	if (alloc) {
+		*brief = kzalloc(policybrief_len, GFP_KERNEL);
+	} else if (*len < policybrief_len) {
+		/* put in *len the string size we need to write */
+		*len = policybrief_len;
+		return -ENAMETOOLONG;
+	}
+
+	if (*brief == NULL)
+		return -ENOMEM;
+
+	read_lock(&policy_rwlock);
+	strcpy(*brief, policydb.policybrief);
+	/* *len is the length of the output string */
+	*len = policybrief_len - 1;
+	read_unlock(&policy_rwlock);
+
+	return 0;
+}
+
+void security_policydb_update_info(u32 requested)
+{
+	/* policy brief is in the form:
+	 * selinux(enforce=<0 or 1>:checkreqprot=<0 or 1>:<hashalg>=<checksum>)
+	 */
+	char enforce[] = "enforce=";
+	char checkreqprot[] = "checkreqprot=";
+	char *p, *str;
+	int val;
+
+	if (!ss_initialized)
+		return;
+
+	if (requested == SECURITY__SETENFORCE) {
+		str = enforce;
+		val = selinux_enforcing;
+	} else if (requested == SECURITY__SETCHECKREQPROT) {
+		str = checkreqprot;
+		val = selinux_checkreqprot;
+	}
+
+	/* update global policydb, needs write lock */
+	write_lock_irq(&policy_rwlock);
+	p = strstr(policydb.policybrief, str);
+	if (p) {
+		p += strlen(str);
+		*p = '0' + val;
+	}
+	write_unlock_irq(&policy_rwlock);
+}
+
+/**
  * security_port_sid - Obtain the SID for a port.
  * @protocol: protocol number
  * @port: port number
-- 
1.8.3.1

[PATCH v5 1/2] selinux: add brief info to policydb

[Sebastien Buisson]

Add policybrief field to struct policydb. It holds a brief info
of the policydb, made of colon separated name and value pairs
that give information about how the policy is applied in the
security module(s).
Note that the ordering of the fields in the string may change.

Policy brief is computed every time the policy is loaded, and when
enforce or checkreqprot are changed.

Add security_policy_brief hook to give access to policy brief to
the rest of the kernel. It is useful for any network or
distributed file system that cares about how SELinux is enforced
on its client nodes. This information is used to detect changes
to the policy on file system client nodes, and can be forwarded
to file system server nodes. Depending on how the policy is
enforced on client side, server can refuse connection.

Signed-off-by: Sebastien Buisson <sbuisson@ddn.com>
---
 include/linux/lsm_hooks.h           | 20 +++++++++
 include/linux/security.h            |  7 +++
 security/security.c                 |  8 ++++
 security/selinux/hooks.c            |  7 +++
 security/selinux/include/security.h |  2 +
 security/selinux/selinuxfs.c        |  2 +
 security/selinux/ss/policydb.c      | 85 +++++++++++++++++++++++++++++++++++++
 security/selinux/ss/policydb.h      |  3 ++
 security/selinux/ss/services.c      | 67 +++++++++++++++++++++++++++++
 9 files changed, 201 insertions(+)

diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h
index 1aa6333..e4dc2b4 100644
--- a/include/linux/lsm_hooks.h
+++ b/include/linux/lsm_hooks.h
@@ -1331,6 +1331,24 @@
  *	@inode we wish to get the security context of.
  *	@ctx is a pointer in which to place the allocated security context.
  *	@ctxlen points to the place to put the length of @ctx.
+ *
+ * Security hooks for policy brief
+ *
+ * @policy_brief:
+ *
+ *	Returns a string containing a brief info of the policydb. The string
+ *	contains colon separated name and value pairs that give information
+ *	about how the policy is applied in the security module(s).
+ *	Note that the ordering of the fields in the string may change.
+ *
+ *	@brief: pointer to buffer holding brief
+ *	@len: in: brief buffer length if no alloc, out: brief string len
+ *	@alloc: whether to allocate buffer for brief or not
+ *	If @alloc, *brief must be kfreed by caller.
+ *	If not @alloc, caller must pass a buffer that can hold policy brief
+ *	info (including terminating NUL).
+ *	On success 0 is returned , or negative value on error.
+ *
  * This is the main security structure.
  */
 
@@ -1562,6 +1580,7 @@
 	int (*inode_setsecctx)(struct dentry *dentry, void *ctx, u32 ctxlen);
 	int (*inode_getsecctx)(struct inode *inode, void **ctx, u32 *ctxlen);
 
+	int (*policy_brief)(char **brief, size_t *len, bool alloc);
 #ifdef CONFIG_SECURITY_NETWORK
 	int (*unix_stream_connect)(struct sock *sock, struct sock *other,
 					struct sock *newsk);
@@ -1806,6 +1825,7 @@ struct security_hook_heads {
 	struct list_head inode_notifysecctx;
 	struct list_head inode_setsecctx;
 	struct list_head inode_getsecctx;
+	struct list_head policy_brief;
 #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 97df7ba..35ac97f 100644
--- a/include/linux/security.h
+++ b/include/linux/security.h
@@ -376,6 +376,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);
+
+int security_policy_brief(char **brief, size_t *len, bool alloc);
 #else /* CONFIG_SECURITY */
 struct security_mnt_opts {
 };
@@ -1159,6 +1161,11 @@ static inline int security_inode_getsecctx(struct inode *inode, void **ctx, u32
 {
 	return -EOPNOTSUPP;
 }
+
+static inline int security_policy_brief(char **brief, size_t *len, bool alloc)
+{
+	return -EOPNOTSUPP;
+}
 #endif	/* CONFIG_SECURITY */
 
 #ifdef CONFIG_SECURITY_NETWORK
diff --git a/security/security.c b/security/security.c
index d6d18a3..46052d3 100644
--- a/security/security.c
+++ b/security/security.c
@@ -1269,6 +1269,12 @@ int security_inode_getsecctx(struct inode *inode, void **ctx, u32 *ctxlen)
 }
 EXPORT_SYMBOL(security_inode_getsecctx);
 
+int security_policy_brief(char **brief, size_t *len, bool alloc)
+{
+	return call_int_hook(policy_brief, -EOPNOTSUPP, brief, len, alloc);
+}
+EXPORT_SYMBOL(security_policy_brief);
+
 #ifdef CONFIG_SECURITY_NETWORK
 
 int security_unix_stream_connect(struct sock *sock, struct sock *other, struct sock *newsk)
@@ -1868,6 +1874,8 @@ struct security_hook_heads security_hook_heads __lsm_ro_after_init = {
 		LIST_HEAD_INIT(security_hook_heads.inode_setsecctx),
 	.inode_getsecctx =
 		LIST_HEAD_INIT(security_hook_heads.inode_getsecctx),
+	.policy_brief =
+		LIST_HEAD_INIT(security_hook_heads.policy_brief),
 #ifdef CONFIG_SECURITY_NETWORK
 	.unix_stream_connect =
 		LIST_HEAD_INIT(security_hook_heads.unix_stream_connect),
diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
index e67a526..da245e8 100644
--- a/security/selinux/hooks.c
+++ b/security/selinux/hooks.c
@@ -6063,6 +6063,11 @@ static int selinux_inode_getsecctx(struct inode *inode, void **ctx, u32 *ctxlen)
 	*ctxlen = len;
 	return 0;
 }
+
+static int selinux_policy_brief(char **brief, size_t *len, bool alloc)
+{
+	return security_policydb_brief(brief, len, alloc);
+}
 #ifdef CONFIG_KEYS
 
 static int selinux_key_alloc(struct key *k, const struct cred *cred,
@@ -6277,6 +6282,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_brief, selinux_policy_brief),
+
 	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..a0d4d7d 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);
+int security_policydb_brief(char **brief, size_t *len, bool alloc);
+void security_policydb_update_info(u32 requested);
 
 int security_policycap_supported(unsigned int req_cap);
 
diff --git a/security/selinux/selinuxfs.c b/security/selinux/selinuxfs.c
index ce71718..e8fe914 100644
--- a/security/selinux/selinuxfs.c
+++ b/security/selinux/selinuxfs.c
@@ -159,6 +159,7 @@ static ssize_t sel_write_enforce(struct file *file, const char __user *buf,
 			from_kuid(&init_user_ns, audit_get_loginuid(current)),
 			audit_get_sessionid(current));
 		selinux_enforcing = new_value;
+		security_policydb_update_info(SECURITY__SETENFORCE);
 		if (selinux_enforcing)
 			avc_ss_reset(0);
 		selnl_notify_setenforce(selinux_enforcing);
@@ -621,6 +622,7 @@ static ssize_t sel_write_checkreqprot(struct file *file, const char __user *buf,
 		goto out;
 
 	selinux_checkreqprot = new_value ? 1 : 0;
+	security_policydb_update_info(SECURITY__SETCHECKREQPROT);
 	length = count;
 out:
 	kfree(page);
diff --git a/security/selinux/ss/policydb.c b/security/selinux/ss/policydb.c
index 0080122..83309a4 100644
--- a/security/selinux/ss/policydb.c
+++ b/security/selinux/ss/policydb.c
@@ -32,13 +32,20 @@
 #include <linux/errno.h>
 #include <linux/audit.h>
 #include <linux/flex_array.h>
+#include <crypto/hash.h>
 #include "security.h"
 
 #include "policydb.h"
 #include "conditional.h"
 #include "mls.h"
+#include "objsec.h"
 #include "services.h"
 
+static unsigned int policybrief_hash_size;
+static struct crypto_shash *policybrief_tfm;
+static const char policybrief_hash_alg[] = "sha256";
+unsigned int policybrief_len;
+
 #define _DEBUG_HASHES
 
 #ifdef DEBUG_HASHES
@@ -879,6 +886,8 @@ void policydb_destroy(struct policydb *p)
 	ebitmap_destroy(&p->filename_trans_ttypes);
 	ebitmap_destroy(&p->policycaps);
 	ebitmap_destroy(&p->permissive_map);
+
+	kfree(p->policybrief);
 }
 
 /*
@@ -2220,6 +2229,49 @@ static int ocontext_read(struct policydb *p, struct policydb_compat_info *info,
 }
 
 /*
+ * Compute summary of a policy database binary representation file,
+ * and store it into a policy database structure.
+ */
+static int policydb_brief(struct policydb *policydb, void *ptr)
+{
+	SHASH_DESC_ON_STACK(desc, policybrief_tfm);
+	struct policy_file *fp = ptr;
+	u8 *hashval;
+	int rc = 0;
+
+	BUG_ON(policydb->policybrief);
+
+	hashval = kmalloc(policybrief_hash_size, GFP_KERNEL);
+	if (hashval == NULL)
+		return -ENOMEM;
+
+	desc->tfm = policybrief_tfm;
+	desc->flags = 0;
+	rc = crypto_shash_digest(desc, fp->data, fp->len, hashval);
+	if (rc) {
+		printk(KERN_ERR "Failed crypto_shash_digest: %d\n", rc);
+		goto out_free;
+	}
+
+	/* policy brief is in the form:
+	 * selinux(enforce=<0 or 1>:checkreqprot=<0 or 1>:<hashalg>=<checksum>)
+	 */
+	policydb->policybrief = kasprintf(GFP_KERNEL,
+					  "selinux(enforce=%d:checkreqprot=%d:%s=%*phN)",
+					  selinux_enforcing,
+					  selinux_checkreqprot,
+					  policybrief_hash_alg,
+					  policybrief_hash_size, hashval);
+	if (policydb->policybrief == NULL)
+		rc = -ENOMEM;
+
+out_free:
+	kfree(hashval);
+
+	return rc;
+}
+
+/*
  * Read the configuration data from a policy database binary
  * representation file into a policy database structure.
  */
@@ -2238,6 +2290,11 @@ int policydb_read(struct policydb *p, void *fp)
 	if (rc)
 		return rc;
 
+	/* Compute summary of policy, and store it in policydb */
+	rc = policydb_brief(p, fp);
+	if (rc)
+		goto bad;
+
 	/* Read the magic number and string length. */
 	rc = next_entry(buf, fp, sizeof(u32) * 2);
 	if (rc)
@@ -3456,3 +3513,31 @@ int policydb_write(struct policydb *p, void *fp)
 
 	return 0;
 }
+
+static int __init init_policybrief_hash(void)
+{
+	struct crypto_shash *tfm;
+
+	if (!selinux_enabled)
+		return 0;
+
+	tfm = crypto_alloc_shash(policybrief_hash_alg, 0, 0);
+	if (IS_ERR(tfm)) {
+		printk(KERN_ERR "Failed to alloc crypto hash %s\n",
+		       policybrief_hash_alg);
+		return PTR_ERR(tfm);
+	}
+
+	policybrief_tfm = tfm;
+	policybrief_hash_size = crypto_shash_digestsize(policybrief_tfm);
+
+	/* policy brief is in the form:
+	 * selinux(enforce=<0 or 1>:checkreqprot=<0 or 1>:<hashalg>=<checksum>)
+	 */
+	policybrief_len = 35 + strlen(policybrief_hash_alg) +
+		2*policybrief_hash_size + 1;
+
+	return 0;
+}
+
+late_initcall(init_policybrief_hash);
diff --git a/security/selinux/ss/policydb.h b/security/selinux/ss/policydb.h
index 725d594..2e5048c 100644
--- a/security/selinux/ss/policydb.h
+++ b/security/selinux/ss/policydb.h
@@ -293,6 +293,8 @@ struct policydb {
 	size_t len;
 
 	unsigned int policyvers;
+	/* summary computed on the policy */
+	unsigned char *policybrief;
 
 	unsigned int reject_unknown : 1;
 	unsigned int allow_unknown : 1;
@@ -309,6 +311,7 @@ struct policydb {
 extern int policydb_role_isvalid(struct policydb *p, unsigned int role);
 extern int policydb_read(struct policydb *p, void *fp);
 extern int policydb_write(struct policydb *p, void *fp);
+extern unsigned int policybrief_len;
 
 #define PERM_SYMTAB_SIZE 32
 
diff --git a/security/selinux/ss/services.c b/security/selinux/ss/services.c
index 60d9b02..67eb80d 100644
--- a/security/selinux/ss/services.c
+++ b/security/selinux/ss/services.c
@@ -2170,6 +2170,73 @@ size_t security_policydb_len(void)
 }
 
 /**
+ * security_policydb_brief - Get policydb brief
+ * @brief: pointer to buffer holding brief
+ * @len: in: brief buffer length if no alloc, out: brief string len
+ * @alloc: whether to allocate buffer for brief or not
+ *
+ * If @alloc, *brief must be kfreed by caller.
+ * If not @alloc, caller must pass a buffer that can hold policybrief_len
+ * chars (including terminating NUL).
+ * On success 0 is returned , or negative value on error.
+ **/
+int security_policydb_brief(char **brief, size_t *len, bool alloc)
+{
+	if (!ss_initialized || brief == NULL)
+		return -EINVAL;
+
+	if (alloc) {
+		*brief = kzalloc(policybrief_len, GFP_KERNEL);
+	} else if (*len < policybrief_len) {
+		/* put in *len the string size we need to write */
+		*len = policybrief_len;
+		return -ENAMETOOLONG;
+	}
+
+	if (*brief == NULL)
+		return -ENOMEM;
+
+	read_lock(&policy_rwlock);
+	strcpy(*brief, policydb.policybrief);
+	/* *len is the length of the output string */
+	*len = policybrief_len - 1;
+	read_unlock(&policy_rwlock);
+
+	return 0;
+}
+
+void security_policydb_update_info(u32 requested)
+{
+	/* policy brief is in the form:
+	 * selinux(enforce=<0 or 1>:checkreqprot=<0 or 1>:<hashalg>=<checksum>)
+	 */
+	char enforce[] = "enforce=";
+	char checkreqprot[] = "checkreqprot=";
+	char *p, *str;
+	int val;
+
+	if (!ss_initialized)
+		return;
+
+	if (requested == SECURITY__SETENFORCE) {
+		str = enforce;
+		val = selinux_enforcing;
+	} else if (requested == SECURITY__SETCHECKREQPROT) {
+		str = checkreqprot;
+		val = selinux_checkreqprot;
+	}
+
+	/* update global policydb, needs write lock */
+	write_lock_irq(&policy_rwlock);
+	p = strstr(policydb.policybrief, str);
+	if (p) {
+		p += strlen(str);
+		*p = '0' + val;
+	}
+	write_unlock_irq(&policy_rwlock);
+}
+
+/**
  * security_port_sid - Obtain the SID for a port.
  * @protocol: protocol number
  * @port: port number
-- 
1.8.3.1

--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v5 2/2] selinux: expose policy brief via selinuxfs

[Sebastien Buisson]

Expose policy brief via selinuxfs.

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

diff --git a/security/selinux/selinuxfs.c b/security/selinux/selinuxfs.c
index e8fe914..2561f96 100644
--- a/security/selinux/selinuxfs.c
+++ b/security/selinux/selinuxfs.c
@@ -99,6 +99,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_POLICYBRIEF,/* return policy summary */
 	SEL_INO_NEXT,	/* The next inode number to use */
 };
 
@@ -314,6 +315,29 @@ static ssize_t sel_read_policyvers(struct file *filp, char __user *buf,
 	.llseek		= generic_file_llseek,
 };
 
+static ssize_t sel_read_policybrief(struct file *filp, char __user *buf,
+				    size_t count, loff_t *ppos)
+{
+	char *tmpbuf;
+	size_t len;
+	ssize_t rc;
+
+	rc = security_policydb_brief(&tmpbuf, &len, true);
+	if (rc)
+		return rc;
+
+	rc = simple_read_from_buffer(buf, count, ppos, tmpbuf, len);
+
+	kfree(tmpbuf);
+
+	return rc;
+}
+
+static const struct file_operations sel_policybrief_ops = {
+	.read		= sel_read_policybrief,
+	.llseek		= generic_file_llseek,
+};
+
 /* declaration for sel_write_load */
 static int sel_make_bools(void);
 static int sel_make_classes(void);
@@ -1827,6 +1851,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_POLICYBRIEF] = {"policybrief", &sel_policybrief_ops,
+				     S_IRUGO},
 		/* last one */ {""}
 	};
 	ret = simple_fill_super(sb, SELINUX_MAGIC, selinux_files);
-- 
1.8.3.1

[PATCH v5 2/2] selinux: expose policy brief via selinuxfs

[Sebastien Buisson]

Expose policy brief via selinuxfs.

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

diff --git a/security/selinux/selinuxfs.c b/security/selinux/selinuxfs.c
index e8fe914..2561f96 100644
--- a/security/selinux/selinuxfs.c
+++ b/security/selinux/selinuxfs.c
@@ -99,6 +99,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_POLICYBRIEF,/* return policy summary */
 	SEL_INO_NEXT,	/* The next inode number to use */
 };
 
@@ -314,6 +315,29 @@ static ssize_t sel_read_policyvers(struct file *filp, char __user *buf,
 	.llseek		= generic_file_llseek,
 };
 
+static ssize_t sel_read_policybrief(struct file *filp, char __user *buf,
+				    size_t count, loff_t *ppos)
+{
+	char *tmpbuf;
+	size_t len;
+	ssize_t rc;
+
+	rc = security_policydb_brief(&tmpbuf, &len, true);
+	if (rc)
+		return rc;
+
+	rc = simple_read_from_buffer(buf, count, ppos, tmpbuf, len);
+
+	kfree(tmpbuf);
+
+	return rc;
+}
+
+static const struct file_operations sel_policybrief_ops = {
+	.read		= sel_read_policybrief,
+	.llseek		= generic_file_llseek,
+};
+
 /* declaration for sel_write_load */
 static int sel_make_bools(void);
 static int sel_make_classes(void);
@@ -1827,6 +1851,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_POLICYBRIEF] = {"policybrief", &sel_policybrief_ops,
+				     S_IRUGO},
 		/* last one */ {""}
 	};
 	ret = simple_fill_super(sb, SELINUX_MAGIC, selinux_files);
-- 
1.8.3.1

--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v5 1/2] selinux: add brief info to policydb

[Stephen Smalley]

On Tue, 2017-05-16 at 18:51 +0900, Sebastien Buisson wrote:
> Add policybrief field to struct policydb. It holds a brief info
> of the policydb, made of colon separated name and value pairs
> that give information about how the policy is applied in the
> security module(s).
> Note that the ordering of the fields in the string may change.
> 
> Policy brief is computed every time the policy is loaded, and when
> enforce or checkreqprot are changed.
> 
> Add security_policy_brief hook to give access to policy brief to
> the rest of the kernel. It is useful for any network or
> distributed file system that cares about how SELinux is enforced
> on its client nodes. This information is used to detect changes
> to the policy on file system client nodes, and can be forwarded
> to file system server nodes. Depending on how the policy is
> enforced on client side, server can refuse connection.
> 
> Signed-off-by: Sebastien Buisson <sbuisson@ddn.com>
> ---
>  include/linux/lsm_hooks.h           | 20 +++++++++
>  include/linux/security.h            |  7 +++
>  security/security.c                 |  8 ++++
>  security/selinux/hooks.c            |  7 +++
>  security/selinux/include/security.h |  2 +
>  security/selinux/selinuxfs.c        |  2 +
>  security/selinux/ss/policydb.c      | 85
> +++++++++++++++++++++++++++++++++++++
>  security/selinux/ss/policydb.h      |  3 ++
>  security/selinux/ss/services.c      | 67
> +++++++++++++++++++++++++++++
>  9 files changed, 201 insertions(+)
> 
> diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h
> index 1aa6333..e4dc2b4 100644
> --- a/include/linux/lsm_hooks.h
> +++ b/include/linux/lsm_hooks.h
> @@ -1331,6 +1331,24 @@
>   *	@inode we wish to get the security context of.
>   *	@ctx is a pointer in which to place the allocated security
> context.
>   *	@ctxlen points to the place to put the length of @ctx.
> + *
> + * Security hooks for policy brief
> + *
> + * @policy_brief:
> + *
> + *	Returns a string containing a brief info of the policydb.
> The string
> + *	contains colon separated name and value pairs that give
> information
> + *	about how the policy is applied in the security module(s).
> + *	Note that the ordering of the fields in the string may
> change.
> + *
> + *	@brief: pointer to buffer holding brief
> + *	@len: in: brief buffer length if no alloc, out: brief
> string len
> + *	@alloc: whether to allocate buffer for brief or not
> + *	If @alloc, *brief must be kfreed by caller.
> + *	If not @alloc, caller must pass a buffer that can hold
> policy brief
> + *	info (including terminating NUL).
> + *	On success 0 is returned , or negative value on error.
> + *
>   * This is the main security structure.
>   */
>  
> @@ -1562,6 +1580,7 @@
>  	int (*inode_setsecctx)(struct dentry *dentry, void *ctx, u32
> ctxlen);
>  	int (*inode_getsecctx)(struct inode *inode, void **ctx, u32
> *ctxlen);
>  
> +	int (*policy_brief)(char **brief, size_t *len, bool alloc);
>  #ifdef CONFIG_SECURITY_NETWORK
>  	int (*unix_stream_connect)(struct sock *sock, struct sock
> *other,
>  					struct sock *newsk);
> @@ -1806,6 +1825,7 @@ struct security_hook_heads {
>  	struct list_head inode_notifysecctx;
>  	struct list_head inode_setsecctx;
>  	struct list_head inode_getsecctx;
> +	struct list_head policy_brief;
>  #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 97df7ba..35ac97f 100644
> --- a/include/linux/security.h
> +++ b/include/linux/security.h
> @@ -376,6 +376,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);
> +
> +int security_policy_brief(char **brief, size_t *len, bool alloc);
>  #else /* CONFIG_SECURITY */
>  struct security_mnt_opts {
>  };
> @@ -1159,6 +1161,11 @@ static inline int
> security_inode_getsecctx(struct inode *inode, void **ctx, u32
>  {
>  	return -EOPNOTSUPP;
>  }
> +
> +static inline int security_policy_brief(char **brief, size_t *len,
> bool alloc)
> +{
> +	return -EOPNOTSUPP;
> +}
>  #endif	/* CONFIG_SECURITY */
>  
>  #ifdef CONFIG_SECURITY_NETWORK
> diff --git a/security/security.c b/security/security.c
> index d6d18a3..46052d3 100644
> --- a/security/security.c
> +++ b/security/security.c
> @@ -1269,6 +1269,12 @@ int security_inode_getsecctx(struct inode
> *inode, void **ctx, u32 *ctxlen)
>  }
>  EXPORT_SYMBOL(security_inode_getsecctx);
>  
> +int security_policy_brief(char **brief, size_t *len, bool alloc)
> +{
> +	return call_int_hook(policy_brief, -EOPNOTSUPP, brief, len,
> alloc);
> +}
> +EXPORT_SYMBOL(security_policy_brief);
> +
>  #ifdef CONFIG_SECURITY_NETWORK
>  
>  int security_unix_stream_connect(struct sock *sock, struct sock
> *other, struct sock *newsk)
> @@ -1868,6 +1874,8 @@ struct security_hook_heads security_hook_heads
> __lsm_ro_after_init = {
>  		LIST_HEAD_INIT(security_hook_heads.inode_setsecctx),
>  	.inode_getsecctx =
>  		LIST_HEAD_INIT(security_hook_heads.inode_getsecctx),
> +	.policy_brief =
> +		LIST_HEAD_INIT(security_hook_heads.policy_brief),
>  #ifdef CONFIG_SECURITY_NETWORK
>  	.unix_stream_connect =
>  		LIST_HEAD_INIT(security_hook_heads.unix_stream_conne
> ct),
> diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
> index e67a526..da245e8 100644
> --- a/security/selinux/hooks.c
> +++ b/security/selinux/hooks.c
> @@ -6063,6 +6063,11 @@ static int selinux_inode_getsecctx(struct
> inode *inode, void **ctx, u32 *ctxlen)
>  	*ctxlen = len;
>  	return 0;
>  }
> +
> +static int selinux_policy_brief(char **brief, size_t *len, bool
> alloc)
> +{
> +	return security_policydb_brief(brief, len, alloc);
> +}
>  #ifdef CONFIG_KEYS
>  
>  static int selinux_key_alloc(struct key *k, const struct cred *cred,
> @@ -6277,6 +6282,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_brief, selinux_policy_brief),
> +
>  	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..a0d4d7d 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);
> +int security_policydb_brief(char **brief, size_t *len, bool alloc);
> +void security_policydb_update_info(u32 requested);
>  
>  int security_policycap_supported(unsigned int req_cap);
>  
> diff --git a/security/selinux/selinuxfs.c
> b/security/selinux/selinuxfs.c
> index ce71718..e8fe914 100644
> --- a/security/selinux/selinuxfs.c
> +++ b/security/selinux/selinuxfs.c
> @@ -159,6 +159,7 @@ static ssize_t sel_write_enforce(struct file
> *file, const char __user *buf,
>  			from_kuid(&init_user_ns,
> audit_get_loginuid(current)),
>  			audit_get_sessionid(current));
>  		selinux_enforcing = new_value;
> +		security_policydb_update_info(SECURITY__SETENFORCE);
>  		if (selinux_enforcing)
>  			avc_ss_reset(0);
>  		selnl_notify_setenforce(selinux_enforcing);
> @@ -621,6 +622,7 @@ static ssize_t sel_write_checkreqprot(struct file
> *file, const char __user *buf,
>  		goto out;
>  
>  	selinux_checkreqprot = new_value ? 1 : 0;
> +	security_policydb_update_info(SECURITY__SETCHECKREQPROT);
>  	length = count;
>  out:
>  	kfree(page);
> diff --git a/security/selinux/ss/policydb.c
> b/security/selinux/ss/policydb.c
> index 0080122..83309a4 100644
> --- a/security/selinux/ss/policydb.c
> +++ b/security/selinux/ss/policydb.c
> @@ -32,13 +32,20 @@
>  #include <linux/errno.h>
>  #include <linux/audit.h>
>  #include <linux/flex_array.h>
> +#include <crypto/hash.h>
>  #include "security.h"
>  
>  #include "policydb.h"
>  #include "conditional.h"
>  #include "mls.h"
> +#include "objsec.h"
>  #include "services.h"
>  
> +static unsigned int policybrief_hash_size;
> +static struct crypto_shash *policybrief_tfm;
> +static const char policybrief_hash_alg[] = "sha256";
> +unsigned int policybrief_len;
> +
>  #define _DEBUG_HASHES
>  
>  #ifdef DEBUG_HASHES
> @@ -879,6 +886,8 @@ void policydb_destroy(struct policydb *p)
>  	ebitmap_destroy(&p->filename_trans_ttypes);
>  	ebitmap_destroy(&p->policycaps);
>  	ebitmap_destroy(&p->permissive_map);
> +
> +	kfree(p->policybrief);
>  }
>  
>  /*
> @@ -2220,6 +2229,49 @@ static int ocontext_read(struct policydb *p,
> struct policydb_compat_info *info,
>  }
>  
>  /*
> + * Compute summary of a policy database binary representation file,
> + * and store it into a policy database structure.
> + */
> +static int policydb_brief(struct policydb *policydb, void *ptr)
> +{
> +	SHASH_DESC_ON_STACK(desc, policybrief_tfm);
> +	struct policy_file *fp = ptr;
> +	u8 *hashval;
> +	int rc = 0;
> +
> +	BUG_ON(policydb->policybrief);
> +
> +	hashval = kmalloc(policybrief_hash_size, GFP_KERNEL);
> +	if (hashval == NULL)
> +		return -ENOMEM;
> +
> +	desc->tfm = policybrief_tfm;
> +	desc->flags = 0;
> +	rc = crypto_shash_digest(desc, fp->data, fp->len, hashval);
> +	if (rc) {
> +		printk(KERN_ERR "Failed crypto_shash_digest: %d\n",
> rc);
> +		goto out_free;
> +	}
> +
> +	/* policy brief is in the form:
> +	 * selinux(enforce=<0 or 1>:checkreqprot=<0 or
> 1>:<hashalg>=<checksum>)
> +	 */
> +	policydb->policybrief = kasprintf(GFP_KERNEL,
> +					  "selinux(enforce=%d:checkr
> eqprot=%d:%s=%*phN)",
> +					  selinux_enforcing,
> +					  selinux_checkreqprot,
> +					  policybrief_hash_alg,
> +					  policybrief_hash_size,
> hashval);

kasprintf() calls vsnprintf() twice, once to compute the length and
once to output the string. But you already precomputed the length
during init in policybrief_len.  So kmalloc() + snprintf() should
suffice.  You can also then check that snprintf() returns the expected
value.

> +	if (policydb->policybrief == NULL)
> +		rc = -ENOMEM;
> +
> +out_free:
> +	kfree(hashval);
> +
> +	return rc;
> +}
> +
> +/*
>   * Read the configuration data from a policy database binary
>   * representation file into a policy database structure.
>   */
> @@ -2238,6 +2290,11 @@ int policydb_read(struct policydb *p, void
> *fp)
>  	if (rc)
>  		return rc;
>  
> +	/* Compute summary of policy, and store it in policydb */
> +	rc = policydb_brief(p, fp);
> +	if (rc)
> +		goto bad;
> +
>  	/* Read the magic number and string length. */
>  	rc = next_entry(buf, fp, sizeof(u32) * 2);
>  	if (rc)
> @@ -3456,3 +3513,31 @@ int policydb_write(struct policydb *p, void
> *fp)
>  
>  	return 0;
>  }
> +
> +static int __init init_policybrief_hash(void)
> +{
> +	struct crypto_shash *tfm;
> +
> +	if (!selinux_enabled)
> +		return 0;
> +
> +	tfm = crypto_alloc_shash(policybrief_hash_alg, 0, 0);
> +	if (IS_ERR(tfm)) {
> +		printk(KERN_ERR "Failed to alloc crypto hash %s\n",
> +		       policybrief_hash_alg);
> +		return PTR_ERR(tfm);
> +	}
> +
> +	policybrief_tfm = tfm;
> +	policybrief_hash_size =
> crypto_shash_digestsize(policybrief_tfm);
> +
> +	/* policy brief is in the form:
> +	 * selinux(enforce=<0 or 1>:checkreqprot=<0 or
> 1>:<hashalg>=<checksum>)
> +	 */
> +	policybrief_len = 35 + strlen(policybrief_hash_alg) +
> +		2*policybrief_hash_size + 1;
> +
> +	return 0;
> +}
> +
> +late_initcall(init_policybrief_hash);
> diff --git a/security/selinux/ss/policydb.h
> b/security/selinux/ss/policydb.h
> index 725d594..2e5048c 100644
> --- a/security/selinux/ss/policydb.h
> +++ b/security/selinux/ss/policydb.h
> @@ -293,6 +293,8 @@ struct policydb {
>  	size_t len;
>  
>  	unsigned int policyvers;
> +	/* summary computed on the policy */
> +	unsigned char *policybrief;
>  
>  	unsigned int reject_unknown : 1;
>  	unsigned int allow_unknown : 1;
> @@ -309,6 +311,7 @@ struct policydb {
>  extern int policydb_role_isvalid(struct policydb *p, unsigned int
> role);
>  extern int policydb_read(struct policydb *p, void *fp);
>  extern int policydb_write(struct policydb *p, void *fp);
> +extern unsigned int policybrief_len;
>  
>  #define PERM_SYMTAB_SIZE 32
>  
> diff --git a/security/selinux/ss/services.c
> b/security/selinux/ss/services.c
> index 60d9b02..67eb80d 100644
> --- a/security/selinux/ss/services.c
> +++ b/security/selinux/ss/services.c
> @@ -2170,6 +2170,73 @@ size_t security_policydb_len(void)
>  }
>  
>  /**
> + * security_policydb_brief - Get policydb brief
> + * @brief: pointer to buffer holding brief
> + * @len: in: brief buffer length if no alloc, out: brief string len
> + * @alloc: whether to allocate buffer for brief or not
> + *
> + * If @alloc, *brief must be kfreed by caller.
> + * If not @alloc, caller must pass a buffer that can hold
> policybrief_len
> + * chars (including terminating NUL).
> + * On success 0 is returned , or negative value on error.
> + **/
> +int security_policydb_brief(char **brief, size_t *len, bool alloc)
> +{
> +	if (!ss_initialized || brief == NULL)
> +		return -EINVAL;
> +
> +	if (alloc) {
> +		*brief = kzalloc(policybrief_len, GFP_KERNEL);
> +	} else if (*len < policybrief_len) {
> +		/* put in *len the string size we need to write */
> +		*len = policybrief_len;
> +		return -ENAMETOOLONG;
> +	}
> +
> +	if (*brief == NULL)
> +		return -ENOMEM;
> +
> +	read_lock(&policy_rwlock);
> +	strcpy(*brief, policydb.policybrief);
> +	/* *len is the length of the output string */
> +	*len = policybrief_len - 1;

Is there a particular reason to not just return policybrief_len here as
well, for consistency in the interface?  How do you intend to use this
value in the caller?

> +	read_unlock(&policy_rwlock);
> +
> +	return 0;
> +}
> +
> +void security_policydb_update_info(u32 requested)
> +{
> +	/* policy brief is in the form:
> +	 * selinux(enforce=<0 or 1>:checkreqprot=<0 or
> 1>:<hashalg>=<checksum>)
> +	 */
> +	char enforce[] = "enforce=";
> +	char checkreqprot[] = "checkreqprot=";
> +	char *p, *str;
> +	int val;
> +
> +	if (!ss_initialized)
> +		return;
> +
> +	if (requested == SECURITY__SETENFORCE) {
> +		str = enforce;
> +		val = selinux_enforcing;
> +	} else if (requested == SECURITY__SETCHECKREQPROT) {
> +		str = checkreqprot;
> +		val = selinux_checkreqprot;
> +	}
> +
> +	/* update global policydb, needs write lock */
> +	write_lock_irq(&policy_rwlock);
> +	p = strstr(policydb.policybrief, str);
> +	if (p) {
> +		p += strlen(str);
> +		*p = '0' + val;
> +	}
> +	write_unlock_irq(&policy_rwlock);
> +}
> +
> +/**
>   * security_port_sid - Obtain the SID for a port.
>   * @protocol: protocol number
>   * @port: port number

[PATCH v5 1/2] selinux: add brief info to policydb

[Stephen Smalley]

On Tue, 2017-05-16 at 18:51 +0900, Sebastien Buisson wrote:
> Add policybrief field to struct policydb. It holds a brief info
> of the policydb, made of colon separated name and value pairs
> that give information about how the policy is applied in the
> security module(s).
> Note that the ordering of the fields in the string may change.
> 
> Policy brief is computed every time the policy is loaded, and when
> enforce or checkreqprot are changed.
> 
> Add security_policy_brief hook to give access to policy brief to
> the rest of the kernel. It is useful for any network or
> distributed file system that cares about how SELinux is enforced
> on its client nodes. This information is used to detect changes
> to the policy on file system client nodes, and can be forwarded
> to file system server nodes. Depending on how the policy is
> enforced on client side, server can refuse connection.
> 
> Signed-off-by: Sebastien Buisson <sbuisson@ddn.com>
> ---
> ?include/linux/lsm_hooks.h???????????| 20 +++++++++
> ?include/linux/security.h????????????|??7 +++
> ?security/security.c?????????????????|??8 ++++
> ?security/selinux/hooks.c????????????|??7 +++
> ?security/selinux/include/security.h |??2 +
> ?security/selinux/selinuxfs.c????????|??2 +
> ?security/selinux/ss/policydb.c??????| 85
> +++++++++++++++++++++++++++++++++++++
> ?security/selinux/ss/policydb.h??????|??3 ++
> ?security/selinux/ss/services.c??????| 67
> +++++++++++++++++++++++++++++
> ?9 files changed, 201 insertions(+)
> 
> diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h
> index 1aa6333..e4dc2b4 100644
> --- a/include/linux/lsm_hooks.h
> +++ b/include/linux/lsm_hooks.h
> @@ -1331,6 +1331,24 @@
> ? *	@inode we wish to get the security context of.
> ? *	@ctx is a pointer in which to place the allocated security
> context.
> ? *	@ctxlen points to the place to put the length of @ctx.
> + *
> + * Security hooks for policy brief
> + *
> + * @policy_brief:
> + *
> + *	Returns a string containing a brief info of the policydb.
> The string
> + *	contains colon separated name and value pairs that give
> information
> + *	about how the policy is applied in the security module(s).
> + *	Note that the ordering of the fields in the string may
> change.
> + *
> + *	@brief: pointer to buffer holding brief
> + *	@len: in: brief buffer length if no alloc, out: brief
> string len
> + *	@alloc: whether to allocate buffer for brief or not
> + *	If @alloc, *brief must be kfreed by caller.
> + *	If not @alloc, caller must pass a buffer that can hold
> policy brief
> + *	info (including terminating NUL).
> + *	On success 0 is returned , or negative value on error.
> + *
> ? * This is the main security structure.
> ? */
> ?
> @@ -1562,6 +1580,7 @@
> ?	int (*inode_setsecctx)(struct dentry *dentry, void *ctx, u32
> ctxlen);
> ?	int (*inode_getsecctx)(struct inode *inode, void **ctx, u32
> *ctxlen);
> ?
> +	int (*policy_brief)(char **brief, size_t *len, bool alloc);
> ?#ifdef CONFIG_SECURITY_NETWORK
> ?	int (*unix_stream_connect)(struct sock *sock, struct sock
> *other,
> ?					struct sock *newsk);
> @@ -1806,6 +1825,7 @@ struct security_hook_heads {
> ?	struct list_head inode_notifysecctx;
> ?	struct list_head inode_setsecctx;
> ?	struct list_head inode_getsecctx;
> +	struct list_head policy_brief;
> ?#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 97df7ba..35ac97f 100644
> --- a/include/linux/security.h
> +++ b/include/linux/security.h
> @@ -376,6 +376,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);
> +
> +int security_policy_brief(char **brief, size_t *len, bool alloc);
> ?#else /* CONFIG_SECURITY */
> ?struct security_mnt_opts {
> ?};
> @@ -1159,6 +1161,11 @@ static inline int
> security_inode_getsecctx(struct inode *inode, void **ctx, u32
> ?{
> ?	return -EOPNOTSUPP;
> ?}
> +
> +static inline int security_policy_brief(char **brief, size_t *len,
> bool alloc)
> +{
> +	return -EOPNOTSUPP;
> +}
> ?#endif	/* CONFIG_SECURITY */
> ?
> ?#ifdef CONFIG_SECURITY_NETWORK
> diff --git a/security/security.c b/security/security.c
> index d6d18a3..46052d3 100644
> --- a/security/security.c
> +++ b/security/security.c
> @@ -1269,6 +1269,12 @@ int security_inode_getsecctx(struct inode
> *inode, void **ctx, u32 *ctxlen)
> ?}
> ?EXPORT_SYMBOL(security_inode_getsecctx);
> ?
> +int security_policy_brief(char **brief, size_t *len, bool alloc)
> +{
> +	return call_int_hook(policy_brief, -EOPNOTSUPP, brief, len,
> alloc);
> +}
> +EXPORT_SYMBOL(security_policy_brief);
> +
> ?#ifdef CONFIG_SECURITY_NETWORK
> ?
> ?int security_unix_stream_connect(struct sock *sock, struct sock
> *other, struct sock *newsk)
> @@ -1868,6 +1874,8 @@ struct security_hook_heads security_hook_heads
> __lsm_ro_after_init = {
> ?		LIST_HEAD_INIT(security_hook_heads.inode_setsecctx),
> ?	.inode_getsecctx =
> ?		LIST_HEAD_INIT(security_hook_heads.inode_getsecctx),
> +	.policy_brief =
> +		LIST_HEAD_INIT(security_hook_heads.policy_brief),
> ?#ifdef CONFIG_SECURITY_NETWORK
> ?	.unix_stream_connect =
> ?		LIST_HEAD_INIT(security_hook_heads.unix_stream_conne
> ct),
> diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
> index e67a526..da245e8 100644
> --- a/security/selinux/hooks.c
> +++ b/security/selinux/hooks.c
> @@ -6063,6 +6063,11 @@ static int selinux_inode_getsecctx(struct
> inode *inode, void **ctx, u32 *ctxlen)
> ?	*ctxlen = len;
> ?	return 0;
> ?}
> +
> +static int selinux_policy_brief(char **brief, size_t *len, bool
> alloc)
> +{
> +	return security_policydb_brief(brief, len, alloc);
> +}
> ?#ifdef CONFIG_KEYS
> ?
> ?static int selinux_key_alloc(struct key *k, const struct cred *cred,
> @@ -6277,6 +6282,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_brief, selinux_policy_brief),
> +
> ?	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..a0d4d7d 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);
> +int security_policydb_brief(char **brief, size_t *len, bool alloc);
> +void security_policydb_update_info(u32 requested);
> ?
> ?int security_policycap_supported(unsigned int req_cap);
> ?
> diff --git a/security/selinux/selinuxfs.c
> b/security/selinux/selinuxfs.c
> index ce71718..e8fe914 100644
> --- a/security/selinux/selinuxfs.c
> +++ b/security/selinux/selinuxfs.c
> @@ -159,6 +159,7 @@ static ssize_t sel_write_enforce(struct file
> *file, const char __user *buf,
> ?			from_kuid(&init_user_ns,
> audit_get_loginuid(current)),
> ?			audit_get_sessionid(current));
> ?		selinux_enforcing = new_value;
> +		security_policydb_update_info(SECURITY__SETENFORCE);
> ?		if (selinux_enforcing)
> ?			avc_ss_reset(0);
> ?		selnl_notify_setenforce(selinux_enforcing);
> @@ -621,6 +622,7 @@ static ssize_t sel_write_checkreqprot(struct file
> *file, const char __user *buf,
> ?		goto out;
> ?
> ?	selinux_checkreqprot = new_value ? 1 : 0;
> +	security_policydb_update_info(SECURITY__SETCHECKREQPROT);
> ?	length = count;
> ?out:
> ?	kfree(page);
> diff --git a/security/selinux/ss/policydb.c
> b/security/selinux/ss/policydb.c
> index 0080122..83309a4 100644
> --- a/security/selinux/ss/policydb.c
> +++ b/security/selinux/ss/policydb.c
> @@ -32,13 +32,20 @@
> ?#include <linux/errno.h>
> ?#include <linux/audit.h>
> ?#include <linux/flex_array.h>
> +#include <crypto/hash.h>
> ?#include "security.h"
> ?
> ?#include "policydb.h"
> ?#include "conditional.h"
> ?#include "mls.h"
> +#include "objsec.h"
> ?#include "services.h"
> ?
> +static unsigned int policybrief_hash_size;
> +static struct crypto_shash *policybrief_tfm;
> +static const char policybrief_hash_alg[] = "sha256";
> +unsigned int policybrief_len;
> +
> ?#define _DEBUG_HASHES
> ?
> ?#ifdef DEBUG_HASHES
> @@ -879,6 +886,8 @@ void policydb_destroy(struct policydb *p)
> ?	ebitmap_destroy(&p->filename_trans_ttypes);
> ?	ebitmap_destroy(&p->policycaps);
> ?	ebitmap_destroy(&p->permissive_map);
> +
> +	kfree(p->policybrief);
> ?}
> ?
> ?/*
> @@ -2220,6 +2229,49 @@ static int ocontext_read(struct policydb *p,
> struct policydb_compat_info *info,
> ?}
> ?
> ?/*
> + * Compute summary of a policy database binary representation file,
> + * and store it into a policy database structure.
> + */
> +static int policydb_brief(struct policydb *policydb, void *ptr)
> +{
> +	SHASH_DESC_ON_STACK(desc, policybrief_tfm);
> +	struct policy_file *fp = ptr;
> +	u8 *hashval;
> +	int rc = 0;
> +
> +	BUG_ON(policydb->policybrief);
> +
> +	hashval = kmalloc(policybrief_hash_size, GFP_KERNEL);
> +	if (hashval == NULL)
> +		return -ENOMEM;
> +
> +	desc->tfm = policybrief_tfm;
> +	desc->flags = 0;
> +	rc = crypto_shash_digest(desc, fp->data, fp->len, hashval);
> +	if (rc) {
> +		printk(KERN_ERR "Failed crypto_shash_digest: %d\n",
> rc);
> +		goto out_free;
> +	}
> +
> +	/* policy brief is in the form:
> +	?* selinux(enforce=<0 or 1>:checkreqprot=<0 or
> 1>:<hashalg>=<checksum>)
> +	?*/
> +	policydb->policybrief = kasprintf(GFP_KERNEL,
> +					??"selinux(enforce=%d:checkr
> eqprot=%d:%s=%*phN)",
> +					??selinux_enforcing,
> +					??selinux_checkreqprot,
> +					??policybrief_hash_alg,
> +					??policybrief_hash_size,
> hashval);

kasprintf() calls vsnprintf() twice, once to compute the length and
once to output the string. But you already precomputed the length
during init in policybrief_len.  So kmalloc() + snprintf() should
suffice.  You can also then check that snprintf() returns the expected
value.

> +	if (policydb->policybrief == NULL)
> +		rc = -ENOMEM;
> +
> +out_free:
> +	kfree(hashval);
> +
> +	return rc;
> +}
> +
> +/*
> ? * Read the configuration data from a policy database binary
> ? * representation file into a policy database structure.
> ? */
> @@ -2238,6 +2290,11 @@ int policydb_read(struct policydb *p, void
> *fp)
> ?	if (rc)
> ?		return rc;
> ?
> +	/* Compute summary of policy, and store it in policydb */
> +	rc = policydb_brief(p, fp);
> +	if (rc)
> +		goto bad;
> +
> ?	/* Read the magic number and string length. */
> ?	rc = next_entry(buf, fp, sizeof(u32) * 2);
> ?	if (rc)
> @@ -3456,3 +3513,31 @@ int policydb_write(struct policydb *p, void
> *fp)
> ?
> ?	return 0;
> ?}
> +
> +static int __init init_policybrief_hash(void)
> +{
> +	struct crypto_shash *tfm;
> +
> +	if (!selinux_enabled)
> +		return 0;
> +
> +	tfm = crypto_alloc_shash(policybrief_hash_alg, 0, 0);
> +	if (IS_ERR(tfm)) {
> +		printk(KERN_ERR "Failed to alloc crypto hash %s\n",
> +		???????policybrief_hash_alg);
> +		return PTR_ERR(tfm);
> +	}
> +
> +	policybrief_tfm = tfm;
> +	policybrief_hash_size =
> crypto_shash_digestsize(policybrief_tfm);
> +
> +	/* policy brief is in the form:
> +	?* selinux(enforce=<0 or 1>:checkreqprot=<0 or
> 1>:<hashalg>=<checksum>)
> +	?*/
> +	policybrief_len = 35 + strlen(policybrief_hash_alg) +
> +		2*policybrief_hash_size + 1;
> +
> +	return 0;
> +}
> +
> +late_initcall(init_policybrief_hash);
> diff --git a/security/selinux/ss/policydb.h
> b/security/selinux/ss/policydb.h
> index 725d594..2e5048c 100644
> --- a/security/selinux/ss/policydb.h
> +++ b/security/selinux/ss/policydb.h
> @@ -293,6 +293,8 @@ struct policydb {
> ?	size_t len;
> ?
> ?	unsigned int policyvers;
> +	/* summary computed on the policy */
> +	unsigned char *policybrief;
> ?
> ?	unsigned int reject_unknown : 1;
> ?	unsigned int allow_unknown : 1;
> @@ -309,6 +311,7 @@ struct policydb {
> ?extern int policydb_role_isvalid(struct policydb *p, unsigned int
> role);
> ?extern int policydb_read(struct policydb *p, void *fp);
> ?extern int policydb_write(struct policydb *p, void *fp);
> +extern unsigned int policybrief_len;
> ?
> ?#define PERM_SYMTAB_SIZE 32
> ?
> diff --git a/security/selinux/ss/services.c
> b/security/selinux/ss/services.c
> index 60d9b02..67eb80d 100644
> --- a/security/selinux/ss/services.c
> +++ b/security/selinux/ss/services.c
> @@ -2170,6 +2170,73 @@ size_t security_policydb_len(void)
> ?}
> ?
> ?/**
> + * security_policydb_brief - Get policydb brief
> + * @brief: pointer to buffer holding brief
> + * @len: in: brief buffer length if no alloc, out: brief string len
> + * @alloc: whether to allocate buffer for brief or not
> + *
> + * If @alloc, *brief must be kfreed by caller.
> + * If not @alloc, caller must pass a buffer that can hold
> policybrief_len
> + * chars (including terminating NUL).
> + * On success 0 is returned , or negative value on error.
> + **/
> +int security_policydb_brief(char **brief, size_t *len, bool alloc)
> +{
> +	if (!ss_initialized || brief == NULL)
> +		return -EINVAL;
> +
> +	if (alloc) {
> +		*brief = kzalloc(policybrief_len, GFP_KERNEL);
> +	} else if (*len < policybrief_len) {
> +		/* put in *len the string size we need to write */
> +		*len = policybrief_len;
> +		return -ENAMETOOLONG;
> +	}
> +
> +	if (*brief == NULL)
> +		return -ENOMEM;
> +
> +	read_lock(&policy_rwlock);
> +	strcpy(*brief, policydb.policybrief);
> +	/* *len is the length of the output string */
> +	*len = policybrief_len - 1;

Is there a particular reason to not just return policybrief_len here as
well, for consistency in the interface?  How do you intend to use this
value in the caller?

> +	read_unlock(&policy_rwlock);
> +
> +	return 0;
> +}
> +
> +void security_policydb_update_info(u32 requested)
> +{
> +	/* policy brief is in the form:
> +	?* selinux(enforce=<0 or 1>:checkreqprot=<0 or
> 1>:<hashalg>=<checksum>)
> +	?*/
> +	char enforce[] = "enforce=";
> +	char checkreqprot[] = "checkreqprot=";
> +	char *p, *str;
> +	int val;
> +
> +	if (!ss_initialized)
> +		return;
> +
> +	if (requested == SECURITY__SETENFORCE) {
> +		str = enforce;
> +		val = selinux_enforcing;
> +	} else if (requested == SECURITY__SETCHECKREQPROT) {
> +		str = checkreqprot;
> +		val = selinux_checkreqprot;
> +	}
> +
> +	/* update global policydb, needs write lock */
> +	write_lock_irq(&policy_rwlock);
> +	p = strstr(policydb.policybrief, str);
> +	if (p) {
> +		p += strlen(str);
> +		*p = '0' + val;
> +	}
> +	write_unlock_irq(&policy_rwlock);
> +}
> +
> +/**
> ? * security_port_sid - Obtain the SID for a port.
> ? * @protocol: protocol number
> ? * @port: port number
--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v5 1/2] selinux: add brief info to policydb

[Sebastien Buisson]

2017-05-16 22:40 GMT+02:00 Stephen Smalley <sds@tycho.nsa.gov>:
>> +     strcpy(*brief, policydb.policybrief);
>> +     /* *len is the length of the output string */
>> +     *len = policybrief_len - 1;
>
> Is there a particular reason to not just return policybrief_len here as
> well, for consistency in the interface?  How do you intend to use this
> value in the caller?

As called in the other patch to expose policy brief via selinuxfs
(sel_read_policybrief), the intent is to provide the caller with the
length of the string returned.
Or should I set *len to policy brief_len here, and just make the
caller aware that the returned length is in fact the length of the
buffer (i.e. including terminating NUL byte)?

[PATCH v5 1/2] selinux: add brief info to policydb

[Sebastien Buisson]

2017-05-16 22:40 GMT+02:00 Stephen Smalley <sds@tycho.nsa.gov>:
>> +     strcpy(*brief, policydb.policybrief);
>> +     /* *len is the length of the output string */
>> +     *len = policybrief_len - 1;
>
> Is there a particular reason to not just return policybrief_len here as
> well, for consistency in the interface?  How do you intend to use this
> value in the caller?

As called in the other patch to expose policy brief via selinuxfs
(sel_read_policybrief), the intent is to provide the caller with the
length of the string returned.
Or should I set *len to policy brief_len here, and just make the
caller aware that the returned length is in fact the length of the
buffer (i.e. including terminating NUL byte)?
--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v5 1/2] selinux: add brief info to policydb

[William Roberts]

On Wed, May 17, 2017 at 7:59 AM, Sebastien Buisson
<sbuisson.ddn@gmail.com> wrote:
> 2017-05-16 22:40 GMT+02:00 Stephen Smalley <sds@tycho.nsa.gov>:
>>> +     strcpy(*brief, policydb.policybrief);
>>> +     /* *len is the length of the output string */
>>> +     *len = policybrief_len - 1;
>>
>> Is there a particular reason to not just return policybrief_len here as
>> well, for consistency in the interface?  How do you intend to use this
>> value in the caller?
>
> As called in the other patch to expose policy brief via selinuxfs
> (sel_read_policybrief), the intent is to provide the caller with the
> length of the string returned.
> Or should I set *len to policy brief_len here, and just make the
> caller aware that the returned length is in fact the length of the
> buffer (i.e. including terminating NUL byte)?

What is the caller supposed to do with length? This interface seemed kind of
odd. If it's guaranteed NUL byte terminated, do they even need length?

[PATCH v5 1/2] selinux: add brief info to policydb

[William Roberts]

On Wed, May 17, 2017 at 7:59 AM, Sebastien Buisson
<sbuisson.ddn@gmail.com> wrote:
> 2017-05-16 22:40 GMT+02:00 Stephen Smalley <sds@tycho.nsa.gov>:
>>> +     strcpy(*brief, policydb.policybrief);
>>> +     /* *len is the length of the output string */
>>> +     *len = policybrief_len - 1;
>>
>> Is there a particular reason to not just return policybrief_len here as
>> well, for consistency in the interface?  How do you intend to use this
>> value in the caller?
>
> As called in the other patch to expose policy brief via selinuxfs
> (sel_read_policybrief), the intent is to provide the caller with the
> length of the string returned.
> Or should I set *len to policy brief_len here, and just make the
> caller aware that the returned length is in fact the length of the
> buffer (i.e. including terminating NUL byte)?

What is the caller supposed to do with length? This interface seemed kind of
odd. If it's guaranteed NUL byte terminated, do they even need length?
--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v5 1/2] selinux: add brief info to policydb

[Sebastien Buisson]

2017-05-17 17:09 GMT+02:00 William Roberts <bill.c.roberts@gmail.com>:
> On Wed, May 17, 2017 at 7:59 AM, Sebastien Buisson
> <sbuisson.ddn@gmail.com> wrote:
>> 2017-05-16 22:40 GMT+02:00 Stephen Smalley <sds@tycho.nsa.gov>:
>>>> +     strcpy(*brief, policydb.policybrief);
>>>> +     /* *len is the length of the output string */
>>>> +     *len = policybrief_len - 1;
>>>
>>> Is there a particular reason to not just return policybrief_len here as
>>> well, for consistency in the interface?  How do you intend to use this
>>> value in the caller?
>>
>> As called in the other patch to expose policy brief via selinuxfs
>> (sel_read_policybrief), the intent is to provide the caller with the
>> length of the string returned.
>> Or should I set *len to policy brief_len here, and just make the
>> caller aware that the returned length is in fact the length of the
>> buffer (i.e. including terminating NUL byte)?
>
> What is the caller supposed to do with length? This interface seemed kind of
> odd. If it's guaranteed NUL byte terminated, do they even need length?

The length is useful as an input parameter in case the caller provides
its own buffer (instead of letting the function allocate one), and as
an output parameter in case the buffer given in input is not large
enough.
In any case, it can spare the caller the effort of recomputing the
length. As an example, sel_read_policybrief() in the 2/2 patch needs
to know the length of the string to put in the user buffer.

[PATCH v5 1/2] selinux: add brief info to policydb

[Sebastien Buisson]

2017-05-17 17:09 GMT+02:00 William Roberts <bill.c.roberts@gmail.com>:
> On Wed, May 17, 2017 at 7:59 AM, Sebastien Buisson
> <sbuisson.ddn@gmail.com> wrote:
>> 2017-05-16 22:40 GMT+02:00 Stephen Smalley <sds@tycho.nsa.gov>:
>>>> +     strcpy(*brief, policydb.policybrief);
>>>> +     /* *len is the length of the output string */
>>>> +     *len = policybrief_len - 1;
>>>
>>> Is there a particular reason to not just return policybrief_len here as
>>> well, for consistency in the interface?  How do you intend to use this
>>> value in the caller?
>>
>> As called in the other patch to expose policy brief via selinuxfs
>> (sel_read_policybrief), the intent is to provide the caller with the
>> length of the string returned.
>> Or should I set *len to policy brief_len here, and just make the
>> caller aware that the returned length is in fact the length of the
>> buffer (i.e. including terminating NUL byte)?
>
> What is the caller supposed to do with length? This interface seemed kind of
> odd. If it's guaranteed NUL byte terminated, do they even need length?

The length is useful as an input parameter in case the caller provides
its own buffer (instead of letting the function allocate one), and as
an output parameter in case the buffer given in input is not large
enough.
In any case, it can spare the caller the effort of recomputing the
length. As an example, sel_read_policybrief() in the 2/2 patch needs
to know the length of the string to put in the user buffer.
--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v5 1/2] selinux: add brief info to policydb

[William Roberts]

On Wed, May 17, 2017 at 8:24 AM, Sebastien Buisson
<sbuisson.ddn@gmail.com> wrote:
> 2017-05-17 17:09 GMT+02:00 William Roberts <bill.c.roberts@gmail.com>:
>> On Wed, May 17, 2017 at 7:59 AM, Sebastien Buisson
>> <sbuisson.ddn@gmail.com> wrote:
>>> 2017-05-16 22:40 GMT+02:00 Stephen Smalley <sds@tycho.nsa.gov>:
>>>>> +     strcpy(*brief, policydb.policybrief);
>>>>> +     /* *len is the length of the output string */
>>>>> +     *len = policybrief_len - 1;
>>>>
>>>> Is there a particular reason to not just return policybrief_len here as
>>>> well, for consistency in the interface?  How do you intend to use this
>>>> value in the caller?
>>>
>>> As called in the other patch to expose policy brief via selinuxfs
>>> (sel_read_policybrief), the intent is to provide the caller with the
>>> length of the string returned.
>>> Or should I set *len to policy brief_len here, and just make the
>>> caller aware that the returned length is in fact the length of the
>>> buffer (i.e. including terminating NUL byte)?
>>
>> What is the caller supposed to do with length? This interface seemed kind of
>> odd. If it's guaranteed NUL byte terminated, do they even need length?
>
> The length is useful as an input parameter in case the caller provides
> its own buffer (instead of letting the function allocate one), and as

This is what I don't get, why doesn't the function just always allocate?

> an output parameter in case the buffer given in input is not large
> enough.

This interface seems "Windowsy" (inout parameters)... Iv'e been looking at
it on and off for a few days and it just seems odd. Not odd enough for me
to give it more negative review comments.

> In any case, it can spare the caller the effort of recomputing the
> length. As an example, sel_read_policybrief() in the 2/2 patch needs
> to know the length of the string to put in the user buffer.

Oh yeah, IIRC offhand, you're adding each LSMs brief info and using strcpy
+ length instead of strcat avoiding the null iteration?

[PATCH v5 1/2] selinux: add brief info to policydb

[William Roberts]

On Wed, May 17, 2017 at 8:24 AM, Sebastien Buisson
<sbuisson.ddn@gmail.com> wrote:
> 2017-05-17 17:09 GMT+02:00 William Roberts <bill.c.roberts@gmail.com>:
>> On Wed, May 17, 2017 at 7:59 AM, Sebastien Buisson
>> <sbuisson.ddn@gmail.com> wrote:
>>> 2017-05-16 22:40 GMT+02:00 Stephen Smalley <sds@tycho.nsa.gov>:
>>>>> +     strcpy(*brief, policydb.policybrief);
>>>>> +     /* *len is the length of the output string */
>>>>> +     *len = policybrief_len - 1;
>>>>
>>>> Is there a particular reason to not just return policybrief_len here as
>>>> well, for consistency in the interface?  How do you intend to use this
>>>> value in the caller?
>>>
>>> As called in the other patch to expose policy brief via selinuxfs
>>> (sel_read_policybrief), the intent is to provide the caller with the
>>> length of the string returned.
>>> Or should I set *len to policy brief_len here, and just make the
>>> caller aware that the returned length is in fact the length of the
>>> buffer (i.e. including terminating NUL byte)?
>>
>> What is the caller supposed to do with length? This interface seemed kind of
>> odd. If it's guaranteed NUL byte terminated, do they even need length?
>
> The length is useful as an input parameter in case the caller provides
> its own buffer (instead of letting the function allocate one), and as

This is what I don't get, why doesn't the function just always allocate?

> an output parameter in case the buffer given in input is not large
> enough.

This interface seems "Windowsy" (inout parameters)... Iv'e been looking at
it on and off for a few days and it just seems odd. Not odd enough for me
to give it more negative review comments.

> In any case, it can spare the caller the effort of recomputing the
> length. As an example, sel_read_policybrief() in the 2/2 patch needs
> to know the length of the string to put in the user buffer.

Oh yeah, IIRC offhand, you're adding each LSMs brief info and using strcpy
+ length instead of strcat avoiding the null iteration?
--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v5 1/2] selinux: add brief info to policydb

[Stephen Smalley]

On Wed, 2017-05-17 at 16:59 +0200, Sebastien Buisson wrote:
> 2017-05-16 22:40 GMT+02:00 Stephen Smalley <sds@tycho.nsa.gov>:
> > > +     strcpy(*brief, policydb.policybrief);
> > > +     /* *len is the length of the output string */
> > > +     *len = policybrief_len - 1;
> > 
> > Is there a particular reason to not just return policybrief_len
> > here as
> > well, for consistency in the interface?  How do you intend to use
> > this
> > value in the caller?
> 
> As called in the other patch to expose policy brief via selinuxfs
> (sel_read_policybrief), the intent is to provide the caller with the
> length of the string returned.
> Or should I set *len to policy brief_len here, and just make the
> caller aware that the returned length is in fact the length of the
> buffer (i.e. including terminating NUL byte)?

Looking at the caller usage in the other patch, I guess it makes sense
in its current form.

[PATCH v5 1/2] selinux: add brief info to policydb

[Stephen Smalley]

On Wed, 2017-05-17 at 16:59 +0200, Sebastien Buisson wrote:
> 2017-05-16 22:40 GMT+02:00 Stephen Smalley <sds@tycho.nsa.gov>:
> > > +?????strcpy(*brief, policydb.policybrief);
> > > +?????/* *len is the length of the output string */
> > > +?????*len = policybrief_len - 1;
> > 
> > Is there a particular reason to not just return policybrief_len
> > here as
> > well, for consistency in the interface???How do you intend to use
> > this
> > value in the caller?
> 
> As called in the other patch to expose policy brief via selinuxfs
> (sel_read_policybrief), the intent is to provide the caller with the
> length of the string returned.
> Or should I set *len to policy brief_len here, and just make the
> caller aware that the returned length is in fact the length of the
> buffer (i.e. including terminating NUL byte)?

Looking at the caller usage in the other patch, I guess it makes sense
in its current form.


--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v5 1/2] selinux: add brief info to policydb

[Sebastien Buisson]

2017-05-17 17:34 GMT+02:00 William Roberts <bill.c.roberts@gmail.com>:
>>>>> Is there a particular reason to not just return policybrief_len here as
>>>>> well, for consistency in the interface?  How do you intend to use this
>>>>> value in the caller?
>>>>
>>>> As called in the other patch to expose policy brief via selinuxfs
>>>> (sel_read_policybrief), the intent is to provide the caller with the
>>>> length of the string returned.
>>>> Or should I set *len to policy brief_len here, and just make the
>>>> caller aware that the returned length is in fact the length of the
>>>> buffer (i.e. including terminating NUL byte)?
>>>
>>> What is the caller supposed to do with length? This interface seemed kind of
>>> odd. If it's guaranteed NUL byte terminated, do they even need length?
>>
>> The length is useful as an input parameter in case the caller provides
>> its own buffer (instead of letting the function allocate one), and as
>
> This is what I don't get, why doesn't the function just always allocate?

For performance reasons mainly. The caller would have a statically
allocated buffer, reused every time it needs to get the policy brief
info.

[PATCH v5 1/2] selinux: add brief info to policydb

[Sebastien Buisson]

2017-05-17 17:34 GMT+02:00 William Roberts <bill.c.roberts@gmail.com>:
>>>>> Is there a particular reason to not just return policybrief_len here as
>>>>> well, for consistency in the interface?  How do you intend to use this
>>>>> value in the caller?
>>>>
>>>> As called in the other patch to expose policy brief via selinuxfs
>>>> (sel_read_policybrief), the intent is to provide the caller with the
>>>> length of the string returned.
>>>> Or should I set *len to policy brief_len here, and just make the
>>>> caller aware that the returned length is in fact the length of the
>>>> buffer (i.e. including terminating NUL byte)?
>>>
>>> What is the caller supposed to do with length? This interface seemed kind of
>>> odd. If it's guaranteed NUL byte terminated, do they even need length?
>>
>> The length is useful as an input parameter in case the caller provides
>> its own buffer (instead of letting the function allocate one), and as
>
> This is what I don't get, why doesn't the function just always allocate?

For performance reasons mainly. The caller would have a statically
allocated buffer, reused every time it needs to get the policy brief
info.
--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v5 1/2] selinux: add brief info to policydb

[William Roberts]

On Wed, May 17, 2017 at 8:43 AM, Sebastien Buisson
<sbuisson.ddn@gmail.com> wrote:
> 2017-05-17 17:34 GMT+02:00 William Roberts <bill.c.roberts@gmail.com>:
>>>>>> Is there a particular reason to not just return policybrief_len here as
>>>>>> well, for consistency in the interface?  How do you intend to use this
>>>>>> value in the caller?
>>>>>
>>>>> As called in the other patch to expose policy brief via selinuxfs
>>>>> (sel_read_policybrief), the intent is to provide the caller with the
>>>>> length of the string returned.
>>>>> Or should I set *len to policy brief_len here, and just make the
>>>>> caller aware that the returned length is in fact the length of the
>>>>> buffer (i.e. including terminating NUL byte)?
>>>>
>>>> What is the caller supposed to do with length? This interface seemed kind of
>>>> odd. If it's guaranteed NUL byte terminated, do they even need length?
>>>
>>> The length is useful as an input parameter in case the caller provides
>>> its own buffer (instead of letting the function allocate one), and as
>>
>> This is what I don't get, why doesn't the function just always allocate?
>
> For performance reasons mainly. The caller would have a statically
> allocated buffer, reused every time it needs to get the policy brief
> info.

I'm assuming in the Lustre code you're going to call security_policy_brief(),
how would the caller know how big that buffer is going to be?

I'm looking at both v5 patches, I don't see where it's being called with alloc
set to false.

I don't see how this works with LSM stacking, I would imagine the security
hook needs to call this routine for each LSM and join them together in
some module name spaced way, which was mentioned before, but I don't
see that either, am I missing it?


-- 
Respectfully,

William C Roberts

[PATCH v5 1/2] selinux: add brief info to policydb

[William Roberts]

On Wed, May 17, 2017 at 8:43 AM, Sebastien Buisson
<sbuisson.ddn@gmail.com> wrote:
> 2017-05-17 17:34 GMT+02:00 William Roberts <bill.c.roberts@gmail.com>:
>>>>>> Is there a particular reason to not just return policybrief_len here as
>>>>>> well, for consistency in the interface?  How do you intend to use this
>>>>>> value in the caller?
>>>>>
>>>>> As called in the other patch to expose policy brief via selinuxfs
>>>>> (sel_read_policybrief), the intent is to provide the caller with the
>>>>> length of the string returned.
>>>>> Or should I set *len to policy brief_len here, and just make the
>>>>> caller aware that the returned length is in fact the length of the
>>>>> buffer (i.e. including terminating NUL byte)?
>>>>
>>>> What is the caller supposed to do with length? This interface seemed kind of
>>>> odd. If it's guaranteed NUL byte terminated, do they even need length?
>>>
>>> The length is useful as an input parameter in case the caller provides
>>> its own buffer (instead of letting the function allocate one), and as
>>
>> This is what I don't get, why doesn't the function just always allocate?
>
> For performance reasons mainly. The caller would have a statically
> allocated buffer, reused every time it needs to get the policy brief
> info.

I'm assuming in the Lustre code you're going to call security_policy_brief(),
how would the caller know how big that buffer is going to be?

I'm looking at both v5 patches, I don't see where it's being called with alloc
set to false.

I don't see how this works with LSM stacking, I would imagine the security
hook needs to call this routine for each LSM and join them together in
some module name spaced way, which was mentioned before, but I don't
see that either, am I missing it?


-- 
Respectfully,

William C Roberts
--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v5 1/2] selinux: add brief info to policydb

[William Roberts]

On Wed, May 17, 2017 at 9:04 AM, William Roberts
<bill.c.roberts@gmail.com> wrote:
> On Wed, May 17, 2017 at 8:43 AM, Sebastien Buisson
> <sbuisson.ddn@gmail.com> wrote:
>> 2017-05-17 17:34 GMT+02:00 William Roberts <bill.c.roberts@gmail.com>:
>>>>>>> Is there a particular reason to not just return policybrief_len here as
>>>>>>> well, for consistency in the interface?  How do you intend to use this
>>>>>>> value in the caller?
>>>>>>
>>>>>> As called in the other patch to expose policy brief via selinuxfs
>>>>>> (sel_read_policybrief), the intent is to provide the caller with the
>>>>>> length of the string returned.
>>>>>> Or should I set *len to policy brief_len here, and just make the
>>>>>> caller aware that the returned length is in fact the length of the
>>>>>> buffer (i.e. including terminating NUL byte)?
>>>>>
>>>>> What is the caller supposed to do with length? This interface seemed kind of
>>>>> odd. If it's guaranteed NUL byte terminated, do they even need length?
>>>>
>>>> The length is useful as an input parameter in case the caller provides
>>>> its own buffer (instead of letting the function allocate one), and as
>>>
>>> This is what I don't get, why doesn't the function just always allocate?
>>
>> For performance reasons mainly. The caller would have a statically
>> allocated buffer, reused every time it needs to get the policy brief
>> info.
>
> I'm assuming in the Lustre code you're going to call security_policy_brief(),
> how would the caller know how big that buffer is going to be?
>
> I'm looking at both v5 patches, I don't see where it's being called with alloc
> set to false.
>
> I don't see how this works with LSM stacking, I would imagine the security
> hook needs to call this routine for each LSM and join them together in
> some module name spaced way, which was mentioned before, but I don't
> see that either, am I missing it?

Disregard this last statement, I thought more of stacking was in.

[PATCH v5 1/2] selinux: add brief info to policydb

[William Roberts]

On Wed, May 17, 2017 at 9:04 AM, William Roberts
<bill.c.roberts@gmail.com> wrote:
> On Wed, May 17, 2017 at 8:43 AM, Sebastien Buisson
> <sbuisson.ddn@gmail.com> wrote:
>> 2017-05-17 17:34 GMT+02:00 William Roberts <bill.c.roberts@gmail.com>:
>>>>>>> Is there a particular reason to not just return policybrief_len here as
>>>>>>> well, for consistency in the interface?  How do you intend to use this
>>>>>>> value in the caller?
>>>>>>
>>>>>> As called in the other patch to expose policy brief via selinuxfs
>>>>>> (sel_read_policybrief), the intent is to provide the caller with the
>>>>>> length of the string returned.
>>>>>> Or should I set *len to policy brief_len here, and just make the
>>>>>> caller aware that the returned length is in fact the length of the
>>>>>> buffer (i.e. including terminating NUL byte)?
>>>>>
>>>>> What is the caller supposed to do with length? This interface seemed kind of
>>>>> odd. If it's guaranteed NUL byte terminated, do they even need length?
>>>>
>>>> The length is useful as an input parameter in case the caller provides
>>>> its own buffer (instead of letting the function allocate one), and as
>>>
>>> This is what I don't get, why doesn't the function just always allocate?
>>
>> For performance reasons mainly. The caller would have a statically
>> allocated buffer, reused every time it needs to get the policy brief
>> info.
>
> I'm assuming in the Lustre code you're going to call security_policy_brief(),
> how would the caller know how big that buffer is going to be?
>
> I'm looking at both v5 patches, I don't see where it's being called with alloc
> set to false.
>
> I don't see how this works with LSM stacking, I would imagine the security
> hook needs to call this routine for each LSM and join them together in
> some module name spaced way, which was mentioned before, but I don't
> see that either, am I missing it?

Disregard this last statement, I thought more of stacking was in.
--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v5 1/2] selinux: add brief info to policydb

[Sebastien Buisson]

2017-05-17 18:04 GMT+02:00 William Roberts <bill.c.roberts@gmail.com>:
> I'm assuming in the Lustre code you're going to call security_policy_brief(),
> how would the caller know how big that buffer is going to be?

We can determine it at configure time for instance, given that len as
an output parameter would give the size necessary to store the policy
brief info.

> I'm looking at both v5 patches, I don't see where it's being called with alloc
> set to false.

It would be called with alloc set to false from network and
distributed file systems like Lustre.

[PATCH v5 1/2] selinux: add brief info to policydb

[Sebastien Buisson]

2017-05-17 18:04 GMT+02:00 William Roberts <bill.c.roberts@gmail.com>:
> I'm assuming in the Lustre code you're going to call security_policy_brief(),
> how would the caller know how big that buffer is going to be?

We can determine it at configure time for instance, given that len as
an output parameter would give the size necessary to store the policy
brief info.

> I'm looking at both v5 patches, I don't see where it's being called with alloc
> set to false.

It would be called with alloc set to false from network and
distributed file systems like Lustre.
--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v5 1/2] selinux: add brief info to policydb

[William Roberts]

On Wed, May 17, 2017 at 10:00 AM, Sebastien Buisson
<sbuisson.ddn@gmail.com> wrote:
> 2017-05-17 18:04 GMT+02:00 William Roberts <bill.c.roberts@gmail.com>:
>> I'm assuming in the Lustre code you're going to call security_policy_brief(),
>> how would the caller know how big that buffer is going to be?
>
> We can determine it at configure time for instance, given that len as
> an output parameter would give the size necessary to store the policy
> brief info.
>
>> I'm looking at both v5 patches, I don't see where it's being called with alloc
>> set to false.
>
> It would be called with alloc set to false from network and
> distributed file systems like Lustre.

That doesn't seem like a good way at all.
1. What happens as the brief is changed, all callers with false
    would potentially need there buffer size increased.
2. There is no guarantee at runtime that as brief changes,
    that the size will remain bounded. fields could be
    added/changed/removed.
3. If/when stacking needs to be supported, brief size can
    change dramatically, bringing us back to issue 1.

-- 
Respectfully,

William C Roberts

[PATCH v5 1/2] selinux: add brief info to policydb

[William Roberts]

On Wed, May 17, 2017 at 10:00 AM, Sebastien Buisson
<sbuisson.ddn@gmail.com> wrote:
> 2017-05-17 18:04 GMT+02:00 William Roberts <bill.c.roberts@gmail.com>:
>> I'm assuming in the Lustre code you're going to call security_policy_brief(),
>> how would the caller know how big that buffer is going to be?
>
> We can determine it at configure time for instance, given that len as
> an output parameter would give the size necessary to store the policy
> brief info.
>
>> I'm looking at both v5 patches, I don't see where it's being called with alloc
>> set to false.
>
> It would be called with alloc set to false from network and
> distributed file systems like Lustre.

That doesn't seem like a good way at all.
1. What happens as the brief is changed, all callers with false
    would potentially need there buffer size increased.
2. There is no guarantee at runtime that as brief changes,
    that the size will remain bounded. fields could be
    added/changed/removed.
3. If/when stacking needs to be supported, brief size can
    change dramatically, bringing us back to issue 1.

-- 
Respectfully,

William C Roberts
--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html