From ae1ba1676b88e6c62368a433c7e2d0417e9879fd Mon Sep 17 00:00:00 2001 From: Matthew Garrett Date: Tue, 7 Nov 2017 07:18:35 -0800 Subject: EVM: Allow userland to permit modification of EVM-protected metadata When EVM is enabled it forbids modification of metadata protected by EVM unless there is already a valid EVM signature. If any modification is made, the kernel will then generate a new EVM HMAC. However, this does not map well on use cases which use only asymmetric EVM signatures, as in this scenario the kernel is unable to generate new signatures. This patch extends the /sys/kernel/security/evm interface to allow userland to request that modification of these xattrs be permitted. This is only permitted if no keys have already been loaded. In this configuration, modifying the metadata will invalidate the EVM appraisal on the file in question. This allows packaging systems to write out new files, set the relevant extended attributes and then move them into place. There's also some refactoring of the use of evm_initialized in order to avoid heading down codepaths that assume there's a key available. Signed-off-by: Matthew Garrett Signed-off-by: Mimi Zohar --- security/integrity/evm/evm.h | 7 +++++-- security/integrity/evm/evm_main.c | 38 +++++++++++++++++++++++++++++++------- security/integrity/evm/evm_secfs.c | 20 +++++++++++++++++--- 3 files changed, 53 insertions(+), 12 deletions(-) (limited to 'security') diff --git a/security/integrity/evm/evm.h b/security/integrity/evm/evm.h index 241aca315b0c..3d05250e8313 100644 --- a/security/integrity/evm/evm.h +++ b/security/integrity/evm/evm.h @@ -23,9 +23,12 @@ #define EVM_INIT_HMAC 0x0001 #define EVM_INIT_X509 0x0002 -#define EVM_SETUP 0x80000000 /* userland has signaled key load */ +#define EVM_ALLOW_METADATA_WRITES 0x0004 +#define EVM_SETUP_COMPLETE 0x80000000 /* userland has signaled key load */ -#define EVM_INIT_MASK (EVM_INIT_HMAC | EVM_INIT_X509 | EVM_SETUP) +#define EVM_KEY_MASK (EVM_INIT_HMAC | EVM_INIT_X509) +#define EVM_INIT_MASK (EVM_INIT_HMAC | EVM_INIT_X509 | EVM_SETUP_COMPLETE | \ + EVM_ALLOW_METADATA_WRITES) extern int evm_initialized; extern char *evm_hmac; diff --git a/security/integrity/evm/evm_main.c b/security/integrity/evm/evm_main.c index 9826c02e2db8..ba89c2468298 100644 --- a/security/integrity/evm/evm_main.c +++ b/security/integrity/evm/evm_main.c @@ -76,6 +76,11 @@ static void __init evm_init_config(void) pr_info("HMAC attrs: 0x%x\n", evm_hmac_attrs); } +static bool evm_key_loaded(void) +{ + return (bool)(evm_initialized & EVM_KEY_MASK); +} + static int evm_find_protected_xattrs(struct dentry *dentry) { struct inode *inode = d_backing_inode(dentry); @@ -241,7 +246,7 @@ enum integrity_status evm_verifyxattr(struct dentry *dentry, void *xattr_value, size_t xattr_value_len, struct integrity_iint_cache *iint) { - if (!evm_initialized || !evm_protected_xattr(xattr_name)) + if (!evm_key_loaded() || !evm_protected_xattr(xattr_name)) return INTEGRITY_UNKNOWN; if (!iint) { @@ -265,7 +270,7 @@ static enum integrity_status evm_verify_current_integrity(struct dentry *dentry) { struct inode *inode = d_backing_inode(dentry); - if (!evm_initialized || !S_ISREG(inode->i_mode) || evm_fixmode) + if (!evm_key_loaded() || !S_ISREG(inode->i_mode) || evm_fixmode) return 0; return evm_verify_hmac(dentry, NULL, NULL, 0, NULL); } @@ -299,6 +304,7 @@ static int evm_protect_xattr(struct dentry *dentry, const char *xattr_name, return 0; goto out; } + evm_status = evm_verify_current_integrity(dentry); if (evm_status == INTEGRITY_NOXATTRS) { struct integrity_iint_cache *iint; @@ -345,6 +351,12 @@ int evm_inode_setxattr(struct dentry *dentry, const char *xattr_name, { const struct evm_ima_xattr_data *xattr_data = xattr_value; + /* Policy permits modification of the protected xattrs even though + * there's no HMAC key loaded + */ + if (evm_initialized & EVM_ALLOW_METADATA_WRITES) + return 0; + if (strcmp(xattr_name, XATTR_NAME_EVM) == 0) { if (!xattr_value_len) return -EINVAL; @@ -365,6 +377,12 @@ int evm_inode_setxattr(struct dentry *dentry, const char *xattr_name, */ int evm_inode_removexattr(struct dentry *dentry, const char *xattr_name) { + /* Policy permits modification of the protected xattrs even though + * there's no HMAC key loaded + */ + if (evm_initialized & EVM_ALLOW_METADATA_WRITES) + return 0; + return evm_protect_xattr(dentry, xattr_name, NULL, 0); } @@ -393,8 +411,8 @@ static void evm_reset_status(struct inode *inode) void evm_inode_post_setxattr(struct dentry *dentry, const char *xattr_name, const void *xattr_value, size_t xattr_value_len) { - if (!evm_initialized || (!evm_protected_xattr(xattr_name) - && !posix_xattr_acl(xattr_name))) + if (!evm_key_loaded() || (!evm_protected_xattr(xattr_name) + && !posix_xattr_acl(xattr_name))) return; evm_reset_status(dentry->d_inode); @@ -414,7 +432,7 @@ void evm_inode_post_setxattr(struct dentry *dentry, const char *xattr_name, */ void evm_inode_post_removexattr(struct dentry *dentry, const char *xattr_name) { - if (!evm_initialized || !evm_protected_xattr(xattr_name)) + if (!evm_key_loaded() || !evm_protected_xattr(xattr_name)) return; evm_reset_status(dentry->d_inode); @@ -431,6 +449,12 @@ int evm_inode_setattr(struct dentry *dentry, struct iattr *attr) unsigned int ia_valid = attr->ia_valid; enum integrity_status evm_status; + /* Policy permits modification of the protected attrs even though + * there's no HMAC key loaded + */ + if (evm_initialized & EVM_ALLOW_METADATA_WRITES) + return 0; + if (!(ia_valid & (ATTR_MODE | ATTR_UID | ATTR_GID))) return 0; evm_status = evm_verify_current_integrity(dentry); @@ -456,7 +480,7 @@ int evm_inode_setattr(struct dentry *dentry, struct iattr *attr) */ void evm_inode_post_setattr(struct dentry *dentry, int ia_valid) { - if (!evm_initialized) + if (!evm_key_loaded()) return; if (ia_valid & (ATTR_MODE | ATTR_UID | ATTR_GID)) @@ -473,7 +497,7 @@ int evm_inode_init_security(struct inode *inode, struct evm_ima_xattr_data *xattr_data; int rc; - if (!evm_initialized || !evm_protected_xattr(lsm_xattr->name)) + if (!evm_key_loaded() || !evm_protected_xattr(lsm_xattr->name)) return 0; xattr_data = kzalloc(sizeof(*xattr_data), GFP_NOFS); diff --git a/security/integrity/evm/evm_secfs.c b/security/integrity/evm/evm_secfs.c index 319cf16d6603..feba03bbedae 100644 --- a/security/integrity/evm/evm_secfs.c +++ b/security/integrity/evm/evm_secfs.c @@ -40,7 +40,7 @@ static ssize_t evm_read_key(struct file *filp, char __user *buf, if (*ppos != 0) return 0; - sprintf(temp, "%d", (evm_initialized & ~EVM_SETUP)); + sprintf(temp, "%d", (evm_initialized & ~EVM_SETUP_COMPLETE)); rc = simple_read_from_buffer(buf, count, ppos, temp, strlen(temp)); return rc; @@ -63,7 +63,7 @@ static ssize_t evm_write_key(struct file *file, const char __user *buf, { int i, ret; - if (!capable(CAP_SYS_ADMIN) || (evm_initialized & EVM_SETUP)) + if (!capable(CAP_SYS_ADMIN) || (evm_initialized & EVM_SETUP_COMPLETE)) return -EPERM; ret = kstrtoint_from_user(buf, count, 0, &i); @@ -75,16 +75,30 @@ static ssize_t evm_write_key(struct file *file, const char __user *buf, if (!i || (i & ~EVM_INIT_MASK) != 0) return -EINVAL; + /* Don't allow a request to freshly enable metadata writes if + * keys are loaded. + */ + if ((i & EVM_ALLOW_METADATA_WRITES) && + ((evm_initialized & EVM_KEY_MASK) != 0) && + !(evm_initialized & EVM_ALLOW_METADATA_WRITES)) + return -EPERM; + if (i & EVM_INIT_HMAC) { ret = evm_init_key(); if (ret != 0) return ret; /* Forbid further writes after the symmetric key is loaded */ - i |= EVM_SETUP; + i |= EVM_SETUP_COMPLETE; } evm_initialized |= i; + /* Don't allow protected metadata modification if a symmetric key + * is loaded + */ + if (evm_initialized & EVM_INIT_HMAC) + evm_initialized &= ~(EVM_ALLOW_METADATA_WRITES); + return count; } -- cgit v1.2.3