Hi Brendan, > >> + if (nla_put(msg, NL80211_ATTR_FRAME, len, buf)) > >> + goto fail; > > One of the reasons to have this path was to enable/disable encryption on > > this particular frame, IIRC. Shouldn't > > NL80211_ATTR_CONTROL_PORT_NO_ENCRYPT get set here at least in some > > cases? > > Well actually my goal is strictly to _disable_ encryption for this frame. Right, I think you mentioned that. > In our > driver we currently have a double-awkward workaround for the key 802.11 key > installation flaw: we explicitly disable encyption for the initial handshake, to > avoid the MAC inadvertently encrypting handshake packets before the peer has > installed the PTK... but for rekeying handshakes we then have to _stop_ > explicitly disabling encryption because if we send unencapsulated packets during > an RSNA they will have no packet number. Handling that properly would require > more workarounds in the GCMP replay detection code on the RX side. But now that > we are encrypting handshake packets we are back to having the race condition: > now we can end up inadvertently encrypting frames with a key that the peer has > not yet installed. It's not clear to me what this change gives you though. At least in theory, nothing stops you from detecting that it's an EAPOL packet, and then stopping the queues and all in a similar fashion to what you'd do when you get the EAPOL packet through nl80211. One of the things we did want to have here was the ability to *control* whether the frames were encrypted, there are some protocols (WAPI?) that require encryption, while the normal case (WPA/RSN) require _no_ encryption. Hence my comment regarding being able to control it in the supplicant, although I guess since it doesn't actually support any paths that want encryption it doesn't matter for now (and for upstream). > The goal of this patch is ultimately to let the driver know that when it is > asked to install a new PTK to the MAC, the handshake frames have already entered > its TX ring. So if we flush the rings before installing a key, we avoid the race > condition. So if I understand correctly: you want the EAPOL over nl80211 so you don't have a race between sending the EAPOL and installing the key? I guess that makes more sense now. > On the other hand, this now makes me worry that there could be other drivers out > there that implement .tx_control_port, but do not take advantage of it to avoid > the key installation race... in that case this patch could break handshakes for > those drivers. On the other-other hand I don't know why you'd implement > .tx_control_port except for this very reason. I'm not sure how it'd break anything, it'd just behave as before? But this is new, nobody really implements it yet. So I don't think we have to worry about that even if I'm not sure I understand your concern. johannes _______________________________________________ Hostap mailing list Hostap@xxxxxxxxxxxxxxxxxxx http://lists.infradead.org/mailman/listinfo/hostap
