iwd.lists.linux.dev archive mirror
 help / color / mirror / Atom feed
* [PATCH 00/11] Basic SAE support for AP mode
@ 2024-04-21 12:50 John Brandt
  2024-04-21 12:50 ` [PATCH 01/11] ap: ability to advertise PSK and SAE John Brandt
                   ` (11 more replies)
  0 siblings, 12 replies; 23+ messages in thread
From: John Brandt @ 2024-04-21 12:50 UTC (permalink / raw)
  To: iwd; +Cc: John Brandt

This set of patches adds basic SAE support for IWD in AP mode. It has
been tested by connecting to IWD AP using wpa_supplicant. Note that this
does not yet correspond to WPA3, since WPA3 would also require the
support of Management Frame Protection.

Normal client functionality has also been confirmed to still work. After
applying these patches it remains possible for IWD client to connect to
WPA3/SAE network.

Remaining TODOs are to include better sanity-checking of received
frames.

John Brandt (11):
  ap: ability to advertise PSK and SAE
  ap: accept PSK/SAE in auth depending on config
  sae: add function sae_set_group
  sae: refactor and add function sae_calculate_keys
  sae: make sae_process_commit callable in AP mode
  sae: verify offered group in AP mode
  sae: support reception of Confirm frame by AP
  ap: add support to handle SAE authentication
  ap: enable start of 4-way HS after SAE
  eapol: support PTK derivation with SHA256
  eapol: encrypt key data for AKM-defined ciphers

 src/ap.c    | 135 +++++++++++++++++++++++++++++++++-------
 src/eapol.c |  58 ++++++++++++-----
 src/sae.c   | 175 +++++++++++++++++++++++++++++++++-------------------
 3 files changed, 265 insertions(+), 103 deletions(-)

-- 
2.44.0


^ permalink raw reply	[flat|nested] 23+ messages in thread

* [PATCH 01/11] ap: ability to advertise PSK and SAE
  2024-04-21 12:50 [PATCH 00/11] Basic SAE support for AP mode John Brandt
@ 2024-04-21 12:50 ` John Brandt
  2024-04-21 12:50 ` [PATCH 02/11] ap: accept PSK/SAE in auth depending on config John Brandt
                   ` (10 subsequent siblings)
  11 siblings, 0 replies; 23+ messages in thread
From: John Brandt @ 2024-04-21 12:50 UTC (permalink / raw)
  To: iwd; +Cc: John Brandt

Add the configuration option AKMSuites under Security so it becomes
possible to support both PSK and SAE. This influences the advertised
AKMs in the beacon.
---
 src/ap.c | 26 +++++++++++++++++++++++++-
 1 file changed, 25 insertions(+), 1 deletion(-)

diff --git a/src/ap.c b/src/ap.c
index b4e7593e..d50f9e4f 100644
--- a/src/ap.c
+++ b/src/ap.c
@@ -80,6 +80,7 @@ struct ap_state {
 
 	unsigned int ciphers;
 	enum ie_rsn_cipher_suite group_cipher;
+	unsigned int akm_suites;
 	uint32_t beacon_interval;
 	struct l_uintset *rates;
 	uint32_t start_stop_cmd_id;
@@ -631,7 +632,7 @@ static void ap_drop_rsna(struct sta_state *sta)
 static void ap_set_rsn_info(struct ap_state *ap, struct ie_rsn_info *rsn)
 {
 	memset(rsn, 0, sizeof(*rsn));
-	rsn->akm_suites = IE_RSN_AKM_SUITE_PSK;
+	rsn->akm_suites = ap->akm_suites;
 	rsn->pairwise_ciphers = ap->ciphers;
 	rsn->group_cipher = ap->group_cipher;
 }
@@ -3620,6 +3621,7 @@ static int ap_load_config(struct ap_state *ap, const struct l_settings *config,
 	size_t len;
 	L_AUTO_FREE_VAR(char *, strval) = NULL;
 	_auto_(l_strv_free) char **ciphers_str = NULL;
+	_auto_(l_strv_free) char **akms_str = NULL;
 	uint16_t cipher_mask;
 	int err;
 	int i;
@@ -3838,6 +3840,28 @@ static int ap_load_config(struct ap_state *ap, const struct l_settings *config,
 		ap->ciphers |= cipher;
 	}
 
+	akms_str = l_settings_get_string_list(config, "Security",
+						"AKMSuites", ',');
+	for (i = 0; akms_str && akms_str[i]; i++) {
+		if (!strcmp(akms_str[i], "PSK"))
+			ap->akm_suites |= IE_RSN_AKM_SUITE_PSK;
+		else if (!strcmp(akms_str[i], "SAE"))
+			ap->akm_suites |= IE_RSN_AKM_SUITE_SAE_SHA256;
+		else {
+			l_warn("Unsupported or unknown AKM suite %s",
+					akms_str[i]);
+			return -ENOTSUP;
+		}
+	}
+
+	if (ap->akm_suites == 0) {
+		/*
+		 * Default behavior if no AKMs are specified but a passphrase
+		 * is to only enable PSK == WPA2.
+		 */
+		 ap->akm_suites |= IE_RSN_AKM_SUITE_PSK;
+	}
+
 	if (!ap->ciphers) {
 		/*
 		 * Default behavior if no ciphers are specified, disable TKIP
-- 
2.44.0


^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [PATCH 02/11] ap: accept PSK/SAE in auth depending on config
  2024-04-21 12:50 [PATCH 00/11] Basic SAE support for AP mode John Brandt
  2024-04-21 12:50 ` [PATCH 01/11] ap: ability to advertise PSK and SAE John Brandt
@ 2024-04-21 12:50 ` John Brandt
  2024-04-24 12:05   ` James Prestwood
  2024-04-21 12:50 ` [PATCH 03/11] sae: add function sae_set_group John Brandt
                   ` (9 subsequent siblings)
  11 siblings, 1 reply; 23+ messages in thread
From: John Brandt @ 2024-04-21 12:50 UTC (permalink / raw)
  To: iwd; +Cc: John Brandt

On reception of an authentication frame, accept both PSK and SAE as AKM
depending on the config. Save the client's AKM for later use.
---
 src/ap.c | 28 +++++++++++++++++++++-------
 1 file changed, 21 insertions(+), 7 deletions(-)

