[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <1f58acef-1e93-ed7a-3ad8-dd0927dacddc@aisec.fraunhofer.de>
Date: Fri, 17 Mar 2023 09:00:25 +0100
From: Michael Weiß <michael.weiss@...ec.fraunhofer.de>
To: Mike Snitzer <snitzer@...nel.org>
CC: Paul Moore <paul@...l-moore.com>,
Richard Guy Briggs <rgb@...hat.com>,
Mikulas Patocka <mpatocka@...hat.com>,
<gyroidos@...ec.fraunhofer.de>, Alasdair Kergon <agk@...hat.com>,
"maintainer:DEVICE-MAPPER (LVM)" <dm-devel@...hat.com>,
Eric Paris <eparis@...hat.com>,
open list <linux-kernel@...r.kernel.org>,
"open list:AUDIT SUBSYSTEM" <audit@...r.kernel.org>
Subject: Re: [PATCH] dm verity: log audit events for dm-verity target
On 02.03.23 03:25, Paul Moore wrote:
> On Wed, Mar 1, 2023 at 6:34 AM Michael Weiß
> <michael.weiss@...ec.fraunhofer.de> wrote:
>>
>> dm-verity 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. Analogous to dm-integrity, we also use the
>> dm-audit submodule allowing to emit audit events on verification
>> failures of metadata and data blocks as well as if max corrupted
>> errors are reached.
>>
>> The construction and destruction of verity device mappings are
>> also relevant for auditing a system. Thus, those events are also
>> logged as audit events.
>>
>> We tested this by starting a container with the container manager
>> (cmld) of GyroidOS which uses a dm-verity protected rootfs image
>> root.img mapped to /dev/mapper/<uuid>-root. We than manipulated
>> one block in the underlying image file and reading it from the
>> protected mapper device again and again until we reach the max
>> corrupted errors like this:
>>
>> dd if=/dev/urandom of=root.img bs=512 count=1 seek=1000
>> for i in range {1..101}; do \
>> dd if=/dev/mapper/<uuid>-root of=/dev/null bs=4096 \
>> count=1 skip=1000 \
>> done
>>
>> The resulting audit log looks as follows:
>>
>> type=DM_CTRL msg=audit(1677618791.876:962):
>> module=verity op=ctr ppid=4876 pid=29102 auid=0 uid=0 gid=0
>> euid=0 suid=0 fsuid=0 egid=0 sgid=0 fsgid=0 tty=(none) ses=44
>> comm="cmld" exe="/usr/sbin/cml/cmld" subj=unconfined
>> dev=254:3 error_msg='success' res=1
>>
>> type=DM_EVENT msg=audit(1677619463.786:1074): module=verity
>> op=verify-data dev=7:0 sector=1000 res=0
>> ...
>> type=DM_EVENT msg=audit(1677619596.727:1162): module=verity
>> op=verify-data dev=7:0 sector=1000 res=0
>>
>> type=DM_EVENT msg=audit(1677619596.731:1163): module=verity
>> op=max-corrupted-errors dev=254:3 sector=? res=0
>>
>> Signed-off-by: Michael Weiß <michael.weiss@...ec.fraunhofer.de>
>> ---
>> drivers/md/dm-verity-target.c | 20 ++++++++++++++++++--
>> 1 file changed, 18 insertions(+), 2 deletions(-)
>
> This looks reasonable to me from an audit perspective.
>
> Acked-by: Paul Moore <paul@...l-moore.com>
Hi Mike, since Paul already gave his ack from audit perspective,
do you pick this up? Or is there anything which I can improve from device
mapper side?
Thanx,
Michael
>
>> diff --git a/drivers/md/dm-verity-target.c b/drivers/md/dm-verity-target.c
>> index ade83ef3b439..8beeb4ea66d1 100644
>> --- a/drivers/md/dm-verity-target.c
>> +++ b/drivers/md/dm-verity-target.c
>> @@ -16,6 +16,7 @@
>> #include "dm-verity.h"
>> #include "dm-verity-fec.h"
>> #include "dm-verity-verify-sig.h"
>> +#include "dm-audit.h"
>> #include <linux/module.h>
>> #include <linux/reboot.h>
>> #include <linux/scatterlist.h>
>> @@ -248,8 +249,10 @@ static int verity_handle_err(struct dm_verity *v, enum verity_block_type type,
>> DMERR_LIMIT("%s: %s block %llu is corrupted", v->data_dev->name,
>> type_str, block);
>>
>> - if (v->corrupted_errs == DM_VERITY_MAX_CORRUPTED_ERRS)
>> + if (v->corrupted_errs == DM_VERITY_MAX_CORRUPTED_ERRS) {
>> DMERR("%s: reached maximum errors", v->data_dev->name);
>> + dm_audit_log_target(DM_MSG_PREFIX, "max-corrupted-errors", v->ti, 0);
>> + }
>>
>> snprintf(verity_env, DM_VERITY_ENV_LENGTH, "%s=%d,%llu",
>> DM_VERITY_ENV_VAR_NAME, type, block);
>> @@ -340,6 +343,11 @@ static int verity_verify_level(struct dm_verity *v, struct dm_verity_io *io,
>> else if (verity_handle_err(v,
>> DM_VERITY_BLOCK_TYPE_METADATA,
>> hash_block)) {
>> + struct bio *bio =
>> + dm_bio_from_per_bio_data(io,
>> + v->ti->per_io_data_size);
>> + dm_audit_log_bio(DM_MSG_PREFIX, "verify-metadata", bio,
>> + block, 0);
>> r = -EIO;
>> goto release_ret_r;
>> }
>> @@ -590,8 +598,11 @@ static int verity_verify_io(struct dm_verity_io *io)
>> return -EIO;
>> }
>> if (verity_handle_err(v, DM_VERITY_BLOCK_TYPE_DATA,
>> - cur_block))
>> + cur_block)) {
>> + dm_audit_log_bio(DM_MSG_PREFIX, "verify-data",
>> + bio, cur_block, 0);
>> return -EIO;
>> + }
>> }
>> }
>>
>> @@ -975,6 +986,8 @@ static void verity_dtr(struct dm_target *ti)
>> static_branch_dec(&use_tasklet_enabled);
>>
>> kfree(v);
>> +
>> + dm_audit_log_dtr(DM_MSG_PREFIX, ti, 1);
>> }
>>
>> static int verity_alloc_most_once(struct dm_verity *v)
>> @@ -1429,11 +1442,14 @@ static int verity_ctr(struct dm_target *ti, unsigned int argc, char **argv)
>>
>> verity_verify_sig_opts_cleanup(&verify_args);
>>
>> + dm_audit_log_ctr(DM_MSG_PREFIX, ti, 1);
>> +
>> return 0;
>>
>> bad:
>>
>> verity_verify_sig_opts_cleanup(&verify_args);
>> + dm_audit_log_ctr(DM_MSG_PREFIX, ti, 0);
>> verity_dtr(ti);
>>
>> return r;
>> --
>> 2.30.2
>
Powered by blists - more mailing lists