| Message ID | 1587720951-9222-2-git-send-email-murugana@codeaurora.org (mailing list archive) |
|---|---|
| State | Rejected |
| Delegated to: | Johannes Berg |
| Headers | show |
| Series | mac80211: enable TKIP when using encapsulation offloading | expand |
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
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
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
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
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
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
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
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
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);
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(-)