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

[Paul Moore]

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>

> 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

-- 
paul-moore.com