Messages in this thread | | | Subject | Re: [PATCH] MODSIGN: Change default key details [ver #2] | From | David Woodhouse <> | Date | Wed, 20 May 2015 11:17:01 +0100 |
| |
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 <aricart@memnix.com> > --- > > 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 Woodhouse Open Source Technology Centre David.Woodhouse@intel.com Intel Corporation [unhandled content-type:application/x-pkcs7-signature] | |