RE: [PATCH v4 04/11] ima: Move ima_reset_appraise_flags() call to post hooks
From: Roberto Sassu <roberto.sassu@huawei.com>
Date: 2021-04-27 09:25:22
Also in:
linux-fsdevel, linux-security-module, lkml
From: Mimi Zohar [mailto:zohar@linux.ibm.com] Sent: Monday, April 26, 2021 9:49 PM On Fri, 2021-03-05 at 09:30 -0800, Casey Schaufler wrote:quoted
On 3/5/2021 7:19 AM, Roberto Sassu wrote:quoted
ima_inode_setxattr() and ima_inode_removexattr() hooks are called beforeanquoted
quoted
operation is performed. Thus, ima_reset_appraise_flags() should not be called there, as flags might be unnecessarily reset if the operation is denied. This patch introduces the post hooks ima_inode_post_setxattr() and ima_inode_post_removexattr(), and adds the call to ima_reset_appraise_flags() in the new functions.I don't see anything wrong with this patch in light of the way IMA and EVM have been treated to date. However ... The special casing of IMA and EVM in security.c is getting out of hand, and appears to be unnecessary. By my count there are 9 IMA hooks and 5 EVM hooks that have been hard coded. Adding this IMA hook makes 10. It would be really easy to register IMA and EVM as security modules. That would remove the dependency they currently have on security sub-system approval for changes like this one. I know there has been resistance to "IMA as an LSM" in the past, but it's pretty hard to see how it wouldn't be a win.Somehow I missed the new "lsm=" boot command line option, which dynamically allows enabling/disabling LSMs, being upstreamed. This would be one of the reasons for not making IMA/EVM full LSMs.
Hi Mimi one could argue why IMA/EVM should receive a special treatment. I understand that this was a necessity without LSM stacking. Now that LSM stacking is available, I don't see any valid reason why IMA/EVM should not be managed by the LSM infrastructure.
Both IMA and EVM file data/metadata is persistent across boots. If either one or the other is not enabled the file data hash or file metadata HMAC will not properly be updated, potentially preventing the system from booting when re-enabled. Re-enabling IMA and EVM would require "fixing" the mutable file data hash and HMAC, without any knowledge of what the "fixed" values should be. Dave Safford referred to this as "blessing" the newly calculated values.
IMA/EVM can be easily disabled in other ways, for example by moving the IMA policy or the EVM keys elsewhere. Also other LSMs rely on a dynamic and persistent state (for example for file transitions in SELinux), which cannot be trusted anymore if LSMs are even temporarily disabled. If IMA/EVM have to be enabled to prevent misconfiguration, I think the same can be achieved if they are full LSMs, for example by preventing that the list of enabled LSMs changes at run-time. Roberto HUAWEI TECHNOLOGIES Duesseldorf GmbH, HRB 56063 Managing Director: Li Peng, Li Jian, Shi Yanli