EAPoL over NL80211
Subsystems (3)
v3 cover 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
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 cfg80211_rx_control_port.
- TX path reworked completely. tx_control_port method has been introduced to cfg80211_ops. An implementation of tx_control_port for mac80211 was added.
Denis Kenzior (6):
uapi: Add 802.11 Preauthentication to if_ether
nl80211: Add CONTROL_PORT_OVER_NL80211 attribute
nl80211: Add CMD_CONTROL_PORT_FRAME API
mac80211: Send control port frames over nl80211
nl80211: Implement TX of control port frames
mac80211: Add support for tx_control_port
include/net/cfg80211.h | 29 +++++++++ include/uapi/linux/if_ether.h | 1 + include/uapi/linux/nl80211.h | 32 +++++++++- net/mac80211/cfg.c | 3 + net/mac80211/ieee80211_i.h | 4 ++ net/mac80211/main.c | 2 + net/mac80211/mlme.c | 2 + net/mac80211/rx.c | 34 +++++++++-- net/mac80211/tx.c | 46 +++++++++++++++ net/wireless/nl80211.c | 134 +++++++++++++++++++++++++++++++++++++++++- net/wireless/rdev-ops.h | 15 +++++ net/wireless/trace.h | 46 +++++++++++++++ 12 files changed, 341 insertions(+), 7 deletions(-)
Signature · 1 line
2.13.5
PATCH v3 1/6 uapi: Add 802.11 Preauthentication to if_ether
This adds 0x88c7 protocol type to if_ether.
--- include/uapi/linux/if_ether.h | 1 + 1 file changed, 1 insertion(+)
include/uapi/linux/if_ether.h +1 -0
@@ -89,6 +89,7 @@#define ETH_P_AOE 0x88A2/* ATA over Ethernet */ #define ETH_P_8021AD 0x88A8/* 802.1ad Service VLAN */ #define ETH_P_802_EX1 0x88B5/* 802.1 Local Experimental 1. */ +#define ETH_P_PREAUTH 0x88C7/* 802.11 Preauthentication */ #define ETH_P_TIPC 0x88CA/* TIPC */ #define ETH_P_MACSEC 0x88E5/* 802.1ae MACsec */ #define ETH_P_8021AH 0x88E7/* 802.1ah Backbone Service Tag */
Signature · 1 line
2.13.5
PATCH v3 2/6 nl80211: Add CONTROL_PORT_OVER_NL80211 attribute
In reply to Denis Kenzior--- include/net/cfg80211.h | 3 +++ include/uapi/linux/nl80211.h | 17 ++++++++++++++++- net/wireless/nl80211.c | 12 ++++++++++++ 3 files changed, 31 insertions(+), 1 deletion(-)
include/net/cfg80211.h +3 -0
@@ -646,6 +646,8 @@ struct survey_info {* allowed through even on unauthorized ports * @control_port_no_encrypt: TRUE to prevent encryption of control port * protocol frames. +* @control_port_over_nl80211: TRUE if userspace expects to exchange control +* port frames over NL80211 instead of the network interface. * @wep_keys: static WEP keys, if not NULL points to an array of * CFG80211_MAX_WEP_KEYS WEP keys * @wep_tx_key: key index (0..3) of the default TX static WEP key
@@ -661,6 +663,7 @@ struct cfg80211_crypto_settings {bool control_port;__be16 control_port_ethertype;bool control_port_no_encrypt; +bool control_port_over_nl80211;struct key_params *wep_keys;int wep_tx_key;const u8 *psk;
include/uapi/linux/nl80211.h +16 -1
@@ -542,7 +542,8 @@* IEs in %NL80211_ATTR_IE, %NL80211_ATTR_AUTH_TYPE, %NL80211_ATTR_USE_MFP, * %NL80211_ATTR_MAC, %NL80211_ATTR_WIPHY_FREQ, %NL80211_ATTR_CONTROL_PORT, * %NL80211_ATTR_CONTROL_PORT_ETHERTYPE, -* %NL80211_ATTR_CONTROL_PORT_NO_ENCRYPT, %NL80211_ATTR_MAC_HINT, and +* %NL80211_ATTR_CONTROL_PORT_NO_ENCRYPT, +* %NL80211_ATTR_CONTROL_PORT_OVER_NL80211, %NL80211_ATTR_MAC_HINT, and * %NL80211_ATTR_WIPHY_FREQ_HINT. * If included, %NL80211_ATTR_MAC and %NL80211_ATTR_WIPHY_FREQ are * restrictions on BSS selection, i.e., they effectively prevent roaming
@@ -1475,6 +1476,15 @@ enum nl80211_commands {* @NL80211_ATTR_CONTROL_PORT_NO_ENCRYPT: When included along with * %NL80211_ATTR_CONTROL_PORT_ETHERTYPE, indicates that the custom * ethertype frames used for key negotiation must not be encrypted. +* @NL80211_ATTR_CONTROL_PORT_OVER_NL80211: A flag indicating whether control +* port frames (e.g. of type given in %NL80211_ATTR_CONTROL_PORT_ETHERTYPE) +* will be sent directly to the network interface or sent via the NL80211 +* socket. If this attribute is missing, then legacy behavior of sending +* control port frames directly to the network interface is used. If the +* flag is included, then control port frames are sent over NL80211 instead +* using %CMD_CONTROL_PORT_FRAME. If control port routing over NL80211 is +* to be used then userspace must also use the %NL80211_ATTR_SOCKET_OWNER +* flag. * * @NL80211_ATTR_TESTDATA: Testmode data blob, passed through to the driver. * We recommend using nested, driver-specific attributes within this.
@@ -2627,6 +2637,8 @@ enum nl80211_attrs {NL80211_ATTR_NSS , +NL80211_ATTR_CONTROL_PORT_OVER_NL80211 , +/* add attributes here, update the policy in nl80211.c */ __NL80211_ATTR_AFTER_LAST,
@@ -4996,6 +5008,8 @@ enum nl80211_feature_flags {* @NL80211_EXT_FEATURE_LOW_SPAN_SCAN: Driver supports low span scan. * @NL80211_EXT_FEATURE_LOW_POWER_SCAN: Driver supports low power scan. * @NL80211_EXT_FEATURE_HIGH_ACCURACY_SCAN: Driver supports high accuracy scan. +* @NL80211_EXT_FEATURE_CONTROL_PORT_OVER_NL80211: Driver supports sending and +* receiving control port frames over NL80211 instead of the netdevice. * * @NUM_NL80211_EXT_FEATURES: number of extended features. * @MAX_NL80211_EXT_FEATURES: highest extended feature index.
@@ -5026,6 +5040,7 @@ enum nl80211_ext_feature_index {NL80211_EXT_FEATURE_LOW_SPAN_SCAN ,NL80211_EXT_FEATURE_LOW_POWER_SCAN ,NL80211_EXT_FEATURE_HIGH_ACCURACY_SCAN , +NL80211_EXT_FEATURE_CONTROL_PORT_OVER_NL80211 ,/* add new features before the definition below */ NUM_NL80211_EXT_FEATURES,
net/wireless/nl80211.c +12 -0
@@ -286,6 +286,7 @@ static const struct nla_policy nl80211_policy[NUM_NL80211_ATTR] = { [NL80211_ATTR_CONTROL_PORT] = { .type = NLA_FLAG }, [NL80211_ATTR_CONTROL_PORT_ETHERTYPE] = { .type = NLA_U16 }, [NL80211_ATTR_CONTROL_PORT_NO_ENCRYPT] = { .type = NLA_FLAG }, + [NL80211_ATTR_CONTROL_PORT_OVER_NL80211] = { .type = NLA_FLAG }, [NL80211_ATTR_PRIVACY] = { .type = NLA_FLAG }, [NL80211_ATTR_CIPHER_SUITE_GROUP] = { .type = NLA_U32 }, [NL80211_ATTR_WPA_VERSIONS] = { .type = NLA_U32 },
@@ -8225,6 +8226,17 @@ static int nl80211_crypto_settings(struct cfg80211_registered_device *rdev, }else settings->control_port_ethertype =cpu_to_be16 (ETH_P_PAE ); +if (info->attrs[NL80211_ATTR_CONTROL_PORT_OVER_NL80211 ]) { +if (!info->attrs[NL80211_ATTR_SOCKET_OWNER ]) +return -EINVAL ; + +if (!wiphy_ext_feature_isset (&rdev->wiphy, +NL80211_EXT_FEATURE_CONTROL_PORT_OVER_NL80211 )) +return -EOPNOTSUPP ; + + settings->control_port_over_nl80211 = true; + } +if (info->attrs[NL80211_ATTR_CIPHER_SUITES_PAIRWISE ]) {void *data;int len, i;
Signature · 1 line
2.13.5
PATCH v3 3/6 nl80211: Add CMD_CONTROL_PORT_FRAME API
In reply to Denis KenziorThis commit also adds cfg80211_rx_control_port function. This is used to generate a CMD_CONTROL_PORT_FRAME event out to userspace. The conn_owner_nlportid is used as the unicast destination. This means that userspace must specify NL80211_ATTR_SOCKET_OWNER flag if control port over nl80211 routing is requested in NL80211_CMD_CONNECT or NL80211_CMD_ASSOCIATE
--- include/net/cfg80211.h | 17 +++++++++++++ include/uapi/linux/nl80211.h | 15 +++++++++++ net/wireless/nl80211.c | 59 ++++++++++++++++++++++++++++++++++++++++++++ net/wireless/trace.h | 21 ++++++++++++++++ 4 files changed, 112 insertions(+)
include/net/cfg80211.h +17 -0
@@ -5693,6 +5693,23 @@ void cfg80211_mgmt_tx_status(struct wireless_dev *wdev, u64 cookie,/** +* cfg80211_rx_control_port - inform userspace about a received control port +* frame, e.g. EAPoL. This is used if userspace has specified it wants to +* receive control port frames over NL80211. +* @dev: The device the frame matched to +* @buf: control port frame +* @len: length of the frame data +* @addr: The peer from which the frame was received +* @proto: frame protocol, typically PAE or Pre-authentication +* @unencrypted: Whether the frame was received unencrypted +* +* Return: %true if the frame was passed to userspace +*/ +bool cfg80211_rx_control_port (struct net_device *dev, +const u8 *buf,size_t len, +const u8 *addr,u16 proto,bool unencrypted); + +/** * cfg80211_cqm_rssi_notify - connection quality monitoring rssi event * @dev: network device * @rssi_event: the triggered RSSI event
include/uapi/linux/nl80211.h +15 -0
@@ -991,6 +991,17 @@* &NL80211_CMD_CONNECT or &NL80211_CMD_ROAM. If the 4 way handshake failed * &NL80211_CMD_DISCONNECT should be indicated instead. * +* @NL80211_CMD_CONTROL_PORT_FRAME: Control Port (e.g. PAE) frame TX request +* and RX notification. This command is used both as a request to transmit +* a control port frame and as a notification that a control port frame +* has been received. %NL80211_ATTR_FRAME is used to specify the +* frame contents. The frame is the raw EAPoL data, without ethernet or +* 802.11 headers. +* When used as an event indication %NL80211_ATTR_CONTROL_PORT_ETHERTYPE, +* %NL80211_ATTR_CONTROL_PORT_NO_ENCRYPT and %NL80211_ATTR_MAC are added +* indicating the protocol type of the received frame; whether the frame +* was received unencrypted and the MAC address of the peer respectively. +* * @NL80211_CMD_RELOAD_REGDB: Request that the regdb firmware file is reloaded. * * @NL80211_CMD_EXTERNAL_AUTH: This interface is exclusively defined for host
@@ -1229,6 +1240,8 @@ enum nl80211_commands {NL80211_CMD_STA_OPMODE_CHANGED , + NL80211_CMD_CONTROL_PORT_FRAME, +/* add new commands above here */ /* used to define NL80211_CMD_MAX below */
@@ -1476,6 +1489,8 @@ enum nl80211_commands {* @NL80211_ATTR_CONTROL_PORT_NO_ENCRYPT: When included along with * %NL80211_ATTR_CONTROL_PORT_ETHERTYPE, indicates that the custom * ethertype frames used for key negotiation must not be encrypted. +* When included in %NL80211_CMD_CONTROL_PORT_FRAME it means that the +* control port frame was received unencrypted. * @NL80211_ATTR_CONTROL_PORT_OVER_NL80211: A flag indicating whether control * port frames (e.g. of type given in %NL80211_ATTR_CONTROL_PORT_ETHERTYPE) * will be sent directly to the network interface or sent via the NL80211
net/wireless/nl80211.c +59 -0
65 lines not on this page · raw message
net/wireless/trace.h +21 -0
@@ -2600,6 +2600,27 @@ TRACE_EVENT(cfg80211_mgmt_tx_status, WDEV_PR_ARG, __entry->cookie, BOOL_TO_STR(__entry->ack)) ); +TRACE_EVENT (cfg80211_rx_control_port, + TP_PROTO(struct net_device *netdev, const u8 *buf, size_t len, + const u8 *addr, u16 proto, bool unencrypted), +TP_ARGS (netdev, buf, len, addr, proto, unencrypted), +TP_STRUCT__entry ( + NETDEV_ENTRY + MAC_ENTRY(addr) + __field(u16, proto) +__field (bool, unencrypted) + ), +TP_fast_assign ( + NETDEV_ASSIGN; + MAC_ASSIGN(addr, addr); + __entry->proto = proto; + __entry->unencrypted = unencrypted; + ), +TP_printk (NETDEV_PR_FMT ", " MAC_PR_FMT " proto: %x, unencrypted: %s" , +NETDEV_PR_ARG ,MAC_PR_ARG (addr), + __entry->proto,BOOL_TO_STR (__entry->unencrypted)) +); + TRACE_EVENT(cfg80211_cqm_rssi_notify, TP_PROTO(struct net_device *netdev, enum nl80211_cqm_rssi_threshold_event rssi_event,
Signature · 1 line
2.13.5
Johannes Berg
maintainerRe: [PATCH 3/6] nl80211: Add CMD_CONTROL_PORT_FRAME API
Quoted 5 lines · Denis Kenzior
/**
+ *cfg80211_rx_control_port- inform userspace about a received control port
+ * frame, e.g. EAPoL. This is used if userspace has specified it wants to
+ * receive control port frames over NL80211.
nitpick - the short description must fit on a single line, you can have a longer description separately (after the arguments, I'd probably even put it after the return)
Quoted 3 lines
+ * @dev: The device the frame matched to
+ * @buf: control port frame
+ * @len: length of the frame data
You should document what exactly is in this frame data?
Should it be with the ethernet header removed? It would seem easier if it's with the ethernet header included, but then why do you need the proto argument?
Quoted 6 lines
+ * @NL80211_CMD_CONTROL_PORT_FRAME: Control Port (e.g. PAE) frame TX request
+ * and RX notification. This command is used both as a request to transmit
+ * a control port frame and as a notification that a control port frame
+ * has been received. %NL80211_ATTR_FRAME is used to specify the
+ * frame contents. The frame is the raw EAPoL data, without ethernet or
+ * 802.11 headers.
Never mind, so it's without Ethernet header. Is that really desirable though? I mean, it could be that the Ethernet address even matters (not sure) and it'd probably be easier to handle in (existing) userspace where Ethernet frames are expected now?
net/wireless/nl80211.c void cfg80211_mgmt_tx_status(struct wireless_dev *wdev, u64 cookie, · line 14604
Quoted 2 lines
+
nla_put_failure:net/wireless/nl80211.c +1
-0+genlmsg_cancel (msg, hdr);
nit: there's no point in cancelling if you free it (immediately).
net/wireless/nl80211.c void cfg80211_mgmt_tx_status(struct wireless_dev *wdev, u64 cookie, · line 14618
Quoted 10 lines
net/wireless/nl80211.c +10
-0+bool cfg80211_rx_control_port (struct net_device *dev, +const u8 *buf,size_t len, +const u8 *addr,u16 proto,bool unencrypted) +{ +bool ret; + +trace_cfg80211_rx_control_port (dev, buf, len, addr, proto, unencrypted); + ret =__nl80211_rx_control_port (dev, buf, len, addr, proto, + unencrypted,GFP_ATOMIC ); +trace_cfg80211_return_bool (ret);
this seems wrong - you return -ERROR from __nl80211_rx_control_port() so you need either to pass that on as an integer to the caller, or put an == 0 here or something?
"Return: %true if the frame was passed to userspace"
johannes
Re: [PATCH 3/6] nl80211: Add CMD_CONTROL_PORT_FRAME API
Hi Johannes,
Quoted 11 lines
Quoted 6 lines
+ * @NL80211_CMD_CONTROL_PORT_FRAME: Control Port (e.g. PAE) frame TX request
+ * and RX notification. This command is used both as a request to transmit
+ * a control port frame and as a notification that a control port frame
+ * has been received. %NL80211_ATTR_FRAME is used to specify the
+ * frame contents. The frame is the raw EAPoL data, without ethernet or
+ * 802.11 headers.Never mind, so it's without Ethernet header. Is that really desirable though? I mean, it could be that the Ethernet address even matters (not sure) and it'd probably be easier to handle in (existing) userspace where Ethernet frames are expected now?
I also include the from address inside the NL80211 message as ATTR_MAC. The protocol as well. I wrote the docs first and never updated the little details afterwards. Will fix.
Quoted 5 lines
net/wireless/nl80211.c void cfg80211_mgmt_tx_status(struct wireless_dev *wdev, u64 cookie, · line 14604
Quoted 3 lines
+
nla_put_failure:net/wireless/nl80211.c +1
-0+genlmsg_cancel (msg, hdr);nit: there's no point in cancelling if you free it (immediately).
Just following some existing code, but will fix.
Quoted 16 lines
net/wireless/nl80211.c void cfg80211_mgmt_tx_status(struct wireless_dev *wdev, u64 cookie, · line 14618
Quoted 11 lines
net/wireless/nl80211.c +10
-0+bool cfg80211_rx_control_port (struct net_device *dev, +const u8 *buf,size_t len, +const u8 *addr,u16 proto,bool unencrypted) +{ +bool ret; + +trace_cfg80211_rx_control_port (dev, buf, len, addr, proto, unencrypted); + ret =__nl80211_rx_control_port (dev, buf, len, addr, proto, + unencrypted,GFP_ATOMIC ); +trace_cfg80211_return_bool (ret);this seems wrong - you return
-ERRORfrom__nl80211_rx_control_port()so you need either to pass that on as an integer to the caller, or put an == 0 here or something?
Quoted 2 lines
"Return: %true if the frame was passed to userspace"
Yep, will fix.
Regards,
-Denis
Johannes Berg
maintainerRe: [PATCH 3/6] nl80211: Add CMD_CONTROL_PORT_FRAME API
Quoted 9 lines · Denis Kenzior
Hi Johannes,
Quoted 7 lines
Quoted 6 lines
+ * @NL80211_CMD_CONTROL_PORT_FRAME: Control Port (e.g. PAE) frame TX request
+ * and RX notification. This command is used both as a request to transmit
+ * a control port frame and as a notification that a control port frame
+ * has been received. %NL80211_ATTR_FRAME is used to specify the
+ * frame contents. The frame is the raw EAPoL data, without ethernet or
+ * 802.11 headers.
Quoted 8 lines
Quoted 4 lines
Never mind, so it's without Ethernet header. Is that really desirable though? I mean, it could be that the Ethernet address even matters (not sure) and it'd probably be easier to handle in (existing) userspace where Ethernet frames are expected now?
I also include the from address inside the NL80211 message as
ATTR_MAC. The protocol as well. I wrote the docs first and never updated the little details afterwards. Will fix.
Good point, I could've seen that.
Still not sure it makes a big difference, but I guess it doesn't really matter that much (though in a sense it'd be easier to take Ethernet header apart than putting it back together - but even that can be done in place in the message buffer)
Quoted 6 lines
Quoted 4 lines
net/wireless/nl80211.c void cfg80211_mgmt_tx_status(struct wireless_dev *wdev, u64 cookie, · line 14604
Quoted 2 lines
+
nla_put_failure:net/wireless/nl80211.c +1
-0+genlmsg_cancel (msg, hdr);nit: there's no point in cancelling if you free it (immediately).
Just following some existing code, but will fix.
Yeah I just never got around to cleaning up that antipattern ... I'll make an spatch.
johannes
PATCH v3 4/6 mac80211: Send control port frames over nl80211
In reply to Denis KenziorIf userspace requested control port frames to go over 80211, then do so. The control packets are intercepted just prior to delivery of the packet to the underlying network device.
Pre-authentication type frames (protocol: 0x88c7) are also forwarded over nl80211.
--- net/mac80211/cfg.c | 2 ++ net/mac80211/ieee80211_i.h | 1 + net/mac80211/mlme.c | 2 ++ net/mac80211/rx.c | 34 +++++++++++++++++++++++++++++----- 4 files changed, 34 insertions(+), 5 deletions(-)
net/mac80211/cfg.c +2 -0
@@ -925,6 +925,7 @@ static int ieee80211_start_ap(struct wiphy *wiphy, struct net_device *dev,*/ sdata->control_port_protocol = params->crypto.control_port_ethertype; sdata->control_port_no_encrypt = params->crypto.control_port_no_encrypt; + sdata->control_port_over_nl80211 = false; sdata->encrypt_headroom =ieee80211_cs_headroom (sdata->local, ¶ms->crypto, sdata->vif.type);
@@ -934,6 +935,7 @@ static int ieee80211_start_ap(struct wiphy *wiphy, struct net_device *dev, params->crypto.control_port_ethertype; vlan->control_port_no_encrypt = params->crypto.control_port_no_encrypt; + vlan->control_port_over_nl80211 = false; vlan->encrypt_headroom = ieee80211_cs_headroom(sdata->local, ¶ms->crypto,
net/mac80211/ieee80211_i.h +1 -0
@@ -899,6 +899,7 @@ struct ieee80211_sub_if_data {u16 sequence_number;__be16 control_port_protocol;bool control_port_no_encrypt; +bool control_port_over_nl80211;int encrypt_headroom;atomic_t num_tx_queued;
net/mac80211/mlme.c +2 -0
@@ -4830,6 +4830,8 @@ int ieee80211_mgd_assoc(struct ieee80211_sub_if_data *sdata, sdata->control_port_protocol = req->crypto.control_port_ethertype; sdata->control_port_no_encrypt = req->crypto.control_port_no_encrypt; + sdata->control_port_over_nl80211 = + req->crypto.control_port_over_nl80211; sdata->encrypt_headroom =ieee80211_cs_headroom (local, &req->crypto, sdata->vif.type);
net/mac80211/rx.c +29 -5
@@ -2243,6 +2243,33 @@ static bool ieee80211_frame_allowed(struct ieee80211_rx_data *rx, __le16 fc)return true; } +static void ieee80211_deliver_skb_to_local_stack (struct sk_buff *skb, +struct ieee80211_rx_data *rx) +{ +struct ieee80211_sub_if_data *sdata = rx->sdata; +struct net_device *dev = sdata->dev; + +if (unlikely ((skb->protocol == sdata->control_port_protocol || + skb->protocol ==cpu_to_be16 (ETH_P_PREAUTH )) && + sdata->control_port_over_nl80211)) { +struct ieee80211_rx_status *status =IEEE80211_SKB_RXCB (skb); +bool noencrypt = status->flag &RX_FLAG_DECRYPTED ; +struct ethhdr *ehdr =eth_hdr (skb); + +if (!cfg80211_rx_control_port (dev, skb->data, skb->len, + ehdr->h_source, +be16_to_cpu (skb->protocol), + noencrypt)) +dev_kfree_skb (skb); + }else { +/* deliver to local stack */ +if (rx->napi) +napi_gro_receive (rx->napi, skb); +else +netif_receive_skb (skb); + } +} +/* * requires that rx->skb is a frame with ethernet header */
@@ -2327,13 +2354,10 @@ ieee80211_deliver_skb(struct ieee80211_rx_data *rx)#endif if (skb) { -/* deliver to local stack */ skb->protocol =eth_type_trans (skb, dev);memset (skb->cb,0 ,sizeof (skb->cb)); -if (rx->napi) -napi_gro_receive (rx->napi, skb); -else -netif_receive_skb (skb); + +ieee80211_deliver_skb_to_local_stack (skb, rx); }if (xmit_skb) {
Signature · 1 line
2.13.5
Johannes Berg
maintainerRe: [PATCH 4/6] mac80211: Send control port frames over nl80211
net/mac80211/rx.c static bool ieee80211_frame_allowed(struct ieee80211_rx_data *rx, __le16 fc) · line 2263
Quoted 6 lines · Denis Kenzior
net/mac80211/rx.c +5
-0+if (!cfg80211_rx_control_port (dev, skb->data, skb->len, + ehdr->h_source, +be16_to_cpu (skb->protocol), + noencrypt)) +dev_kfree_skb (skb);
Err, you should free it unconditionally, no? You don't do anything with the skb afterwards, since you can't get into the else branch and deliver it to the netdev now, so afaict it'd be leaked if not freed?
Which explains why you didn't notice the error in the previous patch, it was making this code correct by freeing it all the time (never hitting an error, presumably, and even if you wouldn't notice the small leaks)
johannes
PATCH v3 5/6 nl80211: Implement TX of control port frames
In reply to Denis KenziorThis commit implements the TX side of NL80211_CMD_CONTROL_PORT_FRAME. Userspace provides the raw EAPoL frame using NL80211_ATTR_FRAME. Userspace should also provide the destination address and the protocol type to use when sending the frame. This is used to implement TX of Pre-authentication frames. If CONTROL_PORT_ETHERTYPE_NO_ENCRYPT is specified, then the driver will be asked not to encrypt the outgoing frame.
--- include/net/cfg80211.h | 9 +++++++ net/wireless/nl80211.c | 63 ++++++++++++++++++++++++++++++++++++++++++++++++- net/wireless/rdev-ops.h | 15 ++++++++++++ net/wireless/trace.h | 25 ++++++++++++++++++++ 4 files changed, 111 insertions(+), 1 deletion(-)
include/net/cfg80211.h +9 -0
@@ -2961,6 +2961,9 @@ struct cfg80211_external_auth_params {* * @external_auth: indicates result of offloaded authentication processing from * user space +* +* @tx_control_port: TX a control port frame (EAPoL). The noencrypt parameter +* tells the driver that the frame should not be encrypted. */ struct cfg80211_ops {int (*suspend)(struct wiphy *wiphy,struct cfg80211_wowlan *wow);
@@ -3256,6 +3259,12 @@ struct cfg80211_ops { const u8 *aa);int (*external_auth)(struct wiphy *wiphy,struct net_device *dev,struct cfg80211_external_auth_params *params); + +int (*tx_control_port)(struct wiphy *wiphy, +struct net_device *dev, +const u8 *buf,size_t len, +const u8 *dest,const u16 proto, +const bool noencrypt); };/*
net/wireless/nl80211.c +62 -1
75 lines not on this page · raw message
net/wireless/rdev-ops.h +15 -0
@@ -714,6 +714,21 @@ static inline int rdev_mgmt_tx(struct cfg80211_registered_device *rdev,return ret; } +static inline int rdev_tx_control_port (struct cfg80211_registered_device *rdev, +struct net_device *dev, +const void *buf,size_t len, +const u8 *dest,u16 proto, +const bool noencrypt) +{ +int ret; +trace_rdev_tx_control_port (&rdev->wiphy, dev, buf, len, + dest, proto, noencrypt); + ret = rdev->ops->tx_control_port (&rdev->wiphy, dev, buf, len, + dest, proto, noencrypt); +trace_rdev_return_int (&rdev->wiphy, ret); +return ret; +} +static inline int rdev_mgmt_tx_cancel_wait (struct cfg80211_registered_device *rdev,struct wireless_dev *wdev,u64 cookie)
net/wireless/trace.h +25 -0
@@ -1882,6 +1882,31 @@ TRACE_EVENT(rdev_mgmt_tx, BOOL_TO_STR(__entry->dont_wait_for_ack)) ); +TRACE_EVENT (rdev_tx_control_port, + TP_PROTO(struct wiphy *wiphy, struct net_device *netdev, + const u8 *buf, size_t len, const u8 *dest, u16 proto, + bool unencrypted), +TP_ARGS (wiphy, netdev, buf, len, dest, proto, unencrypted), +TP_STRUCT__entry ( + WIPHY_ENTRY + NETDEV_ENTRY + MAC_ENTRY(dest) + __field(u16, proto) +__field (bool, unencrypted) + ), +TP_fast_assign ( + WIPHY_ASSIGN; + NETDEV_ASSIGN; + MAC_ASSIGN(dest, dest); + __entry->proto = proto; + __entry->unencrypted = unencrypted; + ), +TP_printk (WIPHY_PR_FMT ", " NETDEV_PR_FMT ", " MAC_PR_FMT "," +" proto: %x, unencrypted: %s" , +WIPHY_PR_ARG ,NETDEV_PR_ARG ,MAC_PR_ARG (dest), + __entry->proto,BOOL_TO_STR (__entry->unencrypted)) +); + TRACE_EVENT(rdev_set_noack_map, TP_PROTO(struct wiphy *wiphy, struct net_device *netdev, u16 noack_map),
Signature · 1 line
2.13.5
Johannes Berg
maintainerRe: [PATCH 5/6] nl80211: Implement TX of control port frames
net/wireless/nl80211.c static int nl80211_external_auth(struct sk_buff *skb, struct genl_info *info) · line 12577
Quoted 3 lines · Denis Kenzior
net/wireless/nl80211.c +2
-0+return rdev_tx_control_port (rdev, dev, buf, len, + dest,cpu_to_be16 (proto), noencrypt);
You're passing __be16 here
net/wireless/rdev-ops.h static inline int rdev_mgmt_tx(struct cfg80211_registered_device *rdev, · line 721
Quoted 10 lines
+++
b/net/wireless/rdev-ops.hnet/wireless/rdev-ops.h +5
-0@@ -714,6 +714,21 @@ static inline int rdev_mgmt_tx(struct cfg80211_registered_device *rdev,return ret; } +static inline int rdev_tx_control_port (struct cfg80211_registered_device *rdev, +struct net_device *dev, +const void *buf,size_t len, +const u8 *dest,u16 proto, +const bool noencrypt)
but have u16 here - doesn't that make sparse unhappy?
Should also declare the API as __be16.
johannes
Johannes Berg
maintainerRe: [PATCH 5/6] nl80211: Implement TX of control port frames
In reply to Denis KenziorQuoted 9 lines
net/wireless/nl80211.c +8
-0+static int nl80211_tx_control_port (struct sk_buff *skb,struct genl_info *info) +{ +struct cfg80211_registered_device *rdev = info->user_ptr[0 ]; +struct net_device *dev = info->user_ptr[1 ]; +struct wireless_dev *wdev = dev->ieee80211_ptr; +const u8 *buf; +size_t len; +u8 *dest;
net/wireless/nl80211.c static int nl80211_external_auth(struct sk_buff *skb, struct genl_info *info) · line 12547
Quoted 10 lines
net/wireless/nl80211.c +10
-0+u16 proto; +bool noencrypt; +int err; + +if (!wiphy_ext_feature_isset (&rdev->wiphy, +NL80211_EXT_FEATURE_CONTROL_PORT_OVER_NL80211 )) +return -EOPNOTSUPP ; + +if (!rdev->ops->tx_control_port) +return -EOPNOTSUPP ;
I wonder if maybe we should accept this command only from the socket owner? Is there a use case for something else?
Actually, then again, that's awkward because doing events and commands on the same socket doesn't mix all _that_ well. Perhaps we just need to fix that problem in libnl or something and be done with it ...
johannes
Re: [PATCH 5/6] nl80211: Implement TX of control port frames
Hi Johannes,
Quoted 14 lines · Johannes Berg
Quoted 14 lines
net/wireless/nl80211.c +13
-0+static int nl80211_tx_control_port(struct sk_buff *skb, struct genl_info *info) +{ + struct cfg80211_registered_device *rdev = info->user_ptr[0]; + struct net_device *dev = info->user_ptr[1]; + struct wireless_dev *wdev = dev->ieee80211_ptr; + const u8 *buf; + size_t len; + u8 *dest; + u16 proto; + bool noencrypt; + int err; + + if (!wiphy_ext_feature_isset(&rdev->wiphy,
Quoted 8 lines
net/wireless/nl80211.c static int nl80211_external_auth(struct sk_buff *skb, struct genl_info *info) · line 12547
Quoted 5 lines
net/wireless/nl80211.c +5
-0+ NL80211_EXT_FEATURE_CONTROL_PORT_OVER_NL80211)) +return -EOPNOTSUPP ; + +if (!rdev->ops->tx_control_port) +return -EOPNOTSUPP ;I wonder if maybe we should accept this command only from the socket owner? Is there a use case for something else?
Yes I thought about adding such a check as I think it wouldn't make sense to accept control port frames from any other application besides the socket owner. However the socket owner stuff was a bit controversial, so I left it out.
I would support adding this check...
Quoted 5 lines
Actually, then again, that's awkward because doing events and commands on the same socket doesn't mix all _that_ well. Perhaps we just need to fix that problem in libnl or something and be done with it ...
There are no issues on our side..
Regards,
-Denis
Johannes Berg
maintainerRe: [PATCH 5/6] nl80211: Implement TX of control port frames
Quoted 6 lines · Denis Kenzior
Quoted 4 lines
Actually, then again, that's awkward because doing events and commands on the same socket doesn't mix all _that_ well. Perhaps we just need to fix that problem in libnl or something and be done with it ...
There are no issues on our side..
I guess you don't use libnl then :-) It doesn't exactly make it easy to receive events while waiting for an ACK.
johannes
PATCH v3 6/6 mac80211: Add support for tx_control_port
In reply to Denis Kenzior--- net/mac80211/cfg.c | 1 + net/mac80211/ieee80211_i.h | 3 +++ net/mac80211/main.c | 2 ++ net/mac80211/tx.c | 46 ++++++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 52 insertions(+)
net/mac80211/cfg.c +1 -0
@@ -3787,4 +3787,5 @@ const struct cfg80211_ops mac80211_config_ops = { .add_nan_func = ieee80211_add_nan_func, .del_nan_func = ieee80211_del_nan_func, .set_multicast_to_unicast = ieee80211_set_multicast_to_unicast, + .tx_control_port = ieee80211_tx_control_port, };
net/mac80211/ieee80211_i.h +3 -0
@@ -1735,6 +1735,9 @@ void ieee80211_check_fast_xmit(struct sta_info *sta);void ieee80211_check_fast_xmit_all (struct ieee80211_local *local);void ieee80211_check_fast_xmit_iface (struct ieee80211_sub_if_data *sdata);void ieee80211_clear_fast_xmit (struct sta_info *sta); +int ieee80211_tx_control_port (struct wiphy *wiphy,struct net_device *dev, +const u8 *buf,size_t len, +const u8 *dest,u16 proto,bool unencrypted);/* HT */ void ieee80211_apply_htcap_overrides(struct ieee80211_sub_if_data *sdata,
net/mac80211/main.c +2 -0
@@ -554,6 +554,8 @@ struct ieee80211_hw *ieee80211_alloc_hw_nm(size_t priv_data_len,NL80211_FEATURE_USERSPACE_MPM |NL80211_FEATURE_FULL_AP_CLIENT_STATE ;wiphy_ext_feature_set (wiphy,NL80211_EXT_FEATURE_FILS_STA ); +wiphy_ext_feature_set (wiphy, +NL80211_EXT_FEATURE_CONTROL_PORT_OVER_NL80211 );if (!ops->hw_scan) wiphy->features |= NL80211_FEATURE_LOW_PRIORITY_SCAN |
net/mac80211/tx.c +46 -0
49 lines not on this page · raw message
Signature · 1 line
2.13.5
Johannes Berg
maintainerLooks pretty good! Some comments over in separate emails.
Maybe you should consider reordering:
[1/6] keep
[2/6] keep
[3/6] introduce TX (now patch 5)
[4/6] introduce RX (now patch 3)
modify this to require TX from the driver in order to be able to set the new flag
[5/6] add TX to mac80211
[6/6] add RX to mac80211 and set the flag
Ok, you don't really have to do that, since you only set the flag in the last patch anyway, but that'd make it better to add the check that requires TX, so you don't have to do that in the TX patch and modify the RX portion again.
johannes
Hi Johannes,
Quoted 13 lines · Johannes Berg
Looks pretty good! Some comments over in separate emails.
Maybe you should consider reordering:
[1/6] keep
[2/6] keep
[3/6] introduce TX (now patch 5)
[4/6] introduce RX (now patch 3)
modify this to require TX from the driver in order to be able to set the new flag
[5/6] add TX to mac80211
[6/6] add RX to mac80211 and set the flag
Quoted 4 lines
Ok, you don't really have to do that, since you only set the flag in the last patch anyway, but that'd make it better to add the check that requires TX, so you don't have to do that in the TX patch and modify the RX portion again.
Err, Johannes Berg presents: Fun with Flags ;) So I re-read this about half a dozen times and I think I figured out what you wanted. It seems you mean different flags in 4/6 and 6/6. I reordered V4 with my interpretation. Doesn't quite look like the above, but hopefully I groked what you wanted correctly.
I didn't put in the extra owner check on the tx operation since it wasn't clear whether you wanted that or not.
Thanks for the review.
Regards,
-Denis
Johannes Berg
maintainerHi,
Quoted 9 lines
Quoted 9 lines
[1/6] keep
[2/6] keep
[3/6] introduce TX (now patch 5)
[4/6] introduce RX (now patch 3)
modify this to require TX from the driver in order to be able to set the new flag
[5/6] add TX to mac80211
[6/6] add RX to mac80211 and set the flag
Quoted 8 lines
Quoted 4 lines
Ok, you don't really have to do that, since you only set the flag in the last patch anyway, but that'd make it better to add the check that requires TX, so you don't have to do that in the TX patch and modify the RX portion again.
Err, Johannes Berg presents: Fun with Flags ;) So I re-read this about half a dozen times and I think I figured out what you wanted. It seems you mean different flags in 4/6 and 6/6.
Err, I didn't think so, but then it was pretty late. I was thinking of the feature flag that says whether userspace can expect to see this or not.
Quoted 3 lines
I reordered V4 with my
interpretation. Doesn't quite look like the above, but hopefully I groked what you wanted correctly.
I guess I can always reorder things myself if I really think it necessary, thanks :-)
Quoted 2 lines
I didn't put in the extra owner check on the tx operation since it wasn't clear whether you wanted that or not.
Yeah I can't make up my mind. I'm tempted to say "let's do the right thing" and fix the problems we have in userspace ...
johannes