mbox series

[v2,0/4] Extended Key ID support

Message ID 20190319203410.25145-1-alexander@wetzel-home.de (mailing list archive)
Headers show
Series Extended Key ID support | expand

Message

Alexander Wetzel March 19, 2019, 8:34 p.m. UTC
This patch series adds support for IEEE 802.11-2016 Extended Key ID
support. Compared to the last RFC there are again quite some API
changes, but also some bug fixes. (The bug fixes I remember are outlined
in the different patches.)

The main differences are:
1) This series drops support to let the driver decide which key/keyid
   shall be used to encrypt a MPDU. The drivers must now always encrypt
   the MPDU with the key mac80211 has selected for it. This allows us to
   use the "normal" key install API for drivers and gets rid of special
   calls for Extended Key ID between mac80211 and the drivers.

2) It also drops the overly complex handling for tailroom needed and
   just handles the Rx-only keys like Rx/Tx ones.

3) The "old" Rx-only key flag has been replaced by
   IEEE80211_KEY_FLAG_NO_AUTO_TX. It's no longer cleared and primarily
   intended to stop ieee80211_key_replace() to activating the key for
   Tx, but also allows COMPAT driver to determine if Rx HW crypto can be
   activated or not.

4) COMPAT Extended Key ID will enable Rx decryption offload only after
   (at least) one MPDU has been decoded for the key with SW crypto.

5) COMPAT Extended Key ID support now has two dedicated key calls for
   activating/deactivating Rx HW offload.

6) A-MPDU border signal is now generated unconditionally, so there will
   always be one more packet with the old key, regardless of the time
   passed since the new key has been activated.

I think the API here is much simpler to understand and use, but it's
also a reversal of the decision from the first RFC version to not use
key flags to distinguish between normal and Extended Key ID installs.
(Normally only COMPAT drivers should care about the flag.)

I've tested the patches, but mostly only the full series.

Changes compared to v1 of the patch:
 - Native Extended Key ID is enabled automatically for drivers not
   supporting hardware encryption and do not offer a set_key() callback.

Alexander Wetzel (4):
  nl80211/cfg80211: Extended Key ID support
  mac80211: IEEE 802.11 Extended Key ID support
  mac80211: Compatibility Extended Key ID support
  mac80211: Mark A-MPDU keyid borders for drivers

 include/net/cfg80211.h       |   2 +
 include/net/mac80211.h       |  58 +++++++++++++-
 include/uapi/linux/nl80211.h |  28 +++++++
 net/mac80211/cfg.c           |  36 +++++++++
 net/mac80211/debugfs.c       |   2 +
 net/mac80211/ieee80211_i.h   |   2 +-
 net/mac80211/key.c           | 143 ++++++++++++++++++++++++++++++-----
 net/mac80211/key.h           |   7 ++
 net/mac80211/main.c          |  24 ++++++
 net/mac80211/rx.c            |  80 +++++++++++---------
 net/mac80211/sta_info.c      |  12 +++
 net/mac80211/sta_info.h      |   7 +-
 net/mac80211/tx.c            |  73 +++++++++++++-----
 net/wireless/nl80211.c       |  32 +++++++-
 net/wireless/rdev-ops.h      |   3 +-
 net/wireless/trace.h         |  31 ++++++--
 net/wireless/util.c          |  21 +++--
 17 files changed, 468 insertions(+), 93 deletions(-)

Comments

Johannes Berg April 8, 2019, 7:57 p.m. UTC | #1
On Tue, 2019-03-19 at 21:34 +0100, Alexander Wetzel wrote:
> This patch series adds support for IEEE 802.11-2016 Extended Key ID
> support. Compared to the last RFC there are again quite some API
> changes, but also some bug fixes. (The bug fixes I remember are outlined
> in the different patches.)

FWIW, I've applied the first two patches here.

I'd really like you to continue with only that for now, and (try to) get
hostapd/wpa_supplicant changes upstream, perhaps with corresponding
hwsim tests. That way, we can see this working live.

We briefly discussed this at the wireless workshop, and the general
feeling seems to have been that the compat mode is probably not really
worthwhile, but I haven't quite made up my mind on that.

Still, having tests and being able to check it out would help a lot,
also as part of perhaps building a case for compat mode.

