crypto: authencesn: reject short ahash digests during instance creation
A message that is not in the archive (sent off-list or not yet received): [email protected]
PATCH 1/1 crypto: authencesn: reject short ahash digests during instance creation
From: Yucheng Lu <[email protected]>
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.
--- crypto/authencesn.c | 5 +++++ 1 file changed, 5 insertions(+)
crypto/authencesn.c +5 -0
@@ -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; + } + err = -ENAMETOOLONG ; if (snprintf(inst->alg.base.cra_name, CRYPTO_MAX_ALG_NAME, "authencesn(%s,%s)", auth_base->cra_name,
Signature · 1 line
2.47.3
(cc Eric)
Quoted 40 lines · Ren Wei
From: Yucheng Lu <[email protected]>
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 copiedauth->digestsizeintoinst->alg.maxauthsizewithout 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 thoughsetauthsize()would have rejected the same value.AF_ALGcould 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")Reported-by Yuan Tan <[email protected]>Reported-by Yifan Wu <[email protected]>Reported-by Juefei Pu <[email protected]>Reported-by Xin Liu <[email protected]>Tested-by Yuhang Zheng <[email protected]>Signed-off-by Yucheng Lu <[email protected]>Signed-off-by Ren Wei <[email protected]>--- crypto/authencesn.c | 5 +++++ 1 file changed, 5 insertions(+)crypto/authencesn.c +0
-0
crypto/authencesn.c static int crypto_authenc_esn_create(struct · line 392
Quoted 9 lines
crypto/authencesn.c +5
-0@@ -384,6 +384,11 @@ static int crypto_authenc_esn_create(structcrypto_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?
Eric Biggers
Reviewed-byQuoted 9 lines · Ard Biesheuvel
Quoted 9 lines
crypto/authencesn.c +0
-0@@ -384,6 +384,11 @@ static int crypto_authenc_esn_create(structcrypto_template *tmpl, goto err_free_inst; enc =crypto_spawn_skcipher_alg_common (&ctx->enc);
Quoted 7 lines
crypto/authencesn.c static int crypto_authenc_esn_create(struct crypto_template *tmpl, · line 391
Quoted 5 lines
crypto/authencesn.c +5
-0+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.
Of course, while this patch needs to be applied, this also doesn't go nearly far enough.
https://lore.kernel.org/linux-crypto/[email protected]/
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
Herbert Xu
maintainerQuoted 36 lines · Ren Wei
From: Yucheng Lu <[email protected]>
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 copiedauth->digestsizeintoinst->alg.maxauthsizewithout 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 thoughsetauthsize()would have rejected the same value.AF_ALGcould 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")Reported-by Yifan Wu <[email protected]>Reported-by Juefei Pu <[email protected]>Co-developed-by Yuan Tan <[email protected]>Signed-off-by Yuan Tan <[email protected]>Suggested-by Xin Liu <[email protected]>Tested-by Yuhang Zheng <[email protected]>Reviewed-by Eric Biggers <[email protected]>Signed-off-by Yucheng Lu <[email protected]>Signed-off-by Ren Wei <[email protected]>---
changes in v2:
- move the short digest size check to immediately after
Quoted 7 lines
auth_base = &auth->base;- add Reviewed-by from Eric Biggers
- fix the [email protected] 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.
Signature · 3 lines
Email: Herbert Xu <[email protected]> Home Page: http://gondor.apana.org.au/~herbert/ PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt