Linux Integrity Measurement development
 help / color / mirror / Atom feed
* Re: [PATCH v15 00/11] LSM: Three basic syscalls
       [not found]             ` <CAHC9VhTjHT-DGKu0=cZPVb=+kMwmbPdr8HiVWJq-yzaDiYk_SA@mail.gmail.com>
@ 2023-10-17 16:07               ` Roberto Sassu
  2023-10-18  9:31                 ` Roberto Sassu
  0 siblings, 1 reply; 10+ messages in thread
From: Roberto Sassu @ 2023-10-17 16:07 UTC (permalink / raw)
  To: Paul Moore, Mimi Zohar
  Cc: Casey Schaufler, linux-security-module, jmorris, serge, keescook,
	john.johansen, penguin-kernel, stephen.smalley.work, linux-kernel,
	linux-api, mic, linux-integrity

On Tue, 2023-10-17 at 11:58 -0400, Paul Moore wrote:
> On Tue, Oct 17, 2023 at 3:01 AM Roberto Sassu
> <roberto.sassu@huaweicloud.com> wrote:
> > On Mon, 2023-10-16 at 11:06 -0400, Paul Moore wrote:
> > > On Mon, Oct 16, 2023 at 8:05 AM Roberto Sassu
> > > <roberto.sassu@huaweicloud.com> wrote:
> > > > 
> > > > Sorry, I just noticed LSM_ID_IMA. Since we have the 'integrity' LSM, I
> > > > think it should be LSM_ID_INTEGRITY.
> > > > 
> > > > Mimi, all, do you agree? If yes, I send a patch shortly.
> > > 
> > > I believe LSM_ID_IMA is the better option, despite "integrity" already
> > > being present in Kconfig and possibly other areas.  "IMA" is a
> > > specific thing/LSM whereas "integrity" is a property, principle, or
> > > quality.  Especially as we move forward with promoting IMA as a full
> > > and proper LSM, we should work towards referring to it as "IMA" and
> > > not "integrity".
> > > 
> > > If anything we should be working to support "IMA" in places where we
> > > currently have "integrity" so that we can eventually deprecate
> > > "integrity".
> > 
> > Hi Paul
> > 
> > I fully understand your argument. However, 'integrity' has been the
> > word to identify the integrity subsystem since long time ago.
> > 
> > Reducing the scope to 'ima' would create some confusion since, while
> > 'ima' is associated to integrity, it would not encompass EVM.
> 
> Using LSM_ID_IMA to reference the combination of IMA+EVM makes much
> more sense to me than using LSM_ID_INTEGRITY, especially as we move
> towards promoting IMA+EVM and adopting LSM hooks for integrity
> verification, opening the door for other integrity focused LSMs.

+ Mimi, linux-integrity

Ok, just to understand before posting v4, the code looks like this:

+const struct lsm_id integrity_lsmid = {
+	.name = "integrity",
+	.id = LSM_ID_IMA,
+};
+
 DEFINE_LSM(integrity) = {
 	.name = "integrity",
-	.init = integrity_iintcache_init,
+	.init = integrity_lsm_init,
 	.order = LSM_ORDER_LAST,
 };
 
Is it ok?

Thanks

Roberto



^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v15 00/11] LSM: Three basic syscalls
  2023-10-17 16:07               ` [PATCH v15 00/11] LSM: Three basic syscalls Roberto Sassu
@ 2023-10-18  9:31                 ` Roberto Sassu
  2023-10-18 13:09                   ` Mimi Zohar
  0 siblings, 1 reply; 10+ messages in thread
From: Roberto Sassu @ 2023-10-18  9:31 UTC (permalink / raw)
  To: Paul Moore, Mimi Zohar
  Cc: Casey Schaufler, linux-security-module, jmorris, serge, keescook,
	john.johansen, penguin-kernel, stephen.smalley.work, linux-kernel,
	linux-api, mic, linux-integrity