Thanks,
johannes
Alexander Wetzel April 9, 2019, 7:39 p.m. UTC | #2
Am 08.04.19 um 21:57 schrieb Johannes Berg:
> On Tue, 2019-03-19 at 21:34 +0100, Alexander Wetzel wrote:
>> This patch series adds support for IEEE 802.11-2016 Extended Key ID
>> support. Compared to the last RFC there are again quite some API
>> changes, but also some bug fixes. (The bug fixes I remember are outlined
>> in the different patches.)
> 
> FWIW, I've applied the first two patches here.
> 
> I'd really like you to continue with only that for now, and (try to) get
> hostapd/wpa_supplicant changes upstream, perhaps with corresponding
> hwsim tests. That way, we can see this working live.

That's splendid:-)
I'll try to get the hostapd/wpa_supplicant patches finalized in the next 
weeks. Hopefully I have something to submit here once the Extended Key 
ID API is in mainline immediately.

But if you or someone else want to run some tests prior to that.
I've updated the patches available here with the ones I'm currently 
using: https://www.awhome.eu/index.php/s/AJJXBLsZmzHdxpX

They are fully functional and I'm using them on my main AP and 
workstation for months now.
They are on top of the current hostapd HEAD and should not cause any 
regressions. They are also comparable simple to port between versions. 
The code we touch here has not been changed in any relevant way since I 
started working on the patches. Just really copy nl80211_copy.h from the 
kernel...

A sad side note:
I'm pretty sure I've already found one broken Wlan implementation: 
Samsung Galaxy Tap S3
This device seems to just copy the RSN Capability (bit) from the AP and 
then fails when the AP rekeys the connection and starts using keyid 1...
But it's probably a Samsung specific bug, other Android devices - 
including a Google Pixel 3 - don't "lie" in the RSN and are using 
"legacy" key installs as they should.

> 
> We briefly discussed this at the wireless workshop, and the general
> feeling seems to have been that the compat mode is probably not really
> worthwhile, but I haven't quite made up my mind on that.

I would like to be able to support Extended Key ID for the the Atheros 
cards < ath10k... Maybe that gets the attention of someone who can fix 
the ath10k firmware to also support it, hopefully in NATIVE mode:-)
But I see that the COMPAT mode may not be worth the added complexity in 
the long term.

Which reminds me:
One potential follow up on the COMPAT code would be seamless Rx during 
rekey even when not using Extended Key ID. It would need some slight of 
hand but I think it's possible. Basically we just would have to try the 
second key also if the first one did not decode the MPTU. So far I've 
shelved any plans into that direction as too complex for too little 
gain. And after all people should just start using Extended Key ID 
instead. After all that's the point of having standards...

> Still, having tests and being able to check it out would help a lot,
> also as part of perhaps building a case for compat mode.

Besides some review of the patches with my improved 802.11/code 
understanding the only thing missing in the current 
wpa_supplicant/hostapd patches *are* the test cases.

The existing rekey tests are already working fine with Extended Key ID. 
Separating Extended Key ID rekey from "legacy" rekeys tests is probably 
half of the work here...
With the patches you merged and the linked hostapd/wpa_supplicant 
patches any (WPA2) rekey test will use Extended Key ID. (They won't test 
any of the really interesting code pathes in the kernel, trough. No 
A-MPDU, no TX-Pull and of course no HW crypto or RX/TX fast path if I 
remember it right.)

Thanks,
Alexander
Marcel Holtmann April 10, 2019, 2:37 p.m. UTC | #3
Ho Alexander,

>>> This patch series adds support for IEEE 802.11-2016 Extended Key ID
>>> support. Compared to the last RFC there are again quite some API
>>> changes, but also some bug fixes. (The bug fixes I remember are outlined
>>> in the different patches.)
>> FWIW, I've applied the first two patches here.
>> I'd really like you to continue with only that for now, and (try to) get
>> hostapd/wpa_supplicant changes upstream, perhaps with corresponding
>> hwsim tests. That way, we can see this working live.
> 
> That's splendid:-)
> I'll try to get the hostapd/wpa_supplicant patches finalized in the next weeks. Hopefully I have something to submit here once the Extended Key ID API is in mainline immediately.

have you tried to add Extended Key ID support into iwd?

Regards

Marcel