2013-01-17 18:03:53

by David Howells

[permalink] [raw]
Subject: [PATCH 1/3] KEYS: Load *.x509 files into kernel keyring

Load all the files matching the pattern "*.x509" that are to be found in kernel
base source dir and base build dir into the module signing keyring.

The "extra_certificates" file is then redundant.

Signed-off-by: David Howells <[email protected]>
---

kernel/Makefile | 33 +++++++++++++++++++++++++++------
kernel/modsign_certificate.S | 3 +--
2 files changed, 28 insertions(+), 8 deletions(-)


diff --git a/kernel/Makefile b/kernel/Makefile
index 6c072b6..9fe74ff 100644
--- a/kernel/Makefile
+++ b/kernel/Makefile
@@ -134,17 +134,38 @@ $(obj)/timeconst.h: $(src)/timeconst.pl FORCE
$(call if_changed,timeconst)

ifeq ($(CONFIG_MODULE_SIG),y)
+###############################################################################
#
-# Pull the signing certificate and any extra certificates into the kernel
+# Roll all the X.509 certificates that we can find together and pull
+# them into the kernel.
#
+###############################################################################
+X509_CERTIFICATES := $(sort signing_key.x509 $(wildcard *.x509) $(wildcard $(srctree)/*.x509))
+
+ifeq ($(X509_CERTIFICATES),)
+$(warning *** No X.509 certificates found ***)
+endif
+
+ifneq ($(wildcard $(obj)/.x509.list),)
+ifneq ($(shell cat $(obj)/.x509.list),$(X509_CERTIFICATES))
+$(info X.509 certificate list changed)
+$(shell rm $(obj)/.x509.list)
+endif
+endif
+
+kernel/modsign_certificate.o: $(obj)/x509_certificate_list

-quiet_cmd_touch = TOUCH $@
- cmd_touch = touch $@
+quiet_cmd_x509certs = CERTS $@
+ cmd_x509certs = cat $(X509_CERTIFICATES) /dev/null >$@
+targets += $(obj)/x509_certificate_list
+$(obj)/x509_certificate_list: $(X509_CERTIFICATES) $(obj)/.x509.list
+ $(call if_changed,x509certs)

-extra_certificates:
- $(call cmd,touch)
+targets += $(obj)/.x509.list
+$(obj)/.x509.list:
+ @echo $(X509_CERTIFICATES) >$@

-kernel/modsign_certificate.o: signing_key.x509 extra_certificates
+clean-files := x509_certificate_list .x509.list

###############################################################################
#
diff --git a/kernel/modsign_certificate.S b/kernel/modsign_certificate.S
index 246b4c6..0a60203 100644
--- a/kernel/modsign_certificate.S
+++ b/kernel/modsign_certificate.S
@@ -14,6 +14,5 @@
.section ".init.data","aw"

GLOBAL(modsign_certificate_list)
- .incbin "signing_key.x509"
- .incbin "extra_certificates"
+ .incbin "kernel/x509_certificate_list"
GLOBAL(modsign_certificate_list_end)


2013-01-17 18:04:00

by David Howells

[permalink] [raw]
Subject: [PATCH 2/3] KEYS: Separate the kernel signature checking keyring from module signing

Separate the kernel signature checking keyring from module signing so that it
can be used by code other than the module-signing code.

Signed-off-by: David Howells <[email protected]>
---

init/Kconfig | 13 +++++
kernel/Makefile | 17 ++++---
kernel/modsign_certificate.S | 18 -------
kernel/modsign_pubkey.c | 104 ------------------------------------------
kernel/module-internal.h | 2 -
kernel/module_signing.c | 3 +
kernel/system_certificates.S | 18 +++++++
kernel/system_keyring.c | 101 +++++++++++++++++++++++++++++++++++++++++
8 files changed, 145 insertions(+), 131 deletions(-)
delete mode 100644 kernel/modsign_certificate.S
delete mode 100644 kernel/modsign_pubkey.c
create mode 100644 kernel/system_certificates.S
create mode 100644 kernel/system_keyring.c


diff --git a/init/Kconfig b/init/Kconfig
index 7d30240..a5363d2 100644
--- a/init/Kconfig
+++ b/init/Kconfig
@@ -1568,6 +1568,18 @@ config BASE_SMALL
default 0 if BASE_FULL
default 1 if !BASE_FULL

+config SYSTEM_TRUSTED_KEYRING
+ bool "Provide system-wide ring of trusted keys"
+ select KEYS
+ help
+ Provide a system keyring to which trusted keys can be added. Keys in
+ the keyring are considered to be trusted. Keys may be added at will
+ by the kernel from compiled-in data and from hardware key stores, but
+ userspace may only add extra keys if those keys can be verified by
+ keys already in the keyring.
+
+ Keys in this keyring are used by module signature checking.
+
menuconfig MODULES
bool "Enable loadable module support"
help
@@ -1640,6 +1652,7 @@ config MODULE_SRCVERSION_ALL
config MODULE_SIG
bool "Module signature verification"
depends on MODULES
+ select SYSTEM_TRUSTED_KEYS
select KEYS
select CRYPTO
select ASYMMETRIC_KEY_TYPE
diff --git a/kernel/Makefile b/kernel/Makefile
index 9fe74ff..658e250 100644
--- a/kernel/Makefile
+++ b/kernel/Makefile
@@ -53,8 +53,9 @@ obj-$(CONFIG_SMP) += spinlock.o
obj-$(CONFIG_DEBUG_SPINLOCK) += spinlock.o
obj-$(CONFIG_PROVE_LOCKING) += spinlock.o
obj-$(CONFIG_UID16) += uid16.o
+obj-$(CONFIG_SYSTEM_TRUSTED_KEYRING) += system_keyring.o system_certificates.o
obj-$(CONFIG_MODULES) += module.o
-obj-$(CONFIG_MODULE_SIG) += module_signing.o modsign_pubkey.o modsign_certificate.o
+obj-$(CONFIG_MODULE_SIG) += module_signing.o
obj-$(CONFIG_KALLSYMS) += kallsyms.o
obj-$(CONFIG_BSD_PROCESS_ACCT) += acct.o
obj-$(CONFIG_KEXEC) += kexec.o
@@ -133,13 +134,14 @@ targets += timeconst.h
$(obj)/timeconst.h: $(src)/timeconst.pl FORCE
$(call if_changed,timeconst)

-ifeq ($(CONFIG_MODULE_SIG),y)
###############################################################################
#
-# Roll all the X.509 certificates that we can find together and pull
-# them into the kernel.
+# 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.
#
###############################################################################
+ifeq ($(CONFIG_SYSTEM_TRUSTED_KEYRING),y)
X509_CERTIFICATES := $(sort signing_key.x509 $(wildcard *.x509) $(wildcard $(srctree)/*.x509))

ifeq ($(X509_CERTIFICATES),)
@@ -153,10 +155,11 @@ $(shell rm $(obj)/.x509.list)
endif
endif

-kernel/modsign_certificate.o: $(obj)/x509_certificate_list
+kernel/system_certificates.o: $(obj)/x509_certificate_list

quiet_cmd_x509certs = CERTS $@
- cmd_x509certs = cat $(X509_CERTIFICATES) /dev/null >$@
+ cmd_x509certs = cat $(X509_CERTIFICATES) /dev/null >$@ $(foreach X509,$(X509_CERTIFICATES),; echo " - Including cert $(X509)")
+
targets += $(obj)/x509_certificate_list
$(obj)/x509_certificate_list: $(X509_CERTIFICATES) $(obj)/.x509.list
$(call if_changed,x509certs)
@@ -166,7 +169,9 @@ $(obj)/.x509.list:
@echo $(X509_CERTIFICATES) >$@

clean-files := x509_certificate_list .x509.list
+endif

+ifeq ($(CONFIG_MODULE_SIG),y)
###############################################################################
#
# If module signing is requested, say by allyesconfig, but a key has not been
diff --git a/kernel/modsign_certificate.S b/kernel/modsign_certificate.S
deleted file mode 100644
index 0a60203..0000000
--- a/kernel/modsign_certificate.S
+++ /dev/null
@@ -1,18 +0,0 @@
-/* SYMBOL_PREFIX defined on commandline from CONFIG_SYMBOL_PREFIX */
-#ifndef SYMBOL_PREFIX
-#define ASM_SYMBOL(sym) sym
-#else
-#define PASTE2(x,y) x##y
-#define PASTE(x,y) PASTE2(x,y)
-#define ASM_SYMBOL(sym) PASTE(SYMBOL_PREFIX, sym)
-#endif
-
-#define GLOBAL(name) \
- .globl ASM_SYMBOL(name); \
- ASM_SYMBOL(name):
-
- .section ".init.data","aw"
-
-GLOBAL(modsign_certificate_list)
- .incbin "kernel/x509_certificate_list"
-GLOBAL(modsign_certificate_list_end)
diff --git a/kernel/modsign_pubkey.c b/kernel/modsign_pubkey.c
deleted file mode 100644
index 2b6e699..0000000
--- a/kernel/modsign_pubkey.c
+++ /dev/null
@@ -1,104 +0,0 @@
-/* Public keys for module signature verification
- *
- * Copyright (C) 2012 Red Hat, Inc. All Rights Reserved.
- * Written by David Howells ([email protected])
- *
- * 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.
- */
-
-#include <linux/kernel.h>
-#include <linux/sched.h>
-#include <linux/cred.h>
-#include <linux/err.h>
-#include <keys/asymmetric-type.h>
-#include "module-internal.h"
-
-struct key *modsign_keyring;
-
-extern __initdata const u8 modsign_certificate_list[];
-extern __initdata const u8 modsign_certificate_list_end[];
-
-/*
- * We need to make sure ccache doesn't cache the .o file as it doesn't notice
- * if modsign.pub changes.
- */
-static __initdata const char annoy_ccache[] = __TIME__ "foo";
-
-/*
- * Load the compiled-in keys
- */
-static __init int module_verify_init(void)
-{
- pr_notice("Initialise module verification\n");
-
- modsign_keyring = keyring_alloc(".module_sign",
- KUIDT_INIT(0), KGIDT_INIT(0),
- current_cred(),
- ((KEY_POS_ALL & ~KEY_POS_SETATTR) |
- KEY_USR_VIEW | KEY_USR_READ),
- KEY_ALLOC_NOT_IN_QUOTA, NULL);
- if (IS_ERR(modsign_keyring))
- panic("Can't allocate module signing keyring\n");
-
- return 0;
-}
-
-/*
- * Must be initialised before we try and load the keys into the keyring.
- */
-device_initcall(module_verify_init);
-
-/*
- * Load the compiled-in keys
- */
-static __init int load_module_signing_keys(void)
-{
- key_ref_t key;
- const u8 *p, *end;
- size_t plen;
-
- pr_notice("Loading module verification certificates\n");
-
- end = modsign_certificate_list_end;
- p = modsign_certificate_list;
- while (p < end) {
- /* Each cert begins with an ASN.1 SEQUENCE tag and must be more
- * than 256 bytes in size.
- */
- if (end - p < 4)
- goto dodgy_cert;
- if (p[0] != 0x30 &&
- p[1] != 0x82)
- goto dodgy_cert;
- plen = (p[2] << 8) | p[3];
- plen += 4;
- if (plen > end - p)
- goto dodgy_cert;
-
- key = key_create_or_update(make_key_ref(modsign_keyring, 1),
- "asymmetric",
- NULL,
- p,
- plen,
- (KEY_POS_ALL & ~KEY_POS_SETATTR) |
- KEY_USR_VIEW,
- KEY_ALLOC_NOT_IN_QUOTA);
- if (IS_ERR(key))
- pr_err("MODSIGN: Problem loading in-kernel X.509 certificate (%ld)\n",
- PTR_ERR(key));
- else
- pr_notice("MODSIGN: Loaded cert '%s'\n",
- key_ref_to_ptr(key)->description);
- p += plen;
- }
-
- return 0;
-
-dodgy_cert:
- pr_err("MODSIGN: Problem parsing in-kernel X.509 certificate list\n");
- return 0;
-}
-late_initcall(load_module_signing_keys);
diff --git a/kernel/module-internal.h b/kernel/module-internal.h
index 24f9247..915e123 100644
--- a/kernel/module-internal.h
+++ b/kernel/module-internal.h
@@ -9,6 +9,4 @@
* 2 of the Licence, or (at your option) any later version.
*/

-extern struct key *modsign_keyring;
-
extern int mod_verify_sig(const void *mod, unsigned long *_modlen);
diff --git a/kernel/module_signing.c b/kernel/module_signing.c
index f2970bd..0034e36 100644
--- a/kernel/module_signing.c
+++ b/kernel/module_signing.c
@@ -14,6 +14,7 @@
#include <crypto/public_key.h>
#include <crypto/hash.h>
#include <keys/asymmetric-type.h>
+#include <keys/system_keyring.h>
#include "module-internal.h"

/*
@@ -157,7 +158,7 @@ static struct key *request_asymmetric_key(const char *signer, size_t signer_len,

pr_debug("Look up: \"%s\"\n", id);

- key = keyring_search(make_key_ref(modsign_keyring, 1),
+ key = keyring_search(make_key_ref(system_trusted_keyring, 1),
&key_type_asymmetric, id);
if (IS_ERR(key))
pr_warn("Request for unknown module key '%s' err %ld\n",
diff --git a/kernel/system_certificates.S b/kernel/system_certificates.S
new file mode 100644
index 0000000..86240df
--- /dev/null
+++ b/kernel/system_certificates.S
@@ -0,0 +1,18 @@
+/* SYMBOL_PREFIX defined on commandline from CONFIG_SYMBOL_PREFIX */
+#ifndef SYMBOL_PREFIX
+#define ASM_SYMBOL(sym) sym
+#else
+#define PASTE2(x,y) x##y
+#define PASTE(x,y) PASTE2(x,y)
+#define ASM_SYMBOL(sym) PASTE(SYMBOL_PREFIX, sym)
+#endif
+
+#define GLOBAL(name) \
+ .globl ASM_SYMBOL(name); \
+ ASM_SYMBOL(name):
+
+ .section ".init.data","aw"
+
+GLOBAL(system_certificate_list)
+ .incbin "kernel/x509_certificate_list"
+GLOBAL(system_certificate_list_end)
diff --git a/kernel/system_keyring.c b/kernel/system_keyring.c
new file mode 100644
index 0000000..a3ca76f
--- /dev/null
+++ b/kernel/system_keyring.c
@@ -0,0 +1,101 @@
+/* System trusted keyring for trusted public keys
+ *
+ * Copyright (C) 2012 Red Hat, Inc. All Rights Reserved.
+ * Written by David Howells ([email protected])
+ *
+ * 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.
+ */
+
+#include <linux/export.h>
+#include <linux/kernel.h>
+#include <linux/sched.h>
+#include <linux/cred.h>
+#include <linux/err.h>
+#include <keys/asymmetric-type.h>
+#include <keys/system_keyring.h>
+#include "module-internal.h"
+
+struct key *system_trusted_keyring;
+EXPORT_SYMBOL_GPL(system_trusted_keyring);
+
+extern __initdata const u8 system_certificate_list[];
+extern __initdata const u8 system_certificate_list_end[];
+
+/*
+ * Load the compiled-in keys
+ */
+static __init int system_trusted_keyring_init(void)
+{
+ pr_notice("Initialise system trusted keyring\n");
+
+ system_trusted_keyring =
+ keyring_alloc(".system_keyring",
+ KUIDT_INIT(0), KGIDT_INIT(0), current_cred(),
+ ((KEY_POS_ALL & ~KEY_POS_SETATTR) |
+ KEY_USR_VIEW | KEY_USR_READ),
+ KEY_ALLOC_NOT_IN_QUOTA, NULL);
+ if (IS_ERR(system_trusted_keyring))
+ panic("Can't allocate system trusted keyring\n");
+
+ return 0;
+}
+
+/*
+ * Must be initialised before we try and load the keys into the keyring.
+ */
+device_initcall(system_trusted_keyring_init);
+
+/*
+ * Load the compiled-in list of X.509 certificates.
+ */
+static __init int load_system_certificate_list(void)
+{
+ key_ref_t key;
+ const u8 *p, *end;
+ size_t plen;
+
+ pr_notice("Loading compiled-in X.509 certificates\n");
+
+ end = system_certificate_list_end;
+ p = system_certificate_list;
+ while (p < end) {
+ /* Each cert begins with an ASN.1 SEQUENCE tag and must be more
+ * than 256 bytes in size.
+ */
+ if (end - p < 4)
+ goto dodgy_cert;
+ if (p[0] != 0x30 &&
+ p[1] != 0x82)
+ goto dodgy_cert;
+ plen = (p[2] << 8) | p[3];
+ plen += 4;
+ if (plen > end - p)
+ goto dodgy_cert;
+
+ key = key_create_or_update(make_key_ref(system_trusted_keyring, 1),
+ "asymmetric",
+ NULL,
+ p,
+ plen,
+ (KEY_POS_ALL & ~KEY_POS_SETATTR) |
+ KEY_USR_VIEW,
+ KEY_ALLOC_NOT_IN_QUOTA);
+ if (IS_ERR(key))
+ pr_err("Problem loading in-kernel X.509 certificate (%ld)\n",
+ PTR_ERR(key));
+ else
+ pr_notice("Loaded X.509 cert '%s'\n",
+ key_ref_to_ptr(key)->description);
+ p += plen;
+ }
+
+ return 0;
+
+dodgy_cert:
+ pr_err("Problem parsing in-kernel X.509 certificate list\n");
+ return 0;
+}
+late_initcall(load_system_certificate_list);

2013-01-17 18:04:07

by David Howells

[permalink] [raw]
Subject: [PATCH 3/3] KEYS: Add a 'trusted' flag and a 'trusted only' flag

Add KEY_FLAG_TRUSTED to indicate that a key either comes from a trusted source
or had a cryptographic signature chain that led back to a trusted key the
kernel already possessed.

Add KEY_FLAGS_TRUSTED_ONLY to indicate that a keyring will only accept links to
keys marked with KEY_FLAGS_TRUSTED.

Signed-off-by: David Howells <[email protected]>
Reviewed-by: Kees Cook <[email protected]>
---

include/linux/key-type.h | 1 +
include/linux/key.h | 3 +++
kernel/system_keyring.c | 4 +++-
security/keys/key.c | 8 ++++++++
security/keys/keyring.c | 4 ++++
5 files changed, 19 insertions(+), 1 deletion(-)


diff --git a/include/linux/key-type.h b/include/linux/key-type.h
index 518a53a..f942b2d 100644
--- a/include/linux/key-type.h
+++ b/include/linux/key-type.h
@@ -45,6 +45,7 @@ struct key_preparsed_payload {
const void *data; /* Raw data */
size_t datalen; /* Raw datalen */
size_t quotalen; /* Quota length for proposed payload */
+ bool trusted; /* True if key is trusted */
};

typedef int (*request_key_actor_t)(struct key_construction *key,
diff --git a/include/linux/key.h b/include/linux/key.h
index 4dfde11..0b32a09 100644
--- a/include/linux/key.h
+++ b/include/linux/key.h
@@ -162,6 +162,8 @@ struct key {
#define KEY_FLAG_NEGATIVE 5 /* set if key is negative */
#define KEY_FLAG_ROOT_CAN_CLEAR 6 /* set if key can be cleared by root without permission */
#define KEY_FLAG_INVALIDATED 7 /* set if key has been invalidated */
+#define KEY_FLAG_TRUSTED 8 /* set if key is trusted */
+#define KEY_FLAG_TRUSTED_ONLY 9 /* set if keyring only accepts links to trusted keys */

/* the description string
* - this is used to match a key against search criteria
@@ -203,6 +205,7 @@ extern struct key *key_alloc(struct key_type *type,
#define KEY_ALLOC_IN_QUOTA 0x0000 /* add to quota, reject if would overrun */
#define KEY_ALLOC_QUOTA_OVERRUN 0x0001 /* add to quota, permit even if overrun */
#define KEY_ALLOC_NOT_IN_QUOTA 0x0002 /* not in quota */
+#define KEY_ALLOC_TRUSTED 0x0004 /* Key should be flagged as trusted */

extern void key_revoke(struct key *key);
extern void key_invalidate(struct key *key);
diff --git a/kernel/system_keyring.c b/kernel/system_keyring.c
index a3ca76f..dae8778 100644
--- a/kernel/system_keyring.c
+++ b/kernel/system_keyring.c
@@ -40,6 +40,7 @@ static __init int system_trusted_keyring_init(void)
if (IS_ERR(system_trusted_keyring))
panic("Can't allocate system trusted keyring\n");

+ set_bit(KEY_FLAG_TRUSTED_ONLY, &system_trusted_keyring->flags);
return 0;
}

@@ -82,7 +83,8 @@ static __init int load_system_certificate_list(void)
plen,
(KEY_POS_ALL & ~KEY_POS_SETATTR) |
KEY_USR_VIEW,
- KEY_ALLOC_NOT_IN_QUOTA);
+ KEY_ALLOC_NOT_IN_QUOTA |
+ KEY_ALLOC_TRUSTED);
if (IS_ERR(key))
pr_err("Problem loading in-kernel X.509 certificate (%ld)\n",
PTR_ERR(key));
diff --git a/security/keys/key.c b/security/keys/key.c
index 8fb7c7b..f3de9e4 100644
--- a/security/keys/key.c
+++ b/security/keys/key.c
@@ -299,6 +299,8 @@ struct key *key_alloc(struct key_type *type, const char *desc,

if (!(flags & KEY_ALLOC_NOT_IN_QUOTA))
key->flags |= 1 << KEY_FLAG_IN_QUOTA;
+ if (flags & KEY_ALLOC_TRUSTED)
+ key->flags |= 1 << KEY_FLAG_TRUSTED;

memset(&key->type_data, 0, sizeof(key->type_data));

@@ -813,6 +815,7 @@ key_ref_t key_create_or_update(key_ref_t keyring_ref,
prep.data = payload;
prep.datalen = plen;
prep.quotalen = ktype->def_datalen;
+ prep.trusted = flags & KEY_ALLOC_TRUSTED;
if (ktype->preparse) {
ret = ktype->preparse(&prep);
if (ret < 0) {
@@ -826,6 +829,11 @@ key_ref_t key_create_or_update(key_ref_t keyring_ref,
goto error_free_prep;
}

+ key_ref = ERR_PTR(-EPERM);
+ if (!prep.trusted && test_bit(KEY_FLAG_TRUSTED_ONLY, &keyring->flags))
+ goto error_free_prep;
+ flags |= prep.trusted ? KEY_ALLOC_TRUSTED : 0;
+
ret = __key_link_begin(keyring, ktype, description, &prealloc);
if (ret < 0) {
key_ref = ERR_PTR(ret);
diff --git a/security/keys/keyring.c b/security/keys/keyring.c
index 6ece7f2..f18d7ff 100644
--- a/security/keys/keyring.c
+++ b/security/keys/keyring.c
@@ -1006,6 +1006,10 @@ int key_link(struct key *keyring, struct key *key)
key_check(keyring);
key_check(key);

+ if (test_bit(KEY_FLAG_TRUSTED_ONLY, &keyring->flags) &&
+ !test_bit(KEY_FLAG_TRUSTED, &key->flags))
+ return -EPERM;
+
ret = __key_link_begin(keyring, key->type, key->description, &prealloc);
if (ret == 0) {
ret = __key_link_check_live_key(keyring, key);


2013-01-17 18:44:37

by Mimi Zohar

[permalink] [raw]
Subject: Re: [PATCH 1/3] KEYS: Load *.x509 files into kernel keyring

On Thu, 2013-01-17 at 18:03 +0000, David Howells wrote:
> Load all the files matching the pattern "*.x509" that are to be found in kernel
> base source dir and base build dir into the module signing keyring.

Do we really want certificates cluttering up the base source tree? Any
reason not to define an x509 directory?

> The "extra_certificates" file is then redundant.

Ok.

Mimi

2013-01-17 18:57:39

by Mimi Zohar

[permalink] [raw]
Subject: Re: [PATCH 2/3] KEYS: Separate the kernel signature checking keyring from module signing

On Thu, 2013-01-17 at 18:04 +0000, David Howells wrote:
> Separate the kernel signature checking keyring from module signing so that it
> can be used by code other than the module-signing code.
>
> Signed-off-by: David Howells <[email protected]>

Sounds good, but comment below...

> ---
>
> init/Kconfig | 13 +++++
> kernel/Makefile | 17 ++++---
> kernel/modsign_certificate.S | 18 -------
> kernel/modsign_pubkey.c | 104 ------------------------------------------
> kernel/module-internal.h | 2 -
> kernel/module_signing.c | 3 +
> kernel/system_certificates.S | 18 +++++++
> kernel/system_keyring.c | 101 +++++++++++++++++++++++++++++++++++++++++
> 8 files changed, 145 insertions(+), 131 deletions(-)
> delete mode 100644 kernel/modsign_certificate.S
> delete mode 100644 kernel/modsign_pubkey.c
> create mode 100644 kernel/system_certificates.S
> create mode 100644 kernel/system_keyring.c
>
>
> diff --git a/init/Kconfig b/init/Kconfig
> index 7d30240..a5363d2 100644
> --- a/init/Kconfig
> +++ b/init/Kconfig
> @@ -1568,6 +1568,18 @@ config BASE_SMALL
> default 0 if BASE_FULL
> default 1 if !BASE_FULL
>
> +config SYSTEM_TRUSTED_KEYRING
> + bool "Provide system-wide ring of trusted keys"
> + select KEYS
> + help
> + Provide a system keyring to which trusted keys can be added. Keys in
> + the keyring are considered to be trusted. Keys may be added at will
> + by the kernel from compiled-in data and from hardware key stores, but
> + userspace may only add extra keys if those keys can be verified by
> + keys already in the keyring.
> +

Lets assume accepting built in keys should is acceptable for all use
cases. Adding additional keys from userspace is probably not acceptable
for all use cases. Those keys should be added to specific 'trusted'
keyrings.

EVM and IMA-appraisal have separate keyrings for this reason. I might
be interested in allowing third party packages to be installed and
executed, but that doesn't imply that a security.evm extended attribute,
signed by a third party application, is acceptable.

thanks,

Mimi

2013-01-17 21:20:28

by David Howells

[permalink] [raw]
Subject: Re: [PATCH 2/3] KEYS: Separate the kernel signature checking keyring from module signing

Mimi Zohar <[email protected]> wrote:

> Lets assume accepting built in keys should is acceptable for all use
> cases. Adding additional keys from userspace is probably not acceptable
> for all use cases. Those keys should be added to specific 'trusted'
> keyrings.
>
> EVM and IMA-appraisal have separate keyrings for this reason. I might
> be interested in allowing third party packages to be installed and
> executed, but that doesn't imply that a security.evm extended attribute,
> signed by a third party application, is acceptable.

We should probably look at using the capability of X.509 certificates to
indicate what a key may be used for and noting that in the public_key struct.

David

2013-01-30 08:29:35

by Kasatkin, Dmitry

[permalink] [raw]
Subject: Re: [PATCH 3/3] KEYS: Add a 'trusted' flag and a 'trusted only' flag

On Thu, Jan 17, 2013 at 8:04 PM, David Howells <[email protected]> wrote:
> Add KEY_FLAG_TRUSTED to indicate that a key either comes from a trusted source
> or had a cryptographic signature chain that led back to a trusted key the
> kernel already possessed.
>
> Add KEY_FLAGS_TRUSTED_ONLY to indicate that a keyring will only accept links to
> keys marked with KEY_FLAGS_TRUSTED.
>
> Signed-off-by: David Howells <[email protected]>
> Reviewed-by: Kees Cook <[email protected]>
> ---
>
> include/linux/key-type.h | 1 +
> include/linux/key.h | 3 +++
> kernel/system_keyring.c | 4 +++-
> security/keys/key.c | 8 ++++++++
> security/keys/keyring.c | 4 ++++
> 5 files changed, 19 insertions(+), 1 deletion(-)
>
>
> diff --git a/include/linux/key-type.h b/include/linux/key-type.h
> index 518a53a..f942b2d 100644
> --- a/include/linux/key-type.h
> +++ b/include/linux/key-type.h
> @@ -45,6 +45,7 @@ struct key_preparsed_payload {
> const void *data; /* Raw data */
> size_t datalen; /* Raw datalen */
> size_t quotalen; /* Quota length for proposed payload */
> + bool trusted; /* True if key is trusted */
> };
>
> typedef int (*request_key_actor_t)(struct key_construction *key,
> diff --git a/include/linux/key.h b/include/linux/key.h
> index 4dfde11..0b32a09 100644
> --- a/include/linux/key.h
> +++ b/include/linux/key.h
> @@ -162,6 +162,8 @@ struct key {
> #define KEY_FLAG_NEGATIVE 5 /* set if key is negative */
> #define KEY_FLAG_ROOT_CAN_CLEAR 6 /* set if key can be cleared by root without permission */
> #define KEY_FLAG_INVALIDATED 7 /* set if key has been invalidated */
> +#define KEY_FLAG_TRUSTED 8 /* set if key is trusted */
> +#define KEY_FLAG_TRUSTED_ONLY 9 /* set if keyring only accepts links to trusted keys */
>
> /* the description string
> * - this is used to match a key against search criteria
> @@ -203,6 +205,7 @@ extern struct key *key_alloc(struct key_type *type,
> #define KEY_ALLOC_IN_QUOTA 0x0000 /* add to quota, reject if would overrun */
> #define KEY_ALLOC_QUOTA_OVERRUN 0x0001 /* add to quota, permit even if overrun */
> #define KEY_ALLOC_NOT_IN_QUOTA 0x0002 /* not in quota */
> +#define KEY_ALLOC_TRUSTED 0x0004 /* Key should be flagged as trusted */
>
> extern void key_revoke(struct key *key);
> extern void key_invalidate(struct key *key);
> diff --git a/kernel/system_keyring.c b/kernel/system_keyring.c
> index a3ca76f..dae8778 100644
> --- a/kernel/system_keyring.c
> +++ b/kernel/system_keyring.c
> @@ -40,6 +40,7 @@ static __init int system_trusted_keyring_init(void)
> if (IS_ERR(system_trusted_keyring))
> panic("Can't allocate system trusted keyring\n");
>
> + set_bit(KEY_FLAG_TRUSTED_ONLY, &system_trusted_keyring->flags);
> return 0;
> }
>
> @@ -82,7 +83,8 @@ static __init int load_system_certificate_list(void)
> plen,
> (KEY_POS_ALL & ~KEY_POS_SETATTR) |
> KEY_USR_VIEW,
> - KEY_ALLOC_NOT_IN_QUOTA);
> + KEY_ALLOC_NOT_IN_QUOTA |
> + KEY_ALLOC_TRUSTED);
> if (IS_ERR(key))
> pr_err("Problem loading in-kernel X.509 certificate (%ld)\n",
> PTR_ERR(key));
> diff --git a/security/keys/key.c b/security/keys/key.c
> index 8fb7c7b..f3de9e4 100644
> --- a/security/keys/key.c
> +++ b/security/keys/key.c
> @@ -299,6 +299,8 @@ struct key *key_alloc(struct key_type *type, const char *desc,
>
> if (!(flags & KEY_ALLOC_NOT_IN_QUOTA))
> key->flags |= 1 << KEY_FLAG_IN_QUOTA;
> + if (flags & KEY_ALLOC_TRUSTED)
> + key->flags |= 1 << KEY_FLAG_TRUSTED;
>
> memset(&key->type_data, 0, sizeof(key->type_data));
>
> @@ -813,6 +815,7 @@ key_ref_t key_create_or_update(key_ref_t keyring_ref,
> prep.data = payload;
> prep.datalen = plen;
> prep.quotalen = ktype->def_datalen;
> + prep.trusted = flags & KEY_ALLOC_TRUSTED;
> if (ktype->preparse) {
> ret = ktype->preparse(&prep);
> if (ret < 0) {
> @@ -826,6 +829,11 @@ key_ref_t key_create_or_update(key_ref_t keyring_ref,
> goto error_free_prep;
> }
>
> + key_ref = ERR_PTR(-EPERM);
> + if (!prep.trusted && test_bit(KEY_FLAG_TRUSTED_ONLY, &keyring->flags))
> + goto error_free_prep;
> + flags |= prep.trusted ? KEY_ALLOC_TRUSTED : 0;
> +
> ret = __key_link_begin(keyring, ktype, description, &prealloc);
> if (ret < 0) {
> key_ref = ERR_PTR(ret);
> diff --git a/security/keys/keyring.c b/security/keys/keyring.c
> index 6ece7f2..f18d7ff 100644
> --- a/security/keys/keyring.c
> +++ b/security/keys/keyring.c
> @@ -1006,6 +1006,10 @@ int key_link(struct key *keyring, struct key *key)
> key_check(keyring);
> key_check(key);
>
> + if (test_bit(KEY_FLAG_TRUSTED_ONLY, &keyring->flags) &&
> + !test_bit(KEY_FLAG_TRUSTED, &key->flags))
> + return -EPERM;
> +
> ret = __key_link_begin(keyring, key->type, key->description, &prealloc);
> if (ret == 0) {
> ret = __key_link_check_live_key(keyring, key);
>

Hello,

What about the case when running from integrity protected initramfs?
Either embedded into the signed kernel, or verified by the boot loader.
In such case it is possible to assume that all keys which are added by
user space are implicitly trusted.
Later on, before continuing booting normal rootfs, set the key
subsystem state (trust-lock),
so that trusted keyrings accept only explicitly trusted keys...

Does it make sense?

- Dmitry

2013-01-30 10:33:03

by David Howells

[permalink] [raw]
Subject: Re: [PATCH 3/3] KEYS: Add a 'trusted' flag and a 'trusted only' flag

Kasatkin, Dmitry <[email protected]> wrote:

> What about the case when running from integrity protected initramfs?
> Either embedded into the signed kernel, or verified by the boot loader.
> In such case it is possible to assume that all keys which are added by
> user space are implicitly trusted.
> Later on, before continuing booting normal rootfs, set the key
> subsystem state (trust-lock),
> so that trusted keyrings accept only explicitly trusted keys...
>
> Does it make sense?

I'm not sure it does. Initramfs is (re-)fabricated on the machine on which it
runs any time you update one of a set of rpms (such as the kernel rpm) because
it has machine-specific data and drivers in it.

David

2013-02-06 22:18:23

by Kasatkin, Dmitry

[permalink] [raw]
Subject: Re: [PATCH 3/3] KEYS: Add a 'trusted' flag and a 'trusted only' flag

On Wed, Jan 30, 2013 at 12:32 PM, David Howells <[email protected]> wrote:
> Kasatkin, Dmitry <[email protected]> wrote:
>
>> What about the case when running from integrity protected initramfs?
>> Either embedded into the signed kernel, or verified by the boot loader.
>> In such case it is possible to assume that all keys which are added by
>> user space are implicitly trusted.
>> Later on, before continuing booting normal rootfs, set the key
>> subsystem state (trust-lock),
>> so that trusted keyrings accept only explicitly trusted keys...
>>
>> Does it make sense?
>
> I'm not sure it does. Initramfs is (re-)fabricated on the machine on which it
> runs any time you update one of a set of rpms (such as the kernel rpm) because
> it has machine-specific data and drivers in it.
>

Based on my latest post on signed initramfs it might make sense.
But it seems to be that it would be behavior anyway, because "first"
key added is implicitly should be assumed trusted.

- Dmitry

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