crypto: special case ∞+a, a+∞ and a+a in p224.

crypto: special case ∞+a, a+∞ and a+a in p224.

In unrelated work, I found that the group addition formula used in p224.cc
doesn't work when one of the arguments is the point at infinity. This change
catches that case and simplifies the ScalarMult loop as a consequence.

In the course of doing this, I found a couple of bugs in Contract that would
have produced the wrong answer is very rare cases.

I also added a catch for a+a. This can't happen in the ScalarMult loop, but it
could happen from SPAKE2 at a rate of 1 in ~2**220 evaluations.

BUG=none
TEST=crypto_unittests


Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=148815

[willchan no longer on Chromium, 2012-07-25 23:35:26]

Not that I know what this code really does, but LGTM.

http://codereview.chromium.org/10822019/diff/1/crypto/p224.cc
File crypto/p224.cc (right):

http://codereview.chromium.org/10822019/diff/1/crypto/p224.cc#newcode359
crypto/p224.cc:359: uint32 top4AllOnes = 0xffffffffu;
I know you're going to hate me but...
nit: Top4AllOnes is proper camel casing naming style in Chromium.

http://codereview.chromium.org/10822019/diff/1/crypto/p224.cc#newcode374
crypto/p224.cc:374: uint32 bottom3NonZero = out[0] | out[1] | out[2];
Ditto here on CamelCase style.

http://codereview.chromium.org/10822019/diff/1/crypto/p224.cc#newcode424
crypto/p224.cc:424: void CopyConditional(Point* out, const Point& a, uint32
mask);
Google style is to put output parameters last. That said, you seem to have
flouted this Google style rule in the whole file, so it's best that you stay
consistently inconsistent :)

http://codereview.chromium.org/10822019/diff/1/crypto/p224.cc#newcode683
crypto/p224.cc:683: char zeros[56];
Why isn't this just a static const? It'd POD, so the linker should initialize it
to 0. That said, I guess this is only used by tests, so it's not perf critical.
So whatever.

[agl, 2012-07-27 17:21:06]

https://chromiumcodereview.appspot.com/10822019/diff/1/crypto/p224.cc
File crypto/p224.cc (right):

https://chromiumcodereview.appspot.com/10822019/diff/1/crypto/p224.cc#newcode359
crypto/p224.cc:359: uint32 top4AllOnes = 0xffffffffu;
On 2012/07/25 23:35:26, willchan wrote:
> I know you're going to hate me but...
> nit: Top4AllOnes is proper camel casing naming style in Chromium.

No, that's my screw up. I debugged this code in Go and translated across. I've
changed to top_4_all_ones as its a local variable.

https://chromiumcodereview.appspot.com/10822019/diff/1/crypto/p224.cc#newcode374
crypto/p224.cc:374: uint32 bottom3NonZero = out[0] | out[1] | out[2];
On 2012/07/25 23:35:26, willchan wrote:
> Ditto here on CamelCase style.

Done.

https://chromiumcodereview.appspot.com/10822019/diff/1/crypto/p224.cc#newcode424
crypto/p224.cc:424: void CopyConditional(Point* out, const Point& a, uint32
mask);
On 2012/07/25 23:35:26, willchan wrote:
> Google style is to put output parameters last. That said, you seem to have
> flouted this Google style rule in the whole file, so it's best that you stay
> consistently inconsistent :)

Well, I'm just being consistent with all the memcpys and every other system
function :)

https://chromiumcodereview.appspot.com/10822019/diff/1/crypto/p224.cc#newcode683
crypto/p224.cc:683: char zeros[56];
On 2012/07/25 23:35:26, willchan wrote:
> Why isn't this just a static const?

Good point. Done.

[commit-bot: I haz the power, 2012-07-27 17:22:12]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/agl@chromium.org/10822019/6001

[commit-bot: I haz the power, 2012-07-27 17:45:41]

Try job failure for 10822019-6001 (retry) on win_rel for step "compile" (clobber
build).
It's a second try, previously, step "compile" failed.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win_rel&nu...

[commit-bot: I haz the power, 2012-07-27 17:49:57]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/agl@chromium.org/10822019/6003

[commit-bot: I haz the power, 2012-07-27 18:36:39]

Try job failure for 10822019-6003 (retry) on win for step "compile" (clobber
build).
It's a second try, previously, step "compile" failed.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win&number...

[commit-bot: I haz the power, 2012-07-27 19:03:31]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/agl@chromium.org/10822019/2006

[commit-bot: I haz the power, 2012-07-27 21:25:25]

Change committed as 148815