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
| ||
|
Date: Mon, 16 Nov 2020 16:54:54 -0800 From: Jakub Kicinski <kuba@...nel.org> To: Vadim Fedorenko <vfedorenko@...ek.ru> Cc: Boris Pismenny <borisp@...dia.com>, Aviad Yehezkel <aviadye@...dia.com>, netdev@...r.kernel.org Subject: Re: [net v2] net/tls: fix corrupted data in recvmsg On Tue, 17 Nov 2020 00:45:11 +0000 Vadim Fedorenko wrote: > On 17.11.2020 00:26, Jakub Kicinski wrote: > > On Sun, 15 Nov 2020 07:16:00 +0300 Vadim Fedorenko wrote: > >> If tcp socket has more data than Encrypted Handshake Message then > >> tls_sw_recvmsg will try to decrypt next record instead of returning > >> full control message to userspace as mentioned in comment. The next > >> message - usually Application Data - gets corrupted because it uses > >> zero copy for decryption that's why the data is not stored in skb > >> for next iteration. Revert check to not decrypt next record if > >> current is not Application Data. > >> > >> Fixes: 692d7b5d1f91 ("tls: Fix recvmsg() to be able to peek across multiple records") > >> Signed-off-by: Vadim Fedorenko <vfedorenko@...ek.ru> > >> --- > >> net/tls/tls_sw.c | 2 +- > >> 1 file changed, 1 insertion(+), 1 deletion(-) > >> > >> diff --git a/net/tls/tls_sw.c b/net/tls/tls_sw.c > >> index 95ab5545..2fe9e2c 100644 > >> --- a/net/tls/tls_sw.c > >> +++ b/net/tls/tls_sw.c > >> @@ -1913,7 +1913,7 @@ int tls_sw_recvmsg(struct sock *sk, > >> * another message type > >> */ > >> msg->msg_flags |= MSG_EOR; > >> - if (ctx->control != TLS_RECORD_TYPE_DATA) > >> + if (control != TLS_RECORD_TYPE_DATA) > > Sorry I wasn't clear enough, should this be: > > > > if (ctx->control != control) > > > > ? Otherwise if we get a control record first and then data record > > the code will collapse them, which isn't correct, right? > > > >> goto recv_end; > >> } else { > >> break; > I think you mean when ctx->control is control record and control is > data record. Yup. > In this case control message will be decrypted without > zero copy and will be stored in skb for the next recvmsg, but will > not be returned together with data message. Could you point me to a line which breaks the loop in that case? > This behavior is the same > as for TLSv1.3 when record type is known only after decrypting. > But if we want completely different flow for TLSv1.2 and TLSv1.3 > then changing to check difference in message types makes sense.
Powered by blists - more mailing lists