[PATCH v14 22/23] LSM: Add /proc attr entry for full LSM context

[Casey Schaufler]

On 2/3/2020 11:37 AM, Stephen Smalley wrote:
> On 2/3/20 1:54 PM, Stephen Smalley wrote:
>> I'd suggest something like the following instead:
>> * @getprocattr
>> *   Get the value of process attribute @name for task @p into a buffer
>> *   allocated by the security module and returned via @value.  The
>> *   caller will free the returned buffer via kfree.  The set of
>> *   attribute names is fixed by proc but the format of @value is up
>> *   to the security module authors except for the "context" attribute,
>> *   whose value is required to be a NUL-terminated printable ASCII
>> *   string without trailing whitespace.
>> *   @p the task whose attribute is being fetched
>> *   @name the name of the process attribute being fetched
>> *   @value set to point to the buffer containing the attribute value
>> *   Return the length of @value including the NUL on success, or -errno on error.

I'm fine with either description, so I'll adopt yours.

>>
>> The printable ASCII bit is based on what the dbus maintainer requested in previous discussions.  The question of whether the terminating NUL should be included in the returned length was otherwise left ambiguous and inconsistent in your patch among the different security modules; if you prefer not including it in the length returned by the security modules, you'll need to adjust SELinux at least to not do so for "context".

append_ctx() is supposed to take the possibility that the module specific context string
may or may not include a trailing '\0' into account. Alas, I see a problem in the current
version for the "no trailing '\0'" case. On the other hand, with your proposed description
the trailing '\0' is required, so it's a matter of verifying that all modules providing
a "context" provide one that includes a trailing '\0' and return strlen()+1.

>
> BTW, I think the above guarantees with the exception of no trailing whitespace and whether the NUL byte is included or excluded from length are in reality also required for "current" (and SO_PEERSEC) or existing userspace will break.

The behavior of interfaces (e.g. "current", "exec") that are module defined
is only of concern with respect to to "display" behavior. If a security module
wants to provide a variable size binary blob in "current" I would object on
principle, but policy as I understand it has long been that if the authors want
to do that, it's their call. The "context" has a defined format, and it would
be up to the authors to come up with a printable ASCII representation of the
binary blob. If they care. They're not required to provide a "context".