[ghak90,(was,ghak32),V4,03/10] audit: log container info of syscalls

Submitted by Richard Guy Briggs on July 31, 2018, 8:07 p.m.

Details

Message ID 34017c395d03a213d6b0d49b9964429bd32b283d.1533065887.git.rgb@redhat.com
State New
Series "audit: implement container identifier"
Headers show

Commit Message

Richard Guy Briggs July 31, 2018, 8:07 p.m.
Create a new audit record AUDIT_CONTAINER to document the audit
container identifier of a process if it is present.

Called from audit_log_exit(), syscalls are covered.

A sample raw event:
type=SYSCALL msg=audit(1519924845.499:257): arch=c000003e syscall=257 success=yes exit=3 a0=ffffff9c a1=56374e1cef30 a2=241 a3=1b6 items=2 ppid=606 pid=635 auid=0 uid=0 gid=0 euid=0 suid=0 fsuid=0 egid=0 sgid=0 fsgid=0 tty=pts0 ses=3 comm="bash" exe="/usr/bin/bash" subj=unconfined_u:unconfined_r:unconfined_t:s0-s0:c0.c1023 key="tmpcontainerid"
type=CWD msg=audit(1519924845.499:257): cwd="/root"
type=PATH msg=audit(1519924845.499:257): item=0 name="/tmp/" inode=13863 dev=00:27 mode=041777 ouid=0 ogid=0 rdev=00:00 obj=system_u:object_r:tmp_t:s0 nametype= PARENT cap_fp=0000000000000000 cap_fi=0000000000000000 cap_fe=0 cap_fver=0
type=PATH msg=audit(1519924845.499:257): item=1 name="/tmp/tmpcontainerid" inode=17729 dev=00:27 mode=0100644 ouid=0 ogid=0 rdev=00:00 obj=unconfined_u:object_r:user_tmp_t:s0 nametype=CREATE cap_fp=0000000000000000 cap_fi=0000000000000000 cap_fe=0 cap_fver=0
type=PROCTITLE msg=audit(1519924845.499:257): proctitle=62617368002D6300736C65657020313B206563686F2074657374203E202F746D702F746D70636F6E7461696E65726964
type=CONTAINER msg=audit(1519924845.499:257): op=task contid=123458

See: https://github.com/linux-audit/audit-kernel/issues/90
See: https://github.com/linux-audit/audit-userspace/issues/51
See: https://github.com/linux-audit/audit-testsuite/issues/64
See: https://github.com/linux-audit/audit-kernel/wiki/RFE-Audit-Container-ID
Signed-off-by: Richard Guy Briggs <rgb@redhat.com>
Acked-by: Serge Hallyn <serge@hallyn.com>
Acked-by: Steve Grubb <sgrubb@redhat.com>
---
 include/linux/audit.h      |  7 +++++++
 include/uapi/linux/audit.h |  1 +
 kernel/audit.c             | 24 ++++++++++++++++++++++++
 kernel/auditsc.c           |  3 +++
 4 files changed, 35 insertions(+)

Patch hide | download patch | download mbox

diff --git a/include/linux/audit.h b/include/linux/audit.h
index 71a6fc6..d5a48dc 100644
--- a/include/linux/audit.h
+++ b/include/linux/audit.h
@@ -155,6 +155,9 @@  extern void		    audit_log_key(struct audit_buffer *ab,
 extern int audit_log_task_context(struct audit_buffer *ab);
 extern void audit_log_task_info(struct audit_buffer *ab,
 				struct task_struct *tsk);
+extern int audit_log_contid(struct task_struct *tsk,
+				    struct audit_context *context,
+				    char *op);
 
 extern int		    audit_update_lsm_rules(void);
 
@@ -205,6 +208,10 @@  static inline int audit_log_task_context(struct audit_buffer *ab)
 static inline void audit_log_task_info(struct audit_buffer *ab,
 				       struct task_struct *tsk)
 { }
+static inline int audit_log_contid(struct task_struct *tsk,
+					   struct audit_context *context,
+					   char *op)
+{ }
 #define audit_enabled AUDIT_OFF
 #endif /* CONFIG_AUDIT */
 
diff --git a/include/uapi/linux/audit.h b/include/uapi/linux/audit.h
index 3474f57..dc259c7 100644
--- a/include/uapi/linux/audit.h
+++ b/include/uapi/linux/audit.h
@@ -115,6 +115,7 @@ 
 #define AUDIT_REPLACE		1329	/* Replace auditd if this packet unanswerd */
 #define AUDIT_KERN_MODULE	1330	/* Kernel Module events */
 #define AUDIT_FANOTIFY		1331	/* Fanotify access decision */
+#define AUDIT_CONTAINER		1332	/* Container ID */
 
 #define AUDIT_AVC		1400	/* SE Linux avc denial or grant */
 #define AUDIT_SELINUX_ERR	1401	/* Internal SE Linux Errors */
diff --git a/kernel/audit.c b/kernel/audit.c
index 2a80587..15f54c7 100644
--- a/kernel/audit.c
+++ b/kernel/audit.c
@@ -2045,6 +2045,30 @@  void audit_log_session_info(struct audit_buffer *ab)
 	audit_log_format(ab, " auid=%u ses=%u", auid, sessionid);
 }
 
