[PATCH BlueZ v3 0/2] mesh: Fix IV Index handling during IV Update

[PATCH BlueZ v3 0/2] mesh: Fix IV Index handling during IV Update

[Michał Lowas-Rzechonek]

This patchset fixes handling of IV Index during IV Update procedure:
 - use incoming IV Index in both network and application/device nonce,
   according to incoming IVI field
 - use current IV Index when relaying packets, according to IV Update
   state

See Mesh Profile v1.0.1, section 3.10.5.

Michał Lowas-Rzechonek (2):
  mesh: Fixed handling of IVI flag in app layer
  mesh: Use current IV Index when relaying

 mesh/net-keys.c |  6 ------
 mesh/net.c      | 17 ++++++++---------
 2 files changed, 8 insertions(+), 15 deletions(-)

-- 
2.19.1

[PATCH BlueZ v3 1/2] mesh: Fixed handling of IVI flag in app layer

[Michał Lowas-Rzechonek]

Since IV Index is used in application nonces, we need to honor IVI flag
not only in network layer crypto, but also in application layer.

This means that if IVI field of incoming packet is different than in
current IV Index, try to decode *both* net and app layers using IV Index
decreased by one.
---
 mesh/net-keys.c |  6 ------
 mesh/net.c      | 17 ++++++++---------
 2 files changed, 8 insertions(+), 15 deletions(-)

diff --git a/mesh/net-keys.c b/mesh/net-keys.c
index 25f4caeb7..5be7e0b58 100644
--- a/mesh/net-keys.c
+++ b/mesh/net-keys.c
@@ -209,12 +209,6 @@ static void decrypt_net_pkt(void *a, void *b)
 uint32_t net_key_decrypt(uint32_t iv_index, const uint8_t *pkt, size_t len,
 					uint8_t **plain, size_t *plain_len)
 {
-	bool iv_flag = !!(iv_index & 1);
-	bool iv_pkt = !!(pkt[0] & 0x80);
-
-	if (iv_pkt != iv_flag)
-		iv_index--;
-
 	/* If we already successfully decrypted this packet, use cached data */
 	if (cache_id && cache_len == len && !memcmp(pkt, cache_pkt, len)) {
 		/* IV Index must match what was used to decrypt */
diff --git a/mesh/net.c b/mesh/net.c
index a597b8794..a5693f154 100644
--- a/mesh/net.c
+++ b/mesh/net.c
@@ -2489,8 +2489,13 @@ static void net_rx(void *net_ptr, void *user_data)
 	size_t out_size;
 	uint32_t key_id;
 	int8_t rssi = 0;
+	bool ivi_net = !!(net->iv_index & 1);
+	bool ivi_pkt = !!(data->data[0] & 0x80);
 
-	key_id = net_key_decrypt(net->iv_index, data->data, data->len,
+	/* if IVI flag differs, use previous IV Index */
+	uint32_t iv_index = net->iv_index - (ivi_pkt ^ ivi_net);
+
+	key_id = net_key_decrypt(iv_index, data->data, data->len,
 							&out, &out_size);
 
 	if (!key_id)
@@ -2504,16 +2509,10 @@ static void net_rx(void *net_ptr, void *user_data)
 		rssi = data->info->rssi;
 	}
 
-	relay_advice = packet_received(net, key_id, net->iv_index,
+	relay_advice = packet_received(net, key_id, iv_index,
 							out, out_size, rssi);
 	if (relay_advice > data->relay_advice) {
-		bool iv_flag = !!(net->iv_index & 1);
-		bool iv_pkt = !!(data->data[0] & 0x80);
-
-		data->iv_index = net->iv_index;
-		if (iv_pkt != iv_flag)
-			data->iv_index--;
-
+		data->iv_index = iv_index;
 		data->relay_advice = relay_advice;
 		data->key_id = key_id;
 		data->net = net;
-- 
2.19.1

[PATCH BlueZ v3 2/2] mesh: Use current IV Index when relaying

[Michał Lowas-Rzechonek]

If we are in IV Update state, packets shall be transmitted using IV
Index - 1, including relayed packets.
---
 mesh/net.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/mesh/net.c b/mesh/net.c
index a5693f154..0a6ff8eac 100644
--- a/mesh/net.c
+++ b/mesh/net.c
@@ -2512,7 +2512,7 @@ static void net_rx(void *net_ptr, void *user_data)
 	relay_advice = packet_received(net, key_id, iv_index,
 							out, out_size, rssi);
 	if (relay_advice > data->relay_advice) {
-		data->iv_index = iv_index;
+		data->iv_index = mesh_net_get_iv_index(net);
 		data->relay_advice = relay_advice;
 		data->key_id = key_id;
 		data->net = net;
-- 
2.19.1

Re: [PATCH BlueZ v3 2/2] mesh: Use current IV Index when relaying

[Michał Lowas-Rzechonek]

On 06/27, Michał Lowas-Rzechonek wrote:
> If we are in IV Update state, packets shall be transmitted using IV
> Index - 1, including relayed packets.
> ---
>  mesh/net.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/mesh/net.c b/mesh/net.c
> index a5693f154..0a6ff8eac 100644
> --- a/mesh/net.c
> +++ b/mesh/net.c
> @@ -2512,7 +2512,7 @@ static void net_rx(void *net_ptr, void *user_data)
>  	relay_advice = packet_received(net, key_id, iv_index,
>  							out, out_size, rssi);
>  	if (relay_advice > data->relay_advice) {
> -		data->iv_index = iv_index;
> +		data->iv_index = mesh_net_get_iv_index(net);

Ew, I misread the spec.

3.4.6.3 explicitly says:

"When a message is retransmitted, as defined below, the IV Index used
when retransmitting the message shall be the same as the IV Index when
it was received"

Please disregard this patch.

cheers
-- 
Michał Lowas-Rzechonek <michal.lowas-rzechonek@silvair.com>
Silvair http://silvair.com
Jasnogórska 44, 31-358 Krakow, POLAND

RE: [PATCH BlueZ v3 2/2] mesh: Use current IV Index when relaying

[Gix, Brian]

Hi Michal,

> -----Original Message-----
> From: linux-bluetooth-owner@vger.kernel.org [mailto:linux-bluetooth-
> owner@vger.kernel.org] On Behalf Of Michal Lowas-Rzechonek
> Sent: Thursday, June 27, 2019 12:41 AM
> To: linux-bluetooth@vger.kernel.org
> Cc: Gix, Brian <brian.gix@intel.com>
> Subject: [PATCH BlueZ v3 2/2] mesh: Use current IV Index when relaying
> 
> If we are in IV Update state, packets shall be transmitted using IV Index - 1,
> including relayed packets.
> ---
>  mesh/net.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/mesh/net.c b/mesh/net.c
> index a5693f154..0a6ff8eac 100644
> --- a/mesh/net.c
> +++ b/mesh/net.c
> @@ -2512,7 +2512,7 @@ static void net_rx(void *net_ptr, void *user_data)
>  	relay_advice = packet_received(net, key_id, iv_index,
>  							out, out_size, rssi);
>  	if (relay_advice > data->relay_advice) {
> -		data->iv_index = iv_index;
> +		data->iv_index = mesh_net_get_iv_index(net);

I don't think this is correct.  *relayed* packets must preserve the originators IV_Index...

At most, two different IV_Index values are considered valid at any time in the mesh.  Each node has it's own sense as to what the *Network* IV_Index is, and it shall accept that IV_Index, and 1 less...  With this difference reflected in the IVI bit of the first octet.

When acting as a relay, incoming messages must be one of these two settings to be recognized, but after decrementing TTL and re-encrypting, the IV_Index of the original sender must be preserved.  The iv_index is part of both the Network nonce and the Access layer nonce.  If a relay uses a different nonce than the originator (SRC), then the receiving DST device will be unable to decrypt the Access layer using the same nonce as the repackaged Network layer.



>  		data->relay_advice = relay_advice;
>  		data->key_id = key_id;
>  		data->net = net;
> --
> 2.19.1

Re: [PATCH BlueZ v3 2/2] mesh: Use current IV Index when relaying

[Michal Lowas-Rzechonek]

Brian,

On 06/27, Gix, Brian wrote:
> I don't think this is correct.  *relayed* packets must preserve the
> originators IV_Index...

Correct. I realized my mistake and quoted the appropriate section of the
spec - there is a message about it on the list.

The other patch stands, though - as it happens, our network is in the
middle of IV Update at the moment and I've been getting failures about
decrypting application payloads.

cheers
-- 
Michał Lowas-Rzechonek <michal.lowas-rzechonek@silvair.com>
Silvair http://silvair.com
Jasnogórska 44, 31-358 Krakow, POLAND

Re: [PATCH BlueZ v3 1/2] mesh: Fixed handling of IVI flag in app layer

[Gix, Brian]

Patch applied, Thanks.


On Thu, 2019-06-27 at 09:40 +0200, Michał Lowas-Rzechonek wrote:
> Since IV Index is used in application nonces, we need to honor IVI flag
> not only in network layer crypto, but also in application layer.
> 
> This means that if IVI field of incoming packet is different than in
> current IV Index, try to decode *both* net and app layers using IV Index
> decreased by one.
> ---
>  mesh/net-keys.c |  6 ------
>  mesh/net.c      | 17 ++++++++---------
>  2 files changed, 8 insertions(+), 15 deletions(-)
> 
> diff --git a/mesh/net-keys.c b/mesh/net-keys.c
> index 25f4caeb7..5be7e0b58 100644
> --- a/mesh/net-keys.c
> +++ b/mesh/net-keys.c
> @@ -209,12 +209,6 @@ static void decrypt_net_pkt(void *a, void *b)
>  uint32_t net_key_decrypt(uint32_t iv_index, const uint8_t *pkt, size_t len,
>  					uint8_t **plain, size_t *plain_len)
>  {
> -	bool iv_flag = !!(iv_index & 1);
> -	bool iv_pkt = !!(pkt[0] & 0x80);
> -
> -	if (iv_pkt != iv_flag)
> -		iv_index--;
> -
>  	/* If we already successfully decrypted this packet, use cached data */
>  	if (cache_id && cache_len == len && !memcmp(pkt, cache_pkt, len)) {
>  		/* IV Index must match what was used to decrypt */
> diff --git a/mesh/net.c b/mesh/net.c
> index a597b8794..a5693f154 100644
> --- a/mesh/net.c
> +++ b/mesh/net.c
> @@ -2489,8 +2489,13 @@ static void net_rx(void *net_ptr, void *user_data)
>  	size_t out_size;
>  	uint32_t key_id;
>  	int8_t rssi = 0;
> +	bool ivi_net = !!(net->iv_index & 1);
> +	bool ivi_pkt = !!(data->data[0] & 0x80);
>  
> -	key_id = net_key_decrypt(net->iv_index, data->data, data->len,
> +	/* if IVI flag differs, use previous IV Index */
> +	uint32_t iv_index = net->iv_index - (ivi_pkt ^ ivi_net);
> +
> +	key_id = net_key_decrypt(iv_index, data->data, data->len,
>  							&out, &out_size);
>  
>  	if (!key_id)
> @@ -2504,16 +2509,10 @@ static void net_rx(void *net_ptr, void *user_data)
>  		rssi = data->info->rssi;
>  	}
>  
> -	relay_advice = packet_received(net, key_id, net->iv_index,
> +	relay_advice = packet_received(net, key_id, iv_index,
>  							out, out_size, rssi);
>  	if (relay_advice > data->relay_advice) {
> -		bool iv_flag = !!(net->iv_index & 1);
> -		bool iv_pkt = !!(data->data[0] & 0x80);
> -
> -		data->iv_index = net->iv_index;
> -		if (iv_pkt != iv_flag)
> -			data->iv_index--;
> -
> +		data->iv_index = iv_index;
>  		data->relay_advice = relay_advice;
>  		data->key_id = key_id;
>  		data->net = net;