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).