[PATCH BlueZ v4 0/3] Add possibility to use remote device keys

[PATCH BlueZ v4 0/3] Add possibility to use remote device keys

[Michał Lowas-Rzechonek]

+Fix build/checkpatch errors
+Remove unused APP_IDX_NET and APP_IDX_ANY
+Fix handling of incoming packets marked with APP_IDX_DEV_REMOTE


This patchset adds support for sending and receiving messages encrypted
with remote device keys.

This plugs a 'leak' in the API where it was possible to exchange such
messages using Send()/MessageReceived() API by using 0x7fff app key
index.

In order to allow the application to receive responses from a local
Config Server model, messages originating from a local node and
encrypted using local device key are also forwarded to the application
via D-Bus (assuming they were not handled by one of internal models).

Michał Lowas-Rzechonek (3):
  mesh: Split APP_IDX_DEV into APP_IDX_DEV_LOCAL and APP_IDX_DEV_REMOTE
  mesh: Implement DevKeySend() method on Node interface
  mesh: Handle messages encrypted with a remote device key
*** BLURB HERE ***

Michał Lowas-Rzechonek (3):
  mesh: Split APP_IDX_DEV into APP_IDX_DEV_LOCAL and APP_IDX_DEV_REMOTE
  mesh: Implement DevKeySend() method on Node interface
  mesh: Handle messages encrypted with a remote device key

 mesh/cfgmod-server.c | 15 +++----
 mesh/model.c         | 98 +++++++++++++++++++++++++++++++++++++-------
 mesh/net.h           |  8 ++--
 mesh/node.c          | 59 +++++++++++++++++++++++++-
 4 files changed, 152 insertions(+), 28 deletions(-)

-- 
2.19.1

[PATCH BlueZ v4 1/3] mesh: Split APP_IDX_DEV into APP_IDX_DEV_LOCAL and APP_IDX_DEV_REMOTE

[Michał Lowas-Rzechonek]

This is needed to distinguish incoming messages encrypted using a device
key: if the key is local, the message can be forwarded to internal
models. If the key is a known remote one, it will be forwarded to the
application via DevKeyMessageReceived() API.
---
 mesh/cfgmod-server.c | 15 ++++++++-------
 mesh/model.c         | 22 +++++++++++++---------
 mesh/net.h           |  8 ++++----
 3 files changed, 25 insertions(+), 20 deletions(-)

diff --git a/mesh/cfgmod-server.c b/mesh/cfgmod-server.c
index 634ac006b..a849b5e99 100644
--- a/mesh/cfgmod-server.c
+++ b/mesh/cfgmod-server.c
@@ -69,7 +69,8 @@ static void send_pub_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_pub_get(struct mesh_node *node, uint16_t src, uint16_t dst,
@@ -254,7 +255,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,
@@ -312,7 +313,7 @@ static bool config_sub_get(struct mesh_node *node, uint16_t src, uint16_t dst,
 
 	*msg_status = (uint8_t) status;
 
-	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;
 }
 
@@ -487,7 +488,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,
@@ -540,7 +541,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);
 	}
 
@@ -758,7 +759,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)) {
@@ -1259,7 +1260,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 7401dcecb..598615c5e 100644
--- a/mesh/model.c
+++ b/mesh/model.c
@@ -306,7 +306,9 @@ 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;
@@ -356,15 +358,15 @@ 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;
+	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;
 }
@@ -952,7 +954,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;
@@ -1381,12 +1383,14 @@ 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..8848e6df0 100644
--- a/mesh/net.h
+++ b/mesh/net.h
@@ -37,10 +37,10 @@ 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	0x6fff
+#define APP_IDX_DEV_LOCAL	0x7fff
 
 #define NET_IDX_INVALID	0xffff
 #define NET_NID_INVALID	0xff
-- 
2.19.1

[PATCH BlueZ v4 2/3] mesh: Implement DevKeySend() method on Node interface

[Michał Lowas-Rzechonek]

This patch implements D-Bus DevKeySend() method of org.bluez.mesh.Node1
interface, allowing the application to send messages encrypted using
a known remote device key.

At the moment the call ignores net_index argument and sends messages
using the primary subnet.

