hostapd: correct IEEE 802.11r options

[devianceluka]

SHORT:

for an absolute minimal working IEEE 802.11r, option ieee80211r '1' is all that is needed in LEDE

everything else is optional

LONG:

IEEE 802.11r has many sub-options inside it's hostapd block of options

in LEDE, we have a UCI special option (option ieee80211r '1') that is not present in hostapd.conf and it enables these hostapd.conf sub-options

hostapd.conf sub-options' default values do not enable a working IEEE 802.11r configuration

ft_psk_generate_local 1 from man:

This avoids use of PMK-R1 push/pull from other APs with FT-PSK networks as the required information (PSK and other session data) is already locally available.

---

1. it would be logical, that UCI special option option ieee80211r '1' actually enables a default working IEEE 802.11r from the start, with minimal sub-options defined

2. if we agree on this logic, it would be best that we start with a default value of 1 in ft_psk_generate_local 1

if it's 0 (previous default), user anyway has to provide unique r1_key_holder r0kh and r1kh as they are not locally generated

if per BSS, ft_psk_generate_local 1 is the new default value left unchanged by user,
previous default value of r1_key_holder is left unchanged by user and r0kh and r1kh are not provided by user, default configuration of option ieee80211r '1' is in a non-working or kind of working default state of IEEE 802.11r

3. then default values of these are not wanted/needed anymore (they can only break something):
   r0_key_lifetime 1000
r1_key_holder "00004f577274"

4. also then, this value wants to be 0:
   pmk_r1_push 0

5. also then lastly, these want to be left empty:
   r0kh
r1kh

6. nasid is mandatory for a working IEEE 802.11r, so a check here would be nice.
   nasid needs to be unique per BSS, so it can atleast be made from BSSID per BSS

7. mobility_domain is also mandatory for a working IEEE 802.11r, so a default value can be hashed, so it's identical per BSS and also unique if there are multi SSIDs per AP

8. r1_key_holder proper default value is BSSID per BSS

9. sum of all mandatory options for a minimal working setup BEFORE this commit:
   option ieee80211r 1
option mobility_domain '<unique per BSS>'
option nasid '<unique per AP, can be bssid>'
option ft_psk_generate_local '1'
cannot disable r0kh r1kh <-- thats why its a non-working or kind of working setup

10. sum of all mandatory options for a minimal working setup AFTER this commit:
    option ieee80211r 1

11. ALL this does NOT break behaviour in our option ieee80211r '1', it only disables mistakes and supercharges it

Signed-off-by: Gospod Nassa devianca@gmail.com

[BigNerd95]

I think they used those default values because they want use the same default values as the upstream hostap project.

