From f64c8e3bee2e9c61cf18c93c6a328dd5d6d8e1b4 Mon Sep 17 00:00:00 2001 From: Matthias Schiffer Date: Tue, 6 Jan 2015 09:15:00 +0100 Subject: 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. --- src/protocols/ec25519_fhmqvc/ec25519_fhmqvc.c | 2 +- src/protocols/ec25519_fhmqvc/ec25519_fhmqvc.h | 2 ++ src/protocols/ec25519_fhmqvc/handshake.c | 35 ++++++++++++++++++++++++--- 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"); -- cgit v1.2.3