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

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 <[email protected]>
---
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


2020-05-26 13:10:10

by Johannes Berg

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

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

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

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

2020-05-29 12:14:41

by Johannes Berg

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

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

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

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

2020-05-29 12:47:38

by Johannes Berg

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

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

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

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

2020-05-29 14:46:48

by Johannes Berg

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

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

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

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