linux-fsdevel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [RFC PATCH V1 00/12] audit: implement container id
@ 2018-03-01 19:41 Richard Guy Briggs
  2018-03-01 19:41 ` [RFC PATCH V1 01/12] audit: add " Richard Guy Briggs
                   ` (13 more replies)
  0 siblings, 14 replies; 31+ messages in thread
From: Richard Guy Briggs @ 2018-03-01 19:41 UTC (permalink / raw)
  To: cgroups, containers, linux-api, Linux-Audit Mailing List,
	linux-fsdevel, LKML, netdev
  Cc: mszeredi, luto, jlayton, carlos, viro, dhowells, simo, trondmy,
	eparis, serge, ebiederm, madzcar, Richard Guy Briggs

Implement audit kernel container ID.

This patchset is a preliminary RFC based on the proposal document (V3)
posted:
	https://www.redhat.com/archives/linux-audit/2018-January/msg00014.html

The first patch implements the proc fs write to set the audit container
ID of a process, emitting an AUDIT_CONTAINER record.

The second implements an auxiliary syscall record AUDIT_CONTAINER_INFO
if a container ID is present on a task.

The third adds filtering to the exit, exclude and user lists.

The 4th, implements reading the container ID from the proc filesystem
for debugging.  This isn't planned for upstream inclusion.

The 5th adds signal and ptrace support.

The 6th attempts to create a local audit context to be able to bind a
standalone record with the container ID record.

The 7th, 8th, 9th, 10th patches add container ID records to standalone
records.  Some of these may end up being syscall auxiliary records and
won't need this specific support since they'll be supported via
syscalls.

The 11th is a temporary workaround due to the AUDIT_CONTAINER records
not showing up as do AUDIT_LOGIN records.  I suspect this is due to its
range (1000 vs 1300), but the intent is to solve it.

The 12th adds debug information not intended for upstream for those
brave souls wanting to tinker with it in this early state.

Feedback please!

Here's a quick and dirty test script:
echo 123455 > /proc/$$/containerid; echo $?
sleep 4&  
child=$!; sleep 1
echo 18446744073709551615 > /proc/$child/containerid; echo $?
echo 123456 > /proc/$child/containerid; echo $?
echo 123457 > /proc/$child/containerid; echo $?
sleep 1
ausearch -ts recent |grep " contid=18446744073709551615"; echo $?
ausearch -ts recent |grep " contid=123456"; echo $?
ausearch -ts recent |grep " contid=123457"; echo $?
echo self:$$ contid:$( cat /proc/$$/containerid)
echo child:$child contid:$( cat /proc/$child/containerid)

containerid=123458
key=tmpcontainerid
auditctl -a exit,always -F dir=/tmp -F perm=wa -F containerid=$containerid -F key=$key || echo failed to add containerid filter rule
bash -c "sleep 1; echo test > /tmp/$key"&
child=$!
echo $containerid > /proc/$child/containerid
sleep 2
rm -f /tmp/$key
ausearch -ts recent -k $key || echo failed to find CONTAINER_INFO record
auditctl -d exit,always -F dir=/tmp -F perm=wa -F containerid=$containerid -F key=$key || echo failed to add containerid filter rule

See:
	https://github.com/linux-audit/audit-kernel/issues/32
	https://github.com/linux-audit/audit-userspace/issues/40
	https://github.com/linux-audit/audit-testsuite/issues/64

Richard Guy Briggs (12):
  audit: add container id
  audit: log container info of syscalls
  audit: add containerid filtering
  audit: read container ID of a process
  audit: add containerid support for ptrace and signals
  audit: add support for non-syscall auxiliary records
  audit: add container aux record to watch/tree/mark
  audit: add containerid support for tty_audit
  audit: add containerid support for config/feature/user records
  audit: add containerid support for seccomp and anom_abend records
  debug audit: add container id
  debug! audit: add container id

 drivers/tty/tty_audit.c    |   5 +-
 fs/proc/base.c             |  63 +++++++++++++++++++
 include/linux/audit.h      |  36 +++++++++++
 include/linux/init_task.h  |   4 +-
 include/linux/sched.h      |   1 +
 include/uapi/linux/audit.h |   9 ++-
 kernel/audit.c             |  74 +++++++++++++++++++---
 kernel/audit.h             |   3 +
 kernel/audit_fsnotify.c    |   5 +-
 kernel/audit_tree.c        |   5 +-
 kernel/audit_watch.c       |  33 +++++-----
 kernel/auditfilter.c       |  52 ++++++++++++++-
 kernel/auditsc.c           | 154 +++++++++++++++++++++++++++++++++++++++++++--
 13 files changed, 408 insertions(+), 36 deletions(-)

-- 
1.8.3.1

^ permalink raw reply	[flat|nested] 31+ messages in thread

* [RFC PATCH V1 01/12] audit: add container id
  2018-03-01 19:41 [RFC PATCH V1 00/12] audit: implement container id Richard Guy Briggs
@ 2018-03-01 19:41 ` Richard Guy Briggs
  2018-03-02  1:41   ` Richard Guy Briggs
                     ` (2 more replies)
  2018-03-01 19:41 ` [RFC PATCH V1 02/12] audit: log container info of syscalls Richard Guy Briggs
                   ` (12 subsequent siblings)
  13 siblings, 3 replies; 31+ messages in thread
From: Richard Guy Briggs @ 2018-03-01 19:41 UTC (permalink / raw)
  To: cgroups, containers, linux-api, Linux-Audit Mailing List,
	linux-fsdevel, LKML, netdev
  Cc: mszeredi, luto, jlayton, carlos, viro, dhowells, simo, trondmy,
	eparis, serge, ebiederm, madzcar, Richard Guy Briggs

Implement the proc fs write to set the audit container ID of a process,
emitting an AUDIT_CONTAINER record to document the event.

This is a write from the container orchestrator task to a proc entry of
the form /proc/PID/containerid where PID is the process ID of the newly
created task that is to become the first task in a container, or an
additional task added to a container.

The write expects up to a u64 value (unset: 18446744073709551615).

This will produce a record such as this:
type=UNKNOWN[1333] msg=audit(1519903238.968:261): op=set pid=596 uid=0 subj=unconfined_u:unconfined_r:unconfined_t:s0-s0:c0.c1023 auid=0 tty=pts0 ses=1 opid=596 old-contid=18446744073709551615 contid=123455 res=0

The "op" field indicates an initial set.  The "pid" to "ses" fields are
the orchestrator while the "opid" field is the object's PID, the process
being "contained".  Old and new container ID values are given in the
"contid" fields, while res indicates its success.

It is not permitted to self-set, unset or re-set the container ID.  A
child inherits its parent's container ID, but then can be set only once
after.

See: https://github.com/linux-audit/audit-kernel/issues/32

Signed-off-by: Richard Guy Briggs <rgb@redhat.com>
---
 fs/proc/base.c             | 37 ++++++++++++++++++++
 include/linux/audit.h      | 16 +++++++++
 include/linux/init_task.h  |  4 ++-
 include/linux/sched.h      |  1 +
 include/uapi/linux/audit.h |  2 ++
 kernel/auditsc.c           | 86 ++++++++++++++++++++++++++++++++++++++++++++++
 6 files changed, 145 insertions(+), 1 deletion(-)

diff --git a/fs/proc/base.c b/fs/proc/base.c
index 60316b5..6ce4fbe 100644
--- a/fs/proc/base.c
+++ b/fs/proc/base.c
@@ -1299,6 +1299,41 @@ static ssize_t proc_sessionid_read(struct file * file, char __user * buf,
 	.read		= proc_sessionid_read,
 	.llseek		= generic_file_llseek,
 };
+
+static ssize_t proc_containerid_write(struct file *file, const char __user *buf,
+				   size_t count, loff_t *ppos)
+{
+	struct inode *inode = file_inode(file);
+	u64 containerid;
+	int rv;
+	struct task_struct *task = get_proc_task(inode);
+
+	if (!task)
+		return -ESRCH;
+	if (*ppos != 0) {
+		/* No partial writes. */
+		put_task_struct(task);
+		return -EINVAL;
+	}
+
+	rv = kstrtou64_from_user(buf, count, 10, &containerid);
+	if (rv < 0) {
+		put_task_struct(task);
+		return rv;
+	}
+
+	rv = audit_set_containerid(task, containerid);
+	put_task_struct(task);
+	if (rv < 0)
+		return rv;
+	return count;
+}
+
+static const struct file_operations proc_containerid_operations = {
+	.write		= proc_containerid_write,
+	.llseek		= generic_file_llseek,
+};
+
 #endif
 
 #ifdef CONFIG_FAULT_INJECTION
@@ -2961,6 +2996,7 @@ static int proc_pid_patch_state(struct seq_file *m, struct pid_namespace *ns,
 #ifdef CONFIG_AUDITSYSCALL
 	REG("loginuid",   S_IWUSR|S_IRUGO, proc_loginuid_operations),
 	REG("sessionid",  S_IRUGO, proc_sessionid_operations),
+	REG("containerid", S_IWUSR, proc_containerid_operations),
 #endif
 #ifdef CONFIG_FAULT_INJECTION
 	REG("make-it-fail", S_IRUGO|S_IWUSR, proc_fault_inject_operations),
@@ -3355,6 +3391,7 @@ static int proc_tid_comm_permission(struct inode *inode, int mask)
 #ifdef CONFIG_AUDITSYSCALL
 	REG("loginuid",  S_IWUSR|S_IRUGO, proc_loginuid_operations),
 	REG("sessionid",  S_IRUGO, proc_sessionid_operations),
+	REG("containerid", S_IWUSR, proc_containerid_operations),
 #endif
 #ifdef CONFIG_FAULT_INJECTION
 	REG("make-it-fail", S_IRUGO|S_IWUSR, proc_fault_inject_operations),
diff --git a/include/linux/audit.h b/include/linux/audit.h
index af410d9..fe4ba3f 100644
--- a/include/linux/audit.h
+++ b/include/linux/audit.h
@@ -29,6 +29,7 @@
 
 #define AUDIT_INO_UNSET ((unsigned long)-1)
 #define AUDIT_DEV_UNSET ((dev_t)-1)
+#define INVALID_CID AUDIT_CID_UNSET
 
 struct audit_sig_info {
 	uid_t		uid;
@@ -321,6 +322,7 @@ static inline void audit_ptrace(struct task_struct *t)
 extern int auditsc_get_stamp(struct audit_context *ctx,
 			      struct timespec64 *t, unsigned int *serial);
 extern int audit_set_loginuid(kuid_t loginuid);
+extern int audit_set_containerid(struct task_struct *tsk, u64 containerid);
 
 static inline kuid_t audit_get_loginuid(struct task_struct *tsk)
 {
@@ -332,6 +334,11 @@ static inline unsigned int audit_get_sessionid(struct task_struct *tsk)
 	return tsk->sessionid;
 }
 
+static inline u64 audit_get_containerid(struct task_struct *tsk)
+{
+	return tsk->containerid;
+}
+
 extern void __audit_ipc_obj(struct kern_ipc_perm *ipcp);
 extern void __audit_ipc_set_perm(unsigned long qbytes, uid_t uid, gid_t gid, umode_t mode);
 extern void __audit_bprm(struct linux_binprm *bprm);
@@ -517,6 +524,10 @@ static inline unsigned int audit_get_sessionid(struct task_struct *tsk)
 {
 	return -1;
 }
+static inline kuid_t audit_get_containerid(struct task_struct *tsk)
+{
+	return INVALID_CID;
+}
 static inline void audit_ipc_obj(struct kern_ipc_perm *ipcp)
 { }
 static inline void audit_ipc_set_perm(unsigned long qbytes, uid_t uid,
@@ -581,6 +592,11 @@ static inline bool audit_loginuid_set(struct task_struct *tsk)
 	return uid_valid(audit_get_loginuid(tsk));
 }
 
+static inline bool audit_containerid_set(struct task_struct *tsk)
+{
+	return audit_get_containerid(tsk) != INVALID_CID;
+}
+
 static inline void audit_log_string(struct audit_buffer *ab, const char *buf)
 {
 	audit_log_n_string(ab, buf, strlen(buf));
diff --git a/include/linux/init_task.h b/include/linux/init_task.h
index 6a53262..046bd0a 100644
--- a/include/linux/init_task.h
+++ b/include/linux/init_task.h
@@ -18,6 +18,7 @@
 #include <linux/sched/rt.h>
 #include <linux/livepatch.h>
 #include <linux/mm_types.h>
+#include <linux/audit.h>
 
 #include <asm/thread_info.h>
 
@@ -120,7 +121,8 @@
 #ifdef CONFIG_AUDITSYSCALL
 #define INIT_IDS \
 	.loginuid = INVALID_UID, \
-	.sessionid = (unsigned int)-1,
+	.sessionid = (unsigned int)-1, \
+	.containerid = INVALID_CID,
 #else
 #define INIT_IDS
 #endif
diff --git a/include/linux/sched.h b/include/linux/sched.h
index d258826..1b82191 100644
--- a/include/linux/sched.h
+++ b/include/linux/sched.h
@@ -796,6 +796,7 @@ struct task_struct {
 #ifdef CONFIG_AUDITSYSCALL
 	kuid_t				loginuid;
 	unsigned int			sessionid;
+	u64				containerid;
 #endif
 	struct seccomp			seccomp;
 
diff --git a/include/uapi/linux/audit.h b/include/uapi/linux/audit.h
index 4e61a9e..921a71f 100644
--- a/include/uapi/linux/audit.h
+++ b/include/uapi/linux/audit.h
@@ -71,6 +71,7 @@
 #define AUDIT_TTY_SET		1017	/* Set TTY auditing status */
 #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		1020	/* Define the container id and information */
 
 #define AUDIT_FIRST_USER_MSG	1100	/* Userspace messages mostly uninteresting to kernel */
 #define AUDIT_USER_AVC		1107	/* We filter this differently */
@@ -465,6 +466,7 @@ struct audit_tty_status {
 };
 
 #define AUDIT_UID_UNSET (unsigned int)-1
+#define AUDIT_CID_UNSET ((u64)-1)
 
 /* audit_rule_data supports filter rules with both integer and string
  * fields.  It corresponds with AUDIT_ADD_RULE, AUDIT_DEL_RULE and
diff --git a/kernel/auditsc.c b/kernel/auditsc.c
index 4e0a4ac..0ee1e59 100644
--- a/kernel/auditsc.c
+++ b/kernel/auditsc.c
@@ -2073,6 +2073,92 @@ int audit_set_loginuid(kuid_t loginuid)
 	return rc;
 }
 
+static int audit_set_containerid_perm(struct task_struct *task, u64 containerid)
+{
+	struct task_struct *parent;
+	u64 pcontainerid, ccontainerid;
+	pid_t ppid;
+
+	/* Don't allow to set our own containerid */
+	if (current == task)
+		return -EPERM;
+	/* Don't allow the containerid to be unset */
+	if (!cid_valid(containerid))
+		return -EINVAL;
+	/* if we don't have caps, reject */
+	if (!capable(CAP_AUDIT_CONTROL))
+		return -EPERM;
+	/* if containerid is unset, allow */
+	if (!audit_containerid_set(task))
+		return 0;
+	/* it is already set, and not inherited from the parent, reject */
+	ccontainerid = audit_get_containerid(task);
+	rcu_read_lock();
+	parent = rcu_dereference(task->real_parent);
+	rcu_read_unlock();
+	task_lock(parent);
+	pcontainerid = audit_get_containerid(parent);
+	ppid = task_tgid_nr(parent);
+	task_unlock(parent);
+	if (ccontainerid != pcontainerid)
+		return -EPERM;
+	return 0;
+}
+
+static void audit_log_set_containerid(struct task_struct *task, u64 oldcontainerid,
+				      u64 containerid, int rc)
+{
+	struct audit_buffer *ab;
+	uid_t uid;
+	struct tty_struct *tty;
+
+	if (!audit_enabled)
+		return;
+
+	ab = audit_log_start(NULL, GFP_KERNEL, AUDIT_CONTAINER);
+	if (!ab)
+		return;
+
+	uid = from_kuid(&init_user_ns, task_uid(current));
+	tty = audit_get_tty(current);
+
+	audit_log_format(ab, "op=set pid=%d uid=%u", task_tgid_nr(current), uid);
+	audit_log_task_context(ab);
+	audit_log_format(ab, " auid=%u tty=%s ses=%u opid=%d old-contid=%llu contid=%llu res=%d",
+			 from_kuid(&init_user_ns, audit_get_loginuid(current)),
+			 tty ? tty_name(tty) : "(none)", audit_get_sessionid(current),
+			 task_tgid_nr(task), oldcontainerid, containerid, !rc);
+
+	audit_put_tty(tty);
+	audit_log_end(ab);
+}
+
+/**
+ * audit_set_containerid - set current task's audit_context containerid
+ * @containerid: containerid value
+ *
+ * Returns 0 on success, -EPERM on permission failure.
+ *
+ * Called (set) from fs/proc/base.c::proc_containerid_write().
+ */
+int audit_set_containerid(struct task_struct *task, u64 containerid)
+{
+	u64 oldcontainerid;
+	int rc;
+
+	oldcontainerid = audit_get_containerid(task);
+
+	rc = audit_set_containerid_perm(task, containerid);
+	if (!rc) {
+		task_lock(task);
+		task->containerid = containerid;
+		task_unlock(task);
+	}
+
+	audit_log_set_containerid(task, oldcontainerid, containerid, rc);
+	return rc;
+}
+
 /**
  * __audit_mq_open - record audit data for a POSIX MQ open
  * @oflag: open flag
-- 
1.8.3.1

^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [RFC PATCH V1 02/12] audit: log container info of syscalls
  2018-03-01 19:41 [RFC PATCH V1 00/12] audit: implement container id Richard Guy Briggs
  2018-03-01 19:41 ` [RFC PATCH V1 01/12] audit: add " Richard Guy Briggs
