Closed Bug 859209 Opened 11 years ago Closed 11 years ago

[Buri][Shira][WIFI]It keep Connecting when input wrong password to connect AP security with WEP

Categories

(Firefox OS Graveyard :: Wifi, defect, P3)

ARM
Gonk (Firefox OS)
defect

Tracking

(blocking-b2g:1.3+)

RESOLVED FIXED
blocking-b2g 1.3+

People

(Reporter: sync-1, Assigned: vchang)

References

()

Details

(Whiteboard: Poland, IOT, Buri, [FT:RIL], POVB)

Attachments

(2 files, 1 obsolete file)

+++ This bug was initially created as a clone of Bug #418614 +++
 
 DEFECT DESCRIPTION:
 It keep Connecting when input wrong password to connect AP security with WEPREPRODUCING PROCEDURES:
 1.Settings->Wi-Fi->input wrong password to connect AP security whit WEP,but it keep Connecting all the time->KO
 EXPECTED BEHAVIOUR:
 1.when input wrong password when connect AP security with WEP,No prompt message, but mobile should not try to connect it after 3 fails
 ASSOCIATE SPECIFICATION:
 
 TEST PLAN REFERENCE:
 
 TOOLS AND PLATFORMS USED:
 
 USER IMPACT:
 
 REPRODUCING RATE:5/5
 
 For FT PR, Please list reference mobile's behavior:
 
 ++++++++++ end of initial bug #418614 description ++++++++++
 
 
 
 CONTACT INFO (Name,Phone number):
 
  DEFECT DESCRIPTION:
 
  REPRODUCING PROCEDURES:
 
  EXPECTED BEHAVIOUR:
 
  ASSOCIATE SPECIFICATION:
 
  TEST PLAN REFERENCE:
 
  TOOLS AND PLATFORMS USED:
 
  USER IMPACT:
 
  REPRODUCING RATE:
 
  For FT PR, Please list reference mobile's behavior:
Dear Mozilla team,
 
 Any update?
Assignee: nobody → vliu
1. In attached log, we don't observe the CTRL-EVENT-ASSOC-REJECT reported from wpa_supplicant when we enter incorrect wep password. We need this event to detect wep password is incorrect. It looks like normal connection without any error indication from wpa_supplicant. 
Can Vendor help to check if they can modify the wpa_supplicant ? Any suggestion is welcome. 

2. When we enter the wep password with incorrect length, the driver or wpa_supplicant seems remove the network directly without indicating any error.
It also makes us very difficult to detect the wep password is incorrect.
Hi Daniel,

Please help to check this issu, thanks.
blocking-b2g: --- → tef?
This bug also needs the help from wpa_supplicant or wifi driver based on the attached file and comment 2. May I know your ideas about it ?
Flags: needinfo?(sync-1)
If we end up blocking on this (depends on the answer to comment 4, and the level of effort), we won't want to take any new UX or strings (unless already available).
Dear mozilla team,
 
 Ive
Flags: needinfo?(sync-1)
Dear mozilla team,
 
 wev have submitted a case to qualcomm to ask for support.Thanks for your effort.
 If any update we will tell u.
 
 Thanks
tef- pending feedback from qualcomm. If this needs to block, please change back to tef? and provide reasoning for blocking on this.
blocking-b2g: tef? → -
Whiteboard: Poland, IOT, Buri
Summary: [Buri][WIFI]It keep Connecting when input wrong password to connect AP security with WEP → [Buri][Shira][WIFI]It keep Connecting when input wrong password to connect AP security with WEP
Priority: P1 → P3
Qualcomm feedback:

WEP security  has no key management, so it can't check key and give the
response to upper layer. You need to disconnect the AP from upper layer when
your DHCP fail, such as:  framework disconnect from the AP when you DHCP fail 3
times.
________________________________________
Feedback from customer:
 Strictly speaking WEP should be
disabled on all devices as it is VERY insecure.

 this is for sure not blocker for 1.0.1  but please could you please provide reply such as the planning to fix...
Flags: needinfo?(jcheng)
hi vincent, what's your comment on this one? thanks
Flags: needinfo?(jcheng) → needinfo?(vchang)
Indeed, WEP is insecure comparing with WPA.
As a Web platform, we should have wider support in features rather than remove them.
But Partners can decide if they want to disable the WEP in customization build by themself.

It's a little weird to me that we also use QC's wpa_supplicant and wifi driver in unagi. We do have received events as I said in comment 2. 
Anyway, we can fix this bug in our framework when DHCP failed for several times.
Flags: needinfo?(vchang)
Checked on 
AU_LINUX_GECKO_ICS_STRAWBERRY.01.01.00.019.164
Firefox os  v1.1
Mozilla build ID:20130715070218

Still not OK, Requested by Shira, mark leo?
blocking-b2g: - → leo?
This would be feature work for v1.2, it's too late to consider this a blocker for 1.1 release.
blocking-b2g: leo? → -
blocking-b2g: - → koi?
Kevin/Sandip, can you take this into your roadmap? Thanks
Flags: needinfo?(skamat)
Flags: needinfo?(khu)
It can't be in v1.2. So, remove koi?. 
We can take it into consideration for future versions, of course. Need to do evaluation first.
blocking-b2g: koi? → 1.3?
Flags: needinfo?(khu)
Whiteboard: Poland, IOT, Buri → Poland, IOT, Buri, [FT:RIL]
Whiteboard: Poland, IOT, Buri, [FT:RIL] → Poland, IOT, Buri, [FT:RIL], POVB
Assignee: vliu → vchang
Attached patch Patch v1.0 for mc (obsolete) — Splinter Review
Add the maximum DHCP retry limitation.
Attachment #813014 - Flags: review?(chulee)
Comment on attachment 813014 [details] [diff] [review]
Patch v1.0 for mc

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

::: dom/wifi/WifiWorker.js
@@ +728,5 @@
>      controlMessage({ cmd: "dhcp_do_request", ifname: ifname }, function(data) {
>        dhcpInfo = data.status ? null : data;
> +      if (dhcpInfo) manager.dhcpFailuresCount = 0;
> +      if (!dhcpInfo &&
> +          ++manager.dhcpFailuresCount > MAX_RETRIES_ON_DHCP_FAILURE - 1) {

I propose a little modification here:

if (dhcpInfo) {
  manager.dhcpFailuresCount = 0;
} else if (++manager.dhcpFailuresCount === MAX_RETRIES_ON_DHCP_FAILURE) {

The conditional branch on |dhcpInfo| can be merged into |if ... else if| form, and reduce number the judgement.
Replacing |>| with |===| can get ride of the tailing minus one.
Attachment #813014 - Flags: review?(chulee) → review+
(In reply to Chuck Lee [:chucklee] from comment #18)
> Comment on attachment 813014 [details] [diff] [review]
> Patch v1.0 for mc
> 
> Review of attachment 813014 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: dom/wifi/WifiWorker.js
> @@ +728,5 @@
> >      controlMessage({ cmd: "dhcp_do_request", ifname: ifname }, function(data) {
> >        dhcpInfo = data.status ? null : data;
> > +      if (dhcpInfo) manager.dhcpFailuresCount = 0;
> > +      if (!dhcpInfo &&
> > +          ++manager.dhcpFailuresCount > MAX_RETRIES_ON_DHCP_FAILURE - 1) {
> 
> I propose a little modification here:
> 
> if (dhcpInfo) {
>   manager.dhcpFailuresCount = 0;
> } else if (++manager.dhcpFailuresCount === MAX_RETRIES_ON_DHCP_FAILURE) {
> 
> The conditional branch on |dhcpInfo| can be merged into |if ... else if|
> form, and reduce number the judgement.
> Replacing |>| with |===| can get ride of the tailing minus one.

Thanks for your suggest. Look good. 
But I would like to replace |>| to |>==|.
Rebase and address the review comments.
Attachment #813014 - Attachment is obsolete: true
Attachment #818903 - Flags: review?(chulee)
Comment on attachment 818903 [details] [diff] [review]
Patch v1.1 for mc

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

Looks good, thank you!
Attachment #818903 - Flags: review?(chulee) → review+
https://hg.mozilla.org/mozilla-central/rev/cfdeb3ec5638
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Flags: needinfo?(skamat)
1.3+ seems to else be security breach.
blocking-b2g: 1.3? → 1.3+
Flags: in-moztrap?
Flags: in-moztrap? → in-moztrap+
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: