Note: There are a few cases of duplicates in user autocompletion which are being worked on.

Allow specifying wildcard that matches all simple netbiosnames in network.automatic-ntlm-auth.trusted-uris

RESOLVED FIXED in Firefox -esr10

Status

()

Core
Networking
--
minor
RESOLVED FIXED
9 years ago
5 years ago

People

(Reporter: Reto Egeter, Assigned: Richard van den Berg)

Tracking

(Blocks: 1 bug, {dev-doc-needed})

unspecified
mozilla13
x86
All
dev-doc-needed
Points:
---

Firefox Tracking Flags

(firefox-esr1015+ fixed)

Details

(Whiteboard: [qa?])

Attachments

(2 attachments, 8 obsolete attachments)

(Reporter)

Description

9 years ago
User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 6.0; en-GB; rv:1.9.0.1) Gecko/2008070208 Firefox/3.0.1 (.NET CLR 3.5.30729)
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 6.0; en-GB; rv:1.9.0.1) Gecko/2008070208 Firefox/3.0.1 (.NET CLR 3.5.30729)

This is a problem on intranets.
Some companies are using a lot of Netbios only names for Intranet websites.
This is because with IE they are automatically added to the "Intranet Zone" and NTLM authentican can be used.

With Firefox I have to add each of these Netbios names manually.
Some mechanism to automate this would be best.

Reproducible: Always

Steps to Reproduce:
1.
2.
3.
(Reporter)

Updated

9 years ago
Summary: All specifying wildcard that matches all simple netbiosnames in network.automatic-ntlm-auth.trusted-uris → Allow specifying wildcard that matches all simple netbiosnames in network.automatic-ntlm-auth.trusted-uris

Comment 1

9 years ago
According to this page:
https://developer.mozilla.org/En/Integrated_Authentication

These are other kinds of authentication that at least let you use a domain or tld. I would agree that wildcards would be great, but at least let us put a top level domain rather than the 100 different sub-domains we have on our corporate (nationwide mobile phone carrier) intranet.

This is one of the reasons why we don't use Firefox as a standard. Our team has to keep a list of URIs.

Steps to Reproduce:
1. go to about:config
2. edit network.automatic-ntlm-auth.trusted-uris with "mycompany.com" or maybe "*.mycompany.com" or maybe "*.mycompany.tld" (top level domain)
3. go to a mycompany.com site and still get prompted for NT login and password

Expected:

1. go to about:config
2. edit network.automatic-ntlm-auth.trusted-uris with "mycompany.com"
3. go to a mycompany.com site and not get prompted for NT login and password.

Comment 2

9 years ago
sorry, I used "tld" and domain interchangeably. It's early. I just meant domain, not tld.
(Assignee)

Comment 3

6 years ago
Mitch, your expected result in comment #2 is the way it works now. The strings in network.automatic-ntlm-auth.trusted-uris are matched with the current URL from the right. So adding mycompany.com should work.

I would however love to have a way to add all hostnames that do not have a domain to network.automatic-ntlm-auth.trusted-uris. Urls like:

http://sharepoint1/
http://sharepoint2/
http://wiki/
http://cmdb/

Updated

6 years ago
Blocks: 720362

Comment 4

6 years ago
The proxy auth code now allows the use of <local> to indicate domains that are non-fqdn; this usage should be expanded to allow <local> to be placed in the network.automatic-ntlm-auth.trusted-uris

Microsoft IE's default behavior for NTLM auth is to allow NTLM on all domains that are non-fqdn (those are the domains considered 'intranet security zone'), and right now, there's no easy way to replicate that in Firefox.
(Assignee)

Comment 5

6 years ago
I checked nsWindowsSystemProxySettings.cpp and "<local>" is replaced by "localhost;127.0.0.1" so this is not entirely the same thing.

I think it would be best to have a network.automatic-ntlm-auth.allow-non-fqdn and network.negotiate-auth.allow-non-fqdn preference to control this. I'll write a patch for this, as it is rather easy to implement.
(Assignee)

Comment 6

6 years ago
Created attachment 593058 [details] [diff] [review]
Possible fix

This is my first Firefox patch, please be kind.
Component: General → Networking
Product: Firefox → Core
QA Contact: general → networking
Comment on attachment 593058 [details] [diff] [review]
Possible fix

Thanks for the patch, Richard!

I think you want to use host.FindChar('.') != kNotFound instead of strchr.

I'll flag someone familiar with network code to provide feedback here.
Attachment #593058 - Flags: feedback?(honzab.moz)
Comment on attachment 593058 [details] [diff] [review]
Possible fix

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

In general, looks good!

Few nits:
- update to the latest mozilla-central code
- use bool instead of PRBool
- always have space after |if|
- as Gavin suggests, use host.FindChar('.') != kNotFound  ->  means "host is FQDN"

::: mozilla-release/extensions/auth/nsHttpNegotiateAuth.cpp
@@ +334,5 @@
>      return val;
>  }
>  
>  PRBool
> +nsHttpNegotiateAuth::TestNonFqdn(nsIURI *uri, const char *pref)

I'd not pass the pref here as an arg.  Just hard code it in the method.

@@ +347,5 @@
> +
> +    if(strchr(host, '.'))
> +        return PR_FALSE
> +    else
> +        return PR_TRUE

Hmm.. missing semicolons :)

::: mozilla-release/modules/libpref/src/init/all.js
@@ +1048,5 @@
>  // system's NTLM implementation to silently authenticate the user with their
>  // Window's domain logon.  The trusted-uris pref follows the format of the
>  // trusted-uris pref for negotiate authentication.
>  pref("network.automatic-ntlm-auth.allow-proxies", true);
> +pref("network.automatic-ntlm-auth.allow-non-fqdn", true);

I'd call this 'non-fqdn-trusted' to emphasize we want to auto neg with them and that the functionality is the same as fqdn names would be added to 'trusted-uris'.

::: mozilla-release/netwerk/protocol/http/nsHttpNTLMAuth.cpp
@@ +226,5 @@
> +        PRBool val;
> +        if (NS_FAILED(prefs->GetBoolPref(kAllowNonFqdn, &val)))
> +            val = PR_FALSE;
> +        LOG(("Default credentials allowed for non-fqdn: %d\n", val));
> +        if (val == PR_TRUE)

Better as:

if (NS_SUCCEEDED(prefs->....(..&val)) && val) {
  LOG(...)
  return true;
}
Attachment #593058 - Flags: feedback?(honzab.moz) → feedback+
(Assignee)

Comment 9

6 years ago
Created attachment 595550 [details] [diff] [review]
Possible fix

Thanks for the review. I've followed all suggestions except for the name of the prefs. Your suggestion 'non-fqdn-trusted' is inconsistent with the 'allow-proxies' prefs. I believe 'allow-non-fqdn' more clearly describes the effect of the prefs.
Attachment #593058 - Attachment is obsolete: true
Comment on attachment 595550 [details] [diff] [review]
Possible fix

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

Found some issues.

r-

::: extensions/auth/nsHttpNegotiateAuth.cpp
@@ +144,5 @@
>          proxyInfo->GetHost(service);
>      }
>      else {
> +        bool allowed = TestNonFqdn(uri);
> +	if (!allowed)

No tabs please, use only spaces.

@@ +347,5 @@
> +
> +    if (host.FindChar('.') != kNotFound)
> +        return false;
> +    else
> +        return true;

No else after return.  You can also just:

return host.FindChar('.') == kNotFound;


However, if the host is an IPv6 address, it will be qualified as non-fqdn too, and that is very bad.

You have to add a check the host is not an IP address when there is not any dot in the name.

There is PR_StringToNetAddr function for the check.  If it returns PR_SUCCESS, then the string is an IP address and you have to return false.

::: modules/libpref/src/init/all.js
@@ +1035,5 @@
>  // This list controls which URIs can support delegation.
>  pref("network.negotiate-auth.delegation-uris", "");
>  
> +// Allow SPNEGO by default when challenged by a local server.
> +pref("network.negotiate-auth.allow-non-fqdn", true);

By default this has to be false.  This pref is on-demand, it is not safe to have it on by default.  Same as the list of trusted URIs it has to be set by user.

@@ +1067,5 @@
>  // system's NTLM implementation to silently authenticate the user with their
>  // Window's domain logon.  The trusted-uris pref follows the format of the
>  // trusted-uris pref for negotiate authentication.
>  pref("network.automatic-ntlm-auth.allow-proxies", true);
> +pref("network.automatic-ntlm-auth.allow-non-fqdn", true);

Same here
Attachment #595550 - Flags: review-
(Assignee)

Comment 11

6 years ago
Created attachment 597168 [details] [diff] [review]
Possible fix

Thanks again for the review. I've applied the suggested changes.

About the default being false: on Windows this functionality is on by default for IE and Chrome, see http://dev.chromium.org/developers/design-documents/http-authentication . I can live with the default being false for Firefox though.
Attachment #595550 - Attachment is obsolete: true
(Assignee)

Comment 12

6 years ago
Created attachment 597347 [details] [diff] [review]
Possible fix

Now also uses PR_StringToNetAddr for NTLM authentication.
Attachment #597168 - Attachment is obsolete: true
Comment on attachment 597347 [details] [diff] [review]
Possible fix

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

I'll find someone to have a second opinion on the default value for these preferences.  I know IE defaults to true, unless setup for "restricted sites".  We may do the same, I just feel we shouldn't.

::: extensions/auth/nsHttpNegotiateAuth.cpp
@@ +347,5 @@
> +    if (NS_FAILED(uri->GetAsciiHost(host)))
> +        return false;
> +
> +    // return true if host does not contain a dot and is not an ip address
> +    return host.FindChar('.') == kNotFound && PR_StringToNetAddr(host.get(), &addr) != PR_SUCCESS;

80 chars per line please, probably just wrap as following:

return host.FindChar('.') == kNotFound &&
       PR_StringToNetAddr(host.get(), &addr) != PR_SUCCESS;

::: netwerk/protocol/http/nsHttpNTLMAuth.cpp
@@ +226,5 @@
>      nsCOMPtr<nsIURI> uri;
>      channel->GetURI(getter_AddRefs(uri));
> +
> +    bool isNonFqdnTrusted = false;
> +    if (IsNonFqdn(uri)) {

Add check for |uri| (non-null).

@@ +228,5 @@
> +
> +    bool isNonFqdnTrusted = false;
> +    if (IsNonFqdn(uri)) {
> +        if (NS_FAILED(prefs->GetBoolPref(kAllowNonFqdn, &isNonFqdnTrusted)))
> +            isNonFqdnTrusted = false;

And maybe you should check the pref first before you even call IsNonFqdn (performance) and also you may just return when it is found trusted.
(Assignee)

Comment 14

6 years ago
Created attachment 597431 [details] [diff] [review]
Possible fix

I've added all suggestions except the last one. I don't want to leave CanUseDefaultCredentials() without logging, so IsNonFqdn() needs to be called to be able to log that we allowed NTLM using the default credentials based on the host being non-fqdn.
Attachment #597347 - Attachment is obsolete: true
(In reply to Richard van den Berg from comment #14)
> Created attachment 597431 [details] [diff] [review]
> Possible fix
> 
> I've added all suggestions except the last one. I don't want to leave
> CanUseDefaultCredentials() without logging, so IsNonFqdn() needs to be
> called to be able to log that we allowed NTLM using the default credentials
> based on the host being non-fqdn.

You can add an extra log before return ;)
(Assignee)

Comment 16

6 years ago
I think I see what you mean. Something like this?

    bool allowNonFqdn = false;
    if (NS_FAILED(prefs->GetBoolPref(kAllowNonFqdn, &allowNonFqdn)))
        allowNonFqdn = false;
    if (allowNonFqdn && uri && IsNonFqdn(uri)) {
        LOG(("Host is non-fqdn, default credentials are allowed\n"));
        return true;
    }
Yep, that's it.  You just don't need to init allowNonFqdn to false.
(Assignee)

Comment 18

6 years ago
Created attachment 597441 [details]
Possible fix

Check pref before testing uri in nsHttpNTLMAuth.cpp
(Assignee)

Comment 19

6 years ago
Created attachment 597442 [details] [diff] [review]
Possible fix

Forgot to label it as a patch.
Attachment #597431 - Attachment is obsolete: true
Attachment #597441 - Attachment is obsolete: true
Boris, Brian: the patch introduces a pref to allow ntlm and negotiate autologon using Windows SSPI for non-fqdn hosts (i.e. hosts in local intranet w/o dots in the name).

I proposed the pref should be off by default.  However, IE in default setting allows this on by default.

As this could be potentially dangerous (credential leak), I believe it needs user explicit consent to allow.

Comment 21

6 years ago
I'm not sure I know enough about this stuff to have an opinion....
cc'ing Jim Mathies, who's been doing a lot of NTLM stuff and might have an opinion.

Comment 23

6 years ago
I would default this to false for now. Admins that distribute fx can customize this, and individual users will figure it out. If after a number of releases we don't see issues with the use of the feature, we can flip it over.

Comment 24

6 years ago
I would be another good enterprise feature if this also worked automagically for the current local [Active Directory] domain as well as non-fqdn.

So if you're logged into the "CORPORATE.LOCAL" domain, that domains that end with "CORPORATE.LOCAL" should also NTLM auth; without having to add *.CORPORATE.LOCAL to the allowed list.  This can be done with configs right now, but the corporate friendly thing to do is to do it automatically.
(Assignee)

Comment 25

6 years ago
Scott B: I agree that allowing NTLM/SPNEGO for the current local domain would be great, but that is another use case. This bug is about the non-fqdn case. That is not possible with the current FF functionality. As you mention, domains can be added manually, so your request is to automate that for the local domain. Please open a new bug for that request.
(Assignee)

Comment 26

6 years ago
@Jim: I'm fine with leaving this off by default. Is the current version of the patch acceptable?

Comment 27

6 years ago
Comment on attachment 597442 [details] [diff] [review]
Possible fix

Honza has already reviewed this pretty completely - seeking his final r+ on the code.
Attachment #597442 - Flags: review?(honzab.moz)
Comment on attachment 597442 [details] [diff] [review]
Possible fix

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

Thanks for the patch!

r=honzab with last few details addressed.

::: extensions/auth/nsHttpNegotiateAuth.cpp
@@ +348,5 @@
> +        return false;
> +
> +    // return true if host does not contain a dot and is not an ip address
> +    return host.FindChar('.') == kNotFound &&
> +           PR_StringToNetAddr(host.get(), &addr) != PR_SUCCESS;

As I'm thinking of it, we should also add a check the |host| is not empty: !host.IsEmpty()

Instead of host.get() do host.BeginReading() please.

::: netwerk/protocol/http/nsHttpNTLMAuth.cpp
@@ +133,5 @@
> +        return false;
> +
> +    // return true if host does not contain a dot and is not an ip address
> +    return host.FindChar('.') == kNotFound && 
> +           PR_StringToNetAddr(host.get(), &addr) != PR_SUCCESS;

As well here.
Attachment #597442 - Flags: review?(honzab.moz) → review+
(Assignee)

Comment 29

6 years ago
Created attachment 598176 [details] [diff] [review]
Possible fix

Please update the documentation at https://developer.mozilla.org/En/Mozilla_internal_string_guide regarding the use of get(). In the current FF code get() is used 6734 times, and BeginReading() 137 times. get() simply returns BeginReading() which should be preferred.
Attachment #597442 - Attachment is obsolete: true
(Assignee)

Comment 30

6 years ago
Created attachment 598187 [details] [diff] [review]
Possible fix

Test !host.IsEmpty() first. Simplied setting |allowed| in nsHttpNegotiateAuth.cpp
Attachment #598176 - Attachment is obsolete: true
Comment on attachment 598187 [details] [diff] [review]
Possible fix

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

Thanks.

r=honzab

Let's get it in.
Attachment #598187 - Flags: review+
Comment on attachment 598187 [details] [diff] [review]
Possible fix

https://hg.mozilla.org/integration/mozilla-inbound/rev/479abf6b3598
Attachment #598187 - Flags: checkin+

Updated

6 years ago
Assignee: nobody → richard
https://hg.mozilla.org/mozilla-central/rev/479abf6b3598

Thanks for the patch! Join us on irc (#developers on irc.mozilla.org) and we'll find some other things for you to work on if you are interested? :-D
Status: NEW → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla13

Comment 34

6 years ago
Should this get some sort of support/dev-doc-needed keyword, or someone cc'd in so it gets published?

Updated

6 years ago
Keywords: dev-doc-needed

Comment 35

5 years ago
Sorry for the dumb question, but when will this feature make it into a released version of Firefox? I assume I have to compile Firefox myself currently in order to use this feature, correct?

Comment 36

5 years ago
This will be in Firefox 13.

You can get a build with this feature today by downloading a Beta (soon to be released as Firefox 13) or Aurora (soon to be released as Firefox 14) build from http://www.mozilla.org/en-US/firefox/channel/#beta or http://www.mozilla.org/en-US/firefox/channel/#aurora respectively.
No longer blocks: 720362
Presuming this blocking removal was accidental.
Blocks: 720362
(Assignee)

Comment 38

5 years ago
Now that this patch has landed in FF 13, it would be great if it could be included in FF 10 ESR. The whole reason for this patch is to accommodate corporate users.
tracking-firefox-esr10: --- → ?
[Triage Comment]

The FF13 train has already left the station so I'm tracking this for the release alongside FF14 on July 17th.  Please nominate for ESR landing approval.
tracking-firefox-esr10: ? → 14+
bumping this back to tracking nomination - too late for 14.
tracking-firefox-esr10: 14+ → ?
(In reply to Richard van den Berg from comment #38)
> Now that this patch has landed in FF 13, it would be great if it could be
> included in FF 10 ESR. The whole reason for this patch is to accommodate
> corporate users.

If this is still wanted for ESR, it needs to be nominated for uplift to the ESR branch - see https://wiki.mozilla.org/Release_Management/ESR_Landing_Process for more details.
(Assignee)

Comment 42

5 years ago
Yes, this is till wanted for ESR. Who can do the uplift and set the appropriate tracking flags? Personally I cannot set status-esr10 to affected or checkin-pending, or set the approval-mozilla-esr10 flag at all.
Approving for ESR, please go ahead and land to that branch as per the wiki intructions in comment 41.
status-firefox-esr10: --- → affected
tracking-firefox-esr10: ? → 15+
Setting checkin-needed, this needs to land to http://hg.mozilla.org/releases/mozilla-esr10/ this week.
Keywords: checkin-needed
https://hg.mozilla.org/releases/mozilla-esr10/rev/4d94b20d0322
status-firefox-esr10: affected → fixed
Keywords: checkin-needed

Comment 46

5 years ago
Mozilla/5.0 (Windows NT 6.1; rv:10.0.7) Gecko/20100101 Firefox/10.0.7 (20120824135927)

In about:config I have:
network.automatic-ntlm-auth.allow-non-fqdn;true
network.negotiate-auth.allow-non-fqdn;true
network.automatic-ntlm-auth.trusted-uris;http://wiki/

When going to http://wiki/, the page gets opened but I am not logged in nor do I get any error messages regarding wrong credentials or anything else.

This is the same behavior as on Firefox 10.0.5 ESR, where the first two prefs in the list above don't exist. 

Perhaps I didn't understand this bug fix right. What I am supposed to see different after setting the above settings in about:config?
Whiteboard: [qa?]
(Assignee)

Comment 47

5 years ago
Ioana, can you post a NSPR log file to see what is going on?

Some info on NSPR logging here:
http://www.mozilla.org/projects/netlib/http/http-debugging.html

Please set at least NSPR_LOG_MODULES=negotiateauth:5
(Assignee)

Comment 48

5 years ago
Is your wiki site using NTLM or negotiate authentication? Just to be sure, set NSPR_LOG_MODULES=nsHttp:3,negotiateauth:5,NTLM:5

Comment 49

5 years ago
Created attachment 655296 [details]
NSPR log file

The prefs were set as in comment 46 in a previous session. The log file information is only for opening the browser and going to http://wiki/.

Please let me know if you need me to take any other steps.
(Assignee)

Comment 50

5 years ago
The site in the log does not use NTLM or negotiate (SPNEGO) authentication. It probably uses a custom form on the site for authentication. Using the NTLM/SPNEGO scheme the server sends a "401 Unauthorized" HTTP response and sets the WWW-Authenticate HTTP header. None of this is present in your trace. NTLM/SPNEGO authentication is typically found on Microsoft Windows servers, but there are open source implementations as well. See http://msdn.microsoft.com/en-us/library/ms995330.aspx

Comment 51

5 years ago
It seems we don't have here the pre-requisites to verify this bug. Richard, if you do have access to the necessary, please try to verify it. If somebody else can help, that would be appreciated. Thanks
(Assignee)

Comment 52

5 years ago
I can confirm that this patch is included in ff 10.0.7esr build1 and that it is working as expected.
You need to log in before you can comment on or make changes to this bug.