Re: [PATCH v2 1/13] LSM: Add the lsmblob data structure.

[Casey Schaufler <casey@schaufler-ca.com>]

On 9/3/2024 5:18 PM, Paul Moore wrote:
> On Aug 29, 2024 Casey Schaufler <casey@schaufler-ca.com> wrote:
>> When more than one security module is exporting data to audit and
>> networking sub-systems a single 32 bit integer is no longer
>> sufficient to represent the data. Add a structure to be used instead.
>>
>> The lsmblob structure definition is intended to keep the LSM
>> specific information private to the individual security modules.
>> The module specific information is included in a new set of
>> header files under include/lsm. Each security module is allowed
>> to define the information included for its use in the lsmblob.
>> SELinux includes a u32 secid. Smack includes a pointer into its
>> global label list. The conditional compilation based on feature
>> inclusion is contained in the include/lsm files.
>>
>> Suggested-by: Paul Moore <paul@paul-moore.com>
>> Signed-off-by: Casey Schaufler <casey@schaufler-ca.com>
>> Cc: apparmor@lists.ubuntu.com
>> Cc: bpf@vger.kernel.org
>> Cc: selinux@vger.kernel.org
>> Cc: linux-security-module@vger.kernel.org
>> ---
>>  include/linux/lsm/apparmor.h | 17 +++++++++++++++++
>>  include/linux/lsm/bpf.h      | 16 ++++++++++++++++
>>  include/linux/lsm/selinux.h  | 16 ++++++++++++++++
>>  include/linux/lsm/smack.h    | 17 +++++++++++++++++
>>  include/linux/security.h     | 20 ++++++++++++++++++++
>>  5 files changed, 86 insertions(+)
>>  create mode 100644 include/linux/lsm/apparmor.h
>>  create mode 100644 include/linux/lsm/bpf.h
>>  create mode 100644 include/linux/lsm/selinux.h
>>  create mode 100644 include/linux/lsm/smack.h
> ..
>
>> diff --git a/include/linux/security.h b/include/linux/security.h
>> index 1390f1efb4f0..0057a22137e8 100644
>> --- a/include/linux/security.h
>> +++ b/include/linux/security.h
>> @@ -34,6 +34,10 @@
>>  #include <linux/sockptr.h>
>>  #include <linux/bpf.h>
>>  #include <uapi/linux/lsm.h>
>> +#include <linux/lsm/selinux.h>
>> +#include <linux/lsm/smack.h>
>> +#include <linux/lsm/apparmor.h>
>> +#include <linux/lsm/bpf.h>
>>  
>>  struct linux_binprm;
>>  struct cred;
>> @@ -140,6 +144,22 @@ enum lockdown_reason {
>>  	LOCKDOWN_CONFIDENTIALITY_MAX,
>>  };
>>  
>> +/* scaffolding */
>> +struct lsmblob_scaffold {
>> +	u32 secid;
>> +};
>> +
>> +/*
>> + * Data exported by the security modules
>> + */
>> +struct lsmblob {
>> +	struct lsmblob_selinux selinux;
>> +	struct lsmblob_smack smack;
>> +	struct lsmblob_apparmor apparmor;
>> +	struct lsmblob_bpf bpf;
>> +	struct lsmblob_scaffold scaffold;
>> +};
> Warning, top shelf bikeshedding follows ...

Not unexpected. :)

> I believe that historically when we've talked about the "LSM blob" we've
> usually been referring to the opaque buffers used to store LSM state that
> we attach to a number of kernel structs using the `void *security` field.
>
> At least that is what I think of when I read "struct lsmblob", and I'd
> like to get ahead of the potential confusion while we still can.
>
> Casey, I'm sure you're priority is simply getting this merged and you
> likely care very little about the name (as long as it isn't too horrible),

I would reject lsmlatefordinner out of hand.

> but what about "lsm_ref"?  Other ideas are most definitely welcome.

I'm not a fan of the underscore, and ref seems to imply memory management.
How about "struct lsmsecid", which is a nod to the past "u32 secid"?
Or, "struct lsmdata", "struct lsmid", "struct lsmattr".
I could live with "struct lsmref", I suppose, although it pulls me toward
"struct lsmreference", which is a bit long.

> I'm not going to comment on all the other related occurrences in the
> patchset, but all the "XXX_lsmblob_XXX" functions should be adjusted based
> on what we name the struct, e.g. "XXX_lsmref_XXX".
>
> --
> paul-moore.com
>