+/*
+ * audit_log_contid - report container info
+ * @tsk: task to be recorded
+ * @context: task or local context for record
+ * @op: contid string description
+ */
+int audit_log_contid(struct task_struct *tsk,
+			     struct audit_context *context, char *op)
+{
+	struct audit_buffer *ab;
+
+	if (!audit_contid_set(tsk))
+		return 0;
+	/* Generate AUDIT_CONTAINER record with container ID */
+	ab = audit_log_start(context, GFP_KERNEL, AUDIT_CONTAINER);
+	if (!ab)
+		return -ENOMEM;
+	audit_log_format(ab, "op=%s contid=%llu",
+			 op, audit_get_contid(tsk));
+	audit_log_end(ab);
+	return 0;
+}
+EXPORT_SYMBOL(audit_log_contid);
+
 void audit_log_key(struct audit_buffer *ab, char *key)
 {
 	audit_log_format(ab, " key=");
diff --git a/kernel/auditsc.c b/kernel/auditsc.c
index 6125cef..39e5633 100644
--- a/kernel/auditsc.c
+++ b/kernel/auditsc.c
@@ -1488,10 +1488,13 @@  static void audit_log_exit(struct audit_context *context, struct task_struct *ts
 
 	audit_log_proctitle(tsk, context);
 
+	audit_log_contid(tsk, context, "task");
+
 	/* Send end of event record to help user space know we are finished */
 	ab = audit_log_start(context, GFP_KERNEL, AUDIT_EOE);
 	if (ab)
 		audit_log_end(ab);
+
 	if (call_panic)
 		audit_panic("error converting sid to string");
 }

Comments

Steve Grubb Aug. 24, 2018, 4:01 p.m.
On Tuesday, July 31, 2018 4:07:38 PM EDT Richard Guy Briggs wrote:
> Create a new audit record AUDIT_CONTAINER to document the audit
> container identifier of a process if it is present.
> 
> Called from audit_log_exit(), syscalls are covered.
> 
> A sample raw event:
> type=SYSCALL msg=audit(1519924845.499:257): arch=c000003e syscall=257
> success=yes exit=3 a0=ffffff9c a1=56374e1cef30 a2=241 a3=1b6 items=2
> ppid=606 pid=635 auid=0 uid=0 gid=0 euid=0 suid=0 fsuid=0 egid=0 sgid=0
> fsgid=0 tty=pts0 ses=3 comm="bash" exe="/usr/bin/bash"
> subj=unconfined_u:unconfined_r:unconfined_t:s0-s0:c0.c1023
> key="tmpcontainerid" type=CWD msg=audit(1519924845.499:257): cwd="/root"
> type=PATH msg=audit(1519924845.499:257): item=0 name="/tmp/" inode=13863
> dev=00:27 mode=041777 ouid=0 ogid=0 rdev=00:00
> obj=system_u:object_r:tmp_t:s0 nametype= PARENT cap_fp=0000000000000000
> cap_fi=0000000000000000 cap_fe=0 cap_fver=0 type=PATH
> msg=audit(1519924845.499:257): item=1 name="/tmp/tmpcontainerid"
> inode=17729 dev=00:27 mode=0100644 ouid=0 ogid=0 rdev=00:00
> obj=unconfined_u:object_r:user_tmp_t:s0 nametype=CREATE
> cap_fp=0000000000000000 cap_fi=0000000000000000 cap_fe=0 cap_fver=0
> type=PROCTITLE msg=audit(1519924845.499:257):
> proctitle=62617368002D6300736C65657020313B206563686F2074657374203E202F746D
> 702F746D70636F6E7461696E65726964 type=CONTAINER
> msg=audit(1519924845.499:257): op=task contid=123458

Ack for the event format.

-Steve

> See: https://github.com/linux-audit/audit-kernel/issues/90
> See: https://github.com/linux-audit/audit-userspace/issues/51
> See: https://github.com/linux-audit/audit-testsuite/issues/64
> See:
> https://github.com/linux-audit/audit-kernel/wiki/RFE-Audit-Container-ID
> Signed-off-by: Richard Guy Briggs <rgb@redhat.com>
> Acked-by: Serge Hallyn <serge@hallyn.com>
> Acked-by: Steve Grubb <sgrubb@redhat.com>
> ---
>  include/linux/audit.h      |  7 +++++++
>  include/uapi/linux/audit.h |  1 +
>  kernel/audit.c             | 24 ++++++++++++++++++++++++
>  kernel/auditsc.c           |  3 +++
>  4 files changed, 35 insertions(+)
> 
> diff --git a/include/linux/audit.h b/include/linux/audit.h
> index 71a6fc6..d5a48dc 100644
> --- a/include/linux/audit.h
> +++ b/include/linux/audit.h
> @@ -155,6 +155,9 @@ extern void		    audit_log_key(struct audit_buffer 
*ab,
> extern int audit_log_task_context(struct audit_buffer *ab);
>  extern void audit_log_task_info(struct audit_buffer *ab,
>  				struct task_struct *tsk);
> +extern int audit_log_contid(struct task_struct *tsk,
> +				    struct audit_context *context,
> +				    char *op);
> 
>  extern int		    audit_update_lsm_rules(void);
> 
> @@ -205,6 +208,10 @@ static inline int audit_log_task_context(struct
> audit_buffer *ab) static inline void audit_log_task_info(struct
> audit_buffer *ab,
>  				       struct task_struct *tsk)
>  { }
> +static inline int audit_log_contid(struct task_struct *tsk,
> +					   struct audit_context *context,
> +					   char *op)
> +{ }
>  #define audit_enabled AUDIT_OFF
>  #endif /* CONFIG_AUDIT */
> 
> diff --git a/include/uapi/linux/audit.h b/include/uapi/linux/audit.h
> index 3474f57..dc259c7 100644
> --- a/include/uapi/linux/audit.h
> +++ b/include/uapi/linux/audit.h
> @@ -115,6 +115,7 @@
>  #define AUDIT_REPLACE		1329	/* Replace auditd if this packet unanswerd 
*/
>  #define AUDIT_KERN_MODULE	1330	/* Kernel Module events */
>  #define AUDIT_FANOTIFY		1331	/* Fanotify access decision */
> +#define AUDIT_CONTAINER		1332	/* Container ID */
> 
>  #define AUDIT_AVC		1400	/* SE Linux avc denial or grant */
>  #define AUDIT_SELINUX_ERR	1401	/* Internal SE Linux Errors */
> diff --git a/kernel/audit.c b/kernel/audit.c
> index 2a80587..15f54c7 100644
> --- a/kernel/audit.c
> +++ b/kernel/audit.c
> @@ -2045,6 +2045,30 @@ void audit_log_session_info(struct audit_buffer *ab)
> audit_log_format(ab, " auid=%u ses=%u", auid, sessionid);
>  }
> 
> +/*
> + * audit_log_contid - report container info
> + * @tsk: task to be recorded
> + * @context: task or local context for record
> + * @op: contid string description
> + */
> +int audit_log_contid(struct task_struct *tsk,
> +			     struct audit_context *context, char *op)
> +{
> +	struct audit_buffer *ab;
> +
> +	if (!audit_contid_set(tsk))
> +		return 0;
> +	/* Generate AUDIT_CONTAINER record with container ID */
> +	ab = audit_log_start(context, GFP_KERNEL, AUDIT_CONTAINER);
> +	if (!ab)
> +		return -ENOMEM;
> +	audit_log_format(ab, "op=%s contid=%llu",
> +			 op, audit_get_contid(tsk));
> +	audit_log_end(ab);
> +	return 0;
> +}
> +EXPORT_SYMBOL(audit_log_contid);
> +
>  void audit_log_key(struct audit_buffer *ab, char *key)
>  {
>  	audit_log_format(ab, " key=");
> diff --git a/kernel/auditsc.c b/kernel/auditsc.c
> index 6125cef..39e5633 100644
> --- a/kernel/auditsc.c
> +++ b/kernel/auditsc.c
> @@ -1488,10 +1488,13 @@ static void audit_log_exit(struct audit_context
> *context, struct task_struct *ts
> 
>  	audit_log_proctitle(tsk, context);
> 
> +	audit_log_contid(tsk, context, "task");
> +
>  	/* Send end of event record to help user space know we are finished */
>  	ab = audit_log_start(context, GFP_KERNEL, AUDIT_EOE);
>  	if (ab)
>  		audit_log_end(ab);
> +
>  	if (call_panic)
>  		audit_panic("error converting sid to string");
>  }
Paul Moore Oct. 19, 2018, 11:16 p.m.
On Sun, Aug 5, 2018 at 4:32 AM Richard Guy Briggs <rgb@redhat.com> wrote:
> Create a new audit record AUDIT_CONTAINER to document the audit
> container identifier of a process if it is present.
>
> Called from audit_log_exit(), syscalls are covered.
>
> A sample raw event:
> type=SYSCALL msg=audit(1519924845.499:257): arch=c000003e syscall=257 success=yes exit=3 a0=ffffff9c a1=56374e1cef30 a2=241 a3=1b6 items=2 ppid=606 pid=635 auid=0 uid=0 gid=0 euid=0 suid=0 fsuid=0 egid=0 sgid=0 fsgid=0 tty=pts0 ses=3 comm="bash" exe="/usr/bin/bash" subj=unconfined_u:unconfined_r:unconfined_t:s0-s0:c0.c1023 key="tmpcontainerid"
> type=CWD msg=audit(1519924845.499:257): cwd="/root"
> type=PATH msg=audit(1519924845.499:257): item=0 name="/tmp/" inode=13863 dev=00:27 mode=041777 ouid=0 ogid=0 rdev=00:00 obj=system_u:object_r:tmp_t:s0 nametype= PARENT cap_fp=0000000000000000 cap_fi=0000000000000000 cap_fe=0 cap_fver=0
> type=PATH msg=audit(1519924845.499:257): item=1 name="/tmp/tmpcontainerid" inode=17729 dev=00:27 mode=0100644 ouid=0 ogid=0 rdev=00:00 obj=unconfined_u:object_r:user_tmp_t:s0 nametype=CREATE cap_fp=0000000000000000 cap_fi=0000000000000000 cap_fe=0 cap_fver=0
> type=PROCTITLE msg=audit(1519924845.499:257): proctitle=62617368002D6300736C65657020313B206563686F2074657374203E202F746D702F746D70636F6E7461696E65726964
> type=CONTAINER msg=audit(1519924845.499:257): op=task contid=123458
>
> See: https://github.com/linux-audit/audit-kernel/issues/90
> See: https://github.com/linux-audit/audit-userspace/issues/51
> See: https://github.com/linux-audit/audit-testsuite/issues/64
> See: https://github.com/linux-audit/audit-kernel/wiki/RFE-Audit-Container-ID
> Signed-off-by: Richard Guy Briggs <rgb@redhat.com>
> Acked-by: Serge Hallyn <serge@hallyn.com>
> Acked-by: Steve Grubb <sgrubb@redhat.com>
> ---
>  include/linux/audit.h      |  7 +++++++
>  include/uapi/linux/audit.h |  1 +
>  kernel/audit.c             | 24 ++++++++++++++++++++++++
>  kernel/auditsc.c           |  3 +++
>  4 files changed, 35 insertions(+)

...

> @@ -2045,6 +2045,30 @@ void audit_log_session_info(struct audit_buffer *ab)
>         audit_log_format(ab, " auid=%u ses=%u", auid, sessionid);
>  }
>
> +/*
> + * audit_log_contid - report container info
> + * @tsk: task to be recorded
> + * @context: task or local context for record
> + * @op: contid string description
> + */
> +int audit_log_contid(struct task_struct *tsk,
> +                            struct audit_context *context, char *op)
> +{
> +       struct audit_buffer *ab;
> +
> +       if (!audit_contid_set(tsk))
> +               return 0;
> +       /* Generate AUDIT_CONTAINER record with container ID */
> +       ab = audit_log_start(context, GFP_KERNEL, AUDIT_CONTAINER);
> +       if (!ab)
> +               return -ENOMEM;
> +       audit_log_format(ab, "op=%s contid=%llu",
> +                        op, audit_get_contid(tsk));
> +       audit_log_end(ab);
> +       return 0;
> +}
> +EXPORT_SYMBOL(audit_log_contid);

As discussed in the previous iteration of the patch, I prefer
AUDIT_CONTAINER_ID here over AUDIT_CONTAINER.  If you feel strongly
about keeping it as-is with AUDIT_CONTAINER I suppose I could live
with that, but it is isn't my first choice.

However, I do care about the "op" field in this record.  It just
doesn't make any sense; the way you are using it it is more of a
context field than an operations field, and even then why is the
context important from a logging and/or security perspective?  Drop it
please.

--
paul moore
www.paul-moore.com
Richard Guy Briggs Oct. 24, 2018, 3:14 p.m.
On 2018-10-19 19:16, Paul Moore wrote:
> On Sun, Aug 5, 2018 at 4:32 AM Richard Guy Briggs <rgb@redhat.com> wrote:
> > Create a new audit record AUDIT_CONTAINER to document the audit
> > container identifier of a process if it is present.
> >
> > Called from audit_log_exit(), syscalls are covered.
> >
> > A sample raw event:
> > type=SYSCALL msg=audit(1519924845.499:257): arch=c000003e syscall=257 success=yes exit=3 a0=ffffff9c a1=56374e1cef30 a2=241 a3=1b6 items=2 ppid=606 pid=635 auid=0 uid=0 gid=0 euid=0 suid=0 fsuid=0 egid=0 sgid=0 fsgid=0 tty=pts0 ses=3 comm="bash" exe="/usr/bin/bash" subj=unconfined_u:unconfined_r:unconfined_t:s0-s0:c0.c1023 key="tmpcontainerid"
> > type=CWD msg=audit(1519924845.499:257): cwd="/root"
> > type=PATH msg=audit(1519924845.499:257): item=0 name="/tmp/" inode=13863 dev=00:27 mode=041777 ouid=0 ogid=0 rdev=00:00 obj=system_u:object_r:tmp_t:s0 nametype= PARENT cap_fp=0000000000000000 cap_fi=0000000000000000 cap_fe=0 cap_fver=0
> > type=PATH msg=audit(1519924845.499:257): item=1 name="/tmp/tmpcontainerid" inode=17729 dev=00:27 mode=0100644 ouid=0 ogid=0 rdev=00:00 obj=unconfined_u:object_r:user_tmp_t:s0 nametype=CREATE cap_fp=0000000000000000 cap_fi=0000000000000000 cap_fe=0 cap_fver=0
> > type=PROCTITLE msg=audit(1519924845.499:257): proctitle=62617368002D6300736C65657020313B206563686F2074657374203E202F746D702F746D70636F6E7461696E65726964
> > type=CONTAINER msg=audit(1519924845.499:257): op=task contid=123458
> >
> > See: https://github.com/linux-audit/audit-kernel/issues/90
> > See: https://github.com/linux-audit/audit-userspace/issues/51
> > See: https://github.com/linux-audit/audit-testsuite/issues/64
> > See: https://github.com/linux-audit/audit-kernel/wiki/RFE-Audit-Container-ID
> > Signed-off-by: Richard Guy Briggs <rgb@redhat.com>
> > Acked-by: Serge Hallyn <serge@hallyn.com>
> > Acked-by: Steve Grubb <sgrubb@redhat.com>
> > ---
> >  include/linux/audit.h      |  7 +++++++
> >  include/uapi/linux/audit.h |  1 +
> >  kernel/audit.c             | 24 ++++++++++++++++++++++++
> >  kernel/auditsc.c           |  3 +++
> >  4 files changed, 35 insertions(+)
> 
> ...
> 
> > @@ -2045,6 +2045,30 @@ void audit_log_session_info(struct audit_buffer *ab)
> >         audit_log_format(ab, " auid=%u ses=%u", auid, sessionid);
> >  }
> >
> > +/*
> > + * audit_log_contid - report container info
> > + * @tsk: task to be recorded
> > + * @context: task or local context for record
> > + * @op: contid string description
> > + */
> > +int audit_log_contid(struct task_struct *tsk,
> > +                            struct audit_context *context, char *op)
> > +{
> > +       struct audit_buffer *ab;
> > +
> > +       if (!audit_contid_set(tsk))
> > +               return 0;
> > +       /* Generate AUDIT_CONTAINER record with container ID */
> > +       ab = audit_log_start(context, GFP_KERNEL, AUDIT_CONTAINER);
> > +       if (!ab)
> > +               return -ENOMEM;
> > +       audit_log_format(ab, "op=%s contid=%llu",
> > +                        op, audit_get_contid(tsk));
> > +       audit_log_end(ab);
> > +       return 0;
> > +}
> > +EXPORT_SYMBOL(audit_log_contid);
> 
> As discussed in the previous iteration of the patch, I prefer
> AUDIT_CONTAINER_ID here over AUDIT_CONTAINER.  If you feel strongly
> about keeping it as-is with AUDIT_CONTAINER I suppose I could live
> with that, but it is isn't my first choice.

