Re: [PATCH v2 3/13] LSM: Add lsmblob_to_secctx hook

[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:
>> Add a new hook security_lsmblob_to_secctx() and its LSM specific
>> implementations. The LSM specific code will use the lsmblob element
>> allocated for that module. This allows for the possibility that more
>> than one module may be called upon to translate a secid to a string,
>> as can occur in the audit code.
>>
>> Signed-off-by: Casey Schaufler <casey@schaufler-ca.com>
>> ---
>>  include/linux/lsm_hook_defs.h     |  2 ++
>>  include/linux/security.h          | 11 ++++++++++-
>>  security/apparmor/include/secid.h |  2 ++
>>  security/apparmor/lsm.c           |  1 +
>>  security/apparmor/secid.c         | 25 +++++++++++++++++++++++--
>>  security/security.c               | 30 ++++++++++++++++++++++++++++++
>>  security/selinux/hooks.c          | 16 ++++++++++++++--
>>  security/smack/smack_lsm.c        | 31 ++++++++++++++++++++++++++-----
>>  8 files changed, 108 insertions(+), 10 deletions(-)
> ..
>
>> diff --git a/security/security.c b/security/security.c
>> index 64a6d6bbd1f4..bb541a3be410 100644
>> --- a/security/security.c
>> +++ b/security/security.c
>> @@ -4192,6 +4192,36 @@ int security_secid_to_secctx(u32 secid, char **secdata, u32 *seclen)
>>  }
>>  EXPORT_SYMBOL(security_secid_to_secctx);
>>  
>> +/**
>> + * security_lsmblob_to_secctx() - Convert a lsmblob to a secctx
>> + * @blob: lsm specific information
>> + * @secdata: secctx
>> + * @seclen: secctx length
>> + *
>> + * Convert a @blob entry to security context.  If @secdata is NULL the
>> + * length of the result will be returned in @seclen, but no @secdata
>> + * will be returned.  This does mean that the length could change between
>> + * calls to check the length and the next call which actually allocates
>> + * and returns the @secdata.
>> + *
>> + * Return: Return 0 on success, error on failure.
>> + */
>> +int security_lsmblob_to_secctx(struct lsmblob *blob, char **secdata,
>> +			       u32 *seclen)
>> +{
>> +	struct security_hook_list *hp;
>> +	int rc;
>> +
>> +	hlist_for_each_entry(hp, &security_hook_heads.secid_to_secctx, list) {
>> +		rc = hp->hook.lsmblob_to_secctx(blob, secdata, seclen);
>> +		if (rc != LSM_RET_DEFAULT(secid_to_secctx))
> Wrong default value/hook, but see below ...
>
>> +			return rc;
>> +	}
>> +
>> +	return LSM_RET_DEFAULT(secid_to_secctx);
> Same problem, I'm guessing a cut-n-paste-o.
>
>> +}
>> +EXPORT_SYMBOL(security_lsmblob_to_secctx);
> We should be using the call_int_hook() macro instead of open coding using
> hlist_for_each_entry() and I believe the code above could be converted
> without any difficulty.
>
> It should also solve the compile problem seen when using lsm/dev or
> lsm/next as the base.

Yup, sorry for being sloppy. Will get fixed in v3.

>
>>  /**
>>   * security_secctx_to_secid() - Convert a secctx to a secid
>>   * @secdata: secctx
>> diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
>> index 55c78c318ccd..102489e6d579 100644
>> --- a/security/selinux/hooks.c
>> +++ b/security/selinux/hooks.c
>> @@ -6610,8 +6610,19 @@ static int selinux_ismaclabel(const char *name)
>>  
>>  static int selinux_secid_to_secctx(u32 secid, char **secdata, u32 *seclen)
>>  {
>> -	return security_sid_to_context(secid,
>> -				       secdata, seclen);
>> +	return security_sid_to_context(secid, secdata, seclen);
>> +}
>> +
>> +static int selinux_lsmblob_to_secctx(struct lsmblob *blob, char **secdata,
>> +				     u32 *seclen)
>> +{
>> +	u32 secid = blob->selinux.secid;
>> +
>> +	/* scaffolding */
>> +	if (!secid)
>> +		secid = blob->scaffold.secid;
>> +
>> +	return security_sid_to_context(secid, secdata, seclen);
> We should probably just call selinux_secid_to_secctx() here so we limit
> the code dup/sync issues.

In SELinux code I'll defer to your style choices.

>>  }
> --
> paul-moore.com
>