> -----Original Message----- > From: Mimi Zohar [mailto:zohar@xxxxxxxxxxxxx] > Sent: Thursday, April 23, 2020 10:52 PM > To: Roberto Sassu <roberto.sassu@xxxxxxxxxx>; mjg59@xxxxxxxxxx > Cc: linux-integrity@xxxxxxxxxxxxxxx; linux-security-module@xxxxxxxxxxxxxxx; > linux-kernel@xxxxxxxxxxxxxxx; Silviu Vlasceanu > <Silviu.Vlasceanu@xxxxxxxxxx> > Subject: Re: [PATCH] ima: Allow imasig requirement to be satisfied by EVM > portable signatures > > On Tue, 2020-04-21 at 11:24 +0200, Roberto Sassu wrote: > > System administrators can require that all accessed files have a signature > > by specifying appraise_type=imasig in a policy rule. > > > > Currently, only IMA signatures satisfy this requirement. However, also > EVM > > portable signatures can satisfy it. Metadata, including security.ima, are > > signed and cannot change. > > Please expand this paragraph with a short comparison of the security > guarantees provided by EVM immutable, portable signatures versus ima- > sig. > > > > > This patch helps in the scenarios where system administrators want to > > enforce this restriction but only EVM portable signatures are available. > > Yes, I agree it "helps", but we still need to address the ability of > setting/removing security.ima, which isn't possible with an IMA > signature. This sounds like we need to define an immutable file hash. I didn't understand. Can you explain better? Thanks Roberto HUAWEI TECHNOLOGIES Duesseldorf GmbH, HRB 56063 Managing Director: Li Peng, Li Jian, Shi Yanli > What do you think? > > > The patch makes the following changes: > > > > file xattr types: > > security.ima: IMA_XATTR_DIGEST/IMA_XATTR_DIGEST_NG > > security.evm: EVM_XATTR_PORTABLE_DIGSIG > > > > execve(), mmap(), open() behavior (with appraise_type=imasig): > > before: denied (file without IMA signature, imasig requirement not met) > > after: allowed (file with EVM portable signature, imasig requirement met) > > > > open(O_WRONLY) behavior (without appraise_type=imasig): > > before: allowed (file without IMA signature, not immutable) > > after: denied (file with EVM portable signature, immutable) > > > > Signed-off-by: Roberto Sassu <roberto.sassu@xxxxxxxxxx> > > --- > > security/integrity/ima/ima_appraise.c | 14 +++++++++----- > > 1 file changed, 9 insertions(+), 5 deletions(-) > > > > diff --git a/security/integrity/ima/ima_appraise.c > b/security/integrity/ima/ima_appraise.c > > index a9649b04b9f1..69a6a958f811 100644 > > --- a/security/integrity/ima/ima_appraise.c > > +++ b/security/integrity/ima/ima_appraise.c > > @@ -219,12 +219,16 @@ static int xattr_verify(enum ima_hooks func, > struct integrity_iint_cache *iint, > > hash_start = 1; > > /* fall through */ > > case IMA_XATTR_DIGEST: > > - if (iint->flags & IMA_DIGSIG_REQUIRED) { > > - *cause = "IMA-signature-required"; > > - *status = INTEGRITY_FAIL; > > - break; > > + if (*status != INTEGRITY_PASS_IMMUTABLE) { > > + if (iint->flags & IMA_DIGSIG_REQUIRED) { > > + *cause = "IMA-signature-required"; > > + *status = INTEGRITY_FAIL; > > + break; > > + } > > + clear_bit(IMA_DIGSIG, &iint->atomic_flags); > > + } else { > > + set_bit(IMA_DIGSIG, &iint->atomic_flags); > > } > > - clear_bit(IMA_DIGSIG, &iint->atomic_flags); > > if (xattr_len - sizeof(xattr_value->type) - hash_start >= > > iint->ima_hash->length) > > /* > > Nice! > > Mimi
