netdev - Re: [net] net/tls: missing received data after fast remote close

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 linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Wed, 18 Nov 2020 00:50:48 +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: missing received data after fast remote close

On 17.11.2020 22:38, Jakub Kicinski wrote:
> On Sun, 15 Nov 2020 14:43:48 +0300 Vadim Fedorenko wrote:
>> In case when tcp socket received FIN after some data and the
>> parser haven't started before reading data caller will receive
>> an empty buffer.
> This is pretty terse, too terse for me to understand..
The flow is simple. Server sends small amount of data right after the
connection is configured and closes the connection. In this case
receiver sees TLS Handshake data, configures TLS socket right after
Change Cipher Spec record. While the configuration is in process, TCP
socket receives small Application Data record, Encrypted Alert record
and FIN packet. So the TCP socket changes sk_shutdown to RCV_SHUTDOWN
and sk_flag with SK_DONE bit set.
>> This behavior differs from plain TCP socket and
>> leads to special treating in user-space. Patch unpauses parser
>> directly if we have unparsed data in tcp receive queue.
> Sure, but why is the parser paused? Does it pause itself on FIN?
No, it doesn't start even once. The trace looks like:

tcp_recvmsg is called
tcp_recvmsg returns 1 (Change Cipher Spec record data)
tls_setsockopt is called
tls_setsockopt returns
tls_recvmsg is called
tls_recvmsg returns 0
__strp_recv is called
stack
         __strp_recv+1
         tcp_read_sock+169
         strp_read_sock+104
         strp_work+68
         process_one_work+436
         worker_thread+80
         kthread+276
         ret_from_fork+34tls_read_size called

So it looks like strp_work was scheduled after tls_recvmsg and
nothing triggered parser because all the data was received before
tls_setsockopt ended the configuration process.
>
>> diff --git a/net/tls/tls_sw.c b/net/tls/tls_sw.c
>> index 2fe9e2c..4db6943 100644
>> --- a/net/tls/tls_sw.c
>> +++ b/net/tls/tls_sw.c
>> @@ -1289,6 +1289,9 @@ static struct sk_buff *tls_wait_data(struct sock *sk, struct sk_psock *psock,
>>   	struct sk_buff *skb;
>>   	DEFINE_WAIT_FUNC(wait, woken_wake_function);
>>   
>> +	if (!ctx->recv_pkt && skb_queue_empty(&sk->sk_receive_queue))
>> +		__strp_unpause(&ctx->strp);
>> +
>>   	while (!(skb = ctx->recv_pkt) && sk_psock_queue_empty(psock)) {
>>   		if (sk->sk_err) {
>>   			*err = sock_error(sk);