Received-SPF: pass (google.com: best guess record for domain of linux-bluetooth-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
From:   Inga Stotland <inga.stotland@intel.com>
To:     linux-bluetooth@vger.kernel.org
Cc:     brian.gix@intel.com, michal.lowas-rzechonek@silvair.com,
        Inga Stotland <inga.stotland@intel.com>
Subject: [PATCH BlueZ] mesh: Add more checks for element properties
Date:   Wed, 11 Dec 2019 15:28:47 -0800
Message-Id: <20191211232847.31501-1-inga.stotland@intel.com>
MIME-Version: 1.0
Content-Transfer-Encoding: 8bit
Sender: linux-bluetooth-owner@vger.kernel.org
Precedence: bulk

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