Date:   Tue, 24 Jan 2023 16:56:23 +0100
From:   Sabrina Dubroca <sd@...asysnail.net>
To:     Boris Pismenny <borisp@...dia.com>, Simo Sorce <simo@...hat.com>,
        Daiki Ueno <dueno@...hat.com>
Cc:     Gal Pressman <gal@...dia.com>, Jakub Kicinski <kuba@...nel.org>,
        Tariq Toukan <tariqt@...dia.com>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        Frantisek Krenzelok <fkrenzel@...hat.com>
Subject: Re: [PATCH net-next 0/5] tls: implement key updates for TLS1.3

(adding Simo and Daiki for the OpenSSL/GnuTLS sides)

2023-01-23, 10:13:58 +0000, Boris Pismenny wrote:
> On 19/01/2023 10:27, Gal Pressman wrote:
> > On 19/01/2023 4:55, Jakub Kicinski wrote:
> >> On Wed, 18 Jan 2023 11:06:25 +0100 Sabrina Dubroca wrote:
> >>> 2023-01-17, 18:03:51 -0800, Jakub Kicinski wrote:
> >>>> 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.
> >>
> >> IDK, we don't interpret any other content types/cmsgs, and for well
> >> behaved user space there should be no problem (right?).
> >> I'm weakly against, if nobody agrees with me you can keep as is.
> >>
> >>>>>  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.
> >>
> >> I'm pretty sure some devices support it. None of the vendors could 
> >> be bothered to plumb in the kernel support, yet, tho.
> > 
> > Our device supports TLS 1.3, it's in our plans to add driver/kernel support.
> > 
> >> I don't know of anyone supporting rekeying.
> > 
> > Boris, Tariq, do you know?
> 
> Rekeying is not trivial to get right with offload. There are at least
> two problems to solve:
> 1. On transmit, we need to handle both the new and the old key for new
> and old (retransmitted) data, respectively. Our device will be able to
> hold both keys in parallel and to choose the right one at the cost of an
> if statement in the data-path. Alternatively, we can just fallback to
> software for the old key and focus on the new key.

We'll need to keep the old key around until we know all the records
using it have been fully received, right?  And that could be multiple
old keys, in case of a quick series of key updates.

> 2. On Rx, packets with the new key may arrive before the key is
> installed unless we design a mechanism for preemptively setting the next
> key in HW. As a result, we may get a resync on every rekey.
> 
> Have you considered an API to preemptively set the next key in the
> kernel such that there is never a need to stop the datapath? I think
> that the change in SSL libraries is minor and it can really help KTLS.

I don't like the idea of having some unused key stored in the kernel
for long durations too much.

You can't be sure that there will be only one rekey in the next short
interval, so you'll need to be able to handle those resyncs anyway, in
case userspace is too slow providing the 3rd key (for the 2nd
rekey). For example, the RFC mentions:

   If implementations independently send their own KeyUpdates with
   request_update set to "update_requested" and they cross in flight,
   then each side will also send a response, with the result that each
   side increments by two generations.

   https://www.rfc-editor.org/rfc/rfc8446#section-4.6.3

So I think what you're suggesting can only be an optimization,
not a full solution.

I hope I'm not getting too confused by all this.

-- 
Sabrina

Powered by blists - more mailing lists