Closed
Bug 833236
Opened 12 years ago
Closed 7 years ago
[FUGU][wifi]It is shown as WAPI PSK type when searching and connecting to the AP which is set as WAP2 PSK.(617001975709)
Categories
(Firefox OS Graveyard :: Wifi, defect)
Tracking
(tracking-b2g:backlog, firefox26 affected, b2g18 affected, b2g-v2.1 affected)
RESOLVED
WONTFIX
tracking-b2g | backlog |
People
(Reporter: Firefox_Mozilla, Assigned: pzhang)
References
Details
(Whiteboard: [MZ][triaged:3/1])
Attachments
(5 files, 4 obsolete files)
46 bytes,
text/x-github-pull-request
|
arthurcc
:
review+
|
Details | Review |
4.90 KB,
patch
|
vchang
:
review+
|
Details | Diff | Splinter Review |
2.04 KB,
patch
|
chucklee
:
review+
|
Details | Diff | Splinter Review |
2.49 KB,
patch
|
chucklee
:
review+
|
Details | Diff | Splinter Review |
8.11 KB,
patch
|
Details | Diff | Splinter Review |
Steps to reproduce:
1. set WIFI server as WAPI PSK authentication type;
2. go into wifi and search the AP of this server.
Expected results:
The access point encryption is WPA2 PSK.
Actual results:
the encryption of the access point is recongnized as WPA-PSK
Reporter | ||
Updated•12 years ago
|
OS: Windows XP → Gonk (Firefox OS)
Hardware: x86 → ARM
Updated•12 years ago
|
Group: mozilla-corporation-confidential
Updated•12 years ago
|
Assignee: nobody → vchang
Status: UNCONFIRMED → NEW
Ever confirmed: true
Comment 1•12 years ago
|
||
(In reply to Firefox_Mozilla from comment #0)
> Steps to reproduce:
> 1. set WIFI server as WAPI PSK authentication type;
> 2. go into wifi and search the AP of this server.
>
> Expected results:
> The access point encryption is WPA2 PSK.
>
> Actual results:
> the encryption of the access point is recongnized as WPA-PSK
Does the wpa_supplicant and wifi driver support WAPI PSK ?
First of all, I would like to have the AP which supports WAPI PSK.
Comment 2•12 years ago
|
||
This is a new feature and needs the support from wpa_supplicant and wifi driver.
Not sure if our Unagi Android supports this feature or not.
Because the wpa_supplicant and Wifi driver is provided by vendor, can you help to double confirm with them ? Also do we have the plan to support this feature ?
Flags: needinfo?(khu)
Comment 3•12 years ago
|
||
Good point. Firefox_Mozilla@126.com, do you have the answer for Vincent's question?
Flags: needinfo?(khu) → needinfo?(Firefox_Mozilla)
Reporter | ||
Comment 4•12 years ago
|
||
I confirm the wpa_supplicant and wifi driver support WAPI PSK. Other Android phone can identify it as WPA-PSK.
Flags: needinfo?(Firefox_Mozilla)
Reporter | ||
Updated•12 years ago
|
Whiteboard: [MZ]
Updated•12 years ago
|
Whiteboard: [MZ] → [MZ][triaged:3/1]
Comment 5•12 years ago
|
||
Let's do it after China market is included & planned.
I was able to repro this issue on the following builds/devices.
Environmental Variables
Device: Buri 1.2 mozRIL
Build ID: 20130916040205
Gecko: http://hg.mozilla.org/mozilla-central/rev/c4bcef90cef9
Gaia: a0079597d510ce8ea0b9cbb02c506030510b9eeb
Platform Version: 26.0a1
Environmental Variables
Device: Leo 1.1 comRIL
Build ID: 20130916041201
Gecko: http://hg.mozilla.org/releases/mozilla-b2g18/rev/bebaca30ea30
Gaia: 8b20ff956f8e366a17a7ef5f64935a6a6a8cb0c0
Platform Version: 18.1
RIL Version: 01.01.00.019.225
status-b2g18:
--- → affected
status-firefox26:
--- → affected
Assignee | ||
Comment 8•11 years ago
|
||
I'm working with Sam@SPRD on this issue.
Assignee: nobody → pzhang
Summary: [OPEN_][wifi]It is shown as WAPI PSK type when searching and connecting to the AP which is set as WAP2 PSK.(617001975709) → [FUGU][wifi]It is shown as WAPI PSK type when searching and connecting to the AP which is set as WAP2 PSK.(617001975709)
Assignee | ||
Comment 9•11 years ago
|
||
Attachment #8348491 -
Flags: review?(arthur.chen)
Assignee | ||
Comment 10•11 years ago
|
||
Attachment #8348492 -
Flags: review?(vchang)
Comment 11•11 years ago
|
||
Comment on attachment 8348492 [details] [diff] [review]
gecko.patch
Review of attachment 8348492 [details] [diff] [review]:
-----------------------------------------------------------------
Thank you for jumping to this. WAPI-PSK is a proprietary security type for China market.
I don't have the Wifi AP that support WAPI-PSK security type here.
Please make sure that you have verified the patch on real device, and it doesn't break the normal function. :-)
Please address my comment and post a new patch.
::: dom/wifi/WifiWorker.js
@@ +67,4 @@
> const WIFI_SECURITY_TYPE_NONE = "open";
> const WIFI_SECURITY_TYPE_WPA_PSK = "wpa-psk";
> const WIFI_SECURITY_TYPE_WPA2_PSK = "wpa2-psk";
> +const WIFI_SECURITY_TYPE_WAPI_PSK = "wapi-psk";
I don't think we need this. It's for wifi hotspot.
@@ +2572,1 @@
>
Why do you need to check the WAPI-PSK here ? This function is for wifi hotspot. Do we also support WAPI-PSK in wifi hotspot mode ?
Attachment #8348492 -
Flags: review?(vchang)
Comment 12•11 years ago
|
||
Comment on attachment 8348491 [details] [review]
https://github.com/mozilla-b2g/gaia/pull/14735
r=me for the settings part. Thanks for the patch, Ping.
Flag Borja for the FTU part.
Attachment #8348491 -
Flags: review?(borja.bugzilla)
Attachment #8348491 -
Flags: review?(arthur.chen)
Attachment #8348491 -
Flags: review+
Assignee | ||
Comment 13•11 years ago
|
||
Addressed
Attachment #8348492 -
Attachment is obsolete: true
Attachment #8351164 -
Flags: review?(vchang)
Comment 14•11 years ago
|
||
Comment on attachment 8351164 [details] [diff] [review]
Patch v2
Review of attachment 8351164 [details] [diff] [review]:
-----------------------------------------------------------------
Looks good. Thank you.
Attachment #8351164 -
Flags: review?(vchang) → review+
Assignee | ||
Comment 15•11 years ago
|
||
Hi Borja, could you help to review the FTU part? Thanks.
Flags: needinfo?(borja.bugzilla)
Comment 16•11 years ago
|
||
Just arrived from Holidays! Taking a look :)
Flags: needinfo?(borja.bugzilla)
Comment 18•11 years ago
|
||
Hi,
This issue was also reported with open device (adding dep to meta bug). We think this it'd be important to be solved for 1.3. Nominating it for discussion
blocking-b2g: --- → 1.3?
Comment 19•11 years ago
|
||
moving to minus since we shipped 1.1/1.2 with this
blocking-b2g: 1.3? → ---
Assignee | ||
Comment 20•11 years ago
|
||
Here is the rebased path, Gaia PR is also rebased on top of the master branch.
Assignee | ||
Comment 23•11 years ago
|
||
(In reply to Borja Salguero [:borjasalguero] from comment #16)
> Just arrived from Holidays! Taking a look :)
Hi Borja, any update?
Flags: needinfo?(borja.bugzilla)
(In reply to Arthur Chen [:arthurcc] from comment #12)
> Comment on attachment 8348491 [details] [review]
> https://github.com/mozilla-b2g/gaia/pull/14735
>
> r=me for the settings part. Thanks for the patch, Ping.
>
> Flag Borja for the FTU part.
I think we don't need the ASCII/HEX select.
At least in WPA-PSK, when string length is 8 to 63, it's ASCII; when string length is 64, it's hex.
is there a different rule in WAPI?
Also I think the hex to char transform should be done in wpa_supplicant, at least it's how WPA-PSK does.
At least it should be moved into gecko so others who don't know/can't use WifiHelper can set passphrase correctly.
Another concern is config values like "key_mgmt=WAPI-PSK" and "proto=WAPI" are proprietary values defined by vendor-specific wpa_supplicant.
What's our policy about supporting vendor-specific functions in master repo?
Assignee | ||
Comment 25•11 years ago
|
||
(In reply to Chuck Lee [:chucklee] from comment #24)
> (In reply to Arthur Chen [:arthurcc] from comment #12)
> > Comment on attachment 8348491 [details] [review]
> > https://github.com/mozilla-b2g/gaia/pull/14735
> >
> > r=me for the settings part. Thanks for the patch, Ping.
> >
> > Flag Borja for the FTU part.
>
> I think we don't need the ASCII/HEX select.
> At least in WPA-PSK, when string length is 8 to 63, it's ASCII; when string
> length is 64, it's hex.
> is there a different rule in WAPI?
The rule of the password length is almost the same for HEX and ASCII, except the length of the HEX password must be even.
>
> Also I think the hex to char transform should be done in wpa_supplicant, at
> least it's how WPA-PSK does.
FUGU did transformation in their wpa_supplicant, but requires a proprietary configure key (psk_key_type), so I decided to implement it in Gaia.
> At least it should be moved into gecko so others who don't know/can't use
> WifiHelper can set passphrase correctly.
I we did this in Gecko, I think we need to modify the API to pass the type of the password, i.e. HEX or ASCII.
>
> Another concern is config values like "key_mgmt=WAPI-PSK" and "proto=WAPI"
> are proprietary values defined by vendor-specific wpa_supplicant.
> What's our policy about supporting vendor-specific functions in master repo?
Sorry, i don't know, but take FM radio for example, GonkFMRadio.cpp only works for Qualcom HW, FUGU add their own GonkFMRadio implementation, so I think we could file another bug for this, and provide the solution when we have some other devices to support, what do you think?
(In reply to Pin Zhang [:pzhang] from comment #25)
> > At least it should be moved into gecko so others who don't know/can't use
> > WifiHelper can set passphrase correctly.
>
> I we did this in Gecko, I think we need to modify the API to pass the type
> of the password, i.e. HEX or ASCII.
In such case, I would prefer:
In Gaia, set |password.type| to "ASCII" or "HEX" to |wifiHelper.setPassword()|, so we don't have to change the function definition, then the network parameter can be passed to |WifiManager.associate()| as normal.
In Gecko, do the psk transform in |netFromDOM()|.
> >
> > Another concern is config values like "key_mgmt=WAPI-PSK" and "proto=WAPI"
> > are proprietary values defined by vendor-specific wpa_supplicant.
> > What's our policy about supporting vendor-specific functions in master repo?
>
> Sorry, i don't know, but take FM radio for example, GonkFMRadio.cpp only
> works for Qualcom HW, FUGU add their own GonkFMRadio implementation, so I
> think we could file another bug for this, and provide the solution when we
> have some other devices to support, what do you think?
Changing FM Radio implementation only affects hal and nothing else, which is hal expected to do.
But supporting WAPI here affects Gecko and Gaia, makes them platform-dependent.
I personally like to leave these patches in partners' own code base.
Assignee | ||
Comment 27•11 years ago
|
||
(In reply to Chuck Lee [:chucklee] from comment #26)
> (In reply to Pin Zhang [:pzhang] from comment #25)
> > > At least it should be moved into gecko so others who don't know/can't use
> > > WifiHelper can set passphrase correctly.
> >
> > I we did this in Gecko, I think we need to modify the API to pass the type
> > of the password, i.e. HEX or ASCII.
>
> In such case, I would prefer:
> In Gaia, set |password.type| to "ASCII" or "HEX" to
> |wifiHelper.setPassword()|, so we don't have to change the function
> definition, then the network parameter can be passed to
> |WifiManager.associate()| as normal.
> In Gecko, do the psk transform in |netFromDOM()|.
It might works, let me take a look.
>
> > >
> > > Another concern is config values like "key_mgmt=WAPI-PSK" and "proto=WAPI"
> > > are proprietary values defined by vendor-specific wpa_supplicant.
> > > What's our policy about supporting vendor-specific functions in master repo?
> >
> > Sorry, i don't know, but take FM radio for example, GonkFMRadio.cpp only
> > works for Qualcom HW, FUGU add their own GonkFMRadio implementation, so I
> > think we could file another bug for this, and provide the solution when we
> > have some other devices to support, what do you think?
>
> Changing FM Radio implementation only affects hal and nothing else, which is
> hal expected to do.
> But supporting WAPI here affects Gecko and Gaia, makes them
> platform-dependent.
> I personally like to leave these patches in partners' own code base.
The WAPI feature must be supported in any devices that be saled in China, so these codes should be maintained in our codebase, and the need-to-be-configured-by-parteners part is very mininal, e.g. the configured values, I think the case is the same as FM radio gonk for which we defined the hal interface that FMRadio API needed, and here what we need to do is defined the wapi-config-interface for parteners.
Assignee | ||
Comment 28•11 years ago
|
||
The is the incremental patch based on attachment 8361461 [details] [diff] [review].
Attachment #8375398 -
Flags: review?(chulee)
Assignee | ||
Comment 29•11 years ago
|
||
This is an incremental patch based on attachment 8348491 [details] [review].
Attachment #8375399 -
Flags: review?(chulee)
Assignee | ||
Comment 30•11 years ago
|
||
Attachment #8375399 -
Attachment is obsolete: true
Attachment #8375399 -
Flags: review?(chulee)
Attachment #8375400 -
Flags: review?(chulee)
Comment on attachment 8375400 [details] [diff] [review]
Do not convert in Gaia
Review of attachment 8375400 [details] [diff] [review]:
-----------------------------------------------------------------
Looks good, thanks!
Please also update the Gaia pull request.
Attachment #8375400 -
Flags: review?(chulee) → review+
Comment on attachment 8375398 [details] [diff] [review]
Convert HEX to ASCII in Gecko
Review of attachment 8375398 [details] [diff] [review]:
-----------------------------------------------------------------
Please add a check on HEX psk before transforming it, others looks good!
::: dom/wifi/WifiWorker.js
@@ +1627,5 @@
> + if (net.key_mgmt == "WAPI-PSK") {
> + net.proto = configured.proto = "WAPI";
> +
> + // convert HEX to ASCII
> + if (net.pskType == "HEX") {
I think we also have to validate the hex string from DOM, but netFromDOM can't return error with message now.
We can just skip the transform on invalid hex string here(and connection will fail due to incorrect password), and file a follow up bug to report error in netToDOM/netFromDOM.
Attachment #8375398 -
Flags: review?(chulee)
Assignee | ||
Comment 33•11 years ago
|
||
Attachment #8375398 -
Attachment is obsolete: true
Attachment #8376008 -
Flags: review?(chulee)
Comment on attachment 8376008 [details] [diff] [review]
Convert HEX to ASCII in Gecko v2
Review of attachment 8376008 [details] [diff] [review]:
-----------------------------------------------------------------
Thank you!
Attachment #8376008 -
Flags: review?(chulee) → review+
Assignee | ||
Comment 35•11 years ago
|
||
Gaia PR is updated.(In reply to Chuck Lee [:chucklee] from comment #34)
> Comment on attachment 8376008 [details] [diff] [review]
> Convert HEX to ASCII in Gecko v2
>
> Review of attachment 8376008 [details] [diff] [review]:
> -----------------------------------------------------------------
>
> Thank you!
Gaia PR has been updated.
Assignee | ||
Comment 36•11 years ago
|
||
This is the full patch for Gecko rebased on top of latest commit, Gaia PR is also rebased.
Attachment #8361461 -
Attachment is obsolete: true
Assignee | ||
Comment 37•11 years ago
|
||
(In reply to Pin Zhang [:pzhang] from comment #23)
> (In reply to Borja Salguero [:borjasalguero] from comment #16)
> > Just arrived from Holidays! Taking a look :)
>
> Hi Borja, any update?
Hi Borja, do you have bandwidth for reviewing the gaia patch? It's OK if you are overloaded right now, I can ask someone else for help.
There's another discussion about implementing WAPI per bug 953237 comment 21.
Updated•11 years ago
|
blocking-b2g: --- → backlog
Comment 39•10 years ago
|
||
Comment on attachment 8348491 [details] [review]
https://github.com/mozilla-b2g/gaia/pull/14735
Removing flag due to https://bugzilla.mozilla.org/show_bug.cgi?id=833236#c38
Attachment #8348491 -
Flags: review?(borja.bugzilla)
Flags: needinfo?(borja.bugzilla)
Updated•10 years ago
|
status-b2g-v2.1:
--- → affected
Updated•10 years ago
|
blocking-b2g: backlog → ---
tracking-b2g:
--- → backlog
Comment 40•7 years ago
|
||
Firefox OS is not being worked on
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → WONTFIX
You need to log in
before you can comment on or make changes to this bug.
Description
•