282270 - Display IDN urls as punycode by default (pref controlled)

Status: VERIFIED FIXED

(Core :: Networking, defect)

Description

[Gervase Markham [:gerv]]

Until we have a better solution, [e-mail] wish network.enableIDN to
be turned off for Firefox 1.0.1 and Mozilla 1.8 beta. IDN should end up disabled
for anyone who updates to these releases, although if they subsequently turn it
on themselves (either using about:config or the XPI in bug 282269) then it
should stay that way.

Gerv

Comment 1

[Daniel Veditz [:dveditz]]

Blocking the upcoming releases. *Not* blocking 1.1 because we're hoping to have
the IDN issues resolved enough to turn it back on then, but in any case if this
is checked in for 1.8b it'll be that way for 1.1

Comment 2

[Darin Fisher]

What about Mozilla 1.7.6?  Looks like dveditz marked blocking1.7.6+, but you
only mentioned 1.0.1 and 1.8b.

Comment 3

[idn]

Why do you not make a messagebox or other sign, if the browser is on a xn--
Domaine ?
Then is the visit possible and not blocked the Idea of IDE

Comment 4

[idn]

And if enable IDN with about:config, no user make this. When, make in the
Preferences a Checkbox to enable.

If meaningfully when is the standard ON

Comment 5

[Gervase Markham [:gerv]]

This does include 1.7.6.

To be clear: we need code which disables IDN, but remembers the original state,
so we can later have code which restores it, when and if we have a longer-term
solution to the IDN problem. But we need a way of people manually re-enabling it
if they wish to.

This is not just a case of setting the default in prefs.js, because that will
apply only to new profiles.

So, I suggest the following implementation strategy:

- Change the pref which the IDN subsystem checks to be "network.enableIDN2". 
- Default that pref to "false" in prefs.js
- Make the XPI in bug 282269 toggle that pref, not another one.

This preserves the original value of the network.enableIDN pref by the simple
expident of not touching it at all. When we have a better solution, we just
change the IDN system back to checking the network.enableIDN pref, and the
network.enableIDN2 pref gets abandoned.

Gerv

Comment 6

[Darin Fisher]

What is the point of making things so complicated?  Let's just change the
default setting for network.enableIDN to false.  If people switch that to true
in their own profiles, then it won't matter if a future version of Firefox also
has the default value switched to true.

As a side note: I think we should make the pref system preserve user choices in
prefs.js even if those user choices match the default choices, and that's
something we could fix for Firefox 1.1 as well.  But, I don't think that that
"bug" is a concern here.

Comment 7

[Darin Fisher]

Attachment: v1 patch

Comment 8

[Gervase Markham [:gerv]]

Darin: this patch protects none of our 25,000,000+ current users! It only
changes the default value for new profiles. 

It also means that, when and if we want to re-enable IDN support, we won't know
what to do. In a particular profile, is IDN off because we switched it off (in
which case, we should switch it back on), or because the user switched it off
(in which case, we shouldn't)? With this proposal, there's no way to tell.

In the future, we can't go around re-enabling IDN for people who've turned it
off explicitly. We need to respect their choice, and so we need to distinguish
between the two cases.

Gerv

Comment 9

[Darin Fisher]

> Darin: this patch protects none of our 25,000,000+ current users! It only
> changes the default value for new profiles. 

I don't follow.  The users preferences are overlaid on top of the default set. 
So, assuming that users have not changed the default value of the pref to false
(i.e., that they did not hand edit all.js), their profiles would not have the
value set to true.  So, changing the default value to false, would affect all
existing profiles as desired.


> It also means that, when and if we want to re-enable IDN support, we won't 
> know what to do.

I believe that we should enable IDN when we have proper anti-spoofing measures
in place.  Yes, this means changing the default value of the preference back to
true.

I understand your concern about giving people a way to say that they don't want
IDN when they install Firefox 1.x (x > 0), but that seems like a different
problem.  Perhaps Firefox 1.x should introduce the user to IDN when first
encountered on the web or something like that so that they can make a decision
at that time about whether or not they want IDN in Firefox 1.x.

Introducing additional prefs at this time seems like unnecessary complication. 
If any users have set network.enableIDN to false now it is because they don't
want IDN now.  Their choice will be preserved when they install Firefox 1.0.1,
and like I said we should not enable IDN in Firefox 1.x without explaining what
that means to the user and letting them choose at that time.

Comment 10

[Gervase Markham [:gerv]]

> The users preferences are overlaid on top of the default set.

Ah, sorry - that's the bit of info I was missing. I thought all.js was the
initial defaults, and got copied into the profile, and was then
overwritten/modified as users changed prefs.

So your solution is fine, then :-)

Gerv

Comment 11

[Daniel Veditz [:dveditz]]

Comment on attachment 174455 [details] [diff] [review]
v1 patch

>-pref("network.enableIDN", true);
>+pref("network.enableIDN", false);

Just to make sure everything is clear on this default pref business (the
comments show some confusion) changing this default means *everyone* will have
IDN turned off by the next release. Our pref system "optimizes" by deleting
user settings that match the default. This causes problems, for example,
sharing profiles between Netscape and Mozilla Suite: any pref that has
different product defaults will end up removed from your profile so you will
get the differing product defaults rather than your preferred setting. Or
switching back and forther between older and newer versions (such as testing
the trunk, using the last stable) when we change a default value.

In this IDN case setting this to false will set everyone to false. Anyone who
currently has explicitly chosen false will have their setting "optimized" away,
anyone who had the default true will get the new default false. When we turn
this pref back on in a future release it will be turned back on for everyone,
even the people who had explicitly turned it off prior to our upcoming
releases.

This should be OK: these people turned it off because of the spoofing problem
and we will presumably have a solution to that before turning it back on. Some
number of them may wish it to remain off and they may be surprised to find that
it is not. Worse, they may trust a spoof because they remember killing the
feature so it "can't" be an IDN domain.

We could avoid this by changing the pref name as Gerv suggests (need changes in
nsStandardURL.cpp, nsDNSService2.cpp, and nsHttpHandler.cpp -- this constant
should have been in a shared header), but this simpler solution is the better
one for most people.

r/sr=dveditz
a=dveditz on behalf of drivers, we discussed this and this is what we want.

Comment 12

[Christian :Biesinger (don't email me, ping me on IRC)]

do note that in case anyone has this pref in user.js, then that setting will not
be affected.

Comment 13

[Robert Kaiser]

I heard some reservations if just turning off the pref will stop links to IDNs
of working and if it will stop us sending IDN names to proxies which resolve
them. Does that help in those cases? If it doesn't, I fear it's a less-than-half
solution where we want a full solution :(

Comment 14

[Darin Fisher]

Attachment: v2 patch

This patch disabled IDN in a slightly different way.  It makes it so that we
convert any unicode hostname to punycode, so that HREFs using IDN will continue
to function.  It also eliminates the need to worry about IDN aware proxies.  To
avoid the spoofing problems, this patch makes it so that we never convert from
punycode back to unicode.  A preference is introduced that allows users to
enable full support for IDN.

Comment 15

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Comment on attachment 174547 [details] [diff] [review]
v2 patch

>+#define NS_NET_PREF_IDNTESTBED      "network.IDN_testbed"
>+#define NS_NET_PREF_IDNPREFIX       "network.IDN_prefix"

Were these preferences previously broken or working?  (That depends on
initialization order, right?  i.e., whether the IDN service was created before
the profile was selected?)  I'm not crazy about the idea of making them work if
they were broken before.

>+  PRBool mShowPunyCode;

Punycode is one word, so the C shouldn't be capitalized.

Feel free to roll in the fix to bug 282520 if you want.

I'll look in more detail later.

Comment 16

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Comment on attachment 174547 [details] [diff] [review]
v2 patch

A few more thoughts:

While this has the advantatage of not breaking links, bookmarks, or
international *input* in the URL bar, does it break cookies?  If so, is there
an easy fix?

The previous patch should be backed out when this is landed (if that patch has
landed).

Comment 17

[Darin Fisher]

> While this has the advantatage of not breaking links, bookmarks, or
> international *input* in the URL bar, does it break cookies?  If so, is there
> an easy fix?

The cookie database should be storing ASCII valued hostnames only, so this
should not impact it.  That doesn't mean that it doesn't, so I'll need to test
that carefully before proceeding with this patch.


> The previous patch should be backed out when this is landed (if that patch has
> landed).

The previous patch did not land.

Comment 18

[Darin Fisher]

The prefs were broken in exactly the same way that they were broken for
network.enableIDN.  See bug 261934.  I'm fixing the same bug with these preferences.

Comment 19

[haferfrost]

Why not simply display all non ISO-8859-1 (or whatever a user considers safe)
characters in RED wherever the URL is shown. That way it would be clearly
visible when some domain name contains cyrillic characters that look like latin
characters or other tricks like that.

Comment 20

[Darin Fisher]

haferfrost: this bug report is about getting a simple stopgap measure in place
ASAP.  bug 279099 is about creating a better auti-spoofing solution for IDN.

Comment 21

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Comment on attachment 174547 [details] [diff] [review]
v2 patch

So I guess my one other comment is that I'm a little uncomfortable with
changing ConvertACEtoUTF8 to do something other than what it says.  Why was
that needed?

Comment 22

[Darin Fisher]

<darin> dbaron: ConvertACEtoUTF8 needs to be hacked because we don't want to
ever perform that conversion.  doing so means that we might show UTF8 instead of
ACE.
<darin> ASCII is a subset of UTF-8 so we aren't violating the interface types
<darin> we are just violating the interface contract
<dbaron> ok
<darin> this might impact global history
<darin> but that is something i'm willing to accept
* dbaron marks r+

Comment 23

[Darin Fisher]

Comment on attachment 174455 [details] [diff] [review]
v1 patch

marking patch obsolete.  we don't want this one.

Comment 24

[Daniel Veditz [:dveditz]]

Comment on attachment 174547 [details] [diff] [review]
v2 patch

Do you want to add an observer for the new pref? or is it temporary so we don't
care? I would think that even in the future, some paranoid group will want to
use that pref.

sr=dveditz
a=dveditz for branches
seek a= from #drivers for 1.8b1, it may be too late

Comment 25

[Darin Fisher]

Attachment: v2.1 patch

revised version of the v2 patch that was checked in on the trunk for 1.8b1. 
i'll land this same patch on the 1.0.1 and 1.7 branches shortly.

Comment 26

[Darin Fisher]

fixed1.7.6, fixed-aviary1.0.1

Comment 27

[Daniel Veditz [:dveditz]]

Changing summary to reflect bug morph.

Comment 28

[sairuh (rarely reading bugmail)]

tested on 2005021714-trunk seamonkey Mac build: both the network.enableIDN and
network.IDN_show_punycode are now true by default. and making a change to either
of these prefs (via about:config) persists between sessions (as should be expected).

Comment 29

[sairuh (rarely reading bugmail)]

see bug 279099 for the root problem.

Comment 30

[sairuh (rarely reading bugmail)]

test site: http://www.shmoo.com/testing_punycode/

when network.IDN_show_punycode is true, mouse over the two pseudo paypal links
and look at the statusbar: it should display www.xn--pypal-4ve.com

but when you turn that pref to false, mousing over those links will
(deceptively) show www.paypal.com

vrfy'd fixed using 2005021714-trunk seamonkey Mac --tracy or jay, could you
quickly check this on windows and linux?

Comment 31

[Tracy Walker [:tracy]]

verified on Windows also

Comment 32

[Wil Tan]

Linking this with bug 237820.

Comment 33

[Wil Tan]

darin: why not remove the IDN_testbed support since Verisign is no longer doing
that? This is really a short-term hack that I believe does not need to stay in
the codebase.

Comment 34

[Darin Fisher]

> darin: why not remove the IDN_testbed support since Verisign is no longer doing
> that? This is really a short-term hack that I believe does not need to stay in
> the codebase.

Wil, yeah.. i had the same thought.  for the stable mozilla branches, i decided
that'd it be better not to remove those.  but, i don't object to cleaning it up
for the trunk.  i'll file a bug.

Comment 35

[Darin Fisher]

see bug 282821

Comment 36

[Jungshik Shin]

This fix is responsible for bug 283992. 

Comment 37

[Juha-Matti Laurio]

This is assigned to SA14163, marked as "Vendor Patch" in Mozilla 1.7.6 and
Thunderbird 1.0.2. It was handled as patched earlier in Firefox 1.0.1 too,
naturally:

http://secunia.com/advisories/14163/

Comment 38

[Pete Riley]

Will Thunderbird eventually show punycode in its status bar?  I'm currently using TB 1.5.0.7 and this does not work (even if I enable both network.IDN_show_punycode and network.enableIDN).

At the risk of telling you something that you already know, I think that there currently exists a security risk in Thunderbird.  When I hover my mouse over an IDN link in an HTML email, the status bar shows some of the unicode characters of the link as ASCII, and it hides other unicode characters, so the user would be fooled into clicking on the link.

For example, Thunderbird shows this link:

http://www.theshmo%d0%begroup.com/

as

http://www.theshmoogroup.com

Comment 39

[Pete Riley]

Here's another example.  This one uses the &#...; format instead of the %number %number format.

If this is in the HTML of the email:

<a href='http://www.theshmo&#1086;group.com/'>Click here to enter TSG</a>

Then Thunderbird again shows this in the status bar:

http://theshmoogroup.com/

If I click on the link in the email, Firefox is launched and it tries to take me to www.theshmo%d0%begroup.com.

Well, that's true if I save the email with UTF-8 encoding.

If I use the "current character encoding" instead of UTF-8, a question mark appears in the status bar:  http://www.shmo?group.com/.  In this case, clicking on the link does nothing.  Firefox is not launched.