208821 - Remove email address as anon. ftp password

Status: RESOLVED FIXED

(SeaMonkey :: Preferences, defect)

Description

[Ninoschka Baca]

Overview: When printng a message which includes an image with a link to an
Anonymous FTP site is dangerous. The user won't see a password dialog, and their
password is transferred without them knowing.  

Expected Results: 
1. In preference go to the Advanced panel and select the "Send this email
address as anonymous FTP password".
2. Select a message with an image which has a link to an anonymous FTP site
3. Print the message

Actual Results: This is dangerous because the user won't see a prompt since it's
using Anonymous FTP. 

Expected Results: Do not send the password.

Comment 1

[Ninoschka Baca]

Marking nsbeta1 since this is a security issue.

Comment 2

[Rafael Ebron (:rebron)]

adt: nsbeta1-

Mitch, please renominate if you disagree.

Comment 3

[Mitchell Stoltz (not reading bugmail)]

- is OK, but I'd like to see this fixed early in 1.5, we realy don't like it
when we send email addresses surreptitiously. marking security-sensitive

Comment 4

[georgi - hopefully not receiving bugspam]

I think that the usage of the option "send email as anon ftp pass" is quite dumb
to use and should never be used. amongst other things it allows harvesting email
addresses when browsing imho.

Comment 5

[Mitchell Stoltz (not reading bugmail)]

Should we get rid of that option? FTP server owners like to see that email
address, but I'll bet most users don't care.

Comment 6

[Daniel Veditz [:dveditz]]

Not sure why this is security sensitive, AFAICT if you've selected this option
we quietly send your email address to any ftp site that asks for it, printing or
not, and this is well known.

I think the option is there because some (very few) sites require it, or used
to. I notice IE doesn't appear to support the option, maybe it would be safe to
drop it from the UI and direct anyone who needs it to about:config. At the very
least if we keep the option we should add discouraging text near it.

Comment 7

[Mitchell Stoltz (not reading bugmail)]

I agree. Let's remove that option from the UI. Seth, who can do this? I'm sure
it's a simple change to remove that checkbox from prefs.

Comment 8

[Ben Bucksch (:BenB)]

Oh, yes, I've always disliked that UI-option and wanted it to go, and I removed
it in Beonex Communicator. Far too easy for users to shoot themselves in the
foot. When comparing the user's massiv loss of privacy with the FTP admin's wish
to see who uses their server, the admins lose big time. Apart from that, it's UI
clutter. If a server really wants that, use a real (dedicated) FTP client :).

Taking bug. Easy kill, big win.

Comment 9

[Ben Bucksch (:BenB)]

Changing subject from "Security issue when printing images with links to an
Anonymous FTP site" to "remove email address as anon. ftp password" to reflect
intended fix.

> The user won't see a password dialog

I'm a bit confused about that. Does Mozilla ever show a password dialog for anon
FTP, if it's set? Or did I misunderstand the bug?

Assuming I understand the bug correctly and Mozilla just does the same while
printing that it does during normal web browser (with links to anon. ftp), I
advocate to open the bug, I don't see a reason to keep it closed.

Comment 10

[Asa Dotzler [:asa]]

Ben, can you contribute your beonex patch here? 

Comment 11

[Ben Bucksch (:BenB)]

It's not applicable for Mozilla, but supplying a patch is trivial.

Comment 12

[Ben Bucksch (:BenB)]

Attachment: Fix, v1 - Removes FTP email pref UI

The promised patch (sorry that I didn't supply it earlier, I totally forgot it,
because I have it fixed in Beonex, so I didn't realize that the prefs is
*still* in seamonkey.)

Comment 13

[Daniel Veditz [:dveditz]]

Comment on attachment 150108 [details] [diff] [review]
Fix, v1 - Removes FTP email pref UI

sr=dveditz

Comment 14

[Ben Bucksch (:BenB)]

Thanks for the fast (and unrequested) sr, dveditz!

Checked into trunk (if I didn't screw up the commit, bonsai behaves strange).
Marking FIXED.

Comment 15

[Ben Bucksch (:BenB)]

.

Comment 16

[timeless]

i'm very very tempted to reopen this bug.

until this time if you had set the pref and we were sending this setting, you
could see it by opening preferences.

now we just silently send the value which you could have set a long time ago and
long since forgotten.

if you're going to continue to send a value based on a preference, *please*
change the preference name(s).

as a normal user, when i'm looking through preferences, if i don't see something
about sending personal information, i'm going to presume we aren't doing it.

nb: normal users don't open preferences, but those who do (and to set the pref,
you had to) think the way the normal user i'm describing here does.

there's no way i'm going to look through both my preferences (in about:config)
and the entire mozilla source tree to find out that we are *even more* silently
sending personal information.

Comment 17

[Ben Bucksch (:BenB)]

> until this time if you had set the pref and we were sending this setting, you
> could see it by opening preferences.
> if you're going to continue to send a value based on a preference, *please*
> change the preference name(s).

Yes, I was already thinking of invalidating the existing pref. Just changing the
pref name is a nice idea (simple to implement).

Any thoughts by others?

Comment 18

[Justin Wood (:Callek)]

I am thinking of that, was already about to jump on it, though I wasnt sure what
to set the new prefs

The two prefs in question are:  advanced.mailftp and network.ftp.anonymous_password

I was thinking of changing them to something like:  advanced.use_mail_in_ftp and
network.ftp.anonpassword respectively

though am not to sure, I sent an e-mail to darin (@meer.net) asking for his
input on that.  (open to suggestions)

Comment 19

[[e-mail]]

We only need to rename one pref... how about renaming advanced.mailftp to
network.ftp.anonymous_password.enabled?

Comment 20

[Justin Wood (:Callek)]

with current tree your right, but renaming both for easability is a help imo,
but I'll just do that one pref then....

Patch will be a few moments

Comment 21

[Justin Wood (:Callek)]

Attachment: change pref name

Here we go, not sure who to r/sr? though

Comment 22

[Ben Bucksch (:BenB)]

I used network.ftp.anonymous_password_set, because we send a (dummy) password
anyways.

Comment 23

[Ben Bucksch (:BenB)]

Attachment: Cause reset by changing pref name, v2

Comment 24

[[e-mail]]

Comment on attachment 150189 [details] [diff] [review]
Cause reset by changing pref name, v2

>+// custom pw for picky anonymous ftp servers.
>+// severe privacy problem (any website can get it).
>+// will send dummy pw, if _set is false.
>+pref("network.ftp.anonymous_password_set", false);
Given your previous comments, the best I could come up with was the somewhat
unwieldy network.ftp.anonymous_password_override.enabled :-)

>+pref("network.ftp.anonymous_password", "");
We could default this to [e-mail] ;-)

Comment 25

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

Eh, hold off a moment, why do we need two prefs? Doesn't one suffice - when it
is set (to a non-empty value) we use it, otherwise we use the default?

(In reply to comment #24)
> We could default this to [e-mail] ;-)
 
Hmm, example.com seems to be a reserved name, so this might do. But why not stay
with what we have now (probably "mozuser@" or something like that)?

Comment 26

[Ben Bucksch (:BenB)]

> Eh, hold off a moment, why do we need two prefs? Doesn't one suffice - when it
> is set (to a non-empty value) we use it, otherwise we use the default?

How do you flush the existing value, then? This is the whole point of the patch.
(However, even if my patch goes in and you later implemented what you propose,
and the user never resets his prefs, the problem would resurface.)

>> We could default this to [e-mail] 
> Hmm, example.com seems to be a reserved name, so this might do. But why not
> stay with what we have now (probably "mozuser@" or something like that)?

The backend currently hardcodes exactly that <[e-mail]>. Neil, I'd
prefer to prevent redundancy and we need it in the backend as fallback anyways.

Can I remove the security-sensitive flag? it's silly IMHO.

Comment 27

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

(In reply to comment #26)
> > Eh, hold off a moment, why do we need two prefs? Doesn't one suffice - when it
> > is set (to a non-empty value) we use it, otherwise we use the default?
> 
> How do you flush the existing value, then?

use a single pref, but with a new name

Comment 28

[Daniel Veditz [:dveditz]]

Removing security flag (no point on a bug to remove a feature which has had a
public UI)

Comment 29

[Daniel Veditz [:dveditz]]

This bug is in the FIXED state but it appears there is work to be done (to clear
a pre-existing password).

Re-opening since the additional patch is here. You could also close and open a
new bug on the new work.

Comment 30

[Mike Hommey [:glandium]]

i don't know if i should file a new bug or what, since it's anonymous ftp
password related, but the password sent should be taken from the
ftp://anonymous:[e-mail]/ when it is given.

Comment 31

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

Mike: that's a different bug. please file a new one.

Comment 32

[Darin Fisher]

Comment on attachment 150189 [details] [diff] [review]
Cause reset by changing pref name, v2

this is fine by me.  r=darin

Comment 33

[benc]

-> networking, this feature never had anything to do w/ mailnews.
qa to me.

Timeless, please move all of #16 and later to a new bug. It really is a
different, followup issue.

I'd like to get this back into the fixed state, so I can mark it verified.

This is fixed on trunk-only, right?

Comment 34

[benc]

-> MAS.
RESOLVED/FIXED.
This bug is fixed for the UI, which only affected MAS/Seamonkey.

Comment 35

[benc]

lxr for the .dtd file says:

1.28	mozilla.BenB%bucksch.org	2006-05-16 19:22	 	bug 208821: Remove pref UI to send email address as FTP password.
r=Neil Rashbrook, sr=dveditz

So, I'd say this was fixed around Seamonkey 1.0.2.

http://www.seamonkey-project.org/releases/1.0.2

setting to the closest milestone I could find.

So, this bug is only about removing the pref from the UI.

The issue about the pref backend needs to be dealt with. I'll research and file a bug against FTP-core.

Comment 36

[benc]

So, here is where we are:

The UI in Seamonkey Pref has been removed.

lxr says:

advanced.mailftp
/modules/libpref/src/init/all.js, line 503 -- pref("advanced.mailftp", false);
/suite/profile/migration/src/nsSeamonkeyProfileMigrator.cpp, line 270 -- MAKESAMETYPEPREFTRANSFORM("advanced.mailftp", Bool),
/netwerk/protocol/ftp/src/nsFtpConnectionThread.cpp, line 737 -- rv = prefs->GetBoolPref("advanced.mailftp", &useRealEmail);

So, no new users are going to see (or use this), unless they flip the switch in about:config. 

So, questions:

Should this (UI) be re-implemented somewhere, in any product? To me, probably not. So, no more UI change requests (no more "remove it" bugs, and no new "add it to product X" bugs.

What happens in existing profiles? Whatever was set before keeps running. If you had set this, the string in "network.ftp.anonymous_password" is your password.

Should this feature be completely removed? Maybe. I need to do some testing w/ anonymous FTP handling.

Comment 37

[benc]

Okay. So I spent some time using FTP w/ a packet tracer to confirm the details.

In short, a user can use the URL format to specify anonymous FTP passwords, like:

ftp://anonymous:[e-mail]/

So, I've filed bug 446196.