2019-12-11 23:29:41

by Stotland, Inga

[permalink] [raw]
Subject: [PATCH BlueZ] mesh: Add more checks for element properties

This adds consistency checks for mandatory properties on
org.bluez.mesh.Element1 interface:
- disallow duplicate models on the same element
- disallow elements with duplicate indices
- disallow configuration server model on any element but primary
---
mesh/node.c | 52 ++++++++++++++++++++++++++++++++--------------------
1 file changed, 32 insertions(+), 20 deletions(-)

diff --git a/mesh/node.c b/mesh/node.c
index f8acc78c3..1f328bd21 100644
--- a/mesh/node.c
+++ b/mesh/node.c
@@ -1081,7 +1081,7 @@ static void app_disc_cb(struct l_dbus *bus, void *user_data)
free_node_dbus_resources(node);
}

-static void get_sig_models_from_properties(struct node_element *ele,
+static bool get_sig_models_from_properties(struct node_element *ele,
struct l_dbus_message_iter *property)
{
struct l_dbus_message_iter ids;
@@ -1091,24 +1091,31 @@ static void get_sig_models_from_properties(struct node_element *ele,
ele->models = l_queue_new();

if (!l_dbus_message_iter_get_variant(property, "aq", &ids))
- return;
+ return false;

/* Bluetooth SIG defined models */
while (l_dbus_message_iter_next_entry(&ids, &mod_id)) {
struct mesh_model *mod;
uint32_t id = mod_id | VENDOR_ID_MASK;

+ /* Allow Config Server Model only on the primary element */
+ if (ele->idx != PRIMARY_ELE_IDX && id == CONFIG_SRV_MODEL)
+ return false;
+
+ /* Disallow duplicates */
if (l_queue_find(ele->models, match_model_id,
L_UINT_TO_PTR(id)))
- continue;
+ return false;

mod = mesh_model_new(ele->idx, id);

l_queue_insert(ele->models, mod, compare_model_id, NULL);
}
+
+ return true;
}

-static void get_vendor_models_from_properties(struct node_element *ele,
+static bool get_vendor_models_from_properties(struct node_element *ele,
struct l_dbus_message_iter *property)
{
struct l_dbus_message_iter ids;
@@ -1118,21 +1125,24 @@ static void get_vendor_models_from_properties(struct node_element *ele,
ele->models = l_queue_new();

if (!l_dbus_message_iter_get_variant(property, "a(qq)", &ids))
- return;
+ return false;

/* Vendor defined models */
while (l_dbus_message_iter_next_entry(&ids, &vendor_id, &mod_id)) {
struct mesh_model *mod;
uint32_t id = mod_id | (vendor_id << 16);

+ /* Disallow duplicates */
if (l_queue_find(ele->models, match_model_id,
L_UINT_TO_PTR(id)))
- continue;
+ return false;

mod = mesh_model_new(ele->idx, id);

l_queue_insert(ele->models, mod, compare_model_id, NULL);
}
+
+ return true;
}

static bool get_element_properties(struct mesh_node *node, const char *path,
@@ -1150,34 +1160,36 @@ static bool get_element_properties(struct mesh_node *node, const char *path,
ele->location = DEFAULT_LOCATION;

while (l_dbus_message_iter_next_entry(properties, &key, &var)) {
- if (!idx && !strcmp(key, "Index")) {
- if (!l_dbus_message_iter_get_variant(&var, "y",
+ if (!strcmp(key, "Index")) {
+
+ if (idx || !l_dbus_message_iter_get_variant(&var, "y",
&ele->idx))
goto fail;
+
idx = true;
- continue;
- }

- if (!mods && !strcmp(key, "Models")) {
- get_sig_models_from_properties(ele, &var);
+ } else if (!strcmp(key, "Models")) {
+
+ if (mods || !get_sig_models_from_properties(ele, &var))
+ goto fail;
+
mods = true;
- continue;
- }
+ } else if (!strcmp(key, "VendorModels")) {
+
+ if (vendor_mods ||
+ !get_vendor_models_from_properties(ele, &var))
+ goto fail;

- if (!vendor_mods && !strcmp(key, "VendorModels")) {
- get_vendor_models_from_properties(ele, &var);
vendor_mods = true;
- continue;
- }

- if (!strcmp(key, "Location")) {
+ } else if (!strcmp(key, "Location")) {
if (!l_dbus_message_iter_get_variant(&var, "q",
&ele->location))
goto fail;
- continue;
}
}

+ /* Check for the presence of the required properties */
if (!idx || !mods || !vendor_mods)
goto fail;

--
2.21.0


2019-12-15 18:00:39

by Gix, Brian

[permalink] [raw]
Subject: Re: [PATCH BlueZ] mesh: Add more checks for element properties

Applied

On Wed, 2019-12-11 at 15:28 -0800, Inga Stotland wrote:
> This adds consistency checks for mandatory properties on
> org.bluez.mesh.Element1 interface:
> - disallow duplicate models on the same element
> - disallow elements with duplicate indices
> - disallow configuration server model on any element but primary
> ---
> mesh/node.c | 52 ++++++++++++++++++++++++++++++++--------------------
> 1 file changed, 32 insertions(+), 20 deletions(-)
>
> diff --git a/mesh/node.c b/mesh/node.c
> index f8acc78c3..1f328bd21 100644
> --- a/mesh/node.c
> +++ b/mesh/node.c
> @@ -1081,7 +1081,7 @@ static void app_disc_cb(struct l_dbus *bus, void *user_data)
> free_node_dbus_resources(node);
> }
>
> -static void get_sig_models_from_properties(struct node_element *ele,
> +static bool get_sig_models_from_properties(struct node_element *ele,
> struct l_dbus_message_iter *property)
> {
> struct l_dbus_message_iter ids;
> @@ -1091,24 +1091,31 @@ static void get_sig_models_from_properties(struct node_element *ele,
> ele->models = l_queue_new();
>
> if (!l_dbus_message_iter_get_variant(property, "aq", &ids))
> - return;
> + return false;
>
> /* Bluetooth SIG defined models */
> while (l_dbus_message_iter_next_entry(&ids, &mod_id)) {
> struct mesh_model *mod;
> uint32_t id = mod_id | VENDOR_ID_MASK;
>
> + /* Allow Config Server Model only on the primary element */
> + if (ele->idx != PRIMARY_ELE_IDX && id == CONFIG_SRV_MODEL)
> + return false;
> +
> + /* Disallow duplicates */
> if (l_queue_find(ele->models, match_model_id,
> L_UINT_TO_PTR(id)))
> - continue;
> + return false;
>
> mod = mesh_model_new(ele->idx, id);
>
> l_queue_insert(ele->models, mod, compare_model_id, NULL);
> }
> +
> + return true;
> }
>
> -static void get_vendor_models_from_properties(struct node_element *ele,
> +static bool get_vendor_models_from_properties(struct node_element *ele,
> struct l_dbus_message_iter *property)
> {
> struct l_dbus_message_iter ids;
> @@ -1118,21 +1125,24 @@ static void get_vendor_models_from_properties(struct node_element *ele,
> ele->models = l_queue_new();
>
> if (!l_dbus_message_iter_get_variant(property, "a(qq)", &ids))
> - return;
> + return false;
>
> /* Vendor defined models */
> while (l_dbus_message_iter_next_entry(&ids, &vendor_id, &mod_id)) {
> struct mesh_model *mod;
> uint32_t id = mod_id | (vendor_id << 16);
>
> + /* Disallow duplicates */
> if (l_queue_find(ele->models, match_model_id,
> L_UINT_TO_PTR(id)))
> - continue;
> + return false;
>
> mod = mesh_model_new(ele->idx, id);
>
> l_queue_insert(ele->models, mod, compare_model_id, NULL);
> }
> +
> + return true;
> }
>
> static bool get_element_properties(struct mesh_node *node, const char *path,
> @@ -1150,34 +1160,36 @@ static bool get_element_properties(struct mesh_node *node, const char *path,
> ele->location = DEFAULT_LOCATION;
>
> while (l_dbus_message_iter_next_entry(properties, &key, &var)) {
> - if (!idx && !strcmp(key, "Index")) {
> - if (!l_dbus_message_iter_get_variant(&var, "y",
> + if (!strcmp(key, "Index")) {
> +
> + if (idx || !l_dbus_message_iter_get_variant(&var, "y",
> &ele->idx))
> goto fail;
> +
> idx = true;
> - continue;
> - }
>
> - if (!mods && !strcmp(key, "Models")) {
> - get_sig_models_from_properties(ele, &var);
> + } else if (!strcmp(key, "Models")) {
> +
> + if (mods || !get_sig_models_from_properties(ele, &var))
> + goto fail;
> +
> mods = true;
> - continue;
> - }
> + } else if (!strcmp(key, "VendorModels")) {
> +
> + if (vendor_mods ||
> + !get_vendor_models_from_properties(ele, &var))
> + goto fail;
>
> - if (!vendor_mods && !strcmp(key, "VendorModels")) {
> - get_vendor_models_from_properties(ele, &var);
> vendor_mods = true;
> - continue;
> - }
>
> - if (!strcmp(key, "Location")) {
> + } else if (!strcmp(key, "Location")) {
> if (!l_dbus_message_iter_get_variant(&var, "q",
> &ele->location))
> goto fail;
> - continue;
> }
> }
>
> + /* Check for the presence of the required properties */
> if (!idx || !mods || !vendor_mods)
> goto fail;
>