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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <b444ffb9-3ea3-4ef4-b53c-954ea66f7037@schaufler-ca.com>
Date: Tue, 3 Sep 2024 17:53:13 -0700
From: Casey Schaufler <casey@...aufler-ca.com>
To: Paul Moore <paul@...l-moore.com>, linux-security-module@...r.kernel.org
Cc: jmorris@...ei.org, serge@...lyn.com, keescook@...omium.org,
 john.johansen@...onical.com, penguin-kernel@...ove.sakura.ne.jp,
 stephen.smalley.work@...il.com, linux-kernel@...r.kernel.org,
 selinux@...r.kernel.org, mic@...ikod.net, apparmor@...ts.ubuntu.com,
 bpf@...r.kernel.org, Casey Schaufler <casey@...aufler-ca.com>
Subject: Re: [PATCH v2 1/13] LSM: Add the lsmblob data structure.

On 9/3/2024 5:18 PM, Paul Moore wrote:
> On Aug 29, 2024 Casey Schaufler <casey@...aufler-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@...l-moore.com>
>> Signed-off-by: Casey Schaufler <casey@...aufler-ca.com>
>> Cc: apparmor@...ts.ubuntu.com
>> Cc: bpf@...r.kernel.org
>> Cc: selinux@...r.kernel.org
>> Cc: linux-security-module@...r.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
>