arkenfox / user.js

Firefox privacy, security and anti-tracking: a comprehensive user.js template for configuration and hardening
MIT License
9.98k stars 511 forks source link

security.mixed_content.upgrade_display_content (round 2) #754

Closed claustromaniac closed 5 years ago

claustromaniac commented 5 years ago

TL;DR: I'm proposing to add the pref in the title, in addition to the 3 prefs that we have for controlling mixed content:

/** MIXED CONTENT ***/
/* 1240: disable insecure active content on https pages
 * [1] https://trac.torproject.org/projects/tor/ticket/21323 ***/
user_pref("security.mixed_content.block_active_content", true); // [DEFAULT: true]
/* 1241: disable insecure passive content (such as images) on https pages [SETUP-WEB] ***/
user_pref("security.mixed_content.block_display_content", true);
/* 1243: block unencrypted requests from Flash on encrypted pages to mitigate MitM attacks [FF59+]
 * [1] https://bugzilla.mozilla.org/1190623 ***/
user_pref("security.mixed_content.block_object_subrequest", true);

For these STR, I will refer to 1240 as blockActive, 1241 as blockDisplay, 1243 as blockPlugin, and security.mixed_content.upgrade_display_content as upgradeDisplay.

  1. Go to about:config

  2. Set blockActive, blockDisplay, and blockPlugin to false

  3. In a new tab, go to https://www.bennish.net/mixed-content.html (which would be a nice addition to the wiki BTW)

  4. Without leaving that tab, go to devtools, network tab (Ctrl+Shift+E) and reload the page. You should see insecure requests being made (hover over the padlocks in the list of requests for more info)

  5. Open the scratchpad, paste the following code and run it:

    (() => {
    const ocat = document.createElement('img');
    ocat.setAttribute('src', 'http://github.githubassets.com/images/spinners/octocat-spinner-128.gif');
    ocat.textContent = '[octocat spinner]';
    document.body.appendChild(ocat);
    })();

    The code simply injects an <img> HTML element that triggers a request to github.githubassets.com over HTTP. The octocat gif should appear at the very bottom of the page.

  6. check the devtools to confirm the octocat image was loaded over http

  7. set upgradeDisplay to true, reload the page and re-run the scratchpad script

  8. check the devtools, notice the green padlock for that request. You can also click the request in the list to confirm (on the right-hand panel) the target url is https://github.githubassets... Also, notice how upgradeDisplay prevented the other images in that site from loading, because they can't be upgraded to HTTPS. (conclusion 1: when the content can't be loaded over HTTPS, upgradeDisplay does not load it over HTTP)

  9. set blockDisplay to true and reload, re-run script

  10. same result as in step 8 (conclusion 2: upgradeDisplay has higher priority than blockDisplay, because the octocat gif was loaded without a hitch over HTTPS)

  11. test the remaining different combinations of prefs (with blockActive and blockPlugin) to confirm they don't conflict with each other (conclusion 3: there don't seem to be any issues with the other prefs)

  12. give me some catnip


Correct me if I'm wrong, but it seems the rationale for not adding this pref after the discussion in #367 was that:

  1. we can live without mixed display content
  2. Mozilla devs must have some reason for leaving it disabled by default

My counter-arguments:

  1. we can live without beer and catnip too, yet ... you know
  2. maybe they just don't want to break stuff by default? would make sense considering they went as far as to give us 2 different prefs for controlling mixed active content (super risky) and mixed display content (not as risky) separately. Presumably, that's also the reason blockDisplay is false by default. There might be other reasons, but that sounds like the most likely explanation to me.

I think this at least deserves some more consideration, and maybe investigating some more. I didn't start digging the source code yet (time constraints as always), but I think we can at the very least add this inactive as a FYI.

Anyway, I know :jeans: wants as few issues open as possible, so feel free to close this whenever you want.

Thorin-Oakenpants commented 5 years ago

I'll let @earthlng respond to this when he returns, hopefully, in about a week - apparently he is on some dark ops thing in Russia on holiday getting wrecked before the Zompocalypse

Edit: Actually .. since he's AWOL for a while ... we could go mad and make 100s more changes :grinning:

ghost commented 5 years ago

Oh! It's nice that you're talking about this pref, I was wondering what to do with it since the ghacks' article!

