2022-10-25 18:56:23

by Casey Schaufler

[permalink] [raw]
Subject: [PATCH v1 3/8] LSM: Identify the process attributes for each module

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 <[email protected]>
---
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 */
};

/*
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 = {
--
2.37.3



2022-10-26 06:02:21

by Greg Kroah-Hartman

[permalink] [raw]
Subject: Re: [PATCH v1 3/8] LSM: Identify the process attributes for each module

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 <[email protected]>
> ---
> 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

2022-11-09 23:46:02

by Paul Moore

[permalink] [raw]
Subject: Re: [PATCH v1 3/8] LSM: Identify the process attributes for each module

On Tue, Oct 25, 2022 at 2:47 PM Casey Schaufler <[email protected]> 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 <[email protected]>
> ---
> 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

2022-11-10 01:43:26

by Casey Schaufler

[permalink] [raw]
Subject: Re: [PATCH v1 3/8] LSM: Identify the process attributes for each module

On 11/9/2022 3:34 PM, Paul Moore wrote:
> On Tue, Oct 25, 2022 at 2:47 PM Casey Schaufler <[email protected]> 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 <[email protected]>
>> ---
>> 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

2022-11-10 03:05:25

by Paul Moore

[permalink] [raw]
Subject: Re: [PATCH v1 3/8] LSM: Identify the process attributes for each module

On Wed, Nov 9, 2022 at 8:03 PM Casey Schaufler <[email protected]> wrote:
> On 11/9/2022 3:34 PM, Paul Moore wrote:
> > On Tue, Oct 25, 2022 at 2:47 PM Casey Schaufler <[email protected]> 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 <[email protected]>
> >> ---
> >> 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.

--
paul-moore.com