diff mbox

[4/9] KEYS: Allow unrestricted boot-time addition of keys to secondary keyring

Message ID 147931987366.16460.12891767069975068260.stgit@warthog.procyon.org.uk (mailing list archive)
State New, archived
Headers show

Commit Message

David Howells Nov. 16, 2016, 6:11 p.m. UTC
Allow keys to be added to the system secondary certificates keyring during
kernel initialisation in an unrestricted fashion.  Such keys are implicitly
trusted and don't have their trust chains checked on link.

This allows keys in the UEFI database to be added in secure boot mode for
the purposes of module signing.

Signed-off-by: David Howells <dhowells@redhat.com>
---

 certs/internal.h       |   18 ++++++++++++++++++
 certs/system_keyring.c |   33 +++++++++++++++++++++++++++++++++
 2 files changed, 51 insertions(+)
 create mode 100644 certs/internal.h


--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Comments

Petko Manolov Nov. 17, 2016, 6:41 a.m. UTC | #1
On 16-11-16 18:11:13, David Howells wrote:
> Allow keys to be added to the system secondary certificates keyring during 
> kernel initialisation in an unrestricted fashion.  Such keys are implicitly 
> trusted and don't have their trust chains checked on link.

Well, I for one do not explicitly trust these keys.  I may even want to 
completely remove or replace them.

> This allows keys in the UEFI database to be added in secure boot mode for the 
> purposes of module signing.

The key import should not be automatic, it should be optional.  Same applies to 
the validation process.


		Petko


