244273 - (CVE-2008-0367) improve current HTTP authentication prompt

Status: RESOLVED FIXED

(Core Graveyard :: Security: UI, enhancement, P2)

Description

[Lukas Petrovicky (CZilla)]

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.

Comment 1

[Lukas Petrovicky (CZilla)]

Attachment: firefox HTTP Auth prompt

Comment 2

[Lukas Petrovicky (CZilla)]

Attachment: IE HTTP Auth prompt

Comment 3

[Peter van der Woude [:Peter6]]

setting->NEW
if it's doable is another question though.

Comment 4

[Ryan Jones-Ward [:sciguyryan]]

*** Bug 262127 has been marked as a duplicate of this bug. ***

Comment 5

[Aleksei Perepelkin [:chaos]]

Attachment: 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

[Dão Gottwald [:dao]]

Why "Firefox needs your keys to unlock this door" if the keys icon seems odd to you? Anyways, I like the metaphor.

Comment 7

[Aleksei Perepelkin [:chaos]]

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 :)

Comment 8

[Justin Dolske [:Dolske]]

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.

Comment 9

[Justin Dolske [:Dolske]]

Attachment: Firefox on OS X

Comment 10

[Justin Dolske [:Dolske]]

Attachment: Safari on OS X

Comment 11

[Justin Dolske [:Dolske]]

Attachment: Firefox on Vista

Comment 12

[Justin Dolske [:Dolske]]

Attachment: IE7 on Vista

Comment 13

[Alex Faaborg [:faaborg] (Firefox UX)]

Attachment: Safari 3 httpauth dialog

Here is what the dialog looks like in Safari 3.

Comment 14

[Nelson Bolyard (seldom reads bugmail)]

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.

Comment 15

[Justin Dolske [:Dolske]]

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

[timeless]

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

[Reed Loden [:reed]]

See http://aviv.raffon.net/2008/01/02/YetAnotherDialogSpoofingFirefoxBasicAuthentication.aspx for problems with the current HTTP auth prompt.

Comment 18

[Dão Gottwald [:dao]]

(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

[Dan Harkless]

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

Comment 20

[Bo]

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

[Alex Faaborg [:faaborg] (Firefox UX)]

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.

Comment 22

[Justin Dolske [:Dolske]]

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

Comment 23

[Justin Dolske [:Dolske]]

Renoming, blocking-ff3+ flag lost in component move.

Comment 24

[Justin Dolske [:Dolske]]

Attachment: 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.

Comment 25

[Justin Dolske [:Dolske]]

Attachment: WIP1 screenshot

Comment 26

[georgi - hopefully not receiving bugspam]

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

[Mike Schroepfer]

Doesn't solve content issues - but this is an easy fix for this particular case - so marking b+

Comment 28

[Biju]

(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

[Kai Engert (:KaiE:)]

Will the UI indicate it's a secure site?
I guess people will have to derive that from https:// vs. http://

Comment 30

[georgi - hopefully not receiving bugspam]

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

Comment 31

[Johnathan Nightingale [:johnath]]

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

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Adding Axel because of string changes if it will find its way into 1.8 branch.

Comment 33

[Justin Dolske [:Dolske]]

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

Comment 34

[Justin Dolske [:Dolske]]

Attachment: 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...".

Comment 35

[Justin Dolske [:Dolske]]

Attachment: Screenshot with patch v.2

(Not my real password length; may be longer or shorter. :-)

Comment 36

[Johnathan Nightingale [:johnath]]

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?

Comment 37

[[e-mail]]

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

[[e-mail]]

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

[Hugh Kennedy]

this appears related to bug 38019 perhaps.

Comment 40

[Benjamin Smedberg]

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 ;-)

Comment 41

[Justin Dolske [:Dolske]]

Attachment: 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.

Comment 42

[Justin Dolske [:Dolske]]

Attachment: 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.

Comment 43

[Justin Dolske [:Dolske]]

Attachment: Screenshot with patch v.4

Comment 44

[georgi - hopefully not receiving bugspam]

  // 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

[[e-mail]]

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 ;-)

Comment 46

[[e-mail]]

(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).

Comment 47

[Justin Dolske [:Dolske]]

(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. :-(

Comment 48

[Justin Dolske [:Dolske]]

Attachment: Patch v.5

Updated with review comments.

Comment 49

[Justin Dolske [:Dolske]]

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.

Comment 50

[Justin Dolske [:Dolske]]

Attachment: Patch v.6

Got a few more comments from Neil and dwitte.

Comment 51

[Justin Dolske [:Dolske]]

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.

Comment 52

[Axel Hecht [:Pike]]

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

[Damon Sicore (:damons)]

Comment on attachment 299879 [details] [diff] [review]
Patch v.6

a1.9+=damons

Comment 54

[Justin Dolske [:Dolske]]

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

Comment 55

[Justin Dolske [:Dolske]]

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

Comment 56

[Justin Dolske [:Dolske]]

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

[Axel Hecht [:Pike]]

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

[Window Snyder]

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

[Samuel Sidler (old account; do not CC)]

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

[Axel Hecht [:Pike]]

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

[Samuel Sidler (old account; do not CC)]

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

[Axel Hecht [:Pike]]

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.

Comment 63

[Justin Dolske [:Dolske]]

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!".

Comment 64

[u81239]

(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

[Daniel Veditz [:dveditz]]

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.

Comment 66

[u279076]

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.

Comment 67

[Justin Dolske [:Dolske]]

I suspect this simply predated the tests added for the auth prompt.