1417901 - Firefox is changing 301 HTTP redirects to HTTPS when done on an HTTPS server

Status: RESOLVED DUPLICATE of bug 1422284

(Core :: Networking: HTTP, defect, P2)

Description

[masj]

User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_11_6) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/62.0.3202.94 Safari/537.36

Steps to reproduce:

A URL served off an HTTPS host responds with a 301 status to redirect to a non-HTTPS location. Firefox will force/rewrite this url to an HTTPS location. This is unexpected behaviour.

This happens regardless of whether the host responds with Upgrade-Insecure-Requests or not.


Actual results:

Firefox is directed to:
https://www.example.com/301_to_non_secure_destination
The server returns a 301 to:
http://www.insecureexample.com
Firefox will try to get:
https://www.insecureexample.com



Expected results:

Firefox should go to: http://www.insecureexample.com

Comment 1

[Honza Bambas (:mayhemer)]

Do you have a real life example?  Can you reproduce with a clean profile [1]?

If you can't share the URL, can you please produce a log according [2] and send it directly to my bugzilla email, best zipped?

Thanks.

[1] https://support.mozilla.org/en-US/kb/profile-manager-create-and-remove-firefox-profiles#w_manage-profiles-when-firefox-is-open
[2] https://developer.mozilla.org/en-US/docs/Mozilla/Debugging/HTTP_logging#Logging_HTTP_activity_by_manually_setting_environment_variables

Comment 2

[masj]

Okay, I did some more testing. This only happens if the first page is served with Upgrade-Insecure-Requests set to true. However, it's still unexpected behaviour because the 2nd url does not set this header and in any case it shouldn't really apply to primary page redirects, only on-page content. Firefox is still upgrading the redirect.

Please see:
https://s.mprd.se/301page/301.html <- served with Upgrade-Insecure-Requests

When you click on the link (on the first page) to:
https://s.mprd.se/301redirect/ <- Not served with Upgrade-Insecure-Requests

Still, firefox will send you to:
httpS://www.neverssl.com (which is not the URL that is returned by the 301)

It should go to:
http://www.neverssl.com as returned by the redirect.


The same behaviour happens with a 302 redirect as well.

I've just emailed you the log file I generated with a clean profile. Please note that the log file was generated using a 302 redirect but the behaviour is the same with a 301 as well.

Comment 3

[Shih-Chiang Chien [:schien] (UTC+8) (use ni? plz)]

IMO this creates a loophole to downgrade to HTTP while Upgrade-Insecure-Requests is specified. By reading the spec [1], the algorithm seems enforcing strict HTTPS upgrade and not fallback to HTTP. So, allowing the behavior described in the description looks dangerous to me.

@tanvi and @annevk might have better insight on the Upgrade-Insecure-Requests policy.

[1] https://w3c.github.io/webappsec-upgrade-insecure-requests/

Comment 4

[masj]

(In reply to Shih-Chiang Chien [:schien] (UTC+8) (use ni? plz) from comment #3)
> IMO this creates a loophole to downgrade to HTTP while
> Upgrade-Insecure-Requests is specified. By reading the spec [1], the
> algorithm seems enforcing strict HTTPS upgrade and not fallback to HTTP. So,
> allowing the behavior described in the description looks dangerous to me.
> 
> @tanvi and @annevk might have better insight on the
> Upgrade-Insecure-Requests policy.
> 
> [1] https://w3c.github.io/webappsec-upgrade-insecure-requests/

The browser is not supposed to make navigational upgrades to links to 3rd party sites. A link click and subsequent redirect should not get automatically upgraded. Is it really a loophole if the initial site's intention is to send the user to a non-secure 3rd party (separate domain) destination?

Comment 5

[Anne (:annevk)]

I filed https://github.com/w3c/webappsec-upgrade-insecure-requests/issues/12 as it seems the normative text of the standard agrees with Firefox, but the non-normative text does not.

Comment 6

[Anne (:annevk)]

Shih-Chiang, given Mike's comment in the issue above I think we should just fix this.

masj, I suppose this does not happen for outright links to neverssl.com, only as part of redirects?

Comment 7

[masj]

It does not happen for direct links to neverssl.com but a redirect from the initial domain (after a link click on a page served with upgrade insecure requests) to an external one always results in firefox replacing the redirect URL with https. Even if the intention was to send the user to http://www.anyothersite.com - firefox will replace this to - httpS://www.anyothersite.com

You can see this from the live example I've put in my 2nd post.

Comment 8

[Shih-Chiang Chien [:schien] (UTC+8) (use ni? plz)]

The upgrage-insecure-requests attribute is inherited via nsLoadInfo.
https://searchfox.org/mozilla-central/rev/919dce54f43356c22d6ff6b81c07ef412b1bf933/netwerk/protocol/http/HttpBaseChannel.cpp#3137

We need to reset the corresponding flag for cross-origin redirection.

Comment 9

[Steffen Weber]

I think this is the same issue that I reported two weeks ago: https://bugzilla.mozilla.org/show_bug.cgi?id=1414676

Maybe they can be merged.

Comment 10

[Anne (:annevk)]

From bug 1414676 comment 2:

> changeset: b91870ea1243520ffa057fad90a655f24a398d5d
> pushlog_url: https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=e013d1324ea834f2d3c62edaeb644e41f5089459&tochange=b91870ea1243520ffa057fad90a655f24a398d5d

Comment 12

[Shih-Chiang Chien [:schien] (UTC+8) (use ni? plz)]

Attachment: Bug 1417901 - perform SameOriginForUIR check on the redirect target URI.

Review commit: https://reviewboard.mozilla.org/r/202448/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/202448/

Comment 13

[Christoph Kerschbaumer [:ckerschb]]

Comment on attachment 8931298 [details]
Bug 1417901 - perform SameOriginForUIR check on the redirect target URI.

https://reviewboard.mozilla.org/r/202448/#review208420

Thanks for putting this together, but I have to r- this first approach. Please address my concerns and flag me again. Ultimately it would be great to have a test for that as well. Maybe we can extend test_upgrade_insecure_navigation.html?

::: netwerk/protocol/http/HttpBaseChannel.cpp:3374
(Diff revision 1)
>        rv = newLoadInfo->SetResultPrincipalURI(newURI);
>        NS_ENSURE_SUCCESS(rv, rv);
>      }
> +
> +    // Disable Upgrade-Insecure-Requests for cross-domain redirection
> +    if (mLoadInfo->GetUpgradeInsecureRequests()) {

Upgrade-insecure-requests should always upgrade non navigational requests, even after a redirect. I guess we only want to special case navigational redirects?

The better place to do all that would be CloneLoadInfoForRedirect. There is already an if statement which we only enter to loads of TYPE_DOCUMENT and TYPE_SUBDOCUMENT [1]. I think this is where we should do the check.

I would hope that some of our tests (dom/security/tests/test_upgrade_insecure*) would break with that patch applied.

[1] https://dxr.mozilla.org/mozilla-central/source/netwerk/protocol/http/HttpBaseChannel.cpp#3142

::: netwerk/protocol/http/HttpBaseChannel.cpp:3385
(Diff revision 1)
> +        nsCOMPtr<nsIPrincipal> resultPrincipal;
> +        nsContentUtils::GetSecurityManager()->GetChannelResultPrincipal(newChannel, getter_AddRefs(resultPrincipal));
> +        MOZ_ASSERT(resultPrincipal);
> +
> +        nsCOMPtr<nsIPrincipal> triggeringPrincipal = newLoadInfo->LoadingPrincipal();
> +        if (!triggeringPrincipal) {

The triggeringPrincipal within the loadInfo should never be null. Did you experience that behavior, if so we should find out why, otherwise please remove that branch.

Comment 14

[Christoph Kerschbaumer [:ckerschb]]

Comment on attachment 8931298 [details]
Bug 1417901 - perform SameOriginForUIR check on the redirect target URI.

Oh sorry, I haven't realized this was a f? request and not an r?. In that case I am adding f+. Thanks for working on this.

Comment 15

[Christoph Kerschbaumer [:ckerschb]]

I am marking this Bug as a duplicate of bug 1422284. :jkt will take a look and get that fixed.