| Message ID | 1493218936-18522-2-git-send-email-sbuisson@ddn.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
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
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.
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.
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.
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.
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.
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.
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?
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.
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
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(-)