2019-07-18 04:03:36

by Stotland, Inga

[permalink] [raw]
Subject: [PATCH BlueZ] mesh: Init keyring storage directory on node Attach()

This adds initialization of keyring storage directory when
a mesh node is attached successfully.
---
mesh/node.c | 8 ++++++++
1 file changed, 8 insertions(+)

diff --git a/mesh/node.c b/mesh/node.c
index 652551756..6b784bf8d 100644
--- a/mesh/node.c
+++ b/mesh/node.c
@@ -1656,6 +1656,14 @@ static void get_managed_objects_cb(struct l_dbus_message *msg, void *user_data)
} else
goto fail;

+ /*
+ * TODO: For now always initialize directory for storing
+ * keyring info. Need to figure out what checks need
+ * to be performed to do this conditionally, i.e., presence of
+ * Provisioner interface, etc.
+ */
+ init_storage_dir(node);
+
} else if (req->type == REQUEST_TYPE_JOIN) {
node_join_ready_func_t cb = req->cb;

--
2.21.0


2019-07-18 09:00:38

by Michał Lowas-Rzechonek

[permalink] [raw]
Subject: Re: [PATCH BlueZ] mesh: Init keyring storage directory on node Attach()

On 07/17, Inga Stotland wrote:
>This adds initialization of keyring storage directory when
>a mesh node is attached successfully.
>---
> mesh/node.c | 8 ++++++++
> 1 file changed, 8 insertions(+)
>
>diff --git a/mesh/node.c b/mesh/node.c
>index 652551756..6b784bf8d 100644
>--- a/mesh/node.c
>+++ b/mesh/node.c
>@@ -1656,6 +1656,14 @@ static void get_managed_objects_cb(struct l_dbus_message *msg, void *user_data)
> } else
> goto fail;
>
>+ /*
>+ * TODO: For now always initialize directory for storing
>+ * keyring info. Need to figure out what checks need
>+ * to be performed to do this conditionally, i.e., presence of
>+ * Provisioner interface, etc.
>+ */
>+ init_storage_dir(node);

I think the keyring should be initialized as soon ad the node is
created. The keyring should always exist, and should contain at least
the local device key - otherwise DevKeySend can't be used to talk to
local Config Server.

--
Michał Lowas-Rzechonek <[email protected]>
Silvair http://silvair.com
Jasnogórska 44, 31-358 Krakow, POLAND

2019-07-18 16:39:32

by Gix, Brian

[permalink] [raw]
Subject: RE: [PATCH BlueZ] mesh: Init keyring storage directory on node Attach()

Hi Inga, Michal,

> On 07/17, Inga Stotland wrote:
> >This adds initialization of keyring storage directory when a mesh node
> >is attached successfully.
> >---
> > mesh/node.c | 8 ++++++++
> >
> >+ /*
> >+ * TODO: For now always initialize directory for storing
> >+ * keyring info. Need to figure out what checks need
> >+ * to be performed to do this conditionally, i.e., presence of
> >+ * Provisioner interface, etc.
> >+ */
> >+ init_storage_dir(node);
>
> I think the keyring should be initialized as soon ad the node is created. The
> keyring should always exist, and should contain at least the local device key -
> otherwise DevKeySend can't be used to talk to local Config Server.

I agree that the keyring should probably always exist, but not really for the reason Michal states... There are no use case allowed in the specification that allows any Config Client except an authorized Provisioner to communicate with a Config Server (even the local Config Server)... Any changes to a nodes configuration states should be tracked by provisioners in a master database, and this is not really possible if any node is allowed to change it's own CFG Server states.

That said, A node can have configuration privileges *transferred* to it, and it is not the responsibility of the daemon to determine when this is. I am fine with creating an (empty) key ring for all nodes.... which in the current architecture just means a few empty folders.


>
> --
> Michał Lowas-Rzechonek <[email protected]>
> Silvair http://silvair.com
> Jasnogórska 44, 31-358 Krakow, POLAND

2019-07-18 17:08:35

by Gix, Brian

[permalink] [raw]
Subject: Re: [PATCH BlueZ] mesh: Init keyring storage directory on node Attach()

Applied

On Wed, 2019-07-17 at 21:01 -0700, Inga Stotland wrote:
> This adds initialization of keyring storage directory when
> a mesh node is attached successfully.
> ---
> mesh/node.c | 8 ++++++++
> 1 file changed, 8 insertions(+)
>
> diff --git a/mesh/node.c b/mesh/node.c
> index 652551756..6b784bf8d 100644
> --- a/mesh/node.c
> +++ b/mesh/node.c
> @@ -1656,6 +1656,14 @@ static void get_managed_objects_cb(struct l_dbus_message *msg, void *user_data)
> } else
> goto fail;
>
> + /*
> + * TODO: For now always initialize directory for storing
> + * keyring info. Need to figure out what checks need
> + * to be performed to do this conditionally, i.e., presence of
> + * Provisioner interface, etc.
> + */
> + init_storage_dir(node);
> +
> } else if (req->type == REQUEST_TYPE_JOIN) {
> node_join_ready_func_t cb = req->cb;
>

2019-07-18 18:55:00

by Michał Lowas-Rzechonek

[permalink] [raw]
Subject: Re: [PATCH BlueZ] mesh: Init keyring storage directory on node Attach()

Brian,

On 07/18, Gix, Brian wrote:
> There are no use case allowed in the specification that allows any
> Config Client except an authorized Provisioner to communicate with a
> Config Server (even the local Config Server)... Any changes to a
> nodes configuration states should be tracked by provisioners in a
> master database, and this is not really possible if any node is
> allowed to change it's own CFG Server states.

OTOH, there is nothing in the spec that forbids it.

For example, it's perfectly legal to create a vendor model that extends
Config Server and adds a few additional opcodes that affect Config
Server states.

Another example is multiple Configuration Clients - the spec only says
that synchronizing Provisioners is implementation specific, and it's not
Provisioners who talk to Config Servers - it's a Configuration Client, a
different role. And sychronizing Configuration Clients is not even
mentioned.

Also, for Device Keys, the spec says they are "known by nodes".
Splitting the node into a "daemon" and an "application" is a
bluez-specific architectural decision that has nothing to do with the
standard. While I appreciate there are reasons to implement the stack in
this manner, I am very much against adding arbitrary restrictions on the
things one can do with such a stack.

One of these restrictions is denying access to keys. This is fine, but
the application needs to be able to use these keys *as if it knew them*.

For example, it should be possible to implement an application-side
server model that uses DevKey security. At the moment this is not
doable, because there is no API to send a message encrypted with *local*
device key.

regards
--
Michał Lowas-Rzechonek <[email protected]>
Silvair http://silvair.com
Jasnogórska 44, 31-358 Krakow, POLAND