* [PATCH] ima: Allow imasig requirement to be satisfied by EVM portable signatures @ 2020-04-21 9:24 Roberto Sassu 2020-04-23 20:51 ` Mimi Zohar 0 siblings, 1 reply; 4+ messages in thread From: Roberto Sassu @ 2020-04-21 9:24 UTC (permalink / raw) To: zohar, mjg59 Cc: linux-integrity, linux-security-module, linux-kernel, silviu.vlasceanu, Roberto Sassu 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. This patch helps in the scenarios where system administrators want to enforce this restriction but only EVM portable signatures are available. 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@huawei.com> --- 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) /* -- 2.17.1 ^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] ima: Allow imasig requirement to be satisfied by EVM portable signatures 2020-04-21 9:24 [PATCH] ima: Allow imasig requirement to be satisfied by EVM portable signatures Roberto Sassu @ 2020-04-23 20:51 ` Mimi Zohar 2020-04-24 10:39 ` Roberto Sassu 0 siblings, 1 reply; 4+ messages in thread From: Mimi Zohar @ 2020-04-23 20:51 UTC (permalink / raw) To: Roberto Sassu, mjg59 Cc: linux-integrity, linux-security-module, linux-kernel, silviu.vlasceanu 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. 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@huawei.com> > --- > 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 ^ permalink raw reply [flat|nested] 4+ messages in thread
* RE: [PATCH] ima: Allow imasig requirement to be satisfied by EVM portable signatures 2020-04-23 20:51 ` Mimi Zohar @ 2020-04-24 10:39 ` Roberto Sassu 2020-05-07 10:21 ` Roberto Sassu 0 siblings, 1 reply; 4+ messages in thread From: Roberto Sassu @ 2020-04-24 10:39 UTC (permalink / raw) To: Mimi Zohar, mjg59 Cc: linux-integrity, linux-security-module, linux-kernel, Silviu Vlasceanu > -----Original Message----- > From: Mimi Zohar [mailto:zohar@linux.ibm.com] > Sent: Thursday, April 23, 2020 10:52 PM > To: Roberto Sassu <roberto.sassu@huawei.com>; mjg59@google.com > Cc: linux-integrity@vger.kernel.org; linux-security-module@vger.kernel.org; > linux-kernel@vger.kernel.org; Silviu Vlasceanu > <Silviu.Vlasceanu@huawei.com> > 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@huawei.com> > > --- > > 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 ^ permalink raw reply [flat|nested] 4+ messages in thread
* RE: [PATCH] ima: Allow imasig requirement to be satisfied by EVM portable signatures 2020-04-24 10:39 ` Roberto Sassu @ 2020-05-07 10:21 ` Roberto Sassu 0 siblings, 0 replies; 4+ messages in thread From: Roberto Sassu @ 2020-05-07 10:21 UTC (permalink / raw) To: Roberto Sassu, Mimi Zohar, mjg59 Cc: linux-integrity, linux-security-module, linux-kernel, Silviu Vlasceanu > -----Original Message----- > From: linux-integrity-owner@vger.kernel.org [mailto:linux-integrity- > owner@vger.kernel.org] On Behalf Of Roberto Sassu > Sent: Friday, April 24, 2020 12:40 PM > To: Mimi Zohar <zohar@linux.ibm.com>; mjg59@google.com > Cc: linux-integrity@vger.kernel.org; linux-security-module@vger.kernel.org; > linux-kernel@vger.kernel.org; Silviu Vlasceanu > <Silviu.Vlasceanu@huawei.com> > Subject: RE: [PATCH] ima: Allow imasig requirement to be satisfied by EVM > portable signatures > > > -----Original Message----- > > From: Mimi Zohar [mailto:zohar@linux.ibm.com] > > Sent: Thursday, April 23, 2020 10:52 PM > > To: Roberto Sassu <roberto.sassu@huawei.com>; mjg59@google.com > > Cc: linux-integrity@vger.kernel.org; linux-security- > module@vger.kernel.org; > > linux-kernel@vger.kernel.org; Silviu Vlasceanu > > <Silviu.Vlasceanu@huawei.com> > > 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? Ok, got it. I wouldn't grant access to new file depending on the security.ima type but depending on the IMA_DIGSIG bit. In both cases, IMA signature and EVM portable signature, the bit is set. There is one remaining issue. Maybe the signature is portable, but you don't get it from evm_verifyxattr() if verification fails. There is a legitimate case when it happens, which is when you extract a file with a portable signature with tar, and the inode uid/gid are not yet correct (fchown() is called later after the open()). In this case, IMA_DIGSIG is not set and the open() fails. To avoid this issue I would introduce the new status INTEGRITY_FAIL_IMMUTABLE, so that IMA_DIGSIG is set even if the verification of the portable signature fails. Roberto HUAWEI TECHNOLOGIES Duesseldorf GmbH, HRB 56063 Managing Director: Li Peng, Li Jian, Shi Yanli > 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@huawei.com> > > > --- > > > 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 ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2020-05-07 10:21 UTC | newest] Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2020-04-21 9:24 [PATCH] ima: Allow imasig requirement to be satisfied by EVM portable signatures Roberto Sassu 2020-04-23 20:51 ` Mimi Zohar 2020-04-24 10:39 ` Roberto Sassu 2020-05-07 10:21 ` Roberto Sassu
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).