From: Mimi Zohar <zohar@linux.ibm.com>
To: Roberto Sassu <roberto.sassu@huawei.com>,
"mjg59@google.com" <mjg59@google.com>,
John Johansen <john.johansen@canonical.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>,
Silviu Vlasceanu <Silviu.Vlasceanu@huawei.com>,
"stable@vger.kernel.org" <stable@vger.kernel.org>
Subject: Re: [PATCH v2 07/12] evm: Introduce EVM_RESET_STATUS atomic flag
Date: Thu, 17 Sep 2020 13:47:52 -0400 [thread overview]
Message-ID: <a19576f23e7f9d3e7c546672d1335b324bf9ca9f.camel@linux.ibm.com> (raw)
In-Reply-To: <581966c47e94412ab3fd5b2ca9aacd3d@huawei.com>
On Thu, 2020-09-17 at 17:36 +0000, Roberto Sassu wrote:
> > > diff --git a/security/integrity/evm/evm_main.c
> > b/security/integrity/evm/evm_main.c
> > > index 4e9f5e8b21d5..05be1ad3e6f3 100644
> > > --- a/security/integrity/evm/evm_main.c
> > > +++ b/security/integrity/evm/evm_main.c
> > > @@ -221,8 +221,15 @@ static enum integrity_status
> > evm_verify_hmac(struct dentry *dentry,
> > > evm_status = (rc == -ENODATA) ?
> > > INTEGRITY_NOXATTRS : INTEGRITY_FAIL;
> > > out:
> > > - if (iint)
> > > + if (iint) {
> > > + /*
> > > + * EVM_RESET_STATUS can be cleared only by
> > evm_verifyxattr()
> > > + * when EVM_ALLOW_METADATA_WRITES is set. This
> > guarantees that
> > > + * IMA sees the EVM_RESET_STATUS flag set before it is
> > cleared.
> > > + */
> > > + clear_bit(EVM_RESET_STATUS, &iint->atomic_flags);
> > > iint->evm_status = evm_status;
> >
> > True IMA is currently the only caller of evm_verifyxattr() in the
> > upstreamed kernel, but it is an exported function, which may be called
> > from elsewhere. The previous version crossed the boundary between EVM
> > & IMA with EVM modifying the IMA flag directly. This version assumes
> > that IMA will be the only caller. Otherwise, I like this version.
>
> Ok, I think it is better, as you suggested, to export a new EVM function
> that tells if evm_reset_status() will be executed in the EVM post hooks, and
> to call this function from IMA. IMA would then call ima_reset_appraise_flags()
> also depending on the result of the new EVM function.
>
> ima_reset_appraise_flags() should be called in a post hook in IMA.
> Should I introduce it?
Yes, so any callers of evm_verifyxattr() will need to implement the
post hook as well. As much as possible, please limit code duplication.
The last time I looked, there didn't seem to be a locking concern, but
please make sure.
thanks,
Mimi
next prev parent reply other threads:[~2020-09-17 17:49 UTC|newest]
Thread overview: 26+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-09-04 9:23 [PATCH v2 00/12] IMA/EVM fixes Roberto Sassu
2020-09-04 9:23 ` [PATCH v2 01/12] ima: Don't ignore errors from crypto_shash_update() Roberto Sassu
2020-09-07 15:03 ` Sasha Levin
2020-09-04 9:23 ` [PATCH v2 02/12] ima: Remove semicolon at the end of ima_get_binary_runtime_size() Roberto Sassu
2020-09-04 9:23 ` [PATCH v2 03/12] evm: Check size of security.evm before using it Roberto Sassu
2020-09-04 9:23 ` [PATCH v2 04/12] evm: Execute evm_inode_init_security() only when the HMAC key is loaded Roberto Sassu
2020-09-07 15:03 ` Sasha Levin
2020-09-16 16:15 ` Mimi Zohar
2020-09-04 9:26 ` [PATCH v2 05/12] evm: Load EVM key in ima_load_x509() to avoid appraisal Roberto Sassu
2020-09-04 9:26 ` [PATCH v2 06/12] evm: Refuse EVM_ALLOW_METADATA_WRITES only if the HMAC key is loaded Roberto Sassu
2020-09-04 9:26 ` [PATCH v2 07/12] evm: Introduce EVM_RESET_STATUS atomic flag Roberto Sassu
2020-09-17 12:01 ` Mimi Zohar
2020-09-17 17:36 ` Roberto Sassu
2020-09-17 17:47 ` Mimi Zohar [this message]
2020-09-04 9:26 ` [PATCH v2 08/12] evm: Allow xattr/attr operations for portable signatures if check fails Roberto Sassu
2020-09-17 12:32 ` Mimi Zohar
2020-09-04 9:26 ` [PATCH v2 09/12] evm: Allow setxattr() and setattr() if metadata digest won't change Roberto Sassu
2020-09-17 13:15 ` Mimi Zohar
2020-09-04 9:26 ` [PATCH v2 10/12] ima: Allow imasig requirement to be satisfied by EVM portable signatures Roberto Sassu
2020-09-04 9:26 ` [PATCH v2 11/12] ima: Introduce template field evmsig and write to field sig as fallback Roberto Sassu
2020-09-17 14:25 ` Mimi Zohar
2020-09-17 15:05 ` Roberto Sassu
2020-09-17 15:55 ` Mimi Zohar
2020-09-04 9:26 ` [PATCH v2 12/12] ima: Don't remove security.ima if file must not be appraised Roberto Sassu
2020-09-16 16:14 ` [PATCH v2 00/12] IMA/EVM fixes Mimi Zohar
2020-09-17 14:33 ` Mimi Zohar
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=a19576f23e7f9d3e7c546672d1335b324bf9ca9f.camel@linux.ibm.com \
--to=zohar@linux.ibm.com \
--cc=Silviu.Vlasceanu@huawei.com \
--cc=john.johansen@canonical.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=roberto.sassu@huawei.com \
--cc=stable@vger.kernel.org \
/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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.