Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Comply with the received Record Size Limit extension #7010

Closed
5 tasks
yanesca opened this issue Feb 1, 2023 · 3 comments · Fixed by #7455
Closed
5 tasks

Comply with the received Record Size Limit extension #7010

yanesca opened this issue Feb 1, 2023 · 3 comments · Fixed by #7455
Labels
component-tls13 enhancement size-s Estimated task size: small (~2d)

Comments

@yanesca
Copy link
Contributor

yanesca commented Feb 1, 2023

Prerequisite: #7007

Make TLS 1.3 client and server in Mbed TLS respect received Record Size Limit extension. This requires adding a new member to the session structure and storing the requested output record limit there. The record layer uses mbedtls_ssl_get_max_out_record_payload() to determine the outgoing record limit. This will need to be updated to calculate payload that will result in the requested record limit.

The fragmentation support in Mbed TLS is not complete (see #1840 and #3817). Completing fragmentation support is out of scope for this task. The goal is to make available fragmentation support use the Record Size Limit extension.

Since Mbed TLS doesn't fragment handshake messages in TLS, we need to make sure that the certificate message fits in the limit when testing.

(No need to consider MTU here as we don't have DTLS 1.3 yet.)

The task is done when

  • A new member is added to mbedtls_ssl_session and the outgoing (the one requested by the peer) record size limit is stored there
  • Affected options (like MBEDTLS_SSL_SESSION_TICKETS or MBEDTLS_SSL_CONTEXT_SERIALIZATION) work with the new member in the struct
  • Outgoing application data is complying with received record size limit extension (as mandated by RFC8449) in both TLS 1.3 clients and servers
  • The changes are tested against GnuTLS in ssl-opt.sh
  • All of the new code and tests are conditional on the MBEDTLS_SSL_RECORD_SIZE_LIMIT option
@yanesca yanesca added enhancement component-tls13 size-s Estimated task size: small (~2d) labels Feb 1, 2023
@yanesca yanesca changed the title Add passive support for Record Size Limit extension Comply with the received Record Size Limit extension Feb 1, 2023
@yanesca
Copy link
Contributor Author

yanesca commented Feb 1, 2023

RFC 8449 says:

When the "record_size_limit" extension is negotiated, an endpoint
MUST NOT generate a protected record with plaintext that is larger
than the RecordSizeLimit value it receives from its peer.

Not complying with this from the peer side has the same effect as not being aware of this extension. The peer needs to be prepared for this and to break the connection when this happens. Catching this and breaking connection on the sender side when we can't comply (TLS handshake messages) doesn't make the situation any better (it is just more complicated, takes longer to implement and maintain).

In summary, we shouldn't try to comply with this part until #1840 is resolved.

@KloolK
Copy link
Contributor

KloolK commented Feb 24, 2023

@yanesca The #3818 you linked above seems unrelated to this. Did you intend to link some other issue?

@yanesca
Copy link
Contributor Author

yanesca commented Feb 24, 2023

Oh, my bad, that is meant to be #3817.

KloolK added a commit to KloolK/mbedtls that referenced this issue Apr 18, 2023
KloolK added a commit to KloolK/mbedtls that referenced this issue Apr 18, 2023
KloolK added a commit to KloolK/mbedtls that referenced this issue Apr 19, 2023
KloolK added a commit to KloolK/mbedtls that referenced this issue Apr 19, 2023
KloolK added a commit to KloolK/mbedtls that referenced this issue Apr 20, 2023
KloolK added a commit to KloolK/mbedtls that referenced this issue Apr 20, 2023
yanrayw pushed a commit to yanrayw/mbedtls that referenced this issue Nov 14, 2023
yanrayw pushed a commit to yanrayw/mbedtls that referenced this issue Nov 14, 2023
yanrayw pushed a commit to yanrayw/mbedtls that referenced this issue Dec 7, 2023
@minosgalanakis minosgalanakis moved this to [3.6] TLS 1.3 Record Size Limit in Past EPICs Apr 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component-tls13 enhancement size-s Estimated task size: small (~2d)
Projects
Status: [3.6] TLS 1.3 Record Size Limit
Development

Successfully merging a pull request may close this issue.

2 participants