linux-wireless.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 1/2] mac80211: Check port authorization in the ieee80211_tx_dequeue() case
@ 2020-03-26 14:51 Johannes Berg
  2020-03-26 14:51 ` [PATCH 2/2] mac80211: mark station unauthorized before key removal Johannes Berg
                   ` (2 more replies)
  0 siblings, 3 replies; 6+ messages in thread
From: Johannes Berg @ 2020-03-26 14:51 UTC (permalink / raw)
  To: linux-wireless; +Cc: Jouni Malinen, stable

From: Jouni Malinen <jouni@codeaurora.org>

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: stable@vger.kernel.org
Signed-off-by: Jouni Malinen <jouni@codeaurora.org>
Signed-off-by: Johannes Berg <johannes.berg@intel.com>
---
 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


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

* [PATCH 2/2] mac80211: mark station unauthorized before key removal
  2020-03-26 14:51 [PATCH 1/2] mac80211: Check port authorization in the ieee80211_tx_dequeue() case Johannes Berg
@ 2020-03-26 14:51 ` Johannes Berg
  2020-03-26 15:22   ` Toke Høiland-Jørgensen
  2020-03-27 15:03   ` Sasha Levin
  2020-03-26 15:21 ` [PATCH 1/2] mac80211: Check port authorization in the ieee80211_tx_dequeue() case Toke Høiland-Jørgensen
  2020-03-27 15:03 ` Sasha Levin
  2 siblings, 2 replies; 6+ messages in thread
From: Johannes Berg @ 2020-03-26 14:51 UTC (permalink / raw)
  To: linux-wireless; +Cc: Johannes Berg, stable

From: Johannes Berg <johannes.berg@intel.com>

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: stable@vger.kernel.org
Signed-off-by: Johannes Berg <johannes.berg@intel.com>
---
 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 <jbenc@suse.cz>
  * 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


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

* Re: [PATCH 1/2] mac80211: Check port authorization in the ieee80211_tx_dequeue() case
  2020-03-26 14:51 [PATCH 1/2] mac80211: Check port authorization in the ieee80211_tx_dequeue() case Johannes Berg
  2020-03-26 14:51 ` [PATCH 2/2] mac80211: mark station unauthorized before key removal Johannes Berg
@ 2020-03-26 15:21 ` Toke Høiland-Jørgensen
  2020-03-27 15:03 ` Sasha Levin
  2 siblings, 0 replies; 6+ messages in thread
From: Toke Høiland-Jørgensen @ 2020-03-26 15:21 UTC (permalink / raw)
  To: Johannes Berg, linux-wireless; +Cc: Jouni Malinen, stable

Johannes Berg <johannes@sipsolutions.net> writes:

> From: Jouni Malinen <jouni@codeaurora.org>
>
> 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: stable@vger.kernel.org
> Signed-off-by: Jouni Malinen <jouni@codeaurora.org>
> Signed-off-by: Johannes Berg <johannes.berg@intel.com>

Ah - nice find!

Acked-by: Toke Høiland-Jørgensen <toke@redhat.com>


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

* Re: [PATCH 2/2] mac80211: mark station unauthorized before key removal
  2020-03-26 14:51 ` [PATCH 2/2] mac80211: mark station unauthorized before key removal Johannes Berg
@ 2020-03-26 15:22   ` Toke Høiland-Jørgensen
  2020-03-27 15:03   ` Sasha Levin
  1 sibling, 0 replies; 6+ messages in thread
From: Toke Høiland-Jørgensen @ 2020-03-26 15:22 UTC (permalink / raw)
  To: Johannes Berg, linux-wireless; +Cc: Johannes Berg, stable

Johannes Berg <johannes@sipsolutions.net> writes:

> From: Johannes Berg <johannes.berg@intel.com>
>
> 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: stable@vger.kernel.org
> Signed-off-by: Johannes Berg <johannes.berg@intel.com>

Acked-by: Toke Høiland-Jørgensen <toke@redhat.com>


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

* Re: [PATCH 2/2] mac80211: mark station unauthorized before key removal
  2020-03-26 14:51 ` [PATCH 2/2] mac80211: mark station unauthorized before key removal Johannes Berg
  2020-03-26 15:22   ` Toke Høiland-Jørgensen
@ 2020-03-27 15:03   ` Sasha Levin
  1 sibling, 0 replies; 6+ messages in thread
From: Sasha Levin @ 2020-03-27 15:03 UTC (permalink / raw)
  To: Sasha Levin, Johannes Berg, Johannes Berg, linux-wireless
  Cc: Johannes Berg, stable, stable, stable

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

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

* Re: [PATCH 1/2] mac80211: Check port authorization in the ieee80211_tx_dequeue() case
  2020-03-26 14:51 [PATCH 1/2] mac80211: Check port authorization in the ieee80211_tx_dequeue() case Johannes Berg
  2020-03-26 14:51 ` [PATCH 2/2] mac80211: mark station unauthorized before key removal Johannes Berg
  2020-03-26 15:21 ` [PATCH 1/2] mac80211: Check port authorization in the ieee80211_tx_dequeue() case Toke Høiland-Jørgensen
@ 2020-03-27 15:03 ` Sasha Levin
  2 siblings, 0 replies; 6+ messages in thread
From: Sasha Levin @ 2020-03-27 15:03 UTC (permalink / raw)
  To: Sasha Levin, Johannes Berg, Jouni Malinen, linux-wireless
  Cc: Jouni Malinen, stable, stable, stable

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: Build OK!
v4.14.174: Build OK!
v4.9.217: Build OK!
v4.4.217: Failed to apply! Possible dependencies:
    311048911758 ("mac80211: allow driver to prevent two stations w/ same address")
    412a6d800c73 ("mac80211: support hw managing reorder logic")
    49ddf8e6e234 ("mac80211: add fast-rx path")
    4f6b1b3daaf1 ("mac80211: fix last RX rate data consistency")
    506bcfa8abeb ("mac80211: limit the A-MSDU Tx based on peer's capabilities")
    52cfa1d6146c ("mac80211: track and tell driver about GO client P2P PS abilities")
    6e0456b54545 ("mac80211: add A-MSDU tx support")
    86c7ec9eb154 ("mac80211: properly free skb when r-o-c for TX fails")
    a2fcfccbad43 ("mac80211: move off-channel/mgmt-tx code to offchannel.c")
    a7201a6c5ea0 ("mac80211: Recalc min chandef when station is associated")
    b8da6b6a99b4 ("mac80211: add separate last_ack variable")
    bb42f2d13ffc ("mac80211: Move reorder-sensitive TX handlers to after TXQ dequeue")
    c9c5962b56c1 ("mac80211: enable collecting station statistics per-CPU")
    dfdfc2beb0dd ("mac80211: Parse legacy and HT rate in injected frames")


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

How should we proceed with this patch?

-- 
Thanks
Sasha

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

end of thread, other threads:[~2020-03-27 15:03 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-03-26 14:51 [PATCH 1/2] mac80211: Check port authorization in the ieee80211_tx_dequeue() case Johannes Berg
2020-03-26 14:51 ` [PATCH 2/2] mac80211: mark station unauthorized before key removal Johannes Berg
2020-03-26 15:22   ` Toke Høiland-Jørgensen
2020-03-27 15:03   ` Sasha Levin
2020-03-26 15:21 ` [PATCH 1/2] mac80211: Check port authorization in the ieee80211_tx_dequeue() case Toke Høiland-Jørgensen
2020-03-27 15:03 ` Sasha Levin

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).