I'll analyze your diff and its impact tomorrow with a clear mind, if sensible
it'll be committed tomorrow


On Mon, Aug 11, 2014 at 06:41:11PM +0200, Stefan Sieg wrote:
> On 11.08.2014 11:33, Gilles Chehade wrote:
> > hi,
> > 
> > can you explain this diff better ?
> > 
> > 
> 
> Hi,
> 
> thank you for your time, i will try to explain it better ...
> 
> This is only what leads me to the crude patch, i am not a programmer and
> i just wanted the mails out of the queue, so please don't hit me to hard
> if this is baloney :) ...
> 
> The more or less non OpenSMTPD part:
> 
> after this SSL diff:
> http://openbsd.cs.toronto.edu/cgi-bin/cvsweb/src/lib/libssl/src/ssl/s23_clnt.c.diff?r1=1.27&r2=1.28
> 
> opensmtpd starts TLS connections with TLS 1.2 and some hosts quit the TLS 
> handshake with an error:
> smtp-out: Error on session fb64ffbc1c251f66: SSL IO Error : 
> error:1407741A:SSL routines:SSL23_GET_SERVER_HELLO:tlsv1 alert decode error
> 
> Or with openssl:
> 
> # openssl s_client -msg -connect mail.example.com:25 -starttls smtp
> CONNECTED(00000003)
> >>> TLS 1.2 Handshake [length 0200], ClientHello
>     01 00 01 fc 03 03 d7 4f 3e 01 4a 90 41 27 c7 bf
>     ..
>     ..
>     ..
> 
> <<< TLS 1.0 Alert [length 0002], fatal decode_error
>     02 32
> 5074867261276:error:1407741A:SSL routines:SSL23_GET_SERVER_HELLO:tlsv1 alert 
> decode error:/usr/src/lib/libssl/ssl/../../libssl/src/ssl/s23_clnt.c:526
> 
> and mails to those hosts stay in the queue.
> (I can send you some addresses to test if you like.)
> 
> Before that SSL diff the handshake started with TLS 1.1 and all mails
> were perfectly relayed.
> 
> So, to not route the mails to those hosts through another relay, nor
> to disable TLS 1.2 in s23_clnt.c i came up with the smtpd diff.
> 
> 
> The OpenSMTPD part:
> 
> In case of a SSL connect error in ioev.c (io_dispatch_connect_ssl)
> we will end in default:
> 
>     ...
>     ...
> 
>     if ((ret = SSL_connect(io->ssl)) > 0) {
>         io->state = IO_STATE_UP;
>         io_callback(io, IO_TLSREADY);
>         goto leave;
>     }
> 
>     switch ((e = SSL_get_error(io->ssl, ret))) {
>         case SSL_ERROR_WANT_READ:
>                 io_reset(io, EV_READ, io_dispatch_connect_ssl);
>                 break;
>         case SSL_ERROR_WANT_WRITE:
>                 io_reset(io, EV_WRITE, io_dispatch_connect_ssl);
>                 break;
>         default:
>                 io->error = io_ssl_error();
>                 ssl_error("io_dispatch_connect_ssl:SSL_connect");
> -               io_callback(io, IO_ERROR);
> +               io_callback(io, IO_TLSERROR);
>                 break;
>         }
> 
> 
> Without the diff this will lead to IO_ERROR: in mta_session.c (mta_io):
> 
>     case IO_ERROR:
>         log_debug("debug: mta: %p: IO error: %s", s, io->error);
>         if (!s->ready) { 
>             mta_error(s, "IO Error: %s", io->error);
>             mta_connect(s);
>             break;
>         }
>         else if (!(s->flags & (MTA_FORCE_TLS|MTA_FORCE_ANYSSL))) {
>             /* error in non-strict SSL negotiation, downgrade to plain */
>             ..
>             ..
> 
> But as MTA_READY is not reached and therefore s->ready is not set to 1
> "downgrade to plain" is also not reached and it will try TLS again.
> At least this is what happens after the TLS handshake error.
> 
> Now the idea was not to use IO_ERROR but to define and use IO_TLSERROR
> and use the "downgrade to plain" part without the "ready" test.
> 
> +       case IO_TLSERROR:
> +               log_debug("debug: mta: %p: TLS IO error: %s", s, io->error);
> +               if (!(s->flags & (MTA_FORCE_TLS|MTA_FORCE_ANYSSL))) {
> +                       /* error in non-strict SSL negotiation, downgrade to 
> plain */
> +                       log_info("smtp-out: TLS Error on session %016"PRIx64
> +                               ": TLS failed, "
> +                               "downgrading to plain", s->id);
> +                       s->flags &= ~MTA_TLS;
> +                       s->flags |= MTA_DOWNGRADE_PLAIN;
> +                       mta_connect(s);
> +                       break;
> +               }
> +               mta_error(s, "IO Error: %s", io->error);
> +               mta_free(s);
> +               break;
> +
> 
> Nothing more but that worked for me.
> 
> Again ... thank you very much.
> 
> Stefan
> 
> -- 
> You received this mail because you are subscribed to [email protected]
> To unsubscribe, send a mail to: [email protected]
> 

-- 
Gilles Chehade

https://www.poolp.org                                          @poolpOrg

-- 
You received this mail because you are subscribed to [email protected]
To unsubscribe, send a mail to: [email protected]

Reply via email to