On 16-02-08 16:12:16, Mimi Zohar wrote:
> On Mon, 2016-02-08 at 10:45 +0000, Dmitry Kasatkin wrote:
>
> > > > @@ -286,9 +322,12 @@ static ssize_t ima_write_policy(struct file *file, const char __user *buf,
> > > > result = mutex_lock_interruptible(&ima_write_mutex);
> > > > if (result < 0)
> > > > goto out_free;
> > > > - result = ima_parse_add_rule(data);
> > > > - mutex_unlock(&ima_write_mutex);
> > > >
> > > > + if (data[0] == '/')
> > >
> > > >It seems that if we feed relative path to ima_policy the update will fail...
> > >
> > > Yes, i think it is always a good idea to pass absolute path.
> >
> > What if we at least emit a warning so people know what's wrong?
>
> The next patch "ima: measure and appraise the IMA policy itself" adds
> the following. Is a failure message enough?
That would be the wrong message. The above code does not handle relative paths
so any attempt to load the policy by "./ima_policy_file" or
"../../ima_policy_file" will fail.
Isn't there a kernel function that checks if given string is a path-name?
> + else if (ima_appraise & IMA_APPRAISE_POLICY) {
> + pr_err("IMA: signed policy required\n");
> + integrity_audit_msg(AUDIT_INTEGRITY_STATUS, NULL, NULL,
> + "policy_update", "signed policy
> required",
> + 1, 0);
> + if (ima_appraise & IMA_APPRAISE_ENFORCE)
> + result = -EACCES;
> + } else
> result = ima_parse_add_rule(data);
>
> >
> > Petko
> >
> > DK: May be a good idea to print that loading policy by path or not.
>
> Are we including the pathname? Are you suggesting a log or audit message?
I guess log is more appropriate.
