[ghak90,V6,08/10] audit: add containerid filtering

Submitted by Richard Guy Briggs on April 9, 2019, 3:39 a.m.

Details

Message ID 0785ee2644804f3ec6af1243cc0dcf89709c1fd4.1554732921.git.rgb@redhat.com
State New
Series "audit: implement container identifier"
Headers show

Commit Message

Richard Guy Briggs April 9, 2019, 3:39 a.m.
Implement audit container identifier filtering using the AUDIT_CONTID
field name to send an 8-character string representing a u64 since the
value field is only u32.

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

The feature indicator is AUDIT_FEATURE_BITMAP_CONTAINERID.

Please see the github audit kernel issue for the contid filter feature:
  https://github.com/linux-audit/audit-kernel/issues/91
Please see the github audit userspace issue for filter additions:
  https://github.com/linux-audit/audit-userspace/issues/40
Please see the github audit testsuiite issue for the test case:
  https://github.com/linux-audit/audit-testsuite/issues/64
Please see the github audit wiki for the feature overview:
  https://github.com/linux-audit/audit-kernel/wiki/RFE-Audit-Container-ID
Signed-off-by: Richard Guy Briggs <rgb@redhat.com>
Acked-by: Serge Hallyn <serge@hallyn.com>
Acked-by: Neil Horman <nhorman@tuxdriver.com>
Reviewed-by: Ondrej Mosnacek <omosnace@redhat.com>
---
 include/linux/audit.h      |  1 +
 include/uapi/linux/audit.h |  5 ++++-
 kernel/audit.h             |  1 +
 kernel/auditfilter.c       | 47 ++++++++++++++++++++++++++++++++++++++++++++++
 kernel/auditsc.c           |  4 ++++
 5 files changed, 57 insertions(+), 1 deletion(-)

Patch hide | download patch | download mbox

diff --git a/include/linux/audit.h b/include/linux/audit.h
index ae03cfd5788a..6e42e6a10736 100644
--- a/include/linux/audit.h
+++ b/include/linux/audit.h
@@ -83,6 +83,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 10cc67926cf1..6d32eb1a96fb 100644
--- a/include/uapi/linux/audit.h
+++ b/include/uapi/linux/audit.h
@@ -266,6 +266,7 @@ 
 #define AUDIT_LOGINUID_SET	24
 #define AUDIT_SESSIONID	25	/* Session ID */
 #define AUDIT_FSTYPE	26	/* FileSystem Type */
+#define AUDIT_CONTID	27	/* Container ID */
 
 				/* These are ONLY useful when checking
 				 * at syscall exit time (AUDIT_AT_EXIT). */
@@ -346,6 +347,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	0x00000080
 
 #define AUDIT_FEATURE_BITMAP_ALL (AUDIT_FEATURE_BITMAP_BACKLOG_LIMIT | \
 				  AUDIT_FEATURE_BITMAP_BACKLOG_WAIT_TIME | \
@@ -353,7 +355,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)
 
 /* deprecated: AUDIT_VERSION_* */
 #define AUDIT_VERSION_LATEST 		AUDIT_FEATURE_BITMAP_ALL
diff --git a/kernel/audit.h b/kernel/audit.h
index 2a1a8b8a8019..3a40b608bf8d 100644
--- a/kernel/audit.h
+++ b/kernel/audit.h
@@ -230,6 +230,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 63f8b3f26fab..407b5bb3b4c6 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_CONTID:
 		if (f->op != Audit_not_equal && f->op != Audit_equal)
 			return -EINVAL;
 		break;
@@ -582,6 +583,14 @@  static struct audit_entry *audit_data_to_entry(struct audit_rule_data *data,
 			}
 			entry->rule.exe = audit_mark;
 			break;
+		case AUDIT_CONTID:
+			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;
 		}
 	}
 
@@ -664,6 +673,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_CONTID:
+			data->buflen += data->values[i] = sizeof(u64);
+			memcpy(bufp, &f->val64, sizeof(u64));
+			bufp += sizeof(u64);
+			break;
 		case AUDIT_LOGINUID_SET:
 			if (krule->pflags & AUDIT_LOGINUID_LEGACY && !f->val) {
 				data->fields[i] = AUDIT_LOGINUID;
@@ -750,6 +764,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_CONTID:
+			if (a->fields[i].val64 != b->fields[i].val64)
+				return 1;
+			break;
 		default:
 			if (a->fields[i].val != b->fields[i].val)
 				return 1;
@@ -1206,6 +1224,31 @@  int audit_comparator(u32 left, u32 op, u32 right)
 	}
 }
 
