Re: [PATCH] dm verity: log audit events for dm-verity target

[Michael Weiß]

On 02.03.23 03:25, Paul Moore wrote:
> On Wed, Mar 1, 2023 at 6:34 AM Michael Weiß
> <michael.weiss@xxxxxxxxxxxxxxxxxxx> 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@xxxxxxxxxxxxxxxxxxx>
>> ---
>>  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@xxxxxxxxxxxxxx>

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
>