2014-11-10 17:29 GMT+03:00 Miod Vallat <m...@online.fr>:
>> > 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.
Looking again I saw no obvious issues.
[...]
>> > + 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.
--
With best wishes
Dmitry
