| Message ID | e9c1216a361c38ebc9cb4089922c259e2cfd5013.1593198710.git.rgb@redhat.com |
|---|---|
| State | Not Applicable |
| Delegated to: | David Miller |
| Headers | show |
| Series | audit: implement container identifier | expand |
On Sat, Jun 27, 2020 at 9:23 AM Richard Guy Briggs <rgb@redhat.com> wrote: > > This also adds support to qualify NETFILTER_PKT records. > > Audit events could happen in a network namespace outside of a task > context due to packets received from the net that trigger an auditing > rule prior to being associated with a running task. The network > namespace could be in use by multiple containers by association to the > tasks in that network namespace. We still want a way to attribute > these events to any potential containers. Keep a list per network > namespace to track these audit container identifiiers. > > Add/increment the audit container identifier on: > - initial setting of the audit container identifier via /proc > - clone/fork call that inherits an audit container identifier > - unshare call that inherits an audit container identifier > - setns call that inherits an audit container identifier > Delete/decrement the audit container identifier on: > - an inherited audit container identifier dropped when child set > - process exit > - unshare call that drops a net namespace > - setns call that drops a net namespace > > Add audit container identifier auxiliary record(s) to NETFILTER_PKT > event standalone records. Iterate through all potential audit container > identifiers associated with a network namespace. > > Please see the github audit kernel issue for contid net support: > https://github.com/linux-audit/audit-kernel/issues/92 > Please see the github audit testsuiite issue for the test case: > https://github.com/linux-audit/audit-testsuite/issues/64 > Please see the github audit wiki for the feature overview: > https://github.com/linux-audit/audit-kernel/wiki/RFE-Audit-Container-ID > Signed-off-by: Richard Guy Briggs <rgb@redhat.com> > Acked-by: Neil Horman <nhorman@tuxdriver.com> > Reviewed-by: Ondrej Mosnacek <omosnace@redhat.com> > --- > include/linux/audit.h | 20 ++++++ > kernel/audit.c | 156 ++++++++++++++++++++++++++++++++++++++++++++++- > kernel/nsproxy.c | 4 ++ > net/netfilter/nft_log.c | 11 +++- > net/netfilter/xt_AUDIT.c | 11 +++- > 5 files changed, 195 insertions(+), 7 deletions(-) > > diff --git a/include/linux/audit.h b/include/linux/audit.h > index c4a755ae0d61..304fbb7c3c5b 100644 > --- a/include/linux/audit.h > +++ b/include/linux/audit.h > @@ -128,6 +128,13 @@ struct audit_task_info { > > extern struct audit_task_info init_struct_audit; > > +struct audit_contobj_netns { > + struct list_head list; > + struct audit_contobj *obj; > + int count; This seems like it might be a good candidate for refcount_t, yes? > + struct rcu_head rcu; > +}; ... > diff --git a/kernel/audit.c b/kernel/audit.c > index 997c34178ee8..a862721dfd9b 100644 > --- a/kernel/audit.c > +++ b/kernel/audit.c > @@ -437,6 +452,136 @@ static struct sock *audit_get_sk(const struct net *net) > return aunet->sk; > } > > +void audit_netns_contid_add(struct net *net, struct audit_contobj *cont) > +{ > + struct audit_net *aunet; > + struct list_head *contobj_list; > + struct audit_contobj_netns *contns; > + > + if (!net) > + return; > + if (!cont) > + return; > + aunet = net_generic(net, audit_net_id); > + if (!aunet) > + return; > + contobj_list = &aunet->contobj_list; > + rcu_read_lock(); > + spin_lock(&aunet->contobj_list_lock); > + list_for_each_entry_rcu(contns, contobj_list, list) > + if (contns->obj == cont) { > + contns->count++; > + goto out; > + } > + contns = kmalloc(sizeof(*contns), GFP_ATOMIC); > + if (contns) { > + INIT_LIST_HEAD(&contns->list); > + contns->obj = cont; > + contns->count = 1; > + list_add_rcu(&contns->list, contobj_list); > + } > +out: > + spin_unlock(&aunet->contobj_list_lock); > + rcu_read_unlock(); > +} > + > +void audit_netns_contid_del(struct net *net, struct audit_contobj *cont) > +{ > + struct audit_net *aunet; > + struct list_head *contobj_list; > + struct audit_contobj_netns *contns = NULL; > + > + if (!net) > + return; > + if (!cont) > + return; > + aunet = net_generic(net, audit_net_id); > + if (!aunet) > + return; > + contobj_list = &aunet->contobj_list; > + rcu_read_lock(); > + spin_lock(&aunet->contobj_list_lock); > + list_for_each_entry_rcu(contns, contobj_list, list) > + if (contns->obj == cont) { > + contns->count--; > + if (contns->count < 1) { One could simplify this with "(--countns->count) < 1", although if it is changed to a refcount_t (which seems like a smart thing), the normal decrement/test would be the best choice. > + list_del_rcu(&contns->list); > + kfree_rcu(contns, rcu); > + } > + break; > + } > + spin_unlock(&aunet->contobj_list_lock); > + rcu_read_unlock(); > +} -- paul moore www.paul-moore.com
On 2020-07-05 11:11, Paul Moore wrote: > On Sat, Jun 27, 2020 at 9:23 AM Richard Guy Briggs <rgb@redhat.com> wrote: > > > > This also adds support to qualify NETFILTER_PKT records. > > > > Audit events could happen in a network namespace outside of a task > > context due to packets received from the net that trigger an auditing > > rule prior to being associated with a running task. The network > > namespace could be in use by multiple containers by association to the > > tasks in that network namespace. We still want a way to attribute > > these events to any potential containers. Keep a list per network > > namespace to track these audit container identifiiers. > > > > Add/increment the audit container identifier on: > > - initial setting of the audit container identifier via /proc > > - clone/fork call that inherits an audit container identifier > > - unshare call that inherits an audit container identifier > > - setns call that inherits an audit container identifier > > Delete/decrement the audit container identifier on: > > - an inherited audit container identifier dropped when child set > > - process exit > > - unshare call that drops a net namespace > > - setns call that drops a net namespace > > > > Add audit container identifier auxiliary record(s) to NETFILTER_PKT > > event standalone records. Iterate through all potential audit container > > identifiers associated with a network namespace. > > > > Please see the github audit kernel issue for contid net support: > > https://github.com/linux-audit/audit-kernel/issues/92 > > Please see the github audit testsuiite issue for the test case: > > https://github.com/linux-audit/audit-testsuite/issues/64 > > Please see the github audit wiki for the feature overview: > > https://github.com/linux-audit/audit-kernel/wiki/RFE-Audit-Container-ID > > Signed-off-by: Richard Guy Briggs <rgb@redhat.com> > > Acked-by: Neil Horman <nhorman@tuxdriver.com> > > Reviewed-by: Ondrej Mosnacek <omosnace@redhat.com> > > --- > > include/linux/audit.h | 20 ++++++ > > kernel/audit.c | 156 ++++++++++++++++++++++++++++++++++++++++++++++- > > kernel/nsproxy.c | 4 ++ > > net/netfilter/nft_log.c | 11 +++- > > net/netfilter/xt_AUDIT.c | 11 +++- > > 5 files changed, 195 insertions(+), 7 deletions(-) > > > > diff --git a/include/linux/audit.h b/include/linux/audit.h > > index c4a755ae0d61..304fbb7c3c5b 100644 > > --- a/include/linux/audit.h > > +++ b/include/linux/audit.h > > @@ -128,6 +128,13 @@ struct audit_task_info { > > > > extern struct audit_task_info init_struct_audit; > > > > +struct audit_contobj_netns { > > + struct list_head list; > > + struct audit_contobj *obj; > > + int count; > > This seems like it might be a good candidate for refcount_t, yes? I considered this before when converting the struct audit_contobj to refcount_t, but decided against it since any updates are in the context of a list traversal where it could be added to the list and so the spinlock is already held anyways. Is there a more efficent or elegant way of doing the locking around the two list traversals below (_add and _del)? I wonder about converting the count to refcount_t and only holding the spinlock for the list_add_rcu() in the _add case. And for the _del case holding the spinlock only for the list_del_rcu(). These are the only two locations items are added or deleted from the lists. Somewhat related to this is does the list order matter? Items are currently added at the end of the list which likely makes locking simpler, though the start of the list is a simple change. However, unless we understand the profile of read use of these lists for reporting contid use in audit_log_netns_contid_list() I don't think order matters significantly. It could be that reporting of a contid goes down in frequency over the lifetime of a contid that inserting them at the beginning of the list would be best. This is not a visible implementation detail so later optimization should pose no problem. > > + struct rcu_head rcu; > > +}; > > ... > > > diff --git a/kernel/audit.c b/kernel/audit.c > > index 997c34178ee8..a862721dfd9b 100644 > > --- a/kernel/audit.c > > +++ b/kernel/audit.c > > @@ -437,6 +452,136 @@ static struct sock *audit_get_sk(const struct net *net) > > return aunet->sk; > > } > > > > +void audit_netns_contid_add(struct net *net, struct audit_contobj *cont) > > +{ > > + struct audit_net *aunet; > > + struct list_head *contobj_list; > > + struct audit_contobj_netns *contns; > > + > > + if (!net) > > + return; > > + if (!cont) > > + return; > > + aunet = net_generic(net, audit_net_id); > > + if (!aunet) > > + return; > > + contobj_list = &aunet->contobj_list; > > + rcu_read_lock(); > > + spin_lock(&aunet->contobj_list_lock); > > + list_for_each_entry_rcu(contns, contobj_list, list) > > + if (contns->obj == cont) { > > + contns->count++; > > + goto out; > > + } > > + contns = kmalloc(sizeof(*contns), GFP_ATOMIC); > > + if (contns) { > > + INIT_LIST_HEAD(&contns->list); > > + contns->obj = cont; > > + contns->count = 1; > > + list_add_rcu(&contns->list, contobj_list); > > + } > > +out: > > + spin_unlock(&aunet->contobj_list_lock); > > + rcu_read_unlock(); > > +} > > + > > +void audit_netns_contid_del(struct net *net, struct audit_contobj *cont) > > +{ > > + struct audit_net *aunet; > > + struct list_head *contobj_list; > > + struct audit_contobj_netns *contns = NULL; > > + > > + if (!net) > > + return; > > + if (!cont) > > + return; > > + aunet = net_generic(net, audit_net_id); > > + if (!aunet) > > + return; > > + contobj_list = &aunet->contobj_list; > > + rcu_read_lock(); > > + spin_lock(&aunet->contobj_list_lock); > > + list_for_each_entry_rcu(contns, contobj_list, list) > > + if (contns->obj == cont) { > > + contns->count--; > > + if (contns->count < 1) { > > One could simplify this with "(--countns->count) < 1", although if it > is changed to a refcount_t (which seems like a smart thing), the > normal decrement/test would be the best choice. Agreed. > > + list_del_rcu(&contns->list); > > + kfree_rcu(contns, rcu); > > + } > > + break; > > + } > > + spin_unlock(&aunet->contobj_list_lock); > > + rcu_read_unlock(); > > +} > > paul moore - RGB -- Richard Guy Briggs <rgb@redhat.com> Sr. S/W Engineer, Kernel Security, Base Operating Systems Remote, Ottawa, Red Hat Canada IRC: rgb, SunRaycer Voice: +1.647.777.2635, Internal: (81) 32635
diff --git a/include/linux/audit.h b/include/linux/audit.h index c4a755ae0d61..304fbb7c3c5b 100644 --- a/include/linux/audit.h +++ b/include/linux/audit.h @@ -128,6 +128,13 @@ struct audit_task_info { extern struct audit_task_info init_struct_audit; +struct audit_contobj_netns { + struct list_head list; + struct audit_contobj *obj; + int count; + struct rcu_head rcu; +}; + extern int is_audit_feature_set(int which); extern int __init audit_register_class(int class, unsigned *list); @@ -233,6 +240,11 @@ static inline u64 audit_get_contid(struct task_struct *tsk) extern void audit_log_container_id(struct audit_context *context, struct audit_contobj *cont); +extern void audit_copy_namespaces(struct net *net, struct task_struct *tsk); +extern void audit_switch_task_namespaces(struct nsproxy *ns, + struct task_struct *p); +extern void audit_log_netns_contid_list(struct net *net, + struct audit_context *context); extern u32 audit_enabled; @@ -306,6 +318,14 @@ static inline u64 audit_get_contid(struct task_struct *tsk) static inline void audit_log_container_id(struct audit_context *context, struct audit_contobj *cont) { } +static inline void audit_copy_namespaces(struct net *net, struct task_struct *tsk) +{ } +static inline void audit_switch_task_namespaces(struct nsproxy *ns, + struct task_struct *p) +{ } +static inline void audit_log_netns_contid_list(struct net *net, + struct audit_context *context) +{ } #define audit_enabled AUDIT_OFF diff --git a/kernel/audit.c b/kernel/audit.c index 997c34178ee8..a862721dfd9b 100644 --- a/kernel/audit.c +++ b/kernel/audit.c @@ -59,6 +59,7 @@ #include <linux/freezer.h> #include <linux/pid_namespace.h> #include <net/netns/generic.h> +#include <net/net_namespace.h> #include "audit.h" @@ -86,9 +87,13 @@ /** * struct audit_net - audit private network namespace data * @sk: communication socket + * @contobj_list: audit container identifier list + * @contobj_list_lock audit container identifier list lock */ struct audit_net { struct sock *sk; + struct list_head contobj_list; + spinlock_t contobj_list_lock; }; /** @@ -214,6 +219,9 @@ struct audit_reply { static struct kmem_cache *audit_task_cache; +void audit_netns_contid_add(struct net *net, struct audit_contobj *cont); +void audit_netns_contid_del(struct net *net, struct audit_contobj *cont); + void __init audit_task_init(void) { audit_task_cache = kmem_cache_create("audit_task", @@ -326,10 +334,17 @@ struct audit_task_info init_struct_audit = { void audit_free(struct task_struct *tsk) { struct audit_task_info *info = tsk->audit; + struct nsproxy *ns = tsk->nsproxy; + struct audit_contobj *cont; audit_free_syscall(tsk); rcu_read_lock(); - _audit_contobj_put(tsk->audit->cont); + cont = _audit_contobj_get(tsk); + if (ns) { + audit_netns_contid_del(ns->net_ns, cont); + _audit_contobj_put(cont); + } + _audit_contobj_put(cont); rcu_read_unlock(); /* Freeing the audit_task_info struct must be performed after * audit_log_exit() due to need for loginuid and sessionid. @@ -437,6 +452,136 @@ static struct sock *audit_get_sk(const struct net *net) return aunet->sk; } +void audit_netns_contid_add(struct net *net, struct audit_contobj *cont) +{ + struct audit_net *aunet; + struct list_head *contobj_list; + struct audit_contobj_netns *contns; + + if (!net) + return; + if (!cont) + return; + aunet = net_generic(net, audit_net_id); + if (!aunet) + return; + contobj_list = &aunet->contobj_list; + rcu_read_lock(); + spin_lock(&aunet->contobj_list_lock); + list_for_each_entry_rcu(contns, contobj_list, list) + if (contns->obj == cont) { + contns->count++; + goto out; + } + contns = kmalloc(sizeof(*contns), GFP_ATOMIC); + if (contns) { + INIT_LIST_HEAD(&contns->list); + contns->obj = cont; + contns->count = 1; + list_add_rcu(&contns->list, contobj_list); + } +out: + spin_unlock(&aunet->contobj_list_lock); + rcu_read_unlock(); +} + +void audit_netns_contid_del(struct net *net, struct audit_contobj *cont) +{ + struct audit_net *aunet; + struct list_head *contobj_list; + struct audit_contobj_netns *contns = NULL; + + if (!net) + return; + if (!cont) + return; + aunet = net_generic(net, audit_net_id); + if (!aunet) + return; + contobj_list = &aunet->contobj_list; + rcu_read_lock(); + spin_lock(&aunet->contobj_list_lock); + list_for_each_entry_rcu(contns, contobj_list, list) + if (contns->obj == cont) { + contns->count--; + if (contns->count < 1) { + list_del_rcu(&contns->list); + kfree_rcu(contns, rcu); + } + break; + } + spin_unlock(&aunet->contobj_list_lock); + rcu_read_unlock(); +} + +void audit_copy_namespaces(struct net *net, struct task_struct *tsk) +{ + struct audit_contobj *cont; + + rcu_read_lock(); + cont = _audit_contobj_get(tsk); + audit_netns_contid_add(net, cont); + rcu_read_unlock(); +} + +void audit_switch_task_namespaces(struct nsproxy *ns, struct task_struct *p) +{ + struct audit_contobj *cont; + struct nsproxy *new = p->nsproxy; + + rcu_read_lock(); + cont = _audit_contobj_get(p); + if (!cont) + goto out; + audit_netns_contid_del(ns->net_ns, cont); + if (new) + audit_netns_contid_add(new->net_ns, cont); + else + _audit_contobj_put(cont); + _audit_contobj_put(cont); +out: + rcu_read_unlock(); +} + +/** + * audit_log_netns_contid_list - List contids for the given network namespace + * @net: the network namespace of interest + * @context: the audit context to use + * + * Description: + * Issues a CONTAINER_ID record with a CSV list of contids associated + * with a network namespace to accompany a NETFILTER_PKT record. + */ +void audit_log_netns_contid_list(struct net *net, struct audit_context *context) +{ + struct audit_buffer *ab = NULL; + struct audit_contobj_netns *cont; + struct audit_net *aunet; + + /* Generate AUDIT_CONTAINER_ID record with container ID CSV list */ + rcu_read_lock(); + aunet = net_generic(net, audit_net_id); + if (!aunet) + goto out; + list_for_each_entry_rcu(cont, &aunet->contobj_list, list) { + if (!ab) { + ab = audit_log_start(context, GFP_ATOMIC, + AUDIT_CONTAINER_ID); + if (!ab) { + audit_log_lost("out of memory in audit_log_netns_contid_list"); + goto out; + } + audit_log_format(ab, "contid="); + } else + audit_log_format(ab, ","); + audit_log_format(ab, "%llu", cont->obj->id); + } + audit_log_end(ab); +out: + rcu_read_unlock(); +} +EXPORT_SYMBOL(audit_log_netns_contid_list); + void audit_panic(const char *message) { switch (audit_failure) { @@ -1786,7 +1931,6 @@ static int __net_init audit_net_init(struct net *net) .flags = NL_CFG_F_NONROOT_RECV, .groups = AUDIT_NLGRP_MAX, }; - struct audit_net *aunet = net_generic(net, audit_net_id); aunet->sk = netlink_kernel_create(net, NETLINK_AUDIT, &cfg); @@ -1795,7 +1939,8 @@ static int __net_init audit_net_init(struct net *net) return -ENOMEM; } aunet->sk->sk_sndtimeo = MAX_SCHEDULE_TIMEOUT; - + INIT_LIST_HEAD(&aunet->contobj_list); + spin_lock_init(&aunet->contobj_list_lock); return 0; } @@ -2585,6 +2730,7 @@ int audit_set_contid(struct task_struct *task, u64 contid) int rc = 0; struct audit_buffer *ab; struct audit_contobj *oldcont = NULL; + struct net *net = task->nsproxy->net_ns; task_lock(task); /* Can't set if audit disabled */ @@ -2657,6 +2803,10 @@ int audit_set_contid(struct task_struct *task, u64 contid) spin_unlock(&audit_contobj_list_lock); task->audit->cont = newcont; _audit_contobj_put(oldcont); + audit_netns_contid_del(net, oldcont); + _audit_contobj_put(oldcont); + _audit_contobj_hold(newcont); + audit_netns_contid_add(net, newcont); } conterror: task_unlock(task); diff --git a/kernel/nsproxy.c b/kernel/nsproxy.c index b03df67621d0..5eddb3377049 100644 --- a/kernel/nsproxy.c +++ b/kernel/nsproxy.c @@ -26,6 +26,7 @@ #include <linux/syscalls.h> #include <linux/cgroup.h> #include <linux/perf_event.h> +#include <linux/audit.h> static struct kmem_cache *nsproxy_cachep; @@ -187,6 +188,8 @@ int copy_namespaces(unsigned long flags, struct task_struct *tsk) } tsk->nsproxy = new_ns; + if (flags & CLONE_NEWNET) + audit_copy_namespaces(new_ns->net_ns, tsk); return 0; } @@ -249,6 +252,7 @@ void switch_task_namespaces(struct task_struct *p, struct nsproxy *new) ns = p->nsproxy; p->nsproxy = new; task_unlock(p); + audit_switch_task_namespaces(ns, p); if (ns && atomic_dec_and_test(&ns->count)) free_nsproxy(ns); diff --git a/net/netfilter/nft_log.c b/net/netfilter/nft_log.c index fe4831f2258f..98d1e7e1a83c 100644 --- a/net/netfilter/nft_log.c +++ b/net/netfilter/nft_log.c @@ -66,13 +66,16 @@ static void nft_log_eval_audit(const struct nft_pktinfo *pkt) struct sk_buff *skb = pkt->skb; struct audit_buffer *ab; int fam = -1; + struct audit_context *context; + struct net *net; if (!audit_enabled) return; - ab = audit_log_start(NULL, GFP_ATOMIC, AUDIT_NETFILTER_PKT); + context = audit_alloc_local(GFP_ATOMIC); + ab = audit_log_start(context, GFP_ATOMIC, AUDIT_NETFILTER_PKT); if (!ab) - return; + goto errout; audit_log_format(ab, "mark=%#x", skb->mark); @@ -99,6 +102,10 @@ static void nft_log_eval_audit(const struct nft_pktinfo *pkt) audit_log_format(ab, " saddr=? daddr=? proto=-1"); audit_log_end(ab); + net = xt_net(&pkt->xt); + audit_log_netns_contid_list(net, context); +errout: + audit_free_context(context); } static void nft_log_eval(const struct nft_expr *expr, diff --git a/net/netfilter/xt_AUDIT.c b/net/netfilter/xt_AUDIT.c index 9cdc16b0d0d8..ecf868a1abde 100644 --- a/net/netfilter/xt_AUDIT.c +++ b/net/netfilter/xt_AUDIT.c @@ -68,10 +68,13 @@ static bool audit_ip6(struct audit_buffer *ab, struct sk_buff *skb) { struct audit_buffer *ab; int fam = -1; + struct audit_context *context; + struct net *net; if (audit_enabled == AUDIT_OFF) - goto errout; - ab = audit_log_start(NULL, GFP_ATOMIC, AUDIT_NETFILTER_PKT); + goto out; + context = audit_alloc_local(GFP_ATOMIC); + ab = audit_log_start(context, GFP_ATOMIC, AUDIT_NETFILTER_PKT); if (ab == NULL) goto errout; @@ -101,7 +104,11 @@ static bool audit_ip6(struct audit_buffer *ab, struct sk_buff *skb) audit_log_end(ab); + net = xt_net(par); + audit_log_netns_contid_list(net, context); errout: + audit_free_context(context); +out: return XT_CONTINUE; }