Re: [PATCH v3 4/8] verifiers: Add possibility to defer verification to other verifiers

[Ross Philipson]

On 10/03/2018 05:36 AM, Daniel Kiper wrote:
> This way if a verifier requires verification of a given file it can
> defer task to other verifier if it is not able to do it itself. E.g.
> shim_lock verifier, posted as a subsequent patch, is able to verify
> only PE files. This means that it is not able to verify any of GRUB2
> modules which have to be trusted on UEFI systems with secure boot
> enabled. So, it can defer verification to other verifier, e.g. PGP one.
> 
> I silently assume that other verifiers are trusted and will do good
> job for us. Or at least they will not do any harm.
> 
> Signed-off-by: Daniel Kiper <address@hidden>
> ---
>  grub-core/commands/verify_helper.c |   23 ++++++++++++++++++++---
>  include/grub/verify.h              |    3 ++-
>  2 files changed, 22 insertions(+), 4 deletions(-)
> 
> diff --git a/grub-core/commands/verify_helper.c 
> b/grub-core/commands/verify_helper.c
> index 7effc5f..ba8b03d 100644
> --- a/grub-core/commands/verify_helper.c
> +++ b/grub-core/commands/verify_helper.c
> @@ -83,6 +83,7 @@ grub_verify_helper_open (grub_file_t io, enum 
> grub_file_type type)
>    void *context;
>    grub_file_t ret = 0;
>    grub_err_t err;
> +  int defer = 0;
>  
>    grub_dprintf ("verify", "file: %s type: %d\n", io->name, type);
>  
> @@ -102,13 +103,27 @@ grub_verify_helper_open (grub_file_t io, enum 
> grub_file_type type)
>        err = ver->init (io, type, &context, &flags);
>        if (err)
>       goto fail_noclose;
> +      if (flags & GRUB_VERIFY_FLAGS_DEFER)
> +     {
> +       defer = 1;
> +       continue;
> +     }
>        if (!(flags & GRUB_VERIFY_FLAGS_SKIP_VERIFICATION))
>       break;
>      }
>  
>    if (!ver)
> -    /* No verifiers wanted to verify. Just return underlying file.  */
> -    return io;
> +    {
> +      if (defer)
> +     {
> +       grub_error (GRUB_ERR_ACCESS_DENIED,
> +                   N_("verification requested but nobody cares: %s"), 
> io->name);
> +       goto fail_noclose;
> +     }

I don't really understand the logic here. Is the idea that if it was
deferred and no one in the chain verified it, it ends up being an error?
Why is that?

Also defer has different meanings. I think in this context it means
defer verification to another authority as opposed to defer until later.

> +
> +      /* No verifiers wanted to verify. Just return underlying file. */
> +      return io;
> +    }
>  
>    ret = grub_malloc (sizeof (*ret));
>    if (!ret)
> @@ -160,7 +175,9 @@ grub_verify_helper_open (grub_file_t io, enum 
> grub_file_type type)
>        err = ver->init (io, type, &context, &flags);
>        if (err)
>       goto fail_noclose;
> -      if (flags & GRUB_VERIFY_FLAGS_SKIP_VERIFICATION)
> +      if (flags & GRUB_VERIFY_FLAGS_SKIP_VERIFICATION ||
> +       /* Verification done earlier. So, we are happy here. */
> +       flags & GRUB_VERIFY_FLAGS_DEFER)
>       continue;
>        err = ver->write (context, verified->buf, ret->size);
>        if (err)
> diff --git a/include/grub/verify.h b/include/grub/verify.h
> index 9f892d8..c385e3d 100644
> --- a/include/grub/verify.h
> +++ b/include/grub/verify.h
> @@ -22,7 +22,8 @@
>  enum grub_verify_flags
>    {
>      GRUB_VERIFY_FLAGS_SKIP_VERIFICATION      = 1,
> -    GRUB_VERIFY_FLAGS_SINGLE_CHUNK   = 2
> +    GRUB_VERIFY_FLAGS_SINGLE_CHUNK   = 2,
> +    GRUB_VERIFY_FLAGS_DEFER          = 4

What is the difference between skip verification and defer?

>    };
>  
>  enum grub_verify_string_type
>