code: plan9front

Info  •  Files  •  Log

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.

--- a/sys/include/ape/libsec.h

+++ b/sys/include/ape/libsec.h

@@ -583,7 +583,7 @@

 /* 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,

--- a/sys/include/libsec.h

+++ b/sys/include/libsec.h

@@ -575,7 +575,7 @@

 /* 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,

--- a/sys/src/cmd/ssh.c

+++ b/sys/src/cmd/ssh.c

@@ -600,7 +600,8 @@

 	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;

--- 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 @@

 	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 @@

 	memset(x, 0, 32);

 	memset(y, 0, 32);

+

+	return tsmemcmp(z, zero, 32) != 0;

 }

--- a/sys/src/libsec/port/tlshand.c

+++ b/sys/src/libsec/port/tlshand.c

@@ -948,7 +948,6 @@

 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 @@

 		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 @@

 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 @@

 			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;