Re: [RFC PATCH] audit, security: allow LSMs to selectively enable audit collection

[Stephen Smalley <sds@xxxxxxxxxxxxx>]

On 8/15/19 1:41 PM, Aaron Goidel wrote:
> Presently, there is no way for LSMs to enable collection of supplemental
> audit records such as path and inode information when a permission denial
> occurs. Provide a LSM hook to allow LSMs to selectively enable collection
> on a per-task basis, even if the audit configuration would otherwise
> disable auditing of a task and/or contains no audit filter rules. If the
> hook returns a non-zero result, collect all available audit information. If
> the hook generates its own audit record, then supplemental audit
> information will be emitted at syscall exit.
>
> In SELinux, we implement this hook by returning the result of a permission
> check on the process. If the new process2:audit_enable permission is
> allowed by the policy, then audit collection will be enabled for that
> process. Otherwise, SELinux will defer to the audit configuration.

Any feedback on this RFC patch?  I know Paul provided some thoughts on 
the general topic of LSM/audit enablement in the other patch thread but 
I haven't seen any response to this patch.

> Signed-off-by: Aaron Goidel <acgoide@xxxxxxxxxxxxx>
> ---
>   include/linux/lsm_hooks.h           |  7 +++++++
>   include/linux/security.h            |  7 ++++++-
>   kernel/auditsc.c                    | 10 +++++++---
>   security/security.c                 |  5 +++++
>   security/selinux/hooks.c            | 11 +++++++++++
>   security/selinux/include/classmap.h |  2 +-
>   6 files changed, 37 insertions(+), 5 deletions(-)
>
> diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h
> index ead98af9c602..7d70a6759621 100644
> --- a/include/linux/lsm_hooks.h
> +++ b/include/linux/lsm_hooks.h
> @@ -1380,6 +1380,11 @@
>    *	audit_rule_init.
>    *	@lsmrule contains the allocated rule
>    *
> + * @audit_enable:
> + *	Allow the security module to selectively enable audit collection
> + *	on permission denials based on whether or not @tsk has the
> + *	process2:audit_enable permission.
> + *
>    * @inode_invalidate_secctx:
>    *	Notify the security module that it must revalidate the security context
>    *	of an inode.
> @@ -1800,6 +1805,7 @@ union security_list_options {
>   	int (*audit_rule_known)(struct audit_krule *krule);
>   	int (*audit_rule_match)(u32 secid, u32 field, u32 op, void *lsmrule);
>   	void (*audit_rule_free)(void *lsmrule);
> +	int (*audit_enable)(struct task_struct *tsk);
>   #endif /* CONFIG_AUDIT */
>   
>   #ifdef CONFIG_BPF_SYSCALL
> @@ -2043,6 +2049,7 @@ struct security_hook_heads {
>   	struct hlist_head audit_rule_known;
>   	struct hlist_head audit_rule_match;
>   	struct hlist_head audit_rule_free;
> +	struct hlist_head audit_enable;
>   #endif /* CONFIG_AUDIT */
>   #ifdef CONFIG_BPF_SYSCALL
>   	struct hlist_head bpf;
> diff --git a/include/linux/security.h b/include/linux/security.h
> index 7d9c1da1f659..7be66db8de4e 100644
> --- a/include/linux/security.h
> +++ b/include/linux/security.h
> @@ -1719,7 +1719,7 @@ int security_audit_rule_init(u32 field, u32 op, char *rulestr, void **lsmrule);
>   int security_audit_rule_known(struct audit_krule *krule);
>   int security_audit_rule_match(u32 secid, u32 field, u32 op, void *lsmrule);
>   void security_audit_rule_free(void *lsmrule);
> -
> +int security_audit_enable(struct task_struct *tsk);
>   #else
>   
>   static inline int security_audit_rule_init(u32 field, u32 op, char *rulestr,
> @@ -1742,6 +1742,11 @@ static inline int security_audit_rule_match(u32 secid, u32 field, u32 op,
>   static inline void security_audit_rule_free(void *lsmrule)
>   { }
>   
> +static inline int security_audit_enable(struct task_struct *tsk)
> +{
> +	return 0;
> +}
> +
>   #endif /* CONFIG_SECURITY */
>   #endif /* CONFIG_AUDIT */
>   
> diff --git a/kernel/auditsc.c b/kernel/auditsc.c
> index 95ae27edd417..7e052b71bc42 100644
> --- a/kernel/auditsc.c
> +++ b/kernel/auditsc.c
> @@ -906,8 +906,12 @@ int audit_alloc(struct task_struct *tsk)
>   
>   	state = audit_filter_task(tsk, &key);
>   	if (state == AUDIT_DISABLED) {
> -		clear_tsk_thread_flag(tsk, TIF_SYSCALL_AUDIT);
> -		return 0;
> +		if (security_audit_enable(tsk)) {
> +			state = AUDIT_BUILD_CONTEXT;
> +		} else {
> +			clear_tsk_thread_flag(tsk, TIF_SYSCALL_AUDIT);
> +			return 0;
> +		}
>   	}
>   
>   	if (!(context = audit_alloc_context(state))) {
> @@ -1623,7 +1627,7 @@ void __audit_syscall_entry(int major, unsigned long a1, unsigned long a2,
>   	if (state == AUDIT_DISABLED)
>   		return;
>   
> -	context->dummy = !audit_n_rules;
> +	context->dummy = !audit_n_rules && !security_audit_enable(current);
>   	if (!context->dummy && state == AUDIT_BUILD_CONTEXT) {
>   		context->prio = 0;
>   		if (auditd_test_task(current))
> diff --git a/security/security.c b/security/security.c
> index 30687e1366b7..04e160e5d4ab 100644
> --- a/security/security.c
> +++ b/security/security.c
> @@ -2333,6 +2333,11 @@ int security_audit_rule_match(u32 secid, u32 field, u32 op, void *lsmrule)
>   {
>   	return call_int_hook(audit_rule_match, 0, secid, field, op, lsmrule);
>   }
> +
> +int security_audit_enable(struct task_struct *tsk)
> +{
> +	return call_int_hook(audit_enable, 0, tsk);
> +}
>   #endif /* CONFIG_AUDIT */
>   
>   #ifdef CONFIG_BPF_SYSCALL
> diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
> index d55571c585ff..88764aa0ab43 100644
> --- a/security/selinux/hooks.c
> +++ b/security/selinux/hooks.c
> @@ -6628,6 +6628,16 @@ static void selinux_ib_free_security(void *ib_sec)
>   }
>   #endif
>   
> +#ifdef CONFIG_AUDIT
> +static int selinux_audit_enable(struct task_struct *tsk)
> +{
> +	u32 sid = current_sid();
> +
> +	return !avc_has_perm(&selinux_state, sid, sid, SECCLASS_PROCESS2,
> +			PROCESS2__AUDIT_ENABLE, NULL);
> +}
> +#endif
> +
>   #ifdef CONFIG_BPF_SYSCALL
>   static int selinux_bpf(int cmd, union bpf_attr *attr,
>   				     unsigned int size)
> @@ -6999,6 +7009,7 @@ static struct security_hook_list selinux_hooks[] __lsm_ro_after_init = {
>   	LSM_HOOK_INIT(audit_rule_known, selinux_audit_rule_known),
>   	LSM_HOOK_INIT(audit_rule_match, selinux_audit_rule_match),
>   	LSM_HOOK_INIT(audit_rule_free, selinux_audit_rule_free),
> +	LSM_HOOK_INIT(audit_enable, selinux_audit_enable),
>   #endif
>   
>   #ifdef CONFIG_BPF_SYSCALL
> diff --git a/security/selinux/include/classmap.h b/security/selinux/include/classmap.h
> index 32e9b03be3dd..d7d856cbd486 100644
> --- a/security/selinux/include/classmap.h
> +++ b/security/selinux/include/classmap.h
> @@ -52,7 +52,7 @@ struct security_class_mapping secclass_map[] = {
>   	    "execmem", "execstack", "execheap", "setkeycreate",
>   	    "setsockcreate", "getrlimit", NULL } },
>   	{ "process2",
> -	  { "nnp_transition", "nosuid_transition", NULL } },
> +	  { "nnp_transition", "nosuid_transition", "audit_enable", NULL } },
>   	{ "system",
>   	  { "ipc_info", "syslog_read", "syslog_mod",
>   	    "syslog_console", "module_request", "module_load", NULL } },