diff mbox series

[v2,04/12] evm: Execute evm_inode_init_security() only when the HMAC key is loaded

Message ID 20200904092339.19598-5-roberto.sassu@huawei.com (mailing list archive)
State New, archived
Headers show
Series IMA/EVM fixes | expand

Commit Message

Roberto Sassu Sept. 4, 2020, 9:23 a.m. UTC
evm_inode_init_security() requires the HMAC key to calculate the HMAC on
initial xattrs provided by LSMs. Unfortunately, with the evm_key_loaded()
check, the function continues even if the HMAC key is not loaded
(evm_key_loaded() returns true also if EVM has been initialized only with a
public key). If the HMAC key is not loaded, evm_inode_init_security()
returns an error later when it calls evm_init_hmac().

Thus, this patch replaces the evm_key_loaded() check with a check of the
EVM_INIT_HMAC flag in evm_initialized, so that evm_inode_init_security()
returns 0 instead of an error.

Cc: stable@vger.kernel.org # 4.5.x
Fixes: 26ddabfe96b ("evm: enable EVM when X509 certificate is loaded")
Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>
Reviewed-by: Mimi Zohar <zohar@linux.ibm.com>
---
 security/integrity/evm/evm_main.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

Comments

Sasha Levin Sept. 7, 2020, 3:03 p.m. UTC | #1
Hi

[This is an automated email]

This commit has been processed because it contains a "Fixes:" tag
fixing commit: 26ddabfe96bb ("evm: enable EVM when X509 certificate is loaded").

The bot has tested the following trees: v5.8.7, v5.4.63, v4.19.143, v4.14.196, v4.9.235.

v5.8.7: Build OK!
v5.4.63: Build OK!
v4.19.143: Build OK!
v4.14.196: Failed to apply! Possible dependencies:
    21af76631476 ("EVM: turn evm_config_xattrnames into a list")
    5feeb61183dd ("evm: Allow non-SHA1 digital signatures")
    650b29dbdf2c ("integrity: Introduce struct evm_xattr")
    ae1ba1676b88 ("EVM: Allow userland to permit modification of EVM-protected metadata")
    b33e3cc5c90b ("Merge branch 'next-integrity' of git://git.kernel.org/pub/scm/linux/kernel/git/jmorris/linux-security")
    f00d79750712 ("EVM: Allow userspace to signal an RSA key has been loaded")

v4.9.235: Failed to apply! Possible dependencies:
    21af76631476 ("EVM: turn evm_config_xattrnames into a list")
    5feeb61183dd ("evm: Allow non-SHA1 digital signatures")
    650b29dbdf2c ("integrity: Introduce struct evm_xattr")
    ae1ba1676b88 ("EVM: Allow userland to permit modification of EVM-protected metadata")
    b33e3cc5c90b ("Merge branch 'next-integrity' of git://git.kernel.org/pub/scm/linux/kernel/git/jmorris/linux-security")
    b4bfec7f4a86 ("security/integrity: Harden against malformed xattrs")
    f00d79750712 ("EVM: Allow userspace to signal an RSA key has been loaded")


NOTE: The patch will not be queued to stable trees until it is upstream.

How should we proceed with this patch?
Mimi Zohar Sept. 16, 2020, 4:15 p.m. UTC | #2
Hi Roberto,

On Fri, 2020-09-04 at 11:23 +0200, Roberto Sassu wrote:
> evm_inode_init_security() requires the HMAC key to calculate the HMAC on
> initial xattrs provided by LSMs. Unfortunately, with the evm_key_loaded()
> check, the function continues even if the HMAC key is not loaded
> (evm_key_loaded() returns true also if EVM has been initialized only with a
> public key). If the HMAC key is not loaded, evm_inode_init_security()
> returns an error later when it calls evm_init_hmac().

This is all true, but the context for why it wasn't an issue previously
is missing.

The original usecase for allowing signature verificaton prior to
loading the HMAC key was a fully signed, possibly immutable, initrd. 
No new files were created or, at least, were in policy until the HMAC
key was loaded.   More recently support for requiring an EVM HMAC key
was removed.  Files having a portable and immutable signature were
given additional privileges.

Please update the patch description with the context of what has
changed.

Mimi
diff mbox series

Patch

diff --git a/security/integrity/evm/evm_main.c b/security/integrity/evm/evm_main.c
index e4b47759ba1c..4e9f5e8b21d5 100644
--- a/security/integrity/evm/evm_main.c
+++ b/security/integrity/evm/evm_main.c
@@ -527,7 +527,8 @@  int evm_inode_init_security(struct inode *inode,
 	struct evm_xattr *xattr_data;
 	int rc;
 
-	if (!evm_key_loaded() || !evm_protected_xattr(lsm_xattr->name))
+	if (!(evm_initialized & EVM_INIT_HMAC) ||
+	    !evm_protected_xattr(lsm_xattr->name))
 		return 0;
 
 	xattr_data = kzalloc(sizeof(*xattr_data), GFP_NOFS);