EAPOL multi-auth patch
j at w1.fi
Mon Jun 11 14:51:46 PDT 2018
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).
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
Jouni Malinen PGP id EFC895FA
More information about the Hostap