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 ? Bye Goetz -BEGIN PGP SIGNATURE- Version: GnuPG v1.4.2 (GNU/Linux) Comment: Using GnuPG with SUSE - http://enigmail.mozdev.org iD8DBQFF3twx2iGqZUF3qPYRAscBAJ9/JrYHEqPOcLfgDShP8onKeRLYFgCffg7+ YqGeshjiakpwo4f9gDtPJa0= =HzVN -END PGP SIGNATURE- __ OpenSSL Project http://www.openssl.org Development Mailing List openssl-dev@openssl.org Automated List Manager [EMAIL PROTECTED]
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...) 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!?) Best regards, Lutz __ OpenSSL Project http://www.openssl.org Development Mailing List openssl-dev@openssl.org Automated List Manager [EMAIL PROTECTED]
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.
>
Yes. I get your point :-)
Best regards,
Lutz
Index: apps/s_client.c
===
RCS file: /e/openssl/cvs/openssl/apps/s_client.c,v
retrieving revision 1.76.2.7
diff -u -r1.76.2.7 s_client.c
--- apps/s_client.c 21 Feb 2007 18:20:33 - 1.76.2.7
+++ apps/s_client.c 21 Feb 2007 19:55:21 -
@@ -736,22 +736,28 @@
if (starttls_proto == PROTO_SMTP)
{
int foundit=0;
+ BIO *fbio = BIO_new(BIO_f_buffer());
+ BIO_push(fbio, sbio);
/* wait for multi-line response to end from SMTP */
do
{
- mbuf_len = BIO_read(sbio,mbuf,BUFSIZZ);
+ mbuf_len = BIO_gets(fbio,mbuf,BUFSIZZ);
}
while (mbuf_len>3 && mbuf[3]=='-');
/* STARTTLS command requires EHLO... */
- BIO_printf(sbio,"EHLO openssl.client.net\r\n");
+ BIO_printf(fbio,"EHLO openssl.client.net\r\n");
+ BIO_flush(fbio);
/* wait for multi-line response to end EHLO SMTP response */
do
{
- mbuf_len = BIO_read(sbio,mbuf,BUFSIZZ);
+ mbuf_len = BIO_gets(fbio,mbuf,BUFSIZZ);
if (strstr(mbuf,"STARTTLS"))
foundit=1;
}
while (mbuf_len>3 && mbuf[3]=='-');
+ BIO_flush(fbio);
+ BIO_pop(fbio);
+ BIO_free(fbio);
if (!foundit)
BIO_printf(bio_err,
"didn't found starttls in server response,"
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. Steve. -- Dr Stephen N. Henson. Email, S/MIME and PGP keys: see homepage OpenSSL project core developer and freelance consultant. Funding needed! Details on homepage. Homepage: http://www.drh-consultancy.demon.co.uk __ OpenSSL Project http://www.openssl.org Development Mailing List openssl-dev@openssl.org Automated List Manager [EMAIL PROTECTED]
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...
Best regards,
Lutz
Index: s_client.c
===
RCS file: /e/openssl/cvs/openssl/apps/s_client.c,v
retrieving revision 1.76.2.7
diff -u -r1.76.2.7 s_client.c
--- s_client.c 21 Feb 2007 18:20:33 - 1.76.2.7
+++ s_client.c 21 Feb 2007 18:53:00 -
@@ -735,7 +735,7 @@
/* This is an ugly hack that does a lot of assumptions */
if (starttls_proto == PROTO_SMTP)
{
- int foundit=0;
+ int foundit=0, response_done = 0;
/* wait for multi-line response to end from SMTP */
do
{
@@ -747,11 +747,15 @@
/* wait for multi-line response to end EHLO SMTP response */
do
{
+ int ll;
mbuf_len = BIO_read(sbio,mbuf,BUFSIZZ);
if (strstr(mbuf,"STARTTLS"))
foundit=1;
+ for (ll = 0; !response_done && ll < mbuf_len - 4; ll++)
+if (mbuf[ll] == '\n' && mbuf[ll + 3] != '-')
+ response_done = 1;
}
- while (mbuf_len>3 && mbuf[3]=='-');
+ while (mbuf_len>3 && mbuf[3]=='-' && !response_done);
if (!foundit)
BIO_printf(bio_err,
"didn't found starttls in server response,"
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...)
Bye
Goetz
-BEGIN PGP SIGNATURE-
Version: GnuPG v1.4.2 (GNU/Linux)
Comment: Using GnuPG with SUSE - http://enigmail.mozdev.org
iD8DBQFF2lTq2iGqZUF3qPYRAirhAJ9+e7H1qRzUH7RZAuHKBGpqUDrVfwCfb2A2
B7Z713+mhzGcIx5/VZHtBNA=
=ABXa
-END PGP SIGNATURE-
Index: apps/s_client.c
===
RCS file: /home/gbe/data/cvs/openssl/openssl/apps/s_client.c,v
retrieving revision 1.100
diff -u -r1.100 s_client.c
--- apps/s_client.c 18 Feb 2007 18:21:57 - 1.100
+++ apps/s_client.c 20 Feb 2007 01:47:50 -
@@ -914,12 +914,27 @@
/* This is an ugly hack that does a lot of assumptions */
if (starttls_proto == PROTO_SMTP)
{
+ int foundit=0;
/* wait for multi-line response to end from SMTP */
do
{
mbuf_len = BIO_read(sbio,mbuf,BUFSIZZ);
}
while (mbuf_len>3 && mbuf[3]=='-');
+ /* STARTTLS command requires EHLO... */
+ BIO_printf(sbio,"EHLO openssl.client.net\r\n");
+ /* wait for multi-line response to end EHLO SMTP response */
+ do
+ {
+ mbuf_len = BIO_read(sbio,mbuf,BUFSIZZ);
+ if (strstr(mbuf,"STARTTLS"))
+foundit=1;
+ }
+ while (mbuf_len>3 && mbuf[3]=='-');
+ if (!foundit)
+ BIO_printf(bio_err,
+ "didn't found starttls in server response,"
+ " try anyway...\n");
BIO_printf(sbio,"STARTTLS\r\n");
BIO_read(sbio,sbuf,BUFSIZZ);
}
@@ -931,8 +946,23 @@
}
else if (starttls_proto == PROTO_IMAP)
{
+ int foundit=0;
BIO_read(sbio,mbuf,BUFSIZZ);
- BIO_printf(sbio,"0 STARTTLS\r\n");
+ /* STARTTLS command requires CAPABILITY... */
+ BIO_printf(sbio,". CAPABILITY\r\n");
+ /* wait for multi-line CAPABILITY response */
+ do
+ {
+ mbuf_len = BIO_read(sbio,mbuf,BUFSIZZ);
+ if (strstr(mbuf,"STARTTLS"))
+foundit=1;
+ }
+ while (mbuf_len>3);
+ if (!foundit)
+ BIO_printf(bio_err,
+ "didn't found STARTTLS in server response,"
+ " try anyway...\n");
+ BIO_printf(sbio,". STARTTLS\r\n");
BIO_read(sbio,sbuf,BUFSIZZ);
}
else if (starttls_proto == PROTO_FTP)
Re: STARTTLS patch for imap and ftp
Goetz Babin-Ebell wrote: > Hello Richard, > > Richard Levitte - VMS Whacker wrote: > > In message <[EMAIL PROTECTED]> on Thu, 15 Feb 2007 > 10:34:23 -0800, > > Kees Cook <[EMAIL PROTECTED]> 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... Best regards, Lutz __ OpenSSL Project http://www.openssl.org Development Mailing List openssl-dev@openssl.org Automated List Manager [EMAIL PROTECTED] __ OpenSSL Project http://www.openssl.org Development Mailing List openssl-dev@openssl.org Automated List Manager [EMAIL PROTECTED]
Re: STARTTLS patch for imap and ftp
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 Hello Richard, Richard Levitte - VMS Whacker wrote: > In message <[EMAIL PROTECTED]> on Thu, 15 Feb 2007 10:34:23 -0800, > Kees Cook <[EMAIL PROTECTED]> 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"... Bye Goetz -BEGIN PGP SIGNATURE- Version: GnuPG v1.4.2 (GNU/Linux) Comment: Using GnuPG with SUSE - http://enigmail.mozdev.org iD8DBQFF1xsY2iGqZUF3qPYRAreLAJ9MF6ht6pP2nnzx5pL5x7kTwuOsuACeLyZb QAA8Z0W0Wd6biFEb0K4D0SA= =72Vc -END PGP SIGNATURE- __ OpenSSL Project http://www.openssl.org Development Mailing List openssl-dev@openssl.org Automated List Manager [EMAIL PROTECTED]
Re: STARTTLS patch for imap and ftp
On Fri, Feb 16, 2007 at 07:12:50PM +0100, Richard Levitte - VMS Whacker wrote: > Yes. Done. Thank you, and sorry you had to wait 3 years for this to > happen. Great! Thank you. :) -- Kees Cook@outflux.net __ OpenSSL Project http://www.openssl.org Development Mailing List openssl-dev@openssl.org Automated List Manager [EMAIL PROTECTED]
Re: STARTTLS patch for imap and ftp
In message <[EMAIL PROTECTED]> on Thu, 15 Feb 2007 10:34:23 -0800, Kees Cook <[EMAIL PROTECTED]> 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. Cheers, Richard - Please consider sponsoring my work on free software. See http://www.free.lp.se/sponsoring.html for details. -- Richard Levitte [EMAIL PROTECTED] http://richard.levitte.org/ "When I became a man I put away childish things, including the fear of childishness and the desire to be very grown up." -- C.S. Lewis __ OpenSSL Project http://www.openssl.org Development Mailing List openssl-dev@openssl.org Automated List Manager [EMAIL PROTECTED]
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?
Thanks,
-Kees
[1] http://marc.theaimsgroup.com/?l=openssl-dev&m=109794442901659&w=2
[2] http://marc.theaimsgroup.com/?l=openssl-dev&m=109803041012966&w=2
[3] http://marc.theaimsgroup.com/?l=openssl-dev&w=2&r=1&s=starttls&q=b
[4] http://www.mail-archive.com/openssl-dev@openssl.org/msg20600.html
--
Kees Cook@outflux.net
Index: apps/s_client.c
===
RCS file: /scratch/src/openssl/upstream-cvs/openssl/apps/s_client.c,v
retrieving revision 1.98
diff -u -p -u -p -r1.98 s_client.c
--- apps/s_client.c 29 Nov 2006 20:54:55 - 1.98
+++ apps/s_client.c 15 Feb 2007 18:32:15 -
@@ -316,7 +316,7 @@ static void sc_usage(void)
BIO_printf(bio_err," -starttls prot - use the STARTTLS command before starting TLS\n");
BIO_printf(bio_err," for those protocols that support it, where\n");
BIO_printf(bio_err," 'prot' defines which one to assume. Currently,\n");
- BIO_printf(bio_err," only \"smtp\" and \"pop3\" are supported.\n");
+ BIO_printf(bio_err," only \"smtp\", \"pop3\", \"imap\", and \"ftp\" are supported.\n");
#ifndef OPENSSL_NO_ENGINE
BIO_printf(bio_err," -engine id- Initialise and use the specified engine\n");
#endif
@@ -348,6 +348,15 @@ static int MS_CALLBACK ssl_servername_cb
}
#endif
+enum
+{
+ PROTO_OFF = 0,
+ PROTO_SMTP,
+ PROTO_POP3,
+ PROTO_IMAP,
+ PROTO_FTP,
+};
+
int MAIN(int, char **);
int MAIN(int argc, char **argv)
@@ -374,12 +383,13 @@ int MAIN(int argc, char **argv)
int write_tty,read_tty,write_ssl,read_ssl,tty_on,ssl_pending;
SSL_CTX *ctx=NULL;
int ret=1,in_init=1,i,nbio_test=0;
- int starttls_proto = 0;
+ int starttls_proto = PROTO_OFF;
int prexit = 0, vflags = 0;
const SSL_METHOD *meth=NULL;
int socket_type=SOCK_STREAM;
BIO *sbio;
char *inrand=NULL;
+ int mbuf_len=0;
#ifndef OPENSSL_NO_ENGINE
char *engine_id=NULL;
ENGINE *e=NULL;
@@ -610,9 +620,13 @@ int MAIN(int argc, char **argv)
if (--argc < 1) goto bad;
++argv;
if (strcmp(*argv,"smtp") == 0)
-starttls_proto = 1;
+starttls_proto = PROTO_SMTP;
else if (strcmp(*argv,"pop3") == 0)
-starttls_proto = 2;
+starttls_proto = PROTO_POP3;
+ else if (strcmp(*argv,"imap") == 0)
+starttls_proto = PROTO_IMAP;
+ else if (strcmp(*argv,"ftp") == 0)
+starttls_proto = PROTO_FTP;
else
goto bad;
}
@@ -898,18 +912,40 @@ re_start:
sbuf_off=0;
/* This is an ugly hack that does a lot of assumptions */
- if (starttls_proto == 1)
+ if (starttls_proto == PROTO_SMTP)
{
- BIO_read(sbio,mbuf,BUFSIZZ);
+ /* wait for multi-line response to end from SMTP */
+ do
+ {
+ mbuf_len = BIO_read(sbio,mbuf,BUFSIZZ);
+ }
+ while (mbuf_len>3 && mbuf[3]=='-');
BIO_printf(sbio,"STARTTLS\r\n");
BIO_read(sbio,sbuf,BUFSIZZ);
}
- if (starttls_proto == 2)
+ else if (starttls_proto == PROTO_POP3)
{
BIO_read(sbio,mbuf,BUFSIZZ);
BIO_printf(sbio,"STLS\r\n");
BIO_read(sbio,sbuf,BUFSIZZ);
}
+ else if (starttls_proto == PROTO_IMAP)
+ {
+ BIO_read(sbio,mbuf,BUFSIZZ);
+ BIO_printf(sbio,"0 STARTTLS\r\n");
+ BIO_read(sbio,sbuf,BUFSIZZ);
+ }
+ else if (starttls_proto == PROTO_FTP)
+ {
+ /* wait for multi-line response to end from FTP */
+ do
+ {
+ mbuf_len = BIO_read(sbio,mbuf,BUFSIZZ);
+ }
+ while (mbuf_len>3 && mbuf[3]=='-');
+ BIO_printf(sbio,"AUTH TLS\r\n");
+ BIO_read(sbio,sbuf,BUFSIZZ);
+ }
for (;;)
{
@@ -940,7 +976,7 @@ re_start:
{
BIO_printf(bio_err,"%s",mbuf);
/* We don't need to know any more */
- starttls_proto = 0;
+ starttls_proto = PROTO_OFF;
}
if (reconnect)
Index: doc/apps/s_client.pod
===
RCS file: /scratch/src/openssl/upstream-cvs/openssl/doc/apps/s_client.pod,v
retrieving revision 1.14
diff -u -p -u -p -r1.14 s_client.pod
--- doc/apps/s_client.pod 10 Mar 2006 23:06:15 - 1.14
+++ doc/apps/s_client.pod 15 Feb 2007 18:19:34 -
@@ -194,7 +194,7 @@ command for more information.
send the protocol-specific message(s) to switch to TLS for communication.
B is a keyword for the intended protocol. Currently, the only
-supported keywords are "smtp" and "pop3".
+supported keywords are "smtp", "pop3", "imap", and "ftp".
=item B<-engine id>
