2020-04-08 21:58:10

by Michał Lowas-Rzechonek

[permalink] [raw]
Subject: [PATCH BlueZ 0/4] mesh: Always deliver tokens via JoinComplete

This patchset implements API change discussed in
https://marc.info/?l=linux-bluetooth&m=157660821400352&w=2

Michał Lowas-Rzechonek (1):
doc/mesh: Change API to deliver tokens via JoinComplete

Przemysław Fierek (3):
mesh: Fix invalid app_path on 'Join'
mesh: Change API to deliver tokens via JoinComplete
tools/mesh-cfgclient: Add waiting for 'JoinComplete'

doc/mesh-api.txt | 22 +++++++++----
mesh/mesh.c | 67 +++++++++++++++++++++++++++-----------
tools/mesh-cfgclient.c | 73 ++++++++++++++++++++++++------------------
3 files changed, 105 insertions(+), 57 deletions(-)

--
2.26.0


2020-04-08 21:58:47

by Michał Lowas-Rzechonek

[permalink] [raw]
Subject: [PATCH BlueZ 2/4] doc/mesh: Change API to deliver tokens via JoinComplete

If Application is not be able to reliably store the token, the daemon
will end up with a uncontrollable node in its database.

Let's fix the issue by always delivering tokens using JoinComplete call,
and expecting a reply - if the application return an error, daemon will
get rid of the node.
---
doc/mesh-api.txt | 22 +++++++++++++++-------
1 file changed, 15 insertions(+), 7 deletions(-)

diff --git a/doc/mesh-api.txt b/doc/mesh-api.txt
index c7374703b..08e34096d 100644
--- a/doc/mesh-api.txt
+++ b/doc/mesh-api.txt
@@ -29,6 +29,10 @@ Methods:
therefore attempting to call this function using already
registered UUID results in an error.

+ When provisioning finishes, the daemon will call either
+ JoinComplete or JoinFailed method on object implementing
+ org.bluez.mesh.Application1 interface.
+
PossibleErrors:
org.bluez.mesh.Error.InvalidArguments
org.bluez.mesh.Error.AlreadyExists,
@@ -123,7 +127,7 @@ Methods:
PossibleErrors:
org.bluez.mesh.Error.InvalidArguments

- uint64 token CreateNetwork(object app_root, array{byte}[16] uuid)
+ void CreateNetwork(object app_root, array{byte}[16] uuid)

This is the first method that an application calls to become
a Provisioner node, and a Configuration Client on a newly
@@ -155,11 +159,14 @@ Methods:
unicast address (0x0001), and create and assign a net_key as the
primary network net_index (0x000).

+ When creation finishes, the daemon will call JoinComplete method
+ on object implementing org.bluez.mesh.Application1 interface.
+
PossibleErrors:
org.bluez.mesh.Error.InvalidArguments
org.bluez.mesh.Error.AlreadyExists,

- uint64 token Import(object app_root, array{byte}[16] uuid,
+ void Import(object app_root, array{byte}[16] uuid,
array{byte}[16] dev_key,
array{byte}[16] net_key, uint16 net_index,
dict flags, uint32 iv_index, uint16 unicast)
@@ -204,11 +211,8 @@ Methods:
The unicast parameter is the primary unicast address of the
imported node.

- The returned token must be preserved by the application in
- order to authenticate itself to the mesh daemon and attach to
- the network as a mesh node by calling Attach() method or
- permanently remove the identity of the mesh node by calling
- Leave() method.
+ When import finishes, the daemon will call JoinComplete method
+ on object implementing org.bluez.mesh.Application1 interface.

PossibleErrors:
org.bluez.mesh.Error.InvalidArguments,
@@ -770,6 +774,10 @@ Methods:
permanently remove the identity of the mesh node by calling
Leave() method.

+ If this method returns an error, the daemon will assume that the
+ application failed to preserve the token, and will remove the
+ freshly created node.
+
void JoinFailed(string reason)

This method is called when the node provisioning initiated by
--
2.26.0

2020-04-08 21:58:47

by Michał Lowas-Rzechonek

[permalink] [raw]
Subject: [PATCH BlueZ 3/4] mesh: Change API to deliver tokens via JoinComplete

From: Przemysław Fierek <[email protected]>

This patch changes Import and CreateNetwork API to deliver tokens via
the JoinComplete method call. When application doesn't raise any error
during handling JoinComplete then it is assumed that the token has been
saved, otherwise when application replies with an error message then the
node is removed.
---
mesh/mesh.c | 63 ++++++++++++++++++++++++++++++++++++++++-------------
1 file changed, 48 insertions(+), 15 deletions(-)

diff --git a/mesh/mesh.c b/mesh/mesh.c
index a9d5d5dea..6ffeb0c2b 100644
--- a/mesh/mesh.c
+++ b/mesh/mesh.c
@@ -429,6 +429,17 @@ static void send_join_failed(const char *owner, const char *path,
free_pending_join_call(true);
}

+static void prov_join_complete_reply_cb(struct l_dbus_message *message,
+ void *user_data)
+{
+ bool failed = l_dbus_message_is_error(message);
+
+ if (!failed)
+ node_attach_io(join_pending->node, mesh.io);
+
+ free_pending_join_call(failed);
+}
+
static bool prov_complete_cb(void *user_data, uint8_t status,
struct mesh_prov_node_info *info)
{
@@ -455,7 +466,6 @@ static bool prov_complete_cb(void *user_data, uint8_t status,
return false;
}

- node_attach_io(join_pending->node, mesh.io);
token = node_get_token(join_pending->node);

msg = l_dbus_message_new_method_call(dbus, owner, path,
@@ -463,10 +473,8 @@ static bool prov_complete_cb(void *user_data, uint8_t status,
"JoinComplete");

l_dbus_message_set_arguments(msg, "t", l_get_be64(token));
-
- l_dbus_send(dbus, msg);
-
- free_pending_join_call(false);
+ l_dbus_send_with_reply(dbus, msg,
+ prov_join_complete_reply_cb, NULL, NULL);

return true;
}
@@ -660,11 +668,28 @@ static struct l_dbus_message *leave_call(struct l_dbus *dbus,
return l_dbus_message_new_method_return(msg);
}

+static void create_join_complete_reply_cb(struct l_dbus_message *message,
+ void *user_data)
+{
+ struct mesh_node *node = user_data;
+
+ if (l_dbus_message_is_error(message)) {
+ node_remove(node);
+ return;
+ }
+
+ node_attach_io(node, mesh.io);
+}
+
static void create_node_ready_cb(void *user_data, int status,
struct mesh_node *node)
{
+ struct l_dbus *dbus = dbus_get_bus();
struct l_dbus_message *reply;
struct l_dbus_message *pending_msg;
+ struct l_dbus_message *msg;
+ const char *owner;
+ const char *path;
const uint8_t *token;

pending_msg = l_queue_find(pending_queue, simple_match, user_data);
@@ -673,20 +698,28 @@ static void create_node_ready_cb(void *user_data, int status,

if (status != MESH_ERROR_NONE) {
reply = dbus_error(pending_msg, status, NULL);
- goto done;
- }

- node_attach_io(node, mesh.io);
+ l_dbus_send(dbus_get_bus(), reply);
+ l_queue_remove(pending_queue, pending_msg);
+ return;
+ }

reply = l_dbus_message_new_method_return(pending_msg);
+
+ l_dbus_send(dbus, reply);
+ l_queue_remove(pending_queue, pending_msg);
+
+ owner = l_dbus_message_get_sender(pending_msg);
+ path = node_get_app_path(node);
token = node_get_token(node);

- l_debug();
- l_dbus_message_set_arguments(reply, "t", l_get_be64(token));
+ msg = l_dbus_message_new_method_call(dbus, owner, path,
+ MESH_APPLICATION_INTERFACE,
+ "JoinComplete");

-done:
- l_dbus_send(dbus_get_bus(), reply);
- l_queue_remove(pending_queue, pending_msg);
+ l_dbus_message_set_arguments(msg, "t", l_get_be64(token));
+ l_dbus_send_with_reply(dbus, msg,
+ create_join_complete_reply_cb, node, NULL);
}

static struct l_dbus_message *create_network_call(struct l_dbus *dbus,
@@ -840,11 +873,11 @@ static void setup_network_interface(struct l_dbus_interface *iface)
"token");

l_dbus_interface_method(iface, "CreateNetwork", 0, create_network_call,
- "t", "oay", "token", "app", "uuid");
+ "", "oay", "app", "uuid");

l_dbus_interface_method(iface, "Import", 0,
import_call,
- "t", "oayayayqa{sv}uq", "token",
+ "", "oayayayqa{sv}uq",
"app", "uuid", "dev_key", "net_key",
"net_index", "flags", "iv_index",
"unicast");
--
2.26.0

2020-04-08 23:05:39

by Stotland, Inga

[permalink] [raw]
Subject: Re: [PATCH BlueZ 2/4] doc/mesh: Change API to deliver tokens via JoinComplete

Hi Michal,

On Wed, 2020-04-08 at 22:52 +0200, Michał Lowas-Rzechonek wrote:
> If Application is not be able to reliably store the token, the daemon
> will end up with a uncontrollable node in its database.
>
> Let's fix the issue by always delivering tokens using JoinComplete call,
> and expecting a reply - if the application return an error, daemon will
> get rid of the node.
> ---
> doc/mesh-api.txt | 22 +++++++++++++++-------
> 1 file changed, 15 insertions(+), 7 deletions(-)
>
> diff --git a/doc/mesh-api.txt b/doc/mesh-api.txt
> index c7374703b..08e34096d 100644
> --- a/doc/mesh-api.txt
> +++ b/doc/mesh-api.txt
> @@ -29,6 +29,10 @@ Methods:
> therefore attempting to call this function using already
> registered UUID results in an error.
>
> + When provisioning finishes, the daemon will call either
> + JoinComplete or JoinFailed method on object implementing
> + org.bluez.mesh.Application1 interface.
> +
> PossibleErrors:
> org.bluez.mesh.Error.InvalidArguments
> org.bluez.mesh.Error.AlreadyExists,
> @@ -123,7 +127,7 @@ Methods:
> PossibleErrors:
> org.bluez.mesh.Error.InvalidArguments
>
> - uint64 token CreateNetwork(object app_root, array{byte}[16] uuid)
> + void CreateNetwork(object app_root, array{byte}[16] uuid)
>
> This is the first method that an application calls to become
> a Provisioner node, and a Configuration Client on a newly
> @@ -155,11 +159,14 @@ Methods:
> unicast address (0x0001), and create and assign a net_key as the
> primary network net_index (0x000).
>
> + When creation finishes, the daemon will call JoinComplete method
> + on object implementing org.bluez.mesh.Application1 interface.
> +
> PossibleErrors:
> org.bluez.mesh.Error.InvalidArguments
> org.bluez.mesh.Error.AlreadyExists,
>
> - uint64 token Import(object app_root, array{byte}[16] uuid,
> + void Import(object app_root, array{byte}[16] uuid,

I would like to better understand the rationale behind this API change.

What situation exactly are we trying to address here:
- an app calling ImportNode or Create methods without the capacity to
actually store the returned token?
- an app calling ImportNode or Create methods and crashing before
receiving/storing the token?


> array{byte}[16] dev_key,
> array{byte}[16] net_key, uint16 net_index,
> dict flags, uint32 iv_index, uint16 unicast)
> @@ -204,11 +211,8 @@ Methods:
> The unicast parameter is the primary unicast address of the
> imported node.
>
> - The returned token must be preserved by the application in
> - order to authenticate itself to the mesh daemon and attach to
> - the network as a mesh node by calling Attach() method or
> - permanently remove the identity of the mesh node by calling
> - Leave() method.
> + When import finishes, the daemon will call JoinComplete method
> + on object implementing org.bluez.mesh.Application1 interface.
>
> PossibleErrors:
> org.bluez.mesh.Error.InvalidArguments,
> @@ -770,6 +774,10 @@ Methods:
> permanently remove the identity of the mesh node by calling
> Leave() method.
>
> + If this method returns an error, the daemon will assume that the
> + application failed to preserve the token, and will remove the
> + freshly created node.
> +
> void JoinFailed(string reason)
>
> This method is called when the node provisioning initiated by