[openssl-dev] [openssl.org #3712] TLS Renegotiation with Java is broken

[Matt Caswell via RT]

On 12/10/15 16:39, Matt Caswell via RT wrote:
> 
> 
> On 12/10/15 16:03, Alessandro Ghedini via RT wrote:
>> On Mon, Oct 12, 2015 at 01:45:20PM +0000, Hubert Kario via RT wrote:
>>> On Friday 09 October 2015 18:05:19 Matt Caswell via RT wrote:
>>>> On 09/10/15 19:02, Hubert Kario via RT wrote:
>>>>> And for good measure, I also created a test script that
>>>>> combines fragmentation with interleaving.
>>>>
>>>> Did you try my patch with it? And if so what happened?
>>>
>>> I'm using interleave-data-102.patch attached to this ticket.
>>>
>>> So, for state-machine-rewrite branch it doesn't apply, there's no 
>>> ssl/s3_pkt.c file.
>>>
>>> For current 1.0.1 branch, the patch applies, test case results are as 
>>> follows:
>>>  * test-openssl-3712.py - pass
>>>  * test-interleaved-application-data-in-renegotiation.py - pass
>>>  * test-interleaved-application-data-and-fragmented-handshakes-in-
>>> renegotiation.py - pass
>>>
>>> For current 1.0.2 branch, the patch applies, tests case results are as 
>>> follows:
>>>  * test-openssl-3712.py - pass
>>>  * test-interleaved-application-data-in-renegotiation.py - pass
>>>  * test-interleaved-application-data-and-fragmented-handshakes-in-
>>> renegotiation.py - pass
>>>
>>> for current master the patch doesn't apply, just like with state-
>>> machine-rewrite there's no ssl/s3_pkt.c file
>>>
>>> Note: the two latter test cases need the s_server run in -www mode, the 
>>> first test case ignores server response so will work regardless, that 
>>> may be why Alessandro testing doesn't show the issue as fixed
>>
>> Ah, yep, with -www it works for me too. Note that on master the file to change
>> should be ssl/record/ssl3_record.c. However, while the patch applies cleanly to
>> this file, all the tests fail (even with -www). It seems that the field
>> in_read_app_data is never true, so the UNEXPECTED_MESSAGE alert is sent.
> 
> The value of "in_read_app_data" not being true when it is supposed to
> appears to be running into a slightly different bug. It's also present
> in 1.0.2 but you have to switch off version negotiation. So running
> s_server like this in 1.0.2 and rerunning Hubert's test will hit it:
> 
> openssl s_server -www -tls1_2
> 
> The 1.0.2 version negotiation is hiding the issue. In master version neg
> has been completely rewritten and simplified - but in doing so no longer
> hides the problem. :-(

Having done some more digging it seems the problem only occurs if you
get the initial handshake, following by a second reneg handshake *and*
interleaved app data all within the scope of a *single* SSL_read call.
AFAICT if SSL_read returns between the first handshake and the second,
you don't get the problem.

That's starting to sound like quite an unlikely scenario and we're only
hitting it now because of the slightly artificial nature of Hubert's
test. I'm wondering whether "will not fix" is the right response to this
second bug? Thoughts? Having said that it would be nice to have a
reliable test for the interleaved-app data issue.

Matt