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

[Jarkko Sakkinen]

On Wed, Apr 08, 2026 at 11:27:01AM +0300, Jarkko Sakkinen wrote:
> 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@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>
> 
> 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 :-)

E.g., in 2026 world perfectly realistic scenario is "agentic devops
team" (unfortunately), which might debug trusted keys issue, and leave
debug flag on. Thus, both warning you suggested and also boot option
for good measure do actually leverage risks involved.

BR, Jarkko