On Tue, 10 Dec 2024 17:16:09 +0100 Sabrina Dubroca wrote:
> > The only reason the check_pending_rekey() can fail is if the message is
> > mis-formatted, I wonder if we are better off ignoring mis-formatted
> > rekeys? User space will see them and break the connection, anyway.
> > Alternatively - we could add a selftest for this.
>
> Going back to tls_check_pending_rekey():
>
> > > + if (rxm->full_len < 1)
> > > + return -EINVAL;
>
> There's no real reason to fail here, we should probably just ignore
> it. It's not a rekey, and it's not a valid handshake message, but one
> could say that's not the kernel's problem. I'll make that return 0
> unless you want to keep -EINVAL.
returning 0 SGTM
> Hard to write a selftest for because we'd have to do a sendmsg with
> len=0, or do the crypto in the selftest.
>
> > > + err = skb_copy_bits(skb, rxm->offset, &hs_type, 1);
> > > + if (err < 0)
> > > + return err;
>
> This probably means that the skb we got from the parser was broken. If
> we can't read 1B with full_len >= 1, something's wrong. Maybe worth a
> DEBUG_NET_WARN_ON_ONCE?
Also SG!
> > > + if (hs_type == TLS_HANDSHAKE_KEYUPDATE) {
>
> Here I don't actually check if it's a correct KeyUpdate message [1],
> we pause decryption and let userspace decide what to do (probably
> break the connection as you said).
