Initial trading ID change of basic trade made safe

A peer is allowed to change their ID during TS_COMMITTMENT

src/daemon_messages.c

@@ -284,9 +284,10 @@ int create_trade_execution(trade_execution_t **execution, const trade_t *trade)
 static int create_trade_execution_basic(trade_execution_basic_t **basic,
 					const trade_t		*trade)
 {
-	trade_execution_basic_t *exec_data;
-	char *script;
-	trade_basic_t *trade_data;
+	trade_execution_basic_t	*exec_data;
+	char			new_id_hex[2 * PUBLIC_KEY_SIZE + 1];
+	char			*script;
+	trade_basic_t		*trade_data;
 
 	trade_data = (trade_basic_t *) trade->data;
 
@@ -302,6 +303,14 @@ static int create_trade_execution_basic(trade_execution_basic_t **basic,
 			memcpy(exec_data->commitment,
 			       trade_data->my_commitment,
 			       SHA3_256_SIZE);
+
+			sodium_bin2hex(new_id_hex,
+				       sizeof(new_id_hex),
+				       trade->my_identity->keypair.public_key,
+				       PUBLIC_KEY_SIZE);
+			sign_message(new_id_hex,
+				     trade->order->owner.me->keypair.secret_key,
+				     exec_data->idsig);
 			break;
 		case TS_KEY_AND_COMMITTED_EXCHANGE:
 			memcpy(exec_data->pubkey,

src/daemon_messages_processor.c

@@ -890,24 +890,22 @@ static int process_trade_execution(const trade_execution_t *trade_execution,
 				   const unsigned char	   *sender_id,
 				   global_state_t	   *global_state)
 {
-	/* counterparty's identifier must remain the same, unless this is
-	 * the trade acceptance */
-	if (memcmp(trade->cp_identifier, sender_id, PUBLIC_KEY_SIZE)) {
-		if (trade->step != TS_PROPOSAL) {
-			log_debug("process_trade_execution - received "
-				  "trade.execution from a wrong peer");
-			return 0;
+	int ret;
+
+	if ((ret = trade_execution_verify(trade_execution, trade, sender_id))) {
+		log_debug("process_trade_execution - received incorrect "
+			  "trade.execution");
+		/* if the trade should be aborted */
+		if (ret == 1) {
+			linkedlist_delete_safely(trade->node, trade_clear);
 		}
-		memcpy(trade->cp_identifier, sender_id, PUBLIC_KEY_SIZE);
-	}
-	if (memcmp(trade->order->id, trade_execution->order, SHA3_256_SIZE)) {
-		log_debug("process_trade_execution - counterparty's "
-			  "trade.execution refering to a different order");
-		linkedlist_delete_safely(trade->node, trade_clear);
 		return 0;
 	}
 
-	if (trade_update(trade, execution_step, trade_execution->data)) {
+	if (trade_update(trade,
+			 execution_step,
+			 trade_execution->data,
+			 sender_id)) {
 		log_debug("process_trade_execution - received incorrect trade "
 			  "data");
 		linkedlist_delete_safely(trade->node, trade_clear);
@@ -1003,7 +1001,7 @@ static int process_trade_proposal(const trade_proposal_t *trade_proposal,
 		return 1;
 	}
 
-	trade_update(trade, TS_PROPOSAL, trade_proposal);
+	trade_update(trade, TS_PROPOSAL, trade_proposal, sender_id);
 
 	trade->step = trade_step_get_next(trade);
 

src/trade.c

@@ -189,15 +189,22 @@ trade_t *trade_find(const linkedlist_t	*trades,
  * @param	trade		Trade to be updated.
  * @param	next_step	Next step of a trade.
  * @param	data		Next step's data.
+ * @param	sender_id	The data received from this id.
  *
  * @return	0		Successfully updated.
  * @return	1		Incorrect data.
  */
-int trade_update(trade_t *trade, enum trade_step next_step, const void *data)
+int trade_update(trade_t		*trade,
+		 enum trade_step	next_step,
+		 const void		*data,
+		 const unsigned char	*sender_id)
 {
 	switch (trade->type) {
 		case TT_BASIC:
-			return trade_basic_update(trade, next_step, data);
+			return trade_basic_update(trade,
+						  next_step,
+						  data,
+						  sender_id);
 	}
 
 	return 1;
@@ -224,6 +231,31 @@ void trade_execution_delete(trade_execution_t	*trade_execution,
 	free(trade_execution);
 }
 
+/**
+ * Verify trade.execution of a trade.
+ *
+ * @param	execution	trade.execution to be verified.
+ * @param	trade		trade.execution for this trade.
+ * @param	sender_id	trade.execution received from this id.
+ *
+ * @return	0		trade.execution is legit.
+ * @return	1		trade.execution is not legit and the trade
+ *				should be aborted.
+ * @return	2		trade.execution is not legit but the trade
+ *				does not need to be aborted.
+ */
+int trade_execution_verify(const trade_execution_t	*execution,
+			   const trade_t		*trade,
+			   const unsigned char		*sender_id)
+{
+	switch (trade->type) {
+		case TT_BASIC:
+			return trade_execution_basic_verify(execution,
+							    trade,
+							    sender_id);
+	}
+}
+
 /**
  * Initialize trade.proposal of a trade.
  *

src/trade.h

@@ -109,11 +109,17 @@ trade_t *trade_find(const linkedlist_t	*trades,
 		    const void		*attribute,
 		    int (*cmp_func) (const trade_t	*trade,
 				    const void		*attribute));
-int trade_update(trade_t *trade, enum trade_step next_step, const void *data);
+int trade_update(trade_t		*trade,
+		 enum trade_step	next_step,
+		 const void		*data,
+		 const unsigned char	*sender_id);
 
 void trade_execution_delete(trade_execution_t	*data,
 			    enum trade_type	type,
 			    enum trade_step	step);
+int trade_execution_verify(const trade_execution_t	*execution,
+			   const trade_t		*trade,
+			   const unsigned char		*sender_id);
 
 void trade_proposal_init(trade_proposal_t	*trade_proposal,
 			 const trade_t		*trade);

src/trade_basic.c

@@ -185,13 +185,15 @@ static int trade_basic_save(const trade_t *trade, const char *trades_dir)
  * @param	trade		The trade.
  * @param	next_step	Next step of a trade.
  * @param	data		The next step's data.
+ * @param	sender_id	The data received from this id.
  *
  * @return	0		Successfully updated.
  * @return	1		Incorrect input data.
  */
-int trade_basic_update(trade_t		*trade,
-		       enum trade_step	next_step,
-		       const void	*data)
+int trade_basic_update(trade_t		   *trade,
+		       enum trade_step	   next_step,
+		       const void	   *data,
+		       const unsigned char *sender_id)
 {
 	enum trade_step		cur_step;
 	trade_execution_basic_t *execution;
@@ -217,6 +219,9 @@ int trade_basic_update(trade_t		*trade,
 			memcpy(trade_data->cp_commitment,
 			       execution->commitment,
 			       SHA3_256_SIZE);
+			memcpy(trade->cp_identifier,
+			       sender_id,
+			       PUBLIC_KEY_SIZE);
 			break;
 		case TS_KEY_AND_COMMITTED_EXCHANGE:
 			memcpy(trade->cp_pubkey,
@@ -292,8 +297,62 @@ void trade_execution_basic_delete(trade_execution_basic_t *execution,
 	    step == TS_KEY_AND_COMMITTED_EXCHANGE) {
 		if (execution->script) {
 			free(execution->script);
+			execution->script = NULL;
+		}
+	}
+}
+
+/**
+ * Verify trade.execution of a basic trade.
+ *
+ * @param	execution	trade.execution to be verified.
+ * @param	trade		trade.execution for this trade.
+ * @param	sender_id	trade.execution received from this id.
+ *
+ * @return	0		trade.execution is legit.
+ * @return	1		trade.execution is not legit and the trade
+ *				should be aborted.
+ * @return	2		trade.execution is not legit but the trade
+ *				does not need to be aborted.
+ */
+int trade_execution_basic_verify(const trade_execution_t *execution,
+				 const trade_t		 *trade,
+				 const unsigned char	 *sender_id)
+{
+	const trade_execution_basic_t *exec_data;
+	char			      new_id_hex[2 * PUBLIC_KEY_SIZE + 1];
+
+	exec_data = (trade_execution_basic_t *) execution->data;
+
+	/* counterparty's identifier must remain the same, unless this is
+	 * the trade acceptance */
+	if (memcmp(trade->cp_identifier, sender_id, PUBLIC_KEY_SIZE)) {
+		if (trade->step != TS_PROPOSAL) {
+			log_debug("trade_execution_basic_verify - received "
+				  "trade.execution from a wrong peer");
+			return 1;
+		}
+		sodium_bin2hex(new_id_hex,
+			       sizeof(new_id_hex),
+			       sender_id,
+			       PUBLIC_KEY_SIZE);
+		if (verify_signature(new_id_hex,
+				     trade->cp_identifier,
+				     exec_data->idsig)) {
+			log_debug("trade_execution_basic_verify - received "
+				  "trade.execution from a wrong peer");
+			/* possible MITM attempt */
+			return 2;
 		}
 	}
+
+	if (memcmp(trade->order->id, execution->order, SHA3_256_SIZE)) {
+		log_debug("trade_execution_basic_verify - counterparty's "
+			  "trade.execution referring to a different order");
+		return 1;
+	}
+
+	return 0;
 }
 
 /**

src/trade_basic.h

@@ -55,6 +55,9 @@ typedef struct s_trade_execution_basic {
 	unsigned char committed[TRADE_BASIC_COMMITTED_SIZE];
 	/** Double hash of the secret 'x' from the first trading script. */
 	unsigned char hx[RIPEMD_160_SIZE];
+	/** Signature of the trade identifier, signed by the order's
+	 *  identity. */
+	unsigned char idsig[SIGNATURE_SIZE];
 	/** Trading public key. */
 	unsigned char pubkey[PUBLIC_KEY_SIZE];
 	/** Trading script. */
@@ -64,12 +67,16 @@ typedef struct s_trade_execution_basic {
 void trade_basic_cancel(trade_t *trade);
 void trade_basic_clear(trade_basic_t *data);
 void trade_basic_init_data(trade_basic_t *data);
-int trade_basic_update(trade_t		*trade,
-		       enum trade_step	next_step,
-		       const void	*data);
+int trade_basic_update(trade_t		   *trade,
+		       enum trade_step	   next_step,
+		       const void	   *data,
+		       const unsigned char *sender_id);
 
 void trade_execution_basic_delete(trade_execution_basic_t *execution,
 				  enum trade_step	  step);
+int trade_execution_basic_verify(const trade_execution_t *execution,
+				 const trade_t		 *trade,
+				 const unsigned char	 *sender_id);
 
 void trade_proposal_basic_init(trade_proposal_t		*trade_proposal,
 			       const trade_basic_t	*trade_data);