Closed
Bug 244273
(CVE-2008-0367)
Opened 20 years ago
Closed 17 years ago
improve current HTTP authentication prompt
Categories
(Core Graveyard :: Security: UI, enhancement, P2)
Core Graveyard
Security: UI
Tracking
(Not tracked)
RESOLVED
FIXED
mozilla1.9beta3
People
(Reporter: petrovicky, Assigned: Dolske)
References
()
Details
(Keywords: late-l10n, Whiteboard: [sg:low spoof] [passwordManager-ui])
Attachments
(8 files, 9 obsolete files)
User-Agent: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8a) Gecko/20040519 Firefox/0.8 Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8a) Gecko/20040519 Firefox/0.8 take a look at two enclosed attachments. one whows IE HTTP Auth prompt, one shows Firefox HTTP Auth prompt. guess which of these is nicer and more user friendly. shouldn't we also have something more nice, when we have IMHO the best browser artwork on the market? Reproducible: Always Steps to Reproduce: use everything that requires HTTP authentication and see the prompt. Actual Results: the prompt is ugly. Expected Results: the prompt should be much more nice, as the IE's prompt is.
Reporter | ||
Comment 1•20 years ago
|
||
Reporter | ||
Comment 2•20 years ago
|
||
Comment 3•20 years ago
|
||
setting->NEW if it's doable is another question though.
Status: UNCONFIRMED → NEW
Ever confirmed: true
Comment 4•18 years ago
|
||
*** Bug 262127 has been marked as a duplicate of this bug. ***
Updated•18 years ago
|
Assignee: bross2 → nobody
QA Contact: general
Updated•17 years ago
|
Blocks: 371000
Component: General → Password Manager
OS: Windows XP → All
QA Contact: general → password.manager
Hardware: PC → All
Version: unspecified → Trunk
Comment 5•17 years ago
|
||
Keys icon in IE's prompt seems odd to me. Dropdown in "User Name" input need some info in the backend i guess.
Comment 6•17 years ago
|
||
Why "Firefox needs your keys to unlock this door" if the keys icon seems odd to you? Anyways, I like the metaphor.
Comment 7•17 years ago
|
||
Well, keys is my (a user) part that i've got, lock is Firefox's one. It shows me that the page is locked and i need to "give/use my keys". I'd prefer keys icon for username(nickname)/password part in some registration form :)
Assignee | ||
Comment 8•17 years ago
|
||
So, I'm probably going to poke at this bug in the process of fixing bug 227632... I'm not sure we really want to add much bling to the prompt; I think the graphics in the IE prompt are are too strong, and overwhelm / clash with the simplicity of the prompt's content. For OS X, I think we're mostly in good shape with the platform's style. Still, some minor tweaks would clean things up... * Reformat the dialog text so it's not just one giant string - Make it easier to identify the server the auth is for - break the HTTP realm string out of the flow - Drop protocol aka locationbar2 / Safari prompt? [For HTTP only, and HTTPS with extra "submitting securely" blurb? * Text box labels on same line as textbox The little icon for Windows does look kind of ugly, and some subtle graphics might be a nice touch, but I'm not sure what to do while staying inside platform conventions.
Whiteboard: [passwordManager-ui]
Assignee | ||
Comment 9•17 years ago
|
||
Assignee | ||
Comment 10•17 years ago
|
||
Assignee | ||
Comment 11•17 years ago
|
||
Attachment #149035 -
Attachment is obsolete: true
Assignee | ||
Comment 12•17 years ago
|
||
Attachment #149036 -
Attachment is obsolete: true
Comment 13•17 years ago
|
||
Here is what the dialog looks like in Safari 3.
Comment 14•17 years ago
|
||
The URL requesting the login is essential. Without it, image servers and other ancillary servers can fool the user into giving out his password for the main site.
Assignee | ||
Comment 15•17 years ago
|
||
Note to self: also trim the max length of the realm string to prevent people from filling up the dialog with garbage, and squishing the buttons off-window or somesuch.
Comment 16•17 years ago
|
||
Please keep in mind embedders. the current API sucks. as for pictures, you can see the results of various clients by canceling the auth prompt for http://timeless.justdave.net/maemo/realm/realm.cgi Note that in the case of http auth, I really don't think we need to use a dialog at all and would rather we used a panel akin to the remember password panel or simply a web page. http://viper.haque.net/~timeless/blog/8/ the reason we need dialogs is when there's a sync call that can't be unwound, however http auth is a proxied call from another thread and as such it should be possible to arrange it so that it can wait forever. IMO, a web page is ideal, keep in mind that an auth dialog is no more or less safe than a prompt from the web page (and practically speaking the content is suggested by the web page, so it really shouldn't be considered "trustworthy"), it's going to the same place over the same channel. If you do use a web page, then we could recycle the web page for nested content (such as images). The only sucky case is http://timeless.justdave.net/maemo/realm/picture.html where there's no visual placeholder. Personally, for this case, I'd go with blocking those loads until the user opens the media tab of page info and selects the images. the media preview could then show the auth request and if the user happens to authenticate, the image can load. If someone really wanted to, they could create an info bar to tell the user that there are pending requests in the media tab. --- If you're going to stick with the current bad design, the following things are probably requirements: 1. clearly distinguish the different auth cases (proxy, realm, other) 2. stick the labels on the same lines as the values 3. make sure that all values are scrollable, selectable, and copyable 4. use a different face/weight for labels than values Note that text claiming that you must log in is false. Just because there's an auth prompt doesn't mean the only content available is the content reachable by entering values into the prompt. Ideally there should be a way to temporarily view the alternate content (401 prompts include alternate content for the failure case) without dismissing the auth dialog, perhaps what the user wants to see isn't really protected, in which case, if the user sees that, the user can then choose to dismiss the dialog. I think we should probably use "remember these credentials" (or something similar) instead of "remember my password". Especially when we're going to remember more than one set of credentials. (Opera 9 simply uses "remember password" which unlike the other variants is not *wrong*.) Personally, I'd like to be able to get help for these dialogs, they're confusing, there's enough context and questions available that IMO help should be provided, however at a glance, only IE and MicroB (and its Opera predecessor, but not desktop Opera) include help. (Among the things to explain is how to delete credentials once you've remembered them - it won't be intuitive.) dolske: please keep in mind that there might be a port floating around. Also, what happens if i load feed:http://timeless.justdave.net/maemo/realm/realm.cgi Nelson: I'm confused, assume we have the server and the realm, and we follow redirects, so the host is gmail.com and the image request is from g.mail.com, will the user really be able to recognize the difference? again, I'd rather not use dialogs. If you're worried about differing servers, I think we need some way to help the user recognize that the servers are different.
Comment 17•17 years ago
|
||
See http://aviv.raffon.net/2008/01/02/YetAnotherDialogSpoofingFirefoxBasicAuthentication.aspx for problems with the current HTTP auth prompt.
Flags: blocking-firefox3?
Updated•17 years ago
|
Whiteboard: [passwordManager-ui] → [sg:moderate spoof] [passwordManager-ui]
Comment 18•17 years ago
|
||
(In reply to comment #17) > See > http://aviv.raffon.net/2008/01/02/YetAnotherDialogSpoofingFirefoxBasicAuthentication.aspx > for problems with the current HTTP auth prompt. iCab seems to have the best UI to prevent this: http://timeless.justdave.net/maemo/realm/realm-icab.png
Comment 19•17 years ago
|
||
> iCab seems to have the best UI to prevent this:
Yes, clearly distinguishing between the server and the realm is clearly a better design than just giving the realm without any label. iCab's lack of a scrollbar for long realms is unfriendly to sites that use long realm values (I've seen legal "This site is only for the use of..." messages stuck in there before), though. And yeah, there definitely needs to be a Help button so users can find out what the heck a realm is.
I think the idea of changing the dialog into a "panel" that's connected to the originating window is a great idea for security. And conversely, making it into a standard web page would be a terrible idea, IMHO. I don't want to have to constantly be clicking on a "Trust ?" button to see whether login requests are real or spoofed.
I think it's very important to render the panel in a form that's impossible for a website to render (in the absence of browser bugs). For instance, it could overlap the status bar yet be without the chrome of a normal browser popup window that would be capable of doing that.
Updated•17 years ago
|
Whiteboard: [sg:moderate spoof] [passwordManager-ui] → [sg:low spoof] [passwordManager-ui]
Updated•17 years ago
|
Flags: blocking-firefox3? → blocking-firefox3+
Priority: -- → P2
Updated•17 years ago
|
Assignee: nobody → dolske
Comment 20•17 years ago
|
||
Can this bug be used to make the authentication prompt not so jarring and modal? What's particularly bad about it for me is that I often open a couple tabs at once and log into the various sites. One of them uses an HTTP auth. prompt, and as soon as it loads in a background tab it steals focus, interrupting my typing, and demands attention. The new "remember password" dialog was a gigantic improvement on the old one - perhaps something like that for this?
Comment 21•17 years ago
|
||
Labeling the field "realm" helps mitigate the spoofing threat, but that iCab dialog is really a textbook example of needlessly exposing the implementation model. At least it starts with a human readable message as opposed to saying "401," and the button is labeled OK instead of GET.
>there definitely needs to be a Help button so users
>can find out what the heck a realm is.
Exposing the implementation model to the user and then requiring them to read documentation to make informed decisions isn't all that user centric. If we are going to say "typically a description of the computer or system being accessed" in help, then let's just label it "description:"
Overall I'm leaning towards a content area message for the following reasons:
-tab specific
-non modal
-very clear who is producing the message since the location bar is visible
-site button is accessible for additional identity information
-consistent with the way you usually log in to web sites, in the content area
-clear that you are giving this information to a particular site, and not just Firefox (that 64x64 Firefox icon in the dialog is really messing up the cues for who is addressing the user).
Johnath should weigh in on this since he is our security UI guru.
Assignee | ||
Comment 22•17 years ago
|
||
So, I think discussion of more radical changes to the authentication prompt should be spun off to another bug(s), and limit the scope of this bug to the incremental changes that can be made in the Firefox 3 timeframe. I suspect major UI changes will require significant plumbing changes, and it's kind of late for that now. I've opened bug 411085 for discussion of such future UI. [Also changing components here, since these prompts are technically not part of the password manager.]
Assignee: dolske → kengert
No longer blocks: 411085
Component: Password Manager → Security: UI
Flags: blocking-firefox3+
Product: Firefox → Core
QA Contact: password.manager → ui
Target Milestone: --- → mozilla1.9 M11
Updated•17 years ago
|
Flags: blocking1.9?
Assignee | ||
Comment 23•17 years ago
|
||
Renoming, blocking-ff3+ flag lost in component move.
Assignee: kengert → dolske
Summary: create much nicer HTTP authentication prompt → improve current HTTP authentication prompt
Assignee | ||
Comment 24•17 years ago
|
||
This juggles the prompt contents around a bit, to make the input fields more compact and handle realms better... Long realms are truncated, and are shown at the end of the message to help avoid some forms of spoofing. The existing implementation is hobbled by its use of nsIPrompt's PromptUsernameAndPassword(), which is... oh. I thought it was frozen, but it's not (neither is nsIAuthPrompt). However, I think from previous discussions there was reluctance to change those interfaces. If we want to change the format of the dialog more, we'll either need to add/change interfaces to allow passing more than 1 message string, or have the nsIPrompt implementation parse things out of the string. The latter is obviously kind of messy, and carries a risk of the parser being tricked into putting things where it shouldn't.
Assignee | ||
Comment 25•17 years ago
|
||
Comment 26•17 years ago
|
||
the bigger problem is that nothing in content area can't be trusted. suppose you fix this auth stuff. via CSS (probably XUL) it is possible to make something that looks *almost exactly like http auth dialog* (the simplest case is to take a screenshoot, set it as a background of pseudo popup, put 2 styled inputs, show the pseudo dialog)
Comment 27•17 years ago
|
||
Doesn't solve content issues - but this is an easy fix for this particular case - so marking b+
Flags: blocking1.9? → blocking1.9+
Comment 28•17 years ago
|
||
(In reply to comment #25) screenshot in attachment 295732 [details] still gives significance to the spoof able realm message from the site. Why not bring the realm message down. And put it in a textarea. Something like following. ______________________________________ / \ | | | Login required for web site | | .----. goog...malicious.site.com | | | Ic | | | | on | User ID [ _____________ ] | | '----' | | Password [ _____________ ] | | | | | | [ Cancel ] [ OK ] | | | | | | .- Message from site --------------. | | | hello "world" hjkdsf ksj lsdl ^ | | | sldsl kllksd hjkf ksj lsdl as | | | | sl dsl kll ksd hjkd ksj lsdl | | | | sldsl kllksd hjkf ksj lsdl as V | | '----------------------------------' | | | \______________________________________/
Comment 29•17 years ago
|
||
Will the UI indicate it's a secure site? I guess people will have to derive that from https:// vs. http://
Comment 30•17 years ago
|
||
(In reply to comment #27) > Doesn't solve content issues - but this is an easy fix for this particular case > - so marking b+ > i am missing something. unexpected dialog asks luser's password and the luser handles it. does it matter if the dialog is the real auth dialog or some CSS perversion - clearly no imo.
Updated•17 years ago
|
Flags: wanted1.8.1.x+
Flags: blocking1.8.1.12?
Updated•17 years ago
|
Flags: blocking1.8.1.12? → blocking1.8.1.13?
Comment 31•17 years ago
|
||
Comment on attachment 295731 [details] [diff] [review] Patch, work in progress (In reply to comment #30) > (In reply to comment #27) > > Doesn't solve content issues - but this is an easy fix for this particular case > > - so marking b+ > > > > i am missing something. > > unexpected dialog asks luser's password and the luser handles it. > > does it matter if the dialog is the real auth dialog or some CSS perversion - > clearly no imo. As schrep says, this obviously doesn't fix password spoofing in content, but basic housekeeping suggests that we not allow arbitrarily long strings to break our dialogs, if nothing else. >-EnterUserPasswordForRealm=Enter username and password for "%1$S" at %2$S >-EnterUserPasswordForProxy=Enter username and password for proxy "%1$S" at %2$S >+# XXX Add a localizer note to always have %1 at the end? >+EnterUserPasswordForRealm=A username and password is being requested by %2$S for "%1$S" >+EnterUserPasswordForProxy=The proxy %2$S is requesting a username and password for "%1$S" Dolske, if it's undesirable to tinker with commonDialog's structure here, what do you think about styling these fields to use -moz-pre-wrap, and then using these strings to break up the text, to pick up some of the spirit of comments like comment 28? Maybe EnterUserPasswordForRealm=A username and password is being requested by %2$S.\n\nThe site says: "%1$S" ... or structure to that effect? I do agree that the localizer note should, if not specify location, at least make it clear that, in direct opposition to the normal approach, these strings should NOT be neatly integrated into the sentence structure, we want them to stand out as being something the site is saying, not firefox. I know you're not at review stage yet, but I suspect we'll also need to rev the entities here? >+ if (realm.Length() > 250) { >+ realm.Truncate(250); >+ realm.Append(NS_LITERAL_STRING("...")); >+ } Looking at your screencap, 250 still looks pretty long. I don't have justification for any specific, smaller number, but I wouldn't personally object to, say, 150. And at some point, you might get the ellipsis police (bug 255051) after you, too. :)
Comment 32•17 years ago
|
||
Adding Axel because of string changes if it will find its way into 1.8 branch.
Status: NEW → ASSIGNED
Assignee | ||
Comment 33•17 years ago
|
||
(In reply to comment #31) > Dolske, if it's undesirable to tinker with commonDialog's structure here, what > do you think about styling these fields to use -moz-pre-wrap, and then using > these strings to break up the text, to pick up some of the spirit of comments > like comment 28? Maybe > > EnterUserPasswordForRealm=A username and password is being requested by > %2$S.\n\nThe site says: "%1$S" Hrmm, perhaps, but that means we'd have to do more filtering on %1 to keep sites from abusing the -moz-pre-wrap (coalescing white-space, stripping new lines, etc). I'm leery about doing that.
Assignee | ||
Comment 34•17 years ago
|
||
Update from WIP1 - shortened the max realm length and tweaked the strings and property values. > And at some point, you might get the ellipsis police (bug 255051) after you, > too. :) I'm not how to fix this properly in code (see bug 407481). Since the "..." shouldn't normally be shown, I'm sorta inclined to leave it as is. The other option would be to to duplicate these strings, and have one for "blahblah %1$S" and "blahblah %1$S...".
Attachment #295731 -
Attachment is obsolete: true
Attachment #295732 -
Attachment is obsolete: true
Attachment #298212 -
Flags: ui-review?(johnath)
Attachment #298212 -
Flags: review?(johnath)
Assignee | ||
Comment 35•17 years ago
|
||
(Not my real password length; may be longer or shorter. :-)
Comment 36•17 years ago
|
||
Comment on attachment 298212 [details] [diff] [review] Patch v.2 for review I like the look of the inlining fields with labels, and the shorter truncation length. "The site says:..." feels a little awkward, but I think it conveys the necessary message, that what follows is a claim of the content. The code changes look good to me, but I'm not technically a peer here. :) Why did you remove the comment blocks in commonDialog.xul? They aren't exactly wordy, but they do provide at-a-glance structure, and match the rest of the file, don't they?
Attachment #298212 -
Flags: ui-review?(johnath)
Attachment #298212 -
Flags: ui-review+
Attachment #298212 -
Flags: review?(johnath)
Assignee | ||
Updated•17 years ago
|
Attachment #298212 -
Flags: review?(neil)
Assignee | ||
Updated•17 years ago
|
Attachment #298212 -
Flags: review?(benjamin)
Comment 37•17 years ago
|
||
Inlining the fields with the labels is a problem for the prompt() function, because its label is blank (it only populates the description).
Comment 38•17 years ago
|
||
How about positioning the textboxes under the icon, but with a min-width for the username/password column so that an empty prompt will line up with the description. (I'm not sure we should make the description line up with the textfields in the username/password case.) You can see various examples of what I've been trying here: http://neil.rashbrook.org/prompt1.xul http://neil.rashbrook.org/prompt2.xul http://neil.rashbrook.org/prompt3.xul
Comment 39•17 years ago
|
||
this appears related to bug 38019 perhaps.
Comment 40•17 years ago
|
||
Comment on attachment 298212 [details] [diff] [review] Patch v.2 for review rubberstamping the embedding parts, but I'm not the right person for a full-on review of this ;-)
Attachment #298212 -
Flags: review?(benjamin) → review+
Assignee | ||
Comment 41•17 years ago
|
||
Updated to change layout, as noted in previous comments. Also slight grammar change: "A username and password are being requested" instead of "...is being requested". I think that's right? The XUL/CSS probably still needs a bit of tweaking, as the dialog seems to have become a bit wider than it was before. Maybe it's just the old width + the new <column>? I should also probably make sure things don't fall apart with a long realm/host without any whitespace.
Attachment #298212 -
Attachment is obsolete: true
Attachment #298213 -
Attachment is obsolete: true
Attachment #299397 -
Flags: review?(neil)
Attachment #298212 -
Flags: review?(neil)
Assignee | ||
Comment 42•17 years ago
|
||
Fixes width problems, the original XUL had a max-width which was deleted. :-) I've added that back via CSS. I also noticed that we were using -moz-pre-wrap, which I removed for the reason I gave in comment 33.
Attachment #299397 -
Attachment is obsolete: true
Attachment #299490 -
Flags: review?(neil)
Attachment #299397 -
Flags: review?(neil)
Assignee | ||
Comment 43•17 years ago
|
||
Comment 44•17 years ago
|
||
// Trim obnoxiously long realms. if (realm.Length() > 150) { realm.Truncate(150); realm.Append(NS_LITERAL_STRING("...")); } this usage of |150| isn't very audit friendly. what about: #define MAXREALMLENGTH 150 ... realm.Truncate(MAXREALMLENGTH); ... #undef MAXREALMLENGTH
Comment 45•17 years ago
|
||
Comment on attachment 299490 [details] [diff] [review] Patch v.4 >+ realm.Append(NS_LITERAL_STRING("...")); This needs to be the localised intl.ellipsis pref. >+ <vbox id="checkboxContainer" align="start" collapsed="true"> >+ <checkbox id="checkbox" oncommand="onCheckboxClick(this);"/> >+ </vbox> The old checkbox container was aligned under the description and I think it needs to stay that way. r=me with that fixed. >+#commonDialog #infoContainer { Nit: double space ;-)
Attachment #299490 -
Flags: review?(neil) → review+
Comment 46•17 years ago
|
||
(In reply to comment #42) >I also noticed that we were using -moz-pre-wrap, which I removed Actually this was added to fix bug 317334 (compare comment #31).
Assignee | ||
Comment 47•17 years ago
|
||
(In reply to comment #44) > this usage of |150| isn't very audit friendly. I think as long as it's just on two sequential lines, it's not a big deal and the #define just makes things murkier. (In reply to comment #46) > >I also noticed that we were using -moz-pre-wrap, which I removed > Actually this was added to fix bug 317334 (compare comment #31). Hmm. I don't really understand why, but I suppose it's useful in another way as it allows formatted prompts. EG: javascript:confirm("orly?\n\nyarly") Since this is shared dialog code, we probably shouldn't break that case. So, I've put the -moz-pre-wrap back in, but that means untrusted content can still make enormous dialogs by embedding newlines. [EG, a http realm with newlines, or simply alert("\n\n\n\n")] Perhaps a simple general-purpose solution would be to add "overflow: -moz-scrollbars-vertical;" so that large content gets scrollbars? I tried experimenting with this, but I can't seem to get it to work right. :-(
Assignee | ||
Comment 48•17 years ago
|
||
Updated with review comments.
Attachment #299490 -
Attachment is obsolete: true
Assignee | ||
Comment 49•17 years ago
|
||
Ahh. Bug 115997 covers the problem of giant dialogs due to excessive text (by adding scrollable areas). Since that's an annoyance but not a spoofing concern, let's go ahead and wrap up this bug as-is. 115997 can fix the annoyance.
Assignee | ||
Comment 50•17 years ago
|
||
Got a few more comments from Neil and dwitte.
Attachment #299652 -
Attachment is obsolete: true
Attachment #299879 -
Flags: review?(neil)
Updated•17 years ago
|
Attachment #299879 -
Flags: review?(neil) → review+
Assignee | ||
Comment 51•17 years ago
|
||
Comment on attachment 299879 [details] [diff] [review] Patch v.6 Requesting approval due to late-l10n... Just two string changes, at the top of the patch.
Attachment #299879 -
Flags: approval1.9?
Comment 52•17 years ago
|
||
That looks great, just small nits on the localization note: # LOCALIZATION NOTE (EnterLoginForRealm, EnterLoginForProxy): # %1 is an untrusted string provided by a remote server. It # could try to take advantage of sentence structure in order to mislead the # user (see bug 244273). %1 should be integrated into the translated # sentences as little as possible. # %2 is the ... domain/url? That's stricter on the format, and a few less "it"s for %1. And %2 wasn't explained at all, as you're at it, it'd be good to add. I don't need a new patch for that.
Comment 53•17 years ago
|
||
Comment on attachment 299879 [details] [diff] [review] Patch v.6 a1.9+=damons
Attachment #299879 -
Flags: approval1.9? → approval1.9+
Assignee | ||
Comment 54•17 years ago
|
||
Checked in. Checking in embedding/components/windowwatcher/src/nsPrompt.cpp; /cvsroot/mozilla/embedding/components/windowwatcher/src/nsPrompt.cpp,v <-- nsPrompt.cpp new revision: 1.34; previous revision: 1.33 done Checking in toolkit/content/commonDialog.css; /cvsroot/mozilla/toolkit/content/commonDialog.css,v <-- commonDialog.css new revision: 1.3; previous revision: 1.2 done Checking in toolkit/content/commonDialog.js; /cvsroot/mozilla/toolkit/content/commonDialog.js,v <-- commonDialog.js new revision: 1.14; previous revision: 1.13 done Checking in toolkit/content/commonDialog.xul; /cvsroot/mozilla/toolkit/content/commonDialog.xul,v <-- commonDialog.xul new revision: 1.16; previous revision: 1.15 done Checking in toolkit/themes/pinstripe/global/formatting.css; /cvsroot/mozilla/toolkit/themes/pinstripe/global/formatting.css,v <-- formatting.css new revision: 1.7; previous revision: 1.6 done Checking in toolkit/themes/pinstripe/global/global.css; /cvsroot/mozilla/toolkit/themes/pinstripe/global/global.css,v <-- global.css new revision: 1.20; previous revision: 1.19 done Checking in dom/locales/en-US/chrome/prompts.properties; /cvsroot/mozilla/dom/locales/en-US/chrome/prompts.properties,v <-- prompts.properties new revision: 1.2; previous revision: 1.1 done
Status: ASSIGNED → RESOLVED
Closed: 17 years ago
Flags: in-testsuite-
Flags: in-litmus?
Keywords: late-l10n
Resolution: --- → FIXED
Assignee | ||
Comment 55•17 years ago
|
||
Oops, I noticed Pike's suggestion for the localization note after my checkin, so I've updated that too. [Comment change only.] Checking in dom/locales/en-US/chrome/prompts.properties; /cvsroot/mozilla/dom/locales/en-US/chrome/prompts.properties,v <-- prompts.properties new revision: 1.3; previous revision: 1.2 done
Assignee | ||
Comment 56•17 years ago
|
||
There was a question about where this got fixed: This is a trunk-only fix currently, and will be included in FF3 Beta 3. It's not yet fixed on branch. Speaking of which, I'm not sure how we handle changes like this on branch. The core of the fix is a string change, can we do that in point releases?
Comment 57•17 years ago
|
||
No, we don't do string changes on the branch. Unless we had a chance to backport the fix including l10n from trunk onto the branch, maybe. But even then, I'd strongly discourage that, as that'd break existing language packs, for which we can't back-port.
Comment 58•17 years ago
|
||
Even though it's low severity, the fix still mitigates a public security bug. We should get this on branch if we can.
Comment 59•17 years ago
|
||
Can we think of a different fix for the branch that simply pulls strings we already have and uses them so localizers don't need to localize a new string? My guess is that we can use existing strings to make this possible on the branch.
Comment 60•17 years ago
|
||
We can't backport to all localizations out there, as there are a few that we just don't have access to, and that are Language packs, claiming to be compatible with Firefox 2.0.0.*. As langpacks don't update from AMO (other story, other bug), there's no way to fix that. Thus, a branch patch would have to fall back to something if the new strings wouldn't be present. Either by falling back to the current UI, or to English strings. We would have to keep the old strings, too, so that then updated langpacks would still work on older Firefox 2.0.0.x versions.
Comment 61•17 years ago
|
||
Axel, sorry, I should've been more clear. I meant using *branch* strings that already exist not creating new strings on the branch. It would require a different fix than we have on the trunk and might not be the ideal, but would help fix this bug. Something along the lines of... Host: <hostname here> Description: <description here> Obviously that's a different fix than on trunk, but assuming we have both the "Host" and "Description" strings, it'd solve our localization issues.
Comment 62•17 years ago
|
||
Sadly, that's not totally safe. Strings that are the same in English don't necessarily imply that they will be the same in other languages. So we wouldn't just need the same strings, we'd need the same strings in a very similar context, and we'd probably want to run that setup through a review of a few localizers, to see if there's a grammar mismatch.
Assignee | ||
Comment 63•17 years ago
|
||
In 1.8, the prompt string is built up at http://mxr.mozilla.org/mozilla1.8/source/netwerk/protocol/http/src/nsHttpChannel.cpp#2697 using the bundle at /netwerk/locales/en-US/necko.properties. I think Pike's suggestion to add new strings with fallback to the old strings (or English) if they're not available sounds like the way to go. I can't think of a better idea. Should we fallback to the old localized string, or English? Using the old string means locales that don't get updated would still have spoofable dialogs. That might be ok, considering that this is a low-severity problem. Also consider that an English fallback would also carry a spoofing risk, in that someone who does not speak English wouldn't understand it, and so the only readable thing might be the attacker's localized HTTP realm spoof. For example, if it was English falling back to a Latin locale, the problem might look like: Καθυστερεί χαρακτηριστικό σε "http://safe.trustme.com" όρο, νέα οι υποψήφιο τελειώσει, μα υλικό φταίει εφαμοργής σας. Ως νέο "Enter your paypal login. It's totally safe and secure!".
Updated•17 years ago
|
Alias: CVE-2008-0367
Comment 64•17 years ago
|
||
(In reply to comment #14) > The URL requesting the login is essential. Without it, image servers and > other ancillary servers can fool the user into giving out his password for > the main site. I think it would probably be better to not depend on the user identifying and comparing this URL. You would be better off doing some automated checks first before throwing it in the user’s face, for example, only show this URL (in red) if it is different from the domain currently opened in the browser chrome. Should I move this comment to bug 411085? ~Grauw
Comment 65•17 years ago
|
||
If we got a patch that fixes this for English and falls back to the existing translated strings if there are not _new_ translations then we could take that.
Updated•17 years ago
|
Flags: blocking1.8.1.13?
Comment 66•14 years ago
|
||
Dolske, you flagged this for in-litmus? 2 years ago. What exactly did you want tested here? We currently have a set of authentication/password manager tests which test the underlying functionality.
Assignee | ||
Comment 67•14 years ago
|
||
I suspect this simply predated the tests added for the auth prompt.
Flags: in-litmus?
Updated•8 years ago
|
Product: Core → Core Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•