@ 2018-03-01 19:41 ` Richard Guy Briggs
  2018-03-01 19:41 ` [RFC PATCH V1 03/12] audit: add containerid filtering Richard Guy Briggs
                   ` (11 subsequent siblings)
  13 siblings, 0 replies; 31+ messages in thread
From: Richard Guy Briggs @ 2018-03-01 19:41 UTC (permalink / raw)
  To: cgroups, containers, linux-api, Linux-Audit Mailing List,
	linux-fsdevel, LKML, netdev
  Cc: mszeredi, luto, jlayton, carlos, viro, dhowells, simo, trondmy,
	eparis, serge, ebiederm, madzcar, Richard Guy Briggs

Create a new audit record AUDIT_CONTAINER_INFO to document the container
ID of a process if it is present.

Called from audit_log_exit(), syscalls are covered.

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

See: https://github.com/linux-audit/audit-kernel/issues/32
Signed-off-by: Richard Guy Briggs <rgb@redhat.com>
---
 include/linux/audit.h      |  5 +++++
 include/uapi/linux/audit.h |  1 +
 kernel/audit.c             | 20 ++++++++++++++++++++
 kernel/auditsc.c           |  2 ++
 4 files changed, 28 insertions(+)

diff --git a/include/linux/audit.h b/include/linux/audit.h
index fe4ba3f..3acbe9d 100644
--- a/include/linux/audit.h
+++ b/include/linux/audit.h
@@ -154,6 +154,8 @@ extern void		    audit_log_link_denied(const char *operation,
 extern int audit_log_task_context(struct audit_buffer *ab);
 extern void audit_log_task_info(struct audit_buffer *ab,
 				struct task_struct *tsk);
+extern int audit_log_container_info(struct task_struct *tsk,
+				     struct audit_context *context);
 
 extern int		    audit_update_lsm_rules(void);
 
@@ -205,6 +207,9 @@ static inline int audit_log_task_context(struct audit_buffer *ab)
 static inline void audit_log_task_info(struct audit_buffer *ab,
 				       struct task_struct *tsk)
 { }
+static inline int audit_log_container_info(struct task_struct *tsk,
+					    struct audit_context *context);
+{ }
 #define audit_enabled 0
 #endif /* CONFIG_AUDIT */
 
diff --git a/include/uapi/linux/audit.h b/include/uapi/linux/audit.h
index 921a71f..e83ccbd 100644
--- a/include/uapi/linux/audit.h
+++ b/include/uapi/linux/audit.h
@@ -115,6 +115,7 @@
 #define AUDIT_REPLACE		1329	/* Replace auditd if this packet unanswerd */
 #define AUDIT_KERN_MODULE	1330	/* Kernel Module events */
 #define AUDIT_FANOTIFY		1331	/* Fanotify access decision */
+#define AUDIT_CONTAINER_INFO	1332	/* Container ID information */
 
 #define AUDIT_AVC		1400	/* SE Linux avc denial or grant */
 #define AUDIT_SELINUX_ERR	1401	/* Internal SE Linux Errors */
diff --git a/kernel/audit.c b/kernel/audit.c
index 5c25449..8dc745f 100644
--- a/kernel/audit.c
+++ b/kernel/audit.c
@@ -1996,6 +1996,26 @@ void audit_log_session_info(struct audit_buffer *ab)
 	audit_log_format(ab, " auid=%u ses=%u", auid, sessionid);
 }
 
+/*
+ * audit_log_container_info - report container info
+ * @tsk: task to be recorded
+ * @context: task or local context for record
+ */
+int audit_log_container_info(struct task_struct *tsk, struct audit_context *context)
+{
+	struct audit_buffer *ab;
+
+	if (!audit_containerid_set(tsk))
+		return 0;
+	/* Generate AUDIT_CONTAINER_INFO with container ID */
+	ab = audit_log_start(context, GFP_KERNEL, AUDIT_CONTAINER_INFO);
+	if (!ab)
+		return -ENOMEM;
+	audit_log_format(ab, "contid=%llu", audit_get_containerid(tsk));
+	audit_log_end(ab);
+	return 0;
+}
+
 void audit_log_key(struct audit_buffer *ab, char *key)
 {
 	audit_log_format(ab, " key=");
diff --git a/kernel/auditsc.c b/kernel/auditsc.c
index 0ee1e59..f015c25 100644
--- a/kernel/auditsc.c
+++ b/kernel/auditsc.c
@@ -1453,6 +1453,8 @@ static void audit_log_exit(struct audit_context *context, struct task_struct *ts
 
 	audit_log_proctitle(tsk, context);
 
+	audit_log_container_info(tsk, context);
+
 	/* Send end of event record to help user space know we are finished */
 	ab = audit_log_start(context, GFP_KERNEL, AUDIT_EOE);
 	if (ab)
-- 
1.8.3.1

^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [RFC PATCH V1 03/12] audit: add containerid filtering
  2018-03-01 19:41 [RFC PATCH V1 00/12] audit: implement container id Richard Guy Briggs
  2018-03-01 19:41 ` [RFC PATCH V1 01/12] audit: add " Richard Guy Briggs
  2018-03-01 19:41 ` [RFC PATCH V1 02/12] audit: log container info of syscalls Richard Guy Briggs
@ 2018-03-01 19:41 ` Richard Guy Briggs
  2018-03-01 19:41 ` [RFC PATCH V1 04/12] audit: read container ID of a process Richard Guy Briggs
                   ` (10 subsequent siblings)
  13 siblings, 0 replies; 31+ messages in thread
From: Richard Guy Briggs @ 2018-03-01 19:41 UTC (permalink / raw)
  To: cgroups, containers, linux-api, Linux-Audit Mailing List,
	linux-fsdevel, LKML, netdev
  Cc: mszeredi, luto, jlayton, carlos, viro, dhowells, simo, trondmy,
	eparis, serge, ebiederm, madzcar, Richard Guy Briggs

Implement container ID filtering using the AUDIT_CONTAINERID 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_FILTER.

This requires support from userspace to be useful.
See: https://github.com/linux-audit/audit-userspace/issues/40
Signed-off-by: Richard Guy Briggs <rgb@redhat.com>
---
 include/linux/audit.h      |  1 +
 include/uapi/linux/audit.h |  5 ++++-
 kernel/audit.h             |  1 +
 kernel/auditfilter.c       | 47 ++++++++++++++++++++++++++++++++++++++++++++++
 kernel/auditsc.c           |  3 +++
 5 files changed, 56 insertions(+), 1 deletion(-)

diff --git a/include/linux/audit.h b/include/linux/audit.h
index 3acbe9d..f10ca1b 100644
--- a/include/linux/audit.h
+++ b/include/linux/audit.h
@@ -76,6 +76,7 @@ struct audit_field {
 	u32				type;
 	union {
 		u32			val;
+		u64			val64;
 		kuid_t			uid;
 		kgid_t			gid;
 		struct {
diff --git a/include/uapi/linux/audit.h b/include/uapi/linux/audit.h
index e83ccbd..8443a8f 100644
--- a/include/uapi/linux/audit.h
+++ b/include/uapi/linux/audit.h
@@ -262,6 +262,7 @@
 #define AUDIT_LOGINUID_SET	24
 #define AUDIT_SESSIONID	25	/* Session ID */
 #define AUDIT_FSTYPE	26	/* FileSystem Type */
+#define AUDIT_CONTAINERID	27	/* Container ID */
 
 				/* These are ONLY useful when checking
 				 * at syscall exit time (AUDIT_AT_EXIT). */
@@ -342,6 +343,7 @@ enum {
 #define AUDIT_FEATURE_BITMAP_SESSIONID_FILTER	0x00000010
 #define AUDIT_FEATURE_BITMAP_LOST_RESET		0x00000020
 #define AUDIT_FEATURE_BITMAP_FILTER_FS		0x00000040
+#define AUDIT_FEATURE_BITMAP_CONTAINERID_FILTER	0x00000080
 
 #define AUDIT_FEATURE_BITMAP_ALL (AUDIT_FEATURE_BITMAP_BACKLOG_LIMIT | \
 				  AUDIT_FEATURE_BITMAP_BACKLOG_WAIT_TIME | \
@@ -349,7 +351,8 @@ enum {
 				  AUDIT_FEATURE_BITMAP_EXCLUDE_EXTEND | \
 				  AUDIT_FEATURE_BITMAP_SESSIONID_FILTER | \
 				  AUDIT_FEATURE_BITMAP_LOST_RESET | \
-				  AUDIT_FEATURE_BITMAP_FILTER_FS)
+				  AUDIT_FEATURE_BITMAP_FILTER_FS | \
+				  AUDIT_FEATURE_BITMAP_CONTAINERID_FILTER)
 
 /* deprecated: AUDIT_VERSION_* */
 #define AUDIT_VERSION_LATEST 		AUDIT_FEATURE_BITMAP_ALL
diff --git a/kernel/audit.h b/kernel/audit.h
index af5bc59..683316a 100644
--- a/kernel/audit.h
+++ b/kernel/audit.h
@@ -234,6 +234,7 @@ static inline int audit_hash_ino(u32 ino)
 
 extern int audit_match_class(int class, unsigned syscall);
 extern int audit_comparator(const u32 left, const u32 op, const u32 right);
+extern int audit_comparator64(const u64 left, const u32 op, const u64 right);
 extern int audit_uid_comparator(kuid_t left, u32 op, kuid_t right);
 extern int audit_gid_comparator(kgid_t left, u32 op, kgid_t right);
 extern int parent_len(const char *path);
diff --git a/kernel/auditfilter.c b/kernel/auditfilter.c
index d7a807e..c4c8746 100644
--- a/kernel/auditfilter.c
+++ b/kernel/auditfilter.c
@@ -410,6 +410,7 @@ static int audit_field_valid(struct audit_entry *entry, struct audit_field *f)
 	/* FALL THROUGH */
 	case AUDIT_ARCH:
 	case AUDIT_FSTYPE:
+	case AUDIT_CONTAINERID:
 		if (f->op != Audit_not_equal && f->op != Audit_equal)
 			return -EINVAL;
 		break;
@@ -584,6 +585,14 @@ static struct audit_entry *audit_data_to_entry(struct audit_rule_data *data,
 			}
 			entry->rule.exe = audit_mark;
 			break;
+		case AUDIT_CONTAINERID:
+			if (f->val != sizeof(u64))
+				goto exit_free;
+			str = audit_unpack_string(&bufp, &remain, f->val);
+			if (IS_ERR(str))
+				goto exit_free;
+			f->val64 = ((u64 *)str)[0];
+			break;
 		}
 	}
 
@@ -666,6 +675,11 @@ static struct audit_rule_data *audit_krule_to_data(struct audit_krule *krule)
 			data->buflen += data->values[i] =
 				audit_pack_string(&bufp, audit_mark_path(krule->exe));
 			break;
+		case AUDIT_CONTAINERID:
+			data->buflen += data->values[i] = sizeof(u64);
+			for (i = 0; i < sizeof(u64); i++)
+				((char *)bufp)[i] = ((char *)&f->val64)[i];
+			break;
 		case AUDIT_LOGINUID_SET:
 			if (krule->pflags & AUDIT_LOGINUID_LEGACY && !f->val) {
 				data->fields[i] = AUDIT_LOGINUID;
@@ -752,6 +766,10 @@ static int audit_compare_rule(struct audit_krule *a, struct audit_krule *b)
 			if (!gid_eq(a->fields[i].gid, b->fields[i].gid))
 				return 1;
 			break;
+		case AUDIT_CONTAINERID:
+			if (a->fields[i].val64 != b->fields[i].val64)
+				return 1;
+			break;
 		default:
 			if (a->fields[i].val != b->fields[i].val)
 				return 1;
@@ -1210,6 +1228,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);
+	case Audit_gt:
+		return (left > right);
+	case Audit_ge:
+		return (left >= right);
+	case Audit_bitmask:
+		return (left & right);
+	case Audit_bittest:
+		return ((left & right) == right);
+	default:
+		BUG();
+		return 0;
+	}
+}
+
 int audit_uid_comparator(kuid_t left, u32 op, kuid_t right)
 {
 	switch (op) {
@@ -1348,6 +1391,10 @@ int audit_filter(int msgtype, unsigned int listtype)
 				result = audit_comparator(audit_loginuid_set(current),
 							  f->op, f->val);
 				break;
+			case AUDIT_CONTAINERID:
+				result = audit_comparator64(audit_get_containerid(current),
+							      f->op, f->val64);
+				break;
 			case AUDIT_MSGTYPE:
 				result = audit_comparator(msgtype, f->op, f->val);
 				break;
diff --git a/kernel/auditsc.c b/kernel/auditsc.c
index f015c25..34396cb 100644
--- a/kernel/auditsc.c
+++ b/kernel/auditsc.c
@@ -614,6 +614,9 @@ static int audit_filter_rules(struct task_struct *tsk,
 		case AUDIT_LOGINUID_SET:
 			result = audit_comparator(audit_loginuid_set(tsk), f->op, f->val);
 			break;
+		case AUDIT_CONTAINERID:
+			result = audit_comparator64(audit_get_containerid(tsk), f->op, f->val64);
+			break;
 		case AUDIT_SUBJ_USER:
 		case AUDIT_SUBJ_ROLE:
 		case AUDIT_SUBJ_TYPE:
-- 
1.8.3.1

^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [RFC PATCH V1 04/12] audit: read container ID of a process
  2018-03-01 19:41 [RFC PATCH V1 00/12] audit: implement container id Richard Guy Briggs
                   ` (2 preceding siblings ...)
  2018-03-01 19:41 ` [RFC PATCH V1 03/12] audit: add containerid filtering Richard Guy Briggs
@ 2018-03-01 19:41 ` Richard Guy Briggs
  2018-03-01 19:41 ` [RFC PATCH V1 05/12] audit: add containerid support for ptrace and signals Richard Guy Briggs
                   ` (9 subsequent siblings)
  13 siblings, 0 replies; 31+ messages in thread
From: Richard Guy Briggs @ 2018-03-01 19:41 UTC (permalink / raw)
  To: cgroups, containers, linux-api, Linux-Audit Mailing List,
	linux-fsdevel, LKML, netdev
  Cc: mszeredi, luto, jlayton, carlos, viro, dhowells, simo, trondmy,
	eparis, serge, ebiederm, madzcar, Richard Guy Briggs

Add support for reading the container ID from the proc filesystem.

This is a read from the proc entry of the form /proc/PID/containerid
where PID is the process ID of the task whose container ID is sought.

The read expects up to a u64 value (unset: 18446744073709551615).

Signed-off-by: Richard Guy Briggs <rgb@redhat.com>
---
 fs/proc/base.c | 20 ++++++++++++++++++--
 1 file changed, 18 insertions(+), 2 deletions(-)

diff --git a/fs/proc/base.c b/fs/proc/base.c
index 6ce4fbe..f66d1e2 100644
--- a/fs/proc/base.c
+++ b/fs/proc/base.c
@@ -1300,6 +1300,21 @@ static ssize_t proc_sessionid_read(struct file * file, char __user * buf,
 	.llseek		= generic_file_llseek,
 };
 
+static ssize_t proc_containerid_read(struct file *file, char __user *buf,
+				  size_t count, loff_t *ppos)
+{
+	struct inode *inode = file_inode(file);
+	struct task_struct *task = get_proc_task(inode);
+	ssize_t length;
+	char tmpbuf[TMPBUFLEN*2];
+
+	if (!task)
+		return -ESRCH;
+	length = scnprintf(tmpbuf, TMPBUFLEN*2, "%llu", audit_get_containerid(task));
+	put_task_struct(task);
+	return simple_read_from_buffer(buf, count, ppos, tmpbuf, length);
+}
+
 static ssize_t proc_containerid_write(struct file *file, const char __user *buf,
 				   size_t count, loff_t *ppos)
 {
@@ -1330,6 +1345,7 @@ static ssize_t proc_containerid_write(struct file *file, const char __user *buf,
 }
 
 static const struct file_operations proc_containerid_operations = {
+	.read		= proc_containerid_read,
 	.write		= proc_containerid_write,
 	.llseek		= generic_file_llseek,
 };
@@ -2996,7 +3012,7 @@ static int proc_pid_patch_state(struct seq_file *m, struct pid_namespace *ns,
 #ifdef CONFIG_AUDITSYSCALL
 	REG("loginuid",   S_IWUSR|S_IRUGO, proc_loginuid_operations),
 	REG("sessionid",  S_IRUGO, proc_sessionid_operations),
-	REG("containerid", S_IWUSR, proc_containerid_operations),
+	REG("containerid", S_IWUSR|S_IRUSR, proc_containerid_operations),
 #endif
 #ifdef CONFIG_FAULT_INJECTION
 	REG("make-it-fail", S_IRUGO|S_IWUSR, proc_fault_inject_operations),
@@ -3391,7 +3407,7 @@ static int proc_tid_comm_permission(struct inode *inode, int mask)
 #ifdef CONFIG_AUDITSYSCALL
 	REG("loginuid",  S_IWUSR|S_IRUGO, proc_loginuid_operations),
 	REG("sessionid",  S_IRUGO, proc_sessionid_operations),
-	REG("containerid", S_IWUSR, proc_containerid_operations),
+	REG("containerid", S_IWUSR|S_IRUSR, proc_containerid_operations),
 #endif
 #ifdef CONFIG_FAULT_INJECTION
 	REG("make-it-fail", S_IRUGO|S_IWUSR, proc_fault_inject_operations),
-- 
1.8.3.1

^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [RFC PATCH V1 05/12] audit: add containerid support for ptrace and signals
  2018-03-01 19:41 [RFC PATCH V1 00/12] audit: implement container id Richard Guy Briggs
                   ` (3 preceding siblings ...)
  2018-03-01 19:41 ` [RFC PATCH V1 04/12] audit: read container ID of a process Richard Guy Briggs
@ 2018-03-01 19:41 ` Richard Guy Briggs
  2018-03-01 19:41 ` [RFC PATCH V1 06/12] audit: add support for non-syscall auxiliary records Richard Guy Briggs
                   ` (8 subsequent siblings)
  13 siblings, 0 replies; 31+ messages in thread
From: Richard Guy Briggs @ 2018-03-01 19:41 UTC (permalink / raw)
  To: cgroups, containers, linux-api, Linux-Audit Mailing List,
	linux-fsdevel, LKML, netdev
  Cc: mszeredi, luto, jlayton, carlos, viro, dhowells, simo, trondmy,
	eparis, serge, ebiederm, madzcar, Richard Guy Briggs

Add container ID support to ptrace and signals.  In particular, the "op"
field provides a way to label the auxiliary record to which it is
associated.

Signed-off-by: Richard Guy Briggs <rgb@redhat.com>
---
 include/linux/audit.h | 16 +++++++++++-----
 kernel/audit.c        | 12 ++++++++----
 kernel/audit.h        |  2 ++
 kernel/auditsc.c      | 19 +++++++++++++++----
 4 files changed, 36 insertions(+), 13 deletions(-)

diff --git a/include/linux/audit.h b/include/linux/audit.h
index f10ca1b..ed16bb6 100644
--- a/include/linux/audit.h
+++ b/include/linux/audit.h
@@ -35,6 +35,7 @@ struct audit_sig_info {
 	uid_t		uid;
 	pid_t		pid;
 	char		ctx[0];
+	u64		cid;
 };
 
 struct audit_buffer;
@@ -155,8 +156,8 @@ extern void		    audit_log_link_denied(const char *operation,
 extern int audit_log_task_context(struct audit_buffer *ab);
 extern void audit_log_task_info(struct audit_buffer *ab,
 				struct task_struct *tsk);
-extern int audit_log_container_info(struct task_struct *tsk,
-				     struct audit_context *context);
+extern int audit_log_container_info(struct audit_context *context,
+				     char *op, u64 containerid);
 
 extern int		    audit_update_lsm_rules(void);
 
@@ -208,8 +209,8 @@ static inline int audit_log_task_context(struct audit_buffer *ab)
 static inline void audit_log_task_info(struct audit_buffer *ab,
 				       struct task_struct *tsk)
 { }
-static inline int audit_log_container_info(struct task_struct *tsk,
-					    struct audit_context *context);
+static inline int audit_log_container_info(struct audit_context *context,
+					    char *op, u64 containerid);
 { }
 #define audit_enabled 0
 #endif /* CONFIG_AUDIT */
@@ -598,9 +599,14 @@ static inline bool audit_loginuid_set(struct task_struct *tsk)
 	return uid_valid(audit_get_loginuid(tsk));
 }
 
+static inline bool cid_valid(u64 containerid)
+{
+	return containerid != INVALID_CID;
+}
+
 static inline bool audit_containerid_set(struct task_struct *tsk)
 {
-	return audit_get_containerid(tsk) != INVALID_CID;
+	return cid_valid(audit_get_containerid(tsk));
 }
 
 static inline void audit_log_string(struct audit_buffer *ab, const char *buf)
diff --git a/kernel/audit.c b/kernel/audit.c
index 8dc745f..f5cd0bc 100644
--- a/kernel/audit.c
+++ b/kernel/audit.c
@@ -142,6 +142,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 = INVALID_CID;
 
 /* Records can be lost in several ways:
    0) [suppressed in audit_alloc]
@@ -1394,6 +1395,7 @@ static int audit_receive_msg(struct sk_buff *skb, struct nlmsghdr *nlh)
 			memcpy(sig_data->ctx, ctx, len);
 			security_release_secctx(ctx, len);
 		}
+		sig_data->cid = audit_sig_cid;
 		audit_send_reply(skb, seq, AUDIT_SIGNAL_INFO, 0, 0,
 				 sig_data, sizeof(*sig_data) + len);
 		kfree(sig_data);
@@ -1998,20 +2000,22 @@ void audit_log_session_info(struct audit_buffer *ab)
 
 /*
  * audit_log_container_info - report container info
- * @tsk: task to be recorded
  * @context: task or local context for record
+ * @op: containerid string description
+ * @containerid: container ID to report
  */
-int audit_log_container_info(struct task_struct *tsk, struct audit_context *context)
+int audit_log_container_info(struct audit_context *context,
+			      char *op, u64 containerid)
 {
 	struct audit_buffer *ab;
 
-	if (!audit_containerid_set(tsk))
+	if (!cid_valid(containerid))
 		return 0;
 	/* Generate AUDIT_CONTAINER_INFO with container ID */
 	ab = audit_log_start(context, GFP_KERNEL, AUDIT_CONTAINER_INFO);
 	if (!ab)
 		return -ENOMEM;
-	audit_log_format(ab, "contid=%llu", audit_get_containerid(tsk));
+	audit_log_format(ab, "op=%s contid=%llu", op, containerid);
 	audit_log_end(ab);
 	return 0;
 }
diff --git a/kernel/audit.h b/kernel/audit.h
index 683316a..abbabba 100644
--- a/kernel/audit.h
+++ b/kernel/audit.h
@@ -147,6 +147,7 @@ struct audit_context {
 	kuid_t		    target_uid;
 	unsigned int	    target_sessionid;
 	u32		    target_sid;
+	u64		    target_cid;
 	char		    target_comm[TASK_COMM_LEN];
 
 	struct audit_tree_refs *trees, *first_trees;
@@ -330,6 +331,7 @@ extern void audit_log_d_path_exe(struct audit_buffer *ab,
 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);
 
diff --git a/kernel/auditsc.c b/kernel/auditsc.c
index 34396cb..0e41884 100644
--- a/kernel/auditsc.c
+++ b/kernel/auditsc.c
@@ -113,6 +113,7 @@ struct audit_aux_data_pids {
 	kuid_t			target_uid[AUDIT_AUX_PIDS];
 	unsigned int		target_sessionid[AUDIT_AUX_PIDS];
 	u32			target_sid[AUDIT_AUX_PIDS];
+	u64			target_cid[AUDIT_AUX_PIDS];
 	char 			target_comm[AUDIT_AUX_PIDS][TASK_COMM_LEN];
 	int			pid_count;
 };
@@ -1422,21 +1423,27 @@ static void audit_log_exit(struct audit_context *context, struct task_struct *ts
 	for (aux = context->aux_pids; aux; aux = aux->next) {
 		struct audit_aux_data_pids *axs = (void *)aux;
 
-		for (i = 0; i < axs->pid_count; i++)
+		for (i = 0; i < axs->pid_count; i++) {
+			char axsn[sizeof("aux0xN ")];
+
+			sprintf(axsn, "aux0x%x", i);
 			if (audit_log_pid_context(context, axs->target_pid[i],
 						  axs->target_auid[i],
 						  axs->target_uid[i],
 						  axs->target_sessionid[i],
 						  axs->target_sid[i],
-						  axs->target_comm[i]))
+						  axs->target_comm[i])
+			    && audit_log_container_info(context, axsn, axs->target_cid[i]))
 				call_panic = 1;
+		}
 	}
 
 	if (context->target_pid &&
 	    audit_log_pid_context(context, context->target_pid,
 				  context->target_auid, context->target_uid,
 				  context->target_sessionid,
-				  context->target_sid, context->target_comm))
+				  context->target_sid, context->target_comm)
+	    && audit_log_container_info(context, "target", context->target_cid))
 			call_panic = 1;
 
 	if (context->pwd.dentry && context->pwd.mnt) {
@@ -1456,7 +1463,7 @@ static void audit_log_exit(struct audit_context *context, struct task_struct *ts
 
 	audit_log_proctitle(tsk, context);
 
-	audit_log_container_info(tsk, context);
+	audit_log_container_info(context, "task", audit_get_containerid(tsk));
 
 	/* Send end of event record to help user space know we are finished */
 	ab = audit_log_start(context, GFP_KERNEL, AUDIT_EOE);
@@ -2354,6 +2361,7 @@ void __audit_ptrace(struct task_struct *t)
 	context->target_uid = task_uid(t);
 	context->target_sessionid = audit_get_sessionid(t);
 	security_task_getsecid(t, &context->target_sid);
+	context->target_cid = audit_get_containerid(t);
 	memcpy(context->target_comm, t->comm, TASK_COMM_LEN);
 }
 
@@ -2381,6 +2389,7 @@ int audit_signal_info(int sig, struct task_struct *t)
 		else
 			audit_sig_uid = uid;
 		security_task_getsecid(tsk, &audit_sig_sid);
+		audit_sig_cid = audit_get_containerid(tsk);
 	}
 
 	if (!audit_signals || audit_dummy_context())
@@ -2394,6 +2403,7 @@ int audit_signal_info(int sig, struct task_struct *t)
 		ctx->target_uid = t_uid;
 		ctx->target_sessionid = audit_get_sessionid(t);
 		security_task_getsecid(t, &ctx->target_sid);
+		ctx->target_cid = audit_get_containerid(t);
 		memcpy(ctx->target_comm, t->comm, TASK_COMM_LEN);
 		return 0;
 	}
