[PATCH v3 0/3] ima: kernel build support for loading the kernel module signing key

[PATCH v3 0/3] ima: kernel build support for loading the kernel module signing key

[Nayna Jain]

Kernel modules are currently only signed when CONFIG_MODULE_SIG is enabled.
The kernel module signing key is a self-signed CA only loaded onto the
.builtin_trusted_key keyring.  On secure boot enabled systems with an arch
specific IMA policy enabled, but without MODULE_SIG enabled, kernel modules
are not signed, nor is the kernel module signing public key loaded onto the
IMA keyring.

In order to load the the kernel module signing key onto the IMA trusted
keyring ('.ima'), the certificate needs to be signed by a CA key either on
the builtin or secondary keyrings. The original version of this patch set
created and loaded a kernel-CA key onto the builtin keyring. The kernel-CA
key signed the kernel module signing key, allowing it to be loaded onto the
IMA trusted keyring.

However, missing from this version was support for the kernel-CA to sign the
hardware token certificate. Adding that support would add additional
complexity.

Since the kernel module signing key is embedded into the Linux kernel at
build time, instead of creating and loading a kernel-CA onto the builtin
trusted keyring, this version makes an exception and allows the 
self-signed kernel module signing key to be loaded directly onto the 
trusted IMA keyring

v3:

* Fix the "Fixes" tag as suggested by Stefan for Patch 1/3.
* Revert back the CA signed module signing key to only self-signed.
* Allow self signed key as exception only for build time generated
module signing key onto .ima keyring.

v2:

* Include feedback from Stefan - corrected the Fixes commit id in Patch 1
and cleaned Patch 5/5.
* Fix the issue reported by kernel test bot.
* Include Jarkko's feedback on patch description.

Nayna Jain (3):
  keys: cleanup build time module signing keys
  ima: enable signing of modules with build time generated key
  ima: enable loading of build time generated key on .ima keyring

 Makefile                      |  6 ++---
 certs/Kconfig                 |  2 +-
 certs/Makefile                |  8 ++++++
 certs/system_certificates.S   | 13 +++++++++-
 certs/system_keyring.c        | 47 +++++++++++++++++++++++++++--------
 include/keys/system_keyring.h |  7 ++++++
 init/Kconfig                  |  6 ++---
 security/integrity/digsig.c   |  2 ++
 8 files changed, 73 insertions(+), 18 deletions(-)

-- 
2.29.2

[PATCH v3 1/3] keys: cleanup build time module signing keys

[Nayna Jain]

The "mrproper" target is still looking for build time generated keys in
the kernel root directory instead of certs directory. Fix the path and
remove the names of the files which are no longer generated.

Fixes: cfc411e7fff3 ("Move certificate handling to its own directory")
Signed-off-by: Nayna Jain <nayna@linux.ibm.com>
Reviewed-by: Stefan Berger <stefanb@linux.ibm.com>
Reviewed-by: Mimi Zohar <zohar@linux.ibm.com>
---
 Makefile | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/Makefile b/Makefile
index d4784d181123..b7c2ed2a8684 100644
--- a/Makefile
+++ b/Makefile
@@ -1523,9 +1523,9 @@ MRPROPER_FILES += include/config include/generated          \
 		  debian snap tar-install \
 		  .config .config.old .version \
 		  Module.symvers \
-		  signing_key.pem signing_key.priv signing_key.x509	\
-		  x509.genkey extra_certificates signing_key.x509.keyid	\
-		  signing_key.x509.signer vmlinux-gdb.py \
+		  certs/signing_key.pem certs/signing_key.x509 \
+		  certs/x509.genkey \
+		  vmlinux-gdb.py \
 		  *.spec
 
 # Directories & files removed with 'make distclean'
-- 
2.29.2

[PATCH v3 2/3] ima: enable signing of modules with build time generated key

[Nayna Jain]

The kernel build process currently only signs kernel modules when
MODULE_SIG is enabled. Also, sign the kernel modules at build time when
IMA_APPRAISE_MODSIG is enabled.

Signed-off-by: Nayna Jain <nayna@linux.ibm.com>
---
 certs/Kconfig  | 2 +-
 certs/Makefile | 8 ++++++++
 init/Kconfig   | 6 +++---
 3 files changed, 12 insertions(+), 4 deletions(-)

diff --git a/certs/Kconfig b/certs/Kconfig
index c94e93d8bccf..48675ad319db 100644
--- a/certs/Kconfig
+++ b/certs/Kconfig
@@ -4,7 +4,7 @@ menu "Certificates for signature checking"
 config MODULE_SIG_KEY
 	string "File name or PKCS#11 URI of module signing key"
 	default "certs/signing_key.pem"
-	depends on MODULE_SIG
+	depends on MODULE_SIG || IMA_APPRAISE_MODSIG
 	help
          Provide the file name of a private key/certificate in PEM format,
          or a PKCS#11 URI according to RFC7512. The file should contain, or
diff --git a/certs/Makefile b/certs/Makefile
index f4c25b67aad9..e3185c57fbd8 100644
--- a/certs/Makefile
+++ b/certs/Makefile
@@ -32,6 +32,14 @@ endif # CONFIG_SYSTEM_TRUSTED_KEYRING
 clean-files := x509_certificate_list .x509.list
 
 ifeq ($(CONFIG_MODULE_SIG),y)
+	SIGN_KEY = y
+endif
+
+ifeq ($(CONFIG_IMA_APPRAISE_MODSIG),y)
+	SIGN_KEY = y
+endif
+
+ifdef SIGN_KEY
 ###############################################################################
 #
 # If module signing is requested, say by allyesconfig, but a key has not been
diff --git a/init/Kconfig b/init/Kconfig
index 5f5c776ef192..85e48a578f90 100644
--- a/init/Kconfig
+++ b/init/Kconfig
@@ -2164,7 +2164,7 @@ config MODULE_SIG_FORCE
 config MODULE_SIG_ALL
 	bool "Automatically sign all modules"
 	default y
-	depends on MODULE_SIG
+	depends on MODULE_SIG || IMA_APPRAISE_MODSIG
 	help
 	  Sign all modules during make modules_install. Without this option,
 	  modules must be signed manually, using the scripts/sign-file tool.
@@ -2174,7 +2174,7 @@ comment "Do not forget to sign required modules with scripts/sign-file"
 
 choice
 	prompt "Which hash algorithm should modules be signed with?"
-	depends on MODULE_SIG
+	depends on MODULE_SIG || IMA_APPRAISE_MODSIG
 	help
 	  This determines which sort of hashing algorithm will be used during
 	  signature generation.  This algorithm _must_ be built into the kernel
@@ -2206,7 +2206,7 @@ endchoice
 
 config MODULE_SIG_HASH
 	string
-	depends on MODULE_SIG
+	depends on MODULE_SIG || IMA_APPRAISE_MODSIG
 	default "sha1" if MODULE_SIG_SHA1
 	default "sha224" if MODULE_SIG_SHA224
 	default "sha256" if MODULE_SIG_SHA256
-- 
2.29.2

[PATCH v3 3/3] ima: enable loading of build time generated key on .ima keyring

[Nayna Jain]

The kernel currently only loads the kernel module signing key onto the
builtin trusted keyring. Load the module signing key onto the IMA keyring
as well.

Signed-off-by: Nayna Jain <nayna@linux.ibm.com>
---
 certs/system_certificates.S   | 13 +++++++++-
 certs/system_keyring.c        | 47 +++++++++++++++++++++++++++--------
 include/keys/system_keyring.h |  7 ++++++
 security/integrity/digsig.c   |  2 ++
 4 files changed, 58 insertions(+), 11 deletions(-)

diff --git a/certs/system_certificates.S b/certs/system_certificates.S
index 8f29058adf93..dcad27ea8527 100644
--- a/certs/system_certificates.S
+++ b/certs/system_certificates.S
@@ -8,9 +8,11 @@
 	.globl system_certificate_list
 system_certificate_list:
 __cert_list_start:
-#ifdef CONFIG_MODULE_SIG
+__module_cert_start:
+#if defined(CONFIG_MODULE_SIG) || defined(CONFIG_IMA_APPRAISE_MODSIG)
 	.incbin "certs/signing_key.x509"
 #endif
+__module_cert_end:
 	.incbin "certs/x509_certificate_list"
 __cert_list_end:
 
@@ -35,3 +37,12 @@ system_certificate_list_size:
 #else
 	.long __cert_list_end - __cert_list_start
 #endif
+
+	.align 8
+	.globl module_cert_size
+module_cert_size:
+#ifdef CONFIG_64BIT
+	.quad __module_cert_end - __module_cert_start
+#else
+	.long __module_cert_end - __module_cert_start
+#endif
diff --git a/certs/system_keyring.c b/certs/system_keyring.c
index 4b693da488f1..bb122bf4cc17 100644
--- a/certs/system_keyring.c
+++ b/certs/system_keyring.c
@@ -27,6 +27,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
@@ -132,19 +133,11 @@ 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)
 {
 	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.
@@ -159,7 +152,7 @@ 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,
@@ -186,6 +179,40 @@ 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)
+{
+	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;
+
+	return load_cert(p, end, keyring);
+}
+
+/*
+ * 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;
+	return load_cert(p, end, builtin_trusted_keys);
+}
 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..f954276c616a 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);
 
 #else
 #define restrict_link_by_builtin_trusted restrict_link_reject
+
+static inline __init int load_module_cert(struct key *keyring)
+{
+	return 0;
+}
+
 #endif
 
 #ifdef CONFIG_SECONDARY_TRUSTED_KEYRING
diff --git a/security/integrity/digsig.c b/security/integrity/digsig.c
index 250fb0836156..3b06a01bd0fd 100644
--- a/security/integrity/digsig.c
+++ b/security/integrity/digsig.c
@@ -111,6 +111,8 @@ 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]);
 	}
 
 	return err;
-- 
2.29.2

Re: [PATCH v3 1/3] keys: cleanup build time module signing keys

[Jarkko Sakkinen]

On Tue, Mar 30, 2021 at 09:16:34AM -0400, Nayna Jain wrote:
> The "mrproper" target is still looking for build time generated keys in
> the kernel root directory instead of certs directory. Fix the path and
> remove the names of the files which are no longer generated.
> 
> Fixes: cfc411e7fff3 ("Move certificate handling to its own directory")
> Signed-off-by: Nayna Jain <nayna@linux.ibm.com>
> Reviewed-by: Stefan Berger <stefanb@linux.ibm.com>
> Reviewed-by: Mimi Zohar <zohar@linux.ibm.com>
> ---
>  Makefile | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/Makefile b/Makefile
> index d4784d181123..b7c2ed2a8684 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -1523,9 +1523,9 @@ MRPROPER_FILES += include/config include/generated          \
>  		  debian snap tar-install \
>  		  .config .config.old .version \
>  		  Module.symvers \
> -		  signing_key.pem signing_key.priv signing_key.x509	\
> -		  x509.genkey extra_certificates signing_key.x509.keyid	\
> -		  signing_key.x509.signer vmlinux-gdb.py \
> +		  certs/signing_key.pem certs/signing_key.x509 \
> +		  certs/x509.genkey \
> +		  vmlinux-gdb.py \
>  		  *.spec
>  
>  # Directories & files removed with 'make distclean'
> -- 
> 2.29.2
> 
> 



Reviewed-by: Jarkko Sakkinen <jarkko@kernel.org>

/Jarkko

Re: [PATCH v3 2/3] ima: enable signing of modules with build time generated key

[Stefan Berger]

On 3/30/21 9:16 AM, Nayna Jain wrote:
> The kernel build process currently only signs kernel modules when
> MODULE_SIG is enabled. Also, sign the kernel modules at build time when
> IMA_APPRAISE_MODSIG is enabled.
>
> Signed-off-by: Nayna Jain <nayna@linux.ibm.com>
Acked-by: Stefan Berger <stefanb@linux.ibm.com>
> ---
>   certs/Kconfig  | 2 +-
>   certs/Makefile | 8 ++++++++
>   init/Kconfig   | 6 +++---
>   3 files changed, 12 insertions(+), 4 deletions(-)
>
> diff --git a/certs/Kconfig b/certs/Kconfig
> index c94e93d8bccf..48675ad319db 100644
> --- a/certs/Kconfig
> +++ b/certs/Kconfig
> @@ -4,7 +4,7 @@ menu "Certificates for signature checking"
>   config MODULE_SIG_KEY
>   	string "File name or PKCS#11 URI of module signing key"
>   	default "certs/signing_key.pem"
> -	depends on MODULE_SIG
> +	depends on MODULE_SIG || IMA_APPRAISE_MODSIG
>   	help
>            Provide the file name of a private key/certificate in PEM format,
>            or a PKCS#11 URI according to RFC7512. The file should contain, or
> diff --git a/certs/Makefile b/certs/Makefile
> index f4c25b67aad9..e3185c57fbd8 100644
> --- a/certs/Makefile
> +++ b/certs/Makefile
> @@ -32,6 +32,14 @@ endif # CONFIG_SYSTEM_TRUSTED_KEYRING
>   clean-files := x509_certificate_list .x509.list
>   
>   ifeq ($(CONFIG_MODULE_SIG),y)
> +	SIGN_KEY = y
> +endif
> +
> +ifeq ($(CONFIG_IMA_APPRAISE_MODSIG),y)
> +	SIGN_KEY = y
> +endif
> +
> +ifdef SIGN_KEY
>   ###############################################################################
>   #
>   # If module signing is requested, say by allyesconfig, but a key has not been
> diff --git a/init/Kconfig b/init/Kconfig
> index 5f5c776ef192..85e48a578f90 100644
> --- a/init/Kconfig
> +++ b/init/Kconfig
> @@ -2164,7 +2164,7 @@ config MODULE_SIG_FORCE
>   config MODULE_SIG_ALL
>   	bool "Automatically sign all modules"
>   	default y
> -	depends on MODULE_SIG
> +	depends on MODULE_SIG || IMA_APPRAISE_MODSIG
>   	help
>   	  Sign all modules during make modules_install. Without this option,
>   	  modules must be signed manually, using the scripts/sign-file tool.
> @@ -2174,7 +2174,7 @@ comment "Do not forget to sign required modules with scripts/sign-file"
>   
>   choice
>   	prompt "Which hash algorithm should modules be signed with?"
> -	depends on MODULE_SIG
> +	depends on MODULE_SIG || IMA_APPRAISE_MODSIG
>   	help
>   	  This determines which sort of hashing algorithm will be used during
>   	  signature generation.  This algorithm _must_ be built into the kernel
> @@ -2206,7 +2206,7 @@ endchoice
>   
>   config MODULE_SIG_HASH
>   	string
> -	depends on MODULE_SIG
> +	depends on MODULE_SIG || IMA_APPRAISE_MODSIG
>   	default "sha1" if MODULE_SIG_SHA1
>   	default "sha224" if MODULE_SIG_SHA224
>   	default "sha256" if MODULE_SIG_SHA256

Re: [PATCH v3 3/3] ima: enable loading of build time generated key on .ima keyring

[Stefan Berger]

On 3/30/21 9:16 AM, Nayna Jain wrote:
> The kernel currently only loads the kernel module signing key onto the
> builtin trusted keyring. Load the module signing key onto the IMA keyring
> as well.
>
> Signed-off-by: Nayna Jain <nayna@linux.ibm.com>
Acked-by: Stefan Berger <stefanb@linux.ibm.com>
> ---
>   certs/system_certificates.S   | 13 +++++++++-
>   certs/system_keyring.c        | 47 +++++++++++++++++++++++++++--------
>   include/keys/system_keyring.h |  7 ++++++
>   security/integrity/digsig.c   |  2 ++
>   4 files changed, 58 insertions(+), 11 deletions(-)
>
> diff --git a/certs/system_certificates.S b/certs/system_certificates.S
> index 8f29058adf93..dcad27ea8527 100644
> --- a/certs/system_certificates.S
> +++ b/certs/system_certificates.S
> @@ -8,9 +8,11 @@
>   	.globl system_certificate_list
>   system_certificate_list:
>   __cert_list_start:
> -#ifdef CONFIG_MODULE_SIG
> +__module_cert_start:
> +#if defined(CONFIG_MODULE_SIG) || defined(CONFIG_IMA_APPRAISE_MODSIG)
>   	.incbin "certs/signing_key.x509"
>   #endif
> +__module_cert_end:
>   	.incbin "certs/x509_certificate_list"
>   __cert_list_end:
>   
> @@ -35,3 +37,12 @@ system_certificate_list_size:
>   #else
>   	.long __cert_list_end - __cert_list_start
>   #endif
> +
> +	.align 8
> +	.globl module_cert_size
> +module_cert_size:
> +#ifdef CONFIG_64BIT
> +	.quad __module_cert_end - __module_cert_start
> +#else
> +	.long __module_cert_end - __module_cert_start
> +#endif
> diff --git a/certs/system_keyring.c b/certs/system_keyring.c
> index 4b693da488f1..bb122bf4cc17 100644
> --- a/certs/system_keyring.c
> +++ b/certs/system_keyring.c
> @@ -27,6 +27,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
> @@ -132,19 +133,11 @@ 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)
>   {
>   	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.
> @@ -159,7 +152,7 @@ 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,
> @@ -186,6 +179,40 @@ 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)
> +{
> +	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;
> +
> +	return load_cert(p, end, keyring);
> +}
> +
> +/*
> + * 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;
> +	return load_cert(p, end, builtin_trusted_keys);
> +}
>   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..f954276c616a 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);
>   
>   #else
>   #define restrict_link_by_builtin_trusted restrict_link_reject
> +
> +static inline __init int load_module_cert(struct key *keyring)
> +{
> +	return 0;
> +}
> +
>   #endif
>   
>   #ifdef CONFIG_SECONDARY_TRUSTED_KEYRING
> diff --git a/security/integrity/digsig.c b/security/integrity/digsig.c
> index 250fb0836156..3b06a01bd0fd 100644
> --- a/security/integrity/digsig.c
> +++ b/security/integrity/digsig.c
> @@ -111,6 +111,8 @@ 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]);
>   	}
>   
>   	return err;