* Re: [PATCH 1/1] crypto: authencesn: reject short ahash digests during instance creation
2026-04-20 8:48 ` [PATCH 1/1] crypto: authencesn: reject short ahash digests during instance creation Ren Wei
@ 2026-04-20 9:21 ` Ard Biesheuvel
2026-04-21 21:27 ` Eric Biggers
2026-04-22 13:45 ` [PATCH v2 " Ren Wei
2026-04-23 5:46 ` Herbert Xu
2 siblings, 1 reply; 5+ messages in thread
From: Ard Biesheuvel @ 2026-04-20 9:21 UTC (permalink / raw)
To: Ren Wei, linux-crypto, Eric Biggers
Cc: Herbert Xu, davem, yuantan098, yifanwucs, tomapufckgml, bird,
z1652074432, kanolyc
(cc Eric)
On Mon, 20 Apr 2026, at 10:48, Ren Wei wrote:
> From: Yucheng Lu <kanolyc@gmail.com>
>
> authencesn requires either a zero authsize or an authsize of at least
> 4 bytes because the ESN encrypt/decrypt paths always move 4 bytes of
> high-order sequence number data at the end of the authenticated data.
>
> While crypto_authenc_esn_setauthsize() already rejects explicit
> non-zero authsizes in the range 1..3, crypto_authenc_esn_create()
> still copied auth->digestsize into inst->alg.maxauthsize without
> validating it. The AEAD core then initialized the tfm's default
> authsize from that value.
>
> As a result, selecting an ahash with digest size 1..3, such as
> cbcmac(cipher_null), exposed authencesn instances whose default
> authsize was invalid even though setauthsize() would have rejected the
> same value. AF_ALG could then trigger the ESN tail handling with a
> too-short tag and hit an out-of-bounds access.
>
> Reject authencesn instances whose ahash digest size is in the invalid
> non-zero range 1..3 so that no tfm can inherit an unsupported default
> authsize.
>
> Fixes: f15f05b0a5de ("crypto: ccm - switch to separate cbcmac driver")
> Cc: stable@kernel.org
> Reported-by: Yuan Tan <yuantan098@gmail.com>
> Reported-by: Yifan Wu <yifanwucs@gmail.com>
> Reported-by: Juefei Pu <tomapufckgml@gmail.com>
> Reported-by: Xin Liu <bird@lzu.edu.cn>
> Tested-by: Yuhang Zheng <z1652074432@gmail.com>
> Signed-off-by: Yucheng Lu <kanolyc@gmail.com>
> Signed-off-by: Ren Wei <n05ec@lzu.edu.cn>
> ---
> crypto/authencesn.c | 5 +++++
> 1 file changed, 5 insertions(+)
>
> diff --git a/crypto/authencesn.c b/crypto/authencesn.c
> index 542a978663b9..bf44f035f7f8 100644
> --- a/crypto/authencesn.c
> +++ b/crypto/authencesn.c
> @@ -384,6 +384,11 @@ static int crypto_authenc_esn_create(struct
> crypto_template *tmpl,
> goto err_free_inst;
> enc = crypto_spawn_skcipher_alg_common(&ctx->enc);
>
> + if (auth->digestsize > 0 && auth->digestsize < 4) {
> + err = -EINVAL;
> + goto err_free_inst;
> + }
> +
Is this the best place for this check?
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH 1/1] crypto: authencesn: reject short ahash digests during instance creation
2026-04-20 9:21 ` Ard Biesheuvel
@ 2026-04-21 21:27 ` Eric Biggers
0 siblings, 0 replies; 5+ messages in thread
From: Eric Biggers @ 2026-04-21 21:27 UTC (permalink / raw)
To: Ard Biesheuvel
Cc: Ren Wei, linux-crypto, Herbert Xu, davem, yuantan098, yifanwucs,
tomapufckgml, bird, z1652074432, kanolyc
On Mon, Apr 20, 2026 at 11:21:54AM +0200, Ard Biesheuvel wrote:
> > diff --git a/crypto/authencesn.c b/crypto/authencesn.c
> > index 542a978663b9..bf44f035f7f8 100644
> > --- a/crypto/authencesn.c
> > +++ b/crypto/authencesn.c
> > @@ -384,6 +384,11 @@ static int crypto_authenc_esn_create(struct
> > crypto_template *tmpl,
> > goto err_free_inst;
> > enc = crypto_spawn_skcipher_alg_common(&ctx->enc);
> >
> > + if (auth->digestsize > 0 && auth->digestsize < 4) {
> > + err = -EINVAL;
> > + goto err_free_inst;
> > + }
> > +
>
> Is this the best place for this check?
I probably would have put it a few lines earlier, right after the line
'auth_base = &auth->base;'. But this works too.
Reviewed-by: Eric Biggers <ebiggers@kernel.org>
Of course, while this patch needs to be applied, this also doesn't go
nearly far enough.
https://lore.kernel.org/linux-crypto/20260420094120.5167-1-ardb@kernel.org/
removes the so-called "cipher_null", which has no reason to exist.
But "authencesn" itself should not be exposed to AF_ALG, let alone exist
in its current form at all. The IPsec sequence numbers should just be
handled internally in the IPsec code itself. That would be simpler and
more efficient, with much less UAPI surface as well.
- Eric
^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH v2 1/1] crypto: authencesn: reject short ahash digests during instance creation
2026-04-20 8:48 ` [PATCH 1/1] crypto: authencesn: reject short ahash digests during instance creation Ren Wei
2026-04-20 9:21 ` Ard Biesheuvel
@ 2026-04-22 13:45 ` Ren Wei
2026-04-23 5:46 ` Herbert Xu
2 siblings, 0 replies; 5+ messages in thread
From: Ren Wei @ 2026-04-22 13:45 UTC (permalink / raw)
To: linux-crypto
Cc: herbert, davem, ardb, yifanwucs, tomapufckgml, yuantan098, bird,
z1652074432, ebiggers, kanolyc, n05ec
From: Yucheng Lu <kanolyc@gmail.com>
authencesn requires either a zero authsize or an authsize of at least
4 bytes because the ESN encrypt/decrypt paths always move 4 bytes of
high-order sequence number data at the end of the authenticated data.
While crypto_authenc_esn_setauthsize() already rejects explicit
non-zero authsizes in the range 1..3, crypto_authenc_esn_create()
still copied auth->digestsize into inst->alg.maxauthsize without
validating it. The AEAD core then initialized the tfm's default
authsize from that value.
As a result, selecting an ahash with digest size 1..3, such as
cbcmac(cipher_null), exposed authencesn instances whose default
authsize was invalid even though setauthsize() would have rejected the
same value. AF_ALG could then trigger the ESN tail handling with a
too-short tag and hit an out-of-bounds access.
Reject authencesn instances whose ahash digest size is in the invalid
non-zero range 1..3 so that no tfm can inherit an unsupported default
authsize.
Fixes: f15f05b0a5de ("crypto: ccm - switch to separate cbcmac driver")
Cc: stable@kernel.org
Reported-by: Yifan Wu <yifanwucs@gmail.com>
Reported-by: Juefei Pu <tomapufckgml@gmail.com>
Co-developed-by: Yuan Tan <yuantan098@gmail.com>
Signed-off-by: Yuan Tan <yuantan098@gmail.com>
Suggested-by: Xin Liu <bird@lzu.edu.cn>
Tested-by: Yuhang Zheng <z1652074432@gmail.com>
Reviewed-by: Eric Biggers <ebiggers@kernel.org>
Signed-off-by: Yucheng Lu <kanolyc@gmail.com>
Signed-off-by: Ren Wei <n05ec@lzu.edu.cn>
---
changes in v2:
- move the short digest size check to immediately after
auth_base = &auth->base;
- add Reviewed-by from Eric Biggers
- fix the stable@kernel.org address typo
- Link: https://lore.kernel.org/all/cb1188757edab9b056961d4d2441be009ac73ce8.1775217403.git.kanolyc@gmail.com/
crypto/authencesn.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/crypto/authencesn.c b/crypto/authencesn.c
index 542a978663b9..f6ac9eefc7d9 100644
--- a/crypto/authencesn.c
+++ b/crypto/authencesn.c
@@ -378,6 +378,11 @@ static int crypto_authenc_esn_create(struct crypto_template *tmpl,
auth = crypto_spawn_ahash_alg(&ctx->auth);
auth_base = &auth->base;
+ if (auth->digestsize > 0 && auth->digestsize < 4) {
+ err = -EINVAL;
+ goto err_free_inst;
+ }
+
err = crypto_grab_skcipher(&ctx->enc, aead_crypto_instance(inst),
crypto_attr_alg_name(tb[2]), 0, mask);
if (err)
--
2.47.3
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [PATCH v2 1/1] crypto: authencesn: reject short ahash digests during instance creation
2026-04-20 8:48 ` [PATCH 1/1] crypto: authencesn: reject short ahash digests during instance creation Ren Wei
2026-04-20 9:21 ` Ard Biesheuvel
2026-04-22 13:45 ` [PATCH v2 " Ren Wei
@ 2026-04-23 5:46 ` Herbert Xu
2 siblings, 0 replies; 5+ messages in thread
From: Herbert Xu @ 2026-04-23 5:46 UTC (permalink / raw)
To: Ren Wei
Cc: linux-crypto, davem, ardb, yifanwucs, tomapufckgml, yuantan098,
bird, z1652074432, ebiggers, kanolyc
On Wed, Apr 22, 2026 at 09:45:04PM +0800, Ren Wei wrote:
> From: Yucheng Lu <kanolyc@gmail.com>
>
> authencesn requires either a zero authsize or an authsize of at least
> 4 bytes because the ESN encrypt/decrypt paths always move 4 bytes of
> high-order sequence number data at the end of the authenticated data.
>
> While crypto_authenc_esn_setauthsize() already rejects explicit
> non-zero authsizes in the range 1..3, crypto_authenc_esn_create()
> still copied auth->digestsize into inst->alg.maxauthsize without
> validating it. The AEAD core then initialized the tfm's default
> authsize from that value.
>
> As a result, selecting an ahash with digest size 1..3, such as
> cbcmac(cipher_null), exposed authencesn instances whose default
> authsize was invalid even though setauthsize() would have rejected the
> same value. AF_ALG could then trigger the ESN tail handling with a
> too-short tag and hit an out-of-bounds access.
>
> Reject authencesn instances whose ahash digest size is in the invalid
> non-zero range 1..3 so that no tfm can inherit an unsupported default
> authsize.
>
> Fixes: f15f05b0a5de ("crypto: ccm - switch to separate cbcmac driver")
> Cc: stable@kernel.org
> Reported-by: Yifan Wu <yifanwucs@gmail.com>
> Reported-by: Juefei Pu <tomapufckgml@gmail.com>
> Co-developed-by: Yuan Tan <yuantan098@gmail.com>
> Signed-off-by: Yuan Tan <yuantan098@gmail.com>
> Suggested-by: Xin Liu <bird@lzu.edu.cn>
> Tested-by: Yuhang Zheng <z1652074432@gmail.com>
> Reviewed-by: Eric Biggers <ebiggers@kernel.org>
> Signed-off-by: Yucheng Lu <kanolyc@gmail.com>
> Signed-off-by: Ren Wei <n05ec@lzu.edu.cn>
> ---
> changes in v2:
> - move the short digest size check to immediately after
> auth_base = &auth->base;
> - add Reviewed-by from Eric Biggers
> - fix the stable@kernel.org address typo
> - Link: https://lore.kernel.org/all/cb1188757edab9b056961d4d2441be009ac73ce8.1775217403.git.kanolyc@gmail.com/
>
> crypto/authencesn.c | 5 +++++
> 1 file changed, 5 insertions(+)
Patch applied. 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
^ permalink raw reply [flat|nested] 5+ messages in thread