mbox series

[v6,00/16] Support seamless PTK rekeys with Extended Key ID

Message ID 20190915200837.196283-1-alexander@wetzel-home.de
Headers show
Series Support seamless PTK rekeys with Extended Key ID | expand

Message

Alexander Wetzel Sept. 15, 2019, 8:08 p.m. UTC
Remark: Most of the text here and the other commends are taken 1:1 from
the v3 of the series and is identical to what was in v4 and v5 with only
minor updates.

Extended Key ID for Individually Addressed Frames has been added to the
IEEE 802.11 - 2012 standard to allow STAs to rekey the pairwise key
without interruption for ongoing data traffic.

A good and easy to follow introduction to Extended Key ID can be found
here:
https://mentor.ieee.org/802.11/dcn/10/11-10-0313-01-000m-rekeying-protocol-fix.ppt
In a nutshell Extended Key ID is just allowing to also use keyid 1 for
pairwise keys: We can now have two pairwise keys in parallel, allowing
to switch between them comparable how it's already done for GTK and
IGTK keys.

All implementations of IEEE 802.11 without Extended Key ID only use the
keyid 0 for unicast MPDUs. Since each STA can only have a single unicast
key installed at any time there is no time frame when the old or the
new key can be used. When rekeying the unicast key under load MPDU
losses are therefore unavoidable: Each STA will get MPDUs encrypted with
a key which is either no longer installed or is not installed, yet.
In both cases the remote STA will be unable to decrypt the MPDUs and
drop them. (Since only the decryption fails the frames are still
acknowledged and are truly lost. It's up to e.g. tcp to retransmit
them.)

Especially on high speed connections this can easily affect a few dozen
MPDUs, drastically throttle the speed for tcp due or cause drastic
audio/video glitches with udp transmissions.
(Most implementations are even not able to rekeys correctly, making
matters worse. But that's not really relevant for this patch series.)

This patch series implements Extended Key ID for both hostapd and
wpa_supplicant to allow unicast key rekeys without MPDU loss and/or
impact to the performance.
While there are still ways to extend or improve this patch set it's
already quite complex and I believe this to be good enough to be merged
in the current state. Or in other words: It's high time to get some
feedback on this big patch set instead of adding more to it.

Since Extended Key ID breaks the long-standing agreement of only one
pairwise key (keyid 0) and also has to postpone the Tx usage of a newly
installed key this can only work with drivers actively supporting
Extended Key ID.
So far Extended Key ID is only supported for linux kernels >=5.2 and
cards only providing SW crypto. This patch series has been developed
using hwsim (works out of the box with a kernel >= 5.2.12) and - not yet
official supported - iwlwifi cards (supporting HW crypto). (All iwlwifi
cards prior to the current 22000 series are working and probably will
officially start to support Extended Key ID with one of the next linux
kernels.)
Some early versions of this series also have been tested with ath9, but
that required special mac80211 workarounds which have not been merged
upstream and abandoned for the time being.

The patch series consist of basically five parts:

01-hostapd: Provide correct keyid to wpa_send_eapol()
02-nl80211: Migrate to current netlink key message format
03-Driver: Add key_type and Extended Key ID driver flag
04-Add new argument key_type to all set_key() functions
05-hostapd: Set the correct key_type for key installs
06-wpa_supplicant: Set the correct key_type for key installs
07-drivers: Migrate drivers from set_tx to key_type API
08-nl80211: Switch to the new key_type API & cleanup
09-nl80211,wpa_supplicant: Drop outdated tdls hack

 1) The first 9 patches are addressing issues not directly tied to the
 Extended Key ID support. More details are directly in the individual patch
 descriptions, but they are primarily related to deprecating the set_tx
 "boolean" used in various set_key() functions and replace it with
 something more flexible which also can handle Extended Key ID key
 installs.

10-hostapd: Add support for Extended Key ID
11-wpa_supplicant: AP Extended Key ID support
12-tests: Extended Key ID tests

 2) The patches 10-12 are handling the Extended Key ID support for
 hostapd/wpa_supplicant and modify/amend to the existing tests to be
 able to work correctly with and without Extended Key ID support.

13-hostapd: FILS Extended Key ID support
14-wpa_supplicant: FILS Extended Key ID support

 3) Patches 13 and 14 are trying to close a gap not covered in the
 official standard. I'm far from sure that this is the correct way
 and hope we can have a discussion here how to best handle FILS and
 probably also FT. (The standard does not have any guidance for that,
 too.)

15-nl80211: Extended Key ID support

 4) The patch 15 is adding Extended Key ID support to nl80211, basically
 activating all the previous Extended Key ID patches when you are using
 linux 5.2 or greater with a compatible card. As of now all cards using
 mac80211 and NOT providing the "set_key()" callback are compatible.
 There is also a patch allowing to use it with most iwlwifi cards:
 https://patchwork.kernel.org/patch/11101659/
 There is an API bug in 5.2 kernels < 5.2.12, preventing unicast keys
 using keyid 1 to be installed for FILS and FT handshakes. This patch
 set therefore requires a kernel >= 5.2.12.

16-Drop set_tx from all set_key() functions
17-hostapd: Extended Key ID stress test

 5) The last two patches, (16+17) are kind of optional. Patch 16 is
 finishing the work the first 9 patches started. Since redesigning the
 set_tx API is the most invasive part of the patch series it is sometimes
 interesting to see how the old set_tx key calls would have acted.
 Patch 17 is so far just a proof that everything still works when we
 use keyid 1 at the initial connect instead of the more traditional
 keyid 0. I have some ideas to extend that, allowing hostapd to detect
 broken STAs claiming to be compatible with Extended Key ID  - due to
 broken capability announcement - and still work with APs supporting it.
 (At least the Samsung Galaxy Tab S3 is setting the RSN capabilities
 wrong and chances are it's not the only one...)

Besides using the hostapd tests I've also verified the function with a
real AP using a patched iwlwifi driver with HW crypto offload.

The integration of Extended Key ID support in hostapd/wpa_supplicant
tries to maintain full backward compatibility with kernels not
supporting Extended Key ID. While this is foreseen in the standard for
normal operation it was more challenging for the built-in test cases.
Instead of basically doubling all test cases and execute them with and
without Extended Key ID support I've just modified all test cases to be
able to run with and without Extended Key ID support. All tests will per
default use Extended Key ID when supported or silently fall back to the
"classical" way to do things. The downside here is, that the same tests
executed on a system supporting Extended Key ID will test something
slightly different than a system not supporting it...

At the moment there are just four special Extended Key ID tests to
verify it's being used and we interact correctly with STAs not
supporting it.

To make it a bit simpler to see what the patches are changing I've
uploaded you some files containing multiple full test runs:

 1) 21dc1627f-orig.tgz
    Tests run with a unpatched hostapd (git version 21dc1627f)

 2) 21dc1627f-patched.tgz
    Test run with hwsim supporting Extended Key ID and all patches of
    the of this series applied.

 3) 21dc1627f-patched-legacy.tgz
    All patches of this series applied. With Extended Key ID detection
    sabotaged to force classical rekeys.

Each file is roughly 190MB big and can be downloaded here:
https://www.awhome.eu/index.php/s/jDQfpZmDGwsLPXA
(The test runs from the older patch sets are also available there)

All tests were executed running wt-2019-09-04 from wireless-testing.
(The kernel has two additional patched on top of the official version
to use Extended Key ID with iwlwifi, and a debug patch with printk's
informing me e.g. when installing, deleting or activating any PTK.

Most of the failed tests are related to a wrong Python call to AES
encrypt data, missing one argument. (I'm using Python 3.6.9)
All failing tests seem to be unrelated to the changes here and can be
reproduced without the patches.

I've added two extra files in each test directory:
 - failed.log
   All tests failed in this run. With a comment when it worked when
   trying the test a second time. (But without adding the output from the
   rerun.)
 - script.log
   The output of the actual test run.

Version history:

Patch version 6:
 - Added the "missing" patch number 1. Should have been part of V3 but
   somehow it never got exported from my local tree...
 - Reverted the incorrect fix for max keyid in the patch
   "wpa_supplicant: AP Extended Key ID support"
 - Moved code disabling Extended Key ID for tkip to a better location
   in patch "hostapd: Add support for Extended Key ID"
 - Tweaked Extended Key ID configuration handling in
   "hostapd: Add support for Extended Key ID": It's now automatically
   disabled 
 - Some minor tweaks to comments and output and a bit more invasive
   rewording of commit messages

Patch version 5:
 - undo incorrect conversion from "int key_type" to
   "enum key_type key_type" (this is also an independent variable)
 - fixed multiple broken "enum key_type" which must be
   "enum key_type key_type"
 - undo pointless variable move in "wpa_supplicant: FILS Extended Key ID
   support" which did break build tests.
 - again minor fixes to commit messages

Patch version 4:
 - Fixed sequence number key queries (undo invalid switch to new API
   names.)
 - get rid of some forgotten "int key_flag" references and switched them
   over to the correct "enum key_flag key_flag"
 - Dropped the workaround patch enabling FILS and FT key installs with
   current released kernels (API is being updated upstream)
 - removed some stray newlines
 - reworded Extended Key ID description in wpa_supplicant.conf
 - minor updates to commit messages
 - Overhauled and updated commit remarks (nothing critical)

Patch version 3:
 - use key_type (enum) instead of key_flag (bit array)
 - migrate set_tx to the new key_type API and clean up related
   workarounds and bugs.
 - also support Extended Key ID for OSEN, FT and FILS
 - wpa_supplicant is now checking RSN capabilities instead of the
   presence of the KeyID KDE to enable Extended Key ID support to follow
   the wording of the standard
 - detect and prevent dropping or adding Extended Key ID support on
   rekey. (Initial connection decides if we can use it.)
 - Don't use Extended Key ID for TKIP. (Not allowed in the standard.)
 - also flip KeyIDs for WPA_REAUTH and WPA_REAUTH_EAPOL
 - fixed many "test-only" bugs (both tests and test only code)
 - split fixes and extensions in different patches
 - dropped "GET drv_flags" and handle it differently
 - better description of the patches

Patch version 2:
 - typo fix in commit message

Patch version 1:
 Initial release


Alexander Wetzel (17):
  hostapd: Provide correct keyid to wpa_send_eapol()
  nl80211: Migrate to current netlink key message format
  Driver: Add key_type and Extended Key ID driver flag
  Add new argument key_type to all set_key() functions
  hostapd: Set the correct key_type for key installs
  wpa_supplicant: Set the correct key_type for key installs
  drivers: Migrate drivers from set_tx to key_type API
  nl80211: Switch to the new key_type API & cleanup
  nl80211,wpa_supplicant: Drop outdated tdls hack
  hostapd: Add support for Extended Key ID
  wpa_supplicant: AP Extended Key ID support
  tests: Extended Key ID tests
  hostapd: FILS Extended Key ID support
  wpa_supplicant: FILS Extended Key ID support
  nl80211: Extended Key ID support
  Drop set_tx from all set_key() functions
  hostapd: Extended Key ID stress test

 hostapd/config_file.c              |   2 +
 hostapd/ctrl_iface.c               |  68 +++++++-----
 hostapd/hostapd.conf               |  10 ++
 src/ap/ap_config.c                 |  10 ++
 src/ap/ap_config.h                 |   1 +
 src/ap/ap_drv_ops.c                |   9 +-
 src/ap/ap_drv_ops.h                |   5 +-
 src/ap/hostapd.c                   |  17 +--
 src/ap/hs20.c                      |   2 +
 src/ap/ieee802_11.c                |   6 +-
 src/ap/ieee802_1x.c                |  14 +--
 src/ap/wpa_auth.c                  | 110 ++++++++++++++-----
 src/ap/wpa_auth.h                  |   4 +-
 src/ap/wpa_auth_ft.c               |  13 ++-
 src/ap/wpa_auth_glue.c             |  38 ++++++-
 src/ap/wpa_auth_i.h                |   3 +
 src/ap/wpa_auth_ie.c               |  50 ++++++++-
 src/common/wpa_common.c            |   1 +
 src/common/wpa_common.h            |   9 ++
 src/drivers/driver.h               |  33 ++++--
 src/drivers/driver_atheros.c       |  10 +-
 src/drivers/driver_bsd.c           |  13 +--
 src/drivers/driver_hostap.c        |   9 +-
 src/drivers/driver_ndis.c          |  25 +++--
 src/drivers/driver_nl80211.c       | 164 ++++++++++++++++-------------
 src/drivers/driver_nl80211_capa.c  |   4 +
 src/drivers/driver_openbsd.c       |   5 +-
 src/drivers/driver_privsep.c       |  12 +--
 src/drivers/driver_wext.c          |  27 ++---
 src/drivers/driver_wext.h          |   4 +-
 src/rsn_supp/tdls.c                |   9 +-
 src/rsn_supp/wpa.c                 | 152 ++++++++++++++++++++++----
 src/rsn_supp/wpa.h                 |   9 +-
 src/rsn_supp/wpa_ft.c              |  17 +--
 src/rsn_supp/wpa_i.h               |  12 ++-
 src/rsn_supp/wpa_ie.c              |  11 ++
 src/rsn_supp/wpa_ie.h              |   1 +
 tests/hwsim/hostapd.py             |  19 +++-
 tests/hwsim/test_ap_ciphers.py     |   2 +-
 tests/hwsim/test_ap_eap.py         |  20 +++-
 tests/hwsim/test_ap_psk.py         | 124 ++++++++++++++++++++--
 tests/hwsim/test_ocv.py            |  13 ++-
 tests/hwsim/test_rrm.py            |   7 +-
 tests/hwsim/wpasupplicant.py       |   7 +-
 wlantest/bss.c                     |   4 +-
 wlantest/rx_eapol.c                |   4 +
 wpa_supplicant/config.c            |   2 +
 wpa_supplicant/config_file.c       |   1 +
 wpa_supplicant/config_ssid.h       |  10 ++
 wpa_supplicant/ctrl_iface.c        |  36 ++++---
 wpa_supplicant/driver_i.h          |  23 ++--
 wpa_supplicant/ibss_rsn.c          |  19 ++--
 wpa_supplicant/mesh_mpm.c          |  12 +--
 wpa_supplicant/mesh_rsn.c          |  14 +--
 wpa_supplicant/preauth_test.c      |   2 +-
 wpa_supplicant/wpa_cli.c           |   3 +-
 wpa_supplicant/wpa_supplicant.c    |  38 +++++--
 wpa_supplicant/wpa_supplicant.conf |   5 +
 wpa_supplicant/wpas_glue.c         |  24 +++--
 59 files changed, 947 insertions(+), 331 deletions(-)

Comments

Jouni Malinen Sept. 21, 2019, 12:55 p.m. UTC | #1
On Sun, Sep 15, 2019 at 10:08:20PM +0200, Alexander Wetzel wrote:
> The patch series consist of basically five parts:
> 
> 01-hostapd: Provide correct keyid to wpa_send_eapol()
> 02-nl80211: Migrate to current netlink key message format

As an initial step, I applied those two now.

> 03-Driver: Add key_type and Extended Key ID driver flag
> 04-Add new argument key_type to all set_key() functions
> 05-hostapd: Set the correct key_type for key installs
> 06-wpa_supplicant: Set the correct key_type for key installs

And those I have went through and have couple of pending questions (or
well, already answered questions, but I have not yet fully reviewed the
responses).

I put my current work versions here so that you can see how exactly I
was thinking of changing these:

https://w1.fi/p/set-key/

It would be good to consider those as baseline for any additional
changes. I have not yet went through all the details in the following
patches, so no changes proposed for those as of now.
Alexander Wetzel Sept. 21, 2019, 4:45 p.m. UTC | #2
> I put my current work versions here so that you can see how exactly I
> was thinking of changing these:
> 
> https://w1.fi/p/set-key/
> 
> It would be good to consider those as baseline for any additional
> changes. I have not yet went through all the details in the following
> patches, so no changes proposed for those as of now.
> 
As mentioned I already have some more fixes pending.
Now a quick test run on Thursday was generally looking good but showed 
something unexpected in the logs.

I had no time to follow up on that, yet... (The new TKIP only AP test 
was working, but I did trip over an unexpected log message, indicating 
something may be off or I messed something up with the test run.)

It does not look like I will be able to look into that till probably 
Monday evening.

So I just rebased my working tree yours and exported the WIP patches:
https://www.awhome.eu/index.php/s/o5m9Q29n37zbEga
You can at least see now how I want to change handle_extended_key_id()

They probably still have some minor errors and the commit messages are 
crap.

Alexander