2019-06-28 12:52:46

by Michał Lowas-Rzechonek

[permalink] [raw]
Subject: [PATCH BlueZ 0/3] Add support for remote dev keys

This patchset adds ability to exchange messages with remote nodes using
known device keys.

Michał Lowas-Rzechonek (3):
mesh: Rename APP_IDX_DEV to APP_IDX_DEV_LOCAL, add APP_IDX_DEV_REMOTE
mesh: Add DevKeySend call
mesh: Handle messages encrypted with a remote dev key

mesh/cfgmod-server.c | 14 ++++++-------
mesh/model.c | 41 +++++++++++++++++++++++++++---------
mesh/net.h | 10 +++++----
mesh/node.c | 49 ++++++++++++++++++++++++++++++++++++++++++++
4 files changed, 93 insertions(+), 21 deletions(-)

--
2.19.1


2019-06-28 12:52:46

by Michał Lowas-Rzechonek

[permalink] [raw]
Subject: [PATCH BlueZ 1/3] mesh: Rename APP_IDX_DEV to APP_IDX_DEV_LOCAL, add APP_IDX_DEV_REMOTE

---
mesh/cfgmod-server.c | 14 +++++++-------
mesh/model.c | 20 +++++++++++---------
mesh/net.h | 10 ++++++----
3 files changed, 24 insertions(+), 20 deletions(-)

diff --git a/mesh/cfgmod-server.c b/mesh/cfgmod-server.c
index 060d7f4e4..39710fc72 100644
--- a/mesh/cfgmod-server.c
+++ b/mesh/cfgmod-server.c
@@ -69,7 +69,7 @@ static void send_pub_status(struct mesh_node *node, uint16_t src, uint16_t dst,
}

mesh_model_send(node, dst, src,
- APP_IDX_DEV, DEFAULT_TTL, msg, n);
+ APP_IDX_DEV_LOCAL, DEFAULT_TTL, msg, n);
}

static bool config_pub_get(struct mesh_node *node, uint16_t src, uint16_t dst,
@@ -261,7 +261,7 @@ static void send_sub_status(struct mesh_node *node, uint16_t src, uint16_t dst,
n += 2;
}

- mesh_model_send(node, dst, src, APP_IDX_DEV, DEFAULT_TTL, msg, n);
+ mesh_model_send(node, dst, src, APP_IDX_DEV_LOCAL, DEFAULT_TTL, msg, n);
}

static bool config_sub_get(struct mesh_node *node, uint16_t src, uint16_t dst,
@@ -319,7 +319,7 @@ static bool config_sub_get(struct mesh_node *node, uint16_t src, uint16_t dst,
else if (ret > 0)
*status = ret;

- mesh_model_send(node, dst, src, APP_IDX_DEV, DEFAULT_TTL, msg, n);
+ mesh_model_send(node, dst, src, APP_IDX_DEV_LOCAL, DEFAULT_TTL, msg, n);
return true;
}

@@ -494,7 +494,7 @@ static void send_model_app_status(struct mesh_node *node, uint16_t src,
l_put_le16(id, msg + n);
n += 2;

- mesh_model_send(node, dst, src, APP_IDX_DEV, DEFAULT_TTL, msg, n);
+ mesh_model_send(node, dst, src, APP_IDX_DEV_LOCAL, DEFAULT_TTL, msg, n);
}

static void model_app_list(struct mesh_node *node, uint16_t src, uint16_t dst,
@@ -547,7 +547,7 @@ static void model_app_list(struct mesh_node *node, uint16_t src, uint16_t dst,

if (result >= 0) {
*status = result;
- mesh_model_send(node, dst, src, APP_IDX_DEV, DEFAULT_TTL,
+ mesh_model_send(node, dst, src, APP_IDX_DEV_LOCAL, DEFAULT_TTL,
msg, n);
}

@@ -765,7 +765,7 @@ static bool cfg_srv_pkt(uint16_t src, uint32_t dst,
uint16_t interval;
uint16_t n;

- if (idx != APP_IDX_DEV)
+ if (idx != APP_IDX_DEV_LOCAL)
return false;

if (mesh_model_opcode_get(pkt, size, &opcode, &n)) {
@@ -1266,7 +1266,7 @@ static bool cfg_srv_pkt(uint16_t src, uint32_t dst,
if (n) {
/* print_packet("App Tx", long_msg ? long_msg : msg, n); */
mesh_model_send(node, unicast, src,
- APP_IDX_DEV, DEFAULT_TTL,
+ APP_IDX_DEV_LOCAL, DEFAULT_TTL,
long_msg ? long_msg : msg, n);
}
l_free(long_msg);
diff --git a/mesh/model.c b/mesh/model.c
index f29ad9af2..6cd630aa9 100644
--- a/mesh/model.c
+++ b/mesh/model.c
@@ -307,7 +307,8 @@ static void forward_model(void *a, void *b)
bool result;

l_debug("model %8.8x with idx %3.3x", mod->id, fwd->idx);
- if (fwd->idx != APP_IDX_DEV && !has_binding(mod->bindings, fwd->idx))
+
+ if (fwd->idx != APP_IDX_DEV_LOCAL && !has_binding(mod->bindings, fwd->idx))
return;

dst = fwd->dst;
@@ -357,15 +358,16 @@ static int dev_packet_decrypt(struct mesh_node *node, const uint8_t *data,
uint16_t dst, uint8_t key_id, uint32_t seq,
uint32_t iv_idx, uint8_t *out)
{
- const uint8_t *dev_key;
+ uint8_t dev_key[16];
+ const uint8_t *key;

- dev_key = node_get_device_key(node);
- if (!dev_key)
+ key = node_get_device_key(node);
+ if (!key)
return false;

if (mesh_crypto_payload_decrypt(NULL, 0, data, size, szmict, src,
- dst, key_id, seq, iv_idx, out, dev_key))
- return APP_IDX_DEV;
+ dst, key_id, seq, iv_idx, out, key))
+ return APP_IDX_DEV_LOCAL;

return -1;
}
@@ -967,7 +969,7 @@ bool mesh_model_send(struct mesh_node *node, uint16_t src, uint16_t target,
if (IS_UNASSIGNED(target))
return false;

- if (app_idx == APP_IDX_DEV) {
+ if (app_idx == APP_IDX_DEV_LOCAL) {
key = node_get_device_key(node);
if (!key)
return false;
@@ -1412,12 +1414,12 @@ struct mesh_model *mesh_model_setup(struct mesh_node *node, uint8_t ele_idx,
if (ele_idx != PRIMARY_ELE_IDX)
return NULL;

- l_queue_push_head(mod->bindings, L_UINT_TO_PTR(APP_IDX_DEV));
+ l_queue_push_head(mod->bindings, L_UINT_TO_PTR(APP_IDX_DEV_LOCAL));
return mod;
}

if (db_mod->id == CONFIG_CLI_MODEL) {
- l_queue_push_head(mod->bindings, L_UINT_TO_PTR(APP_IDX_DEV));
+ l_queue_push_head(mod->bindings, L_UINT_TO_PTR(APP_IDX_DEV_LOCAL));
return mod;
}

diff --git a/mesh/net.h b/mesh/net.h
index f19024766..a9586a99d 100644
--- a/mesh/net.h
+++ b/mesh/net.h
@@ -37,10 +37,12 @@ struct mesh_node;
#define SEQ_MASK 0xffffff

#define CREDFLAG_MASK 0x1000
-#define APP_IDX_MASK 0x0fff
-#define APP_IDX_DEV 0x7fff
-#define APP_IDX_ANY 0x8000
-#define APP_IDX_NET 0xffff
+
+#define APP_IDX_MASK 0x0fff
+#define APP_IDX_DEV_REMOTE 0x7ffe
+#define APP_IDX_DEV_LOCAL 0x7fff
+#define APP_IDX_ANY 0x8000
+#define APP_IDX_NET 0xffff

#define NET_IDX_INVALID 0xffff
#define NET_NID_INVALID 0xff
--
2.19.1

2019-06-28 12:52:46

by Michał Lowas-Rzechonek

[permalink] [raw]
Subject: [PATCH BlueZ 2/3] mesh: Add DevKeySend call

---
mesh/model.c | 7 +++++++
mesh/node.c | 49 +++++++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 56 insertions(+)

diff --git a/mesh/model.c b/mesh/model.c
index 6cd630aa9..f46cce7c1 100644
--- a/mesh/model.c
+++ b/mesh/model.c
@@ -956,6 +956,7 @@ bool mesh_model_send(struct mesh_node *node, uint16_t src, uint16_t target,
const void *msg, uint16_t msg_len)
{
uint8_t key_id;
+ uint8_t dev_key[16];
const uint8_t *key;

/* print_packet("Mod Tx", msg, msg_len); */
@@ -976,6 +977,12 @@ bool mesh_model_send(struct mesh_node *node, uint16_t src, uint16_t target,

l_debug("(%x)", app_idx);
key_id = APP_ID_DEV;
+ } else if (app_idx == APP_IDX_DEV_REMOTE) {
+ if (!keyring_get_remote_dev_key(node, target, dev_key))
+ return false;
+
+ key_id = APP_ID_DEV;
+ key = dev_key;
} else {
key = appkey_get_key(node_get_net(node), app_idx, &key_id);
if (!key) {
diff --git a/mesh/node.c b/mesh/node.c
index a2ac747a1..5733cd4ff 100644
--- a/mesh/node.c
+++ b/mesh/node.c
@@ -1969,6 +1969,52 @@ static struct l_dbus_message *send_call(struct l_dbus *dbus,
return reply;
}

+static struct l_dbus_message *dev_key_send_call(struct l_dbus *dbus,
+ struct l_dbus_message *msg,
+ void *user_data)
+{
+ struct mesh_node *node = user_data;
+ const char *sender, *ele_path;
+ struct l_dbus_message_iter iter_data;
+ struct node_element *ele;
+ uint16_t dst, net_idx, src;
+ uint8_t *data;
+ uint32_t len;
+ struct l_dbus_message *reply;
+
+ l_debug("DevKeySend");
+
+ sender = l_dbus_message_get_sender(msg);
+
+ if (strcmp(sender, node->owner))
+ return dbus_error(msg, MESH_ERROR_NOT_AUTHORIZED, NULL);
+
+ if (!l_dbus_message_get_arguments(msg, "oqqay", &ele_path, &dst,
+ &net_idx, &iter_data))
+ return dbus_error(msg, MESH_ERROR_INVALID_ARGS, NULL);
+
+ ele = l_queue_find(node->elements, match_element_path, ele_path);
+ if (!ele)
+ return dbus_error(msg, MESH_ERROR_NOT_FOUND,
+ "Element not found");
+
+ src = node_get_primary(node) + ele->idx;
+
+ if (!l_dbus_message_iter_get_fixed_array(&iter_data, &data, &len) ||
+ !len || len > MESH_MAX_ACCESS_PAYLOAD)
+ return dbus_error(msg, MESH_ERROR_INVALID_ARGS,
+ "Incorrect data");
+
+ if (!mesh_model_send(node, src, dst, APP_IDX_DEV_REMOTE,
+ mesh_net_get_default_ttl(node->net), data, len))
+ return dbus_error(msg, MESH_ERROR_FAILED, NULL);
+
+ reply = l_dbus_message_new_method_return(msg);
+ l_dbus_message_set_arguments(reply, "");
+
+ return reply;
+}
+
static struct l_dbus_message *publish_call(struct l_dbus *dbus,
struct l_dbus_message *msg,
void *user_data)
@@ -2075,6 +2121,9 @@ static void setup_node_interface(struct l_dbus_interface *iface)
l_dbus_interface_method(iface, "Send", 0, send_call, "", "oqqay",
"element_path", "destination",
"key", "data");
+ l_dbus_interface_method(iface, "DevKeySend", 0, dev_key_send_call,
+ "", "oqqay", "element_path",
+ "destination", "net", "data");
l_dbus_interface_method(iface, "Publish", 0, publish_call, "", "oqay",
"element_path", "model_id", "data");
l_dbus_interface_method(iface, "VendorPublish", 0, vendor_publish_call,
--
2.19.1

2019-06-28 13:30:44

by Michał Lowas-Rzechonek

[permalink] [raw]
Subject: Re: [PATCH BlueZ 2/3] mesh: Add DevKeySend call

Hi,

On 06/28, Michał Lowas-Rzechonek wrote:
> + if (!l_dbus_message_get_arguments(msg, "oqqay", &ele_path, &dst,
> + &net_idx, &iter_data))
> + return dbus_error(msg, MESH_ERROR_INVALID_ARGS, NULL);

I have a question here: what's the idea behind net_index argument in
this API? There is no such argument in regular Send() call, and I don't
quite get why.

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

2019-06-28 14:35:04

by Gix, Brian

[permalink] [raw]
Subject: Re: [PATCH BlueZ 2/3] mesh: Add DevKeySend call


Hi Michał,

On Fri, 2019-06-28 at 15:29 +0200, Michał Lowas-Rzechonek wrote:
> Hi,
>
> On 06/28, Michał Lowas-Rzechonek wrote:
> > + if (!l_dbus_message_get_arguments(msg, "oqqay", &ele_path, &dst,
> > + &net_idx, &iter_data))
> > + return dbus_error(msg, MESH_ERROR_INVALID_ARGS, NULL);
>
> I have a question here: what's the idea behind net_index argument in
> this API? There is no such argument in regular Send() call, and I don't

Unlike App Keys, Device keys do not have a bound Net Key... They can be sent on *any* network key. So while
sending a message on a specific App index implies the Net Key to use, the Dev Key send does not, and so needs
it to be explicit.



> quite get why.
>
> regards

2019-06-28 17:19:22

by Michał Lowas-Rzechonek

[permalink] [raw]
Subject: Re: [PATCH BlueZ 2/3] mesh: Add DevKeySend call

Brian,

On 06/28, Gix, Brian wrote:
> > I have a question here: what's the idea behind net_index argument in
> > this API? There is no such argument in regular Send() call, and I don't
>
> Unlike App Keys, Device keys do not have a bound Net Key...

Right. Thanks for the explanation (and patience...). I'll send v2 after
the weekend.

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

2019-07-01 20:00:54

by Michał Lowas-Rzechonek

[permalink] [raw]
Subject: Re: [PATCH BlueZ 2/3] mesh: Add DevKeySend call

Hi Brian,

On 06/28, Gix, Brian wrote:
> Unlike App Keys, Device keys do not have a bound Net Key... They can
> be sent on *any* network key. So while sending a message on a
> specific App index implies the Net Key to use, the Dev Key send does
> not, and so needs it to be explicit.

After digging through the code, I've noticed that at the moment
bluetooth-meshd doesn't really support sending messages using
non-primary network key - this is because of internal API limitations
(see the TODO next to send_seg function in net.c).

Would it be OK for me to start implementing SendDevKey API in a way that
always uses the primary subnet, like it's currently done with
application keys? The same applies to calling DevKeyMessageReceived() on
the application side.

I am aware that a node is supposed to respond using the same subnet that
a request was sent through, but it's not that simple to implement in one
shot...

I'd very much like to add subnet support as well, but such a patch would
be much, much larger - I think I would need to modify internal APIs to
use mesh_subnet struct instead of mesh_net, and do it in many, many
places.

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

2019-07-01 20:09:17

by Gix, Brian

[permalink] [raw]
Subject: Re: [PATCH BlueZ 2/3] mesh: Add DevKeySend call

Hi Michał,

On Mon, 2019-07-01 at 22:00 +0200, [email protected] wrote:
> Hi Brian,
>
> On 06/28, Gix, Brian wrote:
> > Unlike App Keys, Device keys do not have a bound Net Key... They can
> > be sent on *any* network key. So while sending a message on a
> > specific App index implies the Net Key to use, the Dev Key send does
> > not, and so needs it to be explicit.
>
> After digging through the code, I've noticed that at the moment
> bluetooth-meshd doesn't really support sending messages using
> non-primary network key - this is because of internal API limitations
> (see the TODO next to send_seg function in net.c).
>
> Would it be OK for me to start implementing SendDevKey API in a way that
> always uses the primary subnet, like it's currently done with
> application keys? The same applies to calling DevKeyMessageReceived() on
> the application side.

When this code was originally written, it only supported a single subnet.

I have no problem with functionality being added gradually, but we do eventually
need to be able send *everything* including all segments on the requested
subnet (not neccessarily the primary subnet). And there will be the problem that
nodes may exist that do not even have the primary subnet key.

>
> I am aware that a node is supposed to respond using the same subnet that
> a request was sent through, but it's not that simple to implement in one
> shot...
>
> I'd very much like to add subnet support as well, but such a patch would
> be much, much larger - I think I would need to modify internal APIs to
> use mesh_subnet struct instead of mesh_net, and do it in many, many
> places.


Forward progress is forward progress. I don't think any improvements will be
rejected unless they fundumentally restrict our future ability to make things
100% correct.


>
> regards

2019-07-01 20:14:29

by Michał Lowas-Rzechonek

[permalink] [raw]
Subject: Re: [PATCH BlueZ 2/3] mesh: Add DevKeySend call

Hi Brian,

On 07/01, Gix, Brian wrote:
> I have no problem with functionality being added gradually, but we do eventually
> need to be able send *everything* including all segments on the requested
> subnet (not neccessarily the primary subnet). And there will be the problem that
> nodes may exist that do not even have the primary subnet key.

Yes, I am aware of that. We even plan to use such configurations on
production.

> Forward progress is forward progress. I don't think any improvements will be
> rejected unless they fundumentally restrict our future ability to make things
> 100% correct.

Thanks, I'll try not to break anything.

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