On Tue, 2023-10-17 at 18:07 +0200, Roberto Sassu wrote:
> On Tue, 2023-10-17 at 11:58 -0400, Paul Moore wrote:
> > On Tue, Oct 17, 2023 at 3:01 AM Roberto Sassu
> > <roberto.sassu@huaweicloud.com> wrote:
> > > On Mon, 2023-10-16 at 11:06 -0400, Paul Moore wrote:
> > > > On Mon, Oct 16, 2023 at 8:05 AM Roberto Sassu
> > > > <roberto.sassu@huaweicloud.com> wrote:
> > > > > 
> > > > > Sorry, I just noticed LSM_ID_IMA. Since we have the 'integrity' LSM, I
> > > > > think it should be LSM_ID_INTEGRITY.
> > > > > 
> > > > > Mimi, all, do you agree? If yes, I send a patch shortly.
> > > > 
> > > > I believe LSM_ID_IMA is the better option, despite "integrity" already
> > > > being present in Kconfig and possibly other areas.  "IMA" is a
> > > > specific thing/LSM whereas "integrity" is a property, principle, or
> > > > quality.  Especially as we move forward with promoting IMA as a full
> > > > and proper LSM, we should work towards referring to it as "IMA" and
> > > > not "integrity".
> > > > 
> > > > If anything we should be working to support "IMA" in places where we
> > > > currently have "integrity" so that we can eventually deprecate
> > > > "integrity".
> > > 
> > > Hi Paul
> > > 
> > > I fully understand your argument. However, 'integrity' has been the
> > > word to identify the integrity subsystem since long time ago.
> > > 
> > > Reducing the scope to 'ima' would create some confusion since, while
> > > 'ima' is associated to integrity, it would not encompass EVM.
> > 
> > Using LSM_ID_IMA to reference the combination of IMA+EVM makes much
> > more sense to me than using LSM_ID_INTEGRITY, especially as we move
> > towards promoting IMA+EVM and adopting LSM hooks for integrity
> > verification, opening the door for other integrity focused LSMs.
> 
> + Mimi, linux-integrity
> 
> Ok, just to understand before posting v4, the code looks like this:

I worked on a new proposal. Let me know what you think. It is available
here:

https://github.com/robertosassu/linux/tree/ima-evm-lsms-v4-devel-v6


I made IMA and EVM as standalone LSMs and removed 'integrity'. They
maintain the same properties of 'integrity', i.e. they are the last and
always enabled.

During initialization, 'ima' and 'evm' call integrity_iintcache_init(),
so that they can get integrity metadata. I added a check to ensure that
this function is called only once. I also added the lsmid parameter so
that the integrity-specific functions are added under the LSM ID of the
caller.

I added a new LSM ID for EVM, does not look good that IMA and EVM are
represented by LSM_ID_IMA.

Finally, I had to drop the patch to remove the rbtree, because without
the 'integrity' LSM, space in the security blob cannot be reserved.
Since integrity metadata is shared, it cannot be reserved by 'ima' or
'evm'.

An intermediate solution would be to keep the 'integrity' LSM just to
reserve space in the security blob. Or, we remove the rbtree if/when
IMA and EVM use disjoint integrity metadata.

