Re: [PATCH v2] KEYS: trusted: Debugging as a feature

[Nayna Jain]

On 3/24/26 7:00 AM, Jarkko Sakkinen wrote:
> TPM_DEBUG, and other similar flags, are a non-standard way to specify a
> feature in Linux kernel.  Introduce CONFIG_TRUSTED_KEYS_DEBUG for
> trusted keys, and use it to replace these ad-hoc feature flags.
>
> Given that trusted keys debug dumps can contain sensitive data, harden
> the feature as follows:
>
> 1. In the Kconfig description postulate that pr_debug() statements must be
>     used.
> 2. Use pr_debug() statements in TPM 1.x driver to print the protocol dump.
>
> Traces, when actually needed, can be easily enabled by providing
> trusted.dyndbg='+p' in the kernel command-line.
>
> Cc: Srish Srinivasan <ssrish@xxxxxxxxxxxxx>
> Reported-by: Nayna Jain <nayna@xxxxxxxxxxxxx>
> Closes: https://lore.kernel.org/all/7f8b8478-5cd8-4d97-bfd0-341fd5cf10f9@xxxxxxxxxxxxx/
> Signed-off-by: Jarkko Sakkinen <jarkko@xxxxxxxxxx>
> ---
> v2:
> - Implement for all trusted keys backends.
> - Add HAVE_TRUSTED_KEYS_DEBUG as it is a good practice despite full
>    coverage.
> ---
>   include/keys/trusted-type.h               | 18 +++++-------
>   security/keys/trusted-keys/Kconfig        | 19 ++++++++++++
>   security/keys/trusted-keys/trusted_caam.c |  4 +--
>   security/keys/trusted-keys/trusted_tpm1.c | 36 +++++++++++------------
>   4 files changed, 46 insertions(+), 31 deletions(-)
>
> diff --git a/include/keys/trusted-type.h b/include/keys/trusted-type.h
> index 03527162613f..620a1f890b6b 100644
> --- a/include/keys/trusted-type.h
> +++ b/include/keys/trusted-type.h
> @@ -83,18 +83,16 @@ struct trusted_key_source {
>   
>   extern struct key_type key_type_trusted;
>   
> -#define TRUSTED_DEBUG 0
> -
> -#if TRUSTED_DEBUG
> +#ifdef CONFIG_TRUSTED_KEYS_DEBUG
>   static inline void dump_payload(struct trusted_key_payload *p)
>   {
> -	pr_info("key_len %d\n", p->key_len);
> -	print_hex_dump(KERN_INFO, "key ", DUMP_PREFIX_NONE,
> -		       16, 1, p->key, p->key_len, 0);
> -	pr_info("bloblen %d\n", p->blob_len);
> -	print_hex_dump(KERN_INFO, "blob ", DUMP_PREFIX_NONE,
> -		       16, 1, p->blob, p->blob_len, 0);
> -	pr_info("migratable %d\n", p->migratable);
> +	pr_debug("key_len %d\n", p->key_len);
> +	print_hex_dump_debug("key ", DUMP_PREFIX_NONE,
> +			     16, 1, p->key, p->key_len, 0);
> +	pr_debug("bloblen %d\n", p->blob_len);
> +	print_hex_dump_debug("blob ", DUMP_PREFIX_NONE,
> +			     16, 1, p->blob, p->blob_len, 0);
> +	pr_debug("migratable %d\n", p->migratable);
>   }
>   #else
>   static inline void dump_payload(struct trusted_key_payload *p)
> diff --git a/security/keys/trusted-keys/Kconfig b/security/keys/trusted-keys/Kconfig
> index 9e00482d886a..2ad9ba0e03f1 100644
> --- a/security/keys/trusted-keys/Kconfig
> +++ b/security/keys/trusted-keys/Kconfig
> @@ -1,10 +1,25 @@
>   config HAVE_TRUSTED_KEYS
>   	bool
>   
> +config HAVE_TRUSTED_KEYS_DEBUG
> +	bool
> +
> +config TRUSTED_KEYS_DEBUG
> +	bool "Debug trusted keys"
> +	depends on HAVE_TRUSTED_KEYS_DEBUG
> +	default n
> +	help
> +	  Trusted keys backends and core code that support debug dumps
> +	  can opt-in that feature here. Dumps must only use DEBUG
> +	  level output, as sensitive data may pass by. In the
> +	  kernel-command line traces can be enabled via
> +	  trusted.dyndbg='+p'.

Would it be good idea to add an explicit note/warning:


NOTE: This option is intended for debugging purposes only. Do not enable 
on production systems as debug output may expose sensitive cryptographic 
material.
If you are unsure, say N.

Apart from this, looks good to me.

Reviewed-by: Nayna Jain <nayna@xxxxxxxxxxxxx>