(Extracted from https://w1.fi/cgit/hostap/tree/hostapd/hostapd.conf)

##### IEEE 802.11r configuration ##############################################

# Mobility Domain identifier (dot11FTMobilityDomainID, MDID)
# MDID is used to indicate a group of APs (within an ESS, i.e., sharing the
# same SSID) between which a STA can use Fast BSS Transition.
# 2-octet identifier as a hex string.
#mobility_domain=a1b2

# PMK-R0 Key Holder identifier (dot11FTR0KeyHolderID)
# 1 to 48 octet identifier.
# This is configured with nas_identifier (see RADIUS client section above).

# Default lifetime of the PMK-RO in minutes; range 1..65535
# (dot11FTR0KeyLifetime)
#r0_key_lifetime=10000

# PMK-R1 Key Holder identifier (dot11FTR1KeyHolderID)
# 6-octet identifier as a hex string.
# Defaults to BSSID.
#r1_key_holder=000102030405

# Reassociation deadline in time units (TUs / 1.024 ms; range 1000..65535)
# (dot11FTReassociationDeadline)
#reassociation_deadline=1000

# List of R0KHs in the same Mobility Domain
# format: <MAC address> <NAS Identifier> <256-bit key as hex string>
# This list is used to map R0KH-ID (NAS Identifier) to a destination MAC
# address when requesting PMK-R1 key from the R0KH that the STA used during the
# Initial Mobility Domain Association.
#r0kh=02:01:02:03:04:05 r0kh-1.example.com 000102030405060708090a0b0c0d0e0f000102030405060708090a0b0c0d0e0f
#r0kh=02:01:02:03:04:06 r0kh-2.example.com 00112233445566778899aabbccddeeff00112233445566778899aabbccddeeff
# And so on.. One line per R0KH.
# Wildcard entry:
# Upon receiving a response from R0KH, it will be added to this list, so
# subsequent requests won't be broadcast. If R0KH does not reply, it will be
# blacklisted.
#r0kh=ff:ff:ff:ff:ff:ff * 00112233445566778899aabbccddeeff

# List of R1KHs in the same Mobility Domain
# format: <MAC address> <R1KH-ID> <256-bit key as hex string>
# This list is used to map R1KH-ID to a destination MAC address when sending
# PMK-R1 key from the R0KH. This is also the list of authorized R1KHs in the MD
# that can request PMK-R1 keys.
#r1kh=02:01:02:03:04:05 02:11:22:33:44:55 000102030405060708090a0b0c0d0e0f000102030405060708090a0b0c0d0e0f
#r1kh=02:01:02:03:04:06 02:11:22:33:44:66 00112233445566778899aabbccddeeff00112233445566778899aabbccddeeff
# And so on.. One line per R1KH.
# Wildcard entry:
# Upon receiving a request from an R1KH not yet known, it will be added to this
# list and thus will receive push notifications.
#r1kh=00:00:00:00:00:00 00:00:00:00:00:00 00112233445566778899aabbccddeeff

# Timeout (seconds) for newly discovered R0KH/R1KH (see wildcard entries above)
# Special values: 0 -> do not expire
# Warning: do not cache implies no sequence number validation with wildcards
#rkh_pos_timeout=86400 (default = 1 day)

# Timeout (milliseconds) for requesting PMK-R1 from R0KH using PULL request
# and number of retries.
#rkh_pull_timeout=1000 (default = 1 second)
#rkh_pull_retries=4 (default)

# Timeout (seconds) for non replying R0KH (see wildcard entries above)
# Special values: 0 -> do not cache
# default: 60 seconds
#rkh_neg_timeout=60

# Note: The R0KH/R1KH keys used to be 128-bit in length before the message
# format was changed. That shorter key length is still supported for backwards
# compatibility of the configuration files. If such a shorter key is used, a
# 256-bit key is derived from it. For new deployments, configuring the 256-bit
# key is recommended.

# Whether PMK-R1 push is enabled at R0KH
# 0 = do not push PMK-R1 to all configured R1KHs (default)
# 1 = push PMK-R1 to all configured R1KHs whenever a new PMK-R0 is derived
#pmk_r1_push=1

# Whether to enable FT-over-DS
# 0 = FT-over-DS disabled
# 1 = FT-over-DS enabled (default)
#ft_over_ds=1

# Whether to generate FT response locally for PSK networks
# This avoids use of PMK-R1 push/pull from other APs with FT-PSK networks as
# the required information (PSK and other session data) is already locally
# available.
# 0 = disabled (default)
# 1 = enabled
#ft_psk_generate_local=0

But I don't know why (for example) they used

r1_key_holder "00004f577274"

while in the upstream config is

# Defaults to BSSID.
#r1_key_holder=000102030405

[BigNerd95]

Anyway you should do

git commit --amend

and fix your commit message adding the package name and your sign at the end.
Then you can update the PR with

git push --set-upstream origin --force 802.11r-2

[BigNerd95]

This commit https://w1.fi/cgit/hostap/commit/?id=96590564d658cf344778e9c84bcd58d39764e11d confirms that

for FT-PSK networks, no configuration of inter-AP communication is
needed anymore when using ft_psk_generate_local=1 configuration.

[devianceluka]

Im a new GitHub user, please tell me where and what in GitHub Desktop for Windows should I fix?

As for your comment on default values, please read my whole message. They already calculate them by default (they are going to deprecate them sooner or later - maybe only for very specific reason someone needs them), thats why we dont need them. Our special option should enable 80211r by default, at the moment it does not.

[BigNerd95]

I don't use Windows neither a gui for git :-/
Search how to amend the latest commit on the web.

Yes, I agree with your comment about default values, my response was only a clarification about why they used them.

[devianceluka]

There, please merge, got more coming up

[devianceluka]

@stintel could you review and accept this?

[devianceluka]

In LUCI only these are then mandatory per BSS (non advanced):
ieee80211r 1
mobility_domain <identical group>
Half mandatory:
nasid <unique name> (new default value is system hostname, which should already be unique)

[stintel]

Try converting 00004f577274 (r1_key_holder default) from hex to ascii :-) For nasid, to be truly unique, I suggest interface_name.fqdn

[devianceluka]

For true uniqueness per BSS, even that does not apply to my situation where I have both bands bridged to the same interface.

So.... BIG thank you for stimulating my brain into this matter yet again. True uniqueness is yet again BSSID.

[BigNerd95]

Is the ft_over_ds option necessary even with ft_psk_generate_local enabled?
This commit https://w1.fi/cgit/hostap/commit/?id=96590564d658cf344778e9c84bcd58d39764e11d tells:

Therefore neither caching nor communication between the APs [...] 
is required if the target AP derives the required PMKs locally.

for FT-PSK networks, no configuration of inter-AP communication is
needed anymore when using ft_psk_generate_local=1

So what about moving
append bss_conf "ft_over_ds=$ft_over_ds" "$N"
inside
if [ "$ft_psk_generate_local" -eq "0" ]; then
block?

[devianceluka]

if [ "$ft_psk_generate_local" -eq "0" ]; then is only for keys, nothing else (iow. you answered yourself with that quote from link).

