All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/9] eapol: Store IP address in network byte order
@ 2021-08-07  2:10 Andrew Zaborowski
  2021-08-07  2:10 ` [PATCH 2/9] handshake: Add HANDSHAKE_EVENT_IP_REQUEST Andrew Zaborowski
                   ` (8 more replies)
  0 siblings, 9 replies; 14+ messages in thread
From: Andrew Zaborowski @ 2021-08-07  2:10 UTC (permalink / raw)
  To: iwd

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

Switch handshake_state's .client_ip_addr, .subnet_mask and .go_ip_addr
from host byte order to network by order.
---
 src/eapol.c | 12 ++++++------
 src/p2p.c   |  2 +-
 2 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/src/eapol.c b/src/eapol.c
index 5754d79d..f78bef2f 100644
--- a/src/eapol.c
+++ b/src/eapol.c
@@ -1356,11 +1356,11 @@ static void eapol_send_ptk_3_of_4(struct eapol_sm *sm)
 		key_data_buf[key_data_len++] = 4 + 12;
 		l_put_be32(HANDSHAKE_KDE_IP_ADDRESS_ALLOC,
 				key_data_buf + key_data_len + 0);
-		l_put_be32(sm->handshake->client_ip_addr,
+		l_put_u32(sm->handshake->client_ip_addr,
 				key_data_buf + key_data_len + 4);
-		l_put_be32(sm->handshake->subnet_mask,
+		l_put_u32(sm->handshake->subnet_mask,
 				key_data_buf + key_data_len + 8);
-		l_put_be32(sm->handshake->go_ip_addr,
+		l_put_u32(sm->handshake->go_ip_addr,
 				key_data_buf + key_data_len + 12);
 		key_data_len += 4 + 12;
 	}
@@ -1810,9 +1810,9 @@ static void eapol_handle_ptk_3_of_4(struct eapol_sm *sm,
 		hs->support_ip_allocation = ip_alloc_kde != NULL;
 
 		if (ip_alloc_kde) {
-			hs->client_ip_addr = l_get_be32(ip_alloc_kde);
-			hs->subnet_mask = l_get_be32(ip_alloc_kde + 4);
-			hs->go_ip_addr = l_get_be32(ip_alloc_kde + 8);
+			hs->client_ip_addr = l_get_u32(ip_alloc_kde);
+			hs->subnet_mask = l_get_u32(ip_alloc_kde + 4);
+			hs->go_ip_addr = l_get_u32(ip_alloc_kde + 8);
 		} else
 			l_debug("Authenticator ignored our IP Address Request");
 	}
diff --git a/src/p2p.c b/src/p2p.c
index 4c059173..6790caef 100644
--- a/src/p2p.c
+++ b/src/p2p.c
@@ -1441,7 +1441,7 @@ static void p2p_netdev_event(struct netdev *netdev, enum netdev_event event,
 
 static const char *p2p_ip_to_string(uint32_t addr)
 {
-	struct in_addr ia = { .s_addr = L_CPU_TO_BE32(addr) };
+	struct in_addr ia = { .s_addr = addr };
 
 	return inet_ntoa(ia);
 }
-- 
2.30.2

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

* [PATCH 2/9] handshake: Add HANDSHAKE_EVENT_IP_REQUEST
  2021-08-07  2:10 [PATCH 1/9] eapol: Store IP address in network byte order Andrew Zaborowski
@ 2021-08-07  2:10 ` Andrew Zaborowski
  2021-08-14  1:46   ` Denis Kenzior
  2021-08-07  2:10 ` [PATCH 3/9] ap: Make station removal safer Andrew Zaborowski
                   ` (7 subsequent siblings)
  8 siblings, 1 reply; 14+ messages in thread
From: Andrew Zaborowski @ 2021-08-07  2:10 UTC (permalink / raw)
  To: iwd

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

Add a handshake event for use by the AP side for mechanisms that
allocate client IPs during the handshake: P2P address allocation and
FILS address assignment.  This is emitted only when EAPOL or the
auth_proto is actually about to send the network configuration data to
the client so that ap.c can skip allocating a DHCP leases altogether if
the client doesn't send the required KDE or IE.
---
 src/eapol.c     | 8 ++++++++
 src/handshake.h | 1 +
 src/station.c   | 1 +
 3 files changed, 10 insertions(+)

diff --git a/src/eapol.c b/src/eapol.c
index f78bef2f..e2892c0e 100644
--- a/src/eapol.c
+++ b/src/eapol.c
@@ -1350,6 +1350,14 @@ static void eapol_send_ptk_3_of_4(struct eapol_sm *sm)
 		key_data_len += gtk_kde[1] + 2;
 	}
 
+	if (sm->handshake->support_ip_allocation &&
+			!sm->handshake->client_ip_addr) {
+		handshake_event(sm->handshake, HANDSHAKE_EVENT_IP_REQUEST);
+
+		if (!sm->handshake->client_ip_addr)
+			sm->handshake->support_ip_allocation = false;
+	}
+
 	if (sm->handshake->support_ip_allocation) {
 		/* Wi-Fi P2P Technical Specification v1.7 Table 59 */
 		key_data_buf[key_data_len++] = IE_TYPE_VENDOR_SPECIFIC;
diff --git a/src/handshake.h b/src/handshake.h
index a4c54b5a..1bc1b2fa 100644
--- a/src/handshake.h
+++ b/src/handshake.h
@@ -57,6 +57,7 @@ enum handshake_event {
 	HANDSHAKE_EVENT_REKEY_FAILED,
 	HANDSHAKE_EVENT_EAP_NOTIFY,
 	HANDSHAKE_EVENT_TRANSITION_DISABLE,
+	HANDSHAKE_EVENT_IP_REQUEST,
 };
 
 typedef void (*handshake_event_func_t)(struct handshake_state *hs,
diff --git a/src/station.c b/src/station.c
index 3db0e65a..803db184 100644
--- a/src/station.c
+++ b/src/station.c
@@ -725,6 +725,7 @@ static void station_handshake_event(struct handshake_state *hs,
 	case HANDSHAKE_EVENT_COMPLETE:
 	case HANDSHAKE_EVENT_SETTING_KEYS_FAILED:
 	case HANDSHAKE_EVENT_EAP_NOTIFY:
+	case HANDSHAKE_EVENT_IP_REQUEST:
 		/*
 		 * currently we don't care about any other events. The
 		 * netdev_connect_cb will notify us when the connection is
-- 
2.30.2

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

* [PATCH 3/9] ap: Make station removal safer
  2021-08-07  2:10 [PATCH 1/9] eapol: Store IP address in network byte order Andrew Zaborowski
  2021-08-07  2:10 ` [PATCH 2/9] handshake: Add HANDSHAKE_EVENT_IP_REQUEST Andrew Zaborowski
@ 2021-08-07  2:10 ` Andrew Zaborowski
  2021-08-07  2:10 ` [PATCH 4/9] ap: Fix an invalid access in ap_write_wsc_ie Andrew Zaborowski
                   ` (6 subsequent siblings)
  8 siblings, 0 replies; 14+ messages in thread
From: Andrew Zaborowski @ 2021-08-07  2:10 UTC (permalink / raw)
  To: iwd

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

Replace instances of the ap_del_station() +
ap_sta_free()/ap_remove_sta() with calls to ap_station_disconnect to
make sure we consistently remove the station from the ap->sta_states
queue before using ap_del_station().  ap_del_station() may generate an
event to the ap.h API user (e.g. P2P) and this may end up tearing down
the AP completely.

For that scenario we also don't want ap_sta_free() to access sta->ap so
we make sure ap_del_station() performs these cleanup steps so that
ap_sta_free() has nothing to do that accesses sta->ap.
---
 src/ap.c | 28 ++++++++++++----------------
 1 file changed, 12 insertions(+), 16 deletions(-)

diff --git a/src/ap.c b/src/ap.c
index f78863ad..51cb5602 100644
--- a/src/ap.c
+++ b/src/ap.c
@@ -251,6 +251,11 @@ static void ap_del_station(struct sta_state *sta, uint16_t reason,
 		sta->rsna = false;
 	}
 
+	if (sta->assoc_resp_cmd_id) {
+		l_genl_family_cancel(ap->nl80211, sta->assoc_resp_cmd_id);
+		sta->assoc_resp_cmd_id = 0;
+	}
+
 	if (sta->gtk_query_cmd_id) {
 		l_genl_family_cancel(ap->nl80211, sta->gtk_query_cmd_id);
 		sta->gtk_query_cmd_id = 0;
@@ -258,6 +263,10 @@ static void ap_del_station(struct sta_state *sta, uint16_t reason,
 
 	ap_stop_handshake(sta);
 
+	/*
+	 * If the event handler tears the AP down, we've made sure above that
+	 * a subsequent ap_sta_free(sta) has no need to access sta->ap.
+	 */
 	if (send_event)
 		ap->ops->handle_event(AP_EVENT_STATION_REMOVED, &event_data,
 					ap->user_data);
@@ -2003,7 +2012,6 @@ static void ap_deauth_cb(const struct mmpdu_header *hdr, const void *body,
 				size_t body_len, int rssi, void *user_data)
 {
 	struct ap_state *ap = user_data;
-	struct sta_state *sta;
 	const struct mmpdu_deauthentication *deauth = body;
 	const uint8_t *bssid = netdev_get_address(ap->netdev);
 
@@ -2015,14 +2023,8 @@ static void ap_deauth_cb(const struct mmpdu_header *hdr, const void *body,
 			memcmp(hdr->address_3, bssid, 6))
 		return;
 
-	sta = l_queue_remove_if(ap->sta_states, ap_sta_match_addr,
-				hdr->address_2);
-	if (!sta)
-		return;
-
-	ap_del_station(sta, L_LE16_TO_CPU(deauth->reason_code), false);
-
-	ap_sta_free(sta);
+	ap_station_disconnect(ap, hdr->address_2,
+				L_LE16_TO_CPU(deauth->reason_code));
 }
 
 static void do_debug(const char *str, void *user_data)
@@ -2338,7 +2340,6 @@ cleanup:
 
 static void ap_handle_del_station(struct ap_state *ap, struct l_genl_msg *msg)
 {
-	struct sta_state *sta;
 	struct l_genl_attr attr;
 	uint16_t type;
 	uint16_t len;
@@ -2365,12 +2366,7 @@ static void ap_handle_del_station(struct ap_state *ap, struct l_genl_msg *msg)
 		}
 	}
 
-	sta = l_queue_find(ap->sta_states, ap_sta_match_addr, mac);
-	if (!sta)
-		return;
-
-	ap_del_station(sta, reason, true);
-	ap_remove_sta(sta);
+	ap_station_disconnect(ap, mac, reason);
 }
 
 static void ap_mlme_notify(struct l_genl_msg *msg, void *user_data)
-- 
2.30.2

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

* [PATCH 4/9] ap: Fix an invalid access in ap_write_wsc_ie
  2021-08-07  2:10 [PATCH 1/9] eapol: Store IP address in network byte order Andrew Zaborowski
  2021-08-07  2:10 ` [PATCH 2/9] handshake: Add HANDSHAKE_EVENT_IP_REQUEST Andrew Zaborowski
  2021-08-07  2:10 ` [PATCH 3/9] ap: Make station removal safer Andrew Zaborowski
@ 2021-08-07  2:10 ` Andrew Zaborowski
  2021-08-07  2:10 ` [PATCH 5/9] ap: Implement P2P GO-side 4-way handshake IP Allocation Andrew Zaborowski
                   ` (5 subsequent siblings)
  8 siblings, 0 replies; 14+ messages in thread
From: Andrew Zaborowski @ 2021-08-07  2:10 UTC (permalink / raw)
  To: iwd

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

client_frame is not valid for a beacon frame as beacons are not sent in
response to another frame.  Move the access to client_frame->address_2
to the conditional blocks for Probe Response and Association Response
frames.
---
 src/ap.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/src/ap.c b/src/ap.c
index 51cb5602..a16200df 100644
--- a/src/ap.c
+++ b/src/ap.c
@@ -535,7 +535,6 @@ static size_t ap_write_wsc_ie(struct ap_state *ap,
 				size_t client_frame_len,
 				uint8_t *out_buf)
 {
-	const uint8_t *from = client_frame->address_2;
 	uint8_t *wsc_data;
 	size_t wsc_data_size;
 	uint8_t *wsc_ie;
@@ -544,6 +543,7 @@ static size_t ap_write_wsc_ie(struct ap_state *ap,
 
 	/* WSC IE */
 	if (type == MPDU_MANAGEMENT_SUBTYPE_PROBE_RESPONSE) {
+		const uint8_t *from = client_frame->address_2;
 		struct wsc_probe_response wsc_pr = {};
 		const struct mmpdu_probe_request *req =
 			mmpdu_body(client_frame);
@@ -606,6 +606,7 @@ static size_t ap_write_wsc_ie(struct ap_state *ap,
 		wsc_data = wsc_build_beacon(&wsc_beacon, &wsc_data_size);
 	} else if (L_IN_SET(type, MPDU_MANAGEMENT_SUBTYPE_ASSOCIATION_RESPONSE,
 			MPDU_MANAGEMENT_SUBTYPE_REASSOCIATION_RESPONSE)) {
+		const uint8_t *from = client_frame->address_2;
 		struct wsc_association_response wsc_resp = {};
 		struct sta_state *sta =
 			l_queue_find(ap->sta_states, ap_sta_match_addr, from);
-- 
2.30.2

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

* [PATCH 5/9] ap: Implement P2P GO-side 4-way handshake IP Allocation
  2021-08-07  2:10 [PATCH 1/9] eapol: Store IP address in network byte order Andrew Zaborowski
                   ` (2 preceding siblings ...)
  2021-08-07  2:10 ` [PATCH 4/9] ap: Fix an invalid access in ap_write_wsc_ie Andrew Zaborowski
@ 2021-08-07  2:10 ` Andrew Zaborowski
  2021-08-07  2:10 ` [PATCH 6/9] autotests: Test GO-side IP Allocation in testP2P Andrew Zaborowski
                   ` (4 subsequent siblings)
  8 siblings, 0 replies; 14+ messages in thread
From: Andrew Zaborowski @ 2021-08-07  2:10 UTC (permalink / raw)
  To: iwd

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

Use the struct handshake_state::support_ip_allocation field already
supported in eapol.c authenticator side to enable the P2P IP Allocation
mechanism in ap.c.  Add the P2P_GROUP_CAP_IP_ALLOCATION bit in P2P group
capabilities to signal the feature is now supported.

There's no harm in enabling this feature in every AP (not just P2P Group
Owner) but the clients won't know whether we support it other than
through that P2P-specific group capability bit.
---
 src/ap.c  | 58 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
 src/p2p.c |  1 +
 2 files changed, 59 insertions(+)

diff --git a/src/ap.c b/src/ap.c
index a16200df..052cd0c3 100644
--- a/src/ap.c
+++ b/src/ap.c
@@ -118,6 +118,7 @@ struct sta_state {
 	struct l_settings *wsc_settings;
 	uint8_t wsc_uuid_e[16];
 	bool wsc_v2;
+	struct l_dhcp_lease *ip_alloc_lease;
 };
 
 struct ap_wsc_pbc_probe_record {
@@ -177,6 +178,10 @@ static void ap_sta_free(void *data)
 	if (sta->gtk_query_cmd_id)
 		l_genl_family_cancel(ap->nl80211, sta->gtk_query_cmd_id);
 
+	if (sta->ip_alloc_lease && ap->netconfig_dhcp)
+		l_dhcp_server_lease_remove(ap->netconfig_dhcp,
+						sta->ip_alloc_lease);
+
 	ap_stop_handshake(sta);
 
 	l_free(sta);
@@ -839,6 +844,12 @@ static uint32_t ap_send_mgmt_frame(struct ap_state *ap,
 					callback, user_data, NULL, NULL);
 }
 
+#define IP4_FROM_STR(str)						\
+	(__extension__ ({						\
+		struct in_addr ia;					\
+		inet_pton(AF_INET, str, &ia) == 1 ? ia.s_addr : 0;	\
+	}))
+
 static void ap_start_handshake(struct sta_state *sta, bool use_eapol_start,
 				const uint8_t *gtk_rsc)
 {
@@ -863,6 +874,9 @@ static void ap_start_handshake(struct sta_state *sta, bool use_eapol_start,
 		handshake_state_set_gtk(sta->hs, sta->ap->gtk,
 					sta->ap->gtk_index, gtk_rsc);
 
+	if (ap->netconfig_dhcp)
+		sta->hs->support_ip_allocation = true;
+
 	sta->sm = eapol_sm_new(sta->hs);
 	if (!sta->sm) {
 		ap_stop_handshake(sta);
@@ -886,12 +900,28 @@ static void ap_handshake_event(struct handshake_state *hs,
 		enum handshake_event event, void *user_data, ...)
 {
 	struct sta_state *sta = user_data;
+	struct ap_state *ap = sta->ap;
 	va_list args;
 
 	va_start(args, user_data);
 
 	switch (event) {
 	case HANDSHAKE_EVENT_COMPLETE:
+		if (sta->ip_alloc_lease) {
+			/*
+			 * Move the lease from offered to active state if the
+			 * client has actually used it.  In any case drop our
+			 * reference to the lease, the server owns the lease
+			 * and if we want to keep our reference we'd need to
+			 * react to relevant server events.
+			 */
+			if (hs->support_ip_allocation)
+				l_dhcp_server_request(ap->netconfig_dhcp,
+							sta->ip_alloc_lease);
+
+			sta->ip_alloc_lease = NULL;
+		}
+
 		ap_new_rsna(sta);
 		break;
 	case HANDSHAKE_EVENT_FAILED:
@@ -900,6 +930,34 @@ static void ap_handshake_event(struct handshake_state *hs,
 	case HANDSHAKE_EVENT_SETTING_KEYS_FAILED:
 		sta->sm = NULL;
 		ap_remove_sta(sta);
+		break;
+	case HANDSHAKE_EVENT_IP_REQUEST:
+	{
+		L_AUTO_FREE_VAR(char *, lease_addr_str) = NULL;
+		L_AUTO_FREE_VAR(char *, lease_netmask_str) = NULL;
+		char own_addr_str[INET_ADDRSTRLEN];
+
+		if (!sta->ip_alloc_lease)
+			sta->ip_alloc_lease = l_dhcp_server_discover(
+							ap->netconfig_dhcp,
+							0, NULL, sta->addr);
+
+		if (!sta->ip_alloc_lease) {
+			l_error("l_dhcp_server_discover() failed, see "
+				"IWD_DHCP_DEBUG output");
+			break;
+		}
+
+		lease_addr_str = l_dhcp_lease_get_address(sta->ip_alloc_lease);
+		lease_netmask_str =
+			l_dhcp_lease_get_netmask(sta->ip_alloc_lease);
+		l_rtnl_address_get_address(ap->netconfig_addr4, own_addr_str);
+
+		sta->hs->client_ip_addr = IP4_FROM_STR(lease_addr_str);
+		sta->hs->subnet_mask = IP4_FROM_STR(lease_netmask_str);
+		sta->hs->go_ip_addr = IP4_FROM_STR(own_addr_str);
+		break;
+	}
 	default:
 		break;
 	}
diff --git a/src/p2p.c b/src/p2p.c
index 6790caef..8170bbe0 100644
--- a/src/p2p.c
+++ b/src/p2p.c
@@ -1276,6 +1276,7 @@ static void p2p_group_start(struct p2p_device *dev)
 
 	dev->capability.group_caps |= P2P_GROUP_CAP_GO;
 	dev->capability.group_caps |= P2P_GROUP_CAP_GROUP_FORMATION;
+	dev->capability.group_caps |= P2P_GROUP_CAP_IP_ALLOCATION;
 
 	dev->group = ap_start(dev->conn_netdev, config, &p2p_go_ops, NULL, dev);
 	l_settings_free(config);
-- 
2.30.2

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

* [PATCH 6/9] autotests: Test GO-side IP Allocation in testP2P
  2021-08-07  2:10 [PATCH 1/9] eapol: Store IP address in network byte order Andrew Zaborowski
                   ` (3 preceding siblings ...)
  2021-08-07  2:10 ` [PATCH 5/9] ap: Implement P2P GO-side 4-way handshake IP Allocation Andrew Zaborowski
@ 2021-08-07  2:10 ` Andrew Zaborowski
  2021-08-07  2:10 ` [PATCH 7/9] ap: Expire client's leases on disconnect Andrew Zaborowski
                   ` (3 subsequent siblings)
  8 siblings, 0 replies; 14+ messages in thread
From: Andrew Zaborowski @ 2021-08-07  2:10 UTC (permalink / raw)
  To: iwd

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

Check if our wpa_supplicant P2P Client has received the allocated
Clieant IP/netmask/GO IP values we sent in the 4-Way Handshake.
---
 autotests/testP2P/connection_test.py | 9 ++++++++-
 autotests/util/wpas.py               | 1 +
 2 files changed, 9 insertions(+), 1 deletion(-)

diff --git a/autotests/testP2P/connection_test.py b/autotests/testP2P/connection_test.py
index 22871547..52da0edc 100644
--- a/autotests/testP2P/connection_test.py
+++ b/autotests/testP2P/connection_test.py
@@ -91,6 +91,7 @@ class Test(unittest.TestCase):
 
         wd.wait_for_object_condition(wpas, 'obj.p2p_group is not None', max_wait=3)
         peer_ifname = wpas.p2p_group['ifname']
+        self.assertEqual(wpas.p2p_group['role'], 'GO' if not go else 'client')
 
         if not go:
             ctx.start_process(['ifconfig', peer_ifname, '192.168.1.20', 'netmask', '255.255.255.0'], wait=True)
@@ -102,6 +103,9 @@ class Test(unittest.TestCase):
             client = wpas.p2p_clients[request['peer_iface']]
             self.assertEqual(client['p2p_dev_addr'], wpas_peer['p2p_dev_addr'])
         else:
+            self.assertEqual(wpas.p2p_group['ip_addr'], '192.168.1.2')
+            self.assertEqual(wpas.p2p_group['ip_mask'], '255.255.255.240')
+            self.assertEqual(wpas.p2p_group['go_ip_addr'], '192.168.1.1')
             dhcp = ctx.start_process(['dhclient', '-v', '-d', '--no-pid', '-cf', '/dev/null', '-lf', '/tmp/dhcp.leases',
                 '-sf', '/tmp/dhclient-script', peer_ifname])
             self.dhcp = dhcp
@@ -136,7 +140,10 @@ class Test(unittest.TestCase):
 
     def tearDown(self):
         if self.p2p is not None:
-            self.p2p.enabled = False
+            try:
+                self.p2p.enabled = False
+            except:
+                pass
         if self.wpas is not None:
             self.wpas.clean_up()
             self.wpas = None
diff --git a/autotests/util/wpas.py b/autotests/util/wpas.py
index 1758eede..8fb725f8 100644
--- a/autotests/util/wpas.py
+++ b/autotests/util/wpas.py
@@ -159,6 +159,7 @@ class Wpas:
         elif event['event'] == 'P2P-GROUP-STARTED':
             event.pop('event')
             event['ifname'] = event.pop('arg1')
+            event['role'] = event.pop('arg2')
             self.p2p_group = event
         elif event['event'] == 'P2P-GROUP-REMOVED':
             self.p2p_group = None
-- 
2.30.2

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

* [PATCH 7/9] ap: Expire client's leases on disconnect
  2021-08-07  2:10 [PATCH 1/9] eapol: Store IP address in network byte order Andrew Zaborowski
                   ` (4 preceding siblings ...)
  2021-08-07  2:10 ` [PATCH 6/9] autotests: Test GO-side IP Allocation in testP2P Andrew Zaborowski
@ 2021-08-07  2:10 ` Andrew Zaborowski
  2021-08-07  2:10 ` [PATCH 8/9] ie: Add FILS IP Address Assignment parsers and builders Andrew Zaborowski
                   ` (2 subsequent siblings)
  8 siblings, 0 replies; 14+ messages in thread
From: Andrew Zaborowski @ 2021-08-07  2:10 UTC (permalink / raw)
  To: iwd

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

If netconfig is enabled tell the DHCP server to expire any leases owned
by the client that is disconnecting by using l_dhcp_server_expire_by_mac
to add the IPs back to the IP pool.  They're added to the expired list
so they'd only be used if there are no other addresses left in the pool
and can be reactivated if the client comes back before the address is
used by somebody else.

This should ensure that we're always able to offer an address to a new
client when there are fewer concurrent clients than addresses in the
configured subnet or IP range.
---
 src/ap.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/src/ap.c b/src/ap.c
index 052cd0c3..09fb2467 100644
--- a/src/ap.c
+++ b/src/ap.c
@@ -268,6 +268,17 @@ static void ap_del_station(struct sta_state *sta, uint16_t reason,
 
 	ap_stop_handshake(sta);
 
+	/*
+	 * Expire any DHCP leases owned by this client when it disconnects to
+	 * make it harder for somebody to DoS the IP pool.  If the client
+	 * comes back and the lease is still in the expired leases list they
+	 * will get their IP back.
+	 */
+	if (ap->netconfig_dhcp) {
+		sta->ip_alloc_lease = NULL;
+		l_dhcp_server_expire_by_mac(ap->netconfig_dhcp, sta->addr);
+	}
+
 	/*
 	 * If the event handler tears the AP down, we've made sure above that
 	 * a subsequent ap_sta_free(sta) has no need to access sta->ap.
-- 
2.30.2

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

* [PATCH 8/9] ie: Add FILS IP Address Assignment parsers and builders
  2021-08-07  2:10 [PATCH 1/9] eapol: Store IP address in network byte order Andrew Zaborowski
                   ` (5 preceding siblings ...)
  2021-08-07  2:10 ` [PATCH 7/9] ap: Expire client's leases on disconnect Andrew Zaborowski
@ 2021-08-07  2:10 ` Andrew Zaborowski
  2021-08-14  2:08   ` Denis Kenzior
  2021-08-07  2:10 ` [PATCH 9/9] ap: Support FILS IP Address Assignment IE Andrew Zaborowski
  2021-08-13 15:50 ` [PATCH 1/9] eapol: Store IP address in network byte order Denis Kenzior
  8 siblings, 1 reply; 14+ messages in thread
From: Andrew Zaborowski @ 2021-08-07  2:10 UTC (permalink / raw)
  To: iwd

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

---
 src/ie.c | 336 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
 src/ie.h |  38 +++++++
 2 files changed, 374 insertions(+)

diff --git a/src/ie.c b/src/ie.c
index a73d5bbc..27950025 100644
--- a/src/ie.c
+++ b/src/ie.c
@@ -25,6 +25,7 @@
 #endif
 
 #include <errno.h>
+#include <arpa/inet.h>
 
 #include <ell/ell.h>
 
@@ -2103,3 +2104,338 @@ bool ie_rsnxe_capable(const uint8_t *rsnxe, unsigned int bit)
 
 	return test_bit(rsnxe + 2, bit);
 }
+
+int ie_parse_fils_ip_addr_request(struct ie_tlv_iter *iter,
+				struct ie_fils_ip_addr_request_info *out)
+{
+	unsigned int len = ie_tlv_iter_get_length(iter);
+	const uint8_t *data = ie_tlv_iter_get_data(iter);
+	struct ie_fils_ip_addr_request_info info = {};
+	bool ipv4_specific_addr = false;
+	bool ipv6_specific_addr = false;
+
+	if (len < 1)
+		return -EMSGSIZE;
+
+	if (bit_field(data[0], 0, 2) == 1)
+		return -EINVAL;
+	else if (bit_field(data[0], 0, 2) >= 2) {
+		info.ipv4 = true;
+		ipv4_specific_addr = (bit_field(data[0], 0, 2) == 3);
+	}
+
+	if (bit_field(data[0], 2, 2) == 1)
+		return -EINVAL;
+	else if (bit_field(data[0], 2, 2) >= 2) {
+		info.ipv6 = true;
+		ipv6_specific_addr = (bit_field(data[0], 2, 2) == 3);
+	}
+
+	info.dns = test_bit(data++, 4);
+
+	if (len < 1 + (ipv4_specific_addr ? 4u : 0u) +
+			(ipv4_specific_addr ? 16u : 0u))
+		return -EMSGSIZE;
+
+	if (ipv4_specific_addr) {
+		info.ipv4_requested_addr = l_get_u32(data);
+		data += 4;
+
+		if (!info.ipv4_requested_addr)
+			return -EINVAL;
+	}
+
+	if (ipv6_specific_addr) {
+		memcpy(info.ipv6_requested_addr, data, 16);
+		data += 16;
+
+		if (l_memeqzero(info.ipv6_requested_addr, 16))
+			return -EINVAL;
+	}
+
+	memcpy(out, &info, sizeof(info));
+	return 0;
+}
+
+void ie_build_fils_ip_addr_request(
+				const struct ie_fils_ip_addr_request_info *info,
+				uint8_t *to)
+{
+	uint8_t *len;
+
+	*to++ = IE_TYPE_EXTENSION;
+	len = to++;
+	*to++ = IE_TYPE_FILS_IP_ADDRESS & 0xff;
+	*to++ = ((info->ipv4 ? info->ipv4_requested_addr ? 3 : 2 : 0) << 0) |
+		((info->ipv6 ? !l_memeqzero(info->ipv6_requested_addr, 16) ?
+		  3 : 2 : 0) << 2) |
+		((info->dns ? 1 : 0) << 4);
+
+	if (info->ipv4_requested_addr) {
+		l_put_u32(info->ipv4_requested_addr, to);
+		to += 4;
+	}
+
+	if (!l_memeqzero(info->ipv6_requested_addr, 16)) {
+		memcpy(to, info->ipv6_requested_addr, 16);
+		to += 16;
+	}
+
+	*len = to - (len + 1);
+}
+
+int ie_parse_fils_ip_addr_response(struct ie_tlv_iter *iter,
+				struct ie_fils_ip_addr_response_info *out)
+{
+	unsigned int len = ie_tlv_iter_get_length(iter);
+	const uint8_t *data = ie_tlv_iter_get_data(iter);
+	struct ie_fils_ip_addr_response_info info = {};
+	const uint8_t *response_ctrl;
+	const uint8_t *dns_ctrl;
+
+	if (len < 2)
+		return -EMSGSIZE;
+
+	info.response_pending = test_bit(data, 0);
+
+	if (info.response_pending) {
+		info.response_timeout = bit_field(data[0], 1, 6); /* seconds */
+		return 0;
+	}
+
+	response_ctrl = data++;
+	dns_ctrl = data++;
+	len -= 2;
+
+	if (test_bit(response_ctrl, 1)) {
+		uint32_t netmask;
+
+		if (len < 8)
+			return -EMSGSIZE;
+
+		info.ipv4_addr = l_get_u32(data);
+		netmask = l_get_be32(data + 4);
+		info.ipv4_prefix_len = __builtin_popcount(netmask);
+		data += 8;
+		len -= 8;
+
+		if (!info.ipv4_addr || info.ipv4_prefix_len > 30 || netmask !=
+				util_netmask_from_prefix(info.ipv4_prefix_len))
+			return -EINVAL;
+	}
+
+	if (test_bit(response_ctrl, 2)) {
+		if (len < 10)
+			return -EMSGSIZE;
+
+		info.ipv4_gateway = l_get_u32(data);
+		memcpy(info.ipv4_gateway_mac, data + 4, 6);
+		data += 10;
+		len -= 10;
+
+		/* Check gateway is on the same subnet */
+		if (info.ipv4_addr && (ntohl(info.ipv4_addr ^ info.ipv4_gateway) &
+				util_netmask_from_prefix(info.ipv4_prefix_len)))
+			return -EINVAL;
+	}
+
+	if (test_bit(response_ctrl, 3)) {
+		if (len < 17)
+			return -EMSGSIZE;
+
+		memcpy(info.ipv6_addr, data, 16);
+		info.ipv6_prefix_len = data[16];
+		data += 17;
+		len -= 17;
+
+		if (l_memeqzero(info.ipv6_addr, 16) ||
+				info.ipv6_prefix_len > 126)
+			return -EINVAL;
+	}
+
+	if (test_bit(response_ctrl, 4)) {
+		if (len < 22)
+			return -EMSGSIZE;
+
+		memcpy(info.ipv6_gateway, data, 16);
+		memcpy(info.ipv6_gateway_mac, data + 16, 6);
+		data += 22;
+		len -= 22;
+
+		/* Check gateway is on the same subnet */
+		if (!l_memeqzero(info.ipv6_addr, 12)) {
+			int n = info.ipv6_prefix_len / 8;
+			uint8_t mask = (1 << (info.ipv6_prefix_len & 7)) - 1;
+
+			if (n && memcmp(info.ipv6_addr, info.ipv6_gateway, n))
+				return -EINVAL;
+
+			if (mask && ((info.ipv6_addr[n] ^
+						info.ipv6_gateway[n]) & mask))
+				return -EINVAL;
+		}
+	}
+
+	if (test_bit(response_ctrl, 5)) {
+		if (len < 1)
+			return -EMSGSIZE;
+
+		info.ipv4_lifetime = *data++;
+		len--;
+	}
+
+	if (test_bit(response_ctrl, 6)) {
+		if (len < 1)
+			return -EMSGSIZE;
+
+		info.ipv6_lifetime = *data++;
+		len--;
+	}
+
+	if (test_bit(dns_ctrl, 0)) {
+		if (len < 4)
+			return -EMSGSIZE;
+
+		info.ipv4_dns = l_get_u32(data);
+		data += 4;
+		len -= 4;
+
+		if (!info.ipv4_dns)
+			return -EINVAL;
+	}
+
+	if (test_bit(dns_ctrl, 1)) {
+		if (len < 16)
+			return -EMSGSIZE;
+
+		memcpy(info.ipv6_dns, data, 16);
+		data += 16;
+		len -= 16;
+
+		if (l_memeqzero(info.ipv6_dns, 16))
+			return -EINVAL;
+	}
+
+	if (test_bit(dns_ctrl, 2)) {
+		if (len < 6)
+			return -EMSGSIZE;
+
+		memcpy(info.ipv4_dns_mac, data, 6);
+		data += 6;
+		len -= 6;
+	}
+
+	if (test_bit(dns_ctrl, 3)) {
+		if (len < 6)
+			return -EMSGSIZE;
+
+		memcpy(info.ipv6_dns_mac, data, 6);
+		data += 6;
+		len -= 6;
+	}
+
+	memcpy(out, &info, sizeof(info));
+	return 0;
+}
+
+void ie_build_fils_ip_addr_response(
+			const struct ie_fils_ip_addr_response_info *info,
+			uint8_t *to)
+{
+	uint8_t *len;
+	uint8_t *response_ctrl;
+	uint8_t *dns_ctrl;
+
+	*to++ = IE_TYPE_EXTENSION;
+	len = to++;
+	*to++ = IE_TYPE_FILS_IP_ADDRESS & 0xff;
+	response_ctrl = to++;
+	dns_ctrl = to++;
+
+	*response_ctrl = 0;
+	*dns_ctrl = 0;
+
+	if (info->response_pending) {
+		*response_ctrl |= 1 << 0;
+		*response_ctrl |= info->response_timeout << 1;
+		goto done;
+	}
+
+	if (info->ipv4_addr) {
+		uint32_t netmask =
+			util_netmask_from_prefix(info->ipv4_prefix_len);
+
+		*response_ctrl |= 1 << 1;
+
+		l_put_u32(info->ipv4_addr, to);
+		l_put_u32(htonl(netmask), to + 4);
+		to += 8;
+	}
+
+	if (info->ipv4_gateway) {
+		*response_ctrl |= 1 << 2;
+
+		l_put_u32(info->ipv4_gateway, to);
+		memcpy(to + 4, info->ipv4_gateway_mac, 6);
+		to += 10;
+	}
+
+	if (!l_memeqzero(info->ipv6_addr, 16)) {
+		*response_ctrl |= 1 << 3;
+
+		memcpy(to, info->ipv6_addr, 16);
+		to[16] = info->ipv6_prefix_len;
+		to += 17;
+	}
+
+	if (!l_memeqzero(info->ipv6_gateway, 16)) {
+		*response_ctrl |= 1 << 4;
+
+		memcpy(to, info->ipv6_gateway, 16);
+		memcpy(to + 16, info->ipv6_gateway_mac, 6);
+		to += 22;
+	}
+
+	if (info->ipv4_lifetime) {
+		*response_ctrl |= 1 << 5;
+
+		*to++ = info->ipv4_lifetime;
+	}
+
+	if (info->ipv6_lifetime) {
+		*response_ctrl |= 1 << 6;
+
+		*to++ = info->ipv6_lifetime;
+	}
+
+	if (info->ipv4_dns) {
+		*dns_ctrl |= 1 << 0;
+
+		l_put_u32(info->ipv4_dns, to);
+		to += 4;
+	}
+
+	if (!l_memeqzero(info->ipv6_dns, 16)) {
+		*dns_ctrl |= 1 << 1;
+
+		memcpy(to, info->ipv6_dns, 16);
+		to += 16;
+	}
+
+	if (!l_memeqzero(info->ipv4_dns_mac, 6)) {
+		*dns_ctrl |= 1 << 2;
+
+		memcpy(to, info->ipv4_dns_mac, 6);
+		to += 6;
+	}
+
+	if (!l_memeqzero(info->ipv6_dns_mac, 6)) {
+		*dns_ctrl |= 1 << 3;
+
+		memcpy(to, info->ipv6_dns_mac, 6);
+		to += 6;
+	}
+
+done:
+	*len = to - (len + 1);
+}
diff --git a/src/ie.h b/src/ie.h
index 25b56302..05afce39 100644
--- a/src/ie.h
+++ b/src/ie.h
@@ -459,6 +459,33 @@ struct ie_neighbor_report_info {
 	bool bss_transition_pref_present : 1;
 };
 
+struct ie_fils_ip_addr_request_info {
+	bool ipv4 : 1;
+	uint32_t ipv4_requested_addr;		/* Zero if none */
+	bool ipv6 : 1;
+	uint8_t ipv6_requested_addr[16];	/* Zero if none */
+	bool dns : 1;
+};
+
+struct ie_fils_ip_addr_response_info {
+	bool response_pending : 1;
+	uint8_t response_timeout;	/* Seconds */
+	uint32_t ipv4_addr;		/* Zero if not provided */
+	uint8_t ipv4_prefix_len;
+	uint32_t ipv4_gateway;		/* Zero if not provided */
+	uint8_t ipv4_gateway_mac[6];
+	uint32_t ipv4_dns;		/* Zero if not provided */
+	uint8_t ipv4_dns_mac[6];	/* Zero if not provided */
+	uint8_t ipv4_lifetime;		/* Zero if not provided */
+	uint8_t ipv6_addr[16];		/* Zero if not provided */
+	uint8_t ipv6_prefix_len;
+	uint8_t ipv6_gateway[16];	/* Zero if not provided */
+	uint8_t ipv6_gateway_mac[6];
+	uint8_t ipv6_dns[16];		/* Zero if not provided */
+	uint8_t ipv6_dns_mac[6];	/* Zero if not provided */
+	uint8_t ipv6_lifetime;		/* Zero if not provided */
+};
+
 extern const unsigned char ieee_oui[3];
 extern const unsigned char microsoft_oui[3];
 extern const unsigned char wifi_alliance_oui[3];
@@ -586,3 +613,14 @@ enum ie_rsnx_capability {
 };
 
 bool ie_rsnxe_capable(const uint8_t *rsnxe, unsigned int bit);
+
+int ie_parse_fils_ip_addr_request(struct ie_tlv_iter *iter,
+				struct ie_fils_ip_addr_request_info *out);
+void ie_build_fils_ip_addr_request(
+				const struct ie_fils_ip_addr_request_info *info,
+				uint8_t *to);
+int ie_parse_fils_ip_addr_response(struct ie_tlv_iter *iter,
+				struct ie_fils_ip_addr_response_info *out);
+void ie_build_fils_ip_addr_response(
+			const struct ie_fils_ip_addr_response_info *info,
+			uint8_t *to);
-- 
2.30.2

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

* [PATCH 9/9] ap: Support FILS IP Address Assignment IE
  2021-08-07  2:10 [PATCH 1/9] eapol: Store IP address in network byte order Andrew Zaborowski
                   ` (6 preceding siblings ...)
  2021-08-07  2:10 ` [PATCH 8/9] ie: Add FILS IP Address Assignment parsers and builders Andrew Zaborowski
@ 2021-08-07  2:10 ` Andrew Zaborowski
  2021-08-13 15:50 ` [PATCH 1/9] eapol: Store IP address in network byte order Denis Kenzior
  8 siblings, 0 replies; 14+ messages in thread
From: Andrew Zaborowski @ 2021-08-07  2:10 UTC (permalink / raw)
  To: iwd

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

Handle the 802.11ai FILS IP Address Assignment IEs in Association
Request frames when netconfig is enabled.  Only IPv4 is supported.
Like the P2P IP Allocation mechanism, since the payload format and logic
is independent form the rest of the FILS standard this is enable
unconditionally for clients who want to use it even though we don't
actually do FILS in AP mode.
---
 src/ap.c | 114 +++++++++++++++++++++++++++++++++++++++++++++++--------
 1 file changed, 98 insertions(+), 16 deletions(-)

diff --git a/src/ap.c b/src/ap.c
index 09fb2467..a7f41814 100644
--- a/src/ap.c
+++ b/src/ap.c
@@ -119,6 +119,7 @@ struct sta_state {
 	uint8_t wsc_uuid_e[16];
 	bool wsc_v2;
 	struct l_dhcp_lease *ip_alloc_lease;
+	bool ip_alloc_sent;
 };
 
 struct ap_wsc_pbc_probe_record {
@@ -907,6 +908,25 @@ error:
 	ap_del_station(sta, MMPDU_REASON_CODE_UNSPECIFIED, true);
 }
 
+static bool ap_sta_get_dhcp4_lease(struct sta_state *sta)
+{
+	if (sta->ip_alloc_lease)
+		return true;
+
+	if (!sta->ap->netconfig_dhcp)
+		return false;
+
+	sta->ip_alloc_lease = l_dhcp_server_discover(sta->ap->netconfig_dhcp,
+							0, NULL, sta->addr);
+	if (!sta->ip_alloc_lease) {
+		l_error("l_dhcp_server_discover() failed, see IWD_DHCP_DEBUG "
+			"output");
+		return false;
+	}
+
+	return true;
+}
+
 static void ap_handshake_event(struct handshake_state *hs,
 		enum handshake_event event, void *user_data, ...)
 {
@@ -919,6 +939,9 @@ static void ap_handshake_event(struct handshake_state *hs,
 	switch (event) {
 	case HANDSHAKE_EVENT_COMPLETE:
 		if (sta->ip_alloc_lease) {
+			if (hs->support_ip_allocation)
+				sta->ip_alloc_sent = true;
+
 			/*
 			 * Move the lease from offered to active state if the
 			 * client has actually used it.  In any case drop our
@@ -926,7 +949,7 @@ static void ap_handshake_event(struct handshake_state *hs,
 			 * and if we want to keep our reference we'd need to
 			 * react to relevant server events.
 			 */
-			if (hs->support_ip_allocation)
+			if (sta->ip_alloc_sent)
 				l_dhcp_server_request(ap->netconfig_dhcp,
 							sta->ip_alloc_lease);
 
@@ -948,16 +971,8 @@ static void ap_handshake_event(struct handshake_state *hs,
 		L_AUTO_FREE_VAR(char *, lease_netmask_str) = NULL;
 		char own_addr_str[INET_ADDRSTRLEN];
 
-		if (!sta->ip_alloc_lease)
-			sta->ip_alloc_lease = l_dhcp_server_discover(
-							ap->netconfig_dhcp,
-							0, NULL, sta->addr);
-
-		if (!sta->ip_alloc_lease) {
-			l_error("l_dhcp_server_discover() failed, see "
-				"IWD_DHCP_DEBUG output");
+		if (!ap_sta_get_dhcp4_lease(sta))
 			break;
-		}
 
 		lease_addr_str = l_dhcp_lease_get_address(sta->ip_alloc_lease);
 		lease_netmask_str =
@@ -1377,14 +1392,16 @@ static uint32_t ap_assoc_resp(struct ap_state *ap, struct sta_state *sta,
 				const uint8_t *dest,
 				enum mmpdu_reason_code status_code,
 				bool reassoc, const struct mmpdu_header *req,
-				size_t req_len, frame_xchg_cb_t callback)
+				size_t req_len,
+				const struct ie_fils_ip_addr_request_info *
+				ip_req_info, frame_xchg_cb_t callback)
 {
 	const uint8_t *addr = netdev_get_address(ap->netdev);
 	enum mpdu_management_subtype stype = reassoc ?
 		MPDU_MANAGEMENT_SUBTYPE_REASSOCIATION_RESPONSE :
 		MPDU_MANAGEMENT_SUBTYPE_ASSOCIATION_RESPONSE;
 	L_AUTO_FREE_VAR(uint8_t *, mpdu_buf) =
-		l_malloc(128 + ap_get_extra_ies_len(ap, stype, req, req_len));
+		l_malloc(256 + ap_get_extra_ies_len(ap, stype, req, req_len));
 	struct mmpdu_header *mpdu = (void *) mpdu_buf;
 	struct mmpdu_association_response *resp;
 	size_t ies_len = 0;
@@ -1430,6 +1447,57 @@ static uint32_t ap_assoc_resp(struct ap_state *ap, struct sta_state *sta,
 	ies_len += ap_write_extra_ies(ap, stype, req, req_len,
 					resp->ies + ies_len);
 
+	if (ip_req_info) {
+		struct ie_fils_ip_addr_response_info ip_resp_info = {};
+
+		if (ip_req_info->ipv4 && sta && ap_sta_get_dhcp4_lease(sta)) {
+			L_AUTO_FREE_VAR(char *, lease_addr_str) =
+				l_dhcp_lease_get_address(sta->ip_alloc_lease);
+			L_AUTO_FREE_VAR(char *, lease_netmask_str) =
+				l_dhcp_lease_get_netmask(sta->ip_alloc_lease);
+			uint32_t lease_lifetime =
+				l_dhcp_lease_get_lifetime(sta->ip_alloc_lease);
+			L_AUTO_FREE_VAR(char *, lease_gateway_str) =
+				l_dhcp_lease_get_gateway(sta->ip_alloc_lease);
+			char **lease_dns_str_list =
+				l_dhcp_lease_get_dns(sta->ip_alloc_lease);
+
+			ip_resp_info.ipv4_addr = IP4_FROM_STR(lease_addr_str);
+			ip_resp_info.ipv4_prefix_len =
+				__builtin_popcount(IP4_FROM_STR(
+							lease_netmask_str));
+
+			if (lease_lifetime != 0xffffffff)
+				ip_resp_info.ipv4_lifetime = lease_lifetime;
+
+			if (lease_gateway_str)
+				ip_resp_info.ipv4_gateway =
+					IP4_FROM_STR(lease_gateway_str);
+
+			if (lease_dns_str_list && lease_dns_str_list[0])
+				ip_resp_info.ipv4_dns =
+					IP4_FROM_STR(lease_dns_str_list[0]);
+
+			l_strv_free(lease_dns_str_list);
+			sta->ip_alloc_sent = true;
+		} else if (ip_req_info->ipv4 || ip_req_info->ipv6) {
+			/*
+			 * 802.11ai-2016 Section 11.47.3.3: "If the AP is unable
+			 * to assign an IP address in the (Re)Association
+			 * Response frame, then the AP sets the IP address
+			 * assignment pending flag in the IP Address Response
+			 * Control field of the FILS IP Address Assignment
+			 * element to 1 and sets the IP address request timeout
+			 * to 0 in (Re)Association Response frame."
+			 */
+			ip_resp_info.response_pending = 1;
+			ip_resp_info.response_timeout = 0;
+		}
+
+		ie_build_fils_ip_addr_response(&ip_resp_info, resp->ies + ies_len);
+		ies_len += 2 + resp->ies[ies_len + 1];
+	}
+
 	return ap_send_mgmt_frame(ap, mpdu, resp->ies + ies_len - mpdu_buf,
 					callback, sta);
 }
@@ -1518,6 +1586,8 @@ static void ap_assoc_reassoc(struct sta_state *sta, bool reassoc,
 	struct ie_tlv_iter iter;
 	uint8_t *wsc_data = NULL;
 	ssize_t wsc_data_len;
+	bool fils_ip_req = false;
+	struct ie_fils_ip_addr_request_info fils_ip_req_info;
 
 	if (sta->assoc_resp_cmd_id)
 		return;
@@ -1564,6 +1634,17 @@ static void ap_assoc_reassoc(struct sta_state *sta, bool reassoc,
 
 			rsn = (const uint8_t *) ie_tlv_iter_get_data(&iter) - 2;
 			break;
+
+		case IE_TYPE_FILS_IP_ADDRESS:
+			if (fils_ip_req || ie_parse_fils_ip_addr_request(&iter,
+						&fils_ip_req_info) < 0) {
+				l_debug("Can't parse FILS IP Address Assignment"
+					" IE, ignoring it");
+				break;
+			}
+
+			fils_ip_req = true;
+			break;
 		}
 
 	if (!rates || !ssid || (!wsc_data && !rsn) ||
@@ -1696,7 +1777,8 @@ static void ap_assoc_reassoc(struct sta_state *sta, bool reassoc,
 
 	sta->assoc_resp_cmd_id = ap_assoc_resp(ap, sta, sta->addr, 0, reassoc,
 						req, (void *) ies + ies_len -
-						(void *) req,
+						(void *) req, fils_ip_req ?
+						&fils_ip_req_info : NULL,
 						ap_success_assoc_resp_cb);
 	if (!sta->assoc_resp_cmd_id)
 		l_error("Sending success (Re)Association Response failed");
@@ -1729,7 +1811,7 @@ bad_frame:
 
 	if (!ap_assoc_resp(ap, sta, sta->addr, err, reassoc,
 				req, (void *) ies + ies_len - (void *) req,
-				ap_fail_assoc_resp_cb))
+				NULL, ap_fail_assoc_resp_cb))
 		l_error("Sending error (Re)Association Response failed");
 }
 
@@ -1754,7 +1836,7 @@ static void ap_assoc_req_cb(const struct mmpdu_header *hdr, const void *body,
 		if (!ap_assoc_resp(ap, NULL, from,
 				MMPDU_REASON_CODE_STA_REQ_ASSOC_WITHOUT_AUTH,
 				false, hdr, body + body_len - (void *) hdr,
-				ap_fail_assoc_resp_cb))
+				NULL, ap_fail_assoc_resp_cb))
 			l_error("Sending error Association Response failed");
 
 		return;
@@ -1802,7 +1884,7 @@ static void ap_reassoc_req_cb(const struct mmpdu_header *hdr, const void *body,
 bad_frame:
 	if (!ap_assoc_resp(ap, NULL, from, err, true,
 				hdr, body + body_len - (void *) hdr,
-				ap_fail_assoc_resp_cb))
+				NULL, ap_fail_assoc_resp_cb))
 		l_error("Sending error Reassociation Response failed");
 }
 
-- 
2.30.2

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

* Re: [PATCH 1/9] eapol: Store IP address in network byte order
  2021-08-07  2:10 [PATCH 1/9] eapol: Store IP address in network byte order Andrew Zaborowski
                   ` (7 preceding siblings ...)
  2021-08-07  2:10 ` [PATCH 9/9] ap: Support FILS IP Address Assignment IE Andrew Zaborowski
@ 2021-08-13 15:50 ` Denis Kenzior
  8 siblings, 0 replies; 14+ messages in thread
From: Denis Kenzior @ 2021-08-13 15:50 UTC (permalink / raw)
  To: iwd

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

Hi Andrew,

On 8/6/21 9:10 PM, Andrew Zaborowski wrote:
> Switch handshake_state's .client_ip_addr, .subnet_mask and .go_ip_addr
> from host byte order to network by order.
> ---
>   src/eapol.c | 12 ++++++------
>   src/p2p.c   |  2 +-
>   2 files changed, 7 insertions(+), 7 deletions(-)
> 

Patches 1, 3 and 4 applied, thanks.

Regards,
-Denis

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

* Re: [PATCH 2/9] handshake: Add HANDSHAKE_EVENT_IP_REQUEST
  2021-08-07  2:10 ` [PATCH 2/9] handshake: Add HANDSHAKE_EVENT_IP_REQUEST Andrew Zaborowski
@ 2021-08-14  1:46   ` Denis Kenzior
  2021-08-14  3:00     ` Andrew Zaborowski
  0 siblings, 1 reply; 14+ messages in thread
From: Denis Kenzior @ 2021-08-14  1:46 UTC (permalink / raw)
  To: iwd

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

Hi Andrew,

On 8/6/21 9:10 PM, Andrew Zaborowski wrote:
> Add a handshake event for use by the AP side for mechanisms that
> allocate client IPs during the handshake: P2P address allocation and
> FILS address assignment.  This is emitted only when EAPOL or the
> auth_proto is actually about to send the network configuration data to
> the client so that ap.c can skip allocating a DHCP leases altogether if
> the client doesn't send the required KDE or IE.
> ---
>   src/eapol.c     | 8 ++++++++
>   src/handshake.h | 1 +
>   src/station.c   | 1 +
>   3 files changed, 10 insertions(+)
> 
> diff --git a/src/eapol.c b/src/eapol.c
> index f78bef2f..e2892c0e 100644
> --- a/src/eapol.c
> +++ b/src/eapol.c
> @@ -1350,6 +1350,14 @@ static void eapol_send_ptk_3_of_4(struct eapol_sm *sm)
>   		key_data_len += gtk_kde[1] + 2;
>   	}
>   
> +	if (sm->handshake->support_ip_allocation &&
> +			!sm->handshake->client_ip_addr) {
> +		handshake_event(sm->handshake, HANDSHAKE_EVENT_IP_REQUEST);
> +
> +		if (!sm->handshake->client_ip_addr)

Isn't this condition already checked above?

> +			sm->handshake->support_ip_allocation = false;

What happens on retransmissions of PTK 3/4?

> +	}
> +
>   	if (sm->handshake->support_ip_allocation) {
>   		/* Wi-Fi P2P Technical Specification v1.7 Table 59 */
>   		key_data_buf[key_data_len++] = IE_TYPE_VENDOR_SPECIFIC;
> diff --git a/src/handshake.h b/src/handshake.h
> index a4c54b5a..1bc1b2fa 100644
> --- a/src/handshake.h
> +++ b/src/handshake.h
> @@ -57,6 +57,7 @@ enum handshake_event {
>   	HANDSHAKE_EVENT_REKEY_FAILED,
>   	HANDSHAKE_EVENT_EAP_NOTIFY,
>   	HANDSHAKE_EVENT_TRANSITION_DISABLE,
> +	HANDSHAKE_EVENT_IP_REQUEST,

Do you want to distinguish between P2P IP request and FILS IP Request?  Might 
make things a bit more explicit ?

>   };
>   

Regards,
-Denis

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

* Re: [PATCH 8/9] ie: Add FILS IP Address Assignment parsers and builders
  2021-08-07  2:10 ` [PATCH 8/9] ie: Add FILS IP Address Assignment parsers and builders Andrew Zaborowski
@ 2021-08-14  2:08   ` Denis Kenzior
  0 siblings, 0 replies; 14+ messages in thread
From: Denis Kenzior @ 2021-08-14  2:08 UTC (permalink / raw)
  To: iwd

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

Hi Andrew,

On 8/6/21 9:10 PM, Andrew Zaborowski wrote:
> ---
>   src/ie.c | 336 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
>   src/ie.h |  38 +++++++
>   2 files changed, 374 insertions(+)
> 
> diff --git a/src/ie.c b/src/ie.c
> index a73d5bbc..27950025 100644
> --- a/src/ie.c
> +++ b/src/ie.c
> @@ -25,6 +25,7 @@
>   #endif
>   
>   #include <errno.h>
> +#include <arpa/inet.h>
>   
>   #include <ell/ell.h>
>   
> @@ -2103,3 +2104,338 @@ bool ie_rsnxe_capable(const uint8_t *rsnxe, unsigned int bit)
>   
>   	return test_bit(rsnxe + 2, bit);
>   }
> +
> +int ie_parse_fils_ip_addr_request(struct ie_tlv_iter *iter,
> +				struct ie_fils_ip_addr_request_info *out)
> +{
> +	unsigned int len = ie_tlv_iter_get_length(iter);
> +	const uint8_t *data = ie_tlv_iter_get_data(iter);
> +	struct ie_fils_ip_addr_request_info info = {};
> +	bool ipv4_specific_addr = false;
> +	bool ipv6_specific_addr = false;
> +
> +	if (len < 1)
> +		return -EMSGSIZE;
> +
> +	if (bit_field(data[0], 0, 2) == 1)
> +		return -EINVAL;
> +	else if (bit_field(data[0], 0, 2) >= 2) {
> +		info.ipv4 = true;
> +		ipv4_specific_addr = (bit_field(data[0], 0, 2) == 3);
> +	}
> +
> +	if (bit_field(data[0], 2, 2) == 1)
> +		return -EINVAL;
> +	else if (bit_field(data[0], 2, 2) >= 2) {
> +		info.ipv6 = true;
> +		ipv6_specific_addr = (bit_field(data[0], 2, 2) == 3);
> +	}
> +
> +	info.dns = test_bit(data++, 4);
> +
> +	if (len < 1 + (ipv4_specific_addr ? 4u : 0u) +
> +			(ipv4_specific_addr ? 16u : 0u))

ipv6_specific_addr?

> +		return -EMSGSIZE;
> +
> +	if (ipv4_specific_addr) {
> +		info.ipv4_requested_addr = l_get_u32(data);
> +		data += 4;
> +
> +		if (!info.ipv4_requested_addr)
> +			return -EINVAL;
> +	}
> +
> +	if (ipv6_specific_addr) {
> +		memcpy(info.ipv6_requested_addr, data, 16);
> +		data += 16;
> +
> +		if (l_memeqzero(info.ipv6_requested_addr, 16))
> +			return -EINVAL;
> +	}
> +
> +	memcpy(out, &info, sizeof(info));
> +	return 0;
> +}
> +
> +void ie_build_fils_ip_addr_request(
> +				const struct ie_fils_ip_addr_request_info *info,
> +				uint8_t *to)
> +{
> +	uint8_t *len;
> +
> +	*to++ = IE_TYPE_EXTENSION;
> +	len = to++;
> +	*to++ = IE_TYPE_FILS_IP_ADDRESS & 0xff;
> +	*to++ = ((info->ipv4 ? info->ipv4_requested_addr ? 3 : 2 : 0) << 0) |
> +		((info->ipv6 ? !l_memeqzero(info->ipv6_requested_addr, 16) ?
> +		  3 : 2 : 0) << 2) |
> +		((info->dns ? 1 : 0) << 4);

I think it would look a lot nicer if you used set_bit instead.  Maybe avoid an 
extra l_memeqzero too...

> +
> +	if (info->ipv4_requested_addr) {
> +		l_put_u32(info->ipv4_requested_addr, to);
> +		to += 4;
> +	}
> +
> +	if (!l_memeqzero(info->ipv6_requested_addr, 16)) {
> +		memcpy(to, info->ipv6_requested_addr, 16);
> +		to += 16;
> +	}
> +
> +	*len = to - (len + 1);
> +}
> +
> +int ie_parse_fils_ip_addr_response(struct ie_tlv_iter *iter,
> +				struct ie_fils_ip_addr_response_info *out)
> +{
> +	unsigned int len = ie_tlv_iter_get_length(iter);
> +	const uint8_t *data = ie_tlv_iter_get_data(iter);
> +	struct ie_fils_ip_addr_response_info info = {};
> +	const uint8_t *response_ctrl;
> +	const uint8_t *dns_ctrl;
> +
> +	if (len < 2)
> +		return -EMSGSIZE;
> +
> +	info.response_pending = test_bit(data, 0);
> +
> +	if (info.response_pending) {
> +		info.response_timeout = bit_field(data[0], 1, 6); /* seconds */
> +		return 0;
> +	}
> +
> +	response_ctrl = data++;
> +	dns_ctrl = data++;
> +	len -= 2;
> +
> +	if (test_bit(response_ctrl, 1)) {
> +		uint32_t netmask;
> +
> +		if (len < 8)
> +			return -EMSGSIZE;
> +
> +		info.ipv4_addr = l_get_u32(data);
> +		netmask = l_get_be32(data + 4);
> +		info.ipv4_prefix_len = __builtin_popcount(netmask);
> +		data += 8;
> +		len -= 8;
> +
> +		if (!info.ipv4_addr || info.ipv4_prefix_len > 30 || netmask !=
> +				util_netmask_from_prefix(info.ipv4_prefix_len))
> +			return -EINVAL;
> +	}
> +
> +	if (test_bit(response_ctrl, 2)) {
> +		if (len < 10)
> +			return -EMSGSIZE;
> +
> +		info.ipv4_gateway = l_get_u32(data);
> +		memcpy(info.ipv4_gateway_mac, data + 4, 6);
> +		data += 10;
> +		len -= 10;
> +
> +		/* Check gateway is on the same subnet */
> +		if (info.ipv4_addr && (ntohl(info.ipv4_addr ^ info.ipv4_gateway) &
> +				util_netmask_from_prefix(info.ipv4_prefix_len)))
> +			return -EINVAL;
> +	}
> +
> +	if (test_bit(response_ctrl, 3)) {
> +		if (len < 17)
> +			return -EMSGSIZE;
> +
> +		memcpy(info.ipv6_addr, data, 16);
> +		info.ipv6_prefix_len = data[16];
> +		data += 17;
> +		len -= 17;
> +
> +		if (l_memeqzero(info.ipv6_addr, 16) ||
> +				info.ipv6_prefix_len > 126)
> +			return -EINVAL;
> +	}
> +
> +	if (test_bit(response_ctrl, 4)) {
> +		if (len < 22)
> +			return -EMSGSIZE;
> +
> +		memcpy(info.ipv6_gateway, data, 16);
> +		memcpy(info.ipv6_gateway_mac, data + 16, 6);
> +		data += 22;
> +		len -= 22;
> +
> +		/* Check gateway is on the same subnet */
> +		if (!l_memeqzero(info.ipv6_addr, 12)) {
> +			int n = info.ipv6_prefix_len / 8;
> +			uint8_t mask = (1 << (info.ipv6_prefix_len & 7)) - 1;
> +
> +			if (n && memcmp(info.ipv6_addr, info.ipv6_gateway, n))
> +				return -EINVAL;
> +
> +			if (mask && ((info.ipv6_addr[n] ^
> +						info.ipv6_gateway[n]) & mask))
> +				return -EINVAL;
> +		}
> +	}
> +
> +	if (test_bit(response_ctrl, 5)) {
> +		if (len < 1)
> +			return -EMSGSIZE;
> +
> +		info.ipv4_lifetime = *data++;
> +		len--;
> +	}
> +
> +	if (test_bit(response_ctrl, 6)) {
> +		if (len < 1)
> +			return -EMSGSIZE;
> +
> +		info.ipv6_lifetime = *data++;
> +		len--;
> +	}
> +
> +	if (test_bit(dns_ctrl, 0)) {
> +		if (len < 4)
> +			return -EMSGSIZE;
> +
> +		info.ipv4_dns = l_get_u32(data);
> +		data += 4;
> +		len -= 4;
> +
> +		if (!info.ipv4_dns)
> +			return -EINVAL;
> +	}
> +
> +	if (test_bit(dns_ctrl, 1)) {
> +		if (len < 16)
> +			return -EMSGSIZE;
> +
> +		memcpy(info.ipv6_dns, data, 16);
> +		data += 16;
> +		len -= 16;
> +
> +		if (l_memeqzero(info.ipv6_dns, 16))
> +			return -EINVAL;
> +	}
> +
> +	if (test_bit(dns_ctrl, 2)) {
> +		if (len < 6)
> +			return -EMSGSIZE;
> +
> +		memcpy(info.ipv4_dns_mac, data, 6);
> +		data += 6;
> +		len -= 6;

Can we come up with a clever macro that would automate this somewhat?  Similar 
to RSNE_ADVANCE maybe?

> +	}
> +
> +	if (test_bit(dns_ctrl, 3)) {
> +		if (len < 6)
> +			return -EMSGSIZE;
> +
> +		memcpy(info.ipv6_dns_mac, data, 6);
> +		data += 6;
> +		len -= 6;
> +	}
> +
> +	memcpy(out, &info, sizeof(info));
> +	return 0;
> +}
> +
> +void ie_build_fils_ip_addr_response(
> +			const struct ie_fils_ip_addr_response_info *info,
> +			uint8_t *to)
> +{
> +	uint8_t *len;
> +	uint8_t *response_ctrl;
> +	uint8_t *dns_ctrl;
> +
> +	*to++ = IE_TYPE_EXTENSION;
> +	len = to++;
> +	*to++ = IE_TYPE_FILS_IP_ADDRESS & 0xff;
> +	response_ctrl = to++;
> +	dns_ctrl = to++;
> +
> +	*response_ctrl = 0;
> +	*dns_ctrl = 0;
> +
> +	if (info->response_pending) {
> +		*response_ctrl |= 1 << 0;
> +		*response_ctrl |= info->response_timeout << 1;
> +		goto done;
> +	}
> +
> +	if (info->ipv4_addr) {
> +		uint32_t netmask =
> +			util_netmask_from_prefix(info->ipv4_prefix_len);
> +
> +		*response_ctrl |= 1 << 1;

set_bit please.  May even want to make a private enum for the bits too for 
easier reading.  Similar to enum ie_rsnx_capability maybe?

> +
> +		l_put_u32(info->ipv4_addr, to);
> +		l_put_u32(htonl(netmask), to + 4);
> +		to += 8;
> +	}
> +
> +	if (info->ipv4_gateway) {
> +		*response_ctrl |= 1 << 2;
> +
> +		l_put_u32(info->ipv4_gateway, to);
> +		memcpy(to + 4, info->ipv4_gateway_mac, 6);
> +		to += 10;
> +	}
> +
> +	if (!l_memeqzero(info->ipv6_addr, 16)) {
> +		*response_ctrl |= 1 << 3;
> +
> +		memcpy(to, info->ipv6_addr, 16);
> +		to[16] = info->ipv6_prefix_len;
> +		to += 17;
> +	}
> +
> +	if (!l_memeqzero(info->ipv6_gateway, 16)) {
> +		*response_ctrl |= 1 << 4;
> +
> +		memcpy(to, info->ipv6_gateway, 16);
> +		memcpy(to + 16, info->ipv6_gateway_mac, 6);
> +		to += 22;
> +	}
> +
> +	if (info->ipv4_lifetime) {
> +		*response_ctrl |= 1 << 5;
> +
> +		*to++ = info->ipv4_lifetime;
> +	}
> +
> +	if (info->ipv6_lifetime) {
> +		*response_ctrl |= 1 << 6;
> +
> +		*to++ = info->ipv6_lifetime;
> +	}
> +
> +	if (info->ipv4_dns) {
> +		*dns_ctrl |= 1 << 0;
> +
> +		l_put_u32(info->ipv4_dns, to);
> +		to += 4;
> +	}
> +
> +	if (!l_memeqzero(info->ipv6_dns, 16)) {
> +		*dns_ctrl |= 1 << 1;
> +
> +		memcpy(to, info->ipv6_dns, 16);
> +		to += 16;
> +	}
> +
> +	if (!l_memeqzero(info->ipv4_dns_mac, 6)) {
> +		*dns_ctrl |= 1 << 2;
> +
> +		memcpy(to, info->ipv4_dns_mac, 6);
> +		to += 6;
> +	}
> +
> +	if (!l_memeqzero(info->ipv6_dns_mac, 6)) {
> +		*dns_ctrl |= 1 << 3;
> +
> +		memcpy(to, info->ipv6_dns_mac, 6);
> +		to += 6;
> +	}
> +
> +done:
> +	*len = to - (len + 1);
> +}
> diff --git a/src/ie.h b/src/ie.h
> index 25b56302..05afce39 100644
> --- a/src/ie.h
> +++ b/src/ie.h
> @@ -459,6 +459,33 @@ struct ie_neighbor_report_info {
>   	bool bss_transition_pref_present : 1;
>   };
>   
> +struct ie_fils_ip_addr_request_info {
> +	bool ipv4 : 1;
> +	uint32_t ipv4_requested_addr;		/* Zero if none */
> +	bool ipv6 : 1;
> +	uint8_t ipv6_requested_addr[16];	/* Zero if none */
> +	bool dns : 1;
> +};
> +
> +struct ie_fils_ip_addr_response_info {
> +	bool response_pending : 1;
> +	uint8_t response_timeout;	/* Seconds */
> +	uint32_t ipv4_addr;		/* Zero if not provided */
> +	uint8_t ipv4_prefix_len;
> +	uint32_t ipv4_gateway;		/* Zero if not provided */
> +	uint8_t ipv4_gateway_mac[6];
> +	uint32_t ipv4_dns;		/* Zero if not provided */
> +	uint8_t ipv4_dns_mac[6];	/* Zero if not provided */
> +	uint8_t ipv4_lifetime;		/* Zero if not provided */
> +	uint8_t ipv6_addr[16];		/* Zero if not provided */
> +	uint8_t ipv6_prefix_len;
> +	uint8_t ipv6_gateway[16];	/* Zero if not provided */
> +	uint8_t ipv6_gateway_mac[6];
> +	uint8_t ipv6_dns[16];		/* Zero if not provided */
> +	uint8_t ipv6_dns_mac[6];	/* Zero if not provided */
> +	uint8_t ipv6_lifetime;		/* Zero if not provided */
> +};
> +
>   extern const unsigned char ieee_oui[3];
>   extern const unsigned char microsoft_oui[3];
>   extern const unsigned char wifi_alliance_oui[3];
> @@ -586,3 +613,14 @@ enum ie_rsnx_capability {
>   };
>   
>   bool ie_rsnxe_capable(const uint8_t *rsnxe, unsigned int bit);
> +
> +int ie_parse_fils_ip_addr_request(struct ie_tlv_iter *iter,
> +				struct ie_fils_ip_addr_request_info *out);
> +void ie_build_fils_ip_addr_request(
> +				const struct ie_fils_ip_addr_request_info *info,
> +				uint8_t *to);
> +int ie_parse_fils_ip_addr_response(struct ie_tlv_iter *iter,
> +				struct ie_fils_ip_addr_response_info *out);
> +void ie_build_fils_ip_addr_response(
> +			const struct ie_fils_ip_addr_response_info *info,
> +			uint8_t *to);
> 

Regards,
-Denis

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

* Re: [PATCH 2/9] handshake: Add HANDSHAKE_EVENT_IP_REQUEST
  2021-08-14  1:46   ` Denis Kenzior
@ 2021-08-14  3:00     ` Andrew Zaborowski
  2021-08-14  3:31       ` Denis Kenzior
  0 siblings, 1 reply; 14+ messages in thread
From: Andrew Zaborowski @ 2021-08-14  3:00 UTC (permalink / raw)
  To: iwd

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

On Sat, 14 Aug 2021 at 03:58, Denis Kenzior <denkenz@gmail.com> wrote:
> On 8/6/21 9:10 PM, Andrew Zaborowski wrote:
> > Add a handshake event for use by the AP side for mechanisms that
> > allocate client IPs during the handshake: P2P address allocation and
> > FILS address assignment.  This is emitted only when EAPOL or the
> > auth_proto is actually about to send the network configuration data to
> > the client so that ap.c can skip allocating a DHCP leases altogether if
> > the client doesn't send the required KDE or IE.
> > ---
> >   src/eapol.c     | 8 ++++++++
> >   src/handshake.h | 1 +
> >   src/station.c   | 1 +
> >   3 files changed, 10 insertions(+)
> >
> > diff --git a/src/eapol.c b/src/eapol.c
> > index f78bef2f..e2892c0e 100644
> > --- a/src/eapol.c
> > +++ b/src/eapol.c
> > @@ -1350,6 +1350,14 @@ static void eapol_send_ptk_3_of_4(struct eapol_sm *sm)
> >               key_data_len += gtk_kde[1] + 2;
> >       }
> >
> > +     if (sm->handshake->support_ip_allocation &&
> > +                     !sm->handshake->client_ip_addr) {
> > +             handshake_event(sm->handshake, HANDSHAKE_EVENT_IP_REQUEST);
> > +
> > +             if (!sm->handshake->client_ip_addr)
>
> Isn't this condition already checked above?

The intention was for the handshake_event handler to try to set this,
so this is expected to have changed since then.  But if it's still
NULL then we assume there was some problem with the IP allocation.

>
> > +                     sm->handshake->support_ip_allocation = false;
>
> What happens on retransmissions of PTK 3/4?

We'll have either set sm->handshake->client_ip_addr or cleared
sm->handshake->support_ip_allocation so we shouldn't enter this code
block again and the values won't change during the resends.

>
> > +     }
> > +
> >       if (sm->handshake->support_ip_allocation) {
> >               /* Wi-Fi P2P Technical Specification v1.7 Table 59 */
> >               key_data_buf[key_data_len++] = IE_TYPE_VENDOR_SPECIFIC;
> > diff --git a/src/handshake.h b/src/handshake.h
> > index a4c54b5a..1bc1b2fa 100644
> > --- a/src/handshake.h
> > +++ b/src/handshake.h
> > @@ -57,6 +57,7 @@ enum handshake_event {
> >       HANDSHAKE_EVENT_REKEY_FAILED,
> >       HANDSHAKE_EVENT_EAP_NOTIFY,
> >       HANDSHAKE_EVENT_TRANSITION_DISABLE,
> > +     HANDSHAKE_EVENT_IP_REQUEST,
>
> Do you want to distinguish between P2P IP request and FILS IP Request?  Might
> make things a bit more explicit ?

I thought the API could be the same for both (granted FILS sends more
bits of data from the lease, but I was thinking eventually we'd store
a whole struct l_dhcp_lease object in the handshake_state, replacing
client_ip_addr, subnet_mask and go_ip_addr).

In reality, on the authenticator side we only create a struct
handshake_state after the association, so this only applies for P2P, I
can put P2P in the event name.

Best regards

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

* Re: [PATCH 2/9] handshake: Add HANDSHAKE_EVENT_IP_REQUEST
  2021-08-14  3:00     ` Andrew Zaborowski
@ 2021-08-14  3:31       ` Denis Kenzior
  0 siblings, 0 replies; 14+ messages in thread
From: Denis Kenzior @ 2021-08-14  3:31 UTC (permalink / raw)
  To: iwd

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

Hi Andrew,

>>> +     if (sm->handshake->support_ip_allocation &&
>>> +                     !sm->handshake->client_ip_addr) {
>>> +             handshake_event(sm->handshake, HANDSHAKE_EVENT_IP_REQUEST);
>>> +
>>> +             if (!sm->handshake->client_ip_addr)
>>
>> Isn't this condition already checked above?
> 
> The intention was for the handshake_event handler to try to set this,
> so this is expected to have changed since then.  But if it's still
> NULL then we assume there was some problem with the IP allocation.
> 

This really begs for a comment then.

>> Do you want to distinguish between P2P IP request and FILS IP Request?  Might
>> make things a bit more explicit ?
> 
> I thought the API could be the same for both (granted FILS sends more
> bits of data from the lease, but I was thinking eventually we'd store
> a whole struct l_dhcp_lease object in the handshake_state, replacing
> client_ip_addr, subnet_mask and go_ip_addr).
> 
> In reality, on the authenticator side we only create a struct
> handshake_state after the association, so this only applies for P2P, I
> can put P2P in the event name.

Sounds like adding P2P to the event name would make things clearer.

Regards,
-Denis

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

end of thread, other threads:[~2021-08-14  3:31 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-08-07  2:10 [PATCH 1/9] eapol: Store IP address in network byte order Andrew Zaborowski
2021-08-07  2:10 ` [PATCH 2/9] handshake: Add HANDSHAKE_EVENT_IP_REQUEST Andrew Zaborowski
2021-08-14  1:46   ` Denis Kenzior
2021-08-14  3:00     ` Andrew Zaborowski
2021-08-14  3:31       ` Denis Kenzior
2021-08-07  2:10 ` [PATCH 3/9] ap: Make station removal safer Andrew Zaborowski
2021-08-07  2:10 ` [PATCH 4/9] ap: Fix an invalid access in ap_write_wsc_ie Andrew Zaborowski
2021-08-07  2:10 ` [PATCH 5/9] ap: Implement P2P GO-side 4-way handshake IP Allocation Andrew Zaborowski
2021-08-07  2:10 ` [PATCH 6/9] autotests: Test GO-side IP Allocation in testP2P Andrew Zaborowski
2021-08-07  2:10 ` [PATCH 7/9] ap: Expire client's leases on disconnect Andrew Zaborowski
2021-08-07  2:10 ` [PATCH 8/9] ie: Add FILS IP Address Assignment parsers and builders Andrew Zaborowski
2021-08-14  2:08   ` Denis Kenzior
2021-08-07  2:10 ` [PATCH 9/9] ap: Support FILS IP Address Assignment IE Andrew Zaborowski
2021-08-13 15:50 ` [PATCH 1/9] eapol: Store IP address in network byte order Denis Kenzior

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.