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

[Denis Kenzior <denkenz at gmail.com>]

[-- 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