2017-06-09 22:18 GMT+02:00 Ben Greear <greearb@xxxxxxxxxxxxxxx>: > On 06/09/2017 01:01 PM, Johannes Berg wrote: >> >> On Fri, 2017-06-09 at 06:46 -0700, Ben Greear wrote: >> >>>> However, the solution is far simpler! Once you have nl80211 PAE >>>> transport, you can easily even set the key before transmitting the >>>> packet and simply indicate that this particular packet should _not_ >>>> be encrypted regardless of key presence. >>> >>> >>> My ath10k firmware cannot deal with a case like this: >>> >>> pkt is enqueued before key is set >>> key is set >>> pkt is transmitted (incorrectly) >>> >>> This is because of how the tid's header-length variables are set up >>> and modified when the keys are set, and I don't see any good way to >>> fix this. >> >> >> That seems awful, and anyway will not work with the mentioned non-IEEE >> protocols that require not encrypting the rekeying frames even when >> keys have been set up. >> >> I don't know what to tell you here, I think it'd be best if you fix >> that. > > > The case that fails is basically any packet that is currently > enqueued in the firmware when the key is set is transmitted incorrectly or > not at all. > And, maybe this is only for DATA tids. > > Otherwise, the no-encrypt logic should work fine. So, it is just the race > with the EAPOL 4/4 and set-key that causes issues as far as I can tell. > > It looks like the EAPOL 4/4 and set-key race is fixable without too much > effort, > so I think I will focus on that for now instead of further hacking special > case logic into the firmware. > Ben, as I remember correctly there is a flag, WMI_PEER_NEED_PTK_4_WAY This fix race (you describe) in the firmware/hw. For 636 firmware, where we tested STA mode really hard (5 years ago), that works perfect. So, I am not sure why this flag don't work correctly with your firmware. Regarding EAPOL frames I am not sure you will have real/correct ACK. I remember in case of mgmt frames FW always return ACK - but probably you will check that. I also fight with some EAPOL frames, but forgot why ... Seems my brain removed this information for some reason :-) BR Janusz >>> Stock ath10k firmware goes to great lengths to parse EAPOL frames and >>> try to work around it in that manner, but that breaks .11r (or used >>> to, I haven't tried stock firmware lately) and adds more complexity >>> to the code. >> >> >> It just has to be a single flag saying "don't encrypt this frame" - >> nothing super complicated about that? >> >> In ath10k it looks like HTT_DATA_TX_DESC_FLAGS0_NO_ENCRYPT gets set for >> this, seems easy enough? >> >>> From a patch someone sent to hostapd list last night, it seems we >>> could get the tx-status for the EAPOL 4/4, and in that case, we >>> *know* the pkt has been transmitted, so we can then set the key >>> safely it would seem? >> >> >> I think so, and I don't remember why we dismissed this solution. Could >> be that we just decided solving the bridging issue at the same time, >> while not introducing more latency, was better. >> >> Also, the other way can possibly solve some PTK rekeying issues, so >> overall the solution to go all the way seems better. > > > I guess I don't fully understand the PAE thing, but if you all like it, > then sounds good to me. > > For kernels not supporting this new feature, it seems that having supplicant > wait for tx-status would be a good interim fix? > > Thanks, > Ben > > -- > Ben Greear <greearb@xxxxxxxxxxxxxxx> > Candela Technologies Inc http://www.candelatech.com > > > _______________________________________________ > Hostap mailing list > Hostap@xxxxxxxxxxxxxxxxxxx > http://lists.infradead.org/mailman/listinfo/hostap -- Janusz Dziedzic
