* [PATCH v3 2/2] selinux: expose policy brief via selinuxfs
2017-05-11 12:59 [PATCH v3 1/2] selinux: add brief info to policydb Sebastien Buisson
@ 2017-05-11 12:59 ` Sebastien Buisson
2017-05-11 14:37 ` [PATCH v3 1/2] selinux: add brief info to policydb Stephen Smalley
2017-05-11 15:56 ` Casey Schaufler
2 siblings, 0 replies; 9+ messages in thread
From: Sebastien Buisson @ 2017-05-11 12:59 UTC (permalink / raw)
To: linux-security-module
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 8c9f5b7..50f69c5 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
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH v3 1/2] selinux: add brief info to policydb
2017-05-11 12:59 [PATCH v3 1/2] selinux: add brief info to policydb Sebastien Buisson
2017-05-11 12:59 ` [PATCH v3 2/2] selinux: expose policy brief via selinuxfs Sebastien Buisson
@ 2017-05-11 14:37 ` Stephen Smalley
2017-05-11 15:56 ` Casey Schaufler
2 siblings, 0 replies; 9+ messages in thread
From: Stephen Smalley @ 2017-05-11 14:37 UTC (permalink / raw)
To: linux-security-module
On Thu, 2017-05-11 at 21:59 +0900, Sebastien Buisson wrote:
> Add policybrief field to struct policydb. It holds a brief info
> of the policydb, in the following form:
> <0 or 1 for enforce>:<0 or 1 for checkreqprot>:<hashalg>=<checksum>
> 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. Lustre client makes use of this information
> to detect changes to the policy, and forward it to Lustre servers.
> Depending on how the policy is enforced on Lustre client side,
> Lustre servers can refuse connection.
>
> Signed-off-by: Sebastien Buisson <sbuisson@ddn.com>
> ---
> ?include/linux/lsm_hooks.h???????????| 16 ++++++++
> ?include/linux/security.h????????????|??7 ++++
> ?security/security.c?????????????????|??6 +++
> ?security/selinux/hooks.c????????????|??7 ++++
> ?security/selinux/include/security.h |??2 +
> ?security/selinux/selinuxfs.c????????|??2 +
> ?security/selinux/ss/policydb.c??????| 76
> +++++++++++++++++++++++++++++++++++++
> ?security/selinux/ss/policydb.h??????|??2 +
> ?security/selinux/ss/services.c??????| 62
> ++++++++++++++++++++++++++++++
> ?9 files changed, 180 insertions(+)
>
> diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h
> index 080f34e..9cac282 100644
> --- a/include/linux/lsm_hooks.h
> +++ b/include/linux/lsm_hooks.h
> @@ -1336,6 +1336,20 @@
> ? * @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,
> in the
> + * following form:
> + * <0 or 1 for enforce>:<0 or 1 for
> checkreqprot>:<hashalg>=<checksum>
> + *
> + * @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
> + * On success 0 is returned , or negative value on error.
> + *
> ? * This is the main security structure.
> ? */
> ?
> @@ -1568,6 +1582,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);
> @@ -1813,6 +1828,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 af675b5..3b72053 100644
> --- a/include/linux/security.h
> +++ b/include/linux/security.h
> @@ -377,6 +377,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 {
> ?};
> @@ -1166,6 +1168,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 b9fea39..954b391 100644
> --- a/security/security.c
> +++ b/security/security.c
> @@ -1285,6 +1285,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)
> 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 50062e7..8c9f5b7 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..58e73f5 100644
> --- a/security/selinux/ss/policydb.c
> +++ b/security/selinux/ss/policydb.c
> @@ -32,11 +32,13 @@
> ?#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"
> ?
> ?#define _DEBUG_HASHES
> @@ -879,6 +881,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 +2224,73 @@ 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)
> +{
> + struct policy_file *fp = ptr;
> + struct crypto_shash *tfm;
> + char hashalg[] = "sha256";
const
> + size_t hashsize;
> + u8 *hashval;
> + int idx;
> + unsigned char *p;
> +
> + if (policydb->policybrief)
> + return -EINVAL;
> +
> + tfm = crypto_alloc_shash(hashalg, 0, 0);
> + if (IS_ERR(tfm)) {
> + printk(KERN_ERR "Failed to alloc crypto hash %s\n",
> hashalg);
> + return PTR_ERR(tfm);
> + }
> +
> + hashsize = crypto_shash_digestsize(tfm);
Let's do the crypto_alloc_shash() and crypto_shash_digestsize() once
during init, e.g. see init_profile_hash() in
security/apparmor/crypto.c.
> + hashval = kmalloc(hashsize, GFP_KERNEL);
> + if (hashval == NULL) {
> + crypto_free_shash(tfm);
> + return -ENOMEM;
> + }
> +
> + {
> + int rc;
> +
> + SHASH_DESC_ON_STACK(desc, tfm);
> + desc->tfm = tfm;
> + desc->flags = 0;
> + rc = crypto_shash_digest(desc, fp->data, fp->len,
> hashval);
> + crypto_free_shash(tfm);
> + if (rc) {
> + printk(KERN_ERR "Failed crypto_shash_digest:
> %d\n", rc);
> + kfree(hashval);
> + return rc;
> + }
> + }
> +
> + /* policy brief is in the form:
> + ?* <0 or 1 for enforce>:<0 or 1 for
> checkreqprot>:<hashalg>=<checksum>
> + ?*/
> + policydb->policybrief = kzalloc(5 + strlen(hashalg) +
> 2*hashsize + 1,
We can also compute and save this size once during init; it won't ever
change at runtime.
> + GFP_KERNEL);
> + if (policydb->policybrief == NULL) {
> + kfree(hashval);
> + return -ENOMEM;
> + }
> +
> + sprintf(policydb->policybrief, "%d:%d:%s=",
> + selinux_enforcing, selinux_checkreqprot, hashalg);
> + p = policydb->policybrief + strlen(policydb->policybrief);
> + for (idx = 0; idx < hashsize; idx++) {
> + snprintf(p, 3, "%02x", hashval[idx]);
> + p += 2;
> + }
> + kfree(hashval);
> +
> + return 0;
> +}
> +
> +/*
> ? * Read the configuration data from a policy database binary
> ? * representation file into a policy database structure.
> ? */
> @@ -2238,6 +2309,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)
> diff --git a/security/selinux/ss/policydb.h
> b/security/selinux/ss/policydb.h
> index 725d594..31689d2f 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;
> diff --git a/security/selinux/ss/services.c
> b/security/selinux/ss/services.c
> index 60d9b02..3bbe649 100644
> --- a/security/selinux/ss/services.c
> +++ b/security/selinux/ss/services.c
> @@ -2170,6 +2170,68 @@ 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
> + *
> + * On success 0 is returned , or negative value on error.
> + **/
> +int security_policydb_brief(char **brief, size_t *len, bool alloc)
> +{
> + size_t policybrief_len;
> +
> + if (!ss_initialized || brief == NULL)
> + return -EINVAL;
> +
> + read_lock(&policy_rwlock);
> + policybrief_len = strlen(policydb.policybrief);
> + read_unlock(&policy_rwlock);
We don't need to compute this via strlen(); we can precompute during
init. Then we don't need to fetch anything here.
> +
> + if (alloc)
> + /* *brief must be kfreed by caller in this case */
> + *brief = kzalloc(policybrief_len + 1, GFP_KERNEL);
> + else
> + /*
> + ?* if !alloc, caller must pass a buffer that
> + ?* can hold policybrief_len+1 chars
> + ?*/
> + if (*len < policybrief_len + 1) {
> + /* put in *len the string size we need to
> write */
> + *len = policybrief_len;
> + return -ENAMETOOLONG;
> + }
> +
> + if (*brief == NULL)
> + return -ENOMEM;
> +
> + read_lock(&policy_rwlock);
> + *len = strlen(policydb.policybrief);
> + strncpy(*brief, policydb.policybrief, *len);
If in fact the length could change at runtime, then this would be
unsafe, since it could have changed between dropping the lock and re-
acquiring it. But this doesn't matter; we don't need to compute this
at runtime.
> + read_unlock(&policy_rwlock);
> +
> + return 0;
> +}
> +
> +void security_policydb_update_info(u32 requested)
> +{
> + /* policy brief is in the form:
> + ?* <0 or 1 for enforce>:<0 or 1 for
> checkreqprot>:<hashalg>=<checksum>
> + ?*/
> +
> + if (!ss_initialized)
> + return;
> +
> + /* update global policydb, needs write lock */
> + write_lock_irq(&policy_rwlock);
> + if (requested == SECURITY__SETENFORCE)
> + policydb.policybrief[0] = '0' + selinux_enforcing;
> + else if (requested == SECURITY__SETCHECKREQPROT)
> + policydb.policybrief[2] = '0' +
> selinux_checkreqprot;
> + 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
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH v3 1/2] selinux: add brief info to policydb
2017-05-11 12:59 [PATCH v3 1/2] selinux: add brief info to policydb Sebastien Buisson
2017-05-11 12:59 ` [PATCH v3 2/2] selinux: expose policy brief via selinuxfs Sebastien Buisson
2017-05-11 14:37 ` [PATCH v3 1/2] selinux: add brief info to policydb Stephen Smalley
@ 2017-05-11 15:56 ` Casey Schaufler
2017-05-11 20:22 ` Stephen Smalley
2 siblings, 1 reply; 9+ messages in thread
From: Casey Schaufler @ 2017-05-11 15:56 UTC (permalink / raw)
To: linux-security-module
On 5/11/2017 5:59 AM, Sebastien Buisson wrote:
> Add policybrief field to struct policydb. It holds a brief info
> of the policydb, in the following form:
> <0 or 1 for enforce>:<0 or 1 for checkreqprot>:<hashalg>=<checksum>
> 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. Lustre client makes use of this information
> to detect changes to the policy, and forward it to Lustre servers.
> Depending on how the policy is enforced on Lustre client side,
> Lustre servers can refuse connection.
>
> Signed-off-by: Sebastien Buisson <sbuisson@ddn.com>
> ---
> include/linux/lsm_hooks.h | 16 ++++++++
> include/linux/security.h | 7 ++++
> security/security.c | 6 +++
> security/selinux/hooks.c | 7 ++++
> security/selinux/include/security.h | 2 +
> security/selinux/selinuxfs.c | 2 +
> security/selinux/ss/policydb.c | 76 +++++++++++++++++++++++++++++++++++++
> security/selinux/ss/policydb.h | 2 +
> security/selinux/ss/services.c | 62 ++++++++++++++++++++++++++++++
> 9 files changed, 180 insertions(+)
>
> diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h
> index 080f34e..9cac282 100644
> --- a/include/linux/lsm_hooks.h
> +++ b/include/linux/lsm_hooks.h
> @@ -1336,6 +1336,20 @@
> * @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, in the
> + * following form:
> + * <0 or 1 for enforce>:<0 or 1 for checkreqprot>:<hashalg>=<checksum>
This sure looks like SELinux specific information. If the Spiffy
security module has multiple values for enforcement (e.g. off,
soft and hard) this interface definition does not work. What is a
"checkreqprot", and what is it for?
I expect that you have no interest (or incentive) in supporting
security modules other than SELinux, and that's OK. What's I'm
after is an interface that another security module could use if
someone where interested (or inspired) to do so.
Rather than a string with predefined positional values (something
I was taught not to do when 1 MIPS and 1 MEG was a big computer)
you might use "enforce=<value>:checkreqprot=<value>:hashalg=<checksum>"
for SELinux and define @policy_brief as
A string containing colon separated name and value pairs
that will be parsed and interpreted by the security module
or modules.
You already have it right for the "hashalg" field. If you want to
be really forward looking you could use names field names that
identify the security module that uses them, such as
"selinux/enforce=1:selinux/checkreqprot=0:selinux/MD5=8675309"
Note that I put "selinux/" onto the MD5 as well. You may have
multiple modules that use this interface on a system at the same
time in the not to distant future:
selinux/enforce=1:selinux/checkreqprot=0:
selinux/MD5=8675309:spiffy/enforce=soft:
spiffy/updatefreq=32544:spiffy/SHA256=84521
If you deal with this now it won't be a major headache to deal with
later.
> + *
> + * @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
> + * On success 0 is returned , or negative value on error.
> + *
> * This is the main security structure.
> */
>
> @@ -1568,6 +1582,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);
> @@ -1813,6 +1828,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 af675b5..3b72053 100644
> --- a/include/linux/security.h
> +++ b/include/linux/security.h
> @@ -377,6 +377,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 {
> };
> @@ -1166,6 +1168,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 b9fea39..954b391 100644
> --- a/security/security.c
> +++ b/security/security.c
> @@ -1285,6 +1285,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)
> 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 50062e7..8c9f5b7 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..58e73f5 100644
> --- a/security/selinux/ss/policydb.c
> +++ b/security/selinux/ss/policydb.c
> @@ -32,11 +32,13 @@
> #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"
>
> #define _DEBUG_HASHES
> @@ -879,6 +881,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 +2224,73 @@ 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)
> +{
> + struct policy_file *fp = ptr;
> + struct crypto_shash *tfm;
> + char hashalg[] = "sha256";
> + size_t hashsize;
> + u8 *hashval;
> + int idx;
> + unsigned char *p;
> +
> + if (policydb->policybrief)
> + return -EINVAL;
> +
> + tfm = crypto_alloc_shash(hashalg, 0, 0);
> + if (IS_ERR(tfm)) {
> + printk(KERN_ERR "Failed to alloc crypto hash %s\n", hashalg);
> + return PTR_ERR(tfm);
> + }
> +
> + hashsize = crypto_shash_digestsize(tfm);
> + hashval = kmalloc(hashsize, GFP_KERNEL);
> + if (hashval == NULL) {
> + crypto_free_shash(tfm);
> + return -ENOMEM;
> + }
> +
> + {
> + int rc;
> +
> + SHASH_DESC_ON_STACK(desc, tfm);
> + desc->tfm = tfm;
> + desc->flags = 0;
> + rc = crypto_shash_digest(desc, fp->data, fp->len, hashval);
> + crypto_free_shash(tfm);
> + if (rc) {
> + printk(KERN_ERR "Failed crypto_shash_digest: %d\n", rc);
> + kfree(hashval);
> + return rc;
> + }
> + }
> +
> + /* policy brief is in the form:
> + * <0 or 1 for enforce>:<0 or 1 for checkreqprot>:<hashalg>=<checksum>
> + */
> + policydb->policybrief = kzalloc(5 + strlen(hashalg) + 2*hashsize + 1,
> + GFP_KERNEL);
> + if (policydb->policybrief == NULL) {
> + kfree(hashval);
> + return -ENOMEM;
> + }
> +
> + sprintf(policydb->policybrief, "%d:%d:%s=",
> + selinux_enforcing, selinux_checkreqprot, hashalg);
> + p = policydb->policybrief + strlen(policydb->policybrief);
> + for (idx = 0; idx < hashsize; idx++) {
> + snprintf(p, 3, "%02x", hashval[idx]);
> + p += 2;
> + }
> + kfree(hashval);
> +
> + return 0;
> +}
> +
> +/*
> * Read the configuration data from a policy database binary
> * representation file into a policy database structure.
> */
> @@ -2238,6 +2309,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)
> diff --git a/security/selinux/ss/policydb.h b/security/selinux/ss/policydb.h
> index 725d594..31689d2f 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;
> diff --git a/security/selinux/ss/services.c b/security/selinux/ss/services.c
> index 60d9b02..3bbe649 100644
> --- a/security/selinux/ss/services.c
> +++ b/security/selinux/ss/services.c
> @@ -2170,6 +2170,68 @@ 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
> + *
> + * On success 0 is returned , or negative value on error.
> + **/
> +int security_policydb_brief(char **brief, size_t *len, bool alloc)
> +{
> + size_t policybrief_len;
> +
> + if (!ss_initialized || brief == NULL)
> + return -EINVAL;
> +
> + read_lock(&policy_rwlock);
> + policybrief_len = strlen(policydb.policybrief);
> + read_unlock(&policy_rwlock);
> +
> + if (alloc)
> + /* *brief must be kfreed by caller in this case */
> + *brief = kzalloc(policybrief_len + 1, GFP_KERNEL);
> + else
> + /*
> + * if !alloc, caller must pass a buffer that
> + * can hold policybrief_len+1 chars
> + */
> + if (*len < policybrief_len + 1) {
> + /* put in *len the string size we need to write */
> + *len = policybrief_len;
> + return -ENAMETOOLONG;
> + }
> +
> + if (*brief == NULL)
> + return -ENOMEM;
> +
> + read_lock(&policy_rwlock);
> + *len = strlen(policydb.policybrief);
> + strncpy(*brief, policydb.policybrief, *len);
> + read_unlock(&policy_rwlock);
> +
> + return 0;
> +}
> +
> +void security_policydb_update_info(u32 requested)
> +{
> + /* policy brief is in the form:
> + * <0 or 1 for enforce>:<0 or 1 for checkreqprot>:<hashalg>=<checksum>
> + */
> +
> + if (!ss_initialized)
> + return;
> +
> + /* update global policydb, needs write lock */
> + write_lock_irq(&policy_rwlock);
> + if (requested == SECURITY__SETENFORCE)
> + policydb.policybrief[0] = '0' + selinux_enforcing;
> + else if (requested == SECURITY__SETCHECKREQPROT)
> + policydb.policybrief[2] = '0' + selinux_checkreqprot;
> + 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
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH v3 1/2] selinux: add brief info to policydb
2017-05-11 15:56 ` Casey Schaufler
@ 2017-05-11 20:22 ` Stephen Smalley
2017-05-11 20:45 ` Casey Schaufler
0 siblings, 1 reply; 9+ messages in thread
From: Stephen Smalley @ 2017-05-11 20:22 UTC (permalink / raw)
To: linux-security-module
On Thu, 2017-05-11 at 08:56 -0700, Casey Schaufler wrote:
> On 5/11/2017 5:59 AM, Sebastien Buisson wrote:
> > Add policybrief field to struct policydb. It holds a brief info
> > of the policydb, in the following form:
> > <0 or 1 for enforce>:<0 or 1 for checkreqprot>:<hashalg>=<checksum>
> > 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. Lustre client makes use of this information
> > to detect changes to the policy, and forward it to Lustre servers.
> > Depending on how the policy is enforced on Lustre client side,
> > Lustre servers can refuse connection.
> >
> > Signed-off-by: Sebastien Buisson <sbuisson@ddn.com>
> > ---
> > ?include/linux/lsm_hooks.h???????????| 16 ++++++++
> > ?include/linux/security.h????????????|??7 ++++
> > ?security/security.c?????????????????|??6 +++
> > ?security/selinux/hooks.c????????????|??7 ++++
> > ?security/selinux/include/security.h |??2 +
> > ?security/selinux/selinuxfs.c????????|??2 +
> > ?security/selinux/ss/policydb.c??????| 76
> > +++++++++++++++++++++++++++++++++++++
> > ?security/selinux/ss/policydb.h??????|??2 +
> > ?security/selinux/ss/services.c??????| 62
> > ++++++++++++++++++++++++++++++
> > ?9 files changed, 180 insertions(+)
> >
> > diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h
> > index 080f34e..9cac282 100644
> > --- a/include/linux/lsm_hooks.h
> > +++ b/include/linux/lsm_hooks.h
> > @@ -1336,6 +1336,20 @@
> > ? * @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, in the
> > + * following form:
> > + * <0 or 1 for enforce>:<0 or 1 for
> > checkreqprot>:<hashalg>=<checksum>
>
> This sure looks like SELinux specific information. If the Spiffy
> security module has multiple values for enforcement (e.g. off,
> soft and hard) this interface definition does not work. What is a
> "checkreqprot", and what is it for?
>
> I expect that you have no interest (or incentive) in supporting
> security modules other than SELinux, and that's OK. What's I'm
> after is an interface that another security module could use if
> someone where interested (or inspired) to do so.
>
> Rather than a string with predefined positional values (something
> I was taught not to do when 1 MIPS and 1 MEG was a big computer)
> you might use
> "enforce=<value>:checkreqprot=<value>:hashalg=<checksum>"
No objection to the above, although it makes his updating code for
enforce/checkreqprot a bit uglier.
> for SELinux and define @policy_brief as
>
> A string containing colon separated name and value pairs
> that will be parsed and interpreted by the security module
> or modules.
Actually, I'm not clear it will be parsed or interpreted by the
security module(s). I think he is just fetching it and then doing a
simple comparison to check for inconsistencies between clients and the
server, and optionally admins/users can read it and interpret it as
they see fit.
>
> You already have it right for the "hashalg" field. If you want to
> be really forward looking you could use names field names that
> identify the security module that uses them, such as
>
> "selinux/enforce=1:selinux/checkreqprot=0:selinux/MD5=8675309"
That seems a bit verbose, particularly the duplicated selinux/ bit.
>
> Note that I put "selinux/" onto the MD5 as well. You may have
> multiple modules that use this interface on a system at the same
> time in the not to distant future:
>
> selinux/enforce=1:selinux/checkreqprot=0:
> selinux/MD5=8675309:spiffy/enforce=soft:
> spiffy/updatefreq=32544:spiffy/SHA256=84521
>
> If you deal with this now it won't be a major headache to deal with
> later.
>
> > + *
> > + * @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
> > + * On success 0 is returned , or negative value on error.
> > + *
> > ? * This is the main security structure.
> > ? */
> > ?
> > @@ -1568,6 +1582,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);
> > @@ -1813,6 +1828,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 af675b5..3b72053 100644
> > --- a/include/linux/security.h
> > +++ b/include/linux/security.h
> > @@ -377,6 +377,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 {
> > ?};
> > @@ -1166,6 +1168,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 b9fea39..954b391 100644
> > --- a/security/security.c
> > +++ b/security/security.c
> > @@ -1285,6 +1285,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)
> > 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 50062e7..8c9f5b7 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..58e73f5 100644
> > --- a/security/selinux/ss/policydb.c
> > +++ b/security/selinux/ss/policydb.c
> > @@ -32,11 +32,13 @@
> > ?#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"
> > ?
> > ?#define _DEBUG_HASHES
> > @@ -879,6 +881,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 +2224,73 @@ 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)
> > +{
> > + struct policy_file *fp = ptr;
> > + struct crypto_shash *tfm;
> > + char hashalg[] = "sha256";
> > + size_t hashsize;
> > + u8 *hashval;
> > + int idx;
> > + unsigned char *p;
> > +
> > + if (policydb->policybrief)
> > + return -EINVAL;
> > +
> > + tfm = crypto_alloc_shash(hashalg, 0, 0);
> > + if (IS_ERR(tfm)) {
> > + printk(KERN_ERR "Failed to alloc crypto hash
> > %s\n", hashalg);
> > + return PTR_ERR(tfm);
> > + }
> > +
> > + hashsize = crypto_shash_digestsize(tfm);
> > + hashval = kmalloc(hashsize, GFP_KERNEL);
> > + if (hashval == NULL) {
> > + crypto_free_shash(tfm);
> > + return -ENOMEM;
> > + }
> > +
> > + {
> > + int rc;
> > +
> > + SHASH_DESC_ON_STACK(desc, tfm);
> > + desc->tfm = tfm;
> > + desc->flags = 0;
> > + rc = crypto_shash_digest(desc, fp->data, fp->len,
> > hashval);
> > + crypto_free_shash(tfm);
> > + if (rc) {
> > + printk(KERN_ERR "Failed
> > crypto_shash_digest: %d\n", rc);
> > + kfree(hashval);
> > + return rc;
> > + }
> > + }
> > +
> > + /* policy brief is in the form:
> > + ?* <0 or 1 for enforce>:<0 or 1 for
> > checkreqprot>:<hashalg>=<checksum>
> > + ?*/
> > + policydb->policybrief = kzalloc(5 + strlen(hashalg) +
> > 2*hashsize + 1,
> > + GFP_KERNEL);
> > + if (policydb->policybrief == NULL) {
> > + kfree(hashval);
> > + return -ENOMEM;
> > + }
> > +
> > + sprintf(policydb->policybrief, "%d:%d:%s=",
> > + selinux_enforcing, selinux_checkreqprot, hashalg);
> > + p = policydb->policybrief + strlen(policydb->policybrief);
> > + for (idx = 0; idx < hashsize; idx++) {
> > + snprintf(p, 3, "%02x", hashval[idx]);
> > + p += 2;
> > + }
> > + kfree(hashval);
> > +
> > + return 0;
> > +}
> > +
> > +/*
> > ? * Read the configuration data from a policy database binary
> > ? * representation file into a policy database structure.
> > ? */
> > @@ -2238,6 +2309,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)
> > diff --git a/security/selinux/ss/policydb.h
> > b/security/selinux/ss/policydb.h
> > index 725d594..31689d2f 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;
> > diff --git a/security/selinux/ss/services.c
> > b/security/selinux/ss/services.c
> > index 60d9b02..3bbe649 100644
> > --- a/security/selinux/ss/services.c
> > +++ b/security/selinux/ss/services.c
> > @@ -2170,6 +2170,68 @@ 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
> > + *
> > + * On success 0 is returned , or negative value on error.
> > + **/
> > +int security_policydb_brief(char **brief, size_t *len, bool alloc)
> > +{
> > + size_t policybrief_len;
> > +
> > + if (!ss_initialized || brief == NULL)
> > + return -EINVAL;
> > +
> > + read_lock(&policy_rwlock);
> > + policybrief_len = strlen(policydb.policybrief);
> > + read_unlock(&policy_rwlock);
> > +
> > + if (alloc)
> > + /* *brief must be kfreed by caller in this case */
> > + *brief = kzalloc(policybrief_len + 1, GFP_KERNEL);
> > + else
> > + /*
> > + ?* if !alloc, caller must pass a buffer that
> > + ?* can hold policybrief_len+1 chars
> > + ?*/
> > + if (*len < policybrief_len + 1) {
> > + /* put in *len the string size we need to
> > write */
> > + *len = policybrief_len;
> > + return -ENAMETOOLONG;
> > + }
> > +
> > + if (*brief == NULL)
> > + return -ENOMEM;
> > +
> > + read_lock(&policy_rwlock);
> > + *len = strlen(policydb.policybrief);
> > + strncpy(*brief, policydb.policybrief, *len);
> > + read_unlock(&policy_rwlock);
> > +
> > + return 0;
> > +}
> > +
> > +void security_policydb_update_info(u32 requested)
> > +{
> > + /* policy brief is in the form:
> > + ?* <0 or 1 for enforce>:<0 or 1 for
> > checkreqprot>:<hashalg>=<checksum>
> > + ?*/
> > +
> > + if (!ss_initialized)
> > + return;
> > +
> > + /* update global policydb, needs write lock */
> > + write_lock_irq(&policy_rwlock);
> > + if (requested == SECURITY__SETENFORCE)
> > + policydb.policybrief[0] = '0' + selinux_enforcing;
> > + else if (requested == SECURITY__SETCHECKREQPROT)
> > + policydb.policybrief[2] = '0' +
> > selinux_checkreqprot;
> > + 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
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH v3 1/2] selinux: add brief info to policydb
2017-05-11 20:22 ` Stephen Smalley
@ 2017-05-11 20:45 ` Casey Schaufler
2017-05-12 18:38 ` Roberts, William C
2017-05-12 22:22 ` Paul Moore
0 siblings, 2 replies; 9+ messages in thread
From: Casey Schaufler @ 2017-05-11 20:45 UTC (permalink / raw)
To: linux-security-module
On 5/11/2017 1:22 PM, Stephen Smalley wrote:
> On Thu, 2017-05-11 at 08:56 -0700, Casey Schaufler wrote:
>> On 5/11/2017 5:59 AM, Sebastien Buisson wrote:
>>> Add policybrief field to struct policydb. It holds a brief info
>>> of the policydb, in the following form:
>>> <0 or 1 for enforce>:<0 or 1 for checkreqprot>:<hashalg>=<checksum>
>>> 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. Lustre client makes use of this information
>>> to detect changes to the policy, and forward it to Lustre servers.
>>> Depending on how the policy is enforced on Lustre client side,
>>> Lustre servers can refuse connection.
>>>
>>> Signed-off-by: Sebastien Buisson <sbuisson@ddn.com>
>>> ---
>>> include/linux/lsm_hooks.h | 16 ++++++++
>>> include/linux/security.h | 7 ++++
>>> security/security.c | 6 +++
>>> security/selinux/hooks.c | 7 ++++
>>> security/selinux/include/security.h | 2 +
>>> security/selinux/selinuxfs.c | 2 +
>>> security/selinux/ss/policydb.c | 76
>>> +++++++++++++++++++++++++++++++++++++
>>> security/selinux/ss/policydb.h | 2 +
>>> security/selinux/ss/services.c | 62
>>> ++++++++++++++++++++++++++++++
>>> 9 files changed, 180 insertions(+)
>>>
>>> diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h
>>> index 080f34e..9cac282 100644
>>> --- a/include/linux/lsm_hooks.h
>>> +++ b/include/linux/lsm_hooks.h
>>> @@ -1336,6 +1336,20 @@
>>> * @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, in the
>>> + * following form:
>>> + * <0 or 1 for enforce>:<0 or 1 for
>>> checkreqprot>:<hashalg>=<checksum>
>> This sure looks like SELinux specific information. If the Spiffy
>> security module has multiple values for enforcement (e.g. off,
>> soft and hard) this interface definition does not work. What is a
>> "checkreqprot", and what is it for?
>>
>> I expect that you have no interest (or incentive) in supporting
>> security modules other than SELinux, and that's OK. What's I'm
>> after is an interface that another security module could use if
>> someone where interested (or inspired) to do so.
>>
>> Rather than a string with predefined positional values (something
>> I was taught not to do when 1 MIPS and 1 MEG was a big computer)
>> you might use
>> "enforce=<value>:checkreqprot=<value>:hashalg=<checksum>"
> No objection to the above, although it makes his updating code for
> enforce/checkreqprot a bit uglier.
Sure, but can you imagine trying to use find(1) if the
options where positional?
>> for SELinux and define @policy_brief as
>>
>> A string containing colon separated name and value pairs
>> that will be parsed and interpreted by the security module
>> or modules.
> Actually, I'm not clear it will be parsed or interpreted by the
> security module(s). I think he is just fetching it and then doing a
> simple comparison to check for inconsistencies between clients and the
> server, and optionally admins/users can read it and interpret it as
> they see fit.
OK, in which case human eyes *need* the name as well as the value.
That, and strcmp(value, "enforce=0") is no harder than strcmp(value, "0").
>> You already have it right for the "hashalg" field. If you want to
>> be really forward looking you could use names field names that
>> identify the security module that uses them, such as
>>
>> "selinux/enforce=1:selinux/checkreqprot=0:selinux/MD5=8675309"
> That seems a bit verbose, particularly the duplicated selinux/ bit.
True that, on the other hand
"selinux(enforce=1:checkreqprot=0:MD5=8675309)"
would be harder to parse. Either works for me.
>> Note that I put "selinux/" onto the MD5 as well. You may have
>> multiple modules that use this interface on a system at the same
>> time in the not to distant future:
>>
>> selinux/enforce=1:selinux/checkreqprot=0:
>> selinux/MD5=8675309:spiffy/enforce=soft:
>> spiffy/updatefreq=32544:spiffy/SHA256=84521
>>
>> If you deal with this now it won't be a major headache to deal with
>> later.
>>
>>> + *
>>> + * @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
>>> + * On success 0 is returned , or negative value on error.
>>> + *
>>> * This is the main security structure.
>>> */
>>>
>>> @@ -1568,6 +1582,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);
>>> @@ -1813,6 +1828,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 af675b5..3b72053 100644
>>> --- a/include/linux/security.h
>>> +++ b/include/linux/security.h
>>> @@ -377,6 +377,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 {
>>> };
>>> @@ -1166,6 +1168,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 b9fea39..954b391 100644
>>> --- a/security/security.c
>>> +++ b/security/security.c
>>> @@ -1285,6 +1285,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)
>>> 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 50062e7..8c9f5b7 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..58e73f5 100644
>>> --- a/security/selinux/ss/policydb.c
>>> +++ b/security/selinux/ss/policydb.c
>>> @@ -32,11 +32,13 @@
>>> #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"
>>>
>>> #define _DEBUG_HASHES
>>> @@ -879,6 +881,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 +2224,73 @@ 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)
>>> +{
>>> + struct policy_file *fp = ptr;
>>> + struct crypto_shash *tfm;
>>> + char hashalg[] = "sha256";
>>> + size_t hashsize;
>>> + u8 *hashval;
>>> + int idx;
>>> + unsigned char *p;
>>> +
>>> + if (policydb->policybrief)
>>> + return -EINVAL;
>>> +
>>> + tfm = crypto_alloc_shash(hashalg, 0, 0);
>>> + if (IS_ERR(tfm)) {
>>> + printk(KERN_ERR "Failed to alloc crypto hash
>>> %s\n", hashalg);
>>> + return PTR_ERR(tfm);
>>> + }
>>> +
>>> + hashsize = crypto_shash_digestsize(tfm);
>>> + hashval = kmalloc(hashsize, GFP_KERNEL);
>>> + if (hashval == NULL) {
>>> + crypto_free_shash(tfm);
>>> + return -ENOMEM;
>>> + }
>>> +
>>> + {
>>> + int rc;
>>> +
>>> + SHASH_DESC_ON_STACK(desc, tfm);
>>> + desc->tfm = tfm;
>>> + desc->flags = 0;
>>> + rc = crypto_shash_digest(desc, fp->data, fp->len,
>>> hashval);
>>> + crypto_free_shash(tfm);
>>> + if (rc) {
>>> + printk(KERN_ERR "Failed
>>> crypto_shash_digest: %d\n", rc);
>>> + kfree(hashval);
>>> + return rc;
>>> + }
>>> + }
>>> +
>>> + /* policy brief is in the form:
>>> + * <0 or 1 for enforce>:<0 or 1 for
>>> checkreqprot>:<hashalg>=<checksum>
>>> + */
>>> + policydb->policybrief = kzalloc(5 + strlen(hashalg) +
>>> 2*hashsize + 1,
>>> + GFP_KERNEL);
>>> + if (policydb->policybrief == NULL) {
>>> + kfree(hashval);
>>> + return -ENOMEM;
>>> + }
>>> +
>>> + sprintf(policydb->policybrief, "%d:%d:%s=",
>>> + selinux_enforcing, selinux_checkreqprot, hashalg);
>>> + p = policydb->policybrief + strlen(policydb->policybrief);
>>> + for (idx = 0; idx < hashsize; idx++) {
>>> + snprintf(p, 3, "%02x", hashval[idx]);
>>> + p += 2;
>>> + }
>>> + kfree(hashval);
>>> +
>>> + return 0;
>>> +}
>>> +
>>> +/*
>>> * Read the configuration data from a policy database binary
>>> * representation file into a policy database structure.
>>> */
>>> @@ -2238,6 +2309,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)
>>> diff --git a/security/selinux/ss/policydb.h
>>> b/security/selinux/ss/policydb.h
>>> index 725d594..31689d2f 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;
>>> diff --git a/security/selinux/ss/services.c
>>> b/security/selinux/ss/services.c
>>> index 60d9b02..3bbe649 100644
>>> --- a/security/selinux/ss/services.c
>>> +++ b/security/selinux/ss/services.c
>>> @@ -2170,6 +2170,68 @@ 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
>>> + *
>>> + * On success 0 is returned , or negative value on error.
>>> + **/
>>> +int security_policydb_brief(char **brief, size_t *len, bool alloc)
>>> +{
>>> + size_t policybrief_len;
>>> +
>>> + if (!ss_initialized || brief == NULL)
>>> + return -EINVAL;
>>> +
>>> + read_lock(&policy_rwlock);
>>> + policybrief_len = strlen(policydb.policybrief);
>>> + read_unlock(&policy_rwlock);
>>> +
>>> + if (alloc)
>>> + /* *brief must be kfreed by caller in this case */
>>> + *brief = kzalloc(policybrief_len + 1, GFP_KERNEL);
>>> + else
>>> + /*
>>> + * if !alloc, caller must pass a buffer that
>>> + * can hold policybrief_len+1 chars
>>> + */
>>> + if (*len < policybrief_len + 1) {
>>> + /* put in *len the string size we need to
>>> write */
>>> + *len = policybrief_len;
>>> + return -ENAMETOOLONG;
>>> + }
>>> +
>>> + if (*brief == NULL)
>>> + return -ENOMEM;
>>> +
>>> + read_lock(&policy_rwlock);
>>> + *len = strlen(policydb.policybrief);
>>> + strncpy(*brief, policydb.policybrief, *len);
>>> + read_unlock(&policy_rwlock);
>>> +
>>> + return 0;
>>> +}
>>> +
>>> +void security_policydb_update_info(u32 requested)
>>> +{
>>> + /* policy brief is in the form:
>>> + * <0 or 1 for enforce>:<0 or 1 for
>>> checkreqprot>:<hashalg>=<checksum>
>>> + */
>>> +
>>> + if (!ss_initialized)
>>> + return;
>>> +
>>> + /* update global policydb, needs write lock */
>>> + write_lock_irq(&policy_rwlock);
>>> + if (requested == SECURITY__SETENFORCE)
>>> + policydb.policybrief[0] = '0' + selinux_enforcing;
>>> + else if (requested == SECURITY__SETCHECKREQPROT)
>>> + policydb.policybrief[2] = '0' +
>>> selinux_checkreqprot;
>>> + 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
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH v3 1/2] selinux: add brief info to policydb
2017-05-11 20:45 ` Casey Schaufler
@ 2017-05-12 18:38 ` Roberts, William C
2017-05-12 22:22 ` Paul Moore
1 sibling, 0 replies; 9+ messages in thread
From: Roberts, William C @ 2017-05-12 18:38 UTC (permalink / raw)
To: linux-security-module
> -----Original Message-----
> From: owner-linux-security-module at vger.kernel.org [mailto:owner-linux-
> security-module at vger.kernel.org] On Behalf Of Casey Schaufler
> Sent: Thursday, May 11, 2017 1:46 PM
> To: Stephen Smalley <sds@tycho.nsa.gov>; Sebastien Buisson
> <sbuisson.ddn@gmail.com>; linux-security-module at vger.kernel.org; linux-
> kernel at vger.kernel.org; selinux at tycho.nsa.gov
> Cc: Sebastien Buisson <sbuisson@ddn.com>; james.l.morris at oracle.com
> Subject: Re: [PATCH v3 1/2] selinux: add brief info to policydb
>
> On 5/11/2017 1:22 PM, Stephen Smalley wrote:
> > On Thu, 2017-05-11 at 08:56 -0700, Casey Schaufler wrote:
> >> On 5/11/2017 5:59 AM, Sebastien Buisson wrote:
> >>> Add policybrief field to struct policydb. It holds a brief info of
> >>> the policydb, in the following form:
> >>> <0 or 1 for enforce>:<0 or 1 for checkreqprot>:<hashalg>=<checksum>
> >>> 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. Lustre client makes use of this information to
> >>> detect changes to the policy, and forward it to Lustre servers.
> >>> Depending on how the policy is enforced on Lustre client side,
> >>> Lustre servers can refuse connection.
> >>>
> >>> Signed-off-by: Sebastien Buisson <sbuisson@ddn.com>
> >>> ---
> >>> include/linux/lsm_hooks.h | 16 ++++++++
> >>> include/linux/security.h | 7 ++++
> >>> security/security.c | 6 +++
> >>> security/selinux/hooks.c | 7 ++++
> >>> security/selinux/include/security.h | 2 +
> >>> security/selinux/selinuxfs.c | 2 +
> >>> security/selinux/ss/policydb.c | 76
> >>> +++++++++++++++++++++++++++++++++++++
> >>> security/selinux/ss/policydb.h | 2 +
> >>> security/selinux/ss/services.c | 62
> >>> ++++++++++++++++++++++++++++++
> >>> 9 files changed, 180 insertions(+)
> >>>
> >>> diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h
> >>> index 080f34e..9cac282 100644
> >>> --- a/include/linux/lsm_hooks.h
> >>> +++ b/include/linux/lsm_hooks.h
> >>> @@ -1336,6 +1336,20 @@
> >>> * @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, in the
> >>> + * following form:
> >>> + * <0 or 1 for enforce>:<0 or 1 for
> >>> checkreqprot>:<hashalg>=<checksum>
> >> This sure looks like SELinux specific information. If the Spiffy
> >> security module has multiple values for enforcement (e.g. off, soft
> >> and hard) this interface definition does not work. What is a
> >> "checkreqprot", and what is it for?
> >>
> >> I expect that you have no interest (or incentive) in supporting
> >> security modules other than SELinux, and that's OK. What's I'm after
> >> is an interface that another security module could use if someone
> >> where interested (or inspired) to do so.
> >>
> >> Rather than a string with predefined positional values (something I
> >> was taught not to do when 1 MIPS and 1 MEG was a big computer) you
> >> might use "enforce=<value>:checkreqprot=<value>:hashalg=<checksum>"
> > No objection to the above, although it makes his updating code for
> > enforce/checkreqprot a bit uglier.
>
> Sure, but can you imagine trying to use find(1) if the options where positional?
>
>
> >> for SELinux and define @policy_brief as
> >>
> >> A string containing colon separated name and value pairs
> >> that will be parsed and interpreted by the security module
> >> or modules.
> > Actually, I'm not clear it will be parsed or interpreted by the
> > security module(s). I think he is just fetching it and then doing a
> > simple comparison to check for inconsistencies between clients and the
> > server, and optionally admins/users can read it and interpret it as
> > they see fit.
>
> OK, in which case human eyes *need* the name as well as the value.
> That, and strcmp(value, "enforce=0") is no harder than strcmp(value, "0").
>
> >> You already have it right for the "hashalg" field. If you want to be
> >> really forward looking you could use names field names that identify
> >> the security module that uses them, such as
> >>
> >> "selinux/enforce=1:selinux/checkreqprot=0:selinux/MD5=8675309"
> > That seems a bit verbose, particularly the duplicated selinux/ bit.
>
> True that, on the other hand
>
> "selinux(enforce=1:checkreqprot=0:MD5=8675309)"
>
> would be harder to parse. Either works for me.
>
> >> Note that I put "selinux/" onto the MD5 as well. You may have
> >> multiple modules that use this interface on a system at the same time
> >> in the not to distant future:
> >>
> >> selinux/enforce=1:selinux/checkreqprot=0:
> >> selinux/MD5=8675309:spiffy/enforce=soft:
> >> spiffy/updatefreq=32544:spiffy/SHA256=84521
> >>
> >> If you deal with this now it won't be a major headache to deal with
> >> later.
> >>
> >>> + *
> >>> + * @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
> >>> + * On success 0 is returned , or negative value on error.
> >>> + *
> >>> * This is the main security structure.
> >>> */
> >>>
> >>> @@ -1568,6 +1582,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);
> >>> @@ -1813,6 +1828,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 af675b5..3b72053 100644
> >>> --- a/include/linux/security.h
> >>> +++ b/include/linux/security.h
> >>> @@ -377,6 +377,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 {
> >>> };
> >>> @@ -1166,6 +1168,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
> >>> b9fea39..954b391 100644
> >>> --- a/security/security.c
> >>> +++ b/security/security.c
> >>> @@ -1285,6 +1285,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) 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 50062e7..8c9f5b7 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..58e73f5 100644
> >>> --- a/security/selinux/ss/policydb.c
> >>> +++ b/security/selinux/ss/policydb.c
> >>> @@ -32,11 +32,13 @@
> >>> #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"
> >>>
> >>> #define _DEBUG_HASHES
> >>> @@ -879,6 +881,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 +2224,73 @@ 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) {
> >>> + struct policy_file *fp = ptr;
> >>> + struct crypto_shash *tfm;
> >>> + char hashalg[] = "sha256";
> >>> + size_t hashsize;
> >>> + u8 *hashval;
> >>> + int idx;
> >>> + unsigned char *p;
> >>> +
> >>> + if (policydb->policybrief)
> >>> + return -EINVAL;
> >>> +
> >>> + tfm = crypto_alloc_shash(hashalg, 0, 0);
> >>> + if (IS_ERR(tfm)) {
> >>> + printk(KERN_ERR "Failed to alloc crypto hash
> >>> %s\n", hashalg);
> >>> + return PTR_ERR(tfm);
> >>> + }
> >>> +
> >>> + hashsize = crypto_shash_digestsize(tfm);
> >>> + hashval = kmalloc(hashsize, GFP_KERNEL);
> >>> + if (hashval == NULL) {
> >>> + crypto_free_shash(tfm);
> >>> + return -ENOMEM;
> >>> + }
> >>> +
> >>> + {
> >>> + int rc;
> >>> +
> >>> + SHASH_DESC_ON_STACK(desc, tfm);
> >>> + desc->tfm = tfm;
> >>> + desc->flags = 0;
> >>> + rc = crypto_shash_digest(desc, fp->data, fp->len,
> >>> hashval);
> >>> + crypto_free_shash(tfm);
> >>> + if (rc) {
> >>> + printk(KERN_ERR "Failed
> >>> crypto_shash_digest: %d\n", rc);
> >>> + kfree(hashval);
> >>> + return rc;
> >>> + }
> >>> + }
> >>> +
> >>> + /* policy brief is in the form:
> >>> + * <0 or 1 for enforce>:<0 or 1 for
> >>> checkreqprot>:<hashalg>=<checksum>
> >>> + */
> >>> + policydb->policybrief = kzalloc(5 + strlen(hashalg) +
> >>> 2*hashsize + 1,
> >>> + GFP_KERNEL);
> >>> + if (policydb->policybrief == NULL) {
> >>> + kfree(hashval);
> >>> + return -ENOMEM;
> >>> + }
> >>> +
> >>> + sprintf(policydb->policybrief, "%d:%d:%s=",
> >>> + selinux_enforcing, selinux_checkreqprot, hashalg);
> >>> + p = policydb->policybrief + strlen(policydb->policybrief);
> >>> + for (idx = 0; idx < hashsize; idx++) {
> >>> + snprintf(p, 3, "%02x", hashval[idx]);
> >>> + p += 2;
> >>> + }
> >>> + kfree(hashval);
> >>> +
> >>> + return 0;
> >>> +}
> >>> +
> >>> +/*
> >>> * Read the configuration data from a policy database binary
> >>> * representation file into a policy database structure.
> >>> */
> >>> @@ -2238,6 +2309,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)
> >>> diff --git a/security/selinux/ss/policydb.h
> >>> b/security/selinux/ss/policydb.h index 725d594..31689d2f 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;
> >>> diff --git a/security/selinux/ss/services.c
> >>> b/security/selinux/ss/services.c index 60d9b02..3bbe649 100644
> >>> --- a/security/selinux/ss/services.c
> >>> +++ b/security/selinux/ss/services.c
> >>> @@ -2170,6 +2170,68 @@ 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
> >>> + *
> >>> + * On success 0 is returned , or negative value on error.
> >>> + **/
> >>> +int security_policydb_brief(char **brief, size_t *len, bool alloc)
> >>> +{
> >>> + size_t policybrief_len;
> >>> +
> >>> + if (!ss_initialized || brief == NULL)
> >>> + return -EINVAL;
> >>> +
> >>> + read_lock(&policy_rwlock);
> >>> + policybrief_len = strlen(policydb.policybrief);
> >>> + read_unlock(&policy_rwlock);
> >>> +
> >>> + if (alloc)
> >>> + /* *brief must be kfreed by caller in this case */
> >>> + *brief = kzalloc(policybrief_len + 1, GFP_KERNEL);
> >>> + else
> >>> + /*
> >>> + * if !alloc, caller must pass a buffer that
> >>> + * can hold policybrief_len+1 chars
> >>> + */
> >>> + if (*len < policybrief_len + 1) {
Kind of a weird way to do else if...
> >>> + /* put in *len the string size we need to
> >>> write */
> >>> + *len = policybrief_len;
> >>> + return -ENAMETOOLONG;
> >>> + }
> >>> +
> >>> + if (*brief == NULL)
> >>> + return -ENOMEM;
> >>> +
> >>> + read_lock(&policy_rwlock);
> >>> + *len = strlen(policydb.policybrief);
Isn't this: policybrief_len from above? No need to recompute.
> >>> + strncpy(*brief, policydb.policybrief, *len);
> >>> + read_unlock(&policy_rwlock);
> >>> +
> >>> + return 0;
> >>> +}
> >>> +
> >>> +void security_policydb_update_info(u32 requested) {
> >>> + /* policy brief is in the form:
> >>> + * <0 or 1 for enforce>:<0 or 1 for
> >>> checkreqprot>:<hashalg>=<checksum>
> >>> + */
> >>> +
> >>> + if (!ss_initialized)
> >>> + return;
> >>> +
> >>> + /* update global policydb, needs write lock */
> >>> + write_lock_irq(&policy_rwlock);
> >>> + if (requested == SECURITY__SETENFORCE)
> >>> + policydb.policybrief[0] = '0' + selinux_enforcing;
> >>> + else if (requested == SECURITY__SETCHECKREQPROT)
> >>> + policydb.policybrief[2] = '0' +
> >>> selinux_checkreqprot;
> >>> + 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
????{.n?+???????+%???????\x17??w??{.n?+????{??????????v?^?)????w*\x1fjg???\x1e???????j??\a??G??????\f???j:+v???w?j?m?????\x1e??\x1e?w?????f???h?????????
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH v3 1/2] selinux: add brief info to policydb
2017-05-11 20:45 ` Casey Schaufler
2017-05-12 18:38 ` Roberts, William C
@ 2017-05-12 22:22 ` Paul Moore
2017-05-12 22:55 ` William Roberts
1 sibling, 1 reply; 9+ messages in thread
From: Paul Moore @ 2017-05-12 22:22 UTC (permalink / raw)
To: linux-security-module
On Thu, May 11, 2017 at 4:45 PM, Casey Schaufler <casey@schaufler-ca.com> wrote:
> On 5/11/2017 1:22 PM, Stephen Smalley wrote:
>> On Thu, 2017-05-11 at 08:56 -0700, Casey Schaufler wrote:
>>> On 5/11/2017 5:59 AM, Sebastien Buisson wrote:
>>>> Add policybrief field to struct policydb. It holds a brief info
>>>> of the policydb, in the following form:
>>>> <0 or 1 for enforce>:<0 or 1 for checkreqprot>:<hashalg>=<checksum>
>>>> 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. Lustre client makes use of this information
>>>> to detect changes to the policy, and forward it to Lustre servers.
>>>> Depending on how the policy is enforced on Lustre client side,
>>>> Lustre servers can refuse connection.
>>>>
>>>> Signed-off-by: Sebastien Buisson <sbuisson@ddn.com>
>>>> ---
>>>> include/linux/lsm_hooks.h | 16 ++++++++
>>>> include/linux/security.h | 7 ++++
>>>> security/security.c | 6 +++
>>>> security/selinux/hooks.c | 7 ++++
>>>> security/selinux/include/security.h | 2 +
>>>> security/selinux/selinuxfs.c | 2 +
>>>> security/selinux/ss/policydb.c | 76
>>>> +++++++++++++++++++++++++++++++++++++
>>>> security/selinux/ss/policydb.h | 2 +
>>>> security/selinux/ss/services.c | 62
>>>> ++++++++++++++++++++++++++++++
>>>> 9 files changed, 180 insertions(+)
>>>>
>>>> diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h
>>>> index 080f34e..9cac282 100644
>>>> --- a/include/linux/lsm_hooks.h
>>>> +++ b/include/linux/lsm_hooks.h
>>>> @@ -1336,6 +1336,20 @@
>>>> * @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, in the
>>>> + * following form:
>>>> + * <0 or 1 for enforce>:<0 or 1 for
>>>> checkreqprot>:<hashalg>=<checksum>
>>>
>>> This sure looks like SELinux specific information. If the Spiffy
>>> security module has multiple values for enforcement (e.g. off,
>>> soft and hard) this interface definition does not work. What is a
>>> "checkreqprot", and what is it for?
>>>
>>> I expect that you have no interest (or incentive) in supporting
>>> security modules other than SELinux, and that's OK. What's I'm
>>> after is an interface that another security module could use if
>>> someone where interested (or inspired) to do so.
>>>
>>> Rather than a string with predefined positional values (something
>>> I was taught not to do when 1 MIPS and 1 MEG was a big computer)
>>> you might use
>>> "enforce=<value>:checkreqprot=<value>:hashalg=<checksum>"
>>
>> No objection to the above, although it makes his updating code for
>> enforce/checkreqprot a bit uglier.
>
> Sure, but can you imagine trying to use find(1) if the
> options where positional?
Perhaps I'm suffering from audit induced PTSD, but I think we need to
operate under the assumption that we are going to need to augment this
at some point in the future (no good feature goes un-abused) and I'd
much rather have some sort of name-value pairing to keep my sanity. I
also feel rather strongly that we should make it very explicit in the
comments that the ordering of the fields in the string may change.
>>> for SELinux and define @policy_brief as
>>>
>>> A string containing colon separated name and value pairs
>>> that will be parsed and interpreted by the security module
>>> or modules.
>>
>> Actually, I'm not clear it will be parsed or interpreted by the
>> security module(s). I think he is just fetching it and then doing a
>> simple comparison to check for inconsistencies between clients and the
>> server, and optionally admins/users can read it and interpret it as
>> they see fit.
>
> OK, in which case human eyes *need* the name as well as the value.
> That, and strcmp(value, "enforce=0") is no harder than strcmp(value, "0").
The initial use case may not require parsing the string, but who knows
what will end up using this five years from now. Once again, I agree
with Casey, let's make sure it easily parsed and readable by admins;
imagine this as the output of a file under /proc or /sys.
>>> You already have it right for the "hashalg" field. If you want to
>>> be really forward looking you could use names field names that
>>> identify the security module that uses them, such as
>>>
>>> "selinux/enforce=1:selinux/checkreqprot=0:selinux/MD5=8675309"
>> That seems a bit verbose, particularly the duplicated selinux/ bit.
>
> True that, on the other hand
>
> "selinux(enforce=1:checkreqprot=0:MD5=8675309)"
>
> would be harder to parse. Either works for me.
I'm not sure I care too much about how the name-value pairs get
namespaced, but considering the LSM stacking work that is happening,
we should namespace it somehow.
--
paul moore
www.paul-moore.com
--
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
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH v3 1/2] selinux: add brief info to policydb
2017-05-12 22:22 ` Paul Moore
@ 2017-05-12 22:55 ` William Roberts
0 siblings, 0 replies; 9+ messages in thread
From: William Roberts @ 2017-05-12 22:55 UTC (permalink / raw)
To: linux-security-module
On Fri, May 12, 2017 at 3:22 PM, Paul Moore <paul@paul-moore.com> wrote:
>
> On Thu, May 11, 2017 at 4:45 PM, Casey Schaufler <casey@schaufler-ca.com> wrote:
> > On 5/11/2017 1:22 PM, Stephen Smalley wrote:
> >> On Thu, 2017-05-11 at 08:56 -0700, Casey Schaufler wrote:
> >>> On 5/11/2017 5:59 AM, Sebastien Buisson wrote:
> >>>> Add policybrief field to struct policydb. It holds a brief info
> >>>> of the policydb, in the following form:
> >>>> <0 or 1 for enforce>:<0 or 1 for checkreqprot>:<hashalg>=<checksum>
> >>>> 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. Lustre client makes use of this information
> >>>> to detect changes to the policy, and forward it to Lustre servers.
> >>>> Depending on how the policy is enforced on Lustre client side,
> >>>> Lustre servers can refuse connection.
> >>>>
> >>>> Signed-off-by: Sebastien Buisson <sbuisson@ddn.com>
> >>>> ---
> >>>> include/linux/lsm_hooks.h | 16 ++++++++
> >>>> include/linux/security.h | 7 ++++
> >>>> security/security.c | 6 +++
> >>>> security/selinux/hooks.c | 7 ++++
> >>>> security/selinux/include/security.h | 2 +
> >>>> security/selinux/selinuxfs.c | 2 +
> >>>> security/selinux/ss/policydb.c | 76
> >>>> +++++++++++++++++++++++++++++++++++++
> >>>> security/selinux/ss/policydb.h | 2 +
> >>>> security/selinux/ss/services.c | 62
> >>>> ++++++++++++++++++++++++++++++
> >>>> 9 files changed, 180 insertions(+)
> >>>>
> >>>> diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h
> >>>> index 080f34e..9cac282 100644
> >>>> --- a/include/linux/lsm_hooks.h
> >>>> +++ b/include/linux/lsm_hooks.h
> >>>> @@ -1336,6 +1336,20 @@
> >>>> * @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, in the
> >>>> + * following form:
> >>>> + * <0 or 1 for enforce>:<0 or 1 for
> >>>> checkreqprot>:<hashalg>=<checksum>
> >>>
> >>> This sure looks like SELinux specific information. If the Spiffy
> >>> security module has multiple values for enforcement (e.g. off,
> >>> soft and hard) this interface definition does not work. What is a
> >>> "checkreqprot", and what is it for?
> >>>
> >>> I expect that you have no interest (or incentive) in supporting
> >>> security modules other than SELinux, and that's OK. What's I'm
> >>> after is an interface that another security module could use if
> >>> someone where interested (or inspired) to do so.
> >>>
> >>> Rather than a string with predefined positional values (something
> >>> I was taught not to do when 1 MIPS and 1 MEG was a big computer)
> >>> you might use
> >>> "enforce=<value>:checkreqprot=<value>:hashalg=<checksum>"
> >>
> >> No objection to the above, although it makes his updating code for
> >> enforce/checkreqprot a bit uglier.
> >
> > Sure, but can you imagine trying to use find(1) if the
> > options where positional?
>
> Perhaps I'm suffering from audit induced PTSD, but I think we need to
> operate under the assumption that we are going to need to augment this
> at some point in the future (no good feature goes un-abused) and I'd
> much rather have some sort of name-value pairing to keep my sanity. I
> also feel rather strongly that we should make it very explicit in the
> comments that the ordering of the fields in the string may change.
>
> >>> for SELinux and define @policy_brief as
> >>>
> >>> A string containing colon separated name and value pairs
> >>> that will be parsed and interpreted by the security module
> >>> or modules.
> >>
> >> Actually, I'm not clear it will be parsed or interpreted by the
> >> security module(s). I think he is just fetching it and then doing a
> >> simple comparison to check for inconsistencies between clients and the
> >> server, and optionally admins/users can read it and interpret it as
> >> they see fit.
> >
> > OK, in which case human eyes *need* the name as well as the value.
> > That, and strcmp(value, "enforce=0") is no harder than strcmp(value, "0").
>
> The initial use case may not require parsing the string, but who knows
> what will end up using this five years from now. Once again, I agree
> with Casey, let's make sure it easily parsed and readable by admins;
> imagine this as the output of a file under /proc or /sys.
>
> >>> You already have it right for the "hashalg" field. If you want to
> >>> be really forward looking you could use names field names that
> >>> identify the security module that uses them, such as
> >>>
> >>> "selinux/enforce=1:selinux/checkreqprot=0:selinux/MD5=8675309"
> >> That seems a bit verbose, particularly the duplicated selinux/ bit.
> >
> > True that, on the other hand
> >
> > "selinux(enforce=1:checkreqprot=0:MD5=8675309)"
> >
> > would be harder to parse. Either works for me.
>
> I'm not sure I care too much about how the name-value pairs get
> namespaced, but considering the LSM stacking work that is happening,
> we should namespace it somehow.
>
> --
> paul moore
> www.paul-moore.com
(Take 2, clicks plain text mode) Darn, I cleaned my inbox out, and
forgot to comment. Couldn't you replace the hex string conversion and
kzalloc
with asprintf and the kernel format specifier extension phN as
documented under Documentation/printk-formats.txt.
It looks like asprintf flows down into vsnprintf and into the
pointer() routine properly.
asprintf prototype:
http://elixir.free-electrons.com/linux/latest/source/include/linux/kernel.h#L426
As an aside, I wonder if audit can ditch there hex escaping routine
and use something like *pE
as mentioned in that Documentation under "Raw buffer as an escaped string:"
--
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
^ permalink raw reply [flat|nested] 9+ messages in thread