Closed Bug 1337246 Opened 7 years ago Closed 7 years ago

Insecure password warning appears on local IP address hosts (such as routers)

Categories

(Firefox :: Security, defect, P2)

Product:
Firefox
Component:
Security
Version:
54 Branch
Type:
defect

Tracking

()

Status:
VERIFIED FIXED
Milestone:
Firefox 55
Tracking Flags:
Tracking Status
firefox51 --- unaffected
firefox52 --- wontfix
firefox53 --- wontfix
firefox54 --- wontfix
firefox55 --- fixed

People

(Reporter: TD-Linux, Assigned: johannh)

References

(Depends on 1 open bug, Blocks 1 open bug)

Details

(Whiteboard: [fxprivacy])

Attachments

(7 files)

Bug 1337246 - Part 1 - Add a hostnameIsLocalIPAddress function to nsIIOService.
59 bytes, text/x-review-board-request
valentin
: review+
Details
Bug 1337246 - Part 2 - Detect local IP addresses to ignore for insecure password warnings.
59 bytes, text/x-review-board-request
MattN
: review+
Details
Bug 1337246 - Part 3 - Add a test for hostnameIsLocalIPAddress.
59 bytes, text/x-review-board-request
valentin
: review+
Details
not working in Firefox Nightly 55.0a1 64bit
289.63 KB, image/png
Details
ff 55.0a1 - no frames
381.92 KB, image/png
Details
no frames
98.94 KB, image/png
Details
router-login page showing password warning
3.98 KB, text/plain
Details 
On my OpenWRT router which has a plain HTTP web interface, the insecure login popup appears. This is technically correct, but there is no way to implement HTTPS for a local-only device, so the only reasonable course of action is for the user to ignore the warning, which isn't great.

This might be WONTFIX but I thought I'd at least bring the issue up.
Tanvi and I had discussed this and I think it makes sense to be more conservative about this (at least initially) though I'm not sure we have time to fix this in 52 still. We were kinda waiting to see how many people complained about this and you're the first. It would be easy to exclude local IP ranges when they're used as part of the URL but knowing whether a hostname resolves to a local IP requires Necko help and I'm not sure if we have an API for that from JS. The IP address in the URL is probably the majority case though.