I don't have a strong opinion on this one, mildly preferring the shorter
one only because it is shorter.

Steve?  Can you comment on this one way or the other?

> However, I do care about the "op" field in this record.  It just
> doesn't make any sense; the way you are using it it is more of a
> context field than an operations field, and even then why is the
> context important from a logging and/or security perspective?  Drop it
> please.

I'll rename it to whatever you like.  I'd suggest "ref=".  The reason I
think it is important is there are multiple sources that aren't always
obvious from the other records to which it is associated.  In the case
of ptrace and signals, there can be many target tasks listed (OBJ_PID)
with no other way to distinguish the matching audit container identifier
records all for one event.  This is in addition to the default syscall
container identifier record.  I'm not currently happy with the text
content to link the two, but that should be solvable (most obvious is
taret PID).  Throwing away this information seems shortsighted.

> 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
Paul Moore Oct. 24, 2018, 8:55 p.m.
On Wed, Oct 24, 2018 at 11:15 AM Richard Guy Briggs <rgb@redhat.com> wrote:
> On 2018-10-19 19:16, Paul Moore wrote:
> > On Sun, Aug 5, 2018 at 4:32 AM Richard Guy Briggs <rgb@redhat.com> wrote:
> > > Create a new audit record AUDIT_CONTAINER to document the audit
> > > container identifier of a process if it is present.
> > >
> > > Called from audit_log_exit(), syscalls are covered.
> > >
> > > A sample raw event:
> > > type=SYSCALL msg=audit(1519924845.499:257): arch=c000003e syscall=257 success=yes exit=3 a0=ffffff9c a1=56374e1cef30 a2=241 a3=1b6 items=2 ppid=606 pid=635 auid=0 uid=0 gid=0 euid=0 suid=0 fsuid=0 egid=0 sgid=0 fsgid=0 tty=pts0 ses=3 comm="bash" exe="/usr/bin/bash" subj=unconfined_u:unconfined_r:unconfined_t:s0-s0:c0.c1023 key="tmpcontainerid"
> > > type=CWD msg=audit(1519924845.499:257): cwd="/root"
> > > type=PATH msg=audit(1519924845.499:257): item=0 name="/tmp/" inode=13863 dev=00:27 mode=041777 ouid=0 ogid=0 rdev=00:00 obj=system_u:object_r:tmp_t:s0 nametype= PARENT cap_fp=0000000000000000 cap_fi=0000000000000000 cap_fe=0 cap_fver=0
> > > type=PATH msg=audit(1519924845.499:257): item=1 name="/tmp/tmpcontainerid" inode=17729 dev=00:27 mode=0100644 ouid=0 ogid=0 rdev=00:00 obj=unconfined_u:object_r:user_tmp_t:s0 nametype=CREATE cap_fp=0000000000000000 cap_fi=0000000000000000 cap_fe=0 cap_fver=0
> > > type=PROCTITLE msg=audit(1519924845.499:257): proctitle=62617368002D6300736C65657020313B206563686F2074657374203E202F746D702F746D70636F6E7461696E65726964
> > > type=CONTAINER msg=audit(1519924845.499:257): op=task contid=123458
> > >
> > > See: https://github.com/linux-audit/audit-kernel/issues/90
> > > See: https://github.com/linux-audit/audit-userspace/issues/51
> > > See: https://github.com/linux-audit/audit-testsuite/issues/64
> > > See: https://github.com/linux-audit/audit-kernel/wiki/RFE-Audit-Container-ID
> > > Signed-off-by: Richard Guy Briggs <rgb@redhat.com>
> > > Acked-by: Serge Hallyn <serge@hallyn.com>
> > > Acked-by: Steve Grubb <sgrubb@redhat.com>
> > > ---
> > >  include/linux/audit.h      |  7 +++++++
> > >  include/uapi/linux/audit.h |  1 +
> > >  kernel/audit.c             | 24 ++++++++++++++++++++++++
> > >  kernel/auditsc.c           |  3 +++
> > >  4 files changed, 35 insertions(+)
> >
> > ...
> >
> > > @@ -2045,6 +2045,30 @@ void audit_log_session_info(struct audit_buffer *ab)
> > >         audit_log_format(ab, " auid=%u ses=%u", auid, sessionid);
> > >  }
> > >
> > > +/*
> > > + * audit_log_contid - report container info
> > > + * @tsk: task to be recorded
> > > + * @context: task or local context for record
> > > + * @op: contid string description
> > > + */
> > > +int audit_log_contid(struct task_struct *tsk,
> > > +                            struct audit_context *context, char *op)
> > > +{
> > > +       struct audit_buffer *ab;
> > > +
> > > +       if (!audit_contid_set(tsk))
> > > +               return 0;
> > > +       /* Generate AUDIT_CONTAINER record with container ID */
> > > +       ab = audit_log_start(context, GFP_KERNEL, AUDIT_CONTAINER);
> > > +       if (!ab)
> > > +               return -ENOMEM;
> > > +       audit_log_format(ab, "op=%s contid=%llu",
> > > +                        op, audit_get_contid(tsk));
> > > +       audit_log_end(ab);
> > > +       return 0;
> > > +}
> > > +EXPORT_SYMBOL(audit_log_contid);
> >
> > As discussed in the previous iteration of the patch, I prefer
> > AUDIT_CONTAINER_ID here over AUDIT_CONTAINER.  If you feel strongly
> > about keeping it as-is with AUDIT_CONTAINER I suppose I could live
> > with that, but it is isn't my first choice.
>
> I don't have a strong opinion on this one, mildly preferring the shorter
> one only because it is shorter.

We already have multiple AUDIT_CONTAINER* record types, so it seems as
though we should use "AUDIT_CONTAINER" as a prefix of sorts, rather
than a type itself.

> > However, I do care about the "op" field in this record.  It just
> > doesn't make any sense; the way you are using it it is more of a
> > context field than an operations field, and even then why is the
> > context important from a logging and/or security perspective?  Drop it
> > please.
>
> I'll rename it to whatever you like.  I'd suggest "ref=".  The reason I
> think it is important is there are multiple sources that aren't always
> obvious from the other records to which it is associated.  In the case
> of ptrace and signals, there can be many target tasks listed (OBJ_PID)
> with no other way to distinguish the matching audit container identifier
> records all for one event.  This is in addition to the default syscall
> container identifier record.  I'm not currently happy with the text
> content to link the two, but that should be solvable (most obvious is
> taret PID).  Throwing away this information seems shortsighted.

It would be helpful if you could generate real audit events
demonstrating the problems you are describing, as well as a more
standard syscall event, so we can discuss some possible solutions.
Richard Guy Briggs Oct. 25, 2018, 12:42 a.m.
On 2018-10-24 16:55, Paul Moore wrote:
> On Wed, Oct 24, 2018 at 11:15 AM Richard Guy Briggs <rgb@redhat.com> wrote:
> > On 2018-10-19 19:16, Paul Moore wrote:
> > > On Sun, Aug 5, 2018 at 4:32 AM Richard Guy Briggs <rgb@redhat.com> wrote:
> > > > Create a new audit record AUDIT_CONTAINER to document the audit
> > > > container identifier of a process if it is present.
> > > >
> > > > Called from audit_log_exit(), syscalls are covered.
> > > >
> > > > A sample raw event:
> > > > type=SYSCALL msg=audit(1519924845.499:257): arch=c000003e syscall=257 success=yes exit=3 a0=ffffff9c a1=56374e1cef30 a2=241 a3=1b6 items=2 ppid=606 pid=635 auid=0 uid=0 gid=0 euid=0 suid=0 fsuid=0 egid=0 sgid=0 fsgid=0 tty=pts0 ses=3 comm="bash" exe="/usr/bin/bash" subj=unconfined_u:unconfined_r:unconfined_t:s0-s0:c0.c1023 key="tmpcontainerid"
> > > > type=CWD msg=audit(1519924845.499:257): cwd="/root"
> > > > type=PATH msg=audit(1519924845.499:257): item=0 name="/tmp/" inode=13863 dev=00:27 mode=041777 ouid=0 ogid=0 rdev=00:00 obj=system_u:object_r:tmp_t:s0 nametype= PARENT cap_fp=0000000000000000 cap_fi=0000000000000000 cap_fe=0 cap_fver=0
> > > > type=PATH msg=audit(1519924845.499:257): item=1 name="/tmp/tmpcontainerid" inode=17729 dev=00:27 mode=0100644 ouid=0 ogid=0 rdev=00:00 obj=unconfined_u:object_r:user_tmp_t:s0 nametype=CREATE cap_fp=0000000000000000 cap_fi=0000000000000000 cap_fe=0 cap_fver=0
> > > > type=PROCTITLE msg=audit(1519924845.499:257): proctitle=62617368002D6300736C65657020313B206563686F2074657374203E202F746D702F746D70636F6E7461696E65726964
> > > > type=CONTAINER msg=audit(1519924845.499:257): op=task contid=123458
> > > >
> > > > See: https://github.com/linux-audit/audit-kernel/issues/90
> > > > See: https://github.com/linux-audit/audit-userspace/issues/51
> > > > See: https://github.com/linux-audit/audit-testsuite/issues/64
> > > > See: https://github.com/linux-audit/audit-kernel/wiki/RFE-Audit-Container-ID
> > > > Signed-off-by: Richard Guy Briggs <rgb@redhat.com>
> > > > Acked-by: Serge Hallyn <serge@hallyn.com>
> > > > Acked-by: Steve Grubb <sgrubb@redhat.com>
> > > > ---
> > > >  include/linux/audit.h      |  7 +++++++
> > > >  include/uapi/linux/audit.h |  1 +
> > > >  kernel/audit.c             | 24 ++++++++++++++++++++++++
> > > >  kernel/auditsc.c           |  3 +++
> > > >  4 files changed, 35 insertions(+)
> > >
> > > ...
> > >
> > > > @@ -2045,6 +2045,30 @@ void audit_log_session_info(struct audit_buffer *ab)
> > > >         audit_log_format(ab, " auid=%u ses=%u", auid, sessionid);
> > > >  }
> > > >
> > > > +/*
> > > > + * audit_log_contid - report container info
> > > > + * @tsk: task to be recorded
> > > > + * @context: task or local context for record
> > > > + * @op: contid string description
> > > > + */
> > > > +int audit_log_contid(struct task_struct *tsk,
> > > > +                            struct audit_context *context, char *op)
> > > > +{
> > > > +       struct audit_buffer *ab;
> > > > +
> > > > +       if (!audit_contid_set(tsk))
> > > > +               return 0;
> > > > +       /* Generate AUDIT_CONTAINER record with container ID */
> > > > +       ab = audit_log_start(context, GFP_KERNEL, AUDIT_CONTAINER);
> > > > +       if (!ab)
> > > > +               return -ENOMEM;
> > > > +       audit_log_format(ab, "op=%s contid=%llu",
> > > > +                        op, audit_get_contid(tsk));
> > > > +       audit_log_end(ab);
> > > > +       return 0;
> > > > +}
> > > > +EXPORT_SYMBOL(audit_log_contid);
> > >
> > > As discussed in the previous iteration of the patch, I prefer
> > > AUDIT_CONTAINER_ID here over AUDIT_CONTAINER.  If you feel strongly
> > > about keeping it as-is with AUDIT_CONTAINER I suppose I could live
> > > with that, but it is isn't my first choice.
> >
> > I don't have a strong opinion on this one, mildly preferring the shorter
> > one only because it is shorter.
> 
> We already have multiple AUDIT_CONTAINER* record types, so it seems as
> though we should use "AUDIT_CONTAINER" as a prefix of sorts, rather
> than a type itself.

I'm fine with that.  I'd still like to hear Steve's input.  He had
stronger opinions than me.

> > > However, I do care about the "op" field in this record.  It just
> > > doesn't make any sense; the way you are using it it is more of a
> > > context field than an operations field, and even then why is the
> > > context important from a logging and/or security perspective?  Drop it
> > > please.
> >
> > I'll rename it to whatever you like.  I'd suggest "ref=".  The reason I
> > think it is important is there are multiple sources that aren't always
> > obvious from the other records to which it is associated.  In the case
> > of ptrace and signals, there can be many target tasks listed (OBJ_PID)
> > with no other way to distinguish the matching audit container identifier
> > records all for one event.  This is in addition to the default syscall
> > container identifier record.  I'm not currently happy with the text
> > content to link the two, but that should be solvable (most obvious is
> > taret PID).  Throwing away this information seems shortsighted.
> 
> It would be helpful if you could generate real audit events
> demonstrating the problems you are describing, as well as a more
> standard syscall event, so we can discuss some possible solutions.

If the auditted process is in a container and it ptraces or signals
another process in a container, there will be two AUDIT_CONTAINER
records for the same event that won't be identified as to which record
belongs to which process or other record (SYSCALL vs 1+ OBJ_PID
records).  There could be many signals recorded, each with their own
OBJ_PID record.  The first is stored in the audit context and additional
ones are stored in a chained struct that can accommodate 16 entries each.

(See audit_signal_info(), __audit_ptrace().)

(As a side note, on code inspection it appears that a signal target
would get overwritten by a ptrace action if they were to happen in that
order.)

> 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
Steve Grubb Oct. 25, 2018, 6:06 a.m.
On Wed, 24 Oct 2018 20:42:55 -0400
Richard Guy Briggs <rgb@redhat.com> wrote:

> On 2018-10-24 16:55, Paul Moore wrote:
> > On Wed, Oct 24, 2018 at 11:15 AM Richard Guy Briggs
> > <rgb@redhat.com> wrote:  
> > > On 2018-10-19 19:16, Paul Moore wrote:  
> > > > On Sun, Aug 5, 2018 at 4:32 AM Richard Guy Briggs
> > > > <rgb@redhat.com> wrote:  
> > > > > Create a new audit record AUDIT_CONTAINER to document the
> > > > > audit container identifier of a process if it is present.
> > > > >
> > > > > Called from audit_log_exit(), syscalls are covered.
> > > > >
> > > > > A sample raw event:
> > > > > type=SYSCALL msg=audit(1519924845.499:257): arch=c000003e
> > > > > syscall=257 success=yes exit=3 a0=ffffff9c a1=56374e1cef30
> > > > > a2=241 a3=1b6 items=2 ppid=606 pid=635 auid=0 uid=0 gid=0
> > > > > euid=0 suid=0 fsuid=0 egid=0 sgid=0 fsgid=0 tty=pts0 ses=3
> > > > > comm="bash" exe="/usr/bin/bash"
> > > > > subj=unconfined_u:unconfined_r:unconfined_t:s0-s0:c0.c1023
> > > > > key="tmpcontainerid" type=CWD msg=audit(1519924845.499:257):
> > > > > cwd="/root" type=PATH msg=audit(1519924845.499:257): item=0
> > > > > name="/tmp/" inode=13863 dev=00:27 mode=041777 ouid=0 ogid=0
> > > > > rdev=00:00 obj=system_u:object_r:tmp_t:s0 nametype= PARENT
> > > > > cap_fp=0000000000000000 cap_fi=0000000000000000 cap_fe=0
> > > > > cap_fver=0 type=PATH msg=audit(1519924845.499:257): item=1
> > > > > name="/tmp/tmpcontainerid" inode=17729 dev=00:27 mode=0100644
> > > > > ouid=0 ogid=0 rdev=00:00
> > > > > obj=unconfined_u:object_r:user_tmp_t:s0 nametype=CREATE
> > > > > cap_fp=0000000000000000 cap_fi=0000000000000000 cap_fe=0
> > > > > cap_fver=0 type=PROCTITLE msg=audit(1519924845.499:257):
> > > > > proctitle=62617368002D6300736C65657020313B206563686F2074657374203E202F746D702F746D70636F6E7461696E65726964
> > > > > type=CONTAINER msg=audit(1519924845.499:257): op=task
> > > > > contid=123458
> > > > >
> > > > > See: https://github.com/linux-audit/audit-kernel/issues/90
> > > > > See: https://github.com/linux-audit/audit-userspace/issues/51
> > > > > See: https://github.com/linux-audit/audit-testsuite/issues/64
> > > > > See:
> > > > > https://github.com/linux-audit/audit-kernel/wiki/RFE-Audit-Container-ID
> > > > > Signed-off-by: Richard Guy Briggs <rgb@redhat.com> Acked-by:
> > > > > Serge Hallyn <serge@hallyn.com> Acked-by: Steve Grubb
> > > > > <sgrubb@redhat.com> ---
> > > > >  include/linux/audit.h      |  7 +++++++
> > > > >  include/uapi/linux/audit.h |  1 +
> > > > >  kernel/audit.c             | 24 ++++++++++++++++++++++++
> > > > >  kernel/auditsc.c           |  3 +++
> > > > >  4 files changed, 35 insertions(+)  
> > > >
> > > > ...
> > > >  
> > > > > @@ -2045,6 +2045,30 @@ void audit_log_session_info(struct
> > > > > audit_buffer *ab) audit_log_format(ab, " auid=%u ses=%u",
> > > > > auid, sessionid); }
> > > > >
> > > > > +/*
> > > > > + * audit_log_contid - report container info
> > > > > + * @tsk: task to be recorded
> > > > > + * @context: task or local context for record
> > > > > + * @op: contid string description
> > > > > + */
> > > > > +int audit_log_contid(struct task_struct *tsk,
> > > > > +                            struct audit_context *context,
> > > > > char *op) +{
> > > > > +       struct audit_buffer *ab;
> > > > > +
> > > > > +       if (!audit_contid_set(tsk))
> > > > > +               return 0;
> > > > > +       /* Generate AUDIT_CONTAINER record with container ID
> > > > > */
> > > > > +       ab = audit_log_start(context, GFP_KERNEL,
> > > > > AUDIT_CONTAINER);
> > > > > +       if (!ab)
> > > > > +               return -ENOMEM;
> > > > > +       audit_log_format(ab, "op=%s contid=%llu",
> > > > > +                        op, audit_get_contid(tsk));
> > > > > +       audit_log_end(ab);
> > > > > +       return 0;
> > > > > +}
> > > > > +EXPORT_SYMBOL(audit_log_contid);  
> > > >
> > > > As discussed in the previous iteration of the patch, I prefer
> > > > AUDIT_CONTAINER_ID here over AUDIT_CONTAINER.  If you feel
> > > > strongly about keeping it as-is with AUDIT_CONTAINER I suppose
> > > > I could live with that, but it is isn't my first choice.  
> > >
> > > I don't have a strong opinion on this one, mildly preferring the
> > > shorter one only because it is shorter.  
> > 
> > We already have multiple AUDIT_CONTAINER* record types, so it seems
> > as though we should use "AUDIT_CONTAINER" as a prefix of sorts,
> > rather than a type itself.  
> 
> I'm fine with that.  I'd still like to hear Steve's input.  He had
> stronger opinions than me.

The creation event should be separate and distinct from the continuing
use when its used as a supplemental record. IOW, binding the ID to a
container is part of the lifecycle and needs to be kept distinct.

-Steve

> > > > However, I do care about the "op" field in this record.  It just
> > > > doesn't make any sense; the way you are using it it is more of a
> > > > context field than an operations field, and even then why is the
> > > > context important from a logging and/or security perspective?
> > > > Drop it please.  
> > >
> > > I'll rename it to whatever you like.  I'd suggest "ref=".  The
> > > reason I think it is important is there are multiple sources that
> > > aren't always obvious from the other records to which it is
> > > associated.  In the case of ptrace and signals, there can be many
> > > target tasks listed (OBJ_PID) with no other way to distinguish
> > > the matching audit container identifier records all for one
> > > event.  This is in addition to the default syscall container
> > > identifier record.  I'm not currently happy with the text content
> > > to link the two, but that should be solvable (most obvious is
> > > taret PID).  Throwing away this information seems shortsighted.  
> > 
> > It would be helpful if you could generate real audit events
> > demonstrating the problems you are describing, as well as a more
> > standard syscall event, so we can discuss some possible solutions.  
> 
> If the auditted process is in a container and it ptraces or signals
> another process in a container, there will be two AUDIT_CONTAINER
> records for the same event that won't be identified as to which record
> belongs to which process or other record (SYSCALL vs 1+ OBJ_PID
> records).  There could be many signals recorded, each with their own
> OBJ_PID record.  The first is stored in the audit context and
> additional ones are stored in a chained struct that can accommodate
> 16 entries each.
> 
> (See audit_signal_info(), __audit_ptrace().)
> 
> (As a side note, on code inspection it appears that a signal target
> would get overwritten by a ptrace action if they were to happen in
> that order.)
> 
> > 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
> 
> --
> Linux-audit mailing list
> Linux-audit@redhat.com
> https://www.redhat.com/mailman/listinfo/linux-audit
Paul Moore Oct. 25, 2018, 6:13 a.m.
On October 25, 2018 1:43:16 AM Richard Guy Briggs <rgb@redhat.com> wrote:
> On 2018-10-24 16:55, Paul Moore wrote:
>> On Wed, Oct 24, 2018 at 11:15 AM Richard Guy Briggs <rgb@redhat.com> wrote:
>>> On 2018-10-19 19:16, Paul Moore wrote:
>>>> On Sun, Aug 5, 2018 at 4:32 AM Richard Guy Briggs <rgb@redhat.com> wrote:
>

...

>
>>>> However, I do care about the "op" field in this record.  It just
>>>> doesn't make any sense; the way you are using it it is more of a
>>>> context field than an operations field, and even then why is the
>>>> context important from a logging and/or security perspective?  Drop it
>>>> please.
>>>
>>> I'll rename it to whatever you like.  I'd suggest "ref=".  The reason I
>>> think it is important is there are multiple sources that aren't always
>>> obvious from the other records to which it is associated.  In the case
>>> of ptrace and signals, there can be many target tasks listed (OBJ_PID)
>>> with no other way to distinguish the matching audit container identifier
>>> records all for one event.  This is in addition to the default syscall
>>> container identifier record.  I'm not currently happy with the text
>>> content to link the two, but that should be solvable (most obvious is
>>> taret PID).  Throwing away this information seems shortsighted.
>>
>> It would be helpful if you could generate real audit events
>> demonstrating the problems you are describing, as well as a more
>> standard syscall event, so we can discuss some possible solutions.
>
> If the auditted process is in a container and it ptraces or signals
> another process in a container, there will be two AUDIT_CONTAINER
> records for the same event that won't be identified as to which record
> belongs to which process or other record (SYSCALL vs 1+ OBJ_PID
> records).  There could be many signals recorded, each with their own
> OBJ_PID record.  The first is stored in the audit context and additional
> ones are stored in a chained struct that can accommodate 16 entries each.
>
> (See audit_signal_info(), __audit_ptrace().)
>
> (As a side note, on code inspection it appears that a signal target
> would get overwritten by a ptrace action if they were to happen in that
> order.)

As requested above, please respond with real audit events generated by this patchset so that we can discuss possible solutions.

--
paul moore
www.paul-moore.com
Paul Moore Oct. 25, 2018, 10:49 a.m.
On Thu, Oct 25, 2018 at 2:06 AM Steve Grubb <sgrubb@redhat.com> wrote:
> On Wed, 24 Oct 2018 20:42:55 -0400
> Richard Guy Briggs <rgb@redhat.com> wrote:
> > On 2018-10-24 16:55, Paul Moore wrote:
> > > On Wed, Oct 24, 2018 at 11:15 AM Richard Guy Briggs
> > > <rgb@redhat.com> wrote:
> > > > On 2018-10-19 19:16, Paul Moore wrote:
> > > > > On Sun, Aug 5, 2018 at 4:32 AM Richard Guy Briggs
> > > > > <rgb@redhat.com> wrote:

...

> > > > > > +/*
> > > > > > + * audit_log_contid - report container info
> > > > > > + * @tsk: task to be recorded
> > > > > > + * @context: task or local context for record
> > > > > > + * @op: contid string description
> > > > > > + */
> > > > > > +int audit_log_contid(struct task_struct *tsk,
> > > > > > +                            struct audit_context *context,
> > > > > > char *op) +{
> > > > > > +       struct audit_buffer *ab;
> > > > > > +
> > > > > > +       if (!audit_contid_set(tsk))
> > > > > > +               return 0;
> > > > > > +       /* Generate AUDIT_CONTAINER record with container ID
> > > > > > */
> > > > > > +       ab = audit_log_start(context, GFP_KERNEL,
> > > > > > AUDIT_CONTAINER);
> > > > > > +       if (!ab)
> > > > > > +               return -ENOMEM;
> > > > > > +       audit_log_format(ab, "op=%s contid=%llu",
> > > > > > +                        op, audit_get_contid(tsk));
> > > > > > +       audit_log_end(ab);
> > > > > > +       return 0;
> > > > > > +}
> > > > > > +EXPORT_SYMBOL(audit_log_contid);
> > > > >
> > > > > As discussed in the previous iteration of the patch, I prefer
> > > > > AUDIT_CONTAINER_ID here over AUDIT_CONTAINER.  If you feel
> > > > > strongly about keeping it as-is with AUDIT_CONTAINER I suppose
> > > > > I could live with that, but it is isn't my first choice.
> > > >
> > > > I don't have a strong opinion on this one, mildly preferring the
> > > > shorter one only because it is shorter.
> > >
> > > We already have multiple AUDIT_CONTAINER* record types, so it seems
> > > as though we should use "AUDIT_CONTAINER" as a prefix of sorts,
> > > rather than a type itself.
> >
> > I'm fine with that.  I'd still like to hear Steve's input.  He had
> > stronger opinions than me.
>
> The creation event should be separate and distinct from the continuing
> use when its used as a supplemental record. IOW, binding the ID to a
> container is part of the lifecycle and needs to be kept distinct.

Steve's comment is pretty ambiguous when it comes to AUDIT_CONTAINER
vs AUDIT_CONTAINER_ID, but one could argue that AUDIT_CONTAINER_ID
helps distinguish the audit container id marking record and gets to
what I believe is the spirit of Steve's comment.  Taking this in
context with my previous remarks, let's switch to using
AUDIT_CONTAINER_ID.
Richard Guy Briggs Oct. 25, 2018, 12:22 p.m.
On 2018-10-25 07:13, Paul Moore wrote:
> On October 25, 2018 1:43:16 AM Richard Guy Briggs <rgb@redhat.com> wrote:
> > On 2018-10-24 16:55, Paul Moore wrote:
> >> On Wed, Oct 24, 2018 at 11:15 AM Richard Guy Briggs <rgb@redhat.com> wrote:
> >>> On 2018-10-19 19:16, Paul Moore wrote:
> >>>> On Sun, Aug 5, 2018 at 4:32 AM Richard Guy Briggs <rgb@redhat.com> wrote:
> >
> 
> ...
> 
> >
> >>>> However, I do care about the "op" field in this record.  It just
> >>>> doesn't make any sense; the way you are using it it is more of a
> >>>> context field than an operations field, and even then why is the
> >>>> context important from a logging and/or security perspective?  Drop it
> >>>> please.
> >>>
> >>> I'll rename it to whatever you like.  I'd suggest "ref=".  The reason I
> >>> think it is important is there are multiple sources that aren't always
> >>> obvious from the other records to which it is associated.  In the case
> >>> of ptrace and signals, there can be many target tasks listed (OBJ_PID)
> >>> with no other way to distinguish the matching audit container identifier
> >>> records all for one event.  This is in addition to the default syscall
> >>> container identifier record.  I'm not currently happy with the text
> >>> content to link the two, but that should be solvable (most obvious is
> >>> taret PID).  Throwing away this information seems shortsighted.
> >>
> >> It would be helpful if you could generate real audit events
> >> demonstrating the problems you are describing, as well as a more
> >> standard syscall event, so we can discuss some possible solutions.
> >
> > If the auditted process is in a container and it ptraces or signals
> > another process in a container, there will be two AUDIT_CONTAINER
> > records for the same event that won't be identified as to which record
> > belongs to which process or other record (SYSCALL vs 1+ OBJ_PID
> > records).  There could be many signals recorded, each with their own
> > OBJ_PID record.  The first is stored in the audit context and additional
> > ones are stored in a chained struct that can accommodate 16 entries each.
> >
> > (See audit_signal_info(), __audit_ptrace().)
> >
> > (As a side note, on code inspection it appears that a signal target
> > would get overwritten by a ptrace action if they were to happen in that
> > order.)
> 
> As requested above, please respond with real audit events generated by
> this patchset so that we can discuss possible solutions.

Ok, then we should be developping a test to test ptrace and signal
auditting in general since we don't have current experience/evidence
that those even work (or rip them out if not).

> 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
Richard Guy Briggs Oct. 25, 2018, 12:27 p.m.
On 2018-10-25 06:49, Paul Moore wrote:
> On Thu, Oct 25, 2018 at 2:06 AM Steve Grubb <sgrubb@redhat.com> wrote:
> > On Wed, 24 Oct 2018 20:42:55 -0400
> > Richard Guy Briggs <rgb@redhat.com> wrote:
> > > On 2018-10-24 16:55, Paul Moore wrote:
> > > > On Wed, Oct 24, 2018 at 11:15 AM Richard Guy Briggs
> > > > <rgb@redhat.com> wrote:
> > > > > On 2018-10-19 19:16, Paul Moore wrote:
> > > > > > On Sun, Aug 5, 2018 at 4:32 AM Richard Guy Briggs
> > > > > > <rgb@redhat.com> wrote:
> 
> ...
> 
> > > > > > > +/*
> > > > > > > + * audit_log_contid - report container info
> > > > > > > + * @tsk: task to be recorded
> > > > > > > + * @context: task or local context for record
> > > > > > > + * @op: contid string description
> > > > > > > + */
> > > > > > > +int audit_log_contid(struct task_struct *tsk,
> > > > > > > +                            struct audit_context *context,
> > > > > > > char *op) +{
> > > > > > > +       struct audit_buffer *ab;
> > > > > > > +
> > > > > > > +       if (!audit_contid_set(tsk))
> > > > > > > +               return 0;
> > > > > > > +       /* Generate AUDIT_CONTAINER record with container ID
> > > > > > > */
> > > > > > > +       ab = audit_log_start(context, GFP_KERNEL,
> > > > > > > AUDIT_CONTAINER);
> > > > > > > +       if (!ab)
> > > > > > > +               return -ENOMEM;
> > > > > > > +       audit_log_format(ab, "op=%s contid=%llu",
> > > > > > > +                        op, audit_get_contid(tsk));
> > > > > > > +       audit_log_end(ab);
> > > > > > > +       return 0;
> > > > > > > +}
> > > > > > > +EXPORT_SYMBOL(audit_log_contid);
> > > > > >
> > > > > > As discussed in the previous iteration of the patch, I prefer
> > > > > > AUDIT_CONTAINER_ID here over AUDIT_CONTAINER.  If you feel
> > > > > > strongly about keeping it as-is with AUDIT_CONTAINER I suppose
> > > > > > I could live with that, but it is isn't my first choice.
> > > > >
> > > > > I don't have a strong opinion on this one, mildly preferring the
> > > > > shorter one only because it is shorter.
> > > >
> > > > We already have multiple AUDIT_CONTAINER* record types, so it seems
> > > > as though we should use "AUDIT_CONTAINER" as a prefix of sorts,
> > > > rather than a type itself.
> > >
> > > I'm fine with that.  I'd still like to hear Steve's input.  He had
> > > stronger opinions than me.
> >
> > The creation event should be separate and distinct from the continuing
> > use when its used as a supplemental record. IOW, binding the ID to a
> > container is part of the lifecycle and needs to be kept distinct.
> 
> Steve's comment is pretty ambiguous when it comes to AUDIT_CONTAINER
> vs AUDIT_CONTAINER_ID, but one could argue that AUDIT_CONTAINER_ID
> helps distinguish the audit container id marking record and gets to
> what I believe is the spirit of Steve's comment.  Taking this in
> context with my previous remarks, let's switch to using
> AUDIT_CONTAINER_ID.

I suspect Steve is mixing up AUDIT_CONTAINER_OP with AUDIT_CONTAINER_ID,
confusing the fact that they are two seperate records.  As a summary,
the suggested records are:
	CONTAINER_OP	audit container identifier creation
	CONTAINER	audit container identifier aux record to an event

and what Paul is suggesting (which is fine by me) is:
	CONTAINER_OP	audit container identifier creation event
	CONTAINER_ID	audit container identifier aux record to an event

Steve, please indicate you are fine with this.

> 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
Steve Grubb Oct. 25, 2018, 3:57 p.m.
On Thu, 25 Oct 2018 08:27:32 -0400
Richard Guy Briggs <rgb@redhat.com> wrote:

> On 2018-10-25 06:49, Paul Moore wrote:
> > On Thu, Oct 25, 2018 at 2:06 AM Steve Grubb <sgrubb@redhat.com>
> > wrote:  
> > > On Wed, 24 Oct 2018 20:42:55 -0400
> > > Richard Guy Briggs <rgb@redhat.com> wrote:  
> > > > On 2018-10-24 16:55, Paul Moore wrote:  
> > > > > On Wed, Oct 24, 2018 at 11:15 AM Richard Guy Briggs
> > > > > <rgb@redhat.com> wrote:  
> > > > > > On 2018-10-19 19:16, Paul Moore wrote:  
> > > > > > > On Sun, Aug 5, 2018 at 4:32 AM Richard Guy Briggs
> > > > > > > <rgb@redhat.com> wrote:  
> > 
> > ...
> >   
> > > > > > > > +/*
> > > > > > > > + * audit_log_contid - report container info
> > > > > > > > + * @tsk: task to be recorded
> > > > > > > > + * @context: task or local context for record
> > > > > > > > + * @op: contid string description
> > > > > > > > + */
> > > > > > > > +int audit_log_contid(struct task_struct *tsk,
> > > > > > > > +                            struct audit_context
> > > > > > > > *context, char *op) +{
> > > > > > > > +       struct audit_buffer *ab;
> > > > > > > > +
> > > > > > > > +       if (!audit_contid_set(tsk))
> > > > > > > > +               return 0;
> > > > > > > > +       /* Generate AUDIT_CONTAINER record with
> > > > > > > > container ID */
> > > > > > > > +       ab = audit_log_start(context, GFP_KERNEL,
> > > > > > > > AUDIT_CONTAINER);
> > > > > > > > +       if (!ab)
> > > > > > > > +               return -ENOMEM;
> > > > > > > > +       audit_log_format(ab, "op=%s contid=%llu",
> > > > > > > > +                        op, audit_get_contid(tsk));
> > > > > > > > +       audit_log_end(ab);
> > > > > > > > +       return 0;
> > > > > > > > +}
> > > > > > > > +EXPORT_SYMBOL(audit_log_contid);  
> > > > > > >
> > > > > > > As discussed in the previous iteration of the patch, I
> > > > > > > prefer AUDIT_CONTAINER_ID here over AUDIT_CONTAINER.  If
> > > > > > > you feel strongly about keeping it as-is with
> > > > > > > AUDIT_CONTAINER I suppose I could live with that, but it
> > > > > > > is isn't my first choice.  
> > > > > >
> > > > > > I don't have a strong opinion on this one, mildly
> > > > > > preferring the shorter one only because it is shorter.  
> > > > >
> > > > > We already have multiple AUDIT_CONTAINER* record types, so it
> > > > > seems as though we should use "AUDIT_CONTAINER" as a prefix
> > > > > of sorts, rather than a type itself.  
> > > >
> > > > I'm fine with that.  I'd still like to hear Steve's input.  He
> > > > had stronger opinions than me.  
> > >
> > > The creation event should be separate and distinct from the
> > > continuing use when its used as a supplemental record. IOW,
> > > binding the ID to a container is part of the lifecycle and needs
> > > to be kept distinct.  
> > 
> > Steve's comment is pretty ambiguous when it comes to AUDIT_CONTAINER
> > vs AUDIT_CONTAINER_ID, but one could argue that AUDIT_CONTAINER_ID
> > helps distinguish the audit container id marking record and gets to
> > what I believe is the spirit of Steve's comment.  Taking this in
> > context with my previous remarks, let's switch to using
> > AUDIT_CONTAINER_ID.  
> 
> I suspect Steve is mixing up AUDIT_CONTAINER_OP with
> AUDIT_CONTAINER_ID, confusing the fact that they are two seperate
> records.  As a summary, the suggested records are:
> 	CONTAINER_OP	audit container identifier creation
> 	CONTAINER	audit container identifier aux record to an
> event
> 
> and what Paul is suggesting (which is fine by me) is:
> 	CONTAINER_OP	audit container identifier creation event
> 	CONTAINER_ID	audit container identifier aux record to
> an event
> 
> Steve, please indicate you are fine with this.

I thought it was:

CONTAINER_ID audit container identifier creation event
CONTAINER audit container identifier aux record to an event

Or vice versa. Don't mix up creation of the identifier with operations.

-Steve
Richard Guy Briggs Oct. 25, 2018, 5:38 p.m.
On 2018-10-25 17:57, Steve Grubb wrote:
> On Thu, 25 Oct 2018 08:27:32 -0400
> Richard Guy Briggs <rgb@redhat.com> wrote:
> 
> > On 2018-10-25 06:49, Paul Moore wrote:
> > > On Thu, Oct 25, 2018 at 2:06 AM Steve Grubb <sgrubb@redhat.com>
> > > wrote:  
> > > > On Wed, 24 Oct 2018 20:42:55 -0400
> > > > Richard Guy Briggs <rgb@redhat.com> wrote:  
> > > > > On 2018-10-24 16:55, Paul Moore wrote:  
> > > > > > On Wed, Oct 24, 2018 at 11:15 AM Richard Guy Briggs
> > > > > > <rgb@redhat.com> wrote:  
> > > > > > > On 2018-10-19 19:16, Paul Moore wrote:  
> > > > > > > > On Sun, Aug 5, 2018 at 4:32 AM Richard Guy Briggs
> > > > > > > > <rgb@redhat.com> wrote:  
> > > 
> > > ...
> > >   
> > > > > > > > > +/*
> > > > > > > > > + * audit_log_contid - report container info
> > > > > > > > > + * @tsk: task to be recorded
> > > > > > > > > + * @context: task or local context for record
> > > > > > > > > + * @op: contid string description
> > > > > > > > > + */
> > > > > > > > > +int audit_log_contid(struct task_struct *tsk,
> > > > > > > > > +                            struct audit_context
> > > > > > > > > *context, char *op) +{
> > > > > > > > > +       struct audit_buffer *ab;
> > > > > > > > > +
> > > > > > > > > +       if (!audit_contid_set(tsk))
> > > > > > > > > +               return 0;
> > > > > > > > > +       /* Generate AUDIT_CONTAINER record with
> > > > > > > > > container ID */
> > > > > > > > > +       ab = audit_log_start(context, GFP_KERNEL,
> > > > > > > > > AUDIT_CONTAINER);
> > > > > > > > > +       if (!ab)
> > > > > > > > > +               return -ENOMEM;
> > > > > > > > > +       audit_log_format(ab, "op=%s contid=%llu",
> > > > > > > > > +                        op, audit_get_contid(tsk));
> > > > > > > > > +       audit_log_end(ab);
> > > > > > > > > +       return 0;
> > > > > > > > > +}
> > > > > > > > > +EXPORT_SYMBOL(audit_log_contid);  
> > > > > > > >
> > > > > > > > As discussed in the previous iteration of the patch, I
> > > > > > > > prefer AUDIT_CONTAINER_ID here over AUDIT_CONTAINER.  If
> > > > > > > > you feel strongly about keeping it as-is with
> > > > > > > > AUDIT_CONTAINER I suppose I could live with that, but it
> > > > > > > > is isn't my first choice.  
> > > > > > >
> > > > > > > I don't have a strong opinion on this one, mildly
> > > > > > > preferring the shorter one only because it is shorter.  
> > > > > >
> > > > > > We already have multiple AUDIT_CONTAINER* record types, so it
> > > > > > seems as though we should use "AUDIT_CONTAINER" as a prefix
> > > > > > of sorts, rather than a type itself.  
> > > > >
> > > > > I'm fine with that.  I'd still like to hear Steve's input.  He
> > > > > had stronger opinions than me.  
> > > >
> > > > The creation event should be separate and distinct from the
> > > > continuing use when its used as a supplemental record. IOW,
> > > > binding the ID to a container is part of the lifecycle and needs
> > > > to be kept distinct.  
> > > 
> > > Steve's comment is pretty ambiguous when it comes to AUDIT_CONTAINER
> > > vs AUDIT_CONTAINER_ID, but one could argue that AUDIT_CONTAINER_ID
> > > helps distinguish the audit container id marking record and gets to
> > > what I believe is the spirit of Steve's comment.  Taking this in
> > > context with my previous remarks, let's switch to using
> > > AUDIT_CONTAINER_ID.  
> > 
> > I suspect Steve is mixing up AUDIT_CONTAINER_OP with
> > AUDIT_CONTAINER_ID, confusing the fact that they are two seperate
> > records.  As a summary, the suggested records are:
> > 	CONTAINER_OP	audit container identifier creation
> > 	CONTAINER	audit container identifier aux record to an
> > event
> > 
> > and what Paul is suggesting (which is fine by me) is:
> > 	CONTAINER_OP	audit container identifier creation event
> > 	CONTAINER_ID	audit container identifier aux record to
> > an event
> > 
> > Steve, please indicate you are fine with this.
> 
> I thought it was:

It *was*.  It was changed at Paul's request in this v3 thread:
	https://www.redhat.com/archives/linux-audit/2018-July/msg00087.html

And listed in the examples and changelog to this v4 patchset:
	https://www.redhat.com/archives/linux-audit/2018-July/msg00178.html

It is also listed in this userspace patchset update v4 (which should
also have had a changelog added to it, note to self...):
	https://www.redhat.com/archives/linux-audit/2018-July/msg00189.html

I realize it is hard to keep up with all the detail changes in these
patchsets...

> CONTAINER_ID audit container identifier creation event
> CONTAINER audit container identifier aux record to an event
> 
> Or vice versa. Don't mix up creation of the identifier with operations.

Exactly what I'm trying to avoid...  Worded another way: "Don't mix up
the creation operation with routine reporting of the identifier in
events."  Steve, can you and Paul discuss and agree on what they should
be called?  I don't have a horse in this race, but I need to record the
result of that run.  ;-)

> -Steve

- 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
Paul Moore Oct. 25, 2018, 8:40 p.m.
On Thu, Oct 25, 2018 at 1:38 PM Richard Guy Briggs <rgb@redhat.com> wrote:
> On 2018-10-25 17:57, Steve Grubb wrote:
> > On Thu, 25 Oct 2018 08:27:32 -0400
> > Richard Guy Briggs <rgb@redhat.com> wrote:
> >
> > > On 2018-10-25 06:49, Paul Moore wrote:
> > > > On Thu, Oct 25, 2018 at 2:06 AM Steve Grubb <sgrubb@redhat.com>
> > > > wrote:
> > > > > On Wed, 24 Oct 2018 20:42:55 -0400
> > > > > Richard Guy Briggs <rgb@redhat.com> wrote:
> > > > > > On 2018-10-24 16:55, Paul Moore wrote:
> > > > > > > On Wed, Oct 24, 2018 at 11:15 AM Richard Guy Briggs
> > > > > > > <rgb@redhat.com> wrote:
> > > > > > > > On 2018-10-19 19:16, Paul Moore wrote:
> > > > > > > > > On Sun, Aug 5, 2018 at 4:32 AM Richard Guy Briggs
> > > > > > > > > <rgb@redhat.com> wrote:
> > > >
> > > > ...
> > > >
> > > > > > > > > > +/*
> > > > > > > > > > + * audit_log_contid - report container info
> > > > > > > > > > + * @tsk: task to be recorded
> > > > > > > > > > + * @context: task or local context for record
> > > > > > > > > > + * @op: contid string description
> > > > > > > > > > + */
> > > > > > > > > > +int audit_log_contid(struct task_struct *tsk,
> > > > > > > > > > +                            struct audit_context
> > > > > > > > > > *context, char *op) +{
> > > > > > > > > > +       struct audit_buffer *ab;
> > > > > > > > > > +
> > > > > > > > > > +       if (!audit_contid_set(tsk))
> > > > > > > > > > +               return 0;
> > > > > > > > > > +       /* Generate AUDIT_CONTAINER record with
> > > > > > > > > > container ID */
> > > > > > > > > > +       ab = audit_log_start(context, GFP_KERNEL,
> > > > > > > > > > AUDIT_CONTAINER);
> > > > > > > > > > +       if (!ab)
> > > > > > > > > > +               return -ENOMEM;
> > > > > > > > > > +       audit_log_format(ab, "op=%s contid=%llu",
> > > > > > > > > > +                        op, audit_get_contid(tsk));
> > > > > > > > > > +       audit_log_end(ab);
> > > > > > > > > > +       return 0;
> > > > > > > > > > +}
> > > > > > > > > > +EXPORT_SYMBOL(audit_log_contid);
> > > > > > > > >
> > > > > > > > > As discussed in the previous iteration of the patch, I
> > > > > > > > > prefer AUDIT_CONTAINER_ID here over AUDIT_CONTAINER.  If
> > > > > > > > > you feel strongly about keeping it as-is with
> > > > > > > > > AUDIT_CONTAINER I suppose I could live with that, but it
> > > > > > > > > is isn't my first choice.
> > > > > > > >
> > > > > > > > I don't have a strong opinion on this one, mildly
> > > > > > > > preferring the shorter one only because it is shorter.
> > > > > > >
> > > > > > > We already have multiple AUDIT_CONTAINER* record types, so it
> > > > > > > seems as though we should use "AUDIT_CONTAINER" as a prefix
> > > > > > > of sorts, rather than a type itself.
> > > > > >
> > > > > > I'm fine with that.  I'd still like to hear Steve's input.  He
> > > > > > had stronger opinions than me.
> > > > >
> > > > > The creation event should be separate and distinct from the
> > > > > continuing use when its used as a supplemental record. IOW,
> > > > > binding the ID to a container is part of the lifecycle and needs
> > > > > to be kept distinct.
> > > >
> > > > Steve's comment is pretty ambiguous when it comes to AUDIT_CONTAINER
> > > > vs AUDIT_CONTAINER_ID, but one could argue that AUDIT_CONTAINER_ID
> > > > helps distinguish the audit container id marking record and gets to
> > > > what I believe is the spirit of Steve's comment.  Taking this in
> > > > context with my previous remarks, let's switch to using
> > > > AUDIT_CONTAINER_ID.
> > >
> > > I suspect Steve is mixing up AUDIT_CONTAINER_OP with
> > > AUDIT_CONTAINER_ID, confusing the fact that they are two seperate
> > > records.  As a summary, the suggested records are:
> > >     CONTAINER_OP    audit container identifier creation
> > >     CONTAINER       audit container identifier aux record to an
> > > event
> > >
> > > and what Paul is suggesting (which is fine by me) is:
> > >     CONTAINER_OP    audit container identifier creation event
> > >     CONTAINER_ID    audit container identifier aux record to
> > > an event
> > >
> > > Steve, please indicate you are fine with this.
> >
> > I thought it was:
>
> It *was*.  It was changed at Paul's request in this v3 thread:
>         https://www.redhat.com/archives/linux-audit/2018-July/msg00087.html
>
> And listed in the examples and changelog to this v4 patchset:
>         https://www.redhat.com/archives/linux-audit/2018-July/msg00178.html
>
> It is also listed in this userspace patchset update v4 (which should
> also have had a changelog added to it, note to self...):
>         https://www.redhat.com/archives/linux-audit/2018-July/msg00189.html
>
> I realize it is hard to keep up with all the detail changes in these
> patchsets...
>
> > CONTAINER_ID audit container identifier creation event
> > CONTAINER audit container identifier aux record to an event
> >
> > Or vice versa. Don't mix up creation of the identifier with operations.
>
> Exactly what I'm trying to avoid...  Worded another way: "Don't mix up
> the creation operation with routine reporting of the identifier in
> events."  Steve, can you and Paul discuss and agree on what they should
> be called?  I don't have a horse in this race, but I need to record the
> result of that run.  ;-)

See my previous comments, I think I've been pretty clear on what I
would like to see.
Steve Grubb Oct. 25, 2018, 9:55 p.m.
On Thu, 25 Oct 2018 16:40:19 -0400
Paul Moore <paul@paul-moore.com> wrote:

> On Thu, Oct 25, 2018 at 1:38 PM Richard Guy Briggs <rgb@redhat.com>
> wrote:
> > On 2018-10-25 17:57, Steve Grubb wrote:  
> > > On Thu, 25 Oct 2018 08:27:32 -0400
> > > Richard Guy Briggs <rgb@redhat.com> wrote:
> > >  
> > > > On 2018-10-25 06:49, Paul Moore wrote:  
> > > > > On Thu, Oct 25, 2018 at 2:06 AM Steve Grubb
> > > > > <sgrubb@redhat.com> wrote:  
> > > > > > On Wed, 24 Oct 2018 20:42:55 -0400
> > > > > > Richard Guy Briggs <rgb@redhat.com> wrote:  
> > > > > > > On 2018-10-24 16:55, Paul Moore wrote:  
> > > > > > > > On Wed, Oct 24, 2018 at 11:15 AM Richard Guy Briggs
> > > > > > > > <rgb@redhat.com> wrote:  
> > > > > > > > > On 2018-10-19 19:16, Paul Moore wrote:  
> > > > > > > > > > On Sun, Aug 5, 2018 at 4:32 AM Richard Guy Briggs
> > > > > > > > > > <rgb@redhat.com> wrote:  
> > > > >
> > > > > ...
> > > > >  
> > > > > > > > > > > +/*
> > > > > > > > > > > + * audit_log_contid - report container info
> > > > > > > > > > > + * @tsk: task to be recorded
> > > > > > > > > > > + * @context: task or local context for record
> > > > > > > > > > > + * @op: contid string description
> > > > > > > > > > > + */
> > > > > > > > > > > +int audit_log_contid(struct task_struct *tsk,
> > > > > > > > > > > +                            struct audit_context
> > > > > > > > > > > *context, char *op) +{
> > > > > > > > > > > +       struct audit_buffer *ab;
> > > > > > > > > > > +
> > > > > > > > > > > +       if (!audit_contid_set(tsk))
> > > > > > > > > > > +               return 0;
> > > > > > > > > > > +       /* Generate AUDIT_CONTAINER record with
> > > > > > > > > > > container ID */
> > > > > > > > > > > +       ab = audit_log_start(context, GFP_KERNEL,
> > > > > > > > > > > AUDIT_CONTAINER);
> > > > > > > > > > > +       if (!ab)
> > > > > > > > > > > +               return -ENOMEM;
> > > > > > > > > > > +       audit_log_format(ab, "op=%s contid=%llu",
> > > > > > > > > > > +                        op,
> > > > > > > > > > > audit_get_contid(tsk));
> > > > > > > > > > > +       audit_log_end(ab);
> > > > > > > > > > > +       return 0;
> > > > > > > > > > > +}
> > > > > > > > > > > +EXPORT_SYMBOL(audit_log_contid);  
> > > > > > > > > >
> > > > > > > > > > As discussed in the previous iteration of the
> > > > > > > > > > patch, I prefer AUDIT_CONTAINER_ID here over
> > > > > > > > > > AUDIT_CONTAINER.  If you feel strongly about
> > > > > > > > > > keeping it as-is with AUDIT_CONTAINER I suppose I
> > > > > > > > > > could live with that, but it is isn't my first
> > > > > > > > > > choice.  
> > > > > > > > >
> > > > > > > > > I don't have a strong opinion on this one, mildly
> > > > > > > > > preferring the shorter one only because it is
> > > > > > > > > shorter.  
> > > > > > > >
> > > > > > > > We already have multiple AUDIT_CONTAINER* record types,
> > > > > > > > so it seems as though we should use "AUDIT_CONTAINER"
> > > > > > > > as a prefix of sorts, rather than a type itself.  
> > > > > > >
> > > > > > > I'm fine with that.  I'd still like to hear Steve's
> > > > > > > input.  He had stronger opinions than me.  
> > > > > >
> > > > > > The creation event should be separate and distinct from the
> > > > > > continuing use when its used as a supplemental record. IOW,
> > > > > > binding the ID to a container is part of the lifecycle and
> > > > > > needs to be kept distinct.  
> > > > >
> > > > > Steve's comment is pretty ambiguous when it comes to
> > > > > AUDIT_CONTAINER vs AUDIT_CONTAINER_ID, but one could argue
> > > > > that AUDIT_CONTAINER_ID helps distinguish the audit container
> > > > > id marking record and gets to what I believe is the spirit of
> > > > > Steve's comment.  Taking this in context with my previous
> > > > > remarks, let's switch to using AUDIT_CONTAINER_ID.  
> > > >
> > > > I suspect Steve is mixing up AUDIT_CONTAINER_OP with
> > > > AUDIT_CONTAINER_ID, confusing the fact that they are two
> > > > seperate records.  As a summary, the suggested records are:
> > > >     CONTAINER_OP    audit container identifier creation
> > > >     CONTAINER       audit container identifier aux record to an
> > > > event
> > > >
> > > > and what Paul is suggesting (which is fine by me) is:
> > > >     CONTAINER_OP    audit container identifier creation event
> > > >     CONTAINER_ID    audit container identifier aux record to
> > > > an event
> > > >
> > > > Steve, please indicate you are fine with this.  
> > >
> > > I thought it was:  
> >
> > It *was*.  It was changed at Paul's request in this v3 thread:
> >         https://www.redhat.com/archives/linux-audit/2018-July/msg00087.html
> >
> > And listed in the examples and changelog to this v4 patchset:
> >         https://www.redhat.com/archives/linux-audit/2018-July/msg00178.html
> >
> > It is also listed in this userspace patchset update v4 (which should
> > also have had a changelog added to it, note to self...):
> >         https://www.redhat.com/archives/linux-audit/2018-July/msg00189.html
> >
> > I realize it is hard to keep up with all the detail changes in these
> > patchsets...
> >  
> > > CONTAINER_ID audit container identifier creation event
> > > event. CONTAINER audit container identifier aux record to an
> > > event
> > >
> > > Or vice versa. Don't mix up creation of the identifier with
> > > operations.  
> >
> > Exactly what I'm trying to avoid...  Worded another way: "Don't mix
> > up the creation operation with routine reporting of the identifier
> > in events."  Steve, can you and Paul discuss and agree on what they
> > should be called?  I don't have a horse in this race, but I need to
> > record the result of that run.  ;-)  
> 
> See my previous comments, I think I've been pretty clear on what I
> would like to see.

And historically speaking setting audit loginuid produces a LOGIN
event, so it only makes sense to consider binding container ID to
container as a CONTAINER event. For other supplemental records, we name
things what they are: PATH, CWD, SOCKADDR, etc. So, CONTAINER_ID makes
sense. CONTAINER_OP sounds like its for operations on a container. Do
we have any operations on a container?

-Steve
Paul Moore Oct. 28, 2018, 7:53 a.m.
On Fri, Oct 26, 2018 at 4:13 AM Casey Schaufler <casey@schaufler-ca.com> wrote:
> On 10/25/2018 2:55 PM, Steve Grubb wrote:
> > ...
> > And historically speaking setting audit loginuid produces a LOGIN
> > event, so it only makes sense to consider binding container ID to
> > container as a CONTAINER event. For other supplemental records, we name
> > things what they are: PATH, CWD, SOCKADDR, etc. So, CONTAINER_ID makes
> > sense. CONTAINER_OP sounds like its for operations on a container. Do
> > we have any operations on a container?
>
> The answer has to be "no", because containers are, by emphatic assertion,
> not kernel constructs. Any CONTAINER_OP event has to come from user space.
> I think.

It is very important that we do not confuse operations on the audit
container id with operations on the containers themselves.  Of course
at a higher level, e.g. audit log analysis, we want to equate the two,
and if the container runtime which manages the audit container id is
sane that should be a reasonable assumption, but in this particular
patchset AUDIT_CONTAINER_OP is referring to operations involving just
the audit container id.

If there is a need for additional container operation auditing (note
well that I did not say audit container id here) then those audit
records can, and should, be generated by the container runtime itself,
similar to what we do with libvirt for virtualization.