[openssl-dev] [openssl.org #4623] OpenSSL master regression in handling malformed Client Key Exchange messages in RSA key exchange

[David Benjamin via RT]

On Fri, Jul 22, 2016 at 7:30 PM Hubert Kario via RT <rt at openssl.org> wrote:

> On Friday, 22 July 2016 17:14:43 CEST Stephen Henson via RT wrote:
> > On Fri Jul 22 14:56:11 2016, hkario at redhat.com wrote:
> > > the issue is present in master 0ed26acce328ec16a3aa and looks to have
> > > been
> >
> > > introduced in commit:
> > I tried what I thought was a fix for this which is to simply delete the
> > lines:
> >
> > if (decrypt_len < 0)
> > goto err;
> >
> > from ssl/statem/statem_srvr.c
> >
> > However your reproducer still indicates errors. I checked the message
> logs
> > and it should be now sending as many alerts as the original. The
> difference
> > however is that some of them will be sent immediately whereas originally
> > they would be at the end of the handshake.
> >
> > Could your reproducer possibly not be expecting this?
>
>
> sorry, I copied this part:
>
> > when the OpenSSL receives a Client Key Exchange message that has the
> > encrypted
> > premaster secret comprised only of zero bytes, or equal to server's
> modulus,
> > the server just aborts the connection without sending an Alert message
>
> from the DHE/ECDHE bug reports
>
> the expected behaviour is to continue the connection, but the server should
> select a premaster secret that was not provided by the client, instead
> OpenSSL
> just closes the connection
>

I don't believe this test is correct if the encrypted premaster is equal to
the server's modulus. Such an encrypted premaster is a publicly invalid RSA
ciphertext. It is perfectly reasonable to reject it early, should unpadded
RSA_decrypt fail. The timing sensitive portion is the padding check itself,
which this code should correctly defer failure of, to avoid the padding
oracle.

The reason public decrypt failures did not trigger early alerts before was
because the old code was a little sloppy about error-handling. In addition
to not being constant time, it squashed together codepaths for publicly
invalid keys and the timing sensitive check.

It is true that it no longer sends an alert on public failures. Probably
worth fixing. Meh.

David

-- 
Ticket here: http://rt.openssl.org/Ticket/Display.html?id=4623
Please log in as guest with password guest if prompted