At 10:41 AM +1200 9/3/02, Simon Byrnand wrote:
> Problem: When that 3rd party attempts a POP3 connection to our server, it
> first tries issuing the 'STLS' command. Despite the fact that STLS support
> is not even compiled into QPopper, it recognises the command and responds
> with "ERR - command not enabled", (which is fine) and then closes the
> connection. (Which is not fine)
>
> Closing the connection when a command which support of is not compiled in
> seems wrong to me.
Correct; this Qpopper must not do.
> This makes it impossible for the client to probe for
> STLS and then fall back to a normal connection.
False; the client should issue the CAPA command and see if the server
supports STLS.
> In order to prove this I
> commented the stls line out of the state table in pop_get_command.c thus:
>
> { auth1, "epop", 0, 0, pop_epop, {auth1, auth1} },
> // { auth1, "stls", 0, 0, pop_stls, {halt, auth1} },
> { auth1, "user", 1, 1, pop_user, {auth1, auth2} },
> { auth1, "capa", 0, 0, pop_capa, {auth1, auth1} },
>
> and found that the 3rd party that is probing the STLS command gets an ERR
> message, and then continues to use normal POP3 methods and succeeds.
> Without my change, the connection always gets dumped. (Obviously my change
> is a hack, but it was just a way of testing my theory)
>
> The question is, is QPopper in the wrong ?
Yes, this is a bug and should be fixed.
> Or is the client trying to probe
> STLS in the wrong ?
Yes, the client is wrong and should issue CAPA and see if STLS is supported.
> Or is this just one of those ill defined grey areas of
> interoperability that crops up from time to time.
It's a case of both sides being wrong. If either of the sides did
the correct thing, there would be no visible problem, but with both
sides being incorrect, there is.
>
> It seems to me that if STLS support is not compiled in at all, then the
> STLS command shouldn't be recognised at all, and just return an ERR command
> not recognised, which could probably be done by putting an ifdef around the
> line I commented out, similar to the ones for RPOP and APOP.
True, but it's nicer from an operational point of view to know that
your TLS session is failing because you forgot to compile in support.
>
> But if STLS support is compiled in, but it is DISABLED, then it should
> report that the command is disabled, as it does now, but IMO it should
> *NOT* then drop the connection. I'm guessing, but it looks like that could
> be done by changing the halt to auth1 in the line I commented out, but
> without understanding the code a lot better I wouldn't like to do that.
Yes, changing the 'halt' to 'auth1' would avoid the problem. It
seems to be trying to turn a TLS timeout into a fatal error, even
though there is also an attempt to make a failed TLS negotiation not
be fatal, as the comment in popper/pop_extend.c says. The command
table doesn't have a way to say "go into halt state if the command
failed for this one reason, but stay in the same state if it failed
for this other reason."
I can fix it within popper/pop_extend.c I think, by separating the
pop_stls return value from the message sent to the client.
