The Hotspot currently has a default SSID of FireFoxHotSpot and a default password of 1234567890. There is a risk that users will just enable the hotspot and not change the password, exposing them to data charges or attack from the network. It would be better if this password was randomized, or the user was forced to set the password on first use.

Paul is there someone that this can be assigned to that is working on the HotSpot feature?

Paul where does this need to go

I don't know who is working on the hotspot code but I will try to find out. Does anyone have any thoughts on how critical this is - it will only be worked on if we can't ship with out it. Its not great, but we could probably ship without it. But then again, its probably a minor change.

Actually just noticed that the tethering bug is not marked for basecamp, and unassigned, so I am leaving this until that changes.

Hey all, I made a patch to make the Hotspot password random. It uses the name of a fruit with 4 random digits appended to it. Example random passwords are: kiwi2819, banana1910, grape2617 I think this is important because most people will likely not change a preconfigured password like 1234567890 that we have now. Bug: https://bugzilla.mozilla.org/show_bug.cgi?id=797661 Patch: https://github.com/st3fan/gaia/commit/e54cb4f53806da00ac6608dac472e2586097fc0e Screenshot: http://dl.dropbox.com/u/2988854/Screenshots/Screen%20Shot%202013-01-17%20at%2011.50.16%20AM.png The fruit names can also be localized if we think that is better for non-english speaking markets. I don't have commit access so I am looking for someone who can help me to turn this into something acceptable for inclusion in a future release.

If you want that in v1, nominate as tef+ blocker and ask for review to someone that touched the relevant code in the gaia team.

"currant" and "raisin" are not fruits of themselves; they are dried grapes. Well done for including "tomato", though ;-) (No, this does not require a new patch ;-) Gerv

Nominated this as a tef blocker. If it gets past triage I will ask for review.

I'd vote against localizing the fruit names right now. I'd lobby for reducing the list to more commonly known fruit, though. 'goava, currant, pear, apricot' are among those that I'd probably find hard to remember how to spell. Feel free to add 'cherry' ;-)

While i like the idea, this is easy to brute force. Unless we consider that the hotspot password / wifi access via the hot spot does not need a real protection, I think we should consider something stronger.

Sure, just throw your suggestions in the fruitshed :-) I think the list should be a little longer though. I'm fine with removing 'difficult' names from it but I think the list should actually grow to like 30 items at least. My only concern is that we should pick names that are globally neutral, preferably not english specific, politically correct, 'decent', etc. This is why I picked fruit (like iOS does) but if anyone has better suggestions then please do. How about city names? paris, london, etc.

(In reply to Guillaume Destuynder [:kang] from comment #10) > While i like the idea, this is easy to brute force. > Unless we consider that the hotspot password / wifi access via the hot spot > does not need a real protection, I think we should consider something > stronger. I am not sure how practical a brute force attack is against a slow device like a phone. Specially since most people are likely on the move and dont have the feature turned on for very long. People are likely also not in the same physical location. If we are worried about brute force attacks then I would rather see us submit a patch to the wpa supplicant to deal with that. I think brute force attempts are easily blocked at that level. (Maybe it even already does that?)

The brute force attack does not rely on the speed of the phone - it does not attempt to authenticate. Instead you capture packets with another device (like a laptop) until you get the authentication handshake, and then you crack that.

Just to be sure its in the bug as well: as discussed on IRC, it might also be a good idea to have a random SSID as well to avoid attacks with precomputed keys. I'm not sure where the usability/security trade off should be like, albeit I imagine having something like FirefoxOS_19iejs2skU is just fine as the user dont' have to type it (and can change it if needed). The randomness of the random function probably also matters - but it might be all we have right now.

Not blocking but something worth considering for 19+.

Stefan: please attach a link pointing to your PR.

(In reply to Guillaume Destuynder [:kang] from comment #14) > Just to be sure its in the bug as well: as discussed on IRC, it might also > be a good idea to have a random SSID as well to avoid attacks with > precomputed keys. I'm not sure where the usability/security trade off should > be like, albeit I imagine having something like FirefoxOS_19iejs2skU is just > fine as the user dont' have to type it (and can change it if needed). > > The randomness of the random function probably also matters - but it might > be all we have right now. I think there are two risks here: 1. Someone can access your access point by knowing the default password 2. Someone can compromise a recorded session by bruteforcing your SSID/password To me, (1) is a very high risk, and one we should fix ASAP (and the point of this bug). As the code currently stands this patch addresses this opportunistic attack. For (2), this patch doesn't provide strong protection by default, but I think its appropriate for default. If someone wants strong protection, they can set a strong passsword, or that's my opinion anyways. Most people, in most situations, don't need this level of protection. <fruitshed> A list of ten fruits doesn't add much entropy. Wouldn't just adding a random few letters instead of a fruit add a lot more entropy and be easier to type? e.g. ditl3940 although, in writing this example, it strikes me that homoglyphs create a usability issue. (1 vs l e.g ditl3940 vs dit13940) So perhaps that is justification alone for fruits instead of random chars (although the font is better on the phone) Enough fruitshedding. </fruitshed>

(In reply to Lucas Adamski from comment #16) > Stefan: please attach a link pointing to your PR. Lukas, the pull request is here: https://github.com/mozilla-b2g/gaia/pull/7672

Stefan, If you create a file with that link in it, you can attach it (See "Add an attachment" above). Then you can set the r? flag to ask for a review from someone who is familiar with the settings app. (Kaze or Evelyn come to mind. Or you could ask me.)

As :djf suggested, a link to the pull request.

Comment on attachment 703955 [details] Link to pull request reassign to kaze because I'm supporting app days in Jakarta.

pauljt: I don't have user studies to back me up, but I bet "kiwi1463" is more memorable than either "1463" or "djlq1463". If the aim is only to solve problem 1, then we want the password to be memorable. Gerv

Comment on attachment 703955 [details] Link to pull request The idea of using non-localized fruit names as a default password being accepted already, r=me for the implementation. Please fix the linter errors and squash your commits before requesting an a?.

Fixed the linter errors and squashed the commits into one. New pull request is attached.

I've added some comments to the pull request, including the suggestion that we use major cities of the world instead of fruits because that allows us to have a longer list of familiar words, with fewer l10n issues.

(In reply to David Flanagan [:djf] from comment #26) > I've added some comments to the pull request, including the suggestion that > we use major cities of the world instead of fruits because that allows us to > have a longer list of familiar words, with fewer l10n issues. You must be reading my mind. I didn’t want to discuss this part because it’s already been accepted (including by our l10n guru) but I’d much prefer using l10n-neutral names, too.

(In reply to David Flanagan [:djf] from comment #26) > I've added some comments to the pull request, including the suggestion that > we use major cities of the world instead of fruits because that allows us to > have a longer list of familiar words, with fewer l10n issues. Yeah the city names are nice. I'll submit a modified patch.

(In reply to Fabien Cazenave [:kaze] from comment #27) > I didn’t want to discuss this part because it’s already been accepted > (including by our l10n guru) but I’d much prefer using l10n-neutral names, > too. Fabien, is it too late to submit a new patch? I can have one ready pretty quickly.

(In reply to David Flanagan [:djf] from comment #26) > I've added some comments to the pull request, David your question about defaulting to an open hotspot makes a lot of sense and I already addressed that in bug 831948 I strongly suggest that we use both this bug and 831948. Otherwise people will still use the insecure open default.

As long as Berlin is on the list, I'm cool. ;-)

New pull request added with the fruit names changed to common city names.

It seems a bit bad to me that we're generating a password which is complex enough that it looks safe to most people, but in reality is only safe against the most naive attacks. I would have preferred to generate a completely random 8 character password. It certainly means that people would have changed the password to something less secure, but I would imagine that even a poorly chosen password is more secure than the easy-to-brute-force password that we're now using.

Updated the pull request with fixed city names as :kaze suggested.

(In reply to Jonas Sicking (:sicking) from comment #34) > It seems a bit bad to me that we're generating a password which is complex > enough that it looks safe to most people, but in reality is only safe > against the most naive attacks. > > I would have preferred to generate a completely random 8 character password. > > It certainly means that people would have changed the password to something > less secure, but I would imagine that even a poorly chosen password is more > secure than the easy-to-brute-force password that we're now using. I second Jonas: false impression of security is worse that clear understandable unsecurity. Actually, I do not understand why we are setting a random password. Can't we just ask the user to set a password and force him/her to do so unless he/she insists (like having a checkbox saying "I really don't want to set a password"). Having an option to generate a password could be added. I've never used the hotspot feature so maybe the UI is already doing that.

(In reply to Fabien Cazenave [:kaze] from comment #27) > (In reply to David Flanagan [:djf] from comment #26) > > I've added some comments to the pull request, including the suggestion that > > we use major cities of the world instead of fruits because that allows us to > > have a longer list of familiar words, with fewer l10n issues. > > You must be reading my mind. > > I didn’t want to discuss this part because it’s already been accepted > (including by our l10n guru) but I’d much prefer using l10n-neutral names, > too. Nit: City names are not l10n-neutral (e.g. EN: Paris NL: Parijs) but I agree with David that they will probably lead to fewer l10n issues.

Jonas, Mounir: please r- this patch explicitely if you think it’s *not* a step forward. I’m nowhere near a security expert and Stefan works in the security team, so I don’t feel qualified to r- it. (In reply to Matthew N. [:MattN] from comment #37) > Nit: City names are not l10n-neutral (e.g. EN: Paris NL: Parijs) but I agree > with David that they will probably lead to fewer l10n issues. Ugh, I already commented on github to avoid a few names that I knew weren’t l10n-neutral (e.g. London, Beijing…) but I didn’t know for Paris.

Given the current implementation gives everyone the default password of "1234567890", this patch seems like an improvement regardless of whether we use fruit, cities, or purely random alphanumeric? That said this only provides around 360K combinations, which is not much entropy.

Comment on attachment 705097 [details] Link to pull request Talked out of band with Stefan, this patch goes with bug 831948 and while not perfect it is better than the default password so we're going to approve this to uplift to v1-train and v1.0.0 as well.

This has review plus approval for v1 and only has a few simple comments (2 on Github & comment 37) to address.

Recommended reading: http://lists.owasp.org/pipermail/owasp-mobile-security-project/2013-June/000640.html "Cracking iOS personal hotspots using a Scrabble crossword game word list"

I did a new pull request as it was easier for me to make the requested modifications like this (hope you won't mind Stefan): https://github.com/mozilla-b2g/gaia/pull/14611

Pass by review: This patch needs some unit tests to land. There's a lot of experts in the office if you need help.

Comment on attachment 8346532 [details] [diff] [review] hotspot_random_password.patch LGTM, please r? me again after adding unit tests. Thanks!

(In reply to Stéphanie Ouillon [:arroway] from comment #43) > I did a new pull request as it was easier for me to make the requested > modifications like this (hope you won't mind Stefan): FWIW, I much prefer reviewing PRs with several commits — assuming that you’ll squash all commits before merging. Besides that, we need a PR at some point to get Travis results anyway.

(In reply to Fabien Cazenave [:kaze] from comment #46) > (In reply to Stéphanie Ouillon [:arroway] from comment #43) > FWIW, I much prefer reviewing PRs with several commits — assuming that > you’ll squash all commits before merging. Besides that, we need a PR at some > point to get Travis results anyway. You don't need a new pull request to re-run on Travis. Pushing new commits or a changed history should trigger a new travis run. You can also manually request a re-run on some jobs or a full build when you are logged into Travis.

https://github.com/mozilla-b2g/gaia/pull/14611 I updated the PR with unit tests.

Comment on attachment 8349386 [details] [diff] [review] hotspot-random_password_v2.patch R=me for the rebased PR, with nits addressed.

Should be better now, I updated the PR.

Please fix the indentation before merging.

Merged on master: https://github.com/mozilla-b2g/gaia/commit/2be37d0b91deabc6f3efafbe01b1dec3e8643a67 Thanks Stéphanie!

This bug was partially uplifted. Uplifted 2be37d0b91deabc6f3efafbe01b1dec3e8643a67 to: v1.3: 01e9da49be2cc4bc134eeefc434740d572ec2246 v1.2: 724d36716bcbbc5cee1af18b94cc328c289d0ccc Commit 2be37d0b91deabc6f3efafbe01b1dec3e8643a67 didn't uplift to branch v1-train

Hey John, why was this uplifted? There is no koi+/1.3+ flag here? Is it because of tracking-b2g18+? My understanding is that this flag is now obsolete and should not be used to uplifts at all...

reverted v1.2: 8441587c3b352e052fee07665c21fd192540f19f I'm marking "wontfix" on all branches to make it clear that this should not be uplifted. I leave it in v1.3 for now, that's not my call.