From patchwork Sat Jun 25 10:17:06 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Petko Manolov X-Patchwork-Id: 9198503 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id 11E696077D for ; Sat, 25 Jun 2016 10:16:58 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id E9C61284F9 for ; Sat, 25 Jun 2016 10:16:57 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id DDB4028504; Sat, 25 Jun 2016 10:16:57 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-6.8 required=2.0 tests=BAYES_00,DKIM_SIGNED, RCVD_IN_DNSWL_HI,T_DKIM_INVALID autolearn=ham version=3.3.1 Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 9E3E9284F9 for ; Sat, 25 Jun 2016 10:16:56 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751209AbcFYKQz (ORCPT ); Sat, 25 Jun 2016 06:16:55 -0400 Received: from lan.nucleusys.com ([92.247.61.126]:37508 "EHLO zztop.nucleusys.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1751185AbcFYKQx (ORCPT ); Sat, 25 Jun 2016 06:16:53 -0400 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=mip-labs.com; s=x; h=Content-Type:MIME-Version:Message-ID:Subject:Cc:To:From:Date; bh=ZpPboupKSB8UTKFrpq9TCgN2Vyj+lcDx5JJKSJvjLM8=; b=a7B/vzPLaA2VruRpJU9+6CtyGqZ7myffuO9GaGmC/32/l2ynpYxv27fMwg0iduHR5mhuckALEDDNSX8hiNc/7waMMIqHnWNolnFeteWnhr/AhYewoUMpr3Z0Selr6mU55/9h9JaL0nSNZgFeMf95nz/LEq6RAqA9FZSK1t9v/fA=; Received: from 78-83-74-100.spectrumnet.bg ([78.83.74.100] helo=p310) by zztop.nucleusys.com with esmtpsa (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.84_2) (envelope-from ) id 1bGkdo-0004uI-R9; Sat, 25 Jun 2016 13:16:45 +0300 Date: Sat, 25 Jun 2016 13:17:06 +0300 From: Petko Manolov To: linux-security-module@vger.kernel.org Cc: zohar@linux.vnet.ibm.com, dhowells@redhat.com, mdb@juniper.net Subject: [IMA] [RFC] blacklist keyring Message-ID: <20160625101706.GD19933@p310> Mail-Followup-To: linux-security-module@vger.kernel.org, zohar@linux.vnet.ibm.com, dhowells@redhat.com, mdb@juniper.net MIME-Version: 1.0 Content-Disposition: inline User-Agent: Mutt/1.6.0 (2016-04-01) Sender: owner-linux-security-module@vger.kernel.org Precedence: bulk List-ID: X-Virus-Scanned: ClamAV using ClamSMTP 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. 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. 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. Please let me know what you think, especially in case i've done something stupid. cheers, Petko From 36daa48006830601db177761bdfcb0115ea5f741 Mon Sep 17 00:00:00 2001 From: Petko Manolov Date: Sat, 25 Jun 2016 12:45:13 +0300 Subject: [PATCH] ima blacklist experiment Signed-off-by: Petko Manolov --- security/integrity/digsig.c | 5 ++- security/integrity/digsig_asymmetric.c | 9 ++++- security/integrity/evm/evm_main.c | 8 +++- security/integrity/iint.c | 69 +++++++++++++++++++++++++++++++++- security/integrity/ima/ima_appraise.c | 6 +-- security/integrity/ima/ima_main.c | 8 ++-- security/integrity/integrity.h | 19 ++++++++-- 7 files changed, 108 insertions(+), 16 deletions(-) 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 #include #include +#include +#include +#include #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; }