[openssl-dev] [openssl.org #4439] poly1305-x86.pl produces incorrect output

[David Benjamin via RT]

On Sun, Mar 20, 2016 at 10:47 PM David Benjamin <davidben at google.com> wrote:

> On Sun, Mar 20, 2016 at 5:05 PM Andy Polyakov via RT <rt at openssl.org>
> wrote:
>
>> No, it doesn't depend on call pattern. Please confirm that attached
>> patch solves the problem. Thanks.
>>
>
> (Right, sorry, I meant that the test vectors I have seem to only with
> their corresponding call patterns.)
>
> The patch works on my end, and naively comparing random inputs against a
> reference implementation doesn't reveal any other issues. Thanks for fixing
> it so quickly!
>

Andy, there appears to be a typo in the patch. It says defined(extra)
rather than defined($extra). It was evaluating a bare word and always using
paddq. The $extra version seems to work too, but may I suggest adding some
comments here?

If I'm understanding correctly, the paddd vs paddq decision is about
whether the sum fits in 2^32 rather than needing the full 2^64, right? And
you use paddd preferentially over paddq because paddq is slow on Atom? This
isn't very clear from "because paddq is "broken" on Atom". It's also no
longer next to where $paddx is computed.

Moreover, it seems lazy_reduction conditioning on $extra isn't because
$extra is in itself significant, but because $extra being set means we are
following the tail logic and a horizontal addition, so the bounds don't
hold anymore? This could do with a clear comment.

Finally, where paddd is used, it's probably worth a comment for why the
bounds hold and under what assumptions. I haven't been able to trace
through them myself (based on the paper, it looks like the result of the h4
-> h0 carry after the horizontal addition should be bound by 2^26 + 2^26 *
5 * 2 * 5 = 2^26 * 51, but looking in a debugger, it's larger, so clearly
I'm missing something), so I can't suggest any particular text.

David

PS: By the way, this typo would have been caught by use strict. Have you
all considered moving perlasm to be use strict clean?

-- 
Ticket here: http://rt.openssl.org/Ticket/Display.html?id=4439
Please log in as guest with password guest if prompted