shithub: drawterm

Download patch

ref: df9c9a8f7c088cb52d5feb0757b51e69bffd380d
parent: 368599deabba0aa9d18b05ffc0b6a87d1fc7d22a
author: cinap_lenrek <cinap_lenrek@felloff.net>
date: Sun Jun 20 10:47:03 EDT 2021 

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/include/libsec.h
+++ b/include/libsec.h
@@ -570,7 +570,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/libsec/curve25519_dh.c
+++ b/libsec/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/libsec/tlshand.c
+++ b/libsec/tlshand.c
@@ -952,7 +952,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;
@@ -971,10 +970,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;
@@ -2577,7 +2573,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;
@@ -2595,10 +2590,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;