net80211: reject mixed plaintext/encrypted fragments

[bz]

Summary

ieee80211_defrag() accepts fragmented 802.11 frames in a protected Wi-Fi
network even when some of the fragments are not encrypted.
Track whether the fragments are encrypted or not and only accept
successive ones if they match the state of the first fragment.

This relates to section 6.3 in the 2021 Usenix "FragAttacks" (Fragment
and Forge: Breaking Wi-Fi Through Frame Aggregation and Fragmentation)
paper.

Submitted by: Mathy Vanhoef (Mathy.Vanhoef kuleuven.be)
Security: CVE-2020-26147
PR: 256118

[bz]

See the PR for the original description/patch.

[bz]

If no one has any (further) comments I'll commit these tomorrow morning (UTC).

[emaste]

From the PR,

I'm not sure if I'm using the best way to track whether a fragment was encrypted, but it gets the job done with a low number of code changes.

I'm not really a fan of tracking the enc state this way, but am not familiar enough with the rest of net80211 to have a better idea and it indeed is not too intrusive. I might add an XXX comment where IEEE80211_FC1_PROTECTED is set that mentions this.

But overall no objection to this change.

sys/net80211/ieee80211_input.c

174

bool has_decrypted perhaps?

[bz]

In D30663#726964, @emaste wrote:

From the PR,

I'm not sure if I'm using the best way to track whether a fragment was encrypted, but it gets the job done with a low number of code changes.

I'm not really a fan of tracking the enc state this way, but am not familiar enough with the rest of net80211 to have a better idea and it indeed is not too intrusive. I might add an XXX comment where IEEE80211_FC1_PROTECTED is set that mentions this.

I am not either but adding an mtag for this and all the infrastructure to handle that seems to be overkill.
I was pondering (ab)using other mbuf fields we could at that layer (e.g., PH_loc) but then again that is not easily portable between mbuf implementations I assume.

I am open to alternate suggestions which are cleaner or if one of the two above should be used better?

sys/net80211/ieee80211_input.c

174

I think that's something for an aftermath; I'd rather not start making it all bools (as you'd also want to do the callers)...

266

temporarily

[bz]

Add an XXX to use a non packet-altering storage in the future?

[bz]

Fix spelling and punctuation.

[emaste]

This is fine with me - better to get in as is than to spend a long time trying to find an ideal implementation.