+int audit_comparator64(u64 left, u32 op, u64 right)
+{
+	switch (op) {
+	case Audit_equal:
+		return (left == right);
+	case Audit_not_equal:
+		return (left != right);
+	case Audit_lt:
+		return (left < right);
+	case Audit_le:
+		return (left <= right);
+	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) {
@@ -1344,6 +1387,10 @@  int audit_filter(int msgtype, unsigned int listtype)
 				result = audit_comparator(audit_loginuid_set(current),
 							  f->op, f->val);
 				break;
+			case AUDIT_CONTID:
+				result = audit_comparator64(audit_get_contid(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 b78734878832..deb3df8b62be 100644
--- a/kernel/auditsc.c
+++ b/kernel/auditsc.c
@@ -616,6 +616,10 @@  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_CONTID:
+			result = audit_comparator64(audit_get_contid(tsk),
+						    f->op, f->val64);
+			break;
 		case AUDIT_SUBJ_USER:
 		case AUDIT_SUBJ_ROLE:
 		case AUDIT_SUBJ_TYPE:

Comments

Paul Moore May 29, 2019, 10:16 p.m.
On Mon, Apr 8, 2019 at 11:41 PM Richard Guy Briggs <rgb@redhat.com> wrote:
>
> Implement audit container identifier filtering using the AUDIT_CONTID
> field name to send an 8-character string representing a u64 since the
> value field is only u32.
>
> Sending it as two u32 was considered, but gathering and comparing two
> fields was more complex.
>
> The feature indicator is AUDIT_FEATURE_BITMAP_CONTAINERID.
>
> Please see the github audit kernel issue for the contid filter feature:
>   https://github.com/linux-audit/audit-kernel/issues/91
> Please see the github audit userspace issue for filter additions:
>   https://github.com/linux-audit/audit-userspace/issues/40
> Please see the github audit testsuiite issue for the test case:
>   https://github.com/linux-audit/audit-testsuite/issues/64
> Please see the github audit wiki for the feature overview:
>   https://github.com/linux-audit/audit-kernel/wiki/RFE-Audit-Container-ID
> Signed-off-by: Richard Guy Briggs <rgb@redhat.com>
> Acked-by: Serge Hallyn <serge@hallyn.com>
> Acked-by: Neil Horman <nhorman@tuxdriver.com>
> Reviewed-by: Ondrej Mosnacek <omosnace@redhat.com>
> ---
>  include/linux/audit.h      |  1 +
>  include/uapi/linux/audit.h |  5 ++++-
>  kernel/audit.h             |  1 +
>  kernel/auditfilter.c       | 47 ++++++++++++++++++++++++++++++++++++++++++++++
>  kernel/auditsc.c           |  4 ++++
>  5 files changed, 57 insertions(+), 1 deletion(-)

...

> diff --git a/kernel/auditfilter.c b/kernel/auditfilter.c
> index 63f8b3f26fab..407b5bb3b4c6 100644
> --- a/kernel/auditfilter.c
> +++ b/kernel/auditfilter.c
> @@ -1206,6 +1224,31 @@ int audit_comparator(u32 left, u32 op, u32 right)
>         }
>  }
>
> +int audit_comparator64(u64 left, u32 op, u64 right)
> +{
> +       switch (op) {
> +       case Audit_equal:
> +               return (left == right);
> +       case Audit_not_equal:
> +               return (left != right);
> +       case Audit_lt:
> +               return (left < right);
> +       case Audit_le:
> +               return (left <= right);
> +       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();

A little birdy mentioned the BUG() here as a potential issue and while
I had ignored it in earlier patches because this is likely a
cut-n-paste from another audit comparator function, I took a closer
look this time.  It appears as though we will never have an invalid op
value as audit_data_to_entry()/audit_to_op() ensure that the op value
is a a known good value.  Removing the BUG() from all the audit
comparators is a separate issue, but I think it would be good to
remove it from this newly added comparator; keeping it so that we
return "0" in the default case seems reasoanble.

> +               return 0;
> +       }
> +}

--
paul moore
www.paul-moore.com
Richard Guy Briggs May 30, 2019, 2:19 p.m.
On 2019-05-29 18:16, Paul Moore wrote:
> On Mon, Apr 8, 2019 at 11:41 PM Richard Guy Briggs <rgb@redhat.com> wrote:
> >
> > Implement audit container identifier filtering using the AUDIT_CONTID
> > field name to send an 8-character string representing a u64 since the
> > value field is only u32.
> >
> > Sending it as two u32 was considered, but gathering and comparing two
> > fields was more complex.
> >
> > The feature indicator is AUDIT_FEATURE_BITMAP_CONTAINERID.
> >
> > Please see the github audit kernel issue for the contid filter feature:
> >   https://github.com/linux-audit/audit-kernel/issues/91
> > Please see the github audit userspace issue for filter additions:
> >   https://github.com/linux-audit/audit-userspace/issues/40
> > Please see the github audit testsuiite issue for the test case:
> >   https://github.com/linux-audit/audit-testsuite/issues/64
> > Please see the github audit wiki for the feature overview:
> >   https://github.com/linux-audit/audit-kernel/wiki/RFE-Audit-Container-ID
> > Signed-off-by: Richard Guy Briggs <rgb@redhat.com>
> > Acked-by: Serge Hallyn <serge@hallyn.com>
> > Acked-by: Neil Horman <nhorman@tuxdriver.com>
> > Reviewed-by: Ondrej Mosnacek <omosnace@redhat.com>
> > ---
> >  include/linux/audit.h      |  1 +
> >  include/uapi/linux/audit.h |  5 ++++-
> >  kernel/audit.h             |  1 +
> >  kernel/auditfilter.c       | 47 ++++++++++++++++++++++++++++++++++++++++++++++
> >  kernel/auditsc.c           |  4 ++++
> >  5 files changed, 57 insertions(+), 1 deletion(-)
> 
> ...
> 
> > diff --git a/kernel/auditfilter.c b/kernel/auditfilter.c
> > index 63f8b3f26fab..407b5bb3b4c6 100644
> > --- a/kernel/auditfilter.c
> > +++ b/kernel/auditfilter.c
> > @@ -1206,6 +1224,31 @@ int audit_comparator(u32 left, u32 op, u32 right)
> >         }
> >  }
> >
> > +int audit_comparator64(u64 left, u32 op, u64 right)
> > +{
> > +       switch (op) {
> > +       case Audit_equal:
> > +               return (left == right);
> > +       case Audit_not_equal:
> > +               return (left != right);
> > +       case Audit_lt:
> > +               return (left < right);
> > +       case Audit_le:
> > +               return (left <= right);
> > +       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();
> 
> A little birdy mentioned the BUG() here as a potential issue and while
> I had ignored it in earlier patches because this is likely a
> cut-n-paste from another audit comparator function, I took a closer
> look this time.  It appears as though we will never have an invalid op
> value as audit_data_to_entry()/audit_to_op() ensure that the op value
> is a a known good value.  Removing the BUG() from all the audit
> comparators is a separate issue, but I think it would be good to
> remove it from this newly added comparator; keeping it so that we
> return "0" in the default case seems reasoanble.

Fair enough.  That BUG(); can be removed.

> > +               return 0;
> > +       }
> > +}
> 
> --
> paul moore
> www.paul-moore.com

