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

On 1/24/20 11:28 AM, Casey Schaufler wrote:

On 1/24/2020 8:20 AM, Stephen Smalley wrote:

quoted

On 1/24/20 9:42 AM, Stephen Smalley wrote:

quoted

On 1/23/20 7:23 PM, Casey Schaufler wrote:

quoted

Add an entry /proc/.../attr/context which displays the full
process security "context" in compound format:'
          lsm1\0value\0lsm2\0value\0...
This entry is not writable.

Signed-off-by: Casey Schaufler <casey@schaufler-ca.com>
Cc: linux-api@vger.kernel.org

As previously discussed, there are issues with AppArmor's implementation of getprocattr() particularly around the trailing newline that dbus-daemon and perhaps others would like to see go away in any new interface.  Hence, I don't think we should implement this new API using the existing getprocattr() hook lest it also be locked into the current behavior forever.

Also, it would be good if whatever hook is introduced to support /proc/pid/attr/context could also be leveraged by the SO_PEERCONTEXT implementation in the future so that we are guaranteed a consistent result between the two interfaces, unlike the current situation for /proc/self/attr/current versus SO_PEERSEC.

I don't believe that a new hook is necessary, and that introducing one
just to deal with a '\n' would be pedantic. We really have two rational
options. AppArmor could drop the '\n' from their "context". Or, we can
simply document that the /proc/pid/attr/context interface will trim any
trailing whitespace. I understand that this would be a break from the
notion that the LSM infrastructure does not constrain what a module uses
for its own data. On the other hand, we have been saying that "context"s
are strings, and ignoring trailing whitespace is usual behavior for
strings.

AppArmor can drop the trailing '\n' it is not required. I would say it
could be dropped from /proc/pid/attr/current except there may be some
third party code that requires it.