Roberto


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v15 00/11] LSM: Three basic syscalls
  2023-10-18  9:31                 ` Roberto Sassu
@ 2023-10-18 13:09                   ` Mimi Zohar
  2023-10-18 14:14                     ` Roberto Sassu
  0 siblings, 1 reply; 10+ messages in thread
From: Mimi Zohar @ 2023-10-18 13:09 UTC (permalink / raw)
  To: Roberto Sassu, Paul Moore
  Cc: Casey Schaufler, linux-security-module, jmorris, serge, keescook,
	john.johansen, penguin-kernel, stephen.smalley.work, linux-kernel,
	linux-api, mic, linux-integrity

On Wed, 2023-10-18 at 11:31 +0200, Roberto Sassu wrote:
> On Tue, 2023-10-17 at 18:07 +0200, Roberto Sassu wrote:
> > On Tue, 2023-10-17 at 11:58 -0400, Paul Moore wrote:
> > > On Tue, Oct 17, 2023 at 3:01 AM Roberto Sassu
> > > <roberto.sassu@huaweicloud.com> wrote:
> > > > On Mon, 2023-10-16 at 11:06 -0400, Paul Moore wrote:
> > > > > On Mon, Oct 16, 2023 at 8:05 AM Roberto Sassu
> > > > > <roberto.sassu@huaweicloud.com> wrote:
> > > > > > 
> > > > > > Sorry, I just noticed LSM_ID_IMA. Since we have the 'integrity' LSM, I
> > > > > > think it should be LSM_ID_INTEGRITY.
> > > > > > 
> > > > > > Mimi, all, do you agree? If yes, I send a patch shortly.
> > > > > 
> > > > > I believe LSM_ID_IMA is the better option, despite "integrity" already
> > > > > being present in Kconfig and possibly other areas.  "IMA" is a
> > > > > specific thing/LSM whereas "integrity" is a property, principle, or
> > > > > quality.  Especially as we move forward with promoting IMA as a full
> > > > > and proper LSM, we should work towards referring to it as "IMA" and
> > > > > not "integrity".
> > > > > 
> > > > > If anything we should be working to support "IMA" in places where we
> > > > > currently have "integrity" so that we can eventually deprecate
> > > > > "integrity".
> > > > 
> > > > Hi Paul
> > > > 
> > > > I fully understand your argument. However, 'integrity' has been the
> > > > word to identify the integrity subsystem since long time ago.
> > > > 
> > > > Reducing the scope to 'ima' would create some confusion since, while
> > > > 'ima' is associated to integrity, it would not encompass EVM.
> > > 
> > > Using LSM_ID_IMA to reference the combination of IMA+EVM makes much
> > > more sense to me than using LSM_ID_INTEGRITY, especially as we move
> > > towards promoting IMA+EVM and adopting LSM hooks for integrity
> > > verification, opening the door for other integrity focused LSMs.
> > 
> > + Mimi, linux-integrity
> > 
> > Ok, just to understand before posting v4, the code looks like this:
> 
> I worked on a new proposal. Let me know what you think. It is available
> here:
> 
> https://github.com/robertosassu/linux/tree/ima-evm-lsms-v4-devel-v6
> 
> 
> I made IMA and EVM as standalone LSMs and removed 'integrity'. They
> maintain the same properties of 'integrity', i.e. they are the last and
> always enabled.
> 
> During initialization, 'ima' and 'evm' call integrity_iintcache_init(),
> so that they can get integrity metadata. I added a check to ensure that
> this function is called only once. I also added the lsmid parameter so
> that the integrity-specific functions are added under the LSM ID of the
> caller.
> 
> I added a new LSM ID for EVM, does not look good that IMA and EVM are
> represented by LSM_ID_IMA.
> 
> Finally, I had to drop the patch to remove the rbtree, because without
> the 'integrity' LSM, space in the security blob cannot be reserved.
> Since integrity metadata is shared, it cannot be reserved by 'ima' or
> 'evm'.
> 
> An intermediate solution would be to keep the 'integrity' LSM just to
> reserve space in the security blob. Or, we remove the rbtree if/when
> IMA and EVM use disjoint integrity metadata.

One of the major benefits for making IMA and EVM LSMs was removing the
rbtree and replacing it with the ability of using i_security.

I agree with Roberto.  All three should be defined: LSM_ID_INTEGRITY,
LSM_ID_IMA, LSM_ID_EVM.

-- 
thanks,

Mimi


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v15 00/11] LSM: Three basic syscalls
  2023-10-18 13:09                   ` Mimi Zohar
@ 2023-10-18 14:14                     ` Roberto Sassu
  2023-10-18 16:35                       ` Paul Moore
  2023-10-19  8:49                       ` Roberto Sassu
  0 siblings, 2 replies; 10+ messages in thread
From: Roberto Sassu @ 2023-10-18 14:14 UTC (permalink / raw)
  To: Mimi Zohar, Paul Moore
  Cc: Casey Schaufler, linux-security-module, jmorris, serge, keescook,
	john.johansen, penguin-kernel, stephen.smalley.work, linux-kernel,
	linux-api, mic, linux-integrity

On 10/18/2023 3:09 PM, Mimi Zohar wrote:
> On Wed, 2023-10-18 at 11:31 +0200, Roberto Sassu wrote:
>> On Tue, 2023-10-17 at 18:07 +0200, Roberto Sassu wrote:
>>> On Tue, 2023-10-17 at 11:58 -0400, Paul Moore wrote:
>>>> On Tue, Oct 17, 2023 at 3:01 AM Roberto Sassu
>>>> <roberto.sassu@huaweicloud.com> wrote:
>>>>> On Mon, 2023-10-16 at 11:06 -0400, Paul Moore wrote:
>>>>>> On Mon, Oct 16, 2023 at 8:05 AM Roberto Sassu
>>>>>> <roberto.sassu@huaweicloud.com> wrote:
>>>>>>>
>>>>>>> Sorry, I just noticed LSM_ID_IMA. Since we have the 'integrity' LSM, I
>>>>>>> think it should be LSM_ID_INTEGRITY.
>>>>>>>
>>>>>>> Mimi, all, do you agree? If yes, I send a patch shortly.
>>>>>>
>>>>>> I believe LSM_ID_IMA is the better option, despite "integrity" already
>>>>>> being present in Kconfig and possibly other areas.  "IMA" is a
>>>>>> specific thing/LSM whereas "integrity" is a property, principle, or
>>>>>> quality.  Especially as we move forward with promoting IMA as a full
>>>>>> and proper LSM, we should work towards referring to it as "IMA" and
>>>>>> not "integrity".
>>>>>>
>>>>>> If anything we should be working to support "IMA" in places where we
>>>>>> currently have "integrity" so that we can eventually deprecate
>>>>>> "integrity".
>>>>>
>>>>> Hi Paul
>>>>>
>>>>> I fully understand your argument. However, 'integrity' has been the
>>>>> word to identify the integrity subsystem since long time ago.
>>>>>
>>>>> Reducing the scope to 'ima' would create some confusion since, while
>>>>> 'ima' is associated to integrity, it would not encompass EVM.
>>>>
>>>> Using LSM_ID_IMA to reference the combination of IMA+EVM makes much
>>>> more sense to me than using LSM_ID_INTEGRITY, especially as we move
>>>> towards promoting IMA+EVM and adopting LSM hooks for integrity
>>>> verification, opening the door for other integrity focused LSMs.
>>>
>>> + Mimi, linux-integrity
>>>
>>> Ok, just to understand before posting v4, the code looks like this:
>>
>> I worked on a new proposal. Let me know what you think. It is available
>> here:
>>
>> https://github.com/robertosassu/linux/tree/ima-evm-lsms-v4-devel-v6
>>
>>
>> I made IMA and EVM as standalone LSMs and removed 'integrity'. They
>> maintain the same properties of 'integrity', i.e. they are the last and
>> always enabled.
>>
>> During initialization, 'ima' and 'evm' call integrity_iintcache_init(),
>> so that they can get integrity metadata. I added a check to ensure that
>> this function is called only once. I also added the lsmid parameter so
>> that the integrity-specific functions are added under the LSM ID of the
>> caller.
>>
>> I added a new LSM ID for EVM, does not look good that IMA and EVM are
>> represented by LSM_ID_IMA.
>>
>> Finally, I had to drop the patch to remove the rbtree, because without
>> the 'integrity' LSM, space in the security blob cannot be reserved.
>> Since integrity metadata is shared, it cannot be reserved by 'ima' or
>> 'evm'.
>>
>> An intermediate solution would be to keep the 'integrity' LSM just to
>> reserve space in the security blob. Or, we remove the rbtree if/when
>> IMA and EVM use disjoint integrity metadata.
> 
> One of the major benefits for making IMA and EVM LSMs was removing the
> rbtree and replacing it with the ability of using i_security.
> 
> I agree with Roberto.  All three should be defined: LSM_ID_INTEGRITY,
> LSM_ID_IMA, LSM_ID_EVM.

I did not try yet, but the 'integrity' LSM does not need an LSM ID. With 
the last version adding hooks to 'ima' or 'evm', it should be sufficient 
to keep DEFINE_LSM(integrity) with the request to store a pointer in the 
security blob (even the init function can be a dummy function).

Roberto


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v15 00/11] LSM: Three basic syscalls
  2023-10-18 14:14                     ` Roberto Sassu
@ 2023-10-18 16:35                       ` Paul Moore
  2023-10-18 20:10                         ` Mimi Zohar
  2023-10-19  8:49                       ` Roberto Sassu
  1 sibling, 1 reply; 10+ messages in thread
From: Paul Moore @ 2023-10-18 16:35 UTC (permalink / raw)
  To: Roberto Sassu
  Cc: Mimi Zohar, Casey Schaufler, linux-security-module, jmorris,
	serge, keescook, john.johansen, penguin-kernel,
	stephen.smalley.work, linux-kernel, linux-api, mic,
	linux-integrity

On Wed, Oct 18, 2023 at 10:15 AM Roberto Sassu
<roberto.sassu@huaweicloud.com> wrote:
> On 10/18/2023 3:09 PM, Mimi Zohar wrote:

...

> > I agree with Roberto.  All three should be defined: LSM_ID_INTEGRITY,
> > LSM_ID_IMA, LSM_ID_EVM.
>
> I did not try yet, but the 'integrity' LSM does not need an LSM ID. With
> the last version adding hooks to 'ima' or 'evm', it should be sufficient
> to keep DEFINE_LSM(integrity) with the request to store a pointer in the
> security blob (even the init function can be a dummy function).

First off, this *really* should have been brought up way, way, *way*
before now.  This patchset has been discussed for months, and bringing
up concerns in the eleventh hour is borderline rude.

At least we haven't shipped this in a tagged release from Linus yet,
so there is that.

If you want to add a unique LSM ID for both IMA and EVM, I'm okay with
that, but if we do that I don't see the need for a dedicated ID for
"integrity".  Roberto, Mimi, one of you please send me a patch on top
of lsm/next-queue that updates the LSM ID to look like the following
(I believe EVM was added between AppArmor and Yama, yes?):

#define LSM_ID_UNDEF            0
#define LSM_ID_CAPABILITY       100
#define LSM_ID_SELINUX          101
#define LSM_ID_SMACK            102
#define LSM_ID_TOMOYO           103
#define LSM_ID_IMA              104
#define LSM_ID_APPARMOR         105
#define LSM_ID_EVM              106
#define LSM_ID_YAMA             107
#define LSM_ID_LOADPIN          108
#define LSM_ID_SAFESETID        109
#define LSM_ID_LOCKDOWN         110
#define LSM_ID_BPF              111
#define LSM_ID_LANDLOCK         112

... and also update the LSM registration code for IMA/EVM/etc. to do
the right thing.

Also, just to be clear, you should get this patch out ASAP.

-- 
paul-moore.com

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v15 00/11] LSM: Three basic syscalls
  2023-10-18 16:35                       ` Paul Moore
