linux-bluetooth.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH BlueZ] mesh: Check address range passed to ImportRemoteNode
@ 2019-07-05  9:14 Michał Lowas-Rzechonek
  0 siblings, 0 replies; 3+ messages in thread
From: Michał Lowas-Rzechonek @ 2019-07-05  9:14 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Michał 'Khorne' Lowas-Rzechonek

From: Michał 'Khorne' Lowas-Rzechonek <michal@rzechonek.net>

This patch disallows importing device key for:
 - non-unicast addresses
 - unicast addresses overlapping with local node address range
---
 doc/mesh-api.txt |  8 ++++++++
 mesh/keyring.c   |  9 +++++++++
 mesh/manager.c   | 13 +++++++++++++
 3 files changed, 30 insertions(+)

diff --git a/doc/mesh-api.txt b/doc/mesh-api.txt
index f2ba164a9..4fa2f9690 100644
--- a/doc/mesh-api.txt
+++ b/doc/mesh-api.txt
@@ -609,9 +609,13 @@ Methods:
 
 		This call affects the local bluetooth-meshd key database only.
 
+		It is an error to call this with address range overlapping
+		with local element addresses.
+
 		PossibleErrors:
 			org.bluez.mesh.Error.Failed
 			org.bluez.mesh.Error.InvalidArguments
+			org.bluez.mesh.Error.NotAuthorized
 
 	void DeleteRemoteNode(uint16 primary, uint8 count)
 
@@ -626,8 +630,12 @@ Methods:
 
 		This call affects the local bluetooth-meshd key database only.
 
+		It is an error to call this with address range overlapping
+		with local element addresses.
+
 		PossibleErrors:
 			org.bluez.mesh.Error.InvalidArguments
+			org.bluez.mesh.Error.NotAuthorized
 
 Properties:
 	dict Features [read-only]
diff --git a/mesh/keyring.c b/mesh/keyring.c
index 4c6d2986d..be17fb8a5 100644
--- a/mesh/keyring.c
+++ b/mesh/keyring.c
@@ -131,6 +131,9 @@ bool keyring_put_remote_dev_key(struct mesh_node *node, uint16_t unicast,
 	if (!node)
 		return false;
 
+	if (!IS_UNICAST(unicast) || !IS_UNICAST(unicast + count - 1))
+		return false;
+
 	node_path = node_path_get(node);
 
 	if (strlen(node_path) + strlen(dev_key_dir) + 1 + 4 >= PATH_MAX)
@@ -221,6 +224,9 @@ bool keyring_get_remote_dev_key(struct mesh_node *node, uint16_t unicast,
 	if (!node)
 		return false;
 
+	if (!IS_UNICAST(unicast))
+		return false;
+
 	node_path = node_path_get(node);
 	snprintf(key_file, PATH_MAX, "%s%s/%4.4x", node_path, dev_key_dir,
 								unicast);
@@ -283,6 +289,9 @@ bool keyring_del_remote_dev_key(struct mesh_node *node, uint16_t unicast,
 	if (!node)
 		return false;
 
+	if (!IS_UNICAST(unicast) || !IS_UNICAST(unicast + count - 1))
+		return false;
+
 	node_path = node_path_get(node);
 	for (i = 0; i < count; i++) {
 		snprintf(key_file, PATH_MAX, "%s%s/%4.4x", node_path,
diff --git a/mesh/manager.c b/mesh/manager.c
index ca3562512..d3c4b2e21 100644
--- a/mesh/manager.c
+++ b/mesh/manager.c
@@ -31,6 +31,7 @@
 #include "mesh/node.h"
 #include "mesh/keyring.h"
 #include "mesh/manager.h"
+#include "mesh/net.h"
 
 static struct l_dbus_message *add_node_call(struct l_dbus *dbus,
 						struct l_dbus_message *msg,
@@ -60,6 +61,7 @@ static struct l_dbus_message *import_node_call(struct l_dbus *dbus,
 						void *user_data)
 {
 	struct mesh_node *node = user_data;
+	struct mesh_net *net = node_get_net(node);
 	struct l_dbus_message_iter iter_key;
 	uint16_t primary;
 	uint8_t num_ele;
@@ -75,6 +77,11 @@ static struct l_dbus_message *import_node_call(struct l_dbus *dbus,
 		return dbus_error(msg, MESH_ERROR_INVALID_ARGS,
 							"Bad device key");
 
+	if (mesh_net_is_local_address(net, primary) ||
+			mesh_net_is_local_address(net, primary + num_ele - 1))
+		return dbus_error(msg, MESH_ERROR_NOT_AUTHORIZED,
+					"Cannot overwrite local device key");
+
 	if (!keyring_put_remote_dev_key(node, primary, num_ele, key))
 		return dbus_error(msg, MESH_ERROR_FAILED, NULL);
 
@@ -86,12 +93,18 @@ static struct l_dbus_message *delete_node_call(struct l_dbus *dbus,
 						void *user_data)
 {
 	struct mesh_node *node = user_data;
+	struct mesh_net *net = node_get_net(node);
 	uint16_t primary;
 	uint8_t num_ele;
 
 	if (!l_dbus_message_get_arguments(msg, "qy", &primary, &num_ele))
 		return dbus_error(msg, MESH_ERROR_INVALID_ARGS, NULL);
 
+	if (mesh_net_is_local_address(net, primary) ||
+			mesh_net_is_local_address(net, primary + num_ele - 1))
+		return dbus_error(msg, MESH_ERROR_NOT_AUTHORIZED,
+					"Cannot remove local device key");
+
 	keyring_del_remote_dev_key(node, primary, num_ele);
 
 	return l_dbus_message_new_method_return(msg);
-- 
2.22.0


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH BlueZ] mesh: Check address range passed to ImportRemoteNode
  2019-07-17 19:34 Michał Lowas-Rzechonek
@ 2019-07-17 20:25 ` Stotland, Inga
  0 siblings, 0 replies; 3+ messages in thread
From: Stotland, Inga @ 2019-07-17 20:25 UTC (permalink / raw)
  To: michal.lowas-rzechonek, linux-bluetooth

[-- Attachment #1: Type: text/plain, Size: 4493 bytes --]

Hi Michal,

On Wed, 2019-07-17 at 21:34 +0200, Michał Lowas-Rzechonek wrote:
> This patch disallows importing device key for:
>  - non-unicast addresses
>  - unicast addresses overlapping with local node address range
> ---
>  doc/mesh-api.txt |  8 ++++++++
>  mesh/keyring.c   | 11 +++++++++++
>  mesh/manager.c   | 12 ++++++++++++
>  3 files changed, 31 insertions(+)
> 
> diff --git a/doc/mesh-api.txt b/doc/mesh-api.txt
> index 7c2a1fafa..e5d246ae4 100644
> --- a/doc/mesh-api.txt
> +++ b/doc/mesh-api.txt
> @@ -607,9 +607,13 @@ Methods:
>  
>  		This call affects the local bluetooth-meshd key
> database only.
>  
> +		It is an error to call this with address range
> overlapping
> +		with local element addresses.
> +
>  		PossibleErrors:
>  			org.bluez.mesh.Error.Failed
>  			org.bluez.mesh.Error.InvalidArguments
> +			org.bluez.mesh.Error.NotAuthorized
>  
>  	void DeleteRemoteNode(uint16 primary, uint8 count)
>  
> @@ -624,8 +628,12 @@ Methods:
>  
>  		This call affects the local bluetooth-meshd key
> database only.
>  
> +		It is an error to call this with address range
> overlapping
> +		with local element addresses.
> +
>  		PossibleErrors:
>  			org.bluez.mesh.Error.InvalidArguments
> +			org.bluez.mesh.Error.NotAuthorized
>  
>  Properties:
>  	dict Features [read-only]
> diff --git a/mesh/keyring.c b/mesh/keyring.c
> index 3ea83194c..0b2474139 100644
> --- a/mesh/keyring.c
> +++ b/mesh/keyring.c
> @@ -128,6 +128,9 @@ bool keyring_put_remote_dev_key(struct mesh_node
> *node, uint16_t unicast,
>  	bool result = true;
>  	int fd, i;
>  
> +	if (!IS_UNICAST(unicast) || !IS_UNICAST(unicast + count - 1))
> +		return false;
> +
>  	if (!node)
>  		return false;
>  
> @@ -218,10 +221,14 @@ bool keyring_get_remote_dev_key(struct
> mesh_node *node, uint16_t unicast,
>  	bool result = false;
>  	int fd;
>  
> +	if (!IS_UNICAST(unicast))
> +		return false;
> +
>  	if (!node)
>  		return false;
>  
>  	node_path = node_get_storage_dir(node);
> +
>  	snprintf(key_file, PATH_MAX, "%s%s/%4.4x", node_path,
> dev_key_dir,
>  								unicast
> );
>  
> @@ -280,10 +287,14 @@ bool keyring_del_remote_dev_key(struct
> mesh_node *node, uint16_t unicast,
>  	char key_file[PATH_MAX];
>  	int i;
>  
> +	if (!IS_UNICAST(unicast) || !IS_UNICAST(unicast + count - 1))
> +		return false;
> +

I wonder if this deserves its own macro that can be used for validation
in the number of situations, e.g. node import, config parsing...


>  	if (!node)
>  		return false;
>  
>  	node_path = node_get_storage_dir(node);
> +
>  	for (i = 0; i < count; i++) {
>  		snprintf(key_file, PATH_MAX, "%s%s/%4.4x", node_path,
>  						dev_key_dir, unicast +
> i);
> diff --git a/mesh/manager.c b/mesh/manager.c
> index 77d7b7516..564a848d1 100644
> --- a/mesh/manager.c
> +++ b/mesh/manager.c
> @@ -282,6 +282,7 @@ static struct l_dbus_message
> *import_node_call(struct l_dbus *dbus,
>  						void *user_data)
>  {
>  	struct mesh_node *node = user_data;
> +	struct mesh_net *net = node_get_net(node);
>  	struct l_dbus_message_iter iter_key;
>  	uint16_t primary;
>  	uint8_t num_ele;
> @@ -297,6 +298,11 @@ static struct l_dbus_message
> *import_node_call(struct l_dbus *dbus,
>  		return dbus_error(msg, MESH_ERROR_INVALID_ARGS,
>  							"Bad device
> key");
>  
> +	if (mesh_net_is_local_address(net, primary) ||
> +			mesh_net_is_local_address(net, primary +
> num_ele - 1))
> +		return dbus_error(msg, MESH_ERROR_NOT_AUTHORIZED,
> +					"Cannot overwrite local device
> key");
> +
>  	if (!keyring_put_remote_dev_key(node, primary, num_ele, key))
>  		return dbus_error(msg, MESH_ERROR_FAILED, NULL);
>  
> @@ -308,12 +314,18 @@ static struct l_dbus_message
> *delete_node_call(struct l_dbus *dbus,
>  						void *user_data)
>  {
>  	struct mesh_node *node = user_data;
> +	struct mesh_net *net = node_get_net(node);
>  	uint16_t primary;
>  	uint8_t num_ele;
>  
>  	if (!l_dbus_message_get_arguments(msg, "qy", &primary,
> &num_ele))
>  		return dbus_error(msg, MESH_ERROR_INVALID_ARGS, NULL);
>  
> +	if (mesh_net_is_local_address(net, primary) ||
> +			mesh_net_is_local_address(net, primary +
> num_ele - 1))
> +		return dbus_error(msg, MESH_ERROR_NOT_AUTHORIZED,
> +					"Cannot remove local device
> key");
> +
>  	keyring_del_remote_dev_key(node, primary, num_ele);
>  
>  	return l_dbus_message_new_method_return(msg);

[-- Attachment #2: smime.p7s --]
[-- Type: application/x-pkcs7-signature, Size: 3265 bytes --]

^ permalink raw reply	[flat|nested] 3+ messages in thread

* [PATCH BlueZ] mesh: Check address range passed to ImportRemoteNode
@ 2019-07-17 19:34 Michał Lowas-Rzechonek
  2019-07-17 20:25 ` Stotland, Inga
  0 siblings, 1 reply; 3+ messages in thread
From: Michał Lowas-Rzechonek @ 2019-07-17 19:34 UTC (permalink / raw)
  To: linux-bluetooth

This patch disallows importing device key for:
 - non-unicast addresses
 - unicast addresses overlapping with local node address range
---
 doc/mesh-api.txt |  8 ++++++++
 mesh/keyring.c   | 11 +++++++++++
 mesh/manager.c   | 12 ++++++++++++
 3 files changed, 31 insertions(+)

diff --git a/doc/mesh-api.txt b/doc/mesh-api.txt
index 7c2a1fafa..e5d246ae4 100644
--- a/doc/mesh-api.txt
+++ b/doc/mesh-api.txt
@@ -607,9 +607,13 @@ Methods:
 
 		This call affects the local bluetooth-meshd key database only.
 
+		It is an error to call this with address range overlapping
+		with local element addresses.
+
 		PossibleErrors:
 			org.bluez.mesh.Error.Failed
 			org.bluez.mesh.Error.InvalidArguments
+			org.bluez.mesh.Error.NotAuthorized
 
 	void DeleteRemoteNode(uint16 primary, uint8 count)
 
@@ -624,8 +628,12 @@ Methods:
 
 		This call affects the local bluetooth-meshd key database only.
 
+		It is an error to call this with address range overlapping
+		with local element addresses.
+
 		PossibleErrors:
 			org.bluez.mesh.Error.InvalidArguments
+			org.bluez.mesh.Error.NotAuthorized
 
 Properties:
 	dict Features [read-only]
diff --git a/mesh/keyring.c b/mesh/keyring.c
index 3ea83194c..0b2474139 100644
--- a/mesh/keyring.c
+++ b/mesh/keyring.c
@@ -128,6 +128,9 @@ bool keyring_put_remote_dev_key(struct mesh_node *node, uint16_t unicast,
 	bool result = true;
 	int fd, i;
 
+	if (!IS_UNICAST(unicast) || !IS_UNICAST(unicast + count - 1))
+		return false;
+
 	if (!node)
 		return false;
 
@@ -218,10 +221,14 @@ bool keyring_get_remote_dev_key(struct mesh_node *node, uint16_t unicast,
 	bool result = false;
 	int fd;
 
+	if (!IS_UNICAST(unicast))
+		return false;
+
 	if (!node)
 		return false;
 
 	node_path = node_get_storage_dir(node);
+
 	snprintf(key_file, PATH_MAX, "%s%s/%4.4x", node_path, dev_key_dir,
 								unicast);
 
@@ -280,10 +287,14 @@ bool keyring_del_remote_dev_key(struct mesh_node *node, uint16_t unicast,
 	char key_file[PATH_MAX];
 	int i;
 
+	if (!IS_UNICAST(unicast) || !IS_UNICAST(unicast + count - 1))
+		return false;
+
 	if (!node)
 		return false;
 
 	node_path = node_get_storage_dir(node);
+
 	for (i = 0; i < count; i++) {
 		snprintf(key_file, PATH_MAX, "%s%s/%4.4x", node_path,
 						dev_key_dir, unicast + i);
diff --git a/mesh/manager.c b/mesh/manager.c
index 77d7b7516..564a848d1 100644
--- a/mesh/manager.c
+++ b/mesh/manager.c
@@ -282,6 +282,7 @@ static struct l_dbus_message *import_node_call(struct l_dbus *dbus,
 						void *user_data)
 {
 	struct mesh_node *node = user_data;
+	struct mesh_net *net = node_get_net(node);
 	struct l_dbus_message_iter iter_key;
 	uint16_t primary;
 	uint8_t num_ele;
@@ -297,6 +298,11 @@ static struct l_dbus_message *import_node_call(struct l_dbus *dbus,
 		return dbus_error(msg, MESH_ERROR_INVALID_ARGS,
 							"Bad device key");
 
+	if (mesh_net_is_local_address(net, primary) ||
+			mesh_net_is_local_address(net, primary + num_ele - 1))
+		return dbus_error(msg, MESH_ERROR_NOT_AUTHORIZED,
+					"Cannot overwrite local device key");
+
 	if (!keyring_put_remote_dev_key(node, primary, num_ele, key))
 		return dbus_error(msg, MESH_ERROR_FAILED, NULL);
 
@@ -308,12 +314,18 @@ static struct l_dbus_message *delete_node_call(struct l_dbus *dbus,
 						void *user_data)
 {
 	struct mesh_node *node = user_data;
+	struct mesh_net *net = node_get_net(node);
 	uint16_t primary;
 	uint8_t num_ele;
 
 	if (!l_dbus_message_get_arguments(msg, "qy", &primary, &num_ele))
 		return dbus_error(msg, MESH_ERROR_INVALID_ARGS, NULL);
 
+	if (mesh_net_is_local_address(net, primary) ||
+			mesh_net_is_local_address(net, primary + num_ele - 1))
+		return dbus_error(msg, MESH_ERROR_NOT_AUTHORIZED,
+					"Cannot remove local device key");
+
 	keyring_del_remote_dev_key(node, primary, num_ele);
 
 	return l_dbus_message_new_method_return(msg);
-- 
2.22.0


^ permalink raw reply related	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2019-07-17 20:25 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-07-05  9:14 [PATCH BlueZ] mesh: Check address range passed to ImportRemoteNode Michał Lowas-Rzechonek
2019-07-17 19:34 Michał Lowas-Rzechonek
2019-07-17 20:25 ` Stotland, Inga

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).