From patchwork Mon Sep 26 14:08:08 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Christian Brauner X-Patchwork-Id: 12988933 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id E8C98C6FA97 for ; Mon, 26 Sep 2022 15:24:18 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S236368AbiIZPYR (ORCPT ); Mon, 26 Sep 2022 11:24:17 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:35592 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S236557AbiIZPXn (ORCPT ); Mon, 26 Sep 2022 11:23:43 -0400 Received: from ams.source.kernel.org (ams.source.kernel.org [145.40.68.75]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id B1A2188DD8; Mon, 26 Sep 2022 07:09:16 -0700 (PDT) Received: from smtp.kernel.org (relay.kernel.org [52.25.139.140]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ams.source.kernel.org (Postfix) with ESMTPS id 76575B80A72; Mon, 26 Sep 2022 14:09:14 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id AC556C43141; Mon, 26 Sep 2022 14:09:10 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1664201353; bh=nozG0WcEYu9YjxS/9G2XFppJTyXK2JL22UJ4HGPG+rA=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=L1FtIZNJFBlz+4L6w68g1ebOzAEeWXzHB9dtDQNRyPXjouNPMJMsM0By/IuW2yBdv xWyhe7tSPePVEMG9j0hpldO5Y8kSAQQ5LOcCHwrX7jdvP/texE6Ou5FvY1fIz79mYL FGcfXwk6Xh8QcbeB7Jl6JlLAy3F+/iRvC4noP+wNuqPjcTipQFb/ZuzH50l3G/BXMR pYd9umL2JsR0QWFGTjKI9lZDcDarFQ+a37Bh6tYnZ2MUTW2KwbfaX0+3Mvzq2lkbyE LXz7g6fnvB/r4YaTO8VTwbYeMGc+Kzr8w/fNe24j9AbF/6JSwZ6FnNtsh6N4sFckhb tfd6E3sB8ua5A== From: Christian Brauner To: linux-fsdevel@vger.kernel.org Cc: Christian Brauner , Seth Forshee , Christoph Hellwig , Al Viro , linux-integrity@vger.kernel.org, Paul Moore , Stephen Smalley , Eric Paris , selinux@vger.kernel.org, linux-security-module@vger.kernel.org Subject: [PATCH v2 11/30] selinux: implement set acl hook Date: Mon, 26 Sep 2022 16:08:08 +0200 Message-Id: <20220926140827.142806-12-brauner@kernel.org> X-Mailer: git-send-email 2.34.1 In-Reply-To: <20220926140827.142806-1-brauner@kernel.org> References: <20220926140827.142806-1-brauner@kernel.org> MIME-Version: 1.0 X-Developer-Signature: v=1; a=openpgp-sha256; l=3008; i=brauner@kernel.org; h=from:subject; bh=nozG0WcEYu9YjxS/9G2XFppJTyXK2JL22UJ4HGPG+rA=; b=owGbwMvMwCU28Zj0gdSKO4sYT6slMSQbbrJPyeo7wFljsE9o1n6z8ISjmTnPfUO5zwe8WLuwS7vh lnZtRykLgxgXg6yYIotDu0m43HKeis1GmRowc1iZQIYwcHEKwEQeL2Bk2F9QdMTn6Ym/uRqBL9z9TT edY3ycrT1VYGuQlO77Al3pcwz/3URt/HfviNfQv3bv6K2ahSGT5kdfNjVY6fI4V5WrZvYffgA= X-Developer-Key: i=brauner@kernel.org; a=openpgp; fpr=4880B8C9BD0E5106FC070F4F7B3C391EFEA93624 Precedence: bulk List-ID: X-Mailing-List: linux-integrity@vger.kernel.org The current way of setting and getting posix acls through the generic xattr interface is error prone and type unsafe. The vfs needs to interpret and fixup posix acls before storing or reporting it to userspace. Various hacks exist to make this work. The code is hard to understand and difficult to maintain in it's current form. Instead of making this work by hacking posix acls through xattr handlers we are building a dedicated posix acl api around the get and set inode operations. This removes a lot of hackiness and makes the codepaths easier to maintain. A lot of background can be found in [1]. So far posix acls were passed as a void blob to the security and integrity modules. Some of them like evm then proceed to interpret the void pointer and convert it into the kernel internal struct posix acl representation to perform their integrity checking magic. This is obviously pretty problematic as that requires knowledge that only the vfs is guaranteed to have and has lead to various bugs. Add a proper security hook for setting posix acls and pass down the posix acls in their appropriate vfs format instead of hacking it through a void pointer stored in the uapi format. I spent considerate time in the security module infrastructure and audited all codepaths. SELinux has no restrictions based on the posix acl values passed through it. The capability hook doesn't need to be called either because it only has restrictions on security.* xattrs. So this all becomes a very simple hook for SELinux. Link: https://lore.kernel.org/all/20220801145520.1532837-1-brauner@kernel.org [1] Signed-off-by: Christian Brauner (Microsoft) Acked-by: Paul Moore --- Notes: /* v2 */ unchanged security/selinux/hooks.c | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c index 79573504783b..bbc0ce3bde35 100644 --- a/security/selinux/hooks.c +++ b/security/selinux/hooks.c @@ -3239,6 +3239,13 @@ static int selinux_inode_setxattr(struct user_namespace *mnt_userns, &ad); } +static int selinux_inode_set_acl(struct user_namespace *mnt_userns, + struct dentry *dentry, const char *acl_name, + struct posix_acl *kacl) +{ + return dentry_has_perm(current_cred(), dentry, FILE__SETATTR); +} + static void selinux_inode_post_setxattr(struct dentry *dentry, const char *name, const void *value, size_t size, int flags) @@ -7063,6 +7070,7 @@ static struct security_hook_list selinux_hooks[] __lsm_ro_after_init = { LSM_HOOK_INIT(inode_getxattr, selinux_inode_getxattr), LSM_HOOK_INIT(inode_listxattr, selinux_inode_listxattr), LSM_HOOK_INIT(inode_removexattr, selinux_inode_removexattr), + LSM_HOOK_INIT(inode_set_acl, selinux_inode_set_acl), LSM_HOOK_INIT(inode_getsecurity, selinux_inode_getsecurity), LSM_HOOK_INIT(inode_setsecurity, selinux_inode_setsecurity), LSM_HOOK_INIT(inode_listsecurity, selinux_inode_listsecurity), From patchwork Mon Sep 26 14:08:10 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Christian Brauner X-Patchwork-Id: 12988932 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id B984EC6FA95 for ; Mon, 26 Sep 2022 15:24:17 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S236287AbiIZPYQ (ORCPT ); Mon, 26 Sep 2022 11:24:16 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:49390 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S236600AbiIZPXs (ORCPT ); Mon, 26 Sep 2022 11:23:48 -0400 Received: from dfw.source.kernel.org (dfw.source.kernel.org [139.178.84.217]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id AA3FE895CE; Mon, 26 Sep 2022 07:09:19 -0700 (PDT) Received: from smtp.kernel.org (relay.kernel.org [52.25.139.140]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by dfw.source.kernel.org (Postfix) with ESMTPS id 4BF6960DCB; Mon, 26 Sep 2022 14:09:18 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id DDC11C433D6; Mon, 26 Sep 2022 14:09:15 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1664201357; bh=nl1i991MblUNHLpQmd0aXp6JekgwWKpVmd622eCnFik=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=oqxXRB7fLSPf/LhYaBYCSE2whhM4B9vvdNp14rWfX+71fkbscxq1YZ4XaJPxWLQFq xELRekmTiSerxdZnRgrJF/9ItSQarCmtqHrHBiFq+nc9nuuP6sD3LTrm9YELIT6cKz PEbZB8RpSsWQkKmSJUA9K6lxTc/cG4L0eYnoeLKkKOACQ15SFPosry/Ql0gMynmf/D wqWemRlFLiQ3wSYcVEBeclWqKX0UVbla6Gx06Md2o17+matr8Dz7GEVGkqwnLRyq9r BluHbUgrz6celIc9dGgrHy2v1aLrdvYqo76OAREhZ8za35lM7fW2I8mZShW3cTSQ81 ZeOEE+qNH5JcQ== From: Christian Brauner To: linux-fsdevel@vger.kernel.org Cc: Christian Brauner , Seth Forshee , Christoph Hellwig , Al Viro , Mimi Zohar , linux-integrity@vger.kernel.org, linux-security-module@vger.kernel.org Subject: [PATCH v2 13/30] evm: implement set acl hook Date: Mon, 26 Sep 2022 16:08:10 +0200 Message-Id: <20220926140827.142806-14-brauner@kernel.org> X-Mailer: git-send-email 2.34.1 In-Reply-To: <20220926140827.142806-1-brauner@kernel.org> References: <20220926140827.142806-1-brauner@kernel.org> MIME-Version: 1.0 X-Developer-Signature: v=1; a=openpgp-sha256; l=6863; i=brauner@kernel.org; h=from:subject; bh=nl1i991MblUNHLpQmd0aXp6JekgwWKpVmd622eCnFik=; b=owGbwMvMwCU28Zj0gdSKO4sYT6slMSQbbnJgs2X5sCN51oGV0neL7MRSpvw8tfSK2Zk/C6yL4tXn LM553FHKwiDGxSArpsji0G4SLrecp2KzUaYGzBxWJpAhDFycAjCRjRMYGRrWGt+/INq4tcU4XbTNUO FX3orm6UWLr9zfrCC0jK8u/RDDfxe+6jvNc4uFTr8z+bnSL50tS6Otzcix6t/S7A//TyUF8QIA X-Developer-Key: i=brauner@kernel.org; a=openpgp; fpr=4880B8C9BD0E5106FC070F4F7B3C391EFEA93624 Precedence: bulk List-ID: X-Mailing-List: linux-integrity@vger.kernel.org The current way of setting and getting posix acls through the generic xattr interface is error prone and type unsafe. The vfs needs to interpret and fixup posix acls before storing or reporting it to userspace. Various hacks exist to make this work. The code is hard to understand and difficult to maintain in it's current form. Instead of making this work by hacking posix acls through xattr handlers we are building a dedicated posix acl api around the get and set inode operations. This removes a lot of hackiness and makes the codepaths easier to maintain. A lot of background can be found in [1]. So far posix acls were passed as a void blob to the security and integrity modules. Some of them like evm then proceed to interpret the void pointer and convert it into the kernel internal struct posix acl representation to perform their integrity checking magic. This is obviously pretty problematic as that requires knowledge that only the vfs is guaranteed to have and has lead to various bugs. Add a proper security hook for setting posix acls and pass down the posix acls in their appropriate vfs format instead of hacking it through a void pointer stored in the uapi format. I spent considerate time in the security module and integrity infrastructure and audited all codepaths. EVM is the only part that really has restrictions based on the actual posix acl values passed through it. Before this dedicated hook EVM used to translate from the uapi posix acl format sent to it in the form of a void pointer into the vfs format. This is not a good thing. Instead of hacking around in the uapi struct give EVM the posix acls in the appropriate vfs format and perform sane permissions checks that mirror what it used to to in the generic xattr hook. Link: https://lore.kernel.org/all/20220801145520.1532837-1-brauner@kernel.org [1] Signed-off-by: Christian Brauner (Microsoft) Reviewed-by: Paul Moore --- Notes: /* v2 */ unchanged include/linux/evm.h | 10 +++++ security/integrity/evm/evm_main.c | 66 ++++++++++++++++++++++++++++++- security/security.c | 9 ++++- 3 files changed, 83 insertions(+), 2 deletions(-) diff --git a/include/linux/evm.h b/include/linux/evm.h index aa63e0b3c0a2..aebcfd47d496 100644 --- a/include/linux/evm.h +++ b/include/linux/evm.h @@ -35,6 +35,9 @@ extern int evm_inode_removexattr(struct user_namespace *mnt_userns, struct dentry *dentry, const char *xattr_name); extern void evm_inode_post_removexattr(struct dentry *dentry, const char *xattr_name); +extern int evm_inode_set_acl(struct user_namespace *mnt_userns, + struct dentry *dentry, const char *acl_name, + struct posix_acl *kacl); extern int evm_inode_init_security(struct inode *inode, const struct xattr *xattr_array, struct xattr *evm); @@ -108,6 +111,13 @@ static inline void evm_inode_post_removexattr(struct dentry *dentry, return; } +static inline int evm_inode_set_acl(struct user_namespace *mnt_userns, + struct dentry *dentry, const char *acl_name, + struct posix_acl *kacl) +{ + return 0; +} + static inline int evm_inode_init_security(struct inode *inode, const struct xattr *xattr_array, struct xattr *evm) diff --git a/security/integrity/evm/evm_main.c b/security/integrity/evm/evm_main.c index 23d484e05e6f..15aa5995fff4 100644 --- a/security/integrity/evm/evm_main.c +++ b/security/integrity/evm/evm_main.c @@ -8,7 +8,7 @@ * * File: evm_main.c * implements evm_inode_setxattr, evm_inode_post_setxattr, - * evm_inode_removexattr, and evm_verifyxattr + * evm_inode_removexattr, evm_verifyxattr, and evm_inode_set_acl. */ #define pr_fmt(fmt) "EVM: "fmt @@ -670,6 +670,70 @@ int evm_inode_removexattr(struct user_namespace *mnt_userns, return evm_protect_xattr(mnt_userns, dentry, xattr_name, NULL, 0); } +static int evm_inode_set_acl_change(struct user_namespace *mnt_userns, + struct dentry *dentry, const char *name, + struct posix_acl *kacl) +{ +#ifdef CONFIG_FS_POSIX_ACL + int rc; + umode_t mode; + struct inode *inode = d_backing_inode(dentry); + + rc = posix_acl_update_mode(mnt_userns, inode, &mode, &kacl); + if (rc || (inode->i_mode != mode)) + return 1; +#endif + return 0; +} + +/** + * evm_inode_set_acl - protect the EVM extended attribute for posix acls + * @mnt_userns: user namespace of the idmapped mount + * @dentry: pointer to the affected dentry + * @acl_name: name of the posix acl + * @kacl: pointer to the posix acls + */ +int evm_inode_set_acl(struct user_namespace *mnt_userns, struct dentry *dentry, + const char *acl_name, struct posix_acl *kacl) +{ + enum integrity_status evm_status; + + /* Policy permits modification of the protected xattrs even though + * there's no HMAC key loaded + */ + if (evm_initialized & EVM_ALLOW_METADATA_WRITES) + return 0; + + evm_status = evm_verify_current_integrity(dentry); + if ((evm_status == INTEGRITY_PASS) || + (evm_status == INTEGRITY_NOXATTRS)) + return 0; + + /* Exception if the HMAC is not going to be calculated. */ + if (evm_hmac_disabled() && (evm_status == INTEGRITY_NOLABEL || + evm_status == INTEGRITY_UNKNOWN)) + return 0; + + /* + * Writing other xattrs is safe for portable signatures, as portable + * signatures are immutable and can never be updated. + */ + if (evm_status == INTEGRITY_FAIL_IMMUTABLE) + return 0; + + if (evm_status == INTEGRITY_PASS_IMMUTABLE && + !evm_inode_set_acl_change(mnt_userns, dentry, acl_name, kacl)) + return 0; + + if (evm_status != INTEGRITY_PASS && + evm_status != INTEGRITY_PASS_IMMUTABLE) + integrity_audit_msg(AUDIT_INTEGRITY_METADATA, d_backing_inode(dentry), + dentry->d_name.name, "appraise_metadata", + integrity_status_msg[evm_status], + -EPERM, 0); + return evm_status == INTEGRITY_PASS ? 0 : -EPERM; +} + static void evm_reset_status(struct inode *inode) { struct integrity_iint_cache *iint; diff --git a/security/security.c b/security/security.c index 56d48e7254d6..a12a26a4494e 100644 --- a/security/security.c +++ b/security/security.c @@ -1374,9 +1374,16 @@ int security_inode_set_acl(struct user_namespace *mnt_userns, struct dentry *dentry, const char *acl_name, struct posix_acl *kacl) { + int ret; + if (unlikely(IS_PRIVATE(d_backing_inode(dentry)))) return 0; - return call_int_hook(inode_set_acl, 0, mnt_userns, dentry, acl_name, kacl); + + ret = call_int_hook(inode_set_acl, 0, mnt_userns, dentry, acl_name, kacl); + if (ret) + return ret; + + return evm_inode_set_acl(mnt_userns, dentry, acl_name, kacl); } void security_inode_post_setxattr(struct dentry *dentry, const char *name, From patchwork Mon Sep 26 14:08:12 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Christian Brauner X-Patchwork-Id: 12988934 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id C53B5C6FA9A for ; Mon, 26 Sep 2022 15:24:20 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S236411AbiIZPYR (ORCPT ); Mon, 26 Sep 2022 11:24:17 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:56944 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S236612AbiIZPXu (ORCPT ); Mon, 26 Sep 2022 11:23:50 -0400 Received: from dfw.source.kernel.org (dfw.source.kernel.org [139.178.84.217]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id B7EDF895F5; Mon, 26 Sep 2022 07:09:23 -0700 (PDT) Received: from smtp.kernel.org (relay.kernel.org [52.25.139.140]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by dfw.source.kernel.org (Postfix) with ESMTPS id 8136D60DEB; Mon, 26 Sep 2022 14:09:22 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id 1AD46C433D7; Mon, 26 Sep 2022 14:09:19 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1664201361; bh=4761Hw9EEwamAX3MILsd8qPl7LDxfk4ndzMC4c60hY4=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=JgrhKlhbESAjyQzTk8hzWhzTZC1J2qEkaisrKmFDDuyzzYertdey5qghNHlfqR4fj FGDkbX6+Ihw9d/wYeb5kX3WRuCtFivMM106y/oxEEDIUOrG70aSsfRZXRrqO50IyU8 hdSaQeha9734oF9xuMvg35zWYe+y7myfdrR3N77Y6GFfd5GOXfLGrtUIkBGcG4kzDW 5noxg+LrHxPiO3irPfHkJz6kh+0p6rZ8WQqULUhqKMX55lPdZ4fOc96OXEd6GL8Uyt J9pOFOZ2zegHUqIMlVzuM5t1z0/nU3gYyP/VojIG3X3j7BEZLg91xDWGq2utzOJpTR QsWoyR66KPStQ== From: Christian Brauner To: linux-fsdevel@vger.kernel.org Cc: Christian Brauner , Seth Forshee , Christoph Hellwig , Al Viro , Mimi Zohar , linux-integrity@vger.kernel.org, linux-security-module@vger.kernel.org Subject: [PATCH v2 15/30] evm: add post set acl hook Date: Mon, 26 Sep 2022 16:08:12 +0200 Message-Id: <20220926140827.142806-16-brauner@kernel.org> X-Mailer: git-send-email 2.34.1 In-Reply-To: <20220926140827.142806-1-brauner@kernel.org> References: <20220926140827.142806-1-brauner@kernel.org> MIME-Version: 1.0 X-Developer-Signature: v=1; a=openpgp-sha256; l=3666; i=brauner@kernel.org; h=from:subject; bh=4761Hw9EEwamAX3MILsd8qPl7LDxfk4ndzMC4c60hY4=; b=owGbwMvMwCU28Zj0gdSKO4sYT6slMSQbbnJgzg6+OMeJJbjpaznf0eVliq2sqR922da0Lpq38Pv7 gnNlHaUsDGJcDLJiiiwO7Sbhcst5KjYbZWrAzGFlAhnCwMUpABOZ2s7wP0vgyv3CQ8I35jJFGsw9sP vp0zU1mcVsIozHahKMF60oO8nIMCfowUf3l7yHLmQ/6p9zYn2kDs+fd5b7z73eMS8q0vmTEjMA X-Developer-Key: i=brauner@kernel.org; a=openpgp; fpr=4880B8C9BD0E5106FC070F4F7B3C391EFEA93624 Precedence: bulk List-ID: X-Mailing-List: linux-integrity@vger.kernel.org The security_inode_post_setxattr() hook is used by security modules to update their own security.* xattrs. Consequently none of the security modules operate on posix acls. So we don't need an additional security hook when post setting posix acls. However, the integrity subsystem wants to be informed about posix acl changes and specifically evm to update their hashes when the xattrs change. The callchain for evm_inode_post_setxattr() is: -> evm_inode_post_setxattr() -> evm_update_evmxattr() -> evm_calc_hmac() -> evm_calc_hmac_or_hash() and evm_cacl_hmac_or_hash() walks the global list of protected xattr names evm_config_xattrnames. This global list can be modified via /sys/security/integrity/evm/evm_xattrs. The write to "evm_xattrs" is restricted to security.* xattrs and the default xattrs in evm_config_xattrnames only contains security.* xattrs as well. So the actual value for posix acls is currently completely irrelevant for evm during evm_inode_post_setxattr() and frankly it should stay that way in the future to not cause the vfs any more headaches. But if the actual posix acl values matter then evm shouldn't operate on the binary void blob and try to hack around in the uapi struct anyway. Instead it should then in the future add a dedicated hook which takes a struct posix_acl argument passing the posix acls in the proper vfs format. For now it is sufficient to make evm_inode_post_set_acl() a wrapper around evm_inode_post_setxattr() not passing any actual values down. This will still cause the hashes to be updated as before. Signed-off-by: Christian Brauner (Microsoft) Reviewed-by: Paul Moore --- Notes: /* v2 */ unchanged fs/posix_acl.c | 5 ++++- include/linux/evm.h | 13 +++++++++++++ 2 files changed, 17 insertions(+), 1 deletion(-) diff --git a/fs/posix_acl.c b/fs/posix_acl.c index 471d17fa1611..ef0908a4bc46 100644 --- a/fs/posix_acl.c +++ b/fs/posix_acl.c @@ -25,6 +25,7 @@ #include #include #include +#include #include static struct posix_acl **acl_by_type(struct inode *inode, int type) @@ -1351,8 +1352,10 @@ int vfs_set_acl(struct user_namespace *mnt_userns, struct dentry *dentry, error = -EIO; else error = -EOPNOTSUPP; - if (!error) + if (!error) { fsnotify_xattr(dentry); + evm_inode_post_set_acl(dentry, acl_name, kacl); + } out_inode_unlock: inode_unlock(inode); diff --git a/include/linux/evm.h b/include/linux/evm.h index aebcfd47d496..7811ce56e02f 100644 --- a/include/linux/evm.h +++ b/include/linux/evm.h @@ -38,6 +38,12 @@ extern void evm_inode_post_removexattr(struct dentry *dentry, extern int evm_inode_set_acl(struct user_namespace *mnt_userns, struct dentry *dentry, const char *acl_name, struct posix_acl *kacl); +static inline void evm_inode_post_set_acl(struct dentry *dentry, + const char *acl_name, + struct posix_acl *kacl) +{ + return evm_inode_post_setxattr(dentry, acl_name, NULL, 0); +} extern int evm_inode_init_security(struct inode *inode, const struct xattr *xattr_array, struct xattr *evm); @@ -118,6 +124,13 @@ static inline int evm_inode_set_acl(struct user_namespace *mnt_userns, return 0; } +static inline void evm_inode_post_set_acl(struct dentry *dentry, + const char *acl_name, + struct posix_acl *kacl) +{ + return; +} + static inline int evm_inode_init_security(struct inode *inode, const struct xattr *xattr_array, struct xattr *evm) From patchwork Mon Sep 26 14:08:15 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Christian Brauner X-Patchwork-Id: 12988935 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 870A9C07E9D for ; Mon, 26 Sep 2022 15:24:26 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S236498AbiIZPYX (ORCPT ); Mon, 26 Sep 2022 11:24:23 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:57374 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S236642AbiIZPX4 (ORCPT ); Mon, 26 Sep 2022 11:23:56 -0400 Received: from sin.source.kernel.org (sin.source.kernel.org [IPv6:2604:1380:40e1:4800::1]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 6977324B; Mon, 26 Sep 2022 07:09:32 -0700 (PDT) Received: from smtp.kernel.org (relay.kernel.org [52.25.139.140]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by sin.source.kernel.org (Postfix) with ESMTPS id 8117DCE114D; Mon, 26 Sep 2022 14:09:30 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id 742AAC433D7; Mon, 26 Sep 2022 14:09:26 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1664201368; bh=mZiUJrvg8yaqgBdnDKc8+Ezywv02rJLKTDGCaJnn3OU=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=IqZtyn/JKkfubM8GJuzxSffRB5aOiFKohVcQSGC2SR8LnaGO6xIxS/fKGvWgrzNW0 zuANfLYr0H8e3MwqJumW8OQRasTFzPt15cBkMda73e1S3rxByLNV55HcO6VordKJPB NBhFwDWdca1yfGK8SdEAzBLvOOXNMzNChOWexBWWel4aqZyPwY13NMzrh3MG6UbHBp Q3ii02eu3eayEqZpqoBGWVXGn68ZzKOabEPLBfTn0dYMXOjUpBFiYnRJnh0Cbo719m SQF8QHholkqKtn+60zFLWcqbOh3DdI0sPT9nq03ii7dXDB7w3PkbgBf8r82TvhsM0r Xh/oEQkkUxDLg== From: Christian Brauner To: linux-fsdevel@vger.kernel.org Cc: Christian Brauner , Seth Forshee , Christoph Hellwig , Al Viro , Mimi Zohar , linux-integrity@vger.kernel.org, linux-security-module@vger.kernel.org Subject: [PATCH v2 18/30] evm: simplify evm_xattr_acl_change() Date: Mon, 26 Sep 2022 16:08:15 +0200 Message-Id: <20220926140827.142806-19-brauner@kernel.org> X-Mailer: git-send-email 2.34.1 In-Reply-To: <20220926140827.142806-1-brauner@kernel.org> References: <20220926140827.142806-1-brauner@kernel.org> MIME-Version: 1.0 X-Developer-Signature: v=1; a=openpgp-sha256; l=4006; i=brauner@kernel.org; h=from:subject; bh=mZiUJrvg8yaqgBdnDKc8+Ezywv02rJLKTDGCaJnn3OU=; b=owGbwMvMwCU28Zj0gdSKO4sYT6slMSQbbnI8nD+vU2XRUo206wdMFs3I1JiSnf/iscfV4J7aGWWB 2/x3dJSyMIhxMciKKbI4tJuEyy3nqdhslKkBM4eVCWQIAxenAEzkyEyGfzobjYXyCi9vYWG6+6OB/d WfyYf7LtydL75v3vTlrP2JnnMY/ofcW66kysX8nenC9vUsthMi0tvOiBc8KfE4s3/rOxWLs9wA X-Developer-Key: i=brauner@kernel.org; a=openpgp; fpr=4880B8C9BD0E5106FC070F4F7B3C391EFEA93624 Precedence: bulk List-ID: X-Mailing-List: linux-integrity@vger.kernel.org The posix acl api provides a dedicated security and integrity hook for setting posix acls. This means that evm_protect_xattr() -> evm_xattr_change() -> evm_xattr_acl_change() is now only hit during vfs_remove_acl() at which point we are guaranteed that xattr_value and xattr_value_len are NULL and 0. In this case evm always used to return 1. Simplify this function to do just that. Signed-off-by: Christian Brauner (Microsoft) --- Notes: /* v2 */ unchanged security/integrity/evm/evm_main.c | 62 +++++++------------------------ 1 file changed, 14 insertions(+), 48 deletions(-) diff --git a/security/integrity/evm/evm_main.c b/security/integrity/evm/evm_main.c index 15aa5995fff4..1fbe1b8d0364 100644 --- a/security/integrity/evm/evm_main.c +++ b/security/integrity/evm/evm_main.c @@ -436,62 +436,29 @@ static enum integrity_status evm_verify_current_integrity(struct dentry *dentry) /* * evm_xattr_acl_change - check if passed ACL changes the inode mode - * @mnt_userns: user namespace of the idmapped mount - * @dentry: pointer to the affected dentry * @xattr_name: requested xattr * @xattr_value: requested xattr value * @xattr_value_len: requested xattr value length * - * Check if passed ACL changes the inode mode, which is protected by EVM. + * This is only hit during xattr removal at which point we always return 1. + * Splat a warning in case someone managed to pass data to this function. That + * should never happen. * * Returns 1 if passed ACL causes inode mode change, 0 otherwise. */ -static int evm_xattr_acl_change(struct user_namespace *mnt_userns, - struct dentry *dentry, const char *xattr_name, - const void *xattr_value, size_t xattr_value_len) +static int evm_xattr_acl_change(const void *xattr_value, size_t xattr_value_len) { -#ifdef CONFIG_FS_POSIX_ACL - umode_t mode; - struct posix_acl *acl = NULL, *acl_res; - struct inode *inode = d_backing_inode(dentry); - int rc; - - /* - * An earlier comment here mentioned that the idmappings for - * ACL_{GROUP,USER} don't matter since EVM is only interested in the - * mode stored as part of POSIX ACLs. Nonetheless, if it must translate - * from the uapi POSIX ACL representation to the VFS internal POSIX ACL - * representation it should do so correctly. There's no guarantee that - * we won't change POSIX ACLs in a way that ACL_{GROUP,USER} matters - * for the mode at some point and it's difficult to keep track of all - * the LSM and integrity modules and what they do to POSIX ACLs. - * - * Frankly, EVM shouldn't try to interpret the uapi struct for POSIX - * ACLs it received. It requires knowledge that only the VFS is - * guaranteed to have. - */ - acl = vfs_set_acl_prepare(mnt_userns, i_user_ns(inode), - xattr_value, xattr_value_len); - if (IS_ERR_OR_NULL(acl)) - return 1; - - acl_res = acl; - /* - * Passing mnt_userns is necessary to correctly determine the GID in - * an idmapped mount, as the GID is used to clear the setgid bit in - * the inode mode. - */ - rc = posix_acl_update_mode(mnt_userns, inode, &mode, &acl_res); - - posix_acl_release(acl); - - if (rc) - return 1; + int rc = 0; - if (inode->i_mode != mode) - return 1; +#ifdef CONFIG_FS_POSIX_ACL + WARN_ONCE(xattr_value != NULL, + "Passing xattr value for POSIX ACLs not supported\n"); + WARN_ONCE(xattr_value_len != 0, + "Passing non-zero length for POSIX ACLs not supported\n"); + rc = 1; #endif - return 0; + + return rc; } /* @@ -514,8 +481,7 @@ static int evm_xattr_change(struct user_namespace *mnt_userns, int rc = 0; if (posix_xattr_acl(xattr_name)) - return evm_xattr_acl_change(mnt_userns, dentry, xattr_name, - xattr_value, xattr_value_len); + return evm_xattr_acl_change(xattr_value, xattr_value_len); rc = vfs_getxattr_alloc(&init_user_ns, dentry, xattr_name, &xattr_data, 0, GFP_NOFS);