(In reply to Thomas Daede [:TD-Linux] from comment #0)
> there is no way to implement HTTPS for a local-only device

This isn't technically correct. You can get an SSL certificate for a name which you control but which doesn't resolve to the router and have a local DNS server (e.g. in OpenWRT) resolve that name to your router. i.e. the server that proves ownership of the domain isn't the one that has to use the certificate.

See this thread on Twitter too: https://twitter.com/rlbarnes/status/823603633887526913
status-firefox51: --- → unaffected
status-firefox52: --- → affected
status-firefox53: --- → affected
Whiteboard: [fxprivacy] [triage]
I can look into this.
Assignee: nobody → jhofmann
Status: NEW → ASSIGNED
Comment on attachment 8834822 [details]
Bug 1337246 - Part 1 - Add a hostnameIsLocalIPAddress function to nsIIOService.

Matt, this is an initial take at ignoring local IP addresses as hostnames. I implemented the isLocalIPAddress function natively because there are utilities for detecting this available only to cpp code.

I'm not really happy with putting it in EffectiveTLDService but I didn't want to spend a long time searching for the right place to put it for a POC.

If you're fine with the general approach, can you recommend someone to review the cpp parts?

> It would be easy to exclude local IP ranges when they're used as part of 
> the URL but knowing whether a hostname resolves to a local IP requires 
> Necko help and I'm not sure if we have an API for that from JS.

We could resolve the hostname using the nsDNSService but I guess that makes another DNS request and we don't want that. I didn't find any information if/how/where Necko caches the DNS entry yet.

Another approach is just build a whitelist of common local domains and use that. I've done that before and I think it works pretty well.
Attachment #8834822 - Flags: feedback?(MattN+bmo)
I would like to register my skepticism here.  Pretty certain we should WONTFIX.

The issue is that private IP address space gets used for all sorts of things that are not home routers.  Lots of enterprises, for example, use RFC 1918 space to address networks over much wider areas than a home network.  And you definitely want to use HTTPS in those cases.


That said, if we are going to do something here...

(In reply to Johann Hofmann [:johannh] from comment #4)
> Comment on attachment 8834822 [details]
> > It would be easy to exclude local IP ranges when they're used as part of 
> > the URL but knowing whether a hostname resolves to a local IP requires 
> > Necko help and I'm not sure if we have an API for that from JS.
> 
> We could resolve the hostname using the nsDNSService but I guess that makes
> another DNS request and we don't want that. I didn't find any information
> if/how/where Necko caches the DNS entry yet.

Nope, you would need to look at the actual IP address you're talking to [0].


> Another approach is just build a whitelist of common local domains and use
> that. I've done that before and I think it works pretty well.

Also nope.  Resolver on hosts frequently respond to failures by retrying requests with a search suffix appended.  So even if you have something named "localhost", you might end up resolving "localhost.attacker.com".  Note that "localhost" is not included in the whitelist for Secure Contexts [1] for exactly this reason.  If you were going to whitelist domains, you would need to wire them to specific hosts / networks, as is proposed for localhost [2].


Finally, note that you cannot just look at what's in the URL bar for this.  If *any* non-local script is included in the page, it can steal passwords.  The reason we can look at the URL bar for HTTPS is that we block non-secure scripts, so if the top-level origin is secure, then you know that transitively everything included is.  So in order to implement this safely, the implementation would be on the order of adding active mixed content blocking.


[0] https://en.wikipedia.org/wiki/DNS_rebinding
[1] https://w3c.github.io/webappsec-secure-contexts/#is-origin-trustworthy
[2] https://tools.ietf.org/html/draft-west-let-localhost-be-localhost-03
(In reply to Richard Barnes [:rbarnes] from comment #5)
> I would like to register my skepticism here.  Pretty certain we should
> WONTFIX.

Thanks for the input! I figured focusing on only ignoring IP address URLs would avoid issues and complexity that comes with DNS, but

> The issue is that private IP address space gets used for all sorts of things that are not home routers.

and

> If *any* non-local script is included in the page, it can steal passwords.

probably mean we might want to WONTFIX this.
If we do this, we shouldn't try to do any hostname matching or whitelisting.  It would just be a blanket - don't warn if the url bar says http://ipaddress.  This means we wouldn't warn on places we should - which is okay for v1, because for v1 we've been cautious and prefer false negatives over false positives..

But, I didn't think about the private IP address issues, where we don't necessarily want to provide an exception.

Given we have had only a couple of complaints about this so far, I'm okay with won't fixing this and waiting to see what happens.  Alternatively, we could land this and put it behind a pref.  If, once 52 goes to release, we are inundated with complaints about local routers, we can enable the pref with a hotfix.
Priority: -- → P2
(In reply to Tanvi Vyas - not reading bugmail, PM me [:tanvi] from comment #7)
> But, I didn't think about the private IP address issues, where we don't
> necessarily want to provide an exception.

I don't understand what issue you're referring to? This bug is about LAN hosts (in the summary), not just the router.

FWIW, necko already has code to get the default route and mac address for the default route to calculate a "network ID" so perhaps we could get an API just for the default route (router)?

I honestly don't know what to do about this bug.
Comment on attachment 8834822 [details]
Bug 1337246 - Part 1 - Add a hostnameIsLocalIPAddress function to nsIIOService.

https://reviewboard.mozilla.org/r/110654/#review112262

Btw. we do have code to find the default gateway too if we don't want to allow all local addresses. We use it to calculate the "networkID". e.g. for Windows: https://dxr.mozilla.org/mozilla-central/rev/5e17f9181c6cb0968966280d1c1d96e725702af1/netwerk/system/win32/nsNotifyAddrListener.cpp#238

I'm probably fine with this patch (even doing string matching in JS for the few local network classes) as long as we take frame ancestors into account.

::: netwerk/dns/nsIEffectiveTLDService.idl:39
(Diff revision 1)
>       *         if the host is a numeric IPv4 or IPv6 address (as determined by
>       *         the success of a call to PR_StringToNetAddr()).
>       */
>      ACString getPublicSuffix(in nsIURI aURI);
>  
> +    boolean isLocalIPAddress(in nsIURI aURI);

I think the naming of this is a bit confusing given it takes a URI. Perhaps this would be better on nsIURI but I would check with a relevant peer.

::: toolkit/components/passwordmgr/InsecurePasswordUtils.jsm:91
(Diff revision 1)
>      }
>  
>      return { isFormSubmitHTTP, isFormSubmitSecure };
>    },
>  
> +  isLocalPage(aDocument) {

`isLocalPage` could be mistaken to be referring to the local filesystem instead of local network so I think the name should be clearer.

::: toolkit/components/passwordmgr/InsecurePasswordUtils.jsm:115
(Diff revision 1)
> +    let isLocalPage = this.isLocalPage(aForm.ownerDocument);
>      let { isFormSubmitSecure, isFormSubmitHTTP } = this._checkFormSecurity(aForm);
>  
> -    return isSafePage && (isFormSubmitSecure || !isFormSubmitHTTP);
> +    return (isSafePage || isLocalPage) && (isFormSubmitSecure || !isFormSubmitHTTP);

As Richard said, we need to still take the frame ancestors into account like isSecureContext[IfOpenerIgnored] does (and our old custom code here did).
Attachment #8834822 - Flags: feedback?(MattN+bmo) → feedback+
(In reply to Tanvi Vyas - not reading bugmail, PM me [:tanvi] from comment #7)
> Alternatively, we could land this and put it behind a pref.  If, once 52 goes to release, we
> are inundated with complaints about local routers, we can enable the pref
> with a hotfix.

This seems like the best approach if landing behind a pref is straightforward.  If it is, let's go with this plan.  If not (ie. it's more complex), I'd be completely fine WONTFIXing for now and fixing on a later release, if needed.
Leaving open but lowering priority as it won't affect most users and is not straightforward.  We'll monitor feedback from v52 and will re-prioritize if needed.
Priority: P2 → P3
Whiteboard: [fxprivacy] [triage] → [fxprivacy]
I updated to FF 52.0 a few nights ago as a routine update.
Today I tried to log on to my router as part of routine checking for firmware updates (Draytek Vigor 2760n) using https & got an insecure website popup from somewhere & a failure to log on when I proceeded anyway. retried under http & got the same thing. User id & password were correct.
Retried from both Google Chrome & IE with both http & https & apart from warning me about insecure connections all those permutations worked fine. My router has the captch option enabled, but doesn't have its certificate enabled, in case that makes a difference (which it shouldn't for http & didn't pre FF 52 for https).
I just saw a very good suggestion on the newsgroup mozilla.support.firefox on news.mozilla.org:
Instead of having the pop-up stay until the focus changes, why not have the pop-up stay for just a limited amount of time, say, 5 seconds? If necessary, add a flashing border to draw attention to it.

That way, at least there would be no obscuring of other content on the screen (such as a captcha, or have to select one of various pictures to continue the logon process).
(In reply to Matthew N. [:MattN] (behind on bugmail; PM if requests are blocking you) from comment #1)
> We were kinda waiting to see how many people
> complained about this and you're the first.

+1
Howdy,

I'm with Linksys/Belkin, and in the past ~3 weeks we've had a big surge of support calls/tickets regarding the "insecure login" dialog that's in Firefox 52. Like one of the parent comments says, there's no real way to get valid HTTPS certificates shipped onto individual devices, so we're at a bit of a loss as to how we can prevent our users from seeing this dialog. All input/ideas are welcome
I think we should move forward with this, we definitely need to solve this problem somehow. I'll start to work on Matt's review comments.

> Finally, note that you cannot just look at what's in the URL bar for this.  If *any* non-local script is included in the page, > it can steal passwords.  The reason we can look at the URL bar for HTTPS is that we block non-secure scripts, so if the 
> top-level origin is secure, then you know that transitively everything included is.  So in order to implement this safely, the > implementation would be on the order of adding active mixed content blocking.

Is that really a concern though? Aren't we already accepting that the connection is indeed insecure but the false positives so strongly outweigh the valid use-cases for local IPs that warning here is actually hurting our cause?

> If you were going to whitelist domains, you would need to wire them to specific hosts / networks, as is proposed for localhost [2].

That actually sounds like a neat idea for allowing router vendors to whitelist. But probably in a separate bug.

(In reply to benjamin.samuels from comment #17)
> Howdy,
> 
> I'm with Linksys/Belkin, and in the past ~3 weeks we've had a big surge of
> support calls/tickets regarding the "insecure login" dialog that's in
> Firefox 52. Like one of the parent comments says, there's no real way to get
> valid HTTPS certificates shipped onto individual devices, so we're at a bit
> of a loss as to how we can prevent our users from seeing this dialog. All
> input/ideas are welcome

Do you users normally access the router interface via an IP address? Then this bug would probably help. Otherwise see above.
Priority: P3 → P2
(In reply to Johann Hofmann [:johannh] from comment #18)
> I think we should move forward with this, we definitely need to solve this
> problem somehow. I'll start to work on Matt's review comments.
> 
> > Finally, note that you cannot just look at what's in the URL bar for this.  If *any* non-local script is included in the page, > it can steal passwords.  The reason we can look at the URL bar for HTTPS is that we block non-secure scripts, so if the 
> > top-level origin is secure, then you know that transitively everything included is.  So in order to implement this safely, the > implementation would be on the order of adding active mixed content blocking.
> 
> Is that really a concern though? Aren't we already accepting that the
> connection is indeed insecure but the false positives so strongly outweigh
> the valid use-cases for local IPs that warning here is actually hurting our
> cause?
> 
> > If you were going to whitelist domains, you would need to wire them to specific hosts / networks, as is proposed for localhost [2].
> 
> That actually sounds like a neat idea for allowing router vendors to
> whitelist. But probably in a separate bug.
> 
> (In reply to benjamin.samuels from comment #17)
> > Howdy,
> > 
> > I'm with Linksys/Belkin, and in the past ~3 weeks we've had a big surge of
> > support calls/tickets regarding the "insecure login" dialog that's in
> > Firefox 52. Like one of the parent comments says, there's no real way to get
> > valid HTTPS certificates shipped onto individual devices, so we're at a bit
> > of a loss as to how we can prevent our users from seeing this dialog. All
> > input/ideas are welcome
> 
> Do you users normally access the router interface via an IP address? Then
> this bug would probably help. Otherwise see above.

Power users and long-time users typically navigate to the admin interface using the gateway IP address, usually 192.168.1.1. However the documentation/startup guides tell users to navigate to "linksyssmartwifi.com", "router.local", "belkin.range", or any number of domains that the router aliases to 192.168.1.1. 

> Finally, note that you cannot just look at what's in the URL bar for this.
> If *any* non-local script is included in the page it can steal passwords.

There are router setup flows where we expect the user has absolutely no internet access when they plug in their router for the first time, so those locally hosted admin portals are completely self contained. The only outside traffic these portals generate are GETs to check if the internet is up or down.


I like that vendor whitelisting idea, as long as you guys have the resources for it. If whitelisting doesnt work out, one neat alternative might be to check if the IP being resolved is the IP of the local gateway. If we know the destination of the traffic is on the same layer 2 segment as the user, you might be able to display a more tame message like "Users on your local network may be able to see your password"
Comment on attachment 8834822 [details]
Bug 1337246 - Part 1 - Add a hostnameIsLocalIPAddress function to nsIIOService.

https://reviewboard.mozilla.org/r/110654/#review127412

::: netwerk/dns/nsEffectiveTLDService.cpp:162
(Diff revision 1)
> +  nsCOMPtr<nsIURI> innerURI = NS_GetInnermostURI(aURI);
> +  NS_ENSURE_ARG_POINTER(innerURI);
> +
> +  nsAutoCString host;
> +  nsresult rv = innerURI->GetAsciiHost(host);
> +  if (NS_FAILED(rv))

nit: Add brackets.

::: netwerk/dns/nsEffectiveTLDService.cpp:172
(Diff revision 1)
> +  PRNetAddr addr;
> +  PRStatus result = PR_StringToNetAddr(host.get(), &addr);
> +  if (result == PR_SUCCESS) {
> +    NetAddr netAddr;
> +    PRNetAddrToNetAddr(&addr, &netAddr);
> +    if (IsIPAddrLocal(&netAddr))

nit: Add brackets

::: netwerk/dns/nsIEffectiveTLDService.idl:39
(Diff revision 1)
>       *         if the host is a numeric IPv4 or IPv6 address (as determined by
>       *         the success of a call to PR_StringToNetAddr()).
>       */
>      ACString getPublicSuffix(in nsIURI aURI);
>  
> +    boolean isLocalIPAddress(in nsIURI aURI);

I would suggest either nsIDNSService, as there is already a IsLocalIPAddress in DNS.cpp, or even nsIIOService if you prefer to be able to call Services.io.isLocalIPAddress.
Comment on attachment 8834822 [details]
Bug 1337246 - Part 1 - Add a hostnameIsLocalIPAddress function to nsIIOService.

https://reviewboard.mozilla.org/r/110654/#review112262

> I think the naming of this is a bit confusing given it takes a URI. Perhaps this would be better on nsIURI but I would check with a relevant peer.

I think this is fine, but maybe you want a better name for the method, such as hostnameIsLocalIPAddress ?
I updated the patch and put the function in nsIIOService. I removed that check that we are doing in _detectInsecureFormLikes, so that we're still warning in the identity block and in the browser console, but don't show the in-content warning anymore.

The check does not consider frame ancestors in its current state, and it would mark HTTP iframes on the page as insecure afaict. Matt and I talked about this on IRC and I originally wanted to ignore any insecure resources on the page as long as the top-level (urlbar) hostname is a local IP address. We could do that by always checking the top window in isFormSecure, I'd leave it for Matt to decide. This would mean that HTTP iframes on windows that have a LAN address would not be marked as insecure.

Unfortunately I really don't know how to access a mochitest page on a LAN address for testing this. Do you have an idea for that, Valentin?
Comment on attachment 8834822 [details]
Bug 1337246 - Part 1 - Add a hostnameIsLocalIPAddress function to nsIIOService.

https://reviewboard.mozilla.org/r/110654/#review128200

::: netwerk/base/nsIIOService.idl:182
(Diff revision 2)
>       */
>      ACString extractScheme(in AUTF8String urlString);
> +
> +    /**
> +     * Checks if a URI host, obtained using nsIURI::GetAsciiHost(),
> +     * is a local IPv4 or IPv6 address literal.

No need to say "obtained using nsIURI::GetAsciiHost(),"
Attachment #8834822 - Flags: review?(valentin.gosu) → review+
Tanvi, Peter: are you in agreement that this should land?
Flags: needinfo?(tanvi)
Flags: needinfo?(pdolanjski)
Matt, thinking about my earlier comment I realized that the ideal setting would probably be to only allow local IP addresses on a top-level window or local IP address iframes in a local top-level window. So I implemented that now.

I also split up the commits and put this behind a pref, so that it's easier for us to turn it off if we find evidence that this is getting abused somehow.
Adding myself as I have just discovered this issue with my LAN (running Centos 7) in regards to Comment #1 "[...] We were kinda waiting to see how many people complained about this [...]"
Be careful about how LAN hosts are identified.  From what I can find, there is no standard as to how internal (LAN) IP addresses are supposed to be assigned.  Within larger corporate LANs, portions might indeed require secure connections with IDs, passwords, and site certificates.  Those portions might use internal IP addresses that are used differently in other LANs.
Comment on attachment 8853541 [details]
Bug 1337246 - Part 2 - Detect local IP addresses to ignore for insecure password warnings.

https://reviewboard.mozilla.org/r/125588/#review129718

::: toolkit/components/passwordmgr/InsecurePasswordUtils.jsm:27
(Diff revision 1)
> +XPCOMUtils.defineLazyGetter(this, "log", () => {
> +  let logger = LoginHelper.createLogger("InsecurePasswordUtils");
> +  return logger.log.bind(logger);

Nit: Please use:
```js
return LoginHelper.createLogger("InsecurePasswordUtils")
```
And change the callers to specify the log level (`log.debug` propbably). The only reason the pattern you copied is used in some of pwmgr is for backwards compat since there was previously just a bare `log` function.

::: toolkit/components/passwordmgr/InsecurePasswordUtils.jsm:91
(Diff revision 1)
>      }
>  
>      return { isFormSubmitHTTP, isFormSubmitSecure };
>    },
>  
> +  _hostnameIsLocalIPAddress(aDocument) {

The method name doesn't really match the argument IMO but I guess it's okay since I don't have great suggestions.

::: toolkit/components/passwordmgr/InsecurePasswordUtils.jsm:93
(Diff revision 1)
>      return { isFormSubmitHTTP, isFormSubmitSecure };
>    },
>  
> +  _hostnameIsLocalIPAddress(aDocument) {
> +    try {
> +      let uri = Services.io.newURI(aDocument.documentURI);

Security-related code should be using principals, not documentURI:
```js
let uri = aDocument.nodePrincipal.URI;
```

which makes me think this method should be named something like `_isPrincipalForLocalIPAddress` and pass in the .nodePrincipal of the actual "formlike" element (`formLike.rootElement.nodePrincipal`) for the one call.

::: toolkit/components/passwordmgr/InsecurePasswordUtils.jsm:95
(Diff revision 1)
>  
> +  _hostnameIsLocalIPAddress(aDocument) {
> +    try {
> +      let uri = Services.io.newURI(aDocument.documentURI);
> +      if (Services.io.hostnameIsLocalIPAddress(uri)) {
> +        log("hasInsecureLoginForms: detected local IP address: ", uri);

Nit: You don't need a space before the closing quote with console.jsm since it puts whitespace between all arguments for you.

::: toolkit/components/passwordmgr/InsecurePasswordUtils.jsm:99
(Diff revision 1)
> +      if (Services.io.hostnameIsLocalIPAddress(uri)) {
> +        log("hasInsecureLoginForms: detected local IP address: ", uri);
> +        return true;
> +      }
> +    } catch (e) {
> +      log("hasInsecureLoginForms: unable to check for local IP address: ", e);

Same here
Attachment #8853541 - Flags: review?(MattN+bmo) → review+
(In reply to Selena Deckelmann :selenamarie :selena use ni? pronoun: she from comment #25)
> Tanvi, Peter: are you in agreement that this should land?

Yes, with the caveat being that we should provide a better longer term solution which still warns users but provides the opportunity to say don't show me this again for this domain/address.
Flags: needinfo?(pdolanjski)
Sorry if I add noise to this bug, but this message really starts to bother me:
- when I log in to my webcams it says the connection is not secure
- when I log in on my nas it says the connection is not secure
- when I log in on my router it says the connection is not secure
- when I log in on the web interface of mythtv it says the connection is not secure
- when I log in on my self hosted gitea instance it says the connection is not secure
- when I log in to my self hosted nextcloud it says the connection is not secure
- when I log in to the configuration page of my toaster it says the connection is not secure

All these things are on my lan, and on most things there is no way to install a tls cert on them, nor would I want to do that.

I am starting to ignore the message. And the one time it was useful (when I almost logged in with my username and password on http instead on https on a page that exposed both without redirecting http -> https) I almost ignored it because of all the noise I have to go though every day with this warning on things where it should not come out.

Please get rid of it.

I ❤ firefox.
I don't think it is a matter of "get rid of it" but more of the take of comment #32 of "give me the control to say that I know what this is so don't flag the 'not secure warning'". I think we should assume per comment #33 that personal LANs should be able to make the decision about what is secure within the local LAN (without outside judgement of whether that is a good or bad decision in the eyes of firefox).

To address comment #30, my concern is only for personal LANs and not what corporations have .. I am only looking at this problem from a "my home LAN point-of-view". I deal with an address space of 192.168.2.xxx and feel that I ought to be able to determine what is trusted or not within that space. I also think that if I make a mistake it is my bad that I have to own ... I accept the responsibility that comes with saying "let me make the call"

Thanks
I don't think it is a matter of "get rid of it" but more of the take of comment #32 of "give me the control to say that I know what this is so don't flag the 'not secure warning'". I think we should assume per comment #33 that personal LANs should be able to make the decision about what is secure within the local LAN (without outside judgement of whether that is a good or bad decision in the eyes of firefox).

To address comment #30, my concern is only for personal LANs and not what corporations have .. I am only looking at this problem from a "my home LAN point-of-view". I deal with an address space of 192.168.2.xxx and feel that I ought to be able to determine what is trusted or not within that space. I also think that if I make a mistake it is my bad that I have to own ... I accept the responsibility that comes with saying "let me make the call"

Thanks
Opps ... hit the wrong button and caused my comments to be sent twice ... my apologies
I tried getting Mochitest and local IPs to work for quite some time to be able to write a browser test for this, but it didn't really work out. I'd like to land this now and so I hope writing a unit test for the ioService.ishostnameLocalIPAddress function is sufficient for that.
Comment on attachment 8865275 [details]
Bug 1337246 - Part 3 - Add a test for hostnameIsLocalIPAddress.

https://reviewboard.mozilla.org/r/136918/#review139946

Everybody <3s tests
Attachment #8865275 - Flags: review?(valentin.gosu) → review+
Pushed by jhofmann@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/bae6dd0500bc
Part 1 - Add a hostnameIsLocalIPAddress function to nsIIOService. r=valentin
https://hg.mozilla.org/integration/autoland/rev/c22ec6ec1d1d
Part 2 - Detect local IP addresses to ignore for insecure password warnings. r=MattN
https://hg.mozilla.org/integration/autoland/rev/f63b9bbe87b7
Part 3 - Add a test for hostnameIsLocalIPAddress. r=valentin
https://hg.mozilla.org/mozilla-central/rev/bae6dd0500bc
https://hg.mozilla.org/mozilla-central/rev/c22ec6ec1d1d
https://hg.mozilla.org/mozilla-central/rev/f63b9bbe87b7
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
status-firefox55: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 55
Dear Firefox developers:

Thanks for fixing and I look forward to getting Firefox 55

In the meantime, can I get a short "user level" explanation about what the fix is and what I should expect as a user?

Thanks,
Paul
(In reply to paul from comment #50)
> Thanks for fixing and I look forward to getting Firefox 55

This may make it into Firefox 54 if things go well and it gets approved.

> In the meantime, can I get a short "user level" explanation about what the
> fix is and what I should expect as a user?

Webpages using the HTTP scheme and a local IP address for the hostname and which contain logins fields will no longer show the insecure login field warning.

They are still insecure and should be secured, where possible, but in some cases, like routers, they are harder for users to do anything about and we don't want users to get warning fatigue.

Note that this bug doesn't change the behaviour for domain names which resolve to a local IP address, such as "router.local".
Summary: Insecure password warning appears on LAN hosts such as routers → Insecure password warning appears on local IP address hosts (such as routers)
I have reproduced this bug with nightly 54.0a1 (2017-02-07) (64-bit) on Manjaro 17 (64bit).

I can verify that this bug is fixed in Latest Nightly 55.0a1

Build ID 	20170510183715
User Agent 	Mozilla/5.0 (X11; Linux x86_64; rv:55.0) Gecko/20100101 Firefox/55.0

[bugday-20170503]
regarding comment 51:

Please pardon my ignorance, but does that last paragraph mean that not only do I have to give (for example) 192.168.2.xxx rather than a name "router" which points to that address. but any machine/printer/WAP/secondaryRouter has to be 192.168.2.xxx rather than a name. My memory is that I only saw this problem for the router where I was using 192.168.2.xxx and not for anything on the LAN where I used the name from /etc/host (must admit, I haven't tested my WAP to a secondary router nor accessing my printer as I have not seen a case where this comes into play and normal usage doesn't show the problem)

Thanks in advance,
Paul
The warning is still there for me in 55.0a1 using the ip address in the url bar.

The warning is also there when logging in on the isp router using the hostname "vodafone.station" as described by the isp.
(In reply to Simon from comment #54)
> Created attachment 8866658 [details]
> not working in Firefox Nightly 55.0a1 64bit
> 
> The warning is still there for me in 55.0a1 using the ip address in the url
> bar.

Can you check if that page is actually embedding an iframe (and do another update to be sure it's the right version)?

> 
> The warning is also there when logging in on the isp router using the
> hostname "vodafone.station" as described by the isp.

Yes, that's expected, sorry.
Flags: needinfo?(mozilla)
Attached image ff 55.0a1 - no frames — 
Downloaded the latest nightly and tested with a fresh profile.
As shown in the screenshot the page does not contain frames (hope I checked correctly).
The only thing I changed from all the defaults is that I set a socks proxy, and I am accessing the page though that proxy.
Flags: needinfo?(mozilla)
(In reply to Simon from comment #56)
> Created attachment 8866764 [details]
> ff 55.0a1 - no frames
> 
> Downloaded the latest nightly and tested with a fresh profile.
> As shown in the screenshot the page does not contain frames (hope I checked
> correctly).
> The only thing I changed from all the defaults is that I set a socks proxy,
> and I am accessing the page though that proxy.

You need to check window.frames.length instead (the correct tag name is "iframe" and the window.frames object is a little weird https://developer.mozilla.org/en-US/docs/Web/API/Window/frames).

An even better way to check would be to right-click inside the login field and see if there's a "This Frame" context menu item.
Attached image no frames — 
No, there are really no frames. Attaching page source as well.
Depends on: 1364080
Oh, thanks for the source. It seems we still warn against insecure actions. Let's make a bug for that.
Depends on: 1364208
Marking this as verified based on comment 52. We'll take care of the rest in follow-up bugs.
Status: RESOLVED → VERIFIED
Flags: needinfo?(tanvi)
Matt, in comment 51 it sounded like you were considering getting this uplifted to 54, if so please request approval when you get a chance.
status-firefox52: fix-optional → wontfix
status-firefox53: affected → wontfix
status-firefox54: affected → fix-optional
Flags: needinfo?(MattN+bmo)
I don't think uplifts to 54 are happening anymore.
status-firefox54: fix-optional → wontfix
Flags: needinfo?(MattN+bmo)
This seems to have returned for me on nightly - I see the warning on 10.0.0.1. Could someone verify it isn't just me?
(In reply to Thomas Daede [:TD-Linux] from comment #64)
> This seems to have returned for me on nightly - I see the warning on
> 10.0.0.1. Could someone verify it isn't just me?

See the dependencies of this bug.
Depends on: 1410070
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: