Hi Brian,
On Thu, 2019-09-26 at 11:14 -0700, Brian Gix wrote:
> When sending or receiving Device Key (privileged) mesh messages, the
> remote vs local device key must be specified. This allows Apps to
> specify Key Ring stored device keys, and sanity checks that the
> correct
> key exists before allowing the transmission. Loopback messages to
> local
> servers *must* use keys from the Key Ring to indicate privilege has
> been
> granted.
> ---
> mesh/model.c | 11 +++++++----
> mesh/node.c | 25 +++++++++++++++----------
> 2 files changed, 22 insertions(+), 14 deletions(-)
>
> diff --git a/mesh/model.c b/mesh/model.c
> index a06b684a5..e9b346102 100644
> --- a/mesh/model.c
> +++ b/mesh/model.c
> @@ -735,14 +735,16 @@ static int add_sub(struct mesh_net *net, struct
> mesh_model *mod,
> }
>
> 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)
> + uint16_t src, uint16_t app_idx,
> + 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;
> + bool remote = (app_idx != APP_IDX_DEV_LOCAL);
>
> owner = node_get_owner(node);
> path = node_get_element_path(node, ele_idx);
> @@ -758,6 +760,7 @@ static void send_dev_key_msg_rcvd(struct
> mesh_node *node, uint8_t ele_idx,
> builder = l_dbus_message_builder_new(msg);
>
> l_dbus_message_builder_append_basic(builder, 'q', &src);
> + l_dbus_message_builder_append_basic(builder, 'b', &remote);
> l_dbus_message_builder_append_basic(builder, 'q', &net_idx);
> dbus_append_byte_array(builder, data, size);
>
> @@ -936,8 +939,8 @@ bool mesh_model_rx(struct mesh_node *node, bool
> szmict, uint32_t seq0,
> else if (decrypt_idx == APP_IDX_DEV_REMOTE ||
> (decrypt_idx == APP_IDX_DEV_LOCAL &&
> mesh_net_is_local_address(net, src,
> 1)))
> - send_dev_key_msg_rcvd(node, i, src, 0,
> - forward.size,
> forward.data);
> + send_dev_key_msg_rcvd(node, i, src,
> decrypt_idx,
> + 0, forward.size,
> forward.data);
> }
>
> /*
> diff --git a/mesh/node.c b/mesh/node.c
> index b6824f505..833377e99 100644
> --- a/mesh/node.c
> +++ b/mesh/node.c
> @@ -1976,7 +1976,8 @@ static struct l_dbus_message
> *dev_key_send_call(struct l_dbus *dbus,
> const char *sender, *ele_path;
> struct l_dbus_message_iter iter_data;
> struct node_element *ele;
> - uint16_t dst, net_idx, src;
> + uint16_t dst, app_idx, net_idx, src;
> + bool remote;
> uint8_t *data;
> uint32_t len;
>
> @@ -1987,8 +1988,12 @@ static struct l_dbus_message
> *dev_key_send_call(struct l_dbus *dbus,
> 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))
> + if (!l_dbus_message_get_arguments(msg, "oqbqay", &ele_path,
> &dst,
> + &remote, &net_idx,
> &iter_data))
> + return dbus_error(msg, MESH_ERROR_INVALID_ARGS, NULL);
> +
> + /* Loopbacks to local servers must use *remote* addressing */
> + if (!remote && mesh_net_is_local_address(node->net, dst, 1))
> return dbus_error(msg, MESH_ERROR_INVALID_ARGS, NULL);
>
> ele = l_queue_find(node->elements, match_element_path,
> ele_path);
> @@ -1999,13 +2004,13 @@ static struct l_dbus_message
> *dev_key_send_call(struct l_dbus *dbus,
> src = node_get_primary(node) + ele->idx;
>
> if (!l_dbus_message_iter_get_fixed_array(&iter_data, &data,
> &len) ||
> - !len || len > MAX_MSG_LEN)
> + !len || len >
> MAX_MSG_LEN)
> 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,
> net_idx,
> - DEFAULT_TTL,
> data, len))
> + app_idx = remote ? APP_IDX_DEV_REMOTE : APP_IDX_DEV_LOCAL;
> + if (!mesh_model_send(node, src, dst, app_idx, net_idx,
> DEFAULT_TTL,
> + data,
> len))
> return dbus_error(msg, MESH_ERROR_NOT_FOUND, NULL);
I think that mesh_model_send() should be modified to return an error
code (int) instead of boolean. Otherwise, it may fail for a different
reason than a mismatch in device key and the returned error is
misleading.
In fact, the Send() call returns D-Bus "Failed" error upon getting
"false" from mesh_model_send() and this is not documented in the API
doc.
This probably should go a separate fix.
>
> return l_dbus_message_new_method_return(msg);
> @@ -2226,9 +2231,9 @@ static void setup_node_interface(struct
> l_dbus_interface *iface)
> "element_path",
> "destination",
> "key_index", "data");
> l_dbus_interface_method(iface, "DevKeySend", 0,
> dev_key_send_call,
> - "", "oqqay",
> "element_path",
> - "destination",
> "net_index",
> - "data");
> + "", "oqbqay",
> "element_path",
> + "destination",
> "remote",
> + "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,
Attachment:
smime.p7s
Description: S/MIME cryptographic signature
