Properly support smaller fragment / record size

[rojer]

Suggested enhancement

After implementing incoming message defragmentation, effort should be made to properly support smaller record sizes by implementing support for max_fragment_length (RFC6066) as well as record_size_limit (RFC8449) extensions.

Justification

Mbed TLS needs this because it is popular on embedded platforms for which allocating 16K buffers is not always possible.

[rojer]

For a long time i have maintained a custom set of patches implementing defragmentation, fragmentation and fixing some issues around max fragment length extension (for mongoose-os and now internally for Shelly) - submitted some time ago in #8981.
Arguably the biggest part of that - incominghandshake message defragmentation - has already been merged, what;s left are smaller bits, but before resbmitting I'd like to get an agreement on the approach and potential API changes, since we now have two extensions .

• Mbed TLS already has a setting for max fragment size - mbedtls_ssl_conf_max_frag_len that the documentation acnowledges doesn't work properly on the client side but actually doesn't work on the server size either, since afetr successful negotiation the clients expect us to fragment the messages and error out if we don't.

• Then there is mbedtls_ssl_set_mtu that is kind of the same but not really.

• We have the new record_size_limit extension that is not in the picture at all but should be, somehow.

As I mentioned, we have been using max_fragment_length extension patches successfully with TLS 1.2 in production for a long time now. Support of it exists and is relatively widespread (notably OpenSSL supports it since 1.1.0) with equally notable exceptions (Google, for example).

My quick survey a couple months ago showed that nobody supports record_size_limit on the server yet, but some servers support max_fragment_length for 1.3.

Based on the above, my suggestion is to:

• Include record_size_limit and always include it with both 1.2 and 1.3 client hello message if set explicitly via mbedtls_ssl_set_mtu() or the size of the input buffer is less than 16K.
• If MTU or buffer size happen to match one of the allowed values for max_fragment_length, include that too (so that values for both extensions are always consistent).
• Deprecate mbedtls_ssl_conf_max_frag_len() as it's just another way of setting maximum desired input message size.

Tagging @mpg for discussion.

[rojer]

@minosgalanakis i will provide the wanted help but i'd like to agree on the approach first so as not to waste effort

[minosgalanakis]

@rojer

First off, thank you for your contribution and for actively engaging with this—it's much appreciated.

As you may have noticed, we’ve been going through a lengthy process of splitting the Mbed TLS and TF-PSA Crypto functionality into separate repositories. The upcoming release will be heavily focused on the remaining work related to that split.

The good news is that this presents an opportunity to revise the public-facing API in the next release, assuming we can agree on a design.

The less good news is that we likely won’t have the bandwidth to drive those changes ourselves. That said, we’d be happy to accept small, well-scoped patches—especially those that come with accompanying tests. In fact, during the recent set of PRs (#9989, #10007, #10021, #10045), we extended our test suite to better support the kind of defragmentation work you’re contributing. Thanks to that, it should now be easier to test similar functionality in new PRs.

From what I understand, you're aiming to extend functionality on the TLS and X.509 side of the code, which resides in the Mbed TLS repository under development?

My suggestion would be to focus on TLS 1.3 first, starting with changes that are tightly scoped and low-impact and moving toward more involved updates as needed. We can then revisit TLS 1.2 support if there’s a need.

Out of curiosity, which configuration or use case is most important for your work?

[rojer]

thanks @minosgalanakis

yes, point about testing is taken, i will make sure our future changes are accompanied by tests.

From what I understand, you're aiming to extend functionality on the TLS and X.509 side of the code, which resides in the Mbed TLS repository under development?

Yes, in particular extending support for smaller TLS record size - negotiation via extensions, and fragmentation on the way out as required (reassembly has already been dealt with).

My suggestion would be to focus on TLS 1.3 first, starting with changes that are tightly scoped and low-impact and moving toward more involved updates as needed. We can then revisit TLS 1.2 support if there’s a need.

ack. though as with reassembly, this is layer is common to both, only extension sets are potentially different (but my argument is that they too should be the same).

Out of curiosity, which configuration or use case is most important for your work?

we operate on embedded devices with very small amount of usable memory, support for smaller fragment length is vital to be able to maintain more than one concurrent TLS connection (1/2 persistent cloud connections and transient HTTPS fetches). buffer bloat during OTA updates with server sending a stream of 16K records used to be a big problem for us, until we implemented max_fragment_length on the client (mbedtls) and server (openssl).

so, as step 1, and before i send my patches, i would like to discuss the potential public API change: should mbedtls_ssl_set_mtu() become the only way of setting the desired maximum record size for the connection, both TLS and DTLS? for DTLS it will continue to work as before, for TLS, if set to < 16K, it will produce ClientHello extensions trying to negotiate the desired record_size_limit and max_fragment_length, if applicable.