From: Przemysław Fierek <[email protected]>
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*/
--
2.20.1
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 <[email protected]>
>
> 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*/
Hi Brian,
On 03/27, Gix, Brian wrote:
> 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.
Thanks. This sounds good. We'll get back to you with v2 next week.
regards
--
Michał Lowas-Rzechonek <[email protected]>
Silvair http://silvair.com
Jasnogórska 44, 31-358 Krakow, POLAND