Okay, so, here are my two cents: Based on your STR, and some testing of my own, upgradeDisplay is really just blockDisplay with an additional check over HTTPS before blocking. So it reduces breakage, but ensures the same standard of security, thus entirely replacing blockDisplay if my understanding is correct, especially since upgrade has priority over block. In my opinion, the reason upgradeDisplay is disabled by default is the same as blockDisplay's: breakage. I don't think it's because it's an unfinished product like it was suggested, but rather because the majority of users hates breakage. Therefore, as long as Chrome won't enforce strict HTTPS content, website won't move, and Firefox won't either, and both upgradeDisplay and blockDisplay will remain disabled by default to keep most users happy.

TL;DR:

  1. upgradeDisplay is blockDisplay with less breakage but same security.
  2. upgradeDisplay is not disabled because it's experimental but because blockDisplay is disabled too.
  3. upgradeDisplay replaces blockDisplay in Firefox and IMO should do so too in this repo.
ghost commented 5 years ago

I got a notification that you replied but it seems you deleted your post. If it can help, though, here's my answer to it: I used those terms to conform with OP's terminology. Also, active content is not discussed at all in this issue, the upgrading pref only applies to passive content, so my previous post only refers to that. :) I only differentiate between upgrading passive content and outright blocking it, I hope this makes it a bit clearer. :)

Thorin-Oakenpants commented 5 years ago

I'm so deep in other things right now, that I didn't do due diligence in reading OP, and only after downing six coffees and going for a quick streak around the house in a brisk nor'westerly, was I able to comprehend that there is a fourth pref involved ... might be time to get smashed on some beers

earthlng commented 5 years ago

Hey guys, I'm back! Had some issues with my GH account but everything's okay again (for now?).

Anyway, I studied the test results posted here to better understand how this pref works in combination with the other 2 prefs. Here we go ...

There are 8 tests and I'll refer to them as test1 to test8, from top to bottom. FYI tests 1-4 have upgradeDisplay set to true and tests 5-8 have it set to false.


Conclusions:

if upgradeDisplay is not in the picture (ie set to false), we can see from tests 6 + 7 that

  1. image, media and imageLeavePicture are considered "passive" content
  2. script, xhr, iframe, object, stylesheet as well as imageSrcset, imageSrcsetFallback, imagePicture and imageJoinPicture are treated as "active" content

BUT with upgradeDisplay=true (tests 1-4) things get a bit weird and the line between what's considered active and passive content gets blurred!

Tests 2 + 4 produced the same results, meaning if upgradeDisplay=true and blockActive=false (!!) then the value of blockDisplay has no effect on the outcome ... ... but since we're definitely not gonna allow mixed active content, we can ignore that.

So, that leaves us with tests 1 + 3 ...

what these show is that the value of blockDisplay matters a lot:

If blockDisplay is also set to true then only script, xhr, iframe, object and stylesheet are considered "active" content.

if we want to enable upgradeDisplay then we should probably set blockDisplay to false as well so that imageSrcset, imageSrcsetFallback, imagePicture and imageLeavePicture are still treated as "active" content and therefore blocked instead of attempted to upgrade.

I mean, IDK exactly why these 4 are considered "active" content if upgradeDisplay is out of the picture but there's probably a good reason for that, security-wise I assume.

People who are willing to perhaps sacrifice some security for more convenience can set blockDisplay to true and have more elements potentially upgraded instead of outright blocked. It seems counter-intuitive that you'd have to set a "block" pref to true for less breakage but that's apparently how this works.

any questions? :)

ghost commented 5 years ago

👖: The amount of time you spend drinking beer simply defies everything that modern medecine has ever said about liver damage due to alcohol. Congrats, you have transcended human weakness and evolved into a new, improved species which doesn't require a liver to survive.

Big E: Thanks a lot for the synthesis, helps making things clearer! I only have one question: why do we want to block upgraded "active" content? As I understood, insecure active content is not blocked because it's active, but because it's insecure, the content isn't the actual problem, its plaintext nature is. Therefore, if upgradeDisplay has the ability to do more than we originally thought and can upgrade both passive and some active content, wouldn't it reduce breakage while still achieving the same goal of blocking insecure content? In the end, if it's upgraded, it's no longer something we want to block since it became encrypted, and if the upgrade fails, we block it anyway, ensuring the same level of security. This is all according to my current understanding of this issue, please feel free to correct me.

claustromaniac commented 5 years ago

What @Aeriem said. If we take into account that content that can't be upgraded does not end up loading insecurely, it shouldn't matter if some more stuff is now considered passive content. In fact, it should be better for what upgradeDisplay is trying to achieve.

TBH, I'm not even sure why the pref is meant to upgrade only passive content in the first place... Maybe just in case some module crashes and upgradeDisplay stops working temporarily?

claustromaniac commented 5 years ago

Well, it is an experiment. I guess it's just a protocolar thing. They apparently intend to implement a similar pref for upgrading 1st-party active content too, judging from their comments on that bug.

@earthlng, judging from this comment, I believe what is unexpected to them is that upgradeDisplay does not fully take precedence over blockDisplay when blockDisplay=false, blockActive=true, and upgradeDisplay=true

Upgrade take precedence over mixedDisplay. For some reason when mixedDisplay=false and mixedActive=true it takes precedence over upgradeDisplay. I didn't dig into why as I didn't touch that code but I don't think it will impact us much/at all.

We should leave blockDisplay as true IMO.

earthlng commented 5 years ago

why do we want to block upgraded "active" content?

if it's upgraded, it's no longer something we want to block ... ensuring the same level of security.

I'm not sure. I mean, why did they exclude scripts, xhr, etc from being attempted to upgrade? There must be a reason for that, right? Maybe the "active" ones they do attempt to upgrade are (slightly?) less risky, or maybe it was an oversight (although they seemed to be happy with the test results so that's probably not it). Or maybe they weren't quite sure either. IDK

All I know is that there's a discrepancy between what's considered active and passive content with and w/o upgradeDisplay. And I just thought that, if we're going to enable an experimental feature like this, that we should probably go with the conservative, perhaps less risky option.

FP-ing-wise it's probably not that difficult to detect any combination of these prefs but because we're already diverging from the default FF config by blocking mixed passive content that doesn't really matter. And since we already block mixed passive content we might as well make our default template a bit more convenient for our users by adding this pref (but in the way that keeps the old/original categories of active and passive content that they'd have w/o this pref)

If we end up not adding it at all then at least we learned something and I'll just send the catman a bill for ~2 hours of my time :)

claustromaniac commented 5 years ago

Quoting myself:

@earthlng, judging from this comment, I believe what is unexpected to them is that upgradeDisplay does not fully take precedence over blockDisplay when blockDisplay=false, blockActive=true, and upgradeDisplay=true

Lemme fix that. What I meant to say is that I believe what was unexpected to them was that upgradeDisplay did not take precedence over the parts of blockActive it was meant to when blockDisplay=false + blockActive=true.

claustromaniac commented 5 years ago

Really, reverse engineering these programmers' code is almost easier than reverse engineering their language... I guess that's kind of a hypocritical thing for me to say, but still...

claustromaniac commented 5 years ago

As for why they didn't try to upgrade all mixed content instead, they seem to be going at it stage-by-stage in experiments... They repeatedly refer to apparent plans to eventually upgrade "FP active" too (I guess that's 1st-party active mixed content). If they're just testing then.. mystery solved! No reason to fear upgrading those few things in blockActive that upradeDisplay tries to upgrade.

claustromaniac commented 5 years ago

It is also worth mentioning that they refer to these things as experiments not because they're unstable, but because they mean to gather telemetry out of them. Fully testing them takes a lot of time and manpower, so that's definitely a factor, but what I mean is that they'll continue to refer to these things as experiments even after they're battle-tested and proven to work reliably, because they're not talking only about behavior when they use that term. So, I think we shouldn't base our decisions too much on that, because it can take them forever to stop calling those things experiments (and may never stop doing that, depending on what they end up doing after they gather the data they want). I mean, even RFP is still considered an experiment by them.

ghost commented 5 years ago

@claustromaniac summed up my thoughts on this matter, especially the experiment part. I'll let you three take the final decision. Thanks for the infos you brought!

Thorin-Oakenpants commented 5 years ago

Let me try to simplify this as a step in how to add it to the user.js (if we do)

:small_orange_diamond: first two outcomes we can ignore, since the new pref remains inactive = status quo in terms of changes to outcomes etc

outcome1 (e's test 5): + new pref commented out

outcome 2 (e's test 7): + new pref commented out

:small_orange_diamond: these two outcomes we need to explain the consequences of

outcome 3 (e's test 1): + new pref not commented out

outcome 4 (e's test 3):


I don't see any downsides to adding the new pref, not commented out, and setting it as true: and we don't even need to explain that it's less effective in some configs. Both outcomes (3+4) potentially unbreak shit

/* 1242: enable upgrading some passive content (see 1241) to secure [FF60+]
 * do we need something here
 * [1] https://bugzilla.mozilla.org/1435733 ***/
user_pref("security.mixed_content.upgrade_display_content", true);

Class! Discuss!

ghost commented 5 years ago

It's a yes from me!

claustromaniac commented 5 years ago

I don't see any downsides to adding the new pref, not commented out, and setting it as true

Neither do I.

I understand @earthlng's reasoning that if we're going to make assumptions, we might as well choose the assumptions that lead us to the choices that are least likely to be risky in the end. You are quite the wise guy, 🌎.

But! all things considered, I think my (and @Aeriem's) conclusions are based on the simplest assumptions, and as such are the simplest explanations. If that is not enough, we can always try to get a direct answer from them Mozillian dudes to give 🌎 some more peace of mind.

Something like this should suffice IMO. Maybe add this issue's URL too, IDK.

/* 1242: upgrade passive mixed content to https when possible [FF60+]
 * [1] https://bugzilla.mozilla.org/1435733 ***/
user_pref("security.mixed_content.upgrade_display_content", true);

Class! Discuss!

IDK if you realize how funny it is that you're only saying that now...

claustromaniac commented 5 years ago

We should do something more because 1241 has a [SETUP-WEB] tag: if someone overrides that but not upgradeDisplay, they will get more breakage, which is the opposite of what would be expected... 🤔

Thorin-Oakenpants commented 5 years ago

godamnit

IS that right? So this pref should be paired with 1241 and the instruction to always have them the same (which is what I initially thought before I tried to actually work it out)

reworded 1240 and added a new link so users can learn about active vs passive

/* 1240: disable insecure "active/script" content on https pages
 * [1] https://blog.mozilla.org/tanvi/2013/04/10/mixed-content-blocking-enabled-in-firefox-23/
 * [2] https://trac.torproject.org/projects/tor/ticket/21323 ***/
user_pref("security.mixed_content.block_active_content", true); // [DEFAULT: true]
/* 1241: disable insecure "passive/display" content on https pages but attempt to upgrade
 * [SETUP-WEB] for best results, always set these two prefs the same
 * [1] https://bugzilla.mozilla.org/1435733 ***/
user_pref("security.mixed_content.block_display_content", true);
user_pref("security.mixed_content.upgrade_display_content", true); // [FF60]

Class! Discuss again!

ghost commented 5 years ago
/* 1240: disable insecure "active/script" content on https pages
 * [1] https://blog.mozilla.org/tanvi/2013/04/10/mixed-content-blocking-enabled-in-firefox-23/
 * [2] https://trac.torproject.org/projects/tor/ticket/21323 ***/
user_pref("security.mixed_content.block_active_content", true); // [DEFAULT: true]
/* 1241: disable insecure "passive/display" content on https pages but attempt to upgrade beforehand to reduce breakage
 * [SETUP-WEB] For best results, always set these two prefs the same
 * [1] https://bugzilla.mozilla.org/1435733
 * [2] https://github.com/ghacksuserjs/ghacks-user.js/issues/754 ***/
user_pref("security.mixed_content.block_display_content", true);
user_pref("security.mixed_content.upgrade_display_content", true); // [FF60]

I added a link to this issue, and lengthened the title a bit.

Thorin-Oakenpants commented 5 years ago

you can use add js or css etc to the end of your code starting bit: just edit your last post to see what I mean. I don't want to add links to issues. Issues are messy long discussions not suitable for purpose. If it's that important to someone, they can search the repo.

crssi commented 5 years ago

^^ Typo https://github.com/ghacksuserjs/ghacks-user.js/issues/7544 should be https://github.com/ghacksuserjs/ghacks-user.js/issues/754

ghost commented 5 years ago

Ah, thanks :jeans:! I was wondering what was wrong.

I get what you mean, though I think they should have something explaining why enabling or disabling both is important. I tried fitting the explanation in the the header of the prefs but it's just too long, thus, I resorted to linking this issue as it is pretty well documented.

earthlng commented 5 years ago

I don't see any downsides to adding the new pref, not commented out, and setting it as true

Neither do I.

it will cause a bunch of additional requests that 1) might not result in anything because the resources simply aren't available over https and 2) most FF users don't send, thus making us stand out more.

If we're going with the double-true option then we'll go beyond what even the FF default config does in regards to those 4 "active" imagesrc resources. Regardless of the potential risks, I think that trying to upgrade those as well is totally unnecessary simply because most website admins will do at least some testing with FF to make sure their pages work on vanilla Firefoxes and they'd notice that their imagesrc's would be blocked in a mixed-content situation.

Another thing to remember is that when they tested this pref for a while in nightly, they only enabled the upgrade pref but not the blockDisplay one which means that perhaps nobody has ever given a single thought about the potential benefits or risks of using both together. For example, I know that there are some kind of mime-type protections to make sure that attackers can't sneak in active content through supposed-to-be passive elements like images etc. Those same protections might not cover some or all of these imagesrc elements, IDK. Double-true means we're entering totally uncharted waters!

I might be way too cautious here but yeah, that's why you're paying me the big bucks, right?! (NOT! :)

[SETUP-WEB] For best results, always set these two prefs the same

Best results in what regards? Talk about dumbing it down ... :) I'd argue that it's best to either use none of these 2, or use one but not the other.

just my 2 cents; do whatever you want. 👍

claustromaniac commented 5 years ago

it will cause a bunch of additional requests that 1) might not result in anything because the resources simply aren't available over https and 2) most FF users don't send, thus making us stand out more.

  1. a bunch of additional requests? is mixed content that common?
  2. We are already different than common Firefox users, than Tor Browser users, and than users of every other Firefox fork out there. If I want, I can figure out if someone is using ghacks-user.js specifically, and I'm not even a web dev.

Regardless of the potential risks, I think that trying to upgrade those as well is totally unnecessary simply because most website admins will do at least some testing with FF to make sure their pages work on vanilla Firefoxes and they'd notice that their imagesrc's would be blocked in a mixed-content situation.

If they all did, there would be no such mixed content in the internet. You're contradicting yourself. You just said we would be making a bunch of additional requests.

I know that there are some kind of mime-type protections to make sure that attackers can't sneak in active content through supposed-to-be passive elements like images etc. Those same protections might not cover some or all of these imagesrc elements, IDK.

That is simply irrelevant. As previously stated, what makes mixed content dangerous is that it is served insecurely, not the mime-type. Some types are more dangerous than others, but if you upgrade the requests all of that becomes irrelevant, because they're then no longer any different than any other secure request for such content. Whatever riks they may or may not carry after upgrading have nothing to do with their would-have-been status of "mixed content".

My 2 cents.

Thorin-Oakenpants commented 5 years ago

My 2 cents is to not even add it, because people should just block all mixed content

claustromaniac commented 5 years ago

My 2 cents is to not even add it, because people should just block all mixed content

Mixed content stops being mixed content as soon as it is upgraded...

claustromaniac commented 5 years ago

Look, if you're more comfortable ignoring this pref, go ahead and close this issue. You don't need to turn every single stone looking for any more arguments for doing that. I said it in the OP: feel free to close this issue whenever you want

I just proposed something that is clearly an improvement from my perspective (and apparently I'm not alone on that). That's all.

I'll leave it up to you. Thanks.

claustromaniac commented 5 years ago

Just FYI, regardless of what you end up deciding to do with this pref here, HTTPS Everywhere is (likely) going to flip this to true when 1442990 gets resolved, and I'm going to do the same in HTTPZ.

earthlng commented 5 years ago

it will cause a bunch of additional requests that 1) might not result in anything because the resources simply aren't available over https and 2) most FF users don't send, thus making us stand out more.

  1. a bunch of additional requests? is mixed content that common?

your guess is as good as mine. Maybe Pants can find some numbers for us in the mozilla telemetry data? It must be common enough for Google and Mozilla not wanting to block passive MC by default.

  1. We are already different than common Firefox users, than Tor Browser users, and than users of every other Firefox fork out there. If I want, I can figure out if someone is using ghacks-user.js specifically, and I'm not even a web dev.

oh definitely. But somebody would have to actively look for that whereas with this you'll be sending requests to servers that could be unexpected to them and will show up in their error logs (if a resource is available over http but not https). See what I mean? Not that I think this is particularly bad or anything but IMO it should be something that our users can opt-in instead of just making it the template default. Lets look at it this way ... we have 2 kinds of users here: those who override (and therefore allow) our mixed passive pref and those who don't. IDK if the former would find this new pref behavior acceptable or beneficial because it will likely result in more breakage for them. I agree that it is an upgrade for the latter if they don't mind the additional probing requests that may or may not result in anything.

You're contradicting yourself

No I'm not! In the part that you quoted I was talking about the 4 active resources like imagesrcset etc. Those are treated as active content for the vast majority of FF users and blocked by default in a mixed content situation. These 4 types are AFAIK also not something that's commonly available for user-contributed content but has to be specified/implemented by the webdevs themselves. So most if not all of the mixed content out there are regular image and media elements, not imagesrcset and the likes. But because browsers don't block passive mixed content image and media elements by default, even some devs might not notice/care that they're using an http url for their regular images/media. However, any semi-competent webdev should notice if they fuck up the imagesrcset etc parts of their sites.

That is simply irrelevant. ...

Yeah I think you're right. My thinking was that the server serving the http content could be a different one than the one serving the https content and one might be hacked etc. But yeah the risk of that happening is probably very slim to non-existent. My bad. I wasn't trying to "turn every single stone" to badmouth your proposal though! just trying to think of all the potential pros and cons.

Thorin-Oakenpants commented 5 years ago

OK, let's try this again ... the first question is: do we add upgradeDisplay to the user.js

🐈 said

Mixed content stops being mixed content as soon as it is upgraded...

Good point, my bad. The only reason to add it, is so that less of the web breaks: we block all active and passive by default, and that will not change (yes, it's a template: so users could change it).

I do not have any data (maybe there's something in Moz's telemetry I could look at: secure page loads that have insecure passive content?), but logic would suggest that this is becoming less of an issue, as the web moves towards deprecating insecure shit. Adding it, only creates complexity that we can't easily and succinctly express in the user.js.

From what I can gather, from E's comments, is that (and I stand corrected, I think) we wouldn't want upgradeDisplay and blockDisplay the same, so the pref would only be added as inactive (as true: i.e you enable the pref to change the behavior from default false)

double true:

Regardless of the potential risks, I think that trying to upgrade those as well is totally unnecessary simply because most website admins will do at least some testing with FF to make sure their pages work on vanilla Firefoxes and they'd notice that their imagesrc's would be blocked in a mixed-content situation

when they tested this pref for a while in nightly, they only enabled the upgrade pref but not the blockDisplay one which means that perhaps nobody has ever given a single thought about the potential benefits or risks of using both together.

I think the double-true might un-break the most, but I'm on E's side here that's it's uncharted

So if added it would be inactive and at true: meaning if someone flipped it thinking it would un-break shit, we put them at risk, unless we explain it all - which is hard to do.


earthlng said

it will cause a bunch of additional requests that 1) might not result in anything because the resources simply aren't available over https and 2) most FF users don't send, thus making us stand out more.

🐈 said

We are already different than common Firefox users, than Tor Browser users, and than users of every other Firefox fork out there. If I want, I can figure out if someone is using ghacks-user.js specifically, and I'm not even a web dev

but it's passive FPing - different kettle of fish .. nice fishy 🐠


I think we should just ignore this pref and not add it. I'll have a look at Moz Telemetry, but even if it shows the percentage of page loads with insecure passive: it doesn't give us any real insight to what could be upgraded: IDK, I've have a dig

Thorin-Oakenpants commented 5 years ago

So nightly shows that of secure pages

Dev shows, same period

So I think we can say that any benefit of this pref being used, even correctly, at most helps about 1.5% of the time. I don't think this is worth it: the possible risks as E points out, and confusing end-users (fuck, even we have a hard time working it out, not to mention the Moz devs). It's too complicated for little to no gain.

Pics: 70nightly

69dev

crssi commented 5 years ago

From my observation... When security.mixed_content.upgrade_display_content = true then security.mixed_content.block_display_content doesn't matter anymore, because the security.mixed_content.upgrade_display_content = true upgrades non-secure to secure and non-secure never happens (it doesn't fail-back to non-secure), so it never gets to security.mixed_content.block_display_content.

Thorin-Oakenpants commented 5 years ago

^^ I don't think that's what E said or his tests showed. I'll have to re-read it yet again, and I hate this issue, TBH

Edit: unless I'm missing something from E's tests from https://github.com/ghacksuserjs/ghacks-user.js/issues/754#issuecomment-508033303 , tests 1 to 4 have upgradeDisplay as true. All four tests show differences, so therefore security.mixed_content.block_display_content does matter and affects the result

crssi commented 5 years ago

Oh... I see, you are right. I was paying the attention only to image type of a request. My bad. Big E was thorough and complete as always 👍 😄 .

NOTE: image is display content and not active content. We already block insecure active content.

claustromaniac commented 5 years ago

what E said or his tests showed

@earthlng didn't test anything as far as I know. According to himself, he just re-posted in his own words the results of tests made by someone else at bugzilla.

Most third-party content these days is served by CDNs, and we know CDNs do support HTTPS (i.e the passive FPing risk pointed out by earthlng would very rarely exist, even within that theoretical 1.5%).

Moreover, telemetry from users that don't use extensions like HTTPS Everywhere, Smart HTTPS (which I don't like, but we can't deny its popularity) or HTTPZ (i.e. most Firefox users) does not mean much to us, because we do use such extensions. I can assure you the numbers would be way higher for us, because users that don't use such extensions load many sites more over HTTP, and insecure resources loaded from insecure top-level documents are not considered mixed content: those are just old-school insecure content (i.e. irrelevant).

Oh well... I'll just have to wait for 1442990 to get solved then.

Thorin-Oakenpants commented 5 years ago

he just re-posted in his own words the results of tests made by someone else at bugzilla

Damn. I didn't pick up on that. And here I was thinking he did a heap of testing, all scientific like.

Thorin-Oakenpants commented 5 years ago

even within that theoretical 1.5%

Well, I didn't quantify it "precisely", but it's about the only data we have. That is on all traffic. If we did a quick dirty hack, and said that 80% of pages are encrypted, we would take 20% off the first column, and thus the percentages of the other three columns would go up. It's still not significant IMO. So 1.5% would become more like 2%, at most.

claustromaniac commented 5 years ago

Well, I didn't quantify it "precisely", but it's about the only data we have. That is on all traffic. If we did a quick dirty hack, and said that 80% of pages are encrypted, we would take 20% off the first column, and thus the percentages of the other three columns would go up. It's still not significant IMO. So 1.5% would become more like 2%, at most.

Wrong, because even if that's on all traffic, insecure requests triggered from insecure sites don't qualify as mixed content to Mozilla (and they shouldn't, because they aren't). If you use extensions that upgrade requests, you see a whole lot more of mixed content. I'm sorry, but that telemetry is simply not useful to us.

claustromaniac commented 5 years ago

Lemme give this another go in case I'm still not being clear:

Even if that's on all traffic, we know 80% of that is not encrypted (what would be the point of using extensions for that if that was the case?) so if 80% of that were encrypted, that 1.5% would rise A LOT, because many insecure requests that previously didn't qualify as mixed content would now qualify.

Thorin-Oakenpants commented 5 years ago

OK, I see. But WTF are insecure sites doing pulling in secure content: I see cases of hotlinking, embedding: e.g tweets, youtube videos. images and so on: but a lot of that would already be secure IMO. So sure, that figure could be higher.

The fact that the page is insecure means you have already leaked your exact URL: so whether or not you connect to 3rd party content on that page or not securely, is almost immaterial, IMO.

Insecure 1st party have no say in this discussion AFAIC

PS: I wasn't even aware that this upgradeDisplay pref even applies to insecure 1st party

Thorin-Oakenpants commented 5 years ago

Lemme give this another go in case I'm still not being clear:

Even if that's on all traffic, we know 80% of that is not encrypted (what would be the point of using extensions for that if that was the case?) so if 80% of that were encrypted, that 1.5% would rise A LOT, because many insecure requests that previously didn't qualify as mixed content would now qualify.

You must have posted that as I was typing up my last one, and I didn't see it. I think you did a typo .. "20% of that is not encrypted" is the approx figure. These extensions are becoming less relevant, or rather less used for upgrading, as the web migrates to removing insecure. More and more servers auto-upgrade (e.g all the old links on web pages, or bookmarks, pointing at http), and so on. I don't know how effective they are: all I can do is logically follow that as more and more of the web changes to secure etc, the less they are needed. And those sites that migrated to HTTPS probably migrated the rest: i.e their content, like cdn.images or amazon.images or whatever. Certainly in the Alexa top 10,000 or whatever.

But regardless: the data given is a real world large sample: some of those people have extensions, some don't. Some are using old http links, some aren't. I don't see how you can say that the figure is a LOT higher: I just don't see it. What am I missing here?

claustromaniac commented 5 years ago

The fact that the page is insecure means you have already leaked your exact URL: so whether or not you connect to 3rd party content on that page or not securely, is almost immaterial, IMO.

I agree, for the most part, but even in those cases HTTPS is (should be?) useful for authentication at the very least.

Insecure 1st party have no say in this discussion AFAIC

I disagree on this, because of extensions and tools that provide opportunistic encryption. For example, HTTPZ will redirect a top-level document request to HTTPS if possible, but it is up to the server to upgrade sub-requests to HTTPS (even 1st-party ones), because I didn't want to mess with CSP directives (for the time being). If servers don't upgrade them, the browser considers them mixed content and either blocks them or loads them insecurely depending on the users' settings.

claustromaniac commented 5 years ago

I think you did a typo .. "20% of that is not encrypted" is the approx figure.

I meant to say that we know not as many as 80% of all requests are encrypted. Last time I checked the numbers were around the 60% or so. Even if 80% or more of all requests are encrypted in reality, that mostly speaks of popular sites, where encryption is pretty much a given. Those figures say nothing about what % of sites support HTTPS but do not redirect to HTTPS by default.

Extensions are particularly useful when you browse sites that aren't as popular as, say... Google, or Facebook. Sites that, incidentally, I don't use, and many other privacy-conscious users don't use. In other words: users like us are not represented in those figures, so they don't mean much to us.

edited for clarity

Thorin-Oakenpants commented 5 years ago

Last time I checked the numbers were around the 60% or so

Oh, I thought it was up to 80 now. Lemme check

Thorin-Oakenpants commented 5 years ago

Here we go: 1st party pages HTTPS

Thorin-Oakenpants commented 5 years ago

FYI: i haven't read it exactly: way too busy

I'll reopen this because I want to learn more about it: so this is my ticket: and I am always open to changing my mind about shit. Cats do that all the time: you know, refuse to eat a something cuz cat reasons: and hurt the owner's feelings, and get anorexic. But then decide, fuck it, I'm hungry and I love that expensive super tasty brand and flavor after all. Meanwhile the owner lives on cardboard and noodles, or cardboard noodles: and cries in the dark, alone, wondering how to pay all the vet bills.

PS: that's not a 🐈 reference at @claustromaniac , I'm just reflecting on my beloved cat's behavior (RIP buddy): I want to emulate him (not the RIP part, just the don't give a fuck part: in general) 😀

crssi commented 5 years ago

I still think that default user.js should have all 3 (blockActive, blockDisplay and upgradeDisplay) set to true... blockActive and blockDisplay already are at true. In that case there are less breakages and security remains on the same level... see test 1.

Cheers

Thorin-Oakenpants commented 5 years ago

After the circus side show of trying to explain caches, I am not going to go down that road again.

Considering that we struggled to get any consensus here, that the pref is old AF and never tested i.e turned on in nightly or anything, that the moz devs themselves struggled (I think), that no-one here did any actual tests that I know of (earthling summarized some old ones), and that the web in the time since this pref was created has moved a lot more towards HTTPS (i.e the impact will become less and less to the point of useless) ...

I hereby close this ticket with a bug fat "NO", not going there, sorry.

claustromaniac commented 5 years ago

no-one here did any actual tests that I know of

I did test what is described in STR in the first post.