mozilla-thunderbird: SMTP down negotiation weakness

That's right and it is by design. It's not the best solution but I did it (in
bug 203785) because servers where CRAM-MD5 authentication fails are widespread
and users would have to often switch the options.

For SMTP there is no switch "Use secure authentication" in the UI and in the
backend it's honestly named "trySecAuth". So TB/SM don't pretend to only use
secure mechanisms for SMTP in any situation.

Sorry, if I was not clear enough: The fallback behaviour is not a bug and should
be left as is. My request for enhancement was to add an option to override this
(security critical) fallback behavior.

mentioned on Bugtraq : <http://www.securityfocus.com/archive/1/414556/30/0/threaded>

It seems to me that the solution could be as simple as adding an option "m_prefTryInsecAuth" which does a similar thing like "m_prefTrySecAuth" for the secure mechanisms: disable PLAIN and LOGIN if it is set to false.

Anyone test this?

To me this seems to be Much Ado About Nothing (with apologies to WS). In my tests, with either "TLS" and "SSL" for "Use secure connection", Thunderbird establishes a secure connection *before* attempting authentication, thus obviating all of the four scenarios. There's only a risk if the user chooses "No" or "TLS, if available" for "Use secure connection", which are valid user choices (whether we approve or disapprove).

Reply to comment #5:
The bug with CRAM-MD5 mostly concerns connections to servers that do not speak SSL/TLS. Of course you are right, if the connection itself is encrypted, it does not matter much if the authentication itself is CRAM-MD5, PLAIN or LOGIN.

Confirming as RFE per reporter's comment 2.

I've a patch that changes behaviour to that of POP and IMAP, that is either secure or insecure (using a new pref useSecAuth [but also introducing an UI] replacing the current trySecAuth).

That would give us a consistent behaviour for all protocols. Would that be ok for you?

Created attachment 250451
proposed patch

Patch to replace trySecAuth by useSecAuth like for POP3 and IMAP including UI.

Comment on attachment 250451
proposed patch

No, sorry that's not good.

Created attachment 250459
patch v2

Another try.

this patch looks good, thx, Christian, but it's going to take me a while before I can try this out on the trunk (it's not intended for the 2.0 branch, which is my highest priority right now)

David, TB 2.0 is out, maybe you want it try out now?

thx for the reminder, Christian. I had forgotten. I think first I need to deal with the other patch about adding the recipient to the smtp error messages, and land that, before I apply your patch, since they both change the same files.

Created attachment 269387
non-bit-rotted patch

Christian, I've applied your patch, un-bit-rotted it, and tweaked the error strings a bit. One question - Is my understanding of this patch correct? It makes SMTP compatible with IMAP/POP3 - we'll only try secure auth if the user has chosen it explicitly, and the pref defaults to false. But before this patch, we would try secure auth always (unless the user explicitly set a hidden pref not to) and then fall back to insecure auth. So for some existing users, on upgrade, we will silently switch to insecure auth? That seems bad.

If all that's correct, what do you think about something like this: try using secure auth once, if available. If that works, automatically set the pref to use secure auth; if it doesn't work, don't put up an error message on the failure, and set a pref not to try this again for this server. Possibly, you could use the existing try secure auth pref for this purpose.

We really want to go to a place where, on new server/account setup, we automatically try to figure out the most secure settings (TLS/SSL, etc) and secure auth automatically, and allow the user to change them later. This would b e a step in that direction.

You're right in your understanding of the code. And yes, it will mean that some users will now authenticate insecure instead of secure. But I don't think that matters because of the reason this bug exists: it's insecure anyway.
So I don't see a security problem. And honestly I don't like having code lying around to be used only once if it's not really necessary.

BTW, I've done a up-to-date patch yesterday and can provide that for the further work.

> But I don't think that matters because of the reason this bug exists: it's
> insecure anyway.
Not sure I understand that. For a user who's had secure auth working just fine, because they've got a correctly functioning server, using insecure auth is *less* secure. Yes, secure auth is not completely secure, but it's more secure.

Some of the code would not be one time only - if we make new server setup be smart enough to try secure auth and set the secure auth pref if it succeeds, and silently fail if it fails, and clear the secure auth pref, then that would share code with this.

With your up to date patch, could you make sure you change the strings that say "you choose" to "you have chosen" like I did in my patch?

Created attachment 269413
patch v3 with autoprobe

What I meant is that it's possible to suppress/filter the usage of secure ones (on a malicous server, a man in the middle or some program on your client). This won't be detected and the password revealed by using insecure ones.
Hm yes, you're right in that it is secure enough if one has only the ability to sniff but not actively communicate.

So here's my proposal how I understand it.
Additionally we could switch trySecAuth off for one server when the use Secure Authentication is switched in the UI.

Regarding probing the servers abilities you mentioned in comment 15. Are there written plans on this?
In general, having the option would be nice, but only if the user either is noticed to review the settings taken or if it's triggered by himself.

I adjusted the strings.

very cool, thx. Yes, I meant the simple sniffing case.

Re probing the server's abilities, I don't know if there's anything explicitly written about it, but easier account setup is a big priority - http://wiki.mozilla.org/Thunderbird:Easy_Account_Setup

If you look at mail.app, it tries to figure out if it should connect on the SSL port, or use TLS, or not, when you go through new account setup. We want to do something like that at least. And the automatic detection of secure auth capabilities and success using secure auth that you've added goes hand in hand with that.

Comment on attachment 269413
patch v3 with autoprobe

Let's see if this is really good.

Comment on attachment 269413
patch v3 with autoprobe

Looks good, thx, Christian. I don't have an smtp server that supports secure auth (and my ISP prevents me from using an other smtp server), but my insecure auth server kept working fine

fixed on trunk, thx, Christian.

It would be interesting to do similar auto-probing for imap and pop3, especially in conjunction with auto-discovery of SSL vs. normal port, and TLS, during account creation. I think I'd want to do it at account creation time, and probably set a flag on the url that says we're doing discovery, so we'd skip putting up error messages, and we'd remember if we connected via ssl or normal port, whether the server advertised secure auth, and/or TLS.

(In reply to comment #21)
> (From update of attachment 269413 [details])
> I don't have an smtp server that supports secure auth
ugh.

> (and my ISP prevents me from using an other smtp server)
Whoah. And it probably doesn't support IMAP idle, or IMAP subfolders, or searching. I bet they only introduced IMAP recently. How much do they pay you to stay with them? You should probably switch to a webmailer.
No, just kidding, couldn't resist.

I don't use my ISP for mail, just a connection to the internet - but they block the smtp port for any server but their own, for the obvious reason...

I use an insecure pop3 server for email, and it's no longer working for me.
After d/l tinderbox zip build Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a7pre) Gecko/2007070512 Thunderbird/3.0a1pre ID:2007070512
I get ..Sending message failed..Unable to authenticate to server ..blah blah
The time stamp on the Thunderbird exe is:
Today, July 05, 2007, 12:59:46 PM
Not really sure if this checkin made it in there.

Since you mention "sending message failed" I guess you mean SMTP server, not POP3 server, yes?

Please have a look if "use secure Authentication" is set for your smtp server or a pref useSecAuth is set for any account (or default) to true in your configuration (Config Editor or prefs.js). If yes, switch it off and try again.
I can think of a potential problem with servers that advertise secure auth but fail with it when actually using it. Before this patch we then simply fall back to unsecure but now stop.

But maybe it's something else.

In any case I'd be interested in a smtp log following the instructions in http://www.mozilla.org/quality/mailnews/mail-troubleshoot.html#smtp and add an attachment for this bug.

Created attachment 271344
smtp log

failure to authenticate

Thanks for the log. Doesn't look your server supports any authentication mechanism, therefore our behaviour is the right one.

But before the latest patch it was wrong since we did send the message without authentication though user told us to use authentication (checkbox "Use name and password").
That means checking is now more strictly and you've to disable above option to continue.

You are absolutely correct. I changed isp's about 4 years ago.
I'm sure the new "correct" behavior will catch others.
Thanks for the attention to my "problem"

This patch appears to cause problems for me.

My server supports the SMTP submission port (587) for MUA-sourced mail. Authentication is required to send messages, and the AUTH methods supported is only displayed *after* STARTTLS succeeds in establishing a secure connection. My SMTP server advertises PLAIN/LOGIN after STARTTLS succeeds.

What appears to be happening is that TB is seeing no AUTH methods initially (before STARTTLS) and telling me that "The server does not support any compatible insecure authentication mechanism but you haven chosen insecure authentication."

Does this patch check available methods *before* or *after* a (user-enabled) STARTTLS secure connection is established?

Thanks Joseph for that clear report and your suspicion is also correct.
The check is performed upon receiving the first EHLO response. But it should be done based on the EHLO right before login which might be the second one.
I'll see what it takes to solve this.

Looking for a solution I found an inconsistency to the new authentication behaviour. A server implementing only HELO would make Mozilla not forcing authentication even if the user checked it.
BTW, the reason for wanting to do so can be seen as "getting what you set", but that's not the main reason.
It would not be a problem for most cases if the user choses to authenticate but the server doesn't demand it. That's because the reason authenticating is that the server can be sure on our identity, nothing else. But some mechanisms also provide ability for the client to check the identity of the server. In these cases bypassing authentication would be a potential security risk.

Talking about security, I see how to repair the new problem of no AUTH before STARTTLS. But it would be easier and in my opinion cleaner to solve if STARTTLS would have only the alternatives fail or succeed.
That means if we would drop the "STARTTLS, if available" option. In my opinion this is more a pretender than a provider of security anyway.

Any other opinions?

I don't remember exactly why we have "STARTTLS if available". It started in Netscape 4.5, iirc. One possibility is that the user may not know if they have STARTTLS, and "if available" is a reasonable default. But I don't think we made it the default, perhaps because back then some servers didn't implement it correctly.

So I have no problem getting rid of it, except that we'd have to upgrade users who have it selected to the appropriate setting, based on whether the server supports it.

You're right, that's something I always forget about.
If I'd remove that option in the UI, none of the remaining options would be selected until sending the first message. At least if not a server probing function will run right after application start.
No, I think we've stick with it. Or do I again ignore something?

I'm curious if Nelson has any insight into the "STARTTLS if available" option, in particular if it has any value over auto-probing. Do servers ever go back and forth between supporting STARTTLS and not? My guess is that users don't know, and that's a safe option to pick.

If you remove the option from the UI, most people would never notice, until after they'd tried to send mail. Perhaps we could only show that option if it was the currently selected option, but once the user sends mail, we set the choice to STARTTLS if it was available, and not, if it wasn't.

I notice Eudora has "STARTTLS if available" as an option as well.

Re your other questions, for offline, for Windows and Linux, we'll know if we're offline, because of auto-detect, and we just have to handle it gracefully, probably by letting the user figure it out, which partly answers your other question, yes, I think we should let the user change the settings. Having the UI be simpler would be nice, but I think the user does have to have control over these settings - whether that happens in the account wizard, as opposed to the account settings dialog, I'm not sure. In general, we can get away with the account wizard having less choices, but we always get complaints.

Created attachment 271575
patch to address STARTTLS flaw and migrate "STARTTLS if available"

This patch addresses the problem reported in comment 30 by moving the test into ProcessAuth(). That's not that great but shouldn't (and doesn't as far as I tested) cause problems.
It also implements a smooth migration away from "STARTTLS if available". The UI problme I wrote about in comment 34 is mitigated by only displaying the option if selected.
Does this look like a way to go?

wow, that was fast! But what happens when the user adds an smtp server? We're not auto-probing for STARTTLS yet, are we?

Not fast, I was working on that for longer. We just had the same idea. But you're right again on the new server thing - I hope it's only because it's late in the night ...
Let's hear if Nelson finds a new aspect before I'll post a new patch.

AFAIK, "StartTLS if available" is there for the user who simply doesn't
know if his server supports it or not, but wants to use it if so.
It simply is more convenient that forcing the user to set startTLS,
try it, and then set something else if that doesn't work.

That pref setting isn't very secure, because it's not difficult for an
attacker to fool the client into believing that the server does not
support StartTLS, even when it does. So, a better name might be
"StartTLS unless I'm being MITMed" :)

I think a reasonable way to implement StartTLS-if-available is to have
the network code change it after the first successful authentication.
If StartTLS worked, then change the pref from "StartTLS-if-available"
to "StartTLS always". If StartTLS didn't work, then change the pref to
"No TLS" (or whatever it is called). It then becomes a one-shot setting,
and means "probe to see if StartTLS works, and permanently select it if so."
I'd suggest you do that, and leave it in the UI. Let users set that pref,
if they don't know, but change the pref once the answer is known.

If you take that choice away (from the UI), users will complain. I hate it
when a product that I've been using for years is changed in a way that
removes functionality that (I think) I depend on, and I'll bet other users
feel that way too. But it doesn't have to remain the insecure implementation
that it has now. That's my $0.02

Thanks for your opinion. You're right in that removing UI elements is a ticklish issue. We can't only add stuff though but must also be able to remove something if it seems wrong or no good. Opinions will vary if that option is such one.
I expect that using that radio button as probe setting will make people cry about "Thunderbird can't hold my setting".

So ok, because we have varying opinions I'll leave it as it currently is. Since I still think it's time for changing it and you mentioned probing, wouldn't be bug 387421 or bug 185631 respectively be the right place do discuss this?

I agree in general that removing UI settings can be frustrating, but I think in this case we'd be figuring it out automatically for the user, which is better...I'd prefer to automatically probe instead of having a temporary probe setting. But it sounds like since the backend probably has to support the "STARTTLS if available" setting anyway, not removing the UI is an option.

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

Roman, Wayne your problems can be caused by the same combination of events like J. Lance's though I'm a bit surprised that much servers work that way. I'd like to be sure.
So would you please have a look if what's happening is the same as described in comment 30? Creating a log (see comment 26) for you can help there. Does switching off Secure Connection help?

Created attachment 271709
smtp log

I can't send mail with todays and yesterdays trunk build. It doesn't support TLS, and I didn't change the setting I had (no TLS).

FWIW, I think dropping the "TLS, if available"-UI wouldn't hurt. I base that on the guess that few users are using it since I doubt users put in much other info than what they get from their ISP, who I feel is not likely to give that option but say what they actually support.

Magnus, your case is the same as Joe's, i.e. your server doesn't offer any mechanism to authenticate. So just disable authentication (uncheck "Use Name and Password").

What do you think should TB say in the alert to give users an idea how to solve the problem themselves?

And thanks for your opinion on "TLS, if available".

Perhaps we can change that error message into an error message and an offer to switch the user to non-authenticated logon, i.e., uncheck that setting for them, if the user says "Yes" instead of No. This might be relatively common when users upgrade to 3.0...

With gmail I can't send emails with STARTTLS, without STARTTLS, with secure authentication, whtout secure authentication, without authentication...

it started on the July 5th build.

Asrail, gmail behaves like J. Lance's (and Wayne's as I now know) server. It demands on STARTTLS but doesn't give AUTH without.

Created attachment 271816
patch to address STARTTLS flaw

I just thought removing that STARTTLS-UI-option could be done casually. But priority should be to get auth with STARTTLS working for the users again.

This patch does this for the cases were the server doesn't give any (compatible) mechanisms before STARTTLS is active.
It also adds new text if the server doesn't advertise AUTH at all and gives hints in the other two cases. I think they're descriptive and bring users on the right way.

Comment on attachment 271816
patch to address STARTTLS flaw

thx, Christian. I'll give this a try - I think the error messages are a bit technical, however. I'd probably remove the text about EHLO, since that doesn't help the user. Also, I'd want to change the text "Switch off authentication for that server" to say something explicit like "uncheck 'use username and password' in the server settings. But if you're OK with those changes, I can make them myself.

My reasons for naming these technical details is to not only tell the user what's up but also make it easier for supporters (in the community as well as service providers) to get an idea of what goes wrong. In this case it might a little to much though.
What about "An error occurred sending mail: Unable to establish a secure link with SMTP server %S using STARTTLS since it doesn't advertise that feature. Switch off STARTTLS for that server or contact your service provider."? In this case we can merge the two different texts.

I'm ok with the "uncheck" text, but be aware that it's not "username" but "name" in the UI.

And oh, there's a wrong second definition for 12581 in composeMsgs.properties for the suite.

Yes, "doesn't advertise that feature" sounds good, and you're right, it's "Use name and password". Do you want to whip up a quick patch, because the changes are more than just changing the strings?

The "use name and password" sounds like a bug to me...

Might be, actually I'd prefer "Authenticate using username and password".

Created attachment 271867
[checked in]another try

I additionally noticed that NS_ERROR_STARTTLS_FAILED_NO_EHLO wasn't even used in the smtp code. This patch addresses these flaws.
Sorry for that problems, I think I've written better patches before.

Comment on attachment 271867
[checked in]another try

no worries, we appreciate you working on this!

I tried the patch, and ran through the various misconfigurations that I could, and it seems to work fine. Assuming this patch is ok with you, I'm going to r= and request sr from Scott.

Comment on attachment 271867
[checked in]another try

how do the alert dialogs look with some of these long error strings we are adding? Does everything get wrapped and look ok?

Thanks for sr. Yes, they look ok here, I get about 3 lines of text. Less would be better to read but also less informational.

marking fixed, thx, Christian.

>+12581=An error occurred sending mail: Unable to authenticate to SMTP server %S. It does not support authentication (SMPT-AUTH) but you have chosen to use authentication. Uncheck 'Use name and password' for that server or contact your service provider.
That should be SMTP-AUTH, not SMPT-AUTH, right?

yes, I'm always doing that - I'll clean them up.

Bug 388300?

Created attachment 278592
SMTP log for failing STARTTLS

So on trunk I get the "no authentication mechanism" error when using STARTTLS. Log is attached. This works in a 2007-01-01 build.

When I try to use SMPT-over-SSL instead of STARTTLS (on port 587, as in the STARTTLS case), the connection just times out and the entire SMTP log is:

  -1208243424[896a548]: SMTP Connecting to: outgoing.mit.edu

The server does support secure auth; see http://web.mit.edu/ist/topics/email/smtpauth/

Do you want a separate bug on this issue? It's pretty much blocking me sending any e-mail right now, so I'd dearly like to get it fixed.

Interesting data point. If I leave the connection as "STARTTLS" and uncheck "use secure authentication" then things work. So is the problem just that the server doesn't support CRAM-MD5? In that case, why is this happening at all? PLAIN or LOGIN auth over STARTTLS is "secure", right? So checking this box in the UI should not cause failure here.

Put another way, as a user I feel unsafe because I have to uncheck "use secure authentication" to connect to this server. Is this feeling justified? If not, then we have a bug.

Christian can chime in, but PLAIN and LOGIN aren't considered secure authentication mechanisms (I believe secure refers to the authentication mechanism, not the underlying channel).

It's not just CRAM-MD5 - there's also NTLM, MSN, and GSSAPI, and probably a couple more that we don't support.

But the user doesn't care whether the "authentication mechanism" is secure. He cares about whether the authentication process is secure. Is PLAIN/LOGIN over TLS secure in the sense that the password is protected? If so, we should treat that situation as secure. And I personally would like to know the answer to this question, still. ;)

If we want to insist that this checkbox just refers to the authentication mechanism, I think the UI needs significant rewording to make it clear exactly what that toggle toggles. Right now it sounds like if you uncheck this checkbox your password is sent in the clear on the wire.

I think the problem is with the phrase "use secure authentication".
The pref that bears that label enables a class of algorithms that use
keyed hashes as a challenge-response protocol. It enables specific
algorithms, but the dumbed-down description is overly broad, so that
users tend to imagine that it includes SSL. It specifies that they are
to be used to the exclusion of "plain text" passwords.

Boris, you seem to be arguing that the pref should be changed to match
the label, rather than the label changed to match the pref.
I think your proposal is equivalent to saying that the pref should mean
"disable the use of plain text password over unencrypted channels".

You can even argue if CRAM-MD5 is *really* secure. Besides of that, the label for the current behaviour really should be "use secure authentication mechanisms".
Besides of that I like what Nelson writes on disabling the use of plain text passwords. That would additionally need some code changes (which shouldn't be hard) and should get filed as a new bug.

On your initial problem, Boris, I currently don't see why it doesn't work for you with "use secure authentication". The server advertises GSSAPI which falls into the secure category. What's the specific error description you get?

> Boris, you seem to be arguing that the pref should be changed to match
> the label, rather than the label changed to match the pref.

Nelson, I'm arguing that what users really care about when doing password auth is that the password is secure from packet-sniffing attacks. So we need to communicate to the user whether this is the case and make it easy to create a setup where this is the case.

One way of doing that is by changing the meaning of the pref as you describe. Another way of doing that is by changing the label to correctly describe what the pref really does and changing other UI to communicate the information the user cares about. I don't really have a preference either way.

Note that I _still_ don't know whether using "STARTTLS" with the "secure authentication" checkbox unchecked is secure in the "disable the use of plain text password over unencrypted channels" sense. No one's actually answered that question.

> Besides of that I like what Nelson writes on disabling the use of plain text
> passwords.

You mean effectively ignore that checkbox for connections that are sending the password over an encrypted channel?

> What's the specific error description you get?

   An error occurred sending mail: Unable to authenticate to SMTP server
   outgoing.mit.edu. The server does not support any compatible secure
   authentication mechanism but you have chosen secure authentication. Try
   switching off secure authentication or contact your service provider.

BTW, my comment 67 wasn't meant to imply any disapproval. Just clarification.
I think we have to decide which to fix: the pref's definition or the label
or both. It's clear (to me :) that changing the pref's label and its
definition to be something like "Don't send plain text passwords over
unencrypted channels" would be most easy for users to understand, and
would probably most closely match what they desire to accomplish.

> BTW, my comment 67 wasn't meant to imply any disapproval.

I didn't think it had. :)

(In reply to comment #69)
> Note that I _still_ don't know whether using "STARTTLS" with the "secure
> authentication" checkbox unchecked is secure in the "disable the use of plain
> text password over unencrypted channels" sense. No one's actually answered
> that question.

Whatever you use with TLS(STARTTLS) is secure since the TLS session is started before authentication is done. In theory it might be possible that the client performs authentication before it does initiate the TLS session but that would be really silly. Some servers enforce STARTTLS by not accepting something else before.

@ Nelson
Yes, that label would be easy to understand and what most people want. On the other hand it would make TB's/SM's behaviour even more unconfigurable. I really think we'll need bug 202148 fixed.

@Boris
There's a bug in our code. I filed bug 394043 with a description.
But why does GSSAPI fail for you in this early stage? I think because do_CreateInstance(NS_AUTH_MODULE_CONTRACTID_PREFIX "sasl-gssapi", &rv) in nsMsgProtocol.cpp#820 fails.

I don't know about the GSSAPI implementation, but might it be that psm isn't compiled in your build?

Christian, I'm using a mozilla.org nightly build. PSM is compiled in it, trust me. ;)

In a local debug build that shows the problem, I do get back an m_authModule on that line.

The negotiateauth:5 log says:

-1208394976[9b8f548]: entering nsAuthGSSAPI::GetNextToken()
-1208394976[9b8f548]: gss_init_sec_context() failed: Miscellaneous failure
No credentials cache found

Then GetNextToken returns NS_ERROR_FAILURE, and the rest is history.

This is not entirely surprising if this code just fails out if I don't have Kerberos credentials: I in fact do not have any.

I filed bug 394053 on the password auth over TLS issue.

Uh, yes, PSM isn't related here, sorry.
So what's really needed is to catch the cases where our internals fail, independently off server messages and especially if GSSAPI as only mechanism fails. That will both go into bug 394043.

(In reply to comment #73)
> Yes, that label would be easy to understand and what most people want. On the
> other hand it would make TB's/SM's behaviour even more unconfigurable. I really
> think we'll need bug 202148 fixed.

Given that that bug rfe isn't even fixed, I think the label text from comment 70 would be fine. Even if bug 202148 would get fixed I really doubt that would get any UI, so the few advanced users changing it would just have to know that if they change the pref, the label might become a bit misleading. (Or then handle switching label in that bug.)