w3c / webappsec-mixed-content

WebAppSec Mixed Content
https://w3c.github.io/webappsec-mixed-content/
Other
12 stars 22 forks source link

Difference between spec and practice in mixed content upgrade #63

Closed moztomer closed 1 year ago

moztomer commented 1 year ago

We plan to ship the automatic upgrading of passive mixed content (as per Mixed Lvl2) in Firefox Nightly.

We checked the behavior of mixed-content display upgrade mechanism of Firefox Nightly and Chrome release related to CORS. E.g., how does mixed content interacts with CORS request where CORS is not allowed. It appears that such CORS requests get upgraded by mixed content before they get blocked in both browser.

STR for chrome and nightly: open an https site and type the following in the webconsole:

let video12 = document.createElement("video")
video12.crossOrigin = "Anonymous"
video12.src = "http://www.youtube.com/watch?v=q5Tp1ZUbAFQ"
document.body.appendChild(video12)

Expected error message: something like: "Access to video at 'http: //videoURL' from origin 'https://someSite ... blocked by Mixed-Content Blocker' Actual error message: "Access to video at 'https://videoURL' from origin 'https://someSite ... blocked by CORS policy"

But the specs says we DON'T upgrade mixed content:

If one or more of the following conditions is met, return without modifying request: ...

  1. request’s mode is CORS.

My question is, should we change the mixed-content spec or the browsers implementation according to mixed-content (level 2) behavior with CORS requests?

mikewest commented 1 year ago

cc @estark37 and @carlosjoan91

annevk commented 1 year ago

(Expected and Actual error message in OP are the same.)

It's not clear to me why we'd exclude "cors" requests here. Would be interested in the rationale.

It's also not clear to me why "imageset" initiator requests are excluded. (Potentially a separate issue.)

moztomer commented 1 year ago

(Expected and Actual error message in OP are the same.)

The difference is the upper letter S in the protocol of the blocked urls. I will edit it to highlight it more :)

annevk commented 1 year ago

@moztomer ah true, but wouldn't you expect it to be blocked by Mixed Content as opposed to CORS?

moztomer commented 1 year ago

Yes, I think you are right. So, in case mixed-content doesn't upgrade the media/ image element to https it should block it unrelated to CORS. My initially expected outcome was incorrect then. Thank you I will correct it.

mozfreddyb commented 1 year ago

We want to align here, but are not sure whether this is incorrect in spec or in Chrome: There are no tests and Chrome is upgrading regardless of the request mode.

@estark37: Can you remember the rationale for not upgrading when the request mode is cors?

annevk commented 1 year ago

It's the same as with "imageset", prior behavior: https://www.w3.org/TR/2016/CR-mixed-content-20160802/#should-block-fetch.

moztomer commented 1 year ago

@annevk I think the difference here is that either chrome as firefox nightly are upgrading requests that enable CORS.

So I would assume, here we have situation where we need to decide whether we update the browser implementations or the spec?

annevk commented 1 year ago

Was the old behavior just never implemented? I'm pretty sure Firefox used to block HTTPS -> HTTP CORS requests.

moztomer commented 1 year ago

It is blocking CORS request if upgrade_display_content pref is disabled. If it is enabled ( which is currently the case in Firefox Nightly) it does upgrade display mixed content even if the request has the cors mode. Latter is also true for the default mixed content implementation of Chrome

annevk commented 1 year ago

I guess @carlosjoan91 and @estark37 need to weigh in why the decision was made differently here. And perhaps you know why Firefox did this differently?

This would potentially result in requests being made that have been blocked for a long time now. Probably no big security downside, but it might impact performance. And it's also rather inconsistent with the "imageset" decision.

mozfreddyb commented 1 year ago

To be clear, upgrading CORS requests is only in Firefox Nightly. We are currently tracking this in bug 1806114, as part of our road to improving the implementation in alignment with the spec and the wider ecosystem. Hence the question..

@estark37 is Chrome going to keep upgrading CORS requests or do you intent to align with the spec?

mozfreddyb commented 1 year ago

After some additional testing, we have learned that both Firefox (before upgrading any mixed content) and Safari Tech Preview 161 allow passive mixed content even if the request mode is CORS.

For those who wish to follow along, here's how I tested

I therefore propose that we change the algorithm "4.1. Upgrade a mixed content request to a potentially trustworthy URL, if appropriate" and remove its 4th step ("request’s mode is CORS.`"), so that it will start allowing an upgrade of CORS requests.

mikewest commented 1 year ago

I'd like @carlosjoan91 and @estark37 to weigh in, but FWIW: if we're upgrading and not blocking, I think it's reasonable to reconsider both the CORS and imageset carveouts. Neither make as much sense as carveouts to drive mixed content down over time if we're going to correct them (and therefore eradicate mixed content entirely).

estark37 commented 1 year ago

I'm sorry I missed this discussion!

First of all I strongly suspect that we didn't make any conscious decision about upgrading CORS requests when implementing autoupgrading in Chrome, and whatever behavior exists today is just what our implementation happened to do. In other words, it's probably a bug.

I agree with @mozfreddyb that it's fine to change the spec to upgrade passive mixed CORS requests. When we shipped autoupgrading, we did want to avoid upgrading requests that had previously been blocked, because it seemed like that could introduce the risk of unexpected behavior -- possibly even security consequences -- without much benefit. But since it sounds like browsers have been already allowing passive mixed CORS requests to load, I don't think there's any harm in upgrading them. I think active mixed CORS requests should stay blocked (hopefully all browsers are already blocking them) and not upgraded.

I believe Chrome at least is correctly blocking and not upgrading imageset, so I think we should leave that one as it is, under the principle that we shouldn't start upgrading and loading a type of request that is already blocked.

mozfreddyb commented 1 year ago

Looks like there is wide agreement then. FYI: I've been curious about the Webkit position so I filed a standards position request in https://github.com/WebKit/standards-positions/issues/124.