@@ -2415,6 +2425,7 @@ int audit_signal_info(int sig, struct task_struct *t)
 	axp->target_uid[axp->pid_count] = t_uid;
 	axp->target_sessionid[axp->pid_count] = audit_get_sessionid(t);
 	security_task_getsecid(t, &axp->target_sid[axp->pid_count]);
+	axp->target_cid[axp->pid_count] = audit_get_containerid(t);
 	memcpy(axp->target_comm[axp->pid_count], t->comm, TASK_COMM_LEN);
 	axp->pid_count++;
 
-- 
1.8.3.1

^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [RFC PATCH V1 06/12] audit: add support for non-syscall auxiliary records
  2018-03-01 19:41 [RFC PATCH V1 00/12] audit: implement container id Richard Guy Briggs
                   ` (4 preceding siblings ...)
  2018-03-01 19:41 ` [RFC PATCH V1 05/12] audit: add containerid support for ptrace and signals Richard Guy Briggs
@ 2018-03-01 19:41 ` Richard Guy Briggs
  2018-03-01 19:41 ` [RFC PATCH V1 07/12] audit: add container aux record to watch/tree/mark Richard Guy Briggs
                   ` (7 subsequent siblings)
  13 siblings, 0 replies; 31+ messages in thread
From: Richard Guy Briggs @ 2018-03-01 19:41 UTC (permalink / raw)
  To: cgroups, containers, linux-api, Linux-Audit Mailing List,
	linux-fsdevel, LKML, netdev
  Cc: mszeredi, luto, jlayton, carlos, viro, dhowells, simo, trondmy,
	eparis, serge, ebiederm, madzcar, Richard Guy Briggs

Standalone audit records have the timestamp and serial number generated
on the fly and as such are unique, making them standalone.  This is a
prototype of a method to generate a local audit context that will be
used only for a standalone record and its auxiliary record.  The context
is discarded immediately after the local associated records are
produced.

Signed-off-by: Richard Guy Briggs <rgb@redhat.com>
---
 include/linux/audit.h |  8 ++++++++
 kernel/auditsc.c      | 20 +++++++++++++++++++-
 2 files changed, 27 insertions(+), 1 deletion(-)

diff --git a/include/linux/audit.h b/include/linux/audit.h
index ed16bb6..c0b83cb 100644
--- a/include/linux/audit.h
+++ b/include/linux/audit.h
@@ -227,7 +227,9 @@ static inline int audit_log_container_info(struct audit_context *context,
 /* These are defined in auditsc.c */
 				/* Public API */
 extern int  audit_alloc(struct task_struct *task);
+extern struct audit_context *audit_alloc_local(void);
 extern void __audit_free(struct task_struct *task);
+extern void audit_free_context(struct audit_context *context);
 extern void __audit_syscall_entry(int major, unsigned long a0, unsigned long a1,
 				  unsigned long a2, unsigned long a3);
 extern void __audit_syscall_exit(int ret_success, long ret_value);
@@ -472,6 +474,12 @@ static inline int audit_alloc(struct task_struct *task)
 {
 	return 0;
 }
+static inline struct audit_context *audit_alloc_local(void)
+{
+	return NULL;
+}
+static inline void audit_free_context(struct audit_context *context)
+{ }
 static inline void audit_free(struct task_struct *task)
 { }
 static inline void audit_syscall_entry(int major, unsigned long a0,
diff --git a/kernel/auditsc.c b/kernel/auditsc.c
index 0e41884..0cbd762 100644
--- a/kernel/auditsc.c
+++ b/kernel/auditsc.c
@@ -959,8 +959,26 @@ int audit_alloc(struct task_struct *tsk)
 	return 0;
 }
 
-static inline void audit_free_context(struct audit_context *context)
+struct audit_context *audit_alloc_local(void)
 {
+	struct audit_context *context;
+
+	if (likely(!audit_ever_enabled))
+		return NULL; /* Return if not auditing. */
+
+	context = audit_alloc_context(AUDIT_RECORD_CONTEXT);
+	if (!context)
+		return NULL;
+	context->serial = audit_serial();
+	context->ctime = current_kernel_time64();
+	context->in_syscall = 1;
+	return context;
+}
+
+inline void audit_free_context(struct audit_context *context)
+{
+	if (!context)
+		return;
 	audit_free_names(context);
 	unroll_tree_refs(context, NULL, 0);
 	free_tree_refs(context);
-- 
1.8.3.1

^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [RFC PATCH V1 07/12] audit: add container aux record to watch/tree/mark
  2018-03-01 19:41 [RFC PATCH V1 00/12] audit: implement container id Richard Guy Briggs
                   ` (5 preceding siblings ...)
  2018-03-01 19:41 ` [RFC PATCH V1 06/12] audit: add support for non-syscall auxiliary records Richard Guy Briggs
@ 2018-03-01 19:41 ` Richard Guy Briggs
  2018-03-01 19:41 ` [RFC PATCH V1 08/12] audit: add containerid support for tty_audit Richard Guy Briggs
                   ` (6 subsequent siblings)
  13 siblings, 0 replies; 31+ messages in thread
From: Richard Guy Briggs @ 2018-03-01 19:41 UTC (permalink / raw)
  To: cgroups, containers, linux-api, Linux-Audit Mailing List,
	linux-fsdevel, LKML, netdev
  Cc: mszeredi, luto, jlayton, carlos, viro, dhowells, simo, trondmy,
	eparis, serge, ebiederm, madzcar, Richard Guy Briggs

Add container ID information to mark, watch and tree rule standalone
records.

Signed-off-by: Richard Guy Briggs <rgb@redhat.com>
---
 kernel/audit_fsnotify.c |  5 ++++-
 kernel/audit_tree.c     |  5 ++++-
 kernel/audit_watch.c    | 33 +++++++++++++++++++--------------
 3 files changed, 27 insertions(+), 16 deletions(-)

