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

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

[Matthew Wilcox (Oracle)]

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
+
+	if ! command -v sbsign >/dev/null; then
+		key=
+	fi
+fi
+
+if [ -n "$key" ]; then
+	sbsign --key $key --cert $cert "$vmlinux" --output "$tmpdir/$installed_image_path"
+else
+	cp "$vmlinux" "$tmpdir/$installed_image_path"
+fi
 
 if is_enabled CONFIG_OF_EARLY_FLATTREE; then
 	# Only some architectures with OF support have this target
-- 
2.33.0

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

[Masahiro Yamada]

+CC the maintainers of CERTIFICATE HANDLING
M:      David Howells <dhowells@redhat.com>
M:      David Woodhouse <dwmw2@infradead.org>
L:      keyrings@vger.kernel.org




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?


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:


commit fb1179499134bc718dc7557c7a6a95dc72f224cb
Author: David Woodhouse <David.Woodhouse@intel.com>
Date:   Mon Jul 20 21:16:30 2015 +0100

    modsign: Use single PEM file for autogenerated key

    The current rule for generating signing_key.priv and signing_key.x509 is
    a classic example of a bad rule which has a tendency to break parallel
    make. When invoked to create *either* target, it generates the other
    target as a side-effect that make didn't predict.

    So let's switch to using a single file signing_key.pem which contains
    both key and certificate. That matches what we do in the case of an
    external key specified by CONFIG_MODULE_SIG_KEY anyway, so it's also
    slightly cleaner.

    Signed-off-by: David Woodhouse <David.Woodhouse@intel.com>
    Signed-off-by: David Howells <dhowells@redhat.com>




Since then, both key and certificate are stored in a single *.pem file.


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)

But, anyway, as long as I read the current code, we never
have a separate *.priv file.


The help message of the config option supports my view.


config MODULE_SIG_KEY
        string "File name or PKCS#11 URI of module signing key"
        default "certs/signing_key.pem"
        depends on MODULE_SIG || (IMA_APPRAISE_MODSIG && MODULES)
        help
         Provide the file name of a private key/certificate in PEM format,
         or a PKCS#11 URI according to RFC7512. The file should contain, or
         the URI should identify, both the certificate and its corresponding
                                  ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
         private key.
         ^^^^^^^^^^^



I CC'ed  David Howells, David Woodhouse, keyrings@vger.kernel.org
in case I understood wrong.












> +       if ! command -v sbsign >/dev/null; then
> +               key=
> +       fi
> +fi
> +
> +if [ -n "$key" ]; then
> +       sbsign --key $key --cert $cert "$vmlinux" --output "$tmpdir/$installed_image_path"
> +else
> +       cp "$vmlinux" "$tmpdir/$installed_image_path"
> +fi
>
>  if is_enabled CONFIG_OF_EARLY_FLATTREE; then
>         # Only some architectures with OF support have this target
> --
> 2.33.0
>


--
Best Regards
Masahiro Yamada

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

[Matthew Wilcox]

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.

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

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

[Masahiro Yamada]

Added "Ben Hutchings <ben@decadent.org.uk>"

On Wed, Jan 5, 2022 at 3:13 AM Matthew Wilcox <willy@infradead.org> 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"



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

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@decadent.org.uk>"
> 
> On Wed, Jan 5, 2022 at 3:13 AM Matthew Wilcox <willy@infradead.org> 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

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

[Masahiro Yamada]

On Mon, Feb 7, 2022 at 10:31 PM Matthew Wilcox <willy@infradead.org> wrote:
>
> On Mon, Feb 07, 2022 at 09:33:46PM +0900, Masahiro Yamada wrote:
> > Added "Ben Hutchings <ben@decadent.org.uk>"
> >
> > On Wed, Jan 5, 2022 at 3:13 AM Matthew Wilcox <willy@infradead.org> 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


I checked the diff.

Back in November, the key/cert were generated in /root directory.

In the latest instruction, the recommended location is
/var/lib/shim-signed/mok/.






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

Sorry, I do not understand what you are talking about.

I assume you are talking about v1 patch.
https://patchwork.kernel.org/project/linux-kbuild/patch/20211013200536.1851070-1-willy@infradead.org/

Honestly, I did not have expert knowledge in the secure boot,
but anyway I observed build errors, and I pointed it out.

Other than the build error, your original patch was written in the way
to use CONFIG_MODULE_SIG_KEY  for signing the kernel.
This is the same between v1 and v2.

Looking at the instruction in the Debian page,
now I am wondering if this is the right thing to do
because signing the kernel and signing the modules
are different things.

The old debian page (I checked the old page back in November)
did not mention CONFIG_MODULE_SIG_KEY either.


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

I agree on this point.
It will be nice to avoid manual work in the kernel signing steps.



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

Sorry for the delay and annoyance.

After reading the latest Debian doc closely, I think we can automate
the kernel signing by using the key located in /var/lib/shim-signed/mok/
instead of CONFIG_MODULE_SIG_KEY.


If I am wrong, I hope some input from people in the CC list.









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



--
Best Regards
Masahiro Yamada

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

[Julian Andres Klode]

On Mon, Feb 07, 2022 at 09:33:46PM +0900, Masahiro Yamada wrote:
> Added "Ben Hutchings <ben@decadent.org.uk>"
> 
> On Wed, Jan 5, 2022 at 3:13 AM Matthew Wilcox <willy@infradead.org> 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"
> 
> 
> 
> 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.

It's worth pointing out that in Ubuntu, the generated MOK key
is for module signing only (extended key usage 1.3.6.1.4.1.2312.16.1.2),
kernels signed with it will NOT be bootable.

-- 
debian developer - deb.li/jak | jak-linux.org - free software dev
ubuntu core developer                              i speak de, en

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

[Matthew Wilcox]

On Tue, Feb 08, 2022 at 12:01:22PM +0100, Julian Andres Klode wrote:
> It's worth pointing out that in Ubuntu, the generated MOK key
> is for module signing only (extended key usage 1.3.6.1.4.1.2312.16.1.2),
> kernels signed with it will NOT be bootable.

Why should these be separate keys?  There's no meaningful security
boundary between a kernel module and the ernel itself; a kernel
modulecan, for example, write to CR3, and that's game over for
any pretence at separation.

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

[Julian Andres Klode]

On Tue, Feb 08, 2022 at 01:10:34PM +0000, Matthew Wilcox wrote:
> On Tue, Feb 08, 2022 at 12:01:22PM +0100, Julian Andres Klode wrote:
> > It's worth pointing out that in Ubuntu, the generated MOK key
> > is for module signing only (extended key usage 1.3.6.1.4.1.2312.16.1.2),
> > kernels signed with it will NOT be bootable.
> 
> Why should these be separate keys?  There's no meaningful security
> boundary between a kernel module and the ernel itself; a kernel
> modulecan, for example, write to CR3, and that's game over for
> any pretence at separation.

I don't really _know_, but I believe the difference is that the
kernel binaries runs in boot services, and calls ExitBootServices,
whereas modules are loaded after ExitBootServices.

But I don't know the full rationale why (a) the feature exists in
the first place and (b) why the Ubuntu security team chose to require
that constraint.

My goal is just to make people aware of that so they can make
informed decisions :)

-- 
debian developer - deb.li/jak | jak-linux.org - free software dev
ubuntu core developer                              i speak de, en

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

[Masahiro Yamada]

On Wed, Feb 9, 2022 at 1:13 AM Julian Andres Klode
<julian.klode@canonical.com> wrote:
>
> On Tue, Feb 08, 2022 at 01:10:34PM +0000, Matthew Wilcox wrote:
> > On Tue, Feb 08, 2022 at 12:01:22PM +0100, Julian Andres Klode wrote:
> > > It's worth pointing out that in Ubuntu, the generated MOK key
> > > is for module signing only (extended key usage 1.3.6.1.4.1.2312.16.1.2),
> > > kernels signed with it will NOT be bootable.
> >
> > Why should these be separate keys?  There's no meaningful security
> > boundary between a kernel module and the ernel itself; a kernel
> > modulecan, for example, write to CR3, and that's game over for
> > any pretence at separation.
>
> I don't really _know_, but I believe the difference is that the
> kernel binaries runs in boot services, and calls ExitBootServices,
> whereas modules are loaded after ExitBootServices.
>
> But I don't know the full rationale why (a) the feature exists in
> the first place and (b) why the Ubuntu security team chose to require
> that constraint.
>
> My goal is just to make people aware of that so they can make
> informed decisions :)
>
> --
> debian developer - deb.li/jak | jak-linux.org - free software dev
> ubuntu core developer                              i speak de, en


So, this is the restriction only for Ubuntu.

If it works for Debian, I am fine with
adding the kernel signing to scripts/package/builddeb.

If it is annoying for some people, maybe we can add
some switch like KDEB_SIGN_KERNEL to conditionally
sign the kernel.

-- 
Best Regards
Masahiro Yamada

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

[James Bottomley]

On Tue, 2022-02-08 at 13:10 +0000, Matthew Wilcox wrote:
> On Tue, Feb 08, 2022 at 12:01:22PM +0100, Julian Andres Klode wrote:
> > It's worth pointing out that in Ubuntu, the generated MOK key
> > is for module signing only (extended key usage
> > 1.3.6.1.4.1.2312.16.1.2), kernels signed with it will NOT be
> > bootable.
> 
> Why should these be separate keys?  There's no meaningful security
> boundary between a kernel module and the ernel itself; a kernel
> modulecan, for example, write to CR3, and that's game over for
> any pretence at separation.

It's standard practice for any automated build private key to be
destroyed immediately to preserve security.  Thus the modules get
signed with a per kernel ephemeral build key but the MoK key is a long
term key with a special signing infrastructure, usually burned into the
distro version of shim.  The kernel signing key usually has to be long
term because you want shim to boot multiple kernels otherwise upgrading
becomes a nightmare.

James

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

[Masahiro Yamada]

On Wed, Feb 9, 2022 at 10:21 PM James Bottomley
<James.Bottomley@hansenpartnership.com> wrote:
>
> On Tue, 2022-02-08 at 13:10 +0000, Matthew Wilcox wrote:
> > On Tue, Feb 08, 2022 at 12:01:22PM +0100, Julian Andres Klode wrote:
> > > It's worth pointing out that in Ubuntu, the generated MOK key
> > > is for module signing only (extended key usage
> > > 1.3.6.1.4.1.2312.16.1.2), kernels signed with it will NOT be
> > > bootable.
> >
> > Why should these be separate keys?  There's no meaningful security
> > boundary between a kernel module and the ernel itself; a kernel
> > modulecan, for example, write to CR3, and that's game over for
> > any pretence at separation.
>
> It's standard practice for any automated build private key to be
> destroyed immediately to preserve security.  Thus the modules get
> signed with a per kernel ephemeral build key but the MoK key is a long
> term key with a special signing infrastructure, usually burned into the
> distro version of shim.  The kernel signing key usually has to be long
> term because you want shim to boot multiple kernels otherwise upgrading
> becomes a nightmare.

Fully agreed.



-- 
Best Regards
Masahiro Yamada

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

[Matthew Wilcox]

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.

> 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:
> 
> 
> commit fb1179499134bc718dc7557c7a6a95dc72f224cb
> Author: David Woodhouse <David.Woodhouse@intel.com>
> Date:   Mon Jul 20 21:16:30 2015 +0100
> 
>     modsign: Use single PEM file for autogenerated key
> 
>     The current rule for generating signing_key.priv and signing_key.x509 is
>     a classic example of a bad rule which has a tendency to break parallel
>     make. When invoked to create *either* target, it generates the other
>     target as a side-effect that make didn't predict.
> 
>     So let's switch to using a single file signing_key.pem which contains
>     both key and certificate. That matches what we do in the case of an
>     external key specified by CONFIG_MODULE_SIG_KEY anyway, so it's also
>     slightly cleaner.
> 
>     Signed-off-by: David Woodhouse <David.Woodhouse@intel.com>
>     Signed-off-by: David Howells <dhowells@redhat.com>
> 
> 
> 
> 
> Since then, both key and certificate are stored in a single *.pem file.

I did read that commit.  I think it's a terrible idea.  If the
secret key & the certificate are stored in the same file, it's
no better than a symmetric cipher.  Not even SSH does this!

> 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)
> 
> But, anyway, as long as I read the current code, we never
> have a separate *.priv file.
> 
> 
> The help message of the config option supports my view.
> 
> 
> config MODULE_SIG_KEY
>         string "File name or PKCS#11 URI of module signing key"
>         default "certs/signing_key.pem"
>         depends on MODULE_SIG || (IMA_APPRAISE_MODSIG && MODULES)
>         help
>          Provide the file name of a private key/certificate in PEM format,
>          or a PKCS#11 URI according to RFC7512. The file should contain, or
>          the URI should identify, both the certificate and its corresponding
>                                   ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
>          private key.
>          ^^^^^^^^^^^
> 
> 
> 
> I CC'ed  David Howells, David Woodhouse, keyrings@vger.kernel.org
> in case I understood wrong.
> 
> 
> 
> 
> 
> 
> 
> 
> 
> 
> 
> 
> > +       if ! command -v sbsign >/dev/null; then
> > +               key=
> > +       fi
> > +fi
> > +
> > +if [ -n "$key" ]; then
> > +       sbsign --key $key --cert $cert "$vmlinux" --output "$tmpdir/$installed_image_path"
> > +else
> > +       cp "$vmlinux" "$tmpdir/$installed_image_path"
> > +fi
> >
> >  if is_enabled CONFIG_OF_EARLY_FLATTREE; then
> >         # Only some architectures with OF support have this target
> > --
> > 2.33.0
> >
> 
> 
> --
> Best Regards
> Masahiro Yamada

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

[Masahiro Yamada]

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.


masahiro@debian:~$ cat /etc/os-release
PRETTY_NAME="Debian GNU/Linux 11 (bullseye)"
NAME="Debian GNU/Linux"
VERSION_ID="11"
VERSION="11 (bullseye)"
VERSION_CODENAME=bullseye
ID=debian
HOME_URL="https://www.debian.org/"
SUPPORT_URL="https://www.debian.org/support"
BUG_REPORT_URL="https://bugs.debian.org/"

masahiro@debian:~$ grep CONFIG_MODULE_SIG  /boot/config-$(uname -r)
CONFIG_MODULE_SIG_FORMAT=y
CONFIG_MODULE_SIG=y
# CONFIG_MODULE_SIG_FORCE is not set
# CONFIG_MODULE_SIG_ALL is not set
# CONFIG_MODULE_SIG_SHA1 is not set
# CONFIG_MODULE_SIG_SHA224 is not set
CONFIG_MODULE_SIG_SHA256=y
# CONFIG_MODULE_SIG_SHA384 is not set
# CONFIG_MODULE_SIG_SHA512 is not set
CONFIG_MODULE_SIG_HASH="sha256"
CONFIG_MODULE_SIG_KEY=""


Presumably, the reason is that distributions cannot
be shipped with the private key.







> > 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:
> >
> >
> > commit fb1179499134bc718dc7557c7a6a95dc72f224cb
> > Author: David Woodhouse <David.Woodhouse@intel.com>
> > Date:   Mon Jul 20 21:16:30 2015 +0100
> >
> >     modsign: Use single PEM file for autogenerated key
> >
> >     The current rule for generating signing_key.priv and signing_key.x509 is
> >     a classic example of a bad rule which has a tendency to break parallel
> >     make. When invoked to create *either* target, it generates the other
> >     target as a side-effect that make didn't predict.
> >
> >     So let's switch to using a single file signing_key.pem which contains
> >     both key and certificate. That matches what we do in the case of an
> >     external key specified by CONFIG_MODULE_SIG_KEY anyway, so it's also
> >     slightly cleaner.
> >
> >     Signed-off-by: David Woodhouse <David.Woodhouse@intel.com>
> >     Signed-off-by: David Howells <dhowells@redhat.com>
> >
> >
> >
> >
> > Since then, both key and certificate are stored in a single *.pem file.
>
> I did read that commit.  I think it's a terrible idea.  If the
> secret key & the certificate are stored in the same file, it's
> no better than a symmetric cipher.  Not even SSH does this!


I am with you on this point, but
separating the cert and priv is another story.












>
> > 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)
> >
> > But, anyway, as long as I read the current code, we never
> > have a separate *.priv file.
> >
> >
> > The help message of the config option supports my view.
> >
> >
> > config MODULE_SIG_KEY
> >         string "File name or PKCS#11 URI of module signing key"
> >         default "certs/signing_key.pem"
> >         depends on MODULE_SIG || (IMA_APPRAISE_MODSIG && MODULES)
> >         help
> >          Provide the file name of a private key/certificate in PEM format,
> >          or a PKCS#11 URI according to RFC7512. The file should contain, or
> >          the URI should identify, both the certificate and its corresponding
> >                                   ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> >          private key.
> >          ^^^^^^^^^^^
> >
> >
> >
> > I CC'ed  David Howells, David Woodhouse, keyrings@vger.kernel.org
> > in case I understood wrong.
> >
> >
> >
> >
> >
> >
> >
> >
> >
> >
> >
> >
> > > +       if ! command -v sbsign >/dev/null; then
> > > +               key=
> > > +       fi
> > > +fi
> > > +
> > > +if [ -n "$key" ]; then
> > > +       sbsign --key $key --cert $cert "$vmlinux" --output "$tmpdir/$installed_image_path"
> > > +else
> > > +       cp "$vmlinux" "$tmpdir/$installed_image_path"
> > > +fi
> > >
> > >  if is_enabled CONFIG_OF_EARLY_FLATTREE; then
> > >         # Only some architectures with OF support have this target
> > > --
> > > 2.33.0
> > >
> >
> >
> > --
> > Best Regards
> > Masahiro Yamada



--
Best Regards
Masahiro Yamada

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

[Ben Hutchings]

[-- 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 --]