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

[Ben Hutchings <ben@decadent.org.uk>]

[-- Attachment #1: Type: text/plain, Size: 3910 bytes --]

On Mon, 2022-02-07 at 22:22 +0900, Masahiro Yamada wrote:
> On Sat, Feb 5, 2022 at 7:30 AM Matthew Wilcox <willy@infradead.org> wrote:
> > 
> > On Wed, Jan 05, 2022 at 12:39:57AM +0900, Masahiro Yamada wrote:
> > > +CC the maintainers of CERTIFICATE HANDLING
> > > M:      David Howells <dhowells@redhat.com>
> > > M:      David Woodhouse <dwmw2@infradead.org>
> > > L:      keyrings@vger.kernel.org
> > 
> > Davids, can one of you respond to this?
> > 
> > > On Sat, Dec 18, 2021 at 12:11 PM Matthew Wilcox (Oracle)
> > > <willy@infradead.org> wrote:
> > > > 
> > > > If the config file specifies a signing key, use it to sign
> > > > the kernel so that machines with SecureBoot enabled can boot.
> > > > See https://wiki.debian.org/SecureBoot
> > > > 
> > > > Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
> > > > ---
> > > > v2:
> > > >  - Handle private keys stored in the pem file as well as adjacent to the
> > > >    certificate
> > > >  - Handle certificate paths specified relative to both dsttree and srctree
> > > >    (as well as absolute)
> > > >  - Only try to sign the executable if EFI_STUB is enabled
> > > >  - Only try to execute sbsign if it's in $PATH
> > > > 
> > > >  scripts/package/builddeb | 25 ++++++++++++++++++++++++-
> > > >  1 file changed, 24 insertions(+), 1 deletion(-)
> > > > 
> > > > diff --git a/scripts/package/builddeb b/scripts/package/builddeb
> > > > index 91a502bb97e8..9dd92fd02b12 100755
> > > > --- a/scripts/package/builddeb
> > > > +++ b/scripts/package/builddeb
> > > > @@ -147,7 +147,30 @@ else
> > > >         cp System.map "$tmpdir/boot/System.map-$version"
> > > >         cp $KCONFIG_CONFIG "$tmpdir/boot/config-$version"
> > > >  fi
> > > > -cp "$($MAKE -s -f $srctree/Makefile image_name)" "$tmpdir/$installed_image_path"
> > > > +
> > > > +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?
> > 
> > I don't think it matters whether *Kbuild* can generate one file or
> > two.  If somebody follows the *Debian* document, they will have
> > two files.  It would surely be desirable that if somebody has followed
> > the Debian instructions that we would then sign the kernel using the
> > keys they previously generated.
> 
> 
> If I am not wrong, extracting the key path from
> CONFIG_MODULE_SIG_KEY is not Debian's way.
> 
> 
> I checked the kernel configuration on bullseye,
> CONFIG_MODULE_SIG_KEY is empty,
> while the module signing itself is enabled.
[...]

For Debian's own packages, we didn't want to use ephemeral module
signing keys (which break reproducibility) or to expose signing keys to
the regular build machines.  Instead, Debian has a separate signing
service that handles the few packages that need it.

The closest thing to "the Debian way" for signing custom kernels would
be that wiki page.  But personally, I think making each computer build
and sign its own kernel and modules undermines any value that Secure
Boot could provide.

Ben.

-- 
Ben Hutchings
Who are all these weirdos? - David Bowie, on joining IRC

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 833 bytes --]