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

Fri Aug 30 15:31:31 UTC 2019

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 at tycho.nsa.gov>
>> ---
>>   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 } },
>>
>