> > Thanks.
>
> Looking again I saw no obvious issues.
\o/
> >> > + if (EC_GROUP_get_order(group, order, ctx) == 0) {
> >> > + /*
> >> > + * XXX EC_GROUP_get_order() will return 0 if successful
> >> > but
> >> > + * XXX order == 0. But then BN_mod below would fail
> >> > anyway.
> >> > + */
> >> > + goto err;
> >> > + }
> >>
> >> In theory it is fine to add
> >> if (BN_is_zero(order)) goto err;
> >>
> >> On the other hand no GOST curves/keys should have order == 0.
> >
> > Agreed. Which is why I did not add explicit BN_is_zero() checks, I think
> > this will not be necessary in practice. The comment is there to stress
> > the fact that the last reviewer is aware of this particular case.
>
> As this causes questions, it might be better to replace a comment with
> BN_is_zero() and be sure that there will be no further questions.
So do you prefer something like this:
if (EC_GROUP_get_order(group, order, ctx) == 0) {
if (!BN_is_zero(order))
goto err;
}
and let the computation fail at the next step?
However I am not sure we can trust the state of `order' if
EC_GROUP_get_order() fails before checking that order is zero.
I'll probably remove these XXX comments since they seem to produce more
confusion, which is not what I intended (-:
Miod
