Message ID | 20230912205658.3432-1-casey@schaufler-ca.com (mailing list archive) |
---|---|
Headers | show |
Series | LSM: Three basic syscalls | expand |
On Tue, Sep 12, 2023 at 4:57 PM Casey Schaufler <casey@schaufler-ca.com> wrote: > > Add three system calls for the Linux Security Module ABI ... First off, a big thank you to Casey who took it upon himself to turn my pseudo-code syscall suggestion into a proper patchset and saw it through 15 revisions. Thanks also go out to everyone that has helped review and comment on this effort; I know everyone is busy, but these reviews are important. I'm happy to say that I think we're in a good place with this revision of the LSM syscall patchset. I only see two outstanding issues, and neither of those are bugs/showstoppers that affect the API, they are simply areas where the implementation could be improved. With the understanding that Casey is busy for the rest of the month, and my desire to make sure this patchset gets a full dev cycle in linux-next, I'm going to suggest merging this into the lsm/next-queue branch soon (likely tomorrow) in preparation for merging it into lsm/next once the upcoming merge window closes. Those who want to help improve the implementation, as suggested in the feedback on this revision or otherwise, are welcome to submit patches against the lsm/next-queue branch and I will merge them into that branch once they pass review. If I don't hear any objections I'll plan on merging this patchset tomorrow, I'll send a follow-up reply to this email when it's done.
On Thu, Oct 12, 2023 at 6:07 PM Paul Moore <paul@paul-moore.com> wrote: > > On Tue, Sep 12, 2023 at 4:57 PM Casey Schaufler <casey@schaufler-ca.com> wrote: > > > > Add three system calls for the Linux Security Module ABI ... > > First off, a big thank you to Casey who took it upon himself to turn > my pseudo-code syscall suggestion into a proper patchset and saw it > through 15 revisions. Thanks also go out to everyone that has helped > review and comment on this effort; I know everyone is busy, but these > reviews are important. > > I'm happy to say that I think we're in a good place with this revision > of the LSM syscall patchset. I only see two outstanding issues, and > neither of those are bugs/showstoppers that affect the API, they are > simply areas where the implementation could be improved. With the > understanding that Casey is busy for the rest of the month, and my > desire to make sure this patchset gets a full dev cycle in linux-next, > I'm going to suggest merging this into the lsm/next-queue branch soon > (likely tomorrow) in preparation for merging it into lsm/next once the > upcoming merge window closes. Those who want to help improve the > implementation, as suggested in the feedback on this revision or > otherwise, are welcome to submit patches against the lsm/next-queue > branch and I will merge them into that branch once they pass review. > > If I don't hear any objections I'll plan on merging this patchset > tomorrow, I'll send a follow-up reply to this email when it's done. Since it's been *almost* a full 24 hours and no objections I went ahead and merged this patchset into lsm/next-queue with the intention of bringing them into lsm/next after the upcoming merge window closes. For those of you who have suggested changes, please feel free to submit patches against the lsm/next-queue branch and we can get them queued up along with these patches. Thanks everyone!
On Fri, 2023-10-13 at 17:55 -0400, Paul Moore wrote: > On Thu, Oct 12, 2023 at 6:07 PM Paul Moore <paul@paul-moore.com> wrote: > > > > On Tue, Sep 12, 2023 at 4:57 PM Casey Schaufler <casey@schaufler-ca.com> wrote: > > > > > > Add three system calls for the Linux Security Module ABI ... > > > > First off, a big thank you to Casey who took it upon himself to turn > > my pseudo-code syscall suggestion into a proper patchset and saw it > > through 15 revisions. Thanks also go out to everyone that has helped > > review and comment on this effort; I know everyone is busy, but these > > reviews are important. > > > > I'm happy to say that I think we're in a good place with this revision > > of the LSM syscall patchset. I only see two outstanding issues, and > > neither of those are bugs/showstoppers that affect the API, they are > > simply areas where the implementation could be improved. With the > > understanding that Casey is busy for the rest of the month, and my > > desire to make sure this patchset gets a full dev cycle in linux-next, > > I'm going to suggest merging this into the lsm/next-queue branch soon > > (likely tomorrow) in preparation for merging it into lsm/next once the > > upcoming merge window closes. Those who want to help improve the > > implementation, as suggested in the feedback on this revision or > > otherwise, are welcome to submit patches against the lsm/next-queue > > branch and I will merge them into that branch once they pass review. > > > > If I don't hear any objections I'll plan on merging this patchset > > tomorrow, I'll send a follow-up reply to this email when it's done. > > Since it's been *almost* a full 24 hours and no objections I went > ahead and merged this patchset into lsm/next-queue with the intention > of bringing them into lsm/next after the upcoming merge window closes. > For those of you who have suggested changes, please feel free to > submit patches against the lsm/next-queue branch and we can get them > queued up along with these patches. 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. Thanks Roberto
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".
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. The term 'integrity', although it is a property, it precisely identifies in the kernel context the scope and goals of the subsystem, and is general enough to encompass new projects going in a similar direction (such as my integrity digest cache). From a technical perspective, at the moment it is not possible to split 'integrity' in two standalone LSMs 'ima' and 'evm', as IMA and EVM work on shared integrity metadata. Also my integrity digest cache is using the same metadata. In addition, making IMA and EVM as standalone LSMs would require a much longer development cycle to make them use disjoint metadata and to define proper communication interfaces. It would be not anymore a technical move of function calls from a place to another, like for the current patch set, but would require substantial time to validate the new design. To submit my patch set in the current state, the only thing I need is to have LSM_ID_INTEGRITY defined. Roberto
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.
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
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
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.
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
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.
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
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.
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
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
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 >
On Tue, Sep 12, 2023 at 4:57 PM Casey Schaufler <casey@schaufler-ca.com> wrote: > > Add three system calls for the Linux Security Module ABI. > > lsm_get_self_attr() provides the security module specific attributes > that have previously been visible in the /proc/self/attr directory. > For each security module that uses the specified attribute on the > current process the system call will return an LSM identifier and > the value of the attribute. The LSM and attribute identifier values > are defined in include/uapi/linux/lsm.h > > LSM identifiers are simple integers and reflect the order in which > the LSM was added to the mainline kernel. This is a convention, not > a promise of the API. LSM identifiers below the value of 100 are > reserved for unspecified future uses. That could include information > about the security infrastructure itself, or about how multiple LSMs > might interact with each other. > > A new LSM hook security_getselfattr() is introduced to get the > required information from the security modules. This is similar > to the existing security_getprocattr() hook, but specifies the > format in which string data is returned and requires the module > to put the information into a userspace destination. > > lsm_set_self_attr() changes the specified LSM attribute. Only one > attribute can be changed at a time, and then only if the specified > security module allows the change. > > A new LSM hook security_setselfattr() is introduced to set the > required information in the security modules. This is similar > to the existing security_setprocattr() hook, but specifies the > format in which string data is presented and requires the module > to get the information from a userspace destination. > > lsm_list_modules() provides the LSM identifiers, in order, of the > security modules that are active on the system. This has been > available in the securityfs file /sys/kernel/security/lsm. > > Patch 0001 changes the LSM registration from passing the name > of the module to passing a lsm_id structure that contains the > name of the module, an LSM identifier number and an attribute > identifier. > Patch 0002 adds the registered lsm_ids to a table. > Patch 0003 changes security_[gs]etprocattr() to use LSM IDs instead > of LSM names. > Patch 0004 implements lsm_get_self_attr() and lsm_set_self_attr(). > New LSM hooks security_getselfattr() and security_setselfattr() are > defined. > Patch 0005 implements lsm_list_modules(). > Patch 0006 wires up the syscalls. > Patch 0007 implements helper functions to make it easier for > security modules to use lsm_ctx structures. > Patch 0008 provides the Smack implementation for [gs]etselfattr(). > Patch 0009 provides the AppArmor implementation for [gs]etselfattr(). > Patch 0010 provides the SELinux implementation for [gs]etselfattr(). > Patch 0011 implements selftests for the three new syscalls. > > https://github.com/cschaufler/lsm-stacking.git#syscalls-6.5-rc7-v14 > > v15: Rebased on 6.6-rc1. > Adopt suggested improvements to security_getprocattr, > making the code easier to read. > Squash a code fix from 0011 to 0004. > v14: Make the handling of LSM_FLAG_SINGLE easier to understand. > Tighten the comments and documentation. > Better use of const, static, and __ro_after_init. > Add selftests for LSM_FLAG_SINGLE cases. > v13: Change the setselfattr code to do a single user copy. > Make the self tests more robust. > Improve use of const. > Change syscall numbers to reflect upstream additions. > v12: Repair a registration time overflow check. > v11: Remove redundent alignment code > Improve a few comments. > Use LSM_ATTR_UNDEF in place of 0 in a few places. > Correct a return of -EINVAL to -E2BIG. > v10: Correct use of __user. > Improve a few comments. > Revert unnecessary changes in module initialization. > v9: Support a flag LSM_FLAG_SINGLE in lsm_get_self_attr() that > instructs the call to provide only the attribute for the LSM > identified in the referenced lsm_ctx structure. > Fix a typing error. > Change some coding style. > v8: Allow an LSM to provide more than one instance of an attribute, > even though none of the existing modules do so. > Pad the data returned by lsm_get_self_attr() to the size of > the struct lsm_ctx. > Change some displeasing varilable names. > v7: Pass the attribute desired to lsm_[gs]et_self_attr in its own > parameter rather than encoding it in the flags. > Change the flags parameters to u32. > Don't shortcut out of calling LSM specific code in the > infrastructure, let the LSM report that doesn't support an > attribute instead. With that it is not necessary to maintain > a set of supported attributes in the lsm_id structure. > Fix a typing error. > v6: Switch from reusing security_[gs]procattr() to using new > security_[gs]selfattr() hooks. Use explicit sized data types > in the lsm_ctx structure. > > v5: Correct syscall parameter data types. > > v4: Restore "reserved" LSM ID values. Add explaination. > Squash patches that introduce fields in lsm_id. > Correct a wireup error. > > v3: Add lsm_set_self_attr(). > Rename lsm_self_attr() to lsm_get_self_attr(). > Provide the values only for a specifed attribute in > lsm_get_self_attr(). > Add selftests for the three new syscalls. > Correct some parameter checking. > > v2: Use user-interface safe data types. > Remove "reserved" LSM ID values. > Improve kerneldoc comments > Include copyright dates > Use more descriptive name for LSM counter > Add documentation > Correct wireup errors > > Casey Schaufler (11): > LSM: Identify modules by more than name > LSM: Maintain a table of LSM attribute data > proc: Use lsmids instead of lsm names for attrs > LSM: syscalls for current process attributes > LSM: Create lsm_list_modules system call > LSM: wireup Linux Security Module syscalls > LSM: Helpers for attribute names and filling lsm_ctx > Smack: implement setselfattr and getselfattr hooks > AppArmor: Add selfattr hooks > SELinux: Add selfattr hooks > LSM: selftests for Linux Security Module syscalls > > Documentation/userspace-api/index.rst | 1 + > Documentation/userspace-api/lsm.rst | 73 +++++ > MAINTAINERS | 2 + > arch/alpha/kernel/syscalls/syscall.tbl | 3 + > arch/arm/tools/syscall.tbl | 3 + > arch/arm64/include/asm/unistd.h | 2 +- > arch/arm64/include/asm/unistd32.h | 6 + > arch/ia64/kernel/syscalls/syscall.tbl | 3 + > arch/m68k/kernel/syscalls/syscall.tbl | 3 + > arch/microblaze/kernel/syscalls/syscall.tbl | 3 + > arch/mips/kernel/syscalls/syscall_n32.tbl | 3 + > arch/mips/kernel/syscalls/syscall_n64.tbl | 3 + > arch/mips/kernel/syscalls/syscall_o32.tbl | 3 + > arch/parisc/kernel/syscalls/syscall.tbl | 3 + > arch/powerpc/kernel/syscalls/syscall.tbl | 3 + > arch/s390/kernel/syscalls/syscall.tbl | 3 + > arch/sh/kernel/syscalls/syscall.tbl | 3 + > arch/sparc/kernel/syscalls/syscall.tbl | 3 + > arch/x86/entry/syscalls/syscall_32.tbl | 3 + > arch/x86/entry/syscalls/syscall_64.tbl | 3 + > arch/xtensa/kernel/syscalls/syscall.tbl | 3 + > fs/proc/base.c | 29 +- > fs/proc/internal.h | 2 +- > include/linux/lsm_hook_defs.h | 4 + > include/linux/lsm_hooks.h | 17 +- > include/linux/security.h | 46 ++- > include/linux/syscalls.h | 6 + > include/uapi/asm-generic/unistd.h | 9 +- > include/uapi/linux/lsm.h | 90 ++++++ > kernel/sys_ni.c | 3 + > security/Makefile | 1 + > security/apparmor/include/procattr.h | 2 +- > security/apparmor/lsm.c | 99 ++++++- > security/apparmor/procattr.c | 10 +- > security/bpf/hooks.c | 9 +- > security/commoncap.c | 8 +- > security/landlock/cred.c | 2 +- > security/landlock/fs.c | 2 +- > security/landlock/ptrace.c | 2 +- > security/landlock/setup.c | 6 + > security/landlock/setup.h | 1 + > security/loadpin/loadpin.c | 9 +- > security/lockdown/lockdown.c | 8 +- > security/lsm_syscalls.c | 120 ++++++++ > security/safesetid/lsm.c | 9 +- > security/security.c | 253 +++++++++++++++- > security/selinux/hooks.c | 143 +++++++-- > security/smack/smack_lsm.c | 103 ++++++- > security/tomoyo/tomoyo.c | 9 +- > security/yama/yama_lsm.c | 8 +- > .../arch/mips/entry/syscalls/syscall_n64.tbl | 3 + > .../arch/powerpc/entry/syscalls/syscall.tbl | 3 + > .../perf/arch/s390/entry/syscalls/syscall.tbl | 3 + > .../arch/x86/entry/syscalls/syscall_64.tbl | 3 + > tools/testing/selftests/Makefile | 1 + > tools/testing/selftests/lsm/.gitignore | 1 + > tools/testing/selftests/lsm/Makefile | 17 ++ > tools/testing/selftests/lsm/common.c | 89 ++++++ > tools/testing/selftests/lsm/common.h | 33 +++ > tools/testing/selftests/lsm/config | 3 + > .../selftests/lsm/lsm_get_self_attr_test.c | 275 ++++++++++++++++++ > .../selftests/lsm/lsm_list_modules_test.c | 140 +++++++++ > .../selftests/lsm/lsm_set_self_attr_test.c | 74 +++++ > 63 files changed, 1694 insertions(+), 93 deletions(-) > create mode 100644 Documentation/userspace-api/lsm.rst > create mode 100644 include/uapi/linux/lsm.h > create mode 100644 security/lsm_syscalls.c > create mode 100644 tools/testing/selftests/lsm/.gitignore > create mode 100644 tools/testing/selftests/lsm/Makefile > create mode 100644 tools/testing/selftests/lsm/common.c > create mode 100644 tools/testing/selftests/lsm/common.h > create mode 100644 tools/testing/selftests/lsm/config > create mode 100644 tools/testing/selftests/lsm/lsm_get_self_attr_test.c > create mode 100644 tools/testing/selftests/lsm/lsm_list_modules_test.c > create mode 100644 tools/testing/selftests/lsm/lsm_set_self_attr_test.c This patchset is now in lsm/dev, thanks everyone!