From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-lf1-f65.google.com ([209.85.167.65]:37633 "EHLO mail-lf1-f65.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S2388643AbeGXVaW (ORCPT ); Tue, 24 Jul 2018 17:30:22 -0400 Received: by mail-lf1-f65.google.com with SMTP id j8-v6so3875795lfb.4 for ; Tue, 24 Jul 2018 13:22:12 -0700 (PDT) MIME-Version: 1.0 References: <1748819.SHaROlQLoH@x2> In-Reply-To: <1748819.SHaROlQLoH@x2> From: Paul Moore Date: Tue, 24 Jul 2018 16:22:00 -0400 Message-ID: Subject: Re: [RFC PATCH ghak90 (was ghak32) V3 08/10] audit: NETFILTER_PKT: record each container ID associated with a netNS To: sgrubb@redhat.com Cc: rgb@redhat.com, cgroups@vger.kernel.org, containers@lists.linux-foundation.org, linux-api@vger.kernel.org, linux-audit@redhat.com, linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org, netdev@vger.kernel.org, ebiederm@xmission.com, luto@kernel.org, jlayton@redhat.com, carlos@redhat.com, dhowells@redhat.com, viro@zeniv.linux.org.uk, simo@redhat.com, Eric Paris , serge@hallyn.com Content-Type: text/plain; charset="UTF-8" Sender: linux-fsdevel-owner@vger.kernel.org List-ID: On Tue, Jul 24, 2018 at 3:48 PM Steve Grubb wrote: > On Friday, July 20, 2018 6:15:00 PM EDT Paul Moore wrote: > > On Wed, Jun 6, 2018 at 1:03 PM Richard Guy Briggs wrote: > > > 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. > > > > > > Signed-off-by: Richard Guy Briggs > > > --- > > > include/linux/audit.h | 5 +++++ > > > kernel/audit.c | 20 +++++++++++++++++++- > > > kernel/auditsc.c | 2 ++ > > > net/netfilter/xt_AUDIT.c | 12 ++++++++++-- > > > 4 files changed, 36 insertions(+), 3 deletions(-) > > > > ... > > > > > diff --git a/include/linux/audit.h b/include/linux/audit.h > > > index 7e2e51c..4560a4e 100644 > > > --- a/include/linux/audit.h > > > +++ b/include/linux/audit.h > > > @@ -167,6 +167,8 @@ extern int audit_log_contid(struct audit_context > > > *context, extern void audit_contid_add(struct net *net, u64 contid); > > > extern void audit_contid_del(struct net *net, u64 contid); > > > extern void audit_switch_task_namespaces(struct nsproxy *ns, struct > > > task_struct *p); +extern void audit_log_contid_list(struct net *net, > > > + struct audit_context *context); > > > > See my comment in previous patches about changing the function name to > > better indicate it's dedicate use for network namespaces. > > > > > extern int audit_update_lsm_rules(void); > > > > > > @@ -231,6 +233,9 @@ static inline void audit_contid_del(struct net *net, > > > u64 contid) { } > > > static inline void audit_switch_task_namespaces(struct nsproxy *ns, > > > struct task_struct *p) { } > > > +static inline void audit_log_contid_list(struct net *net, > > > + struct audit_context *context) > > > +{ } > > > > > > #define audit_enabled 0 > > > #endif /* CONFIG_AUDIT */ > > > diff --git a/kernel/audit.c b/kernel/audit.c > > > index ecd2de4..8cca41a 100644 > > > --- a/kernel/audit.c > > > +++ b/kernel/audit.c > > > @@ -382,6 +382,20 @@ void audit_switch_task_namespaces(struct nsproxy > > > *ns, struct task_struct *p) audit_contid_add(new->net_ns, contid); > > > } > > > > > > +void audit_log_contid_list(struct net *net, struct audit_context > > > *context) +{ > > > + struct audit_contid *cont; > > > + int i = 0; > > > + > > > + list_for_each_entry(cont, audit_get_contid_list(net), list) { > > > + char buf[14]; > > > + > > > + sprintf(buf, "net%u", i++); > > > + audit_log_contid(context, buf, cont->id); > > > > Hmm. It looks like this will generate multiple audit container ID > > records with "op=netX contid=Y" (X=netns number, Y=audit container > > ID), is that what we want? I've mentioned my concern around the "op" > > values in these records earlier in the patchset, that still applies > > here, but now I'm also concerned about the multiple records. I'm > > thinking we might be better served with a single record with either > > multiple "contid" fields, or a single "contid" field with a set of > > comma separated values (or some other delimiter that Steve's tools > > will tolerate). > > > > Steve, thoughts? > > A single record is best. Maybe pattern this after the args listed in an > execve record. I'm concerned that an execve-like approach might not scale very well as would could potentially have a lot of containers sharing a single network namespace ("a%d=%d" vs ",%d"). Further, with execve we log the argument position in addition to the argument itself, that isn't something we need to worry about with the audit container IDs. -- paul moore www.paul-moore.com