-
Notifications
You must be signed in to change notification settings - Fork 525
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
security.mixed_content.upgrade_display_content (round 2) #754
Comments
I'll let @earthlng respond to this when he returns, hopefully, in about a week - apparently he is on Edit: Actually .. since he's AWOL for a while ... we could go mad and make 100s more changes 😀 |
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: TL;DR:
|
I got a notification that you replied but it seems you deleted your post. |
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 |
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.
Conclusions: if upgradeDisplay is not in the picture (ie set to false), we can see from tests 6 + 7 that
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 ... 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 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? :) |
👖: The amount of time you spend drinking beer simply defies everything that modern medecine has ever said about liver damage due to alcohol. Big E: Thanks a lot for the synthesis, helps making things clearer! |
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? |
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
We should leave blockDisplay as |
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. 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 :) |
Quoting myself:
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. |
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... |
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. |
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. |
@claustromaniac summed up my thoughts on this matter, especially the experiment part. |
Let me try to simplify this as a step in how to add it to the user.js (if we do)
🔸 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
🔸 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! |
It's a yes from me! |
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);
IDK if you realize how funny it is that you're only saying that now... |
We should do something more because |
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! |
/* 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. |
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. |
This comment has been minimized.
This comment has been minimized.
Ah, thanks 👖! 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. |
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. 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. I might be way too cautious here but yeah, that's why you're paying me the big bucks, right?! (NOT! :)
Best results in what regards? Talk about dumbing it down ... :) just my 2 cents; do whatever you want. 👍 |
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.
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. |
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... |
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. |
your guess is as good as mine. Maybe Pants can find some numbers for us in the mozilla telemetry data?
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?
No I'm not! In the part that you quoted I was talking about the 4 active resources like imagesrcset etc.
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. |
OK, let's try this again ... the first question is: do we add 🐈 said
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:
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
🐈 said
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 |
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. |
From my observation... |
^^ 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 |
Oh... I see, you are right. I was paying the attention only to NOTE: |
@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. |
Damn. I didn't pick up on that. And here I was thinking he did a heap of testing, all scientific like. |
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. |
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. |
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 |
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? |
I agree, for the most part, but even in those cases HTTPS is (should be?) useful for authentication at the very least.
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. |
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 |
Oh, I thought it was up to 80 now. Lemme check |
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) 😀 |
I still think that default user.js should have all 3 (blockActive, blockDisplay and upgradeDisplay) set to Cheers |
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. |
I did test what is described in STR in the first post. |
Just FYI, 1442990 was set to RESOLVED WONTFIX, because upgrading mixed passive content is going to be the default behavior as per the Mixed Content Level 2 spec. |
Well, that's better than nothing, but by the time it gets implemented (I'm just guessing here), HTTP will probably be deprecated (or near enough with scary NOT SECURE warnings in the urlbar if any content is insecure: i.e most of the top sites will have changed) |
Personally, I've never seen the relevance of the "top sites". Most of those already support HTTPS by default, and that won't change. The Internet is so big that I don't need to use any of those sites, and I'm sure I'm not alone on that (thank gods!) |
True ... and that's only the surface web. But for most people I doubt they stray outside the top 1M or whatever except for their local news or shops maybe. Meanwhile ... I found an excellent image for POOP .. https://thechive.files.wordpress.com/2019/10/0e429b7be7ed04ef0d4706a8e5137ec0.jpg 🤣 |
Right, that change won't be very meaningful for them, which seems to imply that the W3C is trying to improve the Internet for people like me too! (thanks W3C!)
Cool. Does it have a license? Can I borrow it for my page? (assuming I ever manage to get back to working on that thing!). |
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:
For these STR, I will refer to
1240
asblockActive
,1241
asblockDisplay
,1243
asblockPlugin
, andsecurity.mixed_content.upgrade_display_content
asupgradeDisplay
.blockActive
,blockDisplay
, andblockPlugin
tofalse
The code simply injects an
<img>
HTML element that triggers a request togithub.githubassets.com
over HTTP. The octocat gif should appear at the very bottom of the page.upgradeDisplay
totrue
, reload the page and re-run the scratchpad scriptupgradeDisplay
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)blockDisplay
totrue
and reload, re-run scriptupgradeDisplay
has higher priority thanblockDisplay
, because the octocat gif was loaded without a hitch over HTTPS)blockActive
andblockPlugin
) to confirm they don't conflict with each other (conclusion 3: there don't seem to be any issues with the other prefs)Correct me if I'm wrong, but it seems the rationale for not adding this pref after the discussion in #367 was that:
My counter-arguments:
blockDisplay
isfalse
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 👖 wants as few issues open as possible, so feel free to close this whenever you want.
The text was updated successfully, but these errors were encountered: