Closed Bug 232567 Opened 20 years ago Closed 20 years ago

Warn when HTTP URL auth information isn't necessary or when it's provided

Categories

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

defect

Tracking

()

RESOLVED FIXED
mozilla1.7final

People

(Reporter: contact2009, Assigned: darin.moz)

References

(Blocks 1 open bug, )

Details

(Keywords: fixed1.7, Whiteboard: (not for detecting 'phishing' - see bug 122445))

Attachments

(2 files, 7 obsolete files)

Mozilla needs to remove all support for username and password authentication in
the URL from HTTP, HTTPS, and HTTP with SSL. This problematic feature allows
URLs in the form of:

http://username1:password2@example.com

Currently, when Mozilla gets that URL, it takes the user to example.com and logs
the user in as "username1" with "password2". Unfortunately, this can be
exploited. Indeed, it has grown increasingly common that URLs in the form of
http://username:password@example.com are creating large security problems. Take
this example:

http://www.mozilla.org&item%3Dq:20933773d88383h2nf8dhdfjk3jk377d7djk3354@example.com/bad/evil/fraud/identitytheft

Following that URL, a user might think he is going to the friendly confines of
mozilla.org. Unfortunately, he ends up at the unfriendly confines of
http://example.com/bad/evil/fraud/identitytheft. 

This feature is being used today to harm and exploit our users. We have to respond.

Mozilla has made improvements in our handling of these URLs. Further
improvements are on the drawing board. 

Neverthless, any improvements ultimately cannot raise our level of security high
enough. We can try to fashion a dialog box or even highlight the URL in an eye
catching manner. User education must accompany any such enhancement, of course.
The Internet will always have many new, inexperienced, and uneducated users.
Even experienced users sometimes click on the wrong URLs by accident. Therefore,
it is inevitable that many Mozilla users will be exploited by this feature no
matter how we improve on it.

The only way to really solve the problem once and for all is to rip out "URL
authentication" altogether.

RFC 1738 may indeed require that an HTTP(S) implementation (like Mozilla) honor
URL authentication. 

http://www.faqs.org/rfcs/rfc1738.html

Allowing these kinds of URLs was fine a few years ago. Today, however, it
creates a security hole ever more commonly exploited. The Mozilla Project does
need to honor standards. Yet, when the purpose of a certain standard is no
longer relevant, or is outweighed by other concerns--such as security--then we
have to react to the changing real world conditions. Today, the cost of
supporting this feature far outweighs the benefit. By removing this "feature" we
will fix a major security hole for good.

Microsoft has announced it will remove support for these URLs.

http://support.microsoft.com/default.aspx?scid=kb;[LN];834489

Since MSIE currently has an 80%+ market share, most sites that make use of this
feature will soon give it up. Thus, when Mozilla follows suit and removes the
feature, our userbase will not be terribly aggrieved.

Neither FTP nor any other protocol is affected by this bug report.

*** This bug has been marked as a duplicate of 122445 ***
Status: NEW → RESOLVED
Closed: 20 years ago
Resolution: --- → DUPLICATE
> The only way to really solve the problem once and for all is to rip out "URL
> authentication" altogether.

Wrong assertion. Neither is it the only solution nor does it solve the problem
completely (see other bug, e.g. www.microsoft.com.windows.2000.badsite.com)

Verified BADIDEA.
Status: RESOLVED → VERIFIED
Wrong marking.

Bug 122445 calls for a solution that warns the user. This is fundamentally
different. Will resolve as invalid.
Status: VERIFIED → REOPENED
Resolution: DUPLICATE → ---
Invalid, as per comments of others.
Status: REOPENED → RESOLVED
Closed: 20 years ago20 years ago
Resolution: --- → INVALID
This isn't really invalid at all. It's a valid RFE - stop supporting the 
username:password syntax in http and https URLs. Note that these schemes don't 
have official support for the username:password syntax anyway - RFC2616 
describes the format of an http URL as:

http_URL = "http:" "//" host [ ":" port ] [ abs_path [ "?" query ]]
[http://www.w3.org/Protocols/rfc2616/rfc2616-sec3.html#sec3.2.2]

It wouldn't solve all the problems, granted, but it would fix a large class of 
them. Most importantly, although we could never have considered making this 
change before, now that Microsoft have decided to drop support, we would be 
able to justify doing the same thing.

I'm reopening this. We at least need some rationale as to why we should 
continue to support something that: a) is not supported by the HTTP spec, b) is 
recommended against by the URL spec, c) is not supported by the market leader, 
and d) has security problems, both from spoofing and by the use of cleartext 
password.
Status: RESOLVED → REOPENED
Resolution: INVALID → ---
The following site demonstrates the problem.

