diff mbox series

[v30,26/28] Audit: Add record for multiple object security contexts

Message ID 20211124014332.36128-27-casey@schaufler-ca.com (mailing list archive)
State Changes Requested
Delegated to: Paul Moore
Headers show
Series [v30,01/28] integrity: disassociate ima_filter_rule from security_audit_rule | expand

Commit Message

Casey Schaufler Nov. 24, 2021, 1:43 a.m. UTC 
Create a new audit record AUDIT_MAC_OBJ_CONTEXTS.
An example of the MAC_OBJ_CONTEXTS (1421) record is:

    type=UNKNOWN[1421]
    msg=audit(1601152467.009:1050):
    obj_selinux="unconfined_u:object_r:user_home_t:s0"

When an audit event includes a AUDIT_MAC_OBJ_CONTEXTS record
the "obj=" field in other records in the event will be "obj=?".
A AUDIT_MAC_OBJ_CONTEXTS record is supplied when the system has
multiple security modules that may make access decisions based
on an object security context.

Signed-off-by: Casey Schaufler <casey@schaufler-ca.com>
---
 include/linux/audit.h      |  5 ++++
 include/uapi/linux/audit.h |  1 +
 kernel/audit.c             | 61 ++++++++++++++++++++++++++++++++++++++
 kernel/auditsc.c           | 37 ++++-------------------
 4 files changed, 72 insertions(+), 32 deletions(-)

Comments

kernel test robot Nov. 24, 2021, 7:41 a.m. UTC | #1 
Hi Casey,

I love your patch! Yet something to improve:

[auto build test ERROR on nf-next/master]
[also build test ERROR on nf/master linus/master v5.16-rc2]
[cannot apply to pcmoore-audit/next next-20211124]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Casey-Schaufler/integrity-disassociate-ima_filter_rule-from-security_audit_rule/20211124-104307
base:   https://git.kernel.org/pub/scm/linux/kernel/git/pablo/nf-next.git master
config: i386-tinyconfig (https://download.01.org/0day-ci/archive/20211124/202111241506.7V9kCQCo-lkp@intel.com/config)
compiler: gcc-9 (Debian 9.3.0-22) 9.3.0
reproduce (this is a W=1 build):
        # https://github.com/0day-ci/linux/commit/3054c600afec9a016902ed6ed5de86c76d6b0105
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Casey-Schaufler/integrity-disassociate-ima_filter_rule-from-security_audit_rule/20211124-104307
        git checkout 3054c600afec9a016902ed6ed5de86c76d6b0105
        # save the config file to linux build tree
        mkdir build_dir
        make W=1 O=build_dir ARCH=i386 SHELL=/bin/bash

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

   In file included from init/init_task.c:12:
>> include/linux/audit.h:262:1: error: expected identifier or '(' before '{' token
     262 | { }
         | ^
   include/linux/audit.h:260:20: warning: 'audit_log_object_context' declared 'static' but never defined [-Wunused-function]
     260 | static inline void audit_log_object_context(struct audit_buffer *ab,
         |                    ^~~~~~~~~~~~~~~~~~~~~~~~
--
   In file included from kernel/exit.c:49:
>> include/linux/audit.h:262:1: error: expected identifier or '(' before '{' token
     262 | { }
         | ^
   kernel/exit.c:1817:13: warning: no previous prototype for 'abort' [-Wmissing-prototypes]
    1817 | __weak void abort(void)
         |             ^~~~~
   In file included from kernel/exit.c:49:
   include/linux/audit.h:260:20: warning: 'audit_log_object_context' declared 'static' but never defined [-Wunused-function]
     260 | static inline void audit_log_object_context(struct audit_buffer *ab,
         |                    ^~~~~~~~~~~~~~~~~~~~~~~~
--
   In file included from fs/pipe.c:23:
>> include/linux/audit.h:262:1: error: expected identifier or '(' before '{' token
     262 | { }
         | ^
   fs/pipe.c:755:15: warning: no previous prototype for 'account_pipe_buffers' [-Wmissing-prototypes]
     755 | unsigned long account_pipe_buffers(struct user_struct *user,
         |               ^~~~~~~~~~~~~~~~~~~~
   fs/pipe.c:761:6: warning: no previous prototype for 'too_many_pipe_buffers_soft' [-Wmissing-prototypes]
     761 | bool too_many_pipe_buffers_soft(unsigned long user_bufs)
         |      ^~~~~~~~~~~~~~~~~~~~~~~~~~
   fs/pipe.c:768:6: warning: no previous prototype for 'too_many_pipe_buffers_hard' [-Wmissing-prototypes]
     768 | bool too_many_pipe_buffers_hard(unsigned long user_bufs)
         |      ^~~~~~~~~~~~~~~~~~~~~~~~~~
   fs/pipe.c:775:6: warning: no previous prototype for 'pipe_is_unprivileged_user' [-Wmissing-prototypes]
     775 | bool pipe_is_unprivileged_user(void)
         |      ^~~~~~~~~~~~~~~~~~~~~~~~~
   fs/pipe.c:1245:5: warning: no previous prototype for 'pipe_resize_ring' [-Wmissing-prototypes]
    1245 | int pipe_resize_ring(struct pipe_inode_info *pipe, unsigned int nr_slots)
         |     ^~~~~~~~~~~~~~~~
   In file included from fs/pipe.c:23:
   include/linux/audit.h:260:20: warning: 'audit_log_object_context' declared 'static' but never defined [-Wunused-function]
     260 | static inline void audit_log_object_context(struct audit_buffer *ab,
         |                    ^~~~~~~~~~~~~~~~~~~~~~~~


vim +262 include/linux/audit.h

   220	
   221	#else /* CONFIG_AUDIT */
   222	static inline __printf(4, 5)
   223	void audit_log(struct audit_context *ctx, gfp_t gfp_mask, int type,
   224		       const char *fmt, ...)
   225	{ }
   226	static inline struct audit_buffer *audit_log_start(struct audit_context *ctx,
   227							   gfp_t gfp_mask, int type)
   228	{
   229		return NULL;
   230	}
   231	static inline __printf(2, 3)
   232	void audit_log_format(struct audit_buffer *ab, const char *fmt, ...)
   233	{ }
   234	static inline void audit_log_end(struct audit_buffer *ab)
   235	{ }
   236	static inline void audit_log_n_hex(struct audit_buffer *ab,
   237					   const unsigned char *buf, size_t len)
   238	{ }
   239	static inline void audit_log_n_string(struct audit_buffer *ab,
   240					      const char *buf, size_t n)
   241	{ }
   242	static inline void  audit_log_n_untrustedstring(struct audit_buffer *ab,
   243							const char *string, size_t n)
   244	{ }
   245	static inline void audit_log_untrustedstring(struct audit_buffer *ab,
   246						     const char *string)
   247	{ }
   248	static inline void audit_log_d_path(struct audit_buffer *ab,
   249					    const char *prefix,
   250					    const struct path *path)
   251	{ }
   252	static inline void audit_log_key(struct audit_buffer *ab, char *key)
   253	{ }
   254	static inline void audit_log_path_denied(int type, const char *operation)
   255	{ }
   256	static inline int audit_log_task_context(struct audit_buffer *ab)
   257	{
   258		return 0;
   259	}
   260	static inline void audit_log_object_context(struct audit_buffer *ab,
   261						    struct lsmblob *blob);
 > 262	{ }
   263	static inline void audit_log_task_info(struct audit_buffer *ab)
   264	{ }
   265	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
kernel test robot Nov. 24, 2021, 1:40 p.m. UTC | #2 
Hi Casey,

I love your patch! Yet something to improve:

[auto build test ERROR on nf-next/master]
[also build test ERROR on nf/master linus/master v5.16-rc2]
[cannot apply to pcmoore-audit/next jmorris-security/next-testing next-20211124]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Casey-Schaufler/integrity-disassociate-ima_filter_rule-from-security_audit_rule/20211124-104307
base:   https://git.kernel.org/pub/scm/linux/kernel/git/pablo/nf-next.git master
config: powerpc-allnoconfig (https://download.01.org/0day-ci/archive/20211124/202111242114.1WN6oSkW-lkp@intel.com/config)
compiler: powerpc-linux-gcc (GCC) 11.2.0
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/0day-ci/linux/commit/3054c600afec9a016902ed6ed5de86c76d6b0105
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Casey-Schaufler/integrity-disassociate-ima_filter_rule-from-security_audit_rule/20211124-104307
        git checkout 3054c600afec9a016902ed6ed5de86c76d6b0105
        # save the config file to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.2.0 make.cross ARCH=powerpc 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

   In file included from arch/powerpc/kernel/ptrace/ptrace.c:20:
   include/linux/audit.h:262:1: error: expected identifier or '(' before '{' token
     262 | { }
         | ^
>> include/linux/audit.h:260:20: error: 'audit_log_object_context' declared 'static' but never defined [-Werror=unused-function]
     260 | static inline void audit_log_object_context(struct audit_buffer *ab,
         |                    ^~~~~~~~~~~~~~~~~~~~~~~~
   cc1: all warnings being treated as errors


vim +260 include/linux/audit.h

   220	
   221	#else /* CONFIG_AUDIT */
   222	static inline __printf(4, 5)
   223	void audit_log(struct audit_context *ctx, gfp_t gfp_mask, int type,
   224		       const char *fmt, ...)
   225	{ }
   226	static inline struct audit_buffer *audit_log_start(struct audit_context *ctx,
   227							   gfp_t gfp_mask, int type)
   228	{
   229		return NULL;
   230	}
   231	static inline __printf(2, 3)
   232	void audit_log_format(struct audit_buffer *ab, const char *fmt, ...)
   233	{ }
   234	static inline void audit_log_end(struct audit_buffer *ab)
   235	{ }
   236	static inline void audit_log_n_hex(struct audit_buffer *ab,
   237					   const unsigned char *buf, size_t len)
   238	{ }
   239	static inline void audit_log_n_string(struct audit_buffer *ab,
   240					      const char *buf, size_t n)
   241	{ }
   242	static inline void  audit_log_n_untrustedstring(struct audit_buffer *ab,
   243							const char *string, size_t n)
   244	{ }
   245	static inline void audit_log_untrustedstring(struct audit_buffer *ab,
   246						     const char *string)
   247	{ }
   248	static inline void audit_log_d_path(struct audit_buffer *ab,
   249					    const char *prefix,
   250					    const struct path *path)
   251	{ }
   252	static inline void audit_log_key(struct audit_buffer *ab, char *key)
   253	{ }
   254	static inline void audit_log_path_denied(int type, const char *operation)
   255	{ }
   256	static inline int audit_log_task_context(struct audit_buffer *ab)
   257	{
   258		return 0;
   259	}
 > 260	static inline void audit_log_object_context(struct audit_buffer *ab,
   261						    struct lsmblob *blob);
   262	{ }
   263	static inline void audit_log_task_info(struct audit_buffer *ab)
   264	{ }
   265	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
Paul Moore Dec. 6, 2021, 2:45 a.m. UTC | #3 
On Tue, Nov 23, 2021 at 9:12 PM Casey Schaufler <casey@schaufler-ca.com> wrote:
>
> Create a new audit record AUDIT_MAC_OBJ_CONTEXTS.
> An example of the MAC_OBJ_CONTEXTS (1421) record is:
>
>     type=UNKNOWN[1421]
>     msg=audit(1601152467.009:1050):
>     obj_selinux="unconfined_u:object_r:user_home_t:s0"
>
> When an audit event includes a AUDIT_MAC_OBJ_CONTEXTS record
> the "obj=" field in other records in the event will be "obj=?".
> A AUDIT_MAC_OBJ_CONTEXTS record is supplied when the system has
> multiple security modules that may make access decisions based
> on an object security context.
>
> Signed-off-by: Casey Schaufler <casey@schaufler-ca.com>
> ---
>  include/linux/audit.h      |  5 ++++
>  include/uapi/linux/audit.h |  1 +
>  kernel/audit.c             | 61 ++++++++++++++++++++++++++++++++++++++
>  kernel/auditsc.c           | 37 ++++-------------------
>  4 files changed, 72 insertions(+), 32 deletions(-)

My comments on 24/28 and 25/28 should also apply to this patch.

--
paul moore
www.paul-moore.com
diff mbox series

Patch

diff --git a/include/linux/audit.h b/include/linux/audit.h
index 943584128399..8381afb4f49e 100644
--- a/include/linux/audit.h
+++ b/include/linux/audit.h
@@ -192,6 +192,8 @@  extern void		    audit_log_path_denied(int type,
 extern void		    audit_log_lost(const char *message);
 
 extern int audit_log_task_context(struct audit_buffer *ab);
+extern void audit_log_object_context(struct audit_buffer *ab,
+				     struct lsmblob *blob);
 extern void audit_log_task_info(struct audit_buffer *ab);
 
 extern int		    audit_update_lsm_rules(void);
@@ -255,6 +257,9 @@  static inline int audit_log_task_context(struct audit_buffer *ab)
 {
 	return 0;
 }
+static inline void audit_log_object_context(struct audit_buffer *ab,
+					    struct lsmblob *blob);
+{ }
 static inline void audit_log_task_info(struct audit_buffer *ab)
 { }
 
diff --git a/include/uapi/linux/audit.h b/include/uapi/linux/audit.h
index 86ad3da4f0d4..116566d0fc03 100644
--- a/include/uapi/linux/audit.h
+++ b/include/uapi/linux/audit.h
@@ -144,6 +144,7 @@ 
 #define AUDIT_MAC_CALIPSO_ADD	1418	/* NetLabel: add CALIPSO DOI entry */
 #define AUDIT_MAC_CALIPSO_DEL	1419	/* NetLabel: del CALIPSO DOI entry */
 #define AUDIT_MAC_TASK_CONTEXTS	1420	/* Multiple LSM task contexts */
+#define AUDIT_MAC_OBJ_CONTEXTS	1421	/* Multiple LSM objext contexts */
 
 #define AUDIT_FIRST_KERN_ANOM_MSG   1700
 #define AUDIT_LAST_KERN_ANOM_MSG    1799
diff --git a/kernel/audit.c b/kernel/audit.c
index 6c93545a14f3..55fdcc2c88e4 100644
--- a/kernel/audit.c
+++ b/kernel/audit.c
@@ -199,6 +199,7 @@  struct audit_context_entry {
 	int			type;	/* Audit record type */
 	union {
 		struct lsmblob	mac_task_context;
+		struct lsmblob	mac_obj_context;
 	};
 };
 
@@ -2190,6 +2191,44 @@  int audit_log_task_context(struct audit_buffer *ab)
 }
 EXPORT_SYMBOL(audit_log_task_context);
 
+void audit_log_object_context(struct audit_buffer *ab, struct lsmblob *blob)
+{
+	struct audit_context_entry *ace;
+	struct lsmcontext context;
+	int error;
+
+	if (!lsm_multiple_contexts()) {
+		error = security_secid_to_secctx(blob, &context, LSMBLOB_FIRST);
+		if (error) {
+			if (error != -EINVAL)
+				goto error_path;
+			return;
+		}
+		audit_log_format(ab, " obj=%s", context.context);
+		security_release_secctx(&context);
+		return;
+	}
+	/*
+	 * If there is more than one security module that has a
+	 * object "context" it's necessary to put the object data
+	 * into a separate record to maintain compatibility.
+	 */
+	audit_log_format(ab, " obj=?");
+	ace = kzalloc(sizeof(*ace), GFP_KERNEL);
+	if (ace) {
+		INIT_LIST_HEAD(&ace->list);
+		ace->type = AUDIT_MAC_OBJ_CONTEXTS;
+		ace->mac_obj_context = *blob;
+		list_add(&ace->list, &ab->aux_records);
+		return;
+	}
+	error = -ENOMEM;
+
+error_path:
+	audit_panic("error in audit_log_object_context");
+}
+EXPORT_SYMBOL(audit_log_object_context);
+
 void audit_log_d_path_exe(struct audit_buffer *ab,
 			  struct mm_struct *mm)
 {
@@ -2497,6 +2536,28 @@  void audit_log_end(struct audit_buffer *ab)
 				}
 			}
 			break;
+		case AUDIT_MAC_OBJ_CONTEXTS:
+			for (i = 0; i < LSMBLOB_ENTRIES; i++) {
+				if (entry->mac_obj_context.secid[i] == 0)
+					continue;
+				rc = security_secid_to_secctx(
+							&entry->mac_obj_context,
+							&lcontext, i);
+				if (rc) {
+					if (rc != -EINVAL)
+						audit_panic("error in audit_log_end");
+					audit_log_format(mab, "%sobj_%s=\"?\"",
+							 i ? " " : "",
+							 lsm_slot_to_name(i));
+				} else {
+					audit_log_format(mab, "%sobj_%s=\"%s\"",
+							 i ? " " : "",
+							 lsm_slot_to_name(i),
+							 lcontext.context);
+					security_release_secctx(&lcontext);
+				}
+			}
+			break;
 		default:
 			audit_panic("Unknown type in audit_log_end");
 			break;
diff --git a/kernel/auditsc.c b/kernel/auditsc.c
index c128f7e73e89..dc8531a79174 100644
--- a/kernel/auditsc.c
+++ b/kernel/auditsc.c
@@ -1111,7 +1111,6 @@  static int audit_log_pid_context(struct audit_context *context, pid_t pid,
 				 struct lsmblob *blob, char *comm)
 {
 	struct audit_buffer *ab;
-	struct lsmcontext lsmctx;
 	int rc = 0;
 
 	ab = audit_log_start(context, GFP_KERNEL, AUDIT_OBJ_PID);
@@ -1121,15 +1120,8 @@  static int audit_log_pid_context(struct audit_context *context, pid_t pid,
 	audit_log_format(ab, "opid=%d oauid=%d ouid=%d oses=%d", pid,
 			 from_kuid(&init_user_ns, auid),
 			 from_kuid(&init_user_ns, uid), sessionid);
-	if (lsmblob_is_set(blob)) {
-		if (security_secid_to_secctx(blob, &lsmctx, LSMBLOB_FIRST)) {
-			audit_log_format(ab, " obj=(none)");
-			rc = 1;
-		} else {
-			audit_log_format(ab, " obj=%s", lsmctx.context);
-			security_release_secctx(&lsmctx);
-		}
-	}
+	if (lsmblob_is_set(blob))
+		audit_log_object_context(ab, blob);
 	audit_log_format(ab, " ocomm=");
 	audit_log_untrustedstring(ab, comm);
 	audit_log_end(ab);
@@ -1364,18 +1356,10 @@  static void show_special(struct audit_context *context, int *call_panic)
 				 from_kgid(&init_user_ns, context->ipc.gid),
 				 context->ipc.mode);
 		if (osid) {
-			struct lsmcontext lsmcxt;
 			struct lsmblob blob;
 
 			lsmblob_init(&blob, osid);
-			if (security_secid_to_secctx(&blob, &lsmcxt,
-						     LSMBLOB_FIRST)) {
-				audit_log_format(ab, " osid=%u", osid);
-				*call_panic = 1;
-			} else {
-				audit_log_format(ab, " obj=%s", lsmcxt.context);
-				security_release_secctx(&lsmcxt);
-			}
+			audit_log_object_context(ab, &blob);
 		}
 		if (context->ipc.has_perm) {
 			audit_log_end(ab);
@@ -1527,19 +1511,8 @@  static void audit_log_name(struct audit_context *context, struct audit_names *n,
 				 from_kgid(&init_user_ns, n->gid),
 				 MAJOR(n->rdev),
 				 MINOR(n->rdev));
-	if (lsmblob_is_set(&n->lsmblob)) {
-		struct lsmcontext lsmctx;
-
-		if (security_secid_to_secctx(&n->lsmblob, &lsmctx,
-					     LSMBLOB_FIRST)) {
-			audit_log_format(ab, " osid=?");
-			if (call_panic)
-				*call_panic = 2;
-		} else {
-			audit_log_format(ab, " obj=%s", lsmctx.context);
-			security_release_secctx(&lsmctx);
-		}
-	}
+	if (lsmblob_is_set(&n->lsmblob))
+		audit_log_object_context(ab, &n->lsmblob);
 
 	/* log the audit_names record type */
 	switch (n->type) {