2020-04-01 10:25:45

by Michał Lowas-Rzechonek

[permalink] [raw]
Subject: [PATCH BlueZ 0/6] Honor provisioner's capabilities during authentication

This patch adds ability for the provisioner application to declare a set
of supported authentication methods via ProvisionAgent1 API. The daemon
will then select the most secure method available on both ends.

This fixes an issue where nodes declaring OOB public key availability
could not be provisioned by applications lacking means to obtain such
keys.

v2:
- fixed memory leak when displaying OOB data in cfgclient
- fixed action bitmask endianness between mesh_agent_prov_caps and
mesh_net_prov_caps

Michał Lowas-Rzechonek (6):
tools/mesh-cfgclient: Display unprovisioned OOB data
mesh: Remove unused 'server' argument
mesh: Clean up naming of provisioning callbacks
mesh: Refresh provisioner's capabilities
mesh: Honor provisioner's capabilities
doc/mesh-api: OOB Information field is 16 bit, not 32

doc/mesh-api.txt | 2 +-
mesh/agent.c | 105 ++++++++++++++++++++++++-----
mesh/agent.h | 2 +
mesh/manager.c | 33 ++++++---
mesh/prov-initiator.c | 148 +++++++++++++++++++++++++++--------------
mesh/provision.h | 6 +-
tools/mesh-cfgclient.c | 20 ++++++
7 files changed, 236 insertions(+), 80 deletions(-)

--
2.20.1


2020-04-01 10:25:45

by Michał Lowas-Rzechonek

[permalink] [raw]
Subject: [PATCH BlueZ v2 5/6] mesh: Honor provisioner's capabilities

This patch makes the daemon select authentication method based from
capabilities supported by both provisioned node and provisioner
application.
---
mesh/prov-initiator.c | 82 ++++++++++++++++++++++++++----------------
tools/mesh-cfgclient.c | 7 ++++
2 files changed, 58 insertions(+), 31 deletions(-)

diff --git a/mesh/prov-initiator.c b/mesh/prov-initiator.c
index 17bda6521..8d523e9c0 100644
--- a/mesh/prov-initiator.c
+++ b/mesh/prov-initiator.c
@@ -38,6 +38,8 @@
#include "mesh/agent.h"
#include "mesh/error.h"

+#define MIN(x, y) ((x) < (y) ? (x) : (y))
+
/* Quick size sanity check */
static const uint16_t expected_pdu_size[] = {
2, /* PROV_INVITE */
@@ -587,6 +589,44 @@ failure:
int_prov_close(prov, fail_code[1]);
}

+static void int_prov_start_auth(const struct mesh_agent_prov_caps *prov_caps,
+ const struct mesh_net_prov_caps *dev_caps,
+ struct prov_start *start)
+{
+ uint8_t pub_type = prov_caps->pub_type & dev_caps->pub_type;
+ uint8_t static_type = prov_caps->static_type & dev_caps->static_type;
+ uint16_t output_action = prov_caps->output_action &
+ L_BE16_TO_CPU(dev_caps->output_action);
+ uint8_t output_size = MIN(prov_caps->output_size,
+ dev_caps->output_size);
+ uint16_t input_action = prov_caps->input_action &
+ L_BE16_TO_CPU(dev_caps->input_action);
+ uint8_t input_size = MIN(prov_caps->input_size, dev_caps->input_size);
+
+ if (pub_type)
+ start->pub_key = 0x01;
+
+ /* Parse OOB Options, prefer static, then out, then in */
+ if (static_type) {
+ start->auth_method = 0x01;
+ return;
+ }
+
+ if(output_size && output_action) {
+ start->auth_method = 0x02;
+ start->auth_action = u16_high_bit(output_action);
+ start->auth_size = MIN(output_size, 8);
+ return;
+ }
+
+ if (input_size && input_action) {
+ start->auth_method = 0x03;
+ start->auth_action = u16_high_bit(input_action);
+ start->auth_size = MIN(input_size, 8);
+ return;
+ }
+}
+
static void int_prov_rx(void *user_data, const uint8_t *data, uint16_t len)
{
struct mesh_prov_initiator *rx_prov = user_data;
@@ -638,43 +678,23 @@ static void int_prov_rx(void *user_data, const uint8_t *data, uint16_t len)
goto failure;
}

