Linux-Security-Module Archive on lore.kernel.org
 help / color / Atom feed
From: Jarkko Sakkinen <jarkko@kernel.org>
To: "Mickaël Salaün" <mic@digikod.net>
Cc: "David Howells" <dhowells@redhat.com>,
	"David Woodhouse" <dwmw2@infradead.org>,
	"David S . Miller" <davem@davemloft.net>,
	"Eric Snowberg" <eric.snowberg@oracle.com>,
	"Herbert Xu" <herbert@gondor.apana.org.au>,
	"James Morris" <jmorris@namei.org>,
	"Mickaël Salaün" <mic@linux.microsoft.com>,
	"Mimi Zohar" <zohar@linux.ibm.com>,
	"Serge E . Hallyn" <serge@hallyn.com>,
	"Tyler Hicks" <tyhicks@linux.microsoft.com>,
	keyrings@vger.kernel.org, linux-crypto@vger.kernel.org,
	linux-integrity@vger.kernel.org, linux-kernel@vger.kernel.org,
	linux-security-module@vger.kernel.org
Subject: Re: [PATCH v7 4/5] certs: Factor out the blacklist hash creation
Date: Sat, 13 Mar 2021 20:54:04 +0200
Message-ID: <YE0KTAfElytCNh0M@kernel.org> (raw)
In-Reply-To: <20210312171232.2681989-5-mic@digikod.net>

On Fri, Mar 12, 2021 at 06:12:31PM +0100, Mickaël Salaün wrote:
> From: Mickaël Salaün <mic@linux.microsoft.com>
> 
> Factor out the blacklist hash creation with the get_raw_hash() helper.
> This also centralize the "tbs" and "bin" prefixes and make them private,
> which help to manage them consistently.
> 
> Cc: David Howells <dhowells@redhat.com>
> Cc: David S. Miller <davem@davemloft.net>
> Cc: David Woodhouse <dwmw2@infradead.org>
> Cc: Eric Snowberg <eric.snowberg@oracle.com>
> Cc: Herbert Xu <herbert@gondor.apana.org.au>
> Cc: Jarkko Sakkinen <jarkko@kernel.org>
> Signed-off-by: Mickaël Salaün <mic@linux.microsoft.com>
> Link: https://lore.kernel.org/r/20210312171232.2681989-5-mic@digikod.net


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

/Jarkko

> ---
> 
> Changes since v6:
> * Rebase on keys-cve-2020-26541-v3: commit ebd9c2ae369a ("integrity:
>   Load mokx variables into the blacklist keyring").
> 
> Changes since v5:
> * Rebase on keys-next and fix conflict as previously done by David
>   Howells.
> * Fix missing part to effectively handle UEFI DBX blacklisting.
> * Remove Jarkko's Acked-by because of the above changes.
> 
> Changes since v2:
> * Add Jarkko's Acked-by.
> ---
>  certs/blacklist.c                             | 76 ++++++++++++++-----
>  crypto/asymmetric_keys/x509_public_key.c      |  3 +-
>  include/keys/system_keyring.h                 | 14 +++-
>  .../platform_certs/keyring_handler.c          | 26 +------
>  4 files changed, 73 insertions(+), 46 deletions(-)
> 
> diff --git a/certs/blacklist.c b/certs/blacklist.c
> index 97a35cf9a62c..b254c87ceb3a 100644
> --- a/certs/blacklist.c
> +++ b/certs/blacklist.c
> @@ -109,11 +109,43 @@ static struct key_type key_type_blacklist = {
>  	.describe		= blacklist_describe,
>  };
>  
> +static char *get_raw_hash(const u8 *hash, size_t hash_len,
> +		enum blacklist_hash_type hash_type)
> +{
> +	size_t type_len;
> +	const char *type_prefix;
> +	char *buffer, *p;
> +
> +	switch (hash_type) {
> +	case BLACKLIST_HASH_X509_TBS:
> +		type_len = sizeof(tbs_prefix) - 1;
> +		type_prefix = tbs_prefix;
> +		break;
> +	case BLACKLIST_HASH_BINARY:
> +		type_len = sizeof(bin_prefix) - 1;
> +		type_prefix = bin_prefix;
> +		break;
> +	default:
> +		WARN_ON_ONCE(1);
> +		return ERR_PTR(-EINVAL);
> +	}
> +	buffer = kmalloc(type_len + 1 + hash_len * 2 + 1, GFP_KERNEL);
> +	if (!buffer)
> +		return ERR_PTR(-ENOMEM);
> +	p = memcpy(buffer, type_prefix, type_len);
> +	p += type_len;
> +	*p++ = ':';
> +	bin2hex(p, hash, hash_len);
> +	p += hash_len * 2;
> +	*p = '\0';
> +	return buffer;
> +}
> +
>  /**
> - * mark_hash_blacklisted - Add a hash to the system blacklist
> + * mark_raw_hash_blacklisted - Add a hash to the system blacklist
>   * @hash: The hash as a hex string with a type prefix (eg. "tbs:23aa429783")
>   */
> -int mark_hash_blacklisted(const char *hash)
> +static int mark_raw_hash_blacklisted(const char *hash)
>  {
>  	key_ref_t key;
>  
> @@ -133,29 +165,36 @@ int mark_hash_blacklisted(const char *hash)
>  	return 0;
>  }
>  
> +int mark_hash_blacklisted(const u8 *hash, size_t hash_len,
> +		enum blacklist_hash_type hash_type)
> +{
> +	const char *buffer;
> +	int err;
> +
> +	buffer = get_raw_hash(hash, hash_len, hash_type);
> +	if (IS_ERR(buffer))
> +		return PTR_ERR(buffer);
> +	err = mark_raw_hash_blacklisted(buffer);
> +	kfree(buffer);
> +	return err;
> +}
> +
>  /**
>   * is_hash_blacklisted - Determine if a hash is blacklisted
>   * @hash: The hash to be checked as a binary blob
>   * @hash_len: The length of the binary hash
> - * @type: Type of hash
> + * @hash_type: Type of hash
>   */
> -int is_hash_blacklisted(const u8 *hash, size_t hash_len, const char *type)
> +int is_hash_blacklisted(const u8 *hash, size_t hash_len,
> +		enum blacklist_hash_type hash_type)
>  {
>  	key_ref_t kref;
> -	size_t type_len = strlen(type);
> -	char *buffer, *p;
> +	const char *buffer;
>  	int ret = 0;
>  
> -	buffer = kmalloc(type_len + 1 + hash_len * 2 + 1, GFP_KERNEL);
> -	if (!buffer)
> -		return -ENOMEM;
> -	p = memcpy(buffer, type, type_len);
> -	p += type_len;
> -	*p++ = ':';
> -	bin2hex(p, hash, hash_len);
> -	p += hash_len * 2;
> -	*p = 0;
> -
> +	buffer = get_raw_hash(hash, hash_len, hash_type);
> +	if (IS_ERR(buffer))
> +		return PTR_ERR(buffer);
>  	kref = keyring_search(make_key_ref(blacklist_keyring, true),
>  			      &key_type_blacklist, buffer, false);
>  	if (!IS_ERR(kref)) {
> @@ -170,7 +209,8 @@ EXPORT_SYMBOL_GPL(is_hash_blacklisted);
>  
>  int is_binary_blacklisted(const u8 *hash, size_t hash_len)
>  {
> -	if (is_hash_blacklisted(hash, hash_len, "bin") == -EKEYREJECTED)
> +	if (is_hash_blacklisted(hash, hash_len, BLACKLIST_HASH_BINARY) ==
> +			-EKEYREJECTED)
>  		return -EPERM;
>  
>  	return 0;
> @@ -243,7 +283,7 @@ static int __init blacklist_init(void)
>  		panic("Can't allocate system blacklist keyring\n");
>  
>  	for (bl = blacklist_hashes; *bl; bl++)
> -		if (mark_hash_blacklisted(*bl) < 0)
> +		if (mark_raw_hash_blacklisted(*bl) < 0)
>  			pr_err("- blacklisting failed\n");
>  	return 0;
>  }
> diff --git a/crypto/asymmetric_keys/x509_public_key.c b/crypto/asymmetric_keys/x509_public_key.c
> index ae450eb8be14..3b7dba5e4cd9 100644
> --- a/crypto/asymmetric_keys/x509_public_key.c
> +++ b/crypto/asymmetric_keys/x509_public_key.c
> @@ -81,7 +81,8 @@ int x509_get_sig_params(struct x509_certificate *cert)
>  	if (ret < 0)
>  		goto error_2;
>  
> -	ret = is_hash_blacklisted(sig->digest, sig->digest_size, "tbs");
> +	ret = is_hash_blacklisted(sig->digest, sig->digest_size,
> +				  BLACKLIST_HASH_X509_TBS);
>  	if (ret == -EKEYREJECTED) {
>  		pr_err("Cert %*phN is blacklisted\n",
>  		       sig->digest_size, sig->digest);
> diff --git a/include/keys/system_keyring.h b/include/keys/system_keyring.h
> index 875e002a4180..d2597f8d6d7e 100644
> --- a/include/keys/system_keyring.h
> +++ b/include/keys/system_keyring.h
> @@ -10,6 +10,13 @@
>  
>  #include <linux/key.h>
>  
> +enum blacklist_hash_type {
> +	/* TBSCertificate hash */
> +	BLACKLIST_HASH_X509_TBS = 1,
> +	/* Raw data hash */
> +	BLACKLIST_HASH_BINARY = 2,
> +};
> +
>  #ifdef CONFIG_SYSTEM_TRUSTED_KEYRING
>  
>  extern int restrict_link_by_builtin_trusted(struct key *keyring,
> @@ -33,13 +40,14 @@ extern int restrict_link_by_builtin_and_secondary_trusted(
>  
>  extern struct pkcs7_message *pkcs7;
>  #ifdef CONFIG_SYSTEM_BLACKLIST_KEYRING
> -extern int mark_hash_blacklisted(const char *hash);
> +extern int mark_hash_blacklisted(const u8 *hash, size_t hash_len,
> +			       enum blacklist_hash_type hash_type);
>  extern int is_hash_blacklisted(const u8 *hash, size_t hash_len,
> -			       const char *type);
> +			       enum blacklist_hash_type hash_type);
>  extern int is_binary_blacklisted(const u8 *hash, size_t hash_len);
>  #else
>  static inline int is_hash_blacklisted(const u8 *hash, size_t hash_len,
> -				      const char *type)
> +				      enum blacklist_hash_type hash_type)
>  {
>  	return 0;
>  }
> diff --git a/security/integrity/platform_certs/keyring_handler.c b/security/integrity/platform_certs/keyring_handler.c
> index 5604bd57c990..9e4f156b356e 100644
> --- a/security/integrity/platform_certs/keyring_handler.c
> +++ b/security/integrity/platform_certs/keyring_handler.c
> @@ -15,35 +15,13 @@ static efi_guid_t efi_cert_x509_sha256_guid __initdata =
>  	EFI_CERT_X509_SHA256_GUID;
>  static efi_guid_t efi_cert_sha256_guid __initdata = EFI_CERT_SHA256_GUID;
>  
> -/*
> - * Blacklist a hash.
> - */
> -static __init void uefi_blacklist_hash(const char *source, const void *data,
> -				       size_t len, const char *type,
> -				       size_t type_len)
> -{
> -	char *hash, *p;
> -
> -	hash = kmalloc(type_len + len * 2 + 1, GFP_KERNEL);
> -	if (!hash)
> -		return;
> -	p = memcpy(hash, type, type_len);
> -	p += type_len;
> -	bin2hex(p, data, len);
> -	p += len * 2;
> -	*p = 0;
> -
> -	mark_hash_blacklisted(hash);
> -	kfree(hash);
> -}
> -
>  /*
>   * Blacklist an X509 TBS hash.
>   */
>  static __init void uefi_blacklist_x509_tbs(const char *source,
>  					   const void *data, size_t len)
>  {
> -	uefi_blacklist_hash(source, data, len, "tbs:", 4);
> +	mark_hash_blacklisted(data, len, BLACKLIST_HASH_X509_TBS);
>  }
>  
>  /*
> @@ -52,7 +30,7 @@ static __init void uefi_blacklist_x509_tbs(const char *source,
>  static __init void uefi_blacklist_binary(const char *source,
>  					 const void *data, size_t len)
>  {
> -	uefi_blacklist_hash(source, data, len, "bin:", 4);
> +	mark_hash_blacklisted(data, len, BLACKLIST_HASH_BINARY);
>  }
>  
>  /*
> -- 
> 2.30.2
> 
> 

  reply index

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-03-12 17:12 [PATCH v7 0/5] Enable root to update the blacklist keyring Mickaël Salaün
2021-03-12 17:12 ` [PATCH v7 1/5] tools/certs: Add print-cert-tbs-hash.sh Mickaël Salaün
2021-03-15 16:57   ` Eric Snowberg
2021-03-12 17:12 ` [PATCH v7 2/5] certs: Check that builtin blacklist hashes are valid Mickaël Salaün
2021-03-13 18:53   ` Jarkko Sakkinen
2021-03-12 17:12 ` [PATCH v7 3/5] certs: Make blacklist_vet_description() more strict Mickaël Salaün
2021-03-12 17:12 ` [PATCH v7 4/5] certs: Factor out the blacklist hash creation Mickaël Salaün
2021-03-13 18:54   ` Jarkko Sakkinen [this message]
2021-03-12 17:12 ` [PATCH v7 5/5] certs: Allow root user to append signed hashes to the blacklist keyring Mickaël Salaün
2021-03-15 16:59   ` Eric Snowberg
2021-03-15 18:01     ` Mickaël Salaün
2021-03-17 14:48       ` Eric Snowberg
2021-03-17 15:45         ` Mickaël Salaün
2021-03-25 11:36 ` [PATCH v7 0/5] Enable root to update " Mickaël Salaün
2021-04-07 17:21 ` Mickaël Salaün
2021-05-04 10:31   ` Mickaël Salaün

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=YE0KTAfElytCNh0M@kernel.org \
    --to=jarkko@kernel.org \
    --cc=davem@davemloft.net \
    --cc=dhowells@redhat.com \
    --cc=dwmw2@infradead.org \
    --cc=eric.snowberg@oracle.com \
    --cc=herbert@gondor.apana.org.au \
    --cc=jmorris@namei.org \
    --cc=keyrings@vger.kernel.org \
    --cc=linux-crypto@vger.kernel.org \
    --cc=linux-integrity@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-security-module@vger.kernel.org \
    --cc=mic@digikod.net \
    --cc=mic@linux.microsoft.com \
    --cc=serge@hallyn.com \
    --cc=tyhicks@linux.microsoft.com \
    --cc=zohar@linux.ibm.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

Linux-Security-Module Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-security-module/0 linux-security-module/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-security-module linux-security-module/ https://lore.kernel.org/linux-security-module \
		linux-security-module@vger.kernel.org
	public-inbox-index linux-security-module

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-security-module


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git