dm-devel.redhat.com archive mirror
 help / color / mirror / Atom feed
* [dm-devel] [PATCH v4 0/3] dm: audit event logging
@ 2021-09-04  9:59 Michael Weiß
  2021-09-04  9:59 ` [dm-devel] [PATCH v4 1/3] dm: introduce audit event module for device mapper Michael Weiß
                   ` (3 more replies)
  0 siblings, 4 replies; 9+ messages in thread
From: Michael Weiß @ 2021-09-04  9:59 UTC (permalink / raw)
  To: Paul Moore, Casey Schaufler
  Cc: Michael Weiß,
	Mike Snitzer, linux-kernel, Eric Paris, linux-raid, Song Liu,
	dm-devel, linux-audit, Alasdair Kergon

dm integrity and also stacked dm crypt devices track integrity
violations internally. Thus, integrity violations could be polled
from user space, e.g., by 'integritysetup status'.

>From an auditing perspective, we only could see that there were
a number of integrity violations, but not when and where the
violation exactly was taking place. The current error log to
the kernel ring buffer, contains those information, time stamp and
sector on device. However, for auditing the audit subsystem provides
a separate logging mechanism which meets certain criteria for secure
audit logging.

With this small series we make use of the kernel audit framework
and extend the dm driver to log audit events in case of such
integrity violations. Further, we also log construction and
destruction of the device mappings.

We focus on dm-integrity and stacked dm-crypt devices for now.
However, the helper functions to log audit messages should be
applicable to dm-verity too.

The first patch introduce generic audit wrapper functions.
The second patch makes use of the audit wrapper functions in the
dm-integrity.c.
The third patch uses the wrapper functions in dm-crypt.c.

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

v4 Changes:
- Added comments on intended use of wrapper functions in dm-audit.h
- dm_audit_log_bio(): Fixed missing '=' as spotted by Paul
- dm_audit_log_ti(): Handle wrong audit_type as suggested by Paul

v3 Changes:
- Use of two audit event types AUDIT_DM_EVENT und AUDIT_DM_CTRL
- Additionaly use audit_log_task_info in case of AUDIT_DM_CTRL messages
- Provide consistent fields per message type as suggested by Paul
- Added sample events to commit message of [1/3] as suggested by Paul
- Rebased on v5.14

v2 Changes:
- Fixed compile errors if CONFIG_DM_AUDIT is not set
- Fixed formatting and typos as suggested by Casey

Michael Weiß (3):
  dm: introduce audit event module for device mapper
  dm integrity: log audit events for dm-integrity target
  dm crypt: log aead integrity violations to audit subsystem

 drivers/md/Kconfig         | 10 +++++
 drivers/md/Makefile        |  4 ++
 drivers/md/dm-audit.c      | 84 ++++++++++++++++++++++++++++++++++++++
 drivers/md/dm-audit.h      | 66 ++++++++++++++++++++++++++++++
 drivers/md/dm-crypt.c      | 22 ++++++++--
 drivers/md/dm-integrity.c  | 25 ++++++++++--
 include/uapi/linux/audit.h |  2 +
 7 files changed, 205 insertions(+), 8 deletions(-)
 create mode 100644 drivers/md/dm-audit.c
 create mode 100644 drivers/md/dm-audit.h

-- 
2.20.1

--
dm-devel mailing list
dm-devel@redhat.com
https://listman.redhat.com/mailman/listinfo/dm-devel

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

* [dm-devel] [PATCH v4 1/3] dm: introduce audit event module for device mapper
  2021-09-04  9:59 [dm-devel] [PATCH v4 0/3] dm: audit event logging Michael Weiß
@ 2021-09-04  9:59 ` Michael Weiß
  2021-09-04  9:59 ` [dm-devel] [PATCH v4 2/3] dm integrity: log audit events for dm-integrity target Michael Weiß
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 9+ messages in thread
From: Michael Weiß @ 2021-09-04  9:59 UTC (permalink / raw)
  To: Paul Moore, Casey Schaufler
  Cc: Michael Weiß,
	Mike Snitzer, linux-kernel, Eric Paris, linux-raid, Song Liu,
	dm-devel, linux-audit, Alasdair Kergon

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@aisec.fraunhofer.de>
---
 drivers/md/Kconfig         | 10 +++++
 drivers/md/Makefile        |  4 ++
 drivers/md/dm-audit.c      | 84 ++++++++++++++++++++++++++++++++++++++
 drivers/md/dm-audit.h      | 66 ++++++++++++++++++++++++++++++
 include/uapi/linux/audit.h |  2 +
 5 files changed, 166 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..3049dfe67e50
--- /dev/null
+++ b/drivers/md/dm-audit.c
@@ -0,0 +1,84 @@
+// 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@aisec.fraunhofer.de>
+ */
+
+#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 = NULL;
+	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;
+
+	switch (audit_type) {
+	case AUDIT_DM_CTRL:
+		ab = dm_audit_log_start(audit_type, dm_msg_prefix, op);
+		if (unlikely(!ab))
+			return;
+		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:
+		ab = dm_audit_log_start(audit_type, dm_msg_prefix, op);
+		if (unlikely(!ab))
+			return;
+		audit_log_format(ab, " dev=%d:%d sector=?", dev_major,
+				 dev_minor);
+		break;
+	default: /* unintended use */
+		return;
+	}
+
+	audit_log_format(ab, " res=%d", result);
+	audit_log_end(ab);
+}
+EXPORT_SYMBOL_GPL(dm_audit_log_ti);
+
+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);
+	audit_log_end(ab);
+}
+EXPORT_SYMBOL_GPL(dm_audit_log_bio);
diff --git a/drivers/md/dm-audit.h b/drivers/md/dm-audit.h
new file mode 100644
index 000000000000..2385f2b659be
--- /dev/null
+++ b/drivers/md/dm-audit.h
@@ -0,0 +1,66 @@
+/* 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@aisec.fraunhofer.de>
+ */
+
+#ifndef DM_AUDIT_H
+#define DM_AUDIT_H
+
+#include <linux/device-mapper.h>
+#include <linux/audit.h>
+
+#ifdef CONFIG_DM_AUDIT
+void dm_audit_log_bio(const char *dm_msg_prefix, const char *op,
+		      struct bio *bio, sector_t sector, int result);
+
+/*
+ * dm_audit_log_ti() is not intended to be used directly in dm modules,
+ * the wrapper functions below should be called by dm modules instead.
+ */
+void dm_audit_log_ti(int audit_type, const char *dm_msg_prefix, const char *op,
+		     struct dm_target *ti, int result);
+
+static inline void dm_audit_log_ctr(const char *dm_msg_prefix,
+				    struct dm_target *ti, int result)
+{
+	dm_audit_log_ti(AUDIT_DM_CTRL, dm_msg_prefix, "ctr", ti, result);
+}
+
+static inline void dm_audit_log_dtr(const char *dm_msg_prefix,
+				    struct dm_target *ti, int result)
+{
+	dm_audit_log_ti(AUDIT_DM_CTRL, dm_msg_prefix, "dtr", ti, result);
+}
+
+static inline void dm_audit_log_target(const char *dm_msg_prefix, const char *op,
+				       struct dm_target *ti, int result)
+{
+	dm_audit_log_ti(AUDIT_DM_EVENT, dm_msg_prefix, op, ti, result);
+}
+#else
+static inline void dm_audit_log_bio(const char *dm_msg_prefix, const char *op,
+				    struct bio *bio, sector_t sector,
+				    int result)
+{
+}
+static inline void dm_audit_log_target(const char *dm_msg_prefix,
+				       const char *op, struct dm_target *ti,
+				       int result)
+{
+}
+static inline void dm_audit_log_ctr(const char *dm_msg_prefix,
+				    struct dm_target *ti, int result)
+{
+}
+
+static inline void dm_audit_log_dtr(const char *dm_msg_prefix,
+				    struct dm_target *ti, int result)
+{
+}
+#endif
+
+#endif
diff --git a/include/uapi/linux/audit.h b/include/uapi/linux/audit.h
index daa481729e9b..6650ab6def2a 100644
--- a/include/uapi/linux/audit.h
+++ b/include/uapi/linux/audit.h
@@ -118,6 +118,8 @@
 #define AUDIT_TIME_ADJNTPVAL	1333	/* NTP value adjustment */
 #define AUDIT_BPF		1334	/* BPF subsystem */
 #define AUDIT_EVENT_LISTENER	1335	/* Task joined multicast read socket */
+#define AUDIT_DM_CTRL		1336	/* Device Mapper target control */
+#define AUDIT_DM_EVENT		1337	/* Device Mapper events */
 
 #define AUDIT_AVC		1400	/* SE Linux avc denial or grant */
 #define AUDIT_SELINUX_ERR	1401	/* Internal SE Linux Errors */
-- 
2.20.1

--
dm-devel mailing list
dm-devel@redhat.com
https://listman.redhat.com/mailman/listinfo/dm-devel

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

* [dm-devel] [PATCH v4 2/3] dm integrity: log audit events for dm-integrity target
  2021-09-04  9:59 [dm-devel] [PATCH v4 0/3] dm: audit event logging Michael Weiß
  2021-09-04  9:59 ` [dm-devel] [PATCH v4 1/3] dm: introduce audit event module for device mapper Michael Weiß
@ 2021-09-04  9:59 ` Michael Weiß
  2021-09-04  9:59 ` [dm-devel] [PATCH v4 3/3] dm crypt: log aead integrity violations to audit subsystem Michael Weiß
  2021-09-08  0:59 ` [dm-devel] [PATCH v4 0/3] dm: audit event logging Richard Guy Briggs
  3 siblings, 0 replies; 9+ messages in thread
From: Michael Weiß @ 2021-09-04  9:59 UTC (permalink / raw)
  To: Paul Moore, Casey Schaufler
  Cc: Michael Weiß,
	Mike Snitzer, linux-kernel, Eric Paris, linux-raid, Song Liu,
	dm-devel, linux-audit, Alasdair Kergon

dm-integrity signals integrity violations by returning I/O errors
to user space. To identify integrity violations by a controlling
instance, the kernel audit subsystem can be used to emit audit
events to user space. We use the new dm-audit submodule allowing
to emit audit events on relevant I/O errors.

The construction and destruction of integrity device mappings are
also relevant for auditing a system. Thus, those events are also
logged as audit events.

Signed-off-by: Michael Weiß <michael.weiss@aisec.fraunhofer.de>
---
 drivers/md/dm-integrity.c | 25 +++++++++++++++++++++----
 1 file changed, 21 insertions(+), 4 deletions(-)

diff --git a/drivers/md/dm-integrity.c b/drivers/md/dm-integrity.c
index 20f2510db1f6..a881ead4b506 100644
--- a/drivers/md/dm-integrity.c
+++ b/drivers/md/dm-integrity.c
@@ -23,6 +23,8 @@
 #include <linux/async_tx.h>
 #include <linux/dm-bufio.h>
 
+#include "dm-audit.h"
+
 #define DM_MSG_PREFIX "integrity"
 
 #define DEFAULT_INTERLEAVE_SECTORS	32768
@@ -539,6 +541,7 @@ static int sb_mac(struct dm_integrity_c *ic, bool wr)
 		}
 		if (memcmp((__u8 *)ic->sb + (1 << SECTOR_SHIFT) - size, result, size)) {
 			dm_integrity_io_error(ic, "superblock mac", -EILSEQ);
+			dm_audit_log_target(DM_MSG_PREFIX, "mac-superblock", ic->ti, 0);
 			return -EILSEQ;
 		}
 	}
@@ -876,8 +879,10 @@ static void rw_section_mac(struct dm_integrity_c *ic, unsigned section, bool wr)
 		if (likely(wr))
 			memcpy(&js->mac, result + (j * JOURNAL_MAC_PER_SECTOR), JOURNAL_MAC_PER_SECTOR);
 		else {
-			if (memcmp(&js->mac, result + (j * JOURNAL_MAC_PER_SECTOR), JOURNAL_MAC_PER_SECTOR))
+			if (memcmp(&js->mac, result + (j * JOURNAL_MAC_PER_SECTOR), JOURNAL_MAC_PER_SECTOR)) {
 				dm_integrity_io_error(ic, "journal mac", -EILSEQ);
+				dm_audit_log_target(DM_MSG_PREFIX, "mac-journal", ic->ti, 0);
+			}
 		}
 	}
 }
@@ -1782,10 +1787,15 @@ static void integrity_metadata(struct work_struct *w)
 			if (unlikely(r)) {
 				if (r > 0) {
 					char b[BDEVNAME_SIZE];
-					DMERR_LIMIT("%s: Checksum failed at sector 0x%llx", bio_devname(bio, b),
-						    (sector - ((r + ic->tag_size - 1) / ic->tag_size)));
+					sector_t s;
+
+					s = sector - ((r + ic->tag_size - 1) / ic->tag_size);
+					DMERR_LIMIT("%s: Checksum failed at sector 0x%llx",
+						    bio_devname(bio, b), s);
 					r = -EILSEQ;
 					atomic64_inc(&ic->number_of_mismatches);
+					dm_audit_log_bio(DM_MSG_PREFIX, "integrity-checksum",
+							 bio, s, 0);
 				}
 				if (likely(checksums != checksums_onstack))
 					kfree(checksums);
@@ -1991,6 +2001,8 @@ static bool __journal_read_write(struct dm_integrity_io *dio, struct bio *bio,
 					if (unlikely(memcmp(checksums_onstack, journal_entry_tag(ic, je), ic->tag_size))) {
 						DMERR_LIMIT("Checksum failed when reading from journal, at sector 0x%llx",
 							    logical_sector);
+						dm_audit_log_bio(DM_MSG_PREFIX, "journal-checksum",
+								 bio, logical_sector, 0);
 					}
 				}
 #endif
@@ -2534,8 +2546,10 @@ static void do_journal_write(struct dm_integrity_c *ic, unsigned write_start,
 
 					integrity_sector_checksum(ic, sec + ((l - j) << ic->sb->log2_sectors_per_block),
 								  (char *)access_journal_data(ic, i, l), test_tag);
-					if (unlikely(memcmp(test_tag, journal_entry_tag(ic, je2), ic->tag_size)))
+					if (unlikely(memcmp(test_tag, journal_entry_tag(ic, je2), ic->tag_size))) {
 						dm_integrity_io_error(ic, "tag mismatch when replaying journal", -EILSEQ);
+						dm_audit_log_target(DM_MSG_PREFIX, "integrity-replay-journal", ic->ti, 0);
+					}
 				}
 
 				journal_entry_set_unused(je2);
@@ -4490,9 +4504,11 @@ static int dm_integrity_ctr(struct dm_target *ti, unsigned argc, char **argv)
 	if (ic->discard)
 		ti->num_discard_bios = 1;
 
+	dm_audit_log_ctr(DM_MSG_PREFIX, ti, 1);
 	return 0;
 
 bad:
+	dm_audit_log_ctr(DM_MSG_PREFIX, ti, 0);
 	dm_integrity_dtr(ti);
 	return r;
 }
@@ -4566,6 +4582,7 @@ static void dm_integrity_dtr(struct dm_target *ti)
 	free_alg(&ic->journal_mac_alg);
 
 	kfree(ic);
