From patchwork Thu Nov 30 10:56:07 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Roberto Sassu X-Patchwork-Id: 10084593 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 91D0F6035E for ; Thu, 30 Nov 2017 11:00:37 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 6C00A29ED1 for ; Thu, 30 Nov 2017 11:00:37 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 60EC429F3E; Thu, 30 Nov 2017 11:00:37 +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.9 required=2.0 tests=BAYES_00,RCVD_IN_DNSWL_HI 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 7AC2F29ED1 for ; Thu, 30 Nov 2017 11:00:36 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752352AbdK3LAg (ORCPT ); Thu, 30 Nov 2017 06:00:36 -0500 Received: from lhrrgout.huawei.com ([194.213.3.17]:61898 "EHLO huawei.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1752303AbdK3LAe (ORCPT ); Thu, 30 Nov 2017 06:00:34 -0500 Received: from LHREML713-CAH.china.huawei.com (unknown [172.18.7.107]) by Forcepoint Email with ESMTP id 6EDD6DB567DD0; Thu, 30 Nov 2017 11:00:31 +0000 (GMT) Received: from localhost.localdomain (10.204.65.254) by smtpsuk.huawei.com (10.201.108.36) with Microsoft SMTP Server (TLS) id 14.3.361.1; Thu, 30 Nov 2017 11:00:23 +0000 From: Roberto Sassu To: CC: , , Roberto Sassu Subject: [RFC][PATCH v2 6/9] ima: enforce the Biba strict policy on appraised files Date: Thu, 30 Nov 2017 11:56:07 +0100 Message-ID: <20171130105610.15761-7-roberto.sassu@huawei.com> X-Mailer: git-send-email 2.11.0 In-Reply-To: <20171130105610.15761-1-roberto.sassu@huawei.com> References: <20171130105610.15761-1-roberto.sassu@huawei.com> MIME-Version: 1.0 X-Originating-IP: [10.204.65.254] X-CFilter-Loop: Reflected Sender: linux-integrity-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-integrity@vger.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP IMA does not make any distinction between immutable and mutable files. The measurement list often contains unknown digests of mutable files, which cannot be recognized by comparing them with reference values. If mutable files can be modified only by the TCB, and are protected against offline attacks, it wouldn't be necessary to measure them. Security policies for general purpose systems often do not satisfy the requirements of an integrity model. Thus, mutable files have to be measured because their integrity cannot be guaranteed. With this patch, IMA can enforce the constraints of the Biba strict model on top of existing LSMs. The decision is made depending on the IMA policy, which should specify criteria for TCB processes, and depending on the appraisal status. This patch ensures that: files with a valid appraisal status can be written only by processes in the TCB (write down constraint); every file read by a process in the TCB must have a valid appraisal status (read up constraint). Changelog v1: - added loop in integrity_policy_setup() - check read up violations - clear IMA_APPRAISE if there is a policy violation - kernel parameter ima_integrity_policy renamed to ima_integrity_model - detect write down violations only if appraisal status is valid Signed-off-by: Roberto Sassu --- Documentation/admin-guide/kernel-parameters.txt | 4 + security/integrity/ima/ima.h | 16 ++++ security/integrity/ima/ima_appraise.c | 101 ++++++++++++++++++++++++ security/integrity/ima/ima_main.c | 29 ++++--- 4 files changed, 141 insertions(+), 9 deletions(-) diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt index 05496622b4ef..6d184a94ba20 100644 --- a/Documentation/admin-guide/kernel-parameters.txt +++ b/Documentation/admin-guide/kernel-parameters.txt @@ -1532,6 +1532,10 @@ [IMA] Define a custom template format. Format: { "field1|...|fieldN" } + ima_integrity_model= + [IMA] Select an integrity model. + Models: { "biba-strict" } + ima.ahash_minsize= [IMA] Minimum file size for asynchronous hash usage Format: Set the minimal file size for using asynchronous hash. diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h index f2cdd528f8ff..b9fdf0264c9d 100644 --- a/security/integrity/ima/ima.h +++ b/security/integrity/ima/ima.h @@ -36,6 +36,8 @@ enum ima_show_type { IMA_SHOW_BINARY, IMA_SHOW_BINARY_NO_FIELD_LEN, IMA_SHOW_BINARY_OLD_STRING_FMT, IMA_SHOW_ASCII }; enum tpm_pcrs { TPM_PCR0 = 0, TPM_PCR8 = 8 }; +enum integrity_models { BIBA_STRICT = 1, INTEGRITY_POLICY__LAST }; + /* digest size for IMA, fits SHA1 or MD5 */ #define IMA_DIGEST_SIZE SHA1_DIGEST_SIZE #define IMA_EVENT_NAME_LEN_MAX 255 @@ -58,6 +60,7 @@ extern int ima_used_chip; extern int ima_hash_algo; extern int ima_appraise; extern struct path ima_null; +extern int ima_integrity_model; /* IMA event related data */ struct ima_event_data { @@ -249,6 +252,10 @@ enum hash_algo ima_get_hash_algo(struct evm_ima_xattr_data *xattr_value, int xattr_len); int ima_read_xattr(struct dentry *dentry, struct evm_ima_xattr_data **xattr_value); +bool ima_appraise_integrity_check(struct file *file, + struct integrity_iint_cache *iint, + int must_appraise, char **pathbuf, + const char **pathname, char *namebuf); #else static inline int ima_appraise_measurement(enum ima_hooks func, @@ -291,6 +298,15 @@ static inline int ima_read_xattr(struct dentry *dentry, return 0; } +static inline bool ima_appraise_integrity_check(struct file *file, + struct integrity_iint_cache *iint, + int must_appraise, char **pathbuf, + const char **pathname, + char *namebuf) +{ + return false; +} + #endif /* CONFIG_IMA_APPRAISE */ /* LSM based policy rules require audit */ diff --git a/security/integrity/ima/ima_appraise.c b/security/integrity/ima/ima_appraise.c index 23e025e86aed..27bbf48596cb 100644 --- a/security/integrity/ima/ima_appraise.c +++ b/security/integrity/ima/ima_appraise.c @@ -18,6 +18,12 @@ #include "ima.h" +int ima_integrity_model; + +static char *ima_integrity_models_str[INTEGRITY_POLICY__LAST] = { + [BIBA_STRICT] = "biba-strict", +}; + static int __init default_appraise_setup(char *str) { #ifdef CONFIG_IMA_APPRAISE_BOOTPARAM @@ -33,6 +39,27 @@ static int __init default_appraise_setup(char *str) __setup("ima_appraise=", default_appraise_setup); +static int __init integrity_model_setup(char *str) +{ + int i; + + for (i = BIBA_STRICT; i < INTEGRITY_POLICY__LAST; i++) { + if (strcmp(str, ima_integrity_models_str[i]) == 0) { + ima_integrity_model = i; + break; + } + } + + if (i == INTEGRITY_POLICY__LAST) { + pr_err("Invalid model %s, using %s\n", str, + ima_integrity_models_str[BIBA_STRICT]); + ima_integrity_model = BIBA_STRICT; + } + + return 1; +} +__setup("ima_integrity_model=", integrity_model_setup); + /* * is_ima_appraise_enabled - return appraise status * @@ -197,6 +224,80 @@ int ima_read_xattr(struct dentry *dentry, return ret; } +bool ima_inode_is_appraised(struct dentry *dentry, struct inode *inode) +{ + ssize_t len; + + len = __vfs_getxattr(dentry, inode, XATTR_NAME_IMA, NULL, 0); + + return (len > 0 && len >= sizeof(struct evm_ima_xattr_data)); +} + +/* + * ima_appraise_integrity_check - detect violations of an integrity policy + * + * The appraisal policy identifies which subjects belong to the TCB. Files + * with a valid digital signature or HMAC are also part of the TCB. This + * function enforces constraints of the selected integrity model. + * + * Conventions: + * - must_appraise: true (TCB subject), false (non-TCB subject) + * - iint status: INTEGRITY_PASS (TCB object), other values (non-TCB object) + * + * Constraints: + * - write down: TCB object can be written only by TCB subject + * - read up: TCB subject can read only TCB objects + * + * Integrity models: + * - biba-strict: write down, read up + * + * Return: true if constraints are violated, false otherwise + */ +bool ima_appraise_integrity_check(struct file *file, + struct integrity_iint_cache *iint, + int must_appraise, char **pathbuf, + const char **pathname, char *namebuf) +{ + struct inode *inode = file_inode(file); + fmode_t mode = file->f_mode; + char *cause = "write_down_violation"; + int expected_writecount = (mode & FMODE_WRITE) ? 1 : 0; + + /* check write down violations */ + if (!must_appraise && (mode & FMODE_WRITE)) { + if (IS_IMA(inode)) { + if (!iint) + iint = integrity_iint_find(inode); + if (iint->flags & IMA_APPRAISED) + goto out_violation; + } else if (ima_inode_is_appraised(file_dentry(file), inode)) { + goto out_violation; + } + /* concurrent write to non-TCB object by non-TCB subjects */ + } else if (must_appraise && !(iint->flags & IMA_APPRAISED) && + atomic_read(&inode->i_writecount) > expected_writecount) { + /* updating security.ima will cause a write down violation */ + if (mode & FMODE_WRITE) { + cause = "write_down_violation_tcb"; + goto out_violation; + } + + /* check read up violations */ + if (mode & (FMODE_READ | FMODE_EXEC)) { + cause = "read_up_violation"; + goto out_violation; + } + } + + return false; +out_violation: + *pathname = ima_d_path(&file->f_path, pathbuf, namebuf); + integrity_audit_msg(AUDIT_INTEGRITY_DATA, inode, *pathname, + ima_integrity_models_str[ima_integrity_model], + cause, 0, 0); + return true; +} + /* * ima_appraise_measurement - appraise file measurement * diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c index a12f8a148e5e..ee9897c8d0cc 100644 --- a/security/integrity/ima/ima_main.c +++ b/security/integrity/ima/ima_main.c @@ -108,7 +108,8 @@ static void ima_rdwr_violation_check(struct file *file, if (!send_tomtou && !send_writers) return; - *pathname = ima_d_path(&file->f_path, pathbuf, filename); + if (!*pathname) + *pathname = ima_d_path(&file->f_path, pathbuf, filename); if (send_tomtou) ima_add_violation(file, *pathname, iint, @@ -181,7 +182,7 @@ static int process_measurement(struct file *file, const struct cred *cred, int pcr = CONFIG_IMA_MEASURE_PCR_IDX; struct evm_ima_xattr_data *xattr_value = NULL; int xattr_len = 0; - bool violation_check; + bool violation_check, model_violation = false; enum hash_algo hash_algo; if (!ima_policy_flag || !S_ISREG(inode->i_mode)) @@ -194,7 +195,7 @@ static int process_measurement(struct file *file, const struct cred *cred, action = ima_get_action(inode, cred, mask, func, &pcr); violation_check = ((func == FILE_CHECK || func == MMAP_CHECK) && (ima_policy_flag & IMA_MEASURE)); - if (!action && !violation_check) + if (!action && !violation_check && !ima_integrity_model) return 0; must_appraise = action & IMA_APPRAISE; @@ -211,13 +212,21 @@ static int process_measurement(struct file *file, const struct cred *cred, goto out; } - if (violation_check) { + if (ima_integrity_model) { + model_violation = ima_appraise_integrity_check(file, iint, + must_appraise, &pathbuf, + &pathname, filename); + /* access will be denied */ + if (model_violation) + action &= ~IMA_APPRAISE; + } + + if (violation_check) ima_rdwr_violation_check(file, iint, action & IMA_MEASURE, &pathbuf, &pathname, filename); - if (!action) { - rc = 0; - goto out_free; - } + if (!action) { + rc = 0; + goto out_free; } /* Determine if already appraised/measured based on bitmask @@ -275,7 +284,9 @@ static int process_measurement(struct file *file, const struct cred *cred, __putname(pathbuf); out: inode_unlock(inode); - if ((rc && must_appraise) && (ima_appraise & IMA_APPRAISE_ENFORCE)) + if (((rc && must_appraise) || + (ima_integrity_model && model_violation)) && + (ima_appraise & IMA_APPRAISE_ENFORCE)) return -EACCES; return 0; }