2021-01-02 22:31:17

by Michael N. Moran

[permalink] [raw]
Subject: [PATCH BlueZ] mesh: Update AppKeys on transition to Phase 0

At the end of the mesh Key Refresh procedure when a subnet
transitions to Phase 0, local AppKeys that were updated were
not updating until the bluetooth-meshd daemon was restarted.

This patch iterates the AppKeys at the end of mesh Key Refresh
when the subnet transitions to Phase 0, setting the new state
of each updated AppKey.

---
mesh/appkey.c | 18 ++++++++++++++++++
mesh/appkey.h | 1 +
mesh/net.c | 2 ++
3 files changed, 21 insertions(+)

diff --git a/mesh/appkey.c b/mesh/appkey.c
index 549f5a80d..504f67aab 100644
--- a/mesh/appkey.c
+++ b/mesh/appkey.c
@@ -50,6 +50,24 @@ static bool match_bound_key(const void *a, const void *b)
return key->net_idx == idx;
}

+void finish_app_key(void *a, void *b)
+{
+ struct mesh_app_key *key = a;
+ uint16_t net_idx = L_PTR_TO_UINT(b);
+
+ if (key->net_idx != net_idx)
+ return;
+
+ if (key->new_key_aid == NET_NID_INVALID)
+ return;
+
+ key->key_aid = key->new_key_aid;
+
+ key->new_key_aid = NET_NID_INVALID;
+
+ memcpy(key->key, key->new_key, 16);
+}
+
static struct mesh_app_key *app_key_new(void)
{
struct mesh_app_key *key = l_new(struct mesh_app_key, 1);
diff --git a/mesh/appkey.h b/mesh/appkey.h
index 3bb70445b..c83dd03f6 100644
--- a/mesh/appkey.h
+++ b/mesh/appkey.h
@@ -16,6 +16,7 @@ struct mesh_app_key;
bool appkey_key_init(struct mesh_net *net, uint16_t net_idx, uint16_t app_idx,
uint8_t *key_value, uint8_t *new_key_value);
void appkey_key_free(void *data);
+void finish_app_key(void *a, void *b);
const uint8_t *appkey_get_key(struct mesh_net *net, uint16_t app_idx,
uint8_t *key_id);
int appkey_get_key_idx(struct mesh_app_key *app_key,
diff --git a/mesh/net.c b/mesh/net.c
index b24cdba77..22ec03d7a 100644
--- a/mesh/net.c
+++ b/mesh/net.c
@@ -2600,6 +2600,8 @@ static int key_refresh_finish(struct mesh_net *net, uint16_t idx)

l_queue_foreach(net->friends, frnd_kr_phase3, net);

+ l_queue_foreach(net->app_keys, finish_app_key, L_UINT_TO_PTR(idx));
+
if (!mesh_config_net_key_set_phase(node_config_get(net->node), idx,
KEY_REFRESH_PHASE_NONE))
return MESH_STATUS_STORAGE_FAIL;
--
2.26.2


2021-01-02 22:49:37

by bluez.test.bot

[permalink] [raw]
Subject: RE: [BlueZ] mesh: Update AppKeys on transition to Phase 0

This is automated email and please do not reply to this email!

Dear submitter,

Thank you for submitting the patches to the linux bluetooth mailing list.
This is a CI test results with your patch series:
PW Link:https://patchwork.kernel.org/project/bluetooth/list/?series=408205

---Test result---

##############################
Test: CheckPatch - PASS

##############################
Test: CheckGitLint - PASS

##############################
Test: CheckBuild - PASS

##############################
Test: MakeCheck - PASS



---
Regards,
Linux Bluetooth

2021-01-03 23:18:45

by Gix, Brian

[permalink] [raw]
Subject: Re: [PATCH BlueZ] mesh: Update AppKeys on transition to Phase 0

Hi Michael,

On Sat, 2021-01-02 at 17:27 -0500, Michael N. Moran wrote:
> At the end of the mesh Key Refresh procedure when a subnet
> transitions to Phase 0, local AppKeys that were updated were
> not updating until the bluetooth-meshd daemon was restarted.
>
> This patch iterates the AppKeys at the end of mesh Key Refresh
> when the subnet transitions to Phase 0, setting the new state
> of each updated AppKey.
>
> ---
> mesh/appkey.c | 18 ++++++++++++++++++
> mesh/appkey.h | 1 +
> mesh/net.c | 2 ++
> 3 files changed, 21 insertions(+)
>
> diff --git a/mesh/appkey.c b/mesh/appkey.c
> index 549f5a80d..504f67aab 100644
> --- a/mesh/appkey.c
> +++ b/mesh/appkey.c
> @@ -50,6 +50,24 @@ static bool match_bound_key(const void *a, const void *b)
> return key->net_idx == idx;
> }
>
> +void finish_app_key(void *a, void *b)
> +{
> + struct mesh_app_key *key = a;
> + uint16_t net_idx = L_PTR_TO_UINT(b);
> +
> + if (key->net_idx != net_idx)
> + return;
> +
> + if (key->new_key_aid == NET_NID_INVALID)
> + return;
> +
> + key->key_aid = key->new_key_aid;
> +
> + key->new_key_aid = NET_NID_INVALID;
> +
> + memcpy(key->key, key->new_key, 16);
> +}
> +
> static struct mesh_app_key *app_key_new(void)
> {
> struct mesh_app_key *key = l_new(struct mesh_app_key, 1);
> diff --git a/mesh/appkey.h b/mesh/appkey.h
> index 3bb70445b..c83dd03f6 100644
> --- a/mesh/appkey.h
> +++ b/mesh/appkey.h
> @@ -16,6 +16,7 @@ struct mesh_app_key;
> bool appkey_key_init(struct mesh_net *net, uint16_t net_idx, uint16_t app_idx,
> uint8_t *key_value, uint8_t *new_key_value);
> void appkey_key_free(void *data);
> +void finish_app_key(void *a, void *b);
> const uint8_t *appkey_get_key(struct mesh_net *net, uint16_t app_idx,
> uint8_t *key_id);
> int appkey_get_key_idx(struct mesh_app_key *app_key,
> diff --git a/mesh/net.c b/mesh/net.c
> index b24cdba77..22ec03d7a 100644
> --- a/mesh/net.c
> +++ b/mesh/net.c
> @@ -2600,6 +2600,8 @@ static int key_refresh_finish(struct mesh_net *net, uint16_t idx)
>
> l_queue_foreach(net->friends, frnd_kr_phase3, net);
>
> + l_queue_foreach(net->app_keys, finish_app_key, L_UINT_TO_PTR(idx));
> +

I like where you are going with this, BUT...

Probably create a function in appkey.c with the prototype:
void appkey_finalize(struct mesh_net *net, uint16_t net_idx);

and simply calling that from net.c...

1. Note "public" naming convention of appkey_*
2. We try not to make l_queue* callbacks public, so that function is static, called from a wrapper function
3. We have been favoring "finalize" as the prefered terminology for key refresh cpompletion
4. Maybe use either 0xFF instead of the equivilent NET_NID_INVALID.... AID values are *not* NIDs, and we
should be using NIDs and AIDs interchangably... Maybe we need an APP_AID_INVALID which also is 0xFF. This is
an oversight on my part which is not harmful, but also not strictly correct.



> if (!mesh_config_net_key_set_phase(node_config_get(net->node), idx,
> KEY_REFRESH_PHASE_NONE))
> return MESH_STATUS_STORAGE_FAIL;