+	dm_audit_log_dtr(DM_MSG_PREFIX, ti, 1);
 }
 
 static struct target_type integrity_target = {
-- 
2.20.1

--
dm-devel mailing list
dm-devel@redhat.com
https://listman.redhat.com/mailman/listinfo/dm-devel

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

* [dm-devel] [PATCH v4 3/3] dm crypt: log aead integrity violations to audit subsystem
  2021-09-04  9:59 [dm-devel] [PATCH v4 0/3] dm: audit event logging Michael Weiß
  2021-09-04  9:59 ` [dm-devel] [PATCH v4 1/3] dm: introduce audit event module for device mapper Michael Weiß
  2021-09-04  9:59 ` [dm-devel] [PATCH v4 2/3] dm integrity: log audit events for dm-integrity target Michael Weiß
@ 2021-09-04  9:59 ` Michael Weiß
  2021-09-08  0:59 ` [dm-devel] [PATCH v4 0/3] dm: audit event logging Richard Guy Briggs
  3 siblings, 0 replies; 9+ messages in thread
From: Michael Weiß @ 2021-09-04  9:59 UTC (permalink / raw)
  To: Paul Moore, Casey Schaufler
  Cc: Michael Weiß,
	Mike Snitzer, linux-kernel, Eric Paris, linux-raid, Song Liu,
	dm-devel, linux-audit, Alasdair Kergon

Since dm-crypt target can be stacked on dm-integrity targets to
provide authenticated encryption, integrity violations are recognized
here during aead computation. We use the dm-audit submodule to
signal those events to user space, too.

The construction and destruction of crypt device mappings are also
logged as audit events.

Signed-off-by: Michael Weiß <michael.weiss@aisec.fraunhofer.de>
---
 drivers/md/dm-crypt.c | 22 ++++++++++++++++++----
 1 file changed, 18 insertions(+), 4 deletions(-)

diff --git a/drivers/md/dm-crypt.c b/drivers/md/dm-crypt.c
index 50f4cbd600d5..5e02002345fa 100644
--- a/drivers/md/dm-crypt.c
+++ b/drivers/md/dm-crypt.c
@@ -41,6 +41,8 @@
 
 #include <linux/device-mapper.h>
 
+#include "dm-audit.h"
+
 #define DM_MSG_PREFIX "crypt"
 
 /*
@@ -1362,8 +1364,12 @@ static int crypt_convert_block_aead(struct crypt_config *cc,
 
 	if (r == -EBADMSG) {
 		char b[BDEVNAME_SIZE];
-		DMERR_LIMIT("%s: INTEGRITY AEAD ERROR, sector %llu", bio_devname(ctx->bio_in, b),
-			    (unsigned long long)le64_to_cpu(*sector));
+		sector_t s = le64_to_cpu(*sector);
+
+		DMERR_LIMIT("%s: INTEGRITY AEAD ERROR, sector %llu",
+			    bio_devname(ctx->bio_in, b), s);
+		dm_audit_log_bio(DM_MSG_PREFIX, "integrity-aead",
+				 ctx->bio_in, s, 0);
 	}
 
 	if (!r && cc->iv_gen_ops && cc->iv_gen_ops->post)
@@ -2173,8 +2179,12 @@ static void kcryptd_async_done(struct crypto_async_request *async_req,
 
 	if (error == -EBADMSG) {
 		char b[BDEVNAME_SIZE];
-		DMERR_LIMIT("%s: INTEGRITY AEAD ERROR, sector %llu", bio_devname(ctx->bio_in, b),
-			    (unsigned long long)le64_to_cpu(*org_sector_of_dmreq(cc, dmreq)));
+		sector_t s = le64_to_cpu(*org_sector_of_dmreq(cc, dmreq));
+
+		DMERR_LIMIT("%s: INTEGRITY AEAD ERROR, sector %llu",
+			    bio_devname(ctx->bio_in, b), s);
+		dm_audit_log_bio(DM_MSG_PREFIX, "integrity-aead",
+				 ctx->bio_in, s, 0);
 		io->error = BLK_STS_PROTECTION;
 	} else if (error < 0)
 		io->error = BLK_STS_IOERR;
@@ -2729,6 +2739,8 @@ static void crypt_dtr(struct dm_target *ti)
 	dm_crypt_clients_n--;
 	crypt_calculate_pages_per_client();
 	spin_unlock(&dm_crypt_clients_lock);
+
+	dm_audit_log_dtr(DM_MSG_PREFIX, ti, 1);
 }
 
 static int crypt_ctr_ivmode(struct dm_target *ti, const char *ivmode)
@@ -3357,9 +3369,11 @@ static int crypt_ctr(struct dm_target *ti, unsigned int argc, char **argv)
 	ti->num_flush_bios = 1;
 	ti->limit_swap_bios = true;
 
+	dm_audit_log_ctr(DM_MSG_PREFIX, ti, 1);
 	return 0;
 
 bad:
+	dm_audit_log_ctr(DM_MSG_PREFIX, ti, 0);
 	crypt_dtr(ti);
 	return ret;
 }
-- 
2.20.1

--
dm-devel mailing list
dm-devel@redhat.com
https://listman.redhat.com/mailman/listinfo/dm-devel

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

* Re: [dm-devel] [PATCH v4 0/3] dm: audit event logging
  2021-09-04  9:59 [dm-devel] [PATCH v4 0/3] dm: audit event logging Michael Weiß
                   ` (2 preceding siblings ...)
  2021-09-04  9:59 ` [dm-devel] [PATCH v4 3/3] dm crypt: log aead integrity violations to audit subsystem Michael Weiß
@ 2021-09-08  0:59 ` Richard Guy Briggs
  2021-09-08  8:26   ` Weiß, Michael
  3 siblings, 1 reply; 9+ messages in thread
From: Richard Guy Briggs @ 2021-09-08  0:59 UTC (permalink / raw)
  To: Michael Weiß
  Cc: Paul Moore, Mike Snitzer, linux-kernel, Eric Paris, linux-raid,
	Song Liu, dm-devel, linux-audit, Casey Schaufler,
	Alasdair Kergon

On 2021-09-04 11:59, Michael Weiß wrote:
> dm integrity and also stacked dm crypt devices track integrity
> violations internally. Thus, integrity violations could be polled
> from user space, e.g., by 'integritysetup status'.
> 
> >From an auditing perspective, we only could see that there were
> a number of integrity violations, but not when and where the
> violation exactly was taking place. The current error log to
> the kernel ring buffer, contains those information, time stamp and
> sector on device. However, for auditing the audit subsystem provides
> a separate logging mechanism which meets certain criteria for secure
> audit logging.
> 
> With this small series we make use of the kernel audit framework
> and extend the dm driver to log audit events in case of such
> integrity violations. Further, we also log construction and
> destruction of the device mappings.
> 
> We focus on dm-integrity and stacked dm-crypt devices for now.
> However, the helper functions to log audit messages should be
> applicable to dm-verity too.
> 
> The first patch introduce generic audit wrapper functions.
> The second patch makes use of the audit wrapper functions in the
> dm-integrity.c.
> The third patch uses the wrapper functions in dm-crypt.c.
> 
> 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

Are these isolated records, or are they accompanied by a type=SYSCALL
record in your logs?

The reason I ask is that audit_log_task_info() is included in three of
the calling methods (dm_audit_log_{target,ctr,dtr}) which use a
combination of AUDIT_DM_CTRL/AUDIT_DM_EVENT type while the fourth
(dm_audit_log_bio) also uses one of the types above but does not include
audit_log_task_info().  If all these records are accompanied by SYSCALL
records, then the task info would be redundant (and might even conflict
if there's a bug).  Another minor oddity is the double "=" for the subj
field, which doesn't appear to be a bug in your code, but still puzzling.

Are those last 10 records expected to be identical other than event
serial number?

> v4 Changes:
> - Added comments on intended use of wrapper functions in dm-audit.h
> - dm_audit_log_bio(): Fixed missing '=' as spotted by Paul
> - dm_audit_log_ti(): Handle wrong audit_type as suggested by Paul
> 
> v3 Changes:
> - Use of two audit event types AUDIT_DM_EVENT und AUDIT_DM_CTRL
> - Additionaly use audit_log_task_info in case of AUDIT_DM_CTRL messages
> - Provide consistent fields per message type as suggested by Paul
> - Added sample events to commit message of [1/3] as suggested by Paul
> - Rebased on v5.14
> 
> v2 Changes:
> - Fixed compile errors if CONFIG_DM_AUDIT is not set
> - Fixed formatting and typos as suggested by Casey
> 
> Michael Weiß (3):
>   dm: introduce audit event module for device mapper
>   dm integrity: log audit events for dm-integrity target
>   dm crypt: log aead integrity violations to audit subsystem
> 
>  drivers/md/Kconfig         | 10 +++++
>  drivers/md/Makefile        |  4 ++
>  drivers/md/dm-audit.c      | 84 ++++++++++++++++++++++++++++++++++++++
>  drivers/md/dm-audit.h      | 66 ++++++++++++++++++++++++++++++
>  drivers/md/dm-crypt.c      | 22 ++++++++--
>  drivers/md/dm-integrity.c  | 25 ++++++++++--
>  include/uapi/linux/audit.h |  2 +
>  7 files changed, 205 insertions(+), 8 deletions(-)
>  create mode 100644 drivers/md/dm-audit.c
>  create mode 100644 drivers/md/dm-audit.h
> 
> -- 
> 2.20.1
> 
> --
> Linux-audit mailing list
> Linux-audit@redhat.com
> https://listman.redhat.com/mailman/listinfo/linux-audit

- 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

--
dm-devel mailing list
dm-devel@redhat.com
https://listman.redhat.com/mailman/listinfo/dm-devel


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

* Re: [dm-devel] [PATCH v4 0/3] dm: audit event logging
  2021-09-08  0:59 ` [dm-devel] [PATCH v4 0/3] dm: audit event logging Richard Guy Briggs
@ 2021-09-08  8:26   ` Weiß, Michael
  2021-09-08 13:16     ` Richard Guy Briggs
  0 siblings, 1 reply; 9+ messages in thread
From: Weiß, Michael @ 2021-09-08  8:26 UTC (permalink / raw)
  To: rgb
  Cc: paul, snitzer, linux-kernel, eparis, linux-raid, song, dm-devel,
	linux-audit, casey, agk

On Tue, 2021-09-07 at 20:59 -0400, Richard Guy Briggs wrote:
> On 2021-09-04 11:59, Michael Weiß wrote:
> > dm integrity and also stacked dm crypt devices track integrity
> > violations internally. Thus, integrity violations could be polled
> > from user space, e.g., by 'integritysetup status'.
> > 
> > > From an auditing perspective, we only could see that there were
> > a number of integrity violations, but not when and where the
> > violation exactly was taking place. The current error log to
> > the kernel ring buffer, contains those information, time stamp and
> > sector on device. However, for auditing the audit subsystem provides
> > a separate logging mechanism which meets certain criteria for secure
> > audit logging.
> > 
> > With this small series we make use of the kernel audit framework
> > and extend the dm driver to log audit events in case of such
> > integrity violations. Further, we also log construction and
> > destruction of the device mappings.
> > 
> > We focus on dm-integrity and stacked dm-crypt devices for now.
> > However, the helper functions to log audit messages should be
> > applicable to dm-verity too.
> > 
> > The first patch introduce generic audit wrapper functions.
> > The second patch makes use of the audit wrapper functions in the
> > dm-integrity.c.
> > The third patch uses the wrapper functions in dm-crypt.c.
> > 
> > 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
> 
> Are these isolated records, or are they accompanied by a type=SYSCALL
> record in your logs?

You are right the dm_audit_log_{ctr,dtr} functions produce type=AUDIT_DM_CTRL
accompanied by a
type=SYSCALL. This was a mistake by me. I grepped
the audit log with 'grep -e "133[6-7]"' during my
tests, thus I have
missed that. I will remove the audit_log_task_info() call in the
internal dm_audit_log_ti() function
for type=AUDIT_DM_CTRL.

dm_audit_log_target and dm_audit_log_bio are using type=AUDIT_DM_EVENT,
These are isolated events since they are not triggert in user context. 

> 
> The reason I ask is that audit_log_task_info() is included in three of
> the calling methods (dm_audit_log_{target,ctr,dtr}) which use a
> combination of AUDIT_DM_CTRL/AUDIT_DM_EVENT type while the fourth
> (dm_audit_log_bio) also uses one of the types above but does not include
> audit_log_task_info().  If all these records are accompanied by SYSCALL
> records, then the task info would be redundant (and might even conflict
> if there's a bug).  Another minor oddity is the double "=" for the subj
> field, which doesn't appear to be a bug in your code, but still puzzling.

In the test setup, I had Apparmor enabled and set as default security module.
This behavior occurs in any audit_log message.
Seems that this is coming from the label handling there. Having a quick look
at the code there is that they use '=' in the label to provide a root view as
part of their policy virtualization. The corresponding commit is sitting
there since 2017: "26b7899510ae243e392960704ebdba52d05fbb13"

> 
> Are those last 10 records expected to be identical other than event
> serial number?

Yes, because the access to the corrupt sector is made 10 times.
This reflects exactly the same behavior without the audit logging, in the 
kernel debug log.

> 
> > v4 Changes:
> > - Added comments on intended use of wrapper functions in dm-audit.h
> > - dm_audit_log_bio(): Fixed missing '=' as spotted by Paul
> > - dm_audit_log_ti(): Handle wrong audit_type as suggested by Paul
> > 
> > v3 Changes:
> > - Use of two audit event types AUDIT_DM_EVENT und AUDIT_DM_CTRL
> > - Additionaly use audit_log_task_info in case of AUDIT_DM_CTRL messages
> > - Provide consistent fields per message type as suggested by Paul
> > - Added sample events to commit message of [1/3] as suggested by Paul
> > - Rebased on v5.14
> > 
> > v2 Changes:
> > - Fixed compile errors if CONFIG_DM_AUDIT is not set
> > - Fixed formatting and typos as suggested by Casey
> > 
> > Michael Weiß (3):
> >   dm: introduce audit event module for device mapper
> >   dm integrity: log audit events for dm-integrity target
> >   dm crypt: log aead integrity violations to audit subsystem
> > 
> >  drivers/md/Kconfig         | 10 +++++
> >  drivers/md/Makefile        |  4 ++
> >  drivers/md/dm-audit.c      | 84 ++++++++++++++++++++++++++++++++++++++
> >  drivers/md/dm-audit.h      | 66 ++++++++++++++++++++++++++++++
> >  drivers/md/dm-crypt.c      | 22 ++++++++--
> >  drivers/md/dm-integrity.c  | 25 ++++++++++--
> >  include/uapi/linux/audit.h |  2 +
> >  7 files changed, 205 insertions(+), 8 deletions(-)
> >  create mode 100644 drivers/md/dm-audit.c
> >  create mode 100644 drivers/md/dm-audit.h
> > 
> > -- 
> > 2.20.1
> > 
> > --
> > Linux-audit mailing list
> > Linux-audit@redhat.com
> > https://listman.redhat.com/mailman/listinfo/linux-audit
> 
> - 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
> 

Thanks,
Michael

--
dm-devel mailing list
dm-devel@redhat.com
https://listman.redhat.com/mailman/listinfo/dm-devel

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

* Re: [dm-devel] [PATCH v4 0/3] dm: audit event logging
  2021-09-08  8:26   ` Weiß, Michael
@ 2021-09-08 13:16     ` Richard Guy Briggs
  2021-09-08 15:39       ` Steve Grubb
  2021-09-12  9:38       ` Weiß, Michael
  0 siblings, 2 replies; 9+ messages in thread
From: Richard Guy Briggs @ 2021-09-08 13:16 UTC (permalink / raw)
  To: Weiß, Michael, sgrubb
  Cc: paul, snitzer, linux-kernel, eparis, linux-raid, song, dm-devel,
	linux-audit, casey, agk

On 2021-09-08 08:26, Weiß, Michael wrote:
> On Tue, 2021-09-07 at 20:59 -0400, Richard Guy Briggs wrote:
> > On 2021-09-04 11:59, Michael Weiß wrote:
> > > dm integrity and also stacked dm crypt devices track integrity
> > > violations internally. Thus, integrity violations could be polled
> > > from user space, e.g., by 'integritysetup status'.
> > > 
> > > > From an auditing perspective, we only could see that there were
> > > a number of integrity violations, but not when and where the
> > > violation exactly was taking place. The current error log to
> > > the kernel ring buffer, contains those information, time stamp and
> > > sector on device. However, for auditing the audit subsystem provides
> > > a separate logging mechanism which meets certain criteria for secure
> > > audit logging.
> > > 
> > > With this small series we make use of the kernel audit framework
> > > and extend the dm driver to log audit events in case of such
> > > integrity violations. Further, we also log construction and
> > > destruction of the device mappings.
> > > 
> > > We focus on dm-integrity and stacked dm-crypt devices for now.
> > > However, the helper functions to log audit messages should be
> > > applicable to dm-verity too.
> > > 
> > > The first patch introduce generic audit wrapper functions.
> > > The second patch makes use of the audit wrapper functions in the
> > > dm-integrity.c.
> > > The third patch uses the wrapper functions in dm-crypt.c.
> > > 
> > > 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
> > 
> > Are these isolated records, or are they accompanied by a type=SYSCALL
> > record in your logs?
> 
> You are right the dm_audit_log_{ctr,dtr} functions produce type=AUDIT_DM_CTRL
> accompanied by a
> type=SYSCALL. This was a mistake by me. I grepped
> the audit log with 'grep -e "133[6-7]"' during my
> tests, thus I have
> missed that. I will remove the audit_log_task_info() call in the
> internal dm_audit_log_ti() function
> for type=AUDIT_DM_CTRL.

(To get the whole events, use "ausearch ... -m 1336,1337 ...".)

> dm_audit_log_target and dm_audit_log_bio are using type=AUDIT_DM_EVENT,
> These are isolated events since they are not triggert in user context. 

Ok, so it sounds like those events *should* have task_info in their
record format since they are not accompanied by SYSCALL records that
already contain that information.  So it appears that
audit_log_task_info() should be moved from the type=AUDIT_DM_CTRL case
to the type=AUDIT_DM_EVENT case.

> > The reason I ask is that audit_log_task_info() is included in three of
> > the calling methods (dm_audit_log_{target,ctr,dtr}) which use a
> > combination of AUDIT_DM_CTRL/AUDIT_DM_EVENT type while the fourth
> > (dm_audit_log_bio) also uses one of the types above but does not include
> > audit_log_task_info().  If all these records are accompanied by SYSCALL
> > records, then the task info would be redundant (and might even conflict
> > if there's a bug).  Another minor oddity is the double "=" for the subj
> > field, which doesn't appear to be a bug in your code, but still puzzling.
> 
> In the test setup, I had Apparmor enabled and set as default security module.
> This behavior occurs in any audit_log message.
> Seems that this is coming from the label handling there. Having a quick look
> at the code there is that they use '=' in the label to provide a root view as
> part of their policy virtualization. The corresponding commit is sitting
> there since 2017: "26b7899510ae243e392960704ebdba52d05fbb13"

Interesting...  Thanks for tracking down that cause.  I don't know how
much pain that will cause the userspace parsing tools.  I've added Steve
Grubb to the Cc: to get his input, but this should not derail this patch
set.

This has parallels to this previously reported issue with ima/integrity:
	https://github.com/linux-audit/audit-kernel/issues/113

> > Are those last 10 records expected to be identical other than event
> > serial number?
> 
> Yes, because the access to the corrupt sector is made 10 times.
> This reflects exactly the same behavior without the audit logging, in the 
> kernel debug log.

Is there any other distinguishing information for that event other than
audit log serial number that would be useful to add?  (It doesn't sound
like it.)

> > > v4 Changes:
> > > - Added comments on intended use of wrapper functions in dm-audit.h
> > > - dm_audit_log_bio(): Fixed missing '=' as spotted by Paul
> > > - dm_audit_log_ti(): Handle wrong audit_type as suggested by Paul
> > > 
> > > v3 Changes:
> > > - Use of two audit event types AUDIT_DM_EVENT und AUDIT_DM_CTRL
> > > - Additionaly use audit_log_task_info in case of AUDIT_DM_CTRL messages
> > > - Provide consistent fields per message type as suggested by Paul
> > > - Added sample events to commit message of [1/3] as suggested by Paul
> > > - Rebased on v5.14
> > > 
> > > v2 Changes:
> > > - Fixed compile errors if CONFIG_DM_AUDIT is not set
> > > - Fixed formatting and typos as suggested by Casey
> > > 
> > > Michael Weiß (3):
> > >   dm: introduce audit event module for device mapper
> > >   dm integrity: log audit events for dm-integrity target
> > >   dm crypt: log aead integrity violations to audit subsystem
> > > 
> > >  drivers/md/Kconfig         | 10 +++++
> > >  drivers/md/Makefile        |  4 ++
> > >  drivers/md/dm-audit.c      | 84 ++++++++++++++++++++++++++++++++++++++
> > >  drivers/md/dm-audit.h      | 66 ++++++++++++++++++++++++++++++
> > >  drivers/md/dm-crypt.c      | 22 ++++++++--
> > >  drivers/md/dm-integrity.c  | 25 ++++++++++--
> > >  include/uapi/linux/audit.h |  2 +
> > >  7 files changed, 205 insertions(+), 8 deletions(-)
> > >  create mode 100644 drivers/md/dm-audit.c
> > >  create mode 100644 drivers/md/dm-audit.h
> > > 
> > > -- 
> > > 2.20.1
> > > 
> > > --
> > > Linux-audit mailing list
> > > Linux-audit@redhat.com
> > > https://listman.redhat.com/mailman/listinfo/linux-audit
> > 
> > - 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
> > 
> 
> Thanks,
> Michael

- 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

--
dm-devel mailing list
dm-devel@redhat.com
https://listman.redhat.com/mailman/listinfo/dm-devel


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

* Re: [dm-devel] [PATCH v4 0/3] dm: audit event logging
  2021-09-08 13:16     ` Richard Guy Briggs
@ 2021-09-08 15:39       ` Steve Grubb
  2021-09-12  9:38       ` Weiß, Michael
  1 sibling, 0 replies; 9+ messages in thread
From: Steve Grubb @ 2021-09-08 15:39 UTC (permalink / raw)
  To: Weiß, Michael, Richard Guy Briggs
  Cc: paul, snitzer, linux-kernel, eparis, linux-raid, song, dm-devel,
	linux-audit, casey, agk

On Wednesday, September 8, 2021 9:16:16 AM EDT Richard Guy Briggs wrote:
>  Another minor oddity is the double "=" for the subj
> 
> > > field, which doesn't appear to be a bug in your code, but still
> > > puzzling.
> > 
> > In the test setup, I had Apparmor enabled and set as default security
> > module. This behavior occurs in any audit_log message.
> > Seems that this is coming from the label handling there. Having a quick
> > look at the code there is that they use '=' in the label to provide a
> > root view as part of their policy virtualization. The corresponding
> > commit is sitting there since 2017:
> > "26b7899510ae243e392960704ebdba52d05fbb13"
> 
> Interesting...  Thanks for tracking down that cause.  I don't know how
> much pain that will cause the userspace parsing tools.  I've added Steve
> Grubb to the Cc: to get his input, but this should not derail this patch
> set.

It likely breaks any parser. I would even say that it's a malformed event 
that should be corrected. There's been a published a specification for audit 
events  for at least 5 years. Latest copy is here:

https://github.com/linux-audit/audit-documentation/wiki/SPEC-Writing-Good-Events

-Steve



--
dm-devel mailing list
dm-devel@redhat.com
https://listman.redhat.com/mailman/listinfo/dm-devel


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

* Re: [dm-devel] [PATCH v4 0/3] dm: audit event logging
  2021-09-08 13:16     ` Richard Guy Briggs
  2021-09-08 15:39       ` Steve Grubb
@ 2021-09-12  9:38       ` Weiß, Michael
  1 sibling, 0 replies; 9+ messages in thread
From: Weiß, Michael @ 2021-09-12  9:38 UTC (permalink / raw)
  To: sgrubb, rgb
  Cc: paul, snitzer, linux-kernel, eparis, linux-raid, song, dm-devel,
	linux-audit, casey, agk

On Wed, 2021-09-08 at 09:16 -0400, Richard Guy Briggs wrote:
> On 2021-09-08 08:26, Weiß, Michael wrote:
> > On Tue, 2021-09-07 at 20:59 -0400, Richard Guy Briggs wrote:
> > > On 2021-09-04 11:59, Michael Weiß wrote:
> > > > dm integrity and also stacked dm crypt devices track integrity
> > > > violations internally. Thus, integrity violations could be polled
> > > > from user space, e.g., by 'integritysetup status'.
> > > > 
> > > > > From an auditing perspective, we only could see that there were
> > > > a number of integrity violations, but not when and where the
> > > > violation exactly was taking place. The current error log to
> > > > the kernel ring buffer, contains those information, time stamp and
> > > > sector on device. However, for auditing the audit subsystem provides
> > > > a separate logging mechanism which meets certain criteria for secure
> > > > audit logging.
> > > > 
> > > > With this small series we make use of the kernel audit framework
> > > > and extend the dm driver to log audit events in case of such
> > > > integrity violations. Further, we also log construction and
> > > > destruction of the device mappings.
> > > > 
> > > > We focus on dm-integrity and stacked dm-crypt devices for now.
> > > > However, the helper functions to log audit messages should be
> > > > applicable to dm-verity too.
> > > > 
> > > > The first patch introduce generic audit wrapper functions.
> > > > The second patch makes use of the audit wrapper functions in the
> > > > dm-integrity.c.
> > > > The third patch uses the wrapper functions in dm-crypt.c.
> > > > 
> > > > 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
> > > 
> > > Are these isolated records, or are they accompanied by a type=SYSCALL
> > > record in your logs?
> > 
> > You are right the dm_audit_log_{ctr,dtr} functions produce type=AUDIT_DM_CTRL
> > accompanied by a
> > type=SYSCALL. This was a mistake by me. I grepped
> > the audit log with 'grep -e "133[6-7]"' during my
> > tests, thus I have
> > missed that. I will remove the audit_log_task_info() call in the
> > internal dm_audit_log_ti() function
> > for type=AUDIT_DM_CTRL.
> 
> (To get the whole events, use "ausearch ... -m 1336,1337 ...".)
> 
> > dm_audit_log_target and dm_audit_log_bio are using type=AUDIT_DM_EVENT,
> > These are isolated events since they are not triggert in user context. 
> 
> Ok, so it sounds like those events *should* have task_info in their
> record format since they are not accompanied by SYSCALL records that
> already contain that information.  So it appears that
> audit_log_task_info() should be moved from the type=AUDIT_DM_CTRL case
> to the type=AUDIT_DM_EVENT case.

Ok in further testing using ausearch as you proposed, it turned out that
also dm_log_audit_target()
is called in ioctl context and the DM_EVENTs
are accompanied by a SYSCALL record. So it seems that we
can completely
drop it from the dm_audit_log_ti() function.

> > > The reason I ask is that audit_log_task_info() is included in three of
> > > the calling methods (dm_audit_log_{target,ctr,dtr}) which use a
> > > combination of AUDIT_DM_CTRL/AUDIT_DM_EVENT type while the fourth
> > > (dm_audit_log_bio) also uses one of the types above but does not include
> > > audit_log_task_info().  If all these records are accompanied by SYSCALL
> > > records, then the task info would be redundant (and might even conflict
> > > if there's a bug).  Another minor oddity is the double "=" for the subj
> > > field, which doesn't appear to be a bug in your code, but still puzzling.
> > 
> > In the test setup, I had Apparmor enabled and set as default security module.
> > This behavior occurs in any audit_log message.
> > Seems that this is coming from the label handling there. Having a quick look
> > at the code there is that they use '=' in the label to provide a root view as
> > part of their policy virtualization. The corresponding commit is sitting
> > there since 2017: "26b7899510ae243e392960704ebdba52d05fbb13"
> 
> Interesting...  Thanks for tracking down that cause.  I don't know how
> much pain that will cause the userspace parsing tools.  I've added Steve
> Grubb to the Cc: to get his input, but this should not derail this patch
> set.
> 
> This has parallels to this previously reported issue with ima/integrity:
> 	https://github.com/linux-audit/audit-kernel/issues/113
> 
> > > Are those last 10 records expected to be identical other than event
> > > serial number?
> > 
> > Yes, because the access to the corrupt sector is made 10 times.
> > This reflects exactly the same behavior without the audit logging, in the 
> > kernel debug log.
> 
> Is there any other distinguishing information for that event other than
> audit log serial number that would be useful to add?  (It doesn't sound
> like it.)
No there isn't.

Further, I have encountered some other quirks, which I will fix in v5.
The dtr is called internally
on error in ctr, which results in the same
audit log serial of a ctr followed by a dtr event.
And I
missed to synchronize the audit log in dm-integrity.c, which results in
duplicate events logged by
the worker threads.

Thanks for the hint with ausearch. For our use case we do not depend on the
userspace part of linux-audit and I used that only for testing, and obviously
missed some conventions
there.

I think about to rename the error_msg field to reason and to pre-process the
string with strreplace()
to replace spaces by '-' instead of directly logging
error_msg= as quoted String. What do you guys think about this?

I also will run the test suit linked in Steve's wiki to be sure not to break
userspace part of linux-audit before I submit v5.

> 
> > > > v4 Changes:
> > > > - Added comments on intended use of wrapper functions in dm-audit.h
> > > > - dm_audit_log_bio(): Fixed missing '=' as spotted by Paul
> > > > - dm_audit_log_ti(): Handle wrong audit_type as suggested by Paul
> > > > 
> > > > v3 Changes:
> > > > - Use of two audit event types AUDIT_DM_EVENT und AUDIT_DM_CTRL
> > > > - Additionaly use audit_log_task_info in case of AUDIT_DM_CTRL messages
> > > > - Provide consistent fields per message type as suggested by Paul
> > > > - Added sample events to commit message of [1/3] as suggested by Paul
> > > > - Rebased on v5.14
> > > > 
> > > > v2 Changes:
> > > > - Fixed compile errors if CONFIG_DM_AUDIT is not set
> > > > - Fixed formatting and typos as suggested by Casey
> > > > 
> > > > Michael Weiß (3):
> > > >   dm: introduce audit event module for device mapper
> > > >   dm integrity: log audit events for dm-integrity target
> > > >   dm crypt: log aead integrity violations to audit subsystem
> > > > 
> > > >  drivers/md/Kconfig         | 10 +++++
> > > >  drivers/md/Makefile        |  4 ++
> > > >  drivers/md/dm-audit.c      | 84 ++++++++++++++++++++++++++++++++++++++
> > > >  drivers/md/dm-audit.h      | 66 ++++++++++++++++++++++++++++++
> > > >  drivers/md/dm-crypt.c      | 22 ++++++++--
> > > >  drivers/md/dm-integrity.c  | 25 ++++++++++--
> > > >  include/uapi/linux/audit.h |  2 +
> > > >  7 files changed, 205 insertions(+), 8 deletions(-)
> > > >  create mode 100644 drivers/md/dm-audit.c
> > > >  create mode 100644 drivers/md/dm-audit.h
> > > > 
> > > > -- 
> > > > 2.20.1
> > > > 
> > > > --
> > > > Linux-audit mailing list
> > > > Linux-audit@redhat.com
> > > > https://listman.redhat.com/mailman/listinfo/linux-audit
> > > 
> > > - 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
> > > 
> > 
> > Thanks,
> > Michael
> 
> - 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
> 


Thanks,
Michael

--
dm-devel mailing list
dm-devel@redhat.com
https://listman.redhat.com/mailman/listinfo/dm-devel

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

end of thread, other threads:[~2021-09-13  6:40 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-09-04  9:59 [dm-devel] [PATCH v4 0/3] dm: audit event logging Michael Weiß
2021-09-04  9:59 ` [dm-devel] [PATCH v4 1/3] dm: introduce audit event module for device mapper Michael Weiß
2021-09-04  9:59 ` [dm-devel] [PATCH v4 2/3] dm integrity: log audit events for dm-integrity target Michael Weiß
2021-09-04  9:59 ` [dm-devel] [PATCH v4 3/3] dm crypt: log aead integrity violations to audit subsystem Michael Weiß
2021-09-08  0:59 ` [dm-devel] [PATCH v4 0/3] dm: audit event logging Richard Guy Briggs
2021-09-08  8:26   ` Weiß, Michael
2021-09-08 13:16     ` Richard Guy Briggs
2021-09-08 15:39       ` Steve Grubb
2021-09-12  9:38       ` Weiß, Michael

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).