| Message ID | 20190815174111.6309-1-acgoide@tycho.nsa.gov (mailing list archive) |
|---|---|
| State | New, archived |
| Delegated to: | Paul Moore |
| Headers | show |
| Series | [RFC] audit, security: allow LSMs to selectively enable audit collection | expand |
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@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 } }, >
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@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 } }, >> >
On Fri, Aug 30, 2019 at 11:31 AM Casey Schaufler <casey@schaufler-ca.com> wrote: > 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. The idea is that the LSM can request that the audit subsystem selectively enable auditing (per-task, and hopefully per-record-type); the audit policy can still be configured as you normally. This is to work around people (and distros) that disable audit, yet still want to audit some information (yeah, I know ...).
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 } },
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. Signed-off-by: Aaron Goidel <acgoide@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(-)