2015-04-30 13:59:03

by David Howells

[permalink] [raw]
Subject: [PATCH] MODSIGN: Change default key details [ver #2]

Change default key details to be more obviously unspecified.

Reported-by: Linus Torvalds <[email protected]>
Signed-off-by: David Howells <[email protected]>
Acked-by: James Morris <[email protected]>
---

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 = [email protected]
+ #O = Unspecified company
+ CN = Build time autogenerated kernel key
+ #emailAddress = [email protected]

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 = [email protected]"
+ @echo >>x509.genkey "#O = Unspecified company"
+ @echo >>x509.genkey "CN = Build time autogenerated kernel key"
+ @echo >>x509.genkey "#emailAddress = [email protected]"
@echo >>x509.genkey
@echo >>x509.genkey "[ myexts ]"
@echo >>x509.genkey "basicConstraints=critical,CA:FALSE"


2015-04-30 14:39:14

by Sedat Dilek

[permalink] [raw]
Subject: Re: [PATCH] MODSIGN: Change default key details [ver #2]

On Thu, Apr 30, 2015 at 3:58 PM, David Howells <[email protected]> wrote:
> Change default key details to be more obviously unspecified.
>
> Reported-by: Linus Torvalds <[email protected]>
> Signed-off-by: David Howells <[email protected]>
> Acked-by: James Morris <[email protected]>
> ---
>
> 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 = [email protected]
> + #O = Unspecified company
> + CN = Build time autogenerated kernel key
> + #emailAddress = [email protected]
>
> 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 = [email protected]"
> + @echo >>x509.genkey "#O = Unspecified company"
> + @echo >>x509.genkey "CN = Build time autogenerated kernel key"
> + @echo >>x509.genkey "#emailAddress = [email protected]"
> @echo >>x509.genkey
> @echo >>x509.genkey "[ myexts ]"
> @echo >>x509.genkey "basicConstraints=critical,CA:FALSE"
>

[ CC Greg ]

Thanks for considering me.

I reported missing generated files in the "module signature" area.

>From [1]...

[ 2.117022] Request for unknown module key 'Magrathea: Glacier
signing key: 009aa341bb673735a51dc34b238a0ca481d68098' err -11
[ 2.117114] mii: module verification failed: signature and/or
required key missing - tainting kernel

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

$ egrep -i 'signature|module_sig' /boot/config-4.0.1-1-iniza-small |
grep ^CONFIG
CONFIG_MODULE_SIG=y
CONFIG_MODULE_SIG_ALL=y
CONFIG_MODULE_SIG_SHA512=y
CONFIG_MODULE_SIG_HASH="sha512"
CONFIG_INTEGRITY_SIGNATURE=y
CONFIG_SIGNATURE=y

For my quick builds of rcN Linux-kernels I normally do not need
signing my modules.
I really do not use it (it's taken from the Ubuntu kernel-settings).

Attached is my simple build-script for generating Debian/Ubuntu kernel
packages via builddeb script.

If you have any ideas/hints please let me know.

- Sedat -

[1] https://lkml.org/lkml/2015/2/9/396


Attachments:
config-4.0.1-1-iniza-small (122.27 kB)
build_linux-upstream.sh (4.40 kB)
Download all attachments

2015-04-30 14:50:58

by David Howells

[permalink] [raw]
Subject: Re: [PATCH] MODSIGN: Change default key details [ver #2]

Sedat Dilek <[email protected]> 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

2015-04-30 17:49:32

by Sedat Dilek

[permalink] [raw]
Subject: Re: [PATCH] MODSIGN: Change default key details [ver #2]

On Thu, Apr 30, 2015 at 4:50 PM, David Howells <[email protected]> wrote:
> Sedat Dilek <[email protected]> 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 -

2015-04-30 18:00:04

by Linus Torvalds

[permalink] [raw]
Subject: Re: [PATCH] MODSIGN: Change default key details [ver #2]

On Thu, Apr 30, 2015 at 10:49 AM, Sedat Dilek <[email protected]> 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

2015-05-01 21:41:52

by Abelardo Ricart III

[permalink] [raw]
Subject: 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 <[email protected]>
---

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

2015-05-02 04:12:51

by Linus Torvalds

[permalink] [raw]
Subject: Re: [PATCH] MODSIGN: Change default key details [ver #2]

On Fri, May 1, 2015 at 2:41 PM, Abelardo Ricart III <[email protected]> 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

2015-05-02 06:57:16

by Sedat Dilek

[permalink] [raw]
Subject: Re: [PATCH] MODSIGN: Change default key details [ver #2]

On Sat, May 2, 2015 at 6:12 AM, Linus Torvalds
<[email protected]> wrote:
> On Fri, May 1, 2015 at 2:41 PM, Abelardo Ricart III <[email protected]> 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

2015-05-02 09:46:27

by Abelardo Ricart III

[permalink] [raw]
Subject: 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 <[email protected]>
---

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

2015-05-04 01:45:44

by Linus Torvalds

[permalink] [raw]
Subject: Re: [PATCH] MODSIGN: Change default key details [ver #2]

On Sat, May 2, 2015 at 2:46 AM, Abelardo Ricart III <[email protected]> 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

2015-05-04 04:42:43

by Abelardo Ricart III

[permalink] [raw]
Subject: 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 <[email protected]>
> 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.

2015-05-04 07:19:08

by Abelardo Ricart III

[permalink] [raw]
Subject: 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" <[email protected]> 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.

2015-05-04 18:45:33

by Linus Torvalds

[permalink] [raw]
Subject: Re: [PATCH] MODSIGN: Change default key details [ver #2]

On Sun, May 3, 2015 at 6:45 PM, Linus Torvalds
<[email protected]> 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


Attachments:
patch.diff (1.01 kB)

2015-05-04 21:40:59

by Abelardo Ricart III

[permalink] [raw]
Subject: 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" <[email protected]> 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."

2015-05-05 16:07:18

by David Howells

[permalink] [raw]
Subject: Re: [PATCH] MODSIGN: Change default key details [ver #2]

Linus Torvalds <[email protected]> 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

2015-05-05 16:07:24

by David Howells

[permalink] [raw]
Subject: Re: [PATCH] MODSIGN: Change default key details [ver #2]

Abelardo Ricart III <[email protected]> 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

2015-05-05 16:07:32

by David Howells

[permalink] [raw]
Subject: Re: [PATCH] MODSIGN: Change default key details [ver #2]

David Howells <[email protected]> wrote:

> > + $(call filechk,x509_list)

Ah... I'd missed that there was a comma, not an underscore there.

David

2015-05-05 16:08:03

by Linus Torvalds

[permalink] [raw]
Subject: Re: [PATCH] MODSIGN: Change default key details [ver #2]

On Tue, May 5, 2015 at 7:33 AM, David Howells <[email protected]> wrote:
> Linus Torvalds <[email protected]> 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)) < $< > [email protected];

(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

2015-05-05 16:10:05

by Michal Marek

[permalink] [raw]
Subject: 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
> <[email protected]> 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

2015-05-05 16:10:28

by David Howells

[permalink] [raw]
Subject: Re: [PATCH] MODSIGN: Change default key details [ver #2]

Linus Torvalds <[email protected]> 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

2015-05-05 16:11:00

by Linus Torvalds

[permalink] [raw]
Subject: Re: [PATCH] MODSIGN: Change default key details [ver #2]

On Tue, May 5, 2015 at 8:22 AM, Michal Marek <[email protected]> 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

2015-05-05 22:44:37

by Abelardo Ricart III

[permalink] [raw]
Subject: Re: [PATCH] MODSIGN: Change default key details [ver #2]

On Tue, 2015-05-05 at 15:34 +0100, David Howells wrote:
> Abelardo Ricart III <[email protected]> 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).

2015-05-06 12:20:23

by Michal Marek

[permalink] [raw]
Subject: 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 <[email protected]> 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

2015-05-07 11:01:25

by David Howells

[permalink] [raw]
Subject: Re: [PATCH] MODSIGN: Change default key details [ver #2]

Michal Marek <[email protected]> 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

2015-05-07 12:15:56

by Michal Marek

[permalink] [raw]
Subject: Re: [PATCH] MODSIGN: Change default key details [ver #2]

On 2015-05-07 13:00, David Howells wrote:
> Michal Marek <[email protected]> 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 <[email protected]>
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 = [email protected]"
- @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 = [email protected]"
+ @echo >>$@
+ @echo >>$@ "[ myexts ]"
+ @echo >>$@ "basicConstraints=critical,CA:FALSE"
+ @echo >>$@ "keyUsage=digitalSignature"
+ @echo >>$@ "subjectKeyIdentifier=hash"
+ @echo >>$@ "authorityKeyIdentifier=keyid"
endif
--
2.1.4

2015-05-07 12:24:49

by Michal Marek

[permalink] [raw]
Subject: 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 <[email protected]>
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 = [email protected]"
- @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 = [email protected]"
+ @echo >>$@
+ @echo >>$@ "[ myexts ]"
+ @echo >>$@ "basicConstraints=critical,CA:FALSE"
+ @echo >>$@ "keyUsage=digitalSignature"
+ @echo >>$@ "subjectKeyIdentifier=hash"
+ @echo >>$@ "authorityKeyIdentifier=keyid"
endif
--
2.1.4

2015-05-08 13:06:24

by David Howells

[permalink] [raw]
Subject: Re: [PATCH] MODSIGN: Change default key details [ver #2]

Michal Marek <[email protected]> wrote:

> + @echo >>$@ "[ req_distinguished_name ]"
> + @echo >>$@ "O = Magrathea"
> + @echo >>$@ "CN = Glacier signing key"
> + @echo >>$@ "emailAddress = [email protected]"

This bit has changed upstream.

David

2015-05-12 08:51:45

by Michal Marek

[permalink] [raw]
Subject: Re: [PATCH] MODSIGN: Change default key details [ver #2]

On 2015-05-08 15:05, David Howells wrote:
> Michal Marek <[email protected]> wrote:
>
>> + @echo >>$@ "[ req_distinguished_name ]"
>> + @echo >>$@ "O = Magrathea"
>> + @echo >>$@ "CN = Glacier signing key"
>> + @echo >>$@ "emailAddress = [email protected]"
>
> 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

2015-05-15 15:22:18

by David Howells

[permalink] [raw]
Subject: Re: [PATCH] MODSIGN: Change default key details [ver #2]

Michal Marek <[email protected]> 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

2015-05-16 15:39:53

by David Woodhouse

[permalink] [raw]
Subject: 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
> <[email protected]> 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 Woodhouse Open Source Technology Centre
[email protected] Intel Corporation


Attachments:
smime.p7s (5.56 kB)

2015-05-18 10:48:09

by David Howells

[permalink] [raw]
Subject: Re: [PATCH] MODSIGN: Change default key details [ver #2]

David Woodhouse <[email protected]> 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

2015-05-18 10:57:25

by David Howells

[permalink] [raw]
Subject: Re: [PATCH] MODSIGN: Change default key details [ver #2]

David Howells <[email protected]> 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

2015-05-18 11:13:26

by David Woodhouse

[permalink] [raw]
Subject: Re: [PATCH] MODSIGN: Change default key details [ver #2]

On Mon, 2015-05-18 at 11:47 +0100, David Howells wrote:
> David Woodhouse <[email protected]> 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 Woodhouse Open Source Technology Centre
[email protected] Intel Corporation


Attachments:
smime.p7s (5.56 kB)

2015-05-18 16:07:46

by David Woodhouse

[permalink] [raw]
Subject: 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 Woodhouse Open Source Technology Centre
[email protected] Intel Corporation¹ http://git.infradead.org/users/dwmw2/modsign-pkcs11-c.git/commitdiff/3d69ae738


Attachments:
smime.p7s (5.56 kB)

2015-05-19 02:15:56

by Mimi Zohar

[permalink] [raw]
Subject: 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 <[email protected]> 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

2015-05-19 14:14:30

by David Howells

[permalink] [raw]
Subject: 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

2015-05-19 15:19:19

by David Woodhouse

[permalink] [raw]
Subject: 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 Woodhouse Open Source Technology Centre
[email protected] Intel Corporation

2015-05-20 10:17:13

by David Woodhouse

[permalink] [raw]
Subject: 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 <[email protected]>
> ---
>
> 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
[email protected] Intel Corporation


Attachments:
smime.p7s (5.56 kB)

2015-05-20 10:51:31

by David Howells

[permalink] [raw]
Subject: Re: [PATCH] MODSIGN: Change default key details [ver #2]

David Woodhouse <[email protected]> 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

2015-05-20 11:08:54

by David Woodhouse

[permalink] [raw]
Subject: 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 Woodhouse Open Source Technology Centre
[email protected] Intel Corporation

2015-05-20 11:26:49

by David Woodhouse

[permalink] [raw]
Subject: [PATCH] modsign: Use single PEM file for autogenerated key

On Wed, 2015-05-20 at 11:17 +0100, David Woodhouse wrote:
> 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.

What's this, [PATCH 14/8] or thereabouts? Added to the tree at
http://git.infradead.org/users/dwmw2/modsign-pkcs11-c.git

>From b6cf6627d068a8752f619594e6dbbf4101a86a7f Mon Sep 17 00:00:00 2001
From: David Woodhouse <[email protected]>
Date: Wed, 20 May 2015 12:14:29 +0100
Subject: [PATCH] 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 <[email protected]>
---
Until now, people could still drop their own key into
signing_key.{priv,x509}. Doing so was now *deprecated*, and it was
documented that they ought to use CONFIG_MODULE_SIG_KEY for that, but it
did still work. Now it doesn't. Do we care? I don't think so.

I am disinclined to put in place any kind of backward-compatibility to
find and use the old filenames. And I don't much care about changing the
build system 'interface' for the user. I'm *very* keen to break that
$(wildcard *.x509) crap anyway, so we might as well do this at the same
time.

.gitignore | 1 +
Documentation/module-signing.txt | 9 ++++-----
Makefile | 4 ++--
init/Kconfig | 4 ++--
kernel/Makefile | 15 +++++++--------
5 files changed, 16 insertions(+), 17 deletions(-)

diff --git a/.gitignore b/.gitignore
index 4ad4a98..17fa24d 100644
--- a/.gitignore
+++ b/.gitignore
@@ -97,6 +97,7 @@ GTAGS
# Leavings from module signing
#
extra_certificates
+signing_key.pem
signing_key.priv
signing_key.x509
x509.genkey
diff --git a/Documentation/module-signing.txt b/Documentation/module-signing.txt
index 6930019..5d5e4e3 100644
--- a/Documentation/module-signing.txt
+++ b/Documentation/module-signing.txt
@@ -91,7 +91,7 @@ This has a number of options available:
(4) "File name or PKCS#11 URI of module signing key" (CONFIG_MODULE_SIG_KEY)

Setting this option to something other than its default of
- "signing_key.priv" will disable the autogeneration of signing keys and
+ "signing_key.pem" will disable the autogeneration of signing keys and
allow the kernel modules to be signed with a key of your choosing.
The string provided should identify a file containing both a private
key and its corresponding X.509 certificate in PEM form, or — on
@@ -116,11 +116,10 @@ kernel so that it can be used to check the signatures as the modules are
loaded.

Under normal conditions, when CONFIG_MODULE_SIG_KEY is unchanged from its
-default of "signing_key.priv", the kernel build will automatically generate
-a new keypair using openssl if one does not exist in the files:
+default, the kernel build will automatically generate a new keypair using
+openssl if one does not exist in the file:

- signing_key.priv
- signing_key.x509
+ signing_key.pem

during the building of vmlinux (the public part of the key needs to be built
into vmlinux) using parameters in the:
diff --git a/Makefile b/Makefile
index 9590e67..ce2f468 100644
--- a/Makefile
+++ b/Makefile
@@ -1175,8 +1175,8 @@ MRPROPER_DIRS += include/config usr/include include/generated \
arch/*/include/generated .tmp_objdiff
MRPROPER_FILES += .config .config.old .version .old_version \
Module.symvers tags TAGS cscope* GPATH GTAGS GRTAGS GSYMS \
- signing_key.priv signing_key.x509 x509.genkey \
- extra_certificates signing_key.x509.keyid \
+ signing_key.pem signing_key.priv signing_key.x509 \
+ x509.genkey extra_certificates signing_key.x509.keyid \
signing_key.x509.signer vmlinux-gdb.py

# clean - Delete most, but leave enough to build external modules
diff --git a/init/Kconfig b/init/Kconfig
index a4b4f39..a529353 100644
--- a/init/Kconfig
+++ b/init/Kconfig
@@ -1956,7 +1956,7 @@ config MODULE_SIG_HASH

config MODULE_SIG_KEY
string "File name or PKCS#11 URI of module signing key"
- default "signing_key.priv"
+ default "signing_key.pem"
depends on MODULE_SIG
help
Provide the file name of a private key/certificate in PEM format,
@@ -1964,7 +1964,7 @@ config MODULE_SIG_KEY
the URI should identify, both the certificate and its corresponding
private key.

- If this option is unchanged from its default "signing_key.priv",
+ If this option is unchanged from its default "signing_key.pem",
then the kernel will automatically generate the private key and
certificate as described in Documentation/module-signing.txt

diff --git a/kernel/Makefile b/kernel/Makefile
index 9fb259d..541e1e8 100644
--- a/kernel/Makefile
+++ b/kernel/Makefile
@@ -173,8 +173,8 @@ endif
# We do it this way rather than having a boolean option for enabling an
# external private key, because 'make randconfig' might enable such a
# boolean option and we unfortunately can't make it depend on !RANDCONFIG.
-ifeq ($(CONFIG_MODULE_SIG_KEY),"signing_key.priv")
-signing_key.priv signing_key.x509: x509.genkey
+ifeq ($(CONFIG_MODULE_SIG_KEY),"signing_key.pem")
+signing_key.pem: x509.genkey
@echo "###"
@echo "### Now generating an X.509 key pair to be used for signing modules."
@echo "###"
@@ -185,8 +185,8 @@ signing_key.priv signing_key.x509: x509.genkey
@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
+ -outform PEM -out signing_key.pem \
+ -keyout signing_key.pem 2>&1
@echo "###"
@echo "### Key pair generated."
@echo "###"
@@ -210,9 +210,9 @@ x509.genkey:
@echo >>x509.genkey "keyUsage=digitalSignature"
@echo >>x509.genkey "subjectKeyIdentifier=hash"
@echo >>x509.genkey "authorityKeyIdentifier=keyid"
-else
-# For external (PKCS#11 or PEM) key, we need to obtain the certificate from
-# CONFIG_MODULE_SIG_KEY automatically.
+endif
+
+# We need to obtain the certificate from CONFIG_MODULE_SIG_KEY.
quiet_cmd_extract_der = CERT_DER $(2)
cmd_extract_der = scripts/extract-cert "$(2)" signing_key.x509

@@ -249,4 +249,3 @@ endif
signing_key.x509: scripts/extract-cert include/config/module/sig/key.h $(X509_DEP)
$(call cmd,extract_der,$(X509_SOURCE))
endif
-endif
--
2.4.1


--
David Woodhouse Open Source Technology Centre
[email protected] Intel Corporation

2015-05-20 14:56:34

by David Howells

[permalink] [raw]
Subject: Re: [PATCH] modsign: Use single PEM file for autogenerated key

David Woodhouse <[email protected]> wrote:

> I am disinclined to put in place any kind of backward-compatibility to
> find and use the old filenames. And I don't much care about changing the
> build system 'interface' for the user. I'm *very* keen to break that
> $(wildcard *.x509) crap anyway, so we might as well do this at the same
> time.

Should some sort of warning then be emitted if $(wildcard *.x509) *does* turn
up anything? Just so that people don't get unexpectedly surprised when their
auxiliary keys are suddenly ignored.

David

2015-05-20 15:18:53

by David Woodhouse

[permalink] [raw]
Subject: Re: [PATCH] modsign: Use single PEM file for autogenerated key

On Wed, 2015-05-20 at 15:56 +0100, David Howells wrote:
> David Woodhouse <[email protected]> wrote:
>
> > I am disinclined to put in place any kind of backward-compatibility to
> > find and use the old filenames. And I don't much care about changing the
> > build system 'interface' for the user. I'm *very* keen to break that
> > $(wildcard *.x509) crap anyway, so we might as well do this at the same
> > time.
>
> Should some sort of warning then be emitted if $(wildcard *.x509) *does* turn
> up anything? Just so that people don't get unexpectedly surprised when their
> auxiliary keys are suddenly ignored.

Would people even notice such a warning?

People who are using long-term keys and inserting them into the kernel
build probably ought to be paying attention and know what they're doing.
There are relatively few of them, and I think that as long as the change
is clearly documented, we should probably be fine.

Besides, *anyone* relying on that horrid wildcard crap should have
*expected* it to go away because it was an abomination :)

--
David Woodhouse Open Source Technology Centre
[email protected] Intel Corporation

2015-05-21 11:31:48

by David Woodhouse

[permalink] [raw]
Subject: Re: [PATCH] modsign: Use single PEM file for autogenerated key

On Wed, 2015-05-20 at 15:56 +0100, David Howells wrote:
> Should some sort of warning then be emitted if $(wildcard *.x509) *does* turn
> up anything? Just so that people don't get unexpectedly surprised when their
> auxiliary keys are suddenly ignored.

I've added a note to the help text for the CONFIG_SYSTEM_TRUSTED_KEYS
option, to warn people that *.x509 will no longer be implicitly trusted.

I suppose if you *really* insist, we could do something like

ifneq ($(filter-out signing_key.x509,$(wildcard *.x509)),)
$(warning ...)
endif

That detail aside, if we have consensus that the patch below is the way
forward, I may reshuffle the previous patches in the tree a little to
include this...

>From eb12ad71ef7f239979ffc3525d41cd3acb354cf3 Mon Sep 17 00:00:00 2001
From: David Woodhouse <[email protected]>
Date: Thu, 21 May 2015 12:23:55 +0100
Subject: [PATCH] modsign: Add explicit CONFIG_SYSTEM_TRUSTED_KEYS option

Let the user explicitly provide a file containing trusted keys, instead of
just automatically finding files matching *.x509 in the build tree and
trusting whatever we find. This really ought to be an *explicit*
configuration, and the build rules for dealing with the files were
fairly painful too.

Signed-off-by: David Woodhouse <[email protected]>
---
Documentation/module-signing.txt | 15 +++--
init/Kconfig | 13 +++++
kernel/Makefile | 123 ++++++++++++++++++++-------------------
3 files changed, 87 insertions(+), 64 deletions(-)

diff --git a/Documentation/module-signing.txt b/Documentation/module-signing.txt
index 5d5e4e3..4e62bc2 100644
--- a/Documentation/module-signing.txt
+++ b/Documentation/module-signing.txt
@@ -88,6 +88,7 @@ This has a number of options available:
than being a module) so that modules signed with that algorithm can have
their signatures checked without causing a dependency loop.

+
(4) "File name or PKCS#11 URI of module signing key" (CONFIG_MODULE_SIG_KEY)

Setting this option to something other than its default of
@@ -104,6 +105,13 @@ This has a number of options available:
means of the KBUILD_SIGN_PIN variable.


+ (5) "Additional X.509 keys for default system keyring" (CONFIG_SYSTEM_TRUSTED_KEYS)
+
+ This option can be set to the filename of a PEM-encoded file containing
+ additional certificates which will be included in the system keyring by
+ default.
+
+
=======================
GENERATING SIGNING KEYS
=======================
@@ -171,10 +179,9 @@ in a keyring called ".system_keyring" that can be seen by:
302d2d52 I------ 1 perm 1f010000 0 0 asymmetri Fedora kernel signing key: d69a84e6bce3d216b979e9505b3e3ef9a7118079: X509.RSA a7118079 []
...

-Beyond the public key generated specifically for module signing, any file
-placed in the kernel source root directory or the kernel build root directory
-whose name is suffixed with ".x509" will be assumed to be an X.509 public key
-and will be added to the keyring.
+Beyond the public key generated specifically for module signing, additional
+trusted certificates can be provided in a PEM-encoded file referenced by the
+CONFIG_SYSTEM_TRUSTED_KEYS configuration option.

Further, the architecture code may take public keys from a hardware store and
add those in also (e.g. from the UEFI key database).
diff --git a/init/Kconfig b/init/Kconfig
index a529353..b46c195 100644
--- a/init/Kconfig
+++ b/init/Kconfig
@@ -1758,6 +1758,19 @@ config SYSTEM_TRUSTED_KEYRING

Keys in this keyring are used by module signature checking.

+config SYSTEM_TRUSTED_KEYS
+ string "Additional X.509 keys for default system keyring"
+ depends on SYSTEM_TRUSTED_KEYRING
+ help
+ If set, this option should be the filename of a PEM-formatted file
+ containing trusted X.509 certificates to be included in the default
+ system keyring. Any certificate used for module signing is implicitly
+ also trusted.
+
+ NOTE: If you previously provided keys for the system keyring in the
+ form of DER-encoded *.x509 files in the top-level build directory,
+ those are no longer used. You will need to set this option instead.
+
config SYSTEM_DATA_VERIFICATION
def_bool n
select SYSTEM_TRUSTED_KEYRING
diff --git a/kernel/Makefile b/kernel/Makefile
index 541e1e8..4b2b1c7 100644
--- a/kernel/Makefile
+++ b/kernel/Makefile
@@ -114,46 +114,73 @@ $(obj)/config_data.h: $(obj)/config_data.gz FORCE

###############################################################################
#
-# Roll all the X.509 certificates that we can find together and pull them into
-# the kernel so that they get loaded into the system trusted keyring during
-# boot.
+# When a Kconfig string contains a filename, it is suitable for
+# passing to shell commands. It is surrounded by double-quotes, and
+# any double-quotes or backslashes within it are escaped by
+# backslashes.
#
-# We look in the source root and the build root for all files whose name ends
-# in ".x509". Unfortunately, this will generate duplicate filenames, so we
-# have make canonicalise the pathnames and then sort them to discard the
-# duplicates.
+# This is no use for dependencies or $(wildcard). We need to strip the
+# surrounding quotes and the escaping from quotes and backslashes, and
+# we *do* need to escape any spaces in the string. So, for example:
+#
+# Usage: $(eval $(call config_filename,FOO))
+#
+# Defines FOO_FILENAME based on the contents of the CONFIG_FOO option,
+# transformed as described above to be suitable for use within the
+# makefile.
+#
+# Also, if the filename is a relative filename and exists in the source
+# tree but not the build tree, define FOO_SRCPREFIX as $(srctree)/ to
+# be prefixed to *both* command invocation and dependencies.
+#
+# Note: We also print the filenames in the quiet_cmd_foo text, and
+# perhaps ought to have a version specially escaped for that purpose.
+# But it's only cosmetic, and $(patsubst "%",%,$(CONFIG_FOO)) is good
+# enough. It'll strip the quotes in the common case where there's no
+# space and it's a simple filename, and it'll retain the quotes when
+# there's a space. There are some esoteric cases in which it'll print
+# the wrong thing, but we don't really care. The actual dependencies
+# and commands *do* get it right, with various combinations of single
+# and double quotes, backslashes and spaces in the filenames.
#
###############################################################################
-ifeq ($(CONFIG_SYSTEM_TRUSTED_KEYRING),y)
-X509_CERTIFICATES-y := $(wildcard *.x509) $(wildcard $(srctree)/*.x509)
-X509_CERTIFICATES-$(CONFIG_MODULE_SIG) += $(objtree)/signing_key.x509
-X509_CERTIFICATES-raw := $(sort $(foreach CERT,$(X509_CERTIFICATES-y), \
- $(or $(realpath $(CERT)),$(CERT))))
-X509_CERTIFICATES := $(subst $(realpath $(objtree))/,,$(X509_CERTIFICATES-raw))
-
-ifeq ($(X509_CERTIFICATES),)
-$(warning *** No X.509 certificates found ***)
+#
+quote := $(firstword " ")
+space :=
+space +=
+space_escape := %%%SPACE%%%
+#
+define config_filename =
+$(1)_FILENAME := $$(subst \\,\,$$(subst \$$(quote),$$(quote),$$(subst $$(space_escape),\$$(space),$$(patsubst "%",%,$$(subst $$(space),$$(space_escape),$$(CONFIG_$(1)))))))
+ifneq ($$(patsubst /%,%,$$(firstword $$($(1)_FILENAME))),$$(firstword $$($(1)_FILENAME)))
+else
+ifeq ($$(wildcard $$($(1)_FILENAME)),)
+ifneq ($$(wildcard $$(srctree)/$$($(1)_FILENAME)),)
+$(1)_SRCPREFIX := $(srctree)/
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
+endef
+#
+###############################################################################
+
+
+ifeq ($(CONFIG_SYSTEM_TRUSTED_KEYRING),y)
+
+$(eval $(call config_filename,SYSTEM_TRUSTED_KEYS))
+
+SIGNING_X509-$(CONFIG_MODULE_SIG) += signing_key.x509

kernel/system_certificates.o: $(obj)/x509_certificate_list

-quiet_cmd_x509certs = CERTS $@
- cmd_x509certs = cat $(X509_CERTIFICATES) /dev/null >$@ $(foreach X509,$(X509_CERTIFICATES),; $(kecho) " - Including cert $(X509)")
+quiet_cmd_x509certs = CERTS $(SIGNING_X509-y) $(patsubst "%",%,$(2))
+ cmd_x509certs = ( cat $(SIGNING_X509-y) /dev/null; \
+ awk '/-----BEGIN CERTIFICATE-----/{flag=1;next}/-----END CERTIFICATE-----/{flag=0}flag' $(2) /dev/null | base64 -d ) > $@ || ( rm $@; exit 1)

targets += $(obj)/x509_certificate_list
-$(obj)/x509_certificate_list: $(X509_CERTIFICATES) $(obj)/.x509.list
- $(call if_changed,x509certs)
+$(obj)/x509_certificate_list: $(SIGNING_X509-y) include/config/system/trusted/keys.h include/config/module/sig.h $(SYSTEM_TRUSTED_KEYS_SRCPREFIX)$(SYSTEM_TRUSTED_KEYS_FILENAME)
+ $(call if_changed,x509certs,$(SYSTEM_TRUSTED_KEYS_SRCPREFIX)$(CONFIG_SYSTEM_TRUSTED_KEYS))

-targets += $(obj)/.x509.list
-$(obj)/.x509.list:
- @echo $(X509_CERTIFICATES) >$@
endif

clean-files := x509_certificate_list .x509.list
@@ -212,40 +239,16 @@ x509.genkey:
@echo >>x509.genkey "authorityKeyIdentifier=keyid"
endif

-# We need to obtain the certificate from CONFIG_MODULE_SIG_KEY.
-quiet_cmd_extract_der = CERT_DER $(2)
- cmd_extract_der = scripts/extract-cert "$(2)" signing_key.x509
+$(eval $(call config_filename,MODULE_SIG_KEY))

-# CONFIG_MODULE_SIG_KEY is either a PKCS#11 URI or a filename. It is
-# surrounded by quotes, and may contain spaces. To strip the quotes
-# with $(patsubst) we need to turn the spaces into something else.
-# And if it's a filename, those spaces need to be escaped as '\ ' in
-# order to use it in dependencies or $(wildcard).
-space :=
-space +=
-space_escape := %%%SPACE%%%
-X509_SOURCE_temp := $(subst $(space),$(space_escape),$(CONFIG_MODULE_SIG_KEY))
-# We need this to check for absolute paths or PKCS#11 URIs.
-X509_SOURCE_ONEWORD := $(patsubst "%",%,$(X509_SOURCE_temp))
-# This is the actual source filename/URI without the quotes
-X509_SOURCE := $(subst $(space_escape),$(space),$(X509_SOURCE_ONEWORD))
-# This\ version\ with\ spaces\ escaped\ for\ $(wildcard)\ and\ dependencies
-X509_SOURCE_ESCAPED := $(subst $(space_escape),\$(space),$(X509_SOURCE_ONEWORD))
-
-ifeq ($(patsubst pkcs11:%,%,$(X509_SOURCE_ONEWORD)),$(X509_SOURCE_ONEWORD))
-# If it's a filename, depend on it.
-X509_DEP := $(X509_SOURCE_ESCAPED)
-ifeq ($(patsubst /%,%,$(X509_SOURCE_ONEWORD)),$(X509_SOURCE_ONEWORD))
-ifeq ($(wildcard $(X509_SOURCE_ESCAPED)),)
-ifneq ($(wildcard $(srctree)/$(X509_SOURCE_ESCAPED)),)
-# Non-absolute filename, found in source tree and not build tree
-X509_SOURCE := $(srctree)/$(X509_SOURCE)
-X509_DEP := $(srctree)/$(X509_SOURCE_ESCAPED)
-endif
-endif
-endif
+# If CONFIG_MODULE_SIG_KEY isn't a PKCS#11 URI, depend on it
+ifeq ($(patsubst pkcs11:%,%,$(firstword $(MODULE_SIG_KEY_FILENAME))),$(firstword $(MODULE_SIG_KEY_FILENAME)))
+X509_DEP := $(MODULE_SIG_KEY_SRCPREFIX)$(MODULE_SIG_KEY_FILENAME)
endif

+quiet_cmd_extract_der = SIGNING_CERT $(patsubst "%",%,$(2))
+ cmd_extract_der = scripts/extract-cert $(2) signing_key.x509
+
signing_key.x509: scripts/extract-cert include/config/module/sig/key.h $(X509_DEP)
- $(call cmd,extract_der,$(X509_SOURCE))
+ $(call cmd,extract_der,$(MODULE_SIG_KEY_SRCPREFIX)$(CONFIG_MODULE_SIG_KEY))
endif
--
2.4.1



--
David Woodhouse Open Source Technology Centre
[email protected] Intel Corporation