Re: [PATCH 12/21] dpp-util: add crypto for PKEX

[Denis Kenzior <denkenz@gmail.com>]

Hi James,

On 10/12/23 15:01, James Prestwood wrote:
> ---
>   src/dpp-util.c | 208 +++++++++++++++++++++++++++++++++++++++++++++++++
>   src/dpp-util.h |  32 ++++++++
>   2 files changed, 240 insertions(+)
> 

You may want to add some references to the code?  i.e. which part corresponds to 
what section in the spec.

> diff --git a/src/dpp-util.c b/src/dpp-util.c
> index 0406a4dc..b0556917 100644
> --- a/src/dpp-util.c
> +++ b/src/dpp-util.c
> @@ -39,6 +39,32 @@
>   #include "ell/asn1-private.h"
>   #include "src/ie.h"
>   

<snip>

> +
> +struct l_ecc_point *dpp_derive_q(const struct l_ecc_curve *curve,
> +					bool responder,
> +					const char *key,
> +					const char *identifier,
> +					const uint8_t *mac)

Should this use the [static 6] syntax?  Or I guess not since mac can be NULL. 
Why can mac be NULL?

We have derive_l_responder and derive_l_initiator, but derive_q has a boolean 
parameter?  Lets be consistent.

> +{
> +	_auto_(l_ecc_scalar_free) struct l_ecc_scalar *scalar = NULL;
> +	_auto_(l_ecc_point_free) struct l_ecc_point *ret = NULL;
> +	uint8_t hash[L_ECC_SCALAR_MAX_BYTES];
> +	unsigned int bytes = l_ecc_curve_get_scalar_bytes(curve);
> +	enum l_checksum_type type = dpp_sha_from_key_len(bytes);
> +	_auto_(l_ecc_point_free) struct l_ecc_point *p = NULL;
> +	const uint8_t *p_data = responder ? dpp_pkex_responder_p256 :
> +					dpp_pkex_initiator_p256;
> +	struct l_checksum *sha = l_checksum_new(type);
> +
> +	if (mac)
> +		l_checksum_update(sha, mac, 6);
> +
> +	if (identifier)
> +		l_checksum_update(sha, identifier, strlen(identifier));
> +
> +	l_checksum_update(sha, key, strlen(key));
> +	l_checksum_get_digest(sha, hash, bytes);
> +	l_checksum_free(sha);
> +
> +	/* Unlikely but can happen */
> +	scalar = l_ecc_scalar_new(curve, hash, bytes);
> +	if (!scalar)
> +		return NULL;
> +
> +	p = l_ecc_point_from_data(curve, L_ECC_POINT_TYPE_FULL,
> +					p_data, bytes * 2);
> +	if (!p)
> +		return NULL;
> +
> +	ret = l_ecc_point_new(curve);
> +
> +	if (!l_ecc_point_multiply(ret, scalar, p))
> +		return NULL;
> +
> +	return l_steal_ptr(ret);
> +}
> +
> +bool dpp_derive_z(const uint8_t *mac_i, const uint8_t *mac_r,

[static 6]?

> +				const struct l_ecc_point *n,
> +				const struct l_ecc_point *m,
> +				const struct l_ecc_point *k,
> +				const char *key,
> +				const char *identifier,
> +				void *z_out, size_t *z_len)
> +{
> +	const struct l_ecc_curve *curve = l_ecc_point_get_curve(n);
> +	size_t bytes = l_ecc_curve_get_scalar_bytes(curve);
> +	enum l_checksum_type sha = dpp_sha_from_key_len(bytes);
> +	uint8_t k_x[L_ECC_SCALAR_MAX_BYTES];
> +	uint8_t m_x[L_ECC_SCALAR_MAX_BYTES];
> +	uint8_t n_x[L_ECC_SCALAR_MAX_BYTES];
> +	uint8_t prk[L_ECC_SCALAR_MAX_BYTES];
> +
> +	l_ecc_point_get_x(k, k_x, sizeof(k_x));
> +	l_ecc_point_get_x(m, m_x, sizeof(m_x));
> +	l_ecc_point_get_x(n, n_x, sizeof(n_x));
> +
> +	hkdf_extract(sha, NULL, 0, 1, prk, k_x, bytes);
> +
> +	/* HKDF-Extract (since it doesn't take non-string arguments)*/
> +	prf_plus(sha, prk, bytes, z_out, bytes, 5, mac_i, 6, mac_r, 6, m_x,
> +			bytes, n_x, bytes, key, strlen(key));
> +
> +	*z_len = bytes;
> +
> +	return true;
> +}
> +
> +bool dpp_derive_u(const struct l_ecc_point *j,
> +			const uint8_t *mac_i,

[static 6]?

> +			const struct l_ecc_point *a,
> +			const struct l_ecc_point *y,
> +			const struct l_ecc_point *x,
> +			void *u_out, size_t *u_len)
> +{
> +	const struct l_ecc_curve *curve = l_ecc_point_get_curve(y);
> +	uint8_t j_x[L_ECC_SCALAR_MAX_BYTES];
> +	uint8_t a_x[L_ECC_SCALAR_MAX_BYTES];
> +	uint8_t y_x[L_ECC_SCALAR_MAX_BYTES];
> +	uint8_t x_x[L_ECC_SCALAR_MAX_BYTES];
> +	size_t bytes = l_ecc_curve_get_scalar_bytes(curve);
> +	enum l_checksum_type sha = dpp_sha_from_key_len(bytes);
> +	struct l_checksum *hmac;
> +
> +	l_ecc_point_get_x(j, j_x, bytes);
> +	l_ecc_point_get_x(a, a_x, bytes);
> +	l_ecc_point_get_x(y, y_x, bytes);
> +	l_ecc_point_get_x(x, x_x, bytes);
> +
> +	/* u = HMAC(J.x, MAC-Initiator | A.x | Y'.x | X.x)*/
> +	hmac = l_checksum_new_hmac(sha, j_x, bytes);
> +	l_checksum_update(hmac, mac_i, 6);
> +	l_checksum_update(hmac, a_x, bytes);
> +	l_checksum_update(hmac, y_x, bytes);
> +	l_checksum_update(hmac, x_x, bytes);
> +	l_checksum_get_digest(hmac, u_out, bytes);
> +	l_checksum_free(hmac);
> +
> +	*u_len = bytes;
> +
> +	return true;
> +}
> +
> +bool dpp_derive_v(const struct l_ecc_point *l, const uint8_t *mac,

And here?

> +			const struct l_ecc_point *b,
> +			const struct l_ecc_point *x,
> +			const struct l_ecc_point *y,
> +			uint8_t *v_out, size_t *v_len)
> +{
> +	const struct l_ecc_curve *curve = l_ecc_point_get_curve(l);
> +	uint8_t l_x[L_ECC_SCALAR_MAX_BYTES];
> +	uint8_t b_x[L_ECC_SCALAR_MAX_BYTES];
> +	uint8_t x_x[L_ECC_SCALAR_MAX_BYTES];
> +	uint8_t y_x[L_ECC_SCALAR_MAX_BYTES];
> +	size_t bytes = l_ecc_curve_get_scalar_bytes(curve);
> +	enum l_checksum_type sha = dpp_sha_from_key_len(bytes);
> +	struct l_checksum *hmac;
> +
> +	l_ecc_point_get_x(l, l_x, sizeof(l_x));
> +	l_ecc_point_get_x(b, b_x, sizeof(b_x));
> +	l_ecc_point_get_x(x, x_x, sizeof(x_x));
> +	l_ecc_point_get_x(y, y_x, sizeof(y_x));
> +
> +	hmac = l_checksum_new_hmac(sha, l_x, bytes);
> +
> +	if (mac)
> +		l_checksum_update(hmac, mac, 6);
> +
> +	l_checksum_update(hmac, b_x, bytes);
> +	l_checksum_update(hmac, x_x, bytes);
> +	l_checksum_update(hmac, y_x, bytes);
> +	l_checksum_get_digest(hmac, v_out, bytes);
> +	l_checksum_free(hmac);
> +
> +	*v_len = bytes;
> +
> +	return true;
> +}

<snip>

Unit tests?

Regards,
-Denis