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

[Jarkko Sakkinen <jarkko@kernel.org>]

On Mon, Apr 06, 2026 at 10:42:00PM -0400, Nayna Jain wrote:
> 
> 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@linux.ibm.com>
> > Reported-by: Nayna Jain <nayna@linux.ibm.com>
> > Closes: https://lore.kernel.org/all/7f8b8478-5cd8-4d97-bfd0-341fd5cf10f9@linux.ibm.com/
> > Signed-off-by: Jarkko Sakkinen <jarkko@kernel.org>
> > ---
> > 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@linux.ibm.com>

Thank, I'll add your tag but would you mind quickly screening v3 again
where I add "trusted.debug=0|1". And yes, your suggestion about extra
warning makes sense.

Let's make this safe as possible. Mistakes do happen... and then those
measures pay off :-)

BR, Jarkko