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

["Gix, Brian" <brian.gix@intel.com>]

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,