If its necessary, im actually using ft_over_ds=0. Read this: http://www.mikealbano.com/2014/06/80211r-80211k-fast-bss-transition.html?m=1

Im manually patching my APs to have default here 0, but as I said, this PR doesnt break anything it only correctly simplifies everything and still follows upstream default values.

[devianceluka]

IOW ft_over_ds has nothing to do with keys/key generation

[BigNerd95]

Ahh okay, sorry I misunderstood
So ft_over_ds (or over the air) happens in both PSK and EAP network. 👍

[BigNerd95]

What about generating a default mobility_domain based on the SSID?
The SSID is the same on all the APs of the network, so if we use an hash function and then extract only the first 4 bytes we can generate the same mobility_domain on all the APs.

Then optionally the user can change the mobility_domain.

Whit this change, a user using PSK must only set option ieee80211r '1' to get it working.

[devianceluka]

there is already set_default ft_psk_generate_local 1 in this commit.

as for mobility_domain generating, I totally agree with this, I wanted to do it myself but don't have an idea on how to do it. Can you throw me a bone for this one?

[BigNerd95]

Yes sorry for my oversight about ft_psk_generate_local

For mobility_domain what do you think about this?

set_default mobility_domain $(echo -n "$ssid" | md5sum | head -c 4)

[devianceluka]

I just dont understand why is it taking so long to accept this PR. I had more stuff like this to push here but now I just don't care anymore...

[BigNerd95]

I think they are focused on others tasks at the time.
I created a personal branch where I merge all my pending changes.
So I can use them before they accept my PRs.
Anyway if you have other stuff to push, then push it, maybe it will be accepted faster by another mantainer.

[bobafetthotmail]

I just dont understand why is it taking so long to accept this PR.

People reviewing and merging are not paid for this and they also have a job and a life. It's normal to wait for months before someone shows up.

As said by BigNerd95, you should plan accordingly and keep your changes in a branch you keep synced to mainline, so you can use them for your own builds while you wait.

[hauke]

Could you please rebase this on top of current ,master and also squash all patches into one.
One improvement was already done in this commit: 09f90b7

[devianceluka]

Done.

[stintel]

Nothing is perfect, I would use "improve 802.11r options" in subject. Next to that, the commit message now no longer contains any explanation. Please add that again.

[devianceluka]

Done.

[hauke]

Why don't you use this here:
[ -n "$r1_key_holder" ] && append bss_conf "r1_key_holder=$r1_key_holder" "$N"
It says "Defaults to BSSID."

[devianceluka]

line 417: set_default r1_key_holder "${macaddr//\:}"
here it defaults

line 421: append bss_conf "r1_key_holder=$r1_key_holder" "$N"
here it writes

if user wants to change r1_key_holder, he can, otherwise by doing only (changing default) ft_psk_generate_local **0** LEDE will auto-write BSSID if r1_key_holder left unadded/unchanged by user ---
because then 80211r needs a unique r1_key_holder per BSS, it wont auto generate it unique atm non-working state --- r1_key_holder needs to exist --- defaulting BSSID fixes auto non-working state only.

logic is different, coding style is the same as before.

line 411: if [ "$ft_psk_generate_local" -eq "0" ]; then
user wants to change keys (don't need to) and forgets to actually add/change them
line 417: set_default r1_key_holder "${macaddr//\:}"
this only sets BSSID if user did not add/change r1_key_holder
[ -n "$r1_key_holder" ] doesn't make sense because r1_key_holder needs to exist
line 421: append bss_conf "r1_key_holder=$r1_key_holder" "$N"
im appending r1_key_holder in line 421 where it was before.

previously it was
line 400: set_default r1_key_holder "00004f577274" <-- wrong default value, it needs to be unique for multi SSID and per AP (like BSSID is)
line 408: append bss_conf "r1_key_holder=$r1_key_holder" "$N" <-- write
someone deleted both lines in commit 09f90b7 where I referenced it and added:
line 407: [ -n "$r1_key_holder" ] && append bss_conf "r1_key_holder=$r1_key_holder" "$N"
which doesnt make any sense --- it only shows r1_key_holder is not needed if someone wants to change ft_psk_generate_local to 1 --- which it should be by default -- which it is also in my PR in line 401

[devianceluka]

I understand what you mean now, you mean upstream hostap defaults to BSSID? it can be removed, but maybe its better to be left like that, so noone readds something stupid later on?

[devianceluka]

is this better without set_default r1_key_holder "${macaddr//\:}"?

imo with set_default r1_key_holder "${macaddr//\:}" was failproof.

[blogic]

With the remerge in progress, all PRs on the lede-project organisation will be closed. Please help getting this merged or rebase/post it on the openwrt project page (https://github.com/openwrt/openwrt/pulls). All remaining PRs will be closed in 30 days.

[Mushoz]

@devianceluka Could you please re-open this pull request on the OpenWRT Github page? I think I speak for many that a simple 802.11r configuration would be a very welcome addition for OpenWRT.