Bug#906124: Additional debug info

[Vladislav Yarmak]

On Thu, 18 Jul 2019 15:06:55 -0400
Mathieu Trudel-Lapierre <mathieu.tl at gmail.com> wrote:

> 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.

I still hope somebody notices that grub_dl_get in condition in patch.

Let's not ignore sad fact linuxefi module does not fit the bill in first
place, because it loads unsigned ramdrive and intrusion into boot chain
is trivial.

Maybe it at least can protect kernel? No, Microsoft keys already were
used to sign insecure binaries:
https://github.com/ValdikSS/Super-UEFIinSecureBoot-Disk

This way entire shim and grub can be silently replaced. So I just don't
get which security requirements you are referring to.

On the other hand, GRUB2 allows to use signatures even for config
files and ramdrive, and in condition of such secure deployment PGP
signature has cryptographic sense. This way it is possible to build
end-to-end protected boot chain.

But I have to admit, in case of default deployment trust to PGP module
may be used to circumvent linuxefi validation. For example, by trusting
additional key in config file and placing that signature for kernel. It
appears not so easy to fix it and now I doubt if there a real chance to
mix that different security models in source code of single binary,
because valid ways of building trust depend not from code, but from way
binary was signed and it's baked-in config. Thus, for GRUB binary which
was signed by user with embedded public key and enforcing config, trust
to "verify"/"pgp" module is absolutely legit.

Nonetheless, faulty PGP support in Debian 10 GRUB is a clear security
degradation because now Secure Boot support doesn't holds any security
model and user disabled from means to fix it. What do you think about
splitting code of MS-signed binaries and user-generated images with
grub-mkimage? Or maybe there are some other way?


-- 
Best Regards,
Vladislav Yarmak