Re: [PATCH v3 3/3] selinux: add permission names to trace event

[Stephen Smalley <stephen.smalley.work@gmail.com>]

On 8/18/20 12:09 PM, Steven Rostedt wrote:

> On Mon, 17 Aug 2020 16:29:33 -0400
> Steven Rostedt <rostedt@goodmis.org> wrote:
>
>> On Mon, 17 Aug 2020 16:13:29 -0400
>> Stephen Smalley <stephen.smalley.work@gmail.com> wrote:
>>
>>> Does this require a corresponding patch to userspace?  Otherwise, I get
>>> the following:
>>>
>>> libtraceevent: No such file or directory
>>>     [avc:selinux_audited] function avc_trace_perm_to_name not defined
>> Yes, we need to add a plugin to libtraceevent that will add that
>> function.
>>
>> I could possibly write one up real quick.
> Something like this (this is patched on top of trace-cmd, but will work
> for tools/lib/traceevent too).
>
> With CONFIG_TRACE_EVENT_INJECT enabled (to test events), I did the following:
>
>   # echo 'utclass=1 audited=1 denied=0' > /sys/kernel/tracing/events/avc/selinux_audited/inject
>   # trace-cmd extract
>   # trace-cmd report
> cpus=8
>             <...>-1685  [005]  1607.612032: selinux_audited:      requested=0x0 denied=0x0 audited=0x1 result=0 scontext= tcontext= tclass= permissions={ compute_av }
>
> Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
>
> ---
> diff --git a/lib/traceevent/plugins/Makefile b/lib/traceevent/plugins/Makefile
> index 21e933af..13cbcb92 100644
> --- a/lib/traceevent/plugins/Makefile
> +++ b/lib/traceevent/plugins/Makefile
> @@ -16,6 +16,7 @@ PLUGIN_OBJS += plugin_scsi.o
>   PLUGIN_OBJS += plugin_cfg80211.o
>   PLUGIN_OBJS += plugin_blk.o
>   PLUGIN_OBJS += plugin_tlb.o
> +PLUGIN_OBJS += plugin_avc.o
>   
>   PLUGIN_OBJS := $(PLUGIN_OBJS:%.o=$(bdir)/%.o)
>   PLUGIN_BUILD := $(PLUGIN_OBJS:$(bdir)/%.o=$(bdir)/%.so)
> diff --git a/lib/traceevent/plugins/plugin_avc.c b/lib/traceevent/plugins/plugin_avc.c
> new file mode 100644
> index 00000000..76af23b9
> --- /dev/null
> +++ b/lib/traceevent/plugins/plugin_avc.c
> @@ -0,0 +1,312 @@
> +// SPDX-License-Identifier: GPL-2.0
> +#include <stdio.h>
> +#include <string.h>
> +#include "event-parse.h"
> +
> +#define ARRAY_SIZE(arr) (sizeof(arr) / sizeof((arr)[0]))
> +
> +typedef unsigned short u16;
> +
> +/* Class/perm mapping support */
> +struct security_class_mapping {
> +	const char *name;
> +	const char *perms[sizeof(unsigned) * 8 + 1];
> +};
> +
> +#define COMMON_FILE_SOCK_PERMS "ioctl", "read", "write", "create", \
> +    "getattr", "setattr", "lock", "relabelfrom", "relabelto", "append", "map"
> +
> +#define COMMON_FILE_PERMS COMMON_FILE_SOCK_PERMS, "unlink", "link", \
> +    "rename", "execute", "quotaon", "mounton", "audit_access", \
> +	"open", "execmod", "watch", "watch_mount", "watch_sb", \
> +	"watch_with_perm", "watch_reads"
> +
> +#define COMMON_SOCK_PERMS COMMON_FILE_SOCK_PERMS, "bind", "connect", \
> +    "listen", "accept", "getopt", "setopt", "shutdown", "recvfrom",  \
> +    "sendto", "name_bind"
> +
> +#define COMMON_IPC_PERMS "create", "destroy", "getattr", "setattr", "read", \
> +	    "write", "associate", "unix_read", "unix_write"
> +
> +#define COMMON_CAP_PERMS  "chown", "dac_override", "dac_read_search", \
> +	    "fowner", "fsetid", "kill", "setgid", "setuid", "setpcap", \
> +	    "linux_immutable", "net_bind_service", "net_broadcast", \
> +	    "net_admin", "net_raw", "ipc_lock", "ipc_owner", "sys_module", \
> +	    "sys_rawio", "sys_chroot", "sys_ptrace", "sys_pacct", "sys_admin", \
> +	    "sys_boot", "sys_nice", "sys_resource", "sys_time", \
> +	    "sys_tty_config", "mknod", "lease", "audit_write", \
> +	    "audit_control", "setfcap"
> +
> +#define COMMON_CAP2_PERMS  "mac_override", "mac_admin", "syslog", \
> +		"wake_alarm", "block_suspend", "audit_read", "perfmon", "bpf"
> +
> +/*
> + * Note: The name for any socket class should be suffixed by "socket",
> + *	 and doesn't contain more than one substr of "socket".
> + */
> +struct security_class_mapping secclass_map[] = {
> +	{ "security",
> +	  { "compute_av", "compute_create", "compute_member",
> +	    "check_context", "load_policy", "compute_relabel",
> +	    "compute_user", "setenforce", "setbool", "setsecparam",
> +	    "setcheckreqprot", "read_policy", "validate_trans", NULL } },
>
So we'll need to update this plugin whenever we modify 
security/selinux/include/classmap.h to keep them in sync.  Is that a 
concern?  I don't suppose the plugin could directly include classmap.h?  
I guess we'd have to export it as a public header. It isn't considered 
to be part of the kernel API/ABI and can change anytime (but in practice 
changes are not that frequent, and usually just additive in nature).