Re: [RFC PATCH v3 07/13] keys: Add ability to track intended usage of the public key

[Jarkko Sakkinen]

On Thu, Oct 17, 2024 at 09:55:10AM -0600, Eric Snowberg wrote:
> Add two new fields in public_key_signature to track the intended usage of
> the signature.  Also add a flag for the revocation pass.  During signature
> validation, two verifications can take place for the same signature.  One
> to see if it verifies against something on the .blacklist keyring and
> the other to see if it verifies against the supplied keyring. The flag
> is used to determine which stage the verification is in.
> 
> Signed-off-by: Eric Snowberg <eric.snowberg@xxxxxxxxxx>

Mimi, was this the patch set you asked to look at while ago?</offtopic>

This patch adds PKS_REVOCATION_PASS and REVOCATION_SET. It would be
nice to put in simple terms how they are used in down to eart terms.

> ---
>  certs/blacklist.c                     |  3 +++
>  crypto/asymmetric_keys/pkcs7_trust.c  | 20 ++++++++++++++++++++
>  crypto/asymmetric_keys/pkcs7_verify.c |  4 ++++
>  include/crypto/pkcs7.h                |  3 +++
>  include/crypto/public_key.h           |  4 ++++
>  5 files changed, 34 insertions(+)
> 
> diff --git a/certs/blacklist.c b/certs/blacklist.c
> index 675dd7a8f07a..dd34e56a6362 100644
> --- a/certs/blacklist.c
> +++ b/certs/blacklist.c
> @@ -17,6 +17,7 @@
>  #include <linux/uidgid.h>
>  #include <keys/asymmetric-type.h>
>  #include <keys/system_keyring.h>
> +#include <crypto/public_key.h>
>  #include "blacklist.h"
>  
>  /*
> @@ -289,7 +290,9 @@ int is_key_on_revocation_list(struct pkcs7_message *pkcs7)
>  {
>  	int ret;
>  
> +	pkcs7_set_usage_flag(pkcs7, PKS_REVOCATION_PASS);
>  	ret = pkcs7_validate_trust(pkcs7, blacklist_keyring);
> +	pkcs7_clear_usage_flag(pkcs7, PKS_REVOCATION_PASS);
>  
>  	if (ret == 0)
>  		return -EKEYREJECTED;
> diff --git a/crypto/asymmetric_keys/pkcs7_trust.c b/crypto/asymmetric_keys/pkcs7_trust.c
> index 9a87c34ed173..64d70eb68864 100644
> --- a/crypto/asymmetric_keys/pkcs7_trust.c
> +++ b/crypto/asymmetric_keys/pkcs7_trust.c
> @@ -131,6 +131,26 @@ static int pkcs7_validate_trust_one(struct pkcs7_message *pkcs7,
>  	return 0;
>  }
>  
> +void pkcs7_clear_usage_flag(struct pkcs7_message *pkcs7, unsigned long usage)
> +{
> +	struct pkcs7_signed_info *sinfo;
> +
> +	for (sinfo = pkcs7->signed_infos; sinfo; sinfo = sinfo->next) {
> +		if (sinfo->sig)
> +			clear_bit(usage, &sinfo->sig->usage_flags);
> +	}
> +}
> +
> +void pkcs7_set_usage_flag(struct pkcs7_message *pkcs7, unsigned long usage)
> +{
> +	struct pkcs7_signed_info *sinfo;
> +
> +	for (sinfo = pkcs7->signed_infos; sinfo; sinfo = sinfo->next) {
> +		if (sinfo->sig)
> +			set_bit(usage, &sinfo->sig->usage_flags);
> +	}
> +}
> +
>  /**
>   * pkcs7_validate_trust - Validate PKCS#7 trust chain
>   * @pkcs7: The PKCS#7 certificate to validate
> diff --git a/crypto/asymmetric_keys/pkcs7_verify.c b/crypto/asymmetric_keys/pkcs7_verify.c
> index 1dc80e68ce96..44b8bd0ad4d8 100644
> --- a/crypto/asymmetric_keys/pkcs7_verify.c
> +++ b/crypto/asymmetric_keys/pkcs7_verify.c
> @@ -455,6 +455,10 @@ int pkcs7_verify(struct pkcs7_message *pkcs7,
>  			return ret;
>  		}
>  		actual_ret = 0;
> +		if (sinfo->sig) {
> +			sinfo->sig->usage = usage;
> +			set_bit(PKS_USAGE_SET, &sinfo->sig->usage_flags);
> +		}
>  	}
>  
>  	kleave(" = %d", actual_ret);
> diff --git a/include/crypto/pkcs7.h b/include/crypto/pkcs7.h
> index 38ec7f5f9041..6c3c9061b118 100644
> --- a/include/crypto/pkcs7.h
> +++ b/include/crypto/pkcs7.h
> @@ -32,6 +32,9 @@ extern int pkcs7_get_content_data(const struct pkcs7_message *pkcs7,
>  extern int pkcs7_validate_trust(struct pkcs7_message *pkcs7,
>  				struct key *trust_keyring);
>  
> +extern void pkcs7_set_usage_flag(struct pkcs7_message *pkcs7, unsigned long usage);
> +extern void pkcs7_clear_usage_flag(struct pkcs7_message *pkcs7, unsigned long usage);
> +
>  /*
>   * pkcs7_verify.c
>   */
> diff --git a/include/crypto/public_key.h b/include/crypto/public_key.h
> index b7f308977c84..394022b5d856 100644
> --- a/include/crypto/public_key.h
> +++ b/include/crypto/public_key.h
> @@ -49,6 +49,10 @@ struct public_key_signature {
>  	const char *pkey_algo;
>  	const char *hash_algo;
>  	const char *encoding;
> +	u32 usage;		/* Intended usage */

I'd would not size encode this but instead use "unsigned int".

> +	unsigned long usage_flags;

It would be less convoluting if this was just "flags". Now this leaves
to wonder what it is encoded for. E.g. for ioctl API one doees it for
the API to be size agnostic between 32/64-bit kernels.

> +#define PKS_USAGE_SET		0
> +#define PKS_REVOCATION_PASS	1
>  };

I'd add these before the struct and:

/* ... */
#define PKS_USAGE_SET		0

/* ... */
#define PKS_REVOCATION_PASS	1

>  
>  extern void public_key_signature_free(struct public_key_signature *sig);
> -- 
> 2.45.0
> 

BR, Jarkko