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

[Vitaly Chikunov <vt@altlinux.org>]

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