1023115 - [Wifi] WPA-EAP keeps try to connect on incorrect auth info.

Status: RESOLVED FIXED

(Firefox OS Graveyard :: Wifi, defect)

Description

[Chuck Lee [:chucklee, Triggered by NI, Review, and Feedback requests]]

STR:
1. Connect to any AP successfully.
2. Connect to another WPA-EAP AP with wrong user name, password, or certificate.

Expected Result:
Stop connect to WPA-EAP AP and connect to previous AP.

Actual Result:
Keeps try to connect to WPA-EAP AP.

Comment 1

[Chuck Lee [:chucklee, Triggered by NI, Review, and Feedback requests]]

Attachment: 0001. Add retry count for EAP connection error.

Add retry count to WPA-EAP connection errors, and add check rule to prevent disable wrong network while retry.

Comment 2

[Vincent Chang[:vchang][changyihsin]]

Comment on attachment 8437504 [details] [diff] [review]
0001. Add retry count for EAP connection error.

Review of attachment 8437504 [details] [diff] [review]:
-----------------------------------------------------------------

::: dom/wifi/WifiWorker.js
@@ +667,5 @@
>        for (let net in networks) {
>          let network = networks[net];
>          // Trying to get netId from
> +        // 1. network matching SSID if SSID is provided.
> +        // 2. current network if no SSID is provided, it's not garanteed that

Nit: garanteed/guaranteed

@@ +725,5 @@
>            event.indexOf("pre-shared key may be incorrect") != -1) {
>          notify("passwordmaybeincorrect");
> +        if (manager.authenticationFailuresCount > MAX_RETRIES_ON_AUTHENTICATION_FAILURE) {
> +          manager.authenticationFailuresCount = 0;
> +          notify("disconnected", {connectionInfo: manager.connectionInfo});

Do we need to fire disconnected event here? We should wait for DISCONNECTED event and fire disconnected event if failure count is exceeded.

Comment 3

[Chuck Lee [:chucklee, Triggered by NI, Review, and Feedback requests]]

(In reply to Vincent Chang[:vchang] from comment #2)
> Comment on attachment 8437504 [details] [diff] [review]
> 0001. Add retry count for EAP connection error.
> 
> Review of attachment 8437504 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: dom/wifi/WifiWorker.js
> @@ +667,5 @@
> >        for (let net in networks) {
> >          let network = networks[net];
> >          // Trying to get netId from
> > +        // 1. network matching SSID if SSID is provided.
> > +        // 2. current network if no SSID is provided, it's not garanteed that
> 
> Nit: garanteed/guaranteed
> 
> @@ +725,5 @@
> >            event.indexOf("pre-shared key may be incorrect") != -1) {
> >          notify("passwordmaybeincorrect");
> > +        if (manager.authenticationFailuresCount > MAX_RETRIES_ON_AUTHENTICATION_FAILURE) {
> > +          manager.authenticationFailuresCount = 0;
> > +          notify("disconnected", {connectionInfo: manager.connectionInfo});
> 
> Do we need to fire disconnected event here? We should wait for DISCONNECTED
> event and fire disconnected event if failure count is exceeded.

I observes that in flame, wpa_supplicant will disable connection failed network automatically for a short period of time then retry again, as CTRL-EVENT-SSID-TEMP-DISABLED and CTRL-EVENT-SSID-REENABLED in following log:

> <3>Trying to associate with SSID 'Test_PSK_AES'
> <3>CTRL-EVENT-STATE-CHANGE id=-1 state=5 BSSID=00:00:00:00:00:00 SSID=
> <3>CTRL-EVENT-STATE-CHANGE id=0 state=6 BSSID=00:00:00:00:00:00 SSID=Test_PSK_AES
> <3>Associated with 4c:e6:76:a3:07:ea
> <3>CTRL-EVENT-STATE-CHANGE id=0 state=7 BSSID=4c:e6:76:a3:07:ea SSID=Test_PSK_AES
> <3>CTRL-EVENT-DISCONNECTED bssid=4c:e6:76:a3:07:ea reason=15
> <3>WPA: 4-Way Handshake failed - pre-shared key may be incorrect
> <3>CTRL-EVENT-SSID-TEMP-DISABLED id=0 ssid="Test_PSK_AES" auth_failures=3 duration=20
> <3>CTRL-EVENT-STATE-CHANGE id=0 state=0 BSSID=4c:e6:76:a3:07:ea SSID=Test_PSK_AES
> <3>CTRL-EVENT-STATE-CHANGE id=-1 state=3 BSSID=00:00:00:00:00:00 SSID=
> <3>CTRL-EVENT-SCAN-RESULTS 
> <3>WPS-AP-AVAILABLE 
> <3>CTRL-EVENT-SCAN-RESULTS 
> <3>WPS-AP-AVAILABLE 
> <3>CTRL-EVENT-SCAN-RESULTS 
> <3>WPS-AP-AVAILABLE 
> <3>CTRL-EVENT-SCAN-RESULTS 
> <3>WPS-AP-AVAILABLE 
> <3>CTRL-EVENT-SCAN-RESULTS 
> <3>WPS-AP-AVAILABLE 
> <3>CTRL-EVENT-SSID-REENABLED id=0 ssid="Test_PSK_AES"
> <3>Trying to associate with SSID 'Test_PSK_AES'
> <3>CTRL-EVENT-STATE-CHANGE id=-1 state=5 BSSID=00:00:00:00:00:00 SSID=
> <3>CTRL-EVENT-STATE-CHANGE id=0 state=6 BSSID=00:00:00:00:00:00 SSID=Test_PSK_AES
> <3>Associated with 4c:e6:76:a3:07:ea
> <3>CTRL-EVENT-STATE-CHANGE id=0 state=7 BSSID=4c:e6:76:a3:07:ea SSID=Test_PSK_AES
> <3>CTRL-EVENT-DISCONNECTED bssid=4c:e6:76:a3:07:ea reason=15
> <3>WPA: 4-Way Handshake failed - pre-shared key may be incorrect
> <3>CTRL-EVENT-SSID-TEMP-DISABLED id=0 ssid="Test_PSK_AES" auth_failures=4 duration=20

These two events seems to be new mechanism provided in wpa_supplicant, and it breaks current PSK connection failure detection because we receive CTRL-EVENT-DISCONNECTED before retry count reaches threshold. Then Gaia can't delete this network because it can't receive corresponding event.

But I didn't test this patch on other devices, I'll remove this part for safe.

Comment 4

[Chuck Lee [:chucklee, Triggered by NI, Review, and Feedback requests]]

Attachment: Add retry count for EAP connection error. V2

Address comment 2.

Comment 5

[Vincent Chang[:vchang][changyihsin]]

Comment on attachment 8443212 [details] [diff] [review]
Add retry count for EAP connection error. V2

Review of attachment 8443212 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good.

Comment 6

[Chuck Lee [:chucklee, Triggered by NI, Review, and Feedback requests]]

Try looks good: https://tbpl.mozilla.org/?tree=Try&rev=586c6696eb16

Comment 7

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/integration/b2g-inbound/rev/591e76c75a8e

Comment 8

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/591e76c75a8e