Also, it's no longer possible to use 'magic' key_index value 0x7fff
(denoting local device key) when calling regular Send(). Applications
should use DevKeySend() instead.
---
 mesh/model.c |  9 +++++++-
 mesh/node.c  | 59 ++++++++++++++++++++++++++++++++++++++++++++++++++--
 2 files changed, 65 insertions(+), 3 deletions(-)

diff --git a/mesh/model.c b/mesh/model.c
index 598615c5e..aae913d92 100644
--- a/mesh/model.c
+++ b/mesh/model.c
@@ -39,6 +39,7 @@
 #include "mesh/dbus.h"
 #include "mesh/util.h"
 #include "mesh/model.h"
+#include "mesh/keyring.h"
 
 /* Divide and round to ceiling (up) to calculate segment count */
 #define CEILDIV(val, div) (((val) + (div) - 1) / (div))
@@ -941,6 +942,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); */
@@ -959,7 +961,12 @@ bool mesh_model_send(struct mesh_node *node, uint16_t src, uint16_t target,
 		if (!key)
 			return false;
 
-		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 = dev_key;
 		key_id = APP_ID_DEV;
 	} else {
 		key = appkey_get_key(node_get_net(node), app_idx, &key_id);
diff --git a/mesh/node.c b/mesh/node.c
index 1dcb74b4f..7133f5b2d 100644
--- a/mesh/node.c
+++ b/mesh/node.c
@@ -1974,7 +1974,11 @@ static struct l_dbus_message *send_call(struct l_dbus *dbus,
 		return dbus_error(msg, MESH_ERROR_INVALID_ARGS,
 							"Incorrect data");
 
-	if (!mesh_model_send(node, src, dst, app_idx,
+	if ((app_idx & APP_IDX_MASK) != app_idx)
+		return dbus_error(msg, MESH_ERROR_INVALID_ARGS,
+						"Invalid key_index");
+
+	if (!mesh_model_send(node, src, dst, app_idx & APP_IDX_MASK,
 				mesh_net_get_default_ttl(node->net), data, len))
 		return dbus_error(msg, MESH_ERROR_FAILED, NULL);
 
@@ -1984,6 +1988,53 @@ 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");
+
+	/* TODO: use net_idx */
+	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_NOT_FOUND, 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)
@@ -2089,7 +2140,11 @@ 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");
+						"key_index", "data");
+	l_dbus_interface_method(iface, "DevKeySend", 0, dev_key_send_call,
+						"", "oqqay", "element_path",
+						"destination", "net_index",
+						"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

[PATCH BlueZ v4 3/3] mesh: Handle messages encrypted with a remote device key

[Michał Lowas-Rzechonek]

This adds ability to receive messages encrypted using known remote
device key. Such a key must be added to the node's keyring using
ImportRemoteNode() method of org.bluez.mesh.Management1 interface.

Decrypted messages are then forwarded to the application using
DevKeyMessageReceived() D-Bus API.

Also, messages originating from a local node and encrypted using local
device key are forwarde to the application as well, if they weren't
handled by internal model. This allows e.g. receiving status messages
from a local Config Server in the application.
---
 mesh/model.c | 69 +++++++++++++++++++++++++++++++++++++++++++++++-----
 1 file changed, 63 insertions(+), 6 deletions(-)

diff --git a/mesh/model.c b/mesh/model.c
index aae913d92..0ea45987f 100644
--- a/mesh/model.c
+++ b/mesh/model.c
@@ -308,7 +308,7 @@ static void forward_model(void *a, void *b)
 
 	l_debug("model %8.8x with idx %3.3x", mod->id, fwd->idx);
 
-	if (fwd->idx != APP_IDX_DEV_LOCAL &&
+	if (fwd->idx != APP_IDX_DEV_LOCAL && fwd->idx != APP_IDX_DEV_REMOTE &&
 					!has_binding(mod->bindings, fwd->idx))
 		return;
 
@@ -359,16 +359,25 @@ 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)
 {
+	uint8_t dev_key[16];
 	const uint8_t *key;
 
 	key = node_get_device_key(node);
 	if (!key)
-		return false;
+		return -1;
 
 	if (mesh_crypto_payload_decrypt(NULL, 0, data, size, szmict, src,
 					dst, key_id, seq, iv_idx, out, key))
 		return APP_IDX_DEV_LOCAL;
 
+	if (!keyring_get_remote_dev_key(node, src, dev_key))
+		return -1;
+
+	key = dev_key;
+	if (mesh_crypto_payload_decrypt(NULL, 0, data, size, szmict, src,
+					dst, key_id, seq, iv_idx, out, key))
+		return APP_IDX_DEV_REMOTE;
+
 	return -1;
 }
 
@@ -695,6 +704,47 @@ static int add_sub(struct mesh_net *net, struct mesh_model *mod,
 	return MESH_STATUS_SUCCESS;
 }
 
+static void send_dev_key_msg_rcvd(struct mesh_node *node, uint8_t ele_idx,
+					uint16_t src, uint16_t net_idx,
+					uint16_t size, const uint8_t *data)
+{
+	struct l_dbus *dbus = dbus_get_bus();
+	struct l_dbus_message *msg;
+	struct l_dbus_message_builder *builder;
+	const char *owner;
+	const char *path;
+
+	owner = node_get_owner(node);
+	path = node_get_element_path(node, ele_idx);
+	if (!path || !owner)
+		return;
+
+	l_debug("Send \"DevKeyMessageReceived\"");
+
+	msg = l_dbus_message_new_method_call(dbus, owner, path,
+						MESH_ELEMENT_INTERFACE,
+						"DevKeyMessageReceived");
+
+	builder = l_dbus_message_builder_new(msg);
+
+	if (!l_dbus_message_builder_append_basic(builder, 'q', &src))
+		goto error;
+
+	if (!l_dbus_message_builder_append_basic(builder, 'q', &net_idx))
+		goto error;
+
+	if (!dbus_append_byte_array(builder, data, size))
+		goto error;
+
+	if (!l_dbus_message_builder_finalize(builder))
+		goto error;
+
+	l_dbus_send(dbus, msg);
+
+error:
+	l_dbus_message_builder_destroy(builder);
+}
+
 static void send_msg_rcvd(struct mesh_node *node, uint8_t ele_idx, bool is_sub,
 					uint16_t src, uint16_t key_idx,
 					uint16_t size, const uint8_t *data)
@@ -843,10 +893,17 @@ bool mesh_model_rx(struct mesh_node *node, bool szmict, uint32_t seq0,
 		 * Cycle through external models if the message has not been
 		 * handled by internal models
 		 */
-		if (forward.has_dst && !forward.done)
-			send_msg_rcvd(node, i, is_subscription, src,
-					forward.idx, forward.size,
-					forward.data);
+		if (forward.has_dst && !forward.done) {
+			if ((decrypt_idx & APP_IDX_MASK) == decrypt_idx)
+				send_msg_rcvd(node, i, is_subscription, src,
+						forward.idx, forward.size,
+						forward.data);
+			else if (decrypt_idx == APP_IDX_DEV_REMOTE ||
+				(decrypt_idx == APP_IDX_DEV_LOCAL &&
+				 mesh_net_is_local_address(net, src)))
+				send_dev_key_msg_rcvd(node, i, src, 0,
+						forward.size, forward.data);
+		}
 
 		/*
 		 * Either the message has been processed internally or
-- 
2.19.1

Re: [PATCH BlueZ v4 2/3] mesh: Implement DevKeySend() method on Node interface

[Gix, Brian]

On Wed, 2019-07-03 at 13:42 +0200, Michał Lowas-Rzechonek wrote:
> This patch implements D-Bus DevKeySend() method of org.bluez.mesh.Node1
> interface, allowing the application to send messages encrypted using
> a known remote device key.
> 
> At the moment the call ignores net_index argument and sends messages
> using the primary subnet.
> 
> Also, it's no longer possible to use 'magic' key_index value 0x7fff
> (denoting local device key) when calling regular Send(). Applications
> should use DevKeySend() instead.
> ---
>  mesh/model.c |  9 +++++++-
>  mesh/node.c  | 59 ++++++++++++++++++++++++++++++++++++++++++++++++++--
>  2 files changed, 65 insertions(+), 3 deletions(-)
> 
> diff --git a/mesh/model.c b/mesh/model.c
> index 598615c5e..aae913d92 100644
> --- a/mesh/model.c
> +++ b/mesh/model.c
> @@ -39,6 +39,7 @@
>  #include "mesh/dbus.h"
>  #include "mesh/util.h"
>  #include "mesh/model.h"
> +#include "mesh/keyring.h"
>  
>  /* Divide and round to ceiling (up) to calculate segment count */
>  #define CEILDIV(val, div) (((val) + (div) - 1) / (div))
> @@ -941,6 +942,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); */
> @@ -959,7 +961,12 @@ bool mesh_model_send(struct mesh_node *node, uint16_t src, uint16_t target,
>  		if (!key)
>  			return false;
>  
> -		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 = dev_key;
>  		key_id = APP_ID_DEV;
>  	} else {
>  		key = appkey_get_key(node_get_net(node), app_idx, &key_id);
> diff --git a/mesh/node.c b/mesh/node.c
> index 1dcb74b4f..7133f5b2d 100644
> --- a/mesh/node.c
> +++ b/mesh/node.c
> @@ -1974,7 +1974,11 @@ static struct l_dbus_message *send_call(struct l_dbus *dbus,
>  		return dbus_error(msg, MESH_ERROR_INVALID_ARGS,
>  							"Incorrect data");
>  
> -	if (!mesh_model_send(node, src, dst, app_idx,
> +	if ((app_idx & APP_IDX_MASK) != app_idx)
> +		return dbus_error(msg, MESH_ERROR_INVALID_ARGS,
> +						"Invalid key_index");
> +
> +	if (!mesh_model_send(node, src, dst, app_idx & APP_IDX_MASK,
>  				mesh_net_get_default_ttl(node->net), data, len))
>  		return dbus_error(msg, MESH_ERROR_FAILED, NULL);
>  
> @@ -1984,6 +1988,53 @@ 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");
> +
> +	/* TODO: use net_idx */
> +	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_NOT_FOUND, NULL);
> +
> +	reply = l_dbus_message_new_method_return(msg);
> +	l_dbus_message_set_arguments(reply, "");
> +
> +	return reply;

This is technically correct, but if I apply, I will be deleting replay local and reducing the above three lines
to just:

return l_dbus_message_new_method_return(msg);

which is our prefered standard of successful returns with no parameters.



> +}
> +
>  static struct l_dbus_message *publish_call(struct l_dbus *dbus,
>  						struct l_dbus_message *msg,
>  						void *user_data)
> @@ -2089,7 +2140,11 @@ 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");
> +						"key_index", "data");
> +	l_dbus_interface_method(iface, "DevKeySend", 0, dev_key_send_call,
> +						"", "oqqay", "element_path",
> +						"destination", "net_index",
> +						"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,

Re: [PATCH BlueZ v4 3/3] mesh: Handle messages encrypted with a remote device key

[Gix, Brian]

On Wed, 2019-07-03 at 13:42 +0200, Michał Lowas-Rzechonek wrote:
> This adds ability to receive messages encrypted using known remote
> device key. Such a key must be added to the node's keyring using
> ImportRemoteNode() method of org.bluez.mesh.Management1 interface.
> 
> Decrypted messages are then forwarded to the application using
> DevKeyMessageReceived() D-Bus API.
> 
> Also, messages originating from a local node and encrypted using local
> device key are forwarde to the application as well, if they weren't
> handled by internal model. This allows e.g. receiving status messages
> from a local Config Server in the application.
> ---
>  mesh/model.c | 69 +++++++++++++++++++++++++++++++++++++++++++++++-----
>  1 file changed, 63 insertions(+), 6 deletions(-)
> 
> diff --git a/mesh/model.c b/mesh/model.c
> index aae913d92..0ea45987f 100644
> --- a/mesh/model.c
> +++ b/mesh/model.c
> @@ -308,7 +308,7 @@ static void forward_model(void *a, void *b)
>  
>  	l_debug("model %8.8x with idx %3.3x", mod->id, fwd->idx);
>  
> -	if (fwd->idx != APP_IDX_DEV_LOCAL &&
> +	if (fwd->idx != APP_IDX_DEV_LOCAL && fwd->idx != APP_IDX_DEV_REMOTE &&
>  					!has_binding(mod->bindings, fwd->idx))
>  		return;
>  
> @@ -359,16 +359,25 @@ 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)
>  {
> +	uint8_t dev_key[16];
>  	const uint8_t *key;
>  
>  	key = node_get_device_key(node);
>  	if (!key)
> -		return false;
> +		return -1;
>  
>  	if (mesh_crypto_payload_decrypt(NULL, 0, data, size, szmict, src,
>  					dst, key_id, seq, iv_idx, out, key))
>  		return APP_IDX_DEV_LOCAL;
>  
> +	if (!keyring_get_remote_dev_key(node, src, dev_key))
> +		return -1;
> +
> +	key = dev_key;
> +	if (mesh_crypto_payload_decrypt(NULL, 0, data, size, szmict, src,
> +					dst, key_id, seq, iv_idx, out, key))
> +		return APP_IDX_DEV_REMOTE;
> +
>  	return -1;
>  }
>  
> @@ -695,6 +704,47 @@ static int add_sub(struct mesh_net *net, struct mesh_model *mod,
>  	return MESH_STATUS_SUCCESS;
>  }
>  
> +static void send_dev_key_msg_rcvd(struct mesh_node *node, uint8_t ele_idx,
> +					uint16_t src, uint16_t net_idx,
> +					uint16_t size, const uint8_t *data)
> +{
> +	struct l_dbus *dbus = dbus_get_bus();
> +	struct l_dbus_message *msg;
> +	struct l_dbus_message_builder *builder;
> +	const char *owner;
> +	const char *path;
> +
> +	owner = node_get_owner(node);
> +	path = node_get_element_path(node, ele_idx);
> +	if (!path || !owner)
> +		return;
> +
> +	l_debug("Send \"DevKeyMessageReceived\"");
> +
> +	msg = l_dbus_message_new_method_call(dbus, owner, path,
> +						MESH_ELEMENT_INTERFACE,
> +						"DevKeyMessageReceived");
> +
> +	builder = l_dbus_message_builder_new(msg);
> +
> +	if (!l_dbus_message_builder_append_basic(builder, 'q', &src))
> +		goto error;
> +
> +	if (!l_dbus_message_builder_append_basic(builder, 'q', &net_idx))
> +		goto error;
> +
> +	if (!dbus_append_byte_array(builder, data, size))
> +		goto error;
> +
> +	if (!l_dbus_message_builder_finalize(builder))
> +		goto error;
> +
> +	l_dbus_send(dbus, msg);
> +
> +error:
> +	l_dbus_message_builder_destroy(builder);

I think there is a potential memory leak, not just here but in the other methods that use this technique to
construct method calls to the App  (sorry I did not catch this in earlier situations)

To get to the error label, we will have a "msg" which is never sent or freed...   I think here, and every where
else where we have a potentially failed builder, We need to call l_dbus_message_unref(msg) if we don't end up
sending the msg.

But I would like Inga's opinion on this judgement on this as well.

Otherwise, I think I will be ready to apply this whole patch-set.


> +}
> +
>  static void send_msg_rcvd(struct mesh_node *node, uint8_t ele_idx, bool is_sub,
>  					uint16_t src, uint16_t key_idx,
>  					uint16_t size, const uint8_t *data)
> @@ -843,10 +893,17 @@ bool mesh_model_rx(struct mesh_node *node, bool szmict, uint32_t seq0,
>  		 * Cycle through external models if the message has not been
>  		 * handled by internal models
>  		 */
> -		if (forward.has_dst && !forward.done)
> -			send_msg_rcvd(node, i, is_subscription, src,
> -					forward.idx, forward.size,
> -					forward.data);
> +		if (forward.has_dst && !forward.done) {
> +			if ((decrypt_idx & APP_IDX_MASK) == decrypt_idx)
> +				send_msg_rcvd(node, i, is_subscription, src,
> +						forward.idx, forward.size,
> +						forward.data);
> +			else if (decrypt_idx == APP_IDX_DEV_REMOTE ||
> +				(decrypt_idx == APP_IDX_DEV_LOCAL &&
> +				 mesh_net_is_local_address(net, src)))
> +				send_dev_key_msg_rcvd(node, i, src, 0,
> +						forward.size, forward.data);
> +		}
>  
>  		/*
>  		 * Either the message has been processed internally or