| Message ID | 20211218031122.4117631-1-willy@infradead.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v2] builddeb: Support signing kernels with the module signing key | expand |
+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
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!
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
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!
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
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
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
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.
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.
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 :)
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
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.
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.
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.
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
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(-)