[PATCH BlueZ 0/3] mesh: Save/restore application keys

[PATCH BlueZ 0/3] mesh: Save/restore application keys

[Inga Stotland]

This set of patches fixes the application key add/update processing.
An application key needs to be saved to a local node configuration file
when:
	- a new application key is added by a configuration client
    - an existing application key is updated by a configuration client
	  during key refresh procedure.

Also, fix error codes for AppKey List operation

Inga Stotland (3):
  mesh: Separate functions for app key add and update
  mesh: Save newly added or updated app key to config file
  mesh: Return correct error code for Appkey List command

 mesh/appkey.c        |  92 ++++++++++++++++++++++---------------
 mesh/appkey.h        |   4 +-
 mesh/cfgmod-server.c |   8 +++-
 mesh/mesh-db.c       | 107 ++++++++++++++++++++-----------------------
 mesh/mesh-db.h       |   4 +-
 mesh/storage.c       |   5 +-
 6 files changed, 120 insertions(+), 100 deletions(-)

-- 
2.17.2

[PATCH BlueZ 1/3] mesh: Separate functions for app key add and update

[Inga Stotland]

This splits appkey_key_add() into two separate functions:
app_key_add() and app_key_update().
Fix checks for miscellaneous invalid conditions and return
appropriate error status.
---
 mesh/appkey.c        | 89 +++++++++++++++++++++++++-------------------
 mesh/appkey.h        |  4 +-
 mesh/cfgmod-server.c |  8 +++-
 3 files changed, 60 insertions(+), 41 deletions(-)

diff --git a/mesh/appkey.c b/mesh/appkey.c
index a437763db..f799b7782 100644
--- a/mesh/appkey.c
+++ b/mesh/appkey.c
@@ -364,8 +364,8 @@ bool appkey_have_key(struct mesh_net *net, uint16_t app_idx)
 		return true;
 }
 
-int appkey_key_add(struct mesh_net *net, uint16_t net_idx, uint16_t app_idx,
-					const uint8_t *new_key, bool update)
+int appkey_key_update(struct mesh_net *net, uint16_t net_idx, uint16_t app_idx,
+							const uint8_t *new_key)
 {
 	struct mesh_app_key *key;
 	struct l_queue *app_keys;
@@ -375,61 +375,74 @@ int appkey_key_add(struct mesh_net *net, uint16_t net_idx, uint16_t app_idx,
 	if (!app_keys)
 		return MESH_STATUS_INSUFF_RESOURCES;
 
-	key = l_queue_find(app_keys, match_key_index, L_UINT_TO_PTR(app_idx));
-
-	if (!mesh_net_have_key(net, net_idx) ||
-					(update && key->net_idx != net_idx))
+	if (!mesh_net_have_key(net, net_idx))
 		return MESH_STATUS_INVALID_NETKEY;
 
-	if (update && !key)
+	key = l_queue_find(app_keys, match_key_index, L_UINT_TO_PTR(app_idx));
+
+	if (!key)
 		return MESH_STATUS_INVALID_APPKEY;
 
+	if (key->net_idx != net_idx)
+		return MESH_STATUS_INVALID_BINDING;
+
 	mesh_net_key_refresh_phase_get(net, net_idx, &phase);
-	if (update && phase != KEY_REFRESH_PHASE_ONE)
+	if (phase != KEY_REFRESH_PHASE_ONE)
 		return MESH_STATUS_CANNOT_UPDATE;
 
+	/* Check if the key has been already successfully updated */
+	if (memcmp(new_key, key->new_key, 16) == 0)
+		return MESH_STATUS_SUCCESS;
+
+	if (!set_key(key, app_idx, new_key, true))
+		return MESH_STATUS_INSUFF_RESOURCES;
+
+	if (!storage_app_key_add(net, net_idx, app_idx, new_key, true))
+		return MESH_STATUS_STORAGE_FAIL;
+
+	return MESH_STATUS_SUCCESS;
+}
+
+int appkey_key_add(struct mesh_net *net, uint16_t net_idx, uint16_t app_idx,
+							const uint8_t *new_key)
+{
+	struct mesh_app_key *key;
+	struct l_queue *app_keys;
+
+	app_keys = mesh_net_get_app_keys(net);
+	if (!app_keys)
+		return MESH_STATUS_INSUFF_RESOURCES;
+
+	key = l_queue_find(app_keys, match_key_index, L_UINT_TO_PTR(app_idx));
 	if (key) {
 		if (memcmp(new_key, key->key, 16) == 0)
 			return MESH_STATUS_SUCCESS;
-
-		if (!update) {
-			l_debug("Failed to add key: index already stored %x",
-				(net_idx << 16) | app_idx);
+		else
 			return MESH_STATUS_IDX_ALREADY_STORED;
-		}
 	}
 
-	if (!key) {
-		if (!(l_queue_length(app_keys) < MAX_APP_KEYS))
-			return MESH_STATUS_INSUFF_RESOURCES;
-
-		key = app_key_new();
-		if (!key)
-			return MESH_STATUS_INSUFF_RESOURCES;
+	if (!mesh_net_have_key(net, net_idx))
+		return MESH_STATUS_INVALID_NETKEY;
 
-		if (!set_key(key, app_idx, new_key, false)) {
-			appkey_key_free(key);
-			return MESH_STATUS_INSUFF_RESOURCES;
-		}
+	if (l_queue_length(app_keys) >= MAX_APP_KEYS)
+		return MESH_STATUS_INSUFF_RESOURCES;
 
-		if (!storage_app_key_add(net, net_idx, app_idx, new_key,
-								false)) {
-			appkey_key_free(key);
-			return MESH_STATUS_STORAGE_FAIL;
-		}
+	key = app_key_new();
 
-		key->net_idx = net_idx;
-		key->app_idx = app_idx;
-		l_queue_push_tail(app_keys, key);
-	} else {
-		if (!set_key(key, app_idx, new_key, true))
-			return MESH_STATUS_INSUFF_RESOURCES;
+	if (!set_key(key, app_idx, new_key, false)) {
+		appkey_key_free(key);
+		return MESH_STATUS_INSUFF_RESOURCES;
+	}
 
-		if (!storage_app_key_add(net, net_idx, app_idx, new_key,
-								true))
-			return MESH_STATUS_STORAGE_FAIL;
+	if (!storage_app_key_add(net, net_idx, app_idx, new_key, false)) {
+		appkey_key_free(key);
+		return MESH_STATUS_STORAGE_FAIL;
 	}
 
+	key->net_idx = net_idx;
+	key->app_idx = app_idx;
+	l_queue_push_tail(app_keys, key);
+
 	l_queue_clear(key->replay_cache, l_free);
 
 	return MESH_STATUS_SUCCESS;
diff --git a/mesh/appkey.h b/mesh/appkey.h
index 21bc6a70e..eda82ac3b 100644
--- a/mesh/appkey.h
+++ b/mesh/appkey.h
@@ -35,7 +35,9 @@ const uint8_t *appkey_get_key(struct mesh_net *net, uint16_t app_idx,
 							uint8_t *key_id);
 bool appkey_have_key(struct mesh_net *net, uint16_t app_idx);
 int appkey_key_add(struct mesh_net *net, uint16_t net_idx, uint16_t app_idx,
-					const uint8_t *new_key, bool update);
+							const uint8_t *new_key);
+int appkey_key_update(struct mesh_net *net, uint16_t net_idx, uint16_t app_idx,
+							const uint8_t *new_key);
 int appkey_key_delete(struct mesh_net *net, uint16_t net_idx, uint16_t app_idx);
 void appkey_delete_bound_keys(struct mesh_net *net, uint16_t net_idx);
 uint8_t appkey_list(struct mesh_net *net, uint16_t net_idx, uint8_t *buf,
diff --git a/mesh/cfgmod-server.c b/mesh/cfgmod-server.c
index 899bdde2e..992d4ac6a 100644
--- a/mesh/cfgmod-server.c
+++ b/mesh/cfgmod-server.c
@@ -925,8 +925,12 @@ static bool cfg_srv_pkt(uint16_t src, uint32_t dst,
 
 		net_idx = l_get_le16(pkt) & 0xfff;
 		app_idx = l_get_le16(pkt + 1) >> 4;
-		b_res = appkey_key_add(net, net_idx, app_idx, pkt + 3,
-						opcode == OP_APPKEY_UPDATE);
+
+		if (opcode == OP_APPKEY_ADD)
+			b_res = appkey_key_add(net, net_idx, app_idx, pkt + 3);
+		else
+			b_res = appkey_key_update(net, net_idx, app_idx,
+								pkt + 3);
 
 		l_debug("Add/Update AppKey %s: Net_Idx %3.3x, App_Idx %3.3x",
 			(b_res == MESH_STATUS_SUCCESS) ? "success" : "fail",
-- 
2.17.2

[PATCH BlueZ 2/3] mesh: Save newly added or updated app key to config file

[Inga Stotland]

This separates mesh_db_app_key_add() into distinct functions:
mesh_db_app_key_add() and mesh_db_app_key_update() which will be called
based on whether an application key is newly added or updated.
---
 mesh/mesh-db.c | 107 +++++++++++++++++++++++--------------------------
 mesh/mesh-db.h |   4 +-
 mesh/storage.c |   5 ++-
 3 files changed, 57 insertions(+), 59 deletions(-)

diff --git a/mesh/mesh-db.c b/mesh/mesh-db.c
index 6486f7cff..2d518c1aa 100644
--- a/mesh/mesh-db.c
+++ b/mesh/mesh-db.c
@@ -501,85 +501,52 @@ bool mesh_db_write_device_key(json_object *jnode, uint8_t *key)
 }
 
 bool mesh_db_app_key_add(json_object *jobj, uint16_t net_idx, uint16_t app_idx,
-			 const uint8_t key[16], bool update)
+							const uint8_t key[16])
 {
 	json_object *jarray, *jentry = NULL, *jstring = NULL;
 	char buf[5];
 
 	json_object_object_get_ex(jobj, "appKeys", &jarray);
-	if (!jarray && update)
+	if (jarray)
 		return false;
 
 	if (jarray)
 		jentry = get_key_object(jarray, app_idx);
 
-	/* The key entry should exist if the key is updated */
-	if (!jentry  && update)
+	/* Do not allow direct overrwrite */
+	if (jentry)
 		return false;
 
-	if (jentry) {
-		uint8_t buf[16];
-		json_object *jvalue;
-		char *str;
-
-		json_object_object_get_ex(jentry, "key", &jvalue);
-		if (!jvalue)
-			return false;
-
-		str = (char *)json_object_get_string(jvalue);
-		if (!str2hex(str, strlen(str), buf, sizeof(buf)))
-			return false;
-
-		/* If the same key, return success */
-		if (memcmp(key, buf, 16) == 0)
-			return true;
-
+	jentry = json_object_new_object();
+	if (!jentry)
 		return false;
-	}
 
-	if (!update) {
-		jentry = json_object_new_object();
-		if (!jentry)
-			goto fail;
+	snprintf(buf, 5, "%4.4x", app_idx);
+	jstring = json_object_new_string(buf);
+	if (!jstring)
+		goto fail;
 
-		snprintf(buf, 5, "%4.4x", app_idx);
-		jstring = json_object_new_string(buf);
-		if (!jstring)
-			goto fail;
+	json_object_object_add(jentry, "index", jstring);
 
-		json_object_object_add(jentry, "index", jstring);
+	snprintf(buf, 5, "%4.4x", net_idx);
+	jstring = json_object_new_string(buf);
+	if (!jstring)
+		goto fail;
 
-		snprintf(buf, 5, "%4.4x", net_idx);
-		jstring = json_object_new_string(buf);
-		if (!jstring)
-			goto fail;
+	json_object_object_add(jentry, "boundNetKey", jstring);
 
-		json_object_object_add(jentry, "boundNetKey", jstring);
+	if (!add_key_value(jentry, "key", key))
+		goto fail;
 
-		if (!add_key_value(jentry, "key", key))
+	if (!jarray) {
+		jarray = json_object_new_array();
+		if (!jarray)
 			goto fail;
-
-		if (!jarray) {
-			jarray = json_object_new_array();
-			if (!jarray)
-				goto fail;
-			json_object_object_add(jobj, "appKeys", jarray);
-		}
-
-		json_object_array_add(jarray, jentry);
-
-	} else {
-
-		if (!json_object_object_get_ex(jentry, "key", &jstring))
-			return false;
-
-		json_object_object_add(jentry, "oldKey", jstring);
-		json_object_object_del(jentry, "key");
-
-		if (!add_key_value(jentry, "key", key))
-			return false;
+		json_object_object_add(jobj, "appKeys", jarray);
 	}
 
+	json_object_array_add(jarray, jentry);
+
 	return true;
 fail:
 
@@ -589,6 +556,32 @@ fail:
 	return false;
 }
 
+bool mesh_db_app_key_update(json_object *jobj, uint16_t app_idx,
+							const uint8_t key[16])
+{
+	json_object *jarray, *jentry = NULL, *jstring = NULL;
+	const char *str;
+
+	json_object_object_get_ex(jobj, "appKeys", &jarray);
+	if (!jarray)
+		return false;
+
+	/* The key entry should exist if the key is updated */
+	jentry = get_key_object(jarray, app_idx);
+	if (!jentry)
+		return false;
+
+	if (!json_object_object_get_ex(jentry, "key", &jstring))
+		return false;
+
+	str = json_object_get_string(jstring);
+	jstring = json_object_new_string(str);
+	json_object_object_add(jentry, "oldKey", jstring);
+	json_object_object_del(jentry, "key");
+
+	return add_key_value(jentry, "key", key);
+}
+
 bool mesh_db_app_key_del(json_object *jobj, uint16_t net_idx, uint16_t idx)
 {
 	json_object *jarray, *jarray_new;
diff --git a/mesh/mesh-db.h b/mesh/mesh-db.h
index 513ad3861..52ea05528 100644
--- a/mesh/mesh-db.h
+++ b/mesh/mesh-db.h
@@ -130,7 +130,9 @@ bool mesh_db_model_binding_add(json_object *jnode, uint8_t ele_idx, bool vendor,
 bool mesh_db_model_binding_del(json_object *jnode, uint8_t ele_idx, bool vendor,
 					uint32_t mod_id, uint16_t app_idx);
 bool mesh_db_app_key_add(json_object *jnode, uint16_t net_idx, uint16_t app_idx,
-					const uint8_t key[16], bool update);
+							const uint8_t key[16]);
+bool mesh_db_app_key_update(json_object *jobj, uint16_t app_idx,
+							const uint8_t key[16]);
 bool mesh_db_app_key_del(json_object *jobj, uint16_t net_idx, uint16_t idx);
 bool mesh_db_net_key_add(json_object *jobj, uint16_t net_idx,
 							const uint8_t key[16]);
diff --git a/mesh/storage.c b/mesh/storage.c
index d6b566a80..0b0582374 100644
--- a/mesh/storage.c
+++ b/mesh/storage.c
@@ -278,7 +278,10 @@ bool storage_app_key_add(struct mesh_net *net, uint16_t net_idx,
 	if (!jnode)
 		return false;
 
-	return mesh_db_app_key_add(jnode, net_idx, app_idx, key, update);
+	if (update)
+		return mesh_db_app_key_update(jnode, app_idx, key);
+
+	return mesh_db_app_key_add(jnode, net_idx, app_idx, key);
 }
 
 bool storage_app_key_del(struct mesh_net *net, uint16_t net_idx,
-- 
2.17.2

[PATCH BlueZ 3/3] mesh: Return correct error code for AppKey List command

[Inga Stotland]

If AppKey List command is called with a network key index that
is not valid for the node, return Invalid NetKey Index error code
---
 mesh/appkey.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/mesh/appkey.c b/mesh/appkey.c
index f799b7782..3cc1e4f0b 100644
--- a/mesh/appkey.c
+++ b/mesh/appkey.c
@@ -506,6 +506,9 @@ uint8_t appkey_list(struct mesh_net *net, uint16_t net_idx, uint8_t *buf,
 
 	*size = 0;
 
+	if (!mesh_net_have_key(net, net_idx))
+		return MESH_STATUS_INVALID_NETKEY;
+
 	app_keys = mesh_net_get_app_keys(net);
 	if (!app_keys || l_queue_isempty(app_keys))
 		return MESH_STATUS_SUCCESS;
-- 
2.17.2

RE: [PATCH BlueZ 0/3] mesh: Save/restore application keys

[Gix, Brian]

applied patch set

> -----Original Message-----
> From: Stotland, Inga
> Sent: Wednesday, February 13, 2019 7:45 PM
> To: linux-bluetooth@vger.kernel.org
> Cc: Gix, Brian <brian.gix@intel.com>; johan.hedberg@gmail.com;
> luiz.dentz@gmail.com; Stotland, Inga <inga.stotland@intel.com>
> Subject: [PATCH BlueZ 0/3] mesh: Save/restore application keys
> 
> This set of patches fixes the application key add/update processing.
> An application key needs to be saved to a local node configuration file
> when:
> 	- a new application key is added by a configuration client
>     - an existing application key is updated by a configuration client
> 	  during key refresh procedure.
> 
> Also, fix error codes for AppKey List operation
> 
> Inga Stotland (3):
>   mesh: Separate functions for app key add and update
>   mesh: Save newly added or updated app key to config file
>   mesh: Return correct error code for Appkey List command
> 
>  mesh/appkey.c        |  92 ++++++++++++++++++++++---------------
>  mesh/appkey.h        |   4 +-
>  mesh/cfgmod-server.c |   8 +++-
>  mesh/mesh-db.c       | 107 ++++++++++++++++++++-----------------------
>  mesh/mesh-db.h       |   4 +-
>  mesh/storage.c       |   5 +-
>  6 files changed, 120 insertions(+), 100 deletions(-)
> 
> --
> 2.17.2