Closed Bug 1220810 (let-localhost-be-localhost) Opened 8 years ago Closed 3 years ago

Consider hardcoding localhost names to the loopback address

Categories

(Core :: Networking: DNS, enhancement, P3)

enhancement

Tracking

()

RESOLVED FIXED
84 Branch
Webcompat Priority ?
Tracking Status
firefox84 --- fixed

People

(Reporter: jwatt, Assigned: baku)

References

(Blocks 3 open bugs)

Details

(Keywords: dev-doc-complete, perf-alert, Whiteboard: [necko-active])

Attachments

(4 files, 2 obsolete files)

See bug 1162772 comment 15. https://tools.ietf.org/html/rfc6761 suggests that we can treat "localhost" specially and should hardcode localhost names to the loopback address:

  "Application software MAY recognize localhost names as special"

and:

  "Name resolution APIs and libraries SHOULD recognize localhost
   names as special and SHOULD always return the IP loopback address
   for address queries and negative responses for all other query
   types.  Name resolution APIs SHOULD NOT send queries for
   localhost names to their configured caching DNS server(s).)."

Not doing that is concerning:

https://twitter.com/sleevi_/status/649025202722967553

Can we do this?

Backstory: I'm implementing an API that allows consumers to determine whether a document was loaded over TLS, or otherwise transported in a way that guarantees authentication and integrity. I can whitelist 127.0.0.1, but I'd also like to whitelist localhost names for the convenience of web developers who are developing content on a local server. If we don't guarantee that they resolve to a loopback address I can't do that though.
Flags: needinfo?(mcmanus)
(Maybe an alternative for me is to use nsIDNSService::Resolve with the RESOLVE_OFFLINE flag, but there's always the possibility that the response at the time I call it was different than at the time the resource was loaded.)
(And it would be great if nsIDNSService had an IsLoopBackAddress method that took an nsIURI*. I see there's already a method in DNS.h by that name, but it needs an IP4/IP6 address.)
One problem with hardcoding localhost to be 127.0.0.1 is that it locks you into ipv4.. and conceivably someone could be running a v6 service on only ::1 and be setup that way with the local resolver. (its not that there is a v6/v4 advantage in doing so, but if they've set thing up to bind that way we would break them).

I don't like the idea of resolving the hostname before calling asyncopen either. time of check time of use would be a problem with round robin, etc..

There is also the matter of cached or offline operation. Addresses are lost then.

In general, the security model is defined by origin right .. are we sure localhost isn't sufficient here? If someone has overloaded localhost that would seem to be under local control and acceptable; no?
Flags: needinfo?(mcmanus)
We recently made a similar change in Chrome. Right now we always resolve local hostnames to 127.0.0.1/::1 because we found that queries for "localhost.", "foo.localhost", etc. could go out over the network on several platforms, and/or resolve to non-loopback IPs. In some cases this would be due to an obvious local change (such as resolving localhost to an external IP in /etc/hosts), but not always (such as "foo.localhost" or a suffix search for "localhost").

In Chrome's async DNS resolver we resolve to both 127.0.0.1 and ::1 if the query doesn't specify one or the other. Would that prevent breakage in the case of an ipv6 service that mcmanus mentioned? (i.e. would Firefox try ::1 if 127.0.0.1 fails to connect?)

There are other problems that arise with the approach that Chrome has right now, though. See https://crbug.com/510124, for example. I like sleevi's suggestion there of filtering the addresses returned by the system resolver when possible, rather than skipping the system resolver all together.
Jet asked offline if this was a blocker for bug 1162772. After talking with rbarnes, we don't think so. We can just implement per the spec:

"In particular, the user agent SHOULD treat file URLs and URLs with hostnames names equivalent to localhost as potentially trustworthy." [1]

This would seem to fit with mcmanus' comment #3 - localhost should be sufficient here. Maybe there's an optimization possible, but it shouldn't be a blocker for the secure context API.

[1] https://w3c.github.io/webappsec-secure-contexts/
It may not technically block, but I think it's still useful to have that relationship linked in case the question comes up in bug 1220687.
Blocks: 1220687
I don't think I would take this (comment 3) unless it becomes necessary for webcompat. So I'm going to mark it wontfix. If there is a pending patch or strategy (rather than a speculative one) for which its necessary lets talk about what the constraints around that could be in a concrete way. but in general, the os should define what names mean.
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → WONTFIX
So I believe the resolution here means that we should stop treating 'localhost' as secure, per:

https://w3c.github.io/webappsec-secure-contexts/#localhost

Is that correct, Patrick?
Flags: needinfo?(mcmanus)
See Also: → 1346835
Hey folks!

It would be really helpful to get y'all's input on the localhost draft David noted above. There's an ongoing thread in the DNSOP working group (https://www.ietf.org/mail-archive/web/dnsop/current/threads.html#20661) where Richard Barnes has been actively participating. I'd forgotten that he's no longer representing Mozilla, so I didn't think to ping other folks explicitly. :( It would be quite nice if y'all would join in with your opinions (especially if they match mine and Richard's. :) ).
Reopening for McManus to reconsider in the light of the above-linked IETF discussions (and the fact that he seems to be the only one who doesn't want to do this :-) )
Status: RESOLVED → REOPENED
Resolution: WONTFIX → ---
Assignee: nobody → mcmanus
Whiteboard: [necko-active]
DNSOP has opened a Call for Adoption on the document discussed above: https://www.ietf.org/mail-archive/web/dnsop/current/msg20963.html. If Mozilla has some feedback, positive or negative, that would be an excellent thread on which to register it. Comments on adoption are open through the 20th... :)
Bulk priority update: https://bugzilla.mozilla.org/show_bug.cgi?id=1399258
Priority: -- → P1
Ping. Today's the end of the call for adoption for the draft. It would be a lovely time to get some feedback in, if y'all have any. :)
DNSOP has moved the localhost draft discussed above to Last Call: https://mailarchive.ietf.org/arch/msg/dnsop/fz5vTiTgH5V7buUkH8adTy2-_5Y

If y'all have feedback, now would be a lovely time to hop into the conversation. :)
:mt do we have a stance on this IETF proposal before comments close on it, It seems like Patrick wasn't keen on it.
Flags: needinfo?(martin.thomson)
I consider this optional for us.  It's only important if we think that we need to mark http://localhost as a secure context.

I'd prefer to do something else/better for people who are developing sites.  But I'll talk to mcmanus.
Flags: needinfo?(martin.thomson)
> I'd prefer to do something else/better for people who are developing sites.

Yeah we have the meta Bug 1410365 specifically for capturing that. :)

> It's only important if we think that we need to mark http://localhost as a secure context.

I'm fairly certain Chrome and Firefox already do this for the webidl meaning of SecureContext given I can access geolocation in both browsers. I think this is what Mike is trying to address with this spec change, either that or we could start treating localhost as not secure once we have other avenues for developers here.
> I consider this optional for us. It's only important if we think that we need to
> mark http://localhost as a secure context.

That latter bit is something that y'all are already doing, as jkt notes. It's also something that dveditz@, et al have been supportive of in the WebAppSec WG. If it's not something Mozilla actually wants to be doing, then we need to revisit a few more decisions. I'm not terribly enthusiastic about doing so, given the feedback I've gotten from developers. :)

I'd also note that there are some additional considerations from sunset4 noted in the document's introduction around hardcoding loopback addresses that seem reasonable to address from a developer perspective.
it seems that we're committed to the notion that http://localhost is a secure context. I would have preferred to have found an answer for developers that doesn't use a http:// scheme but I'll acknowledge being in the rough. (as a side note we definitely have features that are restricted to https and need to be migrated to use secure context for consistency).

As mt says, this is impt (mostly?) for sc.

I also appreciate that Mike has taken the time and effort to put this through DNS OP - which is where this kind of deviation should live. We can add it to the .onion catalog of weird exceptions.

This was characterized as hardcode localhost to 127.0.0.1 (perhaps just in the tweet) and that formed part of my concern about 127 vs :1.. emily's comments correctly point to a gecko solution where this can be implemented a bit deeper in the resolver where we are already committed to a/quad-a and therefore return both where appropriate.

So based on current events, I'll adjust my position and say I'm willing to merge a patch along those lines unless we've got the stomach to fight the web-compat issues and go a different way on securecontexts. (reverting what we've got).
Flags: needinfo?(mcmanus)
Moving to p3 because no activity for at least 24 weeks.
Priority: P1 → P3
See Also: → CVE-2018-18506
As it currently stands we don't exempt localhost like we do for 127.0.0.1 from upgrade insecure and mixed content blocking.
See Also: → 1488740

