From patchwork Fri Jun 26 22:38:50 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Tyler Hicks X-Patchwork-Id: 11629043 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 0B5BD618 for ; Fri, 26 Jun 2020 22:39:37 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id E75FC20C09 for ; Fri, 26 Jun 2020 22:39:36 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (1024-bit key) header.d=linux.microsoft.com header.i=@linux.microsoft.com header.b="SXnwtulz" Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1726112AbgFZWjd (ORCPT ); Fri, 26 Jun 2020 18:39:33 -0400 Received: from linux.microsoft.com ([13.77.154.182]:37788 "EHLO linux.microsoft.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1725909AbgFZWjc (ORCPT ); Fri, 26 Jun 2020 18:39:32 -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 1F04020B4904; Fri, 26 Jun 2020 15:39:31 -0700 (PDT) DKIM-Filter: OpenDKIM Filter v2.11.0 linux.microsoft.com 1F04020B4904 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linux.microsoft.com; s=default; t=1593211171; bh=/fXCtimfsO5yJFCIUd284uYnqZ76Dz3IDTmbuYiAJNk=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=SXnwtulzDNqU/pVA+HTNv/CBFpfczEL+jCCVzeDihk/mCb/xqi2QWyx8wba+1j/DK UQkxvlHJYH+NAu9AUH+X+62ZwedXDH+9KDuyzI0dD6Ia1KQ8eUomGRiyNofX/cdcYV tDC++6weEFbFLtjFzFSBgMIxjAdcnZjlNMIk8E3A= 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 v2 01/11] ima: Have the LSM free its audit rule Date: Fri, 26 Jun 2020 17:38:50 -0500 Message-Id: <20200626223900.253615-2-tyhicks@linux.microsoft.com> X-Mailer: git-send-email 2.25.1 In-Reply-To: <20200626223900.253615-1-tyhicks@linux.microsoft.com> References: <20200626223900.253615-1-tyhicks@linux.microsoft.com> MIME-Version: 1.0 Sender: linux-integrity-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-integrity@vger.kernel.org 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 --- * 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 Fri Jun 26 22:38:51 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Tyler Hicks X-Patchwork-Id: 11629047 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 2E580618 for ; Fri, 26 Jun 2020 22:39:38 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 1593220B1F for ; Fri, 26 Jun 2020 22:39:38 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (1024-bit key) header.d=linux.microsoft.com header.i=@linux.microsoft.com header.b="lSxdc4Ro" Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1726388AbgFZWjh (ORCPT ); Fri, 26 Jun 2020 18:39:37 -0400 Received: from linux.microsoft.com ([13.77.154.182]:37808 "EHLO linux.microsoft.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1725909AbgFZWje (ORCPT ); Fri, 26 Jun 2020 18:39:34 -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 C0F4A20B4905; Fri, 26 Jun 2020 15:39:32 -0700 (PDT) DKIM-Filter: OpenDKIM Filter v2.11.0 linux.microsoft.com C0F4A20B4905 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linux.microsoft.com; s=default; t=1593211173; bh=MNYV62VxHGPBvdC3jd7a3aa0rJsIPisLZWq2XIgStSk=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=lSxdc4Ro05Xoqs25RSxd+xCJxSmaWQdWNTZ7r9KgBg9Hc886B4bZarIJq7tmBu9eU Lz1NcXBHNX+lWHWbvP8xG/wB8cprMk7RHrjCFkt+soUies0J86wxNaeLZGnjutn6/n xQKdJb/N+IW+shxO+A5fPFm+9D3Z+g67hQPyNDwQ= 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 v2 02/11] ima: Free the entire rule when deleting a list of rules Date: Fri, 26 Jun 2020 17:38:51 -0500 Message-Id: <20200626223900.253615-3-tyhicks@linux.microsoft.com> X-Mailer: git-send-email 2.25.1 In-Reply-To: <20200626223900.253615-1-tyhicks@linux.microsoft.com> References: <20200626223900.253615-1-tyhicks@linux.microsoft.com> MIME-Version: 1.0 Sender: linux-integrity-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-integrity@vger.kernel.org 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 --- * 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 Fri Jun 26 22:38:52 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Tyler Hicks X-Patchwork-Id: 11629051 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 8E8441731 for ; Fri, 26 Jun 2020 22:39:38 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 76B0F20C09 for ; Fri, 26 Jun 2020 22:39:38 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (1024-bit key) header.d=linux.microsoft.com header.i=@linux.microsoft.com header.b="UIiQu+gs" Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1726415AbgFZWjh (ORCPT ); Fri, 26 Jun 2020 18:39:37 -0400 Received: from linux.microsoft.com ([13.77.154.182]:37820 "EHLO linux.microsoft.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726381AbgFZWjf (ORCPT ); Fri, 26 Jun 2020 18:39:35 -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 8A51820B4901; Fri, 26 Jun 2020 15:39:34 -0700 (PDT) DKIM-Filter: OpenDKIM Filter v2.11.0 linux.microsoft.com 8A51820B4901 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linux.microsoft.com; s=default; t=1593211175; bh=wb2RijhIAB6vGdBMZgiQYqvgUSu/Ak6GRz9I3YnxR8I=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=UIiQu+gsLcJEteLDZJ/8tuACF4ht6M7nWC9s484gDmnJvgaGL0wOxRisJPrZEuJcQ jQN5MJxnVvIbazBzwF1dHG/uvK9RLFApoB2G886oB88hbRg0r+t1pxANkPZD86KmQK lVaoJMyPTcqDLeXB7HDW6I8XzUwGR9GuHhJAEwKk= 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 v2 03/11] ima: Free the entire rule if it fails to parse Date: Fri, 26 Jun 2020 17:38:52 -0500 Message-Id: <20200626223900.253615-4-tyhicks@linux.microsoft.com> X-Mailer: git-send-email 2.25.1 In-Reply-To: <20200626223900.253615-1-tyhicks@linux.microsoft.com> References: <20200626223900.253615-1-tyhicks@linux.microsoft.com> MIME-Version: 1.0 Sender: linux-integrity-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-integrity@vger.kernel.org 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 --- * 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 Fri Jun 26 22:38:53 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Tyler Hicks X-Patchwork-Id: 11629053 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 D0ADD618 for ; Fri, 26 Jun 2020 22:39:39 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id B5CA120B1F for ; Fri, 26 Jun 2020 22:39:39 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (1024-bit key) header.d=linux.microsoft.com header.i=@linux.microsoft.com header.b="ngVDXR1W" Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1726506AbgFZWji (ORCPT ); Fri, 26 Jun 2020 18:39:38 -0400 Received: from linux.microsoft.com ([13.77.154.182]:37846 "EHLO linux.microsoft.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1725970AbgFZWjh (ORCPT ); Fri, 26 Jun 2020 18:39:37 -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 1D50520B4904; Fri, 26 Jun 2020 15:39:36 -0700 (PDT) DKIM-Filter: OpenDKIM Filter v2.11.0 linux.microsoft.com 1D50520B4904 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linux.microsoft.com; s=default; t=1593211176; bh=ApKomaGhnLAJ+OBGkdDeyGNla8lfgVB55xcAmYbvTbU=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=ngVDXR1WRd7wt1Lg2eFBJfzMAmeh+Czo60DKde1x8U37UyGZpI8JyNPviRH7Bfdox zZE9qDEoY46XPowwOpeZ6J0Liy5pHVGihCde8SKL8MHE9dWObvau3DfXiQ8+1Fpx5c her5dFTtckEb6ivuBT58W2pFKq7QzHM8bQWdxV7Y= 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 v2 04/11] ima: Fail rule parsing when buffer hook functions have an invalid action Date: Fri, 26 Jun 2020 17:38:53 -0500 Message-Id: <20200626223900.253615-5-tyhicks@linux.microsoft.com> X-Mailer: git-send-email 2.25.1 In-Reply-To: <20200626223900.253615-1-tyhicks@linux.microsoft.com> References: <20200626223900.253615-1-tyhicks@linux.microsoft.com> MIME-Version: 1.0 Sender: linux-integrity-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-integrity@vger.kernel.org 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 --- * v2 - No change security/integrity/ima/ima_policy.c | 36 +++++++++++++++++++++++++++-- 1 file changed, 34 insertions(+), 2 deletions(-) diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c index e458cd47c099..166124d67774 100644 --- a/security/integrity/ima/ima_policy.c +++ b/security/integrity/ima/ima_policy.c @@ -973,6 +973,39 @@ static void check_template_modsig(const struct ima_template_desc *template) #undef MSG } +static bool ima_validate_rule(struct ima_rule_entry *entry) +{ + if (entry->action == UNKNOWN) + return false; + + if (entry->flags & IMA_FUNC) { + 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: + 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 +1183,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 +1388,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 Fri Jun 26 22:38:54 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Tyler Hicks X-Patchwork-Id: 11629081 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 0E018161F for ; Fri, 26 Jun 2020 22:40:19 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id EA65220B1F for ; Fri, 26 Jun 2020 22:40:18 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (1024-bit key) header.d=linux.microsoft.com header.i=@linux.microsoft.com header.b="SYNtdukF" Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1726112AbgFZWkR (ORCPT ); Fri, 26 Jun 2020 18:40:17 -0400 Received: from linux.microsoft.com ([13.77.154.182]:37860 "EHLO linux.microsoft.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726438AbgFZWji (ORCPT ); Fri, 26 Jun 2020 18:39:38 -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 4C9B220B4901; Fri, 26 Jun 2020 15:39:37 -0700 (PDT) DKIM-Filter: OpenDKIM Filter v2.11.0 linux.microsoft.com 4C9B220B4901 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linux.microsoft.com; s=default; t=1593211178; bh=D75oD57lun0GUBDulxMpvcNQCA9bpPWPblxeS6BsHiQ=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=SYNtdukF8WEuz0MozMQkGjt/TuIV1td+43ZBbQaQYfXdSMHB7c7JnasCv+ABi4s58 Yrs4vxRExlI6ToqPUWxi7kFrS73ra+bGLwRG4feO1XSnsul1fTAWcFptNfUuRJ6Mse h6TVINC7vEgxKpaYPE5Xe0zsQta2Rwrv8rax/mYY= 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 v2 05/11] ima: Fail rule parsing when the KEXEC_CMDLINE hook is combined with an invalid cond Date: Fri, 26 Jun 2020 17:38:54 -0500 Message-Id: <20200626223900.253615-6-tyhicks@linux.microsoft.com> X-Mailer: git-send-email 2.25.1 In-Reply-To: <20200626223900.253615-1-tyhicks@linux.microsoft.com> References: <20200626223900.253615-1-tyhicks@linux.microsoft.com> MIME-Version: 1.0 Sender: linux-integrity-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-integrity@vger.kernel.org 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 --- * 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 166124d67774..676d5557af1a 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 @@ -993,6 +1004,16 @@ static bool ima_validate_rule(struct ima_rule_entry *entry) case POLICY_CHECK: 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 Fri Jun 26 22:38:55 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Tyler Hicks X-Patchwork-Id: 11629077 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 05FED161F for ; Fri, 26 Jun 2020 22:40:13 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id DFC292100A for ; Fri, 26 Jun 2020 22:40: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="bsImo1ZE" Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1726096AbgFZWkK (ORCPT ); Fri, 26 Jun 2020 18:40:10 -0400 Received: from linux.microsoft.com ([13.77.154.182]:37888 "EHLO linux.microsoft.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726509AbgFZWjj (ORCPT ); Fri, 26 Jun 2020 18:39:39 -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 7AC5520B4904; Fri, 26 Jun 2020 15:39:38 -0700 (PDT) DKIM-Filter: OpenDKIM Filter v2.11.0 linux.microsoft.com 7AC5520B4904 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linux.microsoft.com; s=default; t=1593211179; bh=Wp7vF9KAeROGzIEtToLc6tm4PKDm4WsK82PPSN81ys4=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=bsImo1ZEQWJt4PwXGw15yqesVY/C1JgDZ/bAk1AjD0ZO3JMKWVYAFQKfKx5ouM/wu wfLGcadifHWBEGDQnIuEKFnN1fhKVmA7amKhLvxBYSoiEOeAzUUPP56ZU/0+3f281f HzbNwRQJbjbKlwX9oDkufoOAFqWDcnFHcqFqklzA= 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 v2 06/11] ima: Fail rule parsing when the KEY_CHECK hook is combined with an invalid cond Date: Fri, 26 Jun 2020 17:38:55 -0500 Message-Id: <20200626223900.253615-7-tyhicks@linux.microsoft.com> X-Mailer: git-send-email 2.25.1 In-Reply-To: <20200626223900.253615-1-tyhicks@linux.microsoft.com> References: <20200626223900.253615-1-tyhicks@linux.microsoft.com> MIME-Version: 1.0 Sender: linux-integrity-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-integrity@vger.kernel.org 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 --- * 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 676d5557af1a..f9b1bdb897da 100644 --- a/security/integrity/ima/ima_policy.c +++ b/security/integrity/ima/ima_policy.c @@ -1018,6 +1018,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 Fri Jun 26 22:38:56 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Tyler Hicks X-Patchwork-Id: 11629061 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 7DDC3618 for ; Fri, 26 Jun 2020 22:40:08 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 65B2421527 for ; Fri, 26 Jun 2020 22:40:08 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (1024-bit key) header.d=linux.microsoft.com header.i=@linux.microsoft.com header.b="TMlmjXk0" Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1726554AbgFZWjt (ORCPT ); Fri, 26 Jun 2020 18:39:49 -0400 Received: from linux.microsoft.com ([13.77.154.182]:37900 "EHLO linux.microsoft.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726518AbgFZWjk (ORCPT ); Fri, 26 Jun 2020 18:39:40 -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 B322D20B4905; Fri, 26 Jun 2020 15:39:39 -0700 (PDT) DKIM-Filter: OpenDKIM Filter v2.11.0 linux.microsoft.com B322D20B4905 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linux.microsoft.com; s=default; t=1593211180; bh=NgaFLRWS7qo3ujL0AXOQBEum655blqojaYopmjuWreY=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=TMlmjXk0FSfUS3DRx2rbvTApQ97nymuEYd2w1SK7TgThP+V41RqmU4Z20wPIc8pkr P/33ku1ExGN/cYx9Kc+qHlM4i5Z5o51b52HkECqtWNR1AL4dhZLJcAygDONh9dzScC RcmcidHdO+Wn0rMw2xLyQSp8mno3Zgb1qBauGVyk= 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 v2 07/11] ima: Shallow copy the args_p member of ima_rule_entry.lsm elements Date: Fri, 26 Jun 2020 17:38:56 -0500 Message-Id: <20200626223900.253615-8-tyhicks@linux.microsoft.com> X-Mailer: git-send-email 2.25.1 In-Reply-To: <20200626223900.253615-1-tyhicks@linux.microsoft.com> References: <20200626223900.253615-1-tyhicks@linux.microsoft.com> MIME-Version: 1.0 Sender: linux-integrity-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-integrity@vger.kernel.org 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 --- * 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 f9b1bdb897da..ef69c54266c6 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 Fri Jun 26 22:38:57 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Tyler Hicks X-Patchwork-Id: 11629059 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 6F516618 for ; Fri, 26 Jun 2020 22:39:51 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 5592C2100A for ; Fri, 26 Jun 2020 22:39:51 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (1024-bit key) header.d=linux.microsoft.com header.i=@linux.microsoft.com header.b="f7hcNvHD" Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1726555AbgFZWjt (ORCPT ); Fri, 26 Jun 2020 18:39:49 -0400 Received: from linux.microsoft.com ([13.77.154.182]:37918 "EHLO linux.microsoft.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726525AbgFZWjm (ORCPT ); Fri, 26 Jun 2020 18:39:42 -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 D458C20B4907; Fri, 26 Jun 2020 15:39:40 -0700 (PDT) DKIM-Filter: OpenDKIM Filter v2.11.0 linux.microsoft.com D458C20B4907 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linux.microsoft.com; s=default; t=1593211181; bh=pFH0+7shWtBIUXAmbHtVKgktKeESMvnpMqcT4wbOKOc=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=f7hcNvHDSHVz89ZYcTH8jm/9afQX8d2xQHOymnOCi8MW2q3iziXSWxRIqVU82cCgx k00c3DtUTLZGu8CBpzcsJsPyUV3oAbOyUe1Irxj3eWlEvraFkb5pbK3V1U/BOtklnw GJI5ljgnWFCuty17kaOGHNS0pdOxiYdYsQkyQuqo= 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 v2 08/11] ima: Use correct type for the args_p member of ima_rule_entry.lsm elements Date: Fri, 26 Jun 2020 17:38:57 -0500 Message-Id: <20200626223900.253615-9-tyhicks@linux.microsoft.com> X-Mailer: git-send-email 2.25.1 In-Reply-To: <20200626223900.253615-1-tyhicks@linux.microsoft.com> References: <20200626223900.253615-1-tyhicks@linux.microsoft.com> MIME-Version: 1.0 Sender: linux-integrity-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-integrity@vger.kernel.org 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 --- * 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 ef69c54266c6..8cdca2399d59 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); @@ -1667,27 +1667,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 Fri Jun 26 22:38:58 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Tyler Hicks X-Patchwork-Id: 11629063 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 A723F161F for ; Fri, 26 Jun 2020 22:40:08 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 85CF220B1F for ; Fri, 26 Jun 2020 22:40:08 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (1024-bit key) header.d=linux.microsoft.com header.i=@linux.microsoft.com header.b="WekBJFwM" Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1726548AbgFZWjt (ORCPT ); Fri, 26 Jun 2020 18:39:49 -0400 Received: from linux.microsoft.com ([13.77.154.182]:37936 "EHLO linux.microsoft.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726528AbgFZWjn (ORCPT ); Fri, 26 Jun 2020 18:39:43 -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 EE15120B4908; Fri, 26 Jun 2020 15:39:41 -0700 (PDT) DKIM-Filter: OpenDKIM Filter v2.11.0 linux.microsoft.com EE15120B4908 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linux.microsoft.com; s=default; t=1593211182; bh=lDUDSl/J2tMl17zuExKPD6HFUxjBs8GdZNpGG6MWTno=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=WekBJFwMheMIKbdKrLA71n4R1zeKG7X4smVnVlKG7hugGkAPMsp8x8Ew7R1+YEfac 63BP0tUtg5iBBMqkjWqBBBtXu5RJjVQJLNHrPtlTTdNmJC3eC2k7CtWCYn3369FAWd SWaRS1p+xRshAazJYS0IAn2o+W1bWmHQSeJT6eM0= 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 v2 09/11] ima: Move validation of the keyrings conditional into ima_validate_rule() Date: Fri, 26 Jun 2020 17:38:58 -0500 Message-Id: <20200626223900.253615-10-tyhicks@linux.microsoft.com> X-Mailer: git-send-email 2.25.1 In-Reply-To: <20200626223900.253615-1-tyhicks@linux.microsoft.com> References: <20200626223900.253615-1-tyhicks@linux.microsoft.com> MIME-Version: 1.0 Sender: linux-integrity-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-integrity@vger.kernel.org Use ima_validate_rule() to ensure that the combination of a hook function and the keyrings conditional is valid and that the keyrings conditional is not specified without an explicit KEY_CHECK func conditional. This is a code cleanup and has no user-facing change. Signed-off-by: Tyler Hicks --- * 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_policy.c | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c index 8cdca2399d59..43d49ad958fb 100644 --- a/security/integrity/ima/ima_policy.c +++ b/security/integrity/ima/ima_policy.c @@ -1000,6 +1000,15 @@ static bool ima_validate_rule(struct ima_rule_entry *entry) case KEXEC_KERNEL_CHECK: case KEXEC_INITRAMFS_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 | + IMA_MODSIG_ALLOWED | + IMA_CHECK_BLACKLIST)) + return false; + break; case KEXEC_CMDLINE: if (entry->action & ~(MEASURE | DONT_MEASURE)) @@ -1027,7 +1036,8 @@ static bool ima_validate_rule(struct ima_rule_entry *entry) default: return false; } - } + } else if (entry->flags & IMA_KEYRINGS) + return false; return true; } @@ -1209,7 +1219,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; From patchwork Fri Jun 26 22:38:59 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Tyler Hicks X-Patchwork-Id: 11629069 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 D57BE618 for ; Fri, 26 Jun 2020 22:40:09 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id BD51E2100A for ; Fri, 26 Jun 2020 22:40:09 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (1024-bit key) header.d=linux.microsoft.com header.i=@linux.microsoft.com header.b="E0vgWKVe" Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1726543AbgFZWjs (ORCPT ); Fri, 26 Jun 2020 18:39:48 -0400 Received: from linux.microsoft.com ([13.77.154.182]:37954 "EHLO linux.microsoft.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726531AbgFZWjo (ORCPT ); Fri, 26 Jun 2020 18:39:44 -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 17B9E20B4909; Fri, 26 Jun 2020 15:39:43 -0700 (PDT) DKIM-Filter: OpenDKIM Filter v2.11.0 linux.microsoft.com 17B9E20B4909 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linux.microsoft.com; s=default; t=1593211183; bh=OnoF3Cvq4ZBg8ldmzl4iVW8t9aRV7I4Rqnsmn7Rh0uU=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=E0vgWKVe7YVIbQjPwJuJUJttUHeG1McRiGzMYia4p4ev5d7vMEfMBJo3ZgCwKJnKe QKc7QOfSm8NoHnUuSqraBwhl/V30HGk6HAnJHB2HC6Tp72oHERJrrLL14fwaL5o/QO zUrHZthudTlom/GjtMo7CewxTtGXuOBHwOYnHIO0= 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 v2 10/11] ima: Use the common function to detect LSM conditionals in a rule Date: Fri, 26 Jun 2020 17:38:59 -0500 Message-Id: <20200626223900.253615-11-tyhicks@linux.microsoft.com> X-Mailer: git-send-email 2.25.1 In-Reply-To: <20200626223900.253615-1-tyhicks@linux.microsoft.com> References: <20200626223900.253615-1-tyhicks@linux.microsoft.com> MIME-Version: 1.0 Sender: linux-integrity-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-integrity@vger.kernel.org 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 --- * 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 43d49ad958fb..5eb14b567a31 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 Fri Jun 26 22:39: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: 11629071 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 082421731 for ; Fri, 26 Jun 2020 22:40:10 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id DCDF12100A for ; Fri, 26 Jun 2020 22:40:09 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (1024-bit key) header.d=linux.microsoft.com header.i=@linux.microsoft.com header.b="GAK98Rzg" Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1726158AbgFZWjs (ORCPT ); Fri, 26 Jun 2020 18:39:48 -0400 Received: from linux.microsoft.com ([13.77.154.182]:37972 "EHLO linux.microsoft.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726530AbgFZWjr (ORCPT ); Fri, 26 Jun 2020 18:39:47 -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 8A46220B4901; Fri, 26 Jun 2020 15:39:44 -0700 (PDT) DKIM-Filter: OpenDKIM Filter v2.11.0 linux.microsoft.com 8A46220B4901 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linux.microsoft.com; s=default; t=1593211185; bh=WG1KYGxBabmnas+RFIfamDI2zDei+OK4HHl8rZcS7h4=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=GAK98Rzg5DPdMyh/1Z+jC5yie/1+3Vl3BHrABfDdHoMgPHmZntawK49gcBxGsF+S1 mRoc2Qp0AQGqi0skLOllegODkRgM3EJIN3QfVzgWYL1q1LAFhZxHTkoHx6x41KlwzH JZusT10BIaZxBrd83W23/7jSR6UJ2rqc373ad980= 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 v2 11/11] ima: Support additional conditionals in the KEXEC_CMDLINE hook function Date: Fri, 26 Jun 2020 17:39:00 -0500 Message-Id: <20200626223900.253615-12-tyhicks@linux.microsoft.com> X-Mailer: git-send-email 2.25.1 In-Reply-To: <20200626223900.253615-1-tyhicks@linux.microsoft.com> References: <20200626223900.253615-1-tyhicks@linux.microsoft.com> MIME-Version: 1.0 Sender: linux-integrity-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-integrity@vger.kernel.org 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 --- * 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 59ec28f5c117..ff2bf57ff0c7 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 5eb14b567a31..294323b36d06 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)) @@ -1007,10 +1003,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,