Wycheproof failing ECC tests

[karel-m]

Let's have a valid signature like:

  0x30,0x45,0x02,0x20,0x2b,0xa3,0xa8,0xbe,0x6b,0x94,0xd5,0xec,0x80,0xa6,0xd9,0xd1,0x19,0x0a,
  0x43,0x6e,0xff,0xe5,0x0d,0x85,0xa1,0xee,0xe8,0x59,0xb8,0xcc,0x6a,0xf9,0xbd,0x5c,0x2e,0x18,
  0x02,0x21,0x00,0xb3,0x29,0xf4,0x79,0xa2,0xbb,0xd0,0xa5,0xc3,0x84,0xee,0x14,0x93,0xb1,0xf5,
  0x18,0x6a,0x87,0x13,0x9c,0xac,0x5d,0xf4,0x08,0x7c,0x13,0x4b,0x49,0x15,0x68,0x47,0xdb,

Now let's patch the first 0x30 to 0x31 like:

  0x31,0x45,0x02,0x20,0x2b,0xa3,0xa8,0xbe,0x6b,0x94,0xd5,0xec,0x80,0xa6,0xd9,0xd1,0x19,0x0a,
  0x43,0x6e,0xff,0xe5,0x0d,0x85,0xa1,0xee,0xe8,0x59,0xb8,0xcc,0x6a,0xf9,0xbd,0x5c,0x2e,0x18,
  0x02,0x21,0x00,0xb3,0x29,0xf4,0x79,0xa2,0xbb,0xd0,0xa5,0xc3,0x84,0xee,0x14,0x93,0xb1,0xf5,
  0x18,0x6a,0x87,0x13,0x9c,0xac,0x5d,0xf4,0x08,0x7c,0x13,0x4b,0x49,0x15,0x68,0x47,0xdb,

We still verify the patched signature whereas wycheproof says that the patched signature should be considered invalid.

This PR contains just a failing test not the fix.

[karel-m]

I have added one more wycheproof failing test labeled "Edge case for Shamir multiplication" - in this case we reject a signature that is valid (which is less dangerous).

[karel-m]

Added to more cases (both ASN.1 length encoding) when we accept signature that wycheproof guys consider invalid.

[sjaeckel]

I tried to look into these failing cases, but haven't been able to find any documentation on what these test cases exactly do. Does anyone else have a hint?

[levitte]

Gods, this has been ages, hasn't it? I can try to have a look in the coming days...

[sjaeckel]

Gods, this has been ages, hasn't it?

Absolutely!

I can try to have a look in the coming days...

Absolutely 2 :)

[levitte]

I tried to look into these failing cases, but haven't been able to find any documentation on what these test cases exactly do. Does anyone else have a hint?

I had a look... and first of all, when I ran the tests of this PR, they all succeed, so not sure what's supposedly failing.

As for the INVALID test vectors that are added in this PR, they seem to be correct (i.e. they are, in fact, invalid, and the test does detect that they fail as they should).

So er, what was the question?

BTW, for anyone that needs a link to wycheproof data, here it is: https://github.com/C2SP/wycheproof/tree/master/testvectors
This PR only adds a select few test vectors, not the whole lot. The wycheproof/ecdsa_test reference in the comments is this file: https://github.com/C2SP/wycheproof/blob/master/testvectors/ecdsa_test.json

[levitte]

As far as I'm concerned, this PR can be merged as is.

For the future, it might be a good thing to consider generating a separate C source file with just data, using the wycheproof JSON files as input. That's gonna be a biiiig source file, though.

[sjaeckel]

@karel-m please correct me if I'm wrong

FTR I've just force-pushed again, because I accidentally wiped my last changes by pushing an old version from my second machine ...

I had a look... and first of all, when I ran the tests of this PR, they all succeed, so not sure what's supposedly failing.

As for the INVALID test vectors that are added in this PR, they seem to be correct (i.e. they are, in fact, invalid, and the test does detect that they fail as they should).

So er, what was the question?

If I run the tests they pass, that's true, but I get this:

$ make CFLAGS="-DLTM_DESC -DUSE_LTM -I../libtommath" EXTRALIBS="../libtommath/libtommath.a" test -j65
[...]
$ ./test ecc
[...]
ecc_test............XXX-TODO should be valid - wycheproof / Edge case for Shamir multiplication
XXX-TODO should be valid - wycheproof / extreme value for k and edgecase s
XXX-TODO should be valid - wycheproof / extreme value for k
XXX-TODO should be valid - wycheproof / extreme value for k and s^-1
passed   8090.779ms
[...]

So these tests should succeed, but we don't accept those signatures because something's wrong in ltc_ecc_mul2add().

For the future, it might be a good thing to consider generating a separate C source file with just data, using the wycheproof JSON files as input. That's gonna be a biiiig source file, though.

IIRC perl-CryptX already does all those tests. I'm not sure whether it'd really make sense to add all those tests here, or whether it'd be better to contribute to perl-CryptX and use it as kind of "test harness".

[levitte]

Argh! I build with CMake, and I just discovered that I have to set MPI_PROVIDER right to get the tests that use MPI to run at all. When I did ctest --test-dir _build --verbose (I did without --verbose before), I got to see that ecc_test was a "nop".

So here I go again, with better build setup. Let's see what I get to see this time around

[levitte]

I built against GMP, and I only get these:

1: ecc_test............XXX-TODO should be valid - wycheproof / extreme value for k and edgecase s
1: XXX-TODO should be valid - wycheproof / extreme value for k
1: passed   7298.617ms

So it could be that at least some of the issues are bound to the math library. I'll dive into that this evening.

[sjaeckel]

FYI if you want to you can also build against all 3 MPI's ¹ and run the tests.

For me it looks as follows:

$ cmake -DWITH_LTM=On -DWITH_TFM=On -DWITH_GMP=On -DBUILD_TESTING=On ..
[...]
$ make -j$(($(nproc)*2+1))
[...]
$ for mpi in ltm gmp tfm; do ./tests/test-ltc ecc $mpi | tail -n7; done
XXX-TODO should be valid - wycheproof / Edge case for Shamir multiplication
XXX-TODO should be valid - wycheproof / extreme value for k and edgecase s
XXX-TODO should be valid - wycheproof / extreme value for k
XXX-TODO should be valid - wycheproof / extreme value for k and s^-1
MP_PROVIDER  = LibTomMath
MP_DIGIT_BIT = 60
sizeof(ltc_mp_digit) = 8

ecc_test............passed   3171.687ms

SUCCESS: passed=1 failed=0 nop=0 duration=3.2sec real=3.2sec
XXX-TODO should be valid - wycheproof / extreme value for k and edgecase s
XXX-TODO should be valid - wycheproof / extreme value for k
MP_PROVIDER  = GNU MP
MP_DIGIT_BIT = 64
sizeof(ltc_mp_digit) = 8

ecc_test............passed   2633.805ms

SUCCESS: passed=1 failed=0 nop=0 duration=2.6sec real=2.6sec
Non-fatal 'no-operation' requested. (2)
/path/to/libtomcrypt/tests/ecc_test.c:1792:s_ecc_import_export()
XXX-TODO should be valid - wycheproof / Edge case for Shamir multiplication
XXX-TODO should be valid - wycheproof / extreme value for k and edgecase s
XXX-TODO should be valid - wycheproof / extreme value for k
XXX-TODO should be valid - wycheproof / extreme value for k and s^-1
Non-fatal 'no-operation' requested. (2)
/path/to/libtomcrypt/tests/ecc_test.c:1800:s_ecc_test_recovery()
MP_PROVIDER  = TomsFastMath
MP_DIGIT_BIT = 64
sizeof(ltc_mp_digit) = 8

ecc_test............passed   1795.282ms

SUCCESS: passed=1 failed=0 nop=0 duration=1.8sec real=1.8sec

So this indeed looks like the first and last failing tests are caused by something inside ltm (CC @czurnieden) resp. tfm, and the two other failing tests are universal ...

Footnotes

1. both ltm and tfm provide CMake projects which are exported to the CMake registry, so you can simply clone the repos and build them in CMake, and from then on the ltc CMake project will find those builds automagically. ↩