- /* If Public Key available Out of Band, use it */
- if (prov->conf_inputs.caps.pub_type) {
- prov->conf_inputs.start.pub_key = 0x01;
+ /*
+ * Select auth mechanism from methods supported by both
+ * parties.
+ */
+ int_prov_start_auth(mesh_agent_get_caps(prov->agent),
+ &prov->conf_inputs.caps,
+ &prov->conf_inputs.start);
+
+ if (prov->conf_inputs.start.pub_key == 0x01) {
prov->expected = PROV_CONFIRM;
/* Prompt Agent for remote Public Key */
mesh_agent_request_public_key(prov->agent,
pub_key_cb, prov);
-
/* Nothing else for us to do now */
} else
prov->expected = PROV_PUB_KEY;

- /* Parse OOB Options, prefer static, then out, then in */
- if (prov->conf_inputs.caps.static_type) {
-
- prov->conf_inputs.start.auth_method = 0x01;
-
- } else if (prov->conf_inputs.caps.output_size &&
- prov->conf_inputs.caps.output_action) {
-
- prov->conf_inputs.start.auth_method = 0x02;
- prov->conf_inputs.start.auth_action =
- u16_high_bit(l_get_be16(data + 6));
- prov->conf_inputs.start.auth_size =
- (data[5] > 8 ? 8 : data[5]);
-
- } else if (prov->conf_inputs.caps.input_size &&
- prov->conf_inputs.caps.input_action) {
-
- prov->conf_inputs.start.auth_method = 0x03;
- prov->conf_inputs.start.auth_action =
- u16_high_bit(l_get_be16(data + 9));
- prov->conf_inputs.start.auth_size =
- (data[8] > 8 ? 8 : data[8]);
-
- }
-
out = l_malloc(1 + sizeof(prov->conf_inputs.start));
out[0] = PROV_START;
memcpy(out + 1, &prov->conf_inputs.start,
@@ -789,7 +809,7 @@ static void int_prov_ack(void *user_data, uint8_t msg_num)
switch (prov->state) {
case INT_PROV_START_SENT:
prov->state = INT_PROV_START_ACKED;
- if (prov->conf_inputs.caps.pub_type == 0)
+ if (!prov->conf_inputs.start.pub_key)
send_pub_key(prov);
break;

@@ -798,7 +818,7 @@ static void int_prov_ack(void *user_data, uint8_t msg_num)
break;

case INT_PROV_KEY_SENT:
- if (prov->conf_inputs.caps.pub_type == 1)
+ if (prov->conf_inputs.start.pub_key)
int_prov_auth();
break;

diff --git a/tools/mesh-cfgclient.c b/tools/mesh-cfgclient.c
index 43588852b..6083b2b1f 100644
--- a/tools/mesh-cfgclient.c
+++ b/tools/mesh-cfgclient.c
@@ -617,6 +617,13 @@ static bool register_agent(void)
return false;
}

+ if (!l_dbus_object_add_interface(dbus, app.agent_path,
+ L_DBUS_INTERFACE_PROPERTIES, NULL)) {
+ l_error("Failed to add interface %s",
+ L_DBUS_INTERFACE_PROPERTIES);
+ return false;
+ }
+
return true;
}

--
2.20.1

2020-04-01 10:27:40

by Michał Lowas-Rzechonek

[permalink] [raw]
Subject: [PATCH BlueZ v2 1/6] tools/mesh-cfgclient: Display unprovisioned OOB data

---
tools/mesh-cfgclient.c | 13 +++++++++++++
1 file changed, 13 insertions(+)

diff --git a/tools/mesh-cfgclient.c b/tools/mesh-cfgclient.c
index ae13c4409..43588852b 100644
--- a/tools/mesh-cfgclient.c
+++ b/tools/mesh-cfgclient.c
@@ -1533,6 +1533,19 @@ static struct l_dbus_message *scan_result_call(struct l_dbus *dbus,
bt_shell_printf("\t" COLOR_GREEN "UUID = %s\n" COLOR_OFF, str);
l_free(str);

+ if (n >= 18) {
+ str = l_util_hexstring_upper(prov_data + 16, 2);
+ bt_shell_printf("\t" COLOR_GREEN "OOB = %s\n" COLOR_OFF, str);
+ l_free(str);
+ }
+
+ if (n >= 22) {
+ str = l_util_hexstring_upper(prov_data + 18, 4);
+ bt_shell_printf("\t" COLOR_GREEN "URI Hash = %s\n" COLOR_OFF,
+ str);
+ l_free(str);
+ }
+
/* TODO: Handle the rest of provisioning data if present */

dev = l_queue_find(devices, match_device_uuid, prov_data);
--
2.20.1

2020-04-01 10:27:40

by Michał Lowas-Rzechonek

[permalink] [raw]
Subject: [PATCH BlueZ v2 4/6] mesh: Refresh provisioner's capabilities

As provisioner's capabilities might change during application lifetime
(e.g. no network link to download OOB key), let's query the agent again
after application calls AddNode().
---
mesh/agent.c | 105 +++++++++++++++++++++++++++++++++++-------
mesh/agent.h | 2 +
mesh/manager.c | 33 +++++++++----
mesh/prov-initiator.c | 47 +++++++++++++++----
mesh/provision.h | 3 ++
5 files changed, 154 insertions(+), 36 deletions(-)

diff --git a/mesh/agent.c b/mesh/agent.c
index 3ab3893a1..02993738c 100644
--- a/mesh/agent.c
+++ b/mesh/agent.c
@@ -40,7 +40,8 @@ typedef enum {
MESH_AGENT_REQUEST_IN_ALPHA,
MESH_AGENT_REQUEST_STATIC_OOB,
MESH_AGENT_REQUEST_PRIVATE_KEY,
- MESH_AGENT_REQUEST_PUBLIC_KEY
+ MESH_AGENT_REQUEST_PUBLIC_KEY,
+ MESH_AGENT_REQUEST_CAPABILITIES,
} agent_request_type_t;

struct agent_request {
@@ -158,6 +159,25 @@ static void parse_oob_info(struct mesh_agent_prov_caps *caps,
}
}

+static void parse_properties(struct mesh_agent *agent,
+ struct l_dbus_message_iter *properties)
+{
+ const char *key, *uri_string;
+ struct l_dbus_message_iter variant;
+
+ while (l_dbus_message_iter_next_entry(properties, &key, &variant)) {
+ if (!strcmp(key, "Capabilities")) {
+ parse_prov_caps(&agent->caps, &variant);
+ } else if (!strcmp(key, "URI")) {
+ l_dbus_message_iter_get_variant(&variant, "s",
+ &uri_string);
+ /* TODO: compute hash */
+ } else if (!strcmp(key, "OutOfBandInfo")) {
+ parse_oob_info(&agent->caps, &variant);
+ }
+ }
+}
+
static void agent_free(void *agent_data)
{
struct mesh_agent *agent = agent_data;
@@ -193,6 +213,7 @@ static void agent_free(void *agent_data)
case MESH_AGENT_REQUEST_VIBRATE:
case MESH_AGENT_REQUEST_OUT_NUMERIC:
case MESH_AGENT_REQUEST_OUT_ALPHA:
+ case MESH_AGENT_REQUEST_CAPABILITIES:
simple_cb = agent->req->cb;
simple_cb(req->user_data, err);
default:
@@ -235,26 +256,13 @@ struct mesh_agent *mesh_agent_create(const char *path, const char *owner,
struct l_dbus_message_iter *properties)
{
struct mesh_agent *agent;
- const char *key, *uri_string;
- struct l_dbus_message_iter variant;

agent = l_new(struct mesh_agent, 1);
-
- while (l_dbus_message_iter_next_entry(properties, &key, &variant)) {
- if (!strcmp(key, "Capabilities")) {
- parse_prov_caps(&agent->caps, &variant);
- } else if (!strcmp(key, "URI")) {
- l_dbus_message_iter_get_variant(&variant, "s",
- &uri_string);
- /* TODO: compute hash */
- } else if (!strcmp(key, "OutOfBandInfo")) {
- parse_oob_info(&agent->caps, &variant);
- }
- }
-
agent->owner = l_strdup(owner);
agent->path = l_strdup(path);

+ parse_properties(agent, properties);
+
l_queue_push_tail(agents, agent);

return agent;
@@ -289,13 +297,76 @@ static int get_reply_error(struct l_dbus_message *reply)
if (l_dbus_message_is_error(reply)) {

l_dbus_message_get_error(reply, &name, &desc);
- l_error("Agent failed output action (%s), %s", name, desc);
+ l_error("Agent failed (%s), %s", name, desc);
return MESH_ERROR_FAILED;
}

return MESH_ERROR_NONE;
}

+static void properties_reply(struct l_dbus_message *reply, void *user_data)
+{
+ struct mesh_agent *agent = user_data;
+ struct agent_request *req;
+ mesh_agent_cb_t cb;
+ struct l_dbus_message_iter properties;
+ int err;
+
+ if (!l_queue_find(agents, simple_match, agent) || !agent->req)
+ return;
+
+ req = agent->req;
+
+ err = get_reply_error(reply);
+
+ if (err != MESH_ERROR_NONE)
+ goto fail;
+
+ if (!l_dbus_message_get_arguments(reply, "a{sv}", &properties)) {
+ err = MESH_ERROR_FAILED;
+ goto fail;
+ }
+
+ parse_properties(agent, &properties);
+fail:
+ if (req->cb)
+ {
+ cb = req->cb;
+ cb(req->user_data, err);
+ }
+
+ l_dbus_message_unref(req->msg);
+ l_free(req);
+ agent->req = NULL;
+}
+
+void mesh_agent_refresh(struct mesh_agent *agent, mesh_agent_cb_t cb,
+ void *user_data)
+{
+ struct l_dbus *dbus = dbus_get_bus();
+ struct l_dbus_message *msg;
+ struct l_dbus_message_builder *builder;
+
+ agent->req = create_request(MESH_AGENT_REQUEST_CAPABILITIES, (void *)cb,
+ user_data);
+
+ msg = l_dbus_message_new_method_call(dbus, agent->owner, agent->path,
+ L_DBUS_INTERFACE_PROPERTIES,
+ "GetAll");
+
+ builder = l_dbus_message_builder_new(msg);
+ l_dbus_message_builder_append_basic(builder, 's',
+ MESH_PROVISION_AGENT_INTERFACE);
+ l_dbus_message_builder_finalize(builder);
+ l_dbus_message_builder_destroy(builder);
+
+ l_dbus_send_with_reply(dbus_get_bus(), msg, properties_reply, agent,
+ NULL);
+
+ agent->req->msg = l_dbus_message_ref(msg);
+}
+
+
static void simple_reply(struct l_dbus_message *reply, void *user_data)
{
struct mesh_agent *agent = user_data;
diff --git a/mesh/agent.h b/mesh/agent.h
index 80333acd5..6cc3d0f71 100644
--- a/mesh/agent.h
+++ b/mesh/agent.h
@@ -42,6 +42,8 @@ void mesh_agent_init(void);
void mesh_agent_cleanup(void);
struct mesh_agent *mesh_agent_create(const char *path, const char *owner,
struct l_dbus_message_iter *properties);
+void mesh_agent_refresh(struct mesh_agent *agent, mesh_agent_cb_t cb,
+ void *user_data);

void mesh_agent_remove(struct mesh_agent *agent);
void mesh_agent_cancel(struct mesh_agent *agent);
diff --git a/mesh/manager.c b/mesh/manager.c
index a4c2f2d41..9ec4a1468 100644
--- a/mesh/manager.c
+++ b/mesh/manager.c
@@ -81,6 +81,9 @@ static void free_pending_add_call()
l_dbus_remove_watch(dbus_get_bus(),
add_pending->disc_watch);

+ if (add_pending->msg)
+ l_dbus_message_unref(add_pending->msg);
+
l_free(add_pending);
add_pending = NULL;
}
@@ -212,6 +215,23 @@ static bool add_data_get(void *user_data, uint8_t num_ele)
return true;
}

+static void add_start(void *user_data, int err)
+{
+ struct l_dbus_message *reply;
+
+ l_info("Start callback");
+
+ if (err == MESH_ERROR_NONE)
+ reply = l_dbus_message_new_method_return(add_pending->msg);
+ else
+ reply = dbus_error(add_pending->msg, MESH_ERROR_FAILED,
+ "Failed to start provisioning initiator");
+
+ l_dbus_send(dbus_get_bus(), reply);
+
+ add_pending->msg = NULL;
+}
+
static struct l_dbus_message *add_node_call(struct l_dbus *dbus,
struct l_dbus_message *msg,
void *user_data)
@@ -243,6 +263,7 @@ static struct l_dbus_message *add_node_call(struct l_dbus *dbus,
/* Invoke Prov Initiator */

add_pending = l_new(struct add_data, 1);
+ add_pending->msg = l_dbus_message_ref(msg);
memcpy(add_pending->uuid, uuid, 16);
add_pending->node = node;
add_pending->agent = node_get_agent(node);
@@ -255,20 +276,14 @@ static struct l_dbus_message *add_node_call(struct l_dbus *dbus,
goto fail;
}

-
- if (!initiator_start(PB_ADV, uuid, 99, 60, add_pending->agent,
- add_data_get, add_cmplt, node, add_pending)) {
- reply = dbus_error(msg, MESH_ERROR_FAILED,
- "Failed to start provisioning initiator");
- goto fail;
- }
+ initiator_start(PB_ADV, uuid, 99, 60, add_pending->agent, add_start,
+ add_data_get, add_cmplt, node, add_pending);

add_pending->disc_watch = l_dbus_add_disconnect_watch(dbus,
node_get_owner(node),
prov_disc_cb, NULL, NULL);

- return l_dbus_message_new_method_return(msg);
-
+ return NULL;
fail:
l_free(add_pending);
add_pending = NULL;
diff --git a/mesh/prov-initiator.c b/mesh/prov-initiator.c
index f2ebff69e..17bda6521 100644
--- a/mesh/prov-initiator.c
+++ b/mesh/prov-initiator.c
@@ -36,6 +36,7 @@
#include "mesh/pb-adv.h"
#include "mesh/mesh.h"
#include "mesh/agent.h"
+#include "mesh/error.h"

/* Quick size sanity check */
static const uint16_t expected_pdu_size[] = {
@@ -77,6 +78,7 @@ enum int_state {
#define MAT_SECRET (MAT_REMOTE_PUBLIC | MAT_LOCAL_PRIVATE)

struct mesh_prov_initiator {
+ mesh_prov_initiator_start_func_t start_cb;
mesh_prov_initiator_complete_func_t complete_cb;
mesh_prov_initiator_data_req_func_t data_req_cb;
prov_trans_tx_t trans_tx;
@@ -102,6 +104,7 @@ struct mesh_prov_initiator {
uint8_t private_key[32];
uint8_t secret[32];
uint8_t rand_auth_workspace[48];
+ uint8_t uuid[16];
};

static struct mesh_prov_initiator *prov = NULL;
@@ -814,17 +817,45 @@ static void int_prov_ack(void *user_data, uint8_t msg_num)
}
}

+static void initiator_open_cb(void *user_data, int err)
+{
+ bool result;
+
+ if (!prov)
+ return;
+
+ if (err != MESH_ERROR_NONE)
+ goto fail;
+
+ /* Always register for PB-ADV */
+ result = pb_adv_reg(true, int_prov_open, int_prov_close, int_prov_rx,
+ int_prov_ack, prov->uuid, prov);
+
+ if (!result) {
+ err = MESH_ERROR_FAILED;
+ goto fail;
+ }
+
+ if (!prov)
+ return;
+
+ prov->start_cb(prov->caller_data, MESH_ERROR_NONE);
+ return;
+fail:
+ prov->start_cb(prov->caller_data, err);
+ initiator_free();
+}
+
bool initiator_start(enum trans_type transport,
uint8_t uuid[16],
uint16_t max_ele,
uint32_t timeout, /* in seconds from mesh.conf */
struct mesh_agent *agent,
+ mesh_prov_initiator_start_func_t start_cb,
mesh_prov_initiator_data_req_func_t data_req_cb,
mesh_prov_initiator_complete_func_t complete_cb,
void *node, void *caller_data)
{
- bool result;
-
/* Invoked from Add() method in mesh-api.txt, to add a
* remote unprovisioned device network.
*/
@@ -837,19 +868,15 @@ bool initiator_start(enum trans_type transport,
prov->node = node;
prov->agent = agent;
prov->complete_cb = complete_cb;
+ prov->start_cb = start_cb;
prov->data_req_cb = data_req_cb;
prov->caller_data = caller_data;
prov->previous = -1;
+ memcpy(prov->uuid, uuid, 16);

- /* Always register for PB-ADV */
- result = pb_adv_reg(true, int_prov_open, int_prov_close, int_prov_rx,
- int_prov_ack, uuid, prov);
-
- if (result)
- return true;
+ mesh_agent_refresh(prov->agent, initiator_open_cb, prov);

- initiator_free();
- return false;
+ return true;
}

void initiator_cancel(void *user_data)
diff --git a/mesh/provision.h b/mesh/provision.h
index 43f53f935..1d78ed8e2 100644
--- a/mesh/provision.h
+++ b/mesh/provision.h
@@ -100,6 +100,8 @@ typedef bool (*mesh_prov_acceptor_complete_func_t)(void *user_data,
uint8_t status,
struct mesh_prov_node_info *info);

+typedef void (*mesh_prov_initiator_start_func_t)(void *user_data, int err);
+
typedef bool (*mesh_prov_initiator_data_req_func_t)(void *user_data,
uint8_t num_elem);

@@ -120,6 +122,7 @@ bool initiator_start(enum trans_type transport,
uint16_t max_ele,
uint32_t timeout, /* in seconds from mesh.conf */
struct mesh_agent *agent,
+ mesh_prov_initiator_start_func_t start_cb,
mesh_prov_initiator_data_req_func_t data_req_cb,
mesh_prov_initiator_complete_func_t complete_cb,
void *node, void *caller_data);
--
2.20.1

2020-04-01 19:10:57

by Gix, Brian

[permalink] [raw]
Subject: Re: [PATCH BlueZ v2 1/6] tools/mesh-cfgclient: Display unprovisioned OOB data

Hi Michał,

This patchset was applied with some style-guide fixes (patches 4 and 5). Please remember
to run checkpatch prior to submitting patches.

On Wed, 2020-04-01 at 12:24 +0200, Michał Lowas-Rzechonek wrote:
> ---
> tools/mesh-cfgclient.c | 13 +++++++++++++
> 1 file changed, 13 insertions(+)
>
> diff --git a/tools/mesh-cfgclient.c b/tools/mesh-cfgclient.c
> index ae13c4409..43588852b 100644
> --- a/tools/mesh-cfgclient.c
> +++ b/tools/mesh-cfgclient.c
> @@ -1533,6 +1533,19 @@ static struct l_dbus_message *scan_result_call(struct l_dbus *dbus,
> bt_shell_printf("\t" COLOR_GREEN "UUID = %s\n" COLOR_OFF, str);
> l_free(str);
>
> + if (n >= 18) {
> + str = l_util_hexstring_upper(prov_data + 16, 2);
> + bt_shell_printf("\t" COLOR_GREEN "OOB = %s\n" COLOR_OFF, str);
> + l_free(str);
> + }
> +
> + if (n >= 22) {
> + str = l_util_hexstring_upper(prov_data + 18, 4);
> + bt_shell_printf("\t" COLOR_GREEN "URI Hash = %s\n" COLOR_OFF,
> + str);
> + l_free(str);
> + }
> +
> /* TODO: Handle the rest of provisioning data if present */
>
> dev = l_queue_find(devices, match_device_uuid, prov_data);