Re: [PATCH] MODSIGN: Change default key details [ver #2]
On Wed, 2015-05-20 at 11:51 +0100, David Howells wrote: > If you're going to do this, make it two, please. One for the key > actually used to sign modules and one for a block of auxiliary keys. I've done the former already. The plan is to do the latter as a separate option, yes. -- David WoodhouseOpen Source Technology Centre david.woodho...@intel.com Intel Corporation -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] MODSIGN: Change default key details [ver #2]
David Woodhouse wrote: > I am increasingly convinced we should just ditch that entire > can of worms and take a single file named in a Kconfig option. If you're going to do this, make it two, please. One for the key actually used to sign modules and one for a block of auxiliary keys. David -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] MODSIGN: Change default key details [ver #2]
On Fri, 2015-05-01 at 17:41 -0400, Abelardo Ricart III wrote:
>
> From module-signing.txt:
>
> > Under normal conditions, the kernel build will automatically generate a new
> > keypair using openssl if one does not exist in the files:
> >
> >signing_key.priv
> >signing_key.x509
>
> Nope, sorry, not true. Even if your keys exist, due to unfortunate
> parallel make/disk write order/racy kbuild/goblins your x509.genkey
> file has a newer timestamp than your keys, and now your keys are
> going to get tossed (regenerated and overwritten, yay!). Worse still,
> I think they could even get tossed AFTER the build decides "hey nice
> keys, I'll just use those". Either way, this patch is ultimately
> correct because this is exactly the kind of racy scenario order-only
> prerequisites was made for. We should not care anything about
> x509.genkey if our signing keys already exist. Period.
>
> Here's my two-line patch strictly defining the build order, for your perusal.
>
> Signed-off-by: Abelardo Ricart III
> ---
>
> diff --git a/kernel/Makefile b/kernel/Makefile
> index 1408b33..10c8df0 100644
> --- a/kernel/Makefile
> +++ b/kernel/Makefile
> @@ -168,7 +168,8 @@ ifndef CONFIG_MODULE_SIG_HASH
> $(error Could not determine digest type to use from kernel config)
> endif
>
> -signing_key.priv signing_key.x509: x509.genkey
> +signing_key.priv signing_key.x509: | x509.genkey
> + $(warning *** X.509 module signing key pair not found in root of
> source tree ***)
> @echo "###"
> @echo "### Now generating an X.509 key pair to be used for signing
> modules."
> @echo "###"
I think we still need this, or something equivalent. I'll take a closer
look at the dependencies and see if there's a better option — that bit
about keys being tossed AFTER the build is already using them does
definitely seem like a bug.
This approach might suffice as a last resort, but I don't like it much.
For an ephemeral key, we *do* want to rebuild it if you touch
x509.genkey. And signing_key.{priv,x509} *are* now purely intended to
be ephemeral — see
https://lkml.org/lkml/2015/5/18/527 and my patches which David Howells
has now merged into his tree.
I suspect the real bug here will turn out to be caused by the fact that
signing_key.priv and signing_key.x509 are distinct Makefile targets and
we are *also* generating each as a side-effect of generating the other.
And make doesn't know about the side-effects.
Maybe we'd do better with a single rule for 'signing_key.pem' which
contains both key and cert. Which is the way it is for an external key
anyway. I'll look at implementing that.
I also wonder if the problem that Linus saw with "X.509 certificate
list changed" was a different issue, in the $(wildcard *.x509)
handling. I am increasingly convinced we should just ditch that entire
can of worms and take a single file named in a Kconfig option. I'll throw that
together too and see if it makes me
happy.
--
David WoodhouseOpen Source Technology Centre
david.woodho...@intel.com Intel Corporation
smime.p7s
Description: S/MIME cryptographic signature
Re: [PATCH] MODSIGN: Change default key details [ver #2]
On Tue, 2015-05-19 at 15:14 +0100, David Howells wrote:
>
> > Right. In this very thread :-). But are you fine with using *.auto for
> > the generated files? I'll send a rebased series.
>
> Can you send me your patches and I'll add them to this:
>
>
> http://git.kernel.org/cgit/linux/kernel/git/dhowells/linux-fs.git/log/?h=modsign-pkcs7
>
> Then I can integrate them with David Woodhouse's patches in the same
> area and pass the branch on to James Morris.
We're talking about these three patches, yes?
https://lkml.org/lkml/2015/2/20/546
https://lkml.org/lkml/2015/5/4/614
https://lkml.org/lkml/2015/5/7/488
I think the third is obsolete now that we allow the user to provide
their own certificate + key by setting CONFIG_MODULE_SIG_KEY, because
the in-tree signing_key.{priv,x509} are *always* autogenerated.
The first two I don't think go far enough. I dislike automatically
picking up files that were lying around in the tree, and it's still
going to break if there's a file with a space in its name.
Let's just introduce a new Kconfig option for 'extra certificates' which
takes the filename of a PEM-format file. We can process that into DER
form with some fairly trivial modifications to scripts/extract-cert.c.
--
David WoodhouseOpen Source Technology Centre
david.woodho...@intel.com Intel Corporation
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] MODSIGN: Change default key details [ver #2]
Hi Michal, > Right. In this very thread :-). But are you fine with using *.auto for > the generated files? I'll send a rebased series. Can you send me your patches and I'll add them to this: http://git.kernel.org/cgit/linux/kernel/git/dhowells/linux-fs.git/log/?h=modsign-pkcs7 Then I can integrate them with David Woodhouse's patches in the same area and pass the branch on to James Morris. David -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] MODSIGN: Change default key details [ver #2]
On Mon, 2015-05-18 at 12:13 +0100, David Woodhouse wrote: > On Mon, 2015-05-18 at 11:47 +0100, David Howells wrote: > > David Woodhouse wrote: > > You could make it so that the make process picks up .pem files and converts > > them to DER-encoded .x509 files. > > I don't actually care whether it's PEM or DER form per se. What I > really care about is the horrid trick of automatically finding the > files to be included with a wildcard, and pulling them into the build. > > That would be icky enough if we *weren't* going to *trust* the things! Agreed! There's no reason for having them in the root of the build tree. > With a PEM file it's common to have multiple certs in a single file, > and you could have a simple config option for the 'additional certs' > file which explicitly pulls it in. Rather than the current hack. > Doing that with multiple certs in the same file in DER form, if that > works, would also be tolerable. Although it's less normal to have a > file in that format. Either method would be preferable. Mimi -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] MODSIGN: Change default key details [ver #2]
On Thu, 2015-05-07 at 14:15 +0200, Michal Marek wrote: > > I like > > Linus's use of the filechk macro on the second - but we shouldn't overwrite > > keys someone has manually placed in the tree if the key generation template > > changes due to git pull altering kernel/Makefile. > > That's the problem with allowing a file to be either user-supplied or > generated. We can use separate files for the user-supplied/generated > cases like below and solve this for good. Alternatively, we could declare that signing_key.priv/signing_key.x509 are *always* auto-generated. If the user wants to use a pregenerated key of their own then they can use CONFIG_MODULE_SIG_KEY¹ for that. -- David WoodhouseOpen Source Technology Centre david.woodho...@intel.com Intel Corporation¹ http://git.infradead.org/users/dwmw2/modsign-pkcs11-c.git/commitdiff/3d69ae738 smime.p7s Description: S/MIME cryptographic signature
Re: [PATCH] MODSIGN: Change default key details [ver #2]
On Mon, 2015-05-18 at 11:47 +0100, David Howells wrote: > David Woodhouse wrote: > > > Why not just take multiple certs in PEM form in a single file, rather > > than automatically including *.x509 in DER form? Wouldn't that be a > > whole lot easier? > > No, for the following reasons: > > (1) Unless we want the kernel to be able to handle PEM form, they have to be > converted to DER form for inclusion in system_certificates.S. It's just base64. It's fairly trivial to convert. > (2) We would have to combine the automatically generated signing cert with > the added certs, though, admittedly, this could be done in > system_certificates.S. Yes, merging the signing cert (be it automatically generated or otherwise) does need to be done. But that's easy enough. And I already have work to do on processing the signing cert, to allow it to come from the same PKCS#11 URI that specifies the key. > (3) We've already told people they must drop DER certs into the source tree > and distribution kernel packages are already doing this, so we have to > make sure they get this right. Yes, absolutely. But I think we can cope with that. > You could make it so that the make process picks up .pem files and converts > them to DER-encoded .x509 files. I don't actually care whether it's PEM or DER form per se. What I really care about is the horrid trick of automatically finding the files to be included with a wildcard, and pulling them into the build. That would be icky enough if we *weren't* going to *trust* the things! With a PEM file it's common to have multiple certs in a single file, and you could have a simple config option for the 'additional certs' file which explicitly pulls it in. Rather than the current hack. Doing that with multiple certs in the same file in DER form, if that works, would also be tolerable. Although it's less normal to have a file in that format. -- David WoodhouseOpen Source Technology Centre david.woodho...@intel.com Intel Corporation smime.p7s Description: S/MIME cryptographic signature
Re: [PATCH] MODSIGN: Change default key details [ver #2]
David Howells wrote: > We could even make the kernel handle PEM. It shouldn't be very much overhead > since it's just a wrapping/encoding of the DER, right? OTOH, PEM is ~33% larger than DER, so we probably want to stick with DER embedded in the kernel. David -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] MODSIGN: Change default key details [ver #2]
David Woodhouse wrote: > Why not just take multiple certs in PEM form in a single file, rather > than automatically including *.x509 in DER form? Wouldn't that be a > whole lot easier? No, for the following reasons: (1) Unless we want the kernel to be able to handle PEM form, they have to be converted to DER form for inclusion in system_certificates.S. (2) We would have to combine the automatically generated signing cert with the added certs, though, admittedly, this could be done in system_certificates.S. (3) We've already told people they must drop DER certs into the source tree and distribution kernel packages are already doing this, so we have to make sure they get this right. You could make it so that the make process picks up .pem files and converts them to DER-encoded .x509 files. You can cat a bunch of DER certs together and the kernel will break them apart when it parses the single buffer that contains all the certs. We could even make the kernel handle PEM. It shouldn't be very much overhead since it's just a wrapping/encoding of the DER, right? So it's by no means impossible, but it's not easier. David -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] MODSIGN: Change default key details [ver #2]
On Mon, 2015-05-04 at 11:45 -0700, Linus Torvalds wrote: > On Sun, May 3, 2015 at 6:45 PM, Linus Torvalds > wrote: > > > > I'd much rather see "x509.genkey" be generated with a move-if-changed > > pattern, so that it only changes if (a) it didn't exist before or (b) > > it actually has new content. > > Hmm. Something like the attached, to make the .x509.list file be > properly generated? > > That still leaves the problem that the X509_CERTIFICATES variable > itself seems to be badly defined, in that it ends up randomly having > the "./" in front of the filename due to confusion between > "signing_key.x509" being both in > >X509_CERTIFICATES-y := $(wildcard *.x509) $(wildcard $(srctree)/*.x509) > > (when that .x509 file was pre-existing), and > >X509_CERTIFICATES-$(CONFIG_MODULE_SIG) += $(objtree)/signing_key.x509 > > where I think that "$(objtree)/" comes in. > > DavidH, comments? Why not just take multiple certs in PEM form in a single file, rather than automatically including *.x509 in DER form? Wouldn't that be a whole lot easier? We can still have a special case for signing_key.x509 if we want it. -- dwmw2 -- David WoodhouseOpen Source Technology Centre david.woodho...@intel.com Intel Corporation smime.p7s Description: S/MIME cryptographic signature
Re: [PATCH] MODSIGN: Change default key details [ver #2]
Michal Marek wrote: > Right. In this very thread :-). But are you fine with using *.auto for > the generated files? I'll send a rebased series. Sure, yes. David -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] MODSIGN: Change default key details [ver #2]
On 2015-05-08 15:05, David Howells wrote: > Michal Marek wrote: > >> +@echo >>$@ "[ req_distinguished_name ]" >> +@echo >>$@ "O = Magrathea" >> +@echo >>$@ "CN = Glacier signing key" >> +@echo >>$@ "emailAddress = slartibartfast@magrathea.h2g2" > > This bit has changed upstream. Right. In this very thread :-). But are you fine with using *.auto for the generated files? I'll send a rebased series. Michal -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] MODSIGN: Change default key details [ver #2]
Michal Marek wrote: > + @echo >>$@ "[ req_distinguished_name ]" > + @echo >>$@ "O = Magrathea" > + @echo >>$@ "CN = Glacier signing key" > + @echo >>$@ "emailAddress = slartibartfast@magrathea.h2g2" This bit has changed upstream. David -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] MODSIGN: Change default key details [ver #2]
On Thu, May 07, 2015 at 02:15:46PM +0200, Michal Marek wrote:
> That's the problem with allowing a file to be either user-supplied or
> generated. We can use separate files for the user-supplied/generated
> cases like below and solve this for good. Not signed off yet, because it
> is only lightly tested and the clean rules and .gitignore need to be
> updated.
Forgot to 'git add' one typo fix.
>From 132a4494b255d2320bcafc729791b8bf26c9d244 Mon Sep 17 00:00:00 2001
From: Michal Marek
Date: Thu, 7 May 2015 13:38:23 +0200
Subject: [PATCH] MODSIGN: Split user-supplied and autogenerated signing key
Allow the users to place signing_key.{x509,priv} and x509.genkey in the
source tree. If any of these files is missing, generate the file in the
build tree with an .auto suffix. This avoids problems with overwriting
user-supplied files.
---
Makefile| 4 ++--
kernel/Makefile | 46 +++---
2 files changed, 25 insertions(+), 25 deletions(-)
diff --git a/Makefile b/Makefile
index 19e256a..69026fc 100644
--- a/Makefile
+++ b/Makefile
@@ -873,8 +873,8 @@ INITRD_COMPRESS-$(CONFIG_RD_LZ4) := lz4
# export INITRD_COMPRESS := $(INITRD_COMPRESS-y)
ifdef CONFIG_MODULE_SIG_ALL
-MODSECKEY = ./signing_key.priv
-MODPUBKEY = ./signing_key.x509
+MODSECKEY = $(firstword $(wildcard $(srctree)/signing_key.priv)
./signing_key.priv.auto)
+MODPUBKEY = $(firstword $(wildcard $(srctree)/signing_key.x509)
./signing_key.x509.auto)
export MODPUBKEY
mod_sign_cmd = perl $(srctree)/scripts/sign-file $(CONFIG_MODULE_SIG_HASH)
$(MODSECKEY) $(MODPUBKEY)
else
diff --git a/kernel/Makefile b/kernel/Makefile
index e072239..4bcf20e 100644
--- a/kernel/Makefile
+++ b/kernel/Makefile
@@ -124,7 +124,7 @@ $(obj)/config_data.h: $(obj)/config_data.gz FORCE
###
ifeq ($(CONFIG_SYSTEM_TRUSTED_KEYRING),y)
X509_CERTIFICATES-y := $(wildcard *.x509)
-X509_CERTIFICATES-$(CONFIG_MODULE_SIG) += signing_key.x509
+X509_CERTIFICATES-$(CONFIG_MODULE_SIG) += $(if $(wildcard
$(srctree)/signing_key.x509),,signing_key.x509.auto)
X509_CERTIFICATES := $(sort $(X509_CERTIFICATES-y))
ifneq ($(objtree),$(srctree))
X509_CERTIFICATES += $(sort $(wildcard $(srctree)/*.x509))
@@ -165,7 +165,7 @@ ifndef CONFIG_MODULE_SIG_HASH
$(error Could not determine digest type to use from kernel config)
endif
-signing_key.priv signing_key.x509: x509.genkey
+signing_key.priv.auto signing_key.x509.auto: $(firstword $(wildcard
$(srctree)/x509.genkey) x509.genkey.auto)
@echo "###"
@echo "### Now generating an X.509 key pair to be used for signing
modules."
@echo "###"
@@ -175,30 +175,30 @@ signing_key.priv signing_key.x509: x509.genkey
@echo "### number generator if one is available."
@echo "###"
openssl req -new -nodes -utf8 -$(CONFIG_MODULE_SIG_HASH) -days 36500 \
- -batch -x509 -config x509.genkey \
- -outform DER -out signing_key.x509 \
- -keyout signing_key.priv 2>&1
+ -batch -x509 -config $< \
+ -outform DER -out signing_key.x509.auto \
+ -keyout signing_key.priv.auto 2>&1
@echo "###"
@echo "### Key pair generated."
@echo "###"
-x509.genkey:
+x509.genkey.auto:
@echo Generating X.509 key generation config
- @echo >x509.genkey "[ req ]"
- @echo >>x509.genkey "default_bits = 4096"
- @echo >>x509.genkey "distinguished_name = req_distinguished_name"
- @echo >>x509.genkey "prompt = no"
- @echo >>x509.genkey "string_mask = utf8only"
- @echo >>x509.genkey "x509_extensions = myexts"
- @echo >>x509.genkey
- @echo >>x509.genkey "[ req_distinguished_name ]"
- @echo >>x509.genkey "O = Magrathea"
- @echo >>x509.genkey "CN = Glacier signing key"
- @echo >>x509.genkey "emailAddress = slartibartfast@magrathea.h2g2"
- @echo >>x509.genkey
- @echo >>x509.genkey "[ myexts ]"
- @echo >>x509.genkey "basicConstraints=critical,CA:FALSE"
- @echo >>x509.genkey "keyUsage=digitalSignature"
- @echo >>x509.genkey "subjectKeyIdentifier=hash"
- @echo >>x509.genkey "authorityKeyIdentifier=keyid"
+ @echo >$@ "[ req ]"
+ @echo >>$@ "default_bits = 4096"
+ @echo >>$@ "distinguished_name = req_distinguished_name"
+ @echo >>$@ "prompt = no"
+ @echo >>$@ "string_mask = utf8only"
+ @echo >>$@ "x509_extensions = myexts"
+ @echo >>$@
+ @echo >>$@ "[ req_distinguished_name ]"
+ @echo >>$@ "O = Magrathea"
+ @echo >>$@ "CN = Glacier signing key"
+ @echo >>$@ "emailAddress = slartibartfast@magrathea.h2g2"
+ @echo >>$@
+ @echo >>$@ "[ myexts ]"
+ @echo >>$@ "basicConstraints=critical,CA:FALSE"
+ @echo >>$@ "keyUsage=digitalSignature"
+ @echo >>$@ "subjectKeyIdentifier=hash"
+ @echo >>$@ "authorityKeyIdentifier=keyid"
endif
--
2.1.
Re: [PATCH] MODSIGN: Change default key details [ver #2]
On 2015-05-07 13:00, David Howells wrote:
> Michal Marek wrote:
>
>> are you fine with these two patches?
>>
>> https://lkml.org/lkml/2015/2/20/546
>> https://lkml.org/lkml/2015/5/4/614
>
> Yeah, I think so. Your reasoning on the first one is sound - but is it
> possible for $(objtree) to != $(srctree) even when they're coincident.
This part is fine. $(objtee) is always '.', the variable is only used as
an annotation. You can of course do 'make O=/symlink/to/current/dir',
but this will fail with
/your/current/dir is not clean, please run 'make mrproper'
> I like
> Linus's use of the filechk macro on the second - but we shouldn't overwrite
> keys someone has manually placed in the tree if the key generation template
> changes due to git pull altering kernel/Makefile.
That's the problem with allowing a file to be either user-supplied or
generated. We can use separate files for the user-supplied/generated
cases like below and solve this for good. Not signed off yet, because it
is only lightly tested and the clean rules and .gitignore need to be
updated.
Michal
>From aa68988b9b669f2c7d17466ba39e84d7e6617c34 Mon Sep 17 00:00:00 2001
From: Michal Marek
Date: Thu, 7 May 2015 13:38:23 +0200
Subject: [PATCH] MODSIGN: Split user-supplied and autogenerated signing key
Allow the users to place signing_key.{x509,priv} and x509.genkey in the
source tree. If any of these files is missing, generate the file in the
build tree with an .auto suffix. This avoids problems with overwriting
user-supplied files.
---
Makefile| 4 ++--
kernel/Makefile | 46 +++---
2 files changed, 25 insertions(+), 25 deletions(-)
diff --git a/Makefile b/Makefile
index 19e256a..b4b8ef5 100644
--- a/Makefile
+++ b/Makefile
@@ -873,8 +873,8 @@ INITRD_COMPRESS-$(CONFIG_RD_LZ4) := lz4
# export INITRD_COMPRESS := $(INITRD_COMPRESS-y)
ifdef CONFIG_MODULE_SIG_ALL
-MODSECKEY = ./signing_key.priv
-MODPUBKEY = ./signing_key.x509
+MODSECKEY = $(firstword $(wildcard
$(srctree)/signing_key.priv),./signing_key.priv.auto)
+MODPUBKEY = $(firstword $(wildcard
$(srctree)/signing_key.x509),./signing_key.x509.auto)
export MODPUBKEY
mod_sign_cmd = perl $(srctree)/scripts/sign-file $(CONFIG_MODULE_SIG_HASH)
$(MODSECKEY) $(MODPUBKEY)
else
diff --git a/kernel/Makefile b/kernel/Makefile
index e072239..4bcf20e 100644
--- a/kernel/Makefile
+++ b/kernel/Makefile
@@ -124,7 +124,7 @@ $(obj)/config_data.h: $(obj)/config_data.gz FORCE
###
ifeq ($(CONFIG_SYSTEM_TRUSTED_KEYRING),y)
X509_CERTIFICATES-y := $(wildcard *.x509)
-X509_CERTIFICATES-$(CONFIG_MODULE_SIG) += signing_key.x509
+X509_CERTIFICATES-$(CONFIG_MODULE_SIG) += $(if $(wildcard
$(srctree)/signing_key.x509),,signing_key.x509.auto)
X509_CERTIFICATES := $(sort $(X509_CERTIFICATES-y))
ifneq ($(objtree),$(srctree))
X509_CERTIFICATES += $(sort $(wildcard $(srctree)/*.x509))
@@ -165,7 +165,7 @@ ifndef CONFIG_MODULE_SIG_HASH
$(error Could not determine digest type to use from kernel config)
endif
-signing_key.priv signing_key.x509: x509.genkey
+signing_key.priv.auto signing_key.x509.auto: $(firstword $(wildcard
$(srctree)/x509.genkey) x509.genkey.auto)
@echo "###"
@echo "### Now generating an X.509 key pair to be used for signing
modules."
@echo "###"
@@ -175,30 +175,30 @@ signing_key.priv signing_key.x509: x509.genkey
@echo "### number generator if one is available."
@echo "###"
openssl req -new -nodes -utf8 -$(CONFIG_MODULE_SIG_HASH) -days 36500 \
- -batch -x509 -config x509.genkey \
- -outform DER -out signing_key.x509 \
- -keyout signing_key.priv 2>&1
+ -batch -x509 -config $< \
+ -outform DER -out signing_key.x509.auto \
+ -keyout signing_key.priv.auto 2>&1
@echo "###"
@echo "### Key pair generated."
@echo "###"
-x509.genkey:
+x509.genkey.auto:
@echo Generating X.509 key generation config
- @echo >x509.genkey "[ req ]"
- @echo >>x509.genkey "default_bits = 4096"
- @echo >>x509.genkey "distinguished_name = req_distinguished_name"
- @echo >>x509.genkey "prompt = no"
- @echo >>x509.genkey "string_mask = utf8only"
- @echo >>x509.genkey "x509_extensions = myexts"
- @echo >>x509.genkey
- @echo >>x509.genkey "[ req_distinguished_name ]"
- @echo >>x509.genkey "O = Magrathea"
- @echo >>x509.genkey "CN = Glacier signing key"
- @echo >>x509.genkey "emailAddress = slartibartfast@magrathea.h2g2"
- @echo >>x509.genkey
- @echo >>x509.genkey "[ myexts ]"
- @echo >>x509.genkey "basicConstraints=critical,CA:FALSE"
- @echo >>x509.genkey "keyUsage=digitalSignature"
- @echo >>x509.genkey "subjectKeyIdentifier=hash"
- @echo >>x509.genkey "authorityKeyIdentifier=keyid"
+ @echo >$@ "[ req ]"
+ @echo >>
Re: [PATCH] MODSIGN: Change default key details [ver #2]
Michal Marek wrote: > are you fine with these two patches? > > https://lkml.org/lkml/2015/2/20/546 > https://lkml.org/lkml/2015/5/4/614 Yeah, I think so. Your reasoning on the first one is sound - but is it possible for $(objtree) to != $(srctree) even when they're coincident. I like Linus's use of the filechk macro on the second - but we shouldn't overwrite keys someone has manually placed in the tree if the key generation template changes due to git pull altering kernel/Makefile. Sometimes I think makescript is like trying to program in Prolog - but worse. David -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] MODSIGN: Change default key details [ver #2]
On 2015-05-05 17:41, Linus Torvalds wrote: > On Tue, May 5, 2015 at 8:22 AM, Michal Marek wrote: >> On 2015-05-04 20:45, Linus Torvalds wrote: >>> >>> That still leaves the problem that the X509_CERTIFICATES variable >>> itself seems to be badly defined [..] >> >> This will be fixed once https://lkml.org/lkml/2015/2/20/546 is merged >> (the two patches are independent of each other). > > Ok, good. > > Willing to take my patch through your Kconfig tree? Add my sign-off > and some sane description.. David, are you fine with these two patches? https://lkml.org/lkml/2015/2/20/546 https://lkml.org/lkml/2015/5/4/614 Michal -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] MODSIGN: Change default key details [ver #2]
On Tue, 2015-05-05 at 15:34 +0100, David Howells wrote: > Abelardo Ricart III wrote: > > > Here's a (barely tested) patch to show what I mean with the config option. > > The > > default case is to always generate a new key at build > > (MODULE_SIG_BUILDGEN=y) > > and fallback on generating keys during build only if one doesn't exist > > (MODULE_SIG_BUILDGEN=n). > > Does it cope with randconfig? > > David Well it would only depend on MODULE_SIG, and switching it on and off again would do exactly what it says it's going to do: either regenerate the signing keys every time, or don't if they already exist. I would have to actually change the logic slightly so it works strictly as intended though. So no, this isn't merge-able at all. I was more wondering if implementing something to this effect would be okay, so we can strictly define the behavior at build time (no surprises). -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] MODSIGN: Change default key details [ver #2]
Abelardo Ricart III wrote: > Here's a (barely tested) patch to show what I mean with the config option. The > default case is to always generate a new key at build (MODULE_SIG_BUILDGEN=y) > and fallback on generating keys during build only if one doesn't exist > (MODULE_SIG_BUILDGEN=n). Does it cope with randconfig? David -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] MODSIGN: Change default key details [ver #2]
David Howells wrote: > > + $(call filechk,x509_list) Ah... I'd missed that there was a comma, not an underscore there. David -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] MODSIGN: Change default key details [ver #2]
Linus Torvalds wrote: > +define filechk_x509_list > + echo $(X509_CERTIFICATES) > +endef > targets += $(obj)/.x509.list > -$(obj)/.x509.list: > - @echo $(X509_CERTIFICATES) >$@ > +$(obj)/.x509.list: Makefile FORCE > + $(call filechk,x509_list) How does that actually work? David -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] MODSIGN: Change default key details [ver #2]
On Tue, May 5, 2015 at 7:33 AM, David Howells wrote: > Linus Torvalds wrote: > >> +define filechk_x509_list >> + echo $(X509_CERTIFICATES) >> +endef >> targets += $(obj)/.x509.list >> -$(obj)/.x509.list: >> - @echo $(X509_CERTIFICATES) >$@ >> +$(obj)/.x509.list: Makefile FORCE >> + $(call filechk,x509_list) > > How does that actually work? So the Makefile dependency is entirely fake, it's just that the "filechk" macro wants to have an input. The FORCE means that it ends up being done every time, regardless of whether the Makefile has changed or not. See scripts/Kbuild.include for details on the "filechk" macro, bit it basically takes another make macro (called "filechk_xyz"), runs that macro as a shell script and outputs it to a temporary file: $(filechk_$(1)) < $< > $@.tmp; (that "$<" is also why the rule wants a some input, in our case "Makefile"). It then does a move-if-changed of the temporary file to the target. So the end result is that we run that "filechk_x509_list" script, compare the output to the old target, and update the target iff it is different. That would seem to be exactly what we want. That said, as mentioned, the whole "X509_CERTIFICATES" thing is unstable, and ends up being "./signing_key.x509" or "signing_key.x509" depending on whether that file existed or not. That needs fixing, so that we get stable output. So some filtering required. Linus -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] MODSIGN: Change default key details [ver #2]
On 2015-05-04 20:45, Linus Torvalds wrote: > On Sun, May 3, 2015 at 6:45 PM, Linus Torvalds > wrote: >> >> I'd much rather see "x509.genkey" be generated with a move-if-changed >> pattern, so that it only changes if (a) it didn't exist before or (b) >> it actually has new content. > > Hmm. Something like the attached, to make the .x509.list file be > properly generated? > > That still leaves the problem that the X509_CERTIFICATES variable > itself seems to be badly defined, in that it ends up randomly having > the "./" in front of the filename due to confusion between > "signing_key.x509" being both in > >X509_CERTIFICATES-y := $(wildcard *.x509) $(wildcard $(srctree)/*.x509) > > (when that .x509 file was pre-existing), and > >X509_CERTIFICATES-$(CONFIG_MODULE_SIG) += $(objtree)/signing_key.x509 > > where I think that "$(objtree)/" comes in. This will be fixed once https://lkml.org/lkml/2015/2/20/546 is merged (the two patches are independent of each other). Michal -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] MODSIGN: Change default key details [ver #2]
Linus Torvalds wrote: > ... > So the end result is that we run that "filechk_x509_list" script, > compare the output to the old target, and update the target iff it is > different. That would seem to be exactly what we want. Yep. > That said, as mentioned, the whole "X509_CERTIFICATES" thing is > unstable, and ends up being "./signing_key.x509" or "signing_key.x509" > depending on whether that file existed or not. That needs fixing, so > that we get stable output. So some filtering required. Yeah, the stability thing is a bit of an irritation. X509_CERTIFICATES might also include "$(srcdir)/signing_key.x509". This is one of the things that has given me difficulties because the source and build trees are sometimes the same and sometimes not (and then throw in a symlink somewhere in the path...). I was trying to use $(realpath ...) to deal with this - but that doesn't work if the path doesn't point to an extant file:-/ Does it make sense to produce an error if the source and build trees are not coincident and we see an x509 cert of the same filename cropping up in both? Also X509_CERTIFICATES might hold other certs - though these shouldn't really be seen in the build dir. I wonder if we should enforce that to make life easier. David -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] MODSIGN: Change default key details [ver #2]
On Tue, May 5, 2015 at 8:22 AM, Michal Marek wrote: > On 2015-05-04 20:45, Linus Torvalds wrote: >> >> That still leaves the problem that the X509_CERTIFICATES variable >> itself seems to be badly defined [..] > > This will be fixed once https://lkml.org/lkml/2015/2/20/546 is merged > (the two patches are independent of each other). Ok, good. Willing to take my patch through your Kconfig tree? Add my sign-off and some sane description.. Linus -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] MODSIGN: Change default key details [ver #2]
On Sun, 2015-05-03 at 22:16 -0700, Linus Torvalds wrote: > On May 3, 2015 21:42, "Abelardo Ricart III" wrote: > > > > That's correct. I was under the impression that having the Makefile generate > > the signing keys was something that was done just to prevent a build failure > > with CONFIG_MODULE_SIG but no keys. > No, that's absolutely not the case. > In fact, the whole "external keys" model is entirely bogus for any same use > case. > The sane use case is to have the build process generate a random key at build > time, that gets thrown away after installing the kernel and modules. That, > together with "require signed modules" makes module as safe as building > everything into the kernel - you won't be open to things like rootkits that > try to load modules, because nobody has access to the key any more. > The only time you will have an external non-generated key is when you either > want to do the insane secure boot thing, or when a distro builds an official > kernel. But those are *not* the common development situations. > So the "generated random throwaway key" is absolutely not some of special > case to not break the build. It should be seen as the *default* case. >Linus Here's a (barely tested) patch to show what I mean with the config option. The default case is to always generate a new key at build (MODULE_SIG_BUILDGEN=y) and fallback on generating keys during build only if one doesn't exist (MODULE_SIG_BUILDGEN=n). This fixes the issues with keys being unexpectedly overwritten when you don't want them to be. Also fixes keys _not_ always being regenerated when they really should be (the default use case). --- diff --git a/init/Kconfig b/init/Kconfig index dc24dec..5ab8b97 100644 --- a/init/Kconfig +++ b/init/Kconfig @@ -1903,6 +1903,16 @@ config MODULE_SIG_ALL comment "Do not forget to sign required modules with scripts/sign-file" depends on MODULE_SIG_FORCE && !MODULE_SIG_ALL +config MODULE_SIG_BUILDGEN + bool "Always generate keys during build" + default y + depends on MODULE_SIG + help + Always generate new signing keys at build time. Only say N here if + you intend on supplying your own signing keys. + + Say Y here unless you know what you are doing. + choice prompt "Which hash algorithm should modules be signed with?" depends on MODULE_SIG diff --git a/kernel/Makefile b/kernel/Makefile index 60c302c..86d836d 100644 --- a/kernel/Makefile +++ b/kernel/Makefile @@ -170,6 +170,15 @@ ifndef CONFIG_MODULE_SIG_HASH $(error Could not determine digest type to use from kernel config) endif +.PHONY: generate_keys +ifeq ($(CONFIG_MODULE_SIG_BUILDGEN),y) + # Always generate new signing keys + generate_keys: signing_key.priv signing_key.x509 FORCE +else + # Only generate signing keys if they don't exist + generate_keys: | signing_key.priv signing_key.x509 +endif + signing_key.priv signing_key.x509: x509.genkey @echo "###" @echo "### Now generating an X.509 key pair to be used for signing modules." -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] MODSIGN: Change default key details [ver #2]
On Sun, May 3, 2015 at 6:45 PM, Linus Torvalds wrote: > > I'd much rather see "x509.genkey" be generated with a move-if-changed > pattern, so that it only changes if (a) it didn't exist before or (b) > it actually has new content. Hmm. Something like the attached, to make the .x509.list file be properly generated? That still leaves the problem that the X509_CERTIFICATES variable itself seems to be badly defined, in that it ends up randomly having the "./" in front of the filename due to confusion between "signing_key.x509" being both in X509_CERTIFICATES-y := $(wildcard *.x509) $(wildcard $(srctree)/*.x509) (when that .x509 file was pre-existing), and X509_CERTIFICATES-$(CONFIG_MODULE_SIG) += $(objtree)/signing_key.x509 where I think that "$(objtree)/" comes in. DavidH, comments? Linus kernel/Makefile | 14 +- 1 file changed, 5 insertions(+), 9 deletions(-) diff --git a/kernel/Makefile b/kernel/Makefile index 60c302cfb4d3..205bdc2b11e7 100644 --- a/kernel/Makefile +++ b/kernel/Makefile @@ -135,13 +135,6 @@ ifeq ($(X509_CERTIFICATES),) $(warning *** No X.509 certificates found ***) endif -ifneq ($(wildcard $(obj)/.x509.list),) -ifneq ($(shell cat $(obj)/.x509.list),$(X509_CERTIFICATES)) -$(info X.509 certificate list changed) -$(shell rm $(obj)/.x509.list) -endif -endif - kernel/system_certificates.o: $(obj)/x509_certificate_list quiet_cmd_x509certs = CERTS $@ @@ -151,9 +144,12 @@ targets += $(obj)/x509_certificate_list $(obj)/x509_certificate_list: $(X509_CERTIFICATES) $(obj)/.x509.list $(call if_changed,x509certs) +define filechk_x509_list + echo $(X509_CERTIFICATES) +endef targets += $(obj)/.x509.list -$(obj)/.x509.list: - @echo $(X509_CERTIFICATES) >$@ +$(obj)/.x509.list: Makefile FORCE + $(call filechk,x509_list) endif clean-files := x509_certificate_list .x509.list
Re: [PATCH] MODSIGN: Change default key details [ver #2]
On Sun, 2015-05-03 at 22:16 -0700, Linus Torvalds wrote: > On May 3, 2015 21:42, "Abelardo Ricart III" wrote: > > > > That's correct. I was under the impression that having the Makefile generate > > the signing keys was something that was done just to prevent a build failure > > with CONFIG_MODULE_SIG but no keys. > No, that's absolutely not the case. > In fact, the whole "external keys" model is entirely bogus for any same use > case. > The sane use case is to have the build process generate a random key at build > time, that gets thrown away after installing the kernel and modules. That, > together with "require signed modules" makes module as safe as building > everything into the kernel - you won't be open to things like rootkits that > try to load modules, because nobody has access to the key any more. > For varying degrees of accessibility. If the key gets overwritten with data during removal that would be ideal. > The only time you will have an external non-generated key is when you either > want to do the insane secure boot thing, or when a distro builds an official > kernel. Or maybe signing and deploying a custom module for a very large amount of machines that enforce module signing? Quite cumbersome when not utilizing your own keys... > But those are *not* the common development situations. > So the "generated random throwaway key" is absolutely not some of special > case to not break the build. It should be seen as the *default* case. >Linus So one-time keys is the default case. What of the idea of a config option for the other case as I'd proposed? One-time key generation being both the default (always regenerate, sign, then throwaway. Overwrite existing keys and config.) as well as the fallback (config option for one-time keys is unset, but external keys are absent or invalid. Generate and use a new key pair as per usual). Thanks. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] MODSIGN: Change default key details [ver #2]
On Sun, 2015-05-03 at 18:45 -0700, Linus Torvalds wrote: > On Sat, May 2, 2015 at 2:46 AM, Abelardo Ricart III > wrote: > > endif > > > > -signing_key.priv signing_key.x509: x509.genkey > > +signing_key.priv signing_key.x509: | x509.genkey > > Hmm. Thinking some more about this, I'm not entirely convinced. > > With this change, if we change kernel/Makefile to have a different > keysigning authority etc, it won't re-generate the keys, because the > old keys still exist. No? That would be wrong. > That's correct. I was under the impression that having the Makefile generate the signing keys was something that was done just to prevent a build failure with CONFIG_MODULE_SIG but no keys. The comment above the key generation target seems to say that. David Howells' patch to make the key details "more obviously unspecified" seems to be along those lines as well; that those generated keys are simply a generic way to have signed modules without managing your own keys. In that case, the actual contents of x509.genkey would be of little significance because those keys are entirely generic and transient. > I'd much rather see "x509.genkey" be generated with a move-if-changed > pattern, so that it only changes if (a) it didn't exist before or (b) > it actually has new content. > And this would indeed trigger key regeneration by make. But what if I do manage my own keys? As I said, I wouldn't want the Makefile to touch them at all, even if the x509.genkey config is missing or was changed. So we have two use cases here: people who use auto-generated keys and people who use their own keys. Sounds like this could be a good case for having a config option that tells make which group you fall under? Something like "CONFIG_MODULE_SIG_GEN_KEY"? If CONFIG_MODULE_SIG_GEN_KEY=y, keys are (re)generated based on the internal x509.genkey. If CONFIG_MODULE_SIG_GEN_KEY is not set, keys are only (re)generated if they don't already exist, which is what my patch would do (and what the documentation implies should be happening now). > On a tangentially related issue: I figured out why I get those (very > annoying) "X.509 certificate list changed" messages. I made it print > out *what* changed: > > X.509 certificate list changed from ./signing_key.x509 to signing_key.x509 > > Note the "./" difference. > > Linus That Makefile could use a makeover. Pun intended. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] MODSIGN: Change default key details [ver #2]
On Sat, May 2, 2015 at 2:46 AM, Abelardo Ricart III wrote: > endif > > -signing_key.priv signing_key.x509: x509.genkey > +signing_key.priv signing_key.x509: | x509.genkey Hmm. Thinking some more about this, I'm not entirely convinced. With this change, if we change kernel/Makefile to have a different keysigning authority etc, it won't re-generate the keys, because the old keys still exist. No? That would be wrong. I'd much rather see "x509.genkey" be generated with a move-if-changed pattern, so that it only changes if (a) it didn't exist before or (b) it actually has new content. On a tangentially related issue: I figured out why I get those (very annoying) "X.509 certificate list changed" messages. I made it print out *what* changed: X.509 certificate list changed from ./signing_key.x509 to signing_key.x509 Note the "./" difference. Linus -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] MODSIGN: Change default key details [ver #2]
On Fri, 2015-05-01 at 21:12 -0700, Linus Torvalds wrote: > So we shouldn't warn about this. The "generate random key" should be > the normal action for just about everybody but actual preduction > vendor builds. It's definitely not an error condition. Since this patch fixes the unexpected build behavior, I agree such a warning would be unnecessary. Removed. Signed-off-by: Abelardo Ricart III --- diff --git a/kernel/Makefile b/kernel/Makefile index 1408b33..81d3df9 100644 --- a/kernel/Makefile +++ b/kernel/Makefile @@ -168,7 +168,7 @@ ifndef CONFIG_MODULE_SIG_HASH $(error Could not determine digest type to use from kernel config) endif -signing_key.priv signing_key.x509: x509.genkey +signing_key.priv signing_key.x509: | x509.genkey @echo "###" @echo "### Now generating an X.509 key pair to be used for signing modules." @echo "###" -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] MODSIGN: Change default key details [ver #2]
On Sat, May 2, 2015 at 6:12 AM, Linus Torvalds wrote: > On Fri, May 1, 2015 at 2:41 PM, Abelardo Ricart III > wrote: >> >> Here's my two-line patch strictly defining the build order, for your perusal. > > Ok, so this looks possible and sounds like it could explain the issues. > > But I'd like somebody who is much more familiar with these kinds of > subtleties in 'make' to take anothe rlook and ack it. Because I had > personally never even heard (much less used) about these magical GNU > make "order-only prerequisites". Live and learn. > >> -signing_key.priv signing_key.x509: x509.genkey >> +signing_key.priv signing_key.x509: | x509.genkey >> + $(warning *** X.509 module signing key pair not found in root of >> source tree ***) > > So we shouldn't warn about this. The "generate random key" should be > the normal action for just about everybody but actual preduction > vendor builds. It's definitely not an error condition. > > But that ": |" syntax is interesting. I quick grep does show that we > do have a few previous uses, so I guess we really *do* use just about > every possible feature of GNU make even if I wasn't aware of this > one.. > > The "generate random key" does seem to be a similar "prep" phase as > the __dtbs_install_prep thing we do in the dtb install. > > Adding Michal Marek to the cc, since I want an Ack from somebody who > knows the details of GNU make more than I do. Anybody else who is a > makefile God? > /me no GNU/make or Makefile guru. When dealing with .PHONY targets [2] within the Freetz router project I bookmarked the below links. So there exists ".NOTPARALLEL" [1]. Maybe its use can be a "hacky" possibility to workaround the issue (cannot tell you about version-prereqs of GNU/make). My 0,02cents. - Sedat - [1] https://www.gnu.org/software/make/manual/html_node/Special-Targets.html [2] https://www.gnu.org/software/make/manual/html_node/Phony-Targets.html -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] MODSIGN: Change default key details [ver #2]
On Fri, May 1, 2015 at 2:41 PM, Abelardo Ricart III wrote: > > Here's my two-line patch strictly defining the build order, for your perusal. Ok, so this looks possible and sounds like it could explain the issues. But I'd like somebody who is much more familiar with these kinds of subtleties in 'make' to take anothe rlook and ack it. Because I had personally never even heard (much less used) about these magical GNU make "order-only prerequisites". Live and learn. > -signing_key.priv signing_key.x509: x509.genkey > +signing_key.priv signing_key.x509: | x509.genkey > + $(warning *** X.509 module signing key pair not found in root of > source tree ***) So we shouldn't warn about this. The "generate random key" should be the normal action for just about everybody but actual preduction vendor builds. It's definitely not an error condition. But that ": |" syntax is interesting. I quick grep does show that we do have a few previous uses, so I guess we really *do* use just about every possible feature of GNU make even if I wasn't aware of this one.. The "generate random key" does seem to be a similar "prep" phase as the __dtbs_install_prep thing we do in the dtb install. Adding Michal Marek to the cc, since I want an Ack from somebody who knows the details of GNU make more than I do. Anybody else who is a makefile God? Linus -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] MODSIGN: Change default key details [ver #2]
Forgive me if this git send-email blows up in my face somehow, as I hadn't been subscribed to this list before this reply. I had some similar and probably related behavior I submitted a patch to the kbuild guys for that fell on deaf ears. Basically, I think an order-only prerequisite would make the most sense in this case because right now the key generation target is going to trigger any time the x509.genkey file has its timestamp touched to where it becomes "newer" than our keys, even if its content remains the same. What if I provided my own keys already, as module-signing.txt said was okay? What if the autogenerated keys from my last build still exist? >From module-signing.txt: > Under normal conditions, the kernel build will automatically generate a new > keypair using openssl if one does not exist in the files: > >signing_key.priv >signing_key.x509 Nope, sorry, not true. Even if your keys exist, due to unfortunate parallel make/disk write order/racy kbuild/goblins your x509.genkey file has a newer timestamp than your keys, and now your keys are going to get tossed (regenerated and overwritten, yay!). Worse still, I think they could even get tossed AFTER the build decides "hey nice keys, I'll just use those". Either way, this patch is ultimately correct because this is exactly the kind of racy scenario order-only prerequisites was made for. We should not care anything about x509.genkey if our signing keys already exist. Period. Here's my two-line patch strictly defining the build order, for your perusal. Signed-off-by: Abelardo Ricart III --- diff --git a/kernel/Makefile b/kernel/Makefile index 1408b33..10c8df0 100644 --- a/kernel/Makefile +++ b/kernel/Makefile @@ -168,7 +168,8 @@ ifndef CONFIG_MODULE_SIG_HASH $(error Could not determine digest type to use from kernel config) endif -signing_key.priv signing_key.x509: x509.genkey +signing_key.priv signing_key.x509: | x509.genkey + $(warning *** X.509 module signing key pair not found in root of source tree ***) @echo "###" @echo "### Now generating an X.509 key pair to be used for signing modules." @echo "###" -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] MODSIGN: Change default key details [ver #2]
On Thu, Apr 30, 2015 at 10:49 AM, Sedat Dilek wrote: >> >> Yeah. I've had other reports, but I can't see anything obvious. I have a >> suspicion there may be something not declared quite right in the makefile - >> or >> even a race in make -jN, but I don't know how to reproduce it. > > Ah OK, (un)nice to read. Yeah. I've gotten "X.509 certificate list changed" messages occasionally, which makes no sense considering that I never have any keys except for the default signing key. So there is something odd foinf on with "kernel/.x509.list". The rules to generate it aren't really _rules_. It's black magic. Actually, it's *generated* with a rule, but then it has that magical "if the contents don't match" thing that is _not_ a real make rule. It would be lovely to replace that with a real "if_changed" rule, because I do think the magic hackery is what the confuses make. Maybe some concurrent build reads the file - to check if it's the same - in one make instance while it's being generated in another. Something. [ Wild handwaving going on ]. Linus -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] MODSIGN: Change default key details [ver #2]
On Thu, Apr 30, 2015 at 4:50 PM, David Howells wrote: > Sedat Dilek wrote: > >> This happened a 2nd time with a different kernel-series! >> Not sure why this was the case. >> It did not happen when rebuilding with the same kernel-config again. >> Not sure if parallel-make-jobs might be a cause for this (see attached >> build-script). > > Yeah. I've had other reports, but I can't see anything obvious. I have a > suspicion there may be something not declared quite right in the makefile - or > even a race in make -jN, but I don't know how to reproduce it. > Ah OK, (un)nice to read. >> For my quick builds of rcN Linux-kernels I normally do not need >> signing my modules. > > Me neither. Having module signing done on installation, not during the build, > is really inconvenient since I don't install the modules on my test machine > but rather copy them over with scp after booting the kernel with tftp. > >> Attached is my simple build-script for generating Debian/Ubuntu kernel >> packages via builddeb script. > > Do you have an rpmbuild version? > RPM? AFAICS Redhat Pro v7.2 was my last rpm-based distro :-). I have plans to install some of the Linux "LTS" on a machine within an honorary|unpaid project where people can switch from WinXP to Linux if they like it - among others one or two rpm-based distro will be taken into account. 1st there is a plan Then plan "b" :-). - Sedat - -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] MODSIGN: Change default key details [ver #2]
Sedat Dilek wrote: > This happened a 2nd time with a different kernel-series! > Not sure why this was the case. > It did not happen when rebuilding with the same kernel-config again. > Not sure if parallel-make-jobs might be a cause for this (see attached > build-script). Yeah. I've had other reports, but I can't see anything obvious. I have a suspicion there may be something not declared quite right in the makefile - or even a race in make -jN, but I don't know how to reproduce it. > For my quick builds of rcN Linux-kernels I normally do not need > signing my modules. Me neither. Having module signing done on installation, not during the build, is really inconvenient since I don't install the modules on my test machine but rather copy them over with scp after booting the kernel with tftp. > Attached is my simple build-script for generating Debian/Ubuntu kernel > packages via builddeb script. Do you have an rpmbuild version? David -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH] MODSIGN: Change default key details [ver #2]
Change default key details to be more obviously unspecified. Reported-by: Linus Torvalds Signed-off-by: David Howells Acked-by: James Morris --- Documentation/module-signing.txt |6 +++--- kernel/Makefile |6 +++--- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/Documentation/module-signing.txt b/Documentation/module-signing.txt index 09c2382ad055..c72702ec1ded 100644 --- a/Documentation/module-signing.txt +++ b/Documentation/module-signing.txt @@ -119,9 +119,9 @@ Most notably, in the x509.genkey file, the req_distinguished_name section should be altered from the default: [ req_distinguished_name ] - O = Magrathea - CN = Glacier signing key - emailAddress = slartibartfast@magrathea.h2g2 + #O = Unspecified company + CN = Build time autogenerated kernel key + #emailAddress = unspecified.user@unspecified.company The generated RSA key size can also be set with: diff --git a/kernel/Makefile b/kernel/Makefile index 0f8f8b0bc1bf..60c302cfb4d3 100644 --- a/kernel/Makefile +++ b/kernel/Makefile @@ -197,9 +197,9 @@ x509.genkey: @echo >>x509.genkey "x509_extensions = myexts" @echo >>x509.genkey @echo >>x509.genkey "[ req_distinguished_name ]" - @echo >>x509.genkey "O = Magrathea" - @echo >>x509.genkey "CN = Glacier signing key" - @echo >>x509.genkey "emailAddress = slartibartfast@magrathea.h2g2" + @echo >>x509.genkey "#O = Unspecified company" + @echo >>x509.genkey "CN = Build time autogenerated kernel key" + @echo >>x509.genkey "#emailAddress = unspecified.user@unspecified.company" @echo >>x509.genkey @echo >>x509.genkey "[ myexts ]" @echo >>x509.genkey "basicConstraints=critical,CA:FALSE" -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