- RGB

--
Richard Guy Briggs <rgb@redhat.com>
Sr. S/W Engineer, Kernel Security, Base Operating Systems
Remote, Ottawa, Red Hat Canada
IRC: rgb, SunRaycer
Voice: +1.647.777.2635, Internal: (81) 32635
Paul Moore May 30, 2019, 2:34 p.m.
On Thu, May 30, 2019 at 10:20 AM Richard Guy Briggs <rgb@redhat.com> wrote:
>
> On 2019-05-29 18:16, Paul Moore wrote:
> > On Mon, Apr 8, 2019 at 11:41 PM Richard Guy Briggs <rgb@redhat.com> wrote:
> > >
> > > Implement audit container identifier filtering using the AUDIT_CONTID
> > > field name to send an 8-character string representing a u64 since the
> > > value field is only u32.
> > >
> > > Sending it as two u32 was considered, but gathering and comparing two
> > > fields was more complex.
> > >
> > > The feature indicator is AUDIT_FEATURE_BITMAP_CONTAINERID.
> > >
> > > Please see the github audit kernel issue for the contid filter feature:
> > >   https://github.com/linux-audit/audit-kernel/issues/91
> > > Please see the github audit userspace issue for filter additions:
> > >   https://github.com/linux-audit/audit-userspace/issues/40
> > > Please see the github audit testsuiite issue for the test case:
> > >   https://github.com/linux-audit/audit-testsuite/issues/64
> > > Please see the github audit wiki for the feature overview:
> > >   https://github.com/linux-audit/audit-kernel/wiki/RFE-Audit-Container-ID
> > > Signed-off-by: Richard Guy Briggs <rgb@redhat.com>
> > > Acked-by: Serge Hallyn <serge@hallyn.com>
> > > Acked-by: Neil Horman <nhorman@tuxdriver.com>
> > > Reviewed-by: Ondrej Mosnacek <omosnace@redhat.com>
> > > ---
> > >  include/linux/audit.h      |  1 +
> > >  include/uapi/linux/audit.h |  5 ++++-
> > >  kernel/audit.h             |  1 +
> > >  kernel/auditfilter.c       | 47 ++++++++++++++++++++++++++++++++++++++++++++++
> > >  kernel/auditsc.c           |  4 ++++
> > >  5 files changed, 57 insertions(+), 1 deletion(-)
> >
> > ...
> >
> > > diff --git a/kernel/auditfilter.c b/kernel/auditfilter.c
> > > index 63f8b3f26fab..407b5bb3b4c6 100644
> > > --- a/kernel/auditfilter.c
> > > +++ b/kernel/auditfilter.c
> > > @@ -1206,6 +1224,31 @@ int audit_comparator(u32 left, u32 op, u32 right)
> > >         }
> > >  }
> > >
> > > +int audit_comparator64(u64 left, u32 op, u64 right)
> > > +{
> > > +       switch (op) {
> > > +       case Audit_equal:
> > > +               return (left == right);
> > > +       case Audit_not_equal:
> > > +               return (left != right);
> > > +       case Audit_lt:
> > > +               return (left < right);
> > > +       case Audit_le:
> > > +               return (left <= right);
> > > +       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();
> >
> > A little birdy mentioned the BUG() here as a potential issue and while
> > I had ignored it in earlier patches because this is likely a
> > cut-n-paste from another audit comparator function, I took a closer
> > look this time.  It appears as though we will never have an invalid op
> > value as audit_data_to_entry()/audit_to_op() ensure that the op value
> > is a a known good value.  Removing the BUG() from all the audit
> > comparators is a separate issue, but I think it would be good to
> > remove it from this newly added comparator; keeping it so that we
> > return "0" in the default case seems reasoanble.
>
> Fair enough.  That BUG(); can be removed.

Please send a fixup patch for this.
Richard Guy Briggs May 30, 2019, 8:37 p.m.
On 2019-05-30 10:34, Paul Moore wrote:
> On Thu, May 30, 2019 at 10:20 AM Richard Guy Briggs <rgb@redhat.com> wrote:
> >
> > On 2019-05-29 18:16, Paul Moore wrote:
> > > On Mon, Apr 8, 2019 at 11:41 PM Richard Guy Briggs <rgb@redhat.com> wrote:
> > > >
> > > > Implement audit container identifier filtering using the AUDIT_CONTID
> > > > field name to send an 8-character string representing a u64 since the
> > > > value field is only u32.
> > > >
> > > > Sending it as two u32 was considered, but gathering and comparing two
> > > > fields was more complex.
> > > >
> > > > The feature indicator is AUDIT_FEATURE_BITMAP_CONTAINERID.
> > > >
> > > > Please see the github audit kernel issue for the contid filter feature:
> > > >   https://github.com/linux-audit/audit-kernel/issues/91
> > > > Please see the github audit userspace issue for filter additions:
> > > >   https://github.com/linux-audit/audit-userspace/issues/40
> > > > Please see the github audit testsuiite issue for the test case:
> > > >   https://github.com/linux-audit/audit-testsuite/issues/64
> > > > Please see the github audit wiki for the feature overview:
> > > >   https://github.com/linux-audit/audit-kernel/wiki/RFE-Audit-Container-ID
> > > > Signed-off-by: Richard Guy Briggs <rgb@redhat.com>
> > > > Acked-by: Serge Hallyn <serge@hallyn.com>
> > > > Acked-by: Neil Horman <nhorman@tuxdriver.com>
> > > > Reviewed-by: Ondrej Mosnacek <omosnace@redhat.com>
> > > > ---
> > > >  include/linux/audit.h      |  1 +
> > > >  include/uapi/linux/audit.h |  5 ++++-
> > > >  kernel/audit.h             |  1 +
> > > >  kernel/auditfilter.c       | 47 ++++++++++++++++++++++++++++++++++++++++++++++
> > > >  kernel/auditsc.c           |  4 ++++
> > > >  5 files changed, 57 insertions(+), 1 deletion(-)
> > >
> > > ...
> > >
> > > > diff --git a/kernel/auditfilter.c b/kernel/auditfilter.c
> > > > index 63f8b3f26fab..407b5bb3b4c6 100644
> > > > --- a/kernel/auditfilter.c
> > > > +++ b/kernel/auditfilter.c
> > > > @@ -1206,6 +1224,31 @@ int audit_comparator(u32 left, u32 op, u32 right)
> > > >         }
> > > >  }
> > > >
> > > > +int audit_comparator64(u64 left, u32 op, u64 right)
> > > > +{
> > > > +       switch (op) {
> > > > +       case Audit_equal:
> > > > +               return (left == right);
> > > > +       case Audit_not_equal:
> > > > +               return (left != right);
> > > > +       case Audit_lt:
> > > > +               return (left < right);
> > > > +       case Audit_le:
> > > > +               return (left <= right);
> > > > +       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();
> > >
> > > A little birdy mentioned the BUG() here as a potential issue and while
> > > I had ignored it in earlier patches because this is likely a
> > > cut-n-paste from another audit comparator function, I took a closer
> > > look this time.  It appears as though we will never have an invalid op
> > > value as audit_data_to_entry()/audit_to_op() ensure that the op value
> > > is a a known good value.  Removing the BUG() from all the audit
> > > comparators is a separate issue, but I think it would be good to
> > > remove it from this newly added comparator; keeping it so that we
> > > return "0" in the default case seems reasoanble.
> >
> > Fair enough.  That BUG(); can be removed.
> 
> Please send a fixup patch for this.

The fixup patch is trivial.  The rebase to v5.2-rc1 audit/next had merge
conflicts with four recent patchsets.  It may be simpler to submit a new
patchset and look at a diff of the two sets.  I'm testing the rebase
now.

> paul moore www.paul-moore.com

- RGB

--
Richard Guy Briggs <rgb@redhat.com>
Sr. S/W Engineer, Kernel Security, Base Operating Systems
Remote, Ottawa, Red Hat Canada
IRC: rgb, SunRaycer
Voice: +1.647.777.2635, Internal: (81) 32635
Paul Moore May 30, 2019, 8:45 p.m.
On Thu, May 30, 2019 at 4:37 PM Richard Guy Briggs <rgb@redhat.com> wrote:
> On 2019-05-30 10:34, Paul Moore wrote:
> > On Thu, May 30, 2019 at 10:20 AM Richard Guy Briggs <rgb@redhat.com> wrote:
> > >
> > > On 2019-05-29 18:16, Paul Moore wrote:
> > > > On Mon, Apr 8, 2019 at 11:41 PM Richard Guy Briggs <rgb@redhat.com> wrote:
> > > > >
> > > > > Implement audit container identifier filtering using the AUDIT_CONTID
> > > > > field name to send an 8-character string representing a u64 since the
> > > > > value field is only u32.
> > > > >
> > > > > Sending it as two u32 was considered, but gathering and comparing two
> > > > > fields was more complex.
> > > > >
> > > > > The feature indicator is AUDIT_FEATURE_BITMAP_CONTAINERID.
> > > > >
> > > > > Please see the github audit kernel issue for the contid filter feature:
> > > > >   https://github.com/linux-audit/audit-kernel/issues/91
> > > > > Please see the github audit userspace issue for filter additions:
> > > > >   https://github.com/linux-audit/audit-userspace/issues/40
> > > > > Please see the github audit testsuiite issue for the test case:
> > > > >   https://github.com/linux-audit/audit-testsuite/issues/64
> > > > > Please see the github audit wiki for the feature overview:
> > > > >   https://github.com/linux-audit/audit-kernel/wiki/RFE-Audit-Container-ID
> > > > > Signed-off-by: Richard Guy Briggs <rgb@redhat.com>
> > > > > Acked-by: Serge Hallyn <serge@hallyn.com>
> > > > > Acked-by: Neil Horman <nhorman@tuxdriver.com>
> > > > > Reviewed-by: Ondrej Mosnacek <omosnace@redhat.com>
> > > > > ---
> > > > >  include/linux/audit.h      |  1 +
> > > > >  include/uapi/linux/audit.h |  5 ++++-
> > > > >  kernel/audit.h             |  1 +
> > > > >  kernel/auditfilter.c       | 47 ++++++++++++++++++++++++++++++++++++++++++++++
> > > > >  kernel/auditsc.c           |  4 ++++
> > > > >  5 files changed, 57 insertions(+), 1 deletion(-)
> > > >
> > > > ...
> > > >
> > > > > diff --git a/kernel/auditfilter.c b/kernel/auditfilter.c
> > > > > index 63f8b3f26fab..407b5bb3b4c6 100644
> > > > > --- a/kernel/auditfilter.c
> > > > > +++ b/kernel/auditfilter.c
> > > > > @@ -1206,6 +1224,31 @@ int audit_comparator(u32 left, u32 op, u32 right)
> > > > >         }
> > > > >  }
> > > > >
> > > > > +int audit_comparator64(u64 left, u32 op, u64 right)
> > > > > +{
> > > > > +       switch (op) {
> > > > > +       case Audit_equal:
> > > > > +               return (left == right);
> > > > > +       case Audit_not_equal:
> > > > > +               return (left != right);
> > > > > +       case Audit_lt:
> > > > > +               return (left < right);
> > > > > +       case Audit_le:
> > > > > +               return (left <= right);
> > > > > +       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();
> > > >
> > > > A little birdy mentioned the BUG() here as a potential issue and while
> > > > I had ignored it in earlier patches because this is likely a
> > > > cut-n-paste from another audit comparator function, I took a closer
> > > > look this time.  It appears as though we will never have an invalid op
> > > > value as audit_data_to_entry()/audit_to_op() ensure that the op value
> > > > is a a known good value.  Removing the BUG() from all the audit
> > > > comparators is a separate issue, but I think it would be good to
> > > > remove it from this newly added comparator; keeping it so that we
> > > > return "0" in the default case seems reasoanble.
> > >
> > > Fair enough.  That BUG(); can be removed.
> >
> > Please send a fixup patch for this.
>
> The fixup patch is trivial.

Yes, I know.

> The rebase to v5.2-rc1 audit/next had merge
> conflicts with four recent patchsets.  It may be simpler to submit a new
> patchset and look at a diff of the two sets.  I'm testing the rebase
> now.

Great thanks.  Although you might want to hold off a bit on posting
the next revision until we sort out the discussion which is happening
in patch 02/10; unfortunately I fear we may need to change some of the
logic.

--
paul moore
www.paul-moore.com
Richard Guy Briggs May 30, 2019, 9:10 p.m.
On 2019-05-30 16:45, Paul Moore wrote:
> On Thu, May 30, 2019 at 4:37 PM Richard Guy Briggs <rgb@redhat.com> wrote:
> > On 2019-05-30 10:34, Paul Moore wrote:
> > > On Thu, May 30, 2019 at 10:20 AM Richard Guy Briggs <rgb@redhat.com> wrote:
> > > >
> > > > On 2019-05-29 18:16, Paul Moore wrote:
> > > > > On Mon, Apr 8, 2019 at 11:41 PM Richard Guy Briggs <rgb@redhat.com> wrote:
> > > > > >
> > > > > > Implement audit container identifier filtering using the AUDIT_CONTID
> > > > > > field name to send an 8-character string representing a u64 since the
> > > > > > value field is only u32.
> > > > > >
> > > > > > Sending it as two u32 was considered, but gathering and comparing two
> > > > > > fields was more complex.
> > > > > >
> > > > > > The feature indicator is AUDIT_FEATURE_BITMAP_CONTAINERID.
> > > > > >
> > > > > > Please see the github audit kernel issue for the contid filter feature:
> > > > > >   https://github.com/linux-audit/audit-kernel/issues/91
> > > > > > Please see the github audit userspace issue for filter additions:
> > > > > >   https://github.com/linux-audit/audit-userspace/issues/40
> > > > > > Please see the github audit testsuiite issue for the test case:
> > > > > >   https://github.com/linux-audit/audit-testsuite/issues/64
> > > > > > Please see the github audit wiki for the feature overview:
> > > > > >   https://github.com/linux-audit/audit-kernel/wiki/RFE-Audit-Container-ID
> > > > > > Signed-off-by: Richard Guy Briggs <rgb@redhat.com>
> > > > > > Acked-by: Serge Hallyn <serge@hallyn.com>
> > > > > > Acked-by: Neil Horman <nhorman@tuxdriver.com>
> > > > > > Reviewed-by: Ondrej Mosnacek <omosnace@redhat.com>
> > > > > > ---
> > > > > >  include/linux/audit.h      |  1 +
> > > > > >  include/uapi/linux/audit.h |  5 ++++-
> > > > > >  kernel/audit.h             |  1 +
> > > > > >  kernel/auditfilter.c       | 47 ++++++++++++++++++++++++++++++++++++++++++++++
> > > > > >  kernel/auditsc.c           |  4 ++++
> > > > > >  5 files changed, 57 insertions(+), 1 deletion(-)
> > > > >
> > > > > ...
> > > > >
> > > > > > diff --git a/kernel/auditfilter.c b/kernel/auditfilter.c
> > > > > > index 63f8b3f26fab..407b5bb3b4c6 100644
> > > > > > --- a/kernel/auditfilter.c
> > > > > > +++ b/kernel/auditfilter.c
> > > > > > @@ -1206,6 +1224,31 @@ int audit_comparator(u32 left, u32 op, u32 right)
> > > > > >         }
> > > > > >  }
> > > > > >
> > > > > > +int audit_comparator64(u64 left, u32 op, u64 right)
> > > > > > +{
> > > > > > +       switch (op) {
> > > > > > +       case Audit_equal:
> > > > > > +               return (left == right);
> > > > > > +       case Audit_not_equal:
> > > > > > +               return (left != right);
> > > > > > +       case Audit_lt:
> > > > > > +               return (left < right);
> > > > > > +       case Audit_le:
> > > > > > +               return (left <= right);
> > > > > > +       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();
> > > > >
> > > > > A little birdy mentioned the BUG() here as a potential issue and while
> > > > > I had ignored it in earlier patches because this is likely a
> > > > > cut-n-paste from another audit comparator function, I took a closer
> > > > > look this time.  It appears as though we will never have an invalid op
> > > > > value as audit_data_to_entry()/audit_to_op() ensure that the op value
> > > > > is a a known good value.  Removing the BUG() from all the audit
> > > > > comparators is a separate issue, but I think it would be good to
> > > > > remove it from this newly added comparator; keeping it so that we
> > > > > return "0" in the default case seems reasoanble.
> > > >
> > > > Fair enough.  That BUG(); can be removed.
> > >
> > > Please send a fixup patch for this.
> >
> > The fixup patch is trivial.
> 
> Yes, I know.
> 
> > The rebase to v5.2-rc1 audit/next had merge
> > conflicts with four recent patchsets.  It may be simpler to submit a new
> > patchset and look at a diff of the two sets.  I'm testing the rebase
> > now.
> 
> Great thanks.  Although you might want to hold off a bit on posting
> the next revision until we sort out the discussion which is happening
> in patch 02/10; unfortunately I fear we may need to change some of the
> logic.

I'm watching...  I have no immediate ideas on how to address that
discussion yet.  I'm optimistic it can be adjusted after the initial
commit without changing the API.

> paul moore www.paul-moore.com

- 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