Message ID | 20160625101706.GD19933@p310 (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Petko Manolov <petkan@mip-labs.com> wrote: > The attached patch is an attempt at making use of the IMA blacklist keyring. > It isn't very pretty as it is out for an RFC only, but is fully functional, > tested and applies cleanly on top of the latest 4.7-rc4+. It really needs to make use of this in some way: http://git.kernel.org/cgit/linux/kernel/git/dhowells/linux-fs.git/log/?h=keys-blacklist which should be more efficient than storing full asymmetric keys. There needs to be a way to get the X.509 parser to generate blacklist type keys instead of asymmetric keys. David -- To unsubscribe from this list: send the line "unsubscribe linux-security-module" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 16-06-26 06:23:21, David Howells wrote: > Petko Manolov <petkan@mip-labs.com> wrote: > > > The attached patch is an attempt at making use of the IMA blacklist keyring. > > It isn't very pretty as it is out for an RFC only, but is fully functional, > > tested and applies cleanly on top of the latest 4.7-rc4+. > > It really needs to make use of this in some way: > > http://git.kernel.org/cgit/linux/kernel/git/dhowells/linux-fs.git/log/?h=keys-blacklist > > which should be more efficient than storing full asymmetric keys. There needs Speaking from IMA perspective, the key (or rather the certificate) may be blacklisted at any time. Do you propose to keep the key into whatever keyring it has been put (.ima, .secondary_trusted_keys, etc.) and blacklist it's TBS hash? Isn't it easier to move the actual certificate from one keyring to another, since it is already taking space? Or maybe i am missing something important? > to be a way to get the X.509 parser to generate blacklist type keys instead of > asymmetric keys. If i understand correctly you mean the x.509 parser should be taught to extract the TBSCertificate hash? If true then some poor bugger would have to implement the functionality. :) Petko -- To unsubscribe from this list: send the line "unsubscribe linux-security-module" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Petko Manolov <petkan@mip-labs.com> wrote: > Speaking from IMA perspective, the key (or rather the certificate) may be > blacklisted at any time. Do you propose to keep the key into whatever > keyring it has been put (.ima, .secondary_trusted_keys, etc.) and blacklist > it's TBS hash? Note that you don't necessarily have to blacklist the TBS hash - that's why the description is prefixed with "tbs:". You can prefix it with something else to use some other attribute. It's just that's what the UEFI database uses as its attribute of choice. > Isn't it easier to move the actual certificate from one keyring to another, > since it is already taking space? Or maybe i am missing something important? That could be done also. There is no limitation that *only* blacklist-type keys can be put on the blacklist keyring. However, you're also potentially trading memory for lookup efficiency, and when you permanently blacklist a asymmetric key, you could just ditch it and merely remember the attribute by which it is blacklisted. > > to be a way to get the X.509 parser to generate blacklist type keys > > instead of asymmetric keys. > > If i understand correctly you mean the x.509 parser should be taught to > extract the TBSCertificate hash? If true then some poor bugger would have > to implement the functionality. :) It has to do this already. "TBS" stands for "To Be Signed". That's the element of the X.509 cert over which the signature is calculated and it's now retained for trust calculation purposes. David -- To unsubscribe from this list: send the line "unsubscribe linux-security-module" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Petko, Sorry I'm so late to the discussion ... On Sa, 2016-06-25 at 13:17 +0300, Petko Manolov wrote: > Hello guys, > > The attached patch is an attempt at making use of the IMA blacklist keyring. It > isn't very pretty as it is out for an RFC only, but is fully functional, tested > and applies cleanly on top of the latest 4.7-rc4+. > > We need to re-validate all cached iint entries once a key is moved to > .ima_blacklist keyring. This is being done on demand when iint is consulted. > > In order do make such verification we need some extra data stored in "struct > iint". One is a time stamp and the other is a pointer to the signature > verification key. I boldly assumed the latter is immutable, but David should > correct me if i am wrong. Right, I'm not sure that having a pointer to the key is so safe. It would probably be better to store whatever identifier will be used for blacklisting the certificate in the iint. As for the timestamp, although I recommended defining a "time stamp", perhaps a counter is large enough? (If incremented once a second a uint32 is 127 years. Even for systems that aren't rebooted often that should be long enough.) > I tried to make the evaluation procedure as quick as possible - that is > iint_bl_check(). It does some obvious sanity checks, but two are worth > mentioning. One, it checks if the blacklist keyring has been modified after the > iint creation. If this fails then we can safely assume the key is still valid. > Next we check if the key used for signature verification has landed on the > .ima_blacklist and act accordingly. The auth_ids, which are currently being used for finding the public key (eg. asymmetric_key_id_partial() or find_asymmetric_key()), could be used for searching the blacklist. > asymmetric_verify() had to be provided with a pointer to iint so we can store > the timestamp and pointer to the key. The other, and bigger, intrusion to the > original code is that integrity_iint_find() and, as a result, > integrity_inode_get() can now also return an error. Luckily there isn't too > many users of both, but i guess they deserve careful look in case i've > accidentally changed the caller's semantics. Couldn't the keyid, used for verifying the signature, be pulled out of the xattr directly? Thanks! Mimi -- To unsubscribe from this list: send the line "unsubscribe linux-security-module" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On So, 2016-06-26 at 06:23 +0100, David Howells wrote: > Petko Manolov <petkan@mip-labs.com> wrote: > > > The attached patch is an attempt at making use of the IMA blacklist keyring. > > It isn't very pretty as it is out for an RFC only, but is fully functional, > > tested and applies cleanly on top of the latest 4.7-rc4+. > > It really needs to make use of this in some way: > > http://git.kernel.org/cgit/linux/kernel/git/dhowells/linux-fs.git/log/?h=keys-blacklist Commit 3fdefc0 "KEYS: Add a system blacklist keyring" seeds the blacklist keyring from a Kconfig defined file. Similarly, assuming the file was signed by a trusted key (on the builtin keyring), additional keys could be blacklisted by userspace. Mimi -- To unsubscribe from this list: send the line "unsubscribe linux-security-module" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 16-07-14 19:20:01, Mimi Zohar wrote: > Hi Petko, > > Sorry I'm so late to the discussion ... > > On Sa, 2016-06-25 at 13:17 +0300, Petko Manolov wrote: > > Hello guys, > > > > The attached patch is an attempt at making use of the IMA blacklist keyring. > > It isn't very pretty as it is out for an RFC only, but is fully functional, > > tested and applies cleanly on top of the latest 4.7-rc4+. > > > > We need to re-validate all cached iint entries once a key is moved to > > .ima_blacklist keyring. This is being done on demand when iint is > > consulted. > > > > In order do make such verification we need some extra data stored in "struct > > iint". One is a time stamp and the other is a pointer to the signature > > verification key. I boldly assumed the latter is immutable, but David > > should correct me if i am wrong. > > Right, I'm not sure that having a pointer to the key is so safe. It would It does not feel right. However, i did not see re-allocation of the key storage memory anywhere in the code. I guess David should be the ultimate decision maker here. > probably be better to store whatever identifier will be used for blacklisting > the certificate in the iint. As for the timestamp, although I recommended Each key is given a unique 32bit ID valid for the lifetime of the system. This ID is stored in a RB tree for faster search. If holding a pointer to the key is dangerous we could always use that. > defining a "time stamp", perhaps a counter is large enough? (If incremented > once a second a uint32 is 127 years. Even for systems that aren't rebooted > often that should be long enough.) The keyrings already have "last used" timestamp. It is very easy to compare against the last time given keyring has been modified/used. > > I tried to make the evaluation procedure as quick as possible - that is > > iint_bl_check(). It does some obvious sanity checks, but two are worth > > mentioning. One, it checks if the blacklist keyring has been modified after > > the iint creation. If this fails then we can safely assume the key is still > > valid. Next we check if the key used for signature verification has landed > > on the .ima_blacklist and act accordingly. > > The auth_ids, which are currently being used for finding the public key (eg. > asymmetric_key_id_partial() or find_asymmetric_key()), could be used for > searching the blacklist. The auth ids are not being kept in the iint struct so this does not help. If we keep a pointer to the key we could just use key->description (which is what i do now), or go through the RB tree and find the key by its ID. Either way we end up searching .ima_blacklist and .blacklist by using keyring_search() which is faster. > > asymmetric_verify() had to be provided with a pointer to iint so we can > > store the timestamp and pointer to the key. The other, and bigger, > > intrusion to the original code is that integrity_iint_find() and, as a > > result, integrity_inode_get() can now also return an error. Luckily there > > isn't too many users of both, but i guess they deserve careful look in case > > i've accidentally changed the caller's semantics. > > Couldn't the keyid, used for verifying the signature, be pulled out of the > xattr directly? iint structure only holds a pointer to the inode structure. Can we extract the xattrs from *inode? cheers, Petko -- To unsubscribe from this list: send the line "unsubscribe linux-security-module" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Sa, 2016-07-16 at 17:07 +0300, Petko Manolov wrote: > On 16-07-14 19:20:01, Mimi Zohar wrote: > > On Sa, 2016-06-25 at 13:17 +0300, Petko Manolov wrote: > > > Hello guys, > > > > > > The attached patch is an attempt at making use of the IMA blacklist keyring. > > > It isn't very pretty as it is out for an RFC only, but is fully functional, > > > tested and applies cleanly on top of the latest 4.7-rc4+. > > > > > > We need to re-validate all cached iint entries once a key is moved to > > > .ima_blacklist keyring. This is being done on demand when iint is > > > consulted. > > > > > > In order do make such verification we need some extra data stored in "struct > > > iint". One is a time stamp and the other is a pointer to the signature > > > verification key. I boldly assumed the latter is immutable, but David > > > should correct me if i am wrong. > > > > Right, I'm not sure that having a pointer to the key is so safe. It would > > It does not feel right. However, i did not see re-allocation of the key storage > memory anywhere in the code. I guess David should be the ultimate decision > maker here. What happens if the key is updated/deleted? Aren't we removing the key from the secondary keyring and adding it to the blacklisted keyring? Is there a difference between the IMA blacklist keyring and the system blacklist keyring in this regards? Mimi -- To unsubscribe from this list: send the line "unsubscribe linux-security-module" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On So, 2016-07-17 at 12:31 -0400, Mimi Zohar wrote: > On Sa, 2016-07-16 at 17:07 +0300, Petko Manolov wrote: > > On 16-07-14 19:20:01, Mimi Zohar wrote: > > > On Sa, 2016-06-25 at 13:17 +0300, Petko Manolov wrote: > > > > Hello guys, > > > > > > > > The attached patch is an attempt at making use of the IMA blacklist keyring. > > > > It isn't very pretty as it is out for an RFC only, but is fully functional, > > > > tested and applies cleanly on top of the latest 4.7-rc4+. > > > > > > > > We need to re-validate all cached iint entries once a key is moved to > > > > .ima_blacklist keyring. This is being done on demand when iint is > > > > consulted. > > > > > > > > In order do make such verification we need some extra data stored in "struct > > > > iint". One is a time stamp and the other is a pointer to the signature > > > > verification key. I boldly assumed the latter is immutable, but David > > > > should correct me if i am wrong. > > > > > > Right, I'm not sure that having a pointer to the key is so safe. It would > > > > It does not feel right. However, i did not see re-allocation of the key storage > > memory anywhere in the code. I guess David should be the ultimate decision > > maker here. > > What happens if the key is updated/deleted? Aren't we removing the key > from the secondary keyring and adding it to the blacklisted keyring? Is > there a difference between the IMA blacklist keyring and the system > blacklist keyring in this regards? That should be the IMA and/or secondary keyrings. Mimi -- To unsubscribe from this list: send the line "unsubscribe linux-security-module" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 16-07-17 12:47:16, Mimi Zohar wrote: > On So, 2016-07-17 at 12:31 -0400, Mimi Zohar wrote: > > On Sa, 2016-07-16 at 17:07 +0300, Petko Manolov wrote: > > > On 16-07-14 19:20:01, Mimi Zohar wrote: > > > > On Sa, 2016-06-25 at 13:17 +0300, Petko Manolov wrote: > > > > > Hello guys, > > > > > > > > > > The attached patch is an attempt at making use of the IMA blacklist keyring. > > > > > It isn't very pretty as it is out for an RFC only, but is fully functional, > > > > > tested and applies cleanly on top of the latest 4.7-rc4+. > > > > > > > > > > We need to re-validate all cached iint entries once a key is moved to > > > > > .ima_blacklist keyring. This is being done on demand when iint is > > > > > consulted. > > > > > > > > > > In order do make such verification we need some extra data stored in "struct > > > > > iint". One is a time stamp and the other is a pointer to the signature > > > > > verification key. I boldly assumed the latter is immutable, but David > > > > > should correct me if i am wrong. > > > > > > > > Right, I'm not sure that having a pointer to the key is so safe. It would > > > > > > It does not feel right. However, i did not see re-allocation of the key storage > > > memory anywhere in the code. I guess David should be the ultimate decision > > > maker here. > > > > What happens if the key is updated/deleted? Aren't we removing the key > > from the secondary keyring and adding it to the blacklisted keyring? Is If a key is updated or moved around different keyrings the originally allocated memory stays the same (iow, same address). If it get's deleted then we're in trouble. I'll modify the patch to use key's unique ID instead of direct pointer. > > there a difference between the IMA blacklist keyring and the system > > blacklist keyring in this regards? > > That should be the IMA and/or secondary keyrings. The system blacklist may hold keys of (new) type "blacklist". It stores blacklisted hash and has no payload. This keyring should also accept normal keys. Petko -- To unsubscribe from this list: send the line "unsubscribe linux-security-module" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mo, 2016-07-18 at 19:40 +0300, Petko Manolov wrote: > On 16-07-17 12:47:16, Mimi Zohar wrote: > > On So, 2016-07-17 at 12:31 -0400, Mimi Zohar wrote: > > > On Sa, 2016-07-16 at 17:07 +0300, Petko Manolov wrote: > > > > On 16-07-14 19:20:01, Mimi Zohar wrote: > > > > > On Sa, 2016-06-25 at 13:17 +0300, Petko Manolov wrote: > > > > > > Hello guys, > > > > > > > > > > > > The attached patch is an attempt at making use of the IMA blacklist keyring. > > > > > > It isn't very pretty as it is out for an RFC only, but is fully functional, > > > > > > tested and applies cleanly on top of the latest 4.7-rc4+. > > > > > > > > > > > > We need to re-validate all cached iint entries once a key is moved to > > > > > > .ima_blacklist keyring. This is being done on demand when iint is > > > > > > consulted. > > > > > > > > > > > > In order do make such verification we need some extra data stored in "struct > > > > > > iint". One is a time stamp and the other is a pointer to the signature > > > > > > verification key. I boldly assumed the latter is immutable, but David > > > > > > should correct me if i am wrong. > > > > > > > > > > Right, I'm not sure that having a pointer to the key is so safe. It would > > > > > > > > It does not feel right. However, i did not see re-allocation of the key storage > > > > memory anywhere in the code. I guess David should be the ultimate decision > > > > maker here. > > > > > > What happens if the key is updated/deleted? Aren't we removing the key > > > from the secondary keyring and adding it to the blacklisted keyring? Is > > If a key is updated or moved around different keyrings the originally allocated > memory stays the same (iow, same address). If it get's deleted then we're in > trouble. > > I'll modify the patch to use key's unique ID instead of direct pointer. Based on the discussion so far, if the key is first added to the IMA/secondary blacklist and then deleted from the keyring, the key's unique ID will remain the same. Is that also true if the key is first deleted, and then added to the blacklist keyring? > > > there a difference between the IMA blacklist keyring and the system > > > blacklist keyring in this regards? > > > > That should be the IMA and/or secondary keyrings. > > The system blacklist may hold keys of (new) type "blacklist". It stores > blacklisted hash and has no payload. This keyring should also accept normal > keys. Ok. Let's assume for a moment that any key on the system blacklisted keyring should not be used for signature verification. The only information we have in the security.ima xattr is the key id. As previously suggested, we could save the key-id in the iint and use it for searching the IMA blacklist keyring. Another option would be to save this new blacklist hash type value in the iint. In both cases, the iint entry size increases by the size of the hash. Mimi -- To unsubscribe from this list: send the line "unsubscribe linux-security-module" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/security/integrity/digsig.c b/security/integrity/digsig.c index 4304372..483f870 100644 --- a/security/integrity/digsig.c +++ b/security/integrity/digsig.c @@ -49,7 +49,8 @@ static bool init_keyring __initdata; #endif int integrity_digsig_verify(const unsigned int id, const char *sig, int siglen, - const char *digest, int digestlen) + const char *digest, int digestlen, + struct integrity_iint_cache *iint) { if (id >= INTEGRITY_KEYRING_MAX) return -EINVAL; @@ -72,7 +73,7 @@ int integrity_digsig_verify(const unsigned int id, const char *sig, int siglen, digest, digestlen); case 2: return asymmetric_verify(keyring[id], sig, siglen, - digest, digestlen); + digest, digestlen, iint); } return -EOPNOTSUPP; diff --git a/security/integrity/digsig_asymmetric.c b/security/integrity/digsig_asymmetric.c index 80052ed..40657df 100644 --- a/security/integrity/digsig_asymmetric.c +++ b/security/integrity/digsig_asymmetric.c @@ -80,7 +80,8 @@ static struct key *request_asymmetric_key(struct key *keyring, uint32_t keyid) } int asymmetric_verify(struct key *keyring, const char *sig, - int siglen, const char *data, int datalen) + int siglen, const char *data, int datalen, + struct integrity_iint_cache *iint) { struct public_key_signature pks; struct signature_v2_hdr *hdr = (struct signature_v2_hdr *)sig; @@ -113,5 +114,11 @@ int asymmetric_verify(struct key *keyring, const char *sig, ret = verify_signature(key, &pks); key_put(key); pr_debug("%s() = %d\n", __func__, ret); +#ifdef CONFIG_IMA_BLACKLIST_KEYRING + if (!ret && iint) { + iint->key = key; + iint->last_time = current_kernel_time().tv_sec; + } +#endif return ret; } diff --git a/security/integrity/evm/evm_main.c b/security/integrity/evm/evm_main.c index b9e2628..0c0a93a 100644 --- a/security/integrity/evm/evm_main.c +++ b/security/integrity/evm/evm_main.c @@ -161,7 +161,7 @@ static enum integrity_status evm_verify_hmac(struct dentry *dentry, break; rc = integrity_digsig_verify(INTEGRITY_KEYRING_EVM, (const char *)xattr_data, xattr_len, - calc.digest, sizeof(calc.digest)); + calc.digest, sizeof(calc.digest), NULL); if (!rc) { /* Replace RSA with HMAC if not mounted readonly and * not immutable @@ -237,7 +237,7 @@ enum integrity_status evm_verifyxattr(struct dentry *dentry, if (!iint) { iint = integrity_iint_find(d_backing_inode(dentry)); - if (!iint) + if (!iint || IS_ERR(iint)) return INTEGRITY_UNKNOWN; } return evm_verify_hmac(dentry, xattr_name, xattr_value, @@ -295,6 +295,8 @@ static int evm_protect_xattr(struct dentry *dentry, const char *xattr_name, struct integrity_iint_cache *iint; iint = integrity_iint_find(d_backing_inode(dentry)); + if (IS_ERR(iint)) + return -EPERM; if (iint && (iint->flags & IMA_NEW_FILE)) return 0; @@ -364,6 +366,8 @@ static void evm_reset_status(struct inode *inode) struct integrity_iint_cache *iint; iint = integrity_iint_find(inode); + if (IS_ERR(iint)) + return; if (iint) iint->evm_status = INTEGRITY_UNKNOWN; } diff --git a/security/integrity/iint.c b/security/integrity/iint.c index 345b759..68f95f3 100644 --- a/security/integrity/iint.c +++ b/security/integrity/iint.c @@ -21,12 +21,62 @@ #include <linux/rbtree.h> #include <linux/file.h> #include <linux/uaccess.h> +#include <linux/audit.h> +#include <keys/system_keyring.h> +#include <keys/asymmetric-type.h> #include "integrity.h" static struct rb_root integrity_iint_tree = RB_ROOT; static DEFINE_RWLOCK(integrity_iint_lock); static struct kmem_cache *iint_cache __read_mostly; +#ifdef CONFIG_IMA_BLACKLIST_KEYRING +static int keys_in_keyring(struct key *keyring) +{ + if (key_is_instantiated(keyring)) { + return keyring->keys.nr_leaves_on_tree; + } + + return -EINVAL; +} + +/* + * returns negative if the key is not blacklisted and 0 if it is; + */ +static int iint_bl_check(struct integrity_iint_cache *iint) +{ + struct key *bl; + key_ref_t bl_t, key=ERR_PTR(-ENOKEY); + + if (!iint) + return -1; + + if (iint->flags & IMA_IINT_BLACKLISTED) + return 0; + + bl = get_ima_blacklist_keyring(); + if (!bl) + return -2; + + if (!keys_in_keyring(bl)) + return -3; + + if (iint->last_time > bl->last_used_at) + return -4; + + bl_t = make_key_ref(bl, 1); + if (iint->key) + key = keyring_search(bl_t, &key_type_asymmetric, iint->key->description); + + if (IS_ERR(key)) + return -5; + + iint->flags |= IMA_IINT_BLACKLISTED; + + return 0; +} +#endif /* CONFIG_IMA_BLACKLIST_KEYRING */ + /* * __integrity_iint_find - return the iint associated with an inode */ @@ -52,7 +102,8 @@ static struct integrity_iint_cache *__integrity_iint_find(struct inode *inode) } /* - * integrity_iint_find - return the iint associated with an inode + * integrity_iint_find - return the iint associated with an inode, NULL or + * an error if the corrsponding key has been blacklisted in the meantime. */ struct integrity_iint_cache *integrity_iint_find(struct inode *inode) { @@ -65,6 +116,14 @@ struct integrity_iint_cache *integrity_iint_find(struct inode *inode) iint = __integrity_iint_find(inode); read_unlock(&integrity_iint_lock); +#ifdef CONFIG_IMA_BLACKLIST_KEYRING + if (!iint_bl_check(iint)) { + integrity_audit_msg(AUDIT_INTEGRITY_DATA, inode, NULL, + "appraise_data", "blacklisted-key", + -EINVAL, 0); + return ERR_PTR(-EINVAL); + } +#endif return iint; } @@ -79,6 +138,10 @@ static void iint_free(struct integrity_iint_cache *iint) iint->ima_bprm_status = INTEGRITY_UNKNOWN; iint->ima_read_status = INTEGRITY_UNKNOWN; iint->evm_status = INTEGRITY_UNKNOWN; +#ifdef CONFIG_IMA_BLACKLIST_KEYRING + iint->key = NULL; + iint->last_time = 0; +#endif kmem_cache_free(iint_cache, iint); } @@ -159,6 +222,10 @@ static void init_once(void *foo) iint->ima_bprm_status = INTEGRITY_UNKNOWN; iint->ima_read_status = INTEGRITY_UNKNOWN; iint->evm_status = INTEGRITY_UNKNOWN; +#ifdef CONFIG_IMA_BLACKLIST_KEYRING + iint->key = NULL; + iint->last_time = 0; +#endif } static int __init integrity_iintcache_init(void) diff --git a/security/integrity/ima/ima_appraise.c b/security/integrity/ima/ima_appraise.c index 1bcbc12..4c8bca2 100644 --- a/security/integrity/ima/ima_appraise.c +++ b/security/integrity/ima/ima_appraise.c @@ -252,7 +252,7 @@ int ima_appraise_measurement(enum ima_hooks func, rc = integrity_digsig_verify(INTEGRITY_KEYRING_IMA, (const char *)xattr_value, rc, iint->ima_hash->digest, - iint->ima_hash->length); + iint->ima_hash->length, iint); if (rc == -EOPNOTSUPP) { status = INTEGRITY_UNKNOWN; } else if (rc) { @@ -330,7 +330,7 @@ void ima_inode_post_setattr(struct dentry *dentry) must_appraise = ima_must_appraise(inode, MAY_ACCESS, POST_SETATTR); iint = integrity_iint_find(inode); - if (iint) { + if (iint && !IS_ERR(iint)) { iint->flags &= ~(IMA_APPRAISE | IMA_APPRAISED | IMA_APPRAISE_SUBMASK | IMA_APPRAISED_SUBMASK | IMA_ACTION_RULE_FLAGS); @@ -366,7 +366,7 @@ static void ima_reset_appraise_flags(struct inode *inode, int digsig) return; iint = integrity_iint_find(inode); - if (!iint) + if (!iint || IS_ERR(iint)) return; iint->flags &= ~IMA_DONE_MASK; diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c index 68b26c3..7c866d8 100644 --- a/security/integrity/ima/ima_main.c +++ b/security/integrity/ima/ima_main.c @@ -90,6 +90,8 @@ static void ima_rdwr_violation_check(struct file *file, if (atomic_read(&inode->i_readcount) && IS_IMA(inode)) { if (!iint) iint = integrity_iint_find(inode); + if (IS_ERR(iint)) + return; /* IMA_MEASURE is set from reader side */ if (iint && (iint->flags & IMA_MEASURE)) send_tomtou = true; @@ -147,7 +149,7 @@ void ima_file_free(struct file *file) return; iint = integrity_iint_find(inode); - if (!iint) + if (!iint || IS_ERR(iint)) return; ima_check_last_writer(iint, inode, file); @@ -190,7 +192,7 @@ static int process_measurement(struct file *file, char *buf, loff_t size, if (action) { iint = integrity_inode_get(inode); - if (!iint) + if (!iint || IS_ERR(iint)) goto out; } @@ -334,7 +336,7 @@ void ima_post_path_mknod(struct dentry *dentry) return; iint = integrity_inode_get(inode); - if (iint) + if (iint && !IS_ERR(iint)) iint->flags |= IMA_NEW_FILE; } diff --git a/security/integrity/integrity.h b/security/integrity/integrity.h index 90bc57d..6076762 100644 --- a/security/integrity/integrity.h +++ b/security/integrity/integrity.h @@ -53,6 +53,9 @@ #define IMA_APPRAISED_SUBMASK (IMA_FILE_APPRAISED | IMA_MMAP_APPRAISED | \ IMA_BPRM_APPRAISED | IMA_READ_APPRAISED) +#define IMA_IINT_BLACKLISTED_BIT (BITS_PER_LONG - 1) +#define IMA_IINT_BLACKLISTED (1UL << IMA_IINT_BLACKLISTED_BIT) + enum evm_ima_xattr_type { IMA_XATTR_DIGEST = 0x01, EVM_XATTR_HMAC, @@ -109,6 +112,10 @@ struct integrity_iint_cache { enum integrity_status ima_read_status:4; enum integrity_status evm_status:4; struct ima_digest_data *ima_hash; +#ifdef CONFIG_IMA_BLACKLIST_KEYRING + struct key *key; + time_t last_time; +#endif }; /* rbtree tree calls to lookup, insert, delete @@ -128,7 +135,8 @@ int __init integrity_read_file(const char *path, char **data); #ifdef CONFIG_INTEGRITY_SIGNATURE int integrity_digsig_verify(const unsigned int id, const char *sig, int siglen, - const char *digest, int digestlen); + const char *digest, int digestlen, + struct integrity_iint_cache *iint); int __init integrity_init_keyring(const unsigned int id); int __init integrity_load_x509(const unsigned int id, const char *path); @@ -136,7 +144,8 @@ int __init integrity_load_x509(const unsigned int id, const char *path); static inline int integrity_digsig_verify(const unsigned int id, const char *sig, int siglen, - const char *digest, int digestlen) + const char *digest, int digestlen, + struct integrity_iint_cache *iint) { return -EOPNOTSUPP; } @@ -149,10 +158,12 @@ static inline int integrity_init_keyring(const unsigned int id) #ifdef CONFIG_INTEGRITY_ASYMMETRIC_KEYS int asymmetric_verify(struct key *keyring, const char *sig, - int siglen, const char *data, int datalen); + int siglen, const char *data, int datalen, + struct integrity_iint_cache *); #else static inline int asymmetric_verify(struct key *keyring, const char *sig, - int siglen, const char *data, int datalen) + int siglen, const char *data, int datalen, + struct integrity_iint_cache *) { return -EOPNOTSUPP; }