@ 2023-10-18 20:10                         ` Mimi Zohar
  2023-10-18 20:40                           ` Paul Moore
  0 siblings, 1 reply; 10+ messages in thread
From: Mimi Zohar @ 2023-10-18 20:10 UTC (permalink / raw)
  To: Paul Moore, Roberto Sassu
  Cc: Casey Schaufler, linux-security-module, jmorris, serge, keescook,
	john.johansen, penguin-kernel, stephen.smalley.work, linux-kernel,
	linux-api, mic, linux-integrity

On Wed, 2023-10-18 at 12:35 -0400, Paul Moore wrote:
> On Wed, Oct 18, 2023 at 10:15 AM Roberto Sassu
> <roberto.sassu@huaweicloud.com> wrote:
> > On 10/18/2023 3:09 PM, Mimi Zohar wrote:
> 
> ...
> 
> > > I agree with Roberto.  All three should be defined: LSM_ID_INTEGRITY,
> > > LSM_ID_IMA, LSM_ID_EVM.
> >
> > I did not try yet, but the 'integrity' LSM does not need an LSM ID. With
> > the last version adding hooks to 'ima' or 'evm', it should be sufficient
> > to keep DEFINE_LSM(integrity) with the request to store a pointer in the
> > security blob (even the init function can be a dummy function).
> 
> First off, this *really* should have been brought up way, way, *way*
> before now.  This patchset has been discussed for months, and bringing
> up concerns in the eleventh hour is borderline rude.

As everyone knows IMA and EVM are not LSMs at this point.

So the only thing that is "rude" is the way you're responding in this
thread.

> 
> At least we haven't shipped this in a tagged release from Linus yet,
> so there is that.

What does that have to do with anything?!  Code changes.

Mimi


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v15 00/11] LSM: Three basic syscalls
  2023-10-18 20:10                         ` Mimi Zohar
@ 2023-10-18 20:40                           ` Paul Moore
  2023-10-19  7:45                             ` Roberto Sassu
  0 siblings, 1 reply; 10+ messages in thread
From: Paul Moore @ 2023-10-18 20:40 UTC (permalink / raw)
  To: Mimi Zohar
  Cc: Roberto Sassu, Casey Schaufler, linux-security-module, jmorris,
	serge, keescook, john.johansen, penguin-kernel,
	stephen.smalley.work, linux-kernel, linux-api, mic,
	linux-integrity

On Wed, Oct 18, 2023 at 4:23 PM Mimi Zohar <zohar@linux.ibm.com> wrote:
> On Wed, 2023-10-18 at 12:35 -0400, Paul Moore wrote:
> > On Wed, Oct 18, 2023 at 10:15 AM Roberto Sassu
> > <roberto.sassu@huaweicloud.com> wrote:
> > > On 10/18/2023 3:09 PM, Mimi Zohar wrote:
> >
> > ...
> >
> > > > I agree with Roberto.  All three should be defined: LSM_ID_INTEGRITY,
> > > > LSM_ID_IMA, LSM_ID_EVM.
> > >
> > > I did not try yet, but the 'integrity' LSM does not need an LSM ID. With
> > > the last version adding hooks to 'ima' or 'evm', it should be sufficient
> > > to keep DEFINE_LSM(integrity) with the request to store a pointer in the
> > > security blob (even the init function can be a dummy function).
> >
> > First off, this *really* should have been brought up way, way, *way*
> > before now.  This patchset has been discussed for months, and bringing
> > up concerns in the eleventh hour is borderline rude.
>
> As everyone knows IMA and EVM are not LSMs at this point.

Considering all the work Roberto has been doing to make that happen,
not to mention the discussions we've had on this topic, that's an
awfully small technicality to use as the basis of an argument.

> So the only thing that is "rude" is the way you're responding in this
> thread.

Agree to disagree.

> > At least we haven't shipped this in a tagged release from Linus yet,
> > so there is that.
>
> What does that have to do with anything?!  Code changes.

Code can change, Linux kernel APIs should not change.

-- 
paul-moore.com

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v15 00/11] LSM: Three basic syscalls
  2023-10-18 20:40                           ` Paul Moore
