Re: [PATCH v4 06/10] dpp: add DPP authentication protocol

[Denis Kenzior <denkenz at gmail.com>]

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

On 12/16/21 12:08, James Prestwood wrote:
> This implements the DPP protocol used to authenticate to a
> DPP configurator.
> 
> Note this is not a full implementation of the protocol and
> there are a few missing features which will be added as
> needed:
> 
>   - Mutual authentication (needed for BLE bootstrapping)
>   - Configurator support
>   - Initiator role
> ---
>   src/dpp.c | 571 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
>   1 file changed, 571 insertions(+)
> 

<snip>

> @@ -143,6 +179,34 @@ static void dpp_send_frame(uint64_t wdev_id, struct iovec *iov, size_t iov_len,
>   		l_error("Could not send CMD_FRAME");
>   }
>   
> +static uint8_t *dpp_unwrap_attr(enum dpp_frame_type type, const void *start,
> +				const void *key, size_t key_len,
> +				const uint8_t *wrapped, size_t wrapped_len,
> +				size_t *unwrapped_len)
> +{
> +	uint8_t ad0[] = { 0x50, 0x6f, 0x9a, 0x1a, 0x01, type };
> +	struct iovec ad[2];
> +	uint8_t *unwrapped;
> +
> +	ad[0].iov_base = ad0;
> +	ad[0].iov_len = sizeof(ad0);
> +
> +	ad[1].iov_base = (void *)start;
> +	ad[1].iov_len = ((wrapped - 4) - ((const uint8_t *)start));

Ok, but I think you may need a comment about this referencing the spec 
explaining the AD data setup being used.

Also, why do you have 'wrapped' as a const uint8_t * and 'start' as a const void 
*?  Might be easier to make them both const void * and avoid this cast.

> +
> +	unwrapped = l_malloc(wrapped_len - 16);
> +
> +	if (!aes_siv_decrypt(key, key_len, wrapped, wrapped_len, ad, 2,
> +				unwrapped)) {
> +		l_free(unwrapped);
> +		return NULL;
> +	}
> +
> +	*unwrapped_len = wrapped_len - 16;
> +
> +	return unwrapped;
> +}
> +
>   static size_t dpp_append_attr(uint8_t *to, enum dpp_attribute_type type,
>   				void *attr, size_t attr_len)
>   {

Do you want to split out the protocol parsers / builders from dpp.c into 
dpp-util or another file so they can be unit tested separately?

Regards,
-Denis