> From: Mimi Zohar [mailto:zohar@xxxxxxxxxxxxx] > Sent: Tuesday, May 11, 2021 3:42 PM > On Wed, 2021-05-05 at 13:29 +0200, Roberto Sassu wrote: > > EVM_ALLOW_METADATA_WRITES is an EVM initialization flag that can be > set to > > temporarily disable metadata verification until all xattrs/attrs necessary > > to verify an EVM portable signature are copied to the file. This flag is > > cleared when EVM is initialized with an HMAC key, to avoid that the HMAC is > > calculated on unverified xattrs/attrs. > > > > Currently EVM unnecessarily denies setting this flag if EVM is initialized > > with a public key, which is not a concern as it cannot be used to trust > > xattrs/attrs updates. This patch removes this limitation. > > > > Cc: stable@xxxxxxxxxxxxxxx # 4.16.x > > Fixes: ae1ba1676b88e ("EVM: Allow userland to permit modification of EVM- > protected metadata") > > Signed-off-by: Roberto Sassu <roberto.sassu@xxxxxxxxxx> > > Once the comments below are addressed, > > Reviewed-by: Mimi Zohar <zohar@xxxxxxxxxxxxx> > > > --- > > Documentation/ABI/testing/evm | 5 +++-- > > security/integrity/evm/evm_secfs.c | 5 ++--- > > 2 files changed, 5 insertions(+), 5 deletions(-) > > > > diff --git a/Documentation/ABI/testing/evm > b/Documentation/ABI/testing/evm > > index 3c477ba48a31..eb6d70fd6fa2 100644 > > --- a/Documentation/ABI/testing/evm > > +++ b/Documentation/ABI/testing/evm > > @@ -49,8 +49,9 @@ Description: > > modification of EVM-protected metadata and > > disable all further modification of policy > > > > - Note that once a key has been loaded, it will no longer be > > - possible to enable metadata modification. > > + Note that once an HMAC key has been loaded, it will no longer > > + be possible to enable metadata modification and, if it is > > + already enabled, it will be disabled. > > It's worth mentioning that echo'ing a new value is additive. Once EVM > metadata modification is enabled, the only way of disabling it is by > enabling an HMAC key. It's also worth mentioning that metadata writes > are only permitted once further changes to the EVM policy are disabled. If I'm not wrong, it is not required to set EVM_SETUP_COMPLETE to allow metadata writes. I think the original idea was to boot a system in a way that portable signatures can be written, and then to enable enforcement. Roberto HUAWEI TECHNOLOGIES Duesseldorf GmbH, HRB 56063 Managing Director: Li Peng, Li Jian, Shi Yanli > Perhaps the best way of explaining this is by including a new example - > echo 6> <securityfs>/evm. > > > > > Until key loading has been signaled EVM can not create > > or validate the 'security.evm' xattr, but returns > > diff --git a/security/integrity/evm/evm_secfs.c > b/security/integrity/evm/evm_secfs.c > > index bbc85637e18b..860c48b9a0c3 100644 > > --- a/security/integrity/evm/evm_secfs.c > > +++ b/security/integrity/evm/evm_secfs.c > > @@ -81,11 +81,10 @@ static ssize_t evm_write_key(struct file *file, const > char __user *buf, > > return -EINVAL; > > > > /* Don't allow a request to freshly enable metadata writes if > > - * keys are loaded. > > + * an HMAC key is loaded. > > */ > > Please drop the word "freshly". While updating the comment, please > move the sentence starting with "Don't" to a new line. > > > if ((i & EVM_ALLOW_METADATA_WRITES) && > > - ((evm_initialized & EVM_KEY_MASK) != 0) && > > - !(evm_initialized & EVM_ALLOW_METADATA_WRITES)) > > + (evm_initialized & EVM_INIT_HMAC) != 0) > > return -EPERM; > > > > if (i & EVM_INIT_HMAC) { >
