ec25519-fhmqvc: additional key checks

Until now, it wasn't checked if a public key was the identity element. I don't
think this mistake allows any actual attacks against the handshake though.

3 files changed, 35 insertions, 4 deletions

diff --git a/src/protocols/ec25519_fhmqvc/ec25519_fhmqvc.c b/src/protocols/ec25519_fhmqvc/ec25519_fhmqvc.c
index c0d633d..c60ce67 100644
--- a/src/protocols/ec25519_fhmqvc/ec25519_fhmqvc.c
+++ b/src/protocols/ec25519_fhmqvc/ec25519_fhmqvc.c
@@ -78,7 +78,7 @@ static fastd_protocol_config_t * protocol_init(void) {
 static fastd_protocol_key_t * protocol_read_key(const char *key) {
 	fastd_protocol_key_t *ret = fastd_new(fastd_protocol_key_t);
 
-	if (!read_key(ret->key.u8, key)) {
+	if (!read_key(ret->key.u8, key) || !fastd_protocol_ec25519_fhmqvc_check_key(&ret->key.int256)) {
 		free(ret);
 		return NULL;
 	}
diff --git a/src/protocols/ec25519_fhmqvc/ec25519_fhmqvc.h b/src/protocols/ec25519_fhmqvc/ec25519_fhmqvc.h
index 55696c1..087cf35 100644
--- a/src/protocols/ec25519_fhmqvc/ec25519_fhmqvc.h
+++ b/src/protocols/ec25519_fhmqvc/ec25519_fhmqvc.h
@@ -120,6 +120,8 @@ fastd_peer_t * fastd_protocol_ec25519_fhmqvc_find_peer(const fastd_protocol_key_
 void fastd_protocol_ec25519_fhmqvc_generate_key(void);
 void fastd_protocol_ec25519_fhmqvc_show_key(void);
 
+bool fastd_protocol_ec25519_fhmqvc_check_key(const ecc_int256_t *key);
+
 void fastd_protocol_ec25519_fhmqvc_set_shell_env(fastd_shell_env_t *env, const fastd_peer_t *peer);
 bool fastd_protocol_ec25519_fhmqvc_describe_peer(const fastd_peer_t *peer, char *buf, size_t len);
 
diff --git a/src/protocols/ec25519_fhmqvc/handshake.c b/src/protocols/ec25519_fhmqvc/handshake.c
index 7c11912..4e519fa 100644
--- a/src/protocols/ec25519_fhmqvc/handshake.c
+++ b/src/protocols/ec25519_fhmqvc/handshake.c
@@ -204,6 +204,29 @@ static inline bool secure_handshake(const fastd_handshake_t *handshake) {
 }
 
 
+static bool check_key(const ecc_25519_work_t *key) {
+	ecc_25519_work_t work;
+
+	if (ecc_25519_is_identity(key))
+		return false;
+
+	ecc_25519_scalarmult(&work, &ecc_25519_gf_order, key);
+	if (!ecc_25519_is_identity(&work))
+		return false;
+
+	return true;
+}
+
+bool fastd_protocol_ec25519_fhmqvc_check_key(const ecc_int256_t *key) {
+	ecc_25519_work_t work;
+
+	if (!ecc_25519_load_packed(&work, key))
+		return false;
+
+	return check_key(&work);
+}
+
+
 /** Derives the shares handshake key for computing the MACs used in the handshake */
 static bool make_shared_handshake_key(const ecc_int256_t *handshake_key, bool initiator,
 				      const aligned_int256_t *A, const aligned_int256_t *B,
@@ -218,8 +241,7 @@ static bool make_shared_handshake_key(const ecc_int256_t *handshake_key, bool in
 	if (!ecc_25519_load_packed(&workXY, initiator ? &Y->int256 : &X->int256))
 		return false;
 
-	ecc_25519_scalarmult(&work, &ecc_25519_gf_order, &workXY);
-	if (!ecc_25519_is_identity(&work))
+	if (!check_key(&workXY))
 		return false;
 
 	if (!ecc_25519_load_packed(&work, initiator ? &B->int256 : &A->int256))
@@ -558,12 +580,19 @@ static fastd_peer_t * add_dynamic(fastd_socket_t *sock, const fastd_peer_address
 		return NULL;
 	}
 
+	aligned_int256_t peer_key;
+	memcpy(&peer_key, key, PUBLICKEYBYTES);
+	if (!fastd_protocol_ec25519_fhmqvc_check_key(&peer_key.int256)) {
+		pr_debug("ignoring handshake from %I (invalid key)", addr);
+		return NULL;
+	}
+
 	fastd_peer_t *peer = fastd_new0(fastd_peer_t);
 	peer->group = conf.peer_group;
 	peer->config_state = CONFIG_DYNAMIC;
 
 	peer->key = fastd_new(fastd_protocol_key_t);
-	memcpy(&peer->key->key, key, PUBLICKEYBYTES);
+	peer->key->key = peer_key;
 
 	if (!fastd_peer_add(peer))
 		exit_bug("failed to add dynamic peer");