code review 6219058: crypto/ecdsa: add full set of NIST test vectors.

crypto/ecdsa: add full set of NIST test vectors.

This includes the NIST test suite for ECDSA and alters the test to
parse and evaluate it.

[agl1, 2012-05-21 23:13:36]

Hello golang-dev@googlegroups.com (cc: golang-dev@googlegroups.com),

I'd like you to review this change to
https://code.google.com/p/go/

[agl1, 2012-05-21 23:15:25]

On Mon, May 21, 2012 at 7:13 PM,  <agl@golang.org> wrote:
> Description:
> crypto/ecdsa: add full set of NIST test vectors.

The binary file in this change is 100KB, which gives me pause.

I'm wondering whether it is better to add a test/ecdsa package in the
go.crypto subrepo instead. That keeps the binary out of the main tree,
at the cost of increasing the distance between the code and the tests.


Cheers

AGL

[bradfitz, 2012-05-21 23:22:38]

On Mon, May 21, 2012 at 4:15 PM, Adam Langley <agl@golang.org> wrote:

> On Mon, May 21, 2012 at 7:13 PM,  <agl@golang.org> wrote:
> > Description:
> > crypto/ecdsa: add full set of NIST test vectors.
>
> The binary file in this change is 100KB, which gives me pause.
>
> I'm wondering whether it is better to add a test/ecdsa package in the
> go.crypto subrepo instead. That keeps the binary out of the main tree,
> at the cost of increasing the distance between the code and the tests.
>

I personally don't care.  I think we have pictures of gophers larger than
that in the tree, and I'd argue tests for _crypto code_ are more important.

[rsc, 2012-05-22 02:24:36]

LGTM

100kB is fine.

Please use ReadString('\n').
You're simulating it with ReadLine instead.

[b, 2012-05-22 04:09:00]

On 2012/05/21 23:15:25, agl1 wrote:
> On Mon, May 21, 2012 at 7:13 PM,  <mailto:agl@golang.org> wrote:
> > Description:
> > crypto/ecdsa: add full set of NIST test vectors.
> 
> The binary file in this change is 100KB, which gives me pause.
> 
> I'm wondering whether it is better to add a test/ecdsa package in the
> go.crypto subrepo instead. That keeps the binary out of the main tree,
> at the cost of increasing the distance between the code and the tests.
> 
> 
> Cheers
> 
> AGL

The latest test vectors are at
http://csrc.nist.gov/groups/STM/cavp/documents/dss/186-3ecdsatestvectors.zip. 
Are you using those or the original ones?


b

[agl1, 2012-05-22 14:30:41]

On Mon, May 21, 2012 at 10:24 PM, Russ Cox <rsc@golang.org> wrote:
> Please use ReadString('\n').
> You're simulating it with ReadLine instead.

Done, although the file has \r\n line endings, which is why I was
using ReadLine. But it's all much the same.


Cheers

AGL

[agl1, 2012-05-22 14:32:31]

On Tue, May 22, 2012 at 12:09 AM,  <b@b3k.us> wrote:
> The latest test vectors are at
> http://csrc.nist.gov/groups/STM/cavp/documents/dss/186-3ecdsatestvectors.zip.
>  Are you using those or the original ones?

Yes, that's the source of the test vectors. There's a comment in the
test directing people there for the future.


Cheers

AGL

[agl1, 2012-05-22 14:33:23]

*** Submitted as http://code.google.com/p/go/source/detail?r=14d317d84389 ***

crypto/ecdsa: add full set of NIST test vectors.

This includes the NIST test suite for ECDSA and alters the test to
parse and evaluate it.

R=golang-dev, bradfitz, rsc, b
CC=golang-dev
http://codereview.appspot.com/6219058