| Message ID | 20221025184519.13231-4-casey@schaufler-ca.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Paul Moore |
| Headers | show |
| Series | LSM: Two basic syscalls | expand |
On Tue, Oct 25, 2022 at 11:45:14AM -0700, Casey Schaufler wrote: > Add an integer member "features" to the struct lsm_id which > identifies the API related data associated with each security > module. The initial set of features maps to information that > has traditionaly been available in /proc/self/attr. > > Signed-off-by: Casey Schaufler <casey@schaufler-ca.com> > --- > include/linux/lsm_hooks.h | 1 + > include/uapi/linux/lsm.h | 14 ++++++++++++++ > security/apparmor/lsm.c | 1 + > security/selinux/hooks.c | 2 ++ > security/smack/smack_lsm.c | 1 + > 5 files changed, 19 insertions(+) > > diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h > index dd4b4d95a172..46b2aa6a677e 100644 > --- a/include/linux/lsm_hooks.h > +++ b/include/linux/lsm_hooks.h > @@ -1608,6 +1608,7 @@ struct security_hook_heads { > struct lsm_id { > const char *lsm; /* Name of the LSM */ > int id; /* LSM ID */ > + int features; /* Set of LSM features */ Again, be explicit about size please. And documentation. > }; > > /* > diff --git a/include/uapi/linux/lsm.h b/include/uapi/linux/lsm.h > index d5bcbb9375df..61e13b1b9ece 100644 > --- a/include/uapi/linux/lsm.h > +++ b/include/uapi/linux/lsm.h > @@ -29,4 +29,18 @@ > #define LSM_ID_BPF 42 > #define LSM_ID_LANDLOCK 43 > > +/* > + * LSM_ATTR_XXX values identify the /proc/.../attr entry that the > + * context represents. Not all security modules provide all of these > + * values. Some security modules provide none of them. > + */ > +/* clang-format off */ Why this comment? That shouldn't be in uapi files. Or any header files. > +#define LSM_ATTR_CURRENT (1UL << 0) > +#define LSM_ATTR_EXEC (1UL << 1) > +#define LSM_ATTR_FSCREATE (1UL << 2) > +#define LSM_ATTR_KEYCREATE (1UL << 3) > +#define LSM_ATTR_PREV (1UL << 4) > +#define LSM_ATTR_SOCKCREATE (1UL << 5) > +/* clang-format on */ Again, please drop. Where is it documented what these attributes actually mean? thanks, greg k-h
On Tue, Oct 25, 2022 at 2:47 PM Casey Schaufler <casey@schaufler-ca.com> wrote: > > Add an integer member "features" to the struct lsm_id which > identifies the API related data associated with each security > module. The initial set of features maps to information that > has traditionaly been available in /proc/self/attr. > > Signed-off-by: Casey Schaufler <casey@schaufler-ca.com> > --- > include/linux/lsm_hooks.h | 1 + > include/uapi/linux/lsm.h | 14 ++++++++++++++ > security/apparmor/lsm.c | 1 + > security/selinux/hooks.c | 2 ++ > security/smack/smack_lsm.c | 1 + > 5 files changed, 19 insertions(+) Everything Greg already said with one additional comment below. > diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h > index dd4b4d95a172..46b2aa6a677e 100644 > --- a/include/linux/lsm_hooks.h > +++ b/include/linux/lsm_hooks.h > @@ -1608,6 +1608,7 @@ struct security_hook_heads { > struct lsm_id { > const char *lsm; /* Name of the LSM */ > int id; /* LSM ID */ > + int features; /* Set of LSM features */ I understand why you called the field "features", but I worry it is a bit too generic for 32-bits of flags. Let's make it specific to the LSM label attributes; how about 'feat_attr', 'sup_attr', or something along those lines? -- paul-moore.com
On 11/9/2022 3:34 PM, Paul Moore wrote: > On Tue, Oct 25, 2022 at 2:47 PM Casey Schaufler <casey@schaufler-ca.com> wrote: >> Add an integer member "features" to the struct lsm_id which >> identifies the API related data associated with each security >> module. The initial set of features maps to information that >> has traditionaly been available in /proc/self/attr. >> >> Signed-off-by: Casey Schaufler <casey@schaufler-ca.com> >> --- >> include/linux/lsm_hooks.h | 1 + >> include/uapi/linux/lsm.h | 14 ++++++++++++++ >> security/apparmor/lsm.c | 1 + >> security/selinux/hooks.c | 2 ++ >> security/smack/smack_lsm.c | 1 + >> 5 files changed, 19 insertions(+) > Everything Greg already said with one additional comment below. > >> diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h >> index dd4b4d95a172..46b2aa6a677e 100644 >> --- a/include/linux/lsm_hooks.h >> +++ b/include/linux/lsm_hooks.h >> @@ -1608,6 +1608,7 @@ struct security_hook_heads { >> struct lsm_id { >> const char *lsm; /* Name of the LSM */ >> int id; /* LSM ID */ >> + int features; /* Set of LSM features */ > I understand why you called the field "features", but I worry it is a > bit too generic for 32-bits of flags. Let's make it specific to the > LSM label attributes; how about 'feat_attr', 'sup_attr', or something > along those lines? How about 'attrs_used'? I'm open to anything except 'late_for_dinner' :) > -- > paul-moore.com
On Wed, Nov 9, 2022 at 8:03 PM Casey Schaufler <casey@schaufler-ca.com> wrote: > On 11/9/2022 3:34 PM, Paul Moore wrote: > > On Tue, Oct 25, 2022 at 2:47 PM Casey Schaufler <casey@schaufler-ca.com> wrote: > >> Add an integer member "features" to the struct lsm_id which > >> identifies the API related data associated with each security > >> module. The initial set of features maps to information that > >> has traditionaly been available in /proc/self/attr. > >> > >> Signed-off-by: Casey Schaufler <casey@schaufler-ca.com> > >> --- > >> include/linux/lsm_hooks.h | 1 + > >> include/uapi/linux/lsm.h | 14 ++++++++++++++ > >> security/apparmor/lsm.c | 1 + > >> security/selinux/hooks.c | 2 ++ > >> security/smack/smack_lsm.c | 1 + > >> 5 files changed, 19 insertions(+) > > Everything Greg already said with one additional comment below. > > > >> diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h > >> index dd4b4d95a172..46b2aa6a677e 100644 > >> --- a/include/linux/lsm_hooks.h > >> +++ b/include/linux/lsm_hooks.h > >> @@ -1608,6 +1608,7 @@ struct security_hook_heads { > >> struct lsm_id { > >> const char *lsm; /* Name of the LSM */ > >> int id; /* LSM ID */ > >> + int features; /* Set of LSM features */ > > I understand why you called the field "features", but I worry it is a > > bit too generic for 32-bits of flags. Let's make it specific to the > > LSM label attributes; how about 'feat_attr', 'sup_attr', or something > > along those lines? > > How about 'attrs_used'? I'm open to anything except 'late_for_dinner' :) Works for me. It's also worth noting that this struct isn't part of the UAPI so if we need to change it in the future we can.
diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h index dd4b4d95a172..46b2aa6a677e 100644 --- a/include/linux/lsm_hooks.h +++ b/include/linux/lsm_hooks.h @@ -1608,6 +1608,7 @@ struct security_hook_heads { struct lsm_id { const char *lsm; /* Name of the LSM */ int id; /* LSM ID */ + int features; /* Set of LSM features */ }; /* diff --git a/include/uapi/linux/lsm.h b/include/uapi/linux/lsm.h index d5bcbb9375df..61e13b1b9ece 100644 --- a/include/uapi/linux/lsm.h +++ b/include/uapi/linux/lsm.h @@ -29,4 +29,18 @@ #define LSM_ID_BPF 42 #define LSM_ID_LANDLOCK 43 +/* + * LSM_ATTR_XXX values identify the /proc/.../attr entry that the + * context represents. Not all security modules provide all of these + * values. Some security modules provide none of them. + */ +/* clang-format off */ +#define LSM_ATTR_CURRENT (1UL << 0) +#define LSM_ATTR_EXEC (1UL << 1) +#define LSM_ATTR_FSCREATE (1UL << 2) +#define LSM_ATTR_KEYCREATE (1UL << 3) +#define LSM_ATTR_PREV (1UL << 4) +#define LSM_ATTR_SOCKCREATE (1UL << 5) +/* clang-format on */ + #endif /* _UAPI_LINUX_LSM_H */ diff --git a/security/apparmor/lsm.c b/security/apparmor/lsm.c index b859b1af6c75..77260026fda0 100644 --- a/security/apparmor/lsm.c +++ b/security/apparmor/lsm.c @@ -1206,6 +1206,7 @@ struct lsm_blob_sizes apparmor_blob_sizes __lsm_ro_after_init = { static struct lsm_id apparmor_lsmid __lsm_ro_after_init = { .lsm = "apparmor", .id = LSM_ID_APPARMOR, + .features = LSM_ATTR_CURRENT | LSM_ATTR_PREV | LSM_ATTR_EXEC, }; static struct security_hook_list apparmor_hooks[] __lsm_ro_after_init = { diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c index 5fcce36267bd..107b944e5d45 100644 --- a/security/selinux/hooks.c +++ b/security/selinux/hooks.c @@ -7018,6 +7018,8 @@ static int selinux_uring_cmd(struct io_uring_cmd *ioucmd) static struct lsm_id selinux_lsmid __lsm_ro_after_init = { .lsm = "selinux", .id = LSM_ID_SELINUX, + .features = LSM_ATTR_CURRENT | LSM_ATTR_EXEC | LSM_ATTR_FSCREATE | + LSM_ATTR_KEYCREATE | LSM_ATTR_PREV | LSM_ATTR_SOCKCREATE, }; /* diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c index c7ba80e20b8d..12ff27c00fe6 100644 --- a/security/smack/smack_lsm.c +++ b/security/smack/smack_lsm.c @@ -4791,6 +4791,7 @@ struct lsm_blob_sizes smack_blob_sizes __lsm_ro_after_init = { static struct lsm_id smack_lsmid __lsm_ro_after_init = { .lsm = "smack", .id = LSM_ID_SMACK, + .features = LSM_ATTR_CURRENT, }; static struct security_hook_list smack_hooks[] __lsm_ro_after_init = {
Add an integer member "features" to the struct lsm_id which identifies the API related data associated with each security module. The initial set of features maps to information that has traditionaly been available in /proc/self/attr. Signed-off-by: Casey Schaufler <casey@schaufler-ca.com> --- include/linux/lsm_hooks.h | 1 + include/uapi/linux/lsm.h | 14 ++++++++++++++ security/apparmor/lsm.c | 1 + security/selinux/hooks.c | 2 ++ security/smack/smack_lsm.c | 1 + 5 files changed, 19 insertions(+)