@ 2023-10-19  7:45                             ` Roberto Sassu
  2023-10-20 16:36                               ` Casey Schaufler
  0 siblings, 1 reply; 10+ messages in thread
From: Roberto Sassu @ 2023-10-19  7:45 UTC (permalink / raw)
  To: Paul Moore, Mimi Zohar
  Cc: Casey Schaufler, linux-security-module, jmorris, serge, keescook,
	john.johansen, penguin-kernel, stephen.smalley.work, linux-kernel,
	linux-api, mic, linux-integrity

On Wed, 2023-10-18 at 16:40 -0400, Paul Moore wrote:
> On Wed, Oct 18, 2023 at 4:23 PM Mimi Zohar <zohar@linux.ibm.com> wrote:
> > On Wed, 2023-10-18 at 12:35 -0400, Paul Moore wrote:
> > > On Wed, Oct 18, 2023 at 10:15 AM Roberto Sassu
> > > <roberto.sassu@huaweicloud.com> wrote:
> > > > On 10/18/2023 3:09 PM, Mimi Zohar wrote:
> > > 
> > > ...
> > > 
> > > > > I agree with Roberto.  All three should be defined: LSM_ID_INTEGRITY,
> > > > > LSM_ID_IMA, LSM_ID_EVM.
> > > > 
> > > > I did not try yet, but the 'integrity' LSM does not need an LSM ID. With
> > > > the last version adding hooks to 'ima' or 'evm', it should be sufficient
> > > > to keep DEFINE_LSM(integrity) with the request to store a pointer in the
> > > > security blob (even the init function can be a dummy function).
> > > 
> > > First off, this *really* should have been brought up way, way, *way*
> > > before now.  This patchset has been discussed for months, and bringing
> > > up concerns in the eleventh hour is borderline rude.
> > 
> > As everyone knows IMA and EVM are not LSMs at this point.
> 
> Considering all the work Roberto has been doing to make that happen,
> not to mention the discussions we've had on this topic, that's an
> awfully small technicality to use as the basis of an argument.

Sorry Paul, but I've been working on this patch set for a long time and
you were also involved in the prerequisites (like making the
'integrity' LSM as the last).

I thought it was clear at this point that we were going to add the
hooks to the 'integrity' LSM.

I really have no problem to rebase my work on top of other work, but I
was very surprised to see LSM_ID_IMA instead of LSM_ID_INTEGRITY, and
at minimum this should have been agreed with Mimi. And also, I was not
convinced with the argument that LSM_ID_IMA should represent IMA+EVM.

Roberto


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v15 00/11] LSM: Three basic syscalls
  2023-10-18 14:14                     ` Roberto Sassu
  2023-10-18 16:35                       ` Paul Moore
