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

2019-08-30 Thread Paul Moore
On Fri, Aug 30, 2019 at 11:31 AM Casey Schaufler  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 ...).

-- 
paul moore
www.paul-moore.com

--
Linux-audit mailing list
Linux-audit@redhat.com
https://www.redhat.com/mailman/listinfo/linux-audit


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

2019-08-30 Thread Casey Schaufler
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 
>> ---
>>   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;
>

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

2019-08-30 Thread Stephen Smalley

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 
---
  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

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

2019-08-15 Thread Aaron Goidel
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 
---
 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
d