2020-04-07 09:33:39

by Rafał Gajda

Subject: [PATCH BlueZ] mesh: Ignore beacons with IVU if IV already updated

When daemon receives beacon with IV=n+1, IVU=False it will
start sending messages with new IV and set sequence to 0.
However if daemon receives another beacon with IV=n+1, IVU=True it
will go back to sending messages with old IV=n (IVU will be set to
True).
Because sequence number has been reset those messages will be dropped
by replay protection and node will lose communication.

Once IV is updated daemon should not go back to using the old value.

This patch adds beacon rejection if IV has already been updated.

---
mesh/net.c | 6 ++++++
1 file changed, 6 insertions(+)

diff --git a/mesh/net.c b/mesh/net.c
index 9a56d2ee8..2f51a5ade 100644
--- a/mesh/net.c
+++ b/mesh/net.c
@@ -2717,6 +2717,12 @@ static void update_iv_ivu_state(struct mesh_net *net, uint32_t iv_index,
return;
}

+ /* Ignore beacons with IVU if IV already updated */
+ if (iv_index == net->iv_index) {
+ l_info("iv already updated");
+ return;
+ }
+
if (!net->iv_update) {
l_info("iv_upd_state = IV_UPD_UPDATING");
net->iv_upd_state = IV_UPD_UPDATING;
--
2.22.0

2020-04-08 19:47:59

by Gix, Brian

Subject: Re: [PATCH BlueZ] mesh: Ignore beacons with IVU if IV already updated

Hi Rafał,

On Tue, 2020-04-07 at 11:32 +0200, Rafał Gajda wrote:
> When daemon receives beacon with IV=n+1, IVU=False it will
> start sending messages with new IV and set sequence to 0.
> However if daemon receives another beacon with IV=n+1, IVU=True it
> will go back to sending messages with old IV=n (IVU will be set to
> True).
> Because sequence number has been reset those messages will be dropped
> by replay protection and node will lose communication.
>
> Once IV is updated daemon should not go back to using the old value.
>
> This patch adds beacon rejection if IV has already been updated.
>
> ---
> mesh/net.c | 6 ++++++
> 1 file changed, 6 insertions(+)
>
> diff --git a/mesh/net.c b/mesh/net.c
> index 9a56d2ee8..2f51a5ade 100644
> --- a/mesh/net.c
> +++ b/mesh/net.c
> @@ -2717,6 +2717,12 @@ static void update_iv_ivu_state(struct mesh_net *net, uint32_t iv_index,
> return;
> }
>
> + /* Ignore beacons with IVU if IV already updated */
> + if (iv_index == net->iv_index) {
> + l_info("iv already updated");
> + return;
> + }
> +

I see the bug you are fixing, and agree with the fix.

I have spun a v2 of this patch I would like to to OK:

1. I have eliminated the l_info output, because with a lot of nodes, this sends out a burst of "iv already
updated" strings each time this occurs (which seems to be a lot).

2. I added the equivilent check to the IV_UPD_INIT state, since this could just as easily happen on restart,
where the first beacon a daemon hears has the old IVU state as well...

I will await your OK before applying.

> if (!net->iv_update) {
> l_info("iv_upd_state = IV_UPD_UPDATING");
> net->iv_upd_state = IV_UPD_UPDATING;