1530577 - (Un)Expected HTTPS upgrade of download due to <meta ... "upgrade-insecure-requests"> tag

Status: NEW

(Core :: DOM: Security, task, P3)

Description

[Karl Dubost💡 :karlcow]

Steps to reproduce.

1. With Firefox Nightly 67, fresh profile.
2. Go to https://www.samsung.com/nl/apps/smart-switch/
3. Scroll to the bottom of the page
4. Select "download voor pc" or "download voor mac"
5. Wait

Expected:
a file is saved locally

Actual:
The connection fails at a point.

Analysis of the HTTP transactions are at
https://webcompat.com/issues/25294#issuecomment-466908757

This works in Chromium.

I tried to log the full transaction which resulted in 101 Mo of logs and it seems at a point that the connection is lost.

I see also

2019-02-26 04:00:28.938175 UTC - [Parent 22256: Socket Thread]: D/nsHostResolver Adding address to blacklist for host [org.downloadcenter.samsung.com], host record [0x138b9b690].used trr=0
2019-02-26 04:00:28.938211 UTC - [Parent 22256: Socket Thread]: D/nsHostResolver Successfully adding address [112.106.5.11] to blacklist for host [org.downloadcenter.samsung.com].
2019-02-26 04:00:28.938242 UTC - [Parent 22256: Socket Thread]: D/nsSocketTransport nsSocketInputStream::OnSocketReady [this=0x11d50de88 cond=804b000e]
2019-02-26 04:00:28.938266 UTC - [Parent 22256: Socket Thread]: D/nsSocketTransport nsSocketOutputStream::OnSocketReady [this=0x11d50dec0 cond=804b000e]
2019-02-26 04:00:28.938296 UTC - [Parent 22256: Socket Thread]: V/nsHttp nsHalfOpenSocket::OnOutputStreamReady [this=0x13a5524a0 ent=org.downloadcenter.samsung.com primary]
2019-02-26 04:00:28.938321 UTC - [Parent 22256: Socket Thread]: V/nsHttp nsHttpConnectionMgr::ConditionallyStopTimeoutTick armed=1 active=14
2019-02-26 04:00:28.938343 UTC - [Parent 22256: Socket Thread]: V/nsHttp nsHalfOpenSocket::CancelBackupTimer()

Comment 1

[Honza Bambas (:mayhemer)]

We do HSTS upgrade of http://org.downloadcenter.samsung.com/downloadfile/ContentsFile.aspx? to https. And there is no server listening for TLS connections (locally checked), only plain http.

There is no response from any samsung servers having Strict-Transport-Security response header. So, this is likely a duplicate bug 1391277 or bug 1419222.

I can't find the investigative log here:
https://searchfox.org/mozilla-central/rev/b10ae6b7a50d176a813900cbe9dc18c85acd604b/dom/html/HTMLMetaElement.cpp#111

but I CAN see this log:
Line 1055109: 2019-02-26 14:54:22.244000 UTC - [Parent 6088: Main Thread]: D/CSPContext nsCSPContext::AppendPolicy added UPGRADE_IF_INSECURE_DIRECTIVE self-uri=https://www.samsung.com/nl/apps/smart-switch/ referrer=
Line 1057886: 2019-02-26 14:54:22.338000 UTC - [Parent 6088: Main Thread]: D/CSPContext nsCSPContext::AppendPolicy added UPGRADE_IF_INSECURE_DIRECTIVE self-uri=https://www.samsung.com/nl/apps/smart-switch/ referrer=
https://searchfox.org/mozilla-central/rev/b10ae6b7a50d176a813900cbe9dc18c85acd604b/dom/security/nsCSPContext.cpp#379

In the BROWSER console I can see:
Content Security Policy: Upgrading insecure request ‘http://org.downloadcenter.samsung.com/downloadfile/ContentsFile.aspx?CDSite=COMMON&CttFileID=7199456’ to use ‘https’
which come from:
https://searchfox.org/mozilla-central/rev/b10ae6b7a50d176a813900cbe9dc18c85acd604b/netwerk/base/nsNetUtil.cpp#2539

It would be great to investigate this ASAP before they change the page. Hence P1 and DOM:Sec. This is CSP, not http, that does something wrong, IMO.

To complete the picture, from https://hstspreload.org:
Status: samsung.com is not preloaded.

Eligibility: In order for samsung.com to be eligible for preloading, the errors below must be resolved:
Error: No HSTS headerResponse error: No HSTS header is present on the response.

Comment 2

[Honza Bambas (:mayhemer)]

(didn't want to change the state, this is likely our bug)

Comment 3

[Boris Zbarsky [:bzbarsky]]

Christoph, could you look into this CSP issue ASAP, please, before the site changes?

Comment 4

[Boris Zbarsky [:bzbarsky]]

So in particular, https://www.samsung.com/nl/apps/smart-switch/ does not have CSP headers or a CSP meta that I can see. At least not at the point when the page finishes loading...

Comment 5

[Christoph Kerschbaumer [:ckerschb]]

Christoph, could you look into this CSP issue ASAP, please, before the site changes?

Unfortunately I only have limited time left today - I can take a closer look tomorrow if needed. From what I've seen so far, the page https://www.samsung.com/nl/apps/smart-switch/ uses the following meta CSP:

<meta http-equiv="content-Security-Policy" content="upgrade-insecure-requests">

when I then click-on |download voor pc| Firefox opens a new tab and since same-origin top-level navigations need to be upgraded by UIR, this docshell code [1] causes the load to be upgraded to https. The console also indicates that upgrade:

Content Security Policy: Upgrading insecure request ‘http://org.downloadcenter.samsung.com/downloadfile/ContentsFile.aspx?CDSite=COMMON&CttFileID=7199456’ to use ‘https’

When I comment out the CSP code within Firefox code, so that no CSP is applied, then the new tab opens and the download manager appears and I can download the *.exe just fine.

[1] https://searchfox.org/mozilla-central/source/docshell/base/nsDocShell.cpp#9569

Comment 6

[Boris Zbarsky [:bzbarsky]]

uses the following meta CSP

Ah, I was searching for the wrong thing.

OK, so given that, the real question seems to be why it works in Chrome. Probably because Chrome doesn't propagate the CSP to the newly-opened about:blank or something... :(

So what does the spec say should happen in this case (funny, I know) and who on the Chrome team would know what their actual behavior is?

Comment 7

[Christoph Kerschbaumer [:ckerschb]]

(In reply to Boris Zbarsky [:bzbarsky, bz on IRC] from comment #6)

So what does the spec say should happen in this case (funny, I know) and who on the Chrome team would know what their actual behavior is?

Mike, what's your take on this one? What should happen for a same-origin download in case uir is set? Can you put me in contact with someone working on this in Chrome?

Comment 8

[Daniel Veditz [:dveditz]]

We read https://w3c.github.io/webappsec-upgrade-insecure-requests/#upgrade-request as saying a same-origin navigation request (which this basically is) should be upgraded. Doesn't say anything about downloads specifically, but downloads grew out of "navigations that couldn't be rendered" which browsers would then ask the user if they wanted to save.

Comment 10

[Christoph Kerschbaumer [:ckerschb]]

:karlcow, I talked to Mike West and he confirmed that Firefox is exhibiting the right behavior. FWIW, I am copying the actual download link here as well, which shows that there are no special attributes which would cause Chrome not to upgrade the download:

<a href="//www.samsung.com/global/download/smartswitchmac" class="c_btn_pre-type2 btn_go-to-website" target="_blank" data-omni-type="microsite_contentinter" data-omni="apps:smart switch:mac download" title="Klik om naar de website te gaan">DOWNLOAD VOOR MAC</a>

So, I guess this could turn into Tech Evangelism bug - otherwise I will close this bug as INVALID. What do you think?

Comment 11

[Boris Zbarsky [:bzbarsky]]

Is there a bug on Chrome to fix their behavior? Do they have a concrete plan to do so? If not, we may need to just duplicate it and get the spec changed accordingly...

Comment 12

[Karl Dubost💡 :karlcow]

As Boris said, Probably we should indeed check with Chrome people first.
I see this which looks similar.
https://bugs.chromium.org/p/chromium/issues/detail?id=589747
https://bugs.chromium.org/p/chromium/issues/detail?id=617584

found with https://bugs.chromium.org/p/chromium/issues/list?can=2&q=upgrade%20insecure%20requests

Comment 13

[Christoph Kerschbaumer [:ckerschb]]

(In reply to Boris Zbarsky [:bzbarsky, bz on IRC] from comment #11)

Is there a bug on Chrome to fix their behavior? Do they have a concrete plan to do so? If not, we may need to just duplicate it and get the spec changed accordingly...

FWIW, I filed a spec issue [1]. Let's take it from there...

[1] https://github.com/w3c/webappsec-upgrade-insecure-requests/issues/16

Comment 14

[Boris Zbarsky [:bzbarsky]]

The spec issue makes it sound like this is about downloads, but all this code is running before we even know it's a download, no?

As in, it's a basic problem with navigations in Chrome, as far as I can tell. Am I wrong?

Comment 15

[Boris Zbarsky [:bzbarsky]]

As in, I would assume you can create the same failure with a non-download pageload in the new window, because again the connection fails before we even get a chance to see the return type and do the "download or render?" decision.

Comment 16

[Christoph Kerschbaumer [:ckerschb]]

(In reply to Boris Zbarsky [:bzbarsky, bz on IRC] from comment #15)

As in, I would assume you can create the same failure with a non-download pageload in the new window, because again the connection fails before we even get a chance to see the return type and do the "download or render?" decision.

Yes, you are right. This code runs way before we even know it's a download. So it seems to me Chrome must have a basic problem with UIR for same-origin navigations.

The spec however is only unclear about downloads - but again, this code happens way before we know it's a download.

Comment 17

[Boris Zbarsky [:bzbarsky]]

So it seems to me Chrome must have a basic problem with UIR for same-origin navigations.

Right. We should either mutate the spec issue to make that clear or file a new spec issue that makes it clear up front what the scope of the problem is.

Comment 18

[Boris Zbarsky [:bzbarsky]]

And in particular, how exactly is Chrome not matching the spec? Is it not propagating the CSP of the opener to the newly opened about:blank? Is it not using that CSP (or the opener's CSP) for the load? Is it using that CSP (e.g. in terms of how redirects are allowed or not) but not following upgrade-insecure-requests directives in it? Something else?

Feels like we should have a clear idea of what the issue is here and then file issues on both Chrome and the spec on the two not matching each other. But we need to understand where the mismatch is, exactly.

Comment 19

[Daniel Veditz [:dveditz]]

chrome bug https://bugs.chromium.org/p/chromium/issues/detail?id=615910 says they're also not doing UIR for iframe sources, but they pass WPT UIR tests with iframe src in the name.

Comment 20

[Boris Zbarsky [:bzbarsky]]

That chrome bug was marked fixed back in 2016, right?

Comment 21

[Christoph Kerschbaumer [:ckerschb]]

FWIW, I filed a bug on Chrome so we can work with a dev to figure out the exact problem why Chrome is not upgrading the navigaton/download, see:
https://bugs.chromium.org/p/chromium/issues/detail?id=944224

Comment 22

[Firefox Bug Husbandry Bot]

Migrating Webcompat whiteboard priorities to project flags. See bug 1547409.

Comment 23

[Firefox Bug Husbandry Bot]

See bug 1547409. Migrating whiteboard priority tags to program flags.

Comment 24

[Jonas Allmann [:jallmann]]

I started working on this and it seems to be closely related to Bug 1422284.
I'll try to summarize the two examples and what has been discussed so far:
Example 1: (this bug)
https://www.samsung.com/nl/apps/smart-switch/ has a meta-CSP applied including Upgrade-Insecure-Requests.

Clicking the download button navigates to same-origin https://www.samsung.com/global/download/smartswitchwin/ in a new tab.

This redirects to insecure, cross-origin (?) http://downloadcenter.samsung.com/content/SW/201909/20190916163610197/SmartSwitchPC_setup.exe

The request is upgraded, the connection times out because the download server does not support https.
Copying and navigating to https://www.samsung.com/global/download/smartswitchwin/ directly in a new tab starts the download because the redirected dowload request is not upgraded.

Example 2: (Bug 1422284)
https://www.halesworth.net/links/list_two.php?code=3 has a CSP HTTP header including Upgrade-Insecure-Requests.

Clicking 'New Cut Arts, Halesworth' navigates to same-origin https://www.halesworth.net/links/linx_two.php?linkid=44 in a new tab.

This redirects to insecure, cross-origin http://www.newcut.org/

The request is upgraded, the browser issues a warning because the cert is invalid (which is not really relevant for us here, it just illustrates how the behaviour is similar to Example 1).

Copying and navigating to https://www.halesworth.net/links/linx_two.php?linkid=44 directly in a new tab redirects successfully to the insecure version of http://www.newcut.org/ because the request in not upgraded.

Correct bahviour:
After consulting the specs and reading the different discussions linked in the two bugs I'm still not sure which is the desired behaviour in both cases.
The last comment in https://bugs.chromium.org/p/chromium/issues/detail?id=944224 suggests that the redirected download request should not be upgraded because http://downloadcenter.samsung.com is not same-origin with https://www.samsung.com. That would apply equally to Example 2 and matches the result of the discussion in Bug 1422284.
That leaves the question open if there should be some kind of warning if a secure site redirects to an insecure download, but I assume that's not part of this Bug.
Am I correct in my analysis? Did I miss anything important? And what would be the best way to move forward here?

Comment 25

[Christoph Kerschbaumer [:ckerschb]]

Clearing out old ni? at the moment. Everything within this bug probably needs to be re-evaluated a this point. Leaving as P3 seems correct though.