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: 24+ 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-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-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 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).