> From: Mimi Zohar [mailto:zohar@xxxxxxxxxxxxx] > Sent: Monday, May 3, 2021 2:13 AM > Hi Roberto, > > On Wed, 2021-04-07 at 12:52 +0200, Roberto Sassu wrote: > > When a file is being created, LSMs can set the initial label with the > > inode_init_security hook. If no HMAC key is loaded, the new file will have > > LSM xattrs but not the HMAC. It is also possible that the file remains > > without protected xattrs after creation if no active LSM provided it. > > > > Unfortunately, EVM will deny any further metadata operation on new files, > > as evm_protect_xattr() will always return the INTEGRITY_NOLABEL error, or > > INTEGRITY_NOXATTRS if no protected xattrs exist. This would limit the > > usability of EVM when only a public key is loaded, as commands such as cp > > or tar with the option to preserve xattrs won't work. > > > > This patch ignores these errors when they won't be an issue, if no HMAC > key > > is loaded and cannot be loaded in the future (which can be enforced by > > setting the EVM_SETUP_COMPLETE initialization flag). > > > > Signed-off-by: Roberto Sassu <roberto.sassu@xxxxxxxxxx> > > --- > > security/integrity/evm/evm_main.c | 23 ++++++++++++++++++++++- > > 1 file changed, 22 insertions(+), 1 deletion(-) > > > > diff --git a/security/integrity/evm/evm_main.c > b/security/integrity/evm/evm_main.c > > index 998818283fda..6556e8c22da9 100644 > > --- a/security/integrity/evm/evm_main.c > > +++ b/security/integrity/evm/evm_main.c > > @@ -90,6 +90,24 @@ static bool evm_key_loaded(void) > > return (bool)(evm_initialized & EVM_KEY_MASK); > > } > > > > +/* > > + * Ignoring INTEGRITY_NOLABEL/INTEGRITY_NOXATTRS is safe if no HMAC > key > > + * is loaded and the EVM_SETUP_COMPLETE initialization flag is set. > > + */ > > +static bool evm_ignore_error_safe(enum integrity_status evm_status) > > +{ > > + if (evm_initialized & EVM_INIT_HMAC) > > + return false; > > + > > + if (!(evm_initialized & EVM_SETUP_COMPLETE)) > > + return false; > > + > > + if (evm_status != INTEGRITY_NOLABEL && evm_status != > INTEGRITY_NOXATTRS) > > + return false; > > + > > + return true; > > +} > > + > > static int evm_find_protected_xattrs(struct dentry *dentry) > > { > > struct inode *inode = d_backing_inode(dentry); > > @@ -354,6 +372,8 @@ static int evm_protect_xattr(struct dentry *dentry, > const char *xattr_name, > > -EPERM, 0); > > } > > out: > > + if (evm_ignore_error_safe(evm_status)) > > + return 0; > > I agree with the concept, but the function name doesn't provide enough > context. Perhaps defining a function more along the lines of > "evm_hmac_disabled()" would be more appropriate and at the same time > self documenting. Since the function checks if the passed error can be ignored, would evm_ignore_error_hmac_disabled() also be ok? > > if (evm_status != INTEGRITY_PASS) > > integrity_audit_msg(AUDIT_INTEGRITY_METADATA, > d_backing_inode(dentry), > > dentry->d_name.name, > "appraise_metadata", > > @@ -515,7 +535,8 @@ int evm_inode_setattr(struct dentry *dentry, struct > iattr *attr) > > return 0; > > evm_status = evm_verify_current_integrity(dentry); > > if ((evm_status == INTEGRITY_PASS) || > > - (evm_status == INTEGRITY_NOXATTRS)) > > + (evm_status == INTEGRITY_NOXATTRS) || > > + (evm_ignore_error_safe(evm_status))) > > It would also remove the INTEGRITY_NOXATTRS test duplication here. Ok. Thanks Roberto HUAWEI TECHNOLOGIES Duesseldorf GmbH, HRB 56063 Managing Director: Li Peng, Li Jian, Shi Yanli > thanks, > > Mimi > > > return 0; > > integrity_audit_msg(AUDIT_INTEGRITY_METADATA, > d_backing_inode(dentry), > > dentry->d_name.name, "appraise_metadata",
