On Tue, Aug 27, 2019 at 10:23 PM Alexander Wetzel <alexander@xxxxxxxxxxxxxx> wrote: > > On 8/27/19 2:49 PM, Brendan Jackman wrote: > > > > On 27/08/2019 19:11, Johannes Berg wrote: > > > > >> So, what if we just don't encrypt the rekeying handshake frames either > > >> (i.e. keep th NOCRYPT workaround going)? The problem is that the peer should > > >> reject them according to the spec. So let's say we just have the peer allow > > >> unencrypted frames during an RSNA as long as they are EAPoL. This is easier said > > >> than done (at least in my case - perhaps I am a special case here?) because the > > >> MAC layer whose job is to discard unencrypted frames has no business peering at > > >> the ethertype or whatever. Similarly for the part where we discard GCMP replays > > >> (unencrypted frames don't have a packet number at all!). > > > > > > In mac80211, we specifically allow unencrypted EAPOL frames to be > > > received, because also some WPA1 APs allow them, and WAPI requires this > > > (IIRC), and for various other reasons. We have to look at the ethertype > > > anyway at some point, so not sure I agree with that part of your > > > statement. > > > > I've never seen unencrypted eapol frames with mac80211 or any other > card. And I don't think disabling encryption for eapol during rekey can > work. It would probably just cause even more issues when (trying) to > rekey a connection. > I see why accepting unencrypted eapol frames at the initial connect > makes sense. But for rekeying - regardless of the technical challenges - > we would need some new RSN feature flag and only do that when both > support it. > > > When I say "has no business peering" I'm not just talking about a philosophical > > distaste for it - I actually mean in our current architecure the components that > > discard frames for RSNA reasons don't have physical access (or it would be > > costly to performance) to the ethernet header. So we'd basically need to change > > those components so that where currently they cause the frame to be discarded, > > they instead cause it to be passed on up and just set a flag somewhere to say > > "discard that frame if it doesn't turn out to be EAPoL". Or, more accurately > > "discard all the MSDUs in that MPDU that don't turn out to be EAPoL". > > > > I must admit: perhaps we'll need to do this in the end anyway for interop > > reasons. And perhaps this is a totally unique issue for our architecture which > > doesn't really justify changing generic code. But anyway perhaps you can see why > > I've been hesitant to do it. > > > > > In general though, yes, I think you're right. We just allow them > > > unencrypted for interoperability, but I'm not sure we actually transmit > > > them unencrypted. > > > > > >> So in our driver we use the NOCRYPT workaround for the initial handshake, but > > >> then _don't_ use it for the rekeying handshake (i.e. we _do_ encrypt EAPoL > > >> frames during rekeying). But that means we are vulnerable to the race condition > > >> and indeed we see rekeying fail if there is heavy traffic on the link. > > >> > > All this nowwcrypto talk may miss the point: This is a workaround > working at the initial connect but that was it. (Looks like 90% of the > cards/drivers are not looking behind the initial connect and mess up the > rekey. So far I found only two devices which can rekey correctly...) > Assuming the driver is supporting Extended Key ID but we can't use it we > could achieve the same by installing the key prior to sending out eapol > #4 RX_ONLY and wait till the frame is acknowledged to allow the key to > be used for TX. But it has the same issue as the no-encrypt idea: When > we rekey it's not working. > > The PTK0 patch in mac80211 attempts to solve the issue in the following way: > > Normally there are next no buffers in the tx path. When you hand over a > frame to the kernel it's - according what I remember when investigating > the issue - an atomic operation till the skb reaches mac80211 or > whatever other driver we use. And the few "buffers" we have are fully > driver controlled and we basically can just make sure those are flushed > when we replace the key. The only potential issue I see are traffic > shapers but then I think I eapol frames bypass them already. the issue of race b/w set_key and send_m4 probably caused the delays in socket_queues/QDISC for M4, can you please elaborate on how EAPoL bypasses QDISC? I don't see PACKET_QDISC_BYPASS being set for l2_packet_send. > Of course using the control port for the frames would be nicer. It > should not be needed, through. > > So mac80211 just makes sure that once the key replacement code is > running any new frames handed over to it are dropped. The drivers then > have to make sure they flush all queues, making sure the eapol #4 frame > is also send out prior when it's instructed to delete an unicast key. > > The key deletion therefore serves as sync. When mac80211 set_key call > returns we know all frames queued to us prior to the call from user > space to replace the key have been send. > > The price of that are some dropped frames (which also could be queued) > which would likely have been caught by the key mismatch between the STAs > anyhow. > > That same procedure should also work for you, or I must have a serious > flaw in my understanding. And has the additional benefit to also work > correctly when mac80211 generates the PN (IV) and not the card. Down > sides are the dropped frames and the needed queue flush. > > > >> So the idea is that by moving part 1. into the control plane to some degree > > >> (i.e. into nl80211) we can fully complete establishment of key 0', using > > >> handshake frames encrypted with key 0, before we install key 0' (by flushing the > > >> TX ring in part 2). So now the NOCRYPT workaround is not necessary to avoid lost > > >> handshake packets, neither for the initial nor for rekeying handshakes. > > > > > > Right, OK. I forgot (again) you were worried about the rekeying mostly. > > > > > > Nevertheless, I think this means that we do need to have the NO_ENCRYPT > > > flag set to keep the *initial* handshake working, no? Or actually fix > > > the race conditions you pointed out above for it. While for the > > > rekeying, well, it basically cannot work and you just move it to the > > > control plane to have more control over it, which also makes sense. > > > > .tx_control_port + a TX ring flush in .add_key should fix both the initial and > > rekeying handshake without any need for NO_ENCRYPT. The handshake frames will > > always go out before the key (re)installation takes place, which means: > > > > - The handshake frames for the initial key setup will all go out unencrypted > > (because the MAC has no keys). > > > > - The handshake frames for rekeying will all go out encrypted with the old key, > > before the new key gets installed. (If there is data flowing, some will > > probably be lost at this point, but the EAPoL frames will be fine which is the > > most important thing). > > > I should also say that with Alexander Wetzels' work on Extended Key > ID support > > going on as it is, my patch here would only end up really mattering for users of > > kernels between v4.19 and v5.3 (I think). So yeah, if you do not think my > > justifications are strong enough, I would understand. > > I think that's way too optimistic. Extended Key ID will not play any > meaningful role for probably decades, maybe even never. > Extended Key ID should have been the only way from the beginning, it's > now quite late to force everyone to switch. > At the moment this is only something which can work in a careful > controlled environment, regardless which kernel you are using. This may > be something to make mandatory in WPA4 or so. But since it will force > many vendors to redesign their chips I don't hold my breath... > > > > > Cheers, > > Brendan > > Alexander > > _______________________________________________ > Hostap mailing list > Hostap@xxxxxxxxxxxxxxxxxxx > http://lists.infradead.org/mailman/listinfo/hostap -- Thanks, Regards, Chaitanya T K. _______________________________________________ Hostap mailing list Hostap@xxxxxxxxxxxxxxxxxxx http://lists.infradead.org/mailman/listinfo/hostap
