Bug 244273 (CVE-2008-0367)

improve current HTTP authentication prompt

RESOLVED FIXED in mozilla1.9beta3

Status

Core Graveyard
Security: UI
P2
enhancement
RESOLVED FIXED
13 years ago
7 months ago

People

(Reporter: Lukas Petrovicky (CZilla), Assigned: Dolske)

Tracking

(Depends on: 1 bug, {late-l10n})

Trunk
mozilla1.9beta3
late-l10n
Dependency tree / graph
Bug Flags:
blocking1.9 +
wanted1.8.1.x +
in-testsuite -

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [sg:low spoof] [passwordManager-ui], URL)

Attachments

(8 attachments, 9 obsolete attachments)

28.38 KB, image/png
Details
25.29 KB, image/png
Details
38.65 KB, image/png
Details
29.18 KB, image/png
Details
42.23 KB, image/png
Details
44.19 KB, image/png
Details
122.17 KB, image/png
Details
11.79 KB, patch
neil@parkwaycc.co.uk
: review+
damons
: approval1.9+
Details | Diff | Splinter Review
(Reporter)

Description

13 years ago
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

13 years ago
Created attachment 149035 [details]
firefox HTTP Auth prompt
(Reporter)

Comment 2

13 years ago
Created attachment 149036 [details]
IE HTTP Auth prompt
setting->NEW
if it's doable is another question though.
Status: UNCONFIRMED → NEW
Ever confirmed: true

Comment 4

11 years ago
*** Bug 262127 has been marked as a duplicate of this bug. ***
Assignee: bross2 → nobody
QA Contact: general

Updated

10 years ago
Blocks: 371000
Component: General → Password Manager
OS: Windows XP → All
QA Contact: general → password.manager
Hardware: PC → All
Version: unspecified → Trunk

Updated

10 years ago
Depends on: 376668
Created attachment 270180 [details]
An mockup for HTTP auth prompt

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

10 years ago
Why "Firefox needs your keys to unlock this door" if the keys icon seems odd to you? Anyways, I like the metaphor.
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

10 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

10 years ago
Created attachment 282061 [details]
Firefox on OS X
(Assignee)

Comment 10

10 years ago
Created attachment 282062 [details]
Safari on OS X
(Assignee)

Comment 11

10 years ago
Created attachment 282063 [details]
Firefox on Vista
Attachment #149035 - Attachment is obsolete: true
(Assignee)

Comment 12

10 years ago
Created attachment 282065 [details]
IE7 on Vista
Attachment #149036 - Attachment is obsolete: true
Created attachment 282066 [details]
Safari 3 httpauth dialog

Here is what the dialog looks like in Safari 3.
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

10 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

10 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.
See http://aviv.raffon.net/2008/01/02/YetAnotherDialogSpoofingFirefoxBasicAuthentication.aspx for problems with the current HTTP auth prompt.
Flags: blocking-firefox3?

Updated

10 years ago
Whiteboard: [passwordManager-ui] → [sg:moderate spoof] [passwordManager-ui]
(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

10 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

10 years ago
Whiteboard: [sg:moderate spoof] [passwordManager-ui] → [sg:low spoof] [passwordManager-ui]

Updated

10 years ago
Flags: blocking-firefox3? → blocking-firefox3+
Priority: -- → P2

Updated

10 years ago
Assignee: nobody → dolske

Comment 20

10 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?

Updated

10 years ago
Depends on: 387652
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)

Updated

10 years ago
Blocks: 411085
(Assignee)

Comment 22

10 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
Flags: blocking1.9?
(Assignee)

Comment 23

10 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

10 years ago
Created attachment 295731 [details] [diff] [review]
Patch, work in progress

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

10 years ago
Created attachment 295732 [details]
WIP1 screenshot
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

10 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

10 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

10 years ago
Will the UI indicate it's a secure site?
I guess people will have to derive that from https:// vs. http://
(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.
Flags: wanted1.8.1.x+
Flags: blocking1.8.1.12?
Flags: blocking1.8.1.12? → blocking1.8.1.13?
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. :)
Adding Axel because of string changes if it will find its way into 1.8 branch.
Status: NEW → ASSIGNED
(Assignee)

Comment 33

10 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

10 years ago
Created attachment 298212 [details] [diff] [review]
Patch v.2 for review

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

10 years ago
Created attachment 298213 [details]
Screenshot with patch v.2

(Not my real password length; may be longer or shorter. :-)
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

10 years ago
Attachment #298212 - Flags: review?(neil)
(Assignee)

Updated

10 years ago
Attachment #298212 - Flags: review?(benjamin)

Comment 37

10 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

10 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

10 years ago
this appears related to bug 38019 perhaps.
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

10 years ago
Created attachment 299397 [details] [diff] [review]
Patch v.3

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

10 years ago
Created attachment 299490 [details] [diff] [review]
Patch v.4

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

10 years ago
Created attachment 299491 [details]
Screenshot with patch v.4
  // 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

10 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

10 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

10 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

10 years ago
Created attachment 299652 [details] [diff] [review]
Patch v.5

Updated with review comments.
Attachment #299490 - Attachment is obsolete: true
(Assignee)

Comment 49

10 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

10 years ago
Created attachment 299879 [details] [diff] [review]
Patch v.6

Got a few more comments from Neil and dwitte.
Attachment #299652 - Attachment is obsolete: true
Attachment #299879 - Flags: review?(neil)

Updated

10 years ago
Attachment #299879 - Flags: review?(neil) → review+
(Assignee)

Comment 51

10 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

10 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 on attachment 299879 [details] [diff] [review]
Patch v.6

a1.9+=damons
Attachment #299879 - Flags: approval1.9? → approval1.9+
(Assignee)

Comment 54

10 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
Last Resolved: 10 years ago
Flags: in-testsuite-
Flags: in-litmus?
Keywords: late-l10n
Resolution: --- → FIXED
(Assignee)

Comment 55

10 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

10 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

10 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

10 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.
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

10 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.
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

10 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

10 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

10 years ago
Alias: CVE-2008-0367
No longer depends on: 387652

Comment 64

10 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
Depends on: 417973
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.
Flags: blocking1.8.1.13?
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

7 years ago
I suspect this simply predated the tests added for the auth prompt.
Flags: in-litmus?
Product: Core → Core Graveyard
You need to log in before you can comment on or make changes to this bug.