Hi, Attached a new patch trying to apply to your comments below Jouni. Best Regards, Peter > -----Original Message----- > From: Hostap <hostap-bounces@xxxxxxxxxxxxxxxxxxx> On Behalf Of Jouni > Malinen > Sent: den 11 juni 2018 23:52 > To: Peter Dersén <Peter.Dersen@xxxxxxxx> > Cc: hostap@xxxxxxxxxxxxxxxxxxx > Subject: Re: EAPOL multi-auth patch > > 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
Attachment:
0001-Avoid-multi-authentication-on-single-port-to-fail.patch
Description: 0001-Avoid-multi-authentication-on-single-port-to-fail.patch
_______________________________________________ Hostap mailing list Hostap@xxxxxxxxxxxxxxxxxxx http://lists.infradead.org/mailman/listinfo/hostap
