* [PATCH] ima: Fix return value of ima_write_policy() @ 2020-04-21 9:04 Roberto Sassu 2020-04-23 2:34 ` Mimi Zohar 0 siblings, 1 reply; 4+ messages in thread From: Roberto Sassu @ 2020-04-21 9:04 UTC (permalink / raw) To: zohar Cc: linux-integrity, linux-security-module, linux-kernel, silviu.vlasceanu, Roberto Sassu, stable Return datalen instead of zero if there is a rule to appraise the policy but that rule is not enforced. Cc: stable@vger.kernel.org Fixes: 19f8a84713edc ("ima: measure and appraise the IMA policy itself") Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com> --- security/integrity/ima/ima_fs.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/security/integrity/ima/ima_fs.c b/security/integrity/ima/ima_fs.c index a71e822a6e92..2c2ea814b954 100644 --- a/security/integrity/ima/ima_fs.c +++ b/security/integrity/ima/ima_fs.c @@ -340,6 +340,8 @@ static ssize_t ima_write_policy(struct file *file, const char __user *buf, 1, 0); if (ima_appraise & IMA_APPRAISE_ENFORCE) result = -EACCES; + else + result = datalen; } else { result = ima_parse_add_rule(data); } -- 2.17.1 ^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] ima: Fix return value of ima_write_policy() 2020-04-21 9:04 [PATCH] ima: Fix return value of ima_write_policy() Roberto Sassu @ 2020-04-23 2:34 ` Mimi Zohar 2020-04-23 9:39 ` Roberto Sassu 0 siblings, 1 reply; 4+ messages in thread From: Mimi Zohar @ 2020-04-23 2:34 UTC (permalink / raw) To: Roberto Sassu Cc: linux-integrity, linux-security-module, linux-kernel, silviu.vlasceanu, stable On Tue, 2020-04-21 at 11:04 +0200, Roberto Sassu wrote: > Return datalen instead of zero if there is a rule to appraise the policy > but that rule is not enforced. > > Cc: stable@vger.kernel.org > Fixes: 19f8a84713edc ("ima: measure and appraise the IMA policy itself") > Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com> > --- > security/integrity/ima/ima_fs.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/security/integrity/ima/ima_fs.c b/security/integrity/ima/ima_fs.c > index a71e822a6e92..2c2ea814b954 100644 > --- a/security/integrity/ima/ima_fs.c > +++ b/security/integrity/ima/ima_fs.c > @@ -340,6 +340,8 @@ static ssize_t ima_write_policy(struct file *file, const char __user *buf, > 1, 0); > if (ima_appraise & IMA_APPRAISE_ENFORCE) > result = -EACCES; > + else > + result = datalen; In all other cases, where the IMA_APPRAISE_ENFORCE is not enabled we allow the action. Here we prevent loading the policy, but don't return an error. One option, as you did, is return some indication that the policy was not loaded. Another option would be to allow loading the policy in LOG or FIX mode, but I don't think that would be productive. Perhaps differentiate between the LOG and FIX modes from the OFF mode. For the LOG and FIX modes, perhaps return -EACCES as well. For the OFF case, loading a policy with appraise rules should not be permitted. Mimi > } else { > result = ima_parse_add_rule(data); > } ^ permalink raw reply [flat|nested] 4+ messages in thread
* RE: [PATCH] ima: Fix return value of ima_write_policy() 2020-04-23 2:34 ` Mimi Zohar @ 2020-04-23 9:39 ` Roberto Sassu 2020-04-23 19:10 ` Mimi Zohar 0 siblings, 1 reply; 4+ messages in thread From: Roberto Sassu @ 2020-04-23 9:39 UTC (permalink / raw) To: Mimi Zohar Cc: linux-integrity, linux-security-module, linux-kernel, Silviu Vlasceanu, stable > -----Original Message----- > From: owner-linux-security-module@vger.kernel.org [mailto:owner-linux- > security-module@vger.kernel.org] On Behalf Of Mimi Zohar > Sent: Thursday, April 23, 2020 4:34 AM > To: Roberto Sassu <roberto.sassu@huawei.com> > Cc: linux-integrity@vger.kernel.org; linux-security-module@vger.kernel.org; > linux-kernel@vger.kernel.org; Silviu Vlasceanu > <Silviu.Vlasceanu@huawei.com>; stable@vger.kernel.org > Subject: Re: [PATCH] ima: Fix return value of ima_write_policy() > > On Tue, 2020-04-21 at 11:04 +0200, Roberto Sassu wrote: > > Return datalen instead of zero if there is a rule to appraise the policy > > but that rule is not enforced. > > > > Cc: stable@vger.kernel.org > > Fixes: 19f8a84713edc ("ima: measure and appraise the IMA policy itself") > > Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com> > > --- > > security/integrity/ima/ima_fs.c | 2 ++ > > 1 file changed, 2 insertions(+) > > > > diff --git a/security/integrity/ima/ima_fs.c > b/security/integrity/ima/ima_fs.c > > index a71e822a6e92..2c2ea814b954 100644 > > --- a/security/integrity/ima/ima_fs.c > > +++ b/security/integrity/ima/ima_fs.c > > @@ -340,6 +340,8 @@ static ssize_t ima_write_policy(struct file *file, > const char __user *buf, > > 1, 0); > > if (ima_appraise & IMA_APPRAISE_ENFORCE) > > result = -EACCES; > > + else > > + result = datalen; > > In all other cases, where the IMA_APPRAISE_ENFORCE is not enabled we > allow the action. Here we prevent loading the policy, but don't > return an error. One option, as you did, is return some indication > that the policy was not loaded. Another option would be to allow > loading the policy in LOG or FIX mode, but I don't think that would be > productive. Perhaps differentiate between the LOG and FIX modes from > the OFF mode. For the LOG and FIX modes, perhaps return -EACCES as > well. For the OFF case, loading a policy with appraise rules should > not be permitted. In LOG or FIX mode, loading a policy with absolute path will succeed. Maybe we should just keep the same behavior also when the policy is directly written to securityfs. Ok for the OFF mode, but probably this should be a separate patch. Roberto HUAWEI TECHNOLOGIES Duesseldorf GmbH, HRB 56063 Managing Director: Li Peng, Li Jian, Shi Yanli > > } else { > > result = ima_parse_add_rule(data); > > } ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] ima: Fix return value of ima_write_policy() 2020-04-23 9:39 ` Roberto Sassu @ 2020-04-23 19:10 ` Mimi Zohar 0 siblings, 0 replies; 4+ messages in thread From: Mimi Zohar @ 2020-04-23 19:10 UTC (permalink / raw) To: Roberto Sassu Cc: linux-integrity, linux-security-module, linux-kernel, Silviu Vlasceanu, stable On Thu, 2020-04-23 at 09:39 +0000, Roberto Sassu wrote: > > On Tue, 2020-04-21 at 11:04 +0200, Roberto Sassu wrote: > > > Return datalen instead of zero if there is a rule to appraise the policy > > > but that rule is not enforced. > > > > > > Cc: stable@vger.kernel.org > > > Fixes: 19f8a84713edc ("ima: measure and appraise the IMA policy itself") > > > Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com> > > > --- > > > security/integrity/ima/ima_fs.c | 2 ++ > > > 1 file changed, 2 insertions(+) > > > > > > diff --git a/security/integrity/ima/ima_fs.c > > b/security/integrity/ima/ima_fs.c > > > index a71e822a6e92..2c2ea814b954 100644 > > > --- a/security/integrity/ima/ima_fs.c > > > +++ b/security/integrity/ima/ima_fs.c > > > @@ -340,6 +340,8 @@ static ssize_t ima_write_policy(struct file *file, > > const char __user *buf, > > > 1, 0); > > > if (ima_appraise & IMA_APPRAISE_ENFORCE) > > > result = -EACCES; > > > + else > > > + result = datalen; > > > > In all other cases, where the IMA_APPRAISE_ENFORCE is not enabled we > > allow the action. Here we prevent loading the policy, but don't > > return an error. One option, as you did, is return some indication > > that the policy was not loaded. Another option would be to allow > > loading the policy in LOG or FIX mode, but I don't think that would be > > productive. Perhaps differentiate between the LOG and FIX modes from > > the OFF mode. For the LOG and FIX modes, perhaps return -EACCES as > > well. For the OFF case, loading a policy with appraise rules should > > not be permitted. > > In LOG or FIX mode, loading a policy with absolute path will succeed. Yes > Maybe we should just keep the same behavior also when the policy > is directly written to securityfs. The purpose of LOG mode is to learn about the filesystem behavior in order to label it properly. FIX mode is for re-calculating and fixing existing file hashes. If we permit directly writing the policy to securityfs, even if the existing policy requires the policy to be signed, then it is behaving differently than it would in enforcing mode. I'm ok with that, as long as the error message is emitted. > Ok for the OFF mode, but probably this should be a separate patch. Agreed Mimi > > > > } else { > > > result = ima_parse_add_rule(data); > > > } > ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2020-04-23 19:10 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:04 [PATCH] ima: Fix return value of ima_write_policy() Roberto Sassu 2020-04-23 2:34 ` Mimi Zohar 2020-04-23 9:39 ` Roberto Sassu 2020-04-23 19:10 ` Mimi Zohar
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).