Re: [PATCH 02/12] dpp-util: add dpp_point_from_asn1()

[Denis Kenzior <denkenz at gmail.com>]

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

Hi James,

On 1/18/22 15:25, James Prestwood wrote:
> Given an ASN1 blob of the right form, parse and create
> an l_ecc_point object. The form used is specific to DPP
> hence why this isn't general purpose and put into dpp-util.
> ---
>   src/dpp-util.c | 69 ++++++++++++++++++++++++++++++++++++++++++++++++++
>   src/dpp-util.h |  1 +
>   2 files changed, 70 insertions(+)
> 

<snip>

> +	/* BITSTRING */
> +	key_data = asn1_der_find_elem(outer_seq, outer_len, 1, &tag, &elen);
> +	if (!key_data || tag != ASN1_ID_BIT_STRING || elen < 34)

Why 34? Aren't you only concerned that elen >= 2?

> +		return NULL;
> +
> +	curve = l_ecc_curve_from_ike_group(curve_num);
> +	if (!curve)
> +		return NULL;

Perhaps this check should be moved up?

> +
> +	return l_ecc_point_from_data(curve, key_data[1],
> +					key_data + 2, elen - 2);

Aren't you relying on ecc_point_from_data to validate the length?  Hence the 
unnecessary '34' magic number above?

Should ecc_point_from_data also validate that all of the data passed in was 
consumed?  Or maybe return the number of bytes consumed so that we can validate 
it here?

Regards,
-Denis