Firefox 69 marks both 127.0.0.1 AND localhost as Secure Context.
What is missing, is support for *.localhost (see details in https://bugzilla.mozilla.org/show_bug.cgi?id=1433933#c6)

Assignee: mcmanus → nobody
Status: REOPENED → NEW
Type: defect → enhancement

The uploaded patch is just a WIP however it also resolves a few other issues including Bug 1488740 and that we don't relate this code to the proxying code and that we have a separate allowlist there.

Blocks: 1488740
See Also: 1488740

:estark37 & :mkwst is there any interest in Chrome implementing support for alternate ipv4 loopback addresses as mentioned in Comment 4? I didn't implement this purely because I didn't want a compatibility issue.

Now that I have made ipv6 resolve to ::1 Try is even less happy as there are a few tests that require the ability to proxy localhost. This we can fix with the hijack pref however we also then treat it as insecure when it's proxied which I think is the right thing to do however some of the tests require this also.
Try run: https://treeherder.mozilla.org/#/jobs?repo=try&revision=a3e647c764c9abca31e0524f108dd34e9e50cf28

Flags: needinfo?(mkwst)
Flags: needinfo?(estark37)

Since this bug is open for more than 4 years I want to stress this:
We are affected here by using atlassian companion app for attachment editing in confluence. A recent problem made itnecessary for attlassian to use non-secure localhost content here. It doesn't work right now with ESR version without setting network.websocket.allowInsecureFromHTTPS to true which would mean more security risks. My guess is that this is affecting many companies that are still using firefox.

:estark37 & :mkwst is there any interest in Chrome implementing support for alternate ipv4 loopback addresses as mentioned in Comment 4? I didn't implement this purely because I didn't want a compatibility issue.

We currently resolve localhost to 127.0.0.1 and ::1 as Emily noted. It seems reasonable to extend that to support any /etc/hosts assertions that fall into 127.0.0.0/8, but that's not something we've heard a whole lot of interest in, though there have certainly been one-off requests. I don't think I'd prioritize that work, given the way things have played out over the last ~4 years, but I don't think it's a bad idea.

Flags: needinfo?(mkwst)
Flags: needinfo?(estark37)

Given that I'm unlikely to have long left to work on this I'll continue with the hardcoding of loopback instead based on Comment 28. I think this is sufficient in reaching our short term goal here and given 4 years have passed with Chrome I think we can say the interest isn't perhaps worth the additional time.

Try push here: https://treeherder.mozilla.org/#/jobs?repo=try&revision=1f81c50581ed3b3b1c34f71dfda10acb27bfa3e9

dragana + ckerschb, I don't know if we can r+ patches outside of phab however this is the same patch with the amendments made. I ditches the effort to try and resolve the address first and filter out non loopback.

I'm still waiting on try which I think has many failures still due to tests expecting loopback to be proxyable whilst also being secure. However if the patch is ok I can clean up these tests, either by removing them or making them use a different hostname perhaps.

Assignee: nobody → jonathan
Flags: needinfo?(dd.mozilla)
Flags: needinfo?(ckerschb)
Comment on attachment 9121260 [details] [diff] [review]
bug-1220810.patch

Review of attachment 9121260 [details] [diff] [review]:
-----------------------------------------------------------------

I think overall this looks fine, obviously dragana is the Necko peer to accept that change. I would like to see a changeset which has all the test changes incorporated needed to get that change landed. thanks!

::: browser/base/content/test/siteIdentity/browser_identityIcon_img_url.js
@@ +65,5 @@
>    {
>      type: "localhost + http frame",
>      testURL: kBaseURILocalhost + "file_csp_block_all_mixedcontent.html",
> +    // The hijack pref makes this insecure
> +    img_url: `url("chrome://browser/skin/connection-mixed-active-loaded.svg")`,

Doesn't that file need to be registered as supportfile for that test in some way?

::: netwerk/dns/DNS.cpp
@@ +173,5 @@
> +  if (StaticPrefs::network_proxy_allow_hijacking_localhost()) {
> +    return false;
> +  }
> +
> +  if (aAsciiHost.EqualsLiteral("localhost") ||

is it possible that we end up aAsciiHost being "Localhost" or something there like?
Probably better to use LowerCaseEqualsASCII

::: netwerk/dns/nsHostResolver.cpp
@@ +823,5 @@
> +  }
> +
> +  RefPtr<AddrInfo> ai;
> +  GetAddrInfo(rec->host, rec->af, addrRec->flags, getter_AddRefs(ai),
> +              addrRec->mGetTtl);

What if GetAddrInfo fails?
Flags: needinfo?(ckerschb)
Blocks: 1346835
  • img_url: url("chrome://browser/skin/connection-mixed-active-loaded.svg"),

Doesn't that file need to be registered as supportfile for that test in some
way?

No it's not needed. It's not part the test, it's a required image of the browser package. I'm going to push this patch + your comments to phab.

Pushed by amarchesini@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/f01596089356
Hardcode localhost to loopback, r=ckerschb,dragana
Backout by malexandru@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/2bf5bbb84993
Backed out changeset f01596089356 for causing crashes in test_performance_attributes_exist_in_object.html
Depends on: 1436778

There's a r+ patch which didn't land and no activity in this bug for 2 weeks.
It would be great for developers if this could land.
Is the assigned field still correct?

Flags: needinfo?(jonathan)

Landing mentioned patch is also crucial for providing Secure Context with proper Origin separation for websites loaded from a local IPFS node via *.ipfs.localhost URLs.

(Chromium already follows RFC6761 and it works as expected, but Firefox 75 still hits the OS DNS resolver, so we need to use HTTP proxy for *.localhost to ensure subdomains work on systems with simpler DNS resolvers, such as Debian 10. And, for some reason, subdomains are missing Secure Context attribute, which is present on localhost itself)

Any way we can help getting this across the finish line?

Hm, looks like the test crash is caused by bug 1436778, which baku noted with that dependency... however, bug 1436778 is a two year old hard-to-repro crash bug :(

Adding request for adding to webcompat list, since this works as expected in Chromium.

(In reply to Marcin Rataj from comment #41)

And, for some reason, subdomains are missing Secure Context
attribute, which is present on localhost itself)

Hm, separate bug?

Webcompat Priority: --- → ?

(In reply to Dietrich Ayala (:dietrich) from comment #42)

(In reply to Marcin Rataj from comment #41)

And, for some reason, subdomains are missing Secure Context
attribute, which is present on localhost itself)

Hm, separate bug?

That looks like it will be fixed by this bug. See https://hg.mozilla.org/integration/autoland/rev/f01596089356#l13.80

Matthew is right. The idea is that web developers can use localhost and dev.localhost and really [anything.including.multiple.labels].localhost on their own machine and get a secure context. This will make web development a bit simpler again, especially given https://blog.mozilla.org/security/2018/01/15/secure-contexts-everywhere/.

This does not solve multiple web developers wanting to use a local server together. In that case Mozilla's advise would be to acquire a test domain name, point it to that server, and use Let's Encrypt or other CA to get certificates for that server.

Adding dev-doc-needed to ensure we put all of this on MDN and in the release notes as once this is fixed this will work at least across Chrome and Firefox which is a pretty big win for web developers.

Andrea, why does bug 1436778 block this? Tentatively assigning to you per the last patch.

Assignee: jonathan → amarchesini
Flags: needinfo?(jonathan)
Keywords: dev-doc-needed

With this patch, the lookup of "localhost" domains is faster. And this makes bug 1436778 more visible.
DocumentChannel introduces a regression in the way we start and count a few timers for the navigation.

I've recently switched to Firefox as my main browser during web development and the lack of built-in *.localhost support add non-trivial complication to the process of setting up the work station (e.g. after a clean OS install). Having to run your own DNS service and/or listing each service manually with sudo-edited /etc/hosts isn't great :)

Flags: needinfo?(dd.mozilla)

(In reply to Andrea Marchesini [:baku] from comment #45)

With this patch, the lookup of "localhost" domains is faster. And this makes bug 1436778 more visible.
DocumentChannel introduces a regression in the way we start and count a few timers for the navigation.

Hi Baku, are you still working on that?

Flags: needinfo?(amarchesini)

This bug is a serious blocker for us in making localhost applications more secure in Firefox. Works in Chrome, which makes it very hard to recommend Firefox while this isn't fixed.

Is this blocked on engineering availability only? Baku, if you do not foresee availability soon, maybe we can find help?

The blocker here is bug 1436778. We need to fix that before landing my patch.
If somebody has time to complete bug 1436778, it would be great.

Flags: needinfo?(amarchesini)

Why is this patch adding a new assertion for the PerformanceTiming constructor? That seems somewhat unrelated to the rest of the changes.

The failing test here (test_performance_attributes_exist_in_object.html) is checking PerformanceTiming for a Document loaded within an <object>.

<object> doesn't create a docshell until we get a response from the network (and know that it's a Document content type), so it's somewhat expected that connect start would be before navigation start.

Flags: needinfo?(amarchesini)
Pushed by amarchesini@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/36f368ba214c
Hardcode localhost to loopback, r=ckerschb,dragana
Backout by malexandru@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/ae539955e454
Backed out changeset 36f368ba214c for causing gecko decision task bustages.
Pushed by amarchesini@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/1b44f13206d0
Hardcode localhost to loopback, r=ckerschb,dragana
Backout by malexandru@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/b34436389d46
Backed out changeset 1b44f13206d0 for causing gecko decision task bustages.
Pushed by amarchesini@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/1b035d80fb9f
Hardcode localhost to loopback, r=ckerschb,dragana
Flags: needinfo?(amarchesini)
Pushed by amarchesini@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/83749e9e67bd
Hardcode localhost to loopback, r=ckerschb,dragana
Attachment #9180089 - Attachment description: Bug 1220810 - Hardcode localhost to loopback, r?ckerschb → Bug 1220810 - Hardcode localhost to loopback, r=ckerschb,dragana

I've rebased the patch and was able to tweak the tests to make all but one (browser_fall_back_to_https.js) pass. I'm afraid I'm not familiar enough with the code to understand what's happening, any idea? See https://phabricator.services.mozilla.com/D92716#3020565

Flags: needinfo?(dd.mozilla)
Flags: needinfo?(ckerschb)

(In reply to Frédéric Wang (:fredw) from comment #62)

I've rebased the patch and was able to tweak the tests to make all but one (browser_fall_back_to_https.js) pass. I'm afraid I'm not familiar enough with the code to understand what's happening, any idea? See https://phabricator.services.mozilla.com/D92716#3020565

It may not be related, but I did notice that in netwerk/test/unit/test_ping_aboutnetworking.js and dom/webauthn/tests/browser/browser_webauthn_ipaddress.js, the corresponding clearUserPref("network.proxy.allow_hijacking_localhost") is missing.

(In reply to Mathew Hodson from comment #63)

(In reply to Frédéric Wang (:fredw) from comment #62)

I've rebased the patch and was able to tweak the tests to make all but one (browser_fall_back_to_https.js) pass. I'm afraid I'm not familiar enough with the code to understand what's happening, any idea? See https://phabricator.services.mozilla.com/D92716#3020565

It may not be related, but I did notice that in netwerk/test/unit/test_ping_aboutnetworking.js and dom/webauthn/tests/browser/browser_webauthn_ipaddress.js, the corresponding clearUserPref("network.proxy.allow_hijacking_localhost") is missing.

Right, I think they probably should. Note that I didn't change or check these two tests in details, this was just copied from the original https://phabricator.services.mozilla.com/D64586. I don't know exactly how prefs are reset between tests, but in any case that probably does not matter for browser_fall_back_to_https.js since 1) I was running this single test locally 2) it fails whether or not the pref is enabled.

(In reply to Frédéric Wang (:fredw) from comment #62)

I've rebased the patch and was able to tweak the tests to make all but one (browser_fall_back_to_https.js) pass. I'm afraid I'm not familiar enough with the code to understand what's happening, any idea? See https://phabricator.services.mozilla.com/D92716#3020565

The test was added for Bug 1002724 - the idea is that entering example.com defaults to http//example.com but in case :80 does not respond, then it should fall back to using https:example.com with port :443. I think the problem is that we don't have any test servers in server-locations.txt that do not respond to port :80 and hence the test uses this workaround to automatically test the fall back to https.

Two options:

  • We find some host we can use besides example.com which does not respond to port :80 but to port :443
  • Maybe (I am saying maybe) we can just flip the pref to false for allow_hijacking_localhost, but it's better to check with a Necko person.
Flags: needinfo?(ckerschb)

Thanks Christoph.

Just to clarify browser_fall_back_to_https.js has two subtests:

  • example.com redirects to http://example.com ; this one still passes with the patch
  • 127.0.0.2 redirects https://127.0.0.2 is broken (whether or not the allow_hijacking_localhost flag is enabled)

(In reply to Christoph Kerschbaumer [:ckerschb] from comment #65)

Two options:

  • We find some host we can use besides example.com which does not respond to port :80 but to port :443

Trying that (with 128.0.0.1 or www.itisatrap.org) I'm hitting an ASSERT saying that non-local addresses are disabled:
https://searchfox.org/mozilla-central/source/netwerk/base/nsSocketTransport2.cpp#1331

We can just try to skip that ASSERT, but then we would still need to have a non-local address that rejects :80 and accepts :443 (and without a server config that redirects from :80 to :443).

  • Maybe (I am saying maybe) we can just flip the pref to false for allow_hijacking_localhost, but it's better to check with a Necko person.

As I previously said, the test fails whatever the state of allow_hijacking_localhost, but I can't explain it.

I'll take a quick look at the test.

Flags: needinfo?(valentin.gosu)

To clarify, the test is failing with your patch but it is working fine without you patch?
can you make a http log with your patch and allow_hijacking_localhost true and another log with allow_hijacking_localhost==false?

Flags: needinfo?(dd.mozilla)

(In reply to Dragana Damjanovic [:dragana] from comment #69)

To clarify, the test is failing with your patch but it is working fine without you patch?

I haven't tested without the patch but I assume that it passes with the trunk behavior. And yes it has been failing with the patch since comment 53.

can you make a http log with your patch and allow_hijacking_localhost true and another log with allow_hijacking_localhost==false?

Sure, I'll give another try later.

(In reply to Frédéric Wang (:fredw) from comment #70)

(In reply to Dragana Damjanovic [:dragana] from comment #69)

To clarify, the test is failing with your patch but it is working fine without you patch?

I haven't tested without the patch but I assume that it passes with the trunk behavior. And yes it has been failing with the patch since comment 53.

Sorry I meant https://bugzilla.mozilla.org/show_bug.cgi?id=1220810#c59

I applied your patch and this diff:

diff --git a/docshell/test/browser/browser_fall_back_to_https.js b/docshell/test/browser/browser_fall_back_to_https.js
--- a/docshell/test/browser/browser_fall_back_to_https.js
+++ b/docshell/test/browser/browser_fall_back_to_https.js
@@ -57,7 +57,7 @@ async function test_one(test_obj) {
 add_task(async function test_bug1002724() {
   await SpecialPowers.pushPrefEnv(
     // Disable HSTS preload just in case.
-    { set: [["network.stricttransportsecurity.preloadlist", false]] }
+    { set: [["network.stricttransportsecurity.preloadlist", false], ["network.proxy.allow_hijacking_localhost", true]] }
   );
 
   for (let test of bug1002724_tests) {

And the test started passing. 🤷

Flags: needinfo?(valentin.gosu) → needinfo?(fred.wang)

Clarification: it only passes if there is no server running on my machine on port 80. 🤔

If your local Linux web server is available on 127.0.0.2:80, the test fails as expected. The test expects the http:// connection to fail to retry via https.
Or does network.proxy.allow_hijacking_localhost unexpectedly rewrite http://127.0.0.2 to http://127.0.0.1?

(If MacOS users can't repro anything: At the moment, the test can't run on macOS as it doesn't support 127.0.0.2 by default: bug 1002724 comment 81).

(In reply to Valentin Gosu [:valentin] (he/him) from comment #73)

Clarification: it only passes if there is no server running on my machine on port 80. 🤔

Thanks. So I had a Apache server running on the two Linux machines I was running the tests on. And it seems by default http://127.0.0.2/ redirects to the Apache default page, so this explains why the test is failing... Stopping the Apache server makes the test pass. I'll try to send the path to the try server with the flag enabled (I think we haven't done that yet, I was only testing locally so far).

Flags: needinfo?(fred.wang)

Adds secureonly.example.com:443 to server-locations.txt - this host is only available on HTTPS.
Regenerates certs using ./mach python build/pgo/genpgocert.py command.
Sets network.dns.native-is-localhost pref in test so we don't trigger assertion.

Attached file Bug 1220810 - Fix genpgocert.py (obsolete) —

Error:

$ ./mach python build/pgo/genpgocert.py
Traceback (most recent call last):
  File "build/pgo/genpgocert.py", line 208, in <module>
    certificateStatus = constructCertDatabase(build, certdir)
  File "build/pgo/genpgocert.py", line 99, in constructCertDatabase
    openssl = distutils.spawn.find_executable("openssl")
AttributeError: module 'distutils' has no attribute 'spawn'

Depends on D94005

The test should pass with the attached patches.
The problem was that 127.0.0.2 was not being proxied, or even when it was the proxy was still falling back to localhost for port 80.
Using a new special host for this test seems more appropriate.

Thanks that's what I wanted to do in comment 67. How about I just merge your patch in mine and we land everything together?

(also incidentally where can I reach people on matrix/element? I was not able to find your name in #Developers)

Flags: needinfo?(valentin.gosu)

(In reply to Frédéric Wang (:fredw) from comment #79)

Thanks that's what I wanted to do in comment 67. How about I just merge your patch in mine and we land everything together?

We can land a stack of commits at the same time.

(also incidentally where can I reach people on matrix/element? I was not able to find your name in #Developers)

I'm in the #necko channel.

Flags: needinfo?(valentin.gosu)

Comment on attachment 9182402 [details]
Bug 1220810 - Fix genpgocert.py

Revision D94006 was moved to bug 1672115. Setting attachment 9182402 [details] to obsolete.

Attachment #9182402 - Attachment is obsolete: true

Comment on attachment 9182401 [details]
Bug 1220810 - Fix browser_fall_back_to_https.js to use actual host r=ckerschb

Revision D94005 was moved to bug 1672127. Setting attachment 9182401 [details] to obsolete.

Attachment #9182401 - Attachment is obsolete: true
Blocks: 1672323
Pushed by btara@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/a5a35a205a44
Hardcode localhost to loopback, r=ckerschb,necko-reviewers,dragana

Backed out for causing test_dns_offline and fontface-override-descriptor-getter-setter failures

Backout link: https://hg.mozilla.org/integration/autoland/rev/14222fee2852881f1cf91aeb62110e52b2d2a91e

Push with failures: https://treeherder.mozilla.org/#/jobs?repo=autoland&collapsedPushes=805300&revision=a5a35a205a44328aaeaf42bb493e7173fbfe0ae2

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

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

"INFO - TEST-START | netwerk/test/unit/test_dns_offline.js
[task 2020-10-21T08:43:40.687Z] 08:43:40 WARNING - TEST-UNEXPECTED-FAIL | netwerk/test/unit/test_dns_offline.js | xpcshell return code: 0
[task 2020-10-21T08:43:40.687Z] 08:43:40 INFO - TEST-INFO took 381ms
[task 2020-10-21T08:43:40.688Z] 08:43:40 INFO - >>>>>>>
[task 2020-10-21T08:43:40.689Z] 08:43:40 INFO - PID 894 | [894, Unnamed thread 7faf7da6eee0] WARNING: XPCOM objects created/destroyed from static ctor/dtor: file /builds/worker/checkouts/gecko/xpcom/base/nsTraceRefcnt.cpp:202
[task 2020-10-21T08:43:40.690Z] 08:43:40 INFO - PID 894 | [894, Unnamed thread 7faf7da6eee0] WARNING: XPCOM objects created/destroyed from static ctor/dtor: file /builds/worker/checkouts/gecko/xpcom/base/nsTraceRefcnt.cpp:202
[task 2020-10-21T08:43:40.691Z] 08:43:40 INFO - PID 894 | Couldn't convert chrome URL: chrome://branding/locale/brand.properties
[task 2020-10-21T08:43:40.691Z] 08:43:40 INFO - PID 894 | [894, Main Thread] WARNING: Couldn't get the user appdata directory. Crash events may not be produced.: file /builds/worker/checkouts/gecko/toolkit/crashreporter/nsExceptionHandler.cpp:2949
[task 2020-10-21T08:43:40.692Z] 08:43:40 INFO - PID 894 | [894, Main Thread] WARNING: Couldn't get the user appdata directory, crash dumps will go in an unusual location: file /builds/worker/checkouts/gecko/toolkit/crashreporter/nsExceptionHandler.cpp:3008
[task 2020-10-21T08:43:40.693Z] 08:43:40 INFO - (xpcshell/head.js) | test MAIN run_test pending (1)
[task 2020-10-21T08:43:40.694Z] 08:43:40 INFO - (xpcshell/head.js) | test pending (2)
[task 2020-10-21T08:43:40.694Z] 08:43:40 INFO - (xpcshell/head.js) | test MAIN run_test finished (2)
[task 2020-10-21T08:43:40.695Z] 08:43:40 INFO - running event loop
[task 2020-10-21T08:43:40.696Z] 08:43:40 INFO - PID 894 | [894, Main Thread] WARNING: Could not get the program name for a cubeb stream.: 'NS_SUCCEEDED(rv)', file /builds/worker/checkouts/gecko/dom/media/CubebUtils.cpp:378
[task 2020-10-21T08:43:40.697Z] 08:43:40 INFO - PID 894 | [908, Unnamed thread 7f464176e040] WARNING: XPCOM objects created/destroyed from static ctor/dtor: file /builds/worker/checkouts/gecko/xpcom/base/nsTraceRefcnt.cpp:202
[task 2020-10-21T08:43:40.697Z] 08:43:40 INFO - PID 894 | [908, Unnamed thread 7f464176e040] WARNING: XPCOM objects created/destroyed from static ctor/dtor: file /builds/worker/checkouts/gecko/xpcom/base/nsTraceRefcnt.cpp:202
[task 2020-10-21T08:43:40.698Z] 08:43:40 INFO - PID 894 | [908, MainThread] WARNING: XPCOM objects created/destroyed from static ctor/dtor: file /builds/worker/checkouts/gecko/xpcom/base/nsTraceRefcnt.cpp:202
[task 2020-10-21T08:43:40.699Z] 08:43:40 INFO - PID 894 | [908, MainThread] WARNING: XPCOM objects created/destroyed from static ctor/dtor: file /builds/worker/checkouts/gecko/xpcom/base/nsTraceRefcnt.cpp:202
[task 2020-10-21T08:43:40.700Z] 08:43:40 INFO - PID 894 | [Socket 908, Main Thread] WARNING: 'NS_FAILED(rv)', file /builds/worker/checkouts/gecko/netwerk/protocol/http/nsHttpHandler.cpp:486
[task 2020-10-21T08:43:40.700Z] 08:43:40 INFO - PID 894 | [Socket 908, Main Thread] WARNING: NS_ENSURE_SUCCESS(rv, kKnownEsrVersion) failed with result 0x80004002 (NS_NOINTERFACE): file /builds/worker/checkouts/gecko/toolkit/components/resistfingerprinting/nsRFPService.cpp:673
[task 2020-10-21T08:43:40.701Z] 08:43:40 WARNING - TEST-UNEXPECTED-FAIL | netwerk/test/unit/test_dns_offline.js | onLookupComplete - [onLookupComplete : 17] 0 == 2152398864
[task 2020-10-21T08:43:40.702Z] 08:43:40 INFO - /builds/worker/workspace/build/tests/xpcshell/tests/netwerk/test/unit/test_dns_offline.js:onLookupComplete:17
[task 2020-10-21T08:43:40.703Z] 08:43:40 INFO - /builds/worker/workspace/build/tests/xpcshell/head.js:_do_main:248
[task 2020-10-21T08:43:40.703Z] 08:43:40 INFO - /builds/worker/workspace/build/tests/xpcshell/head.js:_execute_test:577
[task 2020-10-21T08:43:40.704Z] 08:43:40 INFO - -e:null:1
[task 2020-10-21T08:43:40.705Z] 08:43:40 INFO - exiting test"

Flags: needinfo?(fred.wang)

(In reply to Sandor Molnar from comment #84)

Backed out for causing test_dns_offline and fontface-override-descriptor-getter-setter failures

fontface-override-descriptor-getter-setter seems unrelated, probably it's bug 1669420

I'm not able to reproduce the other failures locally, even by running the whole netwerk/test/unit xpshells with a debug build. I see the management of pref in netwerk/test/unit/test_dns_originAttributes.js was not really good though. I'll investigate...

Flags: needinfo?(fred.wang)

Remaining failures happen with flag network.http.network_access_on_socket_process enabled for the following tests:

netwerk/test/unit/test_dns_offline.js
netwerk/test/unit/test_dns_originAttributes.js

The patch also forces network.proxy.allow_hijacking_localhost to true for these. The failing assertions are the ones expecting a resolution status=NS_ERROR_OFFLINE, when the actual value is status=0.

Any quick idea why it happens and/or which part of the code I should investigate/modify?

Thanks!

Flags: needinfo?(valentin.gosu)
Flags: needinfo?(dd.mozilla)

The socket process isn't enabled yet by default.
In the interest of landing this soon I'd suggest disabling the tests on socket process like this

Flags: needinfo?(valentin.gosu)
Pushed by rmaries@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/dfcb025567da
Hardcode localhost to loopback, r=ckerschb,necko-reviewers,dragana
Status: NEW → RESOLVED
Closed: 8 years ago3 years ago
Resolution: --- → FIXED
Target Milestone: --- → 84 Branch

(In reply to Sandor Molnar from comment #84)

Backed out for causing test_dns_offline and fontface-override-descriptor-getter-setter failures

Backout link: https://hg.mozilla.org/integration/autoland/rev/14222fee2852881f1cf91aeb62110e52b2d2a91e

== Change summary for alert #27329 (as of Fri, 23 Oct 2020 09:22:47 GMT) ==

Improvements:

Ratio Suite Test Platform Options Absolute values (old vs new)
13% build times linux64-shippable nightly taskcluster-m5dn.4xlarge 2,877.45 -> 2,498.69

For up to date results, see: https://treeherder.mozilla.org/perf.html#/alerts?id=27329

Blocks: 1673315
Flags: needinfo?(dd.mozilla)
Flags: needinfo?(amarchesini)

FYI Docs for this have been captured in new section Mixed content > Loading locally delivered mixed-resources, in the bullet point

After this fix, although I point my dev.localhost hostname to the WSL ip inside hosts file, it is not working. From Chrome any *localhost domain will see the WSL without even requiring anything in the hosts file, from FF it's not working. Any ideas?

(In reply to liakoskonstantin from comment #92)

After this fix, although I point my dev.localhost hostname to the WSL ip inside hosts file, it is not working. From Chrome any *localhost domain will see the WSL without even requiring anything in the hosts file, from FF it's not working. Any ideas?

I have the same problem with .localhost in FF.

liakoskonstantin, Ivan, does it work if you flip network.proxy.allow_hijacking_localhost in about:config? I guess either way this requires some kind of follow-up bug since we wanted to match Chrome.

Frédéric, thoughts?

Flags: needinfo?(fred.wang)

(In reply to liakoskonstantin from comment #92)

After this fix, although I point my dev.localhost hostname to the WSL ip inside hosts file, it is not working. From Chrome any *localhost domain will see the WSL without even requiring anything in the hosts file, from FF it's not working. Any ideas?

(In reply to Ivan from comment #93)

I have the same problem with .localhost in FF.

I believe this is working as intended. *.localhost are now hardcoded to the loopback device and any attempts to reroute it should be ignored. This is necessary in order to consider http://*.localhost and http://localhost trusted contexts for the purpose of accessing a number of browser features (like USB access for example). If localhost may not point at the loopback device, then it can no longer be considered a trusted context by the browser and thus those features have to be disabled.

(In reply to Anne (:annevk) from comment #94)

liakoskonstantin, Ivan, does it work if you flip network.proxy.allow_hijacking_localhost in about:config? I guess either way this requires some kind of follow-up bug since we wanted to match Chrome.

Frédéric, thoughts?

You are awesome. Yes, with this little hack it worked.

Worth noticing though that in my laptop it worked without issues, the problem was with my desktop. As far as I checked, I had the same settings in both. Couldn't find out why desktop was not working.

Do you want me to provide any additional info?

(In reply to Micah Zoltu from comment #95)

(In reply to liakoskonstantin from comment #92)

After this fix, although I point my dev.localhost hostname to the WSL ip inside hosts file, it is not working. From Chrome any *localhost domain will see the WSL without even requiring anything in the hosts file, from FF it's not working. Any ideas?

(In reply to Ivan from comment #93)

I have the same problem with .localhost in FF.

I believe this is working as intended. *.localhost are now hardcoded to the loopback device and any attempts to reroute it should be ignored. This is necessary in order to consider http://*.localhost and http://localhost trusted contexts for the purpose of accessing a number of browser features (like USB access for example). If localhost may not point at the loopback device, then it can no longer be considered a trusted context by the browser and thus those features have to be disabled.

How can I check where my localhost is pointing?

(In reply to Anne (:annevk) from comment #94)

does it work if you flip network.proxy.allow_hijacking_localhost in about:config?

Yes, now it works.
Many thanks!

Regressions: 1684241
Regressions: 1686759

(In reply to Anne (:annevk) from comment #94)

liakoskonstantin, Ivan, does it work if you flip network.proxy.allow_hijacking_localhost in about:config? I guess either way this requires some kind of follow-up bug since we wanted to match Chrome.

Frédéric, thoughts?

Sorry, I completely miss this message. Normally, with the default value of network.proxy.allow_hijacking_localhost (false), Firefox should match Chrome's behavior. Setting it to true should only be needed for tests. I'm not sure if people are saying they have the option off or on.

Flags: needinfo?(fred.wang)

For the WSL issue, maybe it's bug 1670146

There is a bug here you may want to consider.

If the secure context feature must consider *.localhost to be hard-coded to the loopback address, then http://foo.localhost. (note the FQDN terminated with the root) should also be hard-coded to the loopback address.

In other words, if you don't like the hard-coded loopback address, you can simply terminate your hostname in the address bar with the root (period) and Firefox will resolve the hostname normally.

TLDR; http://dev.localhost (works as designed) http://dev.localhost. (works around the new hard-coded loopback address feature)

chris, thank you for telling us that. Could you please file a new bug on that?

No longer depends on: 1436778
See Also: 1346835
Flags: needinfo?(chris)

Hi Henrik,

The steps to reproduce this are fairly simple.
There are authoritative nameservers here for the localhost TLD.
In the tree is a subdomain aaa.localhost.
In the aaa.localhost zonefile is an address record boot.aaa.localhost.

I configure boot.aaa.localhost as a virtual host inside nginx to listen on port 80 to use with GRUB network booting.
In the Firefox for Linux v98.0.1 64-bit address bar, if I type "http://boot.aaa.localhost" then Firefox will call out to the loopback address 127.0.0.1 unconditionally (as designed).

If instead, I type "http://boot.aaa.localhost." (by terminating the hostname with the root [period]) then Firefox will call out to the proper IP address found in DNS and not use the loopback IP of 127.0.0.1 as intended.

Best Regards,
-Chris

Flags: needinfo?(chris)

Hi Chris, thanks for the details! Would you mind to file a new bug for that and let us know about it's id (see comment 102)? Thanks in advance.

Flags: needinfo?(chris)

(In reply to Henrik Skupin (:whimboo) [⌚️UTC+1] from comment #104)

Hi Chris, thanks for the details! Would you mind to file a new bug for that and let us know about it's id (see comment 102)? Thanks in advance.

I filed bug 1761414 for you. Thanks again for reporting it!

Flags: needinfo?(chris)

Hi Henrik, thanks for filing the new bug!

You need to log in before you can comment on or make changes to this bug.