* [PATCH 1/3] selinux: Implement LSM notification system
@ 2017-04-26 15:02 Sebastien Buisson
2017-04-26 15:02 ` [PATCH 2/3] selinux: add checksum to policydb Sebastien Buisson
` (2 more replies)
0 siblings, 3 replies; 20+ messages in thread
From: Sebastien Buisson @ 2017-04-26 15:02 UTC (permalink / raw)
To: linux-security-module, linux-kernel, selinux
Cc: serge, james.l.morris, eparis, sds, paul, danielj, Sebastien Buisson
From: Daniel Jurgens <danielj@mellanox.com>
Add a generic notification mechanism in the LSM. Interested consumers
can register a callback with the LSM and security modules can produce
events.
Add a call to the notification mechanism from SELinux when the AVC
cache changes.
Signed-off-by: Daniel Jurgens <danielj@mellanox.com>
Signed-off-by: Sebastien Buisson <sbuisson@ddn.com>
---
include/linux/security.h | 23 +++++++++++++++++++++++
security/security.c | 20 ++++++++++++++++++++
security/selinux/hooks.c | 12 ++++++++++++
3 files changed, 55 insertions(+)
diff --git a/include/linux/security.h b/include/linux/security.h
index af675b5..73a9c93 100644
--- a/include/linux/security.h
+++ b/include/linux/security.h
@@ -68,6 +68,10 @@
struct user_namespace;
struct timezone;
+enum lsm_event {
+ LSM_POLICY_CHANGE,
+};
+
/* These functions are in security/commoncap.c */
extern int cap_capable(const struct cred *cred, struct user_namespace *ns,
int cap, int audit);
@@ -163,6 +167,10 @@ struct security_mnt_opts {
int num_mnt_opts;
};
+int call_lsm_notifier(enum lsm_event event, void *data);
+int register_lsm_notifier(struct notifier_block *nb);
+int unregister_lsm_notifier(struct notifier_block *nb);
+
static inline void security_init_mnt_opts(struct security_mnt_opts *opts)
{
opts->mnt_opts = NULL;
@@ -381,6 +389,21 @@ int security_sem_semop(struct sem_array *sma, struct sembuf *sops,
struct security_mnt_opts {
};
+static inline int call_lsm_notifier(enum lsm_event event, void *data)
+{
+ return 0;
+}
+
+static inline int register_lsm_notifier(struct notifier_block *nb)
+{
+ return 0;
+}
+
+static inline int unregister_lsm_notifier(struct notifier_block *nb)
+{
+ return 0;
+}
+
static inline void security_init_mnt_opts(struct security_mnt_opts *opts)
{
}
diff --git a/security/security.c b/security/security.c
index b9fea39..ef9d9e1 100644
--- a/security/security.c
+++ b/security/security.c
@@ -32,6 +32,8 @@
/* Maximum number of letters for an LSM name string */
#define SECURITY_NAME_MAX 10
+static ATOMIC_NOTIFIER_HEAD(lsm_notifier_chain);
+
struct security_hook_heads security_hook_heads __lsm_ro_after_init;
char *lsm_names;
/* Boot-time LSM user choice */
@@ -146,6 +148,24 @@ void __init security_add_hooks(struct security_hook_list *hooks, int count,
panic("%s - Cannot get early memory.\n", __func__);
}
+int call_lsm_notifier(enum lsm_event event, void *data)
+{
+ return atomic_notifier_call_chain(&lsm_notifier_chain, event, data);
+}
+EXPORT_SYMBOL(call_lsm_notifier);
+
+int register_lsm_notifier(struct notifier_block *nb)
+{
+ return atomic_notifier_chain_register(&lsm_notifier_chain, nb);
+}
+EXPORT_SYMBOL(register_lsm_notifier);
+
+int unregister_lsm_notifier(struct notifier_block *nb)
+{
+ return atomic_notifier_chain_unregister(&lsm_notifier_chain, nb);
+}
+EXPORT_SYMBOL(unregister_lsm_notifier);
+
/*
* Hook list operation macros.
*
diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
index e67a526..a4d36f8 100644
--- a/security/selinux/hooks.c
+++ b/security/selinux/hooks.c
@@ -171,6 +171,14 @@ static int selinux_netcache_avc_callback(u32 event)
return 0;
}
+static int selinux_lsm_notifier_avc_callback(u32 event)
+{
+ if (event == AVC_CALLBACK_RESET)
+ call_lsm_notifier(LSM_POLICY_CHANGE, NULL);
+
+ return 0;
+}
+
/*
* initialise the security for the init task
*/
@@ -6379,6 +6387,10 @@ static __init int selinux_init(void)
if (avc_add_callback(selinux_netcache_avc_callback, AVC_CALLBACK_RESET))
panic("SELinux: Unable to register AVC netcache callback\n");
+ if (avc_add_callback(selinux_lsm_notifier_avc_callback,
+ AVC_CALLBACK_RESET))
+ panic("SELinux: Unable to register AVC LSM notifier callback\n");
+
if (selinux_enforcing)
printk(KERN_DEBUG "SELinux: Starting in enforcing mode\n");
else
--
1.8.3.1
^ permalink raw reply related [flat|nested] 20+ messages in thread* [PATCH 2/3] selinux: add checksum to policydb 2017-04-26 15:02 [PATCH 1/3] selinux: Implement LSM notification system Sebastien Buisson @ 2017-04-26 15:02 ` Sebastien Buisson 2017-04-26 18:30 ` Stephen Smalley 2017-04-26 15:02 ` [PATCH 3/3] selinux: expose policy SHA256 checksum via selinuxfs Sebastien Buisson 2017-04-26 15:38 ` [PATCH 1/3] selinux: Implement LSM notification system Casey Schaufler 2 siblings, 1 reply; 20+ messages in thread From: Sebastien Buisson @ 2017-04-26 15:02 UTC (permalink / raw) To: linux-security-module, linux-kernel, selinux Cc: serge, james.l.morris, eparis, sds, paul, danielj, Sebastien Buisson Add policycksum field to struct policydb. It holds the sha256 checksum computed on the binary policy every time the notifier is called after a policy change. Add security_policy_cksum hook to give access to policy checksum to the rest of the kernel. Lustre client makes use of this information. Signed-off-by: Sebastien Buisson <sbuisson@ddn.com> --- include/linux/lsm_hooks.h | 2 + include/linux/security.h | 7 +++ security/security.c | 6 +++ security/selinux/hooks.c | 12 ++++- security/selinux/include/security.h | 2 + security/selinux/ss/policydb.h | 4 ++ security/selinux/ss/services.c | 91 +++++++++++++++++++++++++++++++++++++ 7 files changed, 123 insertions(+), 1 deletion(-) diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h index 080f34e..7aada73 100644 --- a/include/linux/lsm_hooks.h +++ b/include/linux/lsm_hooks.h @@ -1568,6 +1568,7 @@ int (*inode_setsecctx)(struct dentry *dentry, void *ctx, u32 ctxlen); int (*inode_getsecctx)(struct inode *inode, void **ctx, u32 *ctxlen); + ssize_t (*policy_cksum)(char *cksum, size_t len); #ifdef CONFIG_SECURITY_NETWORK int (*unix_stream_connect)(struct sock *sock, struct sock *other, struct sock *newsk); @@ -1813,6 +1814,7 @@ struct security_hook_heads { struct list_head inode_notifysecctx; struct list_head inode_setsecctx; struct list_head inode_getsecctx; + struct list_head policy_cksum; #ifdef CONFIG_SECURITY_NETWORK struct list_head unix_stream_connect; struct list_head unix_may_send; diff --git a/include/linux/security.h b/include/linux/security.h index 73a9c93..8461d31 100644 --- a/include/linux/security.h +++ b/include/linux/security.h @@ -385,6 +385,8 @@ int security_sem_semop(struct sem_array *sma, struct sembuf *sops, int security_inode_notifysecctx(struct inode *inode, void *ctx, u32 ctxlen); int security_inode_setsecctx(struct dentry *dentry, void *ctx, u32 ctxlen); int security_inode_getsecctx(struct inode *inode, void **ctx, u32 *ctxlen); + +ssize_t security_policy_cksum(char *cksum, size_t len); #else /* CONFIG_SECURITY */ struct security_mnt_opts { }; @@ -1189,6 +1191,11 @@ static inline int security_inode_getsecctx(struct inode *inode, void **ctx, u32 { return -EOPNOTSUPP; } + +static inline ssize_t security_policy_cksum(char *cksum, size_t len) +{ + return -EOPNOTSUPP; +} #endif /* CONFIG_SECURITY */ #ifdef CONFIG_SECURITY_NETWORK diff --git a/security/security.c b/security/security.c index ef9d9e1..a82c08c 100644 --- a/security/security.c +++ b/security/security.c @@ -1305,6 +1305,12 @@ int security_inode_getsecctx(struct inode *inode, void **ctx, u32 *ctxlen) } EXPORT_SYMBOL(security_inode_getsecctx); +ssize_t security_policy_cksum(char *cksum, size_t len) +{ + return call_int_hook(policy_cksum, -EOPNOTSUPP, cksum, len); +} +EXPORT_SYMBOL(security_policy_cksum); + #ifdef CONFIG_SECURITY_NETWORK int security_unix_stream_connect(struct sock *sock, struct sock *other, struct sock *newsk) diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c index a4d36f8..3759198 100644 --- a/security/selinux/hooks.c +++ b/security/selinux/hooks.c @@ -173,8 +173,11 @@ static int selinux_netcache_avc_callback(u32 event) static int selinux_lsm_notifier_avc_callback(u32 event) { - if (event == AVC_CALLBACK_RESET) + if (event == AVC_CALLBACK_RESET) { + if (security_policydb_compute_cksum() != 0) + printk(KERN_ERR "Failed to compute policydb cksum\n"); call_lsm_notifier(LSM_POLICY_CHANGE, NULL); + } return 0; } @@ -6071,6 +6074,11 @@ static int selinux_inode_getsecctx(struct inode *inode, void **ctx, u32 *ctxlen) *ctxlen = len; return 0; } + +static ssize_t selinux_policy_cksum(char *cksum, size_t len) +{ + return security_policydb_cksum(cksum, len); +} #ifdef CONFIG_KEYS static int selinux_key_alloc(struct key *k, const struct cred *cred, @@ -6285,6 +6293,8 @@ static int selinux_key_getsecurity(struct key *key, char **_buffer) LSM_HOOK_INIT(inode_setsecctx, selinux_inode_setsecctx), LSM_HOOK_INIT(inode_getsecctx, selinux_inode_getsecctx), + LSM_HOOK_INIT(policy_cksum, selinux_policy_cksum), + LSM_HOOK_INIT(unix_stream_connect, selinux_socket_unix_stream_connect), LSM_HOOK_INIT(unix_may_send, selinux_socket_unix_may_send), diff --git a/security/selinux/include/security.h b/security/selinux/include/security.h index f979c35..a329571 100644 --- a/security/selinux/include/security.h +++ b/security/selinux/include/security.h @@ -97,6 +97,8 @@ enum { int security_load_policy(void *data, size_t len); int security_read_policy(void **data, size_t *len); size_t security_policydb_len(void); +ssize_t security_policydb_cksum(char *cksum, size_t len); +int security_policydb_compute_cksum(void); int security_policycap_supported(unsigned int req_cap); diff --git a/security/selinux/ss/policydb.h b/security/selinux/ss/policydb.h index 725d594..dc29492 100644 --- a/security/selinux/ss/policydb.h +++ b/security/selinux/ss/policydb.h @@ -25,6 +25,7 @@ #define _SS_POLICYDB_H_ #include <linux/flex_array.h> +#include <crypto/sha.h> #include "symtab.h" #include "avtab.h" @@ -293,6 +294,9 @@ struct policydb { size_t len; unsigned int policyvers; + /* checksum computed on the policy */ + unsigned char policycksum[SHA256_DIGEST_SIZE*2 + 1]; + size_t policycksum_len; unsigned int reject_unknown : 1; unsigned int allow_unknown : 1; diff --git a/security/selinux/ss/services.c b/security/selinux/ss/services.c index 60d9b02..a35d294 100644 --- a/security/selinux/ss/services.c +++ b/security/selinux/ss/services.c @@ -53,6 +53,8 @@ #include <linux/flex_array.h> #include <linux/vmalloc.h> #include <net/netlabel.h> +#include <crypto/hash.h> +#include <crypto/sha.h> #include "flask.h" #include "avc.h" @@ -2170,6 +2172,95 @@ size_t security_policydb_len(void) } /** + * security_policydb_cksum - Get policydb checksum. + * @cksum: string to store checksum to + * @len: length of checksum + */ +ssize_t security_policydb_cksum(char *cksum, size_t len) +{ + int rc; + + read_lock(&policy_rwlock); + if (strlcpy(cksum, policydb.policycksum, len) >= len) + rc = -ENAMETOOLONG; + rc = policydb.policycksum_len; + read_unlock(&policy_rwlock); + + return rc; +} + +/** + * security_policydb_compute_cksum - Compute checksum of a policy database. + */ +int security_policydb_compute_cksum(void) +{ + struct crypto_ahash *tfm; + struct ahash_request *req; + struct scatterlist sl; + char hashval[SHA256_DIGEST_SIZE]; + int idx; + unsigned char *p; + size_t len; + void *data; + int rc; + + rc = security_read_policy(&data, &len); + if (rc) { + printk(KERN_ERR "Failed to read security policy\n"); + return rc; + } + + tfm = crypto_alloc_ahash("sha256", 0, CRYPTO_ALG_ASYNC); + if (IS_ERR(tfm)) { + printk(KERN_ERR "Failed to alloc crypto hash sha256\n"); + vfree(data); + rc = PTR_ERR(tfm); + return rc; + } + + req = ahash_request_alloc(tfm, GFP_KERNEL); + if (!req) { + printk(KERN_ERR "Failed to alloc ahash_request for sha256\n"); + crypto_free_ahash(tfm); + vfree(data); + rc = -ENOMEM; + return rc; + } + + ahash_request_set_callback(req, 0, NULL, NULL); + + rc = crypto_ahash_init(req); + if (rc) { + printk(KERN_ERR "Failed to init ahash\n"); + ahash_request_free(req); + crypto_free_ahash(tfm); + vfree(data); + return rc; + } + + sg_init_one(&sl, (void *)data, len); + ahash_request_set_crypt(req, &sl, hashval, sl.length); + rc = crypto_ahash_digest(req); + + crypto_free_ahash(tfm); + ahash_request_free(req); + vfree(data); + if (rc) { + printk(KERN_ERR "Failed to compute digest\n"); + return rc; + } + + p = policydb.policycksum; + for (idx = 0; idx < SHA256_DIGEST_SIZE; idx++) { + snprintf(p, 3, "%02x", (unsigned char)(hashval[idx])); + p += 2; + } + policydb.policycksum_len = (size_t)(p - policydb.policycksum); + + return 0; +} + +/** * security_port_sid - Obtain the SID for a port. * @protocol: protocol number * @port: port number -- 1.8.3.1 ^ permalink raw reply related [flat|nested] 20+ messages in thread
* Re: [PATCH 2/3] selinux: add checksum to policydb 2017-04-26 15:02 ` [PATCH 2/3] selinux: add checksum to policydb Sebastien Buisson @ 2017-04-26 18:30 ` Stephen Smalley 2017-04-27 8:41 ` Sebastien Buisson 0 siblings, 1 reply; 20+ messages in thread From: Stephen Smalley @ 2017-04-26 18:30 UTC (permalink / raw) To: Sebastien Buisson, linux-security-module, linux-kernel, selinux Cc: serge, james.l.morris, eparis, paul, danielj, Sebastien Buisson On Thu, 2017-04-27 at 00:02 +0900, Sebastien Buisson wrote: > Add policycksum field to struct policydb. It holds the sha256 > checksum computed on the binary policy every time the notifier is > called after a policy change. > Add security_policy_cksum hook to give access to policy checksum to > the rest of the kernel. Lustre client makes use of this information. > > Signed-off-by: Sebastien Buisson <sbuisson@ddn.com> > --- > include/linux/lsm_hooks.h | 2 + > include/linux/security.h | 7 +++ > security/security.c | 6 +++ > security/selinux/hooks.c | 12 ++++- > security/selinux/include/security.h | 2 + > security/selinux/ss/policydb.h | 4 ++ > security/selinux/ss/services.c | 91 > +++++++++++++++++++++++++++++++++++++ > 7 files changed, 123 insertions(+), 1 deletion(-) > > diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c > index a4d36f8..3759198 100644 > --- a/security/selinux/hooks.c > +++ b/security/selinux/hooks.c > @@ -173,8 +173,11 @@ static int selinux_netcache_avc_callback(u32 > event) > > static int selinux_lsm_notifier_avc_callback(u32 event) > { > - if (event == AVC_CALLBACK_RESET) > + if (event == AVC_CALLBACK_RESET) { > + if (security_policydb_compute_cksum() != 0) > + printk(KERN_ERR "Failed to compute policydb > cksum\n"); This seems like an odd place to trigger the computation. Why aren't you just computing it when the policy is loaded directly in security_load_policy()? You already have the (data, len) on entry to that function. Just compute it at load time, save it, and be done. No need for a notifier then for your use case unless I am missing something. I suppose the question is which checksum do you want - the hash of the policy file that was written to /sys/fs/selinux/load by userspace, or the hash of the policy file that the kernel generates on demand if you open /sys/fs/selinux/policy. Those can differ in non-semantic ways due to ordering differences, for example. I think the former is more likely to be of interest to userspace (e.g. to compare the hash value against the hash of the policy file), and is cheaper since you already have a (data, len) pair on entry to security_load_policy() that you can hash immediately rather than requiring the kernel to regenerate the image from the policydb. > call_lsm_notifier(LSM_POLICY_CHANGE, NULL); > + } > > return 0; > } > > diff --git a/security/selinux/ss/services.c > b/security/selinux/ss/services.c > index 60d9b02..a35d294 100644 > --- a/security/selinux/ss/services.c > +++ b/security/selinux/ss/services.c > @@ -53,6 +53,8 @@ > #include <linux/flex_array.h> > #include <linux/vmalloc.h> > #include <net/netlabel.h> > +#include <crypto/hash.h> > +#include <crypto/sha.h> > > #include "flask.h" > #include "avc.h" > @@ -2170,6 +2172,95 @@ size_t security_policydb_len(void) > } > > /** > + * security_policydb_cksum - Get policydb checksum. > + * @cksum: string to store checksum to > + * @len: length of checksum > + */ > +ssize_t security_policydb_cksum(char *cksum, size_t len) > +{ > + int rc; > + > + read_lock(&policy_rwlock); > + if (strlcpy(cksum, policydb.policycksum, len) >= len) > + rc = -ENAMETOOLONG; > + rc = policydb.policycksum_len; Obviously you'll clobber -ENAMETOOLONG here. > + read_unlock(&policy_rwlock); > + > + return rc; > +} You are requiring all callers to know that they are dealing with a sha256 hash string in order to provide an adequately sized buffer. So we either ought to make that evident in the interface, or make the interface more flexible/general. The latter is imho preferable. We could simply allocate a buffer of the right length and return it, like selinux_inode_getsecurity() does. > + > +/** > + * security_policydb_compute_cksum - Compute checksum of a policy > database. > + */ > +int security_policydb_compute_cksum(void) > +{ > + struct crypto_ahash *tfm; > + struct ahash_request *req; > + struct scatterlist sl; > + char hashval[SHA256_DIGEST_SIZE]; > + int idx; > + unsigned char *p; > + size_t len; > + void *data; > + int rc; > + > + rc = security_read_policy(&data, &len); > + if (rc) { > + printk(KERN_ERR "Failed to read security policy\n"); > + return rc; > + } This requires regenerating the policy image from the policydb; simpler if we can just hash what we were given in security_load_policy() and save it at that time. > + > + tfm = crypto_alloc_ahash("sha256", 0, CRYPTO_ALG_ASYNC); Why are you using the async interface? > + if (IS_ERR(tfm)) { > + printk(KERN_ERR "Failed to alloc crypto hash > sha256\n"); > + vfree(data); > + rc = PTR_ERR(tfm); > + return rc; > + } > + > + req = ahash_request_alloc(tfm, GFP_KERNEL); > + if (!req) { > + printk(KERN_ERR "Failed to alloc ahash_request for > sha256\n"); > + crypto_free_ahash(tfm); > + vfree(data); > + rc = -ENOMEM; > + return rc; > + } > + > + ahash_request_set_callback(req, 0, NULL, NULL); > + > + rc = crypto_ahash_init(req); > + if (rc) { > + printk(KERN_ERR "Failed to init ahash\n"); > + ahash_request_free(req); > + crypto_free_ahash(tfm); > + vfree(data); > + return rc; > + } > + > + sg_init_one(&sl, (void *)data, len); > + ahash_request_set_crypt(req, &sl, hashval, sl.length); > + rc = crypto_ahash_digest(req); > + > + crypto_free_ahash(tfm); > + ahash_request_free(req); > + vfree(data); > + if (rc) { > + printk(KERN_ERR "Failed to compute digest\n"); > + return rc; > + } > + > + p = policydb.policycksum; > + for (idx = 0; idx < SHA256_DIGEST_SIZE; idx++) { > + snprintf(p, 3, "%02x", (unsigned > char)(hashval[idx])); > + p += 2; > + } > + policydb.policycksum_len = (size_t)(p - > policydb.policycksum); > + > + return 0; > +} > + > +/** > * security_port_sid - Obtain the SID for a port. > * @protocol: protocol number > * @port: port number ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 2/3] selinux: add checksum to policydb 2017-04-26 18:30 ` Stephen Smalley @ 2017-04-27 8:41 ` Sebastien Buisson 2017-04-27 15:18 ` Stephen Smalley 0 siblings, 1 reply; 20+ messages in thread From: Sebastien Buisson @ 2017-04-27 8:41 UTC (permalink / raw) To: Stephen Smalley Cc: linux-security-module, linux-kernel, selinux, serge, james.l.morris, Eric Paris, Paul Moore, Daniel Jurgens, Sebastien Buisson 2017-04-26 20:30 GMT+02:00 Stephen Smalley <sds@tycho.nsa.gov>: > This seems like an odd place to trigger the computation. I noticed that the policy as exposed via /sys/fs/selinux/policy can also be modified in security_set_bools(). So in order to limit the places from where to compute the policy checksum, I moved the call to checksum computation to selinux_lsm_notifier_avc_callback(). That being said, maybe the hash of /sys/fs/selinux/policy is not the checksum we want. See your comments and my answers below. > Why aren't you > just computing it when the policy is loaded directly in > security_load_policy()? You already have the (data, len) on entry to > that function. Just compute it at load time, save it, and be done. No > need for a notifier then for your use case unless I am missing > something. You are right. Getting from the Lustre client code the SELinux internally computed checksum is cheap, so no need to be notified every time the policy changes, and no need to store the checksum in Lustre at that time. I will drop the "Implement LSM notification system" patch from this series, as I cannot justify its usefulness from a Lustre client standpoint anymore. > I suppose the question is which checksum do you want - the hash of the > policy file that was written to /sys/fs/selinux/load by userspace, or > the hash of the policy file that the kernel generates on demand if you > open /sys/fs/selinux/policy. Those can differ in non-semantic ways due > to ordering differences, for example. I think the former is more > likely to be of interest to userspace (e.g. to compare the hash value > against the hash of the policy file), and is cheaper since you already > have a (data, len) pair on entry to security_load_policy() that you can > hash immediately rather than requiring the kernel to regenerate the > image from the policydb. OK, I understand now why I was seeing differences between the checksum computed on a (data, len) pair on entry to security_load_policy(), and the checksum computed on a (data, len) pair got from security_read_policy(). I thought it was a problem to have a difference between the internally computed checksum and the one a user can get by calling sha256sum on /sys/fs/selinux/policy. But now I see it makes sense to reflect what was loaded by userspace. So I will simplify this patch accordingly. ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 2/3] selinux: add checksum to policydb 2017-04-27 8:41 ` Sebastien Buisson @ 2017-04-27 15:18 ` Stephen Smalley 2017-04-27 17:12 ` Sebastien Buisson 0 siblings, 1 reply; 20+ messages in thread From: Stephen Smalley @ 2017-04-27 15:18 UTC (permalink / raw) To: Sebastien Buisson Cc: linux-security-module, linux-kernel, selinux, serge, james.l.morris, Eric Paris, Paul Moore, Daniel Jurgens, Sebastien Buisson On Thu, 2017-04-27 at 10:41 +0200, Sebastien Buisson wrote: > 2017-04-26 20:30 GMT+02:00 Stephen Smalley <sds@tycho.nsa.gov>: > > This seems like an odd place to trigger the computation. > > I noticed that the policy as exposed via /sys/fs/selinux/policy can > also be modified in security_set_bools(). That's true, but does that matter for your use case? Do you care about non-persistent boolean changes? What is the property you want to ensure? > So in order to limit the > places from where to compute the policy checksum, I moved the call to > checksum computation to selinux_lsm_notifier_avc_callback(). > That being said, maybe the hash of /sys/fs/selinux/policy is not the > checksum we want. See your comments and my answers below. > > > Why aren't you > > just computing it when the policy is loaded directly in > > security_load_policy()? You already have the (data, len) on entry > > to > > that function. Just compute it at load time, save it, and be > > done. No > > need for a notifier then for your use case unless I am missing > > something. > > You are right. Getting from the Lustre client code the SELinux > internally computed checksum is cheap, so no need to be notified > every > time the policy changes, and no need to store the checksum in Lustre > at that time. > I will drop the "Implement LSM notification system" patch from this > series, as I cannot justify its usefulness from a Lustre client > standpoint anymore. > > > I suppose the question is which checksum do you want - the hash of > > the > > policy file that was written to /sys/fs/selinux/load by userspace, > > or > > the hash of the policy file that the kernel generates on demand if > > you > > open /sys/fs/selinux/policy. Those can differ in non-semantic ways > > due > > to ordering differences, for example. I think the former is more > > likely to be of interest to userspace (e.g. to compare the hash > > value > > against the hash of the policy file), and is cheaper since you > > already > > have a (data, len) pair on entry to security_load_policy() that you > > can > > hash immediately rather than requiring the kernel to regenerate the > > image from the policydb. > > OK, I understand now why I was seeing differences between the > checksum > computed on a (data, len) pair on entry to security_load_policy(), > and > the checksum computed on a (data, len) pair got from > security_read_policy(). > I thought it was a problem to have a difference between the > internally > computed checksum and the one a user can get by calling sha256sum on > /sys/fs/selinux/policy. But now I see it makes sense to reflect what > was loaded by userspace. So I will simplify this patch accordingly. Ok, that should work as long as you just want to validate that all the clients loaded the same policy file, and aren't concerned about non- persistent policy boolean changes. You needed to get (global) enforcing mode too, didn't you? That's separate from the policy. Make sure you make the hash algorithm explicit in both what is returned by the hook to lustre and by what is exported via selinuxfs. Can likely just encode the hash algorithm name in the string when you generate it. ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 2/3] selinux: add checksum to policydb 2017-04-27 15:18 ` Stephen Smalley @ 2017-04-27 17:12 ` Sebastien Buisson 2017-04-27 18:47 ` Stephen Smalley 0 siblings, 1 reply; 20+ messages in thread From: Sebastien Buisson @ 2017-04-27 17:12 UTC (permalink / raw) To: Stephen Smalley Cc: linux-security-module, linux-kernel, selinux, serge, james.l.morris, Eric Paris, Paul Moore, Daniel Jurgens, Sebastien Buisson 2017-04-27 17:18 GMT+02:00 Stephen Smalley <sds@tycho.nsa.gov>: > Ok, that should work as long as you just want to validate that all the > clients loaded the same policy file, and aren't concerned about non- > persistent policy boolean changes. As far as I understand, non-persistent policy boolean changes can affect the way the policy is enforced. So that is a problem if the checksum does not reflect it. We want to protect against someone tampering the policy locally on a Lustre client, even if it does not survive a reboot. I just checked, with the method of computing the checksum on a (data, len) pair on entry to security_load_policy() the checksum does not change after using setsebool. So it seems I would need to call security_read_policy() to retrieve the binary representation of the policy as currently enforced by the kernel. Unless you can see another way? > You needed to get (global) enforcing mode too, didn't you? That's > separate from the policy. Exactly, I also need to rework the patch I proposed about this, in light of the comments I received. > Make sure you make the hash algorithm explicit in both what is returned > by the hook to lustre and by what is exported via selinuxfs. Can > likely just encode the hash algorithm name in the string when you > generate it. Sure, I will add "sha256:" at the beginning of the string. ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 2/3] selinux: add checksum to policydb 2017-04-27 17:12 ` Sebastien Buisson @ 2017-04-27 18:47 ` Stephen Smalley 2017-04-28 15:16 ` Sebastien Buisson 0 siblings, 1 reply; 20+ messages in thread From: Stephen Smalley @ 2017-04-27 18:47 UTC (permalink / raw) To: Sebastien Buisson Cc: linux-security-module, linux-kernel, selinux, serge, james.l.morris, Eric Paris, Paul Moore, Daniel Jurgens, Sebastien Buisson On Thu, 2017-04-27 at 19:12 +0200, Sebastien Buisson wrote: > 2017-04-27 17:18 GMT+02:00 Stephen Smalley <sds@tycho.nsa.gov>: > > Ok, that should work as long as you just want to validate that all > > the > > clients loaded the same policy file, and aren't concerned about > > non- > > persistent policy boolean changes. > > As far as I understand, non-persistent policy boolean changes can > affect the way the policy is enforced. So that is a problem if the > checksum does not reflect it. We want to protect against someone > tampering the policy locally on a Lustre client, even if it does not > survive a reboot. A boolean change can affect which TE rules in the policy are enabled/disabled, but only in ways that are defined by the original policy. You can't add arbitrary TE rules that way, just enable/disable blocks that were defined conditionally in the policy. It also has no effect on MLS enforcement, for example. So it depends on your goals. > I just checked, with the method of computing the checksum on a (data, > len) pair on entry to security_load_policy() the checksum does not > change after using setsebool. So it seems I would need to call > security_read_policy() to retrieve the binary representation of the > policy as currently enforced by the kernel. Unless you can see > another > way? I don't think that's a viable option, since security_read_policy() is going to be expensive in order to generate a full policy image, while security_set_bools() is supposed to be substantially cheaper than a full policy load. Also, the advantage of taking the hash of the original input file is that you can independently compute a reference hash offline or on the server from the same policy file and compare them and you can identify which policy file was loaded based on the hash. If you care about the active boolean state, then I'd suggest hashing the active boolean state separately and storing that after the policy hash. You can do that in both security_load_policy() and security_set_bools(). Just iterate through the bools like security_set_bools() does, write the ->state of each boolean into a buffer, and then hash that buffer. > > You needed to get (global) enforcing mode too, didn't you? That's > > separate from the policy. > > Exactly, I also need to rework the patch I proposed about this, in > light of the comments I received. So perhaps what you really want is a hook interface and a selinuxfs interface that returns a single string that encodes all of the policy properties that you care about? Rather than separate hooks and interfaces? You could embed the enforcing status in the string too. Should probably include checkreqprot as well since that affects enforcement of mmap/mprotect checks. > > Make sure you make the hash algorithm explicit in both what is > > returned > > by the hook to lustre and by what is exported via selinuxfs. Can > > likely just encode the hash algorithm name in the string when you > > generate it. > > Sure, I will add "sha256:" at the beginning of the string. ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 2/3] selinux: add checksum to policydb 2017-04-27 18:47 ` Stephen Smalley @ 2017-04-28 15:16 ` Sebastien Buisson 2017-04-28 15:50 ` Stephen Smalley 0 siblings, 1 reply; 20+ messages in thread From: Sebastien Buisson @ 2017-04-28 15:16 UTC (permalink / raw) To: Stephen Smalley Cc: linux-security-module, linux-kernel, selinux, serge, james.l.morris, Eric Paris, Paul Moore, Daniel Jurgens, Sebastien Buisson 2017-04-27 20:47 GMT+02:00 Stephen Smalley <sds@tycho.nsa.gov>: >> I just checked, with the method of computing the checksum on a (data, >> len) pair on entry to security_load_policy() the checksum does not >> change after using setsebool. So it seems I would need to call >> security_read_policy() to retrieve the binary representation of the >> policy as currently enforced by the kernel. Unless you can see >> another >> way? > > I don't think that's a viable option, since security_read_policy() is > going to be expensive in order to generate a full policy image, while > security_set_bools() is supposed to be substantially cheaper than a > full policy load. > > Also, the advantage of taking the hash of the original input file is > that you can independently compute a reference hash offline or on the > server from the same policy file and compare them and you can identify > which policy file was loaded based on the hash. > > If you care about the active boolean state, then I'd suggest hashing > the active boolean state separately and storing that after the policy > hash. You can do that in both security_load_policy() and > security_set_bools(). Just iterate through the bools like > security_set_bools() does, write the ->state of each boolean into a > buffer, and then hash that buffer. I just noticed another issue: with the method of computing the checksum on a (data, len) pair on entry to security_load_policy(), the checksum does not change after inserting a new module with semodule. It is a problem as a module can allow actions by certain users on some file contexts. So not detecting that kind of policy tampering defeats the purpose of the checksum as I imagine it. To address this I propose to come back to the idea of the notifier. The checksum would not be stored inside the struct policydb. The checksum would be computed on a (data, len) pair got from security_read_policy() every time someone is asking for it through the security_policy_cksum() hook. The ones that would potentially call security_policy_cksum() are those that would register a callback on lsm_notifier, and the userspace processes reading /sys/fs/selinux/policycksum. So no matter if computing the checksum gets expensive, that would be the caller's responsibility to use it with care. Just like with /sys/fs/selinux/policy today in fact. >> > You needed to get (global) enforcing mode too, didn't you? That's >> > separate from the policy. >> >> Exactly, I also need to rework the patch I proposed about this, in >> light of the comments I received. > > So perhaps what you really want is a hook interface and a selinuxfs > interface that returns a single string that encodes all of the policy > properties that you care about? Rather than separate hooks and > interfaces? You could embed the enforcing status in the string too. > Should probably include checkreqprot as well since that affects > enforcement of mmap/mprotect checks. True, I should build a string of the form: <0 or 1 for enforce>:<0 or 1 for checkreqprot>:<hashalg>=<global checksum> I should probably rename it 'policybrief' instead of 'policycksum'. I realize that the 'SELinux user to UNIX user' assignments are important as well. If for instance a regular user on a given cluster node is mapped to unconfined_u instead of user_u, this user would erroneously have major privileges. I do not know where I should look for this information, and possibly compute another checksum. ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 2/3] selinux: add checksum to policydb 2017-04-28 15:16 ` Sebastien Buisson @ 2017-04-28 15:50 ` Stephen Smalley 2017-04-28 16:08 ` Sebastien Buisson 0 siblings, 1 reply; 20+ messages in thread From: Stephen Smalley @ 2017-04-28 15:50 UTC (permalink / raw) To: Sebastien Buisson Cc: linux-security-module, linux-kernel, selinux, serge, james.l.morris, Eric Paris, Paul Moore, Daniel Jurgens, Sebastien Buisson On Fri, 2017-04-28 at 17:16 +0200, Sebastien Buisson wrote: > 2017-04-27 20:47 GMT+02:00 Stephen Smalley <sds@tycho.nsa.gov>: > > > I just checked, with the method of computing the checksum on a > > > (data, > > > len) pair on entry to security_load_policy() the checksum does > > > not > > > change after using setsebool. So it seems I would need to call > > > security_read_policy() to retrieve the binary representation of > > > the > > > policy as currently enforced by the kernel. Unless you can see > > > another > > > way? > > > > I don't think that's a viable option, since security_read_policy() > > is > > going to be expensive in order to generate a full policy image, > > while > > security_set_bools() is supposed to be substantially cheaper than a > > full policy load. > > > > Also, the advantage of taking the hash of the original input file > > is > > that you can independently compute a reference hash offline or on > > the > > server from the same policy file and compare them and you can > > identify > > which policy file was loaded based on the hash. > > > > If you care about the active boolean state, then I'd suggest > > hashing > > the active boolean state separately and storing that after the > > policy > > hash. You can do that in both security_load_policy() and > > security_set_bools(). Just iterate through the bools like > > security_set_bools() does, write the ->state of each boolean into a > > buffer, and then hash that buffer. > > I just noticed another issue: with the method of computing the > checksum on a (data, len) pair on entry to security_load_policy(), > the > checksum does not change after inserting a new module with semodule. > It is a problem as a module can allow actions by certain users on > some > file contexts. So not detecting that kind of policy tampering defeats > the purpose of the checksum as I imagine it. You seem to be conflating kernel policy with userspace policy. security_load_policy() is provided with the kernel policy image, which is the result of linking the kernel-relevant portions of all policy modules together. A hash of that image will change if you insert a policy module that affects the kernel policy in any way. But a change that only affects userspace policy isn't ever going to be reflected in the kernel. It doesn't matter where or when you compute your checksum within the kernel; it isn't ever going to reflect those userspace policy changes. > To address this I propose to come back to the idea of the notifier. > The checksum would not be stored inside the struct policydb. The > checksum would be computed on a (data, len) pair got from > security_read_policy() every time someone is asking for it through > the > security_policy_cksum() hook. The ones that would potentially call > security_policy_cksum() are those that would register a callback on > lsm_notifier, and the userspace processes reading > /sys/fs/selinux/policycksum. So no matter if computing the checksum > gets expensive, that would be the caller's responsibility to use it > with care. Just like with /sys/fs/selinux/policy today in fact. This won't detect changes to userspace policy configurations either, and it is less efficient than just computing/updating the checksum in security_load_policy() and security_set_bools(). Also, if all you want is a hash of /sys/fs/selinux/policy, then userspace can already read and hash that itself at any time. You aren't really providing any additional information that way. In contrast, saving and providing a hash of the policy image that was loaded is not something that is currently available, and could be useful in checking against a reference hash of the policy file or in identifying which policy file was loaded. > > > > You needed to get (global) enforcing mode too, didn't > > > > you? That's > > > > separate from the policy. > > > > > > Exactly, I also need to rework the patch I proposed about this, > > > in > > > light of the comments I received. > > > > So perhaps what you really want is a hook interface and a selinuxfs > > interface that returns a single string that encodes all of the > > policy > > properties that you care about? Rather than separate hooks and > > interfaces? You could embed the enforcing status in the string > > too. > > Should probably include checkreqprot as well since that affects > > enforcement of mmap/mprotect checks. > > True, I should build a string of the form: > <0 or 1 for enforce>:<0 or 1 for checkreqprot>:<hashalg>=<global > checksum> > I should probably rename it 'policybrief' instead of 'policycksum'. > > I realize that the 'SELinux user to UNIX user' assignments are > important as well. If for instance a regular user on a given cluster > node is mapped to unconfined_u instead of user_u, this user would > erroneously have major privileges. I do not know where I should look > for this information, and possibly compute another checksum. As above, that's userspace policy configuration, and not something that kernel can or should deal with. ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 2/3] selinux: add checksum to policydb 2017-04-28 15:50 ` Stephen Smalley @ 2017-04-28 16:08 ` Sebastien Buisson 2017-04-28 16:38 ` Stephen Smalley 0 siblings, 1 reply; 20+ messages in thread From: Sebastien Buisson @ 2017-04-28 16:08 UTC (permalink / raw) To: Stephen Smalley Cc: linux-security-module, linux-kernel, selinux, serge, james.l.morris, Eric Paris, Paul Moore, Daniel Jurgens, Sebastien Buisson 2017-04-28 17:50 GMT+02:00 Stephen Smalley <sds@tycho.nsa.gov>: > You seem to be conflating kernel policy with userspace policy. > security_load_policy() is provided with the kernel policy image, which > is the result of linking the kernel-relevant portions of all policy > modules together. A hash of that image will change if you insert a > policy module that affects the kernel policy in any way. But a change > that only affects userspace policy isn't ever going to be reflected in > the kernel. It doesn't matter where or when you compute your checksum > within the kernel; it isn't ever going to reflect those userspace > policy changes. Here is the content of the module is used for my tests: #============= user_t ============== allow user_t mnt_t:dir { write add_name }; allow user_t mnt_t:file { write create }; After loading the .pp corresponding to it, I can see that with the method of computing the checksum on the (data, len) pair on entry to security_load_policy(), the checksum does not change. However, when using the (data, len) pair got from security_read_policy(), the checksum changes. And when I remove the module, the checksum is back to its previous value. So this is what makes me think there is a difference. Am I missing something? ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 2/3] selinux: add checksum to policydb 2017-04-28 16:08 ` Sebastien Buisson @ 2017-04-28 16:38 ` Stephen Smalley 0 siblings, 0 replies; 20+ messages in thread From: Stephen Smalley @ 2017-04-28 16:38 UTC (permalink / raw) To: Sebastien Buisson Cc: linux-security-module, linux-kernel, selinux, serge, james.l.morris, Eric Paris, Paul Moore, Daniel Jurgens, Sebastien Buisson On Fri, 2017-04-28 at 18:08 +0200, Sebastien Buisson wrote: > 2017-04-28 17:50 GMT+02:00 Stephen Smalley <sds@tycho.nsa.gov>: > > You seem to be conflating kernel policy with userspace policy. > > security_load_policy() is provided with the kernel policy image, > > which > > is the result of linking the kernel-relevant portions of all policy > > modules together. A hash of that image will change if you insert a > > policy module that affects the kernel policy in any way. But a > > change > > that only affects userspace policy isn't ever going to be reflected > > in > > the kernel. It doesn't matter where or when you compute your > > checksum > > within the kernel; it isn't ever going to reflect those userspace > > policy changes. > > Here is the content of the module is used for my tests: > > #============= user_t ============== > allow user_t mnt_t:dir { write add_name }; > allow user_t mnt_t:file { write create }; > > After loading the .pp corresponding to it, I can see that with the > method of computing the checksum on the (data, len) pair on entry to > security_load_policy(), the checksum does not change. However, when > using the (data, len) pair got from > security_read_policy(), the checksum changes. And when I remove the > module, the checksum is back to its previous value. > So this is what makes me think there is a difference. Am I missing > something? Policy is loaded via security_load_policy(), so the policy image has to go through it in the first place to be loaded (ignoring kernel exploits or direct /dev/mem access). You couldn't have loaded the modified policy with your new rules without the modified policy getting processed by security_load_policy(). So I'm assuming there is a bug in your code or your testing. ^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH 3/3] selinux: expose policy SHA256 checksum via selinuxfs 2017-04-26 15:02 [PATCH 1/3] selinux: Implement LSM notification system Sebastien Buisson 2017-04-26 15:02 ` [PATCH 2/3] selinux: add checksum to policydb Sebastien Buisson @ 2017-04-26 15:02 ` Sebastien Buisson 2017-04-26 18:31 ` Stephen Smalley 2017-04-26 15:38 ` [PATCH 1/3] selinux: Implement LSM notification system Casey Schaufler 2 siblings, 1 reply; 20+ messages in thread From: Sebastien Buisson @ 2017-04-26 15:02 UTC (permalink / raw) To: linux-security-module, linux-kernel, selinux Cc: serge, james.l.morris, eparis, sds, paul, danielj, Sebastien Buisson Expose policy SHA256 checksum via selinuxfs. Signed-off-by: Sebastien Buisson <sbuisson@ddn.com> --- security/selinux/selinuxfs.c | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/security/selinux/selinuxfs.c b/security/selinux/selinuxfs.c index ce71718..b2d5deb 100644 --- a/security/selinux/selinuxfs.c +++ b/security/selinux/selinuxfs.c @@ -30,6 +30,7 @@ #include <linux/uaccess.h> #include <linux/kobject.h> #include <linux/ctype.h> +#include <crypto/sha.h> /* selinuxfs pseudo filesystem for exporting the security policy API. Based on the proc code and the fs/nfsd/nfsctl.c code. */ @@ -99,6 +100,7 @@ enum sel_inos { SEL_STATUS, /* export current status using mmap() */ SEL_POLICY, /* allow userspace to read the in kernel policy */ SEL_VALIDATE_TRANS, /* compute validatetrans decision */ + SEL_POLICYCKSUM,/* return policy SHA256 checkum */ SEL_INO_NEXT, /* The next inode number to use */ }; @@ -313,6 +315,22 @@ static ssize_t sel_read_policyvers(struct file *filp, char __user *buf, .llseek = generic_file_llseek, }; +static ssize_t sel_read_policycksum(struct file *filp, char __user *buf, + size_t count, loff_t *ppos) +{ + size_t tmpbuflen = SHA256_DIGEST_SIZE*2 + 1; + char tmpbuf[tmpbuflen]; + ssize_t length; + + length = security_policydb_cksum(tmpbuf, tmpbuflen); + return simple_read_from_buffer(buf, count, ppos, tmpbuf, length); +} + +static const struct file_operations sel_policycksum_ops = { + .read = sel_read_policycksum, + .llseek = generic_file_llseek, +}; + /* declaration for sel_write_load */ static int sel_make_bools(void); static int sel_make_classes(void); @@ -1825,6 +1843,8 @@ static int sel_fill_super(struct super_block *sb, void *data, int silent) [SEL_POLICY] = {"policy", &sel_policy_ops, S_IRUGO}, [SEL_VALIDATE_TRANS] = {"validatetrans", &sel_transition_ops, S_IWUGO}, + [SEL_POLICYCKSUM] = {"policycksum", &sel_policycksum_ops, + S_IRUGO}, /* last one */ {""} }; ret = simple_fill_super(sb, SELINUX_MAGIC, selinux_files); -- 1.8.3.1 ^ permalink raw reply related [flat|nested] 20+ messages in thread
* Re: [PATCH 3/3] selinux: expose policy SHA256 checksum via selinuxfs 2017-04-26 15:02 ` [PATCH 3/3] selinux: expose policy SHA256 checksum via selinuxfs Sebastien Buisson @ 2017-04-26 18:31 ` Stephen Smalley 2017-04-27 1:08 ` James Morris 0 siblings, 1 reply; 20+ messages in thread From: Stephen Smalley @ 2017-04-26 18:31 UTC (permalink / raw) To: Sebastien Buisson, linux-security-module, linux-kernel, selinux Cc: Sebastien Buisson, james.l.morris On Thu, 2017-04-27 at 00:02 +0900, Sebastien Buisson wrote: > Expose policy SHA256 checksum via selinuxfs. > > Signed-off-by: Sebastien Buisson <sbuisson@ddn.com> > --- > security/selinux/selinuxfs.c | 20 ++++++++++++++++++++ > 1 file changed, 20 insertions(+) > > diff --git a/security/selinux/selinuxfs.c > b/security/selinux/selinuxfs.c > index ce71718..b2d5deb 100644 > --- a/security/selinux/selinuxfs.c > +++ b/security/selinux/selinuxfs.c > @@ -30,6 +30,7 @@ > #include <linux/uaccess.h> > #include <linux/kobject.h> > #include <linux/ctype.h> > +#include <crypto/sha.h> > > /* selinuxfs pseudo filesystem for exporting the security policy > API. > Based on the proc code and the fs/nfsd/nfsctl.c code. */ > @@ -99,6 +100,7 @@ enum sel_inos { > SEL_STATUS, /* export current status using mmap() */ > SEL_POLICY, /* allow userspace to read the in kernel > policy */ > SEL_VALIDATE_TRANS, /* compute validatetrans decision */ > + SEL_POLICYCKSUM,/* return policy SHA256 checkum */ > SEL_INO_NEXT, /* The next inode number to use */ > }; > > @@ -313,6 +315,22 @@ static ssize_t sel_read_policyvers(struct file > *filp, char __user *buf, > .llseek = generic_file_llseek, > }; > > +static ssize_t sel_read_policycksum(struct file *filp, char __user > *buf, > + size_t count, loff_t *ppos) > +{ > + size_t tmpbuflen = SHA256_DIGEST_SIZE*2 + 1; > + char tmpbuf[tmpbuflen]; > + ssize_t length; > + > + length = security_policydb_cksum(tmpbuf, tmpbuflen); > + return simple_read_from_buffer(buf, count, ppos, tmpbuf, > length); > +} Should we also include information about the hash used, in case it changes in the future? > + > +static const struct file_operations sel_policycksum_ops = { > + .read = sel_read_policycksum, > + .llseek = generic_file_llseek, > +}; > + > /* declaration for sel_write_load */ > static int sel_make_bools(void); > static int sel_make_classes(void); > @@ -1825,6 +1843,8 @@ static int sel_fill_super(struct super_block > *sb, void *data, int silent) > [SEL_POLICY] = {"policy", &sel_policy_ops, S_IRUGO}, > [SEL_VALIDATE_TRANS] = {"validatetrans", > &sel_transition_ops, > S_IWUGO}, > + [SEL_POLICYCKSUM] = {"policycksum", > &sel_policycksum_ops, > + S_IRUGO}, > /* last one */ {""} > }; > ret = simple_fill_super(sb, SELINUX_MAGIC, selinux_files); ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 3/3] selinux: expose policy SHA256 checksum via selinuxfs 2017-04-26 18:31 ` Stephen Smalley @ 2017-04-27 1:08 ` James Morris 0 siblings, 0 replies; 20+ messages in thread From: James Morris @ 2017-04-27 1:08 UTC (permalink / raw) To: Stephen Smalley Cc: Sebastien Buisson, linux-security-module, linux-kernel, selinux, Sebastien Buisson, james.l.morris On Wed, 26 Apr 2017, Stephen Smalley wrote: > > + return simple_read_from_buffer(buf, count, ppos, tmpbuf, > > length); > > +} > > Should we also include information about the hash used, in case it > changes in the future? > Good idea, yes. -- James Morris <jmorris@namei.org> ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 1/3] selinux: Implement LSM notification system 2017-04-26 15:02 [PATCH 1/3] selinux: Implement LSM notification system Sebastien Buisson 2017-04-26 15:02 ` [PATCH 2/3] selinux: add checksum to policydb Sebastien Buisson 2017-04-26 15:02 ` [PATCH 3/3] selinux: expose policy SHA256 checksum via selinuxfs Sebastien Buisson @ 2017-04-26 15:38 ` Casey Schaufler 2017-04-26 15:48 ` Daniel Jurgens 2017-04-26 17:36 ` Stephen Smalley 2 siblings, 2 replies; 20+ messages in thread From: Casey Schaufler @ 2017-04-26 15:38 UTC (permalink / raw) To: Sebastien Buisson, linux-security-module, linux-kernel, selinux Cc: serge, james.l.morris, eparis, sds, paul, danielj, Sebastien Buisson On 4/26/2017 8:02 AM, Sebastien Buisson wrote: > From: Daniel Jurgens <danielj@mellanox.com> > > Add a generic notification mechanism in the LSM. Interested consumers > can register a callback with the LSM and security modules can produce > events. Why is this a generic mechanism? Do you ever see anyone other than SELinux using it? > Add a call to the notification mechanism from SELinux when the AVC > cache changes. This seems like a whole lot of mechanism for something you could accomplish with a log message. What am I missing? > > Signed-off-by: Daniel Jurgens <danielj@mellanox.com> > Signed-off-by: Sebastien Buisson <sbuisson@ddn.com> > --- > include/linux/security.h | 23 +++++++++++++++++++++++ > security/security.c | 20 ++++++++++++++++++++ > security/selinux/hooks.c | 12 ++++++++++++ > 3 files changed, 55 insertions(+) > > diff --git a/include/linux/security.h b/include/linux/security.h > index af675b5..73a9c93 100644 > --- a/include/linux/security.h > +++ b/include/linux/security.h > @@ -68,6 +68,10 @@ > struct user_namespace; > struct timezone; > > +enum lsm_event { > + LSM_POLICY_CHANGE, > +}; > + > /* These functions are in security/commoncap.c */ > extern int cap_capable(const struct cred *cred, struct user_namespace *ns, > int cap, int audit); > @@ -163,6 +167,10 @@ struct security_mnt_opts { > int num_mnt_opts; > }; > > +int call_lsm_notifier(enum lsm_event event, void *data); > +int register_lsm_notifier(struct notifier_block *nb); > +int unregister_lsm_notifier(struct notifier_block *nb); > + > static inline void security_init_mnt_opts(struct security_mnt_opts *opts) > { > opts->mnt_opts = NULL; > @@ -381,6 +389,21 @@ int security_sem_semop(struct sem_array *sma, struct sembuf *sops, > struct security_mnt_opts { > }; > > +static inline int call_lsm_notifier(enum lsm_event event, void *data) > +{ > + return 0; > +} > + > +static inline int register_lsm_notifier(struct notifier_block *nb) > +{ > + return 0; > +} > + > +static inline int unregister_lsm_notifier(struct notifier_block *nb) > +{ > + return 0; > +} > + > static inline void security_init_mnt_opts(struct security_mnt_opts *opts) > { > } > diff --git a/security/security.c b/security/security.c > index b9fea39..ef9d9e1 100644 > --- a/security/security.c > +++ b/security/security.c > @@ -32,6 +32,8 @@ > /* Maximum number of letters for an LSM name string */ > #define SECURITY_NAME_MAX 10 > > +static ATOMIC_NOTIFIER_HEAD(lsm_notifier_chain); > + > struct security_hook_heads security_hook_heads __lsm_ro_after_init; > char *lsm_names; > /* Boot-time LSM user choice */ > @@ -146,6 +148,24 @@ void __init security_add_hooks(struct security_hook_list *hooks, int count, > panic("%s - Cannot get early memory.\n", __func__); > } > > +int call_lsm_notifier(enum lsm_event event, void *data) > +{ > + return atomic_notifier_call_chain(&lsm_notifier_chain, event, data); > +} > +EXPORT_SYMBOL(call_lsm_notifier); > + > +int register_lsm_notifier(struct notifier_block *nb) > +{ > + return atomic_notifier_chain_register(&lsm_notifier_chain, nb); > +} > +EXPORT_SYMBOL(register_lsm_notifier); > + > +int unregister_lsm_notifier(struct notifier_block *nb) > +{ > + return atomic_notifier_chain_unregister(&lsm_notifier_chain, nb); > +} > +EXPORT_SYMBOL(unregister_lsm_notifier); > + > /* > * Hook list operation macros. > * > diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c > index e67a526..a4d36f8 100644 > --- a/security/selinux/hooks.c > +++ b/security/selinux/hooks.c > @@ -171,6 +171,14 @@ static int selinux_netcache_avc_callback(u32 event) > return 0; > } > > +static int selinux_lsm_notifier_avc_callback(u32 event) > +{ > + if (event == AVC_CALLBACK_RESET) > + call_lsm_notifier(LSM_POLICY_CHANGE, NULL); > + > + return 0; > +} > + > /* > * initialise the security for the init task > */ > @@ -6379,6 +6387,10 @@ static __init int selinux_init(void) > if (avc_add_callback(selinux_netcache_avc_callback, AVC_CALLBACK_RESET)) > panic("SELinux: Unable to register AVC netcache callback\n"); > > + if (avc_add_callback(selinux_lsm_notifier_avc_callback, > + AVC_CALLBACK_RESET)) > + panic("SELinux: Unable to register AVC LSM notifier callback\n"); > + > if (selinux_enforcing) > printk(KERN_DEBUG "SELinux: Starting in enforcing mode\n"); > else ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 1/3] selinux: Implement LSM notification system 2017-04-26 15:38 ` [PATCH 1/3] selinux: Implement LSM notification system Casey Schaufler @ 2017-04-26 15:48 ` Daniel Jurgens 2017-04-26 15:57 ` Sebastien Buisson 2017-04-26 16:11 ` Casey Schaufler 2017-04-26 17:36 ` Stephen Smalley 1 sibling, 2 replies; 20+ messages in thread From: Daniel Jurgens @ 2017-04-26 15:48 UTC (permalink / raw) To: Casey Schaufler, Sebastien Buisson, linux-security-module, linux-kernel, selinux Cc: serge, james.l.morris, eparis, sds, paul, Sebastien Buisson On 4/26/2017 10:38 AM, Casey Schaufler wrote: > On 4/26/2017 8:02 AM, Sebastien Buisson wrote: >> From: Daniel Jurgens <danielj@mellanox.com> >> >> Add a generic notification mechanism in the LSM. Interested consumers >> can register a callback with the LSM and security modules can produce >> events. > Why is this a generic mechanism? Do you ever see anyone > other than SELinux using it? I had created an SELinux specific mechanism, Paul Moore requested I make it generic. >> Add a call to the notification mechanism from SELinux when the AVC >> cache changes. > This seems like a whole lot of mechanism for > something you could accomplish with a log message. > What am I missing? This was part of a larger patch set that hasn't been accepted yet. SELinux support for Inifiniband. Subsequent patches in that patch set will use it as well. >> Signed-off-by: Daniel Jurgens <danielj@mellanox.com> >> Signed-off-by: Sebastien Buisson <sbuisson@ddn.com> >> --- >> include/linux/security.h | 23 +++++++++++++++++++++++ >> security/security.c | 20 ++++++++++++++++++++ >> security/selinux/hooks.c | 12 ++++++++++++ >> 3 files changed, 55 insertions(+) >> >> diff --git a/include/linux/security.h b/include/linux/security.h >> index af675b5..73a9c93 100644 >> --- a/include/linux/security.h >> +++ b/include/linux/security.h >> @@ -68,6 +68,10 @@ >> struct user_namespace; >> struct timezone; >> >> +enum lsm_event { >> + LSM_POLICY_CHANGE, >> +}; >> + >> /* These functions are in security/commoncap.c */ >> extern int cap_capable(const struct cred *cred, struct user_namespace *ns, >> int cap, int audit); >> @@ -163,6 +167,10 @@ struct security_mnt_opts { >> int num_mnt_opts; >> }; >> >> +int call_lsm_notifier(enum lsm_event event, void *data); >> +int register_lsm_notifier(struct notifier_block *nb); >> +int unregister_lsm_notifier(struct notifier_block *nb); >> + >> static inline void security_init_mnt_opts(struct security_mnt_opts *opts) >> { >> opts->mnt_opts = NULL; >> @@ -381,6 +389,21 @@ int security_sem_semop(struct sem_array *sma, struct sembuf *sops, >> struct security_mnt_opts { >> }; >> >> +static inline int call_lsm_notifier(enum lsm_event event, void *data) >> +{ >> + return 0; >> +} >> + >> +static inline int register_lsm_notifier(struct notifier_block *nb) >> +{ >> + return 0; >> +} >> + >> +static inline int unregister_lsm_notifier(struct notifier_block *nb) >> +{ >> + return 0; >> +} >> + >> static inline void security_init_mnt_opts(struct security_mnt_opts *opts) >> { >> } >> diff --git a/security/security.c b/security/security.c >> index b9fea39..ef9d9e1 100644 >> --- a/security/security.c >> +++ b/security/security.c >> @@ -32,6 +32,8 @@ >> /* Maximum number of letters for an LSM name string */ >> #define SECURITY_NAME_MAX 10 >> >> +static ATOMIC_NOTIFIER_HEAD(lsm_notifier_chain); >> + >> struct security_hook_heads security_hook_heads __lsm_ro_after_init; >> char *lsm_names; >> /* Boot-time LSM user choice */ >> @@ -146,6 +148,24 @@ void __init security_add_hooks(struct security_hook_list *hooks, int count, >> panic("%s - Cannot get early memory.\n", __func__); >> } >> >> +int call_lsm_notifier(enum lsm_event event, void *data) >> +{ >> + return atomic_notifier_call_chain(&lsm_notifier_chain, event, data); >> +} >> +EXPORT_SYMBOL(call_lsm_notifier); >> + >> +int register_lsm_notifier(struct notifier_block *nb) >> +{ >> + return atomic_notifier_chain_register(&lsm_notifier_chain, nb); >> +} >> +EXPORT_SYMBOL(register_lsm_notifier); >> + >> +int unregister_lsm_notifier(struct notifier_block *nb) >> +{ >> + return atomic_notifier_chain_unregister(&lsm_notifier_chain, nb); >> +} >> +EXPORT_SYMBOL(unregister_lsm_notifier); >> + >> /* >> * Hook list operation macros. >> * >> diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c >> index e67a526..a4d36f8 100644 >> --- a/security/selinux/hooks.c >> +++ b/security/selinux/hooks.c >> @@ -171,6 +171,14 @@ static int selinux_netcache_avc_callback(u32 event) >> return 0; >> } >> >> +static int selinux_lsm_notifier_avc_callback(u32 event) >> +{ >> + if (event == AVC_CALLBACK_RESET) >> + call_lsm_notifier(LSM_POLICY_CHANGE, NULL); >> + >> + return 0; >> +} >> + >> /* >> * initialise the security for the init task >> */ >> @@ -6379,6 +6387,10 @@ static __init int selinux_init(void) >> if (avc_add_callback(selinux_netcache_avc_callback, AVC_CALLBACK_RESET)) >> panic("SELinux: Unable to register AVC netcache callback\n"); >> >> + if (avc_add_callback(selinux_lsm_notifier_avc_callback, >> + AVC_CALLBACK_RESET)) >> + panic("SELinux: Unable to register AVC LSM notifier callback\n"); >> + >> if (selinux_enforcing) >> printk(KERN_DEBUG "SELinux: Starting in enforcing mode\n"); >> else > ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 1/3] selinux: Implement LSM notification system 2017-04-26 15:48 ` Daniel Jurgens @ 2017-04-26 15:57 ` Sebastien Buisson 2017-04-26 16:11 ` Casey Schaufler 1 sibling, 0 replies; 20+ messages in thread From: Sebastien Buisson @ 2017-04-26 15:57 UTC (permalink / raw) To: Daniel Jurgens Cc: Casey Schaufler, linux-security-module, linux-kernel, selinux, serge, james.l.morris, eparis, sds, paul, Sebastien Buisson 2017-04-26 17:48 GMT+02:00 Daniel Jurgens <danielj@mellanox.com>: > This was part of a larger patch set that hasn't been accepted yet. SELinux support for Inifiniband. Subsequent patches in that patch set will use it as well. I revived this patch following Stephen Smalley's suggestion to base the policy checksum computation and change notification on the work you had done on LSM notification callbacks. ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 1/3] selinux: Implement LSM notification system 2017-04-26 15:48 ` Daniel Jurgens 2017-04-26 15:57 ` Sebastien Buisson @ 2017-04-26 16:11 ` Casey Schaufler 1 sibling, 0 replies; 20+ messages in thread From: Casey Schaufler @ 2017-04-26 16:11 UTC (permalink / raw) To: Daniel Jurgens, Sebastien Buisson, linux-security-module, linux-kernel, selinux Cc: serge, james.l.morris, eparis, sds, paul, Sebastien Buisson On 4/26/2017 8:48 AM, Daniel Jurgens wrote: > On 4/26/2017 10:38 AM, Casey Schaufler wrote: >> On 4/26/2017 8:02 AM, Sebastien Buisson wrote: >>> From: Daniel Jurgens <danielj@mellanox.com> >>> >>> Add a generic notification mechanism in the LSM. Interested consumers >>> can register a callback with the LSM and security modules can produce >>> events. >> Why is this a generic mechanism? Do you ever see anyone >> other than SELinux using it? > I had created an SELinux specific mechanism, Paul Moore requested I make it generic. >>> Add a call to the notification mechanism from SELinux when the AVC >>> cache changes. >> This seems like a whole lot of mechanism for >> something you could accomplish with a log message. >> What am I missing? > This was part of a larger patch set that hasn't been accepted yet. SELinux support for Inifiniband. Subsequent patches in that patch set will use it as well. You should include this information in a 0/3 message. Without the context the patch set is curious, but not compelling. >>> Signed-off-by: Daniel Jurgens <danielj@mellanox.com> >>> Signed-off-by: Sebastien Buisson <sbuisson@ddn.com> >>> --- >>> include/linux/security.h | 23 +++++++++++++++++++++++ >>> security/security.c | 20 ++++++++++++++++++++ >>> security/selinux/hooks.c | 12 ++++++++++++ >>> 3 files changed, 55 insertions(+) >>> >>> diff --git a/include/linux/security.h b/include/linux/security.h >>> index af675b5..73a9c93 100644 >>> --- a/include/linux/security.h >>> +++ b/include/linux/security.h >>> @@ -68,6 +68,10 @@ >>> struct user_namespace; >>> struct timezone; >>> >>> +enum lsm_event { >>> + LSM_POLICY_CHANGE, >>> +}; >>> + >>> /* These functions are in security/commoncap.c */ >>> extern int cap_capable(const struct cred *cred, struct user_namespace *ns, >>> int cap, int audit); >>> @@ -163,6 +167,10 @@ struct security_mnt_opts { >>> int num_mnt_opts; >>> }; >>> >>> +int call_lsm_notifier(enum lsm_event event, void *data); >>> +int register_lsm_notifier(struct notifier_block *nb); >>> +int unregister_lsm_notifier(struct notifier_block *nb); >>> + >>> static inline void security_init_mnt_opts(struct security_mnt_opts *opts) >>> { >>> opts->mnt_opts = NULL; >>> @@ -381,6 +389,21 @@ int security_sem_semop(struct sem_array *sma, struct sembuf *sops, >>> struct security_mnt_opts { >>> }; >>> >>> +static inline int call_lsm_notifier(enum lsm_event event, void *data) >>> +{ >>> + return 0; >>> +} >>> + >>> +static inline int register_lsm_notifier(struct notifier_block *nb) >>> +{ >>> + return 0; >>> +} >>> + >>> +static inline int unregister_lsm_notifier(struct notifier_block *nb) >>> +{ >>> + return 0; >>> +} >>> + >>> static inline void security_init_mnt_opts(struct security_mnt_opts *opts) >>> { >>> } >>> diff --git a/security/security.c b/security/security.c >>> index b9fea39..ef9d9e1 100644 >>> --- a/security/security.c >>> +++ b/security/security.c >>> @@ -32,6 +32,8 @@ >>> /* Maximum number of letters for an LSM name string */ >>> #define SECURITY_NAME_MAX 10 >>> >>> +static ATOMIC_NOTIFIER_HEAD(lsm_notifier_chain); >>> + >>> struct security_hook_heads security_hook_heads __lsm_ro_after_init; >>> char *lsm_names; >>> /* Boot-time LSM user choice */ >>> @@ -146,6 +148,24 @@ void __init security_add_hooks(struct security_hook_list *hooks, int count, >>> panic("%s - Cannot get early memory.\n", __func__); >>> } >>> >>> +int call_lsm_notifier(enum lsm_event event, void *data) >>> +{ >>> + return atomic_notifier_call_chain(&lsm_notifier_chain, event, data); >>> +} >>> +EXPORT_SYMBOL(call_lsm_notifier); >>> + >>> +int register_lsm_notifier(struct notifier_block *nb) >>> +{ >>> + return atomic_notifier_chain_register(&lsm_notifier_chain, nb); >>> +} >>> +EXPORT_SYMBOL(register_lsm_notifier); >>> + >>> +int unregister_lsm_notifier(struct notifier_block *nb) >>> +{ >>> + return atomic_notifier_chain_unregister(&lsm_notifier_chain, nb); >>> +} >>> +EXPORT_SYMBOL(unregister_lsm_notifier); >>> + >>> /* >>> * Hook list operation macros. >>> * >>> diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c >>> index e67a526..a4d36f8 100644 >>> --- a/security/selinux/hooks.c >>> +++ b/security/selinux/hooks.c >>> @@ -171,6 +171,14 @@ static int selinux_netcache_avc_callback(u32 event) >>> return 0; >>> } >>> >>> +static int selinux_lsm_notifier_avc_callback(u32 event) >>> +{ >>> + if (event == AVC_CALLBACK_RESET) >>> + call_lsm_notifier(LSM_POLICY_CHANGE, NULL); >>> + >>> + return 0; >>> +} >>> + >>> /* >>> * initialise the security for the init task >>> */ >>> @@ -6379,6 +6387,10 @@ static __init int selinux_init(void) >>> if (avc_add_callback(selinux_netcache_avc_callback, AVC_CALLBACK_RESET)) >>> panic("SELinux: Unable to register AVC netcache callback\n"); >>> >>> + if (avc_add_callback(selinux_lsm_notifier_avc_callback, >>> + AVC_CALLBACK_RESET)) >>> + panic("SELinux: Unable to register AVC LSM notifier callback\n"); >>> + >>> if (selinux_enforcing) >>> printk(KERN_DEBUG "SELinux: Starting in enforcing mode\n"); >>> else > ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 1/3] selinux: Implement LSM notification system 2017-04-26 15:38 ` [PATCH 1/3] selinux: Implement LSM notification system Casey Schaufler 2017-04-26 15:48 ` Daniel Jurgens @ 2017-04-26 17:36 ` Stephen Smalley 2017-04-26 17:47 ` Casey Schaufler 1 sibling, 1 reply; 20+ messages in thread From: Stephen Smalley @ 2017-04-26 17:36 UTC (permalink / raw) To: Casey Schaufler, Sebastien Buisson, linux-security-module, linux-kernel, selinux Cc: Sebastien Buisson, james.l.morris On Wed, 2017-04-26 at 08:38 -0700, Casey Schaufler wrote: > On 4/26/2017 8:02 AM, Sebastien Buisson wrote: > > From: Daniel Jurgens <danielj@mellanox.com> > > > > Add a generic notification mechanism in the LSM. Interested > > consumers > > can register a callback with the LSM and security modules can > > produce > > events. > > Why is this a generic mechanism? Do you ever see anyone > other than SELinux using it? I do - any security module that wants to support access control over Lustre filesystems or Infiniband. Seems ironic for you to be arguing for a SELinux-specific interface rather than a LSM interface. > > > Add a call to the notification mechanism from SELinux when the AVC > > cache changes. > > This seems like a whole lot of mechanism for > something you could accomplish with a log message. > What am I missing? It's a notification to a kernel subsystem that policy has changed so that the subsystem can update any cached state. > > > > > Signed-off-by: Daniel Jurgens <danielj@mellanox.com> > > Signed-off-by: Sebastien Buisson <sbuisson@ddn.com> > > --- > > include/linux/security.h | 23 +++++++++++++++++++++++ > > security/security.c | 20 ++++++++++++++++++++ > > security/selinux/hooks.c | 12 ++++++++++++ > > 3 files changed, 55 insertions(+) > > > > diff --git a/include/linux/security.h b/include/linux/security.h > > index af675b5..73a9c93 100644 > > --- a/include/linux/security.h > > +++ b/include/linux/security.h > > @@ -68,6 +68,10 @@ > > struct user_namespace; > > struct timezone; > > > > +enum lsm_event { > > + LSM_POLICY_CHANGE, > > +}; > > + > > /* These functions are in security/commoncap.c */ > > extern int cap_capable(const struct cred *cred, struct > > user_namespace *ns, > > int cap, int audit); > > @@ -163,6 +167,10 @@ struct security_mnt_opts { > > int num_mnt_opts; > > }; > > > > +int call_lsm_notifier(enum lsm_event event, void *data); > > +int register_lsm_notifier(struct notifier_block *nb); > > +int unregister_lsm_notifier(struct notifier_block *nb); > > + > > static inline void security_init_mnt_opts(struct security_mnt_opts > > *opts) > > { > > opts->mnt_opts = NULL; > > @@ -381,6 +389,21 @@ int security_sem_semop(struct sem_array *sma, > > struct sembuf *sops, > > struct security_mnt_opts { > > }; > > > > +static inline int call_lsm_notifier(enum lsm_event event, void > > *data) > > +{ > > + return 0; > > +} > > + > > +static inline int register_lsm_notifier(struct notifier_block *nb) > > +{ > > + return 0; > > +} > > + > > +static inline int unregister_lsm_notifier(struct notifier_block > > *nb) > > +{ > > + return 0; > > +} > > + > > static inline void security_init_mnt_opts(struct security_mnt_opts > > *opts) > > { > > } > > diff --git a/security/security.c b/security/security.c > > index b9fea39..ef9d9e1 100644 > > --- a/security/security.c > > +++ b/security/security.c > > @@ -32,6 +32,8 @@ > > /* Maximum number of letters for an LSM name string */ > > #define SECURITY_NAME_MAX 10 > > > > +static ATOMIC_NOTIFIER_HEAD(lsm_notifier_chain); > > + > > struct security_hook_heads security_hook_heads > > __lsm_ro_after_init; > > char *lsm_names; > > /* Boot-time LSM user choice */ > > @@ -146,6 +148,24 @@ void __init security_add_hooks(struct > > security_hook_list *hooks, int count, > > panic("%s - Cannot get early memory.\n", > > __func__); > > } > > > > +int call_lsm_notifier(enum lsm_event event, void *data) > > +{ > > + return atomic_notifier_call_chain(&lsm_notifier_chain, > > event, data); > > +} > > +EXPORT_SYMBOL(call_lsm_notifier); > > + > > +int register_lsm_notifier(struct notifier_block *nb) > > +{ > > + return atomic_notifier_chain_register(&lsm_notifier_chain, > > nb); > > +} > > +EXPORT_SYMBOL(register_lsm_notifier); > > + > > +int unregister_lsm_notifier(struct notifier_block *nb) > > +{ > > + return > > atomic_notifier_chain_unregister(&lsm_notifier_chain, nb); > > +} > > +EXPORT_SYMBOL(unregister_lsm_notifier); > > + > > /* > > * Hook list operation macros. > > * > > diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c > > index e67a526..a4d36f8 100644 > > --- a/security/selinux/hooks.c > > +++ b/security/selinux/hooks.c > > @@ -171,6 +171,14 @@ static int selinux_netcache_avc_callback(u32 > > event) > > return 0; > > } > > > > +static int selinux_lsm_notifier_avc_callback(u32 event) > > +{ > > + if (event == AVC_CALLBACK_RESET) > > + call_lsm_notifier(LSM_POLICY_CHANGE, NULL); > > + > > + return 0; > > +} > > + > > /* > > * initialise the security for the init task > > */ > > @@ -6379,6 +6387,10 @@ static __init int selinux_init(void) > > if (avc_add_callback(selinux_netcache_avc_callback, > > AVC_CALLBACK_RESET)) > > panic("SELinux: Unable to register AVC netcache > > callback\n"); > > > > + if (avc_add_callback(selinux_lsm_notifier_avc_callback, > > + AVC_CALLBACK_RESET)) > > + panic("SELinux: Unable to register AVC LSM > > notifier callback\n"); > > + > > if (selinux_enforcing) > > printk(KERN_DEBUG "SELinux: Starting in enforcing > > mode\n"); > > else ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 1/3] selinux: Implement LSM notification system 2017-04-26 17:36 ` Stephen Smalley @ 2017-04-26 17:47 ` Casey Schaufler 0 siblings, 0 replies; 20+ messages in thread From: Casey Schaufler @ 2017-04-26 17:47 UTC (permalink / raw) To: Stephen Smalley, Sebastien Buisson, linux-security-module, linux-kernel, selinux Cc: Sebastien Buisson, james.l.morris On 4/26/2017 10:36 AM, Stephen Smalley wrote: > On Wed, 2017-04-26 at 08:38 -0700, Casey Schaufler wrote: >> On 4/26/2017 8:02 AM, Sebastien Buisson wrote: >>> From: Daniel Jurgens <danielj@mellanox.com> >>> >>> Add a generic notification mechanism in the LSM. Interested >>> consumers >>> can register a callback with the LSM and security modules can >>> produce >>> events. >> Why is this a generic mechanism? Do you ever see anyone >> other than SELinux using it? > I do - any security module that wants to support access control over > Lustre filesystems or Infiniband. Seems ironic for you to be arguing > for a SELinux-specific interface rather than a LSM interface. I lost the connection between this mechanism and Lustre and Infiniband, neither of which are mentioned in the patchset. And I *did* pose it as a question. I am all in favor of general mechanisms where they are generally useful. I am also adverse to special purpose clutter. > >>> Add a call to the notification mechanism from SELinux when the AVC >>> cache changes. >> This seems like a whole lot of mechanism for >> something you could accomplish with a log message. >> What am I missing? > It's a notification to a kernel subsystem that policy has changed so > that the subsystem can update any cached state. > >>> Signed-off-by: Daniel Jurgens <danielj@mellanox.com> >>> Signed-off-by: Sebastien Buisson <sbuisson@ddn.com> >>> --- >>> include/linux/security.h | 23 +++++++++++++++++++++++ >>> security/security.c | 20 ++++++++++++++++++++ >>> security/selinux/hooks.c | 12 ++++++++++++ >>> 3 files changed, 55 insertions(+) >>> >>> diff --git a/include/linux/security.h b/include/linux/security.h >>> index af675b5..73a9c93 100644 >>> --- a/include/linux/security.h >>> +++ b/include/linux/security.h >>> @@ -68,6 +68,10 @@ >>> struct user_namespace; >>> struct timezone; >>> >>> +enum lsm_event { >>> + LSM_POLICY_CHANGE, >>> +}; >>> + >>> /* These functions are in security/commoncap.c */ >>> extern int cap_capable(const struct cred *cred, struct >>> user_namespace *ns, >>> int cap, int audit); >>> @@ -163,6 +167,10 @@ struct security_mnt_opts { >>> int num_mnt_opts; >>> }; >>> >>> +int call_lsm_notifier(enum lsm_event event, void *data); >>> +int register_lsm_notifier(struct notifier_block *nb); >>> +int unregister_lsm_notifier(struct notifier_block *nb); >>> + >>> static inline void security_init_mnt_opts(struct security_mnt_opts >>> *opts) >>> { >>> opts->mnt_opts = NULL; >>> @@ -381,6 +389,21 @@ int security_sem_semop(struct sem_array *sma, >>> struct sembuf *sops, >>> struct security_mnt_opts { >>> }; >>> >>> +static inline int call_lsm_notifier(enum lsm_event event, void >>> *data) >>> +{ >>> + return 0; >>> +} >>> + >>> +static inline int register_lsm_notifier(struct notifier_block *nb) >>> +{ >>> + return 0; >>> +} >>> + >>> +static inline int unregister_lsm_notifier(struct notifier_block >>> *nb) >>> +{ >>> + return 0; >>> +} >>> + >>> static inline void security_init_mnt_opts(struct security_mnt_opts >>> *opts) >>> { >>> } >>> diff --git a/security/security.c b/security/security.c >>> index b9fea39..ef9d9e1 100644 >>> --- a/security/security.c >>> +++ b/security/security.c >>> @@ -32,6 +32,8 @@ >>> /* Maximum number of letters for an LSM name string */ >>> #define SECURITY_NAME_MAX 10 >>> >>> +static ATOMIC_NOTIFIER_HEAD(lsm_notifier_chain); >>> + >>> struct security_hook_heads security_hook_heads >>> __lsm_ro_after_init; >>> char *lsm_names; >>> /* Boot-time LSM user choice */ >>> @@ -146,6 +148,24 @@ void __init security_add_hooks(struct >>> security_hook_list *hooks, int count, >>> panic("%s - Cannot get early memory.\n", >>> __func__); >>> } >>> >>> +int call_lsm_notifier(enum lsm_event event, void *data) >>> +{ >>> + return atomic_notifier_call_chain(&lsm_notifier_chain, >>> event, data); >>> +} >>> +EXPORT_SYMBOL(call_lsm_notifier); >>> + >>> +int register_lsm_notifier(struct notifier_block *nb) >>> +{ >>> + return atomic_notifier_chain_register(&lsm_notifier_chain, >>> nb); >>> +} >>> +EXPORT_SYMBOL(register_lsm_notifier); >>> + >>> +int unregister_lsm_notifier(struct notifier_block *nb) >>> +{ >>> + return >>> atomic_notifier_chain_unregister(&lsm_notifier_chain, nb); >>> +} >>> +EXPORT_SYMBOL(unregister_lsm_notifier); >>> + >>> /* >>> * Hook list operation macros. >>> * >>> diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c >>> index e67a526..a4d36f8 100644 >>> --- a/security/selinux/hooks.c >>> +++ b/security/selinux/hooks.c >>> @@ -171,6 +171,14 @@ static int selinux_netcache_avc_callback(u32 >>> event) >>> return 0; >>> } >>> >>> +static int selinux_lsm_notifier_avc_callback(u32 event) >>> +{ >>> + if (event == AVC_CALLBACK_RESET) >>> + call_lsm_notifier(LSM_POLICY_CHANGE, NULL); >>> + >>> + return 0; >>> +} >>> + >>> /* >>> * initialise the security for the init task >>> */ >>> @@ -6379,6 +6387,10 @@ static __init int selinux_init(void) >>> if (avc_add_callback(selinux_netcache_avc_callback, >>> AVC_CALLBACK_RESET)) >>> panic("SELinux: Unable to register AVC netcache >>> callback\n"); >>> >>> + if (avc_add_callback(selinux_lsm_notifier_avc_callback, >>> + AVC_CALLBACK_RESET)) >>> + panic("SELinux: Unable to register AVC LSM >>> notifier callback\n"); >>> + >>> if (selinux_enforcing) >>> printk(KERN_DEBUG "SELinux: Starting in enforcing >>> mode\n"); >>> else ^ permalink raw reply [flat|nested] 20+ messages in thread
end of thread, other threads:[~2017-04-28 16:34 UTC | newest] Thread overview: 20+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2017-04-26 15:02 [PATCH 1/3] selinux: Implement LSM notification system Sebastien Buisson 2017-04-26 15:02 ` [PATCH 2/3] selinux: add checksum to policydb Sebastien Buisson 2017-04-26 18:30 ` Stephen Smalley 2017-04-27 8:41 ` Sebastien Buisson 2017-04-27 15:18 ` Stephen Smalley 2017-04-27 17:12 ` Sebastien Buisson 2017-04-27 18:47 ` Stephen Smalley 2017-04-28 15:16 ` Sebastien Buisson 2017-04-28 15:50 ` Stephen Smalley 2017-04-28 16:08 ` Sebastien Buisson 2017-04-28 16:38 ` Stephen Smalley 2017-04-26 15:02 ` [PATCH 3/3] selinux: expose policy SHA256 checksum via selinuxfs Sebastien Buisson 2017-04-26 18:31 ` Stephen Smalley 2017-04-27 1:08 ` James Morris 2017-04-26 15:38 ` [PATCH 1/3] selinux: Implement LSM notification system Casey Schaufler 2017-04-26 15:48 ` Daniel Jurgens 2017-04-26 15:57 ` Sebastien Buisson 2017-04-26 16:11 ` Casey Schaufler 2017-04-26 17:36 ` Stephen Smalley 2017-04-26 17:47 ` Casey Schaufler
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).