> Signed-off-by: David Howells <dhowells@redhat.com>
> ---
> 
>  certs/internal.h       |   18 ++++++++++++++++++
>  certs/system_keyring.c |   33 +++++++++++++++++++++++++++++++++
>  2 files changed, 51 insertions(+)
>  create mode 100644 certs/internal.h
> 
> diff --git a/certs/internal.h b/certs/internal.h
> new file mode 100644
> index 000000000000..5dcbefb0c23a
> --- /dev/null
> +++ b/certs/internal.h
> @@ -0,0 +1,18 @@
> +/* Internal definitions
> + *
> + * Copyright (C) 2016 Red Hat, Inc. All Rights Reserved.
> + * Written by David Howells (dhowells@redhat.com)
> + *
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU General Public Licence
> + * as published by the Free Software Foundation; either version
> + * 2 of the Licence, or (at your option) any later version.
> + */
> +
> +/*
> + * system_keyring.c
> + */
> +#ifdef CONFIG_SECONDARY_TRUSTED_KEYRING
> +extern void __init add_trusted_secondary_key(const char *source,
> +					     const void *data, size_t len);
> +#endif
> diff --git a/certs/system_keyring.c b/certs/system_keyring.c
> index 50979d6dcecd..dfddcf6e6c88 100644
> --- a/certs/system_keyring.c
> +++ b/certs/system_keyring.c
> @@ -17,6 +17,7 @@
>  #include <keys/asymmetric-type.h>
>  #include <keys/system_keyring.h>
>  #include <crypto/pkcs7.h>
> +#include "internal.h"
>  
>  static struct key *builtin_trusted_keys;
>  #ifdef CONFIG_SECONDARY_TRUSTED_KEYRING
> @@ -242,3 +243,35 @@ int verify_pkcs7_signature(const void *data, size_t len,
>  EXPORT_SYMBOL_GPL(verify_pkcs7_signature);
>  
>  #endif /* CONFIG_SYSTEM_DATA_VERIFICATION */
> +
> +#ifdef CONFIG_SECONDARY_TRUSTED_KEYRING
> +/**
> + * add_trusted_secondary_key - Add to secondary keyring with no validation
> + * @source: Source of key
> + * @data: The blob holding the key
> + * @len: The length of the data blob
> + *
> + * Add a key to the secondary keyring without checking its trust chain.  This
> + * is available only during kernel initialisation.
> + */
> +void __init add_trusted_secondary_key(const char *source,
> +				      const void *data, size_t len)
> +{
> +	key_ref_t key;
> +
> +	key = key_create_or_update(make_key_ref(secondary_trusted_keys, 1),
> +				   "asymmetric",
> +				   NULL, data, len,
> +				   (KEY_POS_ALL & ~KEY_POS_SETATTR) |
> +				   KEY_USR_VIEW,
> +				   KEY_ALLOC_NOT_IN_QUOTA |
> +				   KEY_ALLOC_BYPASS_RESTRICTION);
> +
> +	if (IS_ERR(key))
> +		pr_err("Problem loading %s X.509 certificate (%ld)\n",
> +		       source, PTR_ERR(key));
> +	else
> +		pr_notice("Loaded %s cert '%s' linked to secondary sys keyring\n",
> +			  source, key_ref_to_ptr(key)->description);
> +}
> +#endif /* CONFIG_SECONDARY_TRUSTED_KEYRING */
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
David Howells Nov. 17, 2016, 9:56 a.m. UTC | #2
Petko Manolov <petkan@mip-labs.com> wrote:

> On 16-11-16 18:11:13, David Howells wrote:
> > Allow keys to be added to the system secondary certificates keyring during 
> > kernel initialisation in an unrestricted fashion.  Such keys are implicitly 
> > trusted and don't have their trust chains checked on link.
> 
> Well, I for one do not explicitly trust these keys.  I may even want to 
> completely remove or replace them.

Fine be me.  However, if you remove them all I would guess that you cannot
perform a secure boot.

Note that it's to be expected that the keys being loaded from the UEFI
database cannot have their signatures checked - which is why they would have
to be implicitly trusted.  For the same reason, the kernel does not check the
signatures on the keys compiled into the kernel image.

> > This allows keys in the UEFI database to be added in secure boot mode for
> > the purposes of module signing.
> 
> The key import should not be automatic, it should be optional.

You can argue this either way.  There's a config option to allow you to turn
this on or off.  Arguably, this should be split in two: one for the whitelist
(db, MokListRT) and one for the blacklist (dbx).

Further, possibly I should add an option that allows this to be restricted to
secure boot mode only.

> Same applies to the validation process.

Depends what you mean by "the validation process"?  The use of secure boot at
all?  The checking of signatures on keys?  Module signing?

David
--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Petko Manolov Nov. 17, 2016, 10:22 a.m. UTC | #3
On 16-11-17 09:56:00, David Howells wrote:
> Petko Manolov <petkan@mip-labs.com> wrote:
> 
> > On 16-11-16 18:11:13, David Howells wrote:
> > > Allow keys to be added to the system secondary certificates keyring during 
> > > kernel initialisation in an unrestricted fashion.  Such keys are 
> > > implicitly trusted and don't have their trust chains checked on link.
> > 
> > Well, I for one do not explicitly trust these keys.  I may even want to 
> > completely remove or replace them.
> 
> Fine be me.  However, if you remove them all I would guess that you cannot 
> perform a secure boot.

Maybe not on PC, but there's plenty of other architectures out there.  What if i 
replace all UEFI keys with my own?

> Note that it's to be expected that the keys being loaded from the UEFI 
> database cannot have their signatures checked - which is why they would have 
> to be implicitly trusted.  For the same reason, the kernel does not check the 
> signatures on the keys compiled into the kernel image.

I build all kernels that matter to me and i _do_ trust myself.  Unfortunately i 
can't say the same for any corporation out there.

Trusting a key because your vendor shipped the HW with it so that you have no 
way to verify the signature is pretty weak argument IMHO.

However, I am also well aware that most people just don't care. :)

> > > This allows keys in the UEFI database to be added in secure boot mode for 
> > > the purposes of module signing.
> > 
> > The key import should not be automatic, it should be optional.
> 
> You can argue this either way.  There's a config option to allow you to turn 
> this on or off.  Arguably, this should be split in two: one for the whitelist 
> (db, MokListRT) and one for the blacklist (dbx).

I did not see the config option.  There is one?

Right now i can't decide whether whitelist should go along with blacklist or 
there should be separate options.  I guess for whoever goes down this path it 
would make sense to use either both or none of them.

> Further, possibly I should add an option that allows this to be restricted to
> secure boot mode only.

Please do.  It doesn't make much sense otherwise.

> > Same applies to the validation process.
> 
> Depends what you mean by "the validation process"?  The use of secure boot at 
> all?  The checking of signatures on keys?  Module signing?

Nevermind.  The keys signature can't be verified in the classic UEFI case.


		Petko
--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
David Howells Nov. 17, 2016, 11:18 a.m. UTC | #4
Petko Manolov <petkan@mip-labs.com> wrote:

> > > Well, I for one do not explicitly trust these keys.  I may even want to 
> > > completely remove or replace them.
> > 
> > Fine be me.  However, if you remove them all I would guess that you cannot 
> > perform a secure boot.
> 
> Maybe not on PC, but there's plenty of other architectures out there.  What
> if i replace all UEFI keys with my own?

Then I would imagine that you can do a secure boot, but that you have to sign
your own shim, grub, kernel, etc..

> > Note that it's to be expected that the keys being loaded from the UEFI
> > database cannot have their signatures checked - which is why they would
> > have to be implicitly trusted.  For the same reason, the kernel does not
> > check the signatures on the keys compiled into the kernel image.
> 
> I build all kernels that matter to me and i _do_ trust myself.
> Unfortunately i can't say the same for any corporation out there.
> 
> Trusting a key because your vendor shipped the HW with it so that you have no 
> way to verify the signature is pretty weak argument IMHO.

I'm not making an argument there.  There is a reason I think that I can't
check them.  Well, possibly I could *if* those keys are actually signed *and*
I have certs built into the kernel by which I can verify all those keys in
UEFI variables.  I don't know whether this is actually practical.

> > You can argue this either way.  There's a config option to allow you to
> > turn this on or off.  Arguably, this should be split in two: one for the
> > whitelist (db, MokListRT) and one for the blacklist (dbx).
> 
> I did not see the config option.  There is one?

See patch 8 where these variables are actually parsed.  CONFIG_LOAD_UEFI_KEYS
is available there.

David
--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Mimi Zohar Nov. 21, 2016, 2:04 p.m. UTC | #5
On Thu, 2016-11-17 at 09:56 +0000, David Howells wrote:
> Petko Manolov <petkan@mip-labs.com> wrote:
> 
> > On 16-11-16 18:11:13, David Howells wrote:
> > > Allow keys to be added to the system secondary certificates keyring during 
> > > kernel initialisation in an unrestricted fashion.  Such keys are implicitly 
> > > trusted and don't have their trust chains checked on link.
> > 
> > Well, I for one do not explicitly trust these keys.  I may even want to 
> > completely remove or replace them.
> 
> Fine be me.  However, if you remove them all I would guess that you cannot
> perform a secure boot.
> 
> Note that it's to be expected that the keys being loaded from the UEFI
> database cannot have their signatures checked - which is why they would have
> to be implicitly trusted.  For the same reason, the kernel does not check the
> signatures on the keys compiled into the kernel image.

Sigh, we've been here before, discussed this before.  Different keys
should be trusted at different levels.  Nothing has changed.  Just
because I trust a key in UEFI for UEFI, doesn't mean that I trust that
same key once the kernel has booted.

This time not only are you bringing the keys from UEFI up to the kernel,
but by adding these keys to the secondary trusted keyring, they are
allowed to add other keys they've signed to the secondary trusted
keyring.

If the UEFI keys are just for verifying kernel modules, why not define a
separate UEFI keyring, which can be used, if enabled, just for verifying
kernel modules, instead of affecting all signature verification?

IMA's root of trust goes back to UEFI, but transitions to the builtin
kernel keyring and, if enabled,  the secondary keyring on boot.

> > > This allows keys in the UEFI database to be added in secure boot mode for
> > > the purposes of module signing.
> > 
> > The key import should not be automatic, it should be optional.
> 
> You can argue this either way.  There's a config option to allow you to turn
> this on or off.  Arguably, this should be split in two: one for the whitelist
> (db, MokListRT) and one for the blacklist (dbx).

By "config", you're not referring to a Kconfig option, but a UEFI db
option, making it hidden/unknown to someone building a kernel.  If you
really want to add this support, make it clear and easily seen by
defining a "restrict_link_by_builtin_or_uefi" function.

Mimi

--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
David Howells Nov. 21, 2016, 3:17 p.m. UTC | #6
Mimi Zohar <zohar@linux.vnet.ibm.com> wrote:

> > > > This allows keys in the UEFI database to be added in secure boot mode
> > > > for the purposes of module signing.
> > > 
> > > The key import should not be automatic, it should be optional.
> > 
> > You can argue this either way.  There's a config option to allow you to
> > turn this on or off.  Arguably, this should be split in two: one for the
> > whitelist (db, MokListRT) and one for the blacklist (dbx).
> 
> By "config", you're not referring to a Kconfig option, but a UEFI db
> option, making it hidden/unknown to someone building a kernel.  If you
> really want to add this support, make it clear and easily seen by
> defining a "restrict_link_by_builtin_or_uefi" function.

No: by "config" I *am* referring to Kconfig.

David
--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Mimi Zohar Nov. 21, 2016, 4:24 p.m. UTC | #7
On Mon, 2016-11-21 at 15:17 +0000, David Howells wrote:
> Mimi Zohar <zohar@linux.vnet.ibm.com> wrote:
> 
> > > > > This allows keys in the UEFI database to be added in secure boot mode
> > > > > for the purposes of module signing.
> > > > 
> > > > The key import should not be automatic, it should be optional.
> > > 
> > > You can argue this either way.  There's a config option to allow you to
> > > turn this on or off.  Arguably, this should be split in two: one for the
> > > whitelist (db, MokListRT) and one for the blacklist (dbx).
> > 
> > By "config", you're not referring to a Kconfig option, but a UEFI db
> > option, making it hidden/unknown to someone building a kernel.  If you
> > really want to add this support, make it clear and easily seen by
> > defining a "restrict_link_by_builtin_or_uefi" function.
> 
> No: by "config" I *am* referring to Kconfig.

Good,  I found the Kconfig LOAD_UEFI_KEYS option for loading the keys on
the keyring.  Lets say that someone does want to use those keys for
kernel modules, but only for kernel modules, not for any other types of
files (eg. kexec kernel image/initramfs)?

Mimi

--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/certs/internal.h b/certs/internal.h
new file mode 100644
index 000000000000..5dcbefb0c23a
--- /dev/null
+++ b/certs/internal.h
@@ -0,0 +1,18 @@ 
+/* Internal definitions
+ *
+ * Copyright (C) 2016 Red Hat, Inc. All Rights Reserved.
+ * Written by David Howells (dhowells@redhat.com)
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public Licence
+ * as published by the Free Software Foundation; either version
+ * 2 of the Licence, or (at your option) any later version.
+ */
+
+/*
+ * system_keyring.c
+ */
+#ifdef CONFIG_SECONDARY_TRUSTED_KEYRING
+extern void __init add_trusted_secondary_key(const char *source,
+					     const void *data, size_t len);
+#endif
diff --git a/certs/system_keyring.c b/certs/system_keyring.c
index 50979d6dcecd..dfddcf6e6c88 100644
--- a/certs/system_keyring.c
+++ b/certs/system_keyring.c
@@ -17,6 +17,7 @@ 
 #include <keys/asymmetric-type.h>
 #include <keys/system_keyring.h>
 #include <crypto/pkcs7.h>
+#include "internal.h"
 
 static struct key *builtin_trusted_keys;
 #ifdef CONFIG_SECONDARY_TRUSTED_KEYRING
@@ -242,3 +243,35 @@  int verify_pkcs7_signature(const void *data, size_t len,
 EXPORT_SYMBOL_GPL(verify_pkcs7_signature);
 
 #endif /* CONFIG_SYSTEM_DATA_VERIFICATION */
+
+#ifdef CONFIG_SECONDARY_TRUSTED_KEYRING
+/**
+ * add_trusted_secondary_key - Add to secondary keyring with no validation
+ * @source: Source of key
+ * @data: The blob holding the key
+ * @len: The length of the data blob
+ *
+ * Add a key to the secondary keyring without checking its trust chain.  This
+ * is available only during kernel initialisation.
+ */
+void __init add_trusted_secondary_key(const char *source,
+				      const void *data, size_t len)
+{
+	key_ref_t key;
+
+	key = key_create_or_update(make_key_ref(secondary_trusted_keys, 1),
+				   "asymmetric",
+				   NULL, data, len,
+				   (KEY_POS_ALL & ~KEY_POS_SETATTR) |
+				   KEY_USR_VIEW,
+				   KEY_ALLOC_NOT_IN_QUOTA |
+				   KEY_ALLOC_BYPASS_RESTRICTION);
+
+	if (IS_ERR(key))
+		pr_err("Problem loading %s X.509 certificate (%ld)\n",
+		       source, PTR_ERR(key));
+	else
+		pr_notice("Loaded %s cert '%s' linked to secondary sys keyring\n",
+			  source, key_ref_to_ptr(key)->description);
+}
+#endif /* CONFIG_SECONDARY_TRUSTED_KEYRING */