Implements: #27827 - RFC 8449 Record Size Limit for TLS #27916
Conversation
…o-server parsing function.
… when set. added tracing of the extension in SSL_trace.
… construction and parse function are not finished.
…or setting the fragment size...
…are still fragmented.
…to feat/rfc-8449/record_size_limit_ext
…rieve the negotiated record_size_limit.
|
@mattcaswell and @t8m, could you take an early look and give me your feedback? Many thanks. |
… not applied if one of the endpoints does not negotiate.
|
Have you seen #18248? |
No, thank you. When i wrote the issue I did not bother to check thoroughly since nobody mentioned this PR, this was a mistake. The codebase has changed quite a bit since ! This is still a valuable source of information for tests. I also did not write any automated tests and I have not implemented the session resumption mechanism. |
| && !SSL_CTX_set_tlsext_record_size_limit(ctx, | ||
| record_size_limit)) { | ||
| goto end; | ||
| } |
There was a problem hiding this comment.
style: no need accolades around one line of code
| goto end; | ||
| } | ||
|
|
||
| if (record_size_limit_set && maxfraglen > 0) { |
There was a problem hiding this comment.
style : one line -> no accolades
| @@ -118,16 +121,16 @@ typedef struct extensions_definition_st { | |||
| * Extensions should be added to test/ext_internal_test.c as well, as that | |||
| * tests the ordering of the extensions. | |||
| * | |||
| * Each extension has an initialiser, a client and | |||
| * server side parser and a finaliser. The initialiser is called (if the | |||
| * Each extension has an initializer, a client and | |||
There was a problem hiding this comment.
your changes are large enough, no need to add this US/GB nits
| int sent) { | ||
| unsigned int proto_record_hard_limit; | ||
|
|
||
| if (s->options & SSL_OP_NO_RECORD_SIZE_LIMIT_EXT) |
There was a problem hiding this comment.
if ((bitfield & MASK) != 0)
| * same value. | ||
| */ | ||
|
|
||
| if (s->options & SSL_OP_NO_RECORD_SIZE_LIMIT_EXT) { |
| @@ -231,7 +269,7 @@ int tls_parse_ctos_srp(SSL_CONNECTION *s, PACKET *pkt, unsigned int context, | |||
| if (!PACKET_strndup(&srp_I, &s->srp_ctx.login)) { | |||
| SSLfatal(s, SSL_AD_INTERNAL_ERROR, ERR_R_INTERNAL_ERROR); | |||
| return 0; | |||
| } | |||
| } | |||
| unsigned int context, X509 *x, | ||
| size_t chainidx) | ||
| { | ||
| if (s->options & SSL_OP_NO_RECORD_SIZE_LIMIT_EXT) |
| || !WPACKET_close(pkt)) { | ||
| SSLfatal(s, SSL_AD_INTERNAL_ERROR, ERR_R_INTERNAL_ERROR); | ||
| return EXT_RETURN_FAIL; | ||
| } |
Hi, this pull request implements the RFC 8449 Record Size Limit extension for TLS. This address the issue here.
This is still a work in progress, but this being my first "major" contribution to the project, I need some early reviews of some more experienced developers in the code base.
This extension provides major differences from the MFL (Maximum Fragment Length) extension already supported by OpenSSL :
RSL (Record Size Limit) does not apply to unprotected messages. That is, all handshake messages in earlier version of TLS ( <= 1.2) does not get fragmented over multiple records. The rationale here is that an implementation can already process an unprotected message at the byte granularity if they want to. This is not the case with protected messages because the entire message must be buffered before decryption and authentication.
The RSL can be asymmetric between a client and a server ; a client can advertise a Record Size Limit of 1024 bytes while a server can be more constraint and only accept 512 bytes. This was impossible with the MFL extension since the RFC mandates that the server have to echo back the MFL sent by the client.
The RSL can take arbitrary value between 64 and the negotiated protocol hard limit (2^14 for <= TLS 1.2 and 2^14 + 1 for TLS 1.3), or a future TLS extension that allow bigger record sizes. This is very different from the hard-coded power of two that the MFL extension uses.
This is the current list of task remaining as of 27/06/2025: