Skip to content
This repository has been archived by the owner on Nov 6, 2023. It is now read-only.

Use upgrade-insecure-requests to resolve MCB issues #8506

Closed
gloomy-ghost opened this issue Feb 15, 2017 · 35 comments
Closed

Use upgrade-insecure-requests to resolve MCB issues #8506

gloomy-ghost opened this issue Feb 15, 2017 · 35 comments
Assignees

Comments

@gloomy-ghost
Copy link
Collaborator

From #7435 (comment):

Hey guys, @ivysrono just talked to me about UpgradeMixedContent. It's an extension that inserts upgrade-insecure-requests to every https page so browsers can rewrite http requests before start mixed content blocking.

The flaw is inserting such a header let browsers also rewrite display content while it's not blocked by default. If the display content isn't available for HTTPS, it fails. It's quite aggressive to do so for normal users who think http images don't really hurt, but it might be a good way for http-nowhere mode. Any thoughts?

Furthermore, we might be able to do something on mixedcontent rulesets.

@gloomy-ghost
Copy link
Collaborator Author

Ping @Hainish

@jeremyn
Copy link
Contributor

jeremyn commented Feb 15, 2017

For reference, here is a link describing the upgrade-insecure-requests CSP.

@Hainish
Copy link
Member

Hainish commented Feb 21, 2017

It makes sense for us to set the upgrade-insecure-requests directive for trivial rulesets. However, if a rule is not trivial setting upgrade-insecure-requests can interfere with passive mixed content from loading, since this is not blocked by the major browsers (yet).

@Hainish
Copy link
Member

Hainish commented Feb 21, 2017

Actually it seems like third-party requests are also upgraded by this directive, so we can't be sure that resources will not be blocked even for trivial rulesets:

With the above header set on a domain example.com that wants to migrate from HTTP to HTTPS, non-navigational insecure resource requests are automatically upgraded (first-party as well as third-party requests).

@Hainish
Copy link
Member

Hainish commented Feb 21, 2017

Ah, I misread the initial comment. Yes, I agree - in HTTP Nowhere mode this should be fine (whether it's a trivial ruleset or not, or whether or not we even have a ruleset for that site).

@galeksandrp
Copy link
Contributor

galeksandrp commented Mar 9, 2017

<target host="such-a-mixedcontent.host.com" option="upgrade-insecure-requests">

Similar #3343

@iki
Copy link

iki commented Aug 3, 2017

+1 for adding this, it's badly needed, Many sites like jsfiddle are broken with HTTPS Everywhere, esp. when embedded and the user can't rewrite the resources used easily.

At the same time, we can't break anything by adding it. It's true that in some cases, the resources can't be loaded by https, so there's no safe solution and the only workaround is to let the user swith to insecure http version of the page easily. These are quite rare for me, I still didn't encounter any, and I hit and solve the MCB issue with HTTPS Everywhere manually quite often. However, while the fix does not work for this corner case, it at least does no damage here, because the https page would not be able to load the http resource due to MCB error anyway.

@strugee
Copy link
Contributor

strugee commented Aug 3, 2017

@iki if you find that sites are broken, that's a bug in the ruleset. You should report those.

@jeremyn
Copy link
Contributor

jeremyn commented Aug 3, 2017

@iki upgrade-insecure-requests is not safe generally for the reasons @Hainish describes in #8506 (comment) and following.

As an example, suppose a request to https://a.example.com loads an image (or other passive content) from http://b.example.com. Also suppose that b.example.com doesn't support HTTPS. If we insert upgrade-insecure-requests into the response from https://a.example.com, then the image request to b.example.com will be changed to HTTPS and will fail. The user then won't see the image that they otherwise would have seen.

@jeremyn
Copy link
Contributor

jeremyn commented Aug 3, 2017

@strugee An active mixed content request is blocked by the browser before HTTPS Everywhere can rewrite the URL (see the FAQ). upgrade-insecure-requests is the way around that problem because it makes the browser rewrite the URL it blocks the request.

@strugee
Copy link
Contributor

strugee commented Aug 3, 2017

@jeremyn if things are breaking because of MCB the ruleset should have platform=mixedcontent (I've forgotten the exact string but I think you know what I mean).

@jeremyn
Copy link
Contributor

jeremyn commented Aug 3, 2017

@strugee I don't know what's happening with JSFiddle, hopefully @iki can describe what they are seeing in more detail. I agree that adding platform is one way to go, although that means it is only active by default on Tor so it's effectively the same as disabling the ruleset by default. We generally let minor active mixed content problems go without marking the ruleset with platform. See issue #6629 for a related discussion.

@strugee
Copy link
Contributor

strugee commented Aug 3, 2017

@jeremyn right, that's why I wanted @iki to report a bug instead of just working around it with upgrade-insecure-requests or whatever ;)

@Bisaloo
Copy link
Collaborator

Bisaloo commented Aug 11, 2017

Just a reminder that this issue is about adding this header in HTTP-Nowhere mode so it wouldn't break more things than it currently is. In the example described by @jeremyn

As an example, suppose a request to https://a.example.com loads an image (or other passive content) from http://b.example.com. Also suppose that b.example.com doesn't support HTTPS. If we insert upgrade-insecure-requests into the response from https://a.example.com, then the image request to b.example.com will be changed to HTTPS and will fail. The user then won't see the image that they otherwise would have seen.

The image will already fail to load in HTTP-Nowhere mode.

@Bisaloo
Copy link
Collaborator

Bisaloo commented Aug 11, 2017

My issue with this is that an additional header increase indentifiability. This would basically allow websites to find out whether users are using HTTP-Nowhere mode or not.

This could potentially be a problem in Tor browser.

@jeremyn
Copy link
Contributor

jeremyn commented Aug 11, 2017

There is no serious defense against browser fingerprinting for an ordinary non-Tor user, so that shouldn't be a concern.

For Tor, the advice is to use all defaults partly to fight against fingerprinting, so if all Tor users send the new header, then they are no worse off than without the new header.

@numismatika
Copy link
Contributor

Is there any progress on the basic firefox issue that causes mcb warnings on ressources we already converted to use tls?

@jeremyn
Copy link
Contributor

jeremyn commented Aug 17, 2017

@numismatika What's an example?

@numismatika
Copy link
Contributor

https://forums.askmrrobot.com/ warns about insecure requests from
http://askmrrobot.discoursehosting.net/uploads/db4666/original/1X/80949fd802e399f623154ccd2b33d33a93461cce.png although it is already rewritten by the AMR rule

@jeremyn
Copy link
Contributor

jeremyn commented Aug 17, 2017

@numismatika Thanks, I understand your example. This looks like a Firefox problem, not an HTTPS Everywhere problem. Do you think so too?

@strugee
Copy link
Contributor

strugee commented Aug 17, 2017

@jeremyn I believe @numismatika was saying it was an upstream issue and was wondering whether there's been progress on it.

@strugee
Copy link
Contributor

strugee commented Aug 17, 2017

https://bugzilla.mozilla.org/show_bug.cgi?id=878890 appears to be the relevant BMO bug.

@zero77
Copy link
Contributor

zero77 commented Sep 30, 2017

upgrade-insecure-requests is used by "Smart HTTPS" and it seams to be working very well without there being any problems.

If there is concern that it may cause problems perhaps, it should be implemented as an experimental option that needs enabling so, it can be tested be for its enabled by default.

@ivysrono
Copy link
Contributor

ivysrono commented Oct 3, 2017

@zero77 No blacklist on Smart HTTPS.

@zero77
Copy link
Contributor

zero77 commented Oct 3, 2017

@ivysrono
There is both a black and white list in Smart HTTPS although, not in HTTPS Everywhere is that what your meaning.

@Bisaloo
Copy link
Collaborator

Bisaloo commented Oct 3, 2017

upgrade-insecure-requests will cause issues in cases where a resource is included from a domain that doesn't support https.

@zero77
Copy link
Contributor

zero77 commented Oct 3, 2017

@Bisaloo
Why not add a time out or look for errors and then downgrade to http.

That way if https is not supported any failed https connection attempts will automatically be downgraded, users will only see a delay and nothing else.

@ivysrono
Copy link
Contributor

ivysrono commented Oct 3, 2017

@zero77 In Smart HTTPS, no blacklist for upgrade-insecure-requests to fix issues in cases where a resource is included from a domain that doesn't support https.

@jeremyn
Copy link
Contributor

jeremyn commented Oct 3, 2017

@zero77 For your comment #8506 (comment), particularly

Why not add a time out or look for errors and then downgrade to http.

you might find the discussion in issue #8239 Better network errors when "block unencrypted requests" is enabled relevant, since it talks about challenges related to HTTPS Everywhere detecting and making decisions about connection results.

@zero77
Copy link
Contributor

zero77 commented Oct 4, 2017

@jeremyn @strugee
Thanks

@justyntemme
Copy link

Any progress on this? If help is needed I am more than happy to contribute some time.

@jeremyn
Copy link
Contributor

jeremyn commented Dec 27, 2017

@justyntemme As far as I know there is no active work on this. We seem agreed that adding upgrade-insecure-requests is fine in Block all unencrypted requests mode, aside from @Bisaloo 's concerns in #8506 (comment) about making a user's browser fingerprint more unique, which I argued was not an issue in #8506 (comment).

If you want to create a pull request adding the functionality I described, go ahead, as long as you understand that there's no guarantee it will be merged. There's almost no chance it will be added unless someone does the work first -- @Hainish / EFF staff need to approve it, and they are almost certainly not going to write it themselves or approve it in advance.

@justyntemme
Copy link

@jeremyn That sounds completely reasonable. I would not expect anything less of the EFF. I just wanted to make sure nobody was working hard on this before I started devoting time. Thank you.

@cschanaj
Copy link
Collaborator

FYI, I've filed a PR in #14600.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests