From 56939dcc24b68ffc70e35a39499987e035b41573 Mon Sep 17 00:00:00 2001 From: Starbeamrainbowlabs Date: Tue, 15 Mar 2022 00:28:27 +0000 Subject: [PATCH] Properly deduplicate connections. By assigning a random connection ID during the initial hello that's both unique to every new connection regardless of target and consistent across both the connection initiator and acceptor, we can do string comparison to determine which string is greater than the other. In doing so, we can ensur the same connection is always terminated, thereby avoiding closing both connections by accident. --- src/lib/agent/Peer.mjs | 23 ++++++++++++++++++---- src/lib/agent/PeerServer.mjs | 32 +++++++++++++++++++++++++------ src/lib/crypto/make_crypto_id.mjs | 10 ++++++++++ 3 files changed, 55 insertions(+), 10 deletions(-) create mode 100644 src/lib/crypto/make_crypto_id.mjs diff --git a/src/lib/agent/Peer.mjs b/src/lib/agent/Peer.mjs index a96a6aa..8f640cb 100644 --- a/src/lib/agent/Peer.mjs +++ b/src/lib/agent/Peer.mjs @@ -5,6 +5,7 @@ import { EventEmitter, once } from 'events'; import log from '../io/NamespacedLog.mjs'; const l = log("peer"); import Connection from '../transport/Connection.mjs'; +import make_crypto_id from '../crypto/make_crypto_id.mjs'; class Peer extends EventEmitter { get address() { return this.connection.address; } @@ -36,6 +37,15 @@ class Peer extends EventEmitter { * @type {string} */ this.id = null; + /** + * The ID of this connection. + * Every time a connection is opened, a new connection ID is negotiated + * in the initial hello handshake. + * The connection ID is consistent across both the connection initiator + * and acceptor. + * @type {string?} + */ + this.conn_id = null; /** * The friendly name of this peer. * Unlike the ID, this *may* not be unique (though it is strongly @@ -70,7 +80,7 @@ class Peer extends EventEmitter { async __accept(connection) { this.connection = connection; const [ msg ] = await once(this.connection, "message-hello"); - if(!this.__handle_hello(msg)) + if(!this.__handle_hello(msg, false)) await this.destroy(); await this.__send_hello(); @@ -85,27 +95,31 @@ class Peer extends EventEmitter { return new Promise((resolve, reject) => { this.connection.once("error", reject); this.connection.once("message-hello", async (msg) => { - if(!this.__handle_hello(msg)) + if(!this.__handle_hello(msg, true)) await this.destroy(); this.connection.off("error", reject); this.emit("connect"); resolve(); }); + this.conn_id = make_crypto_id(); this.__send_hello(); // Set and forget }); } /** * Handles a given hello messaage from this peer. - * @param {Object} msg The hello message to process. + * @param {Object} msg The hello message to process. + * @param {boolean} initiator Whether we were the initiator of this connection or not. * @return {boolean} Whether the peer should stay connected or not. */ - __handle_hello(msg) { + __handle_hello(msg, initiator) { this.id = msg.id; this.name = msg.name; this.listening_address = msg.listening_address; this.listening_port = msg.listening_port; + if(!initiator) + this.conn_id = msg.conn_id; if(this.id === this.server.our_id) { l.warn(`Our id (${this.server.our_id}) is equal to that of the remote (${this.id}), killing connection`); @@ -128,6 +142,7 @@ class Peer extends EventEmitter { async __send_hello() { await this.send("hello", { id: this.server.our_id, + conn_id: this.conn_id, name: this.server.our_name, listening_address: this.server.host, listening_port: this.server.port diff --git a/src/lib/agent/PeerServer.mjs b/src/lib/agent/PeerServer.mjs index cfac39a..6ef3308 100644 --- a/src/lib/agent/PeerServer.mjs +++ b/src/lib/agent/PeerServer.mjs @@ -243,18 +243,38 @@ class PeerServer extends EventEmitter { if(peer === null) return null; // If the ID of the peer is equal to that of one of the existing peers, close the // connection - if(this.peers().some(existing_peer => existing_peer.id === peer.id)) { - l.info(`Closing duplicate connection to ${peer.id_short}`); - await this.remove_peers(peer); + for(const existing_peer of this.peers()) { + if(existing_peer.id === peer.id) { + // If this peer was reaped, don't continue + if(this.dedupe(peer, existing_peer) === peer) + return null; + } } - if(this.connecting_peers.filter(other_peer => other_peer.id !== null) - .some(other_peer => other_peer.id === peer.id)) - await this.remove_peers(peer); this.peer_initialise(peer); return peer; } + /** + * Deduplicates 2 idential peers. + * This function ensures that deduplication does *not* result in 2 closed + * connections, and that 1 connection to the peer in question remains open. + * @param {Peer} peer_a The first duplicate peer. + * @param {Peer} peer_b The second duplicate peer. + * @return {Promise} A Promise that resolves when deduplication is complete. Returns the reaped peer. + */ + async dedupe(peer_a, peer_b) { + if(peer_a.id !== peer_b.id) return null; + if(peer_a.conn_id > peer_b.conn_id) { + await this.remove_peers(peer_b); + return peer_b; + } + else { + await this.remove_peers(peer_a); + return peer_a; + } + } + async remove_peers(...peers) { // No need to remove the peer from the list of connected peers, as // there's already a catch-all event handler attached to do that diff --git a/src/lib/crypto/make_crypto_id.mjs b/src/lib/crypto/make_crypto_id.mjs new file mode 100644 index 0000000..6927371 --- /dev/null +++ b/src/lib/crypto/make_crypto_id.mjs @@ -0,0 +1,10 @@ +"use strict"; + +import tweetnacl from 'tweetnacl'; + + +export default function make_crypto_id() { + return Buffer.from(tweetnacl.randomBytes(256)).toString("base64") + .replace(/\+/g, "-") + .replace(/\//g, "_"); +}