Bug#906124: Additional debug info

[Mathieu Trudel-Lapierre]

On Thu, Jul 18, 2019 at 10:01 AM Colin Watson <cjwatson at debian.org> wrote:
> On Mon, Jul 08, 2019 at 09:15:49PM +0300, Vladislav Yarmak wrote:
> > On Mon, 8 Jul 2019 14:57:08 +0100 Colin Watson <cjwatson at debian.org>
> > wrote:
[...]

> > @@ -720,7 +718,16 @@ grub_cmd_linux (grub_command_t cmd __att
> >                 return GRUB_ERR_NONE;
> >               }
> >             grub_dprintf ("linux", "linuxefi failed (%d)\n", grub_errno);
> > -           grub_errno = GRUB_ERR_NONE;
> > +           /* Preserve default workflow if verify module is loaded and
> > +              signatures are being checked. Condition below is even with
> > +              code which parses "check_signatures" variable in verify.c */
> > +           const char *env_chk_sig = grub_env_get ("check_signatures");
> > +           if (env_chk_sig &&
> > +           (env_chk_sig[0] == '1' || env_chk_sig[0] == 'e') &&
> > +           grub_dl_get("verify"))
> > +             grub_errno = GRUB_ERR_NONE;
> > +           else
> > +             goto fail;
> >           }
> >       }
> >      }

I'm concerned that this approach does not necessarily fit the bill for
every environment either. For one thing, we really don't want things
to return GRUB_ERR_NONE anymore (so the fallback patch ought to go
anyway, it defeats the purpose of Secure Boot); but also, it is not a
given that if SB fails to validate, that verify is sufficient
validation. I see this as a deployment-specific option, and probably a
matter of whether the module is included in an signed GRUB UEFI binary
or not, rather than whether some environment settings are present.
Config and environment can be changed on the fly, a signed binary
succesfully loaded by firmware and enforcing SB is much more difficult
to trick in letting things get loaded. This is further complicated by the fact
that anything could also load, or decide not to load, the verify module.
Whether something is loaded or available as a module is not sufficient to
decide whether you want to further validate and allow a kernel or not.

In short, I have very much the same opinion as what Colin expresses below.

>
> This specific approach isn't suitable, I'm afraid.  The copied code is
> too fragile (things could go badly wrong if the check_signatures parsing
> were changed upstream and we forgot to update this patch to match).
> More importantly, this approach encodes an implicit assumption that if
> that environment variable is set then some other bit of code is actually
> set up to consume it and verify the kernel in some other way.  This is
> not robust, because there's nothing to say that the pgp module is loaded
> if the linux module is loaded: it would be quite possible to have an
> image that contains the linux module but not the pgp module, and then
> you have a UEFI Secure Boot bypass just by setting check_signatures=1.
> (Note that the module layout is a bit different in 2.04 than in 2.02,
> which is why it's important to prepare this patch against the latest
> version.)
>
[...]

> In fact, perhaps part of the solution could be to integrate the shim
> checking with the verifiers framework, and then linuxefi would (if

It's supposed to get ported to the verifiers framework now that we
have that available; I expect making sure PGP can be done via
verifiers too, and then integration would be pretty, efficient, and
solid.

> anything) just need to check that some appropriate verifier has been
> run, rather than the current code that predates verifiers and does the
> check by hand.  This would make much more logical sense: rather than
> scattering verification logic around all over the place, it would all be
> neatly encapsulated in verifiers.  Doing this would probably be part of
> a useful path to getting shim verification upstream, too.  And, even if
> we do end up backporting this to 2.02 which doesn't have verifiers, it's
> always easier to unroll a well-encapsulated approach into a series of
> ad-hoc checks than the other way round.
>

Mathieu Trudel-Lapierre <cyphermox at ubuntu.com>
Freenode: cyphermox, Jabber: mathieu.tl at gmail.com
4096R/65B58DA1 818A D123 0992 275B 23C2  CF89 C67B B4D6 65B5 8DA1