> > The following diff attempts to polish the GOST code in libcrypto and add
> > many missing error checks (probably not exhaustive, but a good start).
>
> I knew that I'm not perfect, but I didn't know the depth of my
> imperfectness...
Well, most of these shortcomings are also present in the `ccgost' engine
code. So, unless you wrote it almost 10 years ago, there is nothing to
be ashamed of.
> I will review your changes later with greater care.
Thanks.
> > I am also considering fixing the gost2001_do_sign() interface violation
> > (it frees something which has been allocated by its caller), if only to
> > make it match gost2001_do_verify(), which leaves the responsibility to
> > the caller; but that will happen in a later diff.
>
> That should be ok.
Note that in the original ccgost code, both do_sign() and do_verify()
free the `md' argument they receive.
> > + 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.
Miod
