Potential bug in src/eap_peer/eap_tls_common.c causes wpa_supplicant to stop trying

[Jouni Malinen]

On Fri, Feb 06, 2015 at 12:07:54PM -0800, Mike Gerow wrote:
> tl;dr: As of commit 1cb3ad73623c20498a9163730c43563a869cf4a0 is it a
> bug that there isn't a "return -1" after
> src/eap_peer/eap_tls_common.c:206 ?

I'm not actually sure.. It looks like the original implementation of
OpenSSL engine support used a single code path that addressed both of
the current TLS_SET_PARAMS_ENGINE_PRV_INIT_FAILED and
TLS_SET_PARAMS_ENGINE_PRV_VERIFY_FAILED cases by clearing config->pin,
requesting a new PIN from user, and setting sm->ignore = TRUE; and
finally "goto done" which got later replaced with this "return -1" which
is now present only for TLS_SET_PARAMS_ENGINE_PRV_VERIFY_FAILED.

The original implementation was split into those two separate error
values ten years ago:
http://w1.fi/cgit/hostap-history/commit/?id=efb47fa42fdff8679c2cc8627446c1e0ca91ec92

The current implementation matches that old interface redesign to cover
other TLS libraries. That said, I'm not sure that that old commit was
correct here. I think it was actually done on purpose since there is a
clear difference in whether the expectation is to ask the user for a new
PIN vs. something failing to initialize a component that could
potentially not have been needed (but still clearing the PIN just in
case it was already tried to avoid retrying incorrect PIN multiple
times).

> We're using wpa_supplicant with eap-tls and having it use a PKCS#11
> module for signing operations. Sometimes we get log entries like this:
> 
> Jan 26 16:07:39 example.com wpa_supplicant[3291]: em1:
> CTRL-EVENT-EAP-STARTED EAP authentication started
> Jan 26 16:07:39 example.com wpa_supplicant[3291]: em1:
> CTRL-EVENT-EAP-PROPOSED-METHOD vendor=0 method=13
> Jan 26 16:07:39 example.com wpa_supplicant[3291]: ENGINE: cannot load
> private key with id 'id_04' [error:26096080:engine
> routines:ENGINE_load_private_key:failed loading private key]
> Jan 26 16:07:39 example.com wpa_supplicant[3291]: em1:
> CTRL-EVENT-EAP-METHOD EAP vendor 0 method 13 (TLS) selected
> Jan 26 16:07:41 example.com wpa_supplicant[3291]: em1:
> CTRL-EVENT-EAP-FAILURE EAP authentication failed
> 
> Which is all well and good, it sounds like our our PKCS#11 back-end
> (opencryptoki in this case) is complaining, but the problem is that
> wpa_supplicant refuses to re-try starting the PKCS#11 module and
> authing ever again until the machine is rebooted. Even disconnecting
> and reconnecting in NetworkManager didn't cause it to re-auth.

This I find somewhat surprising.. Do you know what exactly prevents
retrying? Certainly this would not require machine reboot, but I would
have expected that this would work even without restarting the
wpa_supplicant process. In other words, the next time EAP-TLS is tried
again, this same code path should be executed again.

> Digging through the source src/eap_peer/eap_tls_common.c:206 looked
> suspicious, as if it was failing to properly report the failure in
> this case. Adding a "return -1" in this case seemed to fix the issue
> in that it would at least retry authing when disconnecting and
> reconnecting in NetworkManager.
> 
> So it looks like this function should be reporting the error to the
> caller... but I don't know enough about the wpa_supplicant code to
> know for sure if this was an accident or purposeful.

I'm not very familiar with the OpenSSL engine extensions that have
happened over the years. Originally, this was used only for the case of
the private key being used from a smart card  and that's why all the
clearing of config->pin is there. For the newer uses of PKCS#11 backend
without smartcards, that is likely of much less concern. That said, if
the credential is not ready for whatever reason, there is not really
much point in even trying to associate with the AP. I have a to-do item
on trying to figure out how to robustly figure that out in
wpas_network_disabled() for networks that use EAP.
 
-- 
Jouni Malinen                                            PGP id EFC895FA