ktls: Disallow transmitting empty frames outside of TLS 1.0/CBC mode

[markj]

Summary

There was nothing preventing one from sending an empty fragment on an
arbitrary KTLS TX-enabled socket, but ktls_frame() asserted that this
could not happen. Though the transmit path handles this case, we should
be strict and allow empty fragments only in modes where it is explicitly
allowed.

Modify ktls_frame() to signal an error if an empty mbuf is encountered
when building TLS frames, unless the session explicitly permits it.
Extend the KTLS regression tests to exercise this check.

[jhb]

Tests look good, just one suggestion to consider.

sys/kern/uipc_socket.c

1654

We could check here by doing something like:

if (resid == 0 && tls->params.cipher_algorithm != CRYPTO_AES_CBC)
    return (EINVAL);

1749

My only thought is if we'd like to keep the assertions in ktls_frame and instead check for empty writes in this function. The sendfile case should never fail and keeping the assertion in ktls_frame would avoid doing the checks in non-debug kernels for sendfile.

[markj]

sys/kern/uipc_socket.c

1654

That's not quite as strict as what I did: sosend takes a UIO or an mbuf chain. If an mbuf chain is passed, ktls_frame() asserts that each mbuf in the chain has length > 0, whereas checking resid > 0 only verifies that the entire chain has length > 0. I guess that's fine though since the system call path always passes a UIO...

[gallatin]

sys/kern/uipc_socket.c

1749

I think that makes sense.

[markj]

Apply John's suggestion.

[markj]

Avoid leaking resources in tests.

[jhb]

sys/kern/uipc_socket.c

1618

Note that we compute resid here even in the case that uio is NULL, so it should handle sosend calls that only pass in an mbuf chain as well.

[markj]

sys/kern/uipc_socket.c

1618

But resid > 0 implies only that the whole mbuf chain has length > 0, not that each mbuf in the chain has length > 0, and ktls_frame() asserts the latter. So it seems possible in principle for kernel consumers to violate the assertion, but that should be ok.

[markj]

It occurs to me that this version of the diff will cause zero-byte sends with a control message to be rejected. TCP doesn't handle control messages, and TLS_SET_RECORD_TYPE is consumed in sosend_generic(), so I guess that's ok...