Re: [RFC v5 0/9] EAPoL over NL80211
Hi, > Okay, that makes things easier. However, it does bring up a > question. Should we be symmetric and remove AP_VLAN as a valid target > of control port frames? E.g. drop NL80211_IFTYPE_AP_VLAN case in > patch 2 of the series. In effect we'd require all control port > frame traffic to go over the master interface. Yeah, I think that'd make sense. > I'm the one confused now. You wanted me to add IFTYPE_IBSS in > nl80211_tx_control_port in your earlier feedback :) Hah. My memory is like a sieve... > Let me try to restate what I said earlier in a different way and see > if it makes things a bit clearer: > > So in patch 9, we set sdata->control_port_over_nl80211 based on > parameters passed into start_ap or mgd_assoc. The > control_port_over_nl80211 flag is passed in cfg80211_crypto_settings > structure that is part of the relevant parameters structure. If > sdata->control_port_over_nl80211 is true, then we actually redirect > the control port frames to nl80211. Right. > So my question is, if we want to support IBSS/MESH, should we: > 1. add the whole cfg80211_crypto_settings to the IBSS/MESH > parameters, > 2. add the control_port_over_nl80211 flag directly to IBSS/MESH > parameters > 3. Pass the flag some other way? > 4. Or drop IBSS/MESH from patch 2 (nl80211_tx_control_port) > completely? I'd say 2 or 4. Having all the crypto settings would be confusing, since they aren't (and possibly cannot) be used. 4 is a bit annoying - not that I remember - because it eventually means that when we do want to support it in IBSS/MESH, we'd have to add another flag indicating that now it's supported, etc. But I think we can live with that too, if it's too complex to add this for real. johannes
Re: [RFC v5 0/9] EAPoL over NL80211
Hi Johannes,
However, it doesn't actually matter at all - we shouldn't get there
with VLAN interface. EAPOL frames are always sent out to the
corresponding AP interface, see ieee80211_rx_h_data:
if (rx->sdata->vif.type == NL80211_IFTYPE_AP_VLAN &&
unlikely(port_control) && sdata->bss) {
sdata = container_of(sdata->bss, struct ieee80211_sub_if_data,
u.ap);
dev = sdata->dev;
rx->sdata = sdata;
}
Okay, that makes things easier. However, it does bring up a question.
Should we be symmetric and remove AP_VLAN as a valid target of control
port frames? E.g. drop NL80211_IFTYPE_AP_VLAN case in patch 2 of the
series. In effect we'd require all control port frame traffic to go
over the master interface.
- JOIN_IBSS & JOIN_MESH don't seem to support control_port_ethertype or
control_port_no_encrypt. Should struct cfg80211_crypto_settings parsed inside
nl80211_crypto_settings be added to ibss_params or mesh_config/mesh_setup?
I don't think it matters - they just don't support this now and don't
really need to.
Except that the eapol over nl80211 flag is being sent in security
settings. This covers STA/AP/P2P_GO/P2P_CLIENT. We need some way of
passing this information for mesh & ibss.
Not sure I understand what you're saying. Can't we just say the flag
isn't permitted in those modes?
I'm the one confused now. You wanted me to add IFTYPE_IBSS in
nl80211_tx_control_port in your earlier feedback :)
Let me try to restate what I said earlier in a different way and see if
it makes things a bit clearer:
So in patch 9, we set sdata->control_port_over_nl80211 based on
parameters passed into start_ap or mgd_assoc. The
control_port_over_nl80211 flag is passed in cfg80211_crypto_settings
structure that is part of the relevant parameters structure. If
sdata->control_port_over_nl80211 is true, then we actually redirect the
control port frames to nl80211.
So my question is, if we want to support IBSS/MESH, should we:
1. add the whole cfg80211_crypto_settings to the IBSS/MESH parameters,
2. add the control_port_over_nl80211 flag directly to IBSS/MESH parameters
3. Pass the flag some other way?
4. Or drop IBSS/MESH from patch 2 (nl80211_tx_control_port) completely?
Regards,
-Denis
Re: [RFC v5 0/9] EAPoL over NL80211
On Wed, 2018-03-21 at 10:18 -0500, Denis Kenzior wrote:
>
> Sorry. I assumed people read the change log :)
And I should :-)
> > > - It is unclear to me how AP_VLAN and AP interfaces should synchronize on
> > > conn_owner_nlportid. This is required for tx_control_port to work.
> >
> > I'm not really sure what you mean? Technically I guess an AP_VLAN could
> > have a different owner from an AP, but if the AP goes down all the
> > AP_VLANs go down with it already anyway.
>
> So the issue is that when mac80211 calls cfg80211_rx_control_port and
> subsequently __nl80211_rx_control_port, we grab the nlportid from the
> wdev. So if that isn't synchronized, then AP_VLAN devices won't be
> sending the EAPoL frames to the right place.
Oh, ok, gotcha. I guess mac80211 would have to sync over the data, just
like it does for other things? You also copied over the
control_port_over_nl80211 setting, so could do the same here?
But no, maybe that doesn't make sense, since ...
Hmm. Ok, I think I see what you're getting at.
The key point seems to be that we don't have any sort of "add VLAN to
AP operation" - it's auto-detected based on the MAC address.
However, it doesn't actually matter at all - we shouldn't get there
with VLAN interface. EAPOL frames are always sent out to the
corresponding AP interface, see ieee80211_rx_h_data:
if (rx->sdata->vif.type == NL80211_IFTYPE_AP_VLAN &&
unlikely(port_control) && sdata->bss) {
sdata = container_of(sdata->bss, struct ieee80211_sub_if_data,
u.ap);
dev = sdata->dev;
rx->sdata = sdata;
}
> > > - JOIN_IBSS & JOIN_MESH don't seem to support control_port_ethertype or
> > > control_port_no_encrypt. Should struct cfg80211_crypto_settings parsed
> > > inside
> > > nl80211_crypto_settings be added to ibss_params or mesh_config/mesh_setup?
> >
> > I don't think it matters - they just don't support this now and don't
> > really need to.
> >
>
> Except that the eapol over nl80211 flag is being sent in security
> settings. This covers STA/AP/P2P_GO/P2P_CLIENT. We need some way of
> passing this information for mesh & ibss.
Not sure I understand what you're saying. Can't we just say the flag
isn't permitted in those modes?
johannes
Re: [RFC v5 0/9] EAPoL over NL80211
Hi Johannes, On 03/21/2018 10:13 AM, Johannes Berg wrote: TODO: That was well-hidden :) Sorry. I assumed people read the change log :) - It is unclear to me how AP_VLAN and AP interfaces should synchronize on conn_owner_nlportid. This is required for tx_control_port to work. I'm not really sure what you mean? Technically I guess an AP_VLAN could have a different owner from an AP, but if the AP goes down all the AP_VLANs go down with it already anyway. So the issue is that when mac80211 calls cfg80211_rx_control_port and subsequently __nl80211_rx_control_port, we grab the nlportid from the wdev. So if that isn't synchronized, then AP_VLAN devices won't be sending the EAPoL frames to the right place. - JOIN_IBSS & JOIN_MESH don't seem to support control_port_ethertype or control_port_no_encrypt. Should struct cfg80211_crypto_settings parsed inside nl80211_crypto_settings be added to ibss_params or mesh_config/mesh_setup? I don't think it matters - they just don't support this now and don't really need to. Except that the eapol over nl80211 flag is being sent in security settings. This covers STA/AP/P2P_GO/P2P_CLIENT. We need some way of passing this information for mesh & ibss. Regards, -Denis
Re: [RFC v5 0/9] EAPoL over NL80211
> TODO: That was well-hidden :) > - It is unclear to me how AP_VLAN and AP interfaces should synchronize on > conn_owner_nlportid. This is required for tx_control_port to work. I'm not really sure what you mean? Technically I guess an AP_VLAN could have a different owner from an AP, but if the AP goes down all the AP_VLANs go down with it already anyway. > - JOIN_IBSS & JOIN_MESH don't seem to support control_port_ethertype or > control_port_no_encrypt. Should struct cfg80211_crypto_settings parsed inside > nl80211_crypto_settings be added to ibss_params or mesh_config/mesh_setup? I don't think it matters - they just don't support this now and don't really need to. johannes
[RFC v5 0/9] EAPoL over NL80211
This patchset adds support for running 802.11 authentication mechanisms (e.g. 802.1X, 4-Way Handshake, etc) over NL80211 instead of putting them onto the network device. This has the advantage of fixing several long-standing race conditions that result from userspace operating on multiple transports in order to manage a 802.11 connection (e.g. NL80211 and wireless netdev, wlan0, etc). For example, userspace would sometimes see 4-Way handshake packets before NL80211 signaled that the connection has been established. Leading to ugly hacks or having the STA wait for retransmissions from the AP. This also provides a way to mitigate a particularly nasty race condition where the encryption key could be set prior to the 4-way handshake packet 4/4 being sent. This would result in the packet being sent encrypted and discarded by the peer. The mitigation strategy for this race is for userspace to explicitly tell the kernel that a particular EAPoL packet should not be encrypted. To make this possible this patchset introduces a new NL80211 command and several new attributes. A userspace that is capable of processing EAPoL packets over NL80211 includes a new NL80211_ATTR_CONTROL_PORT_OVER_NL80211 attribute in its NL80211_CMD_ASSOCIATE or NL80211_CMD_CONNECT requests being sent to the kernel. The previously added NL80211_ATTR_SOCKET_OWNER attribute must also be included. The latter is used by the kernel to send NL80211_CMD_CONTROL_PORT_FRAME notifications back to userspace via a netlink unicast. If the NL80211_ATTR_CONTROL_PORT_OVER_NL80211 attribute is not specified, then legacy behavior is kept and control port packets continue to flow over the network interface. If control port over nl80211 transport is requested, then control port packets are intercepted just prior to being handed to the network device and sent over netlink via the NL80211_CMD_CONTROL_PORT_FRAME notification. NL80211_ATTR_CONTROL_PORT_ETHERTYPE and NL80211_ATTR_MAC are included to specify the control port frame protocol and source address respectively. If the control port frame was received unencrypted then NL80211_ATTR_CONTROL_PORT_NO_ENCRYPT flag is also included. NL80211_ATTR_FRAME attribute contains the raw control port frame with all transport layer headers stripped (e.g. this would be the raw EAPoL frame). Userspace can reply to control port frames either via legacy methods (by sending frames to the network device) or via NL80211_CMD_CONTROL_PORT_FRAME request. Userspace would included NL80211_ATTR_FRAME with the raw control port frame as well as NL80211_Attr_MAC and NL80211_ATTR_CONTROL_PORT_ETHERTYPE attributes to specify the destination address and protocol respectively. This allows Pre-Authentication (protocol 0x88c7) frames to be sent via this mechanism as well. Finally, NL80211_ATTR_CONTROL_PORT_NO_ENCRYPT flag can be included to tell the driver to send the frame unencrypted, e.g. for 4-Way handshake 4/4 frames. The proposed patchset has been tested in a mac80211_hwsim based environment with hostapd and iwd. ChangeLog v5 - Johannes' main comment was that we're not handling interface types other than STATION inside tx_control_port (patch 2). This patch was modified to support all interface types that seemed relevant. - Since tx_control_port relies on wdev->conn_owner_nlportid being set, SOCKET_OWNER support was added to JOIN_IBSS, JOIN_MESH and START_AP - SOCKET_OWNER auto-destruction logic was updated to support interface types other than STATION/P2P_CLIENT - Last patch was modified to support control_port_over_nl80211 for mac80211 based AP mode. It also copies necessary bits for AP_VLAN interfaces. This version has been tested on both STATION and AP mode interfaces with SOCKET_OWNER & CONTROL_PORT_OVER_NL80211 attributes provided to CMD_CONNECT and CMD_START_AP. TODO: - It is unclear to me how AP_VLAN and AP interfaces should synchronize on conn_owner_nlportid. This is required for tx_control_port to work. - JOIN_IBSS & JOIN_MESH don't seem to support control_port_ethertype or control_port_no_encrypt. Should struct cfg80211_crypto_settings parsed inside nl80211_crypto_settings be added to ibss_params or mesh_config/mesh_setup? v4 - Reordered the patches to make sure that: when CONTROL_PORT_OVER_NL80211 is provided by userspace, nl80211 checks that both EXT_FEATURE bit is set and the tx_control_port is present in rdev ops. - Fixed up various issues Johannes found in his review v3 - Added ETH_P_PREAUTH to if_ether.h - Moved NL80211 feature bit from wiphy features to ext features - Addressed various comments from Johannes v2 - Added WIPHY_FLAG_CONTROL_PORT_OVER_NL80211 flag. This is a capability flag used by the drivers, e.g. that the driver supports control port over nl80211 capability. This capability is now checked when CONTROL_PORT_OVER_NL80211 is requested. - mac80211 rx path now forwards Pre-Authentication frames over NL80211 as well, if requested. Tweaked the signature of
