Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

random stuff from the Big E #585

Closed
7 tasks done
Thorin-Oakenpants opened this issue Dec 10, 2018 · 14 comments
Closed
7 tasks done

random stuff from the Big E #585

Thorin-Oakenpants opened this issue Dec 10, 2018 · 14 comments

Comments

@Thorin-Oakenpants
Copy link
Contributor

Thorin-Oakenpants commented Dec 10, 2018

🔻 stuff 🌍 brought up

  • remove 0906, 0907, 0908, 0910 - see comment below
    • at this stage, nah .. see reply to comment below, but feel free to keep discussing it
  • 0911 remove network.auth.subresource-img-cross-origin-http-auth-allow
    • default false in FF59+ anyway - 88b747e
    • add to removed prefs scratchpad - f0fbfd3
  • Add network.auth.subresource-http-auth-allow - 5bd5f6b
    • add it to the troubleshooter - 5b0952f
  • 1022 does it even do what we expect it to do? remove?
  • 1700 - add a recommendation for TC - d5ece0f
  • 2002 add test page - 778dc89
  • 2302 add version tag - added in 32, enabled in 44 (still disabled in ESR60!) - 2373309

That's right, the three wise "men" now all have emoji's 👖 🌏 🐈

PS: I don't think I missed anything, but feel free to add anything else minor. We can use this as a miscellaneous checklist.


🐈 said

  • 0911 - remove? or replace with... 🐈: replace! (with user_pref("network.auth.subresource-http-auth-allow", 0); or 1? I'd go with 0, but IDK I'M NEW AT THIS SHIT!!! 🙀 *freaking out*).

I don't know what 🐈 is drinking, but I want some

@earthlng
Copy link
Contributor

earthlng commented Dec 10, 2018

re: 0911

network.auth.subresource-img-cross-origin-http-auth-allow only prevents IMAGES from triggering a login prompt. It was added in FF55 and is default false since FF59 = no need to enforce it anymore. We probably added it before it was set to false

pref("network.auth.subresource-http-auth-allow", 2); was added in FF41 and supports the following values:

// Sub-resources HTTP-authentication:
//   0 - don't allow sub-resources to open HTTP authentication credentials dialogs
//   1 - allow sub-resources to open HTTP authentication credentials dialogs,
//       but don't allow it for cross-origin sub-resources
//   2 - allow the cross-origin authentication as well.

AFAIK HTTP Auth is pretty rare nowadays and 0 should probably be fine. 1 = less breakage

@earthlng
Copy link
Contributor

with the version of when it was added or enabled?

@Thorin-Oakenpants
Copy link
Contributor Author

Thorin-Oakenpants commented Dec 10, 2018

Generally go with added, or in the case of a few I was checking the other day, it wasn't clear, so my second choice was when it was noted as "compatible" e.g on MDN or FF release notes (edit: tey were only 1 version diff to the bugzillas - and that may be due to being too close to stable releases, so got held back? who knows)

Case by case basis. Since this is an old pref, and the difference is so great, I'd go with compat - but lemme try a hybrid to see how it comes out in the real-time project. (see how it screws up the info column)

@Thorin-Oakenpants
Copy link
Contributor Author

Thorin-Oakenpants commented Dec 11, 2018

👍 if you want to do the other two. PR it

@Thorin-Oakenpants
Copy link
Contributor Author

/*** [SECTION 1700]: CONTAINERS
     [1] https://support.mozilla.org/kb/containers-experiment
     [2] https://wiki.mozilla.org/Security/Contextual_Identity_Project/Containers
     [3] https://github.com/mozilla/testpilot-containers
***/

That looks a bit outdated. You guys tell me. AFAIK you don't need the test pilot, and its not an experiment any more. If anything I would just include [2], add a line re TC and a couple of TC links

here's whats on the wiki page

  • Temporary Containers ✔ Privacy (stated on AMO) | GitHub
    • This can achieve almost everything First Party Isolation (FPI) does without breaking cross-domain logins. And (with or without FPI), in a hardened TC setup, this can even isolate repeat visits to the same domain, which FPI alone cannot.
    • Required reading: [1] AMO description [2] Article [3] TC's Wiki

Thorin-Oakenpants added a commit that referenced this issue Dec 11, 2018
it is default false in FF59+
Thorin-Oakenpants added a commit that referenced this issue Dec 13, 2018
@earthlng just squash & merge if you're happy
@earthlng
Copy link
Contributor

/* 0906: disable websites' autocomplete="off" [FF30+]
 * Don't let sites dictate use of saved logins and passwords. Increase security through
 * stronger password use. The trade-off is the convenience. Some sites should never be
 * saved (such as banking sites). Set at true, informed users can make their own choice. ***/
user_pref("signon.storeWhenAutocompleteOff", true); // [DEFAULT: true]
/* 0907: display warnings for logins on non-secure (non HTTPS) pages
 * [1] https://bugzilla.mozilla.org/1217156 ***/
user_pref("security.insecure_password.ui.enabled", true);
/* 0908: remove user & password info when attempting to fix an entered URL (i.e. 0802 is true)
 * e.g. //user:password@foo -> //user@(prefix)foo(suffix) NOT //user:password@(prefix)foo(suffix) ***/
user_pref("browser.fixup.hide_user_pass", true);
/* 0910: disable autofilling saved passwords on HTTP pages and show warning [FF52+]
 * [1] https://www.fxsitecompat.com/en-CA/docs/2017/insecure-login-forms-now-disable-autofill-show-warning-beneath-input-control/
 * [2] https://bugzilla.mozilla.org/buglist.cgi?bug_id=1217152,1319119 ***/
user_pref("signon.autofillForms.http", false);
user_pref("security.insecure_field_warning.contextual.enabled", true);
  • 0906 is default true ever since it was added in FF30
  • 0907 is default true since FF51
  • 0908 is default true since at least FF22. + it gets removed in FF65
  • 0910 same as default ever since they were added in FF52

can we remove them?

@Thorin-Oakenpants
Copy link
Contributor Author

Thorin-Oakenpants commented Dec 14, 2018

sometimes it's about enforcing a default vs bad advice via the internet - I'll have to think about them when i'm sober 🍺 but for sure, lets revisit them

@Thorin-Oakenpants
Copy link
Contributor Author

Thorin-Oakenpants commented Dec 16, 2018

here is a draft (it's really rough)

/* 0912: disable sub-resources HTTP-authentication [FF41+]
 * yada yada potentially malicious requests yada yada
 * [SETUP-WEB] HTTP Auth is pretty rare nowadays and 0 should be fine, 1 would be less breakage
 * 0=don't allow sub-resources to open HTTP authentication credentials dialogs
 * 1=allow sub-resources to open HTTP authentication credentials dialogs,
 *   but don't allow it for cross-origin sub-resources
 * 2=allow the cross-origin authentication as well (default) ***/
user_pref("network.auth.subresource-http-auth-allow", 0);

Link:

Add a secondary line: without something this looks like it's all about HTTP (and excludes HTTPS) and why it's necessary

Note: FF59 https://bugzilla.mozilla.org/show_bug.cgi?id=1357835 added

  • network.auth.subresource-img-cross-origin-http-auth-allow which is default false
  • not saying to add this, just pointing it out
  • mozilla telemetry must be saying something, but I'm happy for us to go with 0

OT: this showed up in one of my searches: https://addons.mozilla.org/en-US/firefox/addon/block-http-authentication/ - just FYI, that's all. It blocks videos & audio as well as images. I do not know if the FF59 new pref only does images.

That's it from me ... @earthlng

@earthlng
Copy link
Contributor

/* 09xx: limit or disable ...
 * ...
 * 0=don't allow sub-resources to open HTTP authentication credentials dialogs
 * 1=don't allow cross-origin sub-resources to open HTTP authentication credentials dialogs
 * 2=allow sub-resources to open HTTP authentication credentials dialogs (default)
 * [1] https://www.fxsitecompat.com/en-CA/docs/2015/http-auth-dialog-can-no-longer-be-triggered-by-cross-origin-resources/ ***/

I think value 1 should be good enough. Maybe add something that cross-origin images are covered in FF59+ regardless of this pref. (but that's also mentioned in [1] so IDK).
And maybe something like "Hardening against potential credentials phishing" as description.

Since this should not create that much breakage, if any, especially with value 1, I don't think it needs a [SETUP-WEB] tag. If we add these tags everywhere they kind of lose their usefulness

@Thorin-Oakenpants
Copy link
Contributor Author

@earthlng don't forget to add network.auth.subresource-http-auth-allow to the troubleshooter

@Thorin-Oakenpants
Copy link
Contributor Author

Thorin-Oakenpants commented Dec 17, 2018

re: remove 0906, 0907, 0908, 0910 .. IDK, see comment re enforcing defaults against the stupidity of the internet, and passwords are pretty important (vs say a tracking cookie)

not calling PK stupid here (from previous general statement), but at least one of these differs from his settings - I think its 0906 and a case can be made for either value. So that one (I'll check what he has) would be good to keep

0908, sure, its goes next release, so no big deal


update: I'd rather we didn't lose anything
I realize PKs js is pretty much unmaintained in a lot of respects - all four are active in PKs at same values except 0906 which he has at false and we're at true. It doesn't really matter for diffs, except 0906 would be neat to hang onto

  • 0906 probably needs to stay, primarily because PK sets it false (I can kinda see his side, i.e if a site, e.g. your bank, sets this, then so should your browser, but then my point, is fuck websites, control your goddamn browser and use passwords, nasty long AF unique ones and help yourself on that front my using saved passwords - so I think PK is wrong, just saying)
  • 0908 can move to deprecated next release. That way we still have a history of it in deprecated sticky, in the user.js etc and for diffs vs other user.js. I hate to lose info if possible. Size/length of the js doesn't matter, as long as it de-clutters the actual current prefs, and that will happen next release.
  • 0907 + 0910 I still think we should leave them in - it's about enforcing defaults to protect end users. e.g users getting annoyed by warnings for http logins and http not saving signon info, so they ask in forums, etc. FF51/52 were Jan/Mar 2017 .. so coming up on two years, and http->https has picked up

Anyone else want to weigh in .. 🐈 Class, discuss!

Thorin-Oakenpants added a commit that referenced this issue Dec 17, 2018
- although we *may* remove some of these prefs (but I doubt it, but persuade me in the open issue).
- I also don't want to pollute the js with tags on every fucking line. You started adding these for your diffs, right?. I'm not sure exactly how adding them to some prefs but not others helps you?
- that said, it is a nice way to show that we're *enforcing* something and does help end users, as well as the shiny overdactyl project
@claustromaniac
Copy link
Contributor

I'd keep 0906, 0907 and 0910, but I see little gain in keeping them anyway.

@earthlng
Copy link
Contributor

Ok 2:1 then. It's nice to have a 3rd person now as a tie breaker :)
I reopened the PR with the default tags in case you want to commit that.

@Thorin-Oakenpants
Copy link
Contributor Author

Don't believe his lies .. he hedged his bets. Also, just quietly, he'd do anything for a treat (or so I'm told)

@Thorin-Oakenpants Thorin-Oakenpants mentioned this issue May 3, 2019
17 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

No branches or pull requests

3 participants