2019-07-17 19:35:39

by Michał Lowas-Rzechonek

[permalink] [raw]

Subject: [PATCH BlueZ] mesh: Check address range passed to ImportRemoteNode

---

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

2019-07-17 20:25:49

by Stotland, Inga

[permalink] [raw]

Subject: Re: [PATCH BlueZ] mesh: Check address range passed to ImportRemoteNode

---

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);

---

Attachments:

smime.p7s (3.19 kB)