diff --git a/kernel/audit_fsnotify.c b/kernel/audit_fsnotify.c
index 52f368b..18c110d 100644
--- a/kernel/audit_fsnotify.c
+++ b/kernel/audit_fsnotify.c
@@ -124,10 +124,11 @@ static void audit_mark_log_rule_change(struct audit_fsnotify_mark *audit_mark, c
 {
 	struct audit_buffer *ab;
 	struct audit_krule *rule = audit_mark->rule;
+	struct audit_context *context = audit_alloc_local();
 
 	if (!audit_enabled)
 		return;
-	ab = audit_log_start(NULL, GFP_NOFS, AUDIT_CONFIG_CHANGE);
+	ab = audit_log_start(context, GFP_NOFS, AUDIT_CONFIG_CHANGE);
 	if (unlikely(!ab))
 		return;
 	audit_log_format(ab, "auid=%u ses=%u op=%s",
@@ -138,6 +139,8 @@ static void audit_mark_log_rule_change(struct audit_fsnotify_mark *audit_mark, c
 	audit_log_key(ab, rule->filterkey);
 	audit_log_format(ab, " list=%d res=1", rule->listnr);
 	audit_log_end(ab);
+	audit_log_container_info(context, "config", audit_get_containerid(current));
+	audit_free_context(context);
 }
 
 void audit_remove_mark(struct audit_fsnotify_mark *audit_mark)
diff --git a/kernel/audit_tree.c b/kernel/audit_tree.c
index fd35312..2ad85d4 100644
--- a/kernel/audit_tree.c
+++ b/kernel/audit_tree.c
@@ -496,8 +496,9 @@ static int tag_chunk(struct inode *inode, struct audit_tree *tree)
 static void audit_tree_log_remove_rule(struct audit_krule *rule)
 {
 	struct audit_buffer *ab;
+	struct audit_context *context = audit_alloc_local();
 
-	ab = audit_log_start(NULL, GFP_KERNEL, AUDIT_CONFIG_CHANGE);
+	ab = audit_log_start(context, GFP_KERNEL, AUDIT_CONFIG_CHANGE);
 	if (unlikely(!ab))
 		return;
 	audit_log_format(ab, "op=remove_rule");
@@ -506,6 +507,8 @@ static void audit_tree_log_remove_rule(struct audit_krule *rule)
 	audit_log_key(ab, rule->filterkey);
 	audit_log_format(ab, " list=%d res=1", rule->listnr);
 	audit_log_end(ab);
+	audit_log_container_info(context, "config", audit_get_containerid(current));
+	audit_free_context(context);
 }
 
 static void kill_rules(struct audit_tree *tree)
diff --git a/kernel/audit_watch.c b/kernel/audit_watch.c
index 9eb8b35..60d75a2 100644
--- a/kernel/audit_watch.c
+++ b/kernel/audit_watch.c
@@ -238,20 +238,25 @@ static struct audit_watch *audit_dupe_watch(struct audit_watch *old)
 
 static void audit_watch_log_rule_change(struct audit_krule *r, struct audit_watch *w, char *op)
 {
-	if (audit_enabled) {
-		struct audit_buffer *ab;
-		ab = audit_log_start(NULL, GFP_NOFS, AUDIT_CONFIG_CHANGE);
-		if (unlikely(!ab))
-			return;
-		audit_log_format(ab, "auid=%u ses=%u op=%s",
-				 from_kuid(&init_user_ns, audit_get_loginuid(current)),
-				 audit_get_sessionid(current), op);
-		audit_log_format(ab, " path=");
-		audit_log_untrustedstring(ab, w->path);
-		audit_log_key(ab, r->filterkey);
-		audit_log_format(ab, " list=%d res=1", r->listnr);
-		audit_log_end(ab);
-	}
+	struct audit_buffer *ab;
+	struct audit_context *context = audit_alloc_local();
+
+	if (!audit_enabled)
+		return;
+
+	ab = audit_log_start(context, GFP_NOFS, AUDIT_CONFIG_CHANGE);
+	if (unlikely(!ab))
+		return;
+	audit_log_format(ab, "auid=%u ses=%u op=%s",
+			 from_kuid(&init_user_ns, audit_get_loginuid(current)),
+			 audit_get_sessionid(current), op);
+	audit_log_format(ab, " path=");
+	audit_log_untrustedstring(ab, w->path);
+	audit_log_key(ab, r->filterkey);
+	audit_log_format(ab, " list=%d res=1", r->listnr);
+	audit_log_end(ab);
+	audit_log_container_info(context, "config", audit_get_containerid(current));
+	audit_free_context(context);
 }
 
 /* Update inode info in audit rules based on filesystem event. */
-- 
1.8.3.1

^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [RFC PATCH V1 08/12] audit: add containerid support for tty_audit
  2018-03-01 19:41 [RFC PATCH V1 00/12] audit: implement container id Richard Guy Briggs
                   ` (6 preceding siblings ...)
  2018-03-01 19:41 ` [RFC PATCH V1 07/12] audit: add container aux record to watch/tree/mark Richard Guy Briggs
@ 2018-03-01 19:41 ` Richard Guy Briggs
  2018-03-01 19:41 ` [RFC PATCH V1 09/12] audit: add containerid support for config/feature/user records Richard Guy Briggs
                   ` (5 subsequent siblings)
  13 siblings, 0 replies; 31+ messages in thread
From: Richard Guy Briggs @ 2018-03-01 19:41 UTC (permalink / raw)
  To: cgroups, containers, linux-api, Linux-Audit Mailing List,
	linux-fsdevel, LKML, netdev
  Cc: mszeredi, luto, jlayton, carlos, viro, dhowells, simo, trondmy,
	eparis, serge, ebiederm, madzcar, Richard Guy Briggs

Add container ID information to tty logging rule standalone records.

Signed-off-by: Richard Guy Briggs <rgb@redhat.com>
---
 drivers/tty/tty_audit.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/tty/tty_audit.c b/drivers/tty/tty_audit.c
index e30aa6b..48ee4b7 100644
--- a/drivers/tty/tty_audit.c
+++ b/drivers/tty/tty_audit.c
@@ -66,8 +66,9 @@ static void tty_audit_log(const char *description, dev_t dev,
 	uid_t uid = from_kuid(&init_user_ns, task_uid(tsk));
 	uid_t loginuid = from_kuid(&init_user_ns, audit_get_loginuid(tsk));
 	unsigned int sessionid = audit_get_sessionid(tsk);
+	struct audit_context *context = audit_alloc_local();
 
-	ab = audit_log_start(NULL, GFP_KERNEL, AUDIT_TTY);
+	ab = audit_log_start(context, GFP_KERNEL, AUDIT_TTY);
 	if (ab) {
 		char name[sizeof(tsk->comm)];
 
@@ -80,6 +81,8 @@ static void tty_audit_log(const char *description, dev_t dev,
 		audit_log_n_hex(ab, data, size);
 		audit_log_end(ab);
 	}
+	audit_log_container_info(context, "tty", audit_get_containerid(tsk));
+	audit_free_context(context);
 }
 
 /**
-- 
1.8.3.1

^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [RFC PATCH V1 09/12] audit: add containerid support for config/feature/user records
  2018-03-01 19:41 [RFC PATCH V1 00/12] audit: implement container id Richard Guy Briggs
                   ` (7 preceding siblings ...)
  2018-03-01 19:41 ` [RFC PATCH V1 08/12] audit: add containerid support for tty_audit Richard Guy Briggs
@ 2018-03-01 19:41 ` Richard Guy Briggs
  2018-03-01 19:41 ` [RFC PATCH V1 10/12] audit: add containerid support for seccomp and anom_abend records Richard Guy Briggs
                   ` (4 subsequent siblings)
  13 siblings, 0 replies; 31+ messages in thread
From: Richard Guy Briggs @ 2018-03-01 19:41 UTC (permalink / raw)
  To: cgroups, containers, linux-api, Linux-Audit Mailing List,
	linux-fsdevel, LKML, netdev
  Cc: mszeredi, luto, jlayton, carlos, viro, dhowells, simo, trondmy,
	eparis, serge, ebiederm, madzcar, Richard Guy Briggs

Add container ID information to configuration change, feature set change
and user generated standalone records.

Signed-off-by: Richard Guy Briggs <rgb@redhat.com>
---
 kernel/audit.c       | 50 ++++++++++++++++++++++++++++++++++++++++----------
 kernel/auditfilter.c |  5 ++++-
 2 files changed, 44 insertions(+), 11 deletions(-)

diff --git a/kernel/audit.c b/kernel/audit.c
index f5cd0bc..a4c0bad 100644
--- a/kernel/audit.c
+++ b/kernel/audit.c
@@ -358,8 +358,9 @@ static int audit_log_config_change(char *function_name, u32 new, u32 old,
 {
 	struct audit_buffer *ab;
 	int rc = 0;
+	struct audit_context *context = audit_alloc_local();
 
-	ab = audit_log_start(NULL, GFP_KERNEL, AUDIT_CONFIG_CHANGE);
+	ab = audit_log_start(context, GFP_KERNEL, AUDIT_CONFIG_CHANGE);
 	if (unlikely(!ab))
 		return rc;
 	audit_log_format(ab, "%s=%u old=%u", function_name, new, old);
@@ -369,6 +370,8 @@ static int audit_log_config_change(char *function_name, u32 new, u32 old,
 		allow_changes = 0; /* Something weird, deny request */
 	audit_log_format(ab, " res=%d", allow_changes);
 	audit_log_end(ab);
+	audit_log_container_info(context, "config", audit_get_containerid(current));
+	audit_free_context(context);
 	return rc;
 }
 
@@ -1016,7 +1019,8 @@ static int audit_netlink_ok(struct sk_buff *skb, u16 msg_type)
 	return err;
 }
 
-static void audit_log_common_recv_msg(struct audit_buffer **ab, u16 msg_type)
+static void audit_log_common_recv_msg(struct audit_context *context,
+				      struct audit_buffer **ab, u16 msg_type)
 {
 	uid_t uid = from_kuid(&init_user_ns, current_uid());
 	pid_t pid = task_tgid_nr(current);
@@ -1026,7 +1030,7 @@ static void audit_log_common_recv_msg(struct audit_buffer **ab, u16 msg_type)
 		return;
 	}
 
-	*ab = audit_log_start(NULL, GFP_KERNEL, msg_type);
+	*ab = audit_log_start(context, GFP_KERNEL, msg_type);
 	if (unlikely(!*ab))
 		return;
 	audit_log_format(*ab, "pid=%d uid=%u", pid, uid);
@@ -1055,16 +1059,19 @@ static void audit_log_feature_change(int which, u32 old_feature, u32 new_feature
 				     u32 old_lock, u32 new_lock, int res)
 {
 	struct audit_buffer *ab;
+	struct audit_context *context = audit_alloc_local();
 
 	if (audit_enabled == AUDIT_OFF)
 		return;
 
-	ab = audit_log_start(NULL, GFP_KERNEL, AUDIT_FEATURE_CHANGE);
+	ab = audit_log_start(context, GFP_KERNEL, AUDIT_FEATURE_CHANGE);
 	audit_log_task_info(ab, current);
 	audit_log_format(ab, " feature=%s old=%u new=%u old_lock=%u new_lock=%u res=%d",
 			 audit_feature_names[which], !!old_feature, !!new_feature,
 			 !!old_lock, !!new_lock, res);
 	audit_log_end(ab);
+	audit_log_container_info(context, "feature", audit_get_containerid(current));
+	audit_free_context(context);
 }
 
 static int audit_set_feature(struct sk_buff *skb)
@@ -1293,13 +1300,15 @@ static int audit_receive_msg(struct sk_buff *skb, struct nlmsghdr *nlh)
 
 		err = audit_filter(msg_type, AUDIT_FILTER_USER);
 		if (err == 1) { /* match or error */
+			struct audit_context *context = audit_alloc_local();
+
 			err = 0;
 			if (msg_type == AUDIT_USER_TTY) {
 				err = tty_audit_push();
 				if (err)
 					break;
 			}
-			audit_log_common_recv_msg(&ab, msg_type);
+			audit_log_common_recv_msg(context, &ab, msg_type);
 			if (msg_type != AUDIT_USER_TTY)
 				audit_log_format(ab, " msg='%.*s'",
 						 AUDIT_MESSAGE_TEXT_MAX,
@@ -1315,6 +1324,9 @@ static int audit_receive_msg(struct sk_buff *skb, struct nlmsghdr *nlh)
 				audit_log_n_untrustedstring(ab, data, size);
 			}
 			audit_log_end(ab);
+			audit_log_container_info(context, "user",
+						 audit_get_containerid(current));
+			audit_free_context(context);
 		}
 		break;
 	case AUDIT_ADD_RULE:
@@ -1322,9 +1334,14 @@ static int audit_receive_msg(struct sk_buff *skb, struct nlmsghdr *nlh)
 		if (nlmsg_len(nlh) < sizeof(struct audit_rule_data))
 			return -EINVAL;
 		if (audit_enabled == AUDIT_LOCKED) {
-			audit_log_common_recv_msg(&ab, AUDIT_CONFIG_CHANGE);
+			struct audit_context *context = audit_alloc_local();
+
+			audit_log_common_recv_msg(context, &ab, AUDIT_CONFIG_CHANGE);
 			audit_log_format(ab, " audit_enabled=%d res=0", audit_enabled);
 			audit_log_end(ab);
+			audit_log_container_info(context, "config",
+						 audit_get_containerid(current));
+			audit_free_context(context);
 			return -EPERM;
 		}
 		err = audit_rule_change(msg_type, seq, data, nlmsg_len(nlh));
@@ -1332,17 +1349,23 @@ static int audit_receive_msg(struct sk_buff *skb, struct nlmsghdr *nlh)
 	case AUDIT_LIST_RULES:
 		err = audit_list_rules_send(skb, seq);
 		break;
-	case AUDIT_TRIM:
+	case AUDIT_TRIM: {
+		struct audit_context *context = audit_alloc_local();
 		audit_trim_trees();
-		audit_log_common_recv_msg(&ab, AUDIT_CONFIG_CHANGE);
+		audit_log_common_recv_msg(context, &ab, AUDIT_CONFIG_CHANGE);
 		audit_log_format(ab, " op=trim res=1");
 		audit_log_end(ab);
+		audit_log_container_info(context, "config",
+					 audit_get_containerid(current));
+		audit_free_context(context);
 		break;
+	}
 	case AUDIT_MAKE_EQUIV: {
 		void *bufp = data;
 		u32 sizes[2];
 		size_t msglen = nlmsg_len(nlh);
 		char *old, *new;
+		struct audit_context *context = audit_alloc_local();
 
 		err = -EINVAL;
 		if (msglen < 2 * sizeof(u32))
@@ -1364,7 +1387,7 @@ static int audit_receive_msg(struct sk_buff *skb, struct nlmsghdr *nlh)
 		/* OK, here comes... */
 		err = audit_tag_tree(old, new);
 
-		audit_log_common_recv_msg(&ab, AUDIT_CONFIG_CHANGE);
+		audit_log_common_recv_msg(context, &ab, AUDIT_CONFIG_CHANGE);
 
 		audit_log_format(ab, " op=make_equiv old=");
 		audit_log_untrustedstring(ab, old);
@@ -1374,6 +1397,9 @@ static int audit_receive_msg(struct sk_buff *skb, struct nlmsghdr *nlh)
 		audit_log_end(ab);
 		kfree(old);
 		kfree(new);
+		audit_log_container_info(context, "config",
+					 audit_get_containerid(current));
+		audit_free_context(context);
 		break;
 	}
 	case AUDIT_SIGNAL_INFO:
@@ -1415,6 +1441,7 @@ static int audit_receive_msg(struct sk_buff *skb, struct nlmsghdr *nlh)
 		struct audit_tty_status s, old;
 		struct audit_buffer	*ab;
 		unsigned int t;
+		struct audit_context *context = audit_alloc_local();
 
 		memset(&s, 0, sizeof(s));
 		/* guard against past and future API changes */
@@ -1433,12 +1460,15 @@ static int audit_receive_msg(struct sk_buff *skb, struct nlmsghdr *nlh)
 		old.enabled = t & AUDIT_TTY_ENABLE;
 		old.log_passwd = !!(t & AUDIT_TTY_LOG_PASSWD);
 
-		audit_log_common_recv_msg(&ab, AUDIT_CONFIG_CHANGE);
+		audit_log_common_recv_msg(context, &ab, AUDIT_CONFIG_CHANGE);
 		audit_log_format(ab, " op=tty_set old-enabled=%d new-enabled=%d"
 				 " old-log_passwd=%d new-log_passwd=%d res=%d",
 				 old.enabled, s.enabled, old.log_passwd,
 				 s.log_passwd, !err);
 		audit_log_end(ab);
+		audit_log_container_info(context, "config",
+					 audit_get_containerid(current));
+		audit_free_context(context);
 		break;
 	}
 	default:
diff --git a/kernel/auditfilter.c b/kernel/auditfilter.c
index c4c8746..5f7f4d6 100644
--- a/kernel/auditfilter.c
+++ b/kernel/auditfilter.c
@@ -1109,11 +1109,12 @@ static void audit_log_rule_change(char *action, struct audit_krule *rule, int re
 	struct audit_buffer *ab;
 	uid_t loginuid = from_kuid(&init_user_ns, audit_get_loginuid(current));
 	unsigned int sessionid = audit_get_sessionid(current);
+	struct audit_context *context = audit_alloc_local();
 
 	if (!audit_enabled)
 		return;
 
-	ab = audit_log_start(NULL, GFP_KERNEL, AUDIT_CONFIG_CHANGE);
+	ab = audit_log_start(context, GFP_KERNEL, AUDIT_CONFIG_CHANGE);
 	if (!ab)
 		return;
 	audit_log_format(ab, "auid=%u ses=%u" ,loginuid, sessionid);
@@ -1122,6 +1123,8 @@ static void audit_log_rule_change(char *action, struct audit_krule *rule, int re
 	audit_log_key(ab, rule->filterkey);
 	audit_log_format(ab, " list=%d res=%d", rule->listnr, res);
 	audit_log_end(ab);
+	audit_log_container_info(context, "config", audit_get_containerid(current));
+	audit_free_context(context);
 }
 
 /**
-- 
1.8.3.1

^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [RFC PATCH V1 10/12] audit: add containerid support for seccomp and anom_abend records
  2018-03-01 19:41 [RFC PATCH V1 00/12] audit: implement container id Richard Guy Briggs
                   ` (8 preceding siblings ...)
  2018-03-01 19:41 ` [RFC PATCH V1 09/12] audit: add containerid support for config/feature/user records Richard Guy Briggs
@ 2018-03-01 19:41 ` Richard Guy Briggs
  2018-03-01 19:41 ` [RFC PATCH V1 11/12] debug audit: add container id Richard Guy Briggs
                   ` (3 subsequent siblings)
  13 siblings, 0 replies; 31+ messages in thread
From: Richard Guy Briggs @ 2018-03-01 19:41 UTC (permalink / raw)
  To: cgroups, containers, linux-api, Linux-Audit Mailing List,
	linux-fsdevel, LKML, netdev
  Cc: mszeredi, luto, jlayton, carlos, viro, dhowells, simo, trondmy,
	eparis, serge, ebiederm, madzcar, Richard Guy Briggs

Add container ID information to secure computing and abnormal end
standalone records.

Signed-off-by: Richard Guy Briggs <rgb@redhat.com>
---
 kernel/auditsc.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/kernel/auditsc.c b/kernel/auditsc.c
index 0cbd762..fcee34e 100644
--- a/kernel/auditsc.c
+++ b/kernel/auditsc.c
@@ -2569,6 +2569,7 @@ static void audit_log_task(struct audit_buffer *ab)
 void audit_core_dumps(long signr)
 {
 	struct audit_buffer *ab;
+	struct audit_context *context = audit_alloc_local();
 
 	if (!audit_enabled)
 		return;
@@ -2576,19 +2577,22 @@ void audit_core_dumps(long signr)
 	if (signr == SIGQUIT)	/* don't care for those */
 		return;
 
-	ab = audit_log_start(NULL, GFP_KERNEL, AUDIT_ANOM_ABEND);
+	ab = audit_log_start(context, GFP_KERNEL, AUDIT_ANOM_ABEND);
 	if (unlikely(!ab))
 		return;
 	audit_log_task(ab);
 	audit_log_format(ab, " sig=%ld res=1", signr);
 	audit_log_end(ab);
+	audit_log_container_info(context, "abend", audit_get_containerid(current));
+	audit_free_context(context);
 }
 
 void __audit_seccomp(unsigned long syscall, long signr, int code)
 {
 	struct audit_buffer *ab;
+	struct audit_context *context = audit_alloc_local();
 
-	ab = audit_log_start(NULL, GFP_KERNEL, AUDIT_SECCOMP);
+	ab = audit_log_start(context, GFP_KERNEL, AUDIT_SECCOMP);
 	if (unlikely(!ab))
 		return;
 	audit_log_task(ab);
@@ -2596,6 +2600,8 @@ void __audit_seccomp(unsigned long syscall, long signr, int code)
 			 signr, syscall_get_arch(), syscall,
 			 in_compat_syscall(), KSTK_EIP(current), code);
 	audit_log_end(ab);
+	audit_log_container_info(context, "seccomp", audit_get_containerid(current));
+	audit_free_context(context);
 }
 
 struct list_head *audit_killed_trees(void)
-- 
1.8.3.1

^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [RFC PATCH V1 11/12] debug audit: add container id
  2018-03-01 19:41 [RFC PATCH V1 00/12] audit: implement container id Richard Guy Briggs
                   ` (9 preceding siblings ...)
  2018-03-01 19:41 ` [RFC PATCH V1 10/12] audit: add containerid support for seccomp and anom_abend records Richard Guy Briggs
@ 2018-03-01 19:41 ` Richard Guy Briggs
  2018-03-01 19:41 ` [RFC PATCH V1 12/12] debug! " Richard Guy Briggs
                   ` (2 subsequent siblings)
  13 siblings, 0 replies; 31+ messages in thread
From: Richard Guy Briggs @ 2018-03-01 19:41 UTC (permalink / raw)
  To: cgroups, containers, linux-api, Linux-Audit Mailing List,
	linux-fsdevel, LKML, netdev
  Cc: mszeredi, luto, jlayton, carlos, viro, dhowells, simo, trondmy,
	eparis, serge, ebiederm, madzcar, Richard Guy Briggs

Switch from the 1000 range to the 1300 range for the prototype until it
can be worked out why the former aren't showing up in the logs.
---
 include/uapi/linux/audit.h | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/include/uapi/linux/audit.h b/include/uapi/linux/audit.h
index 8443a8f..c392b3b 100644
--- a/include/uapi/linux/audit.h
+++ b/include/uapi/linux/audit.h
@@ -71,7 +71,8 @@
 #define AUDIT_TTY_SET		1017	/* Set TTY auditing status */
 #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		1020	/* Define the container id and information */
+//#define AUDIT_CONTAINER		1020	/* Define the container id and information */
+#define AUDIT_CONTAINER		1333	/* Define the container id and information */
 
 #define AUDIT_FIRST_USER_MSG	1100	/* Userspace messages mostly uninteresting to kernel */
 #define AUDIT_USER_AVC		1107	/* We filter this differently */
-- 
1.8.3.1

^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [RFC PATCH V1 12/12] debug! audit: add container id
  2018-03-01 19:41 [RFC PATCH V1 00/12] audit: implement container id Richard Guy Briggs
                   ` (10 preceding siblings ...)
  2018-03-01 19:41 ` [RFC PATCH V1 11/12] debug audit: add container id Richard Guy Briggs
@ 2018-03-01 19:41 ` Richard Guy Briggs
  2018-03-04 21:55 ` [RFC PATCH V1 00/12] audit: implement " Mimi Zohar
  2018-03-06 15:04 ` Serge E. Hallyn
  13 siblings, 0 replies; 31+ messages in thread
From: Richard Guy Briggs @ 2018-03-01 19:41 UTC (permalink / raw)
  To: cgroups, containers, linux-api, Linux-Audit Mailing List,
	linux-fsdevel, LKML, netdev
  Cc: mszeredi, luto, jlayton, carlos, viro, dhowells, simo, trondmy,
	eparis, serge, ebiederm, madzcar, Richard Guy Briggs

Debugging code for verbose output to aid in development.
---
 fs/proc/base.c   | 10 ++++++++++
 kernel/auditsc.c | 16 ++++++++++++++++
 2 files changed, 26 insertions(+)

diff --git a/fs/proc/base.c b/fs/proc/base.c
index f66d1e2..63d1ca4 100644
--- a/fs/proc/base.c
+++ b/fs/proc/base.c
@@ -1309,9 +1309,13 @@ static ssize_t proc_containerid_read(struct file *file, char __user *buf,
 	char tmpbuf[TMPBUFLEN*2];
 
 	if (!task)
+	{
+		pr_info("no inode owner");
 		return -ESRCH;
+	}
 	length = scnprintf(tmpbuf, TMPBUFLEN*2, "%llu", audit_get_containerid(task));
 	put_task_struct(task);
+	pr_info("read: pid=%d opid=%d contid=%llu", pid_nr(task_tgid(current)), pid_nr(task_tgid(task)), audit_get_containerid(task));
 	return simple_read_from_buffer(buf, count, ppos, tmpbuf, length);
 }
 
@@ -1324,14 +1328,19 @@ static ssize_t proc_containerid_write(struct file *file, const char __user *buf,
 	struct task_struct *task = get_proc_task(inode);
 
 	if (!task)
+	{
+		pr_info("no inode owner");
 		return -ESRCH;
+	}
 	if (*ppos != 0) {
 		/* No partial writes. */
 		put_task_struct(task);
+		pr_info("no partial writes");
 		return -EINVAL;
 	}
 
 	rv = kstrtou64_from_user(buf, count, 10, &containerid);
+	pr_info("write: pid=%d rv=%d count=%ld opid=%d contid=%llu", task_tgid_nr(current), rv, count, task_tgid_nr(task), containerid);
 	if (rv < 0) {
 		put_task_struct(task);
 		return rv;
@@ -1339,6 +1348,7 @@ static ssize_t proc_containerid_write(struct file *file, const char __user *buf,
 
 	rv = audit_set_containerid(task, containerid);
 	put_task_struct(task);
+	//pr_info("audit_set_containerid: rv=%d", rv);
 	if (rv < 0)
 		return rv;
 	return count;
diff --git a/kernel/auditsc.c b/kernel/auditsc.c
index fcee34e..39e7dc10 100644
--- a/kernel/auditsc.c
+++ b/kernel/auditsc.c
@@ -2111,16 +2111,28 @@ static int audit_set_containerid_perm(struct task_struct *task, u64 containerid)
 
 	/* Don't allow to set our own containerid */
 	if (current == task)
+	{
+		pr_info("pid=%d can't set own containerid", task_tgid_nr(task));
 		return -EPERM;
+	}
 	/* Don't allow the containerid to be unset */
 	if (!cid_valid(containerid))
+	{
+		pr_info("can't unset containerid");
 		return -EINVAL;
+	}
 	/* if we don't have caps, reject */
 	if (!capable(CAP_AUDIT_CONTROL))
+	{
+		pr_info("don't have CAP_AUDIT_CONTROL");
 		return -EPERM;
+	}
 	/* if containerid is unset, allow */
 	if (!audit_containerid_set(task))
+	{
+		//pr_info("unset, allow");
 		return 0;
+	}
 	/* it is already set, and not inherited from the parent, reject */
 	ccontainerid = audit_get_containerid(task);
 	rcu_read_lock();
@@ -2131,7 +2143,11 @@ static int audit_set_containerid_perm(struct task_struct *task, u64 containerid)
 	ppid = task_tgid_nr(parent);
 	task_unlock(parent);
 	if (ccontainerid != pcontainerid)
+	{
+		pr_info("pid=%d already has contid=%llu set, not inherited from ppid=%d with contid=%llu, can't set containerid %llu",
+			task_tgid_nr(task), ccontainerid, ppid, pcontainerid, containerid);
 		return -EPERM;
+	}
 	return 0;
 }
 
-- 
1.8.3.1

^ permalink raw reply related	[flat|nested] 31+ messages in thread

* Re: [RFC PATCH V1 01/12] audit: add container id
  2018-03-01 19:41 ` [RFC PATCH V1 01/12] audit: add " Richard Guy Briggs
@ 2018-03-02  1:41   ` Richard Guy Briggs
  2018-03-02 15:48     ` Paul Moore
  2018-03-03  9:19   ` Serge E. Hallyn
  2018-03-15 20:27   ` Stefan Berger
  2 siblings, 1 reply; 31+ messages in thread
From: Richard Guy Briggs @ 2018-03-02  1:41 UTC (permalink / raw)
  To: cgroups, containers, linux-api, Linux-Audit Mailing List,
	linux-fsdevel, LKML, netdev
  Cc: mszeredi, luto, jlayton, carlos, viro, dhowells, simo, trondmy,
	eparis, serge, ebiederm, madzcar

On 2018-03-01 14:41, Richard Guy Briggs wrote:
> Implement the proc fs write to set the audit container ID of a process,
> emitting an AUDIT_CONTAINER record to document the event.
> 
> This is a write from the container orchestrator task to a proc entry of
> the form /proc/PID/containerid where PID is the process ID of the newly
> created task that is to become the first task in a container, or an
> additional task added to a container.
> 
> The write expects up to a u64 value (unset: 18446744073709551615).
> 
> This will produce a record such as this:
> type=UNKNOWN[1333] msg=audit(1519903238.968:261): op=set pid=596 uid=0 subj=unconfined_u:unconfined_r:unconfined_t:s0-s0:c0.c1023 auid=0 tty=pts0 ses=1 opid=596 old-contid=18446744073709551615 contid=123455 res=0
> 
> The "op" field indicates an initial set.  The "pid" to "ses" fields are
> the orchestrator while the "opid" field is the object's PID, the process
> being "contained".  Old and new container ID values are given in the
> "contid" fields, while res indicates its success.
> 
> It is not permitted to self-set, unset or re-set the container ID.  A
> child inherits its parent's container ID, but then can be set only once
> after.

There are more restrictions coming later:
- check that the child being set has no children or threads yet, or
  forcibly set them all to the same container ID (assuming they all pass
  the same tests).  This will also prevent an orch from setting its
  parent and other tit-for-tat games to circumvent the basic checks.

> See: https://github.com/linux-audit/audit-kernel/issues/32
> 
> Signed-off-by: Richard Guy Briggs <rgb@redhat.com>
> ---
>  fs/proc/base.c             | 37 ++++++++++++++++++++
>  include/linux/audit.h      | 16 +++++++++
>  include/linux/init_task.h  |  4 ++-
>  include/linux/sched.h      |  1 +
>  include/uapi/linux/audit.h |  2 ++
>  kernel/auditsc.c           | 86 ++++++++++++++++++++++++++++++++++++++++++++++
>  6 files changed, 145 insertions(+), 1 deletion(-)
> 
> diff --git a/fs/proc/base.c b/fs/proc/base.c
> index 60316b5..6ce4fbe 100644
> --- a/fs/proc/base.c
> +++ b/fs/proc/base.c
> @@ -1299,6 +1299,41 @@ static ssize_t proc_sessionid_read(struct file * file, char __user * buf,
>  	.read		= proc_sessionid_read,
>  	.llseek		= generic_file_llseek,
>  };
> +
> +static ssize_t proc_containerid_write(struct file *file, const char __user *buf,
> +				   size_t count, loff_t *ppos)
> +{
> +	struct inode *inode = file_inode(file);
> +	u64 containerid;
> +	int rv;
> +	struct task_struct *task = get_proc_task(inode);
> +
> +	if (!task)
> +		return -ESRCH;
> +	if (*ppos != 0) {
> +		/* No partial writes. */
> +		put_task_struct(task);
> +		return -EINVAL;
> +	}
> +
> +	rv = kstrtou64_from_user(buf, count, 10, &containerid);
> +	if (rv < 0) {
> +		put_task_struct(task);
> +		return rv;
> +	}
> +
> +	rv = audit_set_containerid(task, containerid);
> +	put_task_struct(task);
> +	if (rv < 0)
> +		return rv;
> +	return count;
> +}
> +
> +static const struct file_operations proc_containerid_operations = {
> +	.write		= proc_containerid_write,
> +	.llseek		= generic_file_llseek,
> +};
> +
>  #endif
>  
>  #ifdef CONFIG_FAULT_INJECTION
> @@ -2961,6 +2996,7 @@ static int proc_pid_patch_state(struct seq_file *m, struct pid_namespace *ns,
>  #ifdef CONFIG_AUDITSYSCALL
>  	REG("loginuid",   S_IWUSR|S_IRUGO, proc_loginuid_operations),
>  	REG("sessionid",  S_IRUGO, proc_sessionid_operations),
> +	REG("containerid", S_IWUSR, proc_containerid_operations),
>  #endif
>  #ifdef CONFIG_FAULT_INJECTION
>  	REG("make-it-fail", S_IRUGO|S_IWUSR, proc_fault_inject_operations),
> @@ -3355,6 +3391,7 @@ static int proc_tid_comm_permission(struct inode *inode, int mask)
>  #ifdef CONFIG_AUDITSYSCALL
>  	REG("loginuid",  S_IWUSR|S_IRUGO, proc_loginuid_operations),
>  	REG("sessionid",  S_IRUGO, proc_sessionid_operations),
> +	REG("containerid", S_IWUSR, proc_containerid_operations),
>  #endif
>  #ifdef CONFIG_FAULT_INJECTION
>  	REG("make-it-fail", S_IRUGO|S_IWUSR, proc_fault_inject_operations),
> diff --git a/include/linux/audit.h b/include/linux/audit.h
> index af410d9..fe4ba3f 100644
> --- a/include/linux/audit.h
> +++ b/include/linux/audit.h
> @@ -29,6 +29,7 @@
>  
>  #define AUDIT_INO_UNSET ((unsigned long)-1)
>  #define AUDIT_DEV_UNSET ((dev_t)-1)
> +#define INVALID_CID AUDIT_CID_UNSET
>  
>  struct audit_sig_info {
>  	uid_t		uid;
> @@ -321,6 +322,7 @@ static inline void audit_ptrace(struct task_struct *t)
>  extern int auditsc_get_stamp(struct audit_context *ctx,
>  			      struct timespec64 *t, unsigned int *serial);
>  extern int audit_set_loginuid(kuid_t loginuid);
> +extern int audit_set_containerid(struct task_struct *tsk, u64 containerid);
>  
>  static inline kuid_t audit_get_loginuid(struct task_struct *tsk)
>  {
> @@ -332,6 +334,11 @@ static inline unsigned int audit_get_sessionid(struct task_struct *tsk)
>  	return tsk->sessionid;
>  }
>  
> +static inline u64 audit_get_containerid(struct task_struct *tsk)
> +{
> +	return tsk->containerid;
> +}
> +
>  extern void __audit_ipc_obj(struct kern_ipc_perm *ipcp);
>  extern void __audit_ipc_set_perm(unsigned long qbytes, uid_t uid, gid_t gid, umode_t mode);
>  extern void __audit_bprm(struct linux_binprm *bprm);
> @@ -517,6 +524,10 @@ static inline unsigned int audit_get_sessionid(struct task_struct *tsk)
>  {
>  	return -1;
>  }
> +static inline kuid_t audit_get_containerid(struct task_struct *tsk)
> +{
> +	return INVALID_CID;
> +}
>  static inline void audit_ipc_obj(struct kern_ipc_perm *ipcp)
>  { }
>  static inline void audit_ipc_set_perm(unsigned long qbytes, uid_t uid,
> @@ -581,6 +592,11 @@ static inline bool audit_loginuid_set(struct task_struct *tsk)
>  	return uid_valid(audit_get_loginuid(tsk));
>  }
>  
> +static inline bool audit_containerid_set(struct task_struct *tsk)
> +{
> +	return audit_get_containerid(tsk) != INVALID_CID;
> +}
> +
>  static inline void audit_log_string(struct audit_buffer *ab, const char *buf)
>  {
>  	audit_log_n_string(ab, buf, strlen(buf));
> diff --git a/include/linux/init_task.h b/include/linux/init_task.h
> index 6a53262..046bd0a 100644
> --- a/include/linux/init_task.h
> +++ b/include/linux/init_task.h
> @@ -18,6 +18,7 @@
>  #include <linux/sched/rt.h>
>  #include <linux/livepatch.h>
>  #include <linux/mm_types.h>
> +#include <linux/audit.h>
>  
>  #include <asm/thread_info.h>
>  
> @@ -120,7 +121,8 @@
>  #ifdef CONFIG_AUDITSYSCALL
>  #define INIT_IDS \
>  	.loginuid = INVALID_UID, \
> -	.sessionid = (unsigned int)-1,
> +	.sessionid = (unsigned int)-1, \
> +	.containerid = INVALID_CID,
>  #else
>  #define INIT_IDS
>  #endif
> diff --git a/include/linux/sched.h b/include/linux/sched.h
> index d258826..1b82191 100644
> --- a/include/linux/sched.h
> +++ b/include/linux/sched.h
> @@ -796,6 +796,7 @@ struct task_struct {
>  #ifdef CONFIG_AUDITSYSCALL
>  	kuid_t				loginuid;
>  	unsigned int			sessionid;
> +	u64				containerid;
>  #endif
>  	struct seccomp			seccomp;
>  
> diff --git a/include/uapi/linux/audit.h b/include/uapi/linux/audit.h
> index 4e61a9e..921a71f 100644
> --- a/include/uapi/linux/audit.h
> +++ b/include/uapi/linux/audit.h
> @@ -71,6 +71,7 @@
>  #define AUDIT_TTY_SET		1017	/* Set TTY auditing status */
>  #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		1020	/* Define the container id and information */
>  
>  #define AUDIT_FIRST_USER_MSG	1100	/* Userspace messages mostly uninteresting to kernel */
>  #define AUDIT_USER_AVC		1107	/* We filter this differently */
> @@ -465,6 +466,7 @@ struct audit_tty_status {
>  };
>  
>  #define AUDIT_UID_UNSET (unsigned int)-1
> +#define AUDIT_CID_UNSET ((u64)-1)
>  
>  /* audit_rule_data supports filter rules with both integer and string
>   * fields.  It corresponds with AUDIT_ADD_RULE, AUDIT_DEL_RULE and
> diff --git a/kernel/auditsc.c b/kernel/auditsc.c
> index 4e0a4ac..0ee1e59 100644
> --- a/kernel/auditsc.c
> +++ b/kernel/auditsc.c
> @@ -2073,6 +2073,92 @@ int audit_set_loginuid(kuid_t loginuid)
>  	return rc;
>  }
>  
> +static int audit_set_containerid_perm(struct task_struct *task, u64 containerid)
> +{
> +	struct task_struct *parent;
> +	u64 pcontainerid, ccontainerid;
> +	pid_t ppid;
> +
> +	/* Don't allow to set our own containerid */
> +	if (current == task)
> +		return -EPERM;
> +	/* Don't allow the containerid to be unset */
> +	if (!cid_valid(containerid))
> +		return -EINVAL;
> +	/* if we don't have caps, reject */
> +	if (!capable(CAP_AUDIT_CONTROL))
> +		return -EPERM;
> +	/* if containerid is unset, allow */
> +	if (!audit_containerid_set(task))
> +		return 0;
> +	/* it is already set, and not inherited from the parent, reject */
> +	ccontainerid = audit_get_containerid(task);
> +	rcu_read_lock();
> +	parent = rcu_dereference(task->real_parent);
> +	rcu_read_unlock();
> +	task_lock(parent);
> +	pcontainerid = audit_get_containerid(parent);
> +	ppid = task_tgid_nr(parent);
> +	task_unlock(parent);
> +	if (ccontainerid != pcontainerid)
> +		return -EPERM;
> +	return 0;
> +}
> +
> +static void audit_log_set_containerid(struct task_struct *task, u64 oldcontainerid,
> +				      u64 containerid, int rc)
> +{
> +	struct audit_buffer *ab;
> +	uid_t uid;
> +	struct tty_struct *tty;
> +
> +	if (!audit_enabled)
> +		return;
> +
> +	ab = audit_log_start(NULL, GFP_KERNEL, AUDIT_CONTAINER);
> +	if (!ab)
> +		return;
> +
> +	uid = from_kuid(&init_user_ns, task_uid(current));
> +	tty = audit_get_tty(current);
> +
> +	audit_log_format(ab, "op=set pid=%d uid=%u", task_tgid_nr(current), uid);
> +	audit_log_task_context(ab);
> +	audit_log_format(ab, " auid=%u tty=%s ses=%u opid=%d old-contid=%llu contid=%llu res=%d",
> +			 from_kuid(&init_user_ns, audit_get_loginuid(current)),
> +			 tty ? tty_name(tty) : "(none)", audit_get_sessionid(current),
> +			 task_tgid_nr(task), oldcontainerid, containerid, !rc);
> +
> +	audit_put_tty(tty);
> +	audit_log_end(ab);
> +}
> +
> +/**
> + * audit_set_containerid - set current task's audit_context containerid
> + * @containerid: containerid value
> + *
> + * Returns 0 on success, -EPERM on permission failure.
> + *
> + * Called (set) from fs/proc/base.c::proc_containerid_write().
> + */
> +int audit_set_containerid(struct task_struct *task, u64 containerid)
> +{
> +	u64 oldcontainerid;
> +	int rc;
> +
> +	oldcontainerid = audit_get_containerid(task);
> +
> +	rc = audit_set_containerid_perm(task, containerid);
> +	if (!rc) {
> +		task_lock(task);
> +		task->containerid = containerid;
> +		task_unlock(task);
> +	}
> +
> +	audit_log_set_containerid(task, oldcontainerid, containerid, rc);
> +	return rc;
> +}
> +
>  /**
>   * __audit_mq_open - record audit data for a POSIX MQ open
>   * @oflag: open flag
> -- 
> 1.8.3.1
> 

- RGB

--
Richard Guy Briggs <rgb@redhat.com>
Sr. S/W Engineer, Kernel Security, Base Operating Systems
Remote, Ottawa, Red Hat Canada
IRC: rgb, SunRaycer
Voice: +1.647.777.2635, Internal: (81) 32635

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [RFC PATCH V1 01/12] audit: add container id
  2018-03-02  1:41   ` Richard Guy Briggs
@ 2018-03-02 15:48     ` Paul Moore
  2018-03-02 18:23       ` Matthew Wilcox
  0 siblings, 1 reply; 31+ messages in thread
From: Paul Moore @ 2018-03-02 15:48 UTC (permalink / raw)
  To: Richard Guy Briggs
  Cc: cgroups, containers, linux-api, Linux-Audit Mailing List,
	linux-fsdevel, LKML, netdev, mszeredi, ebiederm, simo, jlayton,
	carlos, dhowells, viro, luto, Eric Paris, trondmy, serge

On Thu, Mar 1, 2018 at 8:41 PM, Richard Guy Briggs <rgb@redhat.com> wrote:
> On 2018-03-01 14:41, Richard Guy Briggs wrote:
>> Implement the proc fs write to set the audit container ID of a process,
>> emitting an AUDIT_CONTAINER record to document the event.
>>
>> This is a write from the container orchestrator task to a proc entry of
>> the form /proc/PID/containerid where PID is the process ID of the newly
>> created task that is to become the first task in a container, or an
>> additional task added to a container.
>>
>> The write expects up to a u64 value (unset: 18446744073709551615).
>>
>> This will produce a record such as this:
>> type=UNKNOWN[1333] msg=audit(1519903238.968:261): op=set pid=596 uid=0 subj=unconfined_u:unconfined_r:unconfined_t:s0-s0:c0.c1023 auid=0 tty=pts0 ses=1 opid=596 old-contid=18446744073709551615 contid=123455 res=0
>>
>> The "op" field indicates an initial set.  The "pid" to "ses" fields are
>> the orchestrator while the "opid" field is the object's PID, the process
>> being "contained".  Old and new container ID values are given in the
>> "contid" fields, while res indicates its success.
>>
>> It is not permitted to self-set, unset or re-set the container ID.  A
>> child inherits its parent's container ID, but then can be set only once
>> after.
>
> There are more restrictions coming later:
> - check that the child being set has no children or threads yet, or
>   forcibly set them all to the same container ID (assuming they all pass
>   the same tests).  This will also prevent an orch from setting its
>   parent and other tit-for-tat games to circumvent the basic checks.

FYI, I think you may have a problem with something in your outgoing
mail path; I didn't receive the original patchset you are referencing
and it doesn't appear in the mail archive either.

-- 
paul moore
www.paul-moore.com

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [RFC PATCH V1 01/12] audit: add container id
  2018-03-02 15:48     ` Paul Moore
@ 2018-03-02 18:23       ` Matthew Wilcox
  2018-03-02 19:25         ` Paul Moore
  0 siblings, 1 reply; 31+ messages in thread
From: Matthew Wilcox @ 2018-03-02 18:23 UTC (permalink / raw)
  To: Paul Moore
  Cc: Richard Guy Briggs, cgroups, containers, linux-api,
	Linux-Audit Mailing List, linux-fsdevel, LKML, netdev, mszeredi,
	ebiederm, simo, jlayton, carlos, dhowells, viro, luto,
	Eric Paris, trondmy, serge

On Fri, Mar 02, 2018 at 10:48:42AM -0500, Paul Moore wrote:
> On Thu, Mar 1, 2018 at 8:41 PM, Richard Guy Briggs <rgb@redhat.com> wrote:
> > On 2018-03-01 14:41, Richard Guy Briggs wrote:
> FYI, I think you may have a problem with something in your outgoing
> mail path; I didn't receive the original patchset you are referencing
> and it doesn't appear in the mail archive either.

I have those patches.  Which mail archive is missing them?

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [RFC PATCH V1 01/12] audit: add container id
  2018-03-02 18:23       ` Matthew Wilcox
@ 2018-03-02 19:25         ` Paul Moore
  2018-03-02 19:41           ` Paul Moore
  0 siblings, 1 reply; 31+ messages in thread
From: Paul Moore @ 2018-03-02 19:25 UTC (permalink / raw)
  To: Matthew Wilcox
  Cc: Richard Guy Briggs, cgroups, containers, linux-api,
	Linux-Audit Mailing List, linux-fsdevel, LKML, netdev, mszeredi,
	ebiederm, simo, jlayton, carlos, dhowells, viro, luto,
	Eric Paris, trondmy, serge

On Fri, Mar 2, 2018 at 1:23 PM, Matthew Wilcox <willy@infradead.org> wrote:
> On Fri, Mar 02, 2018 at 10:48:42AM -0500, Paul Moore wrote:
>> On Thu, Mar 1, 2018 at 8:41 PM, Richard Guy Briggs <rgb@redhat.com> wrote:
>> > On 2018-03-01 14:41, Richard Guy Briggs wrote:
>> FYI, I think you may have a problem with something in your outgoing
>> mail path; I didn't receive the original patchset you are referencing
>> and it doesn't appear in the mail archive either.
>
> I have those patches.  Which mail archive is missing them?

The archive run by the linux-audit mailing list:

* https://www.redhat.com/archives/linux-audit

-- 
paul moore
www.paul-moore.com

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [RFC PATCH V1 01/12] audit: add container id
  2018-03-02 19:25         ` Paul Moore
@ 2018-03-02 19:41           ` Paul Moore
  0 siblings, 0 replies; 31+ messages in thread
From: Paul Moore @ 2018-03-02 19:41 UTC (permalink / raw)
  To: Matthew Wilcox
  Cc: Richard Guy Briggs, containers, Linux-Audit Mailing List,
	linux-fsdevel, LKML, netdev, mszeredi, ebiederm, simo, jlayton,
	carlos, dhowells, viro, luto, trondmy, serge

On Fri, Mar 2, 2018 at 2:25 PM, Paul Moore <paul@paul-moore.com> wrote:
> On Fri, Mar 2, 2018 at 1:23 PM, Matthew Wilcox <willy@infradead.org> wrote:
>> On Fri, Mar 02, 2018 at 10:48:42AM -0500, Paul Moore wrote:
>>> On Thu, Mar 1, 2018 at 8:41 PM, Richard Guy Briggs <rgb@redhat.com> wrote:
>>> > On 2018-03-01 14:41, Richard Guy Briggs wrote:
>>> FYI, I think you may have a problem with something in your outgoing
>>> mail path; I didn't receive the original patchset you are referencing
>>> and it doesn't appear in the mail archive either.
>>
>> I have those patches.  Which mail archive is missing them?
>
> The archive run by the linux-audit mailing list:
>
> * https://www.redhat.com/archives/linux-audit

After having my reply get bounced from the linux-audit list I realized
that Richard had gotten a little overzealous with the number of
recipients (note to Richard, you easily could have dropped some of
those lists/people from the To/CC line).

I was able to get in and free those patches from the moderation queue,
they should be arriving on the linux-audit list shortly.

-- 
paul moore
www.paul-moore.com

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [RFC PATCH V1 01/12] audit: add container id
  2018-03-01 19:41 ` [RFC PATCH V1 01/12] audit: add " Richard Guy Briggs
  2018-03-02  1:41   ` Richard Guy Briggs
@ 2018-03-03  9:19   ` Serge E. Hallyn
  2018-03-04 15:01     ` Paul Moore
  2018-03-15 20:27   ` Stefan Berger
  2 siblings, 1 reply; 31+ messages in thread
From: Serge E. Hallyn @ 2018-03-03  9:19 UTC (permalink / raw)
  To: Richard Guy Briggs
  Cc: cgroups, containers, linux-api, Linux-Audit Mailing List,
	linux-fsdevel, LKML, netdev, mszeredi, luto, jlayton, carlos,
	viro, dhowells, simo, trondmy, eparis, serge, ebiederm, madzcar

On Thu, Mar 01, 2018 at 02:41:04PM -0500, Richard Guy Briggs wrote:
...
> +static inline bool audit_containerid_set(struct task_struct *tsk)

Hi Richard,

the calls to audit_containerid_set() confused me.  Could you make it
is_audit_containerid_set() or audit_containerid_isset()?

> +{
> +	return audit_get_containerid(tsk) != INVALID_CID;
> +}

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [RFC PATCH V1 01/12] audit: add container id
  2018-03-03  9:19   ` Serge E. Hallyn
@ 2018-03-04 15:01     ` Paul Moore
  2018-03-05  8:16       ` Richard Guy Briggs
  0 siblings, 1 reply; 31+ messages in thread
From: Paul Moore @ 2018-03-04 15:01 UTC (permalink / raw)
  To: Serge E. Hallyn, Richard Guy Briggs
  Cc: mszeredi, ebiederm, simo, jlayton, carlos, linux-api, containers,
	LKML, Eric Paris, dhowells, Linux-Audit Mailing List, viro, luto,
	netdev, linux-fsdevel, cgroups, trondmy

On Sat, Mar 3, 2018 at 4:19 AM, Serge E. Hallyn <serge@hallyn.com> wrote:
> On Thu, Mar 01, 2018 at 02:41:04PM -0500, Richard Guy Briggs wrote:
> ...
>> +static inline bool audit_containerid_set(struct task_struct *tsk)
>
> Hi Richard,
>
> the calls to audit_containerid_set() confused me.  Could you make it
> is_audit_containerid_set() or audit_containerid_isset()?

I haven't gone through the entire patchset yet, but I wanted to
quickly comment on this ... I really dislike the
function-names-as-sentences approach and would would greatly prefer
audit_containerid_isset().

>> +{
>> +     return audit_get_containerid(tsk) != INVALID_CID;
>> +}

-- 
paul moore
www.paul-moore.com

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [RFC PATCH V1 00/12] audit: implement container id
  2018-03-01 19:41 [RFC PATCH V1 00/12] audit: implement container id Richard Guy Briggs
                   ` (11 preceding siblings ...)
  2018-03-01 19:41 ` [RFC PATCH V1 12/12] debug! " Richard Guy Briggs
@ 2018-03-04 21:55 ` Mimi Zohar
  2018-03-05  3:31   ` Richard Guy Briggs
  2018-03-06 15:04 ` Serge E. Hallyn
  13 siblings, 1 reply; 31+ messages in thread
From: Mimi Zohar @ 2018-03-04 21:55 UTC (permalink / raw)
  To: Richard Guy Briggs, cgroups, containers, linux-api,
	Linux-Audit Mailing List, linux-fsdevel, LKML, netdev
  Cc: mszeredi, ebiederm, simo, jlayton, carlos, dhowells, viro, luto,
	eparis, trondmy, serge

On Thu, 2018-03-01 at 14:41 -0500, Richard Guy Briggs wrote:
> Implement audit kernel container ID.
> 
> This patchset is a preliminary RFC based on the proposal document (V3)
> posted:
> 	https://www.redhat.com/archives/linux-audit/2018-January/msg00014.html
> 
> The first patch implements the proc fs write to set the audit container
> ID of a process, emitting an AUDIT_CONTAINER record.
> 
> The second implements an auxiliary syscall record AUDIT_CONTAINER_INFO
> if a container ID is present on a task.
> 
> The third adds filtering to the exit, exclude and user lists.
> 
> The 4th, implements reading the container ID from the proc filesystem
> for debugging.  This isn't planned for upstream inclusion.
> 
> The 5th adds signal and ptrace support.
> 
> The 6th attempts to create a local audit context to be able to bind a
> standalone record with the container ID record.
> 
> The 7th, 8th, 9th, 10th patches add container ID records to standalone
> records.  Some of these may end up being syscall auxiliary records and
> won't need this specific support since they'll be supported via
> syscalls.
> 
> The 11th is a temporary workaround due to the AUDIT_CONTAINER records
> not showing up as do AUDIT_LOGIN records.  I suspect this is due to its
> range (1000 vs 1300), but the intent is to solve it.
> 
> The 12th adds debug information not intended for upstream for those
> brave souls wanting to tinker with it in this early state.
> 
> Feedback please!

Which tree can this patch set be applied to?

Mimi

> Here's a quick and dirty test script:
> echo 123455 > /proc/$$/containerid; echo $?
> sleep 4&  
> child=$!; sleep 1
> echo 18446744073709551615 > /proc/$child/containerid; echo $?
> echo 123456 > /proc/$child/containerid; echo $?
> echo 123457 > /proc/$child/containerid; echo $?
> sleep 1
> ausearch -ts recent |grep " contid=18446744073709551615"; echo $?
> ausearch -ts recent |grep " contid=123456"; echo $?
> ausearch -ts recent |grep " contid=123457"; echo $?
> echo self:$$ contid:$( cat /proc/$$/containerid)
> echo child:$child contid:$( cat /proc/$child/containerid)
> 
> containerid=123458
> key=tmpcontainerid
> auditctl -a exit,always -F dir=/tmp -F perm=wa -F containerid=$containerid -F key=$key || echo failed to add containerid filter rule
> bash -c "sleep 1; echo test > /tmp/$key"&
> child=$!
> echo $containerid > /proc/$child/containerid
> sleep 2
> rm -f /tmp/$key
> ausearch -ts recent -k $key || echo failed to find CONTAINER_INFO record
> auditctl -d exit,always -F dir=/tmp -F perm=wa -F containerid=$containerid -F key=$key || echo failed to add containerid filter rule
> 
> See:
> 	https://github.com/linux-audit/audit-kernel/issues/32
> 	https://github.com/linux-audit/audit-userspace/issues/40
> 	https://github.com/linux-audit/audit-testsuite/issues/64
> 
> Richard Guy Briggs (12):
>   audit: add container id
>   audit: log container info of syscalls
>   audit: add containerid filtering
>   audit: read container ID of a process
>   audit: add containerid support for ptrace and signals
>   audit: add support for non-syscall auxiliary records
>   audit: add container aux record to watch/tree/mark
>   audit: add containerid support for tty_audit
>   audit: add containerid support for config/feature/user records
>   audit: add containerid support for seccomp and anom_abend records
>   debug audit: add container id
>   debug! audit: add container id
> 
>  drivers/tty/tty_audit.c    |   5 +-
>  fs/proc/base.c             |  63 +++++++++++++++++++
>  include/linux/audit.h      |  36 +++++++++++
>  include/linux/init_task.h  |   4 +-
>  include/linux/sched.h      |   1 +
>  include/uapi/linux/audit.h |   9 ++-
>  kernel/audit.c             |  74 +++++++++++++++++++---
>  kernel/audit.h             |   3 +
>  kernel/audit_fsnotify.c    |   5 +-
>  kernel/audit_tree.c        |   5 +-
>  kernel/audit_watch.c       |  33 +++++-----
>  kernel/auditfilter.c       |  52 ++++++++++++++-
>  kernel/auditsc.c           | 154 +++++++++++++++++++++++++++++++++++++++++++--
>  13 files changed, 408 insertions(+), 36 deletions(-)
> 

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [RFC PATCH V1 00/12] audit: implement container id
  2018-03-04 21:55 ` [RFC PATCH V1 00/12] audit: implement " Mimi Zohar
@ 2018-03-05  3:31   ` Richard Guy Briggs
  2018-03-05 13:27     ` Mimi Zohar
  0 siblings, 1 reply; 31+ messages in thread
From: Richard Guy Briggs @ 2018-03-05  3:31 UTC (permalink / raw)
  To: Mimi Zohar
  Cc: cgroups, containers, linux-api, Linux-Audit Mailing List,
	linux-fsdevel, LKML, netdev, mszeredi, ebiederm, simo, jlayton,
	carlos, dhowells, viro, luto, eparis, trondmy, serge

On 2018-03-04 16:55, Mimi Zohar wrote:
> On Thu, 2018-03-01 at 14:41 -0500, Richard Guy Briggs wrote:
> > Implement audit kernel container ID.
> > 
> > This patchset is a preliminary RFC based on the proposal document (V3)
> > posted:
> > 	https://www.redhat.com/archives/linux-audit/2018-January/msg00014.html
> > 
> > The first patch implements the proc fs write to set the audit container
> > ID of a process, emitting an AUDIT_CONTAINER record.
> > 
> > The second implements an auxiliary syscall record AUDIT_CONTAINER_INFO
> > if a container ID is present on a task.
> > 
> > The third adds filtering to the exit, exclude and user lists.
> > 
> > The 4th, implements reading the container ID from the proc filesystem
> > for debugging.  This isn't planned for upstream inclusion.
> > 
> > The 5th adds signal and ptrace support.
> > 
> > The 6th attempts to create a local audit context to be able to bind a
> > standalone record with the container ID record.
> > 
> > The 7th, 8th, 9th, 10th patches add container ID records to standalone
> > records.  Some of these may end up being syscall auxiliary records and
> > won't need this specific support since they'll be supported via
> > syscalls.
> > 
> > The 11th is a temporary workaround due to the AUDIT_CONTAINER records
> > not showing up as do AUDIT_LOGIN records.  I suspect this is due to its
> > range (1000 vs 1300), but the intent is to solve it.
> > 
> > The 12th adds debug information not intended for upstream for those
> > brave souls wanting to tinker with it in this early state.
> > 
> > Feedback please!
> 
> Which tree can this patch set be applied to?

git://git.kernel.org/pub/scm/linux/kernel/git/pcmoore/audit.git next

> Mimi
> 
> > Here's a quick and dirty test script:
> > echo 123455 > /proc/$$/containerid; echo $?
> > sleep 4&  
> > child=$!; sleep 1
> > echo 18446744073709551615 > /proc/$child/containerid; echo $?
> > echo 123456 > /proc/$child/containerid; echo $?
> > echo 123457 > /proc/$child/containerid; echo $?
> > sleep 1
> > ausearch -ts recent |grep " contid=18446744073709551615"; echo $?
> > ausearch -ts recent |grep " contid=123456"; echo $?
> > ausearch -ts recent |grep " contid=123457"; echo $?
> > echo self:$$ contid:$( cat /proc/$$/containerid)
> > echo child:$child contid:$( cat /proc/$child/containerid)
> > 
> > containerid=123458
> > key=tmpcontainerid
> > auditctl -a exit,always -F dir=/tmp -F perm=wa -F containerid=$containerid -F key=$key || echo failed to add containerid filter rule
> > bash -c "sleep 1; echo test > /tmp/$key"&
> > child=$!
> > echo $containerid > /proc/$child/containerid
> > sleep 2
> > rm -f /tmp/$key
> > ausearch -ts recent -k $key || echo failed to find CONTAINER_INFO record
> > auditctl -d exit,always -F dir=/tmp -F perm=wa -F containerid=$containerid -F key=$key || echo failed to add containerid filter rule
> > 
> > See:
> > 	https://github.com/linux-audit/audit-kernel/issues/32
> > 	https://github.com/linux-audit/audit-userspace/issues/40
> > 	https://github.com/linux-audit/audit-testsuite/issues/64
> > 
> > Richard Guy Briggs (12):
> >   audit: add container id
> >   audit: log container info of syscalls
> >   audit: add containerid filtering
> >   audit: read container ID of a process
> >   audit: add containerid support for ptrace and signals
> >   audit: add support for non-syscall auxiliary records
> >   audit: add container aux record to watch/tree/mark
> >   audit: add containerid support for tty_audit
> >   audit: add containerid support for config/feature/user records
> >   audit: add containerid support for seccomp and anom_abend records
> >   debug audit: add container id
> >   debug! audit: add container id
> > 
> >  drivers/tty/tty_audit.c    |   5 +-
> >  fs/proc/base.c             |  63 +++++++++++++++++++
> >  include/linux/audit.h      |  36 +++++++++++
> >  include/linux/init_task.h  |   4 +-
> >  include/linux/sched.h      |   1 +
> >  include/uapi/linux/audit.h |   9 ++-
> >  kernel/audit.c             |  74 +++++++++++++++++++---
> >  kernel/audit.h             |   3 +
> >  kernel/audit_fsnotify.c    |   5 +-
> >  kernel/audit_tree.c        |   5 +-
> >  kernel/audit_watch.c       |  33 +++++-----
> >  kernel/auditfilter.c       |  52 ++++++++++++++-
> >  kernel/auditsc.c           | 154 +++++++++++++++++++++++++++++++++++++++++++--
> >  13 files changed, 408 insertions(+), 36 deletions(-)
> > 
> 

- RGB

--
Richard Guy Briggs <rgb@redhat.com>
Sr. S/W Engineer, Kernel Security, Base Operating Systems
Remote, Ottawa, Red Hat Canada
IRC: rgb, SunRaycer
Voice: +1.647.777.2635, Internal: (81) 32635

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [RFC PATCH V1 01/12] audit: add container id
  2018-03-04 15:01     ` Paul Moore
@ 2018-03-05  8:16       ` Richard Guy Briggs
  0 siblings, 0 replies; 31+ messages in thread
From: Richard Guy Briggs @ 2018-03-05  8:16 UTC (permalink / raw)
  To: Paul Moore
  Cc: Serge E. Hallyn, ebiederm, simo, jlayton, carlos, linux-api,
	containers, LKML, Eric Paris, dhowells, Linux-Audit Mailing List,
	viro, luto, netdev, linux-fsdevel, cgroups

On 2018-03-04 10:01, Paul Moore wrote:
> On Sat, Mar 3, 2018 at 4:19 AM, Serge E. Hallyn <serge@hallyn.com> wrote:
> > On Thu, Mar 01, 2018 at 02:41:04PM -0500, Richard Guy Briggs wrote:
> > ...
> >> +static inline bool audit_containerid_set(struct task_struct *tsk)
> >
> > Hi Richard,
> >
> > the calls to audit_containerid_set() confused me.  Could you make it
> > is_audit_containerid_set() or audit_containerid_isset()?
> 
> I haven't gone through the entire patchset yet, but I wanted to
> quickly comment on this ... I really dislike the
> function-names-as-sentences approach and would would greatly prefer
> audit_containerid_isset().

I'd be ok with this latter if necessary, but the naming mimics the
existing loginuid naming convention.

> >> +{
> >> +     return audit_get_containerid(tsk) != INVALID_CID;
> >> +}
> 
> paul moore

- RGB

--
Richard Guy Briggs <rgb@redhat.com>
Sr. S/W Engineer, Kernel Security, Base Operating Systems
Remote, Ottawa, Red Hat Canada
IRC: rgb, SunRaycer
Voice: +1.647.777.2635, Internal: (81) 32635

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [RFC PATCH V1 00/12] audit: implement container id
  2018-03-05  3:31   ` Richard Guy Briggs
@ 2018-03-05 13:27     ` Mimi Zohar
  0 siblings, 0 replies; 31+ messages in thread
From: Mimi Zohar @ 2018-03-05 13:27 UTC (permalink / raw)
  To: Richard Guy Briggs
  Cc: cgroups, containers, linux-api, Linux-Audit Mailing List,
	linux-fsdevel, LKML, netdev, mszeredi, ebiederm, simo, jlayton,
	carlos, dhowells, viro, luto, eparis, trondmy, serge

On Sun, 2018-03-04 at 22:31 -0500, Richard Guy Briggs wrote:
> On 2018-03-04 16:55, Mimi Zohar wrote:
> > On Thu, 2018-03-01 at 14:41 -0500, Richard Guy Briggs wrote:
> > > Implement audit kernel container ID.
> > > 
> > > This patchset is a preliminary RFC based on the proposal document (V3)
> > > posted:
> > > 	https://www.redhat.com/archives/linux-audit/2018-January/msg00014.html
> > > 
> > > The first patch implements the proc fs write to set the audit container
> > > ID of a process, emitting an AUDIT_CONTAINER record.
> > > 
> > > The second implements an auxiliary syscall record AUDIT_CONTAINER_INFO
> > > if a container ID is present on a task.
> > > 
> > > The third adds filtering to the exit, exclude and user lists.
> > > 
> > > The 4th, implements reading the container ID from the proc filesystem
> > > for debugging.  This isn't planned for upstream inclusion.
> > > 
> > > The 5th adds signal and ptrace support.
> > > 
> > > The 6th attempts to create a local audit context to be able to bind a
> > > standalone record with the container ID record.
> > > 
> > > The 7th, 8th, 9th, 10th patches add container ID records to standalone
> > > records.  Some of these may end up being syscall auxiliary records and
> > > won't need this specific support since they'll be supported via
> > > syscalls.
> > > 
> > > The 11th is a temporary workaround due to the AUDIT_CONTAINER records
> > > not showing up as do AUDIT_LOGIN records.  I suspect this is due to its
> > > range (1000 vs 1300), but the intent is to solve it.
> > > 
> > > The 12th adds debug information not intended for upstream for those
> > > brave souls wanting to tinker with it in this early state.
> > > 
> > > Feedback please!
> > 
> > Which tree can this patch set be applied to?
> 
> git://git.kernel.org/pub/scm/linux/kernel/git/pcmoore/audit.git next

Thanks, that worked.  In case anyone else is trying to apply these
patches to a 4.16.0-rc based kernel, commit 4e7e3adbba52 ("Expand
various INIT_* macros and remove") moved .sessionid
to init/init_task.c.

Mimi

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [RFC PATCH V1 00/12] audit: implement container id
  2018-03-01 19:41 [RFC PATCH V1 00/12] audit: implement container id Richard Guy Briggs
                   ` (12 preceding siblings ...)
  2018-03-04 21:55 ` [RFC PATCH V1 00/12] audit: implement " Mimi Zohar
@ 2018-03-06 15:04 ` Serge E. Hallyn
  13 siblings, 0 replies; 31+ messages in thread
From: Serge E. Hallyn @ 2018-03-06 15:04 UTC (permalink / raw)
  To: Richard Guy Briggs
  Cc: cgroups, containers, linux-api, Linux-Audit Mailing List,
	linux-fsdevel, LKML, netdev, mszeredi, luto, jlayton, carlos,
	viro, dhowells, simo, trondmy, eparis, serge, ebiederm, madzcar

Quoting Richard Guy Briggs (rgb@redhat.com):
> Implement audit kernel container ID.
> 
> This patchset is a preliminary RFC based on the proposal document (V3)
> posted:
> 	https://www.redhat.com/archives/linux-audit/2018-January/msg00014.html

Patchset looks good to me.

Acked-by: Serge Hallyn <serge@hallyn.com>

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [RFC PATCH V1 01/12] audit: add container id
  2018-03-01 19:41 ` [RFC PATCH V1 01/12] audit: add " Richard Guy Briggs
  2018-03-02  1:41   ` Richard Guy Briggs
  2018-03-03  9:19   ` Serge E. Hallyn
@ 2018-03-15 20:27   ` Stefan Berger
  2018-03-16  3:58     ` Richard Guy Briggs
  2 siblings, 1 reply; 31+ messages in thread
From: Stefan Berger @ 2018-03-15 20:27 UTC (permalink / raw)
  To: Richard Guy Briggs, cgroups, containers, linux-api,
	Linux-Audit Mailing List, linux-fsdevel, LKML, netdev
  Cc: mszeredi, luto, jlayton, carlos, viro, dhowells, simo, trondmy,
	eparis, serge, ebiederm, madzcar

On 03/01/2018 02:41 PM, Richard Guy Briggs wrote:
> Implement the proc fs write to set the audit container ID of a process,
> emitting an AUDIT_CONTAINER record to document the event.
>
> This is a write from the container orchestrator task to a proc entry of
> the form /proc/PID/containerid where PID is the process ID of the newly
> created task that is to become the first task in a container, or an
> additional task added to a container.
>
> The write expects up to a u64 value (unset: 18446744073709551615).
>
> This will produce a record such as this:
> type=UNKNOWN[1333] msg=audit(1519903238.968:261): op=set pid=596 uid=0 subj=unconfined_u:unconfined_r:unconfined_t:s0-s0:c0.c1023 auid=0 tty=pts0 ses=1 opid=596 old-contid=18446744073709551615 contid=123455 res=0
>
> The "op" field indicates an initial set.  The "pid" to "ses" fields are
> the orchestrator while the "opid" field is the object's PID, the process
> being "contained".  Old and new container ID values are given in the
> "contid" fields, while res indicates its success.
>
> It is not permitted to self-set, unset or re-set the container ID.  A
> child inherits its parent's container ID, but then can be set only once
> after.
>
> See: https://github.com/linux-audit/audit-kernel/issues/32
>
>
>   /* audit_rule_data supports filter rules with both integer and string
>    * fields.  It corresponds with AUDIT_ADD_RULE, AUDIT_DEL_RULE and
> diff --git a/kernel/auditsc.c b/kernel/auditsc.c
> index 4e0a4ac..0ee1e59 100644
> --- a/kernel/auditsc.c
> +++ b/kernel/auditsc.c
> @@ -2073,6 +2073,92 @@ int audit_set_loginuid(kuid_t loginuid)
>   	return rc;
>   }
>
> +static int audit_set_containerid_perm(struct task_struct *task, u64 containerid)
> +{
> +	struct task_struct *parent;
> +	u64 pcontainerid, ccontainerid;
> +	pid_t ppid;
> +
> +	/* Don't allow to set our own containerid */
> +	if (current == task)
> +		return -EPERM;
> +	/* Don't allow the containerid to be unset */
> +	if (!cid_valid(containerid))
> +		return -EINVAL;
> +	/* if we don't have caps, reject */
> +	if (!capable(CAP_AUDIT_CONTROL))
> +		return -EPERM;
> +	/* if containerid is unset, allow */
> +	if (!audit_containerid_set(task))
> +		return 0;

I am wondering whether there should be a check for the target process 
that will receive the containerid to not have CAP_SYS_ADMIN that would 
otherwise allow it to arbitrarily unshare()/clone() and leave the set of 
namespaces that may make up the container whose containerid we assign here?

> +	/* it is already set, and not inherited from the parent, reject */
> +	ccontainerid = audit_get_containerid(task);
> +	rcu_read_lock();
> +	parent = rcu_dereference(task->real_parent);
> +	rcu_read_unlock();
> +	task_lock(parent);
> +	pcontainerid = audit_get_containerid(parent);
> +	ppid = task_tgid_nr(parent);
ppid not needed...

> +	task_unlock(parent);
> +	if (ccontainerid != pcontainerid)
> +		return -EPERM;
> +	return 0;
> +}
> +
> +static void audit_log_set_containerid(struct task_struct *task, u64 oldcontainerid,
> +				      u64 containerid, int rc)
> +{
> +	struct audit_buffer *ab;
> +	uid_t uid;
> +	struct tty_struct *tty;
> +
> +	if (!audit_enabled)
> +		return;
> +
> +	ab = audit_log_start(NULL, GFP_KERNEL, AUDIT_CONTAINER);
> +	if (!ab)
> +		return;
> +
> +	uid = from_kuid(&init_user_ns, task_uid(current));
> +	tty = audit_get_tty(current);
> +
> +	audit_log_format(ab, "op=set pid=%d uid=%u", task_tgid_nr(current), uid);
> +	audit_log_task_context(ab);
> +	audit_log_format(ab, " auid=%u tty=%s ses=%u opid=%d old-contid=%llu contid=%llu res=%d",
> +			 from_kuid(&init_user_ns, audit_get_loginuid(current)),
> +			 tty ? tty_name(tty) : "(none)", audit_get_sessionid(current),
> +			 task_tgid_nr(task), oldcontainerid, containerid, !rc);
> +
> +	audit_put_tty(tty);
> +	audit_log_end(ab);
> +}
> +
> +/**
> + * audit_set_containerid - set current task's audit_context containerid
> + * @containerid: containerid value
> + *
> + * Returns 0 on success, -EPERM on permission failure.
> + *
> + * Called (set) from fs/proc/base.c::proc_containerid_write().
> + */
> +int audit_set_containerid(struct task_struct *task, u64 containerid)
> +{
> +	u64 oldcontainerid;
> +	int rc;
> +
> +	oldcontainerid = audit_get_containerid(task);
> +
> +	rc = audit_set_containerid_perm(task, containerid);
> +	if (!rc) {
> +		task_lock(task);
> +		task->containerid = containerid;
> +		task_unlock(task);
> +	}
> +
> +	audit_log_set_containerid(task, oldcontainerid, containerid, rc);
> +	return rc;
> +}
> +
>   /**
>    * __audit_mq_open - record audit data for a POSIX MQ open
>    * @oflag: open flag

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [RFC PATCH V1 01/12] audit: add container id
  2018-03-15 20:27   ` Stefan Berger
@ 2018-03-16  3:58     ` Richard Guy Briggs
  2018-04-18 18:45       ` Stefan Berger
  0 siblings, 1 reply; 31+ messages in thread
From: Richard Guy Briggs @ 2018-03-16  3:58 UTC (permalink / raw)
  To: Stefan Berger
  Cc: cgroups, containers, linux-api, Linux-Audit Mailing List,
	linux-fsdevel, LKML, netdev, mszeredi, luto, jlayton, carlos,
	viro, dhowells, simo, trondmy, eparis, serge, ebiederm, madzcar

On 2018-03-15 16:27, Stefan Berger wrote:
> On 03/01/2018 02:41 PM, Richard Guy Briggs wrote:
> > Implement the proc fs write to set the audit container ID of a process,
> > emitting an AUDIT_CONTAINER record to document the event.
> > 
> > This is a write from the container orchestrator task to a proc entry of
> > the form /proc/PID/containerid where PID is the process ID of the newly
> > created task that is to become the first task in a container, or an
> > additional task added to a container.
> > 
> > The write expects up to a u64 value (unset: 18446744073709551615).
> > 
> > This will produce a record such as this:
> > type=UNKNOWN[1333] msg=audit(1519903238.968:261): op=set pid=596 uid=0 subj=unconfined_u:unconfined_r:unconfined_t:s0-s0:c0.c1023 auid=0 tty=pts0 ses=1 opid=596 old-contid=18446744073709551615 contid=123455 res=0
> > 
> > The "op" field indicates an initial set.  The "pid" to "ses" fields are
> > the orchestrator while the "opid" field is the object's PID, the process
> > being "contained".  Old and new container ID values are given in the
> > "contid" fields, while res indicates its success.
> > 
> > It is not permitted to self-set, unset or re-set the container ID.  A
> > child inherits its parent's container ID, but then can be set only once
> > after.
> > 
> > See: https://github.com/linux-audit/audit-kernel/issues/32
> > 
> > 
> >   /* audit_rule_data supports filter rules with both integer and string
> >    * fields.  It corresponds with AUDIT_ADD_RULE, AUDIT_DEL_RULE and
> > diff --git a/kernel/auditsc.c b/kernel/auditsc.c
> > index 4e0a4ac..0ee1e59 100644
> > --- a/kernel/auditsc.c
> > +++ b/kernel/auditsc.c
> > @@ -2073,6 +2073,92 @@ int audit_set_loginuid(kuid_t loginuid)
> >   	return rc;
> >   }
> > 
> > +static int audit_set_containerid_perm(struct task_struct *task, u64 containerid)
> > +{
> > +	struct task_struct *parent;
> > +	u64 pcontainerid, ccontainerid;
> > +	pid_t ppid;
> > +
> > +	/* Don't allow to set our own containerid */
> > +	if (current == task)
> > +		return -EPERM;
> > +	/* Don't allow the containerid to be unset */
> > +	if (!cid_valid(containerid))
> > +		return -EINVAL;
> > +	/* if we don't have caps, reject */
> > +	if (!capable(CAP_AUDIT_CONTROL))
> > +		return -EPERM;
> > +	/* if containerid is unset, allow */
> > +	if (!audit_containerid_set(task))
> > +		return 0;
> 
> I am wondering whether there should be a check for the target process that
> will receive the containerid to not have CAP_SYS_ADMIN that would otherwise
> allow it to arbitrarily unshare()/clone() and leave the set of namespaces
> that may make up the container whose containerid we assign here?

This is a reasonable question.  This has been debated and I understood
the conclusion was that without a clear definition of a "container", the
task still remains in that container that just now has more
sub-namespaces (in the case of hierarchical namespaces), we don't want
to restrict it in such a way and that allows it to create nested
containers.  I see setns being more problematic if it could switch to
another existing namespace that was set up by the orchestrator for a
different container.  The coming v2 patchset acknowledges this situation
with the network namespace being potentially shared by multiple
containers.

This is the motivation for the code below that allows to set the
containerid even if it is already inherited from its parent.

> > +	/* it is already set, and not inherited from the parent, reject */
> > +	ccontainerid = audit_get_containerid(task);
> > +	rcu_read_lock();
> > +	parent = rcu_dereference(task->real_parent);
> > +	rcu_read_unlock();
> > +	task_lock(parent);
> > +	pcontainerid = audit_get_containerid(parent);
> > +	ppid = task_tgid_nr(parent);
>
> ppid not needed...

Thanks for catching this.  It was the vestige of a failed devel
experiment that didn't flush that useless appendage.  :-)

> > +	task_unlock(parent);
> > +	if (ccontainerid != pcontainerid)
> > +		return -EPERM;
> > +	return 0;
> > +}
> > +
> > +static void audit_log_set_containerid(struct task_struct *task, u64 oldcontainerid,
> > +				      u64 containerid, int rc)
> > +{
> > +	struct audit_buffer *ab;
> > +	uid_t uid;
> > +	struct tty_struct *tty;
> > +
> > +	if (!audit_enabled)
> > +		return;
> > +
> > +	ab = audit_log_start(NULL, GFP_KERNEL, AUDIT_CONTAINER);
> > +	if (!ab)
> > +		return;
> > +
> > +	uid = from_kuid(&init_user_ns, task_uid(current));
> > +	tty = audit_get_tty(current);
> > +
> > +	audit_log_format(ab, "op=set pid=%d uid=%u", task_tgid_nr(current), uid);
> > +	audit_log_task_context(ab);
> > +	audit_log_format(ab, " auid=%u tty=%s ses=%u opid=%d old-contid=%llu contid=%llu res=%d",
> > +			 from_kuid(&init_user_ns, audit_get_loginuid(current)),
> > +			 tty ? tty_name(tty) : "(none)", audit_get_sessionid(current),
> > +			 task_tgid_nr(task), oldcontainerid, containerid, !rc);
> > +
> > +	audit_put_tty(tty);
> > +	audit_log_end(ab);
> > +}
> > +
> > +/**
> > + * audit_set_containerid - set current task's audit_context containerid
> > + * @containerid: containerid value
> > + *
> > + * Returns 0 on success, -EPERM on permission failure.
> > + *
> > + * Called (set) from fs/proc/base.c::proc_containerid_write().
> > + */
> > +int audit_set_containerid(struct task_struct *task, u64 containerid)
> > +{
> > +	u64 oldcontainerid;
> > +	int rc;
> > +
> > +	oldcontainerid = audit_get_containerid(task);
> > +
> > +	rc = audit_set_containerid_perm(task, containerid);
> > +	if (!rc) {
> > +		task_lock(task);
> > +		task->containerid = containerid;
> > +		task_unlock(task);
> > +	}
> > +
> > +	audit_log_set_containerid(task, oldcontainerid, containerid, rc);
> > +	return rc;
> > +}
> > +
> >   /**
> >    * __audit_mq_open - record audit data for a POSIX MQ open
> >    * @oflag: open flag
> 
> 

- RGB

--
Richard Guy Briggs <rgb@redhat.com>
Sr. S/W Engineer, Kernel Security, Base Operating Systems
Remote, Ottawa, Red Hat Canada
IRC: rgb, SunRaycer
Voice: +1.647.777.2635, Internal: (81) 32635

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [RFC PATCH V1 01/12] audit: add container id
  2018-03-16  3:58     ` Richard Guy Briggs
@ 2018-04-18 18:45       ` Stefan Berger
  2018-04-18 19:23         ` Richard Guy Briggs
  0 siblings, 1 reply; 31+ messages in thread
From: Stefan Berger @ 2018-04-18 18:45 UTC (permalink / raw)
  To: Richard Guy Briggs
  Cc: cgroups, containers, linux-api, Linux-Audit Mailing List,
	linux-fsdevel, LKML, netdev, mszeredi, luto, jlayton, carlos,
	viro, dhowells, simo, trondmy, eparis, serge, ebiederm, madzcar

On 03/15/2018 11:58 PM, Richard Guy Briggs wrote:
> On 2018-03-15 16:27, Stefan Berger wrote:
>> On 03/01/2018 02:41 PM, Richard Guy Briggs wrote:
>>> Implement the proc fs write to set the audit container ID of a process,
>>> emitting an AUDIT_CONTAINER record to document the event.
>>>
>>> This is a write from the container orchestrator task to a proc entry of
>>> the form /proc/PID/containerid where PID is the process ID of the newly
>>> created task that is to become the first task in a container, or an
>>> additional task added to a container.
>>>
>>> The write expects up to a u64 value (unset: 18446744073709551615).
>>>
>>> This will produce a record such as this:
>>> type=UNKNOWN[1333] msg=audit(1519903238.968:261): op=set pid=596 uid=0 subj=unconfined_u:unconfined_r:unconfined_t:s0-s0:c0.c1023 auid=0 tty=pts0 ses=1 opid=596 old-contid=18446744073709551615 contid=123455 res=0
>>>
>>> The "op" field indicates an initial set.  The "pid" to "ses" fields are
>>> the orchestrator while the "opid" field is the object's PID, the process
>>> being "contained".  Old and new container ID values are given in the
>>> "contid" fields, while res indicates its success.
>>>
>>> It is not permitted to self-set, unset or re-set the container ID.  A
>>> child inherits its parent's container ID, but then can be set only once
>>> after.
>>>
>>> See: https://github.com/linux-audit/audit-kernel/issues/32
>>>
>>>
>>>    /* audit_rule_data supports filter rules with both integer and string
>>>     * fields.  It corresponds with AUDIT_ADD_RULE, AUDIT_DEL_RULE and
>>> diff --git a/kernel/auditsc.c b/kernel/auditsc.c
>>> index 4e0a4ac..0ee1e59 100644
>>> --- a/kernel/auditsc.c
>>> +++ b/kernel/auditsc.c
>>> @@ -2073,6 +2073,92 @@ int audit_set_loginuid(kuid_t loginuid)
>>>    	return rc;
>>>    }
>>>
>>> +static int audit_set_containerid_perm(struct task_struct *task, u64 containerid)
>>> +{
>>> +	struct task_struct *parent;
>>> +	u64 pcontainerid, ccontainerid;
>>> +	pid_t ppid;
>>> +
>>> +	/* Don't allow to set our own containerid */
>>> +	if (current == task)
>>> +		return -EPERM;
>>> +	/* Don't allow the containerid to be unset */
>>> +	if (!cid_valid(containerid))
>>> +		return -EINVAL;
>>> +	/* if we don't have caps, reject */
>>> +	if (!capable(CAP_AUDIT_CONTROL))
>>> +		return -EPERM;
>>> +	/* if containerid is unset, allow */
>>> +	if (!audit_containerid_set(task))
>>> +		return 0;
>> I am wondering whether there should be a check for the target process that
>> will receive the containerid to not have CAP_SYS_ADMIN that would otherwise
>> allow it to arbitrarily unshare()/clone() and leave the set of namespaces
>> that may make up the container whose containerid we assign here?
> This is a reasonable question.  This has been debated and I understood
> the conclusion was that without a clear definition of a "container", the
> task still remains in that container that just now has more
> sub-namespaces (in the case of hierarchical namespaces), we don't want
> to restrict it in such a way and that allows it to create nested
> containers.  I see setns being more problematic if it could switch to
> another existing namespace that was set up by the orchestrator for a
> different container.  The coming v2 patchset acknowledges this situation
> with the network namespace being potentially shared by multiple
> containers.

Are you going to post v2 soon? We would like to build on top of it for 
IMA namespacing and auditing inside of IMA namespaces.

    Stefan

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [RFC PATCH V1 01/12] audit: add container id
  2018-04-18 18:45       ` Stefan Berger
@ 2018-04-18 19:23         ` Richard Guy Briggs
  2018-04-18 19:39           ` Stefan Berger
  0 siblings, 1 reply; 31+ messages in thread
From: Richard Guy Briggs @ 2018-04-18 19:23 UTC (permalink / raw)
  To: Stefan Berger
  Cc: mszeredi, ebiederm, simo, jlayton, carlos, linux-api, containers,
	LKML, eparis, dhowells, Linux-Audit Mailing List, viro, luto,
	netdev, linux-fsdevel, cgroups, serge, trondmy

On 2018-04-18 14:45, Stefan Berger wrote:
> On 03/15/2018 11:58 PM, Richard Guy Briggs wrote:
> > On 2018-03-15 16:27, Stefan Berger wrote:
> > > On 03/01/2018 02:41 PM, Richard Guy Briggs wrote:
> > > > Implement the proc fs write to set the audit container ID of a process,
> > > > emitting an AUDIT_CONTAINER record to document the event.
> > > > 
> > > > This is a write from the container orchestrator task to a proc entry of
> > > > the form /proc/PID/containerid where PID is the process ID of the newly
> > > > created task that is to become the first task in a container, or an
> > > > additional task added to a container.
> > > > 
> > > > The write expects up to a u64 value (unset: 18446744073709551615).
> > > > 
> > > > This will produce a record such as this:
> > > > type=UNKNOWN[1333] msg=audit(1519903238.968:261): op=set pid=596 uid=0 subj=unconfined_u:unconfined_r:unconfined_t:s0-s0:c0.c1023 auid=0 tty=pts0 ses=1 opid=596 old-contid=18446744073709551615 contid=123455 res=0
> > > > 
> > > > The "op" field indicates an initial set.  The "pid" to "ses" fields are
> > > > the orchestrator while the "opid" field is the object's PID, the process
> > > > being "contained".  Old and new container ID values are given in the
> > > > "contid" fields, while res indicates its success.
> > > > 
> > > > It is not permitted to self-set, unset or re-set the container ID.  A
> > > > child inherits its parent's container ID, but then can be set only once
> > > > after.
> > > > 
> > > > See: https://github.com/linux-audit/audit-kernel/issues/32
> > > > 
> > > > 
> > > >    /* audit_rule_data supports filter rules with both integer and string
> > > >     * fields.  It corresponds with AUDIT_ADD_RULE, AUDIT_DEL_RULE and
> > > > diff --git a/kernel/auditsc.c b/kernel/auditsc.c
> > > > index 4e0a4ac..0ee1e59 100644
> > > > --- a/kernel/auditsc.c
> > > > +++ b/kernel/auditsc.c
> > > > @@ -2073,6 +2073,92 @@ int audit_set_loginuid(kuid_t loginuid)
> > > >    	return rc;
> > > >    }
> > > > 
> > > > +static int audit_set_containerid_perm(struct task_struct *task, u64 containerid)
> > > > +{
> > > > +	struct task_struct *parent;
> > > > +	u64 pcontainerid, ccontainerid;
> > > > +	pid_t ppid;
> > > > +
> > > > +	/* Don't allow to set our own containerid */
> > > > +	if (current == task)
> > > > +		return -EPERM;
> > > > +	/* Don't allow the containerid to be unset */
> > > > +	if (!cid_valid(containerid))
> > > > +		return -EINVAL;
> > > > +	/* if we don't have caps, reject */
> > > > +	if (!capable(CAP_AUDIT_CONTROL))
> > > > +		return -EPERM;
> > > > +	/* if containerid is unset, allow */
> > > > +	if (!audit_containerid_set(task))
> > > > +		return 0;
> > > I am wondering whether there should be a check for the target process that
> > > will receive the containerid to not have CAP_SYS_ADMIN that would otherwise
> > > allow it to arbitrarily unshare()/clone() and leave the set of namespaces
> > > that may make up the container whose containerid we assign here?
> > This is a reasonable question.  This has been debated and I understood
> > the conclusion was that without a clear definition of a "container", the
> > task still remains in that container that just now has more
> > sub-namespaces (in the case of hierarchical namespaces), we don't want
> > to restrict it in such a way and that allows it to create nested
> > containers.  I see setns being more problematic if it could switch to
> > another existing namespace that was set up by the orchestrator for a
> > different container.  The coming v2 patchset acknowledges this situation
> > with the network namespace being potentially shared by multiple
> > containers.
> 
> Are you going to post v2 soon? We would like to build on top of it for IMA
> namespacing and auditing inside of IMA namespaces.

I don't know if it addresses your specific needs, but V2 was posted on
March 16th along with userspace patches:
	https://www.redhat.com/archives/linux-audit/2018-March/msg00110.html
	https://www.redhat.com/archives/linux-audit/2018-March/msg00124.html

V3 is pending.

>    Stefan

- RGB

--
Richard Guy Briggs <rgb@redhat.com>
Sr. S/W Engineer, Kernel Security, Base Operating Systems
Remote, Ottawa, Red Hat Canada
IRC: rgb, SunRaycer
Voice: +1.647.777.2635, Internal: (81) 32635

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [RFC PATCH V1 01/12] audit: add container id
  2018-04-18 19:23         ` Richard Guy Briggs
@ 2018-04-18 19:39           ` Stefan Berger
  2018-04-18 19:51             ` Richard Guy Briggs
  0 siblings, 1 reply; 31+ messages in thread
From: Stefan Berger @ 2018-04-18 19:39 UTC (permalink / raw)
  To: Richard Guy Briggs
  Cc: mszeredi, ebiederm, simo, jlayton, carlos, linux-api, containers,
	LKML, eparis, dhowells, Linux-Audit Mailing List, viro, luto,
	netdev, linux-fsdevel, cgroups, serge, trondmy

On 04/18/2018 03:23 PM, Richard Guy Briggs wrote:
> On 2018-04-18 14:45, Stefan Berger wrote:
>> On 03/15/2018 11:58 PM, Richard Guy Briggs wrote:
>>> On 2018-03-15 16:27, Stefan Berger wrote:
>>>> On 03/01/2018 02:41 PM, Richard Guy Briggs wrote:
>>>>> Implement the proc fs write to set the audit container ID of a process,
>>>>> emitting an AUDIT_CONTAINER record to document the event.
>>>>>
>>>>> This is a write from the container orchestrator task to a proc entry of
>>>>> the form /proc/PID/containerid where PID is the process ID of the newly
>>>>> created task that is to become the first task in a container, or an
>>>>> additional task added to a container.
>>>>>
>>>>> The write expects up to a u64 value (unset: 18446744073709551615).
>>>>>
>>>>> This will produce a record such as this:
>>>>> type=UNKNOWN[1333] msg=audit(1519903238.968:261): op=set pid=596 uid=0 subj=unconfined_u:unconfined_r:unconfined_t:s0-s0:c0.c1023 auid=0 tty=pts0 ses=1 opid=596 old-contid=18446744073709551615 contid=123455 res=0
>>>>>
>>>>> The "op" field indicates an initial set.  The "pid" to "ses" fields are
>>>>> the orchestrator while the "opid" field is the object's PID, the process
>>>>> being "contained".  Old and new container ID values are given in the
>>>>> "contid" fields, while res indicates its success.
>>>>>
>>>>> It is not permitted to self-set, unset or re-set the container ID.  A
>>>>> child inherits its parent's container ID, but then can be set only once
>>>>> after.
>>>>>
>>>>> See: https://github.com/linux-audit/audit-kernel/issues/32
>>>>>
>>>>>
>>>>>     /* audit_rule_data supports filter rules with both integer and string
>>>>>      * fields.  It corresponds with AUDIT_ADD_RULE, AUDIT_DEL_RULE and
>>>>> diff --git a/kernel/auditsc.c b/kernel/auditsc.c
>>>>> index 4e0a4ac..0ee1e59 100644
>>>>> --- a/kernel/auditsc.c
>>>>> +++ b/kernel/auditsc.c
>>>>> @@ -2073,6 +2073,92 @@ int audit_set_loginuid(kuid_t loginuid)
>>>>>     	return rc;
>>>>>     }
>>>>>
>>>>> +static int audit_set_containerid_perm(struct task_struct *task, u64 containerid)
>>>>> +{
>>>>> +	struct task_struct *parent;
>>>>> +	u64 pcontainerid, ccontainerid;
>>>>> +	pid_t ppid;
>>>>> +
>>>>> +	/* Don't allow to set our own containerid */
>>>>> +	if (current == task)
>>>>> +		return -EPERM;
>>>>> +	/* Don't allow the containerid to be unset */
>>>>> +	if (!cid_valid(containerid))
>>>>> +		return -EINVAL;
>>>>> +	/* if we don't have caps, reject */
>>>>> +	if (!capable(CAP_AUDIT_CONTROL))
>>>>> +		return -EPERM;
>>>>> +	/* if containerid is unset, allow */
>>>>> +	if (!audit_containerid_set(task))
>>>>> +		return 0;
>>>> I am wondering whether there should be a check for the target process that
>>>> will receive the containerid to not have CAP_SYS_ADMIN that would otherwise
>>>> allow it to arbitrarily unshare()/clone() and leave the set of namespaces
>>>> that may make up the container whose containerid we assign here?
>>> This is a reasonable question.  This has been debated and I understood
>>> the conclusion was that without a clear definition of a "container", the
>>> task still remains in that container that just now has more
>>> sub-namespaces (in the case of hierarchical namespaces), we don't want
>>> to restrict it in such a way and that allows it to create nested
>>> containers.  I see setns being more problematic if it could switch to
>>> another existing namespace that was set up by the orchestrator for a
>>> different container.  The coming v2 patchset acknowledges this situation
>>> with the network namespace being potentially shared by multiple
>>> containers.
>> Are you going to post v2 soon? We would like to build on top of it for IMA
>> namespacing and auditing inside of IMA namespaces.
> I don't know if it addresses your specific needs, but V2 was posted on
> March 16th along with userspace patches:
> 	https://www.redhat.com/archives/linux-audit/2018-March/msg00110.html
> 	https://www.redhat.com/archives/linux-audit/2018-March/msg00124.html
>
> V3 is pending.
Thanks. I hadn't actually looked at primarily due to the ghak and ghau 
in the title. Whatever these may mean.

Does V2 or will V3 prevent a privileged process to setns() to a whole 
different set of namespaces and still be audited with that initial 
container id ?

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [RFC PATCH V1 01/12] audit: add container id
  2018-04-18 19:39           ` Stefan Berger
@ 2018-04-18 19:51             ` Richard Guy Briggs
  0 siblings, 0 replies; 31+ messages in thread
From: Richard Guy Briggs @ 2018-04-18 19:51 UTC (permalink / raw)
  To: Stefan Berger
  Cc: mszeredi, ebiederm, simo, jlayton, carlos, linux-api, containers,
	LKML, eparis, dhowells, Linux-Audit Mailing List, viro, luto,
	netdev, linux-fsdevel, cgroups, serge, trondmy

On 2018-04-18 15:39, Stefan Berger wrote:
> On 04/18/2018 03:23 PM, Richard Guy Briggs wrote:
> > On 2018-04-18 14:45, Stefan Berger wrote:
> > > On 03/15/2018 11:58 PM, Richard Guy Briggs wrote:
> > > > On 2018-03-15 16:27, Stefan Berger wrote:
> > > > > On 03/01/2018 02:41 PM, Richard Guy Briggs wrote:
> > > > > > Implement the proc fs write to set the audit container ID of a process,
> > > > > > emitting an AUDIT_CONTAINER record to document the event.
> > > > > > 
> > > > > > This is a write from the container orchestrator task to a proc entry of
> > > > > > the form /proc/PID/containerid where PID is the process ID of the newly
> > > > > > created task that is to become the first task in a container, or an
> > > > > > additional task added to a container.
> > > > > > 
> > > > > > The write expects up to a u64 value (unset: 18446744073709551615).
> > > > > > 
> > > > > > This will produce a record such as this:
> > > > > > type=UNKNOWN[1333] msg=audit(1519903238.968:261): op=set pid=596 uid=0 subj=unconfined_u:unconfined_r:unconfined_t:s0-s0:c0.c1023 auid=0 tty=pts0 ses=1 opid=596 old-contid=18446744073709551615 contid=123455 res=0
> > > > > > 
> > > > > > The "op" field indicates an initial set.  The "pid" to "ses" fields are
> > > > > > the orchestrator while the "opid" field is the object's PID, the process
> > > > > > being "contained".  Old and new container ID values are given in the
> > > > > > "contid" fields, while res indicates its success.
> > > > > > 
> > > > > > It is not permitted to self-set, unset or re-set the container ID.  A
> > > > > > child inherits its parent's container ID, but then can be set only once
> > > > > > after.
> > > > > > 
> > > > > > See: https://github.com/linux-audit/audit-kernel/issues/32
> > > > > > 
> > > > > > 
> > > > > >     /* audit_rule_data supports filter rules with both integer and string
> > > > > >      * fields.  It corresponds with AUDIT_ADD_RULE, AUDIT_DEL_RULE and
> > > > > > diff --git a/kernel/auditsc.c b/kernel/auditsc.c
> > > > > > index 4e0a4ac..0ee1e59 100644
> > > > > > --- a/kernel/auditsc.c
> > > > > > +++ b/kernel/auditsc.c
> > > > > > @@ -2073,6 +2073,92 @@ int audit_set_loginuid(kuid_t loginuid)
> > > > > >     	return rc;
> > > > > >     }
> > > > > > 
> > > > > > +static int audit_set_containerid_perm(struct task_struct *task, u64 containerid)
> > > > > > +{
> > > > > > +	struct task_struct *parent;
> > > > > > +	u64 pcontainerid, ccontainerid;
> > > > > > +	pid_t ppid;
> > > > > > +
> > > > > > +	/* Don't allow to set our own containerid */
> > > > > > +	if (current == task)
> > > > > > +		return -EPERM;
> > > > > > +	/* Don't allow the containerid to be unset */
> > > > > > +	if (!cid_valid(containerid))
> > > > > > +		return -EINVAL;
> > > > > > +	/* if we don't have caps, reject */
> > > > > > +	if (!capable(CAP_AUDIT_CONTROL))
> > > > > > +		return -EPERM;
> > > > > > +	/* if containerid is unset, allow */
> > > > > > +	if (!audit_containerid_set(task))
> > > > > > +		return 0;
> > > > > I am wondering whether there should be a check for the target process that
> > > > > will receive the containerid to not have CAP_SYS_ADMIN that would otherwise
> > > > > allow it to arbitrarily unshare()/clone() and leave the set of namespaces
> > > > > that may make up the container whose containerid we assign here?
> > > > This is a reasonable question.  This has been debated and I understood
> > > > the conclusion was that without a clear definition of a "container", the
> > > > task still remains in that container that just now has more
> > > > sub-namespaces (in the case of hierarchical namespaces), we don't want
> > > > to restrict it in such a way and that allows it to create nested
> > > > containers.  I see setns being more problematic if it could switch to
> > > > another existing namespace that was set up by the orchestrator for a
> > > > different container.  The coming v2 patchset acknowledges this situation
> > > > with the network namespace being potentially shared by multiple
> > > > containers.
> > > Are you going to post v2 soon? We would like to build on top of it for IMA
> > > namespacing and auditing inside of IMA namespaces.
> > I don't know if it addresses your specific needs, but V2 was posted on
> > March 16th along with userspace patches:
> > 	https://www.redhat.com/archives/linux-audit/2018-March/msg00110.html
> > 	https://www.redhat.com/archives/linux-audit/2018-March/msg00124.html
> > 
> > V3 is pending.
> Thanks. I hadn't actually looked at primarily due to the ghak and ghau in
> the title. Whatever these may mean.

They are Github issue numbers:
GHAK: GitHub Audit Kernel
GHAU: GitHub Audit Userspace
GHAD: GitHub Audit Documentation
GHAT: GitHub Audit Testsuite

> Does V2 or will V3 prevent a privileged process to setns() to a whole
> different set of namespaces and still be audited with that initial container
> id ?

No, not significantly different from V1 in that respect.

It does not prevent setns(), but will maintain its containerid.

It will prevent games by blocking a child and parent from setting each
other's containerids.

It does check that the task being conainered does not yet have any
children or peer threads.

- RGB

--
Richard Guy Briggs <rgb@redhat.com>
Sr. S/W Engineer, Kernel Security, Base Operating Systems
Remote, Ottawa, Red Hat Canada
IRC: rgb, SunRaycer
Voice: +1.647.777.2635, Internal: (81) 32635

^ permalink raw reply	[flat|nested] 31+ messages in thread

end of thread, other threads:[~2018-04-18 19:57 UTC | newest]

Thread overview: 31+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-03-01 19:41 [RFC PATCH V1 00/12] audit: implement container id Richard Guy Briggs
2018-03-01 19:41 ` [RFC PATCH V1 01/12] audit: add " Richard Guy Briggs
2018-03-02  1:41   ` Richard Guy Briggs
2018-03-02 15:48     ` Paul Moore
2018-03-02 18:23       ` Matthew Wilcox
2018-03-02 19:25         ` Paul Moore
2018-03-02 19:41           ` Paul Moore
2018-03-03  9:19   ` Serge E. Hallyn
2018-03-04 15:01     ` Paul Moore
2018-03-05  8:16       ` Richard Guy Briggs
2018-03-15 20:27   ` Stefan Berger
2018-03-16  3:58     ` Richard Guy Briggs
2018-04-18 18:45       ` Stefan Berger
2018-04-18 19:23         ` Richard Guy Briggs
2018-04-18 19:39           ` Stefan Berger
2018-04-18 19:51             ` Richard Guy Briggs
2018-03-01 19:41 ` [RFC PATCH V1 02/12] audit: log container info of syscalls Richard Guy Briggs
2018-03-01 19:41 ` [RFC PATCH V1 03/12] audit: add containerid filtering Richard Guy Briggs
2018-03-01 19:41 ` [RFC PATCH V1 04/12] audit: read container ID of a process Richard Guy Briggs
2018-03-01 19:41 ` [RFC PATCH V1 05/12] audit: add containerid support for ptrace and signals Richard Guy Briggs
2018-03-01 19:41 ` [RFC PATCH V1 06/12] audit: add support for non-syscall auxiliary records Richard Guy Briggs
2018-03-01 19:41 ` [RFC PATCH V1 07/12] audit: add container aux record to watch/tree/mark Richard Guy Briggs
2018-03-01 19:41 ` [RFC PATCH V1 08/12] audit: add containerid support for tty_audit Richard Guy Briggs
2018-03-01 19:41 ` [RFC PATCH V1 09/12] audit: add containerid support for config/feature/user records Richard Guy Briggs
2018-03-01 19:41 ` [RFC PATCH V1 10/12] audit: add containerid support for seccomp and anom_abend records Richard Guy Briggs
2018-03-01 19:41 ` [RFC PATCH V1 11/12] debug audit: add container id Richard Guy Briggs
2018-03-01 19:41 ` [RFC PATCH V1 12/12] debug! " Richard Guy Briggs
2018-03-04 21:55 ` [RFC PATCH V1 00/12] audit: implement " Mimi Zohar
2018-03-05  3:31   ` Richard Guy Briggs
2018-03-05 13:27     ` Mimi Zohar
2018-03-06 15:04 ` Serge E. Hallyn

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).