2019-12-20 19:04:15

by Mimi Zohar

[permalink] [raw]
Subject: Re: [PATCH v5 0/2] IMA: Deferred measurement of keys

On Wed, 2019-12-18 at 08:44 -0800, Lakshmi Ramasubramanian wrote:
> This patchset extends the previous version[1] by adding support for
> deferred processing of keys.
>
> With the patchset referenced above, the IMA subsystem supports
> measuring asymmetric keys when the key is created or updated.
> But keys created or updated before a custom IMA policy is loaded
> are currently not measured. This includes keys added to, for instance,
> .builtin_trusted_keys which happens early in the boot process.
>
> This change adds support for queuing keys created or updated before
> a custom IMA policy is loaded. The queued keys are processed when
> a custom policy is loaded. Keys created or updated after a custom policy
> is loaded are measured immediately (not queued).
>
> If the kernel is built with both CONFIG_IMA and
> CONFIG_ASYMMETRIC_PUBLIC_KEY_SUBTYPE enabled then the IMA policy
> must be applied as a custom policy. Not providing a custom policy
> in the above configuration would result in asymmeteric keys being queued
> until a custom policy is loaded. This is by design.

I didn't notice the "This is by design" here, referring to the memory
never being freed.  "This is by design" was suppose to refer to
requiring a custom policy for measuring keys.

For now, these two patches are queued in the next-integrity-testing
branch, but I would appreciate your addressing not freeing the memory
associated with the keys, if a custom policy is not loaded.

Please note that I truncated the 2/2 patch description, as it repeats
the existing verification example in commit ("2b60c0ecedf8 IMA: Read
keyrings= option from the IMA policy").

thanks,

Mimi


2019-12-20 19:28:48

by Lakshmi Ramasubramanian

[permalink] [raw]
Subject: Re: [PATCH v5 0/2] IMA: Deferred measurement of keys

On 12/20/2019 11:01 AM, Mimi Zohar wrote:

Hi Mimi,

>> If the kernel is built with both CONFIG_IMA and
>> CONFIG_ASYMMETRIC_PUBLIC_KEY_SUBTYPE enabled then the IMA policy
>> must be applied as a custom policy. Not providing a custom policy
>> in the above configuration would result in asymmeteric keys being queued
>> until a custom policy is loaded. This is by design.
>
> I didn't notice the "This is by design" here, referring to the memory
> never being freed.  "This is by design" was suppose to refer to
> requiring a custom policy for measuring keys.
>
> For now, these two patches are queued in the next-integrity-testing
> branch, but I would appreciate your addressing not freeing the memory
> associated with the keys, if a custom policy is not loaded.
>
> Please note that I truncated the 2/2 patch description, as it repeats
> the existing verification example in commit ("2b60c0ecedf8 IMA: Read
> keyrings= option from the IMA policy").
>
> thanks,
>
> Mimi
>

Sure - I am fine with truncating the 2/2 patch description. Thanks for
doing that.

Regarding "Freeing the queued keys if custom policy is not loaded":

Shall I create a new patch set to address that and have that be reviewed
independent of this patch set?

Like you'd suggested earlier, we can wait for a certain time, after IMA
is initialized, and free the queue if a custom policy was not loaded.

Please let me know.

thanks,
-lakshmi


2019-12-20 19:38:54

by Mimi Zohar

[permalink] [raw]
Subject: Re: [PATCH v5 0/2] IMA: Deferred measurement of keys

On Fri, 2019-12-20 at 11:25 -0800, Lakshmi Ramasubramanian wrote:
> On 12/20/2019 11:01 AM, Mimi Zohar wrote:
>
> Hi Mimi,
>
> >> If the kernel is built with both CONFIG_IMA and
> >> CONFIG_ASYMMETRIC_PUBLIC_KEY_SUBTYPE enabled then the IMA policy
> >> must be applied as a custom policy. Not providing a custom policy
> >> in the above configuration would result in asymmeteric keys being queued
> >> until a custom policy is loaded. This is by design.
> >
> > I didn't notice the "This is by design" here, referring to the memory
> > never being freed.  "This is by design" was suppose to refer to
> > requiring a custom policy for measuring keys.
> >
> > For now, these two patches are queued in the next-integrity-testing
> > branch, but I would appreciate your addressing not freeing the memory
> > associated with the keys, if a custom policy is not loaded.
> >
> > Please note that I truncated the 2/2 patch description, as it repeats
> > the existing verification example in commit ("2b60c0ecedf8 IMA: Read
> > keyrings= option from the IMA policy").
> >
> > thanks,
> >
> > Mimi
> >
>
> Sure - I am fine with truncating the 2/2 patch description. Thanks for
> doing that.
>
> Regarding "Freeing the queued keys if custom policy is not loaded":
>
> Shall I create a new patch set to address that and have that be reviewed
> independent of this patch set?

If it is just a single additional patch, feel free to post it without
a cover letter.

>
> Like you'd suggested earlier, we can wait for a certain time, after IMA
> is initialized, and free the queue if a custom policy was not loaded.

Different types of systems vary in boot time, but perhaps a certain
amount of time after IMA is initialized would be consistent.  This
would need to work for IoT devices/sensors to servers.

Mimi 

2019-12-20 20:51:23

by Lakshmi Ramasubramanian

[permalink] [raw]
Subject: Re: [PATCH v5 0/2] IMA: Deferred measurement of keys

On 12/20/19 11:36 AM, Mimi Zohar wrote:

>>
>> Shall I create a new patch set to address that and have that be reviewed
>> independent of this patch set?
>
> If it is just a single additional patch, feel free to post it without
> a cover letter.

Sure

>>
>> Like you'd suggested earlier, we can wait for a certain time, after IMA
>> is initialized, and free the queue if a custom policy was not loaded.
>
> Different types of systems vary in boot time, but perhaps a certain
> amount of time after IMA is initialized would be consistent.  This
> would need to work for IoT devices/sensors to servers.
>
> Mimi
>

Yes - I agree.

thanks,
-lakshmi