diff mbox series

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

Message ID 20220202235323.23929-27-casey@schaufler-ca.com (mailing list archive)
State New, archived
Headers show
Series LSM: Module stacking for AppArmor | expand

Commit Message

Casey Schaufler Feb. 2, 2022, 11:53 p.m. UTC 
Create a new audit record AUDIT_MAC_OBJ_CONTEXTS.
An example of the MAC_OBJ_CONTEXTS (1421) record is:

    type=MAC_OBJ_CONTEXTS[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=?".
An 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             | 59 ++++++++++++++++++++++++++++++++++++++
 kernel/auditsc.c           | 37 ++++--------------------
 4 files changed, 70 insertions(+), 32 deletions(-)

Comments

Paul Moore March 3, 2022, 11:36 p.m. UTC | #1 
On Wed, Feb 2, 2022 at 7:23 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=MAC_OBJ_CONTEXTS[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=?".
> An 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             | 59 ++++++++++++++++++++++++++++++++++++++
>  kernel/auditsc.c           | 37 ++++--------------------
>  4 files changed, 70 insertions(+), 32 deletions(-)

...

> diff --git a/kernel/audit.c b/kernel/audit.c
> index e8744e80ef21..3b9ce617b150 100644
> --- a/kernel/audit.c
> +++ b/kernel/audit.c
> @@ -2199,6 +2200,43 @@ 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);
> +       } else {
> +               /*
> +                * 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.
> +                */

I know this is nitpicky, but I'm going to say it anyway ... the
separate record isn't purely for compatibility reasons, it's for size
reasons.  There is a fear that multiple LSM labels could blow past the
record size limit when combined with other fields, so putting them in
their own dedicated record gives us more room.  If that wasn't the
case we could just tack them on the end of existing records.

However, converting the existing "obj=" field into "obj=?" when
multiple LSM labels are present *is* a compatibility nod as it allows
existing userspace tooling that expects a single "obj=" field to
continue to work.

> +               audit_log_format(ab, " obj=?");
> +               ace = kzalloc(sizeof(*ace), ab->gfp_mask);
> +               if (!ace)
> +                       goto error_path;
> +               INIT_LIST_HEAD(&ace->list);
> +               ace->type = AUDIT_MAC_OBJ_CONTEXTS;
> +               ace->lsm_objs = *blob;
> +               list_add(&ace->list, &ab->aux_records);
> +       }
> +       return;
> +
> +error_path:
> +       audit_panic("error in audit_log_object_context");
> +}
> +EXPORT_SYMBOL(audit_log_object_context);
> +
Casey Schaufler March 4, 2022, 1:26 a.m. UTC | #2 
On 3/3/2022 3:36 PM, Paul Moore wrote:
> On Wed, Feb 2, 2022 at 7:23 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=MAC_OBJ_CONTEXTS[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=?".
>> An 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             | 59 ++++++++++++++++++++++++++++++++++++++
>>   kernel/auditsc.c           | 37 ++++--------------------
>>   4 files changed, 70 insertions(+), 32 deletions(-)
> ...
>
>> diff --git a/kernel/audit.c b/kernel/audit.c
>> index e8744e80ef21..3b9ce617b150 100644
>> --- a/kernel/audit.c
>> +++ b/kernel/audit.c
>> @@ -2199,6 +2200,43 @@ 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);
>> +       } else {
>> +               /*
>> +                * 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.
>> +                */
> I know this is nitpicky, but I'm going to say it anyway ... the
> separate record isn't purely for compatibility reasons, it's for size
> reasons.  There is a fear that multiple LSM labels could blow past the
> record size limit when combined with other fields, so putting them in
> their own dedicated record gives us more room.  If that wasn't the
> case we could just tack them on the end of existing records.

Fair enough. I have no objection to adding commentary that will
help the next developer who comes into this code.

>
> However, converting the existing "obj=" field into "obj=?" when
> multiple LSM labels are present *is* a compatibility nod as it allows
> existing userspace tooling that expects a single "obj=" field to
> continue to work.

Likewise here.

>
>> +               audit_log_format(ab, " obj=?");
>> +               ace = kzalloc(sizeof(*ace), ab->gfp_mask);
>> +               if (!ace)
>> +                       goto error_path;
>> +               INIT_LIST_HEAD(&ace->list);
>> +               ace->type = AUDIT_MAC_OBJ_CONTEXTS;
>> +               ace->lsm_objs = *blob;
>> +               list_add(&ace->list, &ab->aux_records);
>> +       }
>> +       return;
>> +
>> +error_path:
>> +       audit_panic("error in audit_log_object_context");
>> +}
>> +EXPORT_SYMBOL(audit_log_object_context);
>> +
diff mbox series

Patch

diff --git a/include/linux/audit.h b/include/linux/audit.h
index 14849d5f84b4..94c87ec043c7 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 af0aaccfaf57..d25d76b29e3c 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 e8744e80ef21..3b9ce617b150 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	lsm_subjs;
+		struct lsmblob	lsm_objs;
 	};
 };
 
@@ -2199,6 +2200,43 @@  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);
+	} else {
+		/*
+		 * 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), ab->gfp_mask);
+		if (!ace)
+			goto error_path;
+		INIT_LIST_HEAD(&ace->list);
+		ace->type = AUDIT_MAC_OBJ_CONTEXTS;
+		ace->lsm_objs = *blob;
+		list_add(&ace->list, &ab->aux_records);
+	}
+	return;
+
+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)
 {
@@ -2505,6 +2543,27 @@  void audit_log_end(struct audit_buffer *ab)
 				}
 			}
 			break;
+		case AUDIT_MAC_OBJ_CONTEXTS:
+			for (i = 0; i < LSMBLOB_ENTRIES; i++) {
+				if (entry->lsm_objs.secid[i] == 0)
+					continue;
+				rc = security_secid_to_secctx(&entry->lsm_objs,
+							      &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 7848e7351cf9..41111b607c78 100644
--- a/kernel/auditsc.c
+++ b/kernel/auditsc.c
@@ -1120,7 +1120,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);
@@ -1130,15 +1129,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);
@@ -1373,18 +1365,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);
@@ -1536,19 +1520,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) {