[Fwd: Re: [Fwd: STARTTLS vulnerabilty and qmail-spamcontrol ucspi-ssl qpsmtpd]]

I've been otherwise occupied but I forwarded this to the rest of our dev
team and our resident security "guru" had this to say

---------------------------- Original Message ----------------------------
Subject: Re: [Fwd: STARTTLS vulnerabilty and qmail-spamcontrol ucspi-ssl
qpsmtpd]
From:    "Peter Samuelson" <[email protected]>
Date:    Thu, June 2, 2011 1:32 pm
To:      [email protected]
--------------------------------------------------------------------------


> Subject: [Fwd: STARTTLS vulnerabilty and qmail-spamcontrol ucspi-ssl
qpsmtpd]
> From:    "Matt Sergeant" <[email protected]>

Heh, this class of vulnerability made the rounds some months ago -
a (perhaps) surprising number of open source software out there that
deals in SSL/TLS had this class of bug.

> I'm pretty sure this is enough to fix it for async:
>
> diff --git a/plugins/tls b/plugins/tls
> index 37fbc9a..f850d2c 100644
> --- a/plugins/tls
> +++ b/plugins/tls
> @@ -275,6 +275,7 @@ sub upgrade_socket {
>       my UpgradeClientSSL $self = shift;
>
>       unless ( $self->{_ssl_started} ) {
> +        $self->{_stashed_qp}->clear_data();
>           IO::Socket::SSL->start_SSL(
>               $self->{_stashed_qp}->{sock}, {
>                   SSL_use_cert => 1,

Yeah, looks right to me.

> But for the non-async scenario I think it's a lot more complex because
> the caching would be done at the C-level, so a fix more like the fix
> (posted below) for postfix is required (switch to non-blocking, get
> whatever data is remaining, flush it, switch back off non-blocking).

I ... disagree.  From my reading of plugins/tls, it looks like there is
no problem at all, in the non-async code path.  It resets STDIN and
STDOUT to a socket created from scratch by the IO::Socket::SSL module.

I haven't looked at IO::Socket::SSL to see if it has this sort of
issue, but it seems unlikely to me.

Peter