Closed Bug 790054 Opened 9 years ago Closed 8 years ago

Disable WPA Enterprise support

Categories

(Firefox OS Graveyard :: General, defect, P2)

x86_64
Linux
defect

Tracking

(blocking-basecamp:+, firefox19 fixed, firefox20 fixed, b2g18 fixed)

RESOLVED FIXED
B2G C3 (12dec-1jan)
blocking-basecamp +
Tracking Status
firefox19 --- fixed
firefox20 --- fixed
b2g18 --- fixed

People

(Reporter: overholt, Assigned: mrbkap)

References

Details

(Whiteboard: [LOE:S])

Attachments

(2 files, 1 obsolete file)

Brian states in bug #789217 comment #2 that until bugs #775499 and #789217 are fixed, we should disable WPA Enterprise support.  Is this better filed as a Gaia issue?
Blocks: 790056
blocking-basecamp: ? → +
Blake, is this something you can do?
Assignee: nobody → mrbkap
Before disabling this, can we check there is no other option?

Part of the development team is based in premises where this is the only WiFi security mechanism allowed (see https://bugzilla.mozilla.org/show_bug.cgi?id=775499#c17)
Whiteboard: [LOE:S]
What needs to be done here?  Who is the best person to do it?
Flags: needinfo?
(In reply to Andrew Overholt [:overholt] from comment #3)
> What needs to be done here?  Who is the best person to do it?

I'd prefer to see this fixed in Gaia. Basically we could disable WPA-EAP support in the backend entirely (which would also be easy) or remove the UI for it in the frontend. This would be a stopgap measure until we have the time and resources to fix bug 789217 which will involve firing events for unrecognized certs.

For developers who only have access to WPA-EAP networks, they could flash a wpa_supplicant.conf to their phone.
Flags: needinfo?
As Daniel said, we would prefer to fix bug 789217 instead of disabling WPA Enterprise. If it's a problem of resources, we can take over bug 789217 on TEF if you wish.
Milestoning for C2 (deadline of 12/10), as this meets the criteria of "known P2 bugs found before or during C1".
Target Milestone: --- → B2G C2 (20nov-10dec)
No update for nearly a month. What needs to happen to move this forward?
I posted a WIP patch for bug 775499 earlier today. I don't think we should disable EAP at all, we can land the patch for 77499 instead in time which will leave us on par with current Android systems in regard to EAP support and certificate verification. 

So in short, I think this bug should be resolved as invalid, and bug 775499 should be marked as blocking instead.
I don't know enough to make that call. Blake?
Thanks Alex. Blake, when can you have a fix for this available?
Deferring, not really necessary for C2 but it would be good to get this off our list.
Target Milestone: B2G C2 (20nov-10dec) → B2G C3 (12dec-1jan)
No update from mrbkap since October. What needs to happen here?
Mrbkap says can fix this week.
Attached patch patch, v1Splinter Review
This patch no longer shows WPA-EAP networks in the available network list unless they were previously configured. Even after this patch, people can manually patch their wpa_supplicant.conf to connect to WPA-EAP networks.
Attachment #694558 - Flags: review?(vchang)
Just spoke with Fabrice and here's the following suggestion to move forward on this bug:

*Let's land this bug once the patch has been r+'ed
*Can we add a pref to custom-prefs.js to allow developers/dogfooders to toggle this on if necessary?  (We have specific partner needs to be able to use WPA enterprise networks for testing purposes.)

Blake, could you confirm if this is feasible?  Thanks.
One additional note:

*Can we hold off landing this patch until we have a path forward on the 2nd bullet regarding the pref?  Thanks.
(In reply to Chris Lee [:clee] from comment #16)
> Blake, could you confirm if this is feasible?  Thanks.

Sure, that's easy.
Note: this is untested (I'm at my parents' house and don't have an EAP network to test on. Fabrice, would you mind testing? Note that we read the pref in onsupplicantconnection, which means that it'll be necessary to turn wifi off and on for the pref to take effect. It's easy to fix that restriction, but I don't know if it's worth it.


Feedback: :fabrice
Attachment #694692 - Flags: review?(vchang)
Attachment #694692 - Flags: feedback?(fabrice)
Comment on attachment 694692 [details] [diff] [review]
Allow WPA-EAP networks based on a gecko pref.

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

look good with a try { } catch(e){} to not blow up.

::: dom/wifi/WifiWorker.js
@@ +1784,5 @@
>        self._enableAllNetworks();
>        WifiManager.saveConfig(function() {})
>      });
>  
> +    self._allowWpaEap = Services.prefs.getBoolPref("b2g.wifi.allow_unsafe_wpa_eap");

I think that will throw if the pref is not defined at all.
Attachment #694692 - Flags: feedback?(fabrice) → feedback+
Comment on attachment 694558 [details] [diff] [review]
patch, v1

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

Looks good.
Attachment #694558 - Flags: review?(vchang) → review+
(In reply to Vincent Chang[:vchang] from comment #21)
> Comment on attachment 694558 [details] [diff] [review]
> patch, v1
> 
> Review of attachment 694558 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Looks good.

I have verified this patch, it looks good.
Comment on attachment 694692 [details] [diff] [review]
Allow WPA-EAP networks based on a gecko pref.

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

It will throw if the pref is not defined.
Do we need to hook it to settings ?
Attachment #694692 - Flags: review?(vchang) → review-
Attached patch Pref patch, v2Splinter Review
This adds the needed try/catch. I don't want to make this a setting as I don't think this is a gaia-level decision. If the user wants to open him/herself up to bug 775499 that's something I'd prefer that they do themselves in a very out-of-the-ordinary way. We should also try to get this patch in sooner rather than later and if people want, adding a setting as well as (or in addition to) the pref can be done in a followup.
Attachment #694692 - Attachment is obsolete: true
Attachment #694961 - Flags: review?(vchang)
Attachment #694961 - Flags: review?(vchang) → review+
You need to log in before you can comment on or make changes to this bug.