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 <r...@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, hka...@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 -- openssl-dev mailing list To unsubscribe: https://mta.openssl.org/mailman/listinfo/openssl-dev