Page MenuHomeFreeBSD

net80211: migrate the group/unicast key check into inline functions
ClosedPublic

Authored by adrian on Jun 6 2024, 5:32 PM.
Referenced Files
Unknown Object (File)
Mon, Nov 4, 4:15 AM
Unknown Object (File)
Sun, Oct 27, 12:55 PM
Unknown Object (File)
Sat, Oct 19, 5:18 PM
Unknown Object (File)
Oct 3 2024, 5:25 AM
Unknown Object (File)
Oct 3 2024, 5:24 AM
Unknown Object (File)
Oct 3 2024, 5:23 AM
Unknown Object (File)
Oct 3 2024, 5:22 AM
Unknown Object (File)
Oct 3 2024, 5:22 AM

Details

Summary

The way that net80211 and drivers are checking for the /type/ of key
is to check if it's in the vap WEP key array and if so, it's a group
key. If not, it's a unicast key.

That's not only kind of terrible, but it's also going to be
problematic with future 802.11 support (for multiple unicast keys
and IGTK keys for management frame protection.)

So as part of this, remove the places where this is done and
instead use a pair inline functions - ieee80211_is_key_global() and
ieee80211_is_key_unicast(). They currenly still use the same logic
but the drivers and net80211 stack isn't doing it itself.

There are still open questions about why keys are not being
correctly tagged as GROUP, GTK, PTK, etc. That will be investigated
and addressed in follow-up work as a pre-cursor to MFP, IGTK, etc.
as mentioned above.

Testing:

  • TBD
Test Plan

Tested so far on STA mode with:

  • iwn 6xxx (software crypto)
  • rtwn (hardware crypto)

more to come, just to make sure I didn't mess up anything for the chipsets that this diff touches.

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Skipped
Unit
Tests Skipped
Build Status
Buildable 58087
Build 54975: arc lint + arc unit

Event Timeline

adrian requested review of this revision.Jun 6 2024, 5:32 PM
adrian added a project: wireless.

Note - I'm landing this as a standalone diff for review, but it hasn't yet had any thorough testing. Stay tuned.

bz requested changes to this revision.Jun 17 2024, 9:53 AM
bz added a subscriber: bz.
bz added inline comments.
sys/dev/ath/if_ath_keycache.c
437

remove extra space

sys/dev/mwl/if_mwl.c
1522

remove extra space

sys/net80211/ieee80211_var.h
985

Can we not make them [__]inline please?
Also given they are bool functions "is X" can we make them use true and false and return a bool please?

This revision now requires changes to proceed.Jun 17 2024, 9:53 AM
sys/net80211/ieee80211_var.h
985

Would you like them to be specifically not inline?

sys/net80211/ieee80211_var.h
985

yes, just not inline; normal functions would be preferred. Especially once you'll change them it'll help if the KPI will stay as not every driver will change; also it'll simplify adding any kind of tracing later.

989

We should really start finding a way that WEP must be "forced on" in the future... and mark all the places that deal with it to be able to compile them out ;-)

feedback from bz:

  • don't make it inline
  • make it return bool
This revision was not accepted when it landed; it landed in state Needs Review.Jul 15 2024, 6:46 PM
This revision was automatically updated to reflect the committed changes.