audit: implement container identifier

Add audit container identifier support to the action of signalling the audit daemon.

Since this would need to add an element to the audit_sig_info struct, a new record type AUDIT_SIGNAL_INFO2 was created with a new audit_sig_info2 struct. Corresponding support is required in the userspace code to reflect the new record request and reply type. An older userspace won't break since it won't know to request this record type.

Signed-off-by Richard Guy Briggs <[email protected]>

---
 include/linux/audit.h       |  7 +++++++
 include/uapi/linux/audit.h  |  1 +
 kernel/audit.c              | 27 +++++++++++++++++++++++++++
 kernel/audit.h              |  1 +
 kernel/auditsc.c            |  1 +
 security/selinux/nlmsgtab.c |  1 +
 6 files changed, 38 insertions(+)

include/linux/audit.h +7 -0

index 43438192ca2a..c2dec9157463 100644
@@ -37,6 +37,13 @@ struct audit_sig_info {
        char            ctx[0];
 };
 
+struct audit_sig_info2 {
+       uid_t           uid;
+       pid_t           pid;
+       u64             cid;
+       char            ctx[0];
+};
+
 struct audit_buffer;
 struct audit_context;
 struct inode;

include/uapi/linux/audit.h +1 -0

index 55fde9970762..10cc67926cf1 100644
@@ -72,6 +72,7 @@
 #define AUDIT_SET_FEATURE      1018    /* Turn an audit feature on or off */
 #define AUDIT_GET_FEATURE      1019    /* Get which features are enabled */
 #define AUDIT_CONTAINER_OP     1020    /* Define the container id and info */
+#define AUDIT_SIGNAL_INFO2     1021    /* Get info auditd signal sender */
 
 #define AUDIT_FIRST_USER_MSG   1100    /* Userspace messages mostly uninteresting to kernel */
 #define AUDIT_USER_AVC         1107    /* We filter this differently */

kernel/audit.c +27 -0

index 3e0af53f3c4d..87e1d367f98c 100644
@@ -138,6 +138,7 @@ struct audit_net {
 kuid_t         audit_sig_uid = INVALID_UID;
 pid_t          audit_sig_pid = -1;
 u32            audit_sig_sid = 0;
+u64            audit_sig_cid = AUDIT_CID_UNSET;
 
 /* Records can be lost in several ways:
    0) [suppressed in audit_alloc]

@@ -1097,6 +1098,7 @@ static int audit_netlink_ok(struct sk_buff *skb, u16 msg_type)
        case AUDIT_ADD_RULE:
        case AUDIT_DEL_RULE:
        case AUDIT_SIGNAL_INFO:
+       case AUDIT_SIGNAL_INFO2:
        case AUDIT_TTY_GET:
        case AUDIT_TTY_SET:
        case AUDIT_TRIM:

@@ -1260,6 +1262,7 @@ static int audit_receive_msg(struct sk_buff *skb, struct nlmsghdr *nlh)
        struct audit_buffer     *ab;
        u16                     msg_type = nlh->nlmsg_type;
        struct audit_sig_info   *sig_data;
+       struct audit_sig_info2  *sig_data2;
        char                    *ctx = NULL;
        u32                     len;
 

@@ -1519,6 +1522,30 @@ static int audit_receive_msg(struct sk_buff *skb, struct nlmsghdr *nlh)
                                 sig_data, sizeof(*sig_data) + len);
                kfree(sig_data);
                break;
+       case AUDIT_SIGNAL_INFO2:
+               len = 0;
+               if (audit_sig_sid) {
+                       err = security_secid_to_secctx(audit_sig_sid, &ctx, &len);
+                       if (err)
+                               return err;
+               }
+               sig_data2 = kmalloc(sizeof(*sig_data2) + len, GFP_KERNEL);
+               if (!sig_data2) {
+                       if (audit_sig_sid)
+                               security_release_secctx(ctx, len);
+                       return -ENOMEM;
+               }
+               sig_data2->uid = from_kuid(&init_user_ns, audit_sig_uid);
+               sig_data2->pid = audit_sig_pid;
+               if (audit_sig_sid) {
+                       memcpy(sig_data2->ctx, ctx, len);
+                       security_release_secctx(ctx, len);
+               }
+               sig_data2->cid = audit_sig_cid;
+               audit_send_reply(skb, seq, AUDIT_SIGNAL_INFO2, 0, 0,
+                                sig_data2, sizeof(*sig_data2) + len);
+               kfree(sig_data2);
+               break;
        case AUDIT_TTY_GET: {
                struct audit_tty_status s;
                unsigned int t;

kernel/audit.h +1 -0

index e2912924af0d..c5ac6436317e 100644
@@ -345,6 +345,7 @@ extern void audit_filter_inodes(struct task_struct *tsk,
 extern pid_t audit_sig_pid;
 extern kuid_t audit_sig_uid;
 extern u32 audit_sig_sid;
+extern u64 audit_sig_cid;
 
 extern int audit_filter(int msgtype, unsigned int listtype);
 

kernel/auditsc.c +1 -0

index eea445b7a181..0a29a00feaf1 100644
@@ -2405,6 +2405,7 @@ int audit_signal_info(int sig, struct task_struct *t)
                else
                        audit_sig_uid = uid;
                security_task_getsecid(current, &audit_sig_sid);
+               audit_sig_cid = audit_get_contid(current);
        }
 
        if (!audit_signals || audit_dummy_context())

security/selinux/nlmsgtab.c +1 -1

index 9cec81209617..682fe7397762 100644
@@ -132,6 +132,7 @@ struct nlmsg_perm {
        { AUDIT_DEL_RULE,       NETLINK_AUDIT_SOCKET__NLMSG_WRITE    },
        { AUDIT_USER,           NETLINK_AUDIT_SOCKET__NLMSG_RELAY    },
        { AUDIT_SIGNAL_INFO,    NETLINK_AUDIT_SOCKET__NLMSG_READ     },
+       { AUDIT_SIGNAL_INFO2,   NETLINK_AUDIT_SOCKET__NLMSG_READ     },
        { AUDIT_TRIM,           NETLINK_AUDIT_SOCKET__NLMSG_WRITE    },
        { AUDIT_MAKE_EQUIV,     NETLINK_AUDIT_SOCKET__NLMSG_WRITE    },
        { AUDIT_TTY_GET,        NETLINK_AUDIT_SOCKET__NLMSG_READ     },
--

1.8.3.1

Quoted 11 lines · Richard Guy Briggs

Add audit container identifier support to the action of signalling the audit daemon.

Since this would need to add an element to the audit_sig_info struct, a new record type AUDIT_SIGNAL_INFO2 was created with a new audit_sig_info2 struct. Corresponding support is required in the userspace code to reflect the new record request and reply type. An older userspace won't break since it won't know to request this record type.

Signed-off-by Richard Guy Briggs <[email protected]>

This looks good to me.

Reviewed-by Ondrej Mosnacek <[email protected]>

Although I'm wondering if we shouldn't try to future-proof the AUDIT_SIGNAL_INFO2 format somehow, so that we don't need to add another AUDIT_SIGNAL_INFO3 when the need arises to add yet-another identifier to it... The simplest solution I can come up with is to add a "version" field at the beginning (set to 2 initially), then v<N>_len at the beginning of data for version <N>. But maybe this is too complicated for too little gain...

Quoted 11 lines · Richard Guy Briggs

Add audit container identifier support to the action of signalling the audit daemon.

Since this would need to add an element to the audit_sig_info struct, a new record type AUDIT_SIGNAL_INFO2 was created with a new audit_sig_info2 struct. Corresponding support is required in the userspace code to reflect the new record request and reply type. An older userspace won't break since it won't know to request this record type.

Signed-off-by Richard Guy Briggs <[email protected]>

This looks good to me.

Reviewed-by Ondrej Mosnacek <[email protected]>

Although I'm wondering if we shouldn't try to future-proof the AUDIT_SIGNAL_INFO2 format somehow, so that we don't need to add another AUDIT_SIGNAL_INFO3 when the need arises to add yet-another identifier to it... The simplest solution I can come up with is to add a "version" field at the beginning (set to 2 initially), then v<N>_len at the beginning of data for version <N>. But maybe this is too complicated for too little gain...

Quoted 135 lines

---
 include/linux/audit.h       |  7 +++++++
 include/uapi/linux/audit.h  |  1 +
 kernel/audit.c              | 27 +++++++++++++++++++++++++++
 kernel/audit.h              |  1 +
 kernel/auditsc.c            |  1 +
 security/selinux/nlmsgtab.c |  1 +
 6 files changed, 38 insertions(+)

include/linux/audit.h +7 -0

index 43438192ca2a..c2dec9157463 100644
@@ -37,6 +37,13 @@ struct audit_sig_info {
        char            ctx[0];
 };
 
+struct audit_sig_info2 {
+       uid_t           uid;
+       pid_t           pid;
+       u64             cid;
+       char            ctx[0];
+};
+
 struct audit_buffer;
 struct audit_context;
 struct inode;

include/uapi/linux/audit.h +1 -0

index 55fde9970762..10cc67926cf1 100644
@@ -72,6 +72,7 @@
 #define AUDIT_SET_FEATURE      1018    /* Turn an audit feature on or off */
 #define AUDIT_GET_FEATURE      1019    /* Get which features are enabled */
 #define AUDIT_CONTAINER_OP     1020    /* Define the container id and info */
+#define AUDIT_SIGNAL_INFO2     1021    /* Get info auditd signal sender */
 
 #define AUDIT_FIRST_USER_MSG   1100    /* Userspace messages mostly uninteresting to kernel */
 #define AUDIT_USER_AVC         1107    /* We filter this differently */

kernel/audit.c +27 -0

index 3e0af53f3c4d..87e1d367f98c 100644
@@ -138,6 +138,7 @@ struct audit_net {
 kuid_t         audit_sig_uid = INVALID_UID;
 pid_t          audit_sig_pid = -1;
 u32            audit_sig_sid = 0;
+u64            audit_sig_cid = AUDIT_CID_UNSET;
 
 /* Records can be lost in several ways:
    0) [suppressed in audit_alloc]

@@ -1097,6 +1098,7 @@ static int audit_netlink_ok(struct sk_buff *skb, u16 msg_type)
        case AUDIT_ADD_RULE:
        case AUDIT_DEL_RULE:
        case AUDIT_SIGNAL_INFO:
+       case AUDIT_SIGNAL_INFO2:
        case AUDIT_TTY_GET:
        case AUDIT_TTY_SET:
        case AUDIT_TRIM:

@@ -1260,6 +1262,7 @@ static int audit_receive_msg(struct sk_buff *skb, struct nlmsghdr *nlh)
        struct audit_buffer     *ab;
        u16                     msg_type = nlh->nlmsg_type;
        struct audit_sig_info   *sig_data;
+       struct audit_sig_info2  *sig_data2;
        char                    *ctx = NULL;
        u32                     len;
 

@@ -1519,6 +1522,30 @@ static int audit_receive_msg(struct sk_buff *skb, struct nlmsghdr *nlh)
                                 sig_data, sizeof(*sig_data) + len);
                kfree(sig_data);
                break;
+       case AUDIT_SIGNAL_INFO2:
+               len = 0;
+               if (audit_sig_sid) {
+                       err = security_secid_to_secctx(audit_sig_sid, &ctx, &len);
+                       if (err)
+                               return err;
+               }
+               sig_data2 = kmalloc(sizeof(*sig_data2) + len, GFP_KERNEL);
+               if (!sig_data2) {
+                       if (audit_sig_sid)
+                               security_release_secctx(ctx, len);
+                       return -ENOMEM;
+               }
+               sig_data2->uid = from_kuid(&init_user_ns, audit_sig_uid);
+               sig_data2->pid = audit_sig_pid;
+               if (audit_sig_sid) {
+                       memcpy(sig_data2->ctx, ctx, len);
+                       security_release_secctx(ctx, len);
+               }
+               sig_data2->cid = audit_sig_cid;
+               audit_send_reply(skb, seq, AUDIT_SIGNAL_INFO2, 0, 0,
+                                sig_data2, sizeof(*sig_data2) + len);
+               kfree(sig_data2);
+               break;
        case AUDIT_TTY_GET: {
                struct audit_tty_status s;
                unsigned int t;

kernel/audit.h +1 -0

index e2912924af0d..c5ac6436317e 100644
@@ -345,6 +345,7 @@ extern void audit_filter_inodes(struct task_struct *tsk,
 extern pid_t audit_sig_pid;
 extern kuid_t audit_sig_uid;
 extern u32 audit_sig_sid;
+extern u64 audit_sig_cid;
 
 extern int audit_filter(int msgtype, unsigned int listtype);
 

kernel/auditsc.c +1 -0

index eea445b7a181..0a29a00feaf1 100644
@@ -2405,6 +2405,7 @@ int audit_signal_info(int sig, struct task_struct *t)
                else
                        audit_sig_uid = uid;
                security_task_getsecid(current, &audit_sig_sid);
+               audit_sig_cid = audit_get_contid(current);
        }
 
        if (!audit_signals || audit_dummy_context())

security/selinux/nlmsgtab.c +1 -1

index 9cec81209617..682fe7397762 100644
@@ -132,6 +132,7 @@ struct nlmsg_perm {
        { AUDIT_DEL_RULE,       NETLINK_AUDIT_SOCKET__NLMSG_WRITE    },
        { AUDIT_USER,           NETLINK_AUDIT_SOCKET__NLMSG_RELAY    },
        { AUDIT_SIGNAL_INFO,    NETLINK_AUDIT_SOCKET__NLMSG_READ     },
+       { AUDIT_SIGNAL_INFO2,   NETLINK_AUDIT_SOCKET__NLMSG_READ     },
        { AUDIT_TRIM,           NETLINK_AUDIT_SOCKET__NLMSG_WRITE    },
        { AUDIT_MAKE_EQUIV,     NETLINK_AUDIT_SOCKET__NLMSG_WRITE    },
        { AUDIT_TTY_GET,        NETLINK_AUDIT_SOCKET__NLMSG_READ     },
--

Quoted 2 lines

1.8.3.1

--
Ondrej Mosnacek <omosnace at redhat dot com>
Software Engineer, Security Technologies
Red Hat, Inc.

Quoted 18 lines · Ondrej Mosnacek

Quoted 11 lines · Richard Guy Briggs

Add audit container identifier support to the action of signalling the audit daemon.

Since this would need to add an element to the audit_sig_info struct, a new record type AUDIT_SIGNAL_INFO2 was created with a new audit_sig_info2 struct. Corresponding support is required in the userspace code to reflect the new record request and reply type. An older userspace won't break since it won't know to request this record type.

Signed-off-by Richard Guy Briggs <[email protected]>

This looks good to me.

Reviewed-by Ondrej Mosnacek <[email protected]>

Quoted 7 lines

Although I'm wondering if we shouldn't try to future-proof the AUDIT_SIGNAL_INFO2 format somehow, so that we don't need to add another AUDIT_SIGNAL_INFO3 when the need arises to add yet-another identifier to it... The simplest solution I can come up with is to add a "version" field at the beginning (set to 2 initially), then v<N>_len at the beginning of data for version <N>. But maybe this is too complicated for too little gain...

FWIW, I believe the long term solution to this is the fabled netlink attribute approach that we haven't talked about in some time, but I keep dreaming about (it has been mostly on the back burner becasue 1) time and 2) didn't want to impact the audit container ID work). While I'm not opposed to trying to make things like this a bit more robust by adding version fields and similar things, there are still so many (so very many) problems with the audit kernel/userspace interface that still need to be addressed.

Signature · 2 lines

paul moore
www.paul-moore.com

Quoted 17 lines · Ondrej Mosnacek

Quoted 11 lines · Richard Guy Briggs

Add audit container identifier support to the action of signalling the audit daemon.

Since this would need to add an element to the audit_sig_info struct, a new record type AUDIT_SIGNAL_INFO2 was created with a new audit_sig_info2 struct. Corresponding support is required in the userspace code to reflect the new record request and reply type. An older userspace won't break since it won't know to request this record type.

Signed-off-by Richard Guy Briggs <[email protected]>

This looks good to me.

Reviewed-by Ondrej Mosnacek <[email protected]>

Quoted 7 lines

Although I'm wondering if we shouldn't try to future-proof the AUDIT_SIGNAL_INFO2 format somehow, so that we don't need to add another AUDIT_SIGNAL_INFO3 when the need arises to add yet-another identifier to it... The simplest solution I can come up with is to add a "version" field at the beginning (set to 2 initially), then v<N>_len at the beginning of data for version <N>. But maybe this is too complicated for too little gain...

FWIW, I believe the long term solution to this is the fabled netlink attribute approach that we haven't talked about in some time, but I keep dreaming about (it has been mostly on the back burner becasue 1) time and 2) didn't want to impact the audit container ID work). While I'm not opposed to trying to make things like this a bit more robust by adding version fields and similar things, there are still so many (so very many) problems with the audit kernel/userspace interface that still need to be addressed.

paul moore

Quoted 27 lines · Paul Moore

Quoted 18 lines · Ondrej Mosnacek

Quoted 11 lines · Richard Guy Briggs

Add audit container identifier support to the action of signalling the audit daemon.

Since this would need to add an element to the audit_sig_info struct, a new record type AUDIT_SIGNAL_INFO2 was created with a new audit_sig_info2 struct. Corresponding support is required in the userspace code to reflect the new record request and reply type. An older userspace won't break since it won't know to request this record type.

Signed-off-by Richard Guy Briggs <[email protected]>

This looks good to me.

Reviewed-by Ondrej Mosnacek <[email protected]>

Quoted 7 lines

Although I'm wondering if we shouldn't try to future-proof the AUDIT_SIGNAL_INFO2 format somehow, so that we don't need to add another AUDIT_SIGNAL_INFO3 when the need arises to add yet-another identifier to it... The simplest solution I can come up with is to add a "version" field at the beginning (set to 2 initially), then v<N>_len at the beginning of data for version <N>. But maybe this is too complicated for too little gain...

Quoted 9 lines

FWIW, I believe the long term solution to this is the fabled netlink attribute approach that we haven't talked about in some time, but I keep dreaming about (it has been mostly on the back burner becasue 1) time and 2) didn't want to impact the audit container ID work). While I'm not opposed to trying to make things like this a bit more robust by adding version fields and similar things, there are still so many (so very many) problems with the audit kernel/userspace interface that still need to be addressed.

Agreed, this change as-is is in keeping with the message structure that audit has today, and so is ok with me, but the long term goal should be a conversion to netlink attributes for all audit messages. Thats a big undertaking and should be addressed separately though.

Quoted 27 lines · Paul Moore

Quoted 18 lines · Ondrej Mosnacek

Quoted 11 lines · Richard Guy Briggs

Add audit container identifier support to the action of signalling the audit daemon.

Since this would need to add an element to the audit_sig_info struct, a new record type AUDIT_SIGNAL_INFO2 was created with a new audit_sig_info2 struct. Corresponding support is required in the userspace code to reflect the new record request and reply type. An older userspace won't break since it won't know to request this record type.

Signed-off-by Richard Guy Briggs <[email protected]>

This looks good to me.

Reviewed-by Ondrej Mosnacek <[email protected]>

Quoted 7 lines

Although I'm wondering if we shouldn't try to future-proof the AUDIT_SIGNAL_INFO2 format somehow, so that we don't need to add another AUDIT_SIGNAL_INFO3 when the need arises to add yet-another identifier to it... The simplest solution I can come up with is to add a "version" field at the beginning (set to 2 initially), then v<N>_len at the beginning of data for version <N>. But maybe this is too complicated for too little gain...

Quoted 8 lines

FWIW, I believe the long term solution to this is the fabled netlink attribute approach that we haven't talked about in some time, but I keep dreaming about (it has been mostly on the back burner becasue 1) time and 2) didn't want to impact the audit container ID work). While I'm not opposed to trying to make things like this a bit more robust by adding version fields and similar things, there are still so many (so very many) problems with the audit kernel/userspace interface that still need to be addressed.

Agreed, this change as-is is in keeping with the message structure that audit has today, and so is ok with me, but the long term goal should be a conversion to netlink attributes for all audit messages. Thats a big undertaking and should be addressed separately though.

Quoted 26 lines · Paul Moore

Quoted 17 lines · Ondrej Mosnacek

Quoted 11 lines · Richard Guy Briggs

Add audit container identifier support to the action of signalling the audit daemon.

Since this would need to add an element to the audit_sig_info struct, a new record type AUDIT_SIGNAL_INFO2 was created with a new audit_sig_info2 struct. Corresponding support is required in the userspace code to reflect the new record request and reply type. An older userspace won't break since it won't know to request this record type.

Signed-off-by Richard Guy Briggs <[email protected]>

This looks good to me.

Reviewed-by Ondrej Mosnacek <[email protected]>

Quoted 7 lines

Although I'm wondering if we shouldn't try to future-proof the AUDIT_SIGNAL_INFO2 format somehow, so that we don't need to add another AUDIT_SIGNAL_INFO3 when the need arises to add yet-another identifier to it... The simplest solution I can come up with is to add a "version" field at the beginning (set to 2 initially), then v<N>_len at the beginning of data for version <N>. But maybe this is too complicated for too little gain...

Quoted 8 lines

FWIW, I believe the long term solution to this is the fabled netlink attribute approach that we haven't talked about in some time, but I keep dreaming about (it has been mostly on the back burner becasue 1) time and 2) didn't want to impact the audit container ID work). While I'm not opposed to trying to make things like this a bit more robust by adding version fields and similar things, there are still so many (so very many) problems with the audit kernel/userspace interface that still need to be addressed.

While this particular message type is used very infrequently, adding a version field to every message type strikes me as a huge overhead for the small likelihood of the format needing to change.

I'd prefer to just key it off the AUDIT_FEATURE_BITMAP or some other easily detectable change in this distinguishing feature, such as the presence of /proc/self/audit_containerid, which is what I've done in the accompanying userspace patchset that I'm preparing to post that works with this change.

Implement kernel audit container identifier.

This patchset is a fifth based on the proposal document (V3) posted:

	https://www.redhat.com/archives/linux-audit/2018-January/msg00014.html

The first patch was the last patch from ghak81 that was absorbed into this patchset since its primary justification is the rest of this patchset.

The second patch implements the proc fs write to set the audit container identifier of a process, emitting an AUDIT_CONTAINER_OP record to announce the registration of that audit container identifier on that process. This patch requires userspace support for record acceptance and proper type display.

The third implements reading the audit container identifier from the proc filesystem for debugging. This patch wasn't planned for upstream inclusion but is starting to become more likely.

The fourth implements the auxiliary record AUDIT_CONTAINER_ID if an audit container identifier is associated with an event. This patch requires userspace support for proper type display.

The 5th adds audit daemon signalling provenance through audit_sig_info2.

The 6th creates a local audit context to be able to bind a standalone record with a locally created auxiliary record.

The 7th patch adds audit container identifier records to the user standalone records.

The 8th adds audit container identifier filtering to the exit, exclude and user lists. This patch adds the AUDIT_CONTID field and requires auditctl userspace support for the --contid option.

The 9th adds network namespace audit container identifier labelling based on member tasks' audit container identifier labels.

The 10th adds audit container identifier support to standalone netfilter records that don't have a task context and lists each container to which that net namespace belongs.

Example: Set an audit container identifier of 123456 to the "sleep" task:

sleep 2&
child=$!
echo 123456 > /proc/$child/audit_containerid; echo $? ausearch -ts recent -m container_op
echo child:$child contid:$( cat /proc/$child/audit_containerid)

This should produce a record such as:

type=CONTAINER_OP msg=audit(2018-06-06 12:39:29.636:26949) : op=set opid=2209 contid=123456 old-contid=18446744073709551615 pid=628 auid=root uid=root tty=ttyS0 ses=1 subj=unconfined_u:unconfined_r:unconfined_t:s0-s0:c0.c1023 comm=bash exe=/usr/bin/bash res=yes

Example: Set a filter on an audit container identifier 123459 on /tmp/tmpcontainerid:

contid=123459
key=tmpcontainerid
auditctl -a exit,always -F dir=/tmp -F perm=wa -F contid=$contid -F key=$key perl -e "sleep 1; open(my \$tmpfile, '>', \"/tmp/$key\"); close(\$tmpfile);" & child=$!
echo $contid > /proc/$child/audit_containerid
sleep 2
ausearch -i -ts recent -k $key
auditctl -d exit,always -F dir=/tmp -F perm=wa -F contid=$contid -F key=$key rm -f /tmp/$key

This should produce an event such as:

  type=CONTAINER_ID msg=audit(2018-06-06 12:46:31.707:26953) : contid=123459
  type=PROCTITLE msg=audit(2018-06-06 12:46:31.707:26953) : proctitle=perl -e sleep 1; open(my $tmpfile, '>', "/tmp/tmpcontainerid"); close($tmpfile);
  type=PATH msg=audit(2018-06-06 12:46:31.707:26953) : item=1 name=/tmp/tmpcontainerid inode=25656 dev=00:26 mode=file,644 ouid=root ogid=root rdev=00:00 obj=unconfined_u:object_r:user_tmp_t:s0 nametype=CREATE cap_fp=none cap_fi=none cap_fe=0 cap_fver=0
  type=PATH msg=audit(2018-06-06 12:46:31.707:26953) : item=0 name=/tmp/ inode=8985 dev=00:26 mode=dir,sticky,777 ouid=root ogid=root rdev=00:00 obj=system_u:object_r:tmp_t:s0 nametype=PARENT cap_fp=none cap_fi=none cap_fe=0 cap_fver=0
  type=CWD msg=audit(2018-06-06 12:46:31.707:26953) : cwd=/root
  type=SYSCALL msg=audit(2018-06-06 12:46:31.707:26953) : arch=x86_64 syscall=openat success=yes exit=3 a0=0xffffffffffffff9c a1=0x5621f2b81900 a2=O_WRONLY|O_CREAT|O_TRUNC a3=0x1b6 items=2 ppid=628 pid=2232 auid=root uid=root gid=root euid=root suid=root fsuid=root egid=root sgid=root fsgid=root tty=ttyS0 ses=1 comm=perl exe=/usr/bin/perl subj=unconfined_u:unconfined_r:unconfined_t:s0-s0:c0.c1023 key=tmpcontainerid

Example: Test multiple containers on one netns:

  sleep 5 &
  child1=$!
  containerid1=123451
  echo $containerid1 > /proc/$child1/audit_containerid
  sleep 5 &
  child2=$!
  containerid2=123452
  echo $containerid2 > /proc/$child2/audit_containerid
  iptables -I INPUT -i lo -p icmp --icmp-type echo-request -j AUDIT --type accept
  iptables -I INPUT  -t mangle -i lo -p icmp --icmp-type echo-request -j MARK --set-mark 0x12345555
  sleep 1;
  bash -c "ping -q -c 1 127.0.0.1 >/dev/null 2>&1"
  sleep 1;
  ausearch -i -m NETFILTER_PKT -ts boot|grep mark=0x12345555
  ausearch -i -m NETFILTER_PKT -ts boot|grep contid=|grep $containerid1|grep $containerid2

This should produce an event such as:

type=NETFILTER_PKT msg=audit(03/15/2019 14:16:13.369:244) : mark=0x12345555 saddr=127.0.0.1 daddr=127.0.0.1 proto=icmp type=CONTAINER_ID msg=audit(03/15/2019 14:16:13.369:244) : contid=123452,123451

Includes the last patch of https://github.com/linux-audit/audit-kernel/issues/81
Please see the github audit kernel issue for the main feature:
https://github.com/linux-audit/audit-kernel/issues/90
and the kernel filter code:
https://github.com/linux-audit/audit-kernel/issues/91
and the network support:
https://github.com/linux-audit/audit-kernel/issues/92
Please see the github audit userspace issue for supporting record types:
https://github.com/linux-audit/audit-userspace/issues/51
and filter code:
https://github.com/linux-audit/audit-userspace/issues/40
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

Changelog:

v6
- change TMPBUFLEN from 11 to 21 to cover the decimal value of contid u64 (nhorman)
- fix bug overwriting ctx in struct audit_sig_info, move cid above ctx[0] (nhorman)
- fix bug skipping remaining fields and not advancing bufp when copying out contid in audit_krule_to_data (omosnacec)
- add acks, tidy commit descriptions, other formatting fixes (checkpatch wrong on audit_log_lost)
- cast ull for u64 prints
- target_cid tracking was moved from the ptrace/signal patch to container_op
- target ptrace and signal records were moved from the ptrace/signal patch to container_id
- auditd signaller tracking was moved to a new AUDIT_SIGNAL_INFO2 request and record
- ditch unnecessary list_empty() checks
- check for null net and aunet in audit_netns_contid_add()
- swap CONTAINER_OP contid/old-contid order to ease parsing

v5
- address loginuid and sessionid syscall scope in ghak104
- address audit_context in CONFIG_AUDIT vs CONFIG_AUDITSYSCALL in ghak105
- remove tty patch, addressed in ghak106
- rebase on audit/next v5.0-rc1 w/ghak59/ghak104/ghak103/ghak100/ghak107/ghak105/ghak106/ghak105sup
- update CONTAINER_ID to CONTAINER_OP in patch description
- move audit_context in audit_task_info to CONFIG_AUDITSYSCALL
- move audit_alloc() and audit_free() out of CONFIG_AUDITSYSCALL and into CONFIG_AUDIT and create audit_{alloc,free}_syscall
- use plain kmem_cache_alloc() rather than kmem_cache_zalloc() in audit_alloc()
- fix audit_get_contid() declaration type error
- move audit_set_contid() from auditsc.c to audit.c
- audit_log_contid() returns void
- audit_log_contid() handed contid rather than tsk
- switch from AUDIT_CONTAINER to AUDIT_CONTAINER_ID for aux record
- move audit_log_contid(tsk/contid) & audit_contid_set(tsk)/audit_contid_valid(contid)
- switch from tsk to current
- audit_alloc_local() calls audit_log_lost() on failure to allocate a context
- add AUDIT_USER* non-syscall contid record
- cosmetic cleanup double parens, goto out on err
- ditch audit_get_ns_contid_list_lock(), fix aunet lock race
- switch from all-cpu read spinlock to rcu, keep spinlock for write
- update audit_alloc_local() to use ktime_get_coarse_real_ts64()
- add nft_log support
- add call from do_exit() in audit_free() to remove contid from netns
- relegate AUDIT_CONTAINER ref= field (was op=) to debug patch

v4
- preface set with ghak81:"collect audit task parameters"
- add shallyn and sgrubb acks
- rename feature bitmap macro
- rename cid_valid() to audit_contid_valid()
- rename AUDIT_CONTAINER_ID to AUDIT_CONTAINER_OP
- delete audit_get_contid_list() from headers
- move work into inner if, delete "found"
- change netns contid list function names
- move exports for audit_log_contid audit_alloc_local audit_free_context to non-syscall patch
- list contids CSV
- pass in gfp flags to audit_alloc_local() (fix audit_alloc_context callers)
- use "local" in lieu of abusing in_syscall for auditsc_get_stamp()
- read_lock(&tasklist_lock) around children and thread check
- task_lock(tsk) should be taken before first check of tsk->audit
- add spin lock to contid list in aunet
- restrict /proc read to CAP_AUDIT_CONTROL
- remove set again prohibition and inherited flag
- delete contidion spelling fix from patchset, send to netdev/linux-wireless

v3
- switched from containerid in task_struct to audit_task_info (depends on ghak81)
- drop INVALID_CID in favour of only AUDIT_CID_UNSET
- check for !audit_task_info, throw -ENOPROTOOPT on set
- changed -EPERM to -EEXIST for parent check
- return AUDIT_CID_UNSET if !audit_enabled
- squash child/thread check patch into AUDIT_CONTAINER_ID patch
- changed -EPERM to -EBUSY for child check
- separate child and thread checks, use -EALREADY for latter
- move addition of op= from ptrace/signal patch to AUDIT_CONTAINER patch
- fix && to || bashism in ptrace/signal patch
- uninline and export function for audit_free_context()
- drop CONFIG_CHANGE, FEATURE_CHANGE, ANOM_ABEND, ANOM_SECCOMP patches
- move audit_enabled check (xt_AUDIT)
- switched from containerid list in struct net to net_generic's struct audit_net
- move containerid list iteration into audit (xt_AUDIT)
- create function to move namespace switch into audit
- switched /proc/PID/ entry from containerid to audit_containerid
- call kzalloc with GFP_ATOMIC on in_atomic() in audit_alloc_context()
- call kzalloc with GFP_ATOMIC on in_atomic() in audit_log_container_info()
- use xt_net(par) instead of sock_net(skb->sk) to get net
- switched record and field names: initial CONTAINER_ID, aux CONTAINER, field CONTID
- allow to set own contid
- open code audit_set_containerid
- add contid inherited flag
- ccontainerid and pcontainerid eliminated due to inherited flag
- change name of container list funcitons
- rename containerid to contid
- convert initial container record to syscall aux
- fix spelling mistake of contidion in net/rfkill/core.c to avoid contid name collision

v2
- add check for children and threads
- add network namespace container identifier list
- add NETFILTER_PKT audit container identifier logging
- patch description and documentation clean-up and example
- reap unused ppid

Richard Guy Briggs (10):
audit: collect audit task parameters
audit: add container id
audit: read container ID of a process
audit: log container info of syscalls
audit: add contid support for signalling the audit daemon
audit: add support for non-syscall auxiliary records
audit: add containerid support for user records
audit: add containerid filtering
audit: add support for containerid to network namespaces
audit: NETFILTER_PKT: record each container ID associated with a netNS

 fs/proc/base.c              |  57 +++++++-
 include/linux/audit.h       | 113 +++++++++++++--
 include/linux/sched.h       |   7 +-
 include/uapi/linux/audit.h  |   9 +-
 init/init_task.c            |   3 +-
 init/main.c                 |   2 +
 kernel/audit.c              | 325 ++++++++++++++++++++++++++++++++++++++++++--
 kernel/audit.h              |   9 ++
 kernel/auditfilter.c        |  47 +++++++
 kernel/auditsc.c            |  90 ++++++++----
 kernel/fork.c               |   1 -
 kernel/nsproxy.c            |   4 +
 net/netfilter/nft_log.c     |  11 +-
 net/netfilter/xt_AUDIT.c    |  11 +-
 security/selinux/nlmsgtab.c |   1 +
 15 files changed, 627 insertions(+), 63 deletions(-)

Signature · 1 line

1.8.3.1

Implement kernel audit container identifier.

This patchset is a fifth based on the proposal document (V3) posted:

	https://www.redhat.com/archives/linux-audit/2018-January/msg00014.html

The first patch was the last patch from ghak81 that was absorbed into this patchset since its primary justification is the rest of this patchset.

The second patch implements the proc fs write to set the audit container identifier of a process, emitting an AUDIT_CONTAINER_OP record to announce the registration of that audit container identifier on that process. This patch requires userspace support for record acceptance and proper type display.

The third implements reading the audit container identifier from the proc filesystem for debugging. This patch wasn't planned for upstream inclusion but is starting to become more likely.

The fourth implements the auxiliary record AUDIT_CONTAINER_ID if an audit container identifier is associated with an event. This patch requires userspace support for proper type display.

The 5th adds audit daemon signalling provenance through audit_sig_info2.

The 6th creates a local audit context to be able to bind a standalone record with a locally created auxiliary record.

The 7th patch adds audit container identifier records to the user standalone records.

The 8th adds audit container identifier filtering to the exit, exclude and user lists. This patch adds the AUDIT_CONTID field and requires auditctl userspace support for the --contid option.

The 9th adds network namespace audit container identifier labelling based on member tasks' audit container identifier labels.

The 10th adds audit container identifier support to standalone netfilter records that don't have a task context and lists each container to which that net namespace belongs.

Example: Set an audit container identifier of 123456 to the "sleep" task:

sleep 2&
child=$!
echo 123456 > /proc/$child/audit_containerid; echo $? ausearch -ts recent -m container_op
echo child:$child contid:$( cat /proc/$child/audit_containerid)

This should produce a record such as:

type=CONTAINER_OP msg=audit(2018-06-06 12:39:29.636:26949) : op=set opid=2209 contid=123456 old-contid=18446744073709551615 pid=628 auid=root uid=root tty=ttyS0 ses=1 subj=unconfined_u:unconfined_r:unconfined_t:s0-s0:c0.c1023 comm=bash exe=/usr/bin/bash res=yes

Example: Set a filter on an audit container identifier 123459 on /tmp/tmpcontainerid:

contid=123459
key=tmpcontainerid
auditctl -a exit,always -F dir=/tmp -F perm=wa -F contid=$contid -F key=$key perl -e "sleep 1; open(my \$tmpfile, '>', \"/tmp/$key\"); close(\$tmpfile);" & child=$!
echo $contid > /proc/$child/audit_containerid
sleep 2
ausearch -i -ts recent -k $key
auditctl -d exit,always -F dir=/tmp -F perm=wa -F contid=$contid -F key=$key rm -f /tmp/$key

This should produce an event such as:

  type=CONTAINER_ID msg=audit(2018-06-06 12:46:31.707:26953) : contid=123459
  type=PROCTITLE msg=audit(2018-06-06 12:46:31.707:26953) : proctitle=perl -e sleep 1; open(my $tmpfile, '>', "/tmp/tmpcontainerid"); close($tmpfile);
  type=PATH msg=audit(2018-06-06 12:46:31.707:26953) : item=1 name=/tmp/tmpcontainerid inode=25656 dev=00:26 mode=file,644 ouid=root ogid=root rdev=00:00 obj=unconfined_u:object_r:user_tmp_t:s0 nametype=CREATE cap_fp=none cap_fi=none cap_fe=0 cap_fver=0
  type=PATH msg=audit(2018-06-06 12:46:31.707:26953) : item=0 name=/tmp/ inode=8985 dev=00:26 mode=dir,sticky,777 ouid=root ogid=root rdev=00:00 obj=system_u:object_r:tmp_t:s0 nametype=PARENT cap_fp=none cap_fi=none cap_fe=0 cap_fver=0
  type=CWD msg=audit(2018-06-06 12:46:31.707:26953) : cwd=/root
  type=SYSCALL msg=audit(2018-06-06 12:46:31.707:26953) : arch=x86_64 syscall=openat success=yes exit=3 a0=0xffffffffffffff9c a1=0x5621f2b81900 a2=O_WRONLY|O_CREAT|O_TRUNC a3=0x1b6 items=2 ppid=628 pid=2232 auid=root uid=root gid=root euid=root suid=root fsuid=root egid=root sgid=root fsgid=root tty=ttyS0 ses=1 comm=perl exe=/usr/bin/perl subj=unconfined_u:unconfined_r:unconfined_t:s0-s0:c0.c1023 key=tmpcontainerid

Example: Test multiple containers on one netns:

  sleep 5 &
  child1=$!
  containerid1=123451
  echo $containerid1 > /proc/$child1/audit_containerid
  sleep 5 &
  child2=$!
  containerid2=123452
  echo $containerid2 > /proc/$child2/audit_containerid
  iptables -I INPUT -i lo -p icmp --icmp-type echo-request -j AUDIT --type accept
  iptables -I INPUT  -t mangle -i lo -p icmp --icmp-type echo-request -j MARK --set-mark 0x12345555
  sleep 1;
  bash -c "ping -q -c 1 127.0.0.1 >/dev/null 2>&1"
  sleep 1;
  ausearch -i -m NETFILTER_PKT -ts boot|grep mark=0x12345555
  ausearch -i -m NETFILTER_PKT -ts boot|grep contid=|grep $containerid1|grep $containerid2

This should produce an event such as:

type=NETFILTER_PKT msg=audit(03/15/2019 14:16:13.369:244) : mark=0x12345555 saddr=127.0.0.1 daddr=127.0.0.1 proto=icmp type=CONTAINER_ID msg=audit(03/15/2019 14:16:13.369:244) : contid=123452,123451

Includes the last patch of https://github.com/linux-audit/audit-kernel/issues/81
Please see the github audit kernel issue for the main feature:
https://github.com/linux-audit/audit-kernel/issues/90
and the kernel filter code:
https://github.com/linux-audit/audit-kernel/issues/91
and the network support:
https://github.com/linux-audit/audit-kernel/issues/92
Please see the github audit userspace issue for supporting record types:
https://github.com/linux-audit/audit-userspace/issues/51
and filter code:
https://github.com/linux-audit/audit-userspace/issues/40
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

Changelog:

v6
- change TMPBUFLEN from 11 to 21 to cover the decimal value of contid u64 (nhorman)
- fix bug overwriting ctx in struct audit_sig_info, move cid above ctx[0] (nhorman)
- fix bug skipping remaining fields and not advancing bufp when copying out contid in audit_krule_to_data (omosnacec)
- add acks, tidy commit descriptions, other formatting fixes (checkpatch wrong on audit_log_lost)
- cast ull for u64 prints
- target_cid tracking was moved from the ptrace/signal patch to container_op
- target ptrace and signal records were moved from the ptrace/signal patch to container_id
- auditd signaller tracking was moved to a new AUDIT_SIGNAL_INFO2 request and record
- ditch unnecessary list_empty() checks
- check for null net and aunet in audit_netns_contid_add()
- swap CONTAINER_OP contid/old-contid order to ease parsing

v5
- address loginuid and sessionid syscall scope in ghak104
- address audit_context in CONFIG_AUDIT vs CONFIG_AUDITSYSCALL in ghak105
- remove tty patch, addressed in ghak106
- rebase on audit/next v5.0-rc1 w/ghak59/ghak104/ghak103/ghak100/ghak107/ghak105/ghak106/ghak105sup
- update CONTAINER_ID to CONTAINER_OP in patch description
- move audit_context in audit_task_info to CONFIG_AUDITSYSCALL
- move audit_alloc() and audit_free() out of CONFIG_AUDITSYSCALL and into CONFIG_AUDIT and create audit_{alloc,free}_syscall
- use plain kmem_cache_alloc() rather than kmem_cache_zalloc() in audit_alloc()
- fix audit_get_contid() declaration type error
- move audit_set_contid() from auditsc.c to audit.c
- audit_log_contid() returns void
- audit_log_contid() handed contid rather than tsk
- switch from AUDIT_CONTAINER to AUDIT_CONTAINER_ID for aux record
- move audit_log_contid(tsk/contid) & audit_contid_set(tsk)/audit_contid_valid(contid)
- switch from tsk to current
- audit_alloc_local() calls audit_log_lost() on failure to allocate a context
- add AUDIT_USER* non-syscall contid record
- cosmetic cleanup double parens, goto out on err
- ditch audit_get_ns_contid_list_lock(), fix aunet lock race
- switch from all-cpu read spinlock to rcu, keep spinlock for write
- update audit_alloc_local() to use ktime_get_coarse_real_ts64()
- add nft_log support
- add call from do_exit() in audit_free() to remove contid from netns
- relegate AUDIT_CONTAINER ref= field (was op=) to debug patch

v4
- preface set with ghak81:"collect audit task parameters"
- add shallyn and sgrubb acks
- rename feature bitmap macro
- rename cid_valid() to audit_contid_valid()
- rename AUDIT_CONTAINER_ID to AUDIT_CONTAINER_OP
- delete audit_get_contid_list() from headers
- move work into inner if, delete "found"
- change netns contid list function names
- move exports for audit_log_contid audit_alloc_local audit_free_context to non-syscall patch
- list contids CSV
- pass in gfp flags to audit_alloc_local() (fix audit_alloc_context callers)
- use "local" in lieu of abusing in_syscall for auditsc_get_stamp()
- read_lock(&tasklist_lock) around children and thread check
- task_lock(tsk) should be taken before first check of tsk->audit
- add spin lock to contid list in aunet
- restrict /proc read to CAP_AUDIT_CONTROL
- remove set again prohibition and inherited flag
- delete contidion spelling fix from patchset, send to netdev/linux-wireless

v3
- switched from containerid in task_struct to audit_task_info (depends on ghak81)
- drop INVALID_CID in favour of only AUDIT_CID_UNSET
- check for !audit_task_info, throw -ENOPROTOOPT on set
- changed -EPERM to -EEXIST for parent check
- return AUDIT_CID_UNSET if !audit_enabled
- squash child/thread check patch into AUDIT_CONTAINER_ID patch
- changed -EPERM to -EBUSY for child check
- separate child and thread checks, use -EALREADY for latter
- move addition of op= from ptrace/signal patch to AUDIT_CONTAINER patch
- fix && to || bashism in ptrace/signal patch
- uninline and export function for audit_free_context()
- drop CONFIG_CHANGE, FEATURE_CHANGE, ANOM_ABEND, ANOM_SECCOMP patches
- move audit_enabled check (xt_AUDIT)
- switched from containerid list in struct net to net_generic's struct audit_net
- move containerid list iteration into audit (xt_AUDIT)
- create function to move namespace switch into audit
- switched /proc/PID/ entry from containerid to audit_containerid
- call kzalloc with GFP_ATOMIC on in_atomic() in audit_alloc_context()
- call kzalloc with GFP_ATOMIC on in_atomic() in audit_log_container_info()
- use xt_net(par) instead of sock_net(skb->sk) to get net
- switched record and field names: initial CONTAINER_ID, aux CONTAINER, field CONTID
- allow to set own contid
- open code audit_set_containerid
- add contid inherited flag
- ccontainerid and pcontainerid eliminated due to inherited flag
- change name of container list funcitons
- rename containerid to contid
- convert initial container record to syscall aux
- fix spelling mistake of contidion in net/rfkill/core.c to avoid contid name collision

v2
- add check for children and threads
- add network namespace container identifier list
- add NETFILTER_PKT audit container identifier logging
- patch description and documentation clean-up and example
- reap unused ppid

Richard Guy Briggs (10):
audit: collect audit task parameters
audit: add container id
audit: read container ID of a process
audit: log container info of syscalls
audit: add contid support for signalling the audit daemon
audit: add support for non-syscall auxiliary records
audit: add containerid support for user records
audit: add containerid filtering
audit: add support for containerid to network namespaces
audit: NETFILTER_PKT: record each container ID associated with a netNS

 fs/proc/base.c              |  57 +++++++-
 include/linux/audit.h       | 113 +++++++++++++--
 include/linux/sched.h       |   7 +-
 include/uapi/linux/audit.h  |   9 +-
 init/init_task.c            |   3 +-
 init/main.c                 |   2 +
 kernel/audit.c              | 325 ++++++++++++++++++++++++++++++++++++++++++--
 kernel/audit.h              |   9 ++
 kernel/auditfilter.c        |  47 +++++++
 kernel/auditsc.c            |  90 ++++++++----
 kernel/fork.c               |   1 -
 kernel/nsproxy.c            |   4 +
 net/netfilter/nft_log.c     |  11 +-
 net/netfilter/xt_AUDIT.c    |  11 +-
 security/selinux/nlmsgtab.c |   1 +
 15 files changed, 627 insertions(+), 63 deletions(-)

Signature · 1 line

1.8.3.1

Quoted 1 line · Richard Guy Briggs

Implement kernel audit container identifier.

I'm sorry, I've lost track of this, where have we landed on it? Are we good for inclusion?

Quoted 5 lines · Neil Horman

Quoted 1 line · Richard Guy Briggs

Implement kernel audit container identifier.

I'm sorry, I've lost track of this, where have we landed on it? Are we good for inclusion?

I haven't finished going through this latest revision, but unless Richard made any significant changes outside of the feedback from the v5 patchset I'm guessing we are "close".

Based on discussions Richard and I had some time ago, I have always envisioned the plan as being get the kernel patchset, tests, docs ready (which Richard has been doing) and then run the actual implemented API by the userland container folks, e.g. cri-o/lxc/etc., to make sure the actual implementation is sane from their perspective. They've already seen the design, so I'm not expecting any real surprises here, but sometimes opinions change when they have actual code in front of them to play with and review.

Beyond that, while the cri-o/lxc/etc. folks are looking it over, whatever additional testing we can do would be a big win. I'm thinking I'll pull it into a separate branch in the audit tree (audit/working-container ?) and include that in my secnext kernels that I build/test on a regular basis; this is also a handy way to keep it based against the current audit/next branch. If any changes are needed Richard can either chose to base those changes on audit/next or the separate audit container ID branch; that's up to him. I've done this with other big changes in other trees, e.g. SELinux, and it has worked well to get some extra testing in and keep the patchset "merge ready" while others outside the subsystem look things over.

Signature · 2 lines

paul moore
www.paul-moore.com

Quoted 4 lines · Neil Horman

Quoted 1 line · Richard Guy Briggs

Implement kernel audit container identifier.

I'm sorry, I've lost track of this, where have we landed on it? Are we good for inclusion?

I haven't finished going through this latest revision, but unless Richard made any significant changes outside of the feedback from the v5 patchset I'm guessing we are "close".

Based on discussions Richard and I had some time ago, I have always envisioned the plan as being get the kernel patchset, tests, docs ready (which Richard has been doing) and then run the actual implemented API by the userland container folks, e.g. cri-o/lxc/etc., to make sure the actual implementation is sane from their perspective. They've already seen the design, so I'm not expecting any real surprises here, but sometimes opinions change when they have actual code in front of them to play with and review.

Beyond that, while the cri-o/lxc/etc. folks are looking it over, whatever additional testing we can do would be a big win. I'm thinking I'll pull it into a separate branch in the audit tree (audit/working-container ?) and include that in my secnext kernels that I build/test on a regular basis; this is also a handy way to keep it based against the current audit/next branch. If any changes are needed Richard can either chose to base those changes on audit/next or the separate audit container ID branch; that's up to him. I've done this with other big changes in other trees, e.g. SELinux, and it has worked well to get some extra testing in and keep the patchset "merge ready" while others outside the subsystem look things over.

Quoted 18 lines · Paul Moore

Quoted 4 lines · Neil Horman

Quoted 1 line · Richard Guy Briggs

Implement kernel audit container identifier.

I'm sorry, I've lost track of this, where have we landed on it? Are we good for inclusion?

I haven't finished going through this latest revision, but unless Richard made any significant changes outside of the feedback from the v5 patchset I'm guessing we are "close".

Based on discussions Richard and I had some time ago, I have always envisioned the plan as being get the kernel patchset, tests, docs ready (which Richard has been doing) and then run the actual implemented API by the userland container folks, e.g. cri-o/lxc/etc., to make sure the actual implementation is sane from their perspective. They've already seen the design, so I'm not expecting any real surprises here, but sometimes opinions change when they have actual code in front of them to play with and review.

Quoted 12 lines

Beyond that, while the cri-o/lxc/etc. folks are looking it over, whatever additional testing we can do would be a big win. I'm thinking I'll pull it into a separate branch in the audit tree (audit/working-container ?) and include that in my secnext kernels that I build/test on a regular basis; this is also a handy way to keep it based against the current audit/next branch. If any changes are needed Richard can either chose to base those changes on audit/next or the separate audit container ID branch; that's up to him. I've done this with other big changes in other trees, e.g. SELinux, and it has worked well to get some extra testing in and keep the patchset "merge ready" while others outside the subsystem look things over.

Mrunal Patel (maintainer of CRI-O) and I have reviewed the API, and believe this is something we can work on in the container runtimes team to implement the container auditing code in CRI-O and Podman.

Quoted 18 lines · Paul Moore

Quoted 4 lines · Neil Horman

Quoted 1 line · Richard Guy Briggs

Implement kernel audit container identifier.

I'm sorry, I've lost track of this, where have we landed on it? Are we good for inclusion?

I haven't finished going through this latest revision, but unless Richard made any significant changes outside of the feedback from the v5 patchset I'm guessing we are "close".

Based on discussions Richard and I had some time ago, I have always envisioned the plan as being get the kernel patchset, tests, docs ready (which Richard has been doing) and then run the actual implemented API by the userland container folks, e.g. cri-o/lxc/etc., to make sure the actual implementation is sane from their perspective. They've already seen the design, so I'm not expecting any real surprises here, but sometimes opinions change when they have actual code in front of them to play with and review.

Quoted 12 lines

Beyond that, while the cri-o/lxc/etc. folks are looking it over, whatever additional testing we can do would be a big win. I'm thinking I'll pull it into a separate branch in the audit tree (audit/working-container ?) and include that in my secnext kernels that I build/test on a regular basis; this is also a handy way to keep it based against the current audit/next branch. If any changes are needed Richard can either chose to base those changes on audit/next or the separate audit container ID branch; that's up to him. I've done this with other big changes in other trees, e.g. SELinux, and it has worked well to get some extra testing in and keep the patchset "merge ready" while others outside the subsystem look things over.

Mrunal Patel (maintainer of CRI-O) and I have reviewed the API, and believe this is something we can work on in the container runtimes team to implement the container auditing code in CRI-O and Podman.

Quoted 2 lines · Daniel Walsh

Quoted 1 line · Paul Moore

On Mon, Apr 22, 2019 at 7:38 AM Neil Horman <[email protected]>

Quoted 31 lines

Quoted 30 lines

Quoted 5 lines · Richard Guy Briggs

Quoted 1 line

Implement kernel audit container identifier.

I'm sorry, I've lost track of this, where have we landed on it? Are we good for inclusion?

I haven't finished going through this latest revision, but unless Richard made any significant changes outside of the feedback from the v5 patchset I'm guessing we are "close".

Based on discussions Richard and I had some time ago, I have always envisioned the plan as being get the kernel patchset, tests, docs ready (which Richard has been doing) and then run the actual implemented API by the userland container folks, e.g. cri-o/lxc/etc., to make sure the actual implementation is sane from their perspective. They've already seen the design, so I'm not expecting any real surprises here, but sometimes opinions change when they have actual code in front of them to play with and review.

Beyond that, while the cri-o/lxc/etc. folks are looking it over, whatever additional testing we can do would be a big win. I'm thinking I'll pull it into a separate branch in the audit tree (audit/working-container ?) and include that in my secnext kernels that I build/test on a regular basis; this is also a handy way to keep it based against the current audit/next branch. If any changes are needed Richard can either chose to base those changes on audit/next or the separate audit container ID branch; that's up to him. I've done this with other big changes in other trees, e.g. SELinux, and it has worked well to get some extra testing in and keep the patchset "merge ready" while others outside the subsystem look things over.

Quoted 3 lines

Mrunal Patel (maintainer of CRI-O) and I have reviewed the API, and believe this is something we can work on in the container runtimes team to implement the container auditing code in CRI-O and Podman.

Thanks Dan. If I pulled this into a branch and built you some test kernels to play with, any idea how long it might take to get a proof of concept working on the cri-o side?

FWIW, I've also reached out to some of the LXC folks I know to get their take on the API. I think if we can get two different container runtimes to give the API a thumbs-up then I think we are in good shape with respect to the userspace interface.

I just finished looking over the last of the pending audit kernel patches that were queued waiting for the merge window to open so this is next on my list to look at. I plan to start doing that tonight/tomorrow, and as long as the changes between v5/v6 are not that big, it shouldn't take too long.

Quoted 3 lines · EDT Paul Moore

Quoted 2 lines · Daniel Walsh

Quoted 1 line · Paul Moore

On Mon, Apr 22, 2019 at 7:38 AM Neil Horman <[email protected]>

Quoted 35 lines · wrote:

Quoted 34 lines

Quoted 19 lines

Quoted 5 lines · Richard Guy Briggs

Quoted 1 line

Implement kernel audit container identifier.

I'm sorry, I've lost track of this, where have we landed on it? Are we good for inclusion?

I haven't finished going through this latest revision, but unless Richard made any significant changes outside of the feedback from the v5 patchset I'm guessing we are "close".

Based on discussions Richard and I had some time ago, I have always envisioned the plan as being get the kernel patchset, tests, docs ready (which Richard has been doing) and then run the actual implemented API by the userland container folks, e.g. cri-o/lxc/etc., to make sure the actual implementation is sane from their perspective. They've already seen the design, so I'm not expecting any real surprises here, but sometimes opinions change when they have actual code in front of them to play with and review.

Quoted 11 lines

Beyond that, while the cri-o/lxc/etc. folks are looking it over, whatever additional testing we can do would be a big win. I'm thinking I'll pull it into a separate branch in the audit tree (audit/working-container ?) and include that in my secnext kernels that I build/test on a regular basis; this is also a handy way to keep it based against the current audit/next branch. If any changes are needed Richard can either chose to base those changes on audit/next or the separate audit container ID branch; that's up to him. I've done this with other big changes in other trees, e.g. SELinux, and it has worked well to get some extra testing in and keep the patchset "merge ready" while others outside the subsystem look things over.

Mrunal Patel (maintainer of CRI-O) and I have reviewed the API, and believe this is something we can work on in the container runtimes team to implement the container auditing code in CRI-O and Podman.

Quoted 3 lines

Thanks Dan. If I pulled this into a branch and built you some test kernels to play with, any idea how long it might take to get a proof of concept working on the cri-o side?

We'd need to merge user space patches and let them use that instead of the raw interface. I'm not going to merge user space until we are pretty sure the patch is going into the kernel.

-Steve

Quoted 10 lines

FWIW, I've also reached out to some of the LXC folks I know to get their take on the API. I think if we can get two different container runtimes to give the API a thumbs-up then I think we are in good shape with respect to the userspace interface.

I just finished looking over the last of the pending audit kernel patches that were queued waiting for the merge window to open so this is next on my list to look at. I plan to start doing that tonight/tomorrow, and as long as the changes between v5/v6 are not that big, it shouldn't take too long.

Quoted 36 lines · Steve Grubb

Quoted 3 lines · EDT Paul Moore

Quoted 2 lines · Daniel Walsh

Quoted 1 line · Paul Moore

On Mon, Apr 22, 2019 at 7:38 AM Neil Horman <[email protected]>

Quoted 31 lines · wrote:

Quoted 31 lines

Quoted 17 lines

Quoted 4 lines · Richard Guy Briggs

Quoted 1 line

Implement kernel audit container identifier.

I'm sorry, I've lost track of this, where have we landed on it? Are we good for inclusion?

I haven't finished going through this latest revision, but unless Richard made any significant changes outside of the feedback from the v5 patchset I'm guessing we are "close".

Based on discussions Richard and I had some time ago, I have always envisioned the plan as being get the kernel patchset, tests, docs ready (which Richard has been doing) and then run the actual implemented API by the userland container folks, e.g. cri-o/lxc/etc., to make sure the actual implementation is sane from their perspective. They've already seen the design, so I'm not expecting any real surprises here, but sometimes opinions change when they have actual code in front of them to play with and review.

Quoted 11 lines

Beyond that, while the cri-o/lxc/etc. folks are looking it over, whatever additional testing we can do would be a big win. I'm thinking I'll pull it into a separate branch in the audit tree (audit/working-container ?) and include that in my secnext kernels that I build/test on a regular basis; this is also a handy way to keep it based against the current audit/next branch. If any changes are needed Richard can either chose to base those changes on audit/next or the separate audit container ID branch; that's up to him. I've done this with other big changes in other trees, e.g. SELinux, and it has worked well to get some extra testing in and keep the patchset "merge ready" while others outside the subsystem look things over.

Mrunal Patel (maintainer of CRI-O) and I have reviewed the API, and believe this is something we can work on in the container runtimes team to implement the container auditing code in CRI-O and Podman.

Quoted 6 lines

Quoted 3 lines

Thanks Dan. If I pulled this into a branch and built you some test kernels to play with, any idea how long it might take to get a proof of concept working on the cri-o side?

We'd need to merge user space patches and let them use that instead of the raw interface. I'm not going to merge user space until we are pretty sure the patch is going into the kernel.

I have an f29 test rpm of the userspace bits if that helps for testing:

	http://people.redhat.com/~rbriggs/ghak90/git-1db7e21/

Here's what it contains (minus the last patch):
https://github.com/linux-audit/audit-userspace/compare/master...rgbriggs:ghau40-containerid-filter.v7.0

Quoted 12 lines

-Steve

Quoted 10 lines

FWIW, I've also reached out to some of the LXC folks I know to get their take on the API. I think if we can get two different container runtimes to give the API a thumbs-up then I think we are in good shape with respect to the userspace interface.

I just finished looking over the last of the pending audit kernel patches that were queued waiting for the merge window to open so this is next on my list to look at. I plan to start doing that tonight/tomorrow, and as long as the changes between v5/v6 are not that big, it shouldn't take too long.

- RGB

--
Richard Guy Briggs <[email protected]>
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

Quoted 2 lines · Steve Grubb

Quoted 1 line · EDT Paul Moore

On Tue, May 28, 2019 at 5:54 PM Daniel Walsh <[email protected]> wrote:

Quoted 11 lines

Quoted 7 lines

Quoted 3 lines

Mrunal Patel (maintainer of CRI-O) and I have reviewed the API, and believe this is something we can work on in the container runtimes team to implement the container auditing code in CRI-O and Podman.

Thanks Dan. If I pulled this into a branch and built you some test kernels to play with, any idea how long it might take to get a proof of concept working on the cri-o side?

We'd need to merge user space patches and let them use that instead of the raw interface. I'm not going to merge user space until we are pretty sure the patch is going into the kernel.

I have an f29 test rpm of the userspace bits if that helps for testing:

        http://people.redhat.com/~rbriggs/ghak90/git-1db7e21/

Here's what it contains (minus the last patch):
https://github.com/linux-audit/audit-userspace/compare/master...rgbriggs:ghau40-containerid-filter.v7.0

Quoted 61 lines · Richard Guy Briggs

Quoted 36 lines · Steve Grubb

Quoted 3 lines · EDT Paul Moore

Quoted 2 lines · Daniel Walsh

Quoted 1 line · Paul Moore

On Mon, Apr 22, 2019 at 7:38 AM Neil Horman <[email protected]>

Quoted 31 lines · wrote:

Quoted 31 lines

Quoted 17 lines

Quoted 4 lines · Richard Guy Briggs

Quoted 1 line

Implement kernel audit container identifier.

I'm sorry, I've lost track of this, where have we landed on it? Are we good for inclusion?

I haven't finished going through this latest revision, but unless Richard made any significant changes outside of the feedback from the v5 patchset I'm guessing we are "close".

Based on discussions Richard and I had some time ago, I have always envisioned the plan as being get the kernel patchset, tests, docs ready (which Richard has been doing) and then run the actual implemented API by the userland container folks, e.g. cri-o/lxc/etc., to make sure the actual implementation is sane from their perspective. They've already seen the design, so I'm not expecting any real surprises here, but sometimes opinions change when they have actual code in front of them to play with and review.

Quoted 11 lines

Beyond that, while the cri-o/lxc/etc. folks are looking it over, whatever additional testing we can do would be a big win. I'm thinking I'll pull it into a separate branch in the audit tree (audit/working-container ?) and include that in my secnext kernels that I build/test on a regular basis; this is also a handy way to keep it based against the current audit/next branch. If any changes are needed Richard can either chose to base those changes on audit/next or the separate audit container ID branch; that's up to him. I've done this with other big changes in other trees, e.g. SELinux, and it has worked well to get some extra testing in and keep the patchset "merge ready" while others outside the subsystem look things over.

Mrunal Patel (maintainer of CRI-O) and I have reviewed the API, and believe this is something we can work on in the container runtimes team to implement the container auditing code in CRI-O and Podman.

Quoted 6 lines

Quoted 3 lines

Thanks Dan. If I pulled this into a branch and built you some test kernels to play with, any idea how long it might take to get a proof of concept working on the cri-o side?

We'd need to merge user space patches and let them use that instead of the raw interface. I'm not going to merge user space until we are pretty sure the patch is going into the kernel.

I have an f29 test rpm of the userspace bits if that helps for testing:

      http://people.redhat.com/~rbriggs/ghak90/git-1db7e21/

Here's what it contains (minus the last patch):
https://github.com/linux-audit/audit-userspace/compare/master...rgbriggs:ghau40-containerid-filter.v7.0

Quoted 12 lines

-Steve

Quoted 10 lines

FWIW, I've also reached out to some of the LXC folks I know to get their take on the API. I think if we can get two different container runtimes to give the API a thumbs-up then I think we are in good shape with respect to the userspace interface.

I just finished looking over the last of the pending audit kernel patches that were queued waiting for the merge window to open so this is next on my list to look at. I plan to start doing that tonight/tomorrow, and as long as the changes between v5/v6 are not that big, it shouldn't take too long.

Quoted 8 lines

- RGB

--
Richard Guy Briggs <[email protected]>
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

Our current thoughts are to put the setting of the ID inside of conmon, and then launching the OCI Runtime. In a perfect world this would happen in the OCI Runtime, but we have no controls over different OCI Runtimes.

By putting it into conmon, then CRI-O and Podman will automatically get the container id support. After we have this we have to plumb it back up through the contianer engines to be able to easily report the link between the Container UUID and The Kernel Container Audit ID.

Quoted 75 lines · Daniel Walsh

Quoted 61 lines · Richard Guy Briggs

Quoted 36 lines · Steve Grubb

Quoted 3 lines · EDT Paul Moore

Quoted 2 lines · Daniel Walsh

Quoted 1 line · Paul Moore

On Mon, Apr 22, 2019 at 7:38 AM Neil Horman <[email protected]>

Quoted 31 lines · wrote:

Quoted 31 lines

Quoted 17 lines

Quoted 4 lines · Richard Guy Briggs

Quoted 1 line

Implement kernel audit container identifier.

I'm sorry, I've lost track of this, where have we landed on it? Are we good for inclusion?

I haven't finished going through this latest revision, but unless Richard made any significant changes outside of the feedback from the v5 patchset I'm guessing we are "close".

Based on discussions Richard and I had some time ago, I have always envisioned the plan as being get the kernel patchset, tests, docs ready (which Richard has been doing) and then run the actual implemented API by the userland container folks, e.g. cri-o/lxc/etc., to make sure the actual implementation is sane from their perspective. They've already seen the design, so I'm not expecting any real surprises here, but sometimes opinions change when they have actual code in front of them to play with and review.

Quoted 11 lines

Beyond that, while the cri-o/lxc/etc. folks are looking it over, whatever additional testing we can do would be a big win. I'm thinking I'll pull it into a separate branch in the audit tree (audit/working-container ?) and include that in my secnext kernels that I build/test on a regular basis; this is also a handy way to keep it based against the current audit/next branch. If any changes are needed Richard can either chose to base those changes on audit/next or the separate audit container ID branch; that's up to him. I've done this with other big changes in other trees, e.g. SELinux, and it has worked well to get some extra testing in and keep the patchset "merge ready" while others outside the subsystem look things over.

Mrunal Patel (maintainer of CRI-O) and I have reviewed the API, and believe this is something we can work on in the container runtimes team to implement the container auditing code in CRI-O and Podman.

Quoted 6 lines

Quoted 3 lines

Thanks Dan. If I pulled this into a branch and built you some test kernels to play with, any idea how long it might take to get a proof of concept working on the cri-o side?

We'd need to merge user space patches and let them use that instead of the raw interface. I'm not going to merge user space until we are pretty sure the patch is going into the kernel.

I have an f29 test rpm of the userspace bits if that helps for testing:

      http://people.redhat.com/~rbriggs/ghak90/git-1db7e21/

Here's what it contains (minus the last patch):
https://github.com/linux-audit/audit-userspace/compare/master...rgbriggs:ghau40-containerid-filter.v7.0

Quoted 12 lines

-Steve

Quoted 10 lines

FWIW, I've also reached out to some of the LXC folks I know to get their take on the API. I think if we can get two different container runtimes to give the API a thumbs-up then I think we are in good shape with respect to the userspace interface.

I just finished looking over the last of the pending audit kernel patches that were queued waiting for the merge window to open so this is next on my list to look at. I plan to start doing that tonight/tomorrow, and as long as the changes between v5/v6 are not that big, it shouldn't take too long.

Quoted 8 lines

- RGB

--
Richard Guy Briggs <[email protected]>
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

Our current thoughts are to put the setting of the ID inside of conmon, and then launching the OCI Runtime. In a perfect world this would happen in the OCI Runtime, but we have no controls over different OCI Runtimes.

Quoted 4 lines

By putting it into conmon, then CRI-O and Podman will automatically get the container id support. After we have this we have to plumb it back up through the contianer engines to be able to easily report the link between the Container UUID and The Kernel Container Audit ID.

I'm glad you guys have a plan, that's encouraging, but sadly I have no idea about the level of complexity/difficulty involved in modifying the various container bits for a proof-of-concept? Are we talking a week or two? A month? More?

Quoted 80 lines · Paul Moore

Quoted 79 lines · Daniel Walsh

Quoted 61 lines · Richard Guy Briggs

Quoted 36 lines · Steve Grubb

Quoted 3 lines · EDT Paul Moore

Quoted 2 lines · Daniel Walsh

Quoted 1 line · Paul Moore

On Mon, Apr 22, 2019 at 7:38 AM Neil Horman <[email protected]>

Quoted 31 lines · wrote:

Quoted 31 lines

Quoted 17 lines

Quoted 4 lines · Richard Guy Briggs

Quoted 1 line

Implement kernel audit container identifier.

I'm sorry, I've lost track of this, where have we landed on it? Are we good for inclusion?

I haven't finished going through this latest revision, but unless Richard made any significant changes outside of the feedback from the v5 patchset I'm guessing we are "close".

Based on discussions Richard and I had some time ago, I have always envisioned the plan as being get the kernel patchset, tests, docs ready (which Richard has been doing) and then run the actual implemented API by the userland container folks, e.g. cri-o/lxc/etc., to make sure the actual implementation is sane from their perspective. They've already seen the design, so I'm not expecting any real surprises here, but sometimes opinions change when they have actual code in front of them to play with and review.

Quoted 11 lines

Beyond that, while the cri-o/lxc/etc. folks are looking it over, whatever additional testing we can do would be a big win. I'm thinking I'll pull it into a separate branch in the audit tree (audit/working-container ?) and include that in my secnext kernels that I build/test on a regular basis; this is also a handy way to keep it based against the current audit/next branch. If any changes are needed Richard can either chose to base those changes on audit/next or the separate audit container ID branch; that's up to him. I've done this with other big changes in other trees, e.g. SELinux, and it has worked well to get some extra testing in and keep the patchset "merge ready" while others outside the subsystem look things over.

Mrunal Patel (maintainer of CRI-O) and I have reviewed the API, and believe this is something we can work on in the container runtimes team to implement the container auditing code in CRI-O and Podman.

Quoted 6 lines

Quoted 3 lines

Thanks Dan. If I pulled this into a branch and built you some test kernels to play with, any idea how long it might take to get a proof of concept working on the cri-o side?

We'd need to merge user space patches and let them use that instead of the raw interface. I'm not going to merge user space until we are pretty sure the patch is going into the kernel.

I have an f29 test rpm of the userspace bits if that helps for testing:

      http://people.redhat.com/~rbriggs/ghak90/git-1db7e21/

Here's what it contains (minus the last patch):
https://github.com/linux-audit/audit-userspace/compare/master...rgbriggs:ghau40-containerid-filter.v7.0

Quoted 12 lines

-Steve

Quoted 10 lines

FWIW, I've also reached out to some of the LXC folks I know to get their take on the API. I think if we can get two different container runtimes to give the API a thumbs-up then I think we are in good shape with respect to the userspace interface.

I just finished looking over the last of the pending audit kernel patches that were queued waiting for the merge window to open so this is next on my list to look at. I plan to start doing that tonight/tomorrow, and as long as the changes between v5/v6 are not that big, it shouldn't take too long.

Quoted 8 lines

- RGB

--
Richard Guy Briggs <[email protected]>
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

Our current thoughts are to put the setting of the ID inside of conmon, and then launching the OCI Runtime. In a perfect world this would happen in the OCI Runtime, but we have no controls over different OCI Runtimes.

By putting it into conmon, then CRI-O and Podman will automatically get the container id support. After we have this we have to plumb it back up through the contianer engines to be able to easily report the link between the Container UUID and The Kernel Container Audit ID.

Quoted 5 lines

I'm glad you guys have a plan, that's encouraging, but sadly I have no idea about the level of complexity/difficulty involved in modifying the various container bits for a proof-of-concept? Are we talking a week or two? A month? More?

If we had the kernel and the libaudit api, it would involve a small effort in conmon, I would figure a few days for a POC. Getting the hole wiring into CRI-O and Podman, would be a little more effort.

It is not permitted to unset the audit container identifier. A child inherits its parent's audit container identifier.

/**
+ * audit_set_contid - set current task's audit contid
+ * @contid: contid value
+ *
+ * Returns 0 on success, -EPERM on permission failure.
+ *
+ * Called (set) from fs/proc/base.c::proc_contid_write().
+ */

kernel/audit.c +25 -0

+int audit_set_contid(struct task_struct *task, u64 contid)
+{
+	u64 oldcontid;
+	int rc = 0;
+	struct audit_buffer *ab;
+	uid_t uid;
+	struct tty_struct *tty;
+	char comm[sizeof(current->comm)];
+
+	task_lock(task);
+	/* Can't set if audit disabled */
+	if (!task->audit) {
+		task_unlock(task);
+		return -ENOPROTOOPT;
+	}
+	oldcontid = audit_get_contid(task);
+	read_lock(&tasklist_lock);
+	/* Don't allow the audit containerid to be unset */
+	if (!audit_contid_valid(contid))
+		rc = -EINVAL;
+	/* if we don't have caps, reject */
+	else if (!capable(CAP_AUDIT_CONTROL))
+		rc = -EPERM;
+	/* if task has children or is not single-threaded, deny */
+	else if (!list_empty(&task->children))

Quoted 2 lines · Richard Guy Briggs

It is not permitted to unset the audit container identifier. A child inherits its parent's audit container identifier.

...

Quoted 41 lines

/**
+ * audit_set_contid - set current task's audit contid
+ * @contid: contid value
+ *
+ * Returns 0 on success, -EPERM on permission failure.
+ *
+ * Called (set) from fs/proc/base.c::proc_contid_write().
+ */

kernel/audit.c +33 -0

+int audit_set_contid(struct task_struct *task, u64 contid)
+{
+     u64 oldcontid;
+     int rc = 0;
+     struct audit_buffer *ab;
+     uid_t uid;
+     struct tty_struct *tty;
+     char comm[sizeof(current->comm)];
+
+     task_lock(task);
+     /* Can't set if audit disabled */
+     if (!task->audit) {
+             task_unlock(task);
+             return -ENOPROTOOPT;
+     }
+     oldcontid = audit_get_contid(task);
+     read_lock(&tasklist_lock);
+     /* Don't allow the audit containerid to be unset */
+     if (!audit_contid_valid(contid))
+             rc = -EINVAL;
+     /* if we don't have caps, reject */
+     else if (!capable(CAP_AUDIT_CONTROL))
+             rc = -EPERM;
+     /* if task has children or is not single-threaded, deny */
+     else if (!list_empty(&task->children))
+             rc = -EBUSY;
+     else if (!(thread_group_leader(task) && thread_group_empty(task)))
+             rc = -EALREADY;
+     read_unlock(&tasklist_lock);
+     if (!rc)
+             task->audit->contid = contid;
+     task_unlock(task);
+

Review

Quoted 2 lines

(quoted diff) +2 -0

+     if (!audit_enabled)
+             return rc;

...but it is allowed to change it (assuming capable(CAP_AUDIT_CONTROL), of course)? Seems like this might be more immediately useful since we still live in the world of majority privileged containers if we didn't allow changing it, in addition to un-setting it.

From a practical perspective, un-setting the audit container ID is

Quoted 48 lines · Tycho Andersen

Quoted 2 lines · Richard Guy Briggs

It is not permitted to unset the audit container identifier. A child inherits its parent's audit container identifier.

...

Quoted 41 lines

/**
+ * audit_set_contid - set current task's audit contid
+ * @contid: contid value
+ *
+ * Returns 0 on success, -EPERM on permission failure.
+ *
+ * Called (set) from fs/proc/base.c::proc_contid_write().
+ */

kernel/audit.c +33 -0

+int audit_set_contid(struct task_struct *task, u64 contid)
+{
+     u64 oldcontid;
+     int rc = 0;
+     struct audit_buffer *ab;
+     uid_t uid;
+     struct tty_struct *tty;
+     char comm[sizeof(current->comm)];
+
+     task_lock(task);
+     /* Can't set if audit disabled */
+     if (!task->audit) {
+             task_unlock(task);
+             return -ENOPROTOOPT;
+     }
+     oldcontid = audit_get_contid(task);
+     read_lock(&tasklist_lock);
+     /* Don't allow the audit containerid to be unset */
+     if (!audit_contid_valid(contid))
+             rc = -EINVAL;
+     /* if we don't have caps, reject */
+     else if (!capable(CAP_AUDIT_CONTROL))
+             rc = -EPERM;
+     /* if task has children or is not single-threaded, deny */
+     else if (!list_empty(&task->children))
+             rc = -EBUSY;
+     else if (!(thread_group_leader(task) && thread_group_empty(task)))
+             rc = -EALREADY;
+     read_unlock(&tasklist_lock);
+     if (!rc)
+             task->audit->contid = contid;
+     task_unlock(task);
+

Quoted 8 lines

Review

Quoted 2 lines

(quoted diff) +2 -0

+     if (!audit_enabled)
+             return rc;

...but it is allowed to change it (assuming capable(CAP_AUDIT_CONTROL), of course)? Seems like this might be more immediately useful since we still live in the world of majority privileged containers if we didn't allow changing it, in addition to un-setting it.

The idea is that only container orchestrators should be able to set/modify the audit container ID, and since setting the audit container ID can have a significant effect on the records captured (and their routing to multiple daemons when we get there) modifying the audit container ID is akin to modifying the audit configuration which is why it is gated by CAP_AUDIT_CONTROL. The current thinking is that you would only change the audit container ID from one set/inherited value to another if you were nesting containers, in which case the nested container orchestrator would need to be granted CAP_AUDIT_CONTROL (which everyone to date seems to agree is a workable compromise).

Quoted 58 lines · Paul Moore

Quoted 48 lines · Tycho Andersen

Quoted 2 lines · Richard Guy Briggs

It is not permitted to unset the audit container identifier. A child inherits its parent's audit container identifier.

...

Quoted 41 lines

/**
+ * audit_set_contid - set current task's audit contid
+ * @contid: contid value
+ *
+ * Returns 0 on success, -EPERM on permission failure.
+ *
+ * Called (set) from fs/proc/base.c::proc_contid_write().
+ */

kernel/audit.c +33 -0

+int audit_set_contid(struct task_struct *task, u64 contid)
+{
+     u64 oldcontid;
+     int rc = 0;
+     struct audit_buffer *ab;
+     uid_t uid;
+     struct tty_struct *tty;
+     char comm[sizeof(current->comm)];
+
+     task_lock(task);
+     /* Can't set if audit disabled */
+     if (!task->audit) {
+             task_unlock(task);
+             return -ENOPROTOOPT;
+     }
+     oldcontid = audit_get_contid(task);
+     read_lock(&tasklist_lock);
+     /* Don't allow the audit containerid to be unset */
+     if (!audit_contid_valid(contid))
+             rc = -EINVAL;
+     /* if we don't have caps, reject */
+     else if (!capable(CAP_AUDIT_CONTROL))
+             rc = -EPERM;
+     /* if task has children or is not single-threaded, deny */
+     else if (!list_empty(&task->children))
+             rc = -EBUSY;
+     else if (!(thread_group_leader(task) && thread_group_empty(task)))
+             rc = -EALREADY;
+     read_unlock(&tasklist_lock);
+     if (!rc)
+             task->audit->contid = contid;
+     task_unlock(task);
+

Quoted 8 lines

Review

Quoted 2 lines

(quoted diff) +2 -0

+     if (!audit_enabled)
+             return rc;

...but it is allowed to change it (assuming capable(CAP_AUDIT_CONTROL), of course)? Seems like this might be more immediately useful since we still live in the world of majority privileged containers if we didn't allow changing it, in addition to un-setting it.

Quoted 11 lines

The idea is that only container orchestrators should be able to set/modify the audit container ID, and since setting the audit container ID can have a significant effect on the records captured (and their routing to multiple daemons when we get there) modifying the audit container ID is akin to modifying the audit configuration which is why it is gated by CAP_AUDIT_CONTROL. The current thinking is that you would only change the audit container ID from one set/inherited value to another if you were nesting containers, in which case the nested container orchestrator would need to be granted CAP_AUDIT_CONTROL (which everyone to date seems to agree is a workable compromise).

But then don't you want some kind of ns_capable() instead (probably not the obvious one, though...)? With capable(), you can't really nest using the audit-id and user namespaces together.

Create a new audit record AUDIT_CONTAINER_ID 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=0 cap_fi=0 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=0 cap_fi=0 cap_fe=0 cap_fver=0
type=PROCTITLE msg=audit(1519924845.499:257): proctitle=62617368002D6300736C65657020313B206563686F2074657374203E202F746D702F746D70636F6E7461696E65726964
type=CONTAINER_ID msg=audit(1519924845.499:257): contid=123458

Please see the github audit kernel issue for the main feature:
https://github.com/linux-audit/audit-kernel/issues/90
Please see the github audit userspace issue for supporting additions:
https://github.com/linux-audit/audit-userspace/issues/51
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 <[email protected]>

Acked-by Serge Hallyn <[email protected]>

Acked-by Steve Grubb <[email protected]>

Acked-by Neil Horman <[email protected]>

Reviewed-by Ondrej Mosnacek <[email protected]>

---
 include/linux/audit.h      |  5 +++++
 include/uapi/linux/audit.h |  1 +
 kernel/audit.c             | 20 ++++++++++++++++++++
 kernel/auditsc.c           | 20 ++++++++++++++------
 4 files changed, 40 insertions(+), 6 deletions(-)

kernel/audit.c +6 -0

index 182b0f2c183d..3e0af53f3c4d 100644
@@ -2127,6 +2127,26 @@ 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
+ * @context: task or local context for record
+ * @contid: container ID to report
+ */
+void audit_log_contid(struct audit_context *context, u64 contid)

kernel/audit.c +3 -0

+       audit_log_end(ab);
+}
+EXPORT_SYMBOL(audit_log_contid);

Implement audit container identifier filtering using the AUDIT_CONTID field name to send an 8-character string representing a u64 since the value field is only u32.

Sending it as two u32 was considered, but gathering and comparing two fields was more complex.

The feature indicator is AUDIT_FEATURE_BITMAP_CONTAINERID.

Please see the github audit kernel issue for the contid filter feature:
https://github.com/linux-audit/audit-kernel/issues/91
Please see the github audit userspace issue for filter additions:
https://github.com/linux-audit/audit-userspace/issues/40
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 <[email protected]>

Acked-by Serge Hallyn <[email protected]>

Acked-by Neil Horman <[email protected]>

Reviewed-by Ondrej Mosnacek <[email protected]>

---
 include/linux/audit.h      |  1 +
 include/uapi/linux/audit.h |  5 ++++-
 kernel/audit.h             |  1 +
 kernel/auditfilter.c       | 47 ++++++++++++++++++++++++++++++++++++++++++++++
 kernel/auditsc.c           |  4 ++++
 5 files changed, 57 insertions(+), 1 deletion(-)

kernel/auditfilter.c +11 -0

index 63f8b3f26fab..407b5bb3b4c6 100644
@@ -1206,6 +1224,31 @@ int audit_comparator(u32 left, u32 op, u32 right)
        }
 }
 
+int audit_comparator64(u64 left, u32 op, u64 right)
+{
+       switch (op) {
+       case Audit_equal:
+               return (left == right);
+       case Audit_not_equal:
+               return (left != right);
+       case Audit_lt:
+               return (left < right);
+       case Audit_le:
+               return (left <= right);

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

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 <[email protected]>

Acked-by Neil Horman <[email protected]>

Reviewed-by Ondrej Mosnacek <[email protected]>

---
 include/linux/audit.h | 19 +++++++++++
 kernel/audit.c        | 88 +++++++++++++++++++++++++++++++++++++++++++++++++--
 kernel/nsproxy.c      |  4 +++
 3 files changed, 108 insertions(+), 3 deletions(-)

kernel/audit.c +13 -0

index 6c742da66b32..996213591617 100644
@@ -376,6 +384,75 @@ static struct sock *audit_get_sk(const struct net *net)
        return aunet->sk;
 }
 
+void audit_netns_contid_add(struct net *net, u64 contid)
+{
+       struct audit_net *aunet;
+       struct list_head *contid_list;
+       struct audit_contid *cont;
+
+       if (!net)
+               return;
+       if (!audit_contid_valid(contid))
+               return;
+       aunet = net_generic(net, audit_net_id);
+       if (!aunet)
+               return;

kernel/audit.c +7 -0

+               cont->id = contid;
+               refcount_set(&cont->refcount, 1);
+               list_add_rcu(&cont->list, contid_list);
+       }
+out:
+       spin_unlock(&aunet->contid_list_lock);
+}

Quoted 5 lines · Neil Horman

Quoted 1 line · Richard Guy Briggs

Implement kernel audit container identifier.

I'm sorry, I've lost track of this, where have we landed on it? Are we good for inclusion?

I haven't finished going through this latest revision, but unless Richard made any significant changes outside of the feedback from the v5 patchset I'm guessing we are "close".

Based on discussions Richard and I had some time ago, I have always envisioned the plan as being get the kernel patchset, tests, docs ready (which Richard has been doing) and then run the actual implemented API by the userland container folks, e.g. cri-o/lxc/etc., to make sure the actual implementation is sane from their perspective. They've already seen the design, so I'm not expecting any real surprises here, but sometimes opinions change when they have actual code in front of them to play with and review.

Beyond that, while the cri-o/lxc/etc. folks are looking it over, whatever additional testing we can do would be a big win. I'm thinking I'll pull it into a separate branch in the audit tree (audit/working-container ?) and include that in my secnext kernels that I build/test on a regular basis; this is also a handy way to keep it based against the current audit/next branch. If any changes are needed Richard can either chose to base those changes on audit/next or the separate audit container ID branch; that's up to him. I've done this with other big changes in other trees, e.g. SELinux, and it has worked well to get some extra testing in and keep the patchset "merge ready" while others outside the subsystem look things over.

Quoted 72 lines · Tycho Andersen

Quoted 69 lines · Paul Moore

Quoted 48 lines · Tycho Andersen

Quoted 2 lines · Richard Guy Briggs

It is not permitted to unset the audit container identifier. A child inherits its parent's audit container identifier.

...

Quoted 41 lines

/**
+ * audit_set_contid - set current task's audit contid
+ * @contid: contid value
+ *
+ * Returns 0 on success, -EPERM on permission failure.
+ *
+ * Called (set) from fs/proc/base.c::proc_contid_write().
+ */

kernel/audit.c +33 -0

+int audit_set_contid(struct task_struct *task, u64 contid)
+{
+     u64 oldcontid;
+     int rc = 0;
+     struct audit_buffer *ab;
+     uid_t uid;
+     struct tty_struct *tty;
+     char comm[sizeof(current->comm)];
+
+     task_lock(task);
+     /* Can't set if audit disabled */
+     if (!task->audit) {
+             task_unlock(task);
+             return -ENOPROTOOPT;
+     }
+     oldcontid = audit_get_contid(task);
+     read_lock(&tasklist_lock);
+     /* Don't allow the audit containerid to be unset */
+     if (!audit_contid_valid(contid))
+             rc = -EINVAL;
+     /* if we don't have caps, reject */
+     else if (!capable(CAP_AUDIT_CONTROL))
+             rc = -EPERM;
+     /* if task has children or is not single-threaded, deny */
+     else if (!list_empty(&task->children))
+             rc = -EBUSY;
+     else if (!(thread_group_leader(task) && thread_group_empty(task)))
+             rc = -EALREADY;
+     read_unlock(&tasklist_lock);
+     if (!rc)
+             task->audit->contid = contid;
+     task_unlock(task);
+

Quoted 8 lines

Review

Quoted 2 lines

(quoted diff) +2 -0

+     if (!audit_enabled)
+             return rc;

...but it is allowed to change it (assuming capable(CAP_AUDIT_CONTROL), of course)? Seems like this might be more immediately useful since we still live in the world of majority privileged containers if we didn't allow changing it, in addition to un-setting it.

The idea is that only container orchestrators should be able to set/modify the audit container ID, and since setting the audit container ID can have a significant effect on the records captured (and their routing to multiple daemons when we get there) modifying the audit container ID is akin to modifying the audit configuration which is why it is gated by CAP_AUDIT_CONTROL. The current thinking is that you would only change the audit container ID from one set/inherited value to another if you were nesting containers, in which case the nested container orchestrator would need to be granted CAP_AUDIT_CONTROL (which everyone to date seems to agree is a workable compromise).

Quoted 3 lines

But then don't you want some kind of ns_capable() instead (probably not the obvious one, though...)? With capable(), you can't really nest using the audit-id and user namespaces together.

You want capable() and not ns_capable() because you want to ensure that the orchestrator has the rights in the init_ns as changes to the audit container ID could have an auditing impact that spans the entire system.

Quoted 6 lines

Quoted 6 lines

The current thinking
is that you would only change the audit container ID from one set/inherited value to another if you were nesting containers, in which case the nested container orchestrator would need to be granted CAP_AUDIT_CONTROL (which everyone to date seems to agree is a workable compromise).

Quoted 73 lines · Paul Moore

Quoted 72 lines · Tycho Andersen

Quoted 69 lines · Paul Moore

Quoted 48 lines · Tycho Andersen

Quoted 2 lines · Richard Guy Briggs

It is not permitted to unset the audit container identifier. A child inherits its parent's audit container identifier.

...

Quoted 41 lines

/**
+ * audit_set_contid - set current task's audit contid
+ * @contid: contid value
+ *
+ * Returns 0 on success, -EPERM on permission failure.
+ *
+ * Called (set) from fs/proc/base.c::proc_contid_write().
+ */

kernel/audit.c +33 -0

+int audit_set_contid(struct task_struct *task, u64 contid)
+{
+     u64 oldcontid;
+     int rc = 0;
+     struct audit_buffer *ab;
+     uid_t uid;
+     struct tty_struct *tty;
+     char comm[sizeof(current->comm)];
+
+     task_lock(task);
+     /* Can't set if audit disabled */
+     if (!task->audit) {
+             task_unlock(task);
+             return -ENOPROTOOPT;
+     }
+     oldcontid = audit_get_contid(task);
+     read_lock(&tasklist_lock);
+     /* Don't allow the audit containerid to be unset */
+     if (!audit_contid_valid(contid))
+             rc = -EINVAL;
+     /* if we don't have caps, reject */
+     else if (!capable(CAP_AUDIT_CONTROL))
+             rc = -EPERM;
+     /* if task has children or is not single-threaded, deny */
+     else if (!list_empty(&task->children))
+             rc = -EBUSY;
+     else if (!(thread_group_leader(task) && thread_group_empty(task)))
+             rc = -EALREADY;
+     read_unlock(&tasklist_lock);
+     if (!rc)
+             task->audit->contid = contid;
+     task_unlock(task);
+

Quoted 8 lines

Review

Quoted 2 lines

(quoted diff) +2 -0

+     if (!audit_enabled)
+             return rc;

...but it is allowed to change it (assuming capable(CAP_AUDIT_CONTROL), of course)? Seems like this might be more immediately useful since we still live in the world of majority privileged containers if we didn't allow changing it, in addition to un-setting it.

The idea is that only container orchestrators should be able to set/modify the audit container ID, and since setting the audit container ID can have a significant effect on the records captured (and their routing to multiple daemons when we get there) modifying the audit container ID is akin to modifying the audit configuration which is why it is gated by CAP_AUDIT_CONTROL. The current thinking is that you would only change the audit container ID from one set/inherited value to another if you were nesting containers, in which case the nested container orchestrator would need to be granted CAP_AUDIT_CONTROL (which everyone to date seems to agree is a workable compromise).

Quoted 8 lines

Quoted 3 lines

But then don't you want some kind of ns_capable() instead (probably not the obvious one, though...)? With capable(), you can't really nest using the audit-id and user namespaces together.

You want capable() and not ns_capable() because you want to ensure that the orchestrator has the rights in the init_ns as changes to the audit container ID could have an auditing impact that spans the entire system.

Ok but,

Quoted 6 lines

Quoted 6 lines

Quoted 6 lines

The current thinking
is that you would only change the audit container ID from one set/inherited value to another if you were nesting containers, in which case the nested container orchestrator would need to be granted CAP_AUDIT_CONTROL (which everyone to date seems to agree is a workable compromise).

won't work in user namespaced containers, because they will never be capable(CAP_AUDIT_CONTROL); so I don't think this will work for nesting as is. But maybe nobody cares :)

Quoted 33 lines · Richard Guy Briggs

Create a new audit record AUDIT_CONTAINER_ID 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=0 cap_fi=0 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=0 cap_fi=0 cap_fe=0 cap_fver=0
type=PROCTITLE msg=audit(1519924845.499:257): proctitle=62617368002D6300736C65657020313B206563686F2074657374203E202F746D702F746D70636F6E7461696E65726964
type=CONTAINER_ID msg=audit(1519924845.499:257): contid=123458

Please see the github audit kernel issue for the main feature:
https://github.com/linux-audit/audit-kernel/issues/90
Please see the github audit userspace issue for supporting additions:
https://github.com/linux-audit/audit-userspace/issues/51
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 <[email protected]>

Acked-by Serge Hallyn <[email protected]>

Acked-by Steve Grubb <[email protected]>

Acked-by Neil Horman <[email protected]>

Reviewed-by Ondrej Mosnacek <[email protected]>

---
 include/linux/audit.h      |  5 +++++
 include/uapi/linux/audit.h |  1 +
 kernel/audit.c             | 20 ++++++++++++++++++++
 kernel/auditsc.c           | 20 ++++++++++++++------
 4 files changed, 40 insertions(+), 6 deletions(-)

...

Quoted 23 lines

kernel/audit.c +15 -0

index 182b0f2c183d..3e0af53f3c4d 100644
@@ -2127,6 +2127,26 @@ 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
+ * @context: task or local context for record
+ * @contid: container ID to report
+ */
+void audit_log_contid(struct audit_context *context, u64 contid)
+{
+       struct audit_buffer *ab;
+
+       if (!audit_contid_valid(contid))
+               return;
+       /* Generate AUDIT_CONTAINER_ID record with container ID */
+       ab = audit_log_start(context, GFP_KERNEL, AUDIT_CONTAINER_ID);
+       if (!ab)
+               return;

kernel/audit.c void audit_log_session_info(struct audit_buffer *ab) · line 2145

Quoted 1 line

kernel/audit.c +1 -0

+       audit_log_format(ab, "contid=%llu", (unsigned long long)contid);

We have a consistency problem regarding how to output the u64 contid values; this function uses an explicit cast, others do not. According to Documentation/core-api/printk-formats.rst the recommendation for u64 is %llu (or %llx, if you want hex). Looking quickly through the printk code this appears to still be correct. I suggest we get rid of the cast (like it was in v5).

kernel/audit.c void audit_log_session_info(struct audit_buffer *ab) · line 2148

Quoted 4 lines

kernel/audit.c +3 -0

+       audit_log_end(ab);
+}
+EXPORT_SYMBOL(audit_log_contid);

--
paul moore
www.paul-moore.com

Quoted 1 line · Paul Moore

On Mon, Apr 22, 2019 at 7:38 AM Neil Horman <[email protected]>

Quoted 19 lines

Quoted 5 lines · Richard Guy Briggs

Quoted 1 line

Implement kernel audit container identifier.

I'm sorry, I've lost track of this, where have we landed on it? Are we good for inclusion?

I haven't finished going through this latest revision, but unless Richard made any significant changes outside of the feedback from the v5 patchset I'm guessing we are "close".

Based on discussions Richard and I had some time ago, I have always envisioned the plan as being get the kernel patchset, tests, docs ready (which Richard has been doing) and then run the actual implemented API by the userland container folks, e.g. cri-o/lxc/etc., to make sure the actual implementation is sane from their perspective. They've already seen the design, so I'm not expecting any real surprises here, but sometimes opinions change when they have actual code in front of them to play with and review.

Quoted 11 lines

Beyond that, while the cri-o/lxc/etc. folks are looking it over, whatever additional testing we can do would be a big win. I'm thinking I'll pull it into a separate branch in the audit tree (audit/working-container ?) and include that in my secnext kernels that I build/test on a regular basis; this is also a handy way to keep it based against the current audit/next branch. If any changes are needed Richard can either chose to base those changes on audit/next or the separate audit container ID branch; that's up to him. I've done this with other big changes in other trees, e.g. SELinux, and it has worked well to get some extra testing in and keep the patchset "merge ready" while others outside the subsystem look things over.

I just sent my feedback on the v6 patchset, and it's small: basically three patches with "one-liner" changes needed.

Richard, it's your call on how you want to proceed from here. You can post a v7 incorporating the feedback, or since the tweaks are so minor, you can post fixup patches; the former being more comprehensive, the later being quicker to review and digest. Regardless of that, while we are waiting on a prototype from the container folks, I think it would be good to pull this into a working branch in the audit repo (as mentioned above), unless you would prefer to keep it as a patchset on the mailing list?

If you want to go with
the working branch approach, I'll keep the branch fresh and (re)based against audit/next and if we notice any problems you can just submit fixes against that branch (depending on the issue they can be fixup patches, or proper patches). My hope is that this will enable the process to move quicker as we get near the finish line.