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

From: Jakub Kicinski <kuba@kernel.org>
To: Sabrina Dubroca <sd@queasysnail.net>
Cc: netdev@vger.kernel.org, Vadim Fedorenko <vfedorenko@novek.ru>,
	Frantisek Krenzelok <fkrenzel@redhat.com>,
	Kuniyuki Iwashima <kuniyu@amazon.com>,
	Apoorv Kothari <apoorvko@amazon.com>,
	Boris Pismenny <borisp@nvidia.com>,
	John Fastabend <john.fastabend@gmail.com>,
	Shuah Khan <shuah@kernel.org>,
	linux-kselftest@vger.kernel.org, Gal Pressman <gal@nvidia.com>,
	Marcel Holtmann <marcel@holtmann.org>
Subject: Re: [PATCH net-next v2 0/5] tls: implement key updates for TLS1.3
Date: Mon, 13 Mar 2023 11:35:10 -0700	[thread overview]
Message-ID: <20230313113510.02c107b3@kernel.org> (raw)
In-Reply-To: <ZA9EMJgoNsxfOhwV@hog>

On Mon, 13 Mar 2023 16:41:36 +0100 Sabrina Dubroca wrote:
> > > Yes, I was looking into that earlier this week. I think we could reuse
> > > a similar mechanism for rekeying. tls_dev_add takes tcp_sk->write_seq,
> > > we could have a tls_dev_rekey op passing the new key and new write_seq
> > > to the driver. I think we can also reuse the ->eor trick from
> > > tls_set_device_offload, and we wouldn't have to look at
> > > skb->decrypted. Close and push the current SW record, mark ->eor, pass
> > > write_seq to the driver along with the key. Also pretty close to what
> > > tls_device_resync_tx does.  
> > 
> > That sounds like you'd expose the rekeying logic to the drivers?
> > New op, having to track seq#...  
> 
> Well, we have to call into the drivers to install the key, whether
> that's a new rekey op, or adding an update argument to ->tls_dev_add,
> or letting the driver guess that it's a rekey (or ignore that and just
> install the key if rekey vs initial key isn't a meaningful
> distinction).
> 
> We already feed drivers the seq# with ->tls_dev_add, so passing it for
> rekeys as well is not a big change.
> 
> Does that seem problematic? Adding a rekey op seemed more natural to
> me than simply using the existing _del + _add ops, but maybe we can
> get away with just using those two ops.

Theoretically a rekey op is nicer and cleaner. Practically the quality
of the driver implementations will vary wildly*, and it's a significant
time investment to review all of them. So for non-technical reasons my
intuition is that we'd deliver a better overall user experience if we
handled the rekey entirely in the core.

Wait for old key to no longer be needed, _del + _add, start using the
offload again.

* One vendor submitted a driver claiming support for TLS 1.3, when 
  TLS 1.3 offload was rejected by the core. So this is the level of
  testing and diligence we're working with :(