218902 - STARTTLS for POP3 not supported

Status: RESOLVED FIXED

(MailNews Core :: Networking: POP, defect)

Description

[Stefan Metzmacher]

User-Agent:       Mozilla/4.0 (compatible; MSIE 6.0; Windows NT 5.1)
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.4) Gecko/20030624

mozilla's isn't able to connect to IMAP4 or POP3 server witch require the using 
of STARTTLS (STLS) on port 143(IMAP4) 110(pop3).

via the GUI it's only possible to setup a SSL connection to port 993(imaps) or 
995(pop3s).

Maybe this feature is limited by the imap client code AND/OR by the GUI code.

I have not looked at the code!

Reproducible: Always

Steps to Reproduce:
1. setup a IMAP and/or POP3 server which supports the STARTTLS/STLS command on 
port 143/110
2. and try to use it with mozilla
3.

Actual Results:  
it's not possible to configure mozilla for such servers

Expected Results:  
able to support such servers

Comment 1

[David :Bienvenu]

Cc'ing Christian for POP3 issue.

Comment 2

[Christian Eyrich]

Oh, didn't know that's possible.
Ah yes, RFC 2595 should be what you're talking about.

You're right, at least our POP3 code doesn't know the STLS command. Besides of
implementing STARTTLS for POP3 (and IMAP) itself we'll get the same problems
like the guys in bug 135357 but vice versa.

Stefan, what servers do you know being capable doing STARTTLS with IMAP4/POP3?

Comment 3

[Stefan Metzmacher]

Courier-Imap supports it

and my unfinished emil-surfer POP3/IMAP4-Server.
(If you just want to test the STARTTLS / STLS commands,
I can send it to you don't need to configure something.
and it just return OK to all pop3 and IMAP commands but didn't do anything for 
now)

Comment 4

[Christian Eyrich]

David, I implemented STLS for POP3 now. But it lacks an UI resp. the question is
how to combine this with POP3 over SSL.
Any ideas?

Comment 5

[Christian Eyrich]

Please see http://bugzilla.mozilla.org/attachment.cgi?id=131402&action=view for
a screenshot of a proposed UI (and bug 135357, comment #102 resp. the whole bug
for the why).

Comment 6

[Christian Eyrich]

Changing summary to be correct.
Should we file a new bug to get IMAP and POP issues disconnected?

Comment 7

[Christian Eyrich]

Attachment: proposed patch for POP3

This is my approach for STLS in POP3.

This is for testing purposes only - it doesn't provide a UI.
The new int pref mail.server.server*.useSSL controls the mode used:
never = 0,
try STLS = 1,
always STLS = 2,
always POP3S = 3

The old pref isSecure has been disabled. At least for now - I see that users
having set isSecure in their prefs would need set the new pref manually.

I'm not happy how m_pop3ConData->capability_flags is resetted in
SendTLSResponse(). But I've no idea how to do it instead.

Please test this and post any results/suggestions.

Comment 8

[Christian Eyrich]

Attachment: proposed patch for POP3 - all files

Ups, previous patch lacked a few files.

Comment 9

[Matthew Elvey]

Can someone name a server that supports POP w/ STARTTLS, but doesn't support POP
on 995?  If not, then the priority of this fix is fairly low, IMO.  It was
mentioned WRT Bug 135357.

Re. proposed patch: 'extention' isn't a word. 

Comment 10

[Christian Eyrich]

Yes, you're right, should be "extension" - "extention" is a typo in since
version source version 1.1.

And you're right that STLS for POP3/IMAP is only rarely used at the moment (I
didn't know of it last week too).

But it's quite easy to implement, an official standard and citing RFC 2595
"Separate 'imaps' and 'pop3s' ports were registered for use with SSL. Use of
these ports is discouraged in favor of the STARTTLS or STLS commands."

AFAIK Hamster doesn't support POP3/SSL over 995 on the local servers.

Comment 11

[Christian Eyrich]

On the URL on the top I'm providing UIs for the panels of SMTP, POP3 and IMAP.
It's my try to build a simple and single layout for the same feature on
different panels.

It would be great if we could agree on one layout (modifications are possible of
course) with the one for bug 135357.

Comment 12

[David :Bienvenu]

pop3 only - imap bug is bug 60377

Comment 13

[Christian Eyrich]

So assigning to me as I have a patch. A version with UI follows tomorrow.

Comment 14

[Christian Eyrich]

Attachment: patch v2

So here is the full patch.
The new radios are like they are in bug 135357. I created a new groupbox
"security settings" for SSL and authentication mechanism. That's not necessary
but can make things clearer.

My main problem is the coexistence of the old isSecure (which is required by
NNTP further on) and the new useSSL. GetIsSecure is used in some cases (eg.
nsMsgIncomingServer::GetPort()). And I disabled the if in am-server.js#46
because it always unhides server.isSecure again in the UI.

I'm sure I overlooked something, so any comments are welcome.

Comment 15

[Heikki Toivonen (remove -bugzilla when emailing directly)]

(In reply to comment #5)
> Please see http://bugzilla.mozilla.org/attachment.cgi?id=131402&action=view for
> a screenshot of a proposed UI (and bug 135357, comment #102 resp. the whole bug
> for the why).

That seems too complicated for me. Why not just have two options for the user -
'Require SSL'/'Use SSL when possible'? In the 'Use SSL when possible' case honor
STARTTLS, but if that fails then retry and ignore STARTTLS.

Comment 16

[Christian Eyrich]

(In reply to comment #15)
> Why not just have two options for the user - 'Require SSL'/'Use SSL when
> possible'? In the 'Use SSL when possible' case honor STARTTLS, but if that
> fails then retry and ignore STARTTLS.

At least three: 'Don't use SSL' is also necessary. And why only try STARTTLS in
the "when possible" case?
Different options for STARTTLS and POP over SSL are IMO also necessary if a
server offers both.

As I wrote, see bug 135357 and the current SMTP options for more info on how it
should look like and why.

Comment 17

[Christian Eyrich]

Attachment: patch v3

This is an updated patch to use the new pref and attribute introduced through
bug 60377. It still includes the UI for POP3 and IMAP though this will still be
matter of taste.
The drawback of the UI is the space it takes up. I guess in the end we'll at
least remove the additional groupbox but I included it for evaluation.

Comment 18

[Christian Eyrich]

Attachment: screenshot of Server Settings Panel

Panel after applying patch v3.

Comment 19

[Roland Stuehmer]

(In reply to comment #17)

> The drawback of the UI is the space it takes up. I guess in the end we'll at
> least remove the additional groupbox but I included it for evaluation.

Hi! I like the UI, and security should be worth the space. The groupbox makes
the whole thing look more structured, I would not want to miss it. :)

One more minor thing: How about making "Never" a separated _checkbox_ that greys
out all of the other radio options if checked:
 ______________________________________________
´
| [_] Use secure connection
|     (_) TLS, if available (_) TLS (_) SSL
|
| [_] Use secure authentication
|
`----------------------------------------------

This would improve the looks somwhat because the two main on/off settings would
be aligned left.

Also, on a different matter, what is secure authentication anyways? Is this
CRAM-MD5? How about mentioning that buzzword in paranthesis?

Good stuff anyhow! I'm looking forward to shutting down the IMAPS port on my
server an only using STARTTLS.

Roland.

Comment 20

[Christian Eyrich]

(In reply to comment #19)
> Hi! I like the UI, and security should be worth the space. The groupbox makes
> the whole thing look more structured, I would not want to miss it. :)

Me too, but after reading a good amount of this-or-that-wastes-space discussions
(and patches) I'm cautious.

> One more minor thing: How about making "Never" a separated _checkbox_ that
> greys out all of the other radio options if checked:
> [...]

Hey, that's great! Don't know why I didn't see that before. That would be good
change for the SMTP panel too.

> Also, on a different matter, what is secure authentication anyways? Is this
> CRAM-MD5?

It is CRAM-MD5 *too*. But also is APOP and NTLM and hopefully GSSAPI or
DIGEST-MD5 in future.

> How about mentioning that buzzword in paranthesis?

As stated above CRAM-MD5 isn't the only secauth. To this we've enough technical
terms here (e.g. SSL and TLS). There are people out there who don't touch
anything they don't fully understand and while SSL is comparatively well known,
CRAM-MD5 is not.

Comment 21

[Roland Stuehmer]

(In reply to comment #20)

> As stated above CRAM-MD5 isn't the only secauth. To this we've enough technical
> terms here (e.g. SSL and TLS). There are people out there who don't touch
> anything they don't fully understand and while SSL is comparatively well known,
> CRAM-MD5 is not.

I agree. Especially having to mention them _all_ would be impossible.

Thanks for your reply, glad you liked my suggestion. ;)

Comment 22

[Christian Eyrich]

Attachment: patch v4

This version addresses the problem, that nsIMsgIncomingServer::useSSL is used
if socketType is switched to the default value (because SetIntValue() clears
the pref if default is choosen).
Also some enhancements in the POP protocol code.

I didn't implement Roland's proposal, see bug 60377, comment 51 for the reason.

Comment 23

[David :Bienvenu]

Comment on attachment 173553 [details] [diff] [review]
patch v4

+    else
+      if (m_socketType == nsIMsgIncomingServer::tryTLS ||
+	   m_socketType == nsIMsgIncomingServer::alwaysUseTLS)
+	 connectionType = "starttls";

"else if..." can be on one line...

Comment 24

[David :Bienvenu]

Attachment: patch for seamonkey, as checked in.

Comment 25

[David :Bienvenu]

fixed - I also made the equivalent thunderbird changes. I also had to make the
account manager UI a little bit taller.

Comment 26

[Stefan Borggraefe]

(In reply to comment #25)
> I also had to make the
> account manager UI a little bit taller.

[increased from 44em to 50em]

I think this is a bad idea. This makes the Account Manager hard to use at lower
screen resolutions. :-( See bug 82122.

How about turning this idea from comment 19

| [_] Use secure connection
|     (_) TLS, if available (_) TLS (_) SSL

into a checkbox combined with a drop-down list containing three items and get
rid of the new groupbox again? We could change the SMTP-Prefs this way as well
to maintain consistency.

I only noticed this bug when the patch was already checked-in. So I'm a little
late with this. :-(

Comment 27

[David :Bienvenu]

The suggestion in Comment 19 doesn't work, because "Use TLS, If available" is
not a secure authentication mechanism, if TLS is not available.

I'm not happy about making it bigger either, obviously, and am open to other
suggestions.

Comment 28

[Christian Eyrich]

(In reply to comment #26)
> [increased from 44em to 50em]
> 
> I think this is a bad idea.

Increasing it not in general. But 50em is too much I think - 45em is sufficient
here.

> into a checkbox combined with a drop-down list containing three items and get
> rid of the new groupbox again? We could change the SMTP-Prefs this way as well
> to maintain consistency.

As I wrote earlier, the groupboxes were only a proposal but they're good for a
clear layout. Regarding drop-down or radios, also see discussion and the
proposals in bug 135357 (were the UI for SMTP was introduced).

Comment 29

[[e-mail]]

(In reply to comment #22)
>This version addresses the problem, that nsIMsgIncomingServer::useSSL is used
>if socketType is switched to the default value (because SetIntValue() clears
>the pref if default is choosen).
Does that means that admins can't usefully change the default value?

(In reply to comment #27)
>I'm not happy about making it bigger either, obviously, and am open to other
>suggestions.
Perhaps we could move some of the server settings into the Advanced dialog?

Comment 30

[Stefan Borggraefe]

(In reply to comment #27)
> The suggestion in Comment 19 doesn't work, because "Use TLS, If available" is
> not a secure authentication mechanism, if TLS is not available.

Ok, I see.

> I'm not happy about making it bigger either, obviously, and am open to other
> suggestions.

Perhaps the Server Settings pane could be split into two. A group of settings
are about the connection to the server and another group is more about how and
where the individual messages are handled. How about creating a new "Connection"
pane for "Server Name", "User Name", "Port" and the "Security Settings"
groupbox? For IMAP "Maximum connections to cache ___" could also be moved to
such a pane.


(In reply to comment #28)
> But 50em is too much I think - 45em is
> sufficient here.

I just tested with a gtk2 build with several (gtk2-)themes and 72dpi. It looks
like 47em would be just enough. I can't currently test with a Windows or Mac OS
X build.

Comment 31

[Mike Cowperthwaite]

(In reply to comment #29)
> Perhaps we could move some of the server settings into the Advanced dialog?

One easy move:
  Empty Trash on Exit   ->   Disk Space page of Account Settings

Also, the Local Directory folder edit field could be moved, either to that same 
page or to a tab on Advanced.

Comment 32

[Christian Eyrich]

(In reply to comment #29)
>Does that means that admins can't usefully change the default value?

If an admin changes the default after the user has switched to anything other
than the default and back to it, the new default won't be respected, yes.
I think that's better than being unable to use the default value at all in case
isSecure is true.

Comment 33

[[e-mail]]

Actually that's not what I meant, but in fact I think there is a bug in the
getter code so that it will never read the default preference.

Comment 34

[Magnus Melin [:mkmelin]]

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