LinuxLists.cc - [PATCH 1/2] mac80211: Check port authorization in the ieee80211_tx

2020-03-26 14:52:17

Subject: [PATCH 1/2] mac80211: Check port authorization in the ieee80211_tx_dequeue() case

From: Jouni Malinen <[email protected]>

mac80211 used to check port authorization in the Data frame enqueue case
when going through start_xmit(). However, that authorization status may
change while the frame is waiting in a queue. Add a similar check in the
dequeue case to avoid sending previously accepted frames after
authorization change. This provides additional protection against
potential leaking of frames after a station has been disconnected and
the keys for it are being removed.

Cc: [email protected]
Signed-off-by: Jouni Malinen <[email protected]>
Signed-off-by: Johannes Berg <[email protected]>
---
net/mac80211/tx.c | 19 ++++++++++++++++++-
1 file changed, 18 insertions(+), 1 deletion(-)

diff --git a/net/mac80211/tx.c b/net/mac80211/tx.c
index 7dbfb9e3cd84..455eb8e6a459 100644
--- a/net/mac80211/tx.c
+++ b/net/mac80211/tx.c
@@ -3604,8 +3604,25 @@ struct sk_buff *ieee80211_tx_dequeue(struct ieee80211_hw *hw,
tx.skb = skb;
tx.sdata = vif_to_sdata(info->control.vif);

- if (txq->sta)
+ if (txq->sta) {
tx.sta = container_of(txq->sta, struct sta_info, sta);
+ /*
+ * Drop unicast frames to unauthorised stations unless they are
+ * EAPOL frames from the local station.
+ */
+ if (unlikely(!ieee80211_vif_is_mesh(&tx.sdata->vif) &&
+ tx.sdata->vif.type != NL80211_IFTYPE_OCB &&
+ !is_multicast_ether_addr(hdr->addr1) &&
+ !test_sta_flag(tx.sta, WLAN_STA_AUTHORIZED) &&
+ (!(info->control.flags &
+ IEEE80211_TX_CTRL_PORT_CTRL_PROTO) ||
+ !ether_addr_equal(tx.sdata->vif.addr,
+ hdr->addr2)))) {
+ I802_DEBUG_INC(local->tx_handlers_drop_unauth_port);
+ ieee80211_free_txskb(&local->hw, skb);
+ goto begin;
+ }
+ }

/*
* The key can be removed while the packet was queued, so need to call
--
2.25.1

2020-03-26 14:53:04

by Johannes Berg

[permalink] [raw]

Subject: [PATCH 2/2] mac80211: mark station unauthorized before key removal

From: Johannes Berg <[email protected]>

If a station is still marked as authorized, mark it as no longer
so before removing its keys. This allows frames transmitted to it
to be rejected, providing additional protection against leaking
plain text data during the disconnection flow.

Cc: [email protected]
Signed-off-by: Johannes Berg <[email protected]>
---
net/mac80211/sta_info.c | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/net/mac80211/sta_info.c b/net/mac80211/sta_info.c
index 0f5f40678885..e3572be307d6 100644
--- a/net/mac80211/sta_info.c
+++ b/net/mac80211/sta_info.c
@@ -4,7 +4,7 @@
* Copyright 2006-2007 Jiri Benc <[email protected]>
* Copyright 2013-2014 Intel Mobile Communications GmbH
* Copyright (C) 2015 - 2017 Intel Deutschland GmbH
- * Copyright (C) 2018-2019 Intel Corporation
+ * Copyright (C) 2018-2020 Intel Corporation
*/

#include <linux/module.h>
@@ -1049,6 +1049,11 @@ static void __sta_info_destroy_part2(struct sta_info *sta)
might_sleep();
lockdep_assert_held(&local->sta_mtx);

+ while (sta->sta_state == IEEE80211_STA_AUTHORIZED) {
+ ret = sta_info_move_state(sta, IEEE80211_STA_ASSOC);
+ WARN_ON_ONCE(ret);
+ }
+
/* now keys can no longer be reached */
ieee80211_free_sta_keys(local, sta);

--
2.25.1

2020-03-26 15:27:30

by Toke Høiland-Jørgensen

[permalink] [raw]

Subject: Re: [PATCH 1/2] mac80211: Check port authorization in the ieee80211_tx_dequeue() case

Johannes Berg <[email protected]> writes:

> From: Jouni Malinen <[email protected]>
>
> mac80211 used to check port authorization in the Data frame enqueue case
> when going through start_xmit(). However, that authorization status may
> change while the frame is waiting in a queue. Add a similar check in the
> dequeue case to avoid sending previously accepted frames after
> authorization change. This provides additional protection against
> potential leaking of frames after a station has been disconnected and
> the keys for it are being removed.
>
> Cc: [email protected]
> Signed-off-by: Jouni Malinen <[email protected]>
> Signed-off-by: Johannes Berg <[email protected]>

Ah - nice find!

Acked-by: Toke Høiland-Jørgensen <[email protected]>

2020-03-26 15:28:54

by Toke Høiland-Jørgensen

[permalink] [raw]

Subject: Re: [PATCH 2/2] mac80211: mark station unauthorized before key removal

Johannes Berg <[email protected]> writes:

> From: Johannes Berg <[email protected]>
>
> If a station is still marked as authorized, mark it as no longer
> so before removing its keys. This allows frames transmitted to it
> to be rejected, providing additional protection against leaking
> plain text data during the disconnection flow.
>
> Cc: [email protected]
> Signed-off-by: Johannes Berg <[email protected]>

Acked-by: Toke Høiland-Jørgensen <[email protected]>

2020-03-27 15:04:31

by Sasha Levin

[permalink] [raw]

Subject: Re: [PATCH 2/2] mac80211: mark station unauthorized before key removal

Hi

[This is an automated email]

This commit has been processed because it contains a -stable tag.
The stable tag indicates that it's relevant for the following trees: all

The bot has tested the following trees: v5.5.11, v5.4.27, v4.19.112, v4.14.174, v4.9.217, v4.4.217.

v5.5.11: Build OK!
v5.4.27: Build OK!
v4.19.112: Failed to apply! Possible dependencies:
1e87fec9fa52 ("mac80211: call rate_control_send_low() internally")
adf8ed01e4fd ("mac80211: add an optional TXQ for other PS-buffered frames")
ba905bf432f6 ("mac80211: store tx power value from user to station")
bd718fc11d5b ("mac80211: use STA info in rate_control_send_low()")
edba6bdad6fe ("mac80211: allow AMSDU size limitation per-TID")

v4.14.174: Failed to apply! Possible dependencies:
0832b603c758 ("mac80211: don't put null-data frames on the normal TXQ")
1e87fec9fa52 ("mac80211: call rate_control_send_low() internally")
7c181f4fcdc6 ("mac80211: add ieee80211_hw flag for QoS NDP support")
94ba92713f83 ("mac80211: Call mgd_prep_tx before transmitting deauthentication")
a403f3bf6390 ("mac80211: remove pointless flags=0 assignment")
adf8ed01e4fd ("mac80211: add an optional TXQ for other PS-buffered frames")
ba905bf432f6 ("mac80211: store tx power value from user to station")
bd718fc11d5b ("mac80211: use STA info in rate_control_send_low()")
e2fb1b839208 ("mac80211: enable TDLS peer buffer STA feature")
e552af058148 ("mac80211: limit wmm params to comply with ETSI requirements")
ecaf71de4143 ("iwlwifi: mvm: rs: introduce new API for rate scaling")
edba6bdad6fe ("mac80211: allow AMSDU size limitation per-TID")

v4.9.217: Failed to apply! Possible dependencies:
06efdbe70f9c ("ath10k: refactor ath10k_peer_assoc_h_phymode()")
50f08edf9809 ("ath9k: Switch to using mac80211 intermediate software queues.")
63fefa050477 ("ath9k: Introduce airtime fairness scheduling between stations")
7f406cd16a0f ("mac80211: encode rate type (legacy, HT, VHT) with fewer bits")
7fdd69c5af21 ("mac80211: clean up rate encoding bits in RX status")
a17d93ff3a95 ("mac80211: fix legacy and invalid rx-rate report")
bc1efd739b61 ("ath10k: add VHT160 support")
da6a4352e7c8 ("mac80211: separate encoding/bandwidth from flags")
dcba665b1f4a ("mac80211: use bitfield macros for encoded rate")

v4.4.217: Failed to apply! Possible dependencies:
0ead2510f8ce ("mac80211: allow the driver to send EOSP when needed")
311048911758 ("mac80211: allow driver to prevent two stations w/ same address")
412a6d800c73 ("mac80211: support hw managing reorder logic")
4f6b1b3daaf1 ("mac80211: fix last RX rate data consistency")
7f406cd16a0f ("mac80211: encode rate type (legacy, HT, VHT) with fewer bits")
7fdd69c5af21 ("mac80211: clean up rate encoding bits in RX status")
a17d93ff3a95 ("mac80211: fix legacy and invalid rx-rate report")
b8da6b6a99b4 ("mac80211: add separate last_ack variable")
bc1efd739b61 ("ath10k: add VHT160 support")
da6a4352e7c8 ("mac80211: separate encoding/bandwidth from flags")
dcba665b1f4a ("mac80211: use bitfield macros for encoded rate")
f59374eb427f ("mac80211: synchronize driver rx queues before removing a station")

NOTE: The patch will not be queued to stable trees until it is upstream.

How should we proceed with this patch?

--
Thanks
Sasha

2020-03-27 15:07:19

by Sasha Levin

[permalink] [raw]