Closed
Bug 745468
Opened 13 years ago
Closed 10 years ago
B2G Wifi: Support EAP-PEAP and EAP-TTLS without private key
Categories
(Firefox OS Graveyard :: Wifi, defect)
Firefox OS Graveyard
Wifi
Tracking
(feature-b2g:2.0, tracking-b2g:backlog)
People
(Reporter: gerard-majax, Assigned: chucklee)
References
Details
Attachments
(3 files, 20 obsolete files)
(deleted),
image/png
|
Details | |
(deleted),
patch
|
Details | Diff | Splinter Review | |
(deleted),
patch
|
Details | Diff | Splinter Review |
User Agent: Mozilla/5.0 (X11; Linux x86_64; rv:13.0) Gecko/20120411 Firefox/13.0a2
Build ID: 20120411042011
Steps to reproduce:
There is currently, at least on a GUI level, no support to specify credentials for a WPA-EAP-TLS WiFi Network. As far as I can tell, there is no support (configuration) for EAP: world-wide "eduroam" network, provided in many universities, is thus not available for example. I did not checked wpa_supplicant's build configuration, but enabling those features at its level should not be a problem, only having a GUI that allows to specify the correct configuration and maybe wpa_supplicant's access to the credentials.
Reporter | ||
Comment 1•13 years ago
|
||
I've been able to get network connectivity by uploading my certificates to /etc/wifi/ and then configure WPA Supplicant through wpa_cli (I also put it in wpa_supplicant.conf, but it did not worked, I did not investigated on this).
> add_network
> scan_results
bssid / frequency / signal level / flags / ssid
00:21:91:f6:57:11 2442 -48 [WPA-EAP-TKIP+CCMP][ESS] wifiSSID
> set_network 0 ssid "wifiSSID"
OK
> set_network 0 proto WPA
OK
> set_network 0 key_mgmt WPA-EAP
OK
> set_network 0 pairwise TKIP
OK
> set_network 0 eap TLS
OK
> set_network 0 identity "username"
OK
> set_network 0 ca_cert "/etc/wifi/CAfile.pem"
OK
> set_network 0 private_key "/etc/wifi/usercert.p12"
OK
> set_network 0 private_key_passwd "passphrase"
OK
> list_networks
network id / ssid / bssid / flags
0 wifiSSID any [DISABLED]
> enable_network 0
OK
<3>CTRL-EVENT-STATE-CHANGE id=-1 state=3 BSSID=00:00:00:00:00:00
<3>CTRL-EVENT-SCAN-RESULTS
<3>Trying to associate with 00:11:22:f6:57:11 (SSID='wifiSSID' freq=2442 MHz)
<3>CTRL-EVENT-STATE-CHANGE id=-1 state=5 BSSID=00:11:22:f6:57:11
<3>CTRL-EVENT-STATE-CHANGE id=0 state=6 BSSID=00:11:22:f6:57:11
<3>Associated with 00:11:22:f6:57:11
<3>CTRL-EVENT-EAP-STARTED EAP authentication started
<3>CTRL-EVENT-EAP-PROPOSED-METHOD vendor=0 method=25 -> NAK
<3>CTRL-EVENT-EAP-PROPOSED-METHOD vendor=0 method=13
<3>CTRL-EVENT-EAP-METHOD EAP vendor 0 method 13 (TLS) selected
<3>CTRL-EVENT-EAP-PEER-CERT depth=1 subject='/C=FR/ST=France/L=Town/O=Orga/OU=WiFi/CN=wifiSSID/emailAddress=root@localhost'
<3>CTRL-EVENT-EAP-PEER-CERT depth=0 subject='/C=FR/ST=France/L=Town/O=Orga/OU=WiFi/CN=192.168.2.254/emailAddress=root@localhost'
<3>CTRL-EVENT-EAP-SUCCESS EAP authentication completed successfully
<3>CTRL-EVENT-STATE-CHANGE id=0 state=7 BSSID=00:00:00:00:00:00
<3>CTRL-EVENT-STATE-CHANGE id=0 state=8 BSSID=00:00:00:00:00:00
<3>WPA: Key negotiation completed with 00:11:22:f6:57:11 [PTK=TKIP GTK=TKIP]
<3>CTRL-EVENT-CONNECTED - Connection to 00:11:22:f6:57:11 completed (auth) [id=0 id_str=]
<3>CTRL-EVENT-STATE-CHANGE id=0 state=9 BSSID=00:00:00:00:00:00
After this, I'm able to connect to the internet :)
Comment 2•13 years ago
|
||
This is going to need frontend work to expose the options and backend work to actually use them. We support identity/password authentication currently but not identity/private-key authentication.
Blocks: b2g-wifi
Status: UNCONFIRMED → NEW
Ever confirmed: true
Summary: No EAP TLS support for WiFi → B2G Wifi: No EAP TLS support for WiFi
Reporter | ||
Comment 3•13 years ago
|
||
Great, with some help of kaze@ I started hacking on it.
Reporter | ||
Comment 4•13 years ago
|
||
This patch introduces EAP-TLS support in Gaia's WiFi settings.
Reporter | ||
Comment 5•13 years ago
|
||
This patch introduces the backend part of the EAP-TLS support, allowing Gecko to configure WPA Supplicant with EAP-TLS provided in Gaia's WiFi settings.
Reporter | ||
Comment 6•13 years ago
|
||
Not yet tested, but it should be good.
Reporter | ||
Comment 7•13 years ago
|
||
Not yet tested, but it should be good.
Reporter | ||
Comment 8•13 years ago
|
||
Tracking changes in EAP authentification (STARTED/SUCCESS/FAILURE only for now) and sending events accordingly.
Reporter | ||
Comment 9•13 years ago
|
||
Using events fired from Gecko to track the EAP authentication status, we notify user of this status.
Reporter | ||
Comment 10•13 years ago
|
||
Adding the missing string for locales en-US and fr to display current EAP authentication state.
Comment 11•13 years ago
|
||
Comment on attachment 615152 [details] [diff] [review]
Gaia support for EAP TLS
Please submit gaia changes as pull requests on github. Bugzilla only tracks gecko changes.
It's also a good idea to define the sequence of your patches (part 1, 2, etc.), to version each part and obsolete older versions. That way it's obvious to everybody how to apply them.
Attachment #615152 -
Attachment is obsolete: true
Updated•13 years ago
|
Attachment #615166 -
Attachment is obsolete: true
Updated•13 years ago
|
Attachment #615170 -
Attachment is obsolete: true
Reporter | ||
Comment 12•13 years ago
|
||
Thanks, I created an account on github and sent a pull request for my branch.
Reporter | ||
Comment 13•13 years ago
|
||
I/wpa_supplicant( 1247): wlan0: Trying to associate with 00:26:3e:25:f5:c2 (SSID='eduroam' freq=2412 MHz)
I/wpa_supplicant( 1247): wlan0: Associated with 00:26:3e:25:f5:c2
I/wpa_supplicant( 1247): wlan0: CTRL-EVENT-EAP-STARTED EAP authentication started
I/wpa_supplicant( 1247): wlan0: CTRL-EVENT-EAP-PROPOSED-METHOD vendor=0 method=25
E/wpa_supplicant( 1247): TLS: Unsupported Phase2 EAP method 'MSCHAPv2'
I/wpa_supplicant( 1247): wlan0: CTRL-EVENT-EAP-METHOD EAP vendor 0 method 25 (PEAP) selected
I/wpa_supplicant( 1247): wlan0: CTRL-EVENT-EAP-PEER-CERT depth=3 subject='/C=SE/O=AddTrust AB/OU=AddTrust External TTP Network/CN=AddTrust External CA Root'
I/wpa_supplicant( 1247): wlan0: CTRL-EVENT-EAP-PEER-CERT depth=3 subject='/C=SE/O=AddTrust AB/OU=AddTrust External TTP Network/CN=AddTrust External CA Root'
I/wpa_supplicant( 1247): wlan0: CTRL-EVENT-EAP-PEER-CERT depth=2 subject='/C=US/ST=UT/L=Salt Lake City/O=The USERTRUST Network/OU=http://www.usertrust.com/CN=UTN-USERFirst-Hardware'
I/wpa_supplicant( 1247): wlan0: CTRL-EVENT-EAP-PEER-CERT depth=1 subject='/C=NL/O=TERENA/CN=TERENA SSL CA'
I/wpa_supplicant( 1247): wlan0: CTRL-EVENT-EAP-PEER-CERT depth=0 subject='/C=FR/L=TOURS/O=UNIVERSITE DE TOURS FRANCOIS RABELAIS/CN=rad1.univ-tours.fr'
I/wpa_supplicant( 1247): EAP-MSCHAPV2: Authentication succeeded
I/wpa_supplicant( 1247): EAP-TLV: TLV Result - Success - EAP-TLV/Phase2 Completed
I/wpa_supplicant( 1247): wlan0: CTRL-EVENT-EAP-SUCCESS EAP authentication completed successfully
I/wpa_supplicant( 1247): wlan0: WPA: Key negotiation completed with 00:26:3e:25:f5:c2 [PTK=CCMP GTK=TKIP]
I/wpa_supplicant( 1247): wlan0: CTRL-EVENT-CONNECTED - Connection to 00:26:3e:25:f5:c2 completed (auth) [id=0 id_str=]
Comment 14•13 years ago
|
||
Comment on attachment 615153 [details] [diff] [review]
Gecko support for EAP TLS
Review of attachment 615153 [details] [diff] [review]:
-----------------------------------------------------------------
Thanks for the patch... I have one major comment, though...
::: dom/wifi/WifiWorker.js
@@ +1186,5 @@
> + if (net.eap) {
> + configured.eap = net.eap;
> + if (net.eap == "TLS") {
> + configured.pairwise = net.pairwise;
> + checkAssign("ca_cert", false);
I think we should avoid these names becoming part of the DOM API. For one thing, underscores class with the DOM's camelCase convention and for another with JS, we have the ability to be a little bit clearer. I'd also like to avoid tainting the DOM with wpa_supplicant-specific stuff.
I don't have any concrete proposals off the top of my head though. Any suggestions would be welcome.
Comment 15•13 years ago
|
||
(In reply to Blake Kaplan (:mrbkap) from comment #14)
> Comment on attachment 615153 [details] [diff] [review]
> Gecko support for EAP TLS
>
> Review of attachment 615153 [details] [diff] [review]:
> -----------------------------------------------------------------
>
> Thanks for the patch... I have one major comment, though...
>
> ::: dom/wifi/WifiWorker.js
> @@ +1186,5 @@
> > + if (net.eap) {
> > + configured.eap = net.eap;
> > + if (net.eap == "TLS") {
> > + configured.pairwise = net.pairwise;
> > + checkAssign("ca_cert", false);
>
> I think we should avoid these names becoming part of the DOM API. For one
> thing, underscores class with the DOM's camelCase convention and for another
> with JS, we have the ability to be a little bit clearer. I'd also like to
> avoid tainting the DOM with wpa_supplicant-specific stuff.
>
> I don't have any concrete proposals off the top of my head though. Any
> suggestions would be welcome.
Just curious. I don't see any *.idl changes here so I assume nothing is exposed to the DOM?
Component: General → DOM: Device Interfaces
Product: Boot2Gecko → Core
QA Contact: general → device-interfaces
Version: unspecified → Trunk
Attachment #615153 -
Flags: review?(gal)
Attachment #615158 -
Flags: review?(gal)
Comment 16•13 years ago
|
||
(In reply to Vivien Nicolas (:vingtetun) from comment #15)
> Just curious. I don't see any *.idl changes here so I assume nothing is
> exposed to the DOM?
The DOM-visible stuff is passed through jsval-based APIs, so there aren't any IDL changes needed to expose them.
Comment 17•13 years ago
|
||
(In reply to Blake Kaplan (:mrbkap) from comment #16)
> (In reply to Vivien Nicolas (:vingtetun) from comment #15)
> > Just curious. I don't see any *.idl changes here so I assume nothing is
> > exposed to the DOM?
>
> The DOM-visible stuff is passed through jsval-based APIs, so there aren't
> any IDL changes needed to expose them.
Make sense and I definitively agree with on abstracting wpa_supplicant.
Is networkConfigurationFields the list of field exposed to the DOM?
If yes this means there is already a few '_' values that should be removed and this can be done in a followup right? Or are those just used internally?
Comment 18•13 years ago
|
||
(In reply to Vivien Nicolas (:vingtetun) from comment #17)
> Is networkConfigurationFields the list of field exposed to the DOM?
> If yes this means there is already a few '_' values that should be removed
> and this can be done in a followup right? Or are those just used internally?
Those are the list of wpa_supplicant variables that we support internally. However, when we take a "network" object from the DOM, we pass it to netFromDOM (http://hg.mozilla.org/mozilla-central/file/cd8b66649278/dom/wifi/WifiWorker.js#l1140) which takes a smaller list of properties off of the object and translates them.
Comment 19•13 years ago
|
||
(In reply to Blake Kaplan (:mrbkap) from comment #18)
> (In reply to Vivien Nicolas (:vingtetun) from comment #17)
> > Is networkConfigurationFields the list of field exposed to the DOM?
> > If yes this means there is already a few '_' values that should be removed
> > and this can be done in a followup right? Or are those just used internally?
>
> Those are the list of wpa_supplicant variables that we support internally.
> However, when we take a "network" object from the DOM, we pass it to
> netFromDOM
> (http://hg.mozilla.org/mozilla-central/file/cd8b66649278/dom/wifi/WifiWorker.
> js#l1140) which take is a smaller list of properties off of the object and
> translates them.
So the missing part of the patch is this translation process?
Comment 20•13 years ago
|
||
(In reply to Vivien Nicolas (:vingtetun) from comment #19)
> So the missing part of the patch is this translation process?
Well, we need to decide what the DOM side looks like so we can translate from/to that. Right now, the patch simply exposes the wpa_supplicant names to the DOM.
Comment 21•13 years ago
|
||
Comment on attachment 615153 [details] [diff] [review]
Gecko support for EAP TLS
r- until we can figure out proper DOM names. I'll try to add another comment here later today with a proposal.
Attachment #615153 -
Flags: review?(gal) → review-
Comment 22•12 years ago
|
||
(In reply to Blake Kaplan (:mrbkap) from comment #21)
> Comment on attachment 615153 [details] [diff] [review]
> Gecko support for EAP TLS
>
> r- until we can figure out proper DOM names. I'll try to add another comment
> here later today with a proposal.
Any proposal?
Updated•12 years ago
|
Attachment #615158 -
Flags: review?(gal) → review+
Comment 23•12 years ago
|
||
Any progress here? I rather go with bad names temporarily than no names.
Reporter | ||
Comment 24•12 years ago
|
||
As I said on GitHub, we planned to work on this topic next weekend with kaze, benefitting from the MozFR meeting that will be held in Paris.
Reporter | ||
Comment 25•12 years ago
|
||
Here are a few ideas; it's linked to the discussion we had with kaze and vingtetun about moving wifi config out of wpa_supplicant.conf:
settings:
- key: network.wifi.enabled = true|false
- key: network.wifi.ssids =
[
"OpenNetwork": {
protection: "plain",
},
"WEP_Protected": {
protection: "wep",
keys: [ "key1", "key2", "key3" ],
},
"WPA_Protected": {
protection: "wpa",
wpa: {
mode: "key",
},
key: "value", // psk
},
"WPA2_Protected": {
protection: "wpa2",
wpa: {
mode: "key",
},
key: "value", // psk
},
"EAP-TLS_Protected": {
protection: "wpa", // or "wpa2"
wpa: {
mode: "eap"
},
eap: {
scheme: "tls",
identity: "user@domain.tld",
ca: "/path/to/ca_cert.pem",
cert: "/path/to/user_cert.pem",
privkey: "/path/to/user_private_key.p12",
// do not store private key password in settings, retrieve from keystore
keystore: "id",
},
},
"EAP-PEAP_Protected": {
protection: "wpa", // or "wpa2"
wpa: {
mode: "eap"
},
eap: {
scheme: "peap",
identity: "user@domain.tld",
ca: "/path/to/ca_cert.pem",
cert: "/path/to/user_cert.pem",
// do not store private key/user password in settings, retrieve from keystore
keystore: "id2",
},
}
]
Version: Trunk → Other Branch
Comment 26•12 years ago
|
||
(In reply to Alexandre LISSY from comment #25)
> Here are a few ideas; it's linked to the discussion we had with kaze and
> vingtetun about moving wifi config out of wpa_supplicant.conf:
This looks exactly right to me (whether or not we use wpa_supplicant.conf or not).
Reporter | ||
Comment 27•11 years ago
|
||
It's been a long time for this one, maybe we could work on this now?
Assignee | ||
Comment 28•11 years ago
|
||
For the certification part, the backend work has already started on bug 867899 and bug 789217.
The plan is import certification files into NSS DB(bug 867899), and replace android's keystore daemon with keystore in gecko with protocol compatibility(bug 789217).
So wpa_supplicant can get certification from NSS without any modification, just use special prefix in config like
ca_cert="keystore://ID_OF_CERTIFICATION"
For the config part, it's on bug 786741.
Lastly, since we already have non-camel names in wifi config in DOM, and we have to modify wifi worker to support changing config name. I think we should fire another bug for this.
Updated•11 years ago
|
Assignee: nobody → chulee
blocking-b2g: --- → 1.3+
Comment 29•11 years ago
|
||
Chuck is handling WPA-EAP now.
Updated•11 years ago
|
Target Milestone: --- → 1.3 Sprint 4 - 11/8
Updated•11 years ago
|
Whiteboard: [FT:RIL]
Updated•11 years ago
|
Target Milestone: 1.3 Sprint 4 - 11/8 → 1.3 Sprint 5 - 11/22
Updated•11 years ago
|
Target Milestone: 1.3 Sprint 5 - 11/22 → 1.3 Sprint 6 - 12/6
Assignee | ||
Comment 30•11 years ago
|
||
Comment on attachment 615157 [details] [diff] [review]
Gaia support for EAP PEAP
Gaia Part will be handled in bug 926334, thanks for the patch here.
Attachment #615157 -
Attachment is obsolete: true
Assignee | ||
Comment 31•11 years ago
|
||
Attachment #615153 -
Attachment is obsolete: true
Attachment #615158 -
Attachment is obsolete: true
Assignee | ||
Comment 32•11 years ago
|
||
Attachment #615165 -
Attachment is obsolete: true
Assignee | ||
Comment 33•11 years ago
|
||
Attachment #8338209 -
Attachment is obsolete: true
Attachment #8339802 -
Flags: review?(vchang)
Assignee | ||
Comment 34•11 years ago
|
||
Attachment #8338295 -
Attachment is obsolete: true
Attachment #8339803 -
Flags: review?(vchang)
Assignee | ||
Updated•11 years ago
|
Attachment #8339803 -
Attachment description: 3002. Support WPA-EAP connection state. → 0002. Support WPA-EAP connection state.
Updated•11 years ago
|
Attachment #8339802 -
Flags: review?(vchang) → review+
Updated•11 years ago
|
Attachment #8339803 -
Flags: review?(vchang) → review+
Assignee | ||
Comment 35•11 years ago
|
||
This patch only works after certificate functionalities are completed, so set block.
Depends on: 917175
Updated•11 years ago
|
Component: DOM: Device Interfaces → Wifi
Product: Core → Firefox OS
Version: Other Branch → unspecified
Comment 36•11 years ago
|
||
The user story bug this blocks (bug 922930) has a target milestone of future, so this is not a committed feature for 1.3. Non-committed features for 1.3 should not block the release.
blocking-b2g: 1.3+ → 1.3?
Assignee | ||
Comment 37•11 years ago
|
||
1. Check configure parameter is valid.
2. Return certificate name to DOM.
Attachment #8339802 -
Attachment is obsolete: true
Attachment #8343525 -
Flags: review?(vchang)
Updated•11 years ago
|
blocking-b2g: 1.3? → 1.4+
Target Milestone: 1.3 Sprint 6 - 12/6 → ---
Updated•11 years ago
|
Whiteboard: [FT:RIL]
Assignee | ||
Comment 40•11 years ago
|
||
update to webIDL.
Attachment #8343525 -
Attachment is obsolete: true
Attachment #8343525 -
Flags: review?(vchang)
Attachment #8406735 -
Flags: review?(vchang)
Attachment #8406735 -
Flags: review?(mrbkap)
Assignee | ||
Comment 41•11 years ago
|
||
Update to webIDL.
Attachment #8343526 -
Attachment is obsolete: true
Attachment #8406738 -
Flags: review?(vchang)
Attachment #8406738 -
Flags: review?(mrbkap)
Comment 42•11 years ago
|
||
Comment on attachment 8406735 [details] [diff] [review]
0001. Support WPA-EAP configure parameters. V3
Review of attachment 8406735 [details] [diff] [review]:
-----------------------------------------------------------------
I'm not the DOM peer, but the patch looks good to me, so feedback+.
::: dom/wifi/WifiWorker.js
@@ +1074,5 @@
> for (var n = 0; n < networkConfigurationFields.length; ++n) {
> let fieldName = networkConfigurationFields[n];
> if (!(fieldName in config) ||
> + (typeof(config[fieldName]) === 'undefined' ||
> + config[fieldName] === null) ||
I'd appreciate if you could refactor here a little bit.
Attachment #8406735 -
Flags: review?(vchang) → feedback+
Comment 43•11 years ago
|
||
Comment on attachment 8406738 [details] [diff] [review]
0002. Support WPA-EAP connection state. V3
Review of attachment 8406738 [details] [diff] [review]:
-----------------------------------------------------------------
Looks good.
Attachment #8406738 -
Flags: review?(vchang) → feedback+
Assignee | ||
Comment 44•11 years ago
|
||
Address comment 42.
Attachment #8406735 -
Attachment is obsolete: true
Attachment #8406735 -
Flags: review?(mrbkap)
Attachment #8408117 -
Flags: review?(mrbkap)
Reporter | ||
Comment 45•11 years ago
|
||
I had a look at the patches, but I'm wondering about something: I don't see any private_key/private_key_passwd occurrence in any of the code added. This is what I'm using to connect to EAP-TLS networks. The code claims to be supporting EAP-TLS.
Dumb question: are you sure this is working?
Flags: needinfo?(chulee)
Assignee | ||
Comment 46•11 years ago
|
||
Support for user certificate and private key is removed for now after security review, so no, TLS won't work for now.
At least we can bring up PEAP and some part of TTLS as first step.
Flags: needinfo?(chulee)
Reporter | ||
Comment 47•11 years ago
|
||
(In reply to Chuck Lee [:chucklee] from comment #46)
> Support for user certificate and private key is removed for now after
> security review, so no, TLS won't work for now.
> At least we can bring up PEAP and some part of TTLS as first step.
Fair enough, but can we just make sure we won't break any user-defined wpa_supplicant.conf connection to an EAP-TLS network ?
Assignee | ||
Comment 48•11 years ago
|
||
Technically yes, but it's like a hack to handle the exception.
I think we have to check if it's acceptable to do that.
Flags: needinfo?(vchang)
Flags: needinfo?(mrbkap)
Flags: needinfo?(hchang)
Updated•11 years ago
|
Flags: needinfo?(vchang)
Summary: B2G Wifi: No EAP TLS support for WiFi → B2G Wifi: Support EAP-PEAP and EAP-TTLS with out private key
Reporter | ||
Comment 49•11 years ago
|
||
(In reply to Chuck Lee [:chucklee] from comment #48)
> Technically yes, but it's like a hack to handle the exception.
> I think we have to check if it's acceptable to do that.
Okay, let me rephrase it: if EAP-TLS gets broken, this technically means I won't be able to dogfood.
Reporter | ||
Comment 50•11 years ago
|
||
(In reply to Chuck Lee [:chucklee] from comment #46)
> Support for user certificate and private key is removed for now after
> security review, so no, TLS won't work for now.
> At least we can bring up PEAP and some part of TTLS as first step.
Can you elaborate and give references regarding this security review ?
Also, how will be stored credentials for EAP-TTLS and EAP-PEAP, if we have no keystore secure enough for EAP-TLS ? Will those be dumped into wpa_supplicant.conf ?
Flags: needinfo?(chulee)
Assignee | ||
Comment 51•11 years ago
|
||
(In reply to Alexandre LISSY :gerard-majax from comment #50)
> (In reply to Chuck Lee [:chucklee] from comment #46)
> > Support for user certificate and private key is removed for now after
> > security review, so no, TLS won't work for now.
> > At least we can bring up PEAP and some part of TTLS as first step.
>
> Can you elaborate and give references regarding this security review ?
>
It's done with briansmith in many discussions including IRC, email, and skype.
It was pointed out in bug 917102 comment 35.
> Also, how will be stored credentials for EAP-TTLS and EAP-PEAP, if we have
> no keystore secure enough for EAP-TLS ? Will those be dumped into
> wpa_supplicant.conf ?
wpa_supplicant.conf protect by I/O permission.
A face to face security review with pauljt also mentioned this, we agreed that storing password in that way is acceptable(for now, as it is from the first day and most android phone), but storing private key file in the almost same security level is riskier than password.
Also bug 786741 is opened for not to do so but we don't have resource for that.
Flags: needinfo?(chulee)
Reporter | ||
Comment 52•11 years ago
|
||
(In reply to Chuck Lee [:chucklee] from comment #51)
> (In reply to Alexandre LISSY :gerard-majax from comment #50)
> > (In reply to Chuck Lee [:chucklee] from comment #46)
> > > Support for user certificate and private key is removed for now after
> > > security review, so no, TLS won't work for now.
> > > At least we can bring up PEAP and some part of TTLS as first step.
> >
> > Can you elaborate and give references regarding this security review ?
> >
>
> It's done with briansmith in many discussions including IRC, email, and
> skype.
> It was pointed out in bug 917102 comment 35.
Thanks for this :)
>
> > Also, how will be stored credentials for EAP-TTLS and EAP-PEAP, if we have
> > no keystore secure enough for EAP-TLS ? Will those be dumped into
> > wpa_supplicant.conf ?
>
> wpa_supplicant.conf protect by I/O permission.
> A face to face security review with pauljt also mentioned this, we agreed
> that storing password in that way is acceptable(for now, as it is from the
> first day and most android phone), but storing private key file in the
> almost same security level is riskier than password.
Well, that's a choice, but I'm unsure about the argument itself: why would a login/password (TTLS/PEAP) would be considered less sensitive than a certificate/passphrase ?
> Also bug 786741 is opened for not to do so but we don't have resource for
> that.
Yes, I remember about this one.
Comment 53•11 years ago
|
||
Comment on attachment 8408117 [details] [diff] [review]
0001. Support WPA-EAP configure parameters. V4
Review of attachment 8408117 [details] [diff] [review]:
-----------------------------------------------------------------
r=me with my comments addressed or answered.
::: dom/wifi/WifiWorker.js
@@ +1084,5 @@
> + function hasValidProperty(name) {
> + return ((name in config) &&
> + typeof(config[name]) !== 'undefined' &&
> + config[name] !== null &&
> + config[fieldName] !== '*' &&
Using 'fieldName' on the last line looks like a bug to me.
@@ +1765,5 @@
> pub.known = true;
> if (net.scan_ssid === 1)
> pub.hidden = true;
> + if ("ca_cert" in net && net.ca_cert &&
> + net.ca_cert.indexOf("keystore://WIFI_SERVERCERT_" === 0))
Nit: Please put braces around the 'then' part of this if statement (due to the multi-line condition).
@@ +1831,5 @@
> }
>
> + function hasValidProperty(name) {
> + return ((name in net) &&
> + (typeof(net[name]) !== 'undefined') && (net[name] !== null));
This second line could be written as |net[name] != null| because |null == undefined|. If that's too clever for your taste, feel free to leave it as it is.
@@ +1844,5 @@
> + net.phase1 = quote(net.phase1);
> +
> + if (hasValidProperty("phase2")) {
> + if (net.eap === "PEAP") {
> + net.phase2 = quote("auth=" + net.phase2);
Should we be validating the contents of net.phase2 somehow before sticking it into our config?
Attachment #8408117 -
Flags: review?(mrbkap) → review+
Comment 54•11 years ago
|
||
Comment on attachment 8406738 [details] [diff] [review]
0002. Support WPA-EAP connection state. V3
Review of attachment 8406738 [details] [diff] [review]:
-----------------------------------------------------------------
Optimistically marking r=me based on vchang's review. Please address my question before landing, though.
::: dom/wifi/WifiWorker.js
@@ +679,5 @@
> + return true;
> + }
> + if (event.indexOf("CTRL-EVENT-EAP-TLS-CERT-ERROR") !== -1) {
> + // Cert Error
> + WifiManager.disconnect(function(ok) {});
IIRC, disconnect will cause us to disconnect from the network and not try again until we issue an associate or reassociate command. Is that really what's intended here?
Attachment #8406738 -
Flags: review?(mrbkap) → review+
Comment 55•11 years ago
|
||
(In reply to Chuck Lee [:chucklee] from comment #48)
> Technically yes, but it's like a hack to handle the exception.
> I think we have to check if it's acceptable to do that.
IMO it's fine to add extra code to allow for user-modified wpa_supplicant configurations (especially for things like this where the UI doesn't allow it).
Flags: needinfo?(mrbkap)
Assignee | ||
Comment 57•11 years ago
|
||
Update reviewer list.
Attachment #8406738 -
Attachment is obsolete: true
Assignee | ||
Comment 58•10 years ago
|
||
Address comment 53.
Attachment #8410923 -
Attachment is obsolete: true
Assignee | ||
Comment 59•10 years ago
|
||
Resolve problem pointed out in comment 54.
Attachment #8410924 -
Attachment is obsolete: true
Assignee | ||
Comment 60•10 years ago
|
||
Fix validation rule.
Attachment #8412499 -
Attachment is obsolete: true
Assignee | ||
Comment 62•10 years ago
|
||
Try result : https://tbpl.mozilla.org/?tree=Try&rev=b586311119d2
This is 4th bug to check in.
Keywords: checkin-needed
Comment 63•10 years ago
|
||
https://hg.mozilla.org/integration/b2g-inbound/rev/6068e0c1fa67
https://hg.mozilla.org/integration/b2g-inbound/rev/dde446669ba6
Keywords: checkin-needed
Comment 64•10 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/6068e0c1fa67
https://hg.mozilla.org/mozilla-central/rev/dde446669ba6
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → 2.0 S1 (9may)
Comment 66•10 years ago
|
||
#775499 is NOT a duplicate of this one!
That other bug asks for a specific security check to get included in UI and woa_supplicant.conf: the "subject_match" which allows the user to specify *which server exactly* to expect during EAP authentication.
This bug here speaks about client certificates, which has nothing to do with server cert name validation. That's a different thing. Servetr cert names should be checked with all EAP types: PEAP, TTLS, TLS - regardless if client certificates are involved!
No comment in this bug here touches the subject_match parameter. Could someone with a clue review this please?
Assignee | ||
Comment 67•10 years ago
|
||
This bug is enabling choosing server certificate for a WPA-EAP network, which is the summary indicates in bug 775499. Also, what the patches did are the same in these two bugs.
Since we enables support for server certificate in wifi now(bug 917102, bug 917176, and bug 917175), I think using server certificate is easier and safer than using subject_match.
Comment 68•10 years ago
|
||
This is not true.
The three bugs you mentioned are about CA certificates, NOT server certificates. What makes a WPA-EAP network secure is that the *tuple*: ( CA + *name of expected server* ) during connection attempt match user configuration.
Your three referenced bugs only enable the configuration of the first half of that tuple - the CA trust root. The server name is the second component to this, only this locks down the connection attempt to exactly your organisation's authentication server.
Bug 775499 was all the time about this second part of the tuple.
Please re-open that bug.
Updated•10 years ago
|
feature-b2g: --- → 2.0
Updated•10 years ago
|
Flags: needinfo?(hchang)
Updated•10 years ago
|
Summary: B2G Wifi: Support EAP-PEAP and EAP-TTLS with out private key → B2G Wifi: Support EAP-PEAP and EAP-TTLS without private key
Updated•10 years ago
|
blocking-b2g: backlog → ---
tracking-b2g:
--- → backlog
You need to log in
before you can comment on or make changes to this bug.
Description
•