diff mbox series

[v1,3/8] LSM: Identify the process attributes for each module

Message ID 20221025184519.13231-4-casey@schaufler-ca.com
State New
Headers
Series LSM: Two basic syscalls |

Commit Message

Casey Schaufler Oct. 25, 2022, 6:45 p.m. UTC 
  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(+)

Comments

Greg KH Oct. 26, 2022, 5:59 a.m. UTC | #1 
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
Paul Moore Nov. 9, 2022, 11:34 p.m. UTC | #2 
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
Casey Schaufler Nov. 10, 2022, 1:03 a.m. UTC | #3 
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
Paul Moore Nov. 10, 2022, 2:39 a.m. UTC | #4 
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 mbox series

Patch

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 = {