[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <Y8le3yLjpEGSm5gH@hog>
Date: Thu, 19 Jan 2023 16:16:47 +0100
From: Sabrina Dubroca <sd@...asysnail.net>
To: Apoorv Kothari <apoorvko@...zon.com>
Cc: fkrenzel@...hat.com, kuniyu@...zon.com, netdev@...r.kernel.org
Subject: Re: [PATCH net-next 3/5] tls: implement rekey for TLS1.3
2023-01-18, 17:25:46 -0800, Apoorv Kothari wrote:
> > 2023-01-17, 15:16:33 -0800, Kuniyuki Iwashima wrote:
> > > Hi,
> > >
> > > Thanks for posting this series!
> > > We were working on the same feature.
> > > CC Apoorv from s2n team.
> >
> > Ah, cool. Does the behavior in those patches match what your
> > implementation?
>
> Thanks for submitting this, it looks great! We are working on testing this now.
>
> >
> > [...]
> > > > diff --git a/net/tls/tls_main.c b/net/tls/tls_main.c
> > > > index fb1da1780f50..9be82aecd13e 100644
> > > > --- a/net/tls/tls_main.c
> > > > +++ b/net/tls/tls_main.c
> > > > @@ -669,9 +669,12 @@ static int tls_getsockopt(struct sock *sk, int level, int optname,
> > > > static int do_tls_setsockopt_conf(struct sock *sk, sockptr_t optval,
> > > > unsigned int optlen, int tx)
> > > > {
> > > > + union tls_crypto_context tmp = {};
> > > > + struct tls_crypto_info *old_crypto_info = NULL;
> > > > struct tls_crypto_info *crypto_info;
> > > > struct tls_crypto_info *alt_crypto_info;
> > > > struct tls_context *ctx = tls_get_ctx(sk);
> > > > + bool update = false;
> > > > size_t optsize;
> > > > int rc = 0;
> > > > int conf;
> > > > @@ -687,9 +690,17 @@ static int do_tls_setsockopt_conf(struct sock *sk, sockptr_t optval,
> > > > alt_crypto_info = &ctx->crypto_send.info;
> > > > }
> > > >
> > > > - /* Currently we don't support set crypto info more than one time */
> > > > - if (TLS_CRYPTO_INFO_READY(crypto_info))
> > > > - return -EBUSY;
> > > > + if (TLS_CRYPTO_INFO_READY(crypto_info)) {
> > > > + /* Currently we only support setting crypto info more
> > > > + * than one time for TLS 1.3
> > > > + */
> > > > + if (crypto_info->version != TLS_1_3_VERSION)
> > > > + return -EBUSY;
> > > > +
> > >
> > > Should we check this ?
> > >
> > > if (!tx && !key_update_pending)
> > > return -EBUSY;
> > >
> > > Otherwise we can set a new RX key even if the other end has not sent
> > > KeyUpdateRequest.
> >
> > Maybe. My thinking was "let userspace shoot itself in the foot if it
> > wants".
>
> I feel avoiding foot-guns is probably the correct thing to do. The RFC also has
> a requirement that re-key(process messages with new key) should only happen after
> a KeyUpdate is received so it would be nice if the kTLS implemention can help
> enforce this.
>
> Based on the RFC https://www.rfc-editor.org/rfc/rfc8446#section-4.6.3:
> Additionally, both sides MUST enforce that a KeyUpdate
> with the old key is received before accepting any messages encrypted
> with the new key. Failure to do so may allow message truncation
> attacks.
Ok. I'll add that in v2, unless someone is strongly against it.
Thanks.
--
Sabrina
Powered by blists - more mailing lists