> From: Mimi Zohar [mailto:zohar@xxxxxxxxxxxxx] > Sent: Tuesday, May 12, 2020 5:50 PM > On Tue, 2020-05-12 at 15:31 +0000, Roberto Sassu wrote: > > > From: owner-linux-security-module@xxxxxxxxxxxxxxx [mailto:owner- > linux- > > > security-module@xxxxxxxxxxxxxxx] On Behalf Of Mimi Zohar > > > Sent: Tuesday, May 12, 2020 4:17 PM > > > On Tue, 2020-05-12 at 07:54 +0000, Roberto Sassu wrote: > > > > > > > Roberto, EVM is only triggered by IMA, unless you've modified > the > > > > > > > kernel to do otherwise. > > > > > > > > > > > > EVM would deny xattr/attr operations even if IMA is disabled in > the > > > > > > kernel configuration. For example, evm_setxattr() returns the > value > > > > > > from evm_protect_xattr(). IMA is not involved there. > > > > > > > > > > Commit ae1ba1676b88 ("EVM: Allow userland to permit modification > of > > > > > EVM-protected metadata") > > > introduced EVM_ALLOW_METADATA_WRITES > > > > > to allow writing the EVM portable and immutable file signatures. > > > > > > > > According to Documentation/ABI/testing/evm: > > > > > > > > Note that once a key has been loaded, it will no longer be > > > > possible to enable metadata modification. > > > > > > Not any key, but the HMAC key. > > > > > > 2 Permit modification of EVM-protected metadata at > > > runtime. Not supported if HMAC validation and > > > creation is enabled. > > > > #ifdef CONFIG_EVM_LOAD_X509 > > void __init evm_load_x509(void) > > { > > [...] > > rc = integrity_load_x509(INTEGRITY_KEYRING_EVM, > CONFIG_EVM_X509_PATH); > > if (!rc) > > evm_initialized |= EVM_INIT_X509; > > > > > > static ssize_t evm_write_key(struct file *file, const char __user *buf, > > size_t count, loff_t *ppos) > > { > > [...] > > /* 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; > > > > Should have been: > > > > if ((i & EVM_ALLOW_METADATA_WRITES) && > > ((evm_initialized & EVM_INIT_HMAC) != 0) && > > !(evm_initialized & EVM_ALLOW_METADATA_WRITES)) > > return -EPERM; > > Ok > > > > > > Each time the EVM protected file metadata is updated, the EVM HMAC > is > > > updated, assuming the existing EVM HMAC is valid. Userspace should > > > not have access to the HMAC key, so we only allow writing EVM > > > signatures. > > > > > > The only difference between writing the original EVM signature and the > > > new portable and immutable signature is the security.ima xattr > > > requirement. Since the new EVM signature does not include the > > > filesystem specific data, something else needs to bind the file > > > metadata to the file data. Thus the IMA xattr requirement. > > > > > > Assuming that the new EVM signature is written last, as long as there > > > is an IMA xattr, there shouldn't be a problem writing the new EVM > > > signature. > > > > /* first need to know the sig type */ > > rc = vfs_getxattr_alloc(dentry, XATTR_NAME_EVM, (char > **)&xattr_data, 0, > > GFP_NOFS); > > if (rc <= 0) { > > evm_status = INTEGRITY_FAIL; > > if (rc == -ENODATA) { > > rc = evm_find_protected_xattrs(dentry); > > if (rc > 0) > > evm_status = INTEGRITY_NOLABEL; > > else if (rc == 0) > > evm_status = INTEGRITY_NOXATTRS; /* new file */ > > > > If EVM_ALLOW_METADATA_WRITES is cleared, only the first xattr > > can be written (status INTEGRITY_NOXATTRS is ok). After, > > evm_find_protected_xattrs() returns rc > 0, so the status is > > INTEGRITY_NOLABEL, which is not ignored by evm_protect_xattr(). > > With EVM HMAC enabled, as a result of writing the first protected > xattr, an EVM HMAC should be calculated and written in > evm_inode_post_setxattr(). To solve the ordering issue, wouldn't allowing setxattr() on a file with portable signature that does not yet pass verification be safe? evm_update_evmxattr() checks if the signature is portable and if yes, does not calculate the HMAC. Roberto HUAWEI TECHNOLOGIES Duesseldorf GmbH, HRB 56063 Managing Director: Li Peng, Li Jian, Shi Yanli
