libsec: move zero check to curve25519_dh_finish()

As checking for all zero has to be done in a timing-safe
way to avoid a side channel, it is best todo this here
instead of letting the caller deal with it.

This adds a return type of int to curve25519_dh_finish()
where returning 0 means we got a all zero shared key.

RFC7748 states:

The check for the all-zero value results from the fact
that the X25519 function produces that value if it
operates on an input corresponding to a point with small
order, where the order divides the cofactor of the curve.
This commit is contained in:
cinap_lenrek 2021-06-20 14:41:26 +00:00
parent 6dd2c638b6
commit 57d95c7325
5 changed files with 10 additions and 14 deletions

View file

@ -583,7 +583,7 @@ void curve25519(uchar mypublic[32], uchar secret[32], uchar basepoint[32]);
/* Curve25519 diffie hellman */ /* Curve25519 diffie hellman */
void curve25519_dh_new(uchar x[32], uchar y[32]); void curve25519_dh_new(uchar x[32], uchar y[32]);
void curve25519_dh_finish(uchar x[32], uchar y[32], uchar z[32]); int curve25519_dh_finish(uchar x[32], uchar y[32], uchar z[32]);
/* password-based key derivation function 2 (rfc2898) */ /* password-based key derivation function 2 (rfc2898) */
void pbkdf2_x(uchar *p, ulong plen, uchar *s, ulong slen, ulong rounds, uchar *d, ulong dlen, void pbkdf2_x(uchar *p, ulong plen, uchar *s, ulong slen, ulong rounds, uchar *d, ulong dlen,

View file

@ -575,7 +575,7 @@ void curve25519(uchar mypublic[32], uchar secret[32], uchar basepoint[32]);
/* Curve25519 diffie hellman */ /* Curve25519 diffie hellman */
void curve25519_dh_new(uchar x[32], uchar y[32]); void curve25519_dh_new(uchar x[32], uchar y[32]);
void curve25519_dh_finish(uchar x[32], uchar y[32], uchar z[32]); int curve25519_dh_finish(uchar x[32], uchar y[32], uchar z[32]);
/* password-based key derivation function 2 (rfc2898) */ /* password-based key derivation function 2 (rfc2898) */
void pbkdf2_x(uchar *p, ulong plen, uchar *s, ulong slen, ulong rounds, uchar *d, ulong dlen, void pbkdf2_x(uchar *p, ulong plen, uchar *s, ulong slen, ulong rounds, uchar *d, ulong dlen,

View file

@ -600,7 +600,8 @@ Next1: switch(recvpkt()){
if((S = ssh2rsasig(sig, nsig)) == nil) if((S = ssh2rsasig(sig, nsig)) == nil)
sysfatal("bad server signature"); sysfatal("bad server signature");
curve25519_dh_finish(x, ys, z); if(!curve25519_dh_finish(x, ys, z))
sysfatal("unlucky shared key");
K = betomp(z, 32, nil); K = betomp(z, 32, nil);
nk = (mpsignif(K)+8)/8; nk = (mpsignif(K)+8)/8;

View file

@ -3,6 +3,7 @@
#include <libsec.h> #include <libsec.h>
static uchar nine[32] = {9}; static uchar nine[32] = {9};
static uchar zero[32] = {0};
void void
curve25519_dh_new(uchar x[32], uchar y[32]) curve25519_dh_new(uchar x[32], uchar y[32])
@ -20,7 +21,7 @@ curve25519_dh_new(uchar x[32], uchar y[32])
y[31] |= b & 0x80; y[31] |= b & 0x80;
} }
void int
curve25519_dh_finish(uchar x[32], uchar y[32], uchar z[32]) curve25519_dh_finish(uchar x[32], uchar y[32], uchar z[32])
{ {
/* remove the random bit */ /* remove the random bit */
@ -31,4 +32,6 @@ curve25519_dh_finish(uchar x[32], uchar y[32], uchar z[32])
memset(x, 0, 32); memset(x, 0, 32);
memset(y, 0, 32); memset(y, 0, 32);
return tsmemcmp(z, zero, 32) != 0;
} }

View file

@ -948,7 +948,6 @@ Out:
static Bytes* static Bytes*
tlsSecECDHEc(TlsSec *sec, int curve, Bytes *Ys) tlsSecECDHEc(TlsSec *sec, int curve, Bytes *Ys)
{ {
static char zero[32] = {0};
ECdomain *dom = &sec->ec.dom; ECdomain *dom = &sec->ec.dom;
ECpriv *Q = &sec->ec.Q; ECpriv *Q = &sec->ec.Q;
ECpub *pub; ECpub *pub;
@ -967,10 +966,7 @@ tlsSecECDHEc(TlsSec *sec, int curve, Bytes *Ys)
Yc = newbytes(32); Yc = newbytes(32);
curve25519_dh_new(sec->X, Yc->data); curve25519_dh_new(sec->X, Yc->data);
Z = newbytes(32); Z = newbytes(32);
curve25519_dh_finish(sec->X, Ys->data, Z->data); if(!curve25519_dh_finish(sec->X, Ys->data, Z->data)){
// rfc wants us to terminate the connection if
// shared secret == all zeroes.
if(tsmemcmp(Z->data, zero, Z->len) == 0){
freebytes(Yc); freebytes(Yc);
freebytes(Z); freebytes(Z);
return nil; return nil;
@ -2573,7 +2569,6 @@ tlsSecECDHEs1(TlsSec *sec)
static int static int
tlsSecECDHEs2(TlsSec *sec, Bytes *Yc) tlsSecECDHEs2(TlsSec *sec, Bytes *Yc)
{ {
static char zero[32] = {0};
ECdomain *dom = &sec->ec.dom; ECdomain *dom = &sec->ec.dom;
ECpriv *Q = &sec->ec.Q; ECpriv *Q = &sec->ec.Q;
ECpoint K; ECpoint K;
@ -2591,10 +2586,7 @@ tlsSecECDHEs2(TlsSec *sec, Bytes *Yc)
return -1; return -1;
} }
Z = newbytes(32); Z = newbytes(32);
curve25519_dh_finish(sec->X, Yc->data, Z->data); if(!curve25519_dh_finish(sec->X, Yc->data, Z->data)){
// rfc wants us to terminate the connection if
// shared secret == all zeroes.
if(tsmemcmp(Z->data, zero, Z->len) == 0){
werrstr("unlucky shared key"); werrstr("unlucky shared key");
freebytes(Z); freebytes(Z);
return -1; return -1;