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

[Kurt Roeckx via RT]

On Fri, Jul 22, 2016 at 10:16:16PM +0000, David Benjamin via RT wrote:
> 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.

I think that the test suite should allow for either of 2
behaviours in case of the public failures:
- It continues with a random premaster secret.
- It generates an error.

>From what I understand it now complains that we don't do the first,
so I think it should get changed to allow both behaviours.

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

I don't care for which behaviour we go, and I guess that if we go
for generating an error we should send an alert.

I don't see the point in wasting time for what most likely seems
to be an attack.

What I don't understand currently is that it also rejected a
ciphertext with all 0's?  Were it too many 0's?


Kurt



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