From: Roberto Sassu <roberto.sassu@huawei.com>
To: Mimi Zohar <zohar@linux.ibm.com>, "mjg59@google.com" <mjg59@google.com>
Cc: "linux-integrity@vger.kernel.org"
<linux-integrity@vger.kernel.org>,
"linux-security-module@vger.kernel.org"
<linux-security-module@vger.kernel.org>,
"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
"stable@vger.kernel.org" <stable@vger.kernel.org>
Subject: RE: [PATCH v6 03/11] evm: Refuse EVM_ALLOW_METADATA_WRITES only if an HMAC key is loaded
Date: Tue, 11 May 2021 14:12:06 +0000 [thread overview]
Message-ID: <1f0530bc9b974951ae0bb1e2beb02422@huawei.com> (raw)
In-Reply-To: <6f5603489b16918de5d3cbb73c1a7c0e835f0671.camel@linux.ibm.com>
> From: Mimi Zohar [mailto:zohar@linux.ibm.com]
> 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@vger.kernel.org # 4.16.x
> > Fixes: ae1ba1676b88e ("EVM: Allow userland to permit modification of EVM-
> protected metadata")
> > Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>
>
> Once the comments below are addressed,
>
> Reviewed-by: Mimi Zohar <zohar@linux.ibm.com>
>
> > ---
> > 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) {
>
next prev parent reply other threads:[~2021-05-11 14:12 UTC|newest]
Thread overview: 25+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-05-05 11:29 [PATCH v6 00/11] evm: Improve usability of portable signatures Roberto Sassu
2021-05-05 11:29 ` [PATCH v6 01/11] evm: Execute evm_inode_init_security() only when an HMAC key is loaded Roberto Sassu
2021-05-05 11:29 ` [PATCH v6 02/11] evm: Load EVM key in ima_load_x509() to avoid appraisal Roberto Sassu
2021-05-05 11:29 ` [PATCH v6 03/11] evm: Refuse EVM_ALLOW_METADATA_WRITES only if an HMAC key is loaded Roberto Sassu
2021-05-11 13:41 ` Mimi Zohar
2021-05-11 14:12 ` Roberto Sassu [this message]
2021-05-11 19:53 ` Mimi Zohar
2021-05-05 11:29 ` [PATCH v6 04/11] evm: Introduce evm_status_revalidate() Roberto Sassu
2021-05-11 13:42 ` Mimi Zohar
2021-05-05 11:33 ` [PATCH v6 05/11] evm: Introduce evm_hmac_disabled() to safely ignore verification errors Roberto Sassu
2021-05-07 13:31 ` [RESEND][PATCH " Roberto Sassu
2021-05-11 13:42 ` Mimi Zohar
2021-05-05 11:33 ` [PATCH v6 06/11] evm: Allow xattr/attr operations for portable signatures Roberto Sassu
2021-05-05 11:33 ` [PATCH v6 07/11] evm: Pass user namespace to set/remove xattr hooks Roberto Sassu
2021-05-05 11:33 ` [PATCH v6 08/11] evm: Allow setxattr() and setattr() for unmodified metadata Roberto Sassu
2021-05-11 14:11 ` Mimi Zohar
2021-05-11 14:21 ` Roberto Sassu
2021-05-11 14:40 ` Mimi Zohar
2021-05-11 14:54 ` Roberto Sassu
2021-05-11 20:00 ` Mimi Zohar
2021-05-05 11:33 ` [PATCH v6 09/11] ima: Allow imasig requirement to be satisfied by EVM portable signatures Roberto Sassu
2021-05-05 11:33 ` [PATCH v6 10/11] ima: Introduce template field evmsig and write to field sig as fallback Roberto Sassu
2021-05-11 22:12 ` Mimi Zohar
2021-05-12 10:55 ` Roberto Sassu
2021-05-05 11:33 ` [PATCH v6 11/11] ima: Don't remove security.ima if file must not be appraised Roberto Sassu
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=1f0530bc9b974951ae0bb1e2beb02422@huawei.com \
--to=roberto.sassu@huawei.com \
--cc=linux-integrity@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-security-module@vger.kernel.org \
--cc=mjg59@google.com \
--cc=stable@vger.kernel.org \
--cc=zohar@linux.ibm.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).