Re: [PATCH v2] builddeb: Support signing kernels with the module signing key

[Matthew Wilcox]

On Mon, Feb 07, 2022 at 09:33:46PM +0900, Masahiro Yamada wrote:
> Added "Ben Hutchings <ben@xxxxxxxxxxxxxxx>"
> 
> On Wed, Jan 5, 2022 at 3:13 AM Matthew Wilcox <willy@xxxxxxxxxxxxx> wrote:
> >
> > On Wed, Jan 05, 2022 at 12:39:57AM +0900, Masahiro Yamada wrote:
> > > > +vmlinux=$($MAKE -s -f $srctree/Makefile image_name)
> > > > +key=
> > > > +if is_enabled CONFIG_EFI_STUB && is_enabled CONFIG_MODULE_SIG; then
> > > > +       cert=$(grep ^CONFIG_MODULE_SIG_KEY= include/config/auto.conf | cut -d\" -f2)
> > > > +       if [ ! -f $cert ]; then
> > > > +               cert=$srctree/$cert
> > > > +       fi
> > > > +
> > > > +       key=${cert%pem}priv
> > > > +       if [ ! -f $key ]; then
> > > > +               key=$cert
> > > > +       fi
> > >
> > >
> > > I still do not understand this part.
> > >
> > > It is true that the Debian document you referred to creates separate files
> > > for the key and the certificate:
> > >   # openssl req -new -x509 -newkey rsa:2048 -keyout MOK.priv -outform
> > > DER -out MOK.der -days 36500 -subj "/CN=My Name/" -nodes
> > >
> > > but, is such a use-case possible in Kbuild?
> >
> > If someone has followed the Debian instructions for creating a MOK,
> > then they will have two separate files.  We should support both the case
> > where someone has created a Debian MOK and the case where someone has
> > used Kbuild to create this foolish blob with both private and public
> > key in one file.
> 
> But, this patch is doing different things than the Debian document.
> 
> 
> The Debian document you referred to says:
>   "Ubuntu puts its MOK key under /var/lib/shim-signed/mok/ and some
>    software such as Oracle's virtualbox package expect the key there
>    so we follow suit (see 989463 for reference) and put it at the same place"

Uhh ... it does now.  It didn't when I originally wrote this patch.
Apparently it was updated in November:
https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=989463

> In Debian, MOK is generated under /var/lib/shim-signed/mok/,
> and its primary use is for signing the kernel.
> Then, you can reuse it for signing modules as well.
> 
> 
> This patch adopts the opposite direction:
>   Kbuild generates the module signing key, then
>   this patch reuses it for singing the kernel.

The patch only does this because you asked it to be changed to do this!
Look back at the version I originally sent out.  It didn't generate
the module signing key at all.  I had no idea the kernel build was even
capable of doing such a thing until you pointed it out.

I followed the instructions in the Debian document *that existed at
the time* (and now apparently we can't see because Debian uses an
inept type of wiki that can't show old versions).  I generated a key
and did not store it in the build tree.  I enrolled that key.
And then I thought "It would be nice if I didn't have to do all this
manual work after installing a new kernel so that my machine would
boot".

And here we are, months later, and you're complaining about ...
something?

> The key is located in the kernel build tree
> (that is, the key is lost when you run "make mrproper").
> 
> You need to "mokutil --import path/to/module/sining/key"
> every time Kbuild generates a new key.
> 
> 
> 
> So, another possible approach is:
> 
> builddeb signs the kernel with the key
> in /var/lib/shim-signed/mok/.
> 
> I think this is more aligned with the debian documenation.
> 
> I added Ben Hutchings, who might give us insights.
> 
> 
> 
> 
> 
> 
> 
> > > In the old days, yes, the key and the certificate were stored in separate files.
> > > (the key in *.priv and the certificate in *.x509)
> > >
> > >
> > > Please read this commit:
> >
> > Yes, I did.
> >
> > > The motivation for this change is still questionable to me;
> > > the commit description sounds like they merged *.priv and *.x509
> > > into *.pem just because they could not write a correct Makefile.
> > > (If requested, I can write a correct Makefile that works in parallel build)
> >
> > I think that would be preferable.  Putting the private and public keys
> > in the same file cannot be good security practice!
> 
> 
> 
> -- 
> Best Regards
> Masahiro Yamada