[PATCH v4 0/4] DPP PKEX Changes

[PATCH v4 0/4] DPP PKEX Changes

[James Prestwood]

v4:
 * Fixed minor documentation issues
 * Documented Stop()
 * Removed Register/Unregister agent APIs, StartConfigurator
   will now take the path directly and unregister upon PKEX/DPP
   finishing.
 * Reworked patches to be more sequential:
       1. StartEnrollee()
       2. ConfigureEnrollee()
       3. StartConfigurator() (agent support)

James Prestwood (4):
  doc: PKEX support for DPP
  dpp: initial version of PKEX enrollee support
  dpp: initial version of PKEX configurator support
  dpp: Add StartConfigurator, PKEX agent support

 doc/device-provisioning-api.txt |  142 +++
 src/dpp.c                       | 1658 +++++++++++++++++++++++++++++--
 2 files changed, 1726 insertions(+), 74 deletions(-)

-- 
2.25.1

[PATCH v4 1/4] doc: PKEX support for DPP

[James Prestwood]

PKEX is part of the WFA EasyConnect specification and is
an additional boostrapping method (like QR codes) for
exchanging public keys between a configurator and enrollee.

PKEX operates over wifi and requires a key/code be exchanged
prior to the protocol. The key is used to encrypt the exchange
of the boostrapping information, then DPP authentication is
started immediately aftewards.

This can be useful for devices which don't have the ability to
scan a QR code, or even as a more convenient way to share
wireless credentials if the PSK is very secure (i.e. not a
human readable string).

PKEX would be used via the three DBus APIs on a new interface
SharedCodeDeviceProvisioning.

ConfigureEnrollee(a{sv}) will start a configurator with a
static shared code (optionally identifier) passed in as the
argument to this method.

StartEnrollee(a{sv}) will start a PKEX enrollee using a static
shared code (optionally identifier) passed as the argument to
the method.

StartConfigurator(o) will start a PKEX configurator and use the
agent specified by the path argument. The configurator will query
the agent for a specific code when an enrollee sends the initial
exchange message.

After the PKEX protocol is finished, DPP bootstrapping keys have
been exchanged and DPP Authentication will start, followed by
configuration.
---
 doc/device-provisioning-api.txt | 142 ++++++++++++++++++++++++++++++++
 1 file changed, 142 insertions(+)

diff --git a/doc/device-provisioning-api.txt b/doc/device-provisioning-api.txt
index ac204f46..2a34d4ae 100644
--- a/doc/device-provisioning-api.txt
+++ b/doc/device-provisioning-api.txt
@@ -71,3 +71,145 @@ Properties	boolean Started [readonly]
 
 			Indicates the DPP URI. This property is only available
 			when Started is true.
+
+
+Interface	net.connman.iwd.SharedCodeDeviceProvisioning [Experimental]
+Object path	/net/connman/iwd/{phy0,phy1,...}/{1,2,...}
+
+		void ConfigureEnrollee(a{sv})
+
+			Starts a DPP configurator using a shared code (and
+			optionally identifier) set in the dictionary argument.
+			Valid dictionary keys are:
+
+			string Code
+				The shared code to use. The code used by both
+				parties (configurator and enrollee) must match.
+
+			string Identifier
+				An optional identifier. The identifier used by
+				both parties must match. Per the DPP spec the
+				identifier "shall be a UTF-8 string not greater
+				than eighty (80) octets"
+
+			As with the DeviceProvisioning interface, configurators
+			must be connected to the network they wish to configure
+			in order to start.
+
+			Once started a configurator (acting as a responder) will
+			listen on the currently connected channel for an
+			enrollee's initial exchange request which will kick off
+			the shared code bootstrapping protocol (PKEX). Once
+			completed DPP will start automatically. Only one
+			enrollee can be configured per call to
+			ConfigureEnrollee, i.e. once PKEX/DPP has finished
+			(including failure) the configurator will stop.
+
+			The SharedCode methods have an eventual timeout and will
+			stop automatically after 2 minutes.
+
+			Possible errors:	net.connman.iwd.Busy
+						net.connman.iwd.NotConnected
+						net.connman.InvalidArguments
+
+		void StartEnrollee(a{sv})
+
+			Starts a DPP enrollee using a shared code (and
+			optionally identifier) set in the dictionary argument
+			(described above in ConfigureEnrollee).
+
+			As with the device provisioning interface, enrollees
+			must be disconnected in order to start.
+
+			Once started an enrollee (acting as an initiator) will
+			iterate channels sending out broadcast exchange requests
+			waiting for a response from a configurator. A response
+			will kick off the shared code bootstrapping protocol
+			(PKEX), followed by DPP if successful. Once the
+			protocols have completed, or failed, the enrollee will
+			stop. If failed, StartEnrollee will need to be called
+			again to retry.
+
+			Possible errors:	net.connman.iwd.Busy
+						net.connman.iwd.InvalidArguments
+
+		void StartConfigurator(object agent_path)
+
+			Start a shared code configurator using an agent
+			(distingushed by 'agent_path') to obtain the shared
+			code. This method is meant for an automated use case
+			where a configurator is capable of configuring multiple
+			enrollees, and distinguishing between them by their
+			identifier.
+
+			If the agent service disappears during the shared code
+			exchange it will be stopped, and the protocol will fail.
+
+			This method behaves nearly the same as ConfigureEnrollee
+			except upon receiving an enrollees first exchange
+			request the registered agent will be asked for the
+			shared code using the RequestSharedCode method.
+
+			Though the agent can provide shared codes for multiple
+			enrollees, this method will only configure a single
+			enrollee at a time. Once completed it will need to be
+			called again to configure additional enrollees.
+
+			Possible errors:	net.connman.iwd.Busy
+						net.connman.iwd.NotConnected
+						net.connman.iwd.NoAgent
+
+		Stop()
+
+			Stop a currently running configurator/enrollee. Note
+			that this will also interrupt DPP if the protocol has
+			advanced that far. Since DPP is initiated implicitly
+			from the shared code APIs it will also be canceled.
+			Calling Stop() if DPP was started via the
+			DeviceProvisioning interface will not stop it.
+
+Properties	boolean Started [readonly]
+
+			True if shared code device provisioning is currently
+			active. (configurator or enrollee is started)
+
+		string Role [readonly, optional]
+
+			Indicates the DPP role. Possible values are "enrollee"
+			or "configurator". This property is only available when
+			Started is true.
+
+SharedCodeAgent hierarchy
+=========================
+
+Service		unique name
+Interface	net.connman.iwd.SharedCodeAgent [Experimental]
+Object path	freely definable
+
+Methods		void Release() [noreply]
+
+			This method gets called when the service daemon
+			unregisters the agent.
+
+		string RequestSharedCode(string identifier)
+
+			This method gets called when a shared code is requested
+			for a particular enrollee, distingushed by the
+			identifier. The shared code agent should lookup the
+			identifier and return the shared code, or return an
+			error if not found.
+
+			Possible Errors:	[service].Error.Canceled
+						[service].Error.NotFound
+
+		void Cancel(string reason) [noreply]
+
+			This method gets called to indicate that the agent
+			request failed before a reply was returned. The
+			argument will indicate why the request is being
+			cancelled and may be "user-canceled", "timed-out" or
+			"shutdown".
+
+Examples	Requesting a shared code for an enrollee identified by "foo"
+
+			RequestSharedCode("foo") ==> "super_secret_code"
-- 
2.25.1

Re: [PATCH v4 1/4] doc: PKEX support for DPP

[Denis Kenzior]

Hi James,

On 11/7/23 11:06, James Prestwood wrote:
> PKEX is part of the WFA EasyConnect specification and is
> an additional boostrapping method (like QR codes) for
> exchanging public keys between a configurator and enrollee.
> 
> PKEX operates over wifi and requires a key/code be exchanged
> prior to the protocol. The key is used to encrypt the exchange
> of the boostrapping information, then DPP authentication is
> started immediately aftewards.
> 
> This can be useful for devices which don't have the ability to
> scan a QR code, or even as a more convenient way to share
> wireless credentials if the PSK is very secure (i.e. not a
> human readable string).
> 
> PKEX would be used via the three DBus APIs on a new interface
> SharedCodeDeviceProvisioning.
> 
> ConfigureEnrollee(a{sv}) will start a configurator with a
> static shared code (optionally identifier) passed in as the
> argument to this method.
> 
> StartEnrollee(a{sv}) will start a PKEX enrollee using a static
> shared code (optionally identifier) passed as the argument to
> the method.
> 
> StartConfigurator(o) will start a PKEX configurator and use the
> agent specified by the path argument. The configurator will query
> the agent for a specific code when an enrollee sends the initial
> exchange message.
> 
> After the PKEX protocol is finished, DPP bootstrapping keys have
> been exchanged and DPP Authentication will start, followed by
> configuration.
> ---
>   doc/device-provisioning-api.txt | 142 ++++++++++++++++++++++++++++++++
>   1 file changed, 142 insertions(+)
> 

Applied, thanks.

Regards,
-Denis

[PATCH v4 2/4] dpp: initial version of PKEX enrollee support

[James Prestwood]

This is the initial support for PKEX enrollees acting as the
initiator. A PKEX initiator starts the protocol by broadcasting
the PKEX exchange request. This request contains a key encrypted
with the pre-shared PKEX code. If accepted the peer sends back
the exchange response with its own encrypted key. The enrollee
decrypts this and performs some crypto/hashing in order to establish
an ephemeral key used to encrypt its own boostrapping key. The
boostrapping key is encrypted and sent to the peer in the PKEX
commit-reveal request. The peer then does the same thing, encrypting
its own bootstrapping key and sending to the initiator as the
PKEX commit-reveal response.

After this, both peers have exchanged their boostrapping keys
securely and can begin DPP authentication, then configuration.

For now the enrollee will only iterate the default channel list
from the Easy Connect spec. Future upates will need to include some
way of discovering non-default channel configurators, but the
protocol needs to be ironed out first.
---
 src/dpp.c | 792 +++++++++++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 788 insertions(+), 4 deletions(-)

diff --git a/src/dpp.c b/src/dpp.c
index b0a79361..4b1c1233 100644
--- a/src/dpp.c
+++ b/src/dpp.c
@@ -57,6 +57,7 @@
 #define DPP_FRAME_MAX_RETRIES 5
 #define DPP_FRAME_RETRY_TIMEOUT 1
 #define DPP_AUTH_PROTO_TIMEOUT 10
+#define DPP_PKEX_PROTO_TIMEOUT 120
 
 static uint32_t netdev_watch;
 static struct l_genl_family *nl80211;
@@ -70,6 +71,8 @@ static uint8_t dpp_prefix[] = { 0x04, 0x09, 0x50, 0x6f, 0x9a, 0x1a, 0x01 };
 enum dpp_state {
 	DPP_STATE_NOTHING,
 	DPP_STATE_PRESENCE,
+	DPP_STATE_PKEX_EXCHANGE,
+	DPP_STATE_PKEX_COMMIT_REVEAL,
 	DPP_STATE_AUTHENTICATING,
 	DPP_STATE_CONFIGURING,
 };
@@ -147,19 +150,66 @@ struct dpp_sm {
 
 	struct l_dbus_message *pending;
 
+	/* PKEX-specific values */
+	char *pkex_id;
+	char *pkex_key;
+	uint8_t pkex_version;
+	struct l_ecc_point *pkex_m;
+	/* Ephemeral key Y' or X' for enrollee or configurator */
+	struct l_ecc_point *y_or_x;
+	/* Ephemeral key pair y/Y or x/X */
+	struct l_ecc_point *pkex_public;
+	struct l_ecc_scalar *pkex_private;
+	uint8_t z[L_ECC_SCALAR_MAX_BYTES];
+	size_t z_len;
+	uint8_t u[L_ECC_SCALAR_MAX_BYTES];
+	size_t u_len;
+
 	bool mcast_support : 1;
 	bool roc_started : 1;
 	bool channel_switch : 1;
 	bool mutual_auth : 1;
 };
 
+static bool dpp_pkex_get_started(struct l_dbus *dbus,
+				struct l_dbus_message *message,
+				struct l_dbus_message_builder *builder,
+				void *user_data)
+{
+	struct dpp_sm *dpp = user_data;
+	bool started = false;
+
+	switch (dpp->state) {
+	case DPP_STATE_PKEX_EXCHANGE:
+	case DPP_STATE_PKEX_COMMIT_REVEAL:
+		started = true;
+		break;
+	default:
+		break;
+	}
+
+	l_dbus_message_builder_append_basic(builder, 'b', &started);
+
+	return true;
+}
+
 static bool dpp_get_started(struct l_dbus *dbus,
 				struct l_dbus_message *message,
 				struct l_dbus_message_builder *builder,
 				void *user_data)
 {
 	struct dpp_sm *dpp = user_data;
-	bool started = (dpp->state != DPP_STATE_NOTHING);
+	bool started = false;
+
+	switch (dpp->state) {
+	case DPP_STATE_PRESENCE:
+	case DPP_STATE_AUTHENTICATING:
+	case DPP_STATE_CONFIGURING:
+		started = true;
+		break;
+	default:
+		break;
+	}
 
 	l_dbus_message_builder_append_basic(builder, 'b', &started);
 
@@ -199,7 +249,9 @@ static bool dpp_get_uri(struct l_dbus *dbus,
 {
 	struct dpp_sm *dpp = user_data;
 
-	if (dpp->state == DPP_STATE_NOTHING)
+	if (dpp->state == DPP_STATE_NOTHING ||
+			dpp->state == DPP_STATE_PKEX_EXCHANGE ||
+			dpp->state == DPP_STATE_PKEX_COMMIT_REVEAL)
 		return false;
 
 	l_dbus_message_builder_append_basic(builder, 's', dpp->uri);
@@ -218,6 +270,16 @@ static void dpp_property_changed_notify(struct dpp_sm *dpp)
 				"URI");
 }
 
+static void dpp_pkex_property_changed_notify(struct dpp_sm *dpp)
+{
+	const char *path = netdev_get_path(dpp->netdev);
+
+	l_dbus_property_changed(dbus_get_bus(), path, IWD_DPP_PKEX_INTERFACE,
+				"Started");
+	l_dbus_property_changed(dbus_get_bus(), path, IWD_DPP_PKEX_INTERFACE,
+				"Role");
+}
+
 static void *dpp_serialize_iovec(struct iovec *iov, size_t iov_len,
 				size_t *out_len)
 {
@@ -242,6 +304,21 @@ static void *dpp_serialize_iovec(struct iovec *iov, size_t iov_len,
 	return ret;
 }
 
+static void dpp_free_pending_pkex_data(struct dpp_sm *dpp)
+{
+	if (dpp->pkex_id) {
+		l_free(dpp->pkex_id);
+		dpp->pkex_id = NULL;
+	}
+
+	if (dpp->pkex_key) {
+		l_free(dpp->pkex_key);
+		dpp->pkex_key = NULL;
+	}
+
+	memset(dpp->peer_addr, 0, sizeof(dpp->peer_addr));
+}
+
 static void dpp_free_auth_data(struct dpp_sm *dpp)
 {
 	if (dpp->own_proto_public) {
@@ -268,6 +345,27 @@ static void dpp_free_auth_data(struct dpp_sm *dpp)
 		l_ecc_scalar_free(dpp->m);
 		dpp->m = NULL;
 	}
+
+	if (dpp->pkex_m) {
+		l_ecc_point_free(dpp->pkex_m);
+		dpp->pkex_m = NULL;
+	}
+
+	if (dpp->y_or_x) {
+		l_ecc_point_free(dpp->y_or_x);
+		dpp->y_or_x = NULL;
+	}
+
+	if (dpp->pkex_public) {
+		l_ecc_point_free(dpp->pkex_public);
+		dpp->pkex_public = NULL;
+	}
+
+	if (dpp->pkex_private) {
+		l_ecc_scalar_free(dpp->pkex_private);
+		dpp->pkex_private = NULL;
+	}
+
 }
 
 static void dpp_reset(struct dpp_sm *dpp)
@@ -321,6 +419,7 @@ static void dpp_reset(struct dpp_sm *dpp)
 	dpp->new_freq = 0;
 	dpp->frame_retry = 0;
 	dpp->frame_cookie = 0;
+	dpp->pkex_version = 0;
 
 	explicit_bzero(dpp->r_nonce, dpp->nonce_len);
 	explicit_bzero(dpp->i_nonce, dpp->nonce_len);
@@ -329,10 +428,15 @@ static void dpp_reset(struct dpp_sm *dpp)
 	explicit_bzero(dpp->k1, dpp->key_len);
 	explicit_bzero(dpp->k2, dpp->key_len);
 	explicit_bzero(dpp->auth_tag, dpp->key_len);
+	explicit_bzero(dpp->z, dpp->key_len);
+	explicit_bzero(dpp->u, dpp->u_len);
+
+	dpp_free_pending_pkex_data(dpp);
 
 	dpp_free_auth_data(dpp);
 
 	dpp_property_changed_notify(dpp);
+	dpp_pkex_property_changed_notify(dpp);
 }
 
 static void dpp_free(struct dpp_sm *dpp)
@@ -357,6 +461,23 @@ static void dpp_free(struct dpp_sm *dpp)
 	l_free(dpp);
 }
 
