* [PATCH BlueZ v2] mesh: Check address range passed to ImportRemoteNode
@ 2019-07-18 8:53 Michał Lowas-Rzechonek
2019-07-18 19:49 ` Gix, Brian
0 siblings, 1 reply; 2+ messages in thread
From: Michał Lowas-Rzechonek @ 2019-07-18 8:53 UTC (permalink / raw)
To: linux-bluetooth
+added macro to check unicast ranges
+changed mesh_net_is_local_address to accept address ranges
---
This patch prevents the applicaton from managing device keys for:
- non-unicast addresses
- unicast addresses overlapping with local node address range
---
doc/mesh-api.txt | 8 ++++++++
mesh/keyring.c | 11 +++++++++++
mesh/manager.c | 10 ++++++++++
mesh/mesh-defs.h | 1 +
mesh/model.c | 2 +-
mesh/net.c | 7 +++++--
mesh/net.h | 3 ++-
7 files changed, 38 insertions(+), 4 deletions(-)
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..4b3d8b296 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_RANGE(unicast, count))
+ 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_RANGE(unicast, count))
+ 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..410e5e1c1 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,10 @@ 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, num_ele))
+ 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 +313,17 @@ 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, num_ele))
+ 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);
diff --git a/mesh/mesh-defs.h b/mesh/mesh-defs.h
index 79b38c56c..5cabf22c1 100644
--- a/mesh/mesh-defs.h
+++ b/mesh/mesh-defs.h
@@ -85,6 +85,7 @@
#define IS_UNASSIGNED(x) ((x) == UNASSIGNED_ADDRESS)
#define IS_UNICAST(x) (((x) > UNASSIGNED_ADDRESS) && \
((x) < VIRTUAL_ADDRESS_LOW))
+#define IS_UNICAST_RANGE(x, c) (IS_UNICAST(x) && IS_UNICAST(x + c - 1))
#define IS_VIRTUAL(x) (((x) >= VIRTUAL_ADDRESS_LOW) && \
((x) <= VIRTUAL_ADDRESS_HIGH))
#define IS_GROUP(x) ((((x) >= GROUP_ADDRESS_LOW) && \
diff --git a/mesh/model.c b/mesh/model.c
index 785becb5f..5dd469e0d 100644
--- a/mesh/model.c
+++ b/mesh/model.c
@@ -883,7 +883,7 @@ bool mesh_model_rx(struct mesh_node *node, bool szmict, uint32_t seq0,
forward.data);
else if (decrypt_idx == APP_IDX_DEV_REMOTE ||
(decrypt_idx == APP_IDX_DEV_LOCAL &&
- mesh_net_is_local_address(net, src)))
+ mesh_net_is_local_address(net, src, 1)))
send_dev_key_msg_rcvd(node, i, src, 0,
forward.size, forward.data);
}
diff --git a/mesh/net.c b/mesh/net.c
index f7f37675b..b73d668a9 100644
--- a/mesh/net.c
+++ b/mesh/net.c
@@ -3880,12 +3880,15 @@ bool mesh_net_have_key(struct mesh_net *net, uint16_t idx)
L_UINT_TO_PTR(idx)) != NULL);
}
-bool mesh_net_is_local_address(struct mesh_net *net, uint16_t addr)
+bool mesh_net_is_local_address(struct mesh_net *net, uint16_t src,
+ uint16_t count)
{
+ const uint16_t last = src + count - 1;
if (!net)
return false;
- return (addr >= net->src_addr && addr <= net->last_addr);
+ return (src >= net->src_addr && src <= net->last_addr) &&
+ (last >= net->src_addr && last <= net->last_addr);
}
void mesh_net_set_window_accuracy(struct mesh_net *net, uint8_t accuracy)
diff --git a/mesh/net.h b/mesh/net.h
index 7e6af8714..80b561d42 100644
--- a/mesh/net.h
+++ b/mesh/net.h
@@ -358,7 +358,8 @@ uint32_t mesh_net_friend_timeout(struct mesh_net *net, uint16_t addr);
struct mesh_io *mesh_net_get_io(struct mesh_net *net);
struct mesh_node *mesh_net_node_get(struct mesh_net *net);
bool mesh_net_have_key(struct mesh_net *net, uint16_t net_idx);
-bool mesh_net_is_local_address(struct mesh_net *net, uint16_t addr);
+bool mesh_net_is_local_address(struct mesh_net *net, uint16_t src,
+ uint16_t count);
void mesh_net_set_window_accuracy(struct mesh_net *net, uint8_t accuracy);
void mesh_net_transmit_params_set(struct mesh_net *net, uint8_t count,
uint16_t interval);
--
2.19.1
^ permalink raw reply related [flat|nested] 2+ messages in thread
* Re: [PATCH BlueZ v2] mesh: Check address range passed to ImportRemoteNode
2019-07-18 8:53 [PATCH BlueZ v2] mesh: Check address range passed to ImportRemoteNode Michał Lowas-Rzechonek
@ 2019-07-18 19:49 ` Gix, Brian
0 siblings, 0 replies; 2+ messages in thread
From: Gix, Brian @ 2019-07-18 19:49 UTC (permalink / raw)
To: michal.lowas-rzechonek, linux-bluetooth
Hi Michał,
On Thu, 2019-07-18 at 10:53 +0200, Michał Lowas-Rzechonek wrote:
> +added macro to check unicast ranges
> +changed mesh_net_is_local_address to accept address ranges
>
> ---
>
> This patch prevents the applicaton from managing device keys for:
> - non-unicast addresses
> - unicast addresses overlapping with local node address range
> ---
> doc/mesh-api.txt | 8 ++++++++
> mesh/keyring.c | 11 +++++++++++
> mesh/manager.c | 10 ++++++++++
> mesh/mesh-defs.h | 1 +
> mesh/model.c | 2 +-
> mesh/net.c | 7 +++++--
> mesh/net.h | 3 ++-
> 7 files changed, 38 insertions(+), 4 deletions(-)
>
> 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
I agree with this patch except for the error(s) thrown (here and above):
NotAuthorized means that you are not authorized to make changes to the key ring... I think in this
case this is simply another version of "InvalidArguments" although with a sub-message of "Overlapping
addresses", or "Unicast out of range" or something like that.
>
> Properties:
> dict Features [read-only]
> diff --git a/mesh/keyring.c b/mesh/keyring.c
> index 3ea83194c..4b3d8b296 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_RANGE(unicast, count))
> + 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_RANGE(unicast, count))
> + 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..410e5e1c1 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,10 @@ 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, num_ele))
> + 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 +313,17 @@ 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, num_ele))
> + 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);
> diff --git a/mesh/mesh-defs.h b/mesh/mesh-defs.h
> index 79b38c56c..5cabf22c1 100644
> --- a/mesh/mesh-defs.h
> +++ b/mesh/mesh-defs.h
> @@ -85,6 +85,7 @@
> #define IS_UNASSIGNED(x) ((x) == UNASSIGNED_ADDRESS)
> #define IS_UNICAST(x) (((x) > UNASSIGNED_ADDRESS) && \
> ((x) < VIRTUAL_ADDRESS_LOW))
> +#define IS_UNICAST_RANGE(x, c) (IS_UNICAST(x) && IS_UNICAST(x + c - 1))
> #define IS_VIRTUAL(x) (((x) >= VIRTUAL_ADDRESS_LOW) && \
> ((x) <= VIRTUAL_ADDRESS_HIGH))
> #define IS_GROUP(x) ((((x) >= GROUP_ADDRESS_LOW) && \
> diff --git a/mesh/model.c b/mesh/model.c
> index 785becb5f..5dd469e0d 100644
> --- a/mesh/model.c
> +++ b/mesh/model.c
> @@ -883,7 +883,7 @@ bool mesh_model_rx(struct mesh_node *node, bool szmict, uint32_t seq0,
> forward.data);
> else if (decrypt_idx == APP_IDX_DEV_REMOTE ||
> (decrypt_idx == APP_IDX_DEV_LOCAL &&
> - mesh_net_is_local_address(net, src)))
> + mesh_net_is_local_address(net, src, 1)))
> send_dev_key_msg_rcvd(node, i, src, 0,
> forward.size, forward.data);
> }
> diff --git a/mesh/net.c b/mesh/net.c
> index f7f37675b..b73d668a9 100644
> --- a/mesh/net.c
> +++ b/mesh/net.c
> @@ -3880,12 +3880,15 @@ bool mesh_net_have_key(struct mesh_net *net, uint16_t idx)
> L_UINT_TO_PTR(idx)) != NULL);
> }
>
> -bool mesh_net_is_local_address(struct mesh_net *net, uint16_t addr)
> +bool mesh_net_is_local_address(struct mesh_net *net, uint16_t src,
> + uint16_t count)
> {
> + const uint16_t last = src + count - 1;
> if (!net)
> return false;
>
> - return (addr >= net->src_addr && addr <= net->last_addr);
> + return (src >= net->src_addr && src <= net->last_addr) &&
> + (last >= net->src_addr && last <= net->last_addr);
> }
>
> void mesh_net_set_window_accuracy(struct mesh_net *net, uint8_t accuracy)
> diff --git a/mesh/net.h b/mesh/net.h
> index 7e6af8714..80b561d42 100644
> --- a/mesh/net.h
> +++ b/mesh/net.h
> @@ -358,7 +358,8 @@ uint32_t mesh_net_friend_timeout(struct mesh_net *net, uint16_t addr);
> struct mesh_io *mesh_net_get_io(struct mesh_net *net);
> struct mesh_node *mesh_net_node_get(struct mesh_net *net);
> bool mesh_net_have_key(struct mesh_net *net, uint16_t net_idx);
> -bool mesh_net_is_local_address(struct mesh_net *net, uint16_t addr);
> +bool mesh_net_is_local_address(struct mesh_net *net, uint16_t src,
> + uint16_t count);
> void mesh_net_set_window_accuracy(struct mesh_net *net, uint8_t accuracy);
> void mesh_net_transmit_params_set(struct mesh_net *net, uint8_t count,
> uint16_t interval);
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2019-07-18 19:49 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-07-18 8:53 [PATCH BlueZ v2] mesh: Check address range passed to ImportRemoteNode Michał Lowas-Rzechonek
2019-07-18 19:49 ` Gix, Brian
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).