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

On 4/8/26 4:26 AM, 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 <[email protected]>
>>> Reported-by: Nayna Jain <[email protected]>
>>> Closes: https://lore.kernel.org/all/[email protected]/
>>> Signed-off-by: Jarkko Sakkinen <[email protected]>
>>> ---
>>> 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 <[email protected]>
> 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.
Sure Jarkko.. However, I don't see v3 version in my inbox or in 
linux-integrity. Or you are about to post it soon.
>
> Let's make this safe as possible. Mistakes do happen... and then those
> measures pay off :-)
Yes agree.
>
> BR, Jarkko

Thanks & Regards,

     - Nayna