Re: [PATCH net-next v6 1/2] net/tls: support setting the maximum payload size

[Wilfred Mallawa <wilfred.opensource@gmail.com>]

On Thu, 2025-10-16 at 00:43 +0200, Sabrina Dubroca wrote:
> 2025-10-15, 11:52:43 +1000, Wilfred Mallawa wrote:
> > diff --git a/Documentation/networking/tls.rst
> > b/Documentation/networking/tls.rst
> > index 36cc7afc2527..dabab17ab84a 100644
> > --- a/Documentation/networking/tls.rst
> > +++ b/Documentation/networking/tls.rst
> > @@ -280,6 +280,17 @@ If the record decrypted turns out to had been
> > padded or is not a data
> >  record it will be decrypted again into a kernel buffer without
> > zero copy.
> >  Such events are counted in the ``TlsDecryptRetry`` statistic.
> >  
> > +TLS_TX_MAX_PAYLOAD_LEN
> > +~~~~~~~~~~~~~~~~~~~~~~
> > +
> > +Sets the maximum size for the plaintext of a protected record.
> > +
> > +When this option is set, the kernel enforces this limit on all
> > transmitted TLS
> > +records, ensuring no plaintext fragment exceeds the specified
> > size. This can be
> > +used to specify the TLS Record Size Limit [1].
> 
> Since this is now "max payload" instead of directly the record size,
> we should probably add something to describe how to derive the value
> to pass to TLS_TX_MAX_PAYLOAD_LEN from the record size limit:
> 
>     For TLS1.2, the record size limit can be used directly.
>     For TLS1.3, limit-1 should be passed, as the record size limit
>     includes 1B for the ContentType.
> 
> 
Good idea, I will add this on.
> And possibly mention that TLS1.3 record padding is currently
> unsupported, so whether it should be counted in the value passed via
> this setsockopt or not is undecided. (I'm not sure we need to go that
> far. Jakub, WDYT?)
> 
> 
> [...]
> > +static int do_tls_setsockopt_tx_payload_len(struct sock *sk,
> > sockptr_t optval,
> > +					    unsigned int optlen)
> > +{
> > +	struct tls_context *ctx = tls_get_ctx(sk);
> > +	struct tls_sw_context_tx *sw_ctx = tls_sw_ctx_tx(ctx);
> > +	u16 value;
> > +
> > +	if (sw_ctx && sw_ctx->open_rec)
> > +		return -EBUSY;
> > +
> > +	if (sockptr_is_null(optval) || optlen != sizeof(value))
> > +		return -EINVAL;
> > +
> > +	if (copy_from_sockptr(&value, optval, sizeof(value)))
> > +		return -EFAULT;
> > +
> > +	if (value < TLS_MIN_RECORD_SIZE_LIM || value >
> > TLS_MAX_PAYLOAD_SIZE)
> 
> For 1.3, should we allow TLS_MIN_RECORD_SIZE_LIM-1? The smallest
> valid
> record size limit (according to rfc8449) is 64
> (TLS_MIN_RECORD_SIZE_LIM), so after userspace subtracts 1 we would
> get
> TLS_MIN_RECORD_SIZE_LIM-1?
> 
> (but this would bring back one "are we 1.2 or 1.3?" check :/)
Yeah I don't think there's a way around this...? I will update the
description to specify these details and add the limit checks. I do
think the payload size approach makes more sense, since, it could be
used for reasons other than just `record_size_limit`.

Regards,
Wilfred
> 
> > +		return -EINVAL;
> > +
> > +	ctx->tx_max_payload_len = value;
> > +
> > +	return 0;
> > +}