Conversation
mkj
force-pushed
the
pr/curve25519-s-check
branch
2 times, most recently
from
938021b to
6065336
Compare
This is required by rfc8032 and avoids signature malleability (S' = S+L would also be accepted). That does not have any security impact on the way it is used in SSH protocol. The check was not present in original tweetnacl. Thanks to YuJie Zhu for the report. Fixes #406
mkj
force-pushed
the
pr/curve25519-s-check
branch
from
6065336 to
fdec3c9
Compare
| } | ||
| } | ||
| return -1; | ||
| } |
There was a problem hiding this comment.
hi Matt, maybe constant time branchless with carry? yes, it's verify time, but why not - it's only 32 bytes
and since we need only lt, ge case can be optimized out (in case of need ge == !lt)
static int lt25519_u8(const u8 *a, const u8 *b)
{
int i,r = 0;
FOR(i,32) r = (a[i] - b[i] - r) < 0;
return r;
}
There was a problem hiding this comment.
I think I prefer my one since it's simpler (I had to run them side by side to convince myself yours actually worked!)
There was a problem hiding this comment.
well, constant-time comparisons like this is quite generic crypto stuff and better fits TweetNaCl style i.e same constant-time crypto_verify_32(), anyway up2u.
btw, I took local TweetNaCl impl for dropbear because libtomcrypt 1.18.2 had no ed25519, maybe worth to update and drop local curve25519.* impl? libtomcrypt is TweetNaCl based, seems easy to migrate
https://github.com/libtom/libtomcrypt/tree/develop/src/pk/ed25519
| u8 t[32],h[64]; | ||
| gf p[4],q[4]; | ||
|
|
||
| if (slen < 64) return -1; |
There was a problem hiding this comment.
shouldn't s+32 be checked early?
There was a problem hiding this comment.
Do you mean call s_lt_l() here? Could do.
| { | ||
| gf t, chk, num, den, den2, den4, den6; | ||
| set25519(r[2],gf1); | ||
| unpack25519(r[1],p); |
There was a problem hiding this comment.
for issue #408 after unpacking with cleared sign, the only left is to compare r[1] here with
#if DROPBEAR_SIGNKEY_VERIFY
static const gf
P = {0xffed, 0xffff, 0xffff, 0xffff, 0xffff, 0xffff, 0xffff, 0xffff, 0xffff, 0xffff, 0xffff, 0xffff, 0xffff, 0xffff, 0xffff, 0x7fff},
with silimar constant-time approach it can be ~
static int lt25519(const gf a, const gf b)
{
i64 i,r = 0;
FOR(i,16) r = (a[i] - b[i] - r) < 0;
return r;
}
2 participants
This is required by rfc8032 and avoids signature malleability (s' = s+L would also be accepted). That does not have any security impact on the way it is used in SSH protocol.
The check was not present in original tweetnacl.
Thanks to YuJie Zhu for the report.
Fixes #406