Re: [PATCH v5 3/4] dpp: add support for configuration protocol

Denis Kenzior
2021-12-17T20:22:03Z

[-- Attachment #1: Type: text/plain, Size: 4939 bytes --]

Hi James,

On 12/17/21 13:14, James Prestwood wrote:
> This is a minimal implementation only supporting legacy network
> configuration, i.e. only SSID and PSK/passphrase are supported.
>
> Missing features include:
> - Fragmentation/comeback delay support
> - DPP AKM support
> - 8021x/PKEX support
> ---
> src/dpp.c | 328 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 328 insertions(+)
>
> v5:
> * Updated use of wrap/unwrap APIs
> * Added comments about how AAD is used with configuration
>

<snip>

> +static void dpp_handle_config_response_frame(const struct mmpdu_header *frame,
> + const void *body, size_t body_len,
> + int rssi, void *user_data)
> +{
> + struct dpp_sm *dpp = user_data;
> + const uint8_t *ptr = body;
> + uint16_t status;
> + uint16_t fragmented; /* Fragmented/Comeback delay field */
> + uint8_t adv_protocol_element[] = { 0x6C, 0x08, 0x7F };
> + uint8_t adv_protocol_id[] = { 0xDD, 0x05, 0x50, 0x6F,
> + 0x9A, 0x1A, 0x01 };
> + uint16_t query_len;
> + struct dpp_attr_iter iter;
> + enum dpp_attribute_type type;
> + size_t len;
> + const uint8_t *data;
> + const char *json = NULL;
> + size_t json_len = 0;
> + int dstatus = -1;
> + const uint8_t *wrapped = NULL;
> + const uint8_t *e_nonce = NULL;
> + size_t wrapped_len = 0;
> + _auto_(l_free) uint8_t *unwrapped = NULL;
> + struct dpp_configuration *config;
> + uint8_t ad0[] = { 0x00, 0x10, 0x01, 0x00, 0x05 };
> +
> + if (dpp->state != DPP_STATE_CONFIGURING)
> + return;
> +
> + ptr += 2;
> +
> + /*
> + * Can a configuration request come from someone other than who you
> + * authenticated to?
> + */
> + if (memcmp(dpp->auth_addr, frame->address_2, 6))
> + return;

Hmm, I'm not seeing any length checking?

> +
> + if (*ptr++ != dpp->diag_token)
> + return;
> +
> + status = l_get_le16(ptr);
> + ptr += 2;
> +
> + if (status != 0) {
> + l_debug("Bad configuration status %u", status);
> + return;
> + }
> +
> + fragmented = l_get_le16(ptr);
> + ptr += 2;
> +
> + /*
> + * TODO: handle 0x0001 (fragmented), as well as comeback delay.
> + */
> + if (fragmented != 0) {
> + l_debug("Fragmented messages not currently supported");
> + return;
> + }
> +
> + if (memcmp(ptr, adv_protocol_element, sizeof(adv_protocol_element))) {
> + l_debug("Invalid Advertisement protocol element");
> + return;
> + }
> +
> + ptr += sizeof(adv_protocol_element);
> +
> + if (memcmp(ptr, adv_protocol_id, sizeof(adv_protocol_id))) {
> + l_debug("Invalid Advertisement protocol ID");
> + return;
> + }
> +
> + ptr += sizeof(adv_protocol_id);
> +
> + query_len = l_get_le16(ptr);
> + ptr += 2;
> +

How do we know query_len is valid for

> + dpp_attr_iter_init(&iter, ptr, query_len);

this invocation?

> +
> + while (dpp_attr_iter_next(&iter, &type, &len, &data)) {
> + switch (type) {
> + case DPP_ATTR_STATUS:
> + dstatus = l_get_u8(data);
> + break;
> + case DPP_ATTR_WRAPPED_DATA:
> + wrapped = data;
> + wrapped_len = len;
> + break;
> + default:
> + /*
> + * TODO: CSR Attribute
> + */
> + break;
> + }
> + }
> +
> + if (dstatus != DPP_STATUS_OK || !wrapped) {
> + l_debug("Bad status or missing attributes");
> + return;
> + }
> +
> + unwrapped = dpp_unwrap_attr(ad0, sizeof(ad0), NULL, 0, dpp->ke,
> + dpp->key_len, wrapped, wrapped_len,
> + &wrapped_len);
> + if (!unwrapped) {
> + l_debug("Failed to unwrap");
> + return;
> + }
> +
> + dpp_attr_iter_init(&iter, unwrapped, wrapped_len);
> +
> + while (dpp_attr_iter_next(&iter, &type, &len, &data)) {
> + switch (type) {
> + case DPP_ATTR_ENROLLEE_NONCE:
> + if (len != dpp->nonce_len)
> + break;
> +
> + if (memcmp(data, dpp->e_nonce, dpp->nonce_len))
> + break;
> +
> + e_nonce = data;
> + break;
> + case DPP_ATTR_CONFIGURATION_OBJECT:
> + json = (const char *)data;
> + json_len = len;
> + break;
> + default:
> + break;
> + }
> + }
> +
> + if (!json || !e_nonce) {
> + l_debug("No configuration object in response");
> + return;
> + }
> +
> + config = dpp_parse_configuration_object(json, json_len);
> + if (!config) {
> + l_error("Configuration object did not parse");
> + return;
> + }
> +
> + dpp_write_config(config);
> + /*
> + * TODO: Depending on the info included in the configuration object a
> + * limited scan could be issued to get autoconnect to trigger faster.
> + * In addition this network may already be in past scan results and
> + * could be joined immediately.
> + *
> + * For now just wait for autoconnect.
> + */
> +
> + dpp_configuration_free(config);
> +
> + send_config_result(dpp, dpp->auth_addr);
> +}
> +
> /*
> * The Authentication protocol has a consistent use of AD components, and this
> * use is defined in 6.3.1.4:

Regards,
-Denis