EAPOL multi-auth patch

Jouni Malinen 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).


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



More information about the Hostap mailing list