@ 2023-10-19  8:49                       ` Roberto Sassu
  1 sibling, 0 replies; 10+ messages in thread
From: Roberto Sassu @ 2023-10-19  8:49 UTC (permalink / raw)
  To: Mimi Zohar, Paul Moore
  Cc: Casey Schaufler, linux-security-module, jmorris, serge, keescook,
	john.johansen, penguin-kernel, stephen.smalley.work, linux-kernel,
	linux-api, mic, linux-integrity

On Wed, 2023-10-18 at 16:14 +0200, Roberto Sassu wrote:
> On 10/18/2023 3:09 PM, Mimi Zohar wrote:
> > On Wed, 2023-10-18 at 11:31 +0200, Roberto Sassu wrote:
> > > On Tue, 2023-10-17 at 18:07 +0200, Roberto Sassu wrote:
> > > > On Tue, 2023-10-17 at 11:58 -0400, Paul Moore wrote:
> > > > > On Tue, Oct 17, 2023 at 3:01 AM Roberto Sassu
> > > > > <roberto.sassu@huaweicloud.com> wrote:
> > > > > > On Mon, 2023-10-16 at 11:06 -0400, Paul Moore wrote:
> > > > > > > On Mon, Oct 16, 2023 at 8:05 AM Roberto Sassu
> > > > > > > <roberto.sassu@huaweicloud.com> wrote:
> > > > > > > > 
> > > > > > > > Sorry, I just noticed LSM_ID_IMA. Since we have the 'integrity' LSM, I
> > > > > > > > think it should be LSM_ID_INTEGRITY.
> > > > > > > > 
> > > > > > > > Mimi, all, do you agree? If yes, I send a patch shortly.
> > > > > > > 
> > > > > > > I believe LSM_ID_IMA is the better option, despite "integrity" already
> > > > > > > being present in Kconfig and possibly other areas.  "IMA" is a
> > > > > > > specific thing/LSM whereas "integrity" is a property, principle, or
> > > > > > > quality.  Especially as we move forward with promoting IMA as a full
> > > > > > > and proper LSM, we should work towards referring to it as "IMA" and
> > > > > > > not "integrity".
> > > > > > > 
> > > > > > > If anything we should be working to support "IMA" in places where we
> > > > > > > currently have "integrity" so that we can eventually deprecate
> > > > > > > "integrity".
> > > > > > 
> > > > > > Hi Paul
> > > > > > 
> > > > > > I fully understand your argument. However, 'integrity' has been the
> > > > > > word to identify the integrity subsystem since long time ago.
> > > > > > 
> > > > > > Reducing the scope to 'ima' would create some confusion since, while
> > > > > > 'ima' is associated to integrity, it would not encompass EVM.
> > > > > 
> > > > > Using LSM_ID_IMA to reference the combination of IMA+EVM makes much
> > > > > more sense to me than using LSM_ID_INTEGRITY, especially as we move
> > > > > towards promoting IMA+EVM and adopting LSM hooks for integrity
> > > > > verification, opening the door for other integrity focused LSMs.
> > > > 
> > > > + Mimi, linux-integrity
> > > > 
> > > > Ok, just to understand before posting v4, the code looks like this:
> > > 
> > > I worked on a new proposal. Let me know what you think. It is available
> > > here:
> > > 
> > > https://github.com/robertosassu/linux/tree/ima-evm-lsms-v4-devel-v6
> > > 
> > > 
> > > I made IMA and EVM as standalone LSMs and removed 'integrity'. They
> > > maintain the same properties of 'integrity', i.e. they are the last and
> > > always enabled.
> > > 
> > > During initialization, 'ima' and 'evm' call integrity_iintcache_init(),
> > > so that they can get integrity metadata. I added a check to ensure that
> > > this function is called only once. I also added the lsmid parameter so
> > > that the integrity-specific functions are added under the LSM ID of the
> > > caller.
> > > 
> > > I added a new LSM ID for EVM, does not look good that IMA and EVM are
> > > represented by LSM_ID_IMA.
> > > 
> > > Finally, I had to drop the patch to remove the rbtree, because without
> > > the 'integrity' LSM, space in the security blob cannot be reserved.
> > > Since integrity metadata is shared, it cannot be reserved by 'ima' or
> > > 'evm'.
> > > 
> > > An intermediate solution would be to keep the 'integrity' LSM just to
> > > reserve space in the security blob. Or, we remove the rbtree if/when
> > > IMA and EVM use disjoint integrity metadata.
> > 
> > One of the major benefits for making IMA and EVM LSMs was removing the
> > rbtree and replacing it with the ability of using i_security.
> > 
> > I agree with Roberto.  All three should be defined: LSM_ID_INTEGRITY,
> > LSM_ID_IMA, LSM_ID_EVM.
> 
> I did not try yet, but the 'integrity' LSM does not need an LSM ID. With 
> the last version adding hooks to 'ima' or 'evm', it should be sufficient 
> to keep DEFINE_LSM(integrity) with the request to store a pointer in the 
> security blob (even the init function can be a dummy function).

Ok, I rebased on top of Paul's 'lsm: drop LSM_ID_IMA', made IMA and EVM
as standalone LSMs, and assigned LSM IDs to them in the correct
chronological order.

Still left DEFINE_LSM(integrity) to reserve space for the
integrity_iint_cache pointer, but it does not register any LSM hook,
then no LSM ID required.

In the future, we might be able to make IMA and EVM use disjoint
metadata, so they will reserve space in the security blob and register
appropriate hooks for the metadata management.

With this intermediate solution, I can keep the rbtree patch, which
will provide performance improvements due to searching metadata in
constant time.

If you want to have a look before I send the patch set, the code is
available here:

https://github.com/robertosassu/linux/commits/ima-evm-lsms-v4-devel-v7

It passes all IMA tests:

https://github.com/robertosassu/ima-evm-utils/actions/runs/6571587570

Roberto


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v15 00/11] LSM: Three basic syscalls
  2023-10-19  7:45                             ` Roberto Sassu