+static bool dpp_check_pkex_identifier(const char *id)
+{
+	const char *end;
+
+	if (!id)
+		return true;
+
+	/*
+	 * "If an optional code identifier is used, it shall be a UTF-8 string
+	 *  not greater than eighty (80) octets"
+	 */
+	if (!l_utf8_validate(id, strlen(id), &end) || end - id > 80)
+		return false;
+
+	return true;
+}
+
 static void dpp_send_frame_cb(struct l_genl_msg *msg, void *user_data)
 {
 	struct dpp_sm *dpp = user_data;
@@ -1512,6 +1633,71 @@ static bool dpp_send_authenticate_request(struct dpp_sm *dpp)
 	return true;
 }
 
+static void dpp_send_pkex_exchange_request(struct dpp_sm *dpp)
+{
+	uint8_t hdr[32];
+	uint8_t attrs[256];
+	uint8_t *ptr = attrs;
+	uint64_t m_data[L_ECC_MAX_DIGITS * 2];
+	uint16_t group;
+	struct iovec iov[2];
+	const uint8_t *own_mac = netdev_get_address(dpp->netdev);
+
+	l_put_le16(l_ecc_curve_get_ike_group(dpp->curve), &group);
+
+	iov[0].iov_len = dpp_build_header(own_mac, broadcast,
+				DPP_FRAME_PKEX_VERSION1_XCHG_REQUST, hdr);
+	iov[0].iov_base = hdr;
+
+	ptr += dpp_append_attr(ptr, DPP_ATTR_PROTOCOL_VERSION,
+				&dpp->pkex_version, 1);
+	ptr += dpp_append_attr(ptr, DPP_ATTR_FINITE_CYCLIC_GROUP,
+				&group, 2);
+
+	if (dpp->pkex_id)
+		ptr += dpp_append_attr(ptr, DPP_ATTR_CODE_IDENTIFIER,
+					dpp->pkex_id, strlen(dpp->pkex_id));
+
+	l_ecc_point_get_data(dpp->pkex_m, m_data, sizeof(m_data));
+
+	ptr += dpp_append_attr(ptr, DPP_ATTR_ENCRYPTED_KEY,
+				m_data, dpp->key_len * 2);
+
+	iov[1].iov_base = attrs;
+	iov[1].iov_len = ptr - attrs;
+
+	dpp_send_frame(dpp, iov, 2, dpp->current_freq);
+}
+
+static void dpp_send_commit_reveal_request(struct dpp_sm *dpp)
+{
+	struct iovec iov[2];
+	uint8_t hdr[41];
+	uint8_t attrs[512];
+	uint8_t *ptr = attrs;
+	uint8_t zero = 0;
+	uint8_t a_pub[L_ECC_POINT_MAX_BYTES];
+	ssize_t a_len;
+
+	a_len = l_ecc_point_get_data(dpp->boot_public, a_pub, sizeof(a_pub));
+
+	iov[0].iov_len = dpp_build_header(netdev_get_address(dpp->netdev),
+					dpp->peer_addr,
+					DPP_FRAME_PKEX_COMMIT_REVEAL_REQUEST,
+					hdr);
+	iov[0].iov_base = hdr;
+
+	ptr += dpp_append_wrapped_data(hdr + 26, 6, &zero, 1, ptr,
+			sizeof(attrs), dpp->z, dpp->z_len, 2,
+			DPP_ATTR_BOOTSTRAPPING_KEY, a_len, a_pub,
+			DPP_ATTR_INITIATOR_AUTH_TAG, dpp->u_len, dpp->u);
+
+	iov[1].iov_base = attrs;
+	iov[1].iov_len = ptr - attrs;
+
+	dpp_send_frame(dpp, iov, 2, dpp->current_freq);
+}
+
 static void dpp_roc_started(void *user_data)
 {
 	struct dpp_sm *dpp = user_data;
@@ -1575,6 +1761,16 @@ static void dpp_roc_started(void *user_data)
 			send_authenticate_response(dpp);
 		}
 
+		break;
+	case DPP_STATE_PKEX_EXCHANGE:
+		if (dpp->role == DPP_CAPABILITY_ENROLLEE)
+			dpp_send_pkex_exchange_request(dpp);
+
+		break;
+	case DPP_STATE_PKEX_COMMIT_REVEAL:
+		if (dpp->role == DPP_CAPABILITY_ENROLLEE)
+			dpp_send_commit_reveal_request(dpp);
+
 		break;
 	default:
 		break;
@@ -1603,6 +1799,7 @@ static void dpp_offchannel_timeout(int error, void *user_data)
 		goto protocol_failed;
 
 	switch (dpp->state) {
+	case DPP_STATE_PKEX_EXCHANGE:
 	case DPP_STATE_PRESENCE:
 		break;
 	case DPP_STATE_NOTHING:
@@ -1610,6 +1807,7 @@ static void dpp_offchannel_timeout(int error, void *user_data)
 		return;
 	case DPP_STATE_AUTHENTICATING:
 	case DPP_STATE_CONFIGURING:
+	case DPP_STATE_PKEX_COMMIT_REVEAL:
 		goto next_roc;
 	}
 
@@ -2172,6 +2370,382 @@ static void dpp_handle_presence_announcement(struct dpp_sm *dpp,
 		dpp->channel_switch = true;
 }
 
+static void dpp_pkex_bad_group(struct dpp_sm *dpp, uint16_t group)
+{
+	uint16_t own_group = l_ecc_curve_get_ike_group(dpp->curve);
+
+	/*
+	 * TODO: The spec allows group negotiation, but it is not yet
+	 *       implemented.
+	 */
+	if (!group)
+		return;
+	/*
+	 * Section 5.6.2
+	 * "If the Responder's offered group offers less security
+	 * than the Initiator's offered group, then the Initiator should
+	 * ignore this message"
+	 */
+	if (group < own_group) {
+		l_debug("Offered group %u is less secure, ignoring",
+				group);
+		return;
+	}
+	/*
+	 * Section 5.6.2
+	 * "If the Responder's offered group offers equivalent or better
+	 * security than the Initiator's offered group, then the
+	 * Initiator may choose to abort its original request and try
+	 * another exchange using the group offered by the Responder"
+	 */
+	if (group >= own_group) {
+		l_debug("Offered group %u is the same or more secure, "
+			" but group negotiation is not supported", group);
+		return;
+	}
+}
+
+static void dpp_pkex_bad_code(struct dpp_sm *dpp)
+{
+	_auto_(l_ecc_point_free) struct l_ecc_point *qr = NULL;
+
+	qr = dpp_derive_qr(dpp->curve, dpp->pkex_key, dpp->pkex_id,
+				netdev_get_address(dpp->netdev));
+	if (!qr || l_ecc_point_is_infinity(qr)) {
+		l_debug("Qr computed to zero, new code should be provisioned");
+		return;
+	}
+
+	l_debug("Qr computed successfully but responder indicated otherwise");
+}
+
+static void dpp_handle_pkex_exchange_response(struct dpp_sm *dpp,
+					const uint8_t *from,
+					const uint8_t *body, size_t body_len)
+{
+	struct dpp_attr_iter iter;
+	enum dpp_attribute_type type;
+	size_t len;
+	const uint8_t *data;
+	const uint8_t *status = NULL;
+	uint8_t version = 0;
+	const char *identifier = NULL;
+	size_t identifier_len = 0;
+	const void *key = NULL;
+	size_t key_len = 0;
+	uint16_t group = 0;
+	_auto_(l_ecc_point_free) struct l_ecc_point *n = NULL;
+	_auto_(l_ecc_point_free) struct l_ecc_point *j = NULL;
+	_auto_(l_ecc_point_free) struct l_ecc_point *qr = NULL;
+	_auto_(l_ecc_point_free) struct l_ecc_point *k = NULL;
+	const uint8_t *own_addr = netdev_get_address(dpp->netdev);
+
+	l_debug("PKEX response "MAC, MAC_STR(from));
+
+	if (dpp->state != DPP_STATE_PKEX_EXCHANGE)
+		return;
+
+	if (dpp->role != DPP_CAPABILITY_ENROLLEE)
+		return;
+
+	memcpy(dpp->peer_addr, from, 6);
+
+	dpp_attr_iter_init(&iter, body + 8, body_len - 8);
+
+	while (dpp_attr_iter_next(&iter, &type, &len, &data)) {
+		switch (type) {
+		case DPP_ATTR_STATUS:
+			if (len != 1)
+				return;
+
+			status = data;
+			break;
+		case DPP_ATTR_PROTOCOL_VERSION:
+			if (len != 1)
+				return;
+
+			version = l_get_u8(data);
+			break;
+		case DPP_ATTR_CODE_IDENTIFIER:
+			identifier = (char *) data;
+			identifier_len = len;
+			break;
+		case DPP_ATTR_ENCRYPTED_KEY:
+			if (len != dpp->key_len * 2)
+				return;
+
+			key = data;
+			key_len = len;
+			break;
+		case DPP_ATTR_FINITE_CYCLIC_GROUP:
+			if (len != 2)
+				return;
+
+			group = l_get_le16(data);
+			break;
+		default:
+			break;
+		}
+	}
+
+	if (!status) {
+		l_debug("No status attribute, ignoring");
+		return;
+	}
+
+	if (!key) {
+		l_debug("No encrypted key, ignoring");
+		return;
+	}
+
+	if (*status != DPP_STATUS_OK)
+		goto handle_status;
+
+	if (dpp->pkex_id) {
+		if (!identifier || identifier_len != strlen(dpp->pkex_id) ||
+					strncmp(dpp->pkex_id, identifier,
+						identifier_len)) {
+			l_debug("mismatch identifier, ignoring");
+			return;
+		}
+	}
+
+	if (version && version != dpp->pkex_version) {
+		l_debug("PKEX version does not match, igoring");
+		return;
+	}
+
+	n = l_ecc_point_from_data(dpp->curve, L_ECC_POINT_TYPE_FULL,
+					key, key_len);
+	if (!n) {
+		l_debug("failed to parse peers encrypted key");
+		goto failed;
+	}
+
+	qr = dpp_derive_qr(dpp->curve, dpp->pkex_key, dpp->pkex_id,
+				dpp->peer_addr);
+	if (!qr)
+		goto failed;
+
+	dpp->y_or_x = l_ecc_point_new(dpp->curve);
+
+	/* Y' = N - Qr */
+	l_ecc_point_inverse(qr);
+	l_ecc_point_add(dpp->y_or_x, n, qr);
+
+	/*
+	 * "The resulting ephemeral key, denoted Y’, is then checked whether it
+	 * is the point-at-infinity. If it is not valid, the protocol ends
+	 * unsuccessfully"
+	 */
+	if (l_ecc_point_is_infinity(dpp->y_or_x)) {
+		l_debug("Y' computed to infinity, failing");
+		goto failed;
+	}
+
+	k = l_ecc_point_new(dpp->curve);
+
+	/* K = Y' * x */
+	l_ecc_point_multiply(k, dpp->pkex_private, dpp->y_or_x);
+
+	dpp_derive_z(own_addr, dpp->peer_addr, n, dpp->pkex_m, k,
+				dpp->pkex_key, dpp->pkex_id,
+				dpp->z, &dpp->z_len);
+
+	/* J = a * Y' */
+	j = l_ecc_point_new(dpp->curve);
+
+	l_ecc_point_multiply(j, dpp->boot_private, dpp->y_or_x);
+
+	if (!dpp_derive_u(j, own_addr, dpp->boot_public, dpp->y_or_x,
+				dpp->pkex_public, dpp->u, &dpp->u_len)) {
+		l_debug("failed to compute u");
+		goto failed;
+	}
+
+	/*
+	 * Now that a response was successfully received, start another
+	 * offchannel with more time for the remainder of the protocol. After
+	 * PKEX, authentication will begin which handles the protocol timeout.
+	 * If the remainder of PKEX (commit-reveal exchange) cannot complete in
+	 * this time it will fail.
+	 */
+	dpp->dwell = (dpp->max_roc < 2000) ? dpp->max_roc : 2000;
+	dpp->state = DPP_STATE_PKEX_COMMIT_REVEAL;
+
+	dpp_pkex_property_changed_notify(dpp);
+
+	dpp_start_offchannel(dpp, dpp->current_freq);
+
+	return;
+
+handle_status:
+	switch (*status) {
+	case DPP_STATUS_BAD_GROUP:
+		dpp_pkex_bad_group(dpp, group);
+		break;
+	case DPP_STATUS_BAD_CODE:
+		dpp_pkex_bad_code(dpp);
+		break;
+	default:
+		l_debug("Unhandled status %u", *status);
+		break;
+	}
+
+failed:
+	dpp_reset(dpp);
+}
+
+static bool dpp_pkex_start_authentication(struct dpp_sm *dpp)
+{
+	dpp->uri = dpp_generate_uri(dpp->own_asn1, dpp->own_asn1_len, 2,
+					netdev_get_address(dpp->netdev),
+					&dpp->current_freq, 1, NULL, NULL);
+
+	l_ecdh_generate_key_pair(dpp->curve, &dpp->proto_private,
+					&dpp->own_proto_public);
+
+	l_getrandom(dpp->i_nonce, dpp->nonce_len);
+
+	dpp->peer_asn1 = dpp_point_to_asn1(dpp->peer_boot_public,
+						&dpp->peer_asn1_len);
+
+	dpp->m = dpp_derive_k1(dpp->peer_boot_public, dpp->proto_private,
+				dpp->k1);
+
+	dpp_hash(L_CHECKSUM_SHA256, dpp->peer_boot_hash, 1, dpp->peer_asn1,
+			dpp->peer_asn1_len);
+
+	dpp->state = DPP_STATE_AUTHENTICATING;
+	dpp->mutual_auth = true;
+
+	dpp_pkex_property_changed_notify(dpp);
+
+	if (dpp->role == DPP_CAPABILITY_ENROLLEE) {
+		dpp->new_freq = dpp->current_freq;
+
+		return dpp_send_authenticate_request(dpp);
+	}
+
+	return true;
+}
+
+static void dpp_handle_pkex_commit_reveal_response(struct dpp_sm *dpp,
+					const uint8_t *from,
+					const uint8_t *body, size_t body_len)
+{
+	struct dpp_attr_iter iter;
+	enum dpp_attribute_type type;
+	size_t len;
+	const uint8_t *data;
+	const uint8_t *wrapped = NULL;
+	size_t wrapped_len = 0;
+	uint8_t one = 1;
+	_auto_(l_free) uint8_t *unwrapped = NULL;
+	size_t unwrapped_len = 0;
+	const uint8_t *boot_key = NULL;
+	size_t boot_key_len = 0;
+	const uint8_t *r_auth = NULL;
+	uint8_t v[L_ECC_SCALAR_MAX_BYTES];
+	size_t v_len;
+	_auto_(l_ecc_point_free) struct l_ecc_point *l = NULL;
+
+	l_debug("PKEX commit reveal "MAC, MAC_STR(from));
+
+	if (dpp->state != DPP_STATE_PKEX_COMMIT_REVEAL)
+		return;
+
+	if (dpp->role != DPP_CAPABILITY_ENROLLEE)
+		return;
+
+	/*
+	 * The URI may not have contained a MAC address, if this announcement
+	 * verifies set peer_addr then.
+	 */
+	if (memcmp(from, dpp->peer_addr, 6)) {
+		l_debug("Unexpected source "MAC" expected "MAC, MAC_STR(from),
+						MAC_STR(dpp->peer_addr));
+		return;
+	}
+
+	dpp_attr_iter_init(&iter, body + 8, body_len - 8);
+
+	while (dpp_attr_iter_next(&iter, &type, &len, &data)) {
+		switch (type) {
+		case DPP_ATTR_WRAPPED_DATA:
+			wrapped = data;
+			wrapped_len = len;
+			break;
+		default:
+			break;
+		}
+	}
+
+	if (!wrapped) {
+		l_debug("No wrapped data");
+		return;
+	}
+
+	unwrapped = dpp_unwrap_attr(body + 2, 6, &one, 1, dpp->z, dpp->z_len,
+					wrapped, wrapped_len, &unwrapped_len);
+	if (!unwrapped) {
+		l_debug("Failed to unwrap Reveal-Commit message");
+		return;
+	}
+
+	dpp_attr_iter_init(&iter, unwrapped, unwrapped_len);
+
+	while (dpp_attr_iter_next(&iter, &type, &len, &data)) {
+		switch (type) {
+		case DPP_ATTR_BOOTSTRAPPING_KEY:
+			if (len != dpp->key_len * 2)
+				return;
+
+			boot_key = data;
+			boot_key_len = len;
+			break;
+		case DPP_ATTR_RESPONDER_AUTH_TAG:
+			if (len != 32)
+				return;
+
+			r_auth = data;
+			break;
+		default:
+			break;
+		}
+	}
+
+	dpp->peer_boot_public = l_ecc_point_from_data(dpp->curve,
+							L_ECC_POINT_TYPE_FULL,
+							boot_key, boot_key_len);
+	if (!dpp->peer_boot_public) {
+		l_debug("Peer public bootstrapping key was invalid");
+		goto failed;
+	}
+
+	/* L = b * X' */
+	l = l_ecc_point_new(dpp->curve);
+
+	l_ecc_point_multiply(l, dpp->pkex_private, dpp->peer_boot_public);
+
+	if (!dpp_derive_v(l, dpp->peer_addr, dpp->peer_boot_public,
+				dpp->pkex_public, dpp->y_or_x, v, &v_len)) {
+		l_debug("Failed to derive v");
+		goto failed;
+	}
+
+	if (memcmp(v, r_auth, v_len)) {
+		l_debug("Bootstrapping data did not verify");
+		goto failed;
+	}
+
+	if (dpp_pkex_start_authentication(dpp))
+		return;
+
+failed:
+	dpp_reset(dpp);
+}
+
 static void dpp_handle_frame(struct dpp_sm *dpp,
 				const struct mmpdu_header *frame,
 				const void *body, size_t body_len)
