On Fri, Jun 08, 2018 at 06:44:38AM +0000, Peter Dersén wrote: > The problem occur when several clients sending authentication requests that are received by clients that > already have started their authentication sequence. The received packets from the other clients cause the > eapol state machine to enter the wrong state making the authentication fail. I guess this really should say "responses" instead of "requests" to match the EAP terminology, but yes, this does sound like a possible issue in environments that do not comply with the expectations of EAPOL frames not being forwarded (e.g., having multiple devices behind a hub that is connected to a single 802.1X port). > Please check the attachment for further details. > > We have used this patch for several years in many installations and have not seen any negative side effects. It breaks LEAP which uses EAP-Response in a very inconvenient manner, but I guess most people would not care about LEAP today. > Please give us your opinion and advice how we should proceed to push a solution into > wpa supplicant official codebase for future releases. On the process front, I'd need to receive a patch with the Signed-off-by: line in the commit message as described in the toplevel CONTRIBUTIONS file in the repository. As far as the source code change is concerned, this is a somewhat inconvenient layer violation since EAPOL state machine should not really know about the contents of the EAPOL EAP-Packet and trying to behave differently based on the EAP Code field can result in issues (like that LEAP example). I think this should try to target the specific case of moving from AUTHENTICATED state to RESTART rather than ignore all EAP-Response messages. Furthermore, there better be a clear comment in the source code to explain why such an apparent mismatch with the standard definition for eapolEap is needed. Or whatever that might be in SM_STEP(SUPP_PAE); maybe "if (sm->eapolEap && sm->eapolEapCode != EAP_CODE_RESPONSE && sm->portValid)" (and filling in that new eapolEapCode whenever setting eapolEap = TRUE). PS. I don't think plen is guaranteed to be >= 4 (EAP header size) and as such, this change could have a mostly theoretical buffer read overflow issue (it needs to check plen >= sizeof(*eaphdr) before dereferencing eaphdr). -- Jouni Malinen PGP id EFC895FA _______________________________________________ Hostap mailing list Hostap@xxxxxxxxxxxxxxxxxxx http://lists.infradead.org/mailman/listinfo/hostap
