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.

diff --git a/sys/include/ape/libsec.h b/sys/include/ape/libsec.h
index 6f44db92dcef8abb2e60ac8017bc639f46132060..d44ff0cde599feb3423c7bd744355d633aa627fd 100644 (file)
--- a/sys/include/ape/libsec.h
+++ b/sys/include/ape/libsec.h
@@ -583,7 +583,7 @@ void curve25519(uchar mypublic[32], uchar secret[32], uchar basepoint[32]);
 
 /* Curve25519 diffie hellman */
 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) */
 void pbkdf2_x(uchar *p, ulong plen, uchar *s, ulong slen, ulong rounds, uchar *d, ulong dlen,

diff --git a/sys/include/libsec.h b/sys/include/libsec.h
index b19d18a762944965dbbb9ebb6901a8e35da70ca1..bebcc98fe6be92b54cd6e8ae1377725c6e120d03 100644 (file)
--- a/sys/include/libsec.h
+++ b/sys/include/libsec.h
@@ -575,7 +575,7 @@ void curve25519(uchar mypublic[32], uchar secret[32], uchar basepoint[32]);
 
 /* Curve25519 diffie hellman */
 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) */
 void pbkdf2_x(uchar *p, ulong plen, uchar *s, ulong slen, ulong rounds, uchar *d, ulong dlen,

diff --git a/sys/src/cmd/ssh.c b/sys/src/cmd/ssh.c
index 239a4f542c2e5d914b5086346311f3029cabaed3..00733bdd780e4371ee0993921a74cb8c0340b095 100644 (file)
--- a/sys/src/cmd/ssh.c
+++ b/sys/src/cmd/ssh.c
@@ -600,7 +600,8 @@ Next1:      switch(recvpkt()){
        if((S = ssh2rsasig(sig, nsig)) == nil)
                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);
        nk = (mpsignif(K)+8)/8;

diff --git a/sys/src/libsec/port/curve25519_dh.c b/sys/src/libsec/port/curve25519_dh.c
index efc16ac8d5fd9a3ce6075cb341a1893e16365e4c..4d1c1a7072eb869e46a642ae6f5869b7a23346c6 100644 (file)
--- a/sys/src/libsec/port/curve25519_dh.c
+++ b/sys/src/libsec/port/curve25519_dh.c
@@ -3,6 +3,7 @@
 #include <libsec.h>
 
 static uchar nine[32] = {9};
+static uchar zero[32] = {0};
 
 void
 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;
 }
 
-void
+int
 curve25519_dh_finish(uchar x[32], uchar y[32], uchar z[32])
 {
        /* 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(y, 0, 32);
+
+       return tsmemcmp(z, zero, 32) != 0;
 }

diff --git a/sys/src/libsec/port/tlshand.c b/sys/src/libsec/port/tlshand.c
index 2da4f8d7c46ca836a85e4b73a39df086e3ded6b1..b1cb615bb263293a89775fce14e24915a9353c42 100644 (file)
--- a/sys/src/libsec/port/tlshand.c
+++ b/sys/src/libsec/port/tlshand.c
@@ -948,7 +948,6 @@ Out:
 static Bytes*
 tlsSecECDHEc(TlsSec *sec, int curve, Bytes *Ys)
 {
-       static char zero[32] = {0};
        ECdomain *dom = &sec->ec.dom;
        ECpriv *Q = &sec->ec.Q;
        ECpub *pub;
@@ -967,10 +966,7 @@ tlsSecECDHEc(TlsSec *sec, int curve, Bytes *Ys)
                Yc = newbytes(32);
                curve25519_dh_new(sec->X, Yc->data);
                Z = newbytes(32);
-               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){
+               if(!curve25519_dh_finish(sec->X, Ys->data, Z->data)){
                        freebytes(Yc);
                        freebytes(Z);
                        return nil;
@@ -2573,7 +2569,6 @@ tlsSecECDHEs1(TlsSec *sec)
 static int
 tlsSecECDHEs2(TlsSec *sec, Bytes *Yc)
 {
-       static char zero[32] = {0};
        ECdomain *dom = &sec->ec.dom;
        ECpriv *Q = &sec->ec.Q;
        ECpoint K;
@@ -2591,10 +2586,7 @@ tlsSecECDHEs2(TlsSec *sec, Bytes *Yc)
                        return -1;
                }
                Z = newbytes(32);
-               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){
+               if(!curve25519_dh_finish(sec->X, Yc->data, Z->data)){
                        werrstr("unlucky shared key");
                        freebytes(Z);
                        return -1;