@@ -2206,6 +2780,14 @@ static void dpp_handle_frame(struct dpp_sm *dpp,
 		dpp_handle_presence_announcement(dpp, frame->address_2,
 							body, body_len);
 		break;
+	case DPP_FRAME_PKEX_XCHG_RESPONSE:
+		dpp_handle_pkex_exchange_response(dpp, frame->address_2, body,
+							body_len);
+		break;
+	case DPP_FRAME_PKEX_COMMIT_REVEAL_RESPONSE:
+		dpp_handle_pkex_commit_reveal_response(dpp, frame->address_2,
+							body, body_len);
+		break;
 	default:
 		l_debug("Unhandled DPP frame %u", *ptr);
 		break;
@@ -2262,10 +2844,16 @@ static void dpp_mlme_notify(struct l_genl_msg *msg, void *user_data)
 	if (!dpp)
 		return;
 
-	if (dpp->state <= DPP_STATE_PRESENCE)
+	/*
+	 * Don't retransmit for presence or PKEX exchange if an enrollee, both
+	 * are broadcast frames which don't expect an ack.
+	 */
+	if (dpp->state == DPP_STATE_NOTHING ||
+			dpp->state == DPP_STATE_PRESENCE ||
+			(dpp->state == DPP_STATE_PKEX_EXCHANGE &&
+			dpp->role == DPP_CAPABILITY_ENROLLEE))
 		return;
 
-
 	if (dpp->frame_cookie != cookie)
 		return;
 
@@ -2435,6 +3023,8 @@ static void dpp_create(struct netdev *netdev)
 
 	l_dbus_object_add_interface(dbus, netdev_get_path(netdev),
 					IWD_DPP_INTERFACE, dpp);
+	l_dbus_object_add_interface(dbus, netdev_get_path(netdev),
+					IWD_DPP_PKEX_INTERFACE, dpp);
 
 	dpp_frame_watch(dpp, 0x00d0, dpp_prefix, sizeof(dpp_prefix));
 
@@ -2730,9 +3320,176 @@ static struct l_dbus_message *dpp_dbus_stop(struct l_dbus *dbus,
 {
 	struct dpp_sm *dpp = user_data;
 
+	/* Don't stop PKEX from the DPP interface */
+	if (!dpp->pkex_version)
+		dpp_reset(dpp);
+
+	return l_dbus_message_new_method_return(message);
+}
+
+/*
+ * Section 5.6.1
+ * In lieu of specific channel information obtained in a manner outside
+ * the scope of this specification, PKEX responders shall select one of
+ * the following channels:
+ *  - 2.4 GHz: Channel 6 (2.437 GHz)
+ *  - 5 GHz: Channel 44 (5.220 GHz) if local regulations permit
+ *           operation only in the 5.150 – 5.250 GHz band and Channel
+ *           149 (5.745 GHz) otherwise
+ */
+static uint32_t *dpp_default_freqs(struct dpp_sm *dpp, size_t *out_len)
+{
+	struct wiphy *wiphy = wiphy_find_by_wdev(dpp->wdev_id);
+	const uint32_t default_channels[] = { 2437, 5220, 5745 };
+	uint32_t *freqs_out;
+	size_t i;
+	size_t len = 1;
+
+	if (wiphy_get_supported_bands(wiphy) & BAND_FREQ_5_GHZ)
+		len += 2;
+
+	freqs_out = l_new(uint32_t, len);
+
+	for (i = 0; i < 3; i++)
+		freqs_out[i] = default_channels[i];
+
+	*out_len = len;
+	return freqs_out;
+}
+
+static bool dpp_start_pkex_enrollee(struct dpp_sm *dpp, const char *key,
+				const char *identifier)
+{
+	struct station *station = station_find(netdev_get_ifindex(dpp->netdev));
+	_auto_(l_ecc_point_free) struct l_ecc_point *qi = NULL;
+
+	if (station && station_get_connected_network(station)) {
+		l_debug("Already connected, disconnect before enrolling");
+		return false;
+	}
+
+	if (identifier)
+		dpp->pkex_id = l_strdup(identifier);
+
+	dpp->pkex_key = l_strdup(key);
+	memcpy(dpp->peer_addr, broadcast, 6);
+	dpp->role = DPP_CAPABILITY_ENROLLEE;
+	dpp->state = DPP_STATE_PKEX_EXCHANGE;
+	/*
+	 * In theory a driver could support a lesser duration than 200ms. This
+	 * complicates things since we would need to tack on additional
+	 * offchannel requests to meet the 200ms requirement. This could be done
+	 * but for now use max_roc or 200ms, whichever is less.
+	 */
+	dpp->dwell = (dpp->max_roc < 200) ? dpp->max_roc : 200;
+	/* "DPP R2 devices are expected to use PKEXv1 by default" */
+	dpp->pkex_version = 1;
+
+	if (!l_ecdh_generate_key_pair(dpp->curve, &dpp->pkex_private,
+					&dpp->pkex_public))
+		goto failed;
+
+	/*
+	 * "If Qi is the point-at-infinity, the code shall be deleted and the
+	 * user should be notified to provision a new code"
+	 */
+	qi = dpp_derive_qi(dpp->curve, dpp->pkex_key, dpp->pkex_id,
+				netdev_get_address(dpp->netdev));
+	if (!qi || l_ecc_point_is_infinity(qi)) {
+		l_debug("Cannot derive Qi, provision a new code");
+		goto failed;
+	}
+
+	dpp->pkex_m = l_ecc_point_new(dpp->curve);
+
+	if (!l_ecc_point_add(dpp->pkex_m, dpp->pkex_public, qi))
+		goto failed;
+
+	dpp_pkex_property_changed_notify(dpp);
+
+	dpp->freqs = dpp_default_freqs(dpp, &dpp->freqs_len);
+	if (!dpp->freqs)
+		goto failed;
+
+	dpp->current_freq = dpp->freqs[dpp->freqs_idx];
+
+	dpp_reset_protocol_timer(dpp, DPP_PKEX_PROTO_TIMEOUT);
+
+	l_debug("PKEX start enrollee (id=%s)", dpp->pkex_id ?: "unset");
+
+	dpp_start_offchannel(dpp, dpp->current_freq);
+
+	return true;
+
+failed:
 	dpp_reset(dpp);
+	return false;
+}
+
+static bool dpp_parse_pkex_args(struct l_dbus_message *message,
+					const char **key_out,
+					const char **id_out)
+{
+	struct l_dbus_message_iter iter;
+	struct l_dbus_message_iter variant;
+	const char *dict_key;
+	const char *key = NULL;
+	const char *id = NULL;
+
+	if (!l_dbus_message_get_arguments(message, "a{sv}", &iter))
+		return false;
+
+	while (l_dbus_message_iter_next_entry(&iter, &dict_key, &variant)) {
+		if (!strcmp(dict_key, "Code")) {
+			if (!l_dbus_message_iter_get_variant(&variant, "s",
+								&key))
+				return false;
+		} else if (!strcmp(dict_key, "Identifier")) {
+			if (!l_dbus_message_iter_get_variant(&variant, "s",
+								&id))
+				return false;
+		}
+	}
+
+	if (!key)
+		return false;
+
+	if (id && !dpp_check_pkex_identifier(id))
+		return false;
+
+	*key_out = key;
+	*id_out = id;
+
+	return true;
+}
+
+static struct l_dbus_message *dpp_dbus_pkex_start_enrollee(struct l_dbus *dbus,
+						struct l_dbus_message *message,
+						void *user_data)
+{
+	struct dpp_sm *dpp = user_data;
+	const char *key;
+	const char *id;
+	struct station *station = station_find(netdev_get_ifindex(dpp->netdev));
+
+	l_debug("");
+
+	if (dpp->state != DPP_STATE_NOTHING)
+		return dbus_error_busy(message);
+
+	if (station_get_connected_network(station))
+		return dbus_error_busy(message);
+
+	if (!dpp_parse_pkex_args(message, &key, &id))
+		goto invalid_args;
+
+	if (!dpp_start_pkex_enrollee(dpp, key, id))
+		goto invalid_args;
 
 	return l_dbus_message_new_method_return(message);
+
+invalid_args:
+	return dbus_error_invalid_args(message);
 }
 
 static void dpp_setup_interface(struct l_dbus_interface *interface)
@@ -2753,6 +3510,31 @@ static void dpp_setup_interface(struct l_dbus_interface *interface)
 	l_dbus_interface_property(interface, "URI", 0, "s", dpp_get_uri, NULL);
 }
 
+static struct l_dbus_message *dpp_dbus_pkex_stop(struct l_dbus *dbus,
+				struct l_dbus_message *message, void *user_data)
+{
+	struct dpp_sm *dpp = user_data;
+
+	/* If PKEX/DPP was not started via this interface don't stop it */
+	if (dpp->pkex_version)
+		dpp_reset(dpp);
+
+	return l_dbus_message_new_method_return(message);
+}
+
+static void dpp_setup_pkex_interface(struct l_dbus_interface *interface)
+{
+	l_dbus_interface_method(interface, "StartEnrollee", 0,
+			dpp_dbus_pkex_start_enrollee, "", "a{sv}", "args");
+	l_dbus_interface_method(interface, "Stop", 0,
+			dpp_dbus_pkex_stop, "", "");
+
+	l_dbus_interface_property(interface, "Started", 0, "b",
+			dpp_pkex_get_started, NULL);
+	l_dbus_interface_property(interface, "Role", 0, "s", dpp_get_role,
+			NULL);
+}
+
 static void dpp_destroy_interface(void *user_data)
 {
 	struct dpp_sm *dpp = user_data;
@@ -2775,6 +3557,8 @@ static int dpp_init(void)
 	l_dbus_register_interface(dbus_get_bus(), IWD_DPP_INTERFACE,
 					dpp_setup_interface,
 					dpp_destroy_interface, false);
+	l_dbus_register_interface(dbus_get_bus(), IWD_DPP_PKEX_INTERFACE,
+					dpp_setup_pkex_interface, NULL, false);
 
 	mlme_watch = l_genl_family_register(nl80211, "mlme", dpp_mlme_notify,
 						NULL, NULL);
-- 
2.25.1

Re: [PATCH v4 2/4] dpp: initial version of PKEX enrollee support

[Denis Kenzior]

Hi James,

On 11/7/23 11:06, James Prestwood wrote:
> This is the initial support for PKEX enrollees acting as the
> initiator. A PKEX initiator starts the protocol by broadcasting
> the PKEX exchange request. This request contains a key encrypted
> with the pre-shared PKEX code. If accepted the peer sends back
> the exchange response with its own encrypted key. The enrollee
> decrypts this and performs some crypto/hashing in order to establish
> an ephemeral key used to encrypt its own boostrapping key. The
> boostrapping key is encrypted and sent to the peer in the PKEX
> commit-reveal request. The peer then does the same thing, encrypting
> its own bootstrapping key and sending to the initiator as the
> PKEX commit-reveal response.
> 
> After this, both peers have exchanged their boostrapping keys
> securely and can begin DPP authentication, then configuration.
> 
> For now the enrollee will only iterate the default channel list
> from the Easy Connect spec. Future upates will need to include some
> way of discovering non-default channel configurators, but the
> protocol needs to be ironed out first.
> ---
>   src/dpp.c | 792 +++++++++++++++++++++++++++++++++++++++++++++++++++++-
>   1 file changed, 788 insertions(+), 4 deletions(-)
> 

<snip>

> +static bool dpp_pkex_get_started(struct l_dbus *dbus,
> +				struct l_dbus_message *message,
> +				struct l_dbus_message_builder *builder,
> +				void *user_data)
> +{
> +	struct dpp_sm *dpp = user_data;
> +	bool started = false;
> +
> +	switch (dpp->state) {
> +	case DPP_STATE_PKEX_EXCHANGE:
> +	case DPP_STATE_PKEX_COMMIT_REVEAL:

So what about other states like DPP_STATE_AUTHENTICATING?

> +		started = true;
> +		break;
> +	default:
> +		break;
> +	}
> +
> +	l_dbus_message_builder_append_basic(builder, 'b', &started);
> +
> +	return true;
> +}
> +
>   static bool dpp_get_started(struct l_dbus *dbus,
>   				struct l_dbus_message *message,
>   				struct l_dbus_message_builder *builder,
>   				void *user_data)
>   {
>   	struct dpp_sm *dpp = user_data;
> -	bool started = (dpp->state != DPP_STATE_NOTHING);
> +	bool started = false;
> +
> +	switch (dpp->state) {
> +	case DPP_STATE_PRESENCE:
> +	case DPP_STATE_AUTHENTICATING:
> +	case DPP_STATE_CONFIGURING:
> +		started = true;
> +		break;
> +	default:
> +		break;
> +	}
Since you're sharing the DPP state machine object between the two interfaces, it 
seems like starting PKEX on the SharedCode interface side-effects the state of 
the regular DeviceProvisioning interface?  I hope that's not intended?

>   
>   	l_dbus_message_builder_append_basic(builder, 'b', &started);
>   
> @@ -199,7 +249,9 @@ static bool dpp_get_uri(struct l_dbus *dbus,
>   {
>   	struct dpp_sm *dpp = user_data;
>   
> -	if (dpp->state == DPP_STATE_NOTHING)
> +	if (dpp->state == DPP_STATE_NOTHING ||
> +			dpp->state == DPP_STATE_PKEX_EXCHANGE ||
> +			dpp->state == DPP_STATE_PKEX_COMMIT_REVEAL)

So what if PKEX is started and proceeds to the AUTHENTICATING state?

>   		return false;
>   
>   	l_dbus_message_builder_append_basic(builder, 's', dpp->uri);

<snip>

> @@ -321,6 +419,7 @@ static void dpp_reset(struct dpp_sm *dpp)
>   	dpp->new_freq = 0;
>   	dpp->frame_retry = 0;
>   	dpp->frame_cookie = 0;
> +	dpp->pkex_version = 0;
>   
>   	explicit_bzero(dpp->r_nonce, dpp->nonce_len);
>   	explicit_bzero(dpp->i_nonce, dpp->nonce_len);
> @@ -329,10 +428,15 @@ static void dpp_reset(struct dpp_sm *dpp)
>   	explicit_bzero(dpp->k1, dpp->key_len);
>   	explicit_bzero(dpp->k2, dpp->key_len);
>   	explicit_bzero(dpp->auth_tag, dpp->key_len);
> +	explicit_bzero(dpp->z, dpp->key_len);
> +	explicit_bzero(dpp->u, dpp->u_len);
> +
> +	dpp_free_pending_pkex_data(dpp);
>   
>   	dpp_free_auth_data(dpp);
>   
>   	dpp_property_changed_notify(dpp);
> +	dpp_pkex_property_changed_notify(dpp);

So are you emitting PropertyChanged on all interfaces, regardless whether that 
interface was actually involved in an operation?

>   }
>   
>   static void dpp_free(struct dpp_sm *dpp)
> @@ -357,6 +461,23 @@ static void dpp_free(struct dpp_sm *dpp)
>   	l_free(dpp);
>   }
>   
> +static bool dpp_check_pkex_identifier(const char *id)
> +{
> +	const char *end;
> +
> +	if (!id)
> +		return true;
> +
> +	/*
> +	 * "If an optional code identifier is used, it shall be a UTF-8 string
> +	 *  not greater than eighty (80) octets"
> +	 */
> +	if (!l_utf8_validate(id, strlen(id), &end) || end - id > 80)

Any strings obtained over d-bus must already be valid utf8 and cannot be NULL. 
You seem to call this function only from dpp_parse_pkex_args()?  I would have 
thought you'd be calling it from the DPP message parser?

> +		return false;
> +
> +	return true;
> +}
> +
>   static void dpp_send_frame_cb(struct l_genl_msg *msg, void *user_data)
>   {
>   	struct dpp_sm *dpp = user_data;
> @@ -1512,6 +1633,71 @@ static bool dpp_send_authenticate_request(struct dpp_sm *dpp)
>   	return true;
>   }
>   
> +static void dpp_send_pkex_exchange_request(struct dpp_sm *dpp)
> +{
> +	uint8_t hdr[32];
> +	uint8_t attrs[256];
> +	uint8_t *ptr = attrs;
> +	uint64_t m_data[L_ECC_MAX_DIGITS * 2];
> +	uint16_t group;
> +	struct iovec iov[2];
> +	const uint8_t *own_mac = netdev_get_address(dpp->netdev);
> +
> +	l_put_le16(l_ecc_curve_get_ike_group(dpp->curve), &group);
> +
> +	iov[0].iov_len = dpp_build_header(own_mac, broadcast,
> +				DPP_FRAME_PKEX_VERSION1_XCHG_REQUST, hdr);

Looks like there's a typo here, 'REQUST'

> +	iov[0].iov_base = hdr;
> +
> +	ptr += dpp_append_attr(ptr, DPP_ATTR_PROTOCOL_VERSION,
> +				&dpp->pkex_version, 1);
> +	ptr += dpp_append_attr(ptr, DPP_ATTR_FINITE_CYCLIC_GROUP,
> +				&group, 2);
> +
> +	if (dpp->pkex_id)
> +		ptr += dpp_append_attr(ptr, DPP_ATTR_CODE_IDENTIFIER,
> +					dpp->pkex_id, strlen(dpp->pkex_id));
> +
> +	l_ecc_point_get_data(dpp->pkex_m, m_data, sizeof(m_data));
> +
> +	ptr += dpp_append_attr(ptr, DPP_ATTR_ENCRYPTED_KEY,
> +				m_data, dpp->key_len * 2);
> +
> +	iov[1].iov_base = attrs;
> +	iov[1].iov_len = ptr - attrs;
> +
> +	dpp_send_frame(dpp, iov, 2, dpp->current_freq);
> +}
> +
> +static void dpp_send_commit_reveal_request(struct dpp_sm *dpp)
> +{
> +	struct iovec iov[2];
> +	uint8_t hdr[41];
> +	uint8_t attrs[512];
> +	uint8_t *ptr = attrs;
> +	uint8_t zero = 0;
> +	uint8_t a_pub[L_ECC_POINT_MAX_BYTES];
> +	ssize_t a_len;
> +
> +	a_len = l_ecc_point_get_data(dpp->boot_public, a_pub, sizeof(a_pub));
> +
> +	iov[0].iov_len = dpp_build_header(netdev_get_address(dpp->netdev),
> +					dpp->peer_addr,
> +					DPP_FRAME_PKEX_COMMIT_REVEAL_REQUEST,
> +					hdr);
> +	iov[0].iov_base = hdr;
> +
> +	ptr += dpp_append_wrapped_data(hdr + 26, 6, &zero, 1, ptr,
> +			sizeof(attrs), dpp->z, dpp->z_len, 2,
> +			DPP_ATTR_BOOTSTRAPPING_KEY, a_len, a_pub,
> +			DPP_ATTR_INITIATOR_AUTH_TAG, dpp->u_len, dpp->u);
> +
> +	iov[1].iov_base = attrs;
> +	iov[1].iov_len = ptr - attrs;
> +
> +	dpp_send_frame(dpp, iov, 2, dpp->current_freq);
> +}
> +
>   static void dpp_roc_started(void *user_data)
>   {
>   	struct dpp_sm *dpp = user_data;

<snip>

> +static void dpp_handle_pkex_exchange_response(struct dpp_sm *dpp,
> +					const uint8_t *from,
> +					const uint8_t *body, size_t body_len)
> +{
> +	struct dpp_attr_iter iter;
> +	enum dpp_attribute_type type;
> +	size_t len;
> +	const uint8_t *data;
> +	const uint8_t *status = NULL;
> +	uint8_t version = 0;
> +	const char *identifier = NULL;
> +	size_t identifier_len = 0;
> +	const void *key = NULL;
> +	size_t key_len = 0;
> +	uint16_t group = 0;
> +	_auto_(l_ecc_point_free) struct l_ecc_point *n = NULL;
> +	_auto_(l_ecc_point_free) struct l_ecc_point *j = NULL;
> +	_auto_(l_ecc_point_free) struct l_ecc_point *qr = NULL;
> +	_auto_(l_ecc_point_free) struct l_ecc_point *k = NULL;
> +	const uint8_t *own_addr = netdev_get_address(dpp->netdev);
> +
> +	l_debug("PKEX response "MAC, MAC_STR(from));
> +
> +	if (dpp->state != DPP_STATE_PKEX_EXCHANGE)
> +		return;
> +
> +	if (dpp->role != DPP_CAPABILITY_ENROLLEE)
> +		return;
> +
> +	memcpy(dpp->peer_addr, from, 6);
> +
> +	dpp_attr_iter_init(&iter, body + 8, body_len - 8);
> +
> +	while (dpp_attr_iter_next(&iter, &type, &len, &data)) {
> +		switch (type) {
> +		case DPP_ATTR_STATUS:
> +			if (len != 1)
> +				return;
> +
> +			status = data;
> +			break;
> +		case DPP_ATTR_PROTOCOL_VERSION:
> +			if (len != 1)
> +				return;
> +
> +			version = l_get_u8(data);
> +			break;
> +		case DPP_ATTR_CODE_IDENTIFIER:
> +			identifier = (char *) data;
> +			identifier_len = len;

Is data guaranteed to be null terminated?  Also, this cast seems fishy.  Why not 
const char?  You can't scribble into this buffer...

> +			break;
> +		case DPP_ATTR_ENCRYPTED_KEY:
> +			if (len != dpp->key_len * 2)
> +				return;
> +
> +			key = data;
> +			key_len = len;
> +			break;
> +		case DPP_ATTR_FINITE_CYCLIC_GROUP:
> +			if (len != 2)
> +				return;
> +
> +			group = l_get_le16(data);
> +			break;
> +		default:
> +			break;
> +		}
> +	}
> +
> +	if (!status) {
> +		l_debug("No status attribute, ignoring");
> +		return;
> +	}
> +
> +	if (!key) {
> +		l_debug("No encrypted key, ignoring");
> +		return;
> +	}
> +
> +	if (*status != DPP_STATUS_OK)
> +		goto handle_status;
> +
> +	if (dpp->pkex_id) {
> +		if (!identifier || identifier_len != strlen(dpp->pkex_id) ||
> +					strncmp(dpp->pkex_id, identifier,
> +						identifier_len)) {

No point in using strncmp there if you just compared the lengths.  Just use 
memcpy instead.  Compiler warnings have made strncmp and strncpy nearly useless.

> +			l_debug("mismatch identifier, ignoring");
> +			return;
> +		}
> +	}
> +
> +	if (version && version != dpp->pkex_version) {
> +		l_debug("PKEX version does not match, igoring");
> +		return;
> +	}
> +
> +	n = l_ecc_point_from_data(dpp->curve, L_ECC_POINT_TYPE_FULL,
> +					key, key_len);
> +	if (!n) {
> +		l_debug("failed to parse peers encrypted key");
> +		goto failed;
> +	}
> +
> +	qr = dpp_derive_qr(dpp->curve, dpp->pkex_key, dpp->pkex_id,
> +				dpp->peer_addr);
> +	if (!qr)
> +		goto failed;
> +
> +	dpp->y_or_x = l_ecc_point_new(dpp->curve);
> +
> +	/* Y' = N - Qr */
> +	l_ecc_point_inverse(qr);
> +	l_ecc_point_add(dpp->y_or_x, n, qr);
> +
> +	/*
> +	 * "The resulting ephemeral key, denoted Y’, is then checked whether it
> +	 * is the point-at-infinity. If it is not valid, the protocol ends
> +	 * unsuccessfully"
> +	 */
> +	if (l_ecc_point_is_infinity(dpp->y_or_x)) {
> +		l_debug("Y' computed to infinity, failing");
> +		goto failed;
> +	}
> +
> +	k = l_ecc_point_new(dpp->curve);
> +
> +	/* K = Y' * x */
> +	l_ecc_point_multiply(k, dpp->pkex_private, dpp->y_or_x);
> +
> +	dpp_derive_z(own_addr, dpp->peer_addr, n, dpp->pkex_m, k,
> +				dpp->pkex_key, dpp->pkex_id,
> +				dpp->z, &dpp->z_len);
> +
> +	/* J = a * Y' */
> +	j = l_ecc_point_new(dpp->curve);
> +
> +	l_ecc_point_multiply(j, dpp->boot_private, dpp->y_or_x);
> +
> +	if (!dpp_derive_u(j, own_addr, dpp->boot_public, dpp->y_or_x,
> +				dpp->pkex_public, dpp->u, &dpp->u_len)) {
> +		l_debug("failed to compute u");
> +		goto failed;
> +	}
> +
> +	/*
> +	 * Now that a response was successfully received, start another
> +	 * offchannel with more time for the remainder of the protocol. After
> +	 * PKEX, authentication will begin which handles the protocol timeout.
> +	 * If the remainder of PKEX (commit-reveal exchange) cannot complete in
> +	 * this time it will fail.
> +	 */
> +	dpp->dwell = (dpp->max_roc < 2000) ? dpp->max_roc : 2000;
> +	dpp->state = DPP_STATE_PKEX_COMMIT_REVEAL;
> +
> +	dpp_pkex_property_changed_notify(dpp);
> +
> +	dpp_start_offchannel(dpp, dpp->current_freq);
> +
> +	return;
> +
> +handle_status:
> +	switch (*status) {
> +	case DPP_STATUS_BAD_GROUP:
> +		dpp_pkex_bad_group(dpp, group);
> +		break;
> +	case DPP_STATUS_BAD_CODE:
> +		dpp_pkex_bad_code(dpp);
> +		break;
> +	default:
> +		l_debug("Unhandled status %u", *status);
> +		break;
> +	}
> +
> +failed:
> +	dpp_reset(dpp);
> +}
> +
> +static bool dpp_pkex_start_authentication(struct dpp_sm *dpp)
> +{
> +	dpp->uri = dpp_generate_uri(dpp->own_asn1, dpp->own_asn1_len, 2,
> +					netdev_get_address(dpp->netdev),
> +					&dpp->current_freq, 1, NULL, NULL);
> +
> +	l_ecdh_generate_key_pair(dpp->curve, &dpp->proto_private,
> +					&dpp->own_proto_public);
> +
> +	l_getrandom(dpp->i_nonce, dpp->nonce_len);
> +
> +	dpp->peer_asn1 = dpp_point_to_asn1(dpp->peer_boot_public,
> +						&dpp->peer_asn1_len);
> +
> +	dpp->m = dpp_derive_k1(dpp->peer_boot_public, dpp->proto_private,
> +				dpp->k1);
> +
> +	dpp_hash(L_CHECKSUM_SHA256, dpp->peer_boot_hash, 1, dpp->peer_asn1,
> +			dpp->peer_asn1_len);
> +
> +	dpp->state = DPP_STATE_AUTHENTICATING;

So you go into the authenticating state even in PKEX.  But now this state might 
be reflected in any property GetAll/Get calls on the DeviceProvisioning interface...

> +	dpp->mutual_auth = true;
> +
> +	dpp_pkex_property_changed_notify(dpp);
> +
> +	if (dpp->role == DPP_CAPABILITY_ENROLLEE) {
> +		dpp->new_freq = dpp->current_freq;
> +
> +		return dpp_send_authenticate_request(dpp);
> +	}
> +
> +	return true;
> +}
> +

<snip>

> @@ -2435,6 +3023,8 @@ static void dpp_create(struct netdev *netdev)
>   
>   	l_dbus_object_add_interface(dbus, netdev_get_path(netdev),
>   					IWD_DPP_INTERFACE, dpp);
> +	l_dbus_object_add_interface(dbus, netdev_get_path(netdev),
> +					IWD_DPP_PKEX_INTERFACE, dpp);

Looks like dpp interfaces share the same underlying object...

>   
>   	dpp_frame_watch(dpp, 0x00d0, dpp_prefix, sizeof(dpp_prefix));
>   
> @@ -2730,9 +3320,176 @@ static struct l_dbus_message *dpp_dbus_stop(struct l_dbus *dbus,
>   {
>   	struct dpp_sm *dpp = user_data;
>   
> +	/* Don't stop PKEX from the DPP interface */
> +	if (!dpp->pkex_version)
> +		dpp_reset(dpp);
> +
> +	return l_dbus_message_new_method_return(message);
> +}
> +
> +/*
> + * Section 5.6.1
> + * In lieu of specific channel information obtained in a manner outside
> + * the scope of this specification, PKEX responders shall select one of
> + * the following channels:
> + *  - 2.4 GHz: Channel 6 (2.437 GHz)
> + *  - 5 GHz: Channel 44 (5.220 GHz) if local regulations permit
> + *           operation only in the 5.150 – 5.250 GHz band and Channel
> + *           149 (5.745 GHz) otherwise
> + */
> +static uint32_t *dpp_default_freqs(struct dpp_sm *dpp, size_t *out_len)
> +{
> +	struct wiphy *wiphy = wiphy_find_by_wdev(dpp->wdev_id);
> +	const uint32_t default_channels[] = { 2437, 5220, 5745 };
> +	uint32_t *freqs_out;
> +	size_t i;
> +	size_t len = 1;
> +
> +	if (wiphy_get_supported_bands(wiphy) & BAND_FREQ_5_GHZ)
> +		len += 2;

Is 2.4GHz band support check needed?

> +
> +	freqs_out = l_new(uint32_t, len);
> +
> +	for (i = 0; i < 3; i++)
> +		freqs_out[i] = default_channels[i];
> +
> +	*out_len = len;
> +	return freqs_out;
> +}
> +

Regards,
-Denis

Re: [PATCH v4 2/4] dpp: initial version of PKEX enrollee support

[James Prestwood]

Hi Denis,

On 11/7/23 7:11 PM, Denis Kenzior wrote:
> Hi James,
> 
> On 11/7/23 11:06, James Prestwood wrote:
>> This is the initial support for PKEX enrollees acting as the
>> initiator. A PKEX initiator starts the protocol by broadcasting
>> the PKEX exchange request. This request contains a key encrypted
>> with the pre-shared PKEX code. If accepted the peer sends back
>> the exchange response with its own encrypted key. The enrollee
>> decrypts this and performs some crypto/hashing in order to establish
>> an ephemeral key used to encrypt its own boostrapping key. The
>> boostrapping key is encrypted and sent to the peer in the PKEX
>> commit-reveal request. The peer then does the same thing, encrypting
>> its own bootstrapping key and sending to the initiator as the
>> PKEX commit-reveal response.
>>
>> After this, both peers have exchanged their boostrapping keys
>> securely and can begin DPP authentication, then configuration.
>>
>> For now the enrollee will only iterate the default channel list
>> from the Easy Connect spec. Future upates will need to include some
>> way of discovering non-default channel configurators, but the
>> protocol needs to be ironed out first.
>> ---
>>   src/dpp.c | 792 +++++++++++++++++++++++++++++++++++++++++++++++++++++-
>>   1 file changed, 788 insertions(+), 4 deletions(-)
>>
> 
> <snip>
> 
>> +static bool dpp_pkex_get_started(struct l_dbus *dbus,
>> +                struct l_dbus_message *message,
>> +                struct l_dbus_message_builder *builder,
>> +                void *user_data)
>> +{
>> +    struct dpp_sm *dpp = user_data;
>> +    bool started = false;
>> +
>> +    switch (dpp->state) {
>> +    case DPP_STATE_PKEX_EXCHANGE:
>> +    case DPP_STATE_PKEX_COMMIT_REVEAL:
> 
> So what about other states like DPP_STATE_AUTHENTICATING?
> 
>> +        started = true;
>> +        break;
>> +    default:
>> +        break;
>> +    }
>> +
>> +    l_dbus_message_builder_append_basic(builder, 'b', &started);
>> +
>> +    return true;
>> +}
>> +
>>   static bool dpp_get_started(struct l_dbus *dbus,
>>                   struct l_dbus_message *message,
>>                   struct l_dbus_message_builder *builder,
>>                   void *user_data)
>>   {
>>       struct dpp_sm *dpp = user_data;
>> -    bool started = (dpp->state != DPP_STATE_NOTHING);
>> +    bool started = false;
>> +
>> +    switch (dpp->state) {
>> +    case DPP_STATE_PRESENCE:
>> +    case DPP_STATE_AUTHENTICATING:
>> +    case DPP_STATE_CONFIGURING:
>> +        started = true;
>> +        break;
>> +    default:
>> +        break;
>> +    }
> Since you're sharing the DPP state machine object between the two 
> interfaces, it seems like starting PKEX on the SharedCode interface 
> side-effects the state of the regular DeviceProvisioning interface?  I 
> hope that's not intended?

Your right, it does. Once PKEX finishes it starts DPP on the 
DeviceProvisioning interface. This was intended, but if we want to keep 
the two isolated I'll have to change gears and think about how we can do it.

May have to create a dpp_sm for each DBus interface not per-wdev/netdev, 
and find some way of communicating which interface the property changed 
calls correspond to.

Apart from the string/cast comments the rest seem to revolve around the 
shared state between PKEX/DPP. If separating them is the way we want to 
go I can do that.

> 
>>       l_dbus_message_builder_append_basic(builder, 'b', &started);
>> @@ -199,7 +249,9 @@ static bool dpp_get_uri(struct l_dbus *dbus,
>>   {
>>       struct dpp_sm *dpp = user_data;
>> -    if (dpp->state == DPP_STATE_NOTHING)
>> +    if (dpp->state == DPP_STATE_NOTHING ||
>> +            dpp->state == DPP_STATE_PKEX_EXCHANGE ||
>> +            dpp->state == DPP_STATE_PKEX_COMMIT_REVEAL)
> 
> So what if PKEX is started and proceeds to the AUTHENTICATING state?
> 
>>           return false;
>>       l_dbus_message_builder_append_basic(builder, 's', dpp->uri);
> 
> <snip>
> 
>> @@ -321,6 +419,7 @@ static void dpp_reset(struct dpp_sm *dpp)
>>       dpp->new_freq = 0;
>>       dpp->frame_retry = 0;
>>       dpp->frame_cookie = 0;
>> +    dpp->pkex_version = 0;
>>       explicit_bzero(dpp->r_nonce, dpp->nonce_len);
>>       explicit_bzero(dpp->i_nonce, dpp->nonce_len);
>> @@ -329,10 +428,15 @@ static void dpp_reset(struct dpp_sm *dpp)
>>       explicit_bzero(dpp->k1, dpp->key_len);
>>       explicit_bzero(dpp->k2, dpp->key_len);
>>       explicit_bzero(dpp->auth_tag, dpp->key_len);
>> +    explicit_bzero(dpp->z, dpp->key_len);
>> +    explicit_bzero(dpp->u, dpp->u_len);
>> +
>> +    dpp_free_pending_pkex_data(dpp);
>>       dpp_free_auth_data(dpp);
>>       dpp_property_changed_notify(dpp);
>> +    dpp_pkex_property_changed_notify(dpp);
> 
> So are you emitting PropertyChanged on all interfaces, regardless 
> whether that interface was actually involved in an operation?
> 
>>   }
>>   static void dpp_free(struct dpp_sm *dpp)
>> @@ -357,6 +461,23 @@ static void dpp_free(struct dpp_sm *dpp)
>>       l_free(dpp);
>>   }
>> +static bool dpp_check_pkex_identifier(const char *id)
>> +{
>> +    const char *end;
>> +
>> +    if (!id)
>> +        return true;
>> +
>> +    /*
>> +     * "If an optional code identifier is used, it shall be a UTF-8 
>> string
>> +     *  not greater than eighty (80) octets"
>> +     */
>> +    if (!l_utf8_validate(id, strlen(id), &end) || end - id > 80)
> 
> Any strings obtained over d-bus must already be valid utf8 and cannot be 
> NULL. You seem to call this function only from dpp_parse_pkex_args()?  I 
> would have thought you'd be calling it from the DPP message parser?
> 
>> +        return false;
>> +
>> +    return true;
>> +}
>> +
>>   static void dpp_send_frame_cb(struct l_genl_msg *msg, void *user_data)
>>   {
>>       struct dpp_sm *dpp = user_data;
>> @@ -1512,6 +1633,71 @@ static bool 
>> dpp_send_authenticate_request(struct dpp_sm *dpp)
>>       return true;
>>   }
>> +static void dpp_send_pkex_exchange_request(struct dpp_sm *dpp)
>> +{
>> +    uint8_t hdr[32];
>> +    uint8_t attrs[256];
>> +    uint8_t *ptr = attrs;
>> +    uint64_t m_data[L_ECC_MAX_DIGITS * 2];
>> +    uint16_t group;
>> +    struct iovec iov[2];
>> +    const uint8_t *own_mac = netdev_get_address(dpp->netdev);
>> +
>> +    l_put_le16(l_ecc_curve_get_ike_group(dpp->curve), &group);
>> +
>> +    iov[0].iov_len = dpp_build_header(own_mac, broadcast,
>> +                DPP_FRAME_PKEX_VERSION1_XCHG_REQUST, hdr);
> 
> Looks like there's a typo here, 'REQUST'
> 
>> +    iov[0].iov_base = hdr;
>> +
>> +    ptr += dpp_append_attr(ptr, DPP_ATTR_PROTOCOL_VERSION,
>> +                &dpp->pkex_version, 1);
>> +    ptr += dpp_append_attr(ptr, DPP_ATTR_FINITE_CYCLIC_GROUP,
>> +                &group, 2);
>> +
>> +    if (dpp->pkex_id)
>> +        ptr += dpp_append_attr(ptr, DPP_ATTR_CODE_IDENTIFIER,
>> +                    dpp->pkex_id, strlen(dpp->pkex_id));
>> +
>> +    l_ecc_point_get_data(dpp->pkex_m, m_data, sizeof(m_data));
>> +
>> +    ptr += dpp_append_attr(ptr, DPP_ATTR_ENCRYPTED_KEY,
>> +                m_data, dpp->key_len * 2);
>> +
>> +    iov[1].iov_base = attrs;
>> +    iov[1].iov_len = ptr - attrs;
>> +
>> +    dpp_send_frame(dpp, iov, 2, dpp->current_freq);
>> +}
>> +
>> +static void dpp_send_commit_reveal_request(struct dpp_sm *dpp)
>> +{
>> +    struct iovec iov[2];
>> +    uint8_t hdr[41];
>> +    uint8_t attrs[512];
>> +    uint8_t *ptr = attrs;
>> +    uint8_t zero = 0;
>> +    uint8_t a_pub[L_ECC_POINT_MAX_BYTES];
>> +    ssize_t a_len;
>> +
>> +    a_len = l_ecc_point_get_data(dpp->boot_public, a_pub, 
>> sizeof(a_pub));
>> +
>> +    iov[0].iov_len = dpp_build_header(netdev_get_address(dpp->netdev),
>> +                    dpp->peer_addr,
>> +                    DPP_FRAME_PKEX_COMMIT_REVEAL_REQUEST,
>> +                    hdr);
>> +    iov[0].iov_base = hdr;
>> +
>> +    ptr += dpp_append_wrapped_data(hdr + 26, 6, &zero, 1, ptr,
>> +            sizeof(attrs), dpp->z, dpp->z_len, 2,
>> +            DPP_ATTR_BOOTSTRAPPING_KEY, a_len, a_pub,
>> +            DPP_ATTR_INITIATOR_AUTH_TAG, dpp->u_len, dpp->u);
>> +
>> +    iov[1].iov_base = attrs;
>> +    iov[1].iov_len = ptr - attrs;
>> +
>> +    dpp_send_frame(dpp, iov, 2, dpp->current_freq);
>> +}
>> +
>>   static void dpp_roc_started(void *user_data)
>>   {
>>       struct dpp_sm *dpp = user_data;
> 
> <snip>
> 
>> +static void dpp_handle_pkex_exchange_response(struct dpp_sm *dpp,
>> +                    const uint8_t *from,
>> +                    const uint8_t *body, size_t body_len)
>> +{
>> +    struct dpp_attr_iter iter;
>> +    enum dpp_attribute_type type;
>> +    size_t len;
>> +    const uint8_t *data;
>> +    const uint8_t *status = NULL;
>> +    uint8_t version = 0;
>> +    const char *identifier = NULL;
>> +    size_t identifier_len = 0;
>> +    const void *key = NULL;
>> +    size_t key_len = 0;
>> +    uint16_t group = 0;
>> +    _auto_(l_ecc_point_free) struct l_ecc_point *n = NULL;
>> +    _auto_(l_ecc_point_free) struct l_ecc_point *j = NULL;
>> +    _auto_(l_ecc_point_free) struct l_ecc_point *qr = NULL;
>> +    _auto_(l_ecc_point_free) struct l_ecc_point *k = NULL;
>> +    const uint8_t *own_addr = netdev_get_address(dpp->netdev);
>> +
>> +    l_debug("PKEX response "MAC, MAC_STR(from));
>> +
>> +    if (dpp->state != DPP_STATE_PKEX_EXCHANGE)
>> +        return;
>> +
>> +    if (dpp->role != DPP_CAPABILITY_ENROLLEE)
>> +        return;
>> +
>> +    memcpy(dpp->peer_addr, from, 6);
>> +
>> +    dpp_attr_iter_init(&iter, body + 8, body_len - 8);
>> +
>> +    while (dpp_attr_iter_next(&iter, &type, &len, &data)) {
>> +        switch (type) {
>> +        case DPP_ATTR_STATUS:
>> +            if (len != 1)
>> +                return;
>> +
>> +            status = data;
>> +            break;
>> +        case DPP_ATTR_PROTOCOL_VERSION:
>> +            if (len != 1)
>> +                return;
>> +
>> +            version = l_get_u8(data);
>> +            break;
>> +        case DPP_ATTR_CODE_IDENTIFIER:
>> +            identifier = (char *) data;
>> +            identifier_len = len;
> 
> Is data guaranteed to be null terminated?  Also, this cast seems fishy.  
> Why not const char?  You can't scribble into this buffer...
> 
>> +            break;
>> +        case DPP_ATTR_ENCRYPTED_KEY:
>> +            if (len != dpp->key_len * 2)
>> +                return;
>> +
>> +            key = data;
>> +            key_len = len;
>> +            break;
>> +        case DPP_ATTR_FINITE_CYCLIC_GROUP:
>> +            if (len != 2)
>> +                return;
>> +
>> +            group = l_get_le16(data);
>> +            break;
>> +        default:
>> +            break;
>> +        }
>> +    }
>> +
>> +    if (!status) {
>> +        l_debug("No status attribute, ignoring");
>> +        return;
>> +    }
>> +
>> +    if (!key) {
>> +        l_debug("No encrypted key, ignoring");
>> +        return;
>> +    }
>> +
>> +    if (*status != DPP_STATUS_OK)
>> +        goto handle_status;
>> +
>> +    if (dpp->pkex_id) {
>> +        if (!identifier || identifier_len != strlen(dpp->pkex_id) ||
>> +                    strncmp(dpp->pkex_id, identifier,
>> +                        identifier_len)) {
> 
> No point in using strncmp there if you just compared the lengths.  Just 
> use memcpy instead.  Compiler warnings have made strncmp and strncpy 
> nearly useless.
> 
>> +            l_debug("mismatch identifier, ignoring");
>> +            return;
>> +        }
>> +    }
>> +
>> +    if (version && version != dpp->pkex_version) {
>> +        l_debug("PKEX version does not match, igoring");
>> +        return;
>> +    }
>> +
>> +    n = l_ecc_point_from_data(dpp->curve, L_ECC_POINT_TYPE_FULL,
>> +                    key, key_len);
>> +    if (!n) {
>> +        l_debug("failed to parse peers encrypted key");
>> +        goto failed;
>> +    }
>> +
>> +    qr = dpp_derive_qr(dpp->curve, dpp->pkex_key, dpp->pkex_id,
>> +                dpp->peer_addr);
>> +    if (!qr)
>> +        goto failed;
>> +
>> +    dpp->y_or_x = l_ecc_point_new(dpp->curve);
>> +
>> +    /* Y' = N - Qr */
>> +    l_ecc_point_inverse(qr);
>> +    l_ecc_point_add(dpp->y_or_x, n, qr);
>> +
>> +    /*
>> +     * "The resulting ephemeral key, denoted Y’, is then checked 
>> whether it
>> +     * is the point-at-infinity. If it is not valid, the protocol ends
>> +     * unsuccessfully"
>> +     */
>> +    if (l_ecc_point_is_infinity(dpp->y_or_x)) {
>> +        l_debug("Y' computed to infinity, failing");
>> +        goto failed;
>> +    }
>> +
>> +    k = l_ecc_point_new(dpp->curve);
>> +
>> +    /* K = Y' * x */
>> +    l_ecc_point_multiply(k, dpp->pkex_private, dpp->y_or_x);
>> +
>> +    dpp_derive_z(own_addr, dpp->peer_addr, n, dpp->pkex_m, k,
>> +                dpp->pkex_key, dpp->pkex_id,
>> +                dpp->z, &dpp->z_len);
>> +
>> +    /* J = a * Y' */
>> +    j = l_ecc_point_new(dpp->curve);
>> +
>> +    l_ecc_point_multiply(j, dpp->boot_private, dpp->y_or_x);
>> +
>> +    if (!dpp_derive_u(j, own_addr, dpp->boot_public, dpp->y_or_x,
>> +                dpp->pkex_public, dpp->u, &dpp->u_len)) {
>> +        l_debug("failed to compute u");
>> +        goto failed;
>> +    }
>> +
>> +    /*
>> +     * Now that a response was successfully received, start another
>> +     * offchannel with more time for the remainder of the protocol. 
>> After
>> +     * PKEX, authentication will begin which handles the protocol 
>> timeout.
>> +     * If the remainder of PKEX (commit-reveal exchange) cannot 
>> complete in
>> +     * this time it will fail.
>> +     */
>> +    dpp->dwell = (dpp->max_roc < 2000) ? dpp->max_roc : 2000;
>> +    dpp->state = DPP_STATE_PKEX_COMMIT_REVEAL;
>> +
>> +    dpp_pkex_property_changed_notify(dpp);
>> +
>> +    dpp_start_offchannel(dpp, dpp->current_freq);
>> +
>> +    return;
>> +
>> +handle_status:
>> +    switch (*status) {
>> +    case DPP_STATUS_BAD_GROUP:
>> +        dpp_pkex_bad_group(dpp, group);
>> +        break;
>> +    case DPP_STATUS_BAD_CODE:
>> +        dpp_pkex_bad_code(dpp);
>> +        break;
>> +    default:
>> +        l_debug("Unhandled status %u", *status);
>> +        break;
>> +    }
>> +
>> +failed:
>> +    dpp_reset(dpp);
>> +}
>> +
>> +static bool dpp_pkex_start_authentication(struct dpp_sm *dpp)
>> +{
>> +    dpp->uri = dpp_generate_uri(dpp->own_asn1, dpp->own_asn1_len, 2,
>> +                    netdev_get_address(dpp->netdev),
>> +                    &dpp->current_freq, 1, NULL, NULL);
>> +
>> +    l_ecdh_generate_key_pair(dpp->curve, &dpp->proto_private,
>> +                    &dpp->own_proto_public);
>> +
>> +    l_getrandom(dpp->i_nonce, dpp->nonce_len);
>> +
>> +    dpp->peer_asn1 = dpp_point_to_asn1(dpp->peer_boot_public,
>> +                        &dpp->peer_asn1_len);
>> +
>> +    dpp->m = dpp_derive_k1(dpp->peer_boot_public, dpp->proto_private,
>> +                dpp->k1);
>> +
>> +    dpp_hash(L_CHECKSUM_SHA256, dpp->peer_boot_hash, 1, dpp->peer_asn1,
>> +            dpp->peer_asn1_len);
>> +
>> +    dpp->state = DPP_STATE_AUTHENTICATING;
> 
> So you go into the authenticating state even in PKEX.  But now this 
> state might be reflected in any property GetAll/Get calls on the 
> DeviceProvisioning interface...
> 
>> +    dpp->mutual_auth = true;
>> +
>> +    dpp_pkex_property_changed_notify(dpp);
>> +
>> +    if (dpp->role == DPP_CAPABILITY_ENROLLEE) {
>> +        dpp->new_freq = dpp->current_freq;
>> +
>> +        return dpp_send_authenticate_request(dpp);
>> +    }
>> +
>> +    return true;
>> +}
>> +
> 
> <snip>
> 
>> @@ -2435,6 +3023,8 @@ static void dpp_create(struct netdev *netdev)
>>       l_dbus_object_add_interface(dbus, netdev_get_path(netdev),
>>                       IWD_DPP_INTERFACE, dpp);
>> +    l_dbus_object_add_interface(dbus, netdev_get_path(netdev),
>> +                    IWD_DPP_PKEX_INTERFACE, dpp);
> 
> Looks like dpp interfaces share the same underlying object...
> 
>>       dpp_frame_watch(dpp, 0x00d0, dpp_prefix, sizeof(dpp_prefix));
>> @@ -2730,9 +3320,176 @@ static struct l_dbus_message 
>> *dpp_dbus_stop(struct l_dbus *dbus,
>>   {
>>       struct dpp_sm *dpp = user_data;
>> +    /* Don't stop PKEX from the DPP interface */
>> +    if (!dpp->pkex_version)
>> +        dpp_reset(dpp);
>> +
>> +    return l_dbus_message_new_method_return(message);
>> +}
>> +
>> +/*
>> + * Section 5.6.1
>> + * In lieu of specific channel information obtained in a manner outside
>> + * the scope of this specification, PKEX responders shall select one of
>> + * the following channels:
>> + *  - 2.4 GHz: Channel 6 (2.437 GHz)
>> + *  - 5 GHz: Channel 44 (5.220 GHz) if local regulations permit
>> + *           operation only in the 5.150 – 5.250 GHz band and Channel
>> + *           149 (5.745 GHz) otherwise
>> + */
>> +static uint32_t *dpp_default_freqs(struct dpp_sm *dpp, size_t *out_len)
>> +{
>> +    struct wiphy *wiphy = wiphy_find_by_wdev(dpp->wdev_id);
>> +    const uint32_t default_channels[] = { 2437, 5220, 5745 };
>> +    uint32_t *freqs_out;
>> +    size_t i;
>> +    size_t len = 1;
>> +
>> +    if (wiphy_get_supported_bands(wiphy) & BAND_FREQ_5_GHZ)
>> +        len += 2;
> 
> Is 2.4GHz band support check needed?
> 
>> +
>> +    freqs_out = l_new(uint32_t, len);
>> +
>> +    for (i = 0; i < 3; i++)
>> +        freqs_out[i] = default_channels[i];
>> +
>> +    *out_len = len;
>> +    return freqs_out;
>> +}
>> +
> 
> Regards,
> -Denis

Re: [PATCH v4 2/4] dpp: initial version of PKEX enrollee support

[Denis Kenzior]

Hi James,

>> Since you're sharing the DPP state machine object between the two interfaces, 
>> it seems like starting PKEX on the SharedCode interface side-effects the state 
>> of the regular DeviceProvisioning interface?  I hope that's not intended?
> 
> Your right, it does. Once PKEX finishes it starts DPP on the DeviceProvisioning 
> interface. This was intended, but if we want to keep the two isolated I'll have 
> to change gears and think about how we can do it.

Ugh :)  This is something no-one but the person who wrote the code would expect 
to happen.  The interfaces should be kept separate.

Internal implementation wise, I think you probably can get away with a shared 
dpp state machine object, but you do have to track which 'interface' the state 
machine is actually bound to at the time.

> 
> May have to create a dpp_sm for each DBus interface not per-wdev/netdev, and 
> find some way of communicating which interface the property changed calls 
> correspond to.
> 

Well, there's always dpp_find or dpp_pkex_find.

> Apart from the string/cast comments the rest seem to revolve around the shared 
> state between PKEX/DPP. If separating them is the way we want to go I can do that.
> 

I didn't look super closely at the spec details, but what I saw seemed reasonable.

Regards,
-Denis

Re: [PATCH v4 2/4] dpp: initial version of PKEX enrollee support

[James Prestwood]

Hi Denis,

On 11/8/23 7:07 AM, Denis Kenzior wrote:
> Hi James,
> 
>>> Since you're sharing the DPP state machine object between the two 
>>> interfaces, it seems like starting PKEX on the SharedCode interface 
>>> side-effects the state of the regular DeviceProvisioning interface?  
>>> I hope that's not intended?
>>
>> Your right, it does. Once PKEX finishes it starts DPP on the 
>> DeviceProvisioning interface. This was intended, but if we want to 
>> keep the two isolated I'll have to change gears and think about how we 
>> can do it.
> 
> Ugh :)  This is something no-one but the person who wrote the code would 
> expect to happen.  The interfaces should be kept separate.
> 
> Internal implementation wise, I think you probably can get away with a 
> shared dpp state machine object, but you do have to track which 
> 'interface' the state machine is actually bound to at the time.

Ok, if your fine with this its much easier to implement than separate 
objects per interface, i.e. store an enum that corresponds to the 
interface using the SM.

> 
>>
>> May have to create a dpp_sm for each DBus interface not 
>> per-wdev/netdev, and find some way of communicating which interface 
>> the property changed calls correspond to.
>>
> 
> Well, there's always dpp_find or dpp_pkex_find.
> 
>> Apart from the string/cast comments the rest seem to revolve around 
>> the shared state between PKEX/DPP. If separating them is the way we 
>> want to go I can do that.
>>
> 
> I didn't look super closely at the spec details, but what I saw seemed 
> reasonable.
> 
> Regards,
> -Denis
> 

[PATCH v4 3/4] dpp: initial version of PKEX configurator support

[James Prestwood]

The PKEX configurator role is currently limited to being a responder.
When started the configurator will listen on its current operating
channel for a PKEX exchange request. Once received it and the
encrypted key is properly decrypted it treats this peer as the
enrollee and won't allow configurations from other peers unless
PKEX is restarted. The configurator will encrypt and send its
encrypted ephemeral key in the PKEX exchange response. The enrollee
then sends its encrypted bootstrapping key (as commit-reveal request)
then the same for the configurator (as commit-reveal response).

After this, PKEX authentication begins. The enrollee is expected to
send the authenticate request, since its the initiator.
---
 src/dpp.c | 504 +++++++++++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 503 insertions(+), 1 deletion(-)

diff --git a/src/dpp.c b/src/dpp.c
index 4b1c1233..86361ba0 100644
--- a/src/dpp.c
+++ b/src/dpp.c
@@ -1887,7 +1887,8 @@ static void authenticate_request(struct dpp_sm *dpp, const uint8_t *from,
 	if (util_is_broadcast_address(from))
 		return;
 
-	if (dpp->state != DPP_STATE_PRESENCE)
+	if (dpp->state != DPP_STATE_PRESENCE &&
+				dpp->state != DPP_STATE_AUTHENTICATING)
 		return;
 
 	l_debug("authenticate request");
@@ -2746,6 +2747,436 @@ failed:
 	dpp_reset(dpp);
 }
 
+static void dpp_send_bad_group(struct dpp_sm *dpp, const uint8_t *addr)
+{
+	uint8_t hdr[32];
+	uint8_t attrs[256];
+	uint8_t *ptr = attrs;
+	uint16_t group;
+	uint8_t status = DPP_STATUS_BAD_GROUP;
+	struct iovec iov[2];
+	const uint8_t *own_mac = netdev_get_address(dpp->netdev);
+
+	l_put_le16(l_ecc_curve_get_ike_group(dpp->curve), &group);
+
+	iov[0].iov_len = dpp_build_header(own_mac, addr,
+				DPP_FRAME_PKEX_XCHG_RESPONSE, hdr);
+	iov[0].iov_base = hdr;
+
+	ptr += dpp_append_attr(ptr, DPP_ATTR_STATUS, &status, 1);
+	ptr += dpp_append_attr(ptr, DPP_ATTR_PROTOCOL_VERSION,
+				&dpp->pkex_version, 1);
+	ptr += dpp_append_attr(ptr, DPP_ATTR_FINITE_CYCLIC_GROUP, &group, 2);
+
+	iov[1].iov_base = attrs;
+	iov[1].iov_len = ptr - attrs;
+
+	dpp_send_frame(dpp, iov, 2, dpp->current_freq);
+}
+
+static void dpp_send_bad_code(struct dpp_sm *dpp, const uint8_t *addr)
+{
+	uint8_t hdr[32];
+	uint8_t attrs[256];
+	uint8_t *ptr = attrs;
+	uint8_t status = DPP_STATUS_BAD_CODE;
+	struct iovec iov[2];
+	const uint8_t *own_mac = netdev_get_address(dpp->netdev);
+
+	iov[0].iov_len = dpp_build_header(own_mac, addr,
+				DPP_FRAME_PKEX_XCHG_RESPONSE, hdr);
+	iov[0].iov_base = hdr;
+
+	ptr += dpp_append_attr(ptr, DPP_ATTR_STATUS, &status, 1);
+	ptr += dpp_append_attr(ptr, DPP_ATTR_PROTOCOL_VERSION,
+				&dpp->pkex_version, 1);
+	if (dpp->pkex_id)
+		ptr += dpp_append_attr(ptr, DPP_ATTR_CODE_IDENTIFIER,
+					dpp->pkex_id, strlen(dpp->pkex_id));
+
+	iov[1].iov_base = attrs;
+	iov[1].iov_len = ptr - attrs;
+
+	dpp_send_frame(dpp, iov, 2, dpp->current_freq);
+}
+
+static void dpp_send_pkex_exchange_response(struct dpp_sm *dpp,
+						struct l_ecc_point *n)
+{
+	uint8_t hdr[32];
+	uint8_t attrs[256];
+	uint8_t *ptr = attrs;
+	uint64_t n_data[L_ECC_MAX_DIGITS * 2];
+	uint16_t group;
+	uint8_t status = DPP_STATUS_OK;
+	struct iovec iov[2];
+	const uint8_t *own_mac = netdev_get_address(dpp->netdev);
+
+	l_put_le16(l_ecc_curve_get_ike_group(dpp->curve), &group);
+
+	iov[0].iov_len = dpp_build_header(own_mac, dpp->peer_addr,
+				DPP_FRAME_PKEX_XCHG_RESPONSE, hdr);
+	iov[0].iov_base = hdr;
+
+	ptr += dpp_append_attr(ptr, DPP_ATTR_STATUS, &status, 1);
+
+	if (dpp->pkex_id)
+		ptr += dpp_append_attr(ptr, DPP_ATTR_CODE_IDENTIFIER,
+					dpp->pkex_id, strlen(dpp->pkex_id));
+
+	l_ecc_point_get_data(n, n_data, sizeof(n_data));
+
+	ptr += dpp_append_attr(ptr, DPP_ATTR_ENCRYPTED_KEY,
+				n_data, dpp->key_len * 2);
+
+	iov[1].iov_base = attrs;
+	iov[1].iov_len = ptr - attrs;
+
+	dpp->state = DPP_STATE_PKEX_COMMIT_REVEAL;
+
+	dpp_pkex_property_changed_notify(dpp);
+
+	dpp_send_frame(dpp, iov, 2, dpp->current_freq);
+}
+
+static void dpp_process_pkex_exchange_request(struct dpp_sm *dpp,
+						struct l_ecc_point *m)
+{
+	_auto_(l_ecc_point_free) struct l_ecc_point *n = NULL;
+	_auto_(l_ecc_point_free) struct l_ecc_point *qr = NULL;
+	_auto_(l_ecc_point_free) struct l_ecc_point *qi = NULL;
+	_auto_(l_ecc_point_free) struct l_ecc_point *k = NULL;
+	const uint8_t *own_addr = netdev_get_address(dpp->netdev);
+
+	/* Qi = H(MAC-Initiator | [identifier | ] code) * Pi */
+	qi = dpp_derive_qi(dpp->curve, dpp->pkex_key, dpp->pkex_id,
+				dpp->peer_addr);
+	if (!qi) {
+		l_debug("could not derive Qi");
+		return;
+	}
+
+	/* X' = M - Qi */
+	dpp->y_or_x = l_ecc_point_new(dpp->curve);
+
+	l_ecc_point_inverse(qi);
+	l_ecc_point_add(dpp->y_or_x, m, qi);
+
+	/*
+	 * "The resulting ephemeral key, denoted X’, is checked whether it is
+	 * the point-at-infinity. If it is not valid, the protocol silently
+	 * fails"
+	 */
+	if (l_ecc_point_is_infinity(dpp->y_or_x)) {
+		l_debug("X' is at infinity, ignore message");
+		dpp_reset(dpp);
+		return;
+	}
+
+	qr = dpp_derive_qr(dpp->curve, dpp->pkex_key, dpp->pkex_id, own_addr);
+	if (!qr || l_ecc_point_is_infinity(qr)) {
+		l_debug("Qr did not derive");
+		l_ecc_point_free(dpp->y_or_x);
+		dpp->y_or_x = NULL;
+		goto bad_code;
+	}
+
+	/*
+	 * "The Responder then generates a random ephemeral keypair, y/Y,
+	 * encrypts Y with Qr to obtain the result, denoted N."
+	 */
+	l_ecdh_generate_key_pair(dpp->curve, &dpp->pkex_private,
+					&dpp->pkex_public);
+
+	/* N = Y + Qr */
+	n = l_ecc_point_new(dpp->curve);
+
+	l_ecc_point_add(n, dpp->pkex_public, qr);
+
+	/* K = y * X' */
+
+	k = l_ecc_point_new(dpp->curve);
+
+	l_ecc_point_multiply(k, dpp->pkex_private, dpp->y_or_x);
+
+	/* z = HKDF(<>, info | M.x | N.x | code, K.x) */
+	dpp_derive_z(dpp->peer_addr, own_addr, n, m, k, dpp->pkex_key,
+				dpp->pkex_id, dpp->z, &dpp->z_len);
+
+	dpp_send_pkex_exchange_response(dpp, n);
+
+	return;
+
+bad_code:
+	dpp_send_bad_code(dpp, dpp->peer_addr);
+	return;
+}
+
+static void dpp_handle_pkex_exchange_request(struct dpp_sm *dpp,
+					const uint8_t *from,
+					const uint8_t *body, size_t body_len)
+{
+	struct dpp_attr_iter iter;
+	enum dpp_attribute_type type;
+	size_t len;
+	const uint8_t *data;
+	uint8_t version = 0;
+	uint16_t group = 0;
+	const char *id = NULL;
+	size_t id_len = 0;
+	const void *key = NULL;
+	size_t key_len = 0;
+	_auto_(l_ecc_point_free) struct l_ecc_point *m = NULL;
+
+	l_debug("PKEX exchange request "MAC, MAC_STR(from));
+
+	if (dpp->state != DPP_STATE_PKEX_EXCHANGE)
+		return;
+
+	if (dpp->role != DPP_CAPABILITY_CONFIGURATOR)
+		return;
+
+	if (!l_memeqzero(dpp->peer_addr, 6)) {
+		l_debug("Already configuring enrollee, ignoring");
+		return;
+	}
+
+	dpp_attr_iter_init(&iter, body + 8, body_len - 8);
+
+	while (dpp_attr_iter_next(&iter, &type, &len, &data)) {
+		switch (type) {
+		case DPP_ATTR_PROTOCOL_VERSION:
+			if (len != 1)
+				return;
+
+			version = l_get_u8(data);
+			break;
+		case DPP_ATTR_FINITE_CYCLIC_GROUP:
+			if (len != 2)
+				return;
+
+			group = l_get_le16(data);
+			break;
+		case DPP_ATTR_CODE_IDENTIFIER:
+			id = (char *) data;
+			id_len = len;
+			break;
+		case DPP_ATTR_ENCRYPTED_KEY:
+			key = data;
+			key_len = len;
+			break;
+		default:
+			break;
+		}
+	}
+
+	if (!key || !group) {
+		l_debug("initiator did not provide group or key, ignoring");
+		return;
+	}
+
+	if (group != l_ecc_curve_get_ike_group(dpp->curve)) {
+		l_debug("initiator is not using the same group");
+		goto bad_group;
+	}
+
+	/*
+	 * If the group isn't the same the key length won't match, so check
+	 * this here after we've determined the groups are equal
+	 */
+	if (key_len != dpp->key_len * 2) {
+		l_debug("Unexpected encrypted key length");
+		return;
+	}
+
+	if (version && version != dpp->pkex_version) {
+		l_debug("initiator is not using the same version, ignoring");
+		return;
+	}
+
+	if (dpp->pkex_id) {
+		if (!id || !dpp_check_pkex_identifier(id) ||
+					id_len != strlen(dpp->pkex_id) ||
+					strncmp(dpp->pkex_id, id, id_len)) {
+			l_debug("mismatch identifier, ignoring");
+			return;
+		}
+	}
+
+	m = l_ecc_point_from_data(dpp->curve, L_ECC_POINT_TYPE_FULL,
+					key, key_len);
+	if (!m) {
+		l_debug("could not parse key from initiator, ignoring");
+		return;
+	}
+
+	memcpy(dpp->peer_addr, from, 6);
+
+	dpp_process_pkex_exchange_request(dpp, m);
+
+	return;
+
+bad_group:
+	dpp_send_bad_group(dpp, from);
+}
+
+static void dpp_send_commit_reveal_response(struct dpp_sm *dpp,
+						const uint8_t *v, size_t v_len)
+{
+	uint8_t hdr[32];
+	uint8_t attrs[256];
+	uint8_t *ptr = attrs;
+	uint8_t one = 1;
+	struct iovec iov[2];
+	const uint8_t *own_mac = netdev_get_address(dpp->netdev);
+	uint8_t b_pub[L_ECC_POINT_MAX_BYTES];
+	size_t b_len;
+
+	b_len = l_ecc_point_get_data(dpp->boot_public, b_pub, sizeof(b_pub));
+
+
+	iov[0].iov_len = dpp_build_header(own_mac, dpp->peer_addr,
+				DPP_FRAME_PKEX_COMMIT_REVEAL_RESPONSE, hdr);
+	iov[0].iov_base = hdr;
+
+	ptr += dpp_append_wrapped_data(hdr + 26, 6, &one, 1, ptr,
+			sizeof(attrs), dpp->z, dpp->z_len, 2,
+			DPP_ATTR_BOOTSTRAPPING_KEY, b_len, b_pub,
+			DPP_ATTR_RESPONDER_AUTH_TAG, v_len, v);
+
+	iov[1].iov_base = attrs;
+	iov[1].iov_len = ptr - attrs;
+
+	dpp_send_frame(dpp, iov, 2, dpp->current_freq);
+}
+
+static void dpp_handle_pkex_commit_reveal_request(struct dpp_sm *dpp,
+					const uint8_t *from,
+					const uint8_t *body, size_t body_len)
+{
+	struct dpp_attr_iter iter;
+	enum dpp_attribute_type type;
+	size_t len;
+	const uint8_t *data;
+	const void *wrapped = NULL;
+	size_t wrapped_len = 0;
+	_auto_(l_free) uint8_t *unwrapped = NULL;
+	size_t unwrapped_len;
+	uint8_t zero = 0;
+	const void *key = 0;
+	size_t key_len = 0;
+	const void *i_auth = NULL;
+	size_t i_auth_len = 0;
+	_auto_(l_ecc_point_free) struct l_ecc_point *j = NULL;
+	_auto_(l_ecc_point_free) struct l_ecc_point *l = NULL;
+	uint8_t u[L_ECC_SCALAR_MAX_BYTES];
+	size_t u_len = 0;
+	uint8_t v[L_ECC_SCALAR_MAX_BYTES];
+	size_t v_len = 0;
+	const uint8_t *own_addr = netdev_get_address(dpp->netdev);
+
+	l_debug("PKEX commit-reveal request "MAC, MAC_STR(from));
+
+	if (dpp->state != DPP_STATE_PKEX_COMMIT_REVEAL)
+		return;
+
+	if (dpp->role != DPP_CAPABILITY_CONFIGURATOR)
+		return;
+
+	dpp_attr_iter_init(&iter, body + 8, body_len - 8);
+
+	while (dpp_attr_iter_next(&iter, &type, &len, &data)) {
+		switch (type) {
+		case DPP_ATTR_WRAPPED_DATA:
+			wrapped = data;
+			wrapped_len = len;
+			break;
+		default:
+			break;
+		}
+	}
+
+	if (!wrapped) {
+		l_debug("No wrapped data");
+		return;
+	}
+
+	unwrapped = dpp_unwrap_attr(body + 2, 6, &zero, 1, dpp->z, dpp->z_len,
+					wrapped, wrapped_len, &unwrapped_len);
+	if (!unwrapped) {
+		l_debug("Failed to unwrap attributes");
+		return;
+	}
+
+	dpp_attr_iter_init(&iter, unwrapped, unwrapped_len);
+
+	while (dpp_attr_iter_next(&iter, &type, &len, &data)) {
+		switch (type) {
+		case DPP_ATTR_BOOTSTRAPPING_KEY:
+			if (len != dpp->key_len * 2)
+				return;
+
+			key = data;
+			key_len = len;
+			break;
+		case DPP_ATTR_INITIATOR_AUTH_TAG:
+			if (len != 32)
+				return;
+
+			i_auth = data;
+			i_auth_len = len;
+			break;
+		default:
+			break;
+		}
+	}
+
+	if (!key || !i_auth) {
+		l_debug("missing attributes");
+		return;
+	}
+
+	dpp->peer_boot_public = l_ecc_point_from_data(dpp->curve,
+					L_ECC_POINT_TYPE_FULL, key, key_len);
+	if (!dpp->peer_boot_public) {
+		l_debug("peers boostrapping key did not validate");
+		goto failed;
+	}
+
+	/* J' = y * A' */
+	j = l_ecc_point_new(dpp->curve);
+
+	l_ecc_point_multiply(j, dpp->pkex_private, dpp->peer_boot_public);
+
+	dpp_derive_u(j, dpp->peer_addr, dpp->peer_boot_public, dpp->pkex_public,
+			dpp->y_or_x, u, &u_len);
+
+	if (memcmp(u, i_auth, i_auth_len)) {
+		l_debug("Initiator auth tag did not verify");
+		goto failed;
+	}
+
+	/* L' = x * B' */
+	l = l_ecc_point_new(dpp->curve);
+
+	l_ecc_point_multiply(l, dpp->boot_private, dpp->y_or_x);
+
+	if (!dpp_derive_v(l, own_addr, dpp->boot_public, dpp->y_or_x,
+				dpp->pkex_public, v, &v_len)) {
+		l_debug("Failed to derive v");
+		goto failed;
+	}
+
+	dpp_send_commit_reveal_response(dpp, v, v_len);
+
+	dpp_pkex_start_authentication(dpp);
+
+	return;
+
+failed:
+	dpp_reset(dpp);
+}
+
 static void dpp_handle_frame(struct dpp_sm *dpp,
 				const struct mmpdu_header *frame,
 				const void *body, size_t body_len)
@@ -2788,6 +3219,14 @@ static void dpp_handle_frame(struct dpp_sm *dpp,
 		dpp_handle_pkex_commit_reveal_response(dpp, frame->address_2,
 							body, body_len);
 		break;
+	case DPP_FRAME_PKEX_VERSION1_XCHG_REQUST:
+		dpp_handle_pkex_exchange_request(dpp, frame->address_2, body,
+							body_len);
+		break;
+	case DPP_FRAME_PKEX_COMMIT_REVEAL_REQUEST:
+		dpp_handle_pkex_commit_reveal_request(dpp, frame->address_2,
+							body, body_len);
+		break;
 	default:
 		l_debug("Unhandled DPP frame %u", *ptr);
 		break;
@@ -3492,6 +3931,67 @@ invalid_args:
 	return dbus_error_invalid_args(message);
 }
 
+static struct l_dbus_message *dpp_start_pkex_configurator(struct dpp_sm *dpp,
+					const char *key, const char *identifier,
+					struct l_dbus_message *message)
+{
+	struct handshake_state *hs = netdev_get_handshake(dpp->netdev);
+	struct station *station = station_find(netdev_get_ifindex(dpp->netdev));
+	struct network *network = station_get_connected_network(station);
+	struct scan_bss *bss = station_get_connected_bss(station);
+	const struct l_settings *settings;
+
+	if (dpp->state != DPP_STATE_NOTHING)
+		return dbus_error_busy(message);
+
+	if (!network || !bss)
+		return dbus_error_not_connected(message);
+
+	settings = network_get_settings(network);
+	if (!settings) {
+		l_debug("No settings for network, is this a known network?");
+		return dbus_error_not_configured(message);
+	}
+
+	if (identifier)
+		dpp->pkex_id = l_strdup(identifier);
+
+	if (key)
+		dpp->pkex_key = l_strdup(key);
+
+	dpp->role = DPP_CAPABILITY_CONFIGURATOR;
+	dpp->state = DPP_STATE_PKEX_EXCHANGE;
+	dpp->current_freq = bss->frequency;
+	dpp->pkex_version = 1;
+	dpp->config = dpp_configuration_new(network_get_settings(network),
+						network_get_ssid(network),
+						hs->akm_suite);
+
+	dpp_reset_protocol_timer(dpp, DPP_PKEX_PROTO_TIMEOUT);
+	dpp_pkex_property_changed_notify(dpp);
+
+	l_debug("Starting PKEX configurator for single enrollee");
+
+	return l_dbus_message_new_method_return(message);
+}
+
+static struct l_dbus_message *dpp_dbus_pkex_configure_enrollee(
+						struct l_dbus *dbus,
+						struct l_dbus_message *message,
+						void *user_data)
+{
+	struct dpp_sm *dpp = user_data;
+	const char *key;
+	const char *id;
+
+	l_debug("");
+
+	if (!dpp_parse_pkex_args(message, &key, &id))
+		return dbus_error_invalid_args(message);
+
+	return dpp_start_pkex_configurator(dpp, key, id, message);
+}
+
 static void dpp_setup_interface(struct l_dbus_interface *interface)
 {
 	l_dbus_interface_method(interface, "StartEnrollee", 0,
@@ -3528,6 +4028,8 @@ static void dpp_setup_pkex_interface(struct l_dbus_interface *interface)
 			dpp_dbus_pkex_start_enrollee, "", "a{sv}", "args");
 	l_dbus_interface_method(interface, "Stop", 0,
 			dpp_dbus_pkex_stop, "", "");
+	l_dbus_interface_method(interface, "ConfigureEnrollee", 0,
+			dpp_dbus_pkex_configure_enrollee, "", "a{sv}", "args");
 
 	l_dbus_interface_property(interface, "Started", 0, "b",
 			dpp_pkex_get_started, NULL);
-- 
2.25.1

[PATCH v4 4/4] dpp: Add StartConfigurator, PKEX agent support

[James Prestwood]

Adds a configurator variant to be used along side an agent. When
called the configurator will start and wait for an initial PKEX
exchange message from an enrollee at which point it will request
the code from an agent. This provides more flexibility for
configurators that are capable of configuring multiple enrollees
with different identifiers/codes.

Note that the timing requirements per the DPP spec still apply
so this is not meant to be used with a human configurator but
within an automated agent which does a quick lookup of potential
identifiers/codes and can reply within the 200ms window.
---
 src/dpp.c | 228 +++++++++++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 226 insertions(+), 2 deletions(-)

diff --git a/src/dpp.c b/src/dpp.c
index 86361ba0..265fa971 100644
--- a/src/dpp.c
+++ b/src/dpp.c
@@ -82,6 +82,13 @@ enum dpp_capability {
 	DPP_CAPABILITY_CONFIGURATOR = 0x02,
 };
 
+struct pkex_agent {
+	char *owner;
+	char *path;
+	unsigned int disconnect_watch;
+	uint32_t pending_id;
+};
+
 struct dpp_sm {
 	struct netdev *netdev;
 	char *uri;
@@ -104,6 +111,8 @@ struct dpp_sm {
 
 	enum dpp_state state;
 
+	struct pkex_agent *agent;
+
 	/*
 	 * List of frequencies to jump between. The presence of this list is
 	 * also used to signify that a configurator is an initiator vs responder
@@ -154,6 +163,7 @@ struct dpp_sm {
 	char *pkex_id;
 	char *pkex_key;
 	uint8_t pkex_version;
+	struct l_ecc_point *peer_encr_key;
 	struct l_ecc_point *pkex_m;
 	/* Ephemeral key Y' or X' for enrollee or configurator */
 	struct l_ecc_point *y_or_x;
@@ -317,6 +327,68 @@ static void dpp_free_pending_pkex_data(struct dpp_sm *dpp)
 	}
 
 	memset(dpp->peer_addr, 0, sizeof(dpp->peer_addr));
+
+	if (dpp->peer_encr_key) {
+		l_ecc_point_free(dpp->peer_encr_key);
+		dpp->peer_encr_key = NULL;
+	}
+}
+
+static void pkex_agent_free(void *data)
+{
+	struct pkex_agent *agent = data;
+
+	l_free(agent->owner);
+	l_free(agent->path);
+	l_dbus_remove_watch(dbus_get_bus(), agent->disconnect_watch);
+	l_free(agent);
+}
+
+static void dpp_agent_cancel(struct dpp_sm *dpp)
+{
+	struct l_dbus_message *msg;
+
+	const char *reason = "shutdown";
+
+	msg = l_dbus_message_new_method_call(dbus_get_bus(),
+						dpp->agent->owner,
+						dpp->agent->path,
+						IWD_SHARED_CODE_AGENT_INTERFACE,
+						"Cancel");
+	l_dbus_message_set_arguments(msg, "s", reason);
+	l_dbus_message_set_no_reply(msg, true);
+	l_dbus_send(dbus_get_bus(), msg);
+}
+
+static void dpp_agent_release(struct dpp_sm *dpp)
+{
+	struct l_dbus_message *msg;
+
+	msg = l_dbus_message_new_method_call(dbus_get_bus(),
+						dpp->agent->owner,
+						dpp->agent->path,
+						IWD_SHARED_CODE_AGENT_INTERFACE,
+						"Release");
+	l_dbus_message_set_no_reply(msg, true);
+	l_dbus_send(dbus_get_bus(), msg);
+}
+
+static void dpp_destroy_agent(struct dpp_sm *dpp)
+{
+	if (!dpp->agent)
+		return;
+
+	if (dpp->agent->pending_id) {
+		dpp_agent_cancel(dpp);
+		l_dbus_cancel(dbus_get_bus(), dpp->agent->pending_id);
+	}
+
+	dpp_agent_release(dpp);
+
+	l_debug("Released SharedCodeAgent on path %s", dpp->agent->path);
+
+	pkex_agent_free(dpp->agent);
+	dpp->agent = NULL;
 }
 
 static void dpp_free_auth_data(struct dpp_sm *dpp)
@@ -415,6 +487,11 @@ static void dpp_reset(struct dpp_sm *dpp)
 		dpp->retry_timeout = NULL;
 	}
 
+	if (dpp->pkex_scan_id) {
+		scan_cancel(dpp->wdev_id, dpp->pkex_scan_id);
+		dpp->pkex_scan_id = 0;
+	}
+
 	dpp->state = DPP_STATE_NOTHING;
 	dpp->new_freq = 0;
 	dpp->frame_retry = 0;
@@ -431,6 +508,8 @@ static void dpp_reset(struct dpp_sm *dpp)
 	explicit_bzero(dpp->z, dpp->key_len);
 	explicit_bzero(dpp->u, dpp->u_len);
 
+	dpp_destroy_agent(dpp);
+
 	dpp_free_pending_pkex_data(dpp);
 
 	dpp_free_auth_data(dpp);
@@ -458,6 +537,11 @@ static void dpp_free(struct dpp_sm *dpp)
 		dpp->boot_private = NULL;
 	}
 
+	if (dpp->agent) {
+		pkex_agent_free(dpp->agent);
+		dpp->agent = NULL;
+	}
+
 	l_free(dpp);
 }
 
@@ -1800,6 +1884,18 @@ static void dpp_offchannel_timeout(int error, void *user_data)
 
 	switch (dpp->state) {
 	case DPP_STATE_PKEX_EXCHANGE:
+		if (dpp->role != DPP_CAPABILITY_CONFIGURATOR || !dpp->agent)
+			break;
+
+		/*
+		 * We have a pending agent request but it did not arrive in
+		 * time, we cant assume the enrollee will be waiting around
+		 * for our response so cancel the request and continue waiting
+		 * for another request
+		 */
+		if (dpp->agent->pending_id)
+			dpp_free_pending_pkex_data(dpp);
+		/* Fall through */
 	case DPP_STATE_PRESENCE:
 		break;
 	case DPP_STATE_NOTHING:
@@ -2912,6 +3008,63 @@ bad_code:
 	return;
 }
 
+static void dpp_pkex_agent_reply(struct l_dbus_message *message,
+					void *user_data)
+{
+	struct dpp_sm *dpp = user_data;
+	const char *error, *text;
+	const char *code;
+
+	dpp->agent->pending_id = 0;
+
+	l_debug("SharedCodeAgent %s path %s replied", dpp->agent->owner,
+			dpp->agent->path);
+
+	if (l_dbus_message_get_error(message, &error, &text)) {
+		l_error("RequestSharedCode(%s) returned %s(\"%s\")",
+				dpp->pkex_id, error, text);
+		goto reset;
+	}
+
+	if (!l_dbus_message_get_arguments(message, "s", &code)) {
+		l_debug("Invalid arguments, check SharedCodeAgent!");
+		goto reset;
+	}
+
+	dpp->pkex_key = l_strdup(code);
+	dpp_process_pkex_exchange_request(dpp, dpp->peer_encr_key);
+
+	return;
+
+reset:
+	dpp_free_pending_pkex_data(dpp);
+}
+
+static bool dpp_pkex_agent_request(struct dpp_sm *dpp)
+{
+	struct l_dbus_message *msg;
+
+	if (!dpp->agent)
+		return false;
+
+	if (L_WARN_ON(dpp->agent->pending_id))
+		return false;
+
+	msg = l_dbus_message_new_method_call(dbus_get_bus(),
+						dpp->agent->owner,
+						dpp->agent->path,
+						IWD_SHARED_CODE_AGENT_INTERFACE,
+						"RequestSharedCode");
+	l_dbus_message_set_arguments(msg, "s", dpp->pkex_id);
+
+
+	dpp->agent->pending_id = l_dbus_send_with_reply(dbus_get_bus(),
+							msg,
+							dpp_pkex_agent_reply,
+							dpp, NULL);
+	return dpp->agent->pending_id != 0;
+}
+
 static void dpp_handle_pkex_exchange_request(struct dpp_sm *dpp,
 					const uint8_t *from,
 					const uint8_t *body, size_t body_len)
@@ -3012,6 +3165,29 @@ static void dpp_handle_pkex_exchange_request(struct dpp_sm *dpp,
 
 	memcpy(dpp->peer_addr, from, 6);
 
+	if (!dpp->pkex_key) {
+		if (!id) {
+			l_debug("Configurator started with agent but enrollee "
+				"sent no identifier, ignoring");
+			return;
+		}
+
+		dpp->pkex_id = l_strndup(id, id_len);
+
+		/* Need to obtain code from agent */
+		if (!dpp_pkex_agent_request(dpp)) {
+			l_debug("Failed to request code from agent!");
+			dpp_free_pending_pkex_data(dpp);
+			return;
+		}
+
+		/* Save the encrypted key/identifier for the agent callback */
+
+		dpp->peer_encr_key = l_steal_ptr(m);
+
+		return;
+	}
+
 	dpp_process_pkex_exchange_request(dpp, m);
 
 	return;
@@ -3931,8 +4107,34 @@ invalid_args:
 	return dbus_error_invalid_args(message);
 }
 
+static void pkex_agent_disconnect(struct l_dbus *dbus, void *user_data)
+{
+	struct dpp_sm *dpp = user_data;
+
+	l_debug("SharedCodeAgent %s disconnected", dpp->agent->path);
+
+	dpp_reset(dpp);
+}
+
+static void dpp_create_agent(struct dpp_sm *dpp, const char *path,
+					struct l_dbus_message *message)
+{
+	const char *sender = l_dbus_message_get_sender(message);
+
+	dpp->agent = l_new(struct pkex_agent, 1);
+	dpp->agent->owner = l_strdup(sender);
+	dpp->agent->path = l_strdup(path);
+	dpp->agent->disconnect_watch = l_dbus_add_disconnect_watch(dbus_get_bus(),
+							sender,
+							pkex_agent_disconnect,
+							dpp, NULL);
+
+	l_debug("Registered a SharedCodeAgent on path %s", path);
+}
+
 static struct l_dbus_message *dpp_start_pkex_configurator(struct dpp_sm *dpp,
 					const char *key, const char *identifier,
+					const char *agent_path,
 					struct l_dbus_message *message)
 {
 	struct handshake_state *hs = netdev_get_handshake(dpp->netdev);
@@ -3959,6 +4161,9 @@ static struct l_dbus_message *dpp_start_pkex_configurator(struct dpp_sm *dpp,
 	if (key)
 		dpp->pkex_key = l_strdup(key);
 
+	if (agent_path)
+		dpp_create_agent(dpp, agent_path, message);
+
 	dpp->role = DPP_CAPABILITY_CONFIGURATOR;
 	dpp->state = DPP_STATE_PKEX_EXCHANGE;
 	dpp->current_freq = bss->frequency;
@@ -3970,7 +4175,10 @@ static struct l_dbus_message *dpp_start_pkex_configurator(struct dpp_sm *dpp,
 	dpp_reset_protocol_timer(dpp, DPP_PKEX_PROTO_TIMEOUT);
 	dpp_pkex_property_changed_notify(dpp);
 
-	l_debug("Starting PKEX configurator for single enrollee");
+	if (dpp->pkex_key)
+		l_debug("Starting PKEX configurator for single enrollee");
+	else
+		l_debug("Starting PKEX configurator with agent");
 
 	return l_dbus_message_new_method_return(message);
 }
@@ -3989,7 +4197,21 @@ static struct l_dbus_message *dpp_dbus_pkex_configure_enrollee(
 	if (!dpp_parse_pkex_args(message, &key, &id))
 		return dbus_error_invalid_args(message);
 
-	return dpp_start_pkex_configurator(dpp, key, id, message);
+	return dpp_start_pkex_configurator(dpp, key, id, NULL, message);
+}
+
+static struct l_dbus_message *dpp_dbus_pkex_start_configurator(
+						struct l_dbus *dbus,
+						struct l_dbus_message *message,
+						void *user_data)
+{
+	struct dpp_sm *dpp = user_data;
+	const char *path;
+
+	if (!l_dbus_message_get_arguments(message, "o", &path))
+		return dbus_error_invalid_args(message);
+
+	return dpp_start_pkex_configurator(dpp, NULL, NULL, path, message);
 }
 
 static void dpp_setup_interface(struct l_dbus_interface *interface)
@@ -4030,6 +4252,8 @@ static void dpp_setup_pkex_interface(struct l_dbus_interface *interface)
 			dpp_dbus_pkex_stop, "", "");
 	l_dbus_interface_method(interface, "ConfigureEnrollee", 0,
 			dpp_dbus_pkex_configure_enrollee, "", "a{sv}", "args");
+	l_dbus_interface_method(interface, "StartConfigurator", 0,
+			dpp_dbus_pkex_start_configurator, "", "o", "path");
 
 	l_dbus_interface_property(interface, "Started", 0, "b",
 			dpp_pkex_get_started, NULL);
-- 
2.25.1