Re: [PATCH BlueZ] mesh: Add net key index to send_seg()

["Gix, Brian" <brian.gix@intel.com>]

Hi Michał,

I like what you are trying to fix here, but I have a slightly different suggestion.

There is one use case which is being missed here: Resends of segments of a segmented Device Key message, which
do not have a bound net_key, and can use *any* net_idx.

When the initial Outgoing message is composed in mesh_net_app_send(), the net_idx is known, and should be fixed
for the life of the segmented send procedure. I think we should just add net_idx to the mesh_sar structure, and
use that each time a segment needs to be resent.

And then next, we will probably need to make sure our outgoing ACKs to incoming SAR messages are sent on the
correct net_key.

On Fri, 2020-03-27 at 14:18 +0100, Michał Lowas-Rzechonek wrote:
> From: Przemysław Fierek <przemyslaw.fierek@silvair.com>
> 
> This patch adds net key index to send_seg(). This fixes problem with
> using invalid network key to encrypt application messages.
> ---
>  mesh/net.c | 26 ++++++++++++++++----------
>  1 file changed, 16 insertions(+), 10 deletions(-)
> 
> diff --git a/mesh/net.c b/mesh/net.c
> index 0343c4c49..c9ab843fa 100644
> --- a/mesh/net.c
> +++ b/mesh/net.c
> @@ -1528,7 +1528,8 @@ static void friend_ack_rxed(struct mesh_net *net, uint32_t iv_index,
>  	l_queue_foreach(net->friends, enqueue_friend_pkt, &frnd_ack);
>  }
>  
> -static bool send_seg(struct mesh_net *net, struct mesh_sar *msg, uint8_t seg);
> +static bool send_seg(struct mesh_net *net, uint16_t net_idx,
> +					struct mesh_sar *msg, uint8_t seg);
>  
>  static void send_frnd_ack(struct mesh_net *net, uint16_t src, uint16_t dst,
>  						uint32_t hdr, uint32_t flags)
> @@ -1684,6 +1685,7 @@ static void ack_received(struct mesh_net *net, bool timeout,
>  	struct mesh_sar *outgoing;
>  	uint32_t seg_flag = 0x00000001;
>  	uint32_t ack_copy = ack_flag;
> +	uint16_t net_idx;
>  	uint16_t i;
>  
>  	l_info("ACK Rxed (%x) (to:%d): %8.8x", seq0, timeout, ack_flag);
> @@ -1734,7 +1736,9 @@ static void ack_received(struct mesh_net *net, bool timeout,
>  		l_info("Resend Seg %d net:%p dst:%x app_idx:%3.3x",
>  				i, net, outgoing->remote, outgoing->app_idx);
>  
> -		send_seg(net, outgoing, i);
> +		net_idx = appkey_net_idx(net, outgoing->app_idx);
> +
> +		send_seg(net, net_idx, outgoing, i);
>  	}
>  
>  	l_timeout_remove(outgoing->seg_timeout);
> @@ -3058,8 +3062,8 @@ bool mesh_net_flush(struct mesh_net *net)
>  	return true;
>  }
>  
> -/* TODO: add net key index */
> -static bool send_seg(struct mesh_net *net, struct mesh_sar *msg, uint8_t segO)
> +static bool send_seg(struct mesh_net *net, uint16_t net_idx,
> +					struct mesh_sar *msg, uint8_t segO)
>  {
>  	struct mesh_subnet *subnet;
>  	uint8_t seg_len;
> @@ -3068,7 +3072,6 @@ static bool send_seg(struct mesh_net *net, struct mesh_sar *msg, uint8_t segO)
>  	uint8_t packet_len;
>  	uint8_t segN = SEG_MAX(msg->segmented, msg->len);
>  	uint16_t seg_off = SEG_OFF(segO);
> -	uint32_t key_id = 0;
>  	uint32_t seq_num;
>  
>  	if (msg->segmented) {
> @@ -3109,10 +3112,13 @@ static bool send_seg(struct mesh_net *net, struct mesh_sar *msg, uint8_t segO)
>  	}
>  	print_packet("Clr-Net Tx", packet + 1, packet_len);
>  
> -	subnet = get_primary_subnet(net);
> -	key_id = subnet->net_key_tx;
> +	subnet = l_queue_find(net->subnets, match_key_index,
> +							L_UINT_TO_PTR(net_idx));
> +	if (!subnet)
> +		return false;
>  
> -	if (!net_key_encrypt(key_id, msg->iv_index, packet + 1, packet_len)) {
> +	if (!net_key_encrypt(subnet->net_key_tx, msg->iv_index, packet + 1,
> +							     packet_len)) {
>  		l_error("Failed to encode packet");
>  		return false;
>  	}
> @@ -3272,11 +3278,11 @@ bool mesh_net_app_send(struct mesh_net *net, bool frnd_cred, uint16_t src,
>  
>  		for (i = 0; i < 4; i++) {
>  			for (seg = 0; seg <= seg_max && result; seg++)
> -				result = send_seg(net, payload, seg);
> +				result = send_seg(net, net_idx, payload, seg);
>  		}
>  	} else {
>  		for (seg = 0; seg <= seg_max && result; seg++)
> -			result = send_seg(net, payload, seg);
> +			result = send_seg(net, net_idx, payload, seg);
>  	}
>  
>  	/* Reliable: Cache; Unreliable: Flush*/