Re: [PATCH 2/2] ima: Support appraise_type=imasig_optional

[Kasatkin, Dmitry]

On Wed, Feb 13, 2013 at 3:44 PM, Mimi Zohar <zohar@xxxxxxxxxxxxxxxxxx> wrote:
> On Wed, 2013-02-13 at 15:13 +0200, Kasatkin, Dmitry wrote:
>> On Wed, Feb 13, 2013 at 2:56 PM, Mimi Zohar <zohar@xxxxxxxxxxxxxxxxxx> wrote:
>> > On Wed, 2013-02-13 at 14:31 +0200, Kasatkin, Dmitry wrote:
>> >> On Mon, Feb 11, 2013 at 10:11 PM, Vivek Goyal <vgoyal@xxxxxxxxxx> wrote:
>> >
>> >> > @@ -158,7 +165,8 @@ int ima_appraise_measurement(int func, struct integrity_iint_cache *iint,
>> >> >         }
>> >> >         switch (xattr_value->type) {
>> >> >         case IMA_XATTR_DIGEST:
>> >> > -               if (iint->flags & IMA_DIGSIG_REQUIRED) {
>> >> > +               if (iint->flags & IMA_DIGSIG_REQUIRED ||
>> >> > +                   iint->flags & IMA_DIGSIG_OPTIONAL) {
>> >> >                         cause = "IMA signature required";
>> >> >                         status = INTEGRITY_FAIL;
>> >> >                         break;
>> >>
>> >> This looks a bit odd... If "optional" signature is missing  - we fail..
>> >> It is optional... Why we should fail?
>> >
>> > 'imasig_optional' does not only mean that the signature is optional, but
>> > also implies that it has to be a digital signature, not a hash.  This
>> > latter part is what IMA_DIGSIG_REQUIRED means.
>> >
>> > If 'imasig_optional' set both 'IMA_DIGSIG_OPTIONAL' and
>> > 'IMA_DIGSIG_REQUIRED', then this change wouldn't be necessary.
>> >
>> > IMA_DIGSIG_REQUIRED would enforce that it is a signature.
>> > IMA_DIGSIG_OPTIONAL would fail only for files with invalid signatures.
>> >
>>
>> It sounds odd that optional signature is actually required.
>> Optional for me means that it may be there or may be not.
>> If it is not there, then it may be hash or nothing.
>>
>> I see it is more logical if it is "appraise_type=optional",
>> which means that we might have no xattr value, hash or signature.
>> It if happens to be a signature, then IMA_DIGSIG flag will be set.
>
> Right, 'appraise_type=' could have been defined either as a comma
> separated list of options (eg. appraise_type=imassig,optional) or, as
> Vivek implemented, a new option.  Eventually, we will need to extend
> 'appraise_type=' (eg. required algorithm), but for now I don't have a
> problem with the new option.
>

It is not exactly what I meant. IOW, I do not want
appraise_type=imasig,optional.

Optional for me is that xattr value is optional. It might be nothing,
hash or imasig...

If it would happen that it contains signature, then IMA_DIGSIG flag
would be set,
and process could get needed capability as Vivek wants.

- Dmitry


> thanks,
>
> Mimi
>
>> I asked Vivek to send a policy file he uses in his system.
>> I would like to see how system configured as a whole...
>>
>> - Dmitry
>
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/