2021-02-11 20:00:58

by Nayna Jain

[permalink] [raw]
Subject: [PATCH 5/5] ima: enable loading of build time generated key to .ima keyring

The kernel currently only loads the kernel module signing key onto
the builtin trusted keyring. To support IMA, load the module signing
key selectively either onto builtin or ima keyring based on MODULE_SIG
or MODULE_APPRAISE_MODSIG config respectively; and loads the CA kernel
key onto builtin trusted keyring.

Signed-off-by: Nayna Jain <[email protected]>
---
certs/system_keyring.c | 56 +++++++++++++++++++++++++++--------
include/keys/system_keyring.h | 9 +++++-
security/integrity/digsig.c | 4 +++
3 files changed, 55 insertions(+), 14 deletions(-)

diff --git a/certs/system_keyring.c b/certs/system_keyring.c
index 798291177186..0bbbe501f8a7 100644
--- a/certs/system_keyring.c
+++ b/certs/system_keyring.c
@@ -26,6 +26,7 @@ static struct key *platform_trusted_keys;

extern __initconst const u8 system_certificate_list[];
extern __initconst const unsigned long system_certificate_list_size;
+extern __initconst const unsigned long module_cert_size;

/**
* restrict_link_to_builtin_trusted - Restrict keyring addition by built in CA
@@ -131,19 +132,12 @@ static __init int system_trusted_keyring_init(void)
*/
device_initcall(system_trusted_keyring_init);

-/*
- * Load the compiled-in list of X.509 certificates.
- */
-static __init int load_system_certificate_list(void)
+static __init int load_cert(const u8 *p, const u8 *end, struct key *keyring,
+ unsigned long flags)
{
key_ref_t key;
- const u8 *p, *end;
size_t plen;

- pr_notice("Loading compiled-in X.509 certificates\n");
-
- p = system_certificate_list;
- end = p + system_certificate_list_size;
while (p < end) {
/* Each cert begins with an ASN.1 SEQUENCE tag and must be more
* than 256 bytes in size.
@@ -158,16 +152,15 @@ static __init int load_system_certificate_list(void)
if (plen > end - p)
goto dodgy_cert;

- key = key_create_or_update(make_key_ref(builtin_trusted_keys, 1),
+ key = key_create_or_update(make_key_ref(keyring, 1),
"asymmetric",
NULL,
p,
plen,
((KEY_POS_ALL & ~KEY_POS_SETATTR) |
KEY_USR_VIEW | KEY_USR_READ),
- KEY_ALLOC_NOT_IN_QUOTA |
- KEY_ALLOC_BUILT_IN |
- KEY_ALLOC_BYPASS_RESTRICTION);
+ flags);
+
if (IS_ERR(key)) {
pr_err("Problem loading in-kernel X.509 certificate (%ld)\n",
PTR_ERR(key));
@@ -185,6 +178,43 @@ static __init int load_system_certificate_list(void)
pr_err("Problem parsing in-kernel X.509 certificate list\n");
return 0;
}
+
+__init int load_module_cert(struct key *keyring, unsigned long flags)
+{
+ const u8 *p, *end;
+
+ if (!IS_ENABLED(CONFIG_IMA_APPRAISE_MODSIG))
+ return 0;
+
+ pr_notice("Loading compiled-in module X.509 certificates\n");
+
+ p = system_certificate_list;
+ end = p + module_cert_size;
+ load_cert(p, end, keyring, flags);
+
+ return 0;
+}
+
+/*
+ * Load the compiled-in list of X.509 certificates.
+ */
+static __init int load_system_certificate_list(void)
+{
+ const u8 *p, *end;
+
+ pr_notice("Loading compiled-in X.509 certificates\n");
+
+#ifdef CONFIG_MODULE_SIG
+ p = system_certificate_list;
+#else
+ p = system_certificate_list + module_cert_size;
+#endif
+ end = p + system_certificate_list_size;
+ load_cert(p, end, builtin_trusted_keys, KEY_ALLOC_NOT_IN_QUOTA |
+ KEY_ALLOC_BUILT_IN |
+ KEY_ALLOC_BYPASS_RESTRICTION);
+ return 0;
+}
late_initcall(load_system_certificate_list);

#ifdef CONFIG_SYSTEM_DATA_VERIFICATION
diff --git a/include/keys/system_keyring.h b/include/keys/system_keyring.h
index fb8b07daa9d1..e91c03376599 100644
--- a/include/keys/system_keyring.h
+++ b/include/keys/system_keyring.h
@@ -16,9 +16,16 @@ extern int restrict_link_by_builtin_trusted(struct key *keyring,
const struct key_type *type,
const union key_payload *payload,
struct key *restriction_key);
-
+extern __init int load_module_cert(struct key *keyring, unsigned long flags);
#else
#define restrict_link_by_builtin_trusted restrict_link_reject
+
+static inline __init int load_module_cert(struct key *keyring,
+ unsigned long flags)
+{
+ return 0;
+}
+
#endif

#ifdef CONFIG_SECONDARY_TRUSTED_KEYRING
diff --git a/security/integrity/digsig.c b/security/integrity/digsig.c
index 0f518dcfde05..4009d1e33fe0 100644
--- a/security/integrity/digsig.c
+++ b/security/integrity/digsig.c
@@ -111,8 +111,12 @@ static int __init __integrity_init_keyring(const unsigned int id,
} else {
if (id == INTEGRITY_KEYRING_PLATFORM)
set_platform_trusted_keys(keyring[id]);
+ if (id == INTEGRITY_KEYRING_IMA)
+ load_module_cert(keyring[id], KEY_ALLOC_NOT_IN_QUOTA);
}

+ pr_info("Loading key to ima keyring\n");
+
return err;
}

--
2.18.1


2021-02-11 22:36:16

by Stefan Berger

[permalink] [raw]
Subject: Re: [PATCH 5/5] ima: enable loading of build time generated key to .ima keyring

On 2/11/21 2:54 PM, Nayna Jain wrote:
> The kernel currently only loads the kernel module signing key onto
> the builtin trusted keyring. To support IMA, load the module signing
> key selectively either onto builtin or ima keyring based on MODULE_SIG
> or MODULE_APPRAISE_MODSIG config respectively; and loads the CA kernel
> key onto builtin trusted keyring.
>
> Signed-off-by: Nayna Jain <[email protected]>
> ---
> certs/system_keyring.c | 56 +++++++++++++++++++++++++++--------
> include/keys/system_keyring.h | 9 +++++-
> security/integrity/digsig.c | 4 +++
> 3 files changed, 55 insertions(+), 14 deletions(-)
>
> diff --git a/certs/system_keyring.c b/certs/system_keyring.c
> index 798291177186..0bbbe501f8a7 100644
> --- a/certs/system_keyring.c
> +++ b/certs/system_keyring.c
> @@ -26,6 +26,7 @@ static struct key *platform_trusted_keys;
>
> extern __initconst const u8 system_certificate_list[];
> extern __initconst const unsigned long system_certificate_list_size;
> +extern __initconst const unsigned long module_cert_size;
>
> /**
> * restrict_link_to_builtin_trusted - Restrict keyring addition by built in CA
> @@ -131,19 +132,12 @@ static __init int system_trusted_keyring_init(void)
> */
> device_initcall(system_trusted_keyring_init);
>
> -/*
> - * Load the compiled-in list of X.509 certificates.
> - */
> -static __init int load_system_certificate_list(void)
> +static __init int load_cert(const u8 *p, const u8 *end, struct key *keyring,
> + unsigned long flags)
> {
> key_ref_t key;
> - const u8 *p, *end;
> size_t plen;
>
> - pr_notice("Loading compiled-in X.509 certificates\n");
> -
> - p = system_certificate_list;
> - end = p + system_certificate_list_size;
> while (p < end) {
> /* Each cert begins with an ASN.1 SEQUENCE tag and must be more
> * than 256 bytes in size.
> @@ -158,16 +152,15 @@ static __init int load_system_certificate_list(void)
> if (plen > end - p)
> goto dodgy_cert;
>
> - key = key_create_or_update(make_key_ref(builtin_trusted_keys, 1),
> + key = key_create_or_update(make_key_ref(keyring, 1),
> "asymmetric",
> NULL,
> p,
> plen,
> ((KEY_POS_ALL & ~KEY_POS_SETATTR) |
> KEY_USR_VIEW | KEY_USR_READ),
> - KEY_ALLOC_NOT_IN_QUOTA |
> - KEY_ALLOC_BUILT_IN |
> - KEY_ALLOC_BYPASS_RESTRICTION);
> + flags);
> +
> if (IS_ERR(key)) {
> pr_err("Problem loading in-kernel X.509 certificate (%ld)\n",
> PTR_ERR(key));
> @@ -185,6 +178,43 @@ static __init int load_system_certificate_list(void)
> pr_err("Problem parsing in-kernel X.509 certificate list\n");
> return 0;
> }
> +
> +__init int load_module_cert(struct key *keyring, unsigned long flags)
> +{
> + const u8 *p, *end;
> +
> + if (!IS_ENABLED(CONFIG_IMA_APPRAISE_MODSIG))
> + return 0;
> +
> + pr_notice("Loading compiled-in module X.509 certificates\n");
> +
> + p = system_certificate_list;
> + end = p + module_cert_size;
> + load_cert(p, end, keyring, flags);
> +
> + return 0;

See my comment below.


> +}
> +
> +/*
> + * Load the compiled-in list of X.509 certificates.
> + */
> +static __init int load_system_certificate_list(void)
> +{
> + const u8 *p, *end;
> +
> + pr_notice("Loading compiled-in X.509 certificates\n");
> +
> +#ifdef CONFIG_MODULE_SIG
> + p = system_certificate_list;
> +#else
> + p = system_certificate_list + module_cert_size;
> +#endif
> + end = p + system_certificate_list_size;
> + load_cert(p, end, builtin_trusted_keys, KEY_ALLOC_NOT_IN_QUOTA |
> + KEY_ALLOC_BUILT_IN |
> + KEY_ALLOC_BYPASS_RESTRICTION);
> + return 0;


The oldĀ  load_system_certificate_list always returned 0 and the new
load_cert also does. You could just do 'return load_cert(p, ...)' here
and still get the 0.



> +}
> late_initcall(load_system_certificate_list);
>
> #ifdef CONFIG_SYSTEM_DATA_VERIFICATION
> diff --git a/include/keys/system_keyring.h b/include/keys/system_keyring.h
> index fb8b07daa9d1..e91c03376599 100644
> --- a/include/keys/system_keyring.h
> +++ b/include/keys/system_keyring.h
> @@ -16,9 +16,16 @@ extern int restrict_link_by_builtin_trusted(struct key *keyring,
> const struct key_type *type,
> const union key_payload *payload,
> struct key *restriction_key);
> -
> +extern __init int load_module_cert(struct key *keyring, unsigned long flags);
> #else
> #define restrict_link_by_builtin_trusted restrict_link_reject
> +
> +static inline __init int load_module_cert(struct key *keyring,
> + unsigned long flags)
> +{
> + return 0;
> +}
> +
> #endif
>
> #ifdef CONFIG_SECONDARY_TRUSTED_KEYRING
> diff --git a/security/integrity/digsig.c b/security/integrity/digsig.c
> index 0f518dcfde05..4009d1e33fe0 100644
> --- a/security/integrity/digsig.c
> +++ b/security/integrity/digsig.c
> @@ -111,8 +111,12 @@ static int __init __integrity_init_keyring(const unsigned int id,
> } else {
> if (id == INTEGRITY_KEYRING_PLATFORM)
> set_platform_trusted_keys(keyring[id]);
> + if (id == INTEGRITY_KEYRING_IMA)
> + load_module_cert(keyring[id], KEY_ALLOC_NOT_IN_QUOTA);
> }
>
> + pr_info("Loading key to ima keyring\n");
> +
> return err;
> }
>

Otherwise lgtm.


2021-02-12 23:51:56

by Jarkko Sakkinen

[permalink] [raw]
Subject: Re: [PATCH 5/5] ima: enable loading of build time generated key to .ima keyring

On Thu, Feb 11, 2021 at 02:54:35PM -0500, Nayna Jain wrote:
> The kernel currently only loads the kernel module signing key onto
> the builtin trusted keyring. To support IMA, load the module signing
> key selectively either onto builtin or ima keyring based on MODULE_SIG
~~~
IMA


> or MODULE_APPRAISE_MODSIG config respectively; and loads the CA kernel
> key onto builtin trusted keyring.
>
> Signed-off-by: Nayna Jain <[email protected]>

/Jarkko

> ---
> certs/system_keyring.c | 56 +++++++++++++++++++++++++++--------
> include/keys/system_keyring.h | 9 +++++-
> security/integrity/digsig.c | 4 +++
> 3 files changed, 55 insertions(+), 14 deletions(-)
>
> diff --git a/certs/system_keyring.c b/certs/system_keyring.c
> index 798291177186..0bbbe501f8a7 100644
> --- a/certs/system_keyring.c
> +++ b/certs/system_keyring.c
> @@ -26,6 +26,7 @@ static struct key *platform_trusted_keys;
>
> extern __initconst const u8 system_certificate_list[];
> extern __initconst const unsigned long system_certificate_list_size;
> +extern __initconst const unsigned long module_cert_size;
>
> /**
> * restrict_link_to_builtin_trusted - Restrict keyring addition by built in CA
> @@ -131,19 +132,12 @@ static __init int system_trusted_keyring_init(void)
> */
> device_initcall(system_trusted_keyring_init);
>
> -/*
> - * Load the compiled-in list of X.509 certificates.
> - */
> -static __init int load_system_certificate_list(void)
> +static __init int load_cert(const u8 *p, const u8 *end, struct key *keyring,
> + unsigned long flags)
> {
> key_ref_t key;
> - const u8 *p, *end;
> size_t plen;
>
> - pr_notice("Loading compiled-in X.509 certificates\n");
> -
> - p = system_certificate_list;
> - end = p + system_certificate_list_size;
> while (p < end) {
> /* Each cert begins with an ASN.1 SEQUENCE tag and must be more
> * than 256 bytes in size.
> @@ -158,16 +152,15 @@ static __init int load_system_certificate_list(void)
> if (plen > end - p)
> goto dodgy_cert;
>
> - key = key_create_or_update(make_key_ref(builtin_trusted_keys, 1),
> + key = key_create_or_update(make_key_ref(keyring, 1),
> "asymmetric",
> NULL,
> p,
> plen,
> ((KEY_POS_ALL & ~KEY_POS_SETATTR) |
> KEY_USR_VIEW | KEY_USR_READ),
> - KEY_ALLOC_NOT_IN_QUOTA |
> - KEY_ALLOC_BUILT_IN |
> - KEY_ALLOC_BYPASS_RESTRICTION);
> + flags);
> +
> if (IS_ERR(key)) {
> pr_err("Problem loading in-kernel X.509 certificate (%ld)\n",
> PTR_ERR(key));
> @@ -185,6 +178,43 @@ static __init int load_system_certificate_list(void)
> pr_err("Problem parsing in-kernel X.509 certificate list\n");
> return 0;
> }
> +
> +__init int load_module_cert(struct key *keyring, unsigned long flags)
> +{
> + const u8 *p, *end;
> +
> + if (!IS_ENABLED(CONFIG_IMA_APPRAISE_MODSIG))
> + return 0;
> +
> + pr_notice("Loading compiled-in module X.509 certificates\n");
> +
> + p = system_certificate_list;
> + end = p + module_cert_size;
> + load_cert(p, end, keyring, flags);
> +
> + return 0;
> +}
> +
> +/*
> + * Load the compiled-in list of X.509 certificates.
> + */
> +static __init int load_system_certificate_list(void)
> +{
> + const u8 *p, *end;
> +
> + pr_notice("Loading compiled-in X.509 certificates\n");
> +
> +#ifdef CONFIG_MODULE_SIG
> + p = system_certificate_list;
> +#else
> + p = system_certificate_list + module_cert_size;
> +#endif
> + end = p + system_certificate_list_size;
> + load_cert(p, end, builtin_trusted_keys, KEY_ALLOC_NOT_IN_QUOTA |
> + KEY_ALLOC_BUILT_IN |
> + KEY_ALLOC_BYPASS_RESTRICTION);
> + return 0;
> +}
> late_initcall(load_system_certificate_list);
>
> #ifdef CONFIG_SYSTEM_DATA_VERIFICATION
> diff --git a/include/keys/system_keyring.h b/include/keys/system_keyring.h
> index fb8b07daa9d1..e91c03376599 100644
> --- a/include/keys/system_keyring.h
> +++ b/include/keys/system_keyring.h
> @@ -16,9 +16,16 @@ extern int restrict_link_by_builtin_trusted(struct key *keyring,
> const struct key_type *type,
> const union key_payload *payload,
> struct key *restriction_key);
> -
> +extern __init int load_module_cert(struct key *keyring, unsigned long flags);
> #else
> #define restrict_link_by_builtin_trusted restrict_link_reject
> +
> +static inline __init int load_module_cert(struct key *keyring,
> + unsigned long flags)
> +{
> + return 0;
> +}
> +
> #endif
>
> #ifdef CONFIG_SECONDARY_TRUSTED_KEYRING
> diff --git a/security/integrity/digsig.c b/security/integrity/digsig.c
> index 0f518dcfde05..4009d1e33fe0 100644
> --- a/security/integrity/digsig.c
> +++ b/security/integrity/digsig.c
> @@ -111,8 +111,12 @@ static int __init __integrity_init_keyring(const unsigned int id,
> } else {
> if (id == INTEGRITY_KEYRING_PLATFORM)
> set_platform_trusted_keys(keyring[id]);
> + if (id == INTEGRITY_KEYRING_IMA)
> + load_module_cert(keyring[id], KEY_ALLOC_NOT_IN_QUOTA);
> }
>
> + pr_info("Loading key to ima keyring\n");
> +
> return err;
> }
>
> --
> 2.18.1
>
>