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 <vfedore...@novek.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.
