Avoid using the same handshake key to establish more than one session

This fix prevents a potential attack using intentional packet reordering to
initialize more than one session with using the same handshake keys, leading
to more that one session to be initialized with the same key data altogether,
allowing to decrypt some packets in the worst case.

6 files changed, 64 insertions, 47 deletions

diff --git a/src/config.c b/src/config.c
index 4613295..92c7a45 100644
--- a/src/config.c
+++ b/src/config.c
@@ -790,7 +790,6 @@ void fastd_reconfigure(fastd_context *ctx, fastd_config *conf) {
 	reconfigure_handle_new_peers(ctx, &conf->peers, temp_conf.peers);
 
 	count_peers(ctx, conf);
-
 }
 
 void fastd_config_release(fastd_context *ctx, fastd_config *conf) {
diff --git a/src/fastd.h b/src/fastd.h
index 8a0fcce..ff7cab9 100644
--- a/src/fastd.h
+++ b/src/fastd.h
@@ -73,6 +73,8 @@ struct _fastd_protocol {
 	void (*handle_recv)(fastd_context *ctx, fastd_peer *peer, fastd_buffer buffer);
 	void (*send)(fastd_context *ctx, fastd_peer *peer, fastd_buffer buffer);
 
+	void (*init_peer_state)(fastd_context *ctx, fastd_peer *peer);
+	void (*reset_peer_state)(fastd_context *ctx, fastd_peer *peer);
 	void (*free_peer_state)(fastd_context *ctx, fastd_peer *peer);
 
 	void (*generate_key)(fastd_context *ctx);
@@ -86,7 +88,7 @@ struct _fastd_method {
 	size_t (*min_encrypt_head_space)(fastd_context *ctx);
 	size_t (*min_decrypt_head_space)(fastd_context *ctx);
 
-	fastd_method_session_state* (*session_init)(fastd_context *ctx, uint8_t *secret, size_t length, bool initiator, fastd_method_session_state *prev_session);
+	fastd_method_session_state* (*session_init)(fastd_context *ctx, uint8_t *secret, size_t length, bool initiator);
 	bool (*session_is_valid)(fastd_context *ctx, fastd_method_session_state *session);
 	bool (*session_is_initiator)(fastd_context *ctx, fastd_method_session_state *session);
 	bool (*session_want_refresh)(fastd_context *ctx, fastd_method_session_state *session);
diff --git a/src/method_null.c b/src/method_null.c
index 848f62d..e8b3fca 100644
--- a/src/method_null.c
+++ b/src/method_null.c
@@ -35,7 +35,7 @@ static size_t method_min_head_space(fastd_context *ctx) {
 	return 0;
 }
 
-static fastd_method_session_state* method_session_init(fastd_context *ctx, uint8_t *secret, size_t length, bool initiator, fastd_method_session_state *old_session) {
+static fastd_method_session_state* method_session_init(fastd_context *ctx, uint8_t *secret, size_t length, bool initiator) {
 	if (initiator)
 		return (fastd_method_session_state*)1;
 	else
diff --git a/src/method_xsalsa20_poly1305.c b/src/method_xsalsa20_poly1305.c
index ca928c5..c7fb040 100644
--- a/src/method_xsalsa20_poly1305.c
+++ b/src/method_xsalsa20_poly1305.c
@@ -86,15 +86,12 @@ static size_t method_min_decrypt_head_space(fastd_context *ctx) {
 	return (crypto_secretbox_xsalsa20poly1305_BOXZEROBYTES - NONCEBYTES);
 }
 
-static fastd_method_session_state* method_session_init(fastd_context *ctx, uint8_t *secret, size_t length, bool initiator, fastd_method_session_state *old_session) {
+static fastd_method_session_state* method_session_init(fastd_context *ctx, uint8_t *secret, size_t length, bool initiator) {
 	int i;
 
 	if (length < crypto_secretbox_xsalsa20poly1305_KEYBYTES)
 		exit_bug(ctx, "xsalsa20-poly1305: tried to init with short secret");
 
-	if (old_session && memcmp(secret, old_session->key, crypto_secretbox_xsalsa20poly1305_KEYBYTES) == 0)
-		return NULL;
-	
 	fastd_method_session_state *session = malloc(sizeof(fastd_method_session_state));
 
 	session->valid_till = ctx->now;
diff --git a/src/peer.c b/src/peer.c
index f412836..a2b6c1a 100644
--- a/src/peer.c
+++ b/src/peer.c
@@ -151,8 +151,7 @@ static inline void reset_peer(fastd_context *ctx, fastd_peer *peer) {
 	if (peer->established)
 		on_disestablish(ctx, peer);
 
-	ctx->conf->protocol->free_peer_state(ctx, peer);
-	peer->protocol_state = NULL;
+	ctx->conf->protocol->reset_peer_state(ctx, peer);
 
 	int i, deleted = 0;
 	for (i = 0; i < ctx->n_eth_addr; i++) {
@@ -187,7 +186,8 @@ static inline void setup_peer(fastd_context *ctx, fastd_peer *peer) {
 	peer->last_handshake_response = (struct timespec){0, 0};
 	peer->last_handshake_response_address.sa.sa_family = AF_UNSPEC;
 
-	peer->protocol_state = NULL;
+	if (!peer->protocol_state)
+		ctx->conf->protocol->init_peer_state(ctx, peer);
 
 	if (!fastd_peer_is_floating(peer))
 		fastd_task_schedule_handshake(ctx, peer, 0);
@@ -204,6 +204,7 @@ static void delete_peer(fastd_context *ctx, fastd_peer *peer) {
 		}
 	}
 
+	ctx->conf->protocol->free_peer_state(ctx, peer);
 	free(peer);
 }
 
@@ -337,6 +338,7 @@ fastd_peer* fastd_peer_add(fastd_context *ctx, fastd_peer_config *peer_conf) {
 	ctx->peers = peer;
 
 	peer->config = peer_conf;
+	peer->protocol_state = NULL;
 	setup_peer(ctx, peer);
 
 	pr_verbose(ctx, "adding peer %P", peer);
diff --git a/src/protocol_ec25519_fhmqvc.c b/src/protocol_ec25519_fhmqvc.c
index 96f0847..8599e29 100644
--- a/src/protocol_ec25519_fhmqvc.c
+++ b/src/protocol_ec25519_fhmqvc.c
@@ -59,6 +59,7 @@ struct _fastd_protocol_config {
 };
 
 typedef struct _handshake_key {
+	uint64_t serial;
 	struct timespec preferred_till;
 	struct timespec valid_till;
 
@@ -87,6 +88,8 @@ typedef struct _protocol_session {
 struct _fastd_protocol_peer_state {
 	protocol_session old_session;
 	protocol_session session;
+
+	uint64_t last_serial;
 };
 
 
@@ -192,32 +195,23 @@ static void protocol_peer_configure(fastd_context *ctx, fastd_peer_config *peer_
 	peer_conf->protocol_config->public_key = key;
 }
 
-static void init_peer_state(fastd_context *ctx, fastd_peer *peer) {
-	if (peer->protocol_state)
-		return;
-
-	peer->protocol_state = malloc(sizeof(fastd_protocol_peer_state));
-	memset(peer->protocol_state, 0, sizeof(fastd_protocol_peer_state));
-}
-
-static inline void free_handshake_key(handshake_key *handshake) {
-	if (handshake) {
-		memset(handshake, 0, sizeof(handshake_key));
-		free(handshake);
-	}
-}
-
-static void maintenance(fastd_context *ctx) {
+static void init_protocol_state(fastd_context *ctx) {
 	if (!ctx->protocol_state) {
 		ctx->protocol_state = malloc(sizeof(fastd_protocol_state));
 		memset(ctx->protocol_state, 0, sizeof(fastd_protocol_state));
 	}
+}
+
+static void maintenance(fastd_context *ctx) {
+	init_protocol_state(ctx);
 
 	if (!is_handshake_key_preferred(ctx, &ctx->protocol_state->handshake_key)) {
 		pr_debug(ctx, "generating new handshake key");
 
 		ctx->protocol_state->prev_handshake_key = ctx->protocol_state->handshake_key;
 
+		ctx->protocol_state->handshake_key.serial++;
+
 		fastd_random_bytes(ctx, ctx->protocol_state->handshake_key.secret_key.s, 32, false);
 		ecc_25519_secret_sanitize(&ctx->protocol_state->handshake_key.secret_key, &ctx->protocol_state->handshake_key.secret_key);
 
@@ -311,28 +305,17 @@ static void respond_handshake(fastd_context *ctx, const fastd_peer_address *addr
 
 static bool establish(fastd_context *ctx, fastd_peer *peer, const fastd_peer_address *address, bool initiator,
 		      const ecc_public_key_256 *A, const ecc_public_key_256 *B, const ecc_public_key_256 *X,
-		      const ecc_public_key_256 *Y, const ecc_public_key_256 *sigma) {
+		      const ecc_public_key_256 *Y, const ecc_public_key_256 *sigma, uint64_t serial) {
 	uint8_t hashinput[5*PUBLICKEYBYTES];
 	uint8_t hash[HASHBYTES];
 
-	pr_verbose(ctx, "%I authorized as %P", address, peer);
-
-	init_peer_state(ctx, peer);
-
-	memcpy(hashinput, X->p, PUBLICKEYBYTES);
-	memcpy(hashinput+PUBLICKEYBYTES, Y->p, PUBLICKEYBYTES);
-	memcpy(hashinput+2*PUBLICKEYBYTES, A->p, PUBLICKEYBYTES);
-	memcpy(hashinput+3*PUBLICKEYBYTES, B->p, PUBLICKEYBYTES);
-	memcpy(hashinput+4*PUBLICKEYBYTES, sigma->p, PUBLICKEYBYTES);
-	crypto_hash_sha256(hash, hashinput, 5*PUBLICKEYBYTES);
-
-	fastd_method_session_state *new_method_state = ctx->conf->method->session_init(ctx, hash, HASHBYTES, initiator, peer->protocol_state->session.method_state);
-
-	if (!new_method_state) {
-		pr_debug(ctx, "not establishing new session with %P[%I] by method choice", peer, address);
+	if (serial <= peer->protocol_state->last_serial) {
+		pr_debug(ctx, "ignoring handshake from %P[%I] because of handshake key reuse", peer, address);
 		return false;
 	}
 
+	pr_verbose(ctx, "%I authorized as %P", address, peer);
+
 	if (is_session_valid(ctx, &peer->protocol_state->session) && !is_session_valid(ctx, &peer->protocol_state->old_session)) {
 		ctx->conf->method->session_free(ctx, peer->protocol_state->old_session.method_state);
 		peer->protocol_state->old_session = peer->protocol_state->session;
@@ -341,10 +324,18 @@ static bool establish(fastd_context *ctx, fastd_peer *peer, const fastd_peer_add
 		ctx->conf->method->session_free(ctx, peer->protocol_state->session.method_state);
 	}
 
+	memcpy(hashinput, X->p, PUBLICKEYBYTES);
+	memcpy(hashinput+PUBLICKEYBYTES, Y->p, PUBLICKEYBYTES);
+	memcpy(hashinput+2*PUBLICKEYBYTES, A->p, PUBLICKEYBYTES);
+	memcpy(hashinput+3*PUBLICKEYBYTES, B->p, PUBLICKEYBYTES);
+	memcpy(hashinput+4*PUBLICKEYBYTES, sigma->p, PUBLICKEYBYTES);
+	crypto_hash_sha256(hash, hashinput, 5*PUBLICKEYBYTES);
+
 	peer->protocol_state->session.established = ctx->now;
 	peer->protocol_state->session.handshakes_cleaned = false;
 	peer->protocol_state->session.refreshing = false;
-	peer->protocol_state->session.method_state = new_method_state;
+	peer->protocol_state->session.method_state = ctx->conf->method->session_init(ctx, hash, HASHBYTES, initiator);
+	peer->protocol_state->last_serial = serial;
 
 	fastd_peer_seen(ctx, peer);
 
@@ -422,7 +413,7 @@ static void finish_handshake(fastd_context *ctx, const fastd_peer_address *addre
 	crypto_auth_hmacsha256(hmacbuf, hashinput, 2*PUBLICKEYBYTES, shared_handshake_key);
 
 	if (!establish(ctx, peer, address, true, &handshake_key->public_key, peer_handshake_key, &ctx->conf->protocol_config->public_key,
-		       &peer->config->protocol_config->public_key, &sigma))
+		       &peer->config->protocol_config->public_key, &sigma, handshake_key->serial))
 		return;
 
 	fastd_buffer buffer = fastd_handshake_new_reply(ctx, handshake, 4*(4+PUBLICKEYBYTES) + 4+HMACBYTES);
@@ -488,7 +479,7 @@ static void handle_finish_handshake(fastd_context *ctx, const fastd_peer_address
 	}
 
 	establish(ctx, peer, address, false, peer_handshake_key, &handshake_key->public_key, &peer->config->protocol_config->public_key,
-		  &ctx->conf->protocol_config->public_key, &sigma);
+		  &ctx->conf->protocol_config->public_key, &sigma, handshake_key->serial);
 }
 
 static const fastd_peer_config* match_sender_key(fastd_context *ctx, const fastd_peer_address *address, const fastd_peer_config *peer_conf, const unsigned char key[32]) {
@@ -726,10 +717,34 @@ static void protocol_send(fastd_context *ctx, fastd_peer *peer, fastd_buffer buf
 	fastd_buffer_free(buffer);
 }
 
+static void protocol_init_peer_state(fastd_context *ctx, fastd_peer *peer) {
+	init_protocol_state(ctx);
+
+	if (peer->protocol_state)
+		exit_bug(ctx, "tried to reinit peer state");
+
+	peer->protocol_state = malloc(sizeof(fastd_protocol_peer_state));
+	memset(peer->protocol_state, 0, sizeof(fastd_protocol_peer_state));
+	peer->protocol_state->last_serial = ctx->protocol_state->handshake_key.serial;
+}
+
+static void reset_session(fastd_context *ctx, protocol_session *session) {
+	ctx->conf->method->session_free(ctx, session->method_state);
+	memset(session, 0, sizeof(protocol_session));
+}
+
+static void protocol_reset_peer_state(fastd_context *ctx, fastd_peer *peer) {
+	if (!peer->protocol_state)
+		return;
+
+	reset_session(ctx, &peer->protocol_state->old_session);
+	reset_session(ctx, &peer->protocol_state->session);
+}
+
 static void protocol_free_peer_state(fastd_context *ctx, fastd_peer *peer) {
 	if (peer->protocol_state) {
-		ctx->conf->method->session_free(ctx, peer->protocol_state->old_session.method_state);
-		ctx->conf->method->session_free(ctx, peer->protocol_state->session.method_state);
+		reset_session(ctx, &peer->protocol_state->old_session);
+		reset_session(ctx, &peer->protocol_state->session);
 
 		free(peer->protocol_state);
 	}
@@ -788,6 +803,8 @@ const fastd_protocol fastd_protocol_ec25519_fhmqvc = {
 	.handle_recv = protocol_handle_recv,
 	.send = protocol_send,
 
+	.init_peer_state = protocol_init_peer_state,
+	.reset_peer_state = protocol_reset_peer_state,
 	.free_peer_state = protocol_free_peer_state,
 
 	.generate_key = protocol_generate_key,