2018-05-22 00:46:09

by Andrew Zaborowski

Subject: [PATCH] nl80211: Reject disconnect commands except from conn_owner

Reject NL80211_CMD_DISCONNECT, NL80211_CMD_DISASSOCIATE,
NL80211_CMD_DEAUTHENTICATE and NL80211_CMD_ASSOCIATE commands
from clients other than the connection owner set in the connect,
authenticate or associate commands, if it was set.

The main point of this check is to prevent chaos when two processes
try to use nl80211 at the same time, it's not a security measure.
The same thing should possibly be done for JOIN_IBSS/LEAVE_IBSS and
START_AP/STOP_AP.

Signed-off-by: Andrew Zaborowski <[email protected]>
---
net/wireless/nl80211.c | 16 ++++++++++++++++
1 file changed, 16 insertions(+)

diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c
index e4a52a2b5e..85f094a564 100644
--- a/net/wireless/nl80211.c
+++ b/net/wireless/nl80211.c
@@ -8506,6 +8506,10 @@ static int nl80211_associate(struct sk_buff *skb, struct genl_info *info)
const u8 *bssid, *ssid;
int err, ssid_len = 0;

+ if (dev->ieee80211_ptr->conn_owner_nlportid &&
+ dev->ieee80211_ptr->conn_owner_nlportid != info->snd_portid)
+ return -EPERM;
+
if (!is_valid_ie_attr(info->attrs[NL80211_ATTR_IE]))
return -EINVAL;

@@ -8628,6 +8632,10 @@ static int nl80211_deauthenticate(struct sk_buff *skb, struct genl_info *info)
u16 reason_code;
bool local_state_change;

+ if (dev->ieee80211_ptr->conn_owner_nlportid &&
+ dev->ieee80211_ptr->conn_owner_nlportid != info->snd_portid)
+ return -EPERM;
+
if (!is_valid_ie_attr(info->attrs[NL80211_ATTR_IE]))
return -EINVAL;

@@ -8675,6 +8683,10 @@ static int nl80211_disassociate(struct sk_buff *skb, struct genl_info *info)
u16 reason_code;
bool local_state_change;

+ if (dev->ieee80211_ptr->conn_owner_nlportid &&
+ dev->ieee80211_ptr->conn_owner_nlportid != info->snd_portid)
+ return -EPERM;
+
if (!is_valid_ie_attr(info->attrs[NL80211_ATTR_IE]))
return -EINVAL;

@@ -9451,6 +9463,10 @@ static int nl80211_disconnect(struct sk_buff *skb, struct genl_info *info)
u16 reason;
int ret;

+ if (dev->ieee80211_ptr->conn_owner_nlportid &&
+ dev->ieee80211_ptr->conn_owner_nlportid != info->snd_portid)
+ return -EPERM;
+
if (!info->attrs[NL80211_ATTR_REASON_CODE])
reason = WLAN_REASON_DEAUTH_LEAVING;
else
--
2.14.1

2018-05-22 10:39:04

by Arend van Spriel

Subject: Re: [PATCH] nl80211: Reject disconnect commands except from conn_owner

On 5/8/2018 5:05 PM, Denis Kenzior wrote:
> Hi Arend,
>
>>
>> Sure. I guess we all have been there kicking of wpa_s and discovering
>> there is already one running in the background. I am just a bit
>> squeamish to change the behavior like this. Hmmmm. Is wpa_s already
>> using SOCKET_OWNER. If so, I might create a patch to opt-out for that
>> so people can knowingly choose chaos ;-)
>>
>
> wpa_s is using SOCKET_OWNER these days. However, with the introduction
> of Control Port over NL80211, just getting rid of SOCKET_OWNER might not
> be that easy.

I have a regression test script employing py80211 which listens for
connect event. Right now I am using an older wpa_s, but the above will
screw it up.

If I recall correctly the mlme notification needed to be unicast,
because multicast was not 100% reliable, right? Would it be acceptable
to send unicast to the socket owner and still do the multicast or are we
already doing that? If not, that would fix my imminent issue.

Regards,
Arend