Code fixes for the P2P overlay implementation

Adding new route fix
Fix of the hosts to string function
Message trace 'from' inclusion
Fixed encoded message handling

README.md

@@ -4,10 +4,10 @@
 - Host: A device that is possibly connected to the Coincer network
 - Identifier: The public key of a cryptographic keypair
 - Identity: A peer representation that we own (identifier + its corresponding
-  secret key)
-- Message trace: A tuple of a peer-to-peer message's nonce and a pointer to a
-  neighbour from which we've received the message. The message traces are
-  being used for routing loop detection
+secret key)
+- Message trace: Contains a peer-to-peer message's nonce, a pointer to a
+  neighbour from which we've received the message and the originator's
+  identifier. The message traces are being used for routing loop detection
 - n2n: neighbour-to-neighbour
 - Neighbour: A host that we are interconnected with
 - Nonce: Ever-increasing integer tied to every message sent for message

src/daemon_messages_processor.c

@@ -36,12 +36,14 @@
  *  presence of one identity. */
 #define ADV_GAP_TIME	10
 
-static enum process_message_result process_message(
-						const message_t	*message,
-						neighbour_t	*sender,
-						global_state_t	*global_state);
+static enum process_message_result
+	process_message(const message_t	*message,
+			const char	*json_message,
+			neighbour_t	*sender,
+			global_state_t	*global_state);
 
 static int process_p2p_bye(const message_t	*message,
+			   const char		*json_message,
 			   neighbour_t		*sender,
 			   peer_t		*sender_peer,
 			   global_state_t	*global_state);
@@ -57,10 +59,12 @@ static int process_p2p_peers_sol(neighbour_t	    *sender,
 static int process_p2p_ping(neighbour_t *sender);
 static int process_p2p_pong(neighbour_t *sender);
 static int process_p2p_route_adv(const message_t *message,
+				 const char	 *json_message,
 				 neighbour_t	 *sender,
 				 peer_t		 *sender_peer,
 				 global_state_t	 *global_state);
 static int process_p2p_route_sol(const message_t *message,
+				 const char	 *json_message,
 				 neighbour_t	 *sender,
 				 global_state_t	 *global_state);
 
@@ -80,36 +84,32 @@ static int process_p2p_route_sol(const message_t *message,
  * @return	PMR_ERR_VERSION		The message is of different
  *					protocol version.
  */
-enum process_message_result process_encoded_message(
-						const char	*json_message,
-						neighbour_t	*sender,
-						global_state_t	*global_state)
+enum process_message_result
+	process_encoded_message(const char	*json_message,
+				neighbour_t	*sender,
+				global_state_t	*global_state)
 {
 	char				*json_message_body;
 	message_t			message;
 	enum process_message_result	ret;
 
 	/* decode JSON message; if parsing JSON message into message_t failed */
-	if (decode_message(json_message, &message)) {
+	if (decode_message(json_message, &message, &json_message_body)) {
 		log_debug("process_encoded_message - decoding a received "
-			  "message has failed");
+			  "message has failed. The message:\n%s", json_message);
 		return PMR_ERR_PARSING;
 	}
 
 	if (message.version != PROTOCOL_VERSION) {
+		free(json_message_body);
 		message_delete(&message);
 		return PMR_ERR_VERSION;
 	}
 
-	/* integrity verification part; get message body in JSON format */
-	if (encode_message_body(&message.body, &json_message_body)) {
-		log_error("Encoding what we've just decoded has failed");
-		message_delete(&message);
-		return PMR_ERR_INTERNAL;
-	}
-	/* if the message integrity is violated */
+	/* integrity verification part; if the message integrity is violated */
 	if (verify_signature(json_message_body, message.from, message.sig)) {
 		log_warn("Someone tampered with a received message");
+		log_debug("The tampered message:\n%s", json_message);
 		free(json_message_body);
 		message_delete(&message);
 		return PMR_ERR_INTEGRITY;
@@ -118,7 +118,7 @@ enum process_message_result process_encoded_message(
 	free(json_message_body);
 
 	/* process the message */
-	ret = process_message(&message, sender, global_state);
+	ret = process_message(&message, json_message, sender, global_state);
 
 	message_delete(&message);
 	return ret;
@@ -128,6 +128,7 @@ enum process_message_result process_encoded_message(
  * Process a message received from its forwarder/sender (our neighbour).
  *
  * @param	message			The received message.
+ * @param	json_message		The received message in the JSON format.
  * @param	sender			The message sender/forwarder.
  * @param	global_state		The global state.
  *
@@ -136,10 +137,11 @@ enum process_message_result process_encoded_message(
  * @return	PMR_ERR_INTERNAL	Internal processing error.
  * @return	PMR_ERR_SEMANTIC	Semantic error.
  */
-static enum process_message_result process_message(
-						const message_t	*message,
-						neighbour_t	*sender,
-						global_state_t	*global_state)
+static enum process_message_result
+	process_message(const message_t	*message,
+			const char	*json_message,
+			neighbour_t	*sender,
+			global_state_t	*global_state)
 {
 	int			cmp_val;
 	linkedlist_t		*hosts;
@@ -269,7 +271,8 @@ static enum process_message_result process_message(
 				/* check if there's a routing loop */
 				if (routing_loop_detect(msg_traces,
 							sender,
-							nonce_value)) {
+							nonce_value,
+							message->from)) {
 					routing_loop_remove(routing_table,
 							    neighbours,
 							    identities,
@@ -291,7 +294,10 @@ static enum process_message_result process_message(
 	 * not meant for us */
 	if (!identity) {
 		/* if forwarding succeeded */
-		if (!message_forward(message, sender, global_state)) {
+		if (!message_forward(message,
+				     json_message,
+				     sender,
+				     global_state)) {
 			/* store message's nonce */
 			nonce_store(&sender_peer->nonces, nonce_value);
 		}
@@ -306,6 +312,7 @@ static enum process_message_result process_message(
 	switch (msg_type) {
 		case P2P_BYE:
 			res = process_p2p_bye(message,
+					      json_message,
 					      sender,
 					      sender_peer,
 					      global_state);
@@ -345,12 +352,14 @@ static enum process_message_result process_message(
 			break;
 		case P2P_ROUTE_ADV:
 			res = process_p2p_route_adv(message,
+						    json_message,
 						    sender,
 						    sender_peer,
 						    global_state);
 			break;
 		case P2P_ROUTE_SOL:
 			res = process_p2p_route_sol(message,
+						    json_message,
 						    sender,
 						    global_state);
 			break;
@@ -380,6 +389,7 @@ static enum process_message_result process_message(
  * Process p2p.bye.
  *
  * @param	message		Process this message.
+ * @param	json_message	The received message in the JSON format.
  * @param	sender		We've received the message from this neighbour.
  * @param	sender_peer	Sender's peer representation.
  * @param	global_state	The global state.
@@ -387,6 +397,7 @@ static enum process_message_result process_message(
  * @return	0		Successfully processed.
  */
 static int process_p2p_bye(const message_t	*message,
+			   const char		*json_message,
 			   neighbour_t		*sender,
 			   peer_t		*sender_peer,
 			   global_state_t	*global_state)
@@ -394,7 +405,7 @@ static int process_p2p_bye(const message_t	*message,
 	route_delete(&global_state->routing_table, message->from);
 	peer_delete(sender_peer);
 
-	message_forward(message, sender, global_state);
+	message_forward(message, json_message, sender, global_state);
 
 	return 0;
 }
@@ -588,6 +599,7 @@ static int process_p2p_pong(neighbour_t *sender)
  * Process p2p.route.adv.
  *
  * @param	message		Process this message.
+ * @param	json_message	The received message in the JSON format.
  * @param	sender		We've received the message from this neighbour.
  * @param	sender_peer	Sender's peer representation.
  * @param	global_state	The global state.
@@ -596,6 +608,7 @@ static int process_p2p_pong(neighbour_t *sender)
  * @return	1		Failure.
  */
 static int process_p2p_route_adv(const message_t *message,
+				 const char	 *json_message,
 				 neighbour_t	 *sender,
 				 peer_t		 *sender_peer,
 				 global_state_t	 *global_state)
@@ -627,7 +640,7 @@ static int process_p2p_route_adv(const message_t *message,
 		return 1;
 	}
 
-	message_forward(message, sender, global_state);
+	message_forward(message, json_message, sender, global_state);
 	return 0;
 }
 
@@ -635,6 +648,7 @@ static int process_p2p_route_adv(const message_t *message,
  * Process p2p.route.sol.
  *
  * @param	message		Process this message.
+ * @param	json_message	The received message in the JSON format.
  * @param	sender		We've received the message from this neighbour.
  * @param	global_state	The global state.
  *
@@ -642,6 +656,7 @@ static int process_p2p_route_adv(const message_t *message,
  * @return	1		Failure.
  */
 static int process_p2p_route_sol(const message_t *message,
+				 const char	 *json_message,
 				 neighbour_t	 *sender,
 				 global_state_t	 *global_state)
 {
@@ -654,6 +669,7 @@ static int process_p2p_route_sol(const message_t *message,
 	/* if we wouldn't rebroadcast the p2p.route.sol, and just sent
 	 * a p2p.route.adv, it could be obvious the 'target' is us */
 	if (message_forward(message,
+			    json_message,
 			    sender,
 			    global_state)) {
 		return 1;

src/daemon_messages_processor.h

@@ -32,9 +32,9 @@ enum process_message_result {
 	PMR_ERR_VERSION
 };
 
-enum process_message_result process_encoded_message(
-						const char	*json_message,
-						neighbour_t	*sender,
-						global_state_t	*global_state);
+enum process_message_result
+	process_encoded_message(const char	*json_message,
+				neighbour_t	*sender,
+				global_state_t	*global_state);
 
 #endif /* DAEMON_MESSAGES_PROCESSOR_H */

src/hosts.c

@@ -161,6 +161,7 @@ int hosts_to_str(const linkedlist_t *hosts, char **output)
 	const char		*separator = ", ";
 	size_t			separator_len;
 	char			text_ip[INET6_ADDRSTRLEN];
+	char			*tmp_out;
 
 	opening_len   = strlen(opening);
 	separator_len = strlen(separator);
@@ -191,12 +192,14 @@ int hosts_to_str(const linkedlist_t *hosts, char **output)
 		if (output_buff_size - output_size < 128) {
 			output_buff_size *= 2;
 
-			*output = (char *) realloc(*output, output_buff_size *
+			tmp_out = (char *) realloc(*output, output_buff_size *
 							    sizeof(char));
-			if (!*output) {
+			if (!tmp_out) {
 				log_error("Reallocating string of hosts");
+				free(output);
 				return 1;
 			}
+			*output = tmp_out;
 		}
 
 		/* binary ip to text ip conversion */

src/json_parser.c

@@ -29,13 +29,16 @@
 /**
  * Decode a JSON message (including its JSON body) into daemon message.
  *
- * @param	json_message	Decode this JSON message.
- * @param	message		Store decoded data in here.
+ * @param	json_message		Decode this JSON message.
+ * @param	message			Store decoded data in here.
+ * @param	json_message_body	Store the message's JSON body in here.
  *
- * @return	0		Decoding successful.
- * @return	1		Failure.
+ * @return	0			Decoding successful.
+ * @return	1			Failure.
  */
-int decode_message(const char *json_message, message_t *message)
+int decode_message(const char	*json_message,
+		   message_t	*message,
+		   char		**json_message_body)
 {
 	char from_hex[65];
 	char to_hex[65];

src/json_parser.h

@@ -34,7 +34,9 @@ static const char *msg_type_str[] = {
 	"p2p.route.sol"
 };
 
-int decode_message(const char *json_message, message_t *message);
+int decode_message(const char	*json_message,
+		   message_t	*message,
+		   char		**json_message_body);
 int decode_message_body(const char *json_body, message_body_t *body);
 int encode_message(const message_t *message, char **json_message);
 int encode_message_body(const message_body_t *body, char **json_body);

src/routing.c

@@ -51,8 +51,12 @@ static int message_send_to_neighbour(const message_t	*message,
 
 static int message_trace_store(linkedlist_t		*msg_traces,
 			       const neighbour_t	*sender,
-			       uint64_t			nonce_value);
+			       uint64_t			nonce_value,
+			       const unsigned char	*from);
 
+static void string_broadcast(const char		*string,
+			     const linkedlist_t	*neighbours,
+			     const neighbour_t	*exception);
 static void string_send_to_neighbour(const char		*string,
 				     neighbour_t	*dest);
 
@@ -200,6 +204,7 @@ static int message_finalize(message_t			*message,
  * Forward someone's message on its way to destination.
  *
  * @param	message		Forward this message.
+ * @param	json_message	The message to be forwarded, just encoded.
  * @param	sender		Neighbour who's sent us the message.
  * @param	global_state	The global state.
  *
@@ -207,26 +212,35 @@ static int message_finalize(message_t			*message,
  * @return	1		Failure.
  */
 int message_forward(const message_t	*message,
+		    const char		*json_message,
 		    const neighbour_t	*sender,
 		    global_state_t	*global_state)
 {
-	const message_body_t *msg_body;
+	neighbour_t	*next_hop;
+	route_t		*route;
 
-	msg_body = &message->body;
-
-	/* if all 'to' bytes are set to 0x0, the message is a broadcast */
-	if (identifier_empty(msg_body->to)) {
+	/* if the 'to' bytes are set to 0x0, the message is a broadcast */
+	if (identifier_empty(message->body.to)) {
 		/* don't send the message to the one who's sent it to us */
-		return message_broadcast_p2p(message,
-					     &global_state->neighbours,
-					     sender);
+		string_broadcast(json_message,
+				 &global_state->neighbours,
+				 sender);
+		return 0;
+	}
+
+	route = route_find(&global_state->routing_table, message->body.to);
+	next_hop = route ? route_next_hop_get(route) : NULL;
+	if (!next_hop) {
+		return 1;
 	}
 
 	message_trace_store(&global_state->message_traces,
 			    sender,
-			    msg_body->nonce);
+			    message->body.nonce,
+			    message->from);
 
-	return message_send_p2p(message, &global_state->routing_table);
+	string_send_to_neighbour(json_message, next_hop);
+	return 0;
 }
 
 /**
@@ -264,22 +278,17 @@ static int message_send_n2n(message_t *message, neighbour_t *dest)
 static int message_send_p2p(const message_t	*message,
 			    const linkedlist_t	*routing_table)
 {
-	neighbour_t		*next_hop;
-	linkedlist_node_t	*next_hop_node;
-	route_t			*route;
+	neighbour_t	*next_hop;
+	route_t		*route;
 
 	route = route_find(routing_table, message->body.to);
-	/* get the next hop or NULL if the message destination is unknown
-	 * or if we have no next hop to reach the destination */
-	next_hop_node = route ? linkedlist_get_first(&route->next_hops) : NULL;
-	if (next_hop_node) {
-		next_hop = (neighbour_t *) next_hop_node->data;
-
-		/* send the message to the next hop */
-		return message_send_to_neighbour(message, next_hop);
+	next_hop = route ? route_next_hop_get(route) : NULL;
+	if (!next_hop) {
+		return 1;
 	}
 
-	return 1;
+	/* send the message to the next hop */
+	return message_send_to_neighbour(message, next_hop);
 }
 
 /**
@@ -329,13 +338,15 @@ int message_trace_is_stale(const message_trace_t *msg_trace,
  * @param	msg_traces	Store to this container.
  * @param	sender		The neighbour whose message we've forwarded.
  * @param	nonce_value	The forwarded message's nonce value.
+ * @param	from		The originator's identifier.
  *
  * @return	0		Successfully stored.
  * @return	1		Failure.
  */
 static int message_trace_store(linkedlist_t		*msg_traces,
 			       const neighbour_t	*sender,
-			       uint64_t			nonce_value)
+			       uint64_t			nonce_value,
+			       const unsigned char	*from)
 {
 	message_trace_t	*msg_trace;
 
@@ -348,6 +359,7 @@ static int message_trace_store(linkedlist_t		*msg_traces,
 	msg_trace->sender = sender;
 	msg_trace->nonce_value = nonce_value;
 	msg_trace->creation = time(NULL);
+	memcpy(msg_trace->from, from, crypto_box_PUBLICKEYBYTES);
 
 	if (!linkedlist_append(msg_traces, msg_trace)) {
 		log_error("Storing a message trace");
@@ -381,12 +393,18 @@ route_t *route_add(linkedlist_t		*routing_table,
 	}
 
 	linkedlist_init(&new_route->next_hops);
-	if (!(new_route->node =
-	      linkedlist_append(&new_route->next_hops, next_hop))) {
+	if (!linkedlist_append(&new_route->next_hops, next_hop)) {
+		free(new_route);
+		log_error("Adding a new route's next hop");
+		return NULL;
+	}
+	if (!(new_route->node = linkedlist_append(routing_table, new_route))) {
+		linkedlist_remove_all(&new_route->next_hops);
 		free(new_route);
 		log_error("Storing a new route");
 		return NULL;
 	}
+
 	new_route->destination = dest;
 	new_route->last_update = time(NULL);
 
@@ -484,6 +502,26 @@ int route_next_hop_add(route_t *route, neighbour_t *next_hop)
 	return 0;
 }
 
+/**
+ * Get a next hop for a given route.
+ *
+ * @param	route		The route.
+ *
+ * @return	neighbour_t	The next hop.
+ * @return	NULL		The route has no next hops.
+ */
+neighbour_t *route_next_hop_get(const route_t *route)
+{
+	linkedlist_node_t *next_hop_node;
+
+	next_hop_node = route ? linkedlist_get_first(&route->next_hops) : NULL;
+	if (!next_hop_node) {
+		return NULL;
+	}
+
+	return next_hop_node->data;
+}
+
 /**
  * Remove a next hop from a route, if exists.
  *
@@ -521,13 +559,15 @@ int route_reset(route_t *route, neighbour_t *next_hop)
  * @param	msg_traces	Recent message traces.
  * @param	neighbour	The forwarder of a new message.
  * @param	nonce_value	The message's nonce value.
+ * @param	from		The identifier of the message originator.
  *
  * @return	1		A routing loop detected.
  * @return	0		No routing loop detected.
  */
 int routing_loop_detect(const linkedlist_t	*msg_traces,
 			const neighbour_t	*neighbour,
-			uint64_t		nonce_value)
+			uint64_t		nonce_value,
+			const unsigned char	*from)
 {
 	const message_trace_t	*msg_trace;
 	const linkedlist_node_t	*node;
@@ -538,7 +578,8 @@ int routing_loop_detect(const linkedlist_t	*msg_traces,
 
 		/* the same message but different neighbour => routing loop */
 		if (msg_trace->nonce_value == nonce_value &&
-		    msg_trace->sender != neighbour) {
+		    msg_trace->sender != neighbour &&
+		    !memcmp(msg_trace->from, from, crypto_box_PUBLICKEYBYTES)) {
 			return 1;
 		}
 
@@ -565,20 +606,18 @@ void routing_loop_remove(linkedlist_t		*routing_table,
 {
 	identity_t		*identity;
 	neighbour_t		*next_hop;
-	linkedlist_node_t	*next_hop_node;
 	route_t			*route;
 
 	route = route_find(routing_table, dest_id);
 
 	/* get the next hop or NULL if the message destination is unknown
 	 * or if we have no next hop to reach the destination */
-	next_hop_node = route ? linkedlist_get_first(&route->next_hops) : NULL;
-	if (next_hop_node) {
-		next_hop = (neighbour_t *) next_hop_node->data;
+	next_hop = route ? route_next_hop_get(route) : NULL;
+	if (next_hop) {
 		route_next_hop_remove(route, next_hop);
 
-		next_hop_node = linkedlist_get_first(&route->next_hops);
-		if (!next_hop_node) {
+		next_hop = route_next_hop_get(route);
+		if (!next_hop) {
 			/* create a temporary identity for the p2p.route.sol */
 			identity = identity_generate(IDENTITY_TMP);
 			/* if appending succeeded */
@@ -833,6 +872,32 @@ int send_p2p_route_sol(linkedlist_t	   *neighbours,
 	return ret;
 }
 
+/**
+ * Broadcast a text (json message) to our neighbour.
+ *
+ * @param	string		Send this text.
+ * @param	neighbours	Our neighbours.
+ * @param	exception	Don't send the message to this neighbour.
+ *				This is typically the neighbour from whom
+ *				we've received the message.
+ */
+static void string_broadcast(const char		*string,
+			     const linkedlist_t	*neighbours,
+			     const neighbour_t	*exception)
+{
+	linkedlist_node_t *node;
+	neighbour_t	  *neighbour;
+
+	node = linkedlist_get_first(neighbours);
+	while (node) {
+		neighbour = (neighbour_t *) node->data;
+		if (neighbour != exception) {
+			string_send_to_neighbour(string, neighbour);
+		}
+		node = linkedlist_get_next(neighbours, node);
+	}
+}
+
 /**
  * Send a text (json message) to a neighbour.
  *

src/routing.h

@@ -38,6 +38,8 @@
 typedef struct s_message_trace {
 	/** Message's nonce value. */
 	uint64_t nonce_value;
+	/** Identifier of the sender. */
+	char from[crypto_box_PUBLICKEYBYTES];
 	/** Neighbour who's sent us the message */
 	const neighbour_t *sender;
 	/** Creation timestamp. */
@@ -59,6 +61,7 @@ typedef struct s_route {
 } route_t;
 
 int message_forward(const message_t	*message,
+		    const char		*json_message,
 		    const neighbour_t	*sender,
 		    global_state_t	*global_state);
 
@@ -74,12 +77,14 @@ route_t *route_find(const linkedlist_t	*routing_table,
 		    const unsigned char	*dest_id);
 int route_is_stale(const route_t *route, const time_t *current_time);
 int route_next_hop_add(route_t *route, neighbour_t *next_hop);
+neighbour_t *route_next_hop_get(const route_t *route);
 void route_next_hop_remove(route_t *route, neighbour_t *next_hop);
 int route_reset(route_t *route, neighbour_t *next_hop);
 
 int routing_loop_detect(const linkedlist_t	*msg_traces,
 			const neighbour_t	*neighbour,
-			uint64_t		nonce_value);
+			uint64_t		nonce_value,
+			const unsigned char	*from);
 void routing_loop_remove(linkedlist_t		*routing_table,
 			 linkedlist_t		*neighbours,
 			 linkedlist_t		*identities,