On 8/30/2019 6:44 AM, Stephen Smalley wrote:
> 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.
Audit policy should be independent of security module policy.
I shouldn't have to change SELinux policy to enable this data
collection. I should be able to change the audit configuration
to get this if I want it.
>
>>
>> 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 } },
>>
>
