STARTTLS patch for imap and ftp

STARTTLS patch for imap and ftp

Hello!

3 years ago, I wrote a patch[1] (and did the TSU[2]) for adding these
features to s_client. Can this please be applied to CVS? I've seen
other people on the mailing list asking for it[3], including fixes for
HELO[4].

This is a pretty trivial patch, and would help a lot of people. I have
updated it (see attached) for current CVS. Is there anything else I
need to help with to see it get committed?

Re: STARTTLS patch for imap and ftp

Richard Levitte - VMS Whacker wrote:
> In message <[hidden email]> on Thu, 15 Feb 2007 10:34:23 -0800,
> Kees Cook <[hidden email]> said:
>
> kees> 3 years ago, I wrote a patch[1] (and did the TSU[2]) for adding
> kees> these features to s_client. Can this please be applied to CVS?
>
> Yes. Done. Thank you, and sorry you had to wait 3 years for this to
> happen.

The problem (not only I have) with the patch is
that at least in SMTP and IMAP it is illegal
to start TLS before an initial protocol handshake is done:

* in SMTP doing a STARTTLS without previous EHLO
will return a
503 STARTTLS command used when not advertised
* in IMAP doing a STARTLS requires a
. CAPABILITY
first.

In both cases the server response should be parsed for
the string "STARTTLS"...

Re: STARTTLS patch for imap and ftp

Goetz Babin-Ebell wrote:

> Hello Richard,
>
> Richard Levitte - VMS Whacker wrote:
> > In message <[hidden email]> on Thu, 15 Feb 2007
> 10:34:23 -0800,
> > Kees Cook <[hidden email]> said:
>
> > kees> 3 years ago, I wrote a patch[1] (and did the TSU[2]) for adding
> > kees> these features to s_client. Can this please be applied to CVS?
>
> > Yes. Done. Thank you, and sorry you had to wait 3 years for this to
> > happen.
>
> The problem (not only I have) with the patch is
> that at least in SMTP and IMAP it is illegal
> to start TLS before an initial protocol handshake is done:
>
> * in SMTP doing a STARTTLS without previous EHLO
> will return a
> 503 STARTTLS command used when not advertised
> * in IMAP doing a STARTLS requires a
> . CAPABILITY
> first.
>
> In both cases the server response should be parsed for
> the string "STARTTLS"...
>

This statement is technically correct. As the s_client tool is however
intended for testing purposes only (you remember that a capital
"R" at the beginning of the line will start a renegotiation instead
of being transferred to the server :-) adding the EHLO and .CAPABILITY
should be sufficient and the more complex parsing of the response
might be omitted...

Re: STARTTLS patch for imap and ftp

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Lutz Jaenicke wrote:
> Goetz Babin-Ebell wrote:
[...]

>> * in SMTP doing a STARTTLS without previous EHLO
>> will return a
>> 503 STARTTLS command used when not advertised
>> * in IMAP doing a STARTLS requires a
>> . CAPABILITY
>> first.
>>
>> In both cases the server response should be parsed for
>> the string "STARTTLS"...
>>
> This statement is technically correct. As the s_client tool is however
> intended for testing purposes only (you remember that a capital
> "R" at the beginning of the line will start a renegotiation instead
> of being transferred to the server :-) adding the EHLO and .CAPABILITY
> should be sufficient and the more complex parsing of the response
> might be omitted...

Do you want something like the attached patch ?
(untested, I'm off to bed...)

Re: STARTTLS patch for imap and ftp

Goetz Babin-Ebell wrote:

> Lutz Jaenicke wrote:
> > Goetz Babin-Ebell wrote:
> [...]
> >> * in SMTP doing a STARTTLS without previous EHLO
> >> will return a
> >> 503 STARTTLS command used when not advertised
> >> * in IMAP doing a STARTLS requires a
> >> . CAPABILITY
> >> first.
> >>
> >> In both cases the server response should be parsed for
> >> the string "STARTTLS"...
> >>
> > This statement is technically correct. As the s_client tool is however
> > intended for testing purposes only (you remember that a capital
> > "R" at the beginning of the line will start a renegotiation instead
> > of being transferred to the server :-) adding the EHLO and .CAPABILITY
> > should be sufficient and the more complex parsing of the response
> > might be omitted...
>
> Do you want something like the attached patch ?
> (untested, I'm off to bed...)
>

Yes, something like this. I have applied your patch to 0.9.8 and -dev... and

was just going to write "thank you" when I discovered that it does not work.
As I just noted BIO_read() does not work "line by line" but on the message
coming in. This message is the complete multi-line response and it has
to be parsed in a different way as attached as a crude hack.

No: BIO_gets() does not work on here (not supported on "connect BIO".

Yes: all other appearances of multi-line handling are broken as well.
The multi-line handling in the SMTP greeting would fail on the first
host with a multi-line greeting and the other protocol handlers are
as buggy. I have thus left your patch in and we have to decide how to
tackle the other occurances...

Re: STARTTLS patch for imap and ftp

On Wed, Feb 21, 2007, Lutz Jaenicke wrote:

> Goetz Babin-Ebell wrote:
> > Lutz Jaenicke wrote:
> > > Goetz Babin-Ebell wrote:
> > [...]
> > >> * in SMTP doing a STARTTLS without previous EHLO
> > >> will return a
> > >> 503 STARTTLS command used when not advertised
> > >> * in IMAP doing a STARTLS requires a
> > >> . CAPABILITY
> > >> first.
> > >>
> > >> In both cases the server response should be parsed for
> > >> the string "STARTTLS"...
> > >>
> > > This statement is technically correct. As the s_client tool is however
> > > intended for testing purposes only (you remember that a capital
> > > "R" at the beginning of the line will start a renegotiation instead
> > > of being transferred to the server :-) adding the EHLO and .CAPABILITY
> > > should be sufficient and the more complex parsing of the response
> > > might be omitted...
> >
> > Do you want something like the attached patch ?
> > (untested, I'm off to bed...)
> >
> Yes, something like this. I have applied your patch to 0.9.8 and -dev... and
> was just going to write "thank you" when I discovered that it does not work.
> As I just noted BIO_read() does not work "line by line" but on the message
> coming in. This message is the complete multi-line response and it has
> to be parsed in a different way as attached as a crude hack.
>
> No: BIO_gets() does not work on here (not supported on "connect BIO".
>

Note that adding a buffering BIO to the chain is a simple way to fix this.

Re: STARTTLS patch for imap and ftp

Dr. Stephen Henson wrote:

> On Wed, Feb 21, 2007, Lutz Jaenicke wrote:
>
>
>> Goetz Babin-Ebell wrote:
>>
>>> Lutz Jaenicke wrote:
>>>
>>>> Goetz Babin-Ebell wrote:
>>>>
>>> [...]
>>>
>>>>> * in SMTP doing a STARTTLS without previous EHLO
>>>>> will return a
>>>>> 503 STARTTLS command used when not advertised
>>>>> * in IMAP doing a STARTLS requires a
>>>>> . CAPABILITY
>>>>> first.
>>>>>
>>>>> In both cases the server response should be parsed for
>>>>> the string "STARTTLS"...
>>>>>
>>>>>
>>>> This statement is technically correct. As the s_client tool is however
>>>> intended for testing purposes only (you remember that a capital
>>>> "R" at the beginning of the line will start a renegotiation instead
>>>> of being transferred to the server :-) adding the EHLO and .CAPABILITY
>>>> should be sufficient and the more complex parsing of the response
>>>> might be omitted...
>>>>
>>> Do you want something like the attached patch ?
>>> (untested, I'm off to bed...)
>>>
>>>
>> Yes, something like this. I have applied your patch to 0.9.8 and -dev... and
>> was just going to write "thank you" when I discovered that it does not work.
>> As I just noted BIO_read() does not work "line by line" but on the message
>> coming in. This message is the complete multi-line response and it has
>> to be parsed in a different way as attached as a crude hack.
>>
>> No: BIO_gets() does not work on here (not supported on "connect BIO".
>>
>>
>
> Note that adding a buffering BIO to the chain is a simple way to fix this.
>

Re: STARTTLS patch for imap and ftp

> Lutz Jaenicke wrote:
> > Goetz Babin-Ebell wrote:
> [...]
> >> * in SMTP doing a STARTTLS without previous EHLO
> >> will return a
> >> 503 STARTTLS command used when not advertised
> >> * in IMAP doing a STARTLS requires a
> >> . CAPABILITY
> >> first.
> >>
> >> In both cases the server response should be parsed for
> >> the string "STARTTLS"...
> >>
> > This statement is technically correct. As the s_client tool is however
> > intended for testing purposes only (you remember that a capital
> > "R" at the beginning of the line will start a renegotiation instead
> > of being transferred to the server :-) adding the EHLO and .CAPABILITY
> > should be sufficient and the more complex parsing of the response
> > might be omitted...
>
> Do you want something like the attached patch ?
> (untested, I'm off to bed...)

Ok, I have reworked this section as discussed by using a buffering BIO and
have committed everything to CVS. I would be most pleased if somebody would
also cross-test it (the part with the multi-line IMAP response may require
some more digging as the termination should be the "." at the beginning
of the response line, not the number of chars being less than 3!?)

Re: STARTTLS patch for imap and ftp

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Hello Lutz,
Lutz Jaenicke wrote:

> Goetz Babin-Ebell wrote:
>> Lutz Jaenicke wrote:
>> [...]
>> Do you want something like the attached patch ?
>> (untested, I'm off to bed...)
> Ok, I have reworked this section as discussed by using a buffering BIO and
> have committed everything to CVS. I would be most pleased if somebody would
> also cross-test it (the part with the multi-line IMAP response may require
> some more digging as the termination should be the "." at the beginning
> of the response line, not the number of chars being less than 3!?)

Testet against cyrus imapd 2.1.18 and exim 4.50: OK.

You may drop the test for mbuf_len>3 in the while() for the IMAP
". CAPABILITY" response.
Has anyone seen a SMTP server return more than one line in the
SMTP opening message ?