From patchwork Thu Jul 9 06:19:00 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Tyler Hicks X-Patchwork-Id: 11653393 Return-Path: Received: from mail.kernel.org (pdx-korg-mail-1.web.codeaurora.org [172.30.200.123]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id E4F5D14DD for ; Thu, 9 Jul 2020 06:19:55 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id CE72020708 for ; Thu, 9 Jul 2020 06:19:55 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (1024-bit key) header.d=linux.microsoft.com header.i=@linux.microsoft.com header.b="biInM4kE" Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1726291AbgGIGTy (ORCPT ); Thu, 9 Jul 2020 02:19:54 -0400 Received: from linux.microsoft.com ([13.77.154.182]:37902 "EHLO linux.microsoft.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1725787AbgGIGTx (ORCPT ); Thu, 9 Jul 2020 02:19:53 -0400 Received: from sequoia.work.tihix.com (162-237-133-238.lightspeed.rcsntx.sbcglobal.net [162.237.133.238]) by linux.microsoft.com (Postfix) with ESMTPSA id 7E12A20B4909; Wed, 8 Jul 2020 23:19:51 -0700 (PDT) DKIM-Filter: OpenDKIM Filter v2.11.0 linux.microsoft.com 7E12A20B4909 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linux.microsoft.com; s=default; t=1594275592; bh=KhZwoc2SCUpP770b/4tJ58Uu4y08B6iRQ7dPnzlnwD8=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=biInM4kEQS4NasN3kKHheyzTLbgnWBk3+nP4pxVXoeom740lgyOMok/eZzqruVW/W coKsxQVN7PgraddnY4TqjfhoM4CZjn1SiNZSKl8pP3QTwmg/IlJWrkJQQR3NuOK0Yz dLQ/EdLr9q0zm5BXLSpPbt9F78sbINTo4tG+vJLY= From: Tyler Hicks To: Mimi Zohar , Dmitry Kasatkin Cc: James Morris , "Serge E . Hallyn" , Lakshmi Ramasubramanian , Prakhar Srivastava , linux-kernel@vger.kernel.org, linux-integrity@vger.kernel.org, linux-security-module@vger.kernel.org, Janne Karhunen , Casey Schaufler Subject: [PATCH v3 01/12] ima: Have the LSM free its audit rule Date: Thu, 9 Jul 2020 01:19:00 -0500 Message-Id: <20200709061911.954326-2-tyhicks@linux.microsoft.com> X-Mailer: git-send-email 2.25.1 In-Reply-To: <20200709061911.954326-1-tyhicks@linux.microsoft.com> References: <20200709061911.954326-1-tyhicks@linux.microsoft.com> MIME-Version: 1.0 Sender: owner-linux-security-module@vger.kernel.org Precedence: bulk List-ID: Ask the LSM to free its audit rule rather than directly calling kfree(). Both AppArmor and SELinux do additional work in their audit_rule_free() hooks. Fix memory leaks by allowing the LSMs to perform necessary work. Fixes: b16942455193 ("ima: use the lsm policy update notifier") Signed-off-by: Tyler Hicks Cc: Janne Karhunen Cc: Casey Schaufler Reviewed-by: Mimi Zohar --- * v3 - No change * v2 - Fixed build warning by dropping the 'return -EINVAL' from the stubbed out security_filter_rule_free() since it has a void return type - Added Mimi's Reviewed-by - Developed a follow-on patch to rename security_filter_rule_*() functions, to address Casey's request, but I'll submit it independently of this patch series since it is somewhat unrelated security/integrity/ima/ima.h | 5 +++++ security/integrity/ima/ima_policy.c | 2 +- 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h index 4515975cc540..59ec28f5c117 100644 --- a/security/integrity/ima/ima.h +++ b/security/integrity/ima/ima.h @@ -420,6 +420,7 @@ static inline void ima_free_modsig(struct modsig *modsig) #ifdef CONFIG_IMA_LSM_RULES #define security_filter_rule_init security_audit_rule_init +#define security_filter_rule_free security_audit_rule_free #define security_filter_rule_match security_audit_rule_match #else @@ -430,6 +431,10 @@ static inline int security_filter_rule_init(u32 field, u32 op, char *rulestr, return -EINVAL; } +static inline void security_filter_rule_free(void *lsmrule) +{ +} + static inline int security_filter_rule_match(u32 secid, u32 field, u32 op, void *lsmrule) { diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c index 66aa3e17a888..d7c268c2b0ce 100644 --- a/security/integrity/ima/ima_policy.c +++ b/security/integrity/ima/ima_policy.c @@ -258,7 +258,7 @@ static void ima_lsm_free_rule(struct ima_rule_entry *entry) int i; for (i = 0; i < MAX_LSM_RULES; i++) { - kfree(entry->lsm[i].rule); + security_filter_rule_free(entry->lsm[i].rule); kfree(entry->lsm[i].args_p); } kfree(entry); From patchwork Thu Jul 9 06:19:01 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Tyler Hicks X-Patchwork-Id: 11653395 Return-Path: Received: from mail.kernel.org (pdx-korg-mail-1.web.codeaurora.org [172.30.200.123]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id D3EAD14B7 for ; Thu, 9 Jul 2020 06:20:00 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id BCE5B20708 for ; Thu, 9 Jul 2020 06:20:00 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (1024-bit key) header.d=linux.microsoft.com header.i=@linux.microsoft.com header.b="YOmyAKXV" Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1726211AbgGIGUA (ORCPT ); Thu, 9 Jul 2020 02:20:00 -0400 Received: from linux.microsoft.com ([13.77.154.182]:37938 "EHLO linux.microsoft.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1725787AbgGIGT6 (ORCPT ); Thu, 9 Jul 2020 02:19:58 -0400 Received: from sequoia.work.tihix.com (162-237-133-238.lightspeed.rcsntx.sbcglobal.net [162.237.133.238]) by linux.microsoft.com (Postfix) with ESMTPSA id CA21A20B4909; Wed, 8 Jul 2020 23:19:56 -0700 (PDT) DKIM-Filter: OpenDKIM Filter v2.11.0 linux.microsoft.com CA21A20B4909 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linux.microsoft.com; s=default; t=1594275597; bh=eIE8kXA186NO1r5n/4ptF6EogSRK2Bt/9uDqsHqZ/7E=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=YOmyAKXV8UChPB689eZr6B/Yd8Wn28MDRLTbiEEQGiOVzdwMBhq60LKzQac2ah3JW dK2Hwov48aj/rB47nUWYLOXcoT9+06sJghtFZ77EOmmHu3T2tzguNb8Yrmv/DTWXwz hbBGzQNg1lvHYhcXAu8VdKMjxKn/FahDpz8Q4go4= From: Tyler Hicks To: Mimi Zohar , Dmitry Kasatkin Cc: James Morris , "Serge E . Hallyn" , Lakshmi Ramasubramanian , Prakhar Srivastava , linux-kernel@vger.kernel.org, linux-integrity@vger.kernel.org, linux-security-module@vger.kernel.org Subject: [PATCH v3 02/12] ima: Free the entire rule when deleting a list of rules Date: Thu, 9 Jul 2020 01:19:01 -0500 Message-Id: <20200709061911.954326-3-tyhicks@linux.microsoft.com> X-Mailer: git-send-email 2.25.1 In-Reply-To: <20200709061911.954326-1-tyhicks@linux.microsoft.com> References: <20200709061911.954326-1-tyhicks@linux.microsoft.com> MIME-Version: 1.0 Sender: owner-linux-security-module@vger.kernel.org Precedence: bulk List-ID: Create a function, ima_free_rule(), to free all memory associated with an ima_rule_entry. Use the new function to fix memory leaks of allocated ima_rule_entry members, such as .fsname and .keyrings, when deleting a list of rules. Make the existing ima_lsm_free_rule() function specific to the LSM audit rule array of an ima_rule_entry and require that callers make an additional call to kfree to free the ima_rule_entry itself. This fixes a memory leak seen when loading by a valid rule that contains an additional piece of allocated memory, such as an fsname, followed by an invalid rule that triggers a policy load failure: # echo -e "dont_measure fsname=securityfs\nbad syntax" > \ /sys/kernel/security/ima/policy -bash: echo: write error: Invalid argument # echo scan > /sys/kernel/debug/kmemleak # cat /sys/kernel/debug/kmemleak unreferenced object 0xffff9bab67ca12c0 (size 16): comm "bash", pid 684, jiffies 4295212803 (age 252.344s) hex dump (first 16 bytes): 73 65 63 75 72 69 74 79 66 73 00 6b 6b 6b 6b a5 securityfs.kkkk. backtrace: [<00000000adc80b1b>] kstrdup+0x2e/0x60 [<00000000d504cb0d>] ima_parse_add_rule+0x7d4/0x1020 [<00000000444825ac>] ima_write_policy+0xab/0x1d0 [<000000002b7f0d6c>] vfs_write+0xde/0x1d0 [<0000000096feedcf>] ksys_write+0x68/0xe0 [<0000000052b544a2>] do_syscall_64+0x56/0xa0 [<000000007ead1ba7>] entry_SYSCALL_64_after_hwframe+0x44/0xa9 Fixes: f1b08bbcbdaf ("ima: define a new policy condition based on the filesystem name") Fixes: 2b60c0ecedf8 ("IMA: Read keyrings= option from the IMA policy") Signed-off-by: Tyler Hicks --- * v3 - No change * v2 - Collapsed patch #2 from v1 of this series, into this patch. This patch now introduces ima_free_rule(). - Existing callers of ima_lsm_free_rule() are doing so to free rules after a successful or failed ima_lsm_copy_rule() and those callers continue to directly call ima_lsm_copy_rule() rather than doing explicit reference ownership and calling ima_free_rule(). - The kfree(entry) of ima_lsm_free_rule() was removed from that function to make it focused on freeing the LSM references. Direct callers of ima_lsm_free_rule() must now call kfree(entry) after ima_lsm_free_rule(). - A comment was added in ima_lsm_update_rule() to clarify why ima_free_rule() isn't being used. security/integrity/ima/ima_policy.c | 29 ++++++++++++++++++++++++----- 1 file changed, 24 insertions(+), 5 deletions(-) diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c index d7c268c2b0ce..bf00b966e87f 100644 --- a/security/integrity/ima/ima_policy.c +++ b/security/integrity/ima/ima_policy.c @@ -261,6 +261,21 @@ static void ima_lsm_free_rule(struct ima_rule_entry *entry) security_filter_rule_free(entry->lsm[i].rule); kfree(entry->lsm[i].args_p); } +} + +static void ima_free_rule(struct ima_rule_entry *entry) +{ + if (!entry) + return; + + /* + * entry->template->fields may be allocated in ima_parse_rule() but that + * reference is owned by the corresponding ima_template_desc element in + * the defined_templates list and cannot be freed here + */ + kfree(entry->fsname); + kfree(entry->keyrings); + ima_lsm_free_rule(entry); kfree(entry); } @@ -302,6 +317,7 @@ static struct ima_rule_entry *ima_lsm_copy_rule(struct ima_rule_entry *entry) out_err: ima_lsm_free_rule(nentry); + kfree(nentry); return NULL; } @@ -315,7 +331,14 @@ static int ima_lsm_update_rule(struct ima_rule_entry *entry) list_replace_rcu(&entry->list, &nentry->list); synchronize_rcu(); + /* + * ima_lsm_copy_rule() shallow copied all references, except for the + * LSM references, from entry to nentry so we only want to free the LSM + * references and the entry itself. All other memory refrences will now + * be owned by nentry. + */ ima_lsm_free_rule(entry); + kfree(entry); return 0; } @@ -1402,15 +1425,11 @@ ssize_t ima_parse_add_rule(char *rule) void ima_delete_rules(void) { struct ima_rule_entry *entry, *tmp; - int i; temp_ima_appraise = 0; list_for_each_entry_safe(entry, tmp, &ima_temp_rules, list) { - for (i = 0; i < MAX_LSM_RULES; i++) - kfree(entry->lsm[i].args_p); - list_del(&entry->list); - kfree(entry); + ima_free_rule(entry); } } From patchwork Thu Jul 9 06:19:02 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Tyler Hicks X-Patchwork-Id: 11653437 Return-Path: Received: from mail.kernel.org (pdx-korg-mail-1.web.codeaurora.org [172.30.200.123]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id CC82D13B1 for ; Thu, 9 Jul 2020 06:20:46 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id B3A5F206C3 for ; Thu, 9 Jul 2020 06:20:46 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (1024-bit key) header.d=linux.microsoft.com header.i=@linux.microsoft.com header.b="CI3VBB9l" Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1726357AbgGIGUF (ORCPT ); Thu, 9 Jul 2020 02:20:05 -0400 Received: from linux.microsoft.com ([13.77.154.182]:37982 "EHLO linux.microsoft.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1725787AbgGIGUC (ORCPT ); Thu, 9 Jul 2020 02:20:02 -0400 Received: from sequoia.work.tihix.com (162-237-133-238.lightspeed.rcsntx.sbcglobal.net [162.237.133.238]) by linux.microsoft.com (Postfix) with ESMTPSA id AEE8E20B4908; Wed, 8 Jul 2020 23:20:00 -0700 (PDT) DKIM-Filter: OpenDKIM Filter v2.11.0 linux.microsoft.com AEE8E20B4908 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linux.microsoft.com; s=default; t=1594275601; bh=mFsiCcE03UNZBsGBJ7WJA9xJRygPO23LFXkCIABirYs=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=CI3VBB9llqU6KKQKyOH1s75SThDSfhmh/I+xrwByx4CB2dE2tzxku+HvARmu0bx6f 1MGXnjGk76xC/vWslwW19xAM4XUlTYn5n06pd1ed/CcmsfLXsFd9hMc2/mZ/wm6vxO 4kY46RZuSJRtBydQqhK+q8Ie4xtQlUPjJUrV6hqo= From: Tyler Hicks To: Mimi Zohar , Dmitry Kasatkin Cc: James Morris , "Serge E . Hallyn" , Lakshmi Ramasubramanian , Prakhar Srivastava , linux-kernel@vger.kernel.org, linux-integrity@vger.kernel.org, linux-security-module@vger.kernel.org Subject: [PATCH v3 03/12] ima: Free the entire rule if it fails to parse Date: Thu, 9 Jul 2020 01:19:02 -0500 Message-Id: <20200709061911.954326-4-tyhicks@linux.microsoft.com> X-Mailer: git-send-email 2.25.1 In-Reply-To: <20200709061911.954326-1-tyhicks@linux.microsoft.com> References: <20200709061911.954326-1-tyhicks@linux.microsoft.com> MIME-Version: 1.0 Sender: owner-linux-security-module@vger.kernel.org Precedence: bulk List-ID: Use ima_free_rule() to fix memory leaks of allocated ima_rule_entry members, such as .fsname and .keyrings, when an error is encountered during rule parsing. Set the args_p pointer to NULL after freeing it in the error path of ima_lsm_rule_init() so that it isn't freed twice. This fixes a memory leak seen when loading an rule that contains an additional piece of allocated memory, such as an fsname, followed by an invalid conditional: # echo "measure fsname=tmpfs bad=cond" > /sys/kernel/security/ima/policy -bash: echo: write error: Invalid argument # echo scan > /sys/kernel/debug/kmemleak # cat /sys/kernel/debug/kmemleak unreferenced object 0xffff98e7e4ece6c0 (size 8): comm "bash", pid 672, jiffies 4294791843 (age 21.855s) hex dump (first 8 bytes): 74 6d 70 66 73 00 6b a5 tmpfs.k. backtrace: [<00000000abab7413>] kstrdup+0x2e/0x60 [<00000000f11ede32>] ima_parse_add_rule+0x7d4/0x1020 [<00000000f883dd7a>] ima_write_policy+0xab/0x1d0 [<00000000b17cf753>] vfs_write+0xde/0x1d0 [<00000000b8ddfdea>] ksys_write+0x68/0xe0 [<00000000b8e21e87>] do_syscall_64+0x56/0xa0 [<0000000089ea7b98>] entry_SYSCALL_64_after_hwframe+0x44/0xa9 Fixes: f1b08bbcbdaf ("ima: define a new policy condition based on the filesystem name") Fixes: 2b60c0ecedf8 ("IMA: Read keyrings= option from the IMA policy") Signed-off-by: Tyler Hicks --- * v3 - No change * v2 - No change security/integrity/ima/ima_policy.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c index bf00b966e87f..e458cd47c099 100644 --- a/security/integrity/ima/ima_policy.c +++ b/security/integrity/ima/ima_policy.c @@ -913,6 +913,7 @@ static int ima_lsm_rule_init(struct ima_rule_entry *entry, if (ima_rules == &ima_default_rules) { kfree(entry->lsm[lsm_rule].args_p); + entry->lsm[lsm_rule].args_p = NULL; result = -EINVAL; } else result = 0; @@ -1404,7 +1405,7 @@ ssize_t ima_parse_add_rule(char *rule) result = ima_parse_rule(p, entry); if (result) { - kfree(entry); + ima_free_rule(entry); integrity_audit_msg(AUDIT_INTEGRITY_STATUS, NULL, NULL, op, "invalid-policy", result, audit_info); From patchwork Thu Jul 9 06:19:03 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Tyler Hicks X-Patchwork-Id: 11653435 Return-Path: Received: from mail.kernel.org (pdx-korg-mail-1.web.codeaurora.org [172.30.200.123]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id 3305517C7 for ; Thu, 9 Jul 2020 06:20:46 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 1BD9B206C3 for ; Thu, 9 Jul 2020 06:20:46 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (1024-bit key) header.d=linux.microsoft.com header.i=@linux.microsoft.com header.b="O4RNW+Dr" Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1726374AbgGIGUF (ORCPT ); Thu, 9 Jul 2020 02:20:05 -0400 Received: from linux.microsoft.com ([13.77.154.182]:37996 "EHLO linux.microsoft.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726356AbgGIGUD (ORCPT ); Thu, 9 Jul 2020 02:20:03 -0400 Received: from sequoia.work.tihix.com (162-237-133-238.lightspeed.rcsntx.sbcglobal.net [162.237.133.238]) by linux.microsoft.com (Postfix) with ESMTPSA id 1AF3320B4909; Wed, 8 Jul 2020 23:20:02 -0700 (PDT) DKIM-Filter: OpenDKIM Filter v2.11.0 linux.microsoft.com 1AF3320B4909 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linux.microsoft.com; s=default; t=1594275602; bh=yI0/g2aB0s9h4ACzf3IyT+lQGzmQDlqQ6wtp8tod5FU=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=O4RNW+DrHFkTk1qvxZmSQHwG7aRBQp4vhDsW1yFMPo79ZpRJ8OOVgj4kYg6+mERq+ tuRJfg5xOoluwdQYo5LFqbRK/juKp98ioIe5aT2QKHwT3atagUB2EelWMZ2ELjos63 ph6+ujmrG9pZwEhRoL4DOgdpq6xRHWPTAGk3cbSc= From: Tyler Hicks To: Mimi Zohar , Dmitry Kasatkin Cc: James Morris , "Serge E . Hallyn" , Lakshmi Ramasubramanian , Prakhar Srivastava , linux-kernel@vger.kernel.org, linux-integrity@vger.kernel.org, linux-security-module@vger.kernel.org Subject: [PATCH v3 04/12] ima: Fail rule parsing when buffer hook functions have an invalid action Date: Thu, 9 Jul 2020 01:19:03 -0500 Message-Id: <20200709061911.954326-5-tyhicks@linux.microsoft.com> X-Mailer: git-send-email 2.25.1 In-Reply-To: <20200709061911.954326-1-tyhicks@linux.microsoft.com> References: <20200709061911.954326-1-tyhicks@linux.microsoft.com> MIME-Version: 1.0 Sender: owner-linux-security-module@vger.kernel.org Precedence: bulk List-ID: Buffer based hook functions, such as KEXEC_CMDLINE and KEY_CHECK, can only measure. The process_buffer_measurement() function quietly ignores all actions except measure so make this behavior clear at the time of policy load. The parsing of the keyrings conditional had a check to ensure that it was only specified with measure actions but the check should be on the hook function and not the keyrings conditional since "appraise func=KEY_CHECK" is not a valid rule. Fixes: b0935123a183 ("IMA: Define a new hook to measure the kexec boot command line arguments") Fixes: 5808611cccb2 ("IMA: Add KEY_CHECK func to measure keys") Signed-off-by: Tyler Hicks --- * v3 - Add comments to ima_validate_rule() to separate/explain the types of validation checks (section for action checks, section for hook function checks, soon to be a section for combination of options checks, etc.) - Removed the "if (entry->flags & IMA_FUNC)" conditional around the switch statement in ima_validate_rule() which reduced the overall indention by a tab. This could be removed because entry->func is NONE when the IMA_FUNC flag is not set. We'll explicitly enforce and then leverage that property in a later patch when we start validating all hook functions in ima_validate_rule(). - Add comment explicitly stating that all hook functions except KEXEC_CMDLINE and KEY_CHECK are still being validated in ima_parse_rule(). * v2 - No change security/integrity/ima/ima_policy.c | 40 +++++++++++++++++++++++++++-- 1 file changed, 38 insertions(+), 2 deletions(-) diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c index e458cd47c099..40c28f1a6a5a 100644 --- a/security/integrity/ima/ima_policy.c +++ b/security/integrity/ima/ima_policy.c @@ -973,6 +973,43 @@ static void check_template_modsig(const struct ima_template_desc *template) #undef MSG } +static bool ima_validate_rule(struct ima_rule_entry *entry) +{ + /* Ensure that the action is set */ + if (entry->action == UNKNOWN) + return false; + + /* + * Ensure that the hook function is compatible with the other + * components of the rule + */ + switch (entry->func) { + case NONE: + case FILE_CHECK: + case MMAP_CHECK: + case BPRM_CHECK: + case CREDS_CHECK: + case POST_SETATTR: + case MODULE_CHECK: + case FIRMWARE_CHECK: + case KEXEC_KERNEL_CHECK: + case KEXEC_INITRAMFS_CHECK: + case POLICY_CHECK: + /* Validation of these hook functions is in ima_parse_rule() */ + break; + case KEXEC_CMDLINE: + case KEY_CHECK: + if (entry->action & ~(MEASURE | DONT_MEASURE)) + return false; + + break; + default: + return false; + } + + return true; +} + static int ima_parse_rule(char *rule, struct ima_rule_entry *entry) { struct audit_buffer *ab; @@ -1150,7 +1187,6 @@ static int ima_parse_rule(char *rule, struct ima_rule_entry *entry) keyrings_len = strlen(args[0].from) + 1; if ((entry->keyrings) || - (entry->action != MEASURE) || (entry->func != KEY_CHECK) || (keyrings_len < 2)) { result = -EINVAL; @@ -1356,7 +1392,7 @@ static int ima_parse_rule(char *rule, struct ima_rule_entry *entry) break; } } - if (!result && (entry->action == UNKNOWN)) + if (!result && !ima_validate_rule(entry)) result = -EINVAL; else if (entry->action == APPRAISE) temp_ima_appraise |= ima_appraise_flag(entry->func); From patchwork Thu Jul 9 06:19:04 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Tyler Hicks X-Patchwork-Id: 11653429 Return-Path: Received: from mail.kernel.org (pdx-korg-mail-1.web.codeaurora.org [172.30.200.123]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id 9885613B1 for ; Thu, 9 Jul 2020 06:20:45 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 7A69D206C3 for ; Thu, 9 Jul 2020 06:20:45 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (1024-bit key) header.d=linux.microsoft.com header.i=@linux.microsoft.com header.b="aJKK1MaG" Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1726410AbgGIGUF (ORCPT ); Thu, 9 Jul 2020 02:20:05 -0400 Received: from linux.microsoft.com ([13.77.154.182]:38020 "EHLO linux.microsoft.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726196AbgGIGUE (ORCPT ); Thu, 9 Jul 2020 02:20:04 -0400 Received: from sequoia.work.tihix.com (162-237-133-238.lightspeed.rcsntx.sbcglobal.net [162.237.133.238]) by linux.microsoft.com (Postfix) with ESMTPSA id 5A5A120B490A; Wed, 8 Jul 2020 23:20:03 -0700 (PDT) DKIM-Filter: OpenDKIM Filter v2.11.0 linux.microsoft.com 5A5A120B490A DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linux.microsoft.com; s=default; t=1594275604; bh=z5BEppbXhpdSAyDudc6kYWlfCNHPdX4otoJgm257sFU=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=aJKK1MaGaR61+T37x4NLFrTDg7UzgxY0bbvDNRKTeO0N00x56mP6ZuB9w0Ri0uvr3 UxXXsGpThVKOY+Fz0po2J1zTGi76glRbydOf7jY8ne+uemlI5bKRPSJOMuzCC0nvtG f/VeQXvFJkeUU50VyYVvW8MN0JKJlcRS1202n82w= From: Tyler Hicks To: Mimi Zohar , Dmitry Kasatkin Cc: James Morris , "Serge E . Hallyn" , Lakshmi Ramasubramanian , Prakhar Srivastava , linux-kernel@vger.kernel.org, linux-integrity@vger.kernel.org, linux-security-module@vger.kernel.org Subject: [PATCH v3 05/12] ima: Fail rule parsing when the KEXEC_CMDLINE hook is combined with an invalid cond Date: Thu, 9 Jul 2020 01:19:04 -0500 Message-Id: <20200709061911.954326-6-tyhicks@linux.microsoft.com> X-Mailer: git-send-email 2.25.1 In-Reply-To: <20200709061911.954326-1-tyhicks@linux.microsoft.com> References: <20200709061911.954326-1-tyhicks@linux.microsoft.com> MIME-Version: 1.0 Sender: owner-linux-security-module@vger.kernel.org Precedence: bulk List-ID: The KEXEC_CMDLINE hook function only supports the pcr conditional. Make this clear at policy load so that IMA policy authors don't assume that other conditionals are supported. Since KEXEC_CMDLINE's inception, ima_match_rules() has always returned true on any loaded KEXEC_CMDLINE rule without any consideration for other conditionals present in the rule. Make it clear that pcr is the only supported KEXEC_CMDLINE conditional by returning an error during policy load. An example of why this is a problem can be explained with the following rule: dont_measure func=KEXEC_CMDLINE obj_type=foo_t An IMA policy author would have assumed that rule is valid because the parser accepted it but the result was that measurements for all KEXEC_CMDLINE operations would be disabled. Fixes: b0935123a183 ("IMA: Define a new hook to measure the kexec boot command line arguments") Signed-off-by: Tyler Hicks Reviewed-by: Mimi Zohar Reviewed-by: Lakshmi Ramasubramanian --- * v3 - Adjust for the indentation change introduced in patch #4 - Added Lakshmi's Reviewed-by * v2 - Added Mimi's Reviewed-by security/integrity/ima/ima_policy.c | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c index 40c28f1a6a5a..1c64bd6f1728 100644 --- a/security/integrity/ima/ima_policy.c +++ b/security/integrity/ima/ima_policy.c @@ -343,6 +343,17 @@ static int ima_lsm_update_rule(struct ima_rule_entry *entry) return 0; } +static bool ima_rule_contains_lsm_cond(struct ima_rule_entry *entry) +{ + int i; + + for (i = 0; i < MAX_LSM_RULES; i++) + if (entry->lsm[i].args_p) + return true; + + return false; +} + /* * The LSM policy can be reloaded, leaving the IMA LSM based rules referring * to the old, stale LSM policy. Update the IMA LSM based rules to reflect @@ -998,6 +1009,16 @@ static bool ima_validate_rule(struct ima_rule_entry *entry) /* Validation of these hook functions is in ima_parse_rule() */ break; case KEXEC_CMDLINE: + if (entry->action & ~(MEASURE | DONT_MEASURE)) + return false; + + if (entry->flags & ~(IMA_FUNC | IMA_PCR)) + return false; + + if (ima_rule_contains_lsm_cond(entry)) + return false; + + break; case KEY_CHECK: if (entry->action & ~(MEASURE | DONT_MEASURE)) return false; From patchwork Thu Jul 9 06:19:05 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Tyler Hicks X-Patchwork-Id: 11653427 Return-Path: Received: from mail.kernel.org (pdx-korg-mail-1.web.codeaurora.org [172.30.200.123]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id 13AAC14B7 for ; Thu, 9 Jul 2020 06:20:44 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id E5553206C3 for ; Thu, 9 Jul 2020 06:20:43 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (1024-bit key) header.d=linux.microsoft.com header.i=@linux.microsoft.com header.b="MRNA0U4/" Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1726497AbgGIGUK (ORCPT ); Thu, 9 Jul 2020 02:20:10 -0400 Received: from linux.microsoft.com ([13.77.154.182]:38036 "EHLO linux.microsoft.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1725787AbgGIGUG (ORCPT ); Thu, 9 Jul 2020 02:20:06 -0400 Received: from sequoia.work.tihix.com (162-237-133-238.lightspeed.rcsntx.sbcglobal.net [162.237.133.238]) by linux.microsoft.com (Postfix) with ESMTPSA id DDD7520B4908; Wed, 8 Jul 2020 23:20:04 -0700 (PDT) DKIM-Filter: OpenDKIM Filter v2.11.0 linux.microsoft.com DDD7520B4908 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linux.microsoft.com; s=default; t=1594275605; bh=Sl81iM0J/lPABj/akhRyUQ0Bb6NMuzvB6SqKp+9RjQc=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=MRNA0U4/MUFYoNQh/1FfyKqfdTvCKZBn+66ULd8wq11Blxi25lvGPAtJDBWuwEPjl tR1H8crl0m6VtMbijXGucsB/D6oEU6YQHadiiz3WwkgCz/kvLuScyEzEcrJJpGY5L8 Stag4eucxD6oH3cAHDitYgbsEUFxjUKeolU5KVp4= From: Tyler Hicks To: Mimi Zohar , Dmitry Kasatkin Cc: James Morris , "Serge E . Hallyn" , Lakshmi Ramasubramanian , Prakhar Srivastava , linux-kernel@vger.kernel.org, linux-integrity@vger.kernel.org, linux-security-module@vger.kernel.org Subject: [PATCH v3 06/12] ima: Fail rule parsing when the KEY_CHECK hook is combined with an invalid cond Date: Thu, 9 Jul 2020 01:19:05 -0500 Message-Id: <20200709061911.954326-7-tyhicks@linux.microsoft.com> X-Mailer: git-send-email 2.25.1 In-Reply-To: <20200709061911.954326-1-tyhicks@linux.microsoft.com> References: <20200709061911.954326-1-tyhicks@linux.microsoft.com> MIME-Version: 1.0 Sender: owner-linux-security-module@vger.kernel.org Precedence: bulk List-ID: The KEY_CHECK function only supports the uid, pcr, and keyrings conditionals. Make this clear at policy load so that IMA policy authors don't assume that other conditionals are supported. Fixes: 5808611cccb2 ("IMA: Add KEY_CHECK func to measure keys") Signed-off-by: Tyler Hicks Reviewed-by: Lakshmi Ramasubramanian --- * v3 - Added Lakshmi's Reviewed-by - Adjust for the indentation change introduced in patch #4 * v2 - No change security/integrity/ima/ima_policy.c | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c index 1c64bd6f1728..81da02071d41 100644 --- a/security/integrity/ima/ima_policy.c +++ b/security/integrity/ima/ima_policy.c @@ -1023,6 +1023,13 @@ static bool ima_validate_rule(struct ima_rule_entry *entry) if (entry->action & ~(MEASURE | DONT_MEASURE)) return false; + if (entry->flags & ~(IMA_FUNC | IMA_UID | IMA_PCR | + IMA_KEYRINGS)) + return false; + + if (ima_rule_contains_lsm_cond(entry)) + return false; + break; default: return false; From patchwork Thu Jul 9 06:19:06 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Tyler Hicks X-Patchwork-Id: 11653401 Return-Path: Received: from mail.kernel.org (pdx-korg-mail-1.web.codeaurora.org [172.30.200.123]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id 880FC618 for ; Thu, 9 Jul 2020 06:20:12 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 6D59120663 for ; Thu, 9 Jul 2020 06:20:12 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (1024-bit key) header.d=linux.microsoft.com header.i=@linux.microsoft.com header.b="rRADIsmh" Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1726196AbgGIGUL (ORCPT ); Thu, 9 Jul 2020 02:20:11 -0400 Received: from linux.microsoft.com ([13.77.154.182]:38084 "EHLO linux.microsoft.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726513AbgGIGUK (ORCPT ); Thu, 9 Jul 2020 02:20:10 -0400 Received: from sequoia.work.tihix.com (162-237-133-238.lightspeed.rcsntx.sbcglobal.net [162.237.133.238]) by linux.microsoft.com (Postfix) with ESMTPSA id 5EFC920B4908; Wed, 8 Jul 2020 23:20:09 -0700 (PDT) DKIM-Filter: OpenDKIM Filter v2.11.0 linux.microsoft.com 5EFC920B4908 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linux.microsoft.com; s=default; t=1594275610; bh=swamC2p17c6UKzr2WevLUTCfz2mHFf5f2mrMDFbXG8M=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=rRADIsmhSYlLAiAJQcVPCqnYL6wmzgpKYtO/Dz5lZF93n2jW9H5eYRZN5lngLAYUH VUKbzAf1KQ7TTYCPIFqUtM/dCJT/3wf1FN3/h4R8i0M2wcUTIwm0gCYjbmiupKmW+l JyRbR4xc18NzQUyh4ecdxDFUsXfJj1tG5xQotZHQ= From: Tyler Hicks To: Mimi Zohar , Dmitry Kasatkin Cc: James Morris , "Serge E . Hallyn" , Lakshmi Ramasubramanian , Prakhar Srivastava , linux-kernel@vger.kernel.org, linux-integrity@vger.kernel.org, linux-security-module@vger.kernel.org, Nayna Jain Subject: [PATCH v3 07/12] ima: Fail rule parsing when appraise_flag=blacklist is unsupportable Date: Thu, 9 Jul 2020 01:19:06 -0500 Message-Id: <20200709061911.954326-8-tyhicks@linux.microsoft.com> X-Mailer: git-send-email 2.25.1 In-Reply-To: <20200709061911.954326-1-tyhicks@linux.microsoft.com> References: <20200709061911.954326-1-tyhicks@linux.microsoft.com> MIME-Version: 1.0 Sender: owner-linux-security-module@vger.kernel.org Precedence: bulk List-ID: The "appraise_flag" option is only appropriate for appraise actions and its "blacklist" value is only appropriate when CONFIG_IMA_APPRAISE_MODSIG is enabled and "appraise_flag=blacklist" is only appropriate when "appraise_type=imasig|modsig" is also present. Make this clear at policy load so that IMA policy authors don't assume that other uses of "appraise_flag=blacklist" are supported. Fixes: 273df864cf74 ("ima: Check against blacklisted hashes for files with modsig") Signed-off-by: Tyler Hicks Cc: Nayna Jain Reviewed-by: Nayna Jain Tested-by: Nayna Jain --- * v3 - New patch security/integrity/ima/ima_policy.c | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c index 81da02071d41..9842e2e0bc6d 100644 --- a/security/integrity/ima/ima_policy.c +++ b/security/integrity/ima/ima_policy.c @@ -1035,6 +1035,11 @@ static bool ima_validate_rule(struct ima_rule_entry *entry) return false; } + /* Ensure that combinations of flags are compatible with each other */ + if (entry->flags & IMA_CHECK_BLACKLIST && + !(entry->flags & IMA_MODSIG_ALLOWED)) + return false; + return true; } @@ -1371,8 +1376,14 @@ static int ima_parse_rule(char *rule, struct ima_rule_entry *entry) result = -EINVAL; break; case Opt_appraise_flag: + if (entry->action != APPRAISE) { + result = -EINVAL; + break; + } + ima_log_string(ab, "appraise_flag", args[0].from); - if (strstr(args[0].from, "blacklist")) + if (IS_ENABLED(CONFIG_IMA_APPRAISE_MODSIG) && + strstr(args[0].from, "blacklist")) entry->flags |= IMA_CHECK_BLACKLIST; break; case Opt_permit_directio: From patchwork Thu Jul 9 06:19:07 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Tyler Hicks X-Patchwork-Id: 11653419 Return-Path: Received: from mail.kernel.org (pdx-korg-mail-1.web.codeaurora.org [172.30.200.123]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id 26DEA618 for ; Thu, 9 Jul 2020 06:20:37 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 03D0B20672 for ; Thu, 9 Jul 2020 06:20:37 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (1024-bit key) header.d=linux.microsoft.com header.i=@linux.microsoft.com header.b="g7f7y5H5" Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1726774AbgGIGUg (ORCPT ); Thu, 9 Jul 2020 02:20:36 -0400 Received: from linux.microsoft.com ([13.77.154.182]:38096 "EHLO linux.microsoft.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726581AbgGIGUM (ORCPT ); Thu, 9 Jul 2020 02:20:12 -0400 Received: from sequoia.work.tihix.com (162-237-133-238.lightspeed.rcsntx.sbcglobal.net [162.237.133.238]) by linux.microsoft.com (Postfix) with ESMTPSA id ED52220B4909; Wed, 8 Jul 2020 23:20:10 -0700 (PDT) DKIM-Filter: OpenDKIM Filter v2.11.0 linux.microsoft.com ED52220B4909 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linux.microsoft.com; s=default; t=1594275611; bh=kdGo+g0Ug/+ndalVCdzuX/XIssnW7sZa5n/LdXUX1OM=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=g7f7y5H5XpYSW/JqRJeaI3n/zw1XVmSYLKBZA3ZBwjh3RAvvCTvC6hDloRL5oNLGT 1ZTeusQjwRyMyX8hLF/RbnyRh9kx0ArCJoWQVWs8CB3V3xt6qevpWkxEMjlG2rTIiD MerMF1DJfyii9F7c6vAGHXSWuZtO3sf2EjsznJx8= From: Tyler Hicks To: Mimi Zohar , Dmitry Kasatkin Cc: James Morris , "Serge E . Hallyn" , Lakshmi Ramasubramanian , Prakhar Srivastava , linux-kernel@vger.kernel.org, linux-integrity@vger.kernel.org, linux-security-module@vger.kernel.org Subject: [PATCH v3 08/12] ima: Shallow copy the args_p member of ima_rule_entry.lsm elements Date: Thu, 9 Jul 2020 01:19:07 -0500 Message-Id: <20200709061911.954326-9-tyhicks@linux.microsoft.com> X-Mailer: git-send-email 2.25.1 In-Reply-To: <20200709061911.954326-1-tyhicks@linux.microsoft.com> References: <20200709061911.954326-1-tyhicks@linux.microsoft.com> MIME-Version: 1.0 Sender: owner-linux-security-module@vger.kernel.org Precedence: bulk List-ID: The args_p member is a simple string that is allocated by ima_rule_init(). Shallow copy it like other non-LSM references in ima_rule_entry structs. There are no longer any necessary error path cleanups to do in ima_lsm_copy_rule(). Signed-off-by: Tyler Hicks Reviewed-by: Konsta Karsisto --- * v3 - No change * v2 - Adjusted context to account for ima_lsm_copy_rule() directly calling ima_lsm_free_rule() and the lack of explicit reference ownership transfers - Added comment to ima_lsm_copy_rule() to document the args_p reference ownership transfer security/integrity/ima/ima_policy.c | 16 +++++++--------- 1 file changed, 7 insertions(+), 9 deletions(-) diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c index 9842e2e0bc6d..b02e1ffd10c9 100644 --- a/security/integrity/ima/ima_policy.c +++ b/security/integrity/ima/ima_policy.c @@ -300,10 +300,13 @@ static struct ima_rule_entry *ima_lsm_copy_rule(struct ima_rule_entry *entry) continue; nentry->lsm[i].type = entry->lsm[i].type; - nentry->lsm[i].args_p = kstrdup(entry->lsm[i].args_p, - GFP_KERNEL); - if (!nentry->lsm[i].args_p) - goto out_err; + nentry->lsm[i].args_p = entry->lsm[i].args_p; + /* + * Remove the reference from entry so that the associated + * memory will not be freed during a later call to + * ima_lsm_free_rule(entry). + */ + entry->lsm[i].args_p = NULL; security_filter_rule_init(nentry->lsm[i].type, Audit_equal, @@ -314,11 +317,6 @@ static struct ima_rule_entry *ima_lsm_copy_rule(struct ima_rule_entry *entry) (char *)entry->lsm[i].args_p); } return nentry; - -out_err: - ima_lsm_free_rule(nentry); - kfree(nentry); - return NULL; } static int ima_lsm_update_rule(struct ima_rule_entry *entry) From patchwork Thu Jul 9 06:19:08 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Tyler Hicks X-Patchwork-Id: 11653413 Return-Path: Received: from mail.kernel.org (pdx-korg-mail-1.web.codeaurora.org [172.30.200.123]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id 5E9F7618 for ; Thu, 9 Jul 2020 06:20:30 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 444932074B for ; Thu, 9 Jul 2020 06:20:30 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (1024-bit key) header.d=linux.microsoft.com header.i=@linux.microsoft.com header.b="KJKtKRLU" Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1726664AbgGIGUP (ORCPT ); Thu, 9 Jul 2020 02:20:15 -0400 Received: from linux.microsoft.com ([13.77.154.182]:38110 "EHLO linux.microsoft.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726624AbgGIGUN (ORCPT ); Thu, 9 Jul 2020 02:20:13 -0400 Received: from sequoia.work.tihix.com (162-237-133-238.lightspeed.rcsntx.sbcglobal.net [162.237.133.238]) by linux.microsoft.com (Postfix) with ESMTPSA id 6076D20B4908; Wed, 8 Jul 2020 23:20:12 -0700 (PDT) DKIM-Filter: OpenDKIM Filter v2.11.0 linux.microsoft.com 6076D20B4908 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linux.microsoft.com; s=default; t=1594275613; bh=b/gDO+rf9EfxqDQma9/U3xASpX0quKJnLQDBOzf+I4c=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=KJKtKRLUoa5UEYM0CWjDMPyGH0lii/OV71RzS5iuN2qnZhRPXaExoAD/Gfw/Yc4hb ScuFZ46zLV61LaA/TtWMBdCXHfKDDy9SB2BhQV9ody45FZbuFU+qJ1IQfrLPuZXwRT K90uNkcAvqV9QUJsDKTUpWOMkF9gq82xm9+LxGo8= From: Tyler Hicks To: Mimi Zohar , Dmitry Kasatkin Cc: James Morris , "Serge E . Hallyn" , Lakshmi Ramasubramanian , Prakhar Srivastava , linux-kernel@vger.kernel.org, linux-integrity@vger.kernel.org, linux-security-module@vger.kernel.org Subject: [PATCH v3 09/12] ima: Use correct type for the args_p member of ima_rule_entry.lsm elements Date: Thu, 9 Jul 2020 01:19:08 -0500 Message-Id: <20200709061911.954326-10-tyhicks@linux.microsoft.com> X-Mailer: git-send-email 2.25.1 In-Reply-To: <20200709061911.954326-1-tyhicks@linux.microsoft.com> References: <20200709061911.954326-1-tyhicks@linux.microsoft.com> MIME-Version: 1.0 Sender: owner-linux-security-module@vger.kernel.org Precedence: bulk List-ID: Make args_p be of the char pointer type rather than have it be a void pointer that gets casted to char pointer when it is used. It is a simple NUL-terminated string as returned by match_strdup(). Signed-off-by: Tyler Hicks --- * v3 - No change * v2 - No change security/integrity/ima/ima_policy.c | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c index b02e1ffd10c9..13a178c70b44 100644 --- a/security/integrity/ima/ima_policy.c +++ b/security/integrity/ima/ima_policy.c @@ -74,7 +74,7 @@ struct ima_rule_entry { int pcr; struct { void *rule; /* LSM file metadata specific */ - void *args_p; /* audit value */ + char *args_p; /* audit value */ int type; /* audit type */ } lsm[MAX_LSM_RULES]; char *fsname; @@ -314,7 +314,7 @@ static struct ima_rule_entry *ima_lsm_copy_rule(struct ima_rule_entry *entry) &nentry->lsm[i].rule); if (!nentry->lsm[i].rule) pr_warn("rule for LSM \'%s\' is undefined\n", - (char *)entry->lsm[i].args_p); + entry->lsm[i].args_p); } return nentry; } @@ -918,7 +918,7 @@ static int ima_lsm_rule_init(struct ima_rule_entry *entry, &entry->lsm[lsm_rule].rule); if (!entry->lsm[lsm_rule].rule) { pr_warn("rule for LSM \'%s\' is undefined\n", - (char *)entry->lsm[lsm_rule].args_p); + entry->lsm[lsm_rule].args_p); if (ima_rules == &ima_default_rules) { kfree(entry->lsm[lsm_rule].args_p); @@ -1682,27 +1682,27 @@ int ima_policy_show(struct seq_file *m, void *v) switch (i) { case LSM_OBJ_USER: seq_printf(m, pt(Opt_obj_user), - (char *)entry->lsm[i].args_p); + entry->lsm[i].args_p); break; case LSM_OBJ_ROLE: seq_printf(m, pt(Opt_obj_role), - (char *)entry->lsm[i].args_p); + entry->lsm[i].args_p); break; case LSM_OBJ_TYPE: seq_printf(m, pt(Opt_obj_type), - (char *)entry->lsm[i].args_p); + entry->lsm[i].args_p); break; case LSM_SUBJ_USER: seq_printf(m, pt(Opt_subj_user), - (char *)entry->lsm[i].args_p); + entry->lsm[i].args_p); break; case LSM_SUBJ_ROLE: seq_printf(m, pt(Opt_subj_role), - (char *)entry->lsm[i].args_p); + entry->lsm[i].args_p); break; case LSM_SUBJ_TYPE: seq_printf(m, pt(Opt_subj_type), - (char *)entry->lsm[i].args_p); + entry->lsm[i].args_p); break; } seq_puts(m, " "); From patchwork Thu Jul 9 06:19:09 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Tyler Hicks X-Patchwork-Id: 11653411 Return-Path: Received: from mail.kernel.org (pdx-korg-mail-1.web.codeaurora.org [172.30.200.123]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id 551AA14B7 for ; Thu, 9 Jul 2020 06:20:29 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 3ED72207C4 for ; Thu, 9 Jul 2020 06:20:29 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (1024-bit key) header.d=linux.microsoft.com header.i=@linux.microsoft.com header.b="njDV7IVW" Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1726693AbgGIGUS (ORCPT ); Thu, 9 Jul 2020 02:20:18 -0400 Received: from linux.microsoft.com ([13.77.154.182]:38134 "EHLO linux.microsoft.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726648AbgGIGUQ (ORCPT ); Thu, 9 Jul 2020 02:20:16 -0400 Received: from sequoia.work.tihix.com (162-237-133-238.lightspeed.rcsntx.sbcglobal.net [162.237.133.238]) by linux.microsoft.com (Postfix) with ESMTPSA id C792520B4909; Wed, 8 Jul 2020 23:20:13 -0700 (PDT) DKIM-Filter: OpenDKIM Filter v2.11.0 linux.microsoft.com C792520B4909 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linux.microsoft.com; s=default; t=1594275614; bh=rN9s22YZtVw6MnRwRHBu0f8F/NRhD2zbr74Gq7VM/zA=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=njDV7IVWMrem4WjDgsC7a5Bj4upgmla+ZcHWV3CDpcdAfFGgEU5IrXD2XUiFgNTOR W4zSXxKxWU3bjpL7xXslltRm6zVSEVT1pPOxscu22Bh/ymvO7DygcFhNmhvCw0usVQ S1JHWW150rgb2V5/sFbiJnMf/nV2/tXclfqWde3o= From: Tyler Hicks To: Mimi Zohar , Dmitry Kasatkin Cc: James Morris , "Serge E . Hallyn" , Lakshmi Ramasubramanian , Prakhar Srivastava , linux-kernel@vger.kernel.org, linux-integrity@vger.kernel.org, linux-security-module@vger.kernel.org Subject: [PATCH v3 10/12] ima: Move comprehensive rule validation checks out of the token parser Date: Thu, 9 Jul 2020 01:19:09 -0500 Message-Id: <20200709061911.954326-11-tyhicks@linux.microsoft.com> X-Mailer: git-send-email 2.25.1 In-Reply-To: <20200709061911.954326-1-tyhicks@linux.microsoft.com> References: <20200709061911.954326-1-tyhicks@linux.microsoft.com> MIME-Version: 1.0 Sender: owner-linux-security-module@vger.kernel.org Precedence: bulk List-ID: Use ima_validate_rule(), at the end of the token parsing stage, to verify combinations of actions, hooks, and flags. This is useful to increase readability by consolidating such checks into a single function and also because rule conditionals can be specified in arbitrary order making it difficult to do comprehensive rule validation until the entire rule has been parsed. This allows for the check that ties together the "keyrings" conditional with the KEY_CHECK function hook to be moved into the final rule validation. The modsig check no longer needs to compiled conditionally because the token parser will ensure that modsig support is enabled before accepting "imasig|modsig" appraise type values. The final rule validation will ensure that appraise_type and appraise_flag options are only present in appraise rules. Finally, this allows for the check that ties together the "pcr" conditional with the measure action to be moved into the final rule validation. Signed-off-by: Tyler Hicks --- * v3 - Significant broadening of the patch's scope along with renaming and re-describing the patch. ima_validate_rule() is now the consolidated location for checking combinations of actions/functions/conditionals and the existing checks are now removed from the token parsing code. + Ensure that the IMA_FUNC flag is only set when a function hook is specified, and vice versa, which allows us to use the NONE case in the switch statement to enforce that "keyrings=", "appraise_type=imasig|modsig", and "appraise_flag=blacklist" cannot be specified on a rule without an appropriate hook function - Adjust for the indentation change introduced in patch #4 - Adjust for new fixes introduced in patch #7 * v2 - Allowed IMA_DIGSIG_REQUIRED, IMA_PERMIT_DIRECTIO, IMA_MODSIG_ALLOWED, and IMA_CHECK_BLACKLIST conditionals to be present in the rule entry flags for non-buffer hook functions. security/integrity/ima/ima.h | 6 --- security/integrity/ima/ima_modsig.c | 20 ---------- security/integrity/ima/ima_policy.c | 57 +++++++++++++++++++---------- 3 files changed, 37 insertions(+), 46 deletions(-) diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h index 59ec28f5c117..ea7e77536f3c 100644 --- a/security/integrity/ima/ima.h +++ b/security/integrity/ima/ima.h @@ -372,7 +372,6 @@ static inline int ima_read_xattr(struct dentry *dentry, #endif /* CONFIG_IMA_APPRAISE */ #ifdef CONFIG_IMA_APPRAISE_MODSIG -bool ima_hook_supports_modsig(enum ima_hooks func); int ima_read_modsig(enum ima_hooks func, const void *buf, loff_t buf_len, struct modsig **modsig); void ima_collect_modsig(struct modsig *modsig, const void *buf, loff_t size); @@ -382,11 +381,6 @@ int ima_get_raw_modsig(const struct modsig *modsig, const void **data, u32 *data_len); void ima_free_modsig(struct modsig *modsig); #else -static inline bool ima_hook_supports_modsig(enum ima_hooks func) -{ - return false; -} - static inline int ima_read_modsig(enum ima_hooks func, const void *buf, loff_t buf_len, struct modsig **modsig) { diff --git a/security/integrity/ima/ima_modsig.c b/security/integrity/ima/ima_modsig.c index d106885cc495..fb25723c65bc 100644 --- a/security/integrity/ima/ima_modsig.c +++ b/security/integrity/ima/ima_modsig.c @@ -32,26 +32,6 @@ struct modsig { u8 raw_pkcs7[]; }; -/** - * ima_hook_supports_modsig - can the policy allow modsig for this hook? - * - * modsig is only supported by hooks using ima_post_read_file(), because only - * they preload the contents of the file in a buffer. FILE_CHECK does that in - * some cases, but not when reached from vfs_open(). POLICY_CHECK can support - * it, but it's not useful in practice because it's a text file so deny. - */ -bool ima_hook_supports_modsig(enum ima_hooks func) -{ - switch (func) { - case KEXEC_KERNEL_CHECK: - case KEXEC_INITRAMFS_CHECK: - case MODULE_CHECK: - return true; - default: - return false; - } -} - /* * ima_read_modsig - Read modsig from buf. * diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c index 13a178c70b44..c4d0a0c1f896 100644 --- a/security/integrity/ima/ima_policy.c +++ b/security/integrity/ima/ima_policy.c @@ -984,10 +984,27 @@ static void check_template_modsig(const struct ima_template_desc *template) static bool ima_validate_rule(struct ima_rule_entry *entry) { - /* Ensure that the action is set */ + /* Ensure that the action is set and is compatible with the flags */ if (entry->action == UNKNOWN) return false; + if (entry->action != MEASURE && entry->flags & IMA_PCR) + return false; + + if (entry->action != APPRAISE && + entry->flags & (IMA_DIGSIG_REQUIRED | IMA_MODSIG_ALLOWED | IMA_CHECK_BLACKLIST)) + return false; + + /* + * The IMA_FUNC bit must be set if and only if there's a valid hook + * function specified, and vice versa. Enforcing this property allows + * for the NONE case below to validate a rule without an explicit hook + * function. + */ + if (((entry->flags & IMA_FUNC) && entry->func == NONE) || + (!(entry->flags & IMA_FUNC) && entry->func != NONE)) + return false; + /* * Ensure that the hook function is compatible with the other * components of the rule @@ -999,12 +1016,27 @@ static bool ima_validate_rule(struct ima_rule_entry *entry) case BPRM_CHECK: case CREDS_CHECK: case POST_SETATTR: - case MODULE_CHECK: case FIRMWARE_CHECK: + case POLICY_CHECK: + if (entry->flags & ~(IMA_FUNC | IMA_MASK | IMA_FSMAGIC | + IMA_UID | IMA_FOWNER | IMA_FSUUID | + IMA_INMASK | IMA_EUID | IMA_PCR | + IMA_FSNAME | IMA_DIGSIG_REQUIRED | + IMA_PERMIT_DIRECTIO)) + return false; + + break; + case MODULE_CHECK: case KEXEC_KERNEL_CHECK: case KEXEC_INITRAMFS_CHECK: - case POLICY_CHECK: - /* Validation of these hook functions is in ima_parse_rule() */ + if (entry->flags & ~(IMA_FUNC | IMA_MASK | IMA_FSMAGIC | + IMA_UID | IMA_FOWNER | IMA_FSUUID | + IMA_INMASK | IMA_EUID | IMA_PCR | + IMA_FSNAME | IMA_DIGSIG_REQUIRED | + IMA_PERMIT_DIRECTIO | IMA_MODSIG_ALLOWED | + IMA_CHECK_BLACKLIST)) + return false; + break; case KEXEC_CMDLINE: if (entry->action & ~(MEASURE | DONT_MEASURE)) @@ -1218,7 +1250,6 @@ static int ima_parse_rule(char *rule, struct ima_rule_entry *entry) keyrings_len = strlen(args[0].from) + 1; if ((entry->keyrings) || - (entry->func != KEY_CHECK) || (keyrings_len < 2)) { result = -EINVAL; break; @@ -1358,15 +1389,10 @@ static int ima_parse_rule(char *rule, struct ima_rule_entry *entry) AUDIT_SUBJ_TYPE); break; case Opt_appraise_type: - if (entry->action != APPRAISE) { - result = -EINVAL; - break; - } - ima_log_string(ab, "appraise_type", args[0].from); if ((strcmp(args[0].from, "imasig")) == 0) entry->flags |= IMA_DIGSIG_REQUIRED; - else if (ima_hook_supports_modsig(entry->func) && + else if (IS_ENABLED(CONFIG_IMA_APPRAISE_MODSIG) && strcmp(args[0].from, "imasig|modsig") == 0) entry->flags |= IMA_DIGSIG_REQUIRED | IMA_MODSIG_ALLOWED; @@ -1374,11 +1400,6 @@ static int ima_parse_rule(char *rule, struct ima_rule_entry *entry) result = -EINVAL; break; case Opt_appraise_flag: - if (entry->action != APPRAISE) { - result = -EINVAL; - break; - } - ima_log_string(ab, "appraise_flag", args[0].from); if (IS_ENABLED(CONFIG_IMA_APPRAISE_MODSIG) && strstr(args[0].from, "blacklist")) @@ -1388,10 +1409,6 @@ static int ima_parse_rule(char *rule, struct ima_rule_entry *entry) entry->flags |= IMA_PERMIT_DIRECTIO; break; case Opt_pcr: - if (entry->action != MEASURE) { - result = -EINVAL; - break; - } ima_log_string(ab, "pcr", args[0].from); result = kstrtoint(args[0].from, 10, &entry->pcr); From patchwork Thu Jul 9 06:19:10 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Tyler Hicks X-Patchwork-Id: 11653415 Return-Path: Received: from mail.kernel.org (pdx-korg-mail-1.web.codeaurora.org [172.30.200.123]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id 34013618 for ; Thu, 9 Jul 2020 06:20:31 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 1275A20708 for ; Thu, 9 Jul 2020 06:20:31 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (1024-bit key) header.d=linux.microsoft.com header.i=@linux.microsoft.com header.b="W8eLV97P" Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1726765AbgGIGUa (ORCPT ); Thu, 9 Jul 2020 02:20:30 -0400 Received: from linux.microsoft.com ([13.77.154.182]:38148 "EHLO linux.microsoft.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726670AbgGIGUQ (ORCPT ); Thu, 9 Jul 2020 02:20:16 -0400 Received: from sequoia.work.tihix.com (162-237-133-238.lightspeed.rcsntx.sbcglobal.net [162.237.133.238]) by linux.microsoft.com (Postfix) with ESMTPSA id 3B7EE20B4908; Wed, 8 Jul 2020 23:20:15 -0700 (PDT) DKIM-Filter: OpenDKIM Filter v2.11.0 linux.microsoft.com 3B7EE20B4908 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linux.microsoft.com; s=default; t=1594275615; bh=BMSHcV0geP8nXHQf+FFpMe4jNDRwHDKhciy8Y9r1M+A=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=W8eLV97P7p0IM4hLJkbuHBItHUgBy+0JExAUlJYkp5ITVj9/c00A9U7e1CUhptlGe 66bplnYDOKmdhx6SXsZLTj8eE2CNEU8q5ucpVitY+vtHoaIQEJUGzK1pmrT2jCV8Pv 8FS1LSPf9KtgFE83S9DUi6KyViJdu4lgj2YT96cI= From: Tyler Hicks To: Mimi Zohar , Dmitry Kasatkin Cc: James Morris , "Serge E . Hallyn" , Lakshmi Ramasubramanian , Prakhar Srivastava , linux-kernel@vger.kernel.org, linux-integrity@vger.kernel.org, linux-security-module@vger.kernel.org Subject: [PATCH v3 11/12] ima: Use the common function to detect LSM conditionals in a rule Date: Thu, 9 Jul 2020 01:19:10 -0500 Message-Id: <20200709061911.954326-12-tyhicks@linux.microsoft.com> X-Mailer: git-send-email 2.25.1 In-Reply-To: <20200709061911.954326-1-tyhicks@linux.microsoft.com> References: <20200709061911.954326-1-tyhicks@linux.microsoft.com> MIME-Version: 1.0 Sender: owner-linux-security-module@vger.kernel.org Precedence: bulk List-ID: Make broader use of ima_rule_contains_lsm_cond() to check if a given rule contains an LSM conditional. This is a code cleanup and has no user-facing change. Signed-off-by: Tyler Hicks Reviewed-by: Mimi Zohar --- * v3 - No change * v2 - No change security/integrity/ima/ima_policy.c | 11 ++--------- 1 file changed, 2 insertions(+), 9 deletions(-) diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c index c4d0a0c1f896..81ee8fd1d83a 100644 --- a/security/integrity/ima/ima_policy.c +++ b/security/integrity/ima/ima_policy.c @@ -360,17 +360,10 @@ static bool ima_rule_contains_lsm_cond(struct ima_rule_entry *entry) static void ima_lsm_update_rules(void) { struct ima_rule_entry *entry, *e; - int i, result, needs_update; + int result; list_for_each_entry_safe(entry, e, &ima_policy_rules, list) { - needs_update = 0; - for (i = 0; i < MAX_LSM_RULES; i++) { - if (entry->lsm[i].args_p) { - needs_update = 1; - break; - } - } - if (!needs_update) + if (!ima_rule_contains_lsm_cond(entry)) continue; result = ima_lsm_update_rule(entry); From patchwork Thu Jul 9 06:19:11 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Tyler Hicks X-Patchwork-Id: 11653405 Return-Path: Received: from mail.kernel.org (pdx-korg-mail-1.web.codeaurora.org [172.30.200.123]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id 89A5013B1 for ; Thu, 9 Jul 2020 06:20:26 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 67EDE20708 for ; Thu, 9 Jul 2020 06:20:26 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (1024-bit key) header.d=linux.microsoft.com header.i=@linux.microsoft.com header.b="FdzmtY1H" Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1726648AbgGIGUW (ORCPT ); Thu, 9 Jul 2020 02:20:22 -0400 Received: from linux.microsoft.com ([13.77.154.182]:38194 "EHLO linux.microsoft.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726718AbgGIGUV (ORCPT ); Thu, 9 Jul 2020 02:20:21 -0400 Received: from sequoia.work.tihix.com (162-237-133-238.lightspeed.rcsntx.sbcglobal.net [162.237.133.238]) by linux.microsoft.com (Postfix) with ESMTPSA id 9F57F20B4908; Wed, 8 Jul 2020 23:20:18 -0700 (PDT) DKIM-Filter: OpenDKIM Filter v2.11.0 linux.microsoft.com 9F57F20B4908 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linux.microsoft.com; s=default; t=1594275619; bh=ZL2qZIw0Z4i7R3PK9I66uvfj7VMID6WB19pysrt0HSc=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=FdzmtY1HERJziztiET8Zy2zK/ScfDyv52tZkBlJJl0ZYnkiBYyMY1RybOxSR4axTk zBR5Rm4cXSnlqIIGFrJM2JNW2j6WSsTOhmU8tOrs4Go731qKekdMi7WuyPLY2Fuq+g 8e5LlOHxpe8J1NgcYHBg5TysmtJ6J9FNKXu8GKLo= From: Tyler Hicks To: Mimi Zohar , Dmitry Kasatkin Cc: James Morris , "Serge E . Hallyn" , Lakshmi Ramasubramanian , Prakhar Srivastava , linux-kernel@vger.kernel.org, linux-integrity@vger.kernel.org, linux-security-module@vger.kernel.org, Eric Biederman , kexec@lists.infradead.org Subject: [PATCH v3 12/12] ima: Support additional conditionals in the KEXEC_CMDLINE hook function Date: Thu, 9 Jul 2020 01:19:11 -0500 Message-Id: <20200709061911.954326-13-tyhicks@linux.microsoft.com> X-Mailer: git-send-email 2.25.1 In-Reply-To: <20200709061911.954326-1-tyhicks@linux.microsoft.com> References: <20200709061911.954326-1-tyhicks@linux.microsoft.com> MIME-Version: 1.0 Sender: owner-linux-security-module@vger.kernel.org Precedence: bulk List-ID: Take the properties of the kexec kernel's inode and the current task ownership into consideration when matching a KEXEC_CMDLINE operation to the rules in the IMA policy. This allows for some uniformity when writing IMA policy rules for KEXEC_KERNEL_CHECK, KEXEC_INITRAMFS_CHECK, and KEXEC_CMDLINE operations. Prior to this patch, it was not possible to write a set of rules like this: dont_measure func=KEXEC_KERNEL_CHECK obj_type=foo_t dont_measure func=KEXEC_INITRAMFS_CHECK obj_type=foo_t dont_measure func=KEXEC_CMDLINE obj_type=foo_t measure func=KEXEC_KERNEL_CHECK measure func=KEXEC_INITRAMFS_CHECK measure func=KEXEC_CMDLINE The inode information associated with the kernel being loaded by a kexec_kernel_load(2) syscall can now be included in the decision to measure or not Additonally, the uid, euid, and subj_* conditionals can also now be used in KEXEC_CMDLINE rules. There was no technical reason as to why those conditionals weren't being considered previously other than ima_match_rules() didn't have a valid inode to use so it immediately bailed out for KEXEC_CMDLINE operations rather than going through the full list of conditional comparisons. Signed-off-by: Tyler Hicks Cc: Eric Biederman Cc: kexec@lists.infradead.org Reviewed-by: Lakshmi Ramasubramanian --- * v3 - Added Lakshmi's Reviewed-by - Adjust for the indentation change introduced in patch #4 * v2 - Moved the inode parameter of process_buffer_measurement() to be the first parameter so that it more closely matches process_masurement() include/linux/ima.h | 4 ++-- kernel/kexec_file.c | 2 +- security/integrity/ima/ima.h | 2 +- security/integrity/ima/ima_api.c | 2 +- security/integrity/ima/ima_appraise.c | 2 +- security/integrity/ima/ima_asymmetric_keys.c | 2 +- security/integrity/ima/ima_main.c | 23 +++++++++++++++----- security/integrity/ima/ima_policy.c | 17 +++++---------- security/integrity/ima/ima_queue_keys.c | 2 +- 9 files changed, 31 insertions(+), 25 deletions(-) diff --git a/include/linux/ima.h b/include/linux/ima.h index 9164e1534ec9..d15100de6cdd 100644 --- a/include/linux/ima.h +++ b/include/linux/ima.h @@ -25,7 +25,7 @@ extern int ima_post_read_file(struct file *file, void *buf, loff_t size, enum kernel_read_file_id id); extern void ima_post_path_mknod(struct dentry *dentry); extern int ima_file_hash(struct file *file, char *buf, size_t buf_size); -extern void ima_kexec_cmdline(const void *buf, int size); +extern void ima_kexec_cmdline(int kernel_fd, const void *buf, int size); #ifdef CONFIG_IMA_KEXEC extern void ima_add_kexec_buffer(struct kimage *image); @@ -103,7 +103,7 @@ static inline int ima_file_hash(struct file *file, char *buf, size_t buf_size) return -EOPNOTSUPP; } -static inline void ima_kexec_cmdline(const void *buf, int size) {} +static inline void ima_kexec_cmdline(int kernel_fd, const void *buf, int size) {} #endif /* CONFIG_IMA */ #ifndef CONFIG_IMA_KEXEC diff --git a/kernel/kexec_file.c b/kernel/kexec_file.c index bb05fd52de85..07df431c1f21 100644 --- a/kernel/kexec_file.c +++ b/kernel/kexec_file.c @@ -287,7 +287,7 @@ kimage_file_prepare_segments(struct kimage *image, int kernel_fd, int initrd_fd, goto out; } - ima_kexec_cmdline(image->cmdline_buf, + ima_kexec_cmdline(kernel_fd, image->cmdline_buf, image->cmdline_buf_len - 1); } diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h index ea7e77536f3c..576ae2c6d418 100644 --- a/security/integrity/ima/ima.h +++ b/security/integrity/ima/ima.h @@ -265,7 +265,7 @@ void ima_store_measurement(struct integrity_iint_cache *iint, struct file *file, struct evm_ima_xattr_data *xattr_value, int xattr_len, const struct modsig *modsig, int pcr, struct ima_template_desc *template_desc); -void process_buffer_measurement(const void *buf, int size, +void process_buffer_measurement(struct inode *inode, const void *buf, int size, const char *eventname, enum ima_hooks func, int pcr, const char *keyring); void ima_audit_measurement(struct integrity_iint_cache *iint, diff --git a/security/integrity/ima/ima_api.c b/security/integrity/ima/ima_api.c index bf22de8b7ce0..4f39fb93f278 100644 --- a/security/integrity/ima/ima_api.c +++ b/security/integrity/ima/ima_api.c @@ -162,7 +162,7 @@ void ima_add_violation(struct file *file, const unsigned char *filename, /** * ima_get_action - appraise & measure decision based on policy. - * @inode: pointer to inode to measure + * @inode: pointer to the inode associated with the object being validated * @cred: pointer to credentials structure to validate * @secid: secid of the task being validated * @mask: contains the permission mask (MAY_READ, MAY_WRITE, MAY_EXEC, diff --git a/security/integrity/ima/ima_appraise.c b/security/integrity/ima/ima_appraise.c index a9649b04b9f1..6c52bf7ea7f0 100644 --- a/security/integrity/ima/ima_appraise.c +++ b/security/integrity/ima/ima_appraise.c @@ -328,7 +328,7 @@ int ima_check_blacklist(struct integrity_iint_cache *iint, rc = is_binary_blacklisted(digest, digestsize); if ((rc == -EPERM) && (iint->flags & IMA_MEASURE)) - process_buffer_measurement(digest, digestsize, + process_buffer_measurement(NULL, digest, digestsize, "blacklisted-hash", NONE, pcr, NULL); } diff --git a/security/integrity/ima/ima_asymmetric_keys.c b/security/integrity/ima/ima_asymmetric_keys.c index aaae80c4e376..1c68c500c26f 100644 --- a/security/integrity/ima/ima_asymmetric_keys.c +++ b/security/integrity/ima/ima_asymmetric_keys.c @@ -58,7 +58,7 @@ void ima_post_key_create_or_update(struct key *keyring, struct key *key, * if the IMA policy is configured to measure a key linked * to the given keyring. */ - process_buffer_measurement(payload, payload_len, + process_buffer_measurement(NULL, payload, payload_len, keyring->description, KEY_CHECK, 0, keyring->description); } diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c index 8351b2fd48e0..8a91711ca79b 100644 --- a/security/integrity/ima/ima_main.c +++ b/security/integrity/ima/ima_main.c @@ -726,6 +726,7 @@ int ima_load_data(enum kernel_load_data_id id) /* * process_buffer_measurement - Measure the buffer to ima log. + * @inode: inode associated with the object being measured (NULL for KEY_CHECK) * @buf: pointer to the buffer that needs to be added to the log. * @size: size of buffer(in bytes). * @eventname: event name to be used for the buffer entry. @@ -735,7 +736,7 @@ int ima_load_data(enum kernel_load_data_id id) * * Based on policy, the buffer is measured into the ima log. */ -void process_buffer_measurement(const void *buf, int size, +void process_buffer_measurement(struct inode *inode, const void *buf, int size, const char *eventname, enum ima_hooks func, int pcr, const char *keyring) { @@ -768,7 +769,7 @@ void process_buffer_measurement(const void *buf, int size, */ if (func) { security_task_getsecid(current, &secid); - action = ima_get_action(NULL, current_cred(), secid, 0, func, + action = ima_get_action(inode, current_cred(), secid, 0, func, &pcr, &template, keyring); if (!(action & IMA_MEASURE)) return; @@ -823,16 +824,26 @@ void process_buffer_measurement(const void *buf, int size, /** * ima_kexec_cmdline - measure kexec cmdline boot args + * @kernel_fd: file descriptor of the kexec kernel being loaded * @buf: pointer to buffer * @size: size of buffer * * Buffers can only be measured, not appraised. */ -void ima_kexec_cmdline(const void *buf, int size) +void ima_kexec_cmdline(int kernel_fd, const void *buf, int size) { - if (buf && size != 0) - process_buffer_measurement(buf, size, "kexec-cmdline", - KEXEC_CMDLINE, 0, NULL); + struct fd f; + + if (!buf || !size) + return; + + f = fdget(kernel_fd); + if (!f.file) + return; + + process_buffer_measurement(file_inode(f.file), buf, size, + "kexec-cmdline", KEXEC_CMDLINE, 0, NULL); + fdput(f); } static int __init init_ima(void) diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c index 81ee8fd1d83a..2e87154c9296 100644 --- a/security/integrity/ima/ima_policy.c +++ b/security/integrity/ima/ima_policy.c @@ -443,13 +443,9 @@ static bool ima_match_rules(struct ima_rule_entry *rule, struct inode *inode, { int i; - if ((func == KEXEC_CMDLINE) || (func == KEY_CHECK)) { - if ((rule->flags & IMA_FUNC) && (rule->func == func)) { - if (func == KEY_CHECK) - return ima_match_keyring(rule, keyring, cred); - return true; - } - return false; + if (func == KEY_CHECK) { + return (rule->flags & IMA_FUNC) && (rule->func == func) && + ima_match_keyring(rule, keyring, cred); } if ((rule->flags & IMA_FUNC) && (rule->func != func && func != POST_SETATTR)) @@ -1035,10 +1031,9 @@ static bool ima_validate_rule(struct ima_rule_entry *entry) if (entry->action & ~(MEASURE | DONT_MEASURE)) return false; - if (entry->flags & ~(IMA_FUNC | IMA_PCR)) - return false; - - if (ima_rule_contains_lsm_cond(entry)) + if (entry->flags & ~(IMA_FUNC | IMA_FSMAGIC | IMA_UID | + IMA_FOWNER | IMA_FSUUID | IMA_EUID | + IMA_PCR | IMA_FSNAME)) return false; break; diff --git a/security/integrity/ima/ima_queue_keys.c b/security/integrity/ima/ima_queue_keys.c index 56ce24a18b66..69a8626a35c0 100644 --- a/security/integrity/ima/ima_queue_keys.c +++ b/security/integrity/ima/ima_queue_keys.c @@ -158,7 +158,7 @@ void ima_process_queued_keys(void) list_for_each_entry_safe(entry, tmp, &ima_keys, list) { if (!timer_expired) - process_buffer_measurement(entry->payload, + process_buffer_measurement(NULL, entry->payload, entry->payload_len, entry->keyring_name, KEY_CHECK, 0,