Fix possible duplicate session establishment

This is causing duplicate nonces in the worst case.
neocturne · Jun 5, 2012 · 0f14f55 · 0f14f55
1 parent 227af67
commit 0f14f55
Show file tree

Hide file tree

Showing 4 changed files with 21 additions and 11 deletions.
diff --git a/src/fastd.h b/src/fastd.h
@@ -86,7 +86,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* (*session_init)(fastd_context *ctx, uint8_t *secret, size_t length, bool initiator, fastd_method_session_state *prev_session);
 	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
@@ -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) {
+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) {
 	if (initiator)
 		return (fastd_method_session_state*)1;
 	else

diff --git a/src/method_xsalsa20_poly1305.c b/src/method_xsalsa20_poly1305.c
@@ -86,11 +86,14 @@ 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) {
+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) {
 	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));
 

diff --git a/src/protocol_ec25519_fhmqvc.c b/src/protocol_ec25519_fhmqvc.c
@@ -319,6 +319,20 @@ static bool establish(fastd_context *ctx, fastd_peer *peer, const fastd_peer_add
 
 	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);
+		return false;
+	}
+
 	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;
@@ -327,17 +341,10 @@ 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 = ctx->conf->method->session_init(ctx, hash, HASHBYTES, initiator);
+	peer->protocol_state->session.method_state = new_method_state;
 
 	fastd_peer_seen(ctx, peer);