s390x: fix x448, add x448 test vector, ctime for x25519 and x448 #10339
Conversation
p-steuer
added
branch: master
levitte
changed the title
|
Nice description. It should end up in the commit messages, at least for the first and second commit |
The s390x x448 implementation does not correctly reduce non-canonical values i.e., u-coordinates >= p = 2^448 - 2^224 - 1. Signed-off-by: Patrick Steuer <[email protected]>
x25519 has such a test vector obtained from wycheproof but wycheproof does not have a corresponding x448 test vector. So add a self-generated test vector for that case. Signed-off-by: Patrick Steuer <[email protected]>
...in constant time. Signed-off-by: Patrick Steuer <[email protected]>
Done. I dont agree with you title change: Its actually x448 which has a bug (not x25519) and the x448 test vector is added to avoid regressions. |
p-steuer
changed the title
Totally fine. I misread it as "... x448 and x448..." instead of "... x448 and add x448...", so took it for a typo |
|
travis red corss not relevant |
crypto/ec/ecx_meth.c
Outdated
| a[i] ^= tmp; | ||
| b[i] ^= tmp; | ||
| } | ||
| } |
There was a problem hiding this comment.
It seems like this function belongs in constant_time.h. We already have uint32_t and uint64_t versions. This just adds another variant.
There was a problem hiding this comment.
okay, i will move it there.
| @@ -966,7 +981,7 @@ static int s390x_x448_mul(unsigned char u_dst[56], | |||
| memcpy(param.x448.d_src, d_src, 56); | |||
|
|
|||
| s390x_flip_endian64(param.x448.u_src, param.x448.u_src); | |||
| s390x_x448_mod_p(param.x448.u_src); | |||
| s390x_x448_mod_p(param.x448.u_src + 8); | |||
There was a problem hiding this comment.
I'm struggling to understand what's going on in this change and the other related change in your first commit. Can you explain in a bit more detail what is happening here?
There was a problem hiding this comment.
The structure of the parameter block the pcc instruction works on is reflected by the param union. The 56 byte values x448 operates on are stored right-aligned in its 64 byte fields. The x448_mod_p reduction function works on 56 bytes, therefore it must start on an 8 bytes offset into the buffer. The bug was that without the offset, the leading 8 bytes were always zero, so reduction never kicked in.
There was a problem hiding this comment.
The second bug in the reduction (which was obv. shadowed by the first bug) was that i should have checked one bit higher for overflow at the end of the reduction (check the least significant carry bit instead of most significant bit in most significant byte of u_red).
Signed-off-by: Patrick Steuer <[email protected]>
mattcaswell
added
approval: done
p-steuer
added
approval: ready to merge
The s390x x448 implementation does not correctly reduce non-canonical values i.e., u-coordinates >= p = 2^448 - 2^224 - 1. Signed-off-by: Patrick Steuer <[email protected]> Reviewed-by: Matt Caswell <[email protected]> (Merged from #10339)
x25519 has such a test vector obtained from wycheproof but wycheproof does not have a corresponding x448 test vector. So add a self-generated test vector for that case. Signed-off-by: Patrick Steuer <[email protected]> Reviewed-by: Matt Caswell <[email protected]> (Merged from #10339)
...in constant time. Signed-off-by: Patrick Steuer <[email protected]> Reviewed-by: Matt Caswell <[email protected]> (Merged from #10339)
x25519 has such a test vector obtained from wycheproof but wycheproof does not have a corresponding x448 test vector. So add a self-generated test vector for that case. Signed-off-by: Patrick Steuer <[email protected]> Reviewed-by: Matt Caswell <[email protected]> (Merged from openssl#10339)
x25519 has such a test vector obtained from wycheproof but wycheproof does not have a corresponding x448 test vector. So add a self-generated test vector for that case. Signed-off-by: Patrick Steuer <[email protected]> Reviewed-by: Matt Caswell <[email protected]> (Merged from openssl#10339) (cherry picked from commit fd60f8da74c68ba56f828bcc59141856503ffa0a) Signed-off-by: Dimitri John Ledkov <[email protected]>
x25519 has such a test vector obtained from wycheproof but wycheproof does not have a corresponding x448 test vector. So add a self-generated test vector for that case. Signed-off-by: Patrick Steuer <[email protected]> Reviewed-by: Matt Caswell <[email protected]> (Merged from openssl#10339) (cherry picked from commit fd60f8da74c68ba56f828bcc59141856503ffa0a) Signed-off-by: Dimitri John Ledkov <[email protected]>
The s390x x448 implementation does not correctly reduce non-canonical values, ie u-coordinates greater than or equal to p = 2^448 - 2^224 - 1. The first commit fixes this.
The bug went unnoticed because there is no test vector for that case. x25519 has such a test vector obtained from wycheproof but wycheproof does not have a corresponding x448 test vector. So the second commit adds a self-generated test vector for that case.
While we are at it, the third commit makes the processing of those non-canonical values constant time for x25519 and x448.