netdev - Re: [PATCH net-next 0/5] tls: implement key updates for TLS1.3

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 Jan 2023 11:06:25 +0100
From:   Sabrina Dubroca <sd@...asysnail.net>
To:     Jakub Kicinski <kuba@...nel.org>
Cc:     netdev@...r.kernel.org, Frantisek Krenzelok <fkrenzel@...hat.com>
Subject: Re: [PATCH net-next 0/5] tls: implement key updates for TLS1.3

2023-01-17, 18:03:51 -0800, Jakub Kicinski wrote:
> Please CC all the maintainers.

Sorry.

> On Tue, 17 Jan 2023 14:45:26 +0100 Sabrina Dubroca wrote:
> > This adds support for receiving KeyUpdate messages (RFC 8446, 4.6.3
> > [1]). A sender transmits a KeyUpdate message and then changes its TX
> > key. The receiver should react by updating its RX key before
> > processing the next message.
> > 
> > This patchset implements key updates by:
> >  1. pausing decryption when a KeyUpdate message is received, to avoid
> >     attempting to use the old key to decrypt a record encrypted with
> >     the new key
> >  2. returning -EKEYEXPIRED to syscalls that cannot receive the
> >     KeyUpdate message, until the rekey has been performed by userspace
> 
> Why? We return to user space after hitting a cmsg, don't we?
> If the user space wants to keep reading with the old key - 🤷️

But they won't be able to read anything. Either we don't pause
decryption, and the socket is just broken when we look at the next
record, or we pause, and there's nothing to read until the rekey is
done. I think that -EKEYEXPIRED is better than breaking the socket
just because a read snuck in between getting the cmsg and setting the
new key.

> >  3. passing the KeyUpdate message to userspace as a control message
> >  4. allowing updates of the crypto_info via the TLS_TX/TLS_RX
> >     setsockopts
> > 
> > This API has been tested with gnutls to make sure that it allows
> > userspace libraries to implement key updates [2]. Thanks to Frantisek
> > Krenzelok <fkrenzel@...hat.com> for providing the implementation in
> > gnutls and testing the kernel patches.
> 
> Please explain why - the kernel TLS is not faster than user space, 
> the point of it is primarily to enable offload. And you don't add
> offload support here.

Well, TLS1.3 support was added 4 years ago, and yet the offload still
doesn't support 1.3 at all.

IIRC support for KeyUpdates is mandatory in TLS1.3, so currently the
kernel can't claim to support 1.3, independent of offloading.

Some folks did tests with and without kTLS using nbdcopy and found a
small but noticeable performance improvement (around 8-10%).

> > Note: in a future series, I'll clean up tls_set_sw_offload and
> > eliminate the per-cipher copy-paste using tls_cipher_size_desc.
> 
> Yeah, I think it's on Vadim's TODO list as well.

I've already done most of the work as I was working on this, I'll
submit it later.

-- 
Sabrina