Message-ID: <CAK7LNAQUChvX3NoukBnjBfJJGu+a96pfbM--xHEHOygWPgE9eA@mail.gmail.com>
Date:   Wed, 5 Jan 2022 00:39:57 +0900
From:   Masahiro Yamada <masahiroy@...nel.org>
To:     "Matthew Wilcox (Oracle)" <willy@...radead.org>
Cc:     Linux Kbuild mailing list <linux-kbuild@...r.kernel.org>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        efi@...ts.einval.com,
        debian-kernel <debian-kernel@...ts.debian.org>,
        linux-efi <linux-efi@...r.kernel.org>,
        Ard Biesheuvel <ardb@...nel.org>,
        David Woodhouse <dwmw2@...radead.org>,
        David Howells <dhowells@...hat.com>, keyrings@...r.kernel.org
Subject: Re: [PATCH v2] builddeb: Support signing kernels with the module
 signing key

+CC the maintainers of CERTIFICATE HANDLING
M:      David Howells <dhowells@...hat.com>
M:      David Woodhouse <dwmw2@...radead.org>
L:      keyrings@...r.kernel.org




On Sat, Dec 18, 2021 at 12:11 PM Matthew Wilcox (Oracle)
<willy@...radead.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@...radead.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@...el.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@...el.com>
    Signed-off-by: David Howells <dhowells@...hat.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@...r.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

Powered by blists - more mailing lists