@ 2023-10-20 16:36                               ` Casey Schaufler
  0 siblings, 0 replies; 10+ messages in thread
From: Casey Schaufler @ 2023-10-20 16:36 UTC (permalink / raw)
  To: Roberto Sassu, Paul Moore, Mimi Zohar
  Cc: linux-security-module, jmorris, serge, keescook, john.johansen,
	penguin-kernel, stephen.smalley.work, linux-kernel, linux-api,
	mic, linux-integrity, Casey Schaufler

On 10/19/2023 12:45 AM, Roberto Sassu wrote:
> On Wed, 2023-10-18 at 16:40 -0400, Paul Moore wrote:
>> On Wed, Oct 18, 2023 at 4:23 PM Mimi Zohar <zohar@linux.ibm.com> wrote:
>>> On Wed, 2023-10-18 at 12:35 -0400, Paul Moore wrote:
>>>> On Wed, Oct 18, 2023 at 10:15 AM Roberto Sassu
>>>> <roberto.sassu@huaweicloud.com> wrote:
>>>>> On 10/18/2023 3:09 PM, Mimi Zohar wrote:
>>>> ...
>>>>
>>>>>> I agree with Roberto.  All three should be defined: LSM_ID_INTEGRITY,
>>>>>> LSM_ID_IMA, LSM_ID_EVM.
>>>>> I did not try yet, but the 'integrity' LSM does not need an LSM ID. With
>>>>> the last version adding hooks to 'ima' or 'evm', it should be sufficient
>>>>> to keep DEFINE_LSM(integrity) with the request to store a pointer in the
>>>>> security blob (even the init function can be a dummy function).
>>>> First off, this *really* should have been brought up way, way, *way*
>>>> before now.  This patchset has been discussed for months, and bringing
>>>> up concerns in the eleventh hour is borderline rude.
>>> As everyone knows IMA and EVM are not LSMs at this point.
>> Considering all the work Roberto has been doing to make that happen,
>> not to mention the discussions we've had on this topic, that's an
>> awfully small technicality to use as the basis of an argument.
> Sorry Paul, but I've been working on this patch set for a long time and
> you were also involved in the prerequisites (like making the
> 'integrity' LSM as the last).
>
> I thought it was clear at this point that we were going to add the
> hooks to the 'integrity' LSM.

There's a chicken/egg issue here. You can hold up the syscalls patch
forever if you insist on it accommodating every patch set that's in the
pipeline. I understand that you've been working on the integrity rework
for some time. I understand that it's frustrating when things change out
from under you. Believe me, I do.

>
> I really have no problem to rebase my work on top of other work, but I
> was very surprised to see LSM_ID_IMA instead of LSM_ID_INTEGRITY, and
> at minimum this should have been agreed with Mimi. And also, I was not
> convinced with the argument that LSM_ID_IMA should represent IMA+EVM.
>
> Roberto
>

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2023-10-20 16:36 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
     [not found] <20230912205658.3432-1-casey.ref@schaufler-ca.com>
     [not found] ` <20230912205658.3432-1-casey@schaufler-ca.com>
     [not found]   ` <CAHC9VhRcbp3iWQwL7FTUrcU1C3OsZ413Nbq+17oTwW7hZ7XvBw@mail.gmail.com>
     [not found]     ` <CAHC9VhSqY5+DR-jXprrftb1=CzDvhTh0Ep66A16RMd4L7W7TYw@mail.gmail.com>
     [not found]       ` <ae39864947debbc7c460db478b8abe1c147b7d5c.camel@huaweicloud.com>
     [not found]         ` <CAHC9VhRQ7xpeSX7b3VZfzQ15noJ8mgauNMuHWo_n3hMgsYMAfQ@mail.gmail.com>
     [not found]           ` <468436cf766732a3cfc55d07ad119a6ccdc815c1.camel@huaweicloud.com>
     [not found]             ` <CAHC9VhTjHT-DGKu0=cZPVb=+kMwmbPdr8HiVWJq-yzaDiYk_SA@mail.gmail.com>
2023-10-17 16:07               ` [PATCH v15 00/11] LSM: Three basic syscalls Roberto Sassu
2023-10-18  9:31                 ` Roberto Sassu
2023-10-18 13:09                   ` Mimi Zohar
2023-10-18 14:14                     ` Roberto Sassu
2023-10-18 16:35                       ` Paul Moore
2023-10-18 20:10                         ` Mimi Zohar
2023-10-18 20:40                           ` Paul Moore
2023-10-19  7:45                             ` Roberto Sassu
2023-10-20 16:36                               ` Casey Schaufler
2023-10-19  8:49                       ` Roberto Sassu

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox