[PATCH] crypto: ecrdsa - fix unknown OID check in ecrdsa_param_curve

[PATCH] crypto: ecrdsa - fix unknown OID check in ecrdsa_param_curve

[Thorsten Blum]

The ->curve_oid check in ecrdsa_param_curve() rejects the valid enum
value 0 (OID_id_dsa_with_sha1), but look_up_OID() returns OID__NR on
lookup failure. Compare ->curve_oid with OID__NR instead to ensure that
only unknown OIDs return -EINVAL.

Fixes: 0d7a78643f69 ("crypto: ecrdsa - add EC-RDSA (GOST 34.10) algorithm")
Signed-off-by: Thorsten Blum <thorsten.blum@linux.dev>
---
 crypto/ecrdsa.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/crypto/ecrdsa.c b/crypto/ecrdsa.c
index 2c0602f0cd40..0cd7eb367604 100644
--- a/crypto/ecrdsa.c
+++ b/crypto/ecrdsa.c
@@ -145,7 +145,7 @@ int ecrdsa_param_curve(void *context, size_t hdrlen, unsigned char tag,
 	struct ecrdsa_ctx *ctx = context;
 
 	ctx->curve_oid = look_up_OID(value, vlen);
-	if (!ctx->curve_oid)
+	if (ctx->curve_oid == OID__NR)
 		return -EINVAL;
 	ctx->curve = get_curve_by_oid(ctx->curve_oid);
 	return 0;

Re: [PATCH] crypto: ecrdsa - fix unknown OID check in ecrdsa_param_curve

[Lukas Wunner]

On Sat, May 02, 2026 at 09:09:04PM +0200, Thorsten Blum wrote:
> The ->curve_oid check in ecrdsa_param_curve() rejects the valid enum
> value 0 (OID_id_dsa_with_sha1), but look_up_OID() returns OID__NR on
> lookup failure. Compare ->curve_oid with OID__NR instead to ensure that
> only unknown OIDs return -EINVAL.
> 
> Fixes: 0d7a78643f69 ("crypto: ecrdsa - add EC-RDSA (GOST 34.10) algorithm")
> Signed-off-by: Thorsten Blum <thorsten.blum@linux.dev>

Reviewed-by: Lukas Wunner <lukas@wunner.de>

> +++ b/crypto/ecrdsa.c
> @@ -145,7 +145,7 @@ int ecrdsa_param_curve(void *context, size_t hdrlen, unsigned char tag,
>  	struct ecrdsa_ctx *ctx = context;
>  
>  	ctx->curve_oid = look_up_OID(value, vlen);
> -	if (!ctx->curve_oid)
> +	if (ctx->curve_oid == OID__NR)
>  		return -EINVAL;
>  	ctx->curve = get_curve_by_oid(ctx->curve_oid);
>  	return 0;

This is a fairly harmless logic bug:  OID_id_dsa_with_sha1 is not
a valid curve and so get_curve_by_oid() returns NULL, which is
assigned to ctx->curve.

The function you're changing, ecrdsa_param_curve(), is called
from the ecrdsa_params ASN.1 parser, which is invoked from
ecrdsa_set_pub_key().  That function does perform a NULL pointer
check for ctx->curve right after the ASN.1 parser returns.

Your patch will change the return value for an unknown OID from
-ENOPKG to -EINVAL, but that probably doesn't matter much.

Thanks,

Lukas

Re: [PATCH] crypto: ecrdsa - fix unknown OID check in ecrdsa_param_curve

[Vitaly Chikunov]

Lukas,

On Sat, May 02, 2026 at 10:08:12PM +0200, Lukas Wunner wrote:
> On Sat, May 02, 2026 at 09:09:04PM +0200, Thorsten Blum wrote:
> > The ->curve_oid check in ecrdsa_param_curve() rejects the valid enum
> > value 0 (OID_id_dsa_with_sha1), but look_up_OID() returns OID__NR on
> > lookup failure. Compare ->curve_oid with OID__NR instead to ensure that
> > only unknown OIDs return -EINVAL.
> > 
> > Fixes: 0d7a78643f69 ("crypto: ecrdsa - add EC-RDSA (GOST 34.10) algorithm")
> > Signed-off-by: Thorsten Blum <thorsten.blum@linux.dev>
> 
> Reviewed-by: Lukas Wunner <lukas@wunner.de>
> 
> > +++ b/crypto/ecrdsa.c
> > @@ -145,7 +145,7 @@ int ecrdsa_param_curve(void *context, size_t hdrlen, unsigned char tag,
> >  	struct ecrdsa_ctx *ctx = context;
> >  
> >  	ctx->curve_oid = look_up_OID(value, vlen);
> > -	if (!ctx->curve_oid)
> > +	if (ctx->curve_oid == OID__NR)
> >  		return -EINVAL;
> >  	ctx->curve = get_curve_by_oid(ctx->curve_oid);
> >  	return 0;
> 
> This is a fairly harmless logic bug:  OID_id_dsa_with_sha1 is not
> a valid curve and so get_curve_by_oid() returns NULL, which is
> assigned to ctx->curve.
> 
> The function you're changing, ecrdsa_param_curve(), is called
> from the ecrdsa_params ASN.1 parser, which is invoked from
> ecrdsa_set_pub_key().  That function does perform a NULL pointer
> check for ctx->curve right after the ASN.1 parser returns.
> 
> Your patch will change the return value for an unknown OID from
> -ENOPKG to -EINVAL, but that probably doesn't matter much.

Great review, thanks. Basically, the check is redundant.

Reviewed-by: Vitaly Chikunov <vt@altlinux.org>

Thanks,


> 
> Thanks,
> 
> Lukas