Re: [PATCH v3 1/3] dm: introduce audit event module for device mapper

From: Weiß, Michael
Date: Sat Sep 04 2021 - 05:25:13 EST

Next message: lisa hugh: "MASSAGE FROM >>MS LISA HUGH."
Previous message: Len Baker: "[PATCH] brcmfmac: Replace zero-length array with flexible array member"
In reply to: Paul Moore: "Re: [PATCH v3 1/3] dm: introduce audit event module for device mapper"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]

On Fri, 2021-09-03 at 13:27 -0400, Paul Moore wrote:
> On Tue, Aug 31, 2021 at 3:19 PM Michael Weiß
> <michael.weiss@xxxxxxxxxxxxxxxxxxx> wrote:
> > To be able to send auditing events to user space, we introduce a
> > generic dm-audit module. It provides helper functions to emit audit
> > events through the kernel audit subsystem. We claim the
> > AUDIT_DM_CTRL type=1336 and AUDIT_DM_EVENT type=1337 out of the
> > audit event messages range in the corresponding userspace api in
> > 'include/uapi/linux/audit.h' for those events.
> >
> > AUDIT_DM_CTRL is used to provide information about creation and
> > destruction of device mapper targets which are triggered by user space
> > admin control actions.
> > AUDIT_DM_EVENT is used to provide information about actual errors
> > during operation of the mapped device, showing e.g. integrity
> > violations in audit log.
> >
> > Following commits to device mapper targets actually will make use of
> > this to emit those events in relevant cases.
> >
> > The audit logs look like this if executing the following simple test:
> >
> > # dd if=/dev/zero of=test.img bs=1M count=1024
> > # losetup -f test.img
> > # integritysetup -vD format --integrity sha256 -t 32 /dev/loop0
> > # integritysetup open -D /dev/loop0 --integrity sha256 integritytest
> > # integritysetup status integritytest
> > # integritysetup close integritytest
> > # integritysetup open -D /dev/loop0 --integrity sha256 integritytest
> > # integritysetup status integritytest
> > # dd if=/dev/urandom of=/dev/loop0 bs=512 count=1 seek=100000
> > # dd if=/dev/mapper/integritytest of=/dev/null
> >
> > -------------------------
> > audit.log from auditd
> >
> > type=UNKNOWN[1336] msg=audit(1630425039.363:184): module=integrity
> > op=ctr ppid=3807 pid=3819 auid=1000 uid=0 gid=0 euid=0 suid=0 fsuid=0
> > egid=0 sgid=0 fsgid=0 tty=pts2 ses=3 comm="integritysetup"
> > exe="/sbin/integritysetup" subj==unconfined dev=254:3
> > error_msg='success' res=1
> > type=UNKNOWN[1336] msg=audit(1630425039.471:185): module=integrity
> > op=dtr ppid=3807 pid=3819 auid=1000 uid=0 gid=0 euid=0 suid=0 fsuid=0
> > egid=0 sgid=0 fsgid=0 tty=pts2 ses=3 comm="integritysetup"
> > exe="/sbin/integritysetup" subj==unconfined dev=254:3
> > error_msg='success' res=1
> > type=UNKNOWN[1336] msg=audit(1630425039.611:186): module=integrity
> > op=ctr ppid=3807 pid=3819 auid=1000 uid=0 gid=0 euid=0 suid=0 fsuid=0
> > egid=0 sgid=0 fsgid=0 tty=pts2 ses=3 comm="integritysetup"
> > exe="/sbin/integritysetup" subj==unconfined dev=254:3
> > error_msg='success' res=1
> > type=UNKNOWN[1336] msg=audit(1630425054.475:187): module=integrity
> > op=dtr ppid=3807 pid=3819 auid=1000 uid=0 gid=0 euid=0 suid=0 fsuid=0
> > egid=0 sgid=0 fsgid=0 tty=pts2 ses=3 comm="integritysetup"
> > exe="/sbin/integritysetup" subj==unconfined dev=254:3
> > error_msg='success' res=1
> >
> > type=UNKNOWN[1336] msg=audit(1630425073.171:191): module=integrity
> > op=ctr ppid=3807 pid=3883 auid=1000 uid=0 gid=0 euid=0 suid=0 fsuid=0
> > egid=0 sgid=0 fsgid=0 tty=pts2 ses=3 comm="integritysetup"
> > exe="/sbin/integritysetup" subj==unconfined dev=254:3
> > error_msg='success' res=1
> >
> > type=UNKNOWN[1336] msg=audit(1630425087.239:192): module=integrity
> > op=dtr ppid=3807 pid=3902 auid=1000 uid=0 gid=0 euid=0 suid=0 fsuid=0
> > egid=0 sgid=0 fsgid=0 tty=pts2 ses=3 comm="integritysetup"
> > exe="/sbin/integritysetup" subj==unconfined dev=254:3
> > error_msg='success' res=1
> >
> > type=UNKNOWN[1336] msg=audit(1630425093.755:193): module=integrity
> > op=ctr ppid=3807 pid=3906 auid=1000 uid=0 gid=0 euid=0 suid=0 fsuid=0
> > egid=0 sgid=0 fsgid=0 tty=pts2 ses=3 comm="integritysetup"
> > exe="/sbin/integritysetup" subj==unconfined dev=254:3
> > error_msg='success' res=1
> >
> > type=UNKNOWN[1337] msg=audit(1630425112.119:194): module=integrity
> > op=integrity-checksum dev=254:3 sector 77480 res=0
> > type=UNKNOWN[1337] msg=audit(1630425112.119:195): module=integrity
> > op=integrity-checksum dev=254:3 sector 77480 res=0
> > type=UNKNOWN[1337] msg=audit(1630425112.119:196): module=integrity
> > op=integrity-checksum dev=254:3 sector 77480 res=0
> > type=UNKNOWN[1337] msg=audit(1630425112.119:197): module=integrity
> > op=integrity-checksum dev=254:3 sector 77480 res=0
> > type=UNKNOWN[1337] msg=audit(1630425112.119:198): module=integrity
> > op=integrity-checksum dev=254:3 sector 77480 res=0
> > type=UNKNOWN[1337] msg=audit(1630425112.119:199): module=integrity
> > op=integrity-checksum dev=254:3 sector 77480 res=0
> > type=UNKNOWN[1337] msg=audit(1630425112.119:200): module=integrity
> > op=integrity-checksum dev=254:3 sector 77480 res=0
> > type=UNKNOWN[1337] msg=audit(1630425112.119:201): module=integrity
> > op=integrity-checksum dev=254:3 sector 77480 res=0
> > type=UNKNOWN[1337] msg=audit(1630425112.119:202): module=integrity
> > op=integrity-checksum dev=254:3 sector 77480 res=0
> > type=UNKNOWN[1337] msg=audit(1630425112.119:203): module=integrity
> > op=integrity-checksum dev=254:3 sector 77480 res=0
> >
> > Signed-off-by: Michael Weiß <michael.weiss@xxxxxxxxxxxxxxxxxxx>
> > ---
> > drivers/md/Kconfig | 10 +++++
> > drivers/md/Makefile | 4 ++
> > drivers/md/dm-audit.c | 79 ++++++++++++++++++++++++++++++++++++++
> > drivers/md/dm-audit.h | 62 ++++++++++++++++++++++++++++++
> > include/uapi/linux/audit.h | 2 +
> > 5 files changed, 157 insertions(+)
> > create mode 100644 drivers/md/dm-audit.c
> > create mode 100644 drivers/md/dm-audit.h
> >
> > diff --git a/drivers/md/Kconfig b/drivers/md/Kconfig
> > index 0602e82a9516..48adbec12148 100644
> > --- a/drivers/md/Kconfig
> > +++ b/drivers/md/Kconfig
> > @@ -608,6 +608,7 @@ config DM_INTEGRITY
> > select CRYPTO
> > select CRYPTO_SKCIPHER
> > select ASYNC_XOR
> > + select DM_AUDIT if AUDIT
> > help
> > This device-mapper target emulates a block device that has
> > additional per-sector tags that can be used for storing
> > @@ -640,4 +641,13 @@ config DM_ZONED
> >
> > If unsure, say N.
> >
> > +config DM_AUDIT
> > + bool "DM audit events"
> > + depends on AUDIT
> > + help
> > + Generate audit events for device-mapper.
> > +
> > + Enables audit logging of several security relevant events in the
> > + particular device-mapper targets, especially the integrity target.
> > +
> > endif # MD
> > diff --git a/drivers/md/Makefile b/drivers/md/Makefile
> > index a74aaf8b1445..2f83d649500d 100644
> > --- a/drivers/md/Makefile
> > +++ b/drivers/md/Makefile
> > @@ -103,3 +103,7 @@ endif
> > ifeq ($(CONFIG_DM_VERITY_VERIFY_ROOTHASH_SIG),y)
> > dm-verity-objs += dm-verity-verify-sig.o
> > endif
> > +
> > +ifeq ($(CONFIG_DM_AUDIT),y)
> > +dm-mod-objs += dm-audit.o
> > +endif
> > diff --git a/drivers/md/dm-audit.c b/drivers/md/dm-audit.c
> > new file mode 100644
> > index 000000000000..761ecfdcd49a
> > --- /dev/null
> > +++ b/drivers/md/dm-audit.c
> > @@ -0,0 +1,79 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * Creating audit records for mapped devices.
> > + *
> > + * Copyright (C) 2021 Fraunhofer AISEC. All rights reserved.
> > + *
> > + * Authors: Michael Weiß <michael.weiss@xxxxxxxxxxxxxxxxxxx>
> > + */
> > +
> > +#include <linux/audit.h>
> > +#include <linux/module.h>
> > +#include <linux/device-mapper.h>
> > +#include <linux/bio.h>
> > +#include <linux/blkdev.h>
> > +
> > +#include "dm-audit.h"
> > +#include "dm-core.h"
> > +
> > +static struct audit_buffer *dm_audit_log_start(int audit_type,
> > + const char *dm_msg_prefix,
> > + const char *op)
> > +{
> > + struct audit_buffer *ab;
> > +
> > + if (audit_enabled == AUDIT_OFF)
> > + return NULL;
> > +
> > + ab = audit_log_start(audit_context(), GFP_KERNEL, audit_type);
> > + if (unlikely(!ab))
> > + return NULL;
> > +
> > + audit_log_format(ab, "module=%s op=%s", dm_msg_prefix, op);
> > + return ab;
> > +}
> > +
> > +void dm_audit_log_ti(int audit_type, const char *dm_msg_prefix, const char *op,
> > + struct dm_target *ti, int result)
> > +{
> > + struct audit_buffer *ab;
> > + struct mapped_device *md = dm_table_get_md(ti->table);
> > + int dev_major = dm_disk(md)->major;
> > + int dev_minor = dm_disk(md)->first_minor;
> > +
> > + ab = dm_audit_log_start(audit_type, dm_msg_prefix, op);
> > + if (unlikely(!ab))
> > + return;
> > +
> > + switch (audit_type) {
> > + case AUDIT_DM_CTRL:
> > + audit_log_task_info(ab);
> > + audit_log_format(ab, " dev=%d:%d error_msg='%s'", dev_major,
> > + dev_minor, !result ? ti->error : "success");
> > + break;
> > + case AUDIT_DM_EVENT:
> > + audit_log_format(ab, " dev=%d:%d sector=?", dev_major,
> > + dev_minor);
> > + break;
> > + }
> > + audit_log_format(ab, " res=%d", result);
> > + audit_log_end(ab);
> > +}
> > +EXPORT_SYMBOL_GPL(dm_audit_log_ti);
>
> Just checking, but are you okay when the inevitable happens and
> someone passes an @audit_type that is not either AUDIT_CM_CTRL or
> AUDIT_DM_EVENT? Right now that will succeed without error and give a
> rather short audit record.
>
Well, this function is not intended to be called directly from other dm
modules. Only the corresponding wrapper functions in dm-audit.h should
be called. I will add a comment in the header and provide a default case
which does not log anything here.

> > +void dm_audit_log_bio(const char *dm_msg_prefix, const char *op,
> > + struct bio *bio, sector_t sector, int result)
> > +{
> > + struct audit_buffer *ab;
> > + int dev_major = MAJOR(bio->bi_bdev->bd_dev);
> > + int dev_minor = MINOR(bio->bi_bdev->bd_dev);
> > +
> > + ab = dm_audit_log_start(AUDIT_DM_EVENT, dm_msg_prefix, op);
> > + if (unlikely(!ab))
> > + return;
> > +
> > + audit_log_format(ab, " dev=%d:%d sector %llu res=%d",
> > + dev_major, dev_minor, sector, result);
>
> I think you forgot the "=" after "sector", e.g. "sector=%llu".
>
Damn, you are right. I'll fix that.

> > + audit_log_end(ab);
> > +}
> > +EXPORT_SYMBOL_GPL(dm_audit_log_bio);

--
Thanks,
Michael

Next message: lisa hugh: "MASSAGE FROM >>MS LISA HUGH."
Previous message: Len Baker: "[PATCH] brcmfmac: Replace zero-length array with flexible array member"
In reply to: Paul Moore: "Re: [PATCH v3 1/3] dm: introduce audit event module for device mapper"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]