Re: [PATCH v22 12/23] LSM: Specify which LSM to display

On 11/7/2020 2:05 PM, John Johansen wrote:

On 11/7/20 1:15 AM, Greg KH wrote:

quoted

On Fri, Nov 06, 2020 at 04:20:43PM -0800, Casey Schaufler wrote:

quoted

On 11/5/2020 1:22 AM, Greg KH wrote:

quoted

On Wed, Nov 04, 2020 at 03:41:03PM -0800, Casey Schaufler wrote:

quoted

Create a new entry "display" in the procfs attr directory for
controlling which LSM security information is displayed for a
process. A process can only read or write its own display value.

The name of an active LSM that supplies hooks for
human readable data may be written to "display" to set the
value. The name of the LSM currently in use can be read from
"display". At this point there can only be one LSM capable
of display active. A helper function lsm_task_display() is
provided to get the display slot for a task_struct.

Setting the "display" requires that all security modules using
setprocattr hooks allow the action. Each security module is
responsible for defining its policy.

AppArmor hook provided by John Johansen [off-list ref]
SELinux hook provided by Stephen Smalley [off-list ref]

Reviewed-by: Kees Cook <redacted>
Acked-by: Stephen Smalley <redacted>
Acked-by: Paul Moore <paul@paul-moore.com>
Signed-off-by: Casey Schaufler <casey@schaufler-ca.com>
Cc: linux-api@vger.kernel.org
---
 fs/proc/base.c                       |   1 +
 include/linux/lsm_hooks.h            |  17 +++
 security/apparmor/include/apparmor.h |   3 +-
 security/apparmor/lsm.c              |  32 +++++
 security/security.c                  | 169 ++++++++++++++++++++++++---
 security/selinux/hooks.c             |  11 ++
 security/selinux/include/classmap.h  |   2 +-
 security/smack/smack_lsm.c           |   7 ++
 8 files changed, 223 insertions(+), 19 deletions(-)

diff --git a/fs/proc/base.c b/fs/proc/base.c
index 0f707003dda5..7432f24f0132 100644
--- a/fs/proc/base.c
+++ b/fs/proc/base.c

@@ -2806,6 +2806,7 @@ static const struct pid_entry attr_dir_stuff[] = {
 	ATTR(NULL, "fscreate",		0666),
 	ATTR(NULL, "keycreate",		0666),
 	ATTR(NULL, "sockcreate",	0666),
+	ATTR(NULL, "display",		0666),

That's a vague name, any chance it can be more descriptive?

Sure. How about lsm_display, or display_lsm? I wouldn't say that
any of the files in /proc/*/attr have especially descriptive names,
but that's hardly an excuse.

I still don't understand what "display" means in this context.  Perhaps

its the LSM thats context is being displayed on the shared interface,
ie. /proc/*/attr/*

thinking about it more owner or even interface_owner might be a better
name

I was hoping for a single word, but I see how something more descriptive
might be in order. How about "lsm_of_current"? Or "lsm_of_dot_slash_current",
if you want to be pedantic. "format_of_current" isn't quite accurate, but
might be easier for some people to understand. Maybe "interface_owning_lsm".

/proc/*/attr/display answers the question "Which LSM is providing the data
I see if I look in /proc/*/attr/current, prev or exec or if that process uses
SO_PEERSEC".

quoted

documentation will help clear it up?

yeah this needs documented.

Agreed. I've noticed that nothing in /proc/*/attr seems documented
in an orderly (documentation/ABI) fashion. I will have to fix some of
that for a description of /proc/*/attr/whatever_it_ends_up_getting_called
to make sense. Working on it.

quoted

thanks,

greg k-h