diff --git a/src/ap.c b/src/ap.c
index d50f9e4f..14268551 100644
--- a/src/ap.c
+++ b/src/ap.c
@@ -132,6 +132,7 @@ struct sta_state {
 	uint8_t *assoc_ies;
 	size_t assoc_ies_len;
 	uint8_t *assoc_rsne;
+	enum ie_rsn_akm_suite akm_suite;
 	struct eapol_sm *sm;
 	struct handshake_state *hs;
 	uint32_t gtk_query_cmd_id;
@@ -2606,6 +2607,7 @@ static void ap_auth_cb(const struct mmpdu_header *hdr, const void *body,
 	const uint8_t *from = hdr->address_2;
 	const uint8_t *bssid = netdev_get_address(ap->netdev);
 	struct sta_state *sta;
+	enum ie_rsn_akm_suite akm_suite;
 
 	l_info("AP Authentication from %s", util_address_to_string(from));
 
@@ -2627,17 +2629,27 @@ static void ap_auth_cb(const struct mmpdu_header *hdr, const void *body,
 		}
 	}
 
-	/* Only Open System authentication implemented here */
-	if (L_LE16_TO_CPU(auth->algorithm) !=
-			MMPDU_AUTH_ALGO_OPEN_SYSTEM) {
+	if ((ap->akm_suites & IE_RSN_AKM_SUITE_SAE_SHA256) &&
+	    (L_LE16_TO_CPU(auth->algorithm) == MMPDU_AUTH_ALGO_SAE) ) {
+		/* When using SAE it must be COMMIT or CONFIRM frame */
+		if (L_LE16_TO_CPU(auth->transaction_sequence) > 2) {
+			ap_auth_reply(ap, from, MMPDU_REASON_CODE_UNSPECIFIED);
+			return;
+		}
+		akm_suite = IE_RSN_AKM_SUITE_SAE_SHA256;
+	} else if ((ap->akm_suites & IE_RSN_AKM_SUITE_PSK) &&
+		   (L_LE16_TO_CPU(auth->algorithm) == MMPDU_AUTH_ALGO_OPEN_SYSTEM) ) {
+		/* When using PSK it must be Open System authentication */
+		if (L_LE16_TO_CPU(auth->transaction_sequence) != 1) {
+			ap_auth_reply(ap, from, MMPDU_REASON_CODE_UNSPECIFIED);
+			return;
+		}
+		akm_suite = IE_RSN_AKM_SUITE_PSK;
+	} else {
 		ap_auth_reply(ap, from, MMPDU_REASON_CODE_UNSPECIFIED);
 		return;
 	}
 
-	if (L_LE16_TO_CPU(auth->transaction_sequence) != 1) {
-		ap_auth_reply(ap, from, MMPDU_REASON_CODE_UNSPECIFIED);
-		return;
-	}
 
 	sta = l_queue_find(ap->sta_states, ap_sta_match_addr, from);
 
@@ -2666,6 +2678,8 @@ static void ap_auth_cb(const struct mmpdu_header *hdr, const void *body,
 	if (!ap->sta_states)
 		ap->sta_states = l_queue_new();
 
+	sta->akm_suite = akm_suite;
+
 	l_queue_push_tail(ap->sta_states, sta);
 
 	/*
-- 
2.44.0


^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [PATCH 03/11] sae: add function sae_set_group
  2024-04-21 12:50 [PATCH 00/11] Basic SAE support for AP mode John Brandt
  2024-04-21 12:50 ` [PATCH 01/11] ap: ability to advertise PSK and SAE John Brandt
  2024-04-21 12:50 ` [PATCH 02/11] ap: accept PSK/SAE in auth depending on config John Brandt
@ 2024-04-21 12:50 ` John Brandt
  2024-04-24 12:05   ` James Prestwood
  2024-04-21 12:50 ` [PATCH 04/11] sae: refactor and add function sae_calculate_keys John Brandt
                   ` (8 subsequent siblings)
  11 siblings, 1 reply; 23+ messages in thread
From: John Brandt @ 2024-04-21 12:50 UTC (permalink / raw)
  To: iwd; +Cc: John Brandt

Refactor code by adding function sae_set_group. This will make the next
commits easier where basic SAE support for APs is added.
---
 src/sae.c | 23 ++++++++++++-----------
 1 file changed, 12 insertions(+), 11 deletions(-)

diff --git a/src/sae.c b/src/sae.c
index bf9fb0ff..c133386f 100644
--- a/src/sae.c
+++ b/src/sae.c
@@ -148,6 +148,16 @@ static void sae_reset_state(struct sae_sm *sm)
 	sm->pwe = NULL;
 }
 
+static int sae_set_group(struct sae_sm *sm, int group)
+{
+	sae_debug("Using group %u", group);
+
+	sm->group = group;
+	sm->curve = l_ecc_curve_from_ike_group(group);
+
+	return 0;
+}
+
 static int sae_choose_next_group(struct sae_sm *sm)
 {
 	const unsigned int *ecc_groups = l_ecc_supported_ike_groups();
@@ -166,9 +176,7 @@ static int sae_choose_next_group(struct sae_sm *sm)
 		sae_debug("Forcing default SAE group 19");
 
 		sm->group_retry++;
-		sm->group = 19;
-
-		goto get_curve;
+		return sae_set_group(sm, 19);
 	}
 
 	do {
@@ -182,14 +190,7 @@ static int sae_choose_next_group(struct sae_sm *sm)
 	if (reset)
 		sae_reset_state(sm);
 
-	sm->group = ecc_groups[sm->group_retry];
-
-get_curve:
-	sae_debug("Using group %u", sm->group);
-
-	sm->curve = l_ecc_curve_from_ike_group(sm->group);
-
-	return 0;
+	return sae_set_group(sm, ecc_groups[sm->group_retry]);
 }
 
 static int sae_valid_group(struct sae_sm *sm, unsigned int group)
-- 
2.44.0


^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [PATCH 04/11] sae: refactor and add function sae_calculate_keys
  2024-04-21 12:50 [PATCH 00/11] Basic SAE support for AP mode John Brandt
                   ` (2 preceding siblings ...)
  2024-04-21 12:50 ` [PATCH 03/11] sae: add function sae_set_group John Brandt
@ 2024-04-21 12:50 ` John Brandt
  2024-04-24 12:06   ` James Prestwood
  2024-04-21 12:50 ` [PATCH 05/11] sae: make sae_process_commit callable in AP mode John Brandt
                   ` (7 subsequent siblings)
  11 siblings, 1 reply; 23+ messages in thread
From: John Brandt @ 2024-04-21 12:50 UTC (permalink / raw)
  To: iwd; +Cc: John Brandt

Refactor code by moving code to the new function sae_calculate_keys.
This will make it easier in the next commits to add SAE support for AP
mode.
---
 src/sae.c | 83 +++++++++++++++++++++++++++++++------------------------
 1 file changed, 47 insertions(+), 36 deletions(-)

diff --git a/src/sae.c b/src/sae.c
index c133386f..314fc28f 100644
--- a/src/sae.c
+++ b/src/sae.c
@@ -683,10 +683,9 @@ static bool sae_send_confirm(struct sae_sm *sm)
 	return true;
 }
 
-static int sae_process_commit(struct sae_sm *sm, const uint8_t *from,
-					const uint8_t *frame, size_t len)
+
+static int sae_calculate_keys(struct sae_sm *sm)
 {
-	uint8_t *ptr = (uint8_t *) frame;
 	unsigned int nbytes = l_ecc_curve_get_scalar_bytes(sm->curve);
 	enum l_checksum_type hash =
 		crypto_sae_hash_from_ecc_prime_len(sm->sae_type, nbytes);
@@ -702,39 +701,6 @@ static int sae_process_commit(struct sae_sm *sm, const uint8_t *from,
 	struct l_ecc_scalar *tmp_scalar;
 	struct l_ecc_scalar *order;
 
-	ptr += 2;
-
-	sm->p_scalar = l_ecc_scalar_new(sm->curve, ptr, nbytes);
-	if (!sm->p_scalar) {
-		l_error("Server sent invalid P_Scalar during commit");
-		return sae_reject(sm, SAE_STATE_COMMITTED,
-				MMPDU_STATUS_CODE_UNSUPP_FINITE_CYCLIC_GROUP);
-	}
-
-	ptr += nbytes;
-
-	sm->p_element = l_ecc_point_from_data(sm->curve, L_ECC_POINT_TYPE_FULL,
-						ptr, nbytes * 2);
-	if (!sm->p_element) {
-		l_error("Server sent invalid P_Element during commit");
-		return sae_reject(sm, SAE_STATE_COMMITTED,
-				MMPDU_STATUS_CODE_UNSUPP_FINITE_CYCLIC_GROUP);
-	}
-
-	/*
-	 * If they match those sent as part of the protocol instance's own
-	 * SAE Commit message, the frame shall be silently discarded (because
-	 * it is evidence of a reflection attack) and the t0 (retransmission)
-	 * timer shall be set.
-	 */
-	if (l_ecc_scalars_are_equal(sm->p_scalar, sm->scalar) ||
-			l_ecc_points_are_equal(sm->p_element, sm->element)) {
-		l_warn("peer scalar or element matched own, discarding frame");
-		return -ENOMSG;
-	}
-
-	sm->sc++;
-
 	/*
 	 * K = scalar-op(rand, (element-op(scalar-op(peer-commit-scalar, PWE),
 	 *			PEER-COMMIT-ELEMENT)))
@@ -823,6 +789,51 @@ static int sae_process_commit(struct sae_sm *sm, const uint8_t *from,
 	/* don't set the handshakes pmkid until confirm is verified */
 	memcpy(sm->pmkid, tmp, 16);
 
+	return 0;
+}
+
+
+static int sae_process_commit(struct sae_sm *sm, const uint8_t *from,
+					const uint8_t *frame, size_t len)
+{
+	uint8_t *ptr = (uint8_t *) frame;
+	unsigned int nbytes = l_ecc_curve_get_scalar_bytes(sm->curve);
+
+	ptr += 2;
+
+	sm->p_scalar = l_ecc_scalar_new(sm->curve, ptr, nbytes);
+	if (!sm->p_scalar) {
+		l_error("Server sent invalid P_Scalar during commit");
+		return sae_reject(sm, SAE_STATE_COMMITTED,
+				MMPDU_STATUS_CODE_UNSUPP_FINITE_CYCLIC_GROUP);
+	}
+
+	ptr += nbytes;
+
+	sm->p_element = l_ecc_point_from_data(sm->curve, L_ECC_POINT_TYPE_FULL,
+						ptr, nbytes * 2);
+	if (!sm->p_element) {
+		l_error("Server sent invalid P_Element during commit");
+		return sae_reject(sm, SAE_STATE_COMMITTED,
+				MMPDU_STATUS_CODE_UNSUPP_FINITE_CYCLIC_GROUP);
+	}
+
+	/*
+	 * If they match those sent as part of the protocol instance's own
+	 * SAE Commit message, the frame shall be silently discarded (because
+	 * it is evidence of a reflection attack) and the t0 (retransmission)
+	 * timer shall be set.
+	 */
+	if (l_ecc_scalars_are_equal(sm->p_scalar, sm->scalar) ||
+			l_ecc_points_are_equal(sm->p_element, sm->element)) {
+		l_warn("peer scalar or element matched own, discarding frame");
+		return -ENOMSG;
+	}
+
+	sm->sc++;
+
+	sae_calculate_keys(sm);
+
 	if (!sae_send_confirm(sm))
 		return -EPROTO;
 
-- 
2.44.0


^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [PATCH 05/11] sae: make sae_process_commit callable in AP mode
  2024-04-21 12:50 [PATCH 00/11] Basic SAE support for AP mode John Brandt
                   ` (3 preceding siblings ...)
  2024-04-21 12:50 ` [PATCH 04/11] sae: refactor and add function sae_calculate_keys John Brandt
@ 2024-04-21 12:50 ` John Brandt
  2024-04-24 12:08   ` James Prestwood
  2024-04-21 12:50 ` [PATCH 06/11] sae: verify offered group " John Brandt
                   ` (6 subsequent siblings)
  11 siblings, 1 reply; 23+ messages in thread
From: John Brandt @ 2024-04-21 12:50 UTC (permalink / raw)
  To: iwd; +Cc: John Brandt

As an AP, the function sae_process_commit will pick the group offered by
the client. In a subsuquent commit the offered group will first be
verified before calling sae_process_commit. The AP will reply with a
Commit frame, calculate current keys, and move to the COMMITTED state.
---
 src/sae.c | 32 ++++++++++++++++++++------------
 1 file changed, 20 insertions(+), 12 deletions(-)

diff --git a/src/sae.c b/src/sae.c
index 314fc28f..2c97d94c 100644
--- a/src/sae.c
+++ b/src/sae.c
@@ -48,6 +48,8 @@ static bool debug;
 #define SAE_SYNC_MAX		3
 #define SAE_MAX_ASSOC_RETRY	3
 
+static bool sae_send_commit(struct sae_sm *sm, bool retry);
+
 #define sae_debug(fmat, ...) \
 ({	\
 	if (debug) \
@@ -797,8 +799,12 @@ static int sae_process_commit(struct sae_sm *sm, const uint8_t *from,
 					const uint8_t *frame, size_t len)
 {
 	uint8_t *ptr = (uint8_t *) frame;
-	unsigned int nbytes = l_ecc_curve_get_scalar_bytes(sm->curve);
+	unsigned int nbytes;
+
+	if (sm->handshake->authenticator && sae_set_group(sm, l_get_le16(frame)) < 0)
+		return -1;
 
+	nbytes = l_ecc_curve_get_scalar_bytes(sm->curve);
 	ptr += 2;
 
 	sm->p_scalar = l_ecc_scalar_new(sm->curve, ptr, nbytes);
@@ -824,20 +830,22 @@ static int sae_process_commit(struct sae_sm *sm, const uint8_t *from,
 	 * it is evidence of a reflection attack) and the t0 (retransmission)
 	 * timer shall be set.
 	 */
-	if (l_ecc_scalars_are_equal(sm->p_scalar, sm->scalar) ||
-			l_ecc_points_are_equal(sm->p_element, sm->element)) {
-		l_warn("peer scalar or element matched own, discarding frame");
+	if ((sm->scalar && l_ecc_scalars_are_equal(sm->p_scalar, sm->scalar)) ||
+			(sm->element && l_ecc_points_are_equal(sm->p_element, sm->element)))
 		return -ENOMSG;
-	}
 
 	sm->sc++;
 
-	sae_calculate_keys(sm);
-
-	if (!sae_send_confirm(sm))
-		return -EPROTO;
-
-	sm->state = SAE_STATE_CONFIRMED;
+	if (sm->handshake->authenticator) {
+		sae_send_commit(sm, false);
+		sae_calculate_keys(sm);
+		sm->state = SAE_STATE_COMMITTED;
+	} else {
+		sae_calculate_keys(sm);
+		if (!sae_send_confirm(sm))
+			return -EPROTO;
+		sm->state = SAE_STATE_CONFIRMED;
+	}
 
 	return 0;
 }
@@ -1008,7 +1016,7 @@ static int sae_verify_nothing(struct sae_sm *sm, uint16_t transaction,
 	/*
 	 * TODO: This does not handle the transition from NOTHING -> CONFIRMED
 	 * as this is only relevant to the AP or in Mesh mode which is not
-	 * yet supported.
+	 * yet fully supported.
 	 */
 	if (transaction != SAE_STATE_COMMITTED)
 		return -EBADMSG;
-- 
2.44.0


^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [PATCH 06/11] sae: verify offered group in AP mode
  2024-04-21 12:50 [PATCH 00/11] Basic SAE support for AP mode John Brandt
                   ` (4 preceding siblings ...)
  2024-04-21 12:50 ` [PATCH 05/11] sae: make sae_process_commit callable in AP mode John Brandt
@ 2024-04-21 12:50 ` John Brandt
  2024-04-21 12:50 ` [PATCH 07/11] sae: support reception of Confirm frame by AP John Brandt
                   ` (5 subsequent siblings)
  11 siblings, 0 replies; 23+ messages in thread
From: John Brandt @ 2024-04-21 12:50 UTC (permalink / raw)
  To: iwd; +Cc: John Brandt

When receiving a Commit frame in AP mode, first verify that we support
the offered group before further processing the frame.
---
 src/sae.c | 15 ++++++++++++++-
 1 file changed, 14 insertions(+), 1 deletion(-)

diff --git a/src/sae.c b/src/sae.c
index 2c97d94c..8a1e311a 100644
--- a/src/sae.c
+++ b/src/sae.c
@@ -214,6 +214,18 @@ static int sae_valid_group(struct sae_sm *sm, unsigned int group)
 	return -ENOENT;
 }
 
+static int sae_supported_group(struct sae_sm *sm, unsigned int group)
+{
+	const unsigned int *ecc_groups = l_ecc_supported_ike_groups();
+	unsigned int i;
+
+	for (i = 0; ecc_groups[i]; i++)
+		if (ecc_groups[i] == group)
+			return true;
+
+	return false;
+}
+
 static bool sae_pwd_seed(const uint8_t *addr1, const uint8_t *addr2,
 				uint8_t *base, size_t base_len,
 				uint8_t counter, uint8_t *out)
@@ -1029,7 +1041,8 @@ static int sae_verify_nothing(struct sae_sm *sm, uint16_t transaction,
 		return -EBADMSG;
 
 	/* reject with unsupported group */
-	if (l_get_le16(frame) != sm->group)
+	if ((sm->handshake->authenticator && sae_supported_group(sm, l_get_le16(frame)) < 0) ||
+	    (!sm->handshake->authenticator && l_get_le16(frame) != sm->group))
 		return sae_reject(sm, SAE_STATE_COMMITTED,
 				MMPDU_STATUS_CODE_UNSUPP_FINITE_CYCLIC_GROUP);
 
-- 
2.44.0


^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [PATCH 07/11] sae: support reception of Confirm frame by AP
  2024-04-21 12:50 [PATCH 00/11] Basic SAE support for AP mode John Brandt
                   ` (5 preceding siblings ...)
  2024-04-21 12:50 ` [PATCH 06/11] sae: verify offered group " John Brandt
@ 2024-04-21 12:50 ` John Brandt
  2024-04-24 12:08   ` James Prestwood
  2024-04-21 12:50 ` [PATCH 08/11] ap: add support to handle SAE authentication John Brandt
                   ` (4 subsequent siblings)
  11 siblings, 1 reply; 23+ messages in thread
From: John Brandt @ 2024-04-21 12:50 UTC (permalink / raw)
  To: iwd; +Cc: John Brandt

Experimental AP-mode support for receiving a Confirm frame when in the
COMMITTED state. The AP will reply with a Confirm frame.

Note that when acting as an AP, on reception of a Commit frame, the AP
only replies with a Commit frame. The protocols allows to also already
send the Confirm frame, but older clients may not support simultaneously
receiving a Commit and Confirm frame.
---
 src/sae.c | 38 +++++++++++++++++++++++++-------------
 1 file changed, 25 insertions(+), 13 deletions(-)

diff --git a/src/sae.c b/src/sae.c
index 8a1e311a..da55c764 100644
--- a/src/sae.c
+++ b/src/sae.c
@@ -906,9 +906,13 @@ static int sae_process_confirm(struct sae_sm *sm, const uint8_t *from,
 
 	sm->state = SAE_STATE_ACCEPTED;
 
-	sae_debug("Sending Associate to "MAC, MAC_STR(sm->handshake->aa));
-
-	sm->tx_assoc(sm->user_data);
+	if (!sm->handshake->authenticator) {
+		sae_debug("Sending Associate to "MAC, MAC_STR(sm->handshake->aa));
+		sm->tx_assoc(sm->user_data);
+	} else {
+		if (!sae_send_confirm(sm))
+			return -EPROTO;
+	}
 
 	return 0;
 }
@@ -1059,16 +1063,24 @@ static int sae_verify_committed(struct sae_sm *sm, uint16_t transaction,
 	unsigned int skip;
 	struct ie_tlv_iter iter;
 
-	/*
-	 * Upon receipt of a Con event...
-	 * Then the protocol instance checks the value of Sync. If it
-	 * is greater than dot11RSNASAESync, the protocol instance shall send a
-	 * Del event to the parent process and transition back to Nothing state.
-	 * If Sync is not greater than dot11RSNASAESync, the protocol instance
-	 * shall increment Sync, transmit the last SAE Commit message sent to
-	 * the peer...
-	 */
-	if (transaction == SAE_STATE_CONFIRMED) {
+	if (sm->handshake->authenticator && transaction == SAE_STATE_CONFIRMED) {
+		/*
+		 * TODO: Sanity-check received Confirm frame from the client. For now
+		 * AP-mode SAE support is experimental and we simply accept the frame.
+		 * Note that the cryptographic confirm field value will still be checked
+		 * before replying with a Confirm frame.
+		 */
+		return 0;
+	} else if (transaction == SAE_STATE_CONFIRMED) {
+		/*
+		 * Upon receipt of a Con event...
+		 * Then the protocol instance checks the value of Sync. If it
+		 * is greater than dot11RSNASAESync, the protocol instance shall send a
+		 * Del event to the parent process and transition back to Nothing state.
+		 * If Sync is not greater than dot11RSNASAESync, the protocol instance
+		 * shall increment Sync, transmit the last SAE Commit message sent to
+		 * the peer...
+		 */
 		if (sm->sync > SAE_SYNC_MAX)
 			return -ETIMEDOUT;
 
-- 
2.44.0


^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [PATCH 08/11] ap: add support to handle SAE authentication
  2024-04-21 12:50 [PATCH 00/11] Basic SAE support for AP mode John Brandt
                   ` (6 preceding siblings ...)
  2024-04-21 12:50 ` [PATCH 07/11] sae: support reception of Confirm frame by AP John Brandt
@ 2024-04-21 12:50 ` John Brandt
  2024-04-24 12:06   ` James Prestwood
  2024-04-21 12:50 ` [PATCH 09/11] ap: enable start of 4-way HS after SAE John Brandt
                   ` (3 subsequent siblings)
  11 siblings, 1 reply; 23+ messages in thread
From: John Brandt @ 2024-04-21 12:50 UTC (permalink / raw)
  To: iwd; +Cc: John Brandt

When the client requests SAE authentication, and it is enabled, allocate
an auth_proto instance to handle SAE authentication. This also adds a
new function to send SAE frames in AP mode that can be used by the
auth_proto instance.
---
 src/ap.c | 64 ++++++++++++++++++++++++++++++++++++++++++++++++--------
 1 file changed, 55 insertions(+), 9 deletions(-)

diff --git a/src/ap.c b/src/ap.c
index 14268551..ab0cbdcd 100644
--- a/src/ap.c
+++ b/src/ap.c
@@ -59,6 +59,8 @@
 #include "src/diagnostic.h"
 #include "src/band.h"
 #include "src/common.h"
+#include "src/sae.h"
+#include "src/auth-proto.h"
 
 struct ap_state {
 	struct netdev *netdev;
@@ -132,6 +134,7 @@ struct sta_state {
 	uint8_t *assoc_ies;
 	size_t assoc_ies_len;
 	uint8_t *assoc_rsne;
+	struct auth_proto *auth_proto;
 	enum ie_rsn_akm_suite akm_suite;
 	struct eapol_sm *sm;
 	struct handshake_state *hs;
@@ -2595,6 +2598,35 @@ static void ap_auth_reply(struct ap_state *ap, const uint8_t *dest,
 				ap_auth_reply_cb, NULL);
 }
 
+static void sae_auth_reply(const uint8_t *data, size_t len, void *user_data)
+{
+	struct sta_state *sta = (struct sta_state *)user_data;
+	struct ap_state *ap = (struct ap_state *)sta->ap;
+	const uint8_t *addr = netdev_get_address(ap->netdev);
+	uint8_t sae_buf[2048];
+	struct mmpdu_header *mpdu = (struct mmpdu_header *) sae_buf;
+	struct mmpdu_authentication *auth;
+
+	memset(mpdu, 0, sizeof(*mpdu));
+
+	/* Header */
+	mpdu->fc.protocol_version = 0;
+	mpdu->fc.type = MPDU_TYPE_MANAGEMENT;
+	mpdu->fc.subtype = MPDU_MANAGEMENT_SUBTYPE_AUTHENTICATION;
+	memcpy(mpdu->address_1, sta->addr, 6);	/* DA */
+	memcpy(mpdu->address_2, addr, 6);	/* SA */
+	memcpy(mpdu->address_3, addr, 6);	/* BSSID */
+
+	/* Authentication body */
+	auth = (void *) mmpdu_body(mpdu);
+	auth->algorithm = L_CPU_TO_LE16(MMPDU_AUTH_ALGO_SAE);
+
+	/* SAE elements */
+	memcpy(sae_buf + 26, data, len);
+	ap_send_mgmt_frame(ap, mpdu, 26 + len,
+				ap_auth_reply_cb, NULL);
+}
+
 /*
  * 802.11-2016 9.3.3.12 (frame format), 802.11-2016 11.3.4.3 and
  * 802.11-2016 12.3.3.2 (MLME/SME)
@@ -2664,7 +2696,7 @@ static void ap_auth_cb(const struct mmpdu_header *hdr, const void *body,
 	 * than State 1."
 	 */
 	if (sta)
-		goto done;
+		goto have_sta;
 
 	/*
 	 * Per 12.3.3.2.3 with Open System the state change is immediate,
@@ -2679,18 +2711,32 @@ static void ap_auth_cb(const struct mmpdu_header *hdr, const void *body,
 		ap->sta_states = l_queue_new();
 
 	sta->akm_suite = akm_suite;
+	if (sta->akm_suite == IE_RSN_AKM_SUITE_SAE_SHA256) {
+		sta->hs = netdev_handshake_state_new(sta->ap->netdev);
+		handshake_state_set_authenticator(sta->hs, true);
+		handshake_state_set_passphrase(sta->hs, ap->passphrase);
+		handshake_state_set_supplicant_address(sta->hs, sta->addr);
+		handshake_state_set_authenticator_address(sta->hs, bssid);
+
+		sta->auth_proto = sae_sm_new(sta->hs, sae_auth_reply, NULL, sta);
+	}
 
 	l_queue_push_tail(ap->sta_states, sta);
 
-	/*
-	 * Nothing to do here netlink-wise as we can't receive any data
-	 * frames until after association anyway.  We do need to add a
-	 * timeout for the authentication and possibly the kernel could
-	 * handle that if we registered the STA with NEW_STATION now (TODO)
-	 */
 
-done:
-	ap_auth_reply(ap, from, 0);
+have_sta:
+	if (sta->akm_suite == IE_RSN_AKM_SUITE_SAE_SHA256) {
+		auth_proto_rx_authenticate(sta->auth_proto, (const uint8_t *)hdr,
+					   body + body_len - (void *) hdr);
+	} else {
+		/*
+		 * Nothing to do here netlink-wise as we can't receive any data
+		 * frames until after association anyway.  We do need to add a
+		 * timeout for the authentication and possibly the kernel could
+		 * handle that if we registered the STA with NEW_STATION now (TODO)
+		 */
+		ap_auth_reply(ap, from, 0);
+	}
 }
 
 /* 802.11-2016 9.3.3.13 (frame format), 802.11-2016 11.3.4.5 (MLME/SME) */
-- 
2.44.0


^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [PATCH 09/11] ap: enable start of 4-way HS after SAE
  2024-04-21 12:50 [PATCH 00/11] Basic SAE support for AP mode John Brandt
                   ` (7 preceding siblings ...)
  2024-04-21 12:50 ` [PATCH 08/11] ap: add support to handle SAE authentication John Brandt
@ 2024-04-21 12:50 ` John Brandt
  2024-04-21 12:50 ` [PATCH 10/11] eapol: support PTK derivation with SHA256 John Brandt
                   ` (2 subsequent siblings)
  11 siblings, 0 replies; 23+ messages in thread
From: John Brandt @ 2024-04-21 12:50 UTC (permalink / raw)
  To: iwd; +Cc: John Brandt

Accept association frames that request SAE if SAE is enabled by the AP.
When SAE is being used, get the PMK as negoticated by SAE.
---
 src/ap.c | 17 ++++++++++++-----
 1 file changed, 12 insertions(+), 5 deletions(-)

diff --git a/src/ap.c b/src/ap.c
index ab0cbdcd..27b30e5b 100644
--- a/src/ap.c
+++ b/src/ap.c
@@ -1500,12 +1500,19 @@ static void ap_handshake_event(struct handshake_state *hs,
 
 static void ap_start_rsna(struct sta_state *sta, const uint8_t *gtk_rsc)
 {
-	/* this handshake setup assumes PSK network */
-	sta->hs = netdev_handshake_state_new(sta->ap->netdev);
-	handshake_state_set_authenticator(sta->hs, true);
+	/* this handshake setup assumes SAE or PSK network */
+	if (sta->hs && sta->akm_suite == IE_RSN_AKM_SUITE_SAE_SHA256) {
+		handshake_state_set_pmk(sta->hs, sta->hs->pmk, 32);
+		handshake_state_set_pmkid(sta->hs, sta->hs->pmkid);
+	} else {
+		sta->hs = netdev_handshake_state_new(sta->ap->netdev);
+		handshake_state_set_authenticator(sta->hs, true);
+		handshake_state_set_pmk(sta->hs, sta->ap->psk, 32);
+	}
+
 	handshake_state_set_event_func(sta->hs, ap_handshake_event, sta);
 	handshake_state_set_supplicant_ie(sta->hs, sta->assoc_rsne);
-	handshake_state_set_pmk(sta->hs, sta->ap->psk, 32);
+
 	ap_start_handshake(sta, false, gtk_rsc);
 }
 
@@ -2258,7 +2265,7 @@ static void ap_assoc_reassoc(struct sta_state *sta, bool reassoc,
 			goto unsupported;
 		}
 
-		if (rsn_info.akm_suites != IE_RSN_AKM_SUITE_PSK) {
+		if ((rsn_info.akm_suites & ap->akm_suites) == 0) {
 			err = MMPDU_REASON_CODE_INVALID_AKMP;
 			goto unsupported;
 		}
-- 
2.44.0


^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [PATCH 10/11] eapol: support PTK derivation with SHA256
  2024-04-21 12:50 [PATCH 00/11] Basic SAE support for AP mode John Brandt
                   ` (8 preceding siblings ...)
  2024-04-21 12:50 ` [PATCH 09/11] ap: enable start of 4-way HS after SAE John Brandt
@ 2024-04-21 12:50 ` John Brandt
  2024-04-21 12:50 ` [PATCH 11/11] eapol: encrypt key data for AKM-defined ciphers John Brandt
  2024-04-22 13:52 ` [PATCH 00/11] Basic SAE support for AP mode James Prestwood
  11 siblings, 0 replies; 23+ messages in thread
From: John Brandt @ 2024-04-21 12:50 UTC (permalink / raw)
  To: iwd; +Cc: John Brandt

Support PTK derivation in case the negotiated AKM requires SHA256. This
is needed to support SAE in AP mode.
---
 src/eapol.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/src/eapol.c b/src/eapol.c
index 3ce14d5c..a9b4f3ba 100644
--- a/src/eapol.c
+++ b/src/eapol.c
@@ -1560,6 +1560,7 @@ static void eapol_handle_ptk_2_of_4(struct eapol_sm *sm,
 	size_t ptk_size;
 	const uint8_t *kck;
 	const uint8_t *aa = sm->handshake->aa;
+	enum l_checksum_type type;
 
 	l_debug("ifindex=%u", sm->handshake->ifindex);
 
@@ -1571,12 +1572,16 @@ static void eapol_handle_ptk_2_of_4(struct eapol_sm *sm,
 
 	ptk_size = handshake_state_get_ptk_size(sm->handshake);
 
+	type = L_CHECKSUM_SHA1;
+	if (sm->handshake->akm_suite == IE_RSN_AKM_SUITE_SAE_SHA256)
+		type = L_CHECKSUM_SHA256;
+
 	if (!crypto_derive_pairwise_ptk(sm->handshake->pmk,
 					sm->handshake->pmk_len,
 					sm->handshake->spa, aa,
 					sm->handshake->anonce, ek->key_nonce,
 					sm->handshake->ptk, ptk_size,
-					L_CHECKSUM_SHA1))
+					type))
 		return;
 
 	kck = handshake_state_get_kck(sm->handshake);
-- 
2.44.0


^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [PATCH 11/11] eapol: encrypt key data for AKM-defined ciphers
  2024-04-21 12:50 [PATCH 00/11] Basic SAE support for AP mode John Brandt
                   ` (9 preceding siblings ...)
  2024-04-21 12:50 ` [PATCH 10/11] eapol: support PTK derivation with SHA256 John Brandt
@ 2024-04-21 12:50 ` John Brandt
  2024-04-22 13:52 ` [PATCH 00/11] Basic SAE support for AP mode James Prestwood
  11 siblings, 0 replies; 23+ messages in thread
From: John Brandt @ 2024-04-21 12:50 UTC (permalink / raw)
  To: iwd; +Cc: John Brandt

Support encrypting key data when the cipher is AKM-defined. This is
needed to support SAE in AP mode.
---
 src/eapol.c | 51 ++++++++++++++++++++++++++++++++++++---------------
 1 file changed, 36 insertions(+), 15 deletions(-)

diff --git a/src/eapol.c b/src/eapol.c
index a9b4f3ba..524a26c9 100644
--- a/src/eapol.c
+++ b/src/eapol.c
@@ -387,6 +387,23 @@ error:
 	return NULL;
 }
 
+static int padded_aes_wrap(const uint8_t *kek, uint8_t *key_data,
+				size_t *key_data_len,
+				struct eapol_key *out_frame, size_t mic_len)
+{
+	if (*key_data_len < 16 || *key_data_len % 8)
+		key_data[(*key_data_len)++] = 0xdd;
+	while (*key_data_len < 16 || *key_data_len % 8)
+		key_data[(*key_data_len)++] = 0x00;
+
+	if (!aes_wrap(kek, key_data, *key_data_len,
+				EAPOL_KEY_DATA(out_frame, mic_len)))
+		return -ENOPROTOOPT;
+
+	*key_data_len += 8;
+	return 0;
+}
+
 /*
  * Pad and encrypt the plaintext Key Data contents in @key_data using
  * the encryption scheme required by @out_frame->key_descriptor_version,
@@ -395,12 +412,12 @@ error:
  * Note that for efficiency @key_data is being modified, including in
  * case of failure, so it must be sufficiently larger than @key_data_len.
  */
-static int eapol_encrypt_key_data(const uint8_t *kek, uint8_t *key_data,
-				size_t key_data_len,
+static int eapol_encrypt_key_data(enum ie_rsn_akm_suite akm, const uint8_t *kek,
+				uint8_t *key_data, size_t key_data_len,
 				struct eapol_key *out_frame, size_t mic_len)
 {
 	uint8_t key[32];
-	bool ret;
+	int ret;
 
 	switch (out_frame->key_descriptor_version) {
 	case EAPOL_KEY_DESCRIPTOR_VERSION_HMAC_MD5_ARC4:
@@ -426,18 +443,21 @@ static int eapol_encrypt_key_data(const uint8_t *kek, uint8_t *key_data,
 		break;
 	case EAPOL_KEY_DESCRIPTOR_VERSION_HMAC_SHA1_AES:
 	case EAPOL_KEY_DESCRIPTOR_VERSION_AES_128_CMAC_AES:
-		if (key_data_len < 16 || key_data_len % 8)
-			key_data[key_data_len++] = 0xdd;
-		while (key_data_len < 16 || key_data_len % 8)
-			key_data[key_data_len++] = 0x00;
-
-		if (!aes_wrap(kek, key_data, key_data_len,
-					EAPOL_KEY_DATA(out_frame, mic_len)))
-			return -ENOPROTOOPT;
-
-		key_data_len += 8;
+		ret = padded_aes_wrap(kek, key_data, &key_data_len, out_frame, mic_len);
+		if (ret < 0)
+			return ret;
 
 		break;
+	case EAPOL_KEY_DESCRIPTOR_VERSION_AKM_DEFINED:
+		switch (akm) {
+		case IE_RSN_AKM_SUITE_SAE_SHA256:
+			ret = padded_aes_wrap(kek, key_data, &key_data_len, out_frame, mic_len);
+			if (ret < 0)
+				return ret;
+			break;
+		default:
+			return -ENOTSUP;
+		}
 	}
 
 	l_put_be16(key_data_len, EAPOL_KEY_DATA(out_frame, mic_len) - 2);
@@ -1467,8 +1487,9 @@ static void eapol_send_ptk_3_of_4(struct eapol_sm *sm)
 	}
 
 	kek = handshake_state_get_kek(sm->handshake);
-	key_data_len = eapol_encrypt_key_data(kek, key_data_buf,
-						key_data_len, ek, sm->mic_len);
+	key_data_len = eapol_encrypt_key_data(sm->handshake->akm_suite, kek,
+						key_data_buf, key_data_len, ek,
+						sm->mic_len);
 	explicit_bzero(key_data_buf, sizeof(key_data_buf));
 
 	if (key_data_len < 0)
-- 
2.44.0


^ permalink raw reply related	[flat|nested] 23+ messages in thread

* Re: [PATCH 00/11] Basic SAE support for AP mode
  2024-04-21 12:50 [PATCH 00/11] Basic SAE support for AP mode John Brandt
                   ` (10 preceding siblings ...)
  2024-04-21 12:50 ` [PATCH 11/11] eapol: encrypt key data for AKM-defined ciphers John Brandt
@ 2024-04-22 13:52 ` James Prestwood
  2024-04-24 12:07   ` James Prestwood
  11 siblings, 1 reply; 23+ messages in thread
From: James Prestwood @ 2024-04-22 13:52 UTC (permalink / raw)
  To: John Brandt, iwd

Hi John,

On 4/21/24 5:50 AM, John Brandt wrote:
> This set of patches adds basic SAE support for IWD in AP mode. It has
> been tested by connecting to IWD AP using wpa_supplicant. Note that this
> does not yet correspond to WPA3, since WPA3 would also require the
> support of Management Frame Protection.
>
> Normal client functionality has also been confirmed to still work. After
> applying these patches it remains possible for IWD client to connect to
> WPA3/SAE network.
>
> Remaining TODOs are to include better sanity-checking of received
> frames.

I took a quick pass and I'm impressed you took the initiative to 
implement this. I do need to take a closer look from the spec side of 
things but overall it looks good.

Assuming we are compliant with the spec my only concern merging this 
would be the TODOs. You do mention this is experimental and certain 
checks are not done, but you'd be surprised at the number of people 
using IWD in AP mode. The minute we merge this we're going to have 
people using it, which in its current form would be insecure. I think we 
first need to get the frame verification implemented as well as MFP. But 
anyways, this is a good start and I'll give it a full review when I have 
some time, hopefully this week.

Thanks,

James

>
> John Brandt (11):
>    ap: ability to advertise PSK and SAE
>    ap: accept PSK/SAE in auth depending on config
>    sae: add function sae_set_group
>    sae: refactor and add function sae_calculate_keys
>    sae: make sae_process_commit callable in AP mode
>    sae: verify offered group in AP mode
>    sae: support reception of Confirm frame by AP
>    ap: add support to handle SAE authentication
>    ap: enable start of 4-way HS after SAE
>    eapol: support PTK derivation with SHA256
>    eapol: encrypt key data for AKM-defined ciphers
>
>   src/ap.c    | 135 +++++++++++++++++++++++++++++++++-------
>   src/eapol.c |  58 ++++++++++++-----
>   src/sae.c   | 175 +++++++++++++++++++++++++++++++++-------------------
>   3 files changed, 265 insertions(+), 103 deletions(-)
>

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH 02/11] ap: accept PSK/SAE in auth depending on config
  2024-04-21 12:50 ` [PATCH 02/11] ap: accept PSK/SAE in auth depending on config John Brandt
@ 2024-04-24 12:05   ` James Prestwood
  0 siblings, 0 replies; 23+ messages in thread
From: James Prestwood @ 2024-04-24 12:05 UTC (permalink / raw)
  To: John Brandt, iwd

Hi John,

On 4/21/24 5:50 AM, John Brandt wrote:
> On reception of an authentication frame, accept both PSK and SAE as AKM
> depending on the config. Save the client's AKM for later use.
> ---
>   src/ap.c | 28 +++++++++++++++++++++-------
>   1 file changed, 21 insertions(+), 7 deletions(-)
>
> diff --git a/src/ap.c b/src/ap.c
> index d50f9e4f..14268551 100644
> --- a/src/ap.c
> +++ b/src/ap.c
> @@ -132,6 +132,7 @@ struct sta_state {
>   	uint8_t *assoc_ies;
>   	size_t assoc_ies_len;
>   	uint8_t *assoc_rsne;
> +	enum ie_rsn_akm_suite akm_suite;
>   	struct eapol_sm *sm;
>   	struct handshake_state *hs;
>   	uint32_t gtk_query_cmd_id;
> @@ -2606,6 +2607,7 @@ static void ap_auth_cb(const struct mmpdu_header *hdr, const void *body,
>   	const uint8_t *from = hdr->address_2;
>   	const uint8_t *bssid = netdev_get_address(ap->netdev);
>   	struct sta_state *sta;
> +	enum ie_rsn_akm_suite akm_suite;
>   
>   	l_info("AP Authentication from %s", util_address_to_string(from));
>   
> @@ -2627,17 +2629,27 @@ static void ap_auth_cb(const struct mmpdu_header *hdr, const void *body,
>   		}
>   	}
>   
> -	/* Only Open System authentication implemented here */
> -	if (L_LE16_TO_CPU(auth->algorithm) !=
> -			MMPDU_AUTH_ALGO_OPEN_SYSTEM) {
> +	if ((ap->akm_suites & IE_RSN_AKM_SUITE_SAE_SHA256) &&
> +	    (L_LE16_TO_CPU(auth->algorithm) == MMPDU_AUTH_ALGO_SAE) ) {
> +		/* When using SAE it must be COMMIT or CONFIRM frame */
> +		if (L_LE16_TO_CPU(auth->transaction_sequence) > 2) {
Probably want to explicitly check for a transaction of 1 or 2. This 
allows zero.
> +			ap_auth_reply(ap, from, MMPDU_REASON_CODE_UNSPECIFIED);
> +			return;
> +		}
> +		akm_suite = IE_RSN_AKM_SUITE_SAE_SHA256;
> +	} else if ((ap->akm_suites & IE_RSN_AKM_SUITE_PSK) &&
> +		   (L_LE16_TO_CPU(auth->algorithm) == MMPDU_AUTH_ALGO_OPEN_SYSTEM) ) {
> +		/* When using PSK it must be Open System authentication */
> +		if (L_LE16_TO_CPU(auth->transaction_sequence) != 1) {
> +			ap_auth_reply(ap, from, MMPDU_REASON_CODE_UNSPECIFIED);
> +			return;
> +		}
> +		akm_suite = IE_RSN_AKM_SUITE_PSK;
> +	} else {
>   		ap_auth_reply(ap, from, MMPDU_REASON_CODE_UNSPECIFIED);
>   		return;
>   	}
>   
> -	if (L_LE16_TO_CPU(auth->transaction_sequence) != 1) {
> -		ap_auth_reply(ap, from, MMPDU_REASON_CODE_UNSPECIFIED);
> -		return;
> -	}
>   
>   	sta = l_queue_find(ap->sta_states, ap_sta_match_addr, from);
>   
> @@ -2666,6 +2678,8 @@ static void ap_auth_cb(const struct mmpdu_header *hdr, const void *body,
>   	if (!ap->sta_states)
>   		ap->sta_states = l_queue_new();
>   
> +	sta->akm_suite = akm_suite;
> +
>   	l_queue_push_tail(ap->sta_states, sta);
>   
>   	/*

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH 03/11] sae: add function sae_set_group
  2024-04-21 12:50 ` [PATCH 03/11] sae: add function sae_set_group John Brandt
@ 2024-04-24 12:05   ` James Prestwood
  0 siblings, 0 replies; 23+ messages in thread
From: James Prestwood @ 2024-04-24 12:05 UTC (permalink / raw)
  To: John Brandt, iwd

Hi John,

On 4/21/24 5:50 AM, John Brandt wrote:
> Refactor code by adding function sae_set_group. This will make the next
> commits easier where basic SAE support for APs is added.
> ---
>   src/sae.c | 23 ++++++++++++-----------
>   1 file changed, 12 insertions(+), 11 deletions(-)
>
> diff --git a/src/sae.c b/src/sae.c
> index bf9fb0ff..c133386f 100644
> --- a/src/sae.c
> +++ b/src/sae.c
> @@ -148,6 +148,16 @@ static void sae_reset_state(struct sae_sm *sm)
>   	sm->pwe = NULL;
>   }
>   
> +static int sae_set_group(struct sae_sm *sm, int group)
> +{
> +	sae_debug("Using group %u", group);
> +
> +	sm->group = group;
> +	sm->curve = l_ecc_curve_from_ike_group(group);
We need to check if the group was supported. For this set it doesn't 
matter since your only using the static set of groups, but patch 5 uses 
the raw frame which may be a bogus/unsupported group number.
> +
> +	return 0;
> +}
> +
>   static int sae_choose_next_group(struct sae_sm *sm)
>   {
>   	const unsigned int *ecc_groups = l_ecc_supported_ike_groups();
> @@ -166,9 +176,7 @@ static int sae_choose_next_group(struct sae_sm *sm)
>   		sae_debug("Forcing default SAE group 19");
>   
>   		sm->group_retry++;
> -		sm->group = 19;
> -
> -		goto get_curve;
> +		return sae_set_group(sm, 19);
>   	}
>   
>   	do {
> @@ -182,14 +190,7 @@ static int sae_choose_next_group(struct sae_sm *sm)
>   	if (reset)
>   		sae_reset_state(sm);
>   
> -	sm->group = ecc_groups[sm->group_retry];
> -
> -get_curve:
> -	sae_debug("Using group %u", sm->group);
> -
> -	sm->curve = l_ecc_curve_from_ike_group(sm->group);
> -
> -	return 0;
> +	return sae_set_group(sm, ecc_groups[sm->group_retry]);
>   }
>   
>   static int sae_valid_group(struct sae_sm *sm, unsigned int group)

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH 04/11] sae: refactor and add function sae_calculate_keys
  2024-04-21 12:50 ` [PATCH 04/11] sae: refactor and add function sae_calculate_keys John Brandt
@ 2024-04-24 12:06   ` James Prestwood
  0 siblings, 0 replies; 23+ messages in thread
From: James Prestwood @ 2024-04-24 12:06 UTC (permalink / raw)
  To: John Brandt, iwd

Hi John,

On 4/21/24 5:50 AM, John Brandt wrote:
> Refactor code by moving code to the new function sae_calculate_keys.
> This will make it easier in the next commits to add SAE support for AP
> mode.
> ---
>   src/sae.c | 83 +++++++++++++++++++++++++++++++------------------------
>   1 file changed, 47 insertions(+), 36 deletions(-)
>
> diff --git a/src/sae.c b/src/sae.c
> index c133386f..314fc28f 100644
> --- a/src/sae.c
> +++ b/src/sae.c
> @@ -683,10 +683,9 @@ static bool sae_send_confirm(struct sae_sm *sm)
>   	return true;
>   }
>   
> -static int sae_process_commit(struct sae_sm *sm, const uint8_t *from,
> -					const uint8_t *frame, size_t len)
> +
> +static int sae_calculate_keys(struct sae_sm *sm)
>   {
> -	uint8_t *ptr = (uint8_t *) frame;
>   	unsigned int nbytes = l_ecc_curve_get_scalar_bytes(sm->curve);
>   	enum l_checksum_type hash =
>   		crypto_sae_hash_from_ecc_prime_len(sm->sae_type, nbytes);
> @@ -702,39 +701,6 @@ static int sae_process_commit(struct sae_sm *sm, const uint8_t *from,
>   	struct l_ecc_scalar *tmp_scalar;
>   	struct l_ecc_scalar *order;
>   
> -	ptr += 2;
> -
> -	sm->p_scalar = l_ecc_scalar_new(sm->curve, ptr, nbytes);
> -	if (!sm->p_scalar) {
> -		l_error("Server sent invalid P_Scalar during commit");
> -		return sae_reject(sm, SAE_STATE_COMMITTED,
> -				MMPDU_STATUS_CODE_UNSUPP_FINITE_CYCLIC_GROUP);
> -	}
> -
> -	ptr += nbytes;
> -
> -	sm->p_element = l_ecc_point_from_data(sm->curve, L_ECC_POINT_TYPE_FULL,
> -						ptr, nbytes * 2);
> -	if (!sm->p_element) {
> -		l_error("Server sent invalid P_Element during commit");
> -		return sae_reject(sm, SAE_STATE_COMMITTED,
> -				MMPDU_STATUS_CODE_UNSUPP_FINITE_CYCLIC_GROUP);
> -	}
> -
> -	/*
> -	 * If they match those sent as part of the protocol instance's own
> -	 * SAE Commit message, the frame shall be silently discarded (because
> -	 * it is evidence of a reflection attack) and the t0 (retransmission)
> -	 * timer shall be set.
> -	 */
> -	if (l_ecc_scalars_are_equal(sm->p_scalar, sm->scalar) ||
> -			l_ecc_points_are_equal(sm->p_element, sm->element)) {
> -		l_warn("peer scalar or element matched own, discarding frame");
> -		return -ENOMSG;
> -	}
> -
> -	sm->sc++;
> -
>   	/*
>   	 * K = scalar-op(rand, (element-op(scalar-op(peer-commit-scalar, PWE),
>   	 *			PEER-COMMIT-ELEMENT)))
> @@ -823,6 +789,51 @@ static int sae_process_commit(struct sae_sm *sm, const uint8_t *from,
>   	/* don't set the handshakes pmkid until confirm is verified */
>   	memcpy(sm->pmkid, tmp, 16);
>   
> +	return 0;
> +}
> +
> +
> +static int sae_process_commit(struct sae_sm *sm, const uint8_t *from,
> +					const uint8_t *frame, size_t len)
> +{
> +	uint8_t *ptr = (uint8_t *) frame;
> +	unsigned int nbytes = l_ecc_curve_get_scalar_bytes(sm->curve);
> +
> +	ptr += 2;
> +
> +	sm->p_scalar = l_ecc_scalar_new(sm->curve, ptr, nbytes);
> +	if (!sm->p_scalar) {
> +		l_error("Server sent invalid P_Scalar during commit");
> +		return sae_reject(sm, SAE_STATE_COMMITTED,
> +				MMPDU_STATUS_CODE_UNSUPP_FINITE_CYCLIC_GROUP);
> +	}
> +
> +	ptr += nbytes;
> +
> +	sm->p_element = l_ecc_point_from_data(sm->curve, L_ECC_POINT_TYPE_FULL,
> +						ptr, nbytes * 2);
> +	if (!sm->p_element) {
> +		l_error("Server sent invalid P_Element during commit");
> +		return sae_reject(sm, SAE_STATE_COMMITTED,
> +				MMPDU_STATUS_CODE_UNSUPP_FINITE_CYCLIC_GROUP);
> +	}
> +
> +	/*
> +	 * If they match those sent as part of the protocol instance's own
> +	 * SAE Commit message, the frame shall be silently discarded (because
> +	 * it is evidence of a reflection attack) and the t0 (retransmission)
> +	 * timer shall be set.
> +	 */
> +	if (l_ecc_scalars_are_equal(sm->p_scalar, sm->scalar) ||
> +			l_ecc_points_are_equal(sm->p_element, sm->element)) {
> +		l_warn("peer scalar or element matched own, discarding frame");
> +		return -ENOMSG;
> +	}
> +
> +	sm->sc++;
> +
> +	sae_calculate_keys(sm);
No return check here. Its likely an impossible scenario (getting the 
x-value) but for consistency might as well check.
> +
>   	if (!sae_send_confirm(sm))
>   		return -EPROTO;
>   

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH 08/11] ap: add support to handle SAE authentication
  2024-04-21 12:50 ` [PATCH 08/11] ap: add support to handle SAE authentication John Brandt
@ 2024-04-24 12:06   ` James Prestwood
  0 siblings, 0 replies; 23+ messages in thread
From: James Prestwood @ 2024-04-24 12:06 UTC (permalink / raw)
  To: John Brandt, iwd

Hi John,

On 4/21/24 5:50 AM, John Brandt wrote:
> When the client requests SAE authentication, and it is enabled, allocate
> an auth_proto instance to handle SAE authentication. This also adds a
> new function to send SAE frames in AP mode that can be used by the
> auth_proto instance.
> ---
>   src/ap.c | 64 ++++++++++++++++++++++++++++++++++++++++++++++++--------
>   1 file changed, 55 insertions(+), 9 deletions(-)
>
> diff --git a/src/ap.c b/src/ap.c
> index 14268551..ab0cbdcd 100644
> --- a/src/ap.c
> +++ b/src/ap.c
> @@ -59,6 +59,8 @@
>   #include "src/diagnostic.h"
>   #include "src/band.h"
>   #include "src/common.h"
> +#include "src/sae.h"
> +#include "src/auth-proto.h"
>   
>   struct ap_state {
>   	struct netdev *netdev;
> @@ -132,6 +134,7 @@ struct sta_state {
>   	uint8_t *assoc_ies;
>   	size_t assoc_ies_len;
>   	uint8_t *assoc_rsne;
> +	struct auth_proto *auth_proto;
>   	enum ie_rsn_akm_suite akm_suite;
>   	struct eapol_sm *sm;
>   	struct handshake_state *hs;
> @@ -2595,6 +2598,35 @@ static void ap_auth_reply(struct ap_state *ap, const uint8_t *dest,
>   				ap_auth_reply_cb, NULL);
>   }
>   
> +static void sae_auth_reply(const uint8_t *data, size_t len, void *user_data)
> +{
> +	struct sta_state *sta = (struct sta_state *)user_data;
> +	struct ap_state *ap = (struct ap_state *)sta->ap;
> +	const uint8_t *addr = netdev_get_address(ap->netdev);
> +	uint8_t sae_buf[2048];

Could probably pare this down a bit. I don't mind if we leave a little 
room but 2048 seems a bit much. In a commit we've got the header + 48 + 
96 bytes for the scalar/element + 32 bytes of a token + some other bytes 
here and there. You could probably at least half this and to be safe you 
could:

if (L_WARN_ON(len > sizeof(sae_buf))
     return;
> +	struct mmpdu_header *mpdu = (struct mmpdu_header *) sae_buf;
> +	struct mmpdu_authentication *auth;
> +
> +	memset(mpdu, 0, sizeof(*mpdu));
> +
> +	/* Header */
> +	mpdu->fc.protocol_version = 0;
> +	mpdu->fc.type = MPDU_TYPE_MANAGEMENT;
> +	mpdu->fc.subtype = MPDU_MANAGEMENT_SUBTYPE_AUTHENTICATION;
> +	memcpy(mpdu->address_1, sta->addr, 6);	/* DA */
> +	memcpy(mpdu->address_2, addr, 6);	/* SA */
> +	memcpy(mpdu->address_3, addr, 6);	/* BSSID */
> +
> +	/* Authentication body */
> +	auth = (void *) mmpdu_body(mpdu);
> +	auth->algorithm = L_CPU_TO_LE16(MMPDU_AUTH_ALGO_SAE);
> +
> +	/* SAE elements */
> +	memcpy(sae_buf + 26, data, len);
> +	ap_send_mgmt_frame(ap, mpdu, 26 + len,
> +				ap_auth_reply_cb, NULL);
> +}
> +
>   /*
>    * 802.11-2016 9.3.3.12 (frame format), 802.11-2016 11.3.4.3 and
>    * 802.11-2016 12.3.3.2 (MLME/SME)
> @@ -2664,7 +2696,7 @@ static void ap_auth_cb(const struct mmpdu_header *hdr, const void *body,
>   	 * than State 1."
>   	 */
>   	if (sta)
> -		goto done;
> +		goto have_sta;
>   
>   	/*
>   	 * Per 12.3.3.2.3 with Open System the state change is immediate,
> @@ -2679,18 +2711,32 @@ static void ap_auth_cb(const struct mmpdu_header *hdr, const void *body,
>   		ap->sta_states = l_queue_new();
>   
>   	sta->akm_suite = akm_suite;
> +	if (sta->akm_suite == IE_RSN_AKM_SUITE_SAE_SHA256) {
> +		sta->hs = netdev_handshake_state_new(sta->ap->netdev);
> +		handshake_state_set_authenticator(sta->hs, true);
> +		handshake_state_set_passphrase(sta->hs, ap->passphrase);
> +		handshake_state_set_supplicant_address(sta->hs, sta->addr);
> +		handshake_state_set_authenticator_address(sta->hs, bssid);
> +
> +		sta->auth_proto = sae_sm_new(sta->hs, sae_auth_reply, NULL, sta);
> +	}
>   
>   	l_queue_push_tail(ap->sta_states, sta);
>   
> -	/*
> -	 * Nothing to do here netlink-wise as we can't receive any data
> -	 * frames until after association anyway.  We do need to add a
> -	 * timeout for the authentication and possibly the kernel could
> -	 * handle that if we registered the STA with NEW_STATION now (TODO)
> -	 */
>   
> -done:
> -	ap_auth_reply(ap, from, 0);
> +have_sta:
> +	if (sta->akm_suite == IE_RSN_AKM_SUITE_SAE_SHA256) {
> +		auth_proto_rx_authenticate(sta->auth_proto, (const uint8_t *)hdr,
> +					   body + body_len - (void *) hdr);
> +	} else {
> +		/*
> +		 * Nothing to do here netlink-wise as we can't receive any data
> +		 * frames until after association anyway.  We do need to add a
> +		 * timeout for the authentication and possibly the kernel could
> +		 * handle that if we registered the STA with NEW_STATION now (TODO)
> +		 */
> +		ap_auth_reply(ap, from, 0);
> +	}
>   }
>   
>   /* 802.11-2016 9.3.3.13 (frame format), 802.11-2016 11.3.4.5 (MLME/SME) */

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH 00/11] Basic SAE support for AP mode
  2024-04-22 13:52 ` [PATCH 00/11] Basic SAE support for AP mode James Prestwood
@ 2024-04-24 12:07   ` James Prestwood
  2024-04-29  0:04     ` John Brandt
  0 siblings, 1 reply; 23+ messages in thread
From: James Prestwood @ 2024-04-24 12:07 UTC (permalink / raw)
  To: John Brandt, iwd


On 4/22/24 6:52 AM, James Prestwood wrote:
> Hi John,
>
> On 4/21/24 5:50 AM, John Brandt wrote:
>> This set of patches adds basic SAE support for IWD in AP mode. It has
>> been tested by connecting to IWD AP using wpa_supplicant. Note that this
>> does not yet correspond to WPA3, since WPA3 would also require the
>> support of Management Frame Protection.
>>
>> Normal client functionality has also been confirmed to still work. After
>> applying these patches it remains possible for IWD client to connect to
>> WPA3/SAE network.
>>
>> Remaining TODOs are to include better sanity-checking of received
>> frames.
>
> I took a quick pass and I'm impressed you took the initiative to 
> implement this. I do need to take a closer look from the spec side of 
> things but overall it looks good.
>
> Assuming we are compliant with the spec my only concern merging this 
> would be the TODOs. You do mention this is experimental and certain 
> checks are not done, but you'd be surprised at the number of people 
> using IWD in AP mode. The minute we merge this we're going to have 
> people using it, which in its current form would be insecure. I think 
> we first need to get the frame verification implemented as well as 
> MFP. But anyways, this is a good start and I'll give it a full review 
> when I have some time, hopefully this week.
>
> Thanks,
>
> James

Reviewed. I also forgot to mention the CI we have showed test-sae failed 
after your patches, so that will need to be addressed as well. I keep 
getting reminded I need to also email the patch submitter.

Thanks,

James

>
>>
>> John Brandt (11):
>>    ap: ability to advertise PSK and SAE
>>    ap: accept PSK/SAE in auth depending on config
>>    sae: add function sae_set_group
>>    sae: refactor and add function sae_calculate_keys
>>    sae: make sae_process_commit callable in AP mode
>>    sae: verify offered group in AP mode
>>    sae: support reception of Confirm frame by AP
>>    ap: add support to handle SAE authentication
>>    ap: enable start of 4-way HS after SAE
>>    eapol: support PTK derivation with SHA256
>>    eapol: encrypt key data for AKM-defined ciphers
>>
>>   src/ap.c    | 135 +++++++++++++++++++++++++++++++++-------
>>   src/eapol.c |  58 ++++++++++++-----
>>   src/sae.c   | 175 +++++++++++++++++++++++++++++++++-------------------
>>   3 files changed, 265 insertions(+), 103 deletions(-)
>>

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH 05/11] sae: make sae_process_commit callable in AP mode
  2024-04-21 12:50 ` [PATCH 05/11] sae: make sae_process_commit callable in AP mode John Brandt
@ 2024-04-24 12:08   ` James Prestwood
  0 siblings, 0 replies; 23+ messages in thread
From: James Prestwood @ 2024-04-24 12:08 UTC (permalink / raw)
  To: John Brandt, iwd

Hi John

On 4/21/24 5:50 AM, John Brandt wrote:
> As an AP, the function sae_process_commit will pick the group offered by
> the client. In a subsuquent commit the offered group will first be
> verified before calling sae_process_commit. The AP will reply with a
> Commit frame, calculate current keys, and move to the COMMITTED state.
> ---
>   src/sae.c | 32 ++++++++++++++++++++------------
>   1 file changed, 20 insertions(+), 12 deletions(-)
>
> diff --git a/src/sae.c b/src/sae.c
> index 314fc28f..2c97d94c 100644
> --- a/src/sae.c
> +++ b/src/sae.c
> @@ -48,6 +48,8 @@ static bool debug;
>   #define SAE_SYNC_MAX		3
>   #define SAE_MAX_ASSOC_RETRY	3
>   
> +static bool sae_send_commit(struct sae_sm *sm, bool retry);
> +
>   #define sae_debug(fmat, ...) \
>   ({	\
>   	if (debug) \
> @@ -797,8 +799,12 @@ static int sae_process_commit(struct sae_sm *sm, const uint8_t *from,
>   					const uint8_t *frame, size_t len)
>   {
>   	uint8_t *ptr = (uint8_t *) frame;
> -	unsigned int nbytes = l_ecc_curve_get_scalar_bytes(sm->curve);
> +	unsigned int nbytes;
> +
> +	if (sm->handshake->authenticator && sae_set_group(sm, l_get_le16(frame)) < 0)
> +		return -1;
>   
> +	nbytes = l_ecc_curve_get_scalar_bytes(sm->curve);
>   	ptr += 2;
>   
>   	sm->p_scalar = l_ecc_scalar_new(sm->curve, ptr, nbytes);
> @@ -824,20 +830,22 @@ static int sae_process_commit(struct sae_sm *sm, const uint8_t *from,
>   	 * it is evidence of a reflection attack) and the t0 (retransmission)
>   	 * timer shall be set.
>   	 */
> -	if (l_ecc_scalars_are_equal(sm->p_scalar, sm->scalar) ||
> -			l_ecc_points_are_equal(sm->p_element, sm->element)) {
> -		l_warn("peer scalar or element matched own, discarding frame");
> +	if ((sm->scalar && l_ecc_scalars_are_equal(sm->p_scalar, sm->scalar)) ||
> +			(sm->element && l_ecc_points_are_equal(sm->p_element, sm->element)))
So we isolated this check to only the supplicant side now, but do we 
also need to do the same check after the keys are calculated for AP mode?
>   		return -ENOMSG;
> -	}
>   
>   	sm->sc++;
>   
> -	sae_calculate_keys(sm);
> -
> -	if (!sae_send_confirm(sm))
> -		return -EPROTO;
> -
> -	sm->state = SAE_STATE_CONFIRMED;
> +	if (sm->handshake->authenticator) {
> +		sae_send_commit(sm, false);
> +		sae_calculate_keys(sm);
> +		sm->state = SAE_STATE_COMMITTED;
> +	} else {
> +		sae_calculate_keys(sm);
> +		if (!sae_send_confirm(sm))
> +			return -EPROTO;
> +		sm->state = SAE_STATE_CONFIRMED;
> +	}
>   
>   	return 0;
>   }
> @@ -1008,7 +1016,7 @@ static int sae_verify_nothing(struct sae_sm *sm, uint16_t transaction,
>   	/*
>   	 * TODO: This does not handle the transition from NOTHING -> CONFIRMED
>   	 * as this is only relevant to the AP or in Mesh mode which is not
> -	 * yet supported.
> +	 * yet fully supported.
>   	 */
>   	if (transaction != SAE_STATE_COMMITTED)
>   		return -EBADMSG;

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH 07/11] sae: support reception of Confirm frame by AP
  2024-04-21 12:50 ` [PATCH 07/11] sae: support reception of Confirm frame by AP John Brandt
@ 2024-04-24 12:08   ` James Prestwood
  0 siblings, 0 replies; 23+ messages in thread
From: James Prestwood @ 2024-04-24 12:08 UTC (permalink / raw)
  To: John Brandt, iwd

Hi John,

On 4/21/24 5:50 AM, John Brandt wrote:
> Experimental AP-mode support for receiving a Confirm frame when in the
> COMMITTED state. The AP will reply with a Confirm frame.
>
> Note that when acting as an AP, on reception of a Commit frame, the AP
> only replies with a Commit frame. The protocols allows to also already
> send the Confirm frame, but older clients may not support simultaneously
> receiving a Commit and Confirm frame.
Could we add some basic unit tests. Mainly just sanity checks that the 
message flow works as expected with handshake->authenticator set.
> ---
>   src/sae.c | 38 +++++++++++++++++++++++++-------------
>   1 file changed, 25 insertions(+), 13 deletions(-)
>
> diff --git a/src/sae.c b/src/sae.c
> index 8a1e311a..da55c764 100644
> --- a/src/sae.c
> +++ b/src/sae.c
> @@ -906,9 +906,13 @@ static int sae_process_confirm(struct sae_sm *sm, const uint8_t *from,
>   
>   	sm->state = SAE_STATE_ACCEPTED;
>   
> -	sae_debug("Sending Associate to "MAC, MAC_STR(sm->handshake->aa));
> -
> -	sm->tx_assoc(sm->user_data);
> +	if (!sm->handshake->authenticator) {
> +		sae_debug("Sending Associate to "MAC, MAC_STR(sm->handshake->aa));
> +		sm->tx_assoc(sm->user_data);
> +	} else {
> +		if (!sae_send_confirm(sm))
> +			return -EPROTO;
> +	}
>   
>   	return 0;
>   }
> @@ -1059,16 +1063,24 @@ static int sae_verify_committed(struct sae_sm *sm, uint16_t transaction,
>   	unsigned int skip;
>   	struct ie_tlv_iter iter;
>   
> -	/*
> -	 * Upon receipt of a Con event...
> -	 * Then the protocol instance checks the value of Sync. If it
> -	 * is greater than dot11RSNASAESync, the protocol instance shall send a
> -	 * Del event to the parent process and transition back to Nothing state.
> -	 * If Sync is not greater than dot11RSNASAESync, the protocol instance
> -	 * shall increment Sync, transmit the last SAE Commit message sent to
> -	 * the peer...
> -	 */
> -	if (transaction == SAE_STATE_CONFIRMED) {
> +	if (sm->handshake->authenticator && transaction == SAE_STATE_CONFIRMED) {
> +		/*
> +		 * TODO: Sanity-check received Confirm frame from the client. For now
> +		 * AP-mode SAE support is experimental and we simply accept the frame.
> +		 * Note that the cryptographic confirm field value will still be checked
> +		 * before replying with a Confirm frame.
> +		 */
> +		return 0;
> +	} else if (transaction == SAE_STATE_CONFIRMED) {
> +		/*
> +		 * Upon receipt of a Con event...
> +		 * Then the protocol instance checks the value of Sync. If it
> +		 * is greater than dot11RSNASAESync, the protocol instance shall send a
> +		 * Del event to the parent process and transition back to Nothing state.
> +		 * If Sync is not greater than dot11RSNASAESync, the protocol instance
> +		 * shall increment Sync, transmit the last SAE Commit message sent to
> +		 * the peer...
> +		 */
>   		if (sm->sync > SAE_SYNC_MAX)
>   			return -ETIMEDOUT;
>   

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH 00/11] Basic SAE support for AP mode
  2024-04-24 12:07   ` James Prestwood
@ 2024-04-29  0:04     ` John Brandt
  2024-04-29 12:00       ` James Prestwood
  0 siblings, 1 reply; 23+ messages in thread
From: John Brandt @ 2024-04-29  0:04 UTC (permalink / raw)
  To: James Prestwood, iwd



On 4/24/24 05:07, James Prestwood wrote:
> 
> On 4/22/24 6:52 AM, James Prestwood wrote:
>> Hi John,
>>
>> On 4/21/24 5:50 AM, John Brandt wrote:
>>> This set of patches adds basic SAE support for IWD in AP mode. It has
>>> been tested by connecting to IWD AP using wpa_supplicant. Note that this
>>> does not yet correspond to WPA3, since WPA3 would also require the
>>> support of Management Frame Protection.
>>>
>>> Normal client functionality has also been confirmed to still work. After
>>> applying these patches it remains possible for IWD client to connect to
>>> WPA3/SAE network.
>>>
>>> Remaining TODOs are to include better sanity-checking of received
>>> frames.
>>
>> I took a quick pass and I'm impressed you took the initiative to 
>> implement this. I do need to take a closer look from the spec side of 
>> things but overall it looks good.
>>
>> Assuming we are compliant with the spec my only concern merging this 
>> would be the TODOs. You do mention this is experimental and certain 
>> checks are not done, but you'd be surprised at the number of people 
>> using IWD in AP mode. The minute we merge this we're going to have 
>> people using it, which in its current form would be insecure. I think 
>> we first need to get the frame verification implemented as well as 
>> MFP. But anyways, this is a good start and I'll give it a full review 
>> when I have some time, hopefully this week.
>>
>> Thanks,
>>
>> James
> 
> Reviewed. I also forgot to mention the CI we have showed test-sae failed 
> after your patches, so that will need to be addressed as well. I keep 
> getting reminded I need to also email the patch submitter.
> 
> Thanks,
> 
> James

Thanks, I've mostly incorporated the feedback, and also added extra 
frame checks. I can post the v2 version later this week.

I'm unsure whether I'll also have the time to experiment with MFP. Maybe 
it's possible to already add SAE support without yet mentioning WPA3 for 
the AP mode?

>>
>>>
>>> John Brandt (11):
>>>    ap: ability to advertise PSK and SAE
>>>    ap: accept PSK/SAE in auth depending on config
>>>    sae: add function sae_set_group
>>>    sae: refactor and add function sae_calculate_keys
>>>    sae: make sae_process_commit callable in AP mode
>>>    sae: verify offered group in AP mode
>>>    sae: support reception of Confirm frame by AP
>>>    ap: add support to handle SAE authentication
>>>    ap: enable start of 4-way HS after SAE
>>>    eapol: support PTK derivation with SHA256
>>>    eapol: encrypt key data for AKM-defined ciphers
>>>
>>>   src/ap.c    | 135 +++++++++++++++++++++++++++++++++-------
>>>   src/eapol.c |  58 ++++++++++++-----
>>>   src/sae.c   | 175 +++++++++++++++++++++++++++++++++-------------------
>>>   3 files changed, 265 insertions(+), 103 deletions(-)
>>>

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH 00/11] Basic SAE support for AP mode
  2024-04-29  0:04     ` John Brandt
@ 2024-04-29 12:00       ` James Prestwood
  2024-04-30 23:27         ` KeithG
  0 siblings, 1 reply; 23+ messages in thread
From: James Prestwood @ 2024-04-29 12:00 UTC (permalink / raw)
  To: John Brandt, iwd

Hi John,

On 4/28/24 5:04 PM, John Brandt wrote:
>
>
> On 4/24/24 05:07, James Prestwood wrote:
>>
>> On 4/22/24 6:52 AM, James Prestwood wrote:
>>> Hi John,
>>>
>>> On 4/21/24 5:50 AM, John Brandt wrote:
>>>> This set of patches adds basic SAE support for IWD in AP mode. It has
>>>> been tested by connecting to IWD AP using wpa_supplicant. Note that 
>>>> this
>>>> does not yet correspond to WPA3, since WPA3 would also require the
>>>> support of Management Frame Protection.
>>>>
>>>> Normal client functionality has also been confirmed to still work. 
>>>> After
>>>> applying these patches it remains possible for IWD client to 
>>>> connect to
>>>> WPA3/SAE network.
>>>>
>>>> Remaining TODOs are to include better sanity-checking of received
>>>> frames.
>>>
>>> I took a quick pass and I'm impressed you took the initiative to 
>>> implement this. I do need to take a closer look from the spec side 
>>> of things but overall it looks good.
>>>
>>> Assuming we are compliant with the spec my only concern merging this 
>>> would be the TODOs. You do mention this is experimental and certain 
>>> checks are not done, but you'd be surprised at the number of people 
>>> using IWD in AP mode. The minute we merge this we're going to have 
>>> people using it, which in its current form would be insecure. I 
>>> think we first need to get the frame verification implemented as 
>>> well as MFP. But anyways, this is a good start and I'll give it a 
>>> full review when I have some time, hopefully this week.
>>>
>>> Thanks,
>>>
>>> James
>>
>> Reviewed. I also forgot to mention the CI we have showed test-sae 
>> failed after your patches, so that will need to be addressed as well. 
>> I keep getting reminded I need to also email the patch submitter.
>>
>> Thanks,
>>
>> James
>
> Thanks, I've mostly incorporated the feedback, and also added extra 
> frame checks. I can post the v2 version later this week.
>
> I'm unsure whether I'll also have the time to experiment with MFP. 
> Maybe it's possible to already add SAE support without yet mentioning 
> WPA3 for the AP mode?

MFP may be sort of automatic, it is for station mode. All IWD does is 
some minimal validation since it has an explicit profile setting to 
enable/disable/require MFP. It also checks the cipher support related to 
MFP. But I think for AP mode all you'd need to do is ensure that 
connecting stations are capable of MFP when connecting via WPA3. And set 
the proper bits in the RSNE.

Thanks,

James

>
>>>
>>>>
>>>> John Brandt (11):
>>>>    ap: ability to advertise PSK and SAE
>>>>    ap: accept PSK/SAE in auth depending on config
>>>>    sae: add function sae_set_group
>>>>    sae: refactor and add function sae_calculate_keys
>>>>    sae: make sae_process_commit callable in AP mode
>>>>    sae: verify offered group in AP mode
>>>>    sae: support reception of Confirm frame by AP
>>>>    ap: add support to handle SAE authentication
>>>>    ap: enable start of 4-way HS after SAE
>>>>    eapol: support PTK derivation with SHA256
>>>>    eapol: encrypt key data for AKM-defined ciphers
>>>>
>>>>   src/ap.c    | 135 +++++++++++++++++++++++++++++++++-------
>>>>   src/eapol.c |  58 ++++++++++++-----
>>>>   src/sae.c   | 175 
>>>> +++++++++++++++++++++++++++++++++-------------------
>>>>   3 files changed, 265 insertions(+), 103 deletions(-)
>>>>

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH 00/11] Basic SAE support for AP mode
  2024-04-29 12:00       ` James Prestwood
@ 2024-04-30 23:27         ` KeithG
  0 siblings, 0 replies; 23+ messages in thread
From: KeithG @ 2024-04-30 23:27 UTC (permalink / raw)
  To: James Prestwood; +Cc: John Brandt, iwd

James,

When this is ready to go, I can test on my Pis...

Just need to know how to tell AP mode how to use wpa3/sae

Keith

Keith

On Mon, Apr 29, 2024 at 7:04 AM James Prestwood <prestwoj@gmail.com> wrote:
>
> Hi John,
>
> On 4/28/24 5:04 PM, John Brandt wrote:
> >
> >
> > On 4/24/24 05:07, James Prestwood wrote:
> >>
> >> On 4/22/24 6:52 AM, James Prestwood wrote:
> >>> Hi John,
> >>>
> >>> On 4/21/24 5:50 AM, John Brandt wrote:
> >>>> This set of patches adds basic SAE support for IWD in AP mode. It has
> >>>> been tested by connecting to IWD AP using wpa_supplicant. Note that
> >>>> this
> >>>> does not yet correspond to WPA3, since WPA3 would also require the
> >>>> support of Management Frame Protection.
> >>>>
> >>>> Normal client functionality has also been confirmed to still work.
> >>>> After
> >>>> applying these patches it remains possible for IWD client to
> >>>> connect to
> >>>> WPA3/SAE network.
> >>>>
> >>>> Remaining TODOs are to include better sanity-checking of received
> >>>> frames.
> >>>
> >>> I took a quick pass and I'm impressed you took the initiative to
> >>> implement this. I do need to take a closer look from the spec side
> >>> of things but overall it looks good.
> >>>
> >>> Assuming we are compliant with the spec my only concern merging this
> >>> would be the TODOs. You do mention this is experimental and certain
> >>> checks are not done, but you'd be surprised at the number of people
> >>> using IWD in AP mode. The minute we merge this we're going to have
> >>> people using it, which in its current form would be insecure. I
> >>> think we first need to get the frame verification implemented as
> >>> well as MFP. But anyways, this is a good start and I'll give it a
> >>> full review when I have some time, hopefully this week.
> >>>
> >>> Thanks,
> >>>
> >>> James
> >>
> >> Reviewed. I also forgot to mention the CI we have showed test-sae
> >> failed after your patches, so that will need to be addressed as well.
> >> I keep getting reminded I need to also email the patch submitter.
> >>
> >> Thanks,
> >>
> >> James
> >
> > Thanks, I've mostly incorporated the feedback, and also added extra
> > frame checks. I can post the v2 version later this week.
> >
> > I'm unsure whether I'll also have the time to experiment with MFP.
> > Maybe it's possible to already add SAE support without yet mentioning
> > WPA3 for the AP mode?
>
> MFP may be sort of automatic, it is for station mode. All IWD does is
> some minimal validation since it has an explicit profile setting to
> enable/disable/require MFP. It also checks the cipher support related to
> MFP. But I think for AP mode all you'd need to do is ensure that
> connecting stations are capable of MFP when connecting via WPA3. And set
> the proper bits in the RSNE.
>
> Thanks,
>
> James
>
> >
> >>>
> >>>>
> >>>> John Brandt (11):
> >>>>    ap: ability to advertise PSK and SAE
> >>>>    ap: accept PSK/SAE in auth depending on config
> >>>>    sae: add function sae_set_group
> >>>>    sae: refactor and add function sae_calculate_keys
> >>>>    sae: make sae_process_commit callable in AP mode
> >>>>    sae: verify offered group in AP mode
> >>>>    sae: support reception of Confirm frame by AP
> >>>>    ap: add support to handle SAE authentication
> >>>>    ap: enable start of 4-way HS after SAE
> >>>>    eapol: support PTK derivation with SHA256
> >>>>    eapol: encrypt key data for AKM-defined ciphers
> >>>>
> >>>>   src/ap.c    | 135 +++++++++++++++++++++++++++++++++-------
> >>>>   src/eapol.c |  58 ++++++++++++-----
> >>>>   src/sae.c   | 175
> >>>> +++++++++++++++++++++++++++++++++-------------------
> >>>>   3 files changed, 265 insertions(+), 103 deletions(-)
> >>>>
>

^ permalink raw reply	[flat|nested] 23+ messages in thread

end of thread, other threads:[~2024-04-30 23:27 UTC | newest]

Thread overview: 23+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-04-21 12:50 [PATCH 00/11] Basic SAE support for AP mode John Brandt
2024-04-21 12:50 ` [PATCH 01/11] ap: ability to advertise PSK and SAE John Brandt
2024-04-21 12:50 ` [PATCH 02/11] ap: accept PSK/SAE in auth depending on config John Brandt
2024-04-24 12:05   ` James Prestwood
2024-04-21 12:50 ` [PATCH 03/11] sae: add function sae_set_group John Brandt
2024-04-24 12:05   ` James Prestwood
2024-04-21 12:50 ` [PATCH 04/11] sae: refactor and add function sae_calculate_keys John Brandt
2024-04-24 12:06   ` James Prestwood
2024-04-21 12:50 ` [PATCH 05/11] sae: make sae_process_commit callable in AP mode John Brandt
2024-04-24 12:08   ` James Prestwood
2024-04-21 12:50 ` [PATCH 06/11] sae: verify offered group " John Brandt
2024-04-21 12:50 ` [PATCH 07/11] sae: support reception of Confirm frame by AP John Brandt
2024-04-24 12:08   ` James Prestwood
2024-04-21 12:50 ` [PATCH 08/11] ap: add support to handle SAE authentication John Brandt
2024-04-24 12:06   ` James Prestwood
2024-04-21 12:50 ` [PATCH 09/11] ap: enable start of 4-way HS after SAE John Brandt
2024-04-21 12:50 ` [PATCH 10/11] eapol: support PTK derivation with SHA256 John Brandt
2024-04-21 12:50 ` [PATCH 11/11] eapol: encrypt key data for AKM-defined ciphers John Brandt
2024-04-22 13:52 ` [PATCH 00/11] Basic SAE support for AP mode James Prestwood
2024-04-24 12:07   ` James Prestwood
2024-04-29  0:04     ` John Brandt
2024-04-29 12:00       ` James Prestwood
2024-04-30 23:27         ` KeithG

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).