[PATCH 0/2] mac80211: enable TKIP when using encapsulation offloading

[PATCH 0/2] mac80211: enable TKIP when using encapsulation offloading

[Sathishkumar Muruganandam]

TKIP was disabled on encap offload mode since if TKIP MIC error
reporting is not capable by the driver using encap offload mode,
then TKIP countermeasures wont work.

Enabling back TKIP bring-up in encap offload mode to leave the
control with the driver to disable hw encap when it is not capable
of TKIP MIC error reporting.

Sathishkumar Muruganandam (2):
  mac80211: enable TKIP when using encapsulation offloading
  ath11k: add 802.3 undecap support to fix TKIP MIC error reporting

 drivers/net/wireless/ath/ath11k/dp_rx.c | 39 ++++++++++++++++++++++++++++++++-
 net/mac80211/iface.c                    |  7 ------
 net/mac80211/key.c                      |  7 ------
 3 files changed, 38 insertions(+), 15 deletions(-)

-- 
2.7.4

[PATCH 1/2] mac80211: enable TKIP when using encapsulation offloading

[Sathishkumar Muruganandam]

TKIP was disabled on encap offload mode since if TKIP MIC error
reporting is not capable by the driver using encap offload mode,
then TKIP countermeasures wont work.

Enabling back TKIP bring-up in encap offload mode to leave the
control with the driver to disable hw encap when it is not capable
of TKIP MIC error reporting.

Fixes: 50ff477a8639 ("mac80211: add 802.11 encapsulation offloading support")
Signed-off-by: Sathishkumar Muruganandam <murugana@codeaurora.org>
---
 net/mac80211/iface.c | 7 -------
 net/mac80211/key.c   | 7 -------
 2 files changed, 14 deletions(-)

diff --git a/net/mac80211/iface.c b/net/mac80211/iface.c
index d069825705d6..68af8acde0f0 100644
--- a/net/mac80211/iface.c
+++ b/net/mac80211/iface.c
@@ -1257,13 +1257,6 @@ bool ieee80211_set_hw_80211_encap(struct ieee80211_vif *vif, bool enable)
 	    (local->hw.wiphy->frag_threshold != (u32)-1))
 		enable = false;
 
-	mutex_lock(&sdata->local->key_mtx);
-	list_for_each_entry(key, &sdata->key_list, list) {
-		if (key->conf.cipher == WLAN_CIPHER_SUITE_TKIP)
-			enable = false;
-	}
-	mutex_unlock(&sdata->local->key_mtx);
-
 	__ieee80211_set_hw_80211_encap(sdata, enable);
 
 	return enable;
diff --git a/net/mac80211/key.c b/net/mac80211/key.c
index 8f403c1bb908..25cab6fc127c 100644
--- a/net/mac80211/key.c
+++ b/net/mac80211/key.c
@@ -177,13 +177,6 @@ static int ieee80211_key_enable_hw_accel(struct ieee80211_key *key)
 		}
 	}
 
-	/* TKIP countermeasures don't work in encap offload mode */
-	if (key->conf.cipher == WLAN_CIPHER_SUITE_TKIP &&
-	    sdata->hw_80211_encap) {
-		sdata_dbg(sdata, "TKIP is not allowed in hw 80211 encap mode\n");
-		return -EINVAL;
-	}
-
 	ret = drv_set_key(key->local, SET_KEY, sdata,
 			  sta ? &sta->sta : NULL, &key->conf);
 
-- 
2.7.4

Re: [PATCH 1/2] mac80211: enable TKIP when using encapsulation offloading

[Johannes Berg]

On Fri, 2020-04-24 at 15:05 +0530, Sathishkumar Muruganandam wrote:
> TKIP was disabled on encap offload mode since if TKIP MIC error
> reporting is not capable by the driver using encap offload mode,
> then TKIP countermeasures wont work.
> 
> Enabling back TKIP bring-up in encap offload mode to leave the
> control with the driver to disable hw encap when it is not capable
> of TKIP MIC error reporting.

So how can the driver do TKIP MIC error reporting?

I don't think it can. It doesn't have all the necessary information to
call cfg80211_michael_mic_failure(), as far as I can tell.

I'm not applying this until I see at least the *ability* for drivers to
do it. I mean, I don't really want to worry about your driver's
correctness, but if you then turn around and realize that you *cannot*
actually implement the right behaviour, that's no good either.

johannes

Re: [PATCH 1/2] mac80211: enable TKIP when using encapsulation offloading

[Sathishkumar Muruganandam]

