lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Date:   Sun, 15 Nov 2020 04:11:05 +0000
From:   Vadim Fedorenko <vfedorenko@...ek.ru>
To:     Jakub Kicinski <kuba@...nel.org>
Cc:     Boris Pismenny <borisp@...dia.com>,
        Aviad Yehezkel <aviadye@...dia.com>, netdev@...r.kernel.org
Subject: Re: [net] net/tls: fix corrupted data in recvmsg



On 15.11.2020 03:54, Jakub Kicinski wrote:
> Please don't top post.
>
> On Sun, 15 Nov 2020 02:26:30 +0000 Vadim Fedorenko wrote:
>> No, I don't have any BPFs in test.
>> If we have Application Data in TCP queue then tls_sw_advance_skb
>> will change ctx->control from 0x16 to 0x17 (TLS_RECORD_TYPE_DATA)
>> and the loop will continue.
> Ah! Missed that, unpausing the parser will make it serve us another
> message and overwrite ctx.
>
>> The patched if will make zc = true and
>> data will be decrypted into msg->msg_iter.
>> After that the loop will break on:
>>                   if (!control)
>>                           control = tlm->control;
>>                   else if (control != tlm->control)
>>                           goto recv_end;
>>
>> and the data will be lost.
>> Next call to recvmsg will find ctx->decrypted set to true and will
>> copy the unencrypted data from skb assuming that it has been decrypted
>> already.
>>
>> The patch that I put into Fixes: changed the check you mentioned to
>> ctx->control, but originally it was checking the value of control that
>> was stored before calling to tls_sw_advance_skb.
> Is there a reason why we wouldn't just go back to checking the stored
> control? Or better - put your condition there (control != ctx->control)?
> Decrypting the next record seems unnecessary given we can't return it.
Going back to checking stored control is working in TLS 1.2 but I cannot
test it with TLS 1.3. But it looks like it will work too in that case.
Variable control should store correct value to pass it in control message
and it's not chaged after that.
Ok, will send v2 with check reverted, thanks!

Powered by blists - more mailing lists