http://www.dce.k12.wi.us/tech/urlspoofing.html
Attached patch patch (obsolete) — Splinter Review
Something like this works ok. This patch makes it impossible to construct an
HTTP channel with a URI that includes a username/password. The error message
that the front-end returns is currently displayed as 'The URL is not valid and
cannot be loaded.', which seems accurate enough to me to leave alone.

I thought about trying to make it impossible to create an http(s) nsStandardURL
with authentication information, but this looks like it achieves what's
required without running the risk of causing too much fallout.
Assignee: darin → malcolm-bmo
Status: REOPENED → ASSIGNED
Comment on attachment 140988 [details] [diff] [review]
patch

Requesting r/sr. I'm expecting this might be a controversial change.
Attachment #140988 - Flags: superreview?(darin)
Attachment #140988 - Flags: review?(dougt)
Comment on attachment 140988 [details] [diff] [review]
patch

darin points out that this'll break HTTP publish from composer.
Attachment #140988 - Attachment is obsolete: true
Attachment #140988 - Flags: superreview?(darin)
Attachment #140988 - Flags: review?(dougt)
  <darin> why don't we do this... in nsHttpChannel::ProcessResponse (which is 
          called from OnStartRequest), we check if we have credentials in the 
          URL... if we do, and the response is not 401/407, then we can put up a 
          warning dialog
  <darin> Malcolm: does that make sense?
<Malcolm> darin: Sure. Exactly what I was thinking.
Can we implement the security-checking in CAPS instead of network? There are
valuable uses of username:password URIs for embedders/chrome that should not be
disabled.
Attached patch patch v2 (obsolete) — Splinter Review
As discussed on IRC.
Attached patch patch v3, now with pref (obsolete) — Splinter Review
Use a confirmation dialog as per the previous patch, and control the whole lot
via a pref.
Attachment #140997 - Attachment is obsolete: true
Attached patch Patch v4 (obsolete) — Splinter Review
Include the hostname in the message, move the flag out of mCapabilities.
Attachment #141002 - Attachment is obsolete: true
Comment on attachment 141005 [details] [diff] [review]
Patch v4

Requesting r/sr again.
Attachment #141005 - Flags: superreview?(darin)
Attachment #141005 - Flags: review?(dougt)
> +SuperfluousAuth=You are visiting %1$S.\n\nThe URL you have provided includes 
> an embedded username and password, but the web server you have connected to 
> does not require authentication. 

Surely evil.com can just work round this by requiring authentication?

Could I suggest some alternative texts for discussion? It's important that we
get this right.

I think we need a dialog for all uses of HTTP auth-in-URL. We should have two
options for the text. One would be used if the "username" begins with "http" or
"www" or "ftp" (exact string set to be decided), the other in other cases. The
difference is that the former is almost certainly a phish, the latter is less
likely to be. This means that when we _do_ jump up and down and shout at the
user, they are more likely to pay attention.

Probably-phishing version:
You are about to visit www.evil.com. This site may be attempting to trick you
into thinking you are visiting a different site. Use extreme caution.
[I understand and will be very careful] [Cancel]

(This doesn't mention the possibly spoofed site name; mentioning it might mean
that people get the wrong idea when they skim the message)

Superfluous auth version:
You are about to log into the site www.foo.com with the username bar, but the
website does not require authentication. This may be an attempt to trick you. Is
www.foo.com the site you want to visit?
[Yes] [No]

Normal version:
You are about to log into the site www.foo.com with the username bar. 
[OK] [Cancel]

These intentionally all have different text on the buttons.

Gerv
This bug strictly calls for the removal of all "URL authentication" support. 

No dialog boxes, no "if the server doesn't support it" or anything. Just turn it
off or rip it out.
(In reply to comment #16)
> > +SuperfluousAuth=You are visiting %1$S.\n\nThe URL you have provided includes 
> > an embedded username and password, but the web server you have connected to 
> > does not require authentication. 
> 
> Surely evil.com can just work round this by requiring authentication?

yes, you are right.  thanks for catching that :-/

i like your suggested text.

i wonder if we want to show the normal auth prompt in some cases?  for example,
we could force the normal auth prompt to be shown whenever the server sends a
challenge.  then if we simply add the case #2 dialog, we'd have convered our
bases.  sure, this solution doesn't involve the dialogs in case #1 and #3, but
it would work... the normal auth prompt is good because it clearly distinguishes
the text of the site from the text of the username, and it gives the user the
ability to enter a different username and password.  in the case of legitmate
username-in-URL uses, the normal auth prompt would not be too awful, and might
even be useful in correcting a bad link (for example).

thoughts?
(In reply to comment #17)
> This bug strictly calls for the removal of all "URL authentication" support. 
> 
> No dialog boxes, no "if the server doesn't support it" or anything. Just turn it
> off or rip it out.

Andrew: ...then make a convincing case of it.  as you can see, this bug is on
its way to yielding a less severe solution.  if sufficient to solve the real
problem, then that's good... removing URL authentication is not necessarily the
only solution.
All right. If Darin says it, I back it.
I think that bug 122445 adequately covers the case where we sniff the
username:password combination to present a 'phishing warning' dialog. I'd like
to leave that case out of this bug, for the moment (that's not to say it's not
useful - it is, but it requires a lot more thought, and I think there's plenty
of useful discussion in bug 122445).

Morphing this bug to make it a bit clearer. Apologies for the fluffy
description; I don't want to pre-judge exactly what we'll do apart from not
spending too much time on a phish-detector :)

gerv is right: evil.com can just switch on http-auth. Higher barrier of entry,
but not by much. I *thought* that we always posed the auth dialog, which is why
I was thinking that it wasn't a problem, and indeed that seems to be what darin
is suggesting.

So really, I'd like to try implementing parts 2 and 3 of gerv's suggestion; 2
being the 'superfluous auth' dialog we have a patch for, and 3 being the
'warning' dialog for normal auth - specifically, by forcing the existing auth
dialog to appear even when credentials are provided in the URL (pre-filling the
username/password, naturally).

I'm open to suggestions for the dialog text in the current patch. Is there a
consensus that gerv's is good? (i.e., that we should mention the 'username' -
which might be 'www.paypal.com' in phishing-type situations).
Summary: Remove from HTTP(S) support for username and password authentication in URL → Warn when HTTP URL auth information isn't necessary or when it's provided
Whiteboard: (not for detecting 'phishing' - see bug 122445)
Darin: I don't quite understand your comment. When you say cases #1, #2 and #3
are you referring to my three scenarios?

My concern about showing a normal auth dialog is this: if a username and
password are embedded in a link, they are either a) correct for the site or b) a
phishing scam. If we just put up some sort of auth dialog with boxes to allow
the user to edit the username and password, there's a danger they'll just click
past it.

So... <thinks out loud>... how can we tell if it's a phishing scam? If the
server doesn't require auth, it's almost certainly phishing. OK, so some smarter
phishers might start doing that, but some won't be smart, and it's a help. If
the username starts with "www" or contains ".com", then it's almost certainly a
phish. 

Perhaps we want two sorts of dialog. One is the "there may be danger" dialog,
and one is the "there's definitely danger" dialog. We can tune which you get in
which circumstances as phishing tactics evolve. But neither should allow editing
of the username and password IMO.

Thoughts?

Gerv
BenB: about comment #2, I filed bug 233865.

As for morphing this bug, I'm thinking we should leave this bug as a strict:

"Microsoft is did this, should we do the same thing?" bug. (I don't agree w/
what they have done, bw).

We need a bug where they strictly focus on what they have done, and if we would
implement the same thing.

Also, the new summary is problematic, because unnecessary auth info is not proof
of a spoof attack, and smart HTTP code as Darin's proposed does not help here.
This is an example of a not-so-useful dialog (vs. dialogs proposed in 122445).

If you believe that username:password encourages spoofing, and you cannot come
up with a good solution (like adam's patch in bug 122445), then you can try to
solve the problem by removing the entire data structure.

If you believe that username:password is just a bad idea (spreading of clear
text passwords), then you can find solutions like limiting the creation of these
URLs in composer and mail, forcing the storing of the passwords in password
manager). If you still could not live with the idea, this would be another
reason to want to rip it all out.

It is also worth noting that, in general, we do have a lot of open bugs on URL
w/ username:password, in both ftp and http. There are probably a lot of
consistency and behavioral things that need to be fixed. Again, if you thought
making this feature work is hopeless, then you might consider disabling everything.
ben: we need to resolve the issue as a whole. Whether we do it here or in bug
122445, it doesn't really matter.

Gerv
Comment on attachment 141005 [details] [diff] [review]
Patch v4

I think a cheap and reasonable solution to this bug would be to combine this
patch (maybe with wording changes to the prompt) with a patch that forces the
auth prompt to always be shown even if prefilled with the values from the URL.  

that way, an attempt to spoof will either hit the  new dialog or the real auth
prompt, giving the user the chance to realize that something bogus is maybe
going on. 

we could go one step further an provide a method or maybe a new loadflag that
can be set by composer and XMLHttpRequest if they want to suppress the prompts.

(note: even microsoft reverted their patch to enable prompt-less authentication
from their "XMLHttpRequest" object.)
Attachment #141005 - Flags: superreview?(darin) → superreview-
hey jst... this is the bug i mentioned.
> we could go one step further an provide a method or maybe a new loadflag that
> can be set by composer and XMLHttpRequest if they want to suppress the prompts.

actually, we don't need to worry about this.  the current patch only shows the
dialog when LOAD_DOCUMENT_URI is set as a load flag.  that flag is set for any
toplevel URI loads that originate from a docshell.  XMLHttpRequest and
nsWebBrowserPersist do not set that flag.  so, we could piggy back
LOAD_DOCUMENT_URI to deal with the problem i mentioned.  that greatly simplifies
this patch b/c we just don't need to worry about screwing over XMLHttpRequest or
Editor's publish feature :-)
Attached patch necko.properties patch (obsolete) — Splinter Review
i applied this patch tonight (in time for the localization freeze).  i just
took gerv's strings from comment #16.  at least that will give us something for
1.7 final, and i want to make sure the translators have something to chew on. 
i'm working on a revised patch now to make use of these strings.

i found a few tricky ways that the superfluous auth prompt could be
circumvented.  for example, a 401 could contain no challenge.  as a result,
we'd just display the body of the 401 message, which is just another HTML
document.  the attacker could use that document to spoof another site.	so, we
need to be very careful to show the superfluous dialog whenever we don't do
auth... not just when the response code isn't 401 (or 407).
> so, we could piggy back LOAD_DOCUMENT_URI to deal with the problem i mentioned.  

i spoke to jst about this over IRC, and he agreed that it sounds reasonable.  i
might actually modify it to use LOAD_INITIAL_DOCUMENT_URI instead, since that
will allow, for example, iframes to bypass this spoof checking.
darin, if you are not familar with HTTP auth ('what is "Authentication"
anyways?') and just quickly glance over the dialog, you may just see
"www.ebay.com" in the dialog and think "OK, eBay, that's what I expected, why
did this stupid dialog come up now? Go away.". Click. For this reason, IMHO any
(warning) dialog should exactly *not* display the credentials at all, otherwise
you'd only make the problem *worse* for some users.
Ben: that's why there's a Phishing version which doesn't include the auth
credentials.

Gerv
Blocks: 228612
Attached patch v5 patch (obsolete) — Splinter Review
taking bug...

this patch enables gerv's "normal" and "superfluous" prompts.  enabling the
"phishing" version can be done subsequently.
Assignee: malcolm-bmo → darin
Attachment #141005 - Attachment is obsolete: true
Attachment #143470 - Attachment is obsolete: true
Severity: normal → major
Flags: blocking1.7b?
Priority: -- → P1
Target Milestone: --- → mozilla1.7beta
Attachment #141005 - Flags: review?(dougt)
> Ben: that's why there's a Phishing version which doesn't include the auth
> credentials.

But darin's patch (also) uses the AutomaticAuth version. As long as that version
is used in *any* potentially malice situation, the dialog is only making matters
worse IMHO, for reasons stated above.

I also don't think it's right to state "This may be an attempt to trick you", if
a non-login was successful. There are many reasons thinkable why (valid, but
possibly stupid) servers may behave like that, e.g. bug 122445 comment 57 or
just a stupid redirect to the homepage or similar.

I do think that a warning dialog (without credentials) is helpful, if it only
appears in cases where we can be very sure that it's spoofing, but I don't know
the criteria for that. That and escaping the username in the URL should suffice,
IMHO.
how close is patch to be ready for reviews?  time is tight on 1.7b
Comment on attachment 143637 [details] [diff] [review]
v5 patch

dveditz:

can you take a look at this patch?  ignoring for a second the disagreements on
what the contents of these dialogs should be, can you please review the
mechanics of this patch?  thanks!!
Attachment #143637 - Flags: superreview?(dveditz+bmo)
Comment on attachment 143637 [details] [diff] [review]
v5 patch

I remember the strings went in with another bug, did the pref default go in
then as well? I don't see it...

Other than that my only concern is that ConfirmAuth() blocks these URLs from
loading if it can't pose the dialog for any reason. I assume the stringbundle
exists in embedded builds, otherwise PromptForIdentity() would likewise fail.
More likely is that URLs with  user:pass will be flat-out blocked in random
localizations until they catch up and add the new format string.

Personally I think it would be better in the failure case to load the URL the
way we always have -- after all this is basically a warning not an error. Your
call though.

sr=dveditz
Attachment #143637 - Flags: superreview?(dveditz+bmo) → superreview+
> Personally I think it would be better in the failure case to load the URL the
> way we always have -- after all this is basically a warning not an error. Your
> call though.

You make a good point.  I was thinking about it in the sense that we currently
fail to authenticate if we cannot show the normal auth prompt, so I treated
failures here similarly.  But, you're right... failures with AuthConfirm are not
as severe.  I'll make that change.
Flags: blocking1.7b?
Flags: blocking1.7b-
Flags: blocking1.7?
Target Milestone: mozilla1.7beta → mozilla1.7final
Attached patch v5.1 patch (obsolete) — Splinter Review
revised patch to make us default to returning TRUE from ConfirmAuth if we are
unable to prompt the user.  this definitely makes more sense (especially when
you consider embedding cases).

i also fixed up the pref in all.js
Attachment #143637 - Attachment is obsolete: true
Attachment #144353 - Flags: review?(cneberg)
If this patch is intended as a security measure, then going forward with page
load and display in the event of a dialog failure might not be a great idea.  If
the page author is actively trying to deceive the user, then being "fail-open"
may make the attacker's job easier---if they can cause the dialog to fail
somehow, perhaps through a bug in Mozilla, or by consuming a great deal of
memory (for example, embedding the link in a very graphics-intensive email),
then they can carry out their deception.  A "fail-closed" design, which would
block the page in the event of a dialog failure, would make it so that nothing
an attacker could do would make their attack more likely to succeed, which would
be a more secure design.
> If this patch is intended as a security measure, then going forward with page
> load and display in the event of a dialog failure might not be a great idea.  

we already suppress the dialog if the load is not toplevel, and all toplevel
http/https URL loads in the browser will have a prompt associated properly.

however, if an embedder does not have a prompt b/c their application doesn't
need to have one, then we shouldn't screw them over.

this is the right thing to do.  we don't need to over-design here.
Flags: blocking1.7? → blocking1.7+
Attachment #144353 - Flags: review?(cneberg) → review?(bzbarsky)
I'm not likely to get to this until Saturday, and more likely until a week from
then (almost two weeks from now).....
Attachment #144353 - Flags: review?(bzbarsky) → review?(dougt)
Comment on attachment 144353 [details] [diff] [review]
v5.1 patch

How about adding an lower limit to the number of chars that must be in the
username+password before this dialog is displayed.  For example, don't bother
displaying the dialog if the username and password is under 16 chars?

Superfluous -- nice adjective in function name. :-)

Not sure if the dialog string resource is named as well "AutomaticAuth"?

Should the dialog have a "remember me" setting?  Should we always show this
dialog?

Parenthesis make this look nicer:
+	 confirmed = choice == 0;

Do we really want to suggest that we might do this and not?  

+	 // XXX we might want to scan |user| for phishy text like (www. or
+	 // .com), and if we get a match we could show the dialog for
+	 // PhishingAuth instead.

People don't like dialogs and will dismiss them without fully understanding
them.  I suggest that we instead:

a) block all such urls
b) fix up the url bar to NEVER display the username or password but instead of
some other kind of UI (maybe a mouseover tooltip that displays login info).

the R= is for the patch as it, but I would like comments on a, b, and c as well
as the lower limit suggestion.
> I suggest that we instead

See bug 122445, esp. bug 122445 comment 68.
> How about adding an lower limit to the number of chars that must be in the
> username+password before this dialog is displayed.  For example, don't bother
> displaying the dialog if the username and password is under 16 chars?

i like this suggestion.


> Not sure if the dialog string resource is named as well "AutomaticAuth"?

yeah, but we can't change it now or else we'll screw up the translators :-/


> Should the dialog have a "remember me" setting?  Should we always show this
> dialog?

not a bad idea, but if you think about it... the user should only hit this once
per site per session _tops_.  especially with the length limit, it shouldn't be
too annoying, right? ;-)



> Do we really want to suggest that we might do this and not?  
> 
> +	 // XXX we might want to scan |user| for phishy text like (www. or
> +	 // .com), and if we get a match we could show the dialog for
> +	 // PhishingAuth instead.

are you saying that we are giving hackers ideas?  do you think we should show
PhishingAuth?  i'm not sure yet what heuristics we'd want to use, and i'm not
sure i can come up with a good set of heuristics for 1.7 final.  so, i'd rather
not try to show the PhishingAuth text right now.


> People don't like dialogs and will dismiss them without fully understanding
> them.  I suggest that we instead:

i agree that this patch is not a solution in and of itself.  however, i believe
that it is part of the solution.  we should do more, but that does not mean that
there is no value in doing this! ;-)


thanks for the r= dougt!
(In reply to comment #43)
> > I suggest that we instead
> 
> See bug 122445, esp. bug 122445 comment 68.

i don't think those solutions are mutually exclusive.  we can do this and that =)
Attached patch v5.2 patchSplinter Review
OK, here's the revised patch that I checked into the trunk.
Attachment #144353 - Attachment is obsolete: true
Attachment #146248 - Flags: approval1.7?
[Not sure if the phishing heuristics thread should go here or in another bug.]

The proposed heuristic for phishing isn't going to find all sorts of sites.
Those of us outside the US have banks with ".co.uk" etc at the end. Also many
sites seem to be dropping "www." as a prefix for their webservers (perhaps they
think web == internet), so this isn't reliable.

How about a heuristic that says if there is a dot in the username then do a DNS
lookup on the username, if its a valid host assume its a phishing scam). If
there are not dots in the user name at worst its a phishing scam for a local
host, which isn't a realistic concern.
Comment on attachment 146248 [details] [diff] [review]
v5.2 patch

+    // If the defensive auth pref is set, then we'll warn the user before

What pref is that? You removed it with the checkin to all.js, didn't you?
(In reply to comment #48)
> What pref is that? You removed it with the checkin to all.js, didn't you?

thanks for catching this.  i've checked in a correction to that comment.
> How about a heuristic that says if there is a dot in the username then do a DNS
> lookup on the username, if its a valid host assume its a phishing scam).

I think in most cases the phishy username isn't a valid hostname.  Often, I
think the username is just prefixed with a valid hostname.  Or, maybe I'm wrong.
 I think the best next step is to do the %-escaping of dots in the username or
password as a part of our standard URL normalization.  This would help make
usernames and passwords look less like hostnames.
Comment on attachment 146248 [details] [diff] [review]
v5.2 patch

we should approve this for 1.7, but since it just landed on the trunk yesterday
I'd like to get a couple days feedback first in case of unexpected problems.

I don't like the addition of the "Phishy length" check--our behavior should be
consistent. Better to always show the auth prompt than to assume phishers won't
rise to the challenge of creating a reasonable-looking URL with a short
user:pass. 16 characters is plenty long enough for
http://ebay.com:acct=@123.45.67.89/longobscurewhatever.html and that's going to
fool just as many people. Any shorter than 16 and you start impacting a high
percentage of legit logins. In fact 16 is already too short for most of the
HTTP auths I personally use that mostly involve email addresses as IDs (though
of course I don't normally put them into the URLs themselves).

Sorry for the side comment... Given no complaints over the weekend we should
approve on Mon 19 Apr
If I read the patch correctly, it will now (for the example URL Daniel used, and
assuming the attacker is smart) show a dialog with the text:

You are about to log into the site "123.45.67.89" with the username "ebay.com"

This suffers from the problem described in problem 30. A user now knowing the
internet as well as us might think that "username" is the "name shown to the
user", what we know as "hostname". She indeed wanted to log into ebay, so that
"log into" also sounds fine to her, i.e. she may think that the dialog told her
that she will log into ebay.

I think that *any* wording will have this problem for users which just glance
over / cross-read the dialog. They'll see "ebay", maybe "log in", and think "OK"
- Click. So, I don't think you should show the possibly spoofed part *at all* in
the dialog, or you'll be making matters only worse, I think.


Also, you have a high likelyness of triggering a dialog in valid cases, which
should IMHO also be avoided to not desensitize users further.
Drivers don't like the length check -- either this is a good solution or it
isn't. Adding an arbitrary length check just forces scammers to get creative and
they've already proven they will rise to the challenge.
so, does that mean that drivers want the length preference to default to 0 or
does that mean that drivers don't want the patch at all?  defaulting to 0 means
that any userpass will generate the warning dialogs.
It doesn't even require them to be creative (I don't think) ebay.com, citi.com,
paypal.com all are easily spoofed under 16 characters. I'd like to (and I think
drivers agree) approve a patch without the limit. 
How is this bug now any different than bug 122445? We never should have let this
bug morph. Essentially, we have a dialog box that was much proposed in that bug,
but without a useful criteria for warning the user.

The "warn if server auth not needed" behavior makes sense, but the 16 character
limit should be removed. It should have been discusssed in another bug, but
since it was checked in here, I'll try to summarize discussion about this from
bug 122445.

The fundamental problem here is that username space is a superset of the FQDN
namespace, and that username spaces can be presented to users as the FQDN of the
URL.

So, we should not be implementing any changes without considerations
charateristics of both name spaces and the effects of their confluence.

As proposed (and Asa and I have boundary tested the daily build):

if username =< 15 characters, nothing happens. No auth and no warning.
if username => 16 characters, a warning (tested only w/ auth-less site).

Thinking about the namespaces, what are the characteristics of limiting
usernames and FQDN's to 15 characters or less?

As far a usernames go, there are no length limits to username in URLs. There
might be some practical system-specific limit, but if so, they would only
suggest that the current limit would leave most OS-specific username spaces as
allowable.

As far as domain names go, a 15 character limit would not help much. Assuming
that we are concerned w/ top-level .com sites, here's how the 15 character
budget is used:

"www." is 4 characters, ".com" is 4 characters. 15 - 4 - 4 = 7.

Sites that would hard to spoof:
www.netscape.com

Sites that would be vulnerable:
www.paypal.com
www.ebay.com
www.amazon.com
www.google.com

If we are going to prevent an attack, we need to effectively remove most of the
possible attacks. A high number of false negatives would allow attackers to
simply focus on the unprotected section of the name space. 

Why we would allow TLD's of 7 characters or less to remain unprotected. DNS
space is heavily congested and overused in the TLD's, especially .com. In other
words, the most desirable names are the shortest, which we are leaving unprotected.

More importantly, we should not forget that we are really talking about ALL FQDN
 namespace, not just the "www.domain.com" range. Domains that have servers
answering to "TLD.com" add another 4 characters to the vulnderable TLD name
space (11 characters + ".com").

Also, these attacks can be used to access any server the client system can
access via local DNS. How many intranet sites are longer than 15 characters? Do
we think that intranet hostmasters have been creating a lot of important sites
which have names longer than 15 characters (and probably not for the reasons we
seem to think they should)?
---

Separately, I only did boundary testing on the username length, if Darin or
someone else could give a clear description of the expected behavior and how it
differs from the pre-fixed behavior, that would be very helpful in future
discussions.
(In reply to comment #56)
> It doesn't even require them to be creative (I don't think) ebay.com, citi.com,
> paypal.com all are easily spoofed under 16 characters. I'd like to (and I think
> drivers agree) approve a patch without the limit. 

ok, i will write up a new patch.  should the user be able to click a checkbox to
disable future prompting?  should the limit pref remain as a hidden pref (with
default value 0)?  please advise.
> should the user be able to click a checkbox to disable future prompting?

Ideally, users should be able to disable the warning on a per-domain basis. So
if there's one place they use this login type, they don't get hassled, but are
still protected from phishes.

Gerv
> Ideally, users should be able to disable the warning on a per-domain basis. So

Agreed, although I think the patch for that would be too large to consider for
1.7.  Want to pick from the two choices I gave?
Hmm. Two options, neither particularly good UI:

1) Have a checkbox, and then have the checkbox change meaning subtly (from
"disable these warnings" to "disable these warnings for this domain") in a
future release.

2) Don't have a checkbox, and irritate the heck out of people who want to use
this auth method for legitimate purposes.

I'd go for 1), with a heavy heart.

Gerv
Blocks: 122445
I don't think we should do the checkbox until we can do it per domain. Right now
those people who use this feature regularly and are savvy enough can disable it
with a hidden pref and everyone else can just suffer the extra click. We didn't
have the checkbox in for the earlier patch for so plenty of users who were using
the user/pass in URL method for legitimate purposes were going to get the dialog
then. The only ones that were escaping the dialog were those who happened to
have user/pass combo shorter than some arbitrary character limit.  

Darin, also, what happens if the evil site sets up auth on its server? Do we
throw the auth dialog when the user/pass are in the URL? I guess what I'm really
asking, is what scenarios do this bugfix cover? Is it just that we warn when the
URL contains auth and server doesn't actually require auth?
> Darin, also, what happens if the evil site sets up auth on its server? Do we
> throw the auth dialog when the user/pass are in the URL? I guess what I'm really
> asking, is what scenarios do this bugfix cover? Is it just that we warn when the
> URL contains auth and server doesn't actually require auth?

asa, we prompt if the URL contains auth and

(1) the server doesn't actually require auth
(2) the server requires auth and we use the auth from the URL

note, if we use the auth from the URL, then it will only ever be taken from the
URL once per session for that domain.
The moment we check in a fix that does comment 62, I will file an RFE to this
feature that depends on bug 169106. We've needed this for ALL security features
for a long time, maybe this will be the final straw.
Agreed, we could really use a general zone (or site) management facility.
Darin: drivers are waiting for a patch that has no phishy length check and has a
non-UI way to turn this off (for automated scripts if nothing else).  If you
want to accomplish this by setting the default phishy length to 0 and let people
extend it that'd be OK for the branch (smaller change)
Firefox 20040427 Win2K3
I'm getting a warning prompt about the user/pass being in the URL when I click
on a bookmarked link that has the user/pass in the URL.  This is understandable
though I wonder if it should be in effect for bookmarked links.
However, I'm getting a second warning about the server not requiring
authentication when it in fact does require authentication.  Should another bug
be filed?
(In reply to comment #67)
> Firefox 20040427 Win2K3
> I'm getting a warning prompt about the user/pass being in the URL when I click
> on a bookmarked link that has the user/pass in the URL.  This is understandable
> though I wonder if it should be in effect for bookmarked links.

Unfortunately, it is difficult (requiring API changes) for us to disable this
check when the link is a bookmark.


> However, I'm getting a second warning about the server not requiring
> authentication when it in fact does require authentication.  Should another bug
> be filed?

Yes, please file a separate bug on that issue.  Thanks!
Comment on attachment 146248 [details] [diff] [review]
v5.2 patch

a=asa for checkin to 1.7 with the change to set the default pref at 0
Attachment #146248 - Flags: approval1.7? → approval1.7+
fixed1.7 with phishy length set to 1.  meaning that any non-zero length username
or password will trigger the prompt.  i made the change to nsHttpHandler.cpp
instead of modifying prefs.  the pref does not appear in all.js.

i have also made the same change from 16 to 1 on the trunk to be consistent.
Status: ASSIGNED → RESOLVED
Closed: 20 years ago20 years ago
Keywords: fixed1.7
Resolution: --- → FIXED
Wouldn't it be better if there was a choice to stop these confirmation dialogs?
I would really appreciate this.

(In reply to comment #70)
> fixed1.7 with phishy length set to 1.  meaning that any non-zero length username
> or password will trigger the prompt.  i made the change to nsHttpHandler.cpp
> instead of modifying prefs.  the pref does not appear in all.js.
> 
> i have also made the same change from 16 to 1 on the trunk to be consistent.

(In reply to comment #71)
> Wouldn't it be better if there was a choice to stop these confirmation dialogs?
> I would really appreciate this.

Haris: please note that you can modify the preference by using about:config

you'll need to add a new preference using about:config to set a value for
network.http.phishy-userpass-length that you like.  something very large
effectively disables this.

i know that you're probably thinking that there should be something in the UI to
make this easier for folks to disable, but at the moment there are no plans to
do that.
>Ideally, users should be able to disable the warning on a per-domain basis.

Yes, fix this in firefox ASAP!
(In reply to comment #63)
> note, if we use the auth from the URL, then it will only ever be taken from the
> URL once per session for that domain.

running 1.7 build Gecko/20040613
I'm surprised to be prompted for confirmation each time I access the server in
the same session. 
Actually, once authenticated, the username/password isn't removed from the URL,
as it used to be (isn't it ??)

> I'm surprised to be prompted for confirmation each time I access the server in
> the same session. 

Yeah, we need to add a patch that remembers the user's decision.


> Actually, once authenticated, the username/password isn't removed from the URL,
> as it used to be (isn't it ??)

That was never done AFAIK.  It was discussed, but never implemented.

I filed bug 248945 to cover having the browser remember that the user accepted
one of these dialogs.
Attachment #144353 - Flags: review?(dougt)
See Also: → 1291540
You need to log in before you can comment on or make changes to this bug.