Closed Bug 232567 Opened 21 years ago Closed 21 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: 21 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: 21 years ago21 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) (deleted) — — 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) (deleted) — — Splinter Review
As discussed on IRC.
Attached patch patch v3, now with pref (obsolete) (deleted) — — 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) (deleted) — — 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) (deleted) — — 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) (deleted) — — 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) (deleted) — — 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 patch (deleted) — — Splinter 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.
Attached patch v5.2 patch comment cleanup (deleted) — — Splinter Review
> 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: 21 years ago21 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)
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: