Re: [RFC PATCH 1/2] ima: Implement support for uncompressed module appended signatures

From: Mimi Zohar <zohar@linux.ibm.com>
To: Eric Snowberg <eric.snowberg@oracle.com>,
	dmitry.kasatkin@gmail.com, jmorris@namei.org, serge@hallyn.com
Cc: dhowells@redhat.com, geert@linux-m68k.org,
	gregkh@linuxfoundation.org, nayna@linux.ibm.com,
	tglx@linutronix.de, bauerman@linux.ibm.com, mpe@ellerman.id.au,
	linux-integrity@vger.kernel.org,
	linux-security-module@vger.kernel.org,
	linux-kernel@vger.kernel.org
Subject: Re: [RFC PATCH 1/2] ima: Implement support for uncompressed module appended signatures
Date: Thu, 06 Feb 2020 13:05:29 -0500	[thread overview]
Message-ID: <1581012329.5585.439.camel@linux.ibm.com> (raw)
In-Reply-To: <20200206164226.24875-2-eric.snowberg@oracle.com>

Hi Eric,

On Thu, 2020-02-06 at 11:42 -0500, Eric Snowberg wrote:
> Currently IMA can validate compressed modules containing appended
> signatures.  This adds the ability to also validate uncompressed
> modules when appraise_type=imasig|modsig.
> 
> Signed-off-by: Eric Snowberg <eric.snowberg@oracle.com>

Your patch description in no way matches the code.

Mimi

> ---
>  security/integrity/digsig.c           | 9 +++++++--
>  security/integrity/ima/ima_appraise.c | 3 +++
>  security/integrity/integrity.h        | 3 ++-
>  3 files changed, 12 insertions(+), 3 deletions(-)
> 
> diff --git a/security/integrity/digsig.c b/security/integrity/digsig.c
> index ea1aae3d07b3..5e0c4d04ab9d 100644
> --- a/security/integrity/digsig.c
> +++ b/security/integrity/digsig.c
> @@ -15,6 +15,7 @@
>  #include <linux/key-type.h>
>  #include <linux/digsig.h>
>  #include <linux/vmalloc.h>
> +#include <linux/verification.h>
>  #include <crypto/public_key.h>
>  #include <keys/system_keyring.h>
>  
> @@ -31,6 +32,7 @@ static const char * const keyring_name[INTEGRITY_KEYRING_MAX] = {
>  	".ima",
>  #endif
>  	".platform",
> +	".builtin_trusted_keys",
>  };
>  
>  #ifdef CONFIG_IMA_KEYRINGS_PERMIT_SIGNED_BY_BUILTIN_OR_SECONDARY
> @@ -45,8 +47,11 @@ static struct key *integrity_keyring_from_id(const unsigned int id)
>  		return ERR_PTR(-EINVAL);
>  
>  	if (!keyring[id]) {
> -		keyring[id] =
> -			request_key(&key_type_keyring, keyring_name[id], NULL);
> +		if (id == INTEGRITY_KEYRING_KERNEL)
> +			keyring[id] = VERIFY_USE_SECONDARY_KEYRING;
> +		else
> +			keyring[id] = request_key(&key_type_keyring,
> +						  keyring_name[id], NULL);
>  		if (IS_ERR(keyring[id])) {
>  			int err = PTR_ERR(keyring[id]);
>  			pr_err("no %s keyring: %d\n", keyring_name[id], err);
> diff --git a/security/integrity/ima/ima_appraise.c b/security/integrity/ima/ima_appraise.c
> index 300c8d2943c5..4c009c55d620 100644
> --- a/security/integrity/ima/ima_appraise.c
> +++ b/security/integrity/ima/ima_appraise.c
> @@ -294,6 +294,9 @@ static int modsig_verify(enum ima_hooks func, const struct modsig *modsig,
>  	    func == KEXEC_KERNEL_CHECK)
>  		rc = integrity_modsig_verify(INTEGRITY_KEYRING_PLATFORM,
>  					     modsig);
> +	if (rc && func == MODULE_CHECK)
> +		rc = integrity_modsig_verify(INTEGRITY_KEYRING_KERNEL, modsig);
> +
>  	if (rc) {
>  		*cause = "invalid-signature";
>  		*status = INTEGRITY_FAIL;
> diff --git a/security/integrity/integrity.h b/security/integrity/integrity.h
> index 73fc286834d7..63f0e6bff0e0 100644
> --- a/security/integrity/integrity.h
> +++ b/security/integrity/integrity.h
> @@ -145,7 +145,8 @@ int integrity_kernel_read(struct file *file, loff_t offset,
>  #define INTEGRITY_KEYRING_EVM		0
>  #define INTEGRITY_KEYRING_IMA		1
>  #define INTEGRITY_KEYRING_PLATFORM	2
> -#define INTEGRITY_KEYRING_MAX		3
> +#define INTEGRITY_KEYRING_KERNEL	3
> +#define INTEGRITY_KEYRING_MAX		4
>  
>  extern struct dentry *integrity_dir;
>  