Re: [PATCH v28 22/25] Audit: Add record for multiple process LSM attributes
On 8/18/2021 5:47 PM, Paul Moore wrote: > ... > I just spent a few minutes tracing the code paths up from audit > through netlink and then through the socket layer and I'm not seeing > anything obvious where the path differs from any other syscall; > current->audit_context *should* be valid just like any other syscall. > However, I do have to ask, are you only seeing these audit records > with a current->audit_context equal to NULL during early boot? Nope. Sorry. -- Linux-audit mailing list Linux-audit@redhat.com https://listman.redhat.com/mailman/listinfo/linux-audit
Re: [PATCH v28 22/25] Audit: Add record for multiple process LSM attributes
On Wed, Aug 18, 2021 at 5:59 PM Casey Schaufler wrote:
>
> On 8/16/2021 11:57 AM, Paul Moore wrote:
> > On Fri, Aug 13, 2021 at 5:47 PM Casey Schaufler
> > wrote:
> >> On 8/13/2021 1:43 PM, Paul Moore wrote:
> ...
> > Yeah, the thought occurred to me, but we are clearly already in the
> > maybe-the-assumptions-are-wrong stage so I'm not going to rely on that
> > being 100%. We definitely need to track this down before we start
> > making to many more guesses about what is working and what is not.
>
> I've been tracking down where the audit context isn't set where
> we'd expect it to be, I've identified 5 cases:
>
> 1000AUDIT_GET - Get Status
> 1001AUDIT_SET - Set status enable/disable/auditd
> 1010AUDIT_SIGNAL_INFO
> 1130AUDIT_SERVICE_START
> 1131AUDIT_SEVICE_STOP
>
> These are all events that relate to the audit system itself.
> It seems plausible that these really aren't syscalls and hence
> shouldn't be expected to have an audit_context. I will create a
> patch that treats these as the special cases I believe them to be.
Yes, all but two of these could be considered to be audit subsystem
control messages, but AUDIT_SERVICE_{START,STOP} I think definitely
fall outside the audit subsystem control message category. The
AUDIT_SERVICE_{START,STOP} records are used to indicate when a
service, e.g. NetworkManager, starts and stops; on my fedora test
system they are generated by systemd since it manages service state on
that system; a quick example is below, but I'm sure you've seen plenty
of these already.
% ausearch -m SERVICE_START
time->Wed Aug 18 20:13:00 2021
type=SERVICE_START msg=audit(1629331980.779:1186): pid=1 uid=0 auid=4294967295 s
es=4294967295 subj=system_u:system_r:init_t:s0 msg='unit=NetworkManager-dispatch
er comm="systemd" exe="/usr/lib/systemd/systemd" hostname=? addr=? terminal=? re
s=success'
However, regardless of if the message is related to controlling the
audit subsystem or not, we do want to be able to associate those
records with other related records, e.g. SYSCALL records. Looking at
the message types you listed above, they are all records that are
triggered by userspace via netlink messages; if you haven't already I
would start poking along that code path to see if something looks
awry.
I just spent a few minutes tracing the code paths up from audit
through netlink and then through the socket layer and I'm not seeing
anything obvious where the path differs from any other syscall;
current->audit_context *should* be valid just like any other syscall.
However, I do have to ask, are you only seeing these audit records
with a current->audit_context equal to NULL during early boot?
--
paul moore
www.paul-moore.com
--
Linux-audit mailing list
Linux-audit@redhat.com
https://listman.redhat.com/mailman/listinfo/linux-audit
Re: [PATCH v28 22/25] Audit: Add record for multiple process LSM attributes
On 8/16/2021 11:57 AM, Paul Moore wrote: > On Fri, Aug 13, 2021 at 5:47 PM Casey Schaufler > wrote: >> On 8/13/2021 1:43 PM, Paul Moore wrote: ... > Yeah, the thought occurred to me, but we are clearly already in the > maybe-the-assumptions-are-wrong stage so I'm not going to rely on that > being 100%. We definitely need to track this down before we start > making to many more guesses about what is working and what is not. I've been tracking down where the audit context isn't set where we'd expect it to be, I've identified 5 cases: 1000AUDIT_GET - Get Status 1001AUDIT_SET - Set status enable/disable/auditd 1010AUDIT_SIGNAL_INFO 1130AUDIT_SERVICE_START 1131AUDIT_SEVICE_STOP These are all events that relate to the audit system itself. It seems plausible that these really aren't syscalls and hence shouldn't be expected to have an audit_context. I will create a patch that treats these as the special cases I believe them to be. -- Linux-audit mailing list Linux-audit@redhat.com https://listman.redhat.com/mailman/listinfo/linux-audit
Re: [PATCH v2 1/3] dm: introduce audit event module for device mapper
On Sat, Aug 14, 2021 at 2:34 PM Michael Weiß
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 type=1336 out of the audit event messages range in the
> corresponding userspace api in 'include/uapi/linux/audit.h' for
> those events.
>
> Following commits to device mapper targets actually will make
> use of this to emit those events in relevant cases.
>
> Signed-off-by: Michael Weiß
Hi Michael,
You went into more detail in your patchset cover letter, e.g. example
audit records, which I think would be helpful here in the commit
description so we have it as part of the git log. I don't want to
discourage you from writing cover letters, but don't forget that the
cover letters can be lost to the ether after a couple of years whereas
the git log has a much longer lifetime (we hope!) and a tighter
binding to the related code.
> ---
> drivers/md/Kconfig | 10 +++
> drivers/md/Makefile| 4 +++
> drivers/md/dm-audit.c | 59 ++
> drivers/md/dm-audit.h | 33 +
> include/uapi/linux/audit.h | 1 +
> 5 files changed, 107 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..4cd47623c742 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 ..c7e5824821bb
> --- /dev/null
> +++ b/drivers/md/dm-audit.c
> @@ -0,0 +1,59 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Creating audit records for mapped devices.
> + *
> + * Copyright (C) 2021 Fraunhofer AISEC. All rights reserved.
> + *
> + * Authors: Michael Weiß
> + */
> +
> +#include
> +#include
> +#include
> +#include
> +#include
> +
> +#include "dm-audit.h"
> +#include "dm-core.h"
> +
> +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;
> +
> + if (audit_enabled == AUDIT_OFF)
> + return;
> +
> + ab = audit_log_start(audit_context(), GFP_KERNEL, AUDIT_DM);
> + if (unlikely(!ab))
> + return;
> +
> + audit_log_format(ab, "module=%s dev=%d:%d op=%s sector=%llu res=%d",
> +dm_msg_prefix, MAJOR(bio->bi_bdev->bd_dev),
> +MINOR(bio->bi_bdev->bd_dev), op, sector, result);
> + audit_log_end(ab);
> +}
> +EXPORT_SYMBOL_GPL(dm_audit_log_bio);
> +
> +void dm_audit_log_target(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);
> +
> + if (audit_enabled == AUDIT_OFF)
> + return;
> +
> + ab = audit_log_start(audit_context(), GFP_KERNEL, AUDIT_DM);
> + if (unlikely(!ab))
> + return;
> +
> + audit_log_format(ab, "module=%s dev=%s op=%s",
> +dm_msg_prefix, dm_device_name(md), op);
> +
> + if (!result && !strcmp("ctr", op))
> + audit_log_format(ab, " error_msg='%s'", ti->error);
> + audit_log_format(ab, " res=%d", result);
> + audit_log_end(ab);
> +}
Generally speaking we try to keep a consistent format and ordering
within a given audit record type. However you appear to have at least
three different formats for the AUDIT_DM record in this patch:
"... module=%s dev=%d:%d op=%s sector=%llu res=%d"
"... module=%s dev=%s op=%s error_msg='%s' res=%d"
"... module=%s dev=%s op=%s res=%d"
The first thing that j