On 2020-05-26 18:14, Johannes Berg wrote:
> On Fri, 2020-04-24 at 15:05 +0530, Sathishkumar Muruganandam wrote:
>> TKIP was disabled on encap offload mode since if TKIP MIC error
>> reporting is not capable by the driver using encap offload mode,
>> then TKIP countermeasures wont work.
>> 
>> Enabling back TKIP bring-up in encap offload mode to leave the
>> control with the driver to disable hw encap when it is not capable
>> of TKIP MIC error reporting.
> 
> So how can the driver do TKIP MIC error reporting?
> 
> I don't think it can. It doesn't have all the necessary information to
> call cfg80211_michael_mic_failure(), as far as I can tell.

Ath11k HW has capability of reporting TKIP MIC error when hw encap is 
enabled as well, so TKIP encryption shall be enabled.

> 
> I'm not applying this until I see at least the *ability* for drivers to
> do it. I mean, I don't really want to worry about your driver's
> correctness, but if you then turn around and realize that you *cannot*
> actually implement the right behaviour, that's no good either.
> 

Shall we have a ieee80211_hw_flags for the driver to advertise TKIP MIC 
error reporting capability when hw encap is enabled ?
TKIP bring-up will be disabled for the drivers which doesn't have the 
capability.

--
Sathishkumar

Re: [PATCH 1/2] mac80211: enable TKIP when using encapsulation offloading

[Johannes Berg]

On Fri, 2020-05-29 at 17:38 +0530, Sathishkumar Muruganandam wrote:

> > I don't think it can. It doesn't have all the necessary information to
> > call cfg80211_michael_mic_failure(), as far as I can tell.
> 
> Ath11k HW has capability of reporting TKIP MIC error when hw encap is 
> enabled as well, so TKIP encryption shall be enabled.

Maybe so. But it cannot tell wpa_supplicant about MIC errors, and so
that cannot do countermeasures, so something isn't right.

> Shall we have a ieee80211_hw_flags for the driver to advertise TKIP MIC 
> error reporting capability when hw encap is enabled ?
> TKIP bring-up will be disabled for the drivers which doesn't have the 
> capability.

That would be better, since for TKIP it's actually more tricky and
requires more work.

But mostly I was thinking that you should make it possible for ath11k to
actually report the MIC errors up to userspace. Right now I don't think
it can, because it doesn't have the netdev pointer?

johannes

Re: [PATCH 1/2] mac80211: enable TKIP when using encapsulation offloading

[Sathishkumar Muruganandam]

On 2020-05-29 17:40, Johannes Berg wrote:
> On Fri, 2020-05-29 at 17:38 +0530, Sathishkumar Muruganandam wrote:
> 
>> > I don't think it can. It doesn't have all the necessary information to
>> > call cfg80211_michael_mic_failure(), as far as I can tell.
>> 
>> Ath11k HW has capability of reporting TKIP MIC error when hw encap is
>> enabled as well, so TKIP encryption shall be enabled.
> 
> Maybe so. But it cannot tell wpa_supplicant about MIC errors, and so
> that cannot do countermeasures, so something isn't right.
> 
>> Shall we have a ieee80211_hw_flags for the driver to advertise TKIP 
>> MIC
>> error reporting capability when hw encap is enabled ?
>> TKIP bring-up will be disabled for the drivers which doesn't have the
>> capability.
> 
> That would be better, since for TKIP it's actually more tricky and
> requires more work.
> 
> But mostly I was thinking that you should make it possible for ath11k 
> to
> actually report the MIC errors up to userspace. Right now I don't think
> it can, because it doesn't have the netdev pointer?
> 

Yes, currently only tx encap support is added and rx decap support is in
progress to do TKIP MIC error reporting to userspace via 
cfg80211_michael_mic_failure().
With NL80211_CMD_MICHAEL_MIC_FAILURE, hostapd will be able to do TKIP 
counter-measures.

Thanks,
Sathishkumar

Re: [PATCH 1/2] mac80211: enable TKIP when using encapsulation offloading

[Johannes Berg]

On Fri, 2020-05-29 at 17:59 +0530, Sathishkumar Muruganandam wrote:
> 
> Yes, currently only tx encap support is added and rx decap support is in
> progress to do TKIP MIC error reporting to userspace via 
> cfg80211_michael_mic_failure().

Yes, but can you actually call that? It requires a netdev. I don't think
you can easily get it from the vif?

johannes

Re: [PATCH 1/2] mac80211: enable TKIP when using encapsulation offloading

[Sathishkumar Muruganandam]

On 2020-05-29 18:16, Johannes Berg wrote:
> On Fri, 2020-05-29 at 17:59 +0530, Sathishkumar Muruganandam wrote:
>> 
>> Yes, currently only tx encap support is added and rx decap support is 
>> in
>> progress to do TKIP MIC error reporting to userspace via
>> cfg80211_michael_mic_failure().
> 
> Yes, but can you actually call that? It requires a netdev. I don't 
> think
> you can easily get it from the vif?
> 

Yes, that's right. Currently the plan is to add new mac80211 api for rx 
decap where
we can call cfg80211_michael_mic_failure() and not called directly from 
driver.

Whether we can expose netdev to driver for doing such cfg80211 call ?

Thanks,
Sathishkumar

Re: [PATCH 1/2] mac80211: enable TKIP when using encapsulation offloading

[Johannes Berg]

On Fri, 2020-05-29 at 20:10 +0530, Sathishkumar Muruganandam wrote:
> On 2020-05-29 18:16, Johannes Berg wrote:
> > On Fri, 2020-05-29 at 17:59 +0530, Sathishkumar Muruganandam wrote:
> > > Yes, currently only tx encap support is added and rx decap support is 
> > > in
> > > progress to do TKIP MIC error reporting to userspace via
> > > cfg80211_michael_mic_failure().
> > 
> > Yes, but can you actually call that? It requires a netdev. I don't 
> > think
> > you can easily get it from the vif?
> > 
> 
> Yes, that's right. Currently the plan is to add new mac80211 api for rx 
> decap where
> we can call cfg80211_michael_mic_failure() and not called directly from 
> driver.

Sure, that sounds fine.

Really what I was saying is that we should have that together, not just
this patch that allows TKIP offload, but then nothing that actually
makes that working properly/safely.

> Whether we can expose netdev to driver for doing such cfg80211 call ?

I've always worried that if we expose the netdev we'll just get all
kinds of messy things happening in the driver :)

But maybe not. I guess it doesn't make a big difference.

johannes

Re: [PATCH 1/2] mac80211: enable TKIP when using encapsulation offloading

[Sathishkumar Muruganandam]

On 2020-05-29 20:14, Johannes Berg wrote:
> On Fri, 2020-05-29 at 20:10 +0530, Sathishkumar Muruganandam wrote:
>> On 2020-05-29 18:16, Johannes Berg wrote:
>> > On Fri, 2020-05-29 at 17:59 +0530, Sathishkumar Muruganandam wrote:
>> > > Yes, currently only tx encap support is added and rx decap support is
>> > > in
>> > > progress to do TKIP MIC error reporting to userspace via
>> > > cfg80211_michael_mic_failure().
>> >
>> > Yes, but can you actually call that? It requires a netdev. I don't
>> > think
>> > you can easily get it from the vif?
>> >
>> 
>> Yes, that's right. Currently the plan is to add new mac80211 api for 
>> rx
>> decap where
>> we can call cfg80211_michael_mic_failure() and not called directly 
>> from
>> driver.
> 
> Sure, that sounds fine.
> 
> Really what I was saying is that we should have that together, not just
> this patch that allows TKIP offload, but then nothing that actually
> makes that working properly/safely.

Sure, thanks Johannes !

Will include this along with rx decap patch.

> 
>> Whether we can expose netdev to driver for doing such cfg80211 call ?
> 
> I've always worried that if we expose the netdev we'll just get all
> kinds of messy things happening in the driver :)
> 
> But maybe not. I guess it doesn't make a big difference.
> 

Sure, will fallback to mac80211 approach for rx decap. Thanks for your 
review !

Thanks,
Sathishkumar

[PATCH 2/2] ath11k: add 802.3 undecap support to fix TKIP MIC error reporting

[Sathishkumar Muruganandam]

Currently DECAP_TYPE_8023 frames with TKIP MIC error is not undecaped
and hence fails to do TKIP MIC error reporting in ieee80211_rx_napi()
path.

Fix this by adding undecap support for ieee80211_rx_napi() to process
these frames and perform TKIP counter-measures when there is MIC error
reported.

Tested with STA triggering TKIP MIC error frames (using debugfs
"tkip_mic_test" in SW encryption mode) twice within a minute and
verify TKIP counter-measures are performed as expected.

Signed-off-by: Sathishkumar Muruganandam <murugana@codeaurora.org>
---
 drivers/net/wireless/ath/ath11k/dp_rx.c | 39 ++++++++++++++++++++++++++++++++-
 1 file changed, 38 insertions(+), 1 deletion(-)

diff --git a/drivers/net/wireless/ath/ath11k/dp_rx.c b/drivers/net/wireless/ath/ath11k/dp_rx.c
index bbd7da48518f..8db42a3d4a1f 100644
--- a/drivers/net/wireless/ath/ath11k/dp_rx.c
+++ b/drivers/net/wireless/ath/ath11k/dp_rx.c
@@ -1986,6 +1986,42 @@ static void ath11k_dp_rx_h_undecap_eth(struct ath11k *ar,
 	ether_addr_copy(ieee80211_get_SA(hdr), sa);
 }
 
+static void ath11k_dp_rx_h_undecap_snap(struct ath11k *ar,
+					struct sk_buff *msdu,
+					u8 *first_hdr,
+					enum hal_encrypt_type enctype,
+					struct ieee80211_rx_status *status)
+{
+	struct ieee80211_hdr *hdr;
+	size_t hdr_len;
+	u8 l3_pad_bytes;
+	struct hal_rx_desc *rx_desc;
+
+	/* Delivered decapped frame:
+	 * [amsdu header] <-- replaced with 802.11 hdr
+	 * [rfc1042/llc]
+	 * [payload]
+	 */
+
+	rx_desc = (void *)msdu->data - sizeof(*rx_desc);
+	l3_pad_bytes = ath11k_dp_rx_h_msdu_end_l3pad(rx_desc);
+
+	skb_put(msdu, l3_pad_bytes);
+	skb_pull(msdu, sizeof(struct ath11k_dp_amsdu_subframe_hdr) + l3_pad_bytes);
+
+	hdr = (struct ieee80211_hdr *)first_hdr;
+	hdr_len = ieee80211_hdrlen(hdr->frame_control);
+
+	if (!(status->flag & RX_FLAG_IV_STRIPPED)) {
+		memcpy(skb_push(msdu,
+				ath11k_dp_rx_crypto_param_len(ar, enctype)),
+		       (void *)hdr + hdr_len,
+			ath11k_dp_rx_crypto_param_len(ar, enctype));
+	}
+
+	memcpy(skb_push(msdu, hdr_len), hdr, hdr_len);
+}
+
 static void ath11k_dp_rx_h_undecap(struct ath11k *ar, struct sk_buff *msdu,
 				   struct hal_rx_desc *rx_desc,
 				   enum hal_encrypt_type enctype,
@@ -2013,7 +2049,8 @@ static void ath11k_dp_rx_h_undecap(struct ath11k *ar, struct sk_buff *msdu,
 					   enctype, status);
 		break;
 	case DP_RX_DECAP_TYPE_8023:
-		/* TODO: Handle undecap for these formats */
+		ath11k_dp_rx_h_undecap_snap(ar, msdu, first_hdr,
+					    enctype, status);
 		break;
 	}
 }
-- 
2.7.4

Re: [PATCH 2/2] ath11k: add 802.3 undecap support to fix TKIP MIC error reporting

[Kalle Valo]

Sathishkumar Muruganandam <murugana@codeaurora.org> wrote:

> Currently DECAP_TYPE_8023 frames with TKIP MIC error is not undecaped
> and hence fails to do TKIP MIC error reporting in ieee80211_rx_napi()
> path.
> 
> Fix this by adding undecap support for ieee80211_rx_napi() to process
> these frames and perform TKIP counter-measures when there is MIC error
> reported.
> 
> Tested with STA triggering TKIP MIC error frames (using debugfs
> "tkip_mic_test" in SW encryption mode) twice within a minute and
> verify TKIP counter-measures are performed as expected.
> 
> Signed-off-by: Sathishkumar Muruganandam <murugana@codeaurora.org>

Dropping this patch as the mac80211 patch was dropped.

Patch set to Rejected.

-- 
https://patchwork.kernel.org/patch/11507411/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches

Re: [PATCH 0/2] mac80211: enable TKIP when using encapsulation offloading

[Kalle Valo]

Sathishkumar Muruganandam <murugana@codeaurora.org> writes:

> TKIP was disabled on encap offload mode since if TKIP MIC error
> reporting is not capable by the driver using encap offload mode,
> then TKIP countermeasures wont work.
>
> Enabling back TKIP bring-up in encap offload mode to leave the
> control with the driver to disable hw encap when it is not capable
> of TKIP MIC error reporting.
>
> Sathishkumar Muruganandam (2):
>   mac80211: enable TKIP when using encapsulation offloading
>   ath11k: add 802.3 undecap support to fix TKIP MIC error reporting

When you resend please increase the version to make it clear what is the
latest version. So this is actually v2, but no need to resend because of
this.

-- 
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches