I can try to pick this up again (I know, it's been a long time). Attaching an updated patch that applies on mozilla-central. This new patch supports the default-port gate requested by :mcmanus in comment 10. The speculative connect is initiated whether or not we're refused http on port 80 because there's potential we might end up getting https soon anyway. The speculative connect is easy to adjust.

The hard part of this appears to be testing. I can't find any live sites that exhibit this behavior (refuse connection on port 80, allow HTTPS on 443). Most either redirect http->https or allow port 80. If anyone knows of a live site, it would help me speed up testing!

I started looking into the other tests suggested by Dave G in comment 16, but the best way to automate testing this is also not obvious. I created a browser mochitest that attempts to do this, but it's not quite right. Looks like we don't end up down the right code path the way I added a server to build/pgo/server-locations.txt: it doesn't generate NS_ERROR_CONNECTION_REFUSED, but seems to go down the NS_ERROR_UNKOWN_HOST || NS_ERROR_NET_RESET code path. I haven't investigated fully, but I don't know how to quickly set up a connection reject on port 80 for a given host that also serves mochitests over HTTPS on port 443.

This patch fools around with the mochitest.youtube.com domain for testing, but I'm not confident that does what we want.

Hi folks, just a side crazy idea (beat me if I'm off here!):
Do we need to first fail the the HTTP and only then try HTTPS?
Can't we (and of course, make this changeable by the user in the config section), as we get the server's IP from the DNS query reply - do a quick "tcp ping", maybe just at only the getting-a-SYN-reply level (to save time), to both ports - 80 and 443, and if 443 reply as open - go to 443 as the first option?

(In reply to Sid Stamm [:geekboy or :sstamm] from comment #41)
Thank you! :O

The hard part of this appears to be testing. I can't find any live sites that exhibit this behavior (refuse connection on port 80, allow HTTPS on 443). Most either redirect http->https or allow port 80. If anyone knows of a live site, it would help me speed up testing!

mozregression --launch 2020-04-21 --pref network.stricttransportsecurity.preloadlist:false -a http://mail.terrax.net -a http://rustls.org

Edit: mail.terrax.net does not support legacy IP, it requires IPv6, but rustls.org is dualstack.
Edit2: http://captured-symphonies.com is also dualstack.

The hard part of this appears to be testing. I can't find any live sites that exhibit this behavior (refuse connection on port 80, allow HTTPS on 443). Most either redirect http->https or allow port 80. If anyone knows of a live site, it would help me speed up testing!

Is this something worth investing in to get added to badssl.com, or is it a temporary need?

(In reply to April King [:April] from comment #44)

Is this something worth investing in to get added to badssl.com, or is it a temporary need?

If it's a non-trivial amount of work to get it added to badssl.com, I wouldn't bother. While that would be nice, automated tests should be sufficient and this is more about convenience than fixing a security problem.

Had to make a minor change to the patch, but this one seems to work.

Tested via code-following in debugger and manually with opt builds both with and without the patch (with clean profile and making sure HSTS preload is disabled):

 $ ./mach run --temp-profile --setpref network.stricttransportsecurity.preloadlist=false

then I type rustls.org in the URL bar and hit enter.

Without the patch, I get a connection error. With the patch, it redirects via HTTPS to a github URL. I still need to automate testing somehow, but I think this should do the job.

If it's a non-trivial amount of work to get it added to badssl.com, I wouldn't bother. While that would be nice, automated tests should be sufficient and this is more about convenience than fixing a security problem.

It is, unfortunately, as it would need an additional IP address. It sounds like you've got some good temporary options, but lemme know if you decide this is something that is worth keeping around.

I spent some time trying to automate testing this patch with browser mochitests, but I'm not convinced it's a good idea. The mochitests route all HTTP connections to the same server, which means the host- and port-based multiplexing happens at the HTTP layer and not at the socket layer so we really can't reject a connection to an individual port-80 host.

First, I tried hacking up the sslproxy, but that totally doesn't make sense since non-https connections don't go through it. I won't upload that attempted patch, but it's based on attachment 571410 [details] [diff] [review] in bug 599295.

Next, I tried modifying the mochitest server files to close connections when accessing a new http://fallback.example.com:80 entry I added to build/pgo/server-locations.txt, but it causes a different error, not NS_ERROR_CONNECTION_REFUSED (which makes sense... the connection is accepted, but later closed). I'm attaching the work for this attempted test in case anyone wants to look at it.

Since the mochitests listen on port 80, we probably can't set another one up and since 127.0.0.1 is already used I'm out of ideas of how to set up another server on port 80 that is accessible via mochitests. Maybe there's a way to use an xpcshell test and a one-off HTTP/HTTPS server to test this, but I'm not sure how that might work.

Does anyone have suggestions of how to create automated tests for this bug?

Could you launch a server on https://[::1]:443/ and try to connect to http://[::1]/? (Or https://127.0.0.13:443/)

In a private conversation, :darkspirit suggested using another loopback interface (like maybe 127.0.0.2 instead of 127.0.0.1) for this test so we can have different server behaviors based on port at the socket layer instead of relying on HTTP host multiplexing. I'll try to investigate later this week.

I investigated more, but it appears to be a substantial undertaking to test using xpcshell (requires making/standing up a server) or to add multi-IP support to mochitests, and this bug is too trivial in my opinion to warrant that much change to the test suites. I also looked more into the URI fixup tests, but this is an additional step beyond URI fixup, and URI fixup doesn't handle hosts that reject connections for which we want to try https upgrades (they are valid/live URIs).

mayhemer: do you happen to have any idea if there's a way to easily test nsDocShell with an HTTP server that can reject attempted connections on port 80? A manual test is described in comment 46, and browser mochitest didn't do this out of the box.

In summary, my goal is to:

1. attempt connection to some host via urlbar entry (unspecified port/scheme, defaulting to 80/http),
2. let the server reject the attempted connection on port 80 (can be a quick socket accept()/close()), then
3. check if nsDocShell re-attempts or "fixes up" to a connection to the same host name using 443/https.

The test can either make a successful connection via https, or the server can reject it too -- I just need to test that nsDocShell tries 443/https if the host is alive but server rejects connections to 80/http.

Hi Sid! We added support for 127.0.0.2 in the test server in https://hg.mozilla.org/mozilla-central/rev/f8632635a60c#l5.14 - would that work? (it gets used here: https://searchfox.org/mozilla-central/rev/54f965e51e4f77866bec42737978d40d4c1fdfc5/dom/webauthn/tests/browser/browser_webauthn_ipaddress.js#38)

Keeler, my hero! Yes, that works beautifully. It's been so long since I wrote a test for Firefox, would you be so kind as to give me a bit of feedback on the short tests?

Thanks, :keeler!

Limited try run to sanity check:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=9023fdbdb425464e68fe50fe7b7023361d6ac27d

Based on the try run, looks like a number of tests are failing because they're upgrading localhost or 127.0.0.1 URLs to https.
For reference, some of the browser tests that are failing due to some URL starting with https instead of http:

• browser/base/content/test/general/browser_decoderDoctor.js
• services/fxaccounts/tests/browser/browser_device_connected.js
• services/fxaccounts/tests/browser/browser_verify_login.js
• toolkit/components/passwordmgr/test/browser/browser_doorhanger_autofill_then_save_password.js
• toolkit/mozapps/extensions/test/browser/browser_html_detail_view.js

I originally thought this had something to do with the speculative loads pre-fetching https versions and then the docshell preferring those over http, so I removed the speculative loading bits but the tests still failed (because https instead of http). So probably it's not due to the speculative load bits.

If I run mochitests with --no-autorun, then manually try to load localhost or 127.0.0.1 without my patches applied, the connections are refused (even when example.com and 127.0.0.1:8888 do load). So that leads me to think these failing tests are expecting a url that doesn't necessarily load but gets built and put into the URL bar (which my patch will try to rewrite if the connection is refused).

But it's not clear to me why localhost and 127.0.0.1 don't properly load mochitest stuff despite being present in server-locations.txt:
https://searchfox.org/mozilla-central/source/build/pgo/server-locations.txt#65
https://searchfox.org/mozilla-central/source/build/pgo/server-locations.txt#317

Dana, do you happen to know off the top of your head why these URLs won't load but other ones do? This feels related to your 127.0.0.2 suggestion...

Sorry for my naive question:
(1) Does the current patch upgrade all failed connections if mAllowKeywordFixup is true? It doesn't seem to check for top frame.

(2) If above does not apply and these tests just want their error page, they should either
a) disable a browser.fixup.fallback-to-https pref before running or
b) be changed to a hard-coded domain that won't be upgraded, e.g. http://offline.invalid/.

A dedicated pref might anyway be useful for debugging.

(1) Does the current patch upgrade all failed connections if mAllowKeywordFixup is true?

Yes, I was attempting to upgrade as much as possible when port 80 connections are rejected. Maybe that's a mistake, but the test failures are all top-level URLs (you can see them in the url bar as the test runs). I'm still confused, however, about why localhost:80 doesn't serve data for the mochitests.

(2) If above does not apply and these tests just want their error page, they should either
a) disable a browser.fixup.fallback-to-https pref before running or
b) be changed to a hard-coded domain that won't be upgraded, e.g. http://offline.invalid/.

A dedicated pref might anyway be useful for debugging.

This seems reasonable, but I'd hate to add more complexity to the browser if we don't need to. Maybe I'll make another patch for the pref just in case. Thanks for the idea!

It looks like the test infrastructure doesn't listen on port 80 while running, so it makes sense that we can't connect to it. That said, I don't know why the test suite attempts to load those URLs (and why the tests pass even if those fail to load?). Maybe those tests need to be fixed? e.g. maybe we need to specify 127.0.0.1:8888 here: https://searchfox.org/mozilla-central/rev/dc4560dcaafd79375b9411fdbbaaebb0a59a93ac/browser/base/content/test/general/browser_decoderDoctor.js#188

Good idea! I tweaked the failed tests (changing the hostname or scheme worked like a charm locally) and am running on try again. Still a bunch of fails, but I'm not certain my patches caused these since many appear to be M-fis tests or intermittent oranges.

Still investigating, here's the try run:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=30df1250f3f81c089f46e22175f23c10dcdd0e98

It appears most of the failures in that try run were intermittent and likely due to my random selection of a mozilla-central revision upon which to apply my patches. There are only a couple of test failures I'm worried about:

• My new test is failing on Mac OS X
• toolkit/components/reader/test/browser_readerMode.js is timing out (that might be due to another revision's changes, not sure how it is related)

I'm not sure how to investigate the Mac OS X failure (don't have Mac OS), and could use help from someone who does and is willing to apply the patches in this bug or in the try run linked above in comment 61.

Looks pretty good rebased on yesterday's mozilla-central:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=6630da75f16538784f8be93ebe7cb7d471e2ad61

The Mac OS X timeouts seem to occur in various tests of the same chunk, so I'm not sure if that's a big problem.

Depends on D75083

Depends on D75084

Depends on D75085

Depends on D75086

Depends on D75087

I requested review from various people on D75088, D75087, and D75086 because I want folks who know stuff about those tests to verify these changes are indeed appropriate for those tests (see comment 60).

:smaug - let me know if you would like to offload the review to someone else who can approve docshell changes. I wasn't sure who to target and :ckerschb recommended you.

:jaws - please take a look at the minor changes in D75086 and let me know if it's ok to change hostnames in those test URLs.

:mstriemer - do you think this itty-bitty URL change in the browser_html_detail_view.js test (see D75088) is acceptable? I can't imagine it will cause any problems...

Thanks for the review, :jaws!

Backed out for bustage on nsDocShell.cpp

backout: https://hg.mozilla.org/integration/autoland/rev/7ecaba5a83ce08a17eaa5b9b7976ed8f8e44894c

push: https://treeherder.mozilla.org/#/jobs?repo=autoland&revision=d481cf074d3baa856f8bdf8b50676fbbfa9b4158&searchStr=build&selectedTaskRun=QaMEfIJ1Sd2nYKsKgtKABw-0

failure log: https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=302745457&repo=autoland&lineNumber=33442

[task 2020-05-18T12:59:40.117Z] 12:59:40 INFO - In file included from Unified_cpp_docshell_base0.cpp:92:
[task 2020-05-18T12:59:40.117Z] 12:59:40 ERROR - /builds/worker/checkouts/gecko/docshell/base/nsDocShell.cpp:6121:13: error: ignoring return value of function declared with 'nodiscard' attribute [-Werror,-Wunused-result]
[task 2020-05-18T12:59:40.117Z] 12:59:40 INFO - NS_MutateURI(url)
[task 2020-05-18T12:59:40.117Z] 12:59:40 INFO - ^~~~~~~~~~~~~~~~~
[task 2020-05-18T12:59:40.117Z] 12:59:40 INFO - 1 error generated.
[task 2020-05-18T12:59:40.117Z] 12:59:40 INFO - /builds/worker/checkouts/gecko/config/rules.mk:746: recipe for target 'Unified_cpp_docshell_base0.o' failed
[task 2020-05-18T12:59:40.117Z] 12:59:40 ERROR - make[4]: *** [Unified_cpp_docshell_base0.o] Error 1
[task 2020-05-18T12:59:40.117Z] 12:59:40 INFO - make[4]: Leaving directory '/builds/worker/workspace/obj-build/docshell/base'
[task 2020-05-18T12:59:40.117Z] 12:59:40 INFO - /builds/worker/checkouts/gecko/config/recurse.mk:74: recipe for target 'docshell/base/target-objects' failed
[task 2020-05-18T12:59:40.117Z] 12:59:40 ERROR - make[3]: *** [docshell/base/target-objects] Error 2
[task 2020-05-18T12:59:40.117Z] 12:59:40 INFO - make[3]: *** Waiting for unfinished jobs....

Whoops, sorry about the bustage. Testing the fix:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=66e2f6528b5e29e2ad815f3d51150f05fc4f61cb

Backed out 6 changesets (Bug 1002724) for failing in browser_fall_back_to_https.js CLOSED TREE

Push with failures: https://treeherder.mozilla.org/#/jobs?repo=autoland&group_state=expanded&selectedTaskRun=MzYw5pIuTFC1iMM_RMeXdA-0&resultStatus=testfailed%2Cbusted%2Cexception&revision=e487b4cd9223b85a7fc4a70090f0d6cf15e35e7c

Failure log: https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=302769989&repo=autoland&lineNumber=21131

Backout: https://hg.mozilla.org/integration/autoland/rev/807de85fcff40339ed6d24a19b62ca28ff7b2712

Argh, looks like the Mac OS X timeouts are consistent and need to be fixed prior to trying to land this again. I'm not sure how to debug this without a Mac (I don't have easy access to one), except maybe using try as a test machine... which sounds really annoying.

:keeler - do you know if there are issues with mochitests using 127.0.0.2 on mac (exclusively)?

What I'm seeing on my mac is the TCP SYN packets to 127.0.0.2 are timing out instead of getting reset/closed. After some googling, it looks like loopback addresses other than 127.0.0.1 aren't enabled by default on macOS (?) so maybe this test isn't even possible as-is (or maybe we want to modify the patch to try upgrading to https on timeouts as well?). So one approach right now would be to mark that test as skip-if = os == 'mac' in the test manifest. Another thing we could potentially do is to modify our test machines to add an interface alias for that address. I don't know how feasible that is, though.

Skipping on macOS sounds right. Fallback on timeout could be a further enhancement.

Thanks, Dana. I was afraid of that.... seems a bit overkill to me to change the test infrastructure for this little bug.

:smaug - are you still ok with these docshell changes if we don't test on Mac? If you are, I will edit the ini file to skip the browser chrome tests on Mac and re-flag you for review on the test (or not if you say it's ok)...but I promise to wait for a try run that doesn't fail on Mac before trying to land it again!

Is there any other way to test this on Mac? But if not, I think skipping on mac might not be totally horrible, since the code as such is cross-platform.

Short of some pretty serious changes to the test framework, we can't shoehorn this into browser chrome tests on Mac, and I couldn't find another way to test this patch in general without standing up a duplicate HTTP/HTTPS server -- seems ridiculous for one test.

We could try to tweak the test infrastructure environment on OS X to open up 127.0.0.2 (see comment 81), but I have no idea how to do that. I'd like to land this change with tests disabled on Mac, then I'll file a lower-priority follow-up bug to explore enabling the tests on Mac.

https://treeherder.mozilla.org/#/jobs?repo=try&revision=b8181099fafbf57c9660941d2c32d85e185e2e47

:smaug and I discussed this out of band and we're going to land this without tests on mac until we figure out how to test it on mac. I'll file a follow-up bug to investigate running that test on OS X when this code lands.

https://hg.mozilla.org/mozilla-central/rev/11515373dbd2
https://hg.mozilla.org/mozilla-central/rev/9b6944448186
https://hg.mozilla.org/mozilla-central/rev/71cdc10cc959
https://hg.mozilla.org/mozilla-central/rev/d643f8c4a1a1
https://hg.mozilla.org/mozilla-central/rev/cb1683a32099
https://hg.mozilla.org/mozilla-central/rev/22855ae7fa00