crypto: caam - remove HMAC key hex dumps from hash_digest_key

[PATCH] crypto: caam - remove HMAC key hex dumps from hash_digest_key

Posted by Thorsten Blum 1 month ago

Stop dumping sensitive HMAC key bytes (original and reduced keys) in
hash_digest_key() to avoid leaking secrets when debug logging is
enabled.

Fixes: 045e36780f11 ("crypto: caam - ahash hmac support")
Fixes: 3f16f6c9d632 ("crypto: caam/qi2 - add support for ahash algorithms")
Cc: stable@vger.kernel.org
Signed-off-by: Thorsten Blum <thorsten.blum@linux.dev>
---
 drivers/crypto/caam/caamalg_qi2.c | 5 -----
 drivers/crypto/caam/caamhash.c    | 6 ------
 2 files changed, 11 deletions(-)

diff --git a/drivers/crypto/caam/caamalg_qi2.c b/drivers/crypto/caam/caamalg_qi2.c
index 167372936ca7..b4a6e4637b79 100644
--- a/drivers/crypto/caam/caamalg_qi2.c
+++ b/drivers/crypto/caam/caamalg_qi2.c
@@ -3269,8 +3269,6 @@ static int hash_digest_key(struct caam_hash_ctx *ctx, u32 *keylen, u8 *key,
 	dpaa2_fl_set_addr(out_fle, key_dma);
 	dpaa2_fl_set_len(out_fle, digestsize);
 
-	print_hex_dump_debug("key_in@" __stringify(__LINE__)": ",
-			     DUMP_PREFIX_ADDRESS, 16, 4, key, *keylen, 1);
 	print_hex_dump_debug("shdesc@" __stringify(__LINE__)": ",
 			     DUMP_PREFIX_ADDRESS, 16, 4, desc, desc_bytes(desc),
 			     1);
@@ -3289,9 +3287,6 @@ static int hash_digest_key(struct caam_hash_ctx *ctx, u32 *keylen, u8 *key,
 		/* in progress */
 		wait_for_completion(&result.completion);
 		ret = result.err;
-		print_hex_dump_debug("digested key@" __stringify(__LINE__)": ",
-				     DUMP_PREFIX_ADDRESS, 16, 4, key,
-				     digestsize, 1);
 	}
 
 	dma_unmap_single(ctx->dev, flc_dma, sizeof(flc->flc) + desc_bytes(desc),
diff --git a/drivers/crypto/caam/caamhash.c b/drivers/crypto/caam/caamhash.c
index 628c43a7efc4..70c252f167c5 100644
--- a/drivers/crypto/caam/caamhash.c
+++ b/drivers/crypto/caam/caamhash.c
@@ -393,8 +393,6 @@ static int hash_digest_key(struct caam_hash_ctx *ctx, u32 *keylen, u8 *key,
 	append_seq_store(desc, digestsize, LDST_CLASS_2_CCB |
 			 LDST_SRCDST_BYTE_CONTEXT);
 
-	print_hex_dump_debug("key_in@"__stringify(__LINE__)": ",
-			     DUMP_PREFIX_ADDRESS, 16, 4, key, *keylen, 1);
 	print_hex_dump_debug("jobdesc@"__stringify(__LINE__)": ",
 			     DUMP_PREFIX_ADDRESS, 16, 4, desc, desc_bytes(desc),
 			     1);
@@ -407,10 +405,6 @@ static int hash_digest_key(struct caam_hash_ctx *ctx, u32 *keylen, u8 *key,
 		/* in progress */
 		wait_for_completion(&result.completion);
 		ret = result.err;
-
-		print_hex_dump_debug("digested key@"__stringify(__LINE__)": ",
-				     DUMP_PREFIX_ADDRESS, 16, 4, key,
-				     digestsize, 1);
 	}
 	dma_unmap_single(jrdev, key_dma, *keylen, DMA_BIDIRECTIONAL);
 
-- 
Thorsten Blum <thorsten.blum@linux.dev>
GPG: 1D60 735E 8AEF 3BE4 73B6  9D84 7336 78FD 8DFE EAD4

Re: [PATCH] crypto: caam - remove HMAC key hex dumps from hash_digest_key

Posted by Herbert Xu 3 weeks, 5 days ago

On Fri, Mar 06, 2026 at 12:12:03PM +0100, Thorsten Blum wrote:
> Stop dumping sensitive HMAC key bytes (original and reduced keys) in
> hash_digest_key() to avoid leaking secrets when debug logging is
> enabled.
> 
> Fixes: 045e36780f11 ("crypto: caam - ahash hmac support")
> Fixes: 3f16f6c9d632 ("crypto: caam/qi2 - add support for ahash algorithms")
> Cc: stable@vger.kernel.org
> Signed-off-by: Thorsten Blum <thorsten.blum@linux.dev>
> ---
>  drivers/crypto/caam/caamalg_qi2.c | 5 -----
>  drivers/crypto/caam/caamhash.c    | 6 ------
>  2 files changed, 11 deletions(-)

What is the rationale for this? When debugging is enabled, all
sorts of things could be dumped, e.g., passwords.

Is there a scenario where production systems will run with debugging
enabled in caam?

Cheers,
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Re: [PATCH] crypto: caam - remove HMAC key hex dumps from hash_digest_key

Posted by Thorsten Blum 3 weeks, 2 days ago

On Sat, Mar 14, 2026 at 01:56:25PM +0900, Herbert Xu wrote:
> On Fri, Mar 06, 2026 at 12:12:03PM +0100, Thorsten Blum wrote:
> > Stop dumping sensitive HMAC key bytes (original and reduced keys) in
> > hash_digest_key() to avoid leaking secrets when debug logging is
> > enabled.
> > 
> > Fixes: 045e36780f11 ("crypto: caam - ahash hmac support")
> > Fixes: 3f16f6c9d632 ("crypto: caam/qi2 - add support for ahash algorithms")
> > Cc: stable@vger.kernel.org
> > Signed-off-by: Thorsten Blum <thorsten.blum@linux.dev>
> > ---
> >  drivers/crypto/caam/caamalg_qi2.c | 5 -----
> >  drivers/crypto/caam/caamhash.c    | 6 ------
> >  2 files changed, 11 deletions(-)
> 
> What is the rationale for this? When debugging is enabled, all
> sorts of things could be dumped, e.g., passwords.

This is not specifically about caam, but (debug) logging of potentially
sensitive key material should generally be avoided, imho. Some other
recent examples:

https://lore.kernel.org/lkml/20260227230008.858641-2-thorsten.blum@linux.dev/
https://lore.kernel.org/lkml/20260303132552.65235-2-thorsten.blum@linux.dev/
https://lore.kernel.org/lkml/20260303190350.78705-2-thorsten.blum@linux.dev/

> Is there a scenario where production systems will run with debugging
> enabled in caam?

I don't know - possibly.

Thanks,
Thorsten

Re: [PATCH] crypto: caam - remove HMAC key hex dumps from hash_digest_key

Posted by Herbert Xu 3 weeks, 1 day ago

On Tue, Mar 17, 2026 at 12:20:30PM +0100, Thorsten Blum wrote:
>
> This is not specifically about caam, but (debug) logging of potentially
> sensitive key material should generally be avoided, imho. Some other
> recent examples:
> 
> https://lore.kernel.org/lkml/20260227230008.858641-2-thorsten.blum@linux.dev/
> https://lore.kernel.org/lkml/20260303132552.65235-2-thorsten.blum@linux.dev/
> https://lore.kernel.org/lkml/20260303190350.78705-2-thorsten.blum@linux.dev/
> 
> > Is there a scenario where production systems will run with debugging
> > enabled in caam?
> 
> I don't know - possibly.

I think a better solution is to turn these sensitive printk's to
pr_devel.  That way you can still get them by recompiling the kernel
but they won't be enabled in any distro kernels.

What do you think?

Thanks,
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Re: [PATCH] crypto: caam - remove HMAC key hex dumps from hash_digest_key

Posted by Thorsten Blum 3 weeks, 1 day ago

On Wed, Mar 18, 2026 at 04:46:34PM +0900, Herbert Xu wrote:
> On Tue, Mar 17, 2026 at 12:20:30PM +0100, Thorsten Blum wrote:
> >
> > This is not specifically about caam, but (debug) logging of potentially
> > sensitive key material should generally be avoided, imho. Some other
> > recent examples:
> > 
> > https://lore.kernel.org/lkml/20260227230008.858641-2-thorsten.blum@linux.dev/
> > https://lore.kernel.org/lkml/20260303132552.65235-2-thorsten.blum@linux.dev/
> > https://lore.kernel.org/lkml/20260303190350.78705-2-thorsten.blum@linux.dev/
> > 
> > > Is there a scenario where production systems will run with debugging
> > > enabled in caam?
> > 
> > I don't know - possibly.
> 
> I think a better solution is to turn these sensitive printk's to
> pr_devel.  That way you can still get them by recompiling the kernel
> but they won't be enabled in any distro kernels.
> 
> What do you think?

Sounds reasonable. However, the code is already using the debug-gated
print_hex_dump_debug(), which can also be enabled at runtime with
CONFIG_DYNAMIC_DEBUG.

So I think the question is not necessarily print_hex_dump_debug() vs.
pr_devel(), but whether we want to:

- keep the debug-only hex dumps
- remove the sensitive dumps

My main concern is that with CONFIG_DYNAMIC_DEBUG enabled, which doesn't
require DEBUG, these raw key dumps can still be turned on at runtime in
a deployed kernel.

If we want to keep the dumps for debug-only kernels, then #ifdef DEBUG
plus print_hex_dump() might be a good compromise.

Re: [PATCH] crypto: caam - remove HMAC key hex dumps from hash_digest_key

Posted by Herbert Xu 3 weeks, 1 day ago

On Wed, Mar 18, 2026 at 01:02:11PM +0100, Thorsten Blum wrote:
>
> My main concern is that with CONFIG_DYNAMIC_DEBUG enabled, which doesn't
> require DEBUG, these raw key dumps can still be turned on at runtime in
> a deployed kernel.
> 
> If we want to keep the dumps for debug-only kernels, then #ifdef DEBUG
> plus print_hex_dump() might be a good compromise.

Exactly.  Having sensitive information printed with DYNAMIC_DEBUG
is arguably a problem, but putting them behind DEBUG is definitely
OK.

Thanks,
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Re: [PATCH] crypto: caam - remove HMAC key hex dumps from hash_digest_key

Posted by Thorsten Blum 3 weeks, 1 day ago

On Wed, Mar 18, 2026 at 09:16:02PM +0900, Herbert Xu wrote:
> On Wed, Mar 18, 2026 at 01:02:11PM +0100, Thorsten Blum wrote:
> >
> > My main concern is that with CONFIG_DYNAMIC_DEBUG enabled, which doesn't
> > require DEBUG, these raw key dumps can still be turned on at runtime in
> > a deployed kernel.
> > 
> > If we want to keep the dumps for debug-only kernels, then #ifdef DEBUG
> > plus print_hex_dump() might be a good compromise.
> 
> Exactly.  Having sensitive information printed with DYNAMIC_DEBUG
> is arguably a problem, but putting them behind DEBUG is definitely
> OK.

Ok, thanks.  I'll send a v2 soon using print_hex_dump() guarded by
#ifdef DEBUG.