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

TLS 1.3 SSL_ERROR_RX_RECORD_TOO_LONG error #8325

Closed
Aethedor opened this issue Oct 7, 2023 · 9 comments
Closed

TLS 1.3 SSL_ERROR_RX_RECORD_TOO_LONG error #8325

Aethedor opened this issue Oct 7, 2023 · 9 comments
Labels
bug component-tls13 size-s Estimated task size: small (~2d)

Comments

@Aethedor
Copy link

Aethedor commented Oct 7, 2023

Summary

I'm using mbed TLS in the Hiawatha webserver. I got TLSv1.3 almost working, but I needed to make in change in mbed TLS to make it actually work.

System information

Mbed TLS version (number or commit id): 3.5.0
Operating system and version: Ubuntu 22.04.3 LTS
Configuration (if not default, please attach mbedtls_config.h): in the default config, I enabled the following:
MBEDTLS_THREADING_PTHREAD
MBEDTLS_THREADING_C
MBEDTLS_SSL_PROTO_TLS1_3
MBEDTLS_SSL_TLS1_3_COMPATIBILITY_MODE
I disabled the following:
MBEDTLS_ECP_DP_SECP192R1_ENABLED
MBEDTLS_ECP_DP_SECP192K1_ENABLED
Compiler and options (if you used a pre-built binary, please indicate how you obtained it):
Additional environment information:

Expected behavior

That it works...

Actual behavior

Firefox reports a SSL_ERROR_RX_RECORD_TOO_LONG error for certain files.

Steps to reproduce

Request https://www.cauldron-vtt.net:4443/images/cauldron.png
Without the port 4443 it works fine. That's via mbed TLS 3.4.0, without TLSv1.3 support.

Additional information

The patch to make it work. With this patch, I can browse my websites normally via TLSv1.3. However, SSLLabs.com reports an `unexpected error' for my server.

--- /library/ssl_msg.c  2023-10-07 14:39:57.987576738 +0200
+++ /library/ssl_msg.c  2023-10-07 14:40:24.611726450 +0200
@@ -5893,7 +5893,7 @@
         return ret;
     }

-    if (len > max_len) {
+    if (len >= max_len) {
 #if defined(MBEDTLS_SSL_PROTO_DTLS)
         if (ssl->conf->transport == MBEDTLS_SSL_TRANSPORT_DATAGRAM) {
             MBEDTLS_SSL_DEBUG_MSG(1, ("fragment larger than the (negotiated) "
@@ -5903,7 +5903,7 @@
             return MBEDTLS_ERR_SSL_BAD_INPUT_DATA;
         } else
 #endif
-        len = max_len;
+        len = max_len - 1;
     }

     if (ssl->out_left != 0) {

I know this patch is probably not the right way to fix it, but I hope it gives you a pointer to the real fix. The credits for this all goes to Kun-Chi Lu [email protected], who reported it to me.

@Aethedor Aethedor changed the title Got TLSv1.3 is almost working... Got TLSv1.3 almost working... Oct 7, 2023
@gilles-peskine-arm
Copy link
Contributor

MBEDTLS_THREADING_PTHREAD
MBEDTLS_THREADING_C
MBEDTLS_SSL_PROTO_TLS1_3

Please note that multithreading and TLS 1.3 are not compatible unless you only use the TLS 1.3 stack from a single thread, because the PSA API implementation is not thread-safe. We plan to fix this in Mbed TLS 3.6.0.

@gilles-peskine-arm gilles-peskine-arm added bug component-tls13 size-s Estimated task size: small (~2d) labels Oct 9, 2023
@ronald-cron-arm
Copy link
Contributor

Not completely sure but at first sight it may be related to #7918.

@yuhaoth
Copy link
Contributor

yuhaoth commented Oct 12, 2023

It might be relative record size limit extension

@yuhaoth
Copy link
Contributor

yuhaoth commented Nov 29, 2023

I could not reproduce this issue in my local.

FireFox send record_size_limit extension with record_size_limit=16385.
Our default record size is 16384(MBEDTLS_SSL_OUT_CONTENT_LEN) , it is lower than firefox. I am not sure what happens.

@irwir
Copy link
Contributor

irwir commented Dec 7, 2023

A minimal html page with a picture should reproduce the issue, provided that image file is bigger than the limit - as could be seen with the cauldron.png link above.
The provided patch resolves the issue, though I did not investigate what part of the current code fails.

@daverodgman daverodgman changed the title Got TLSv1.3 almost working... TLS 1.3 SSL_ERROR_RX_RECORD_TOO_LONG error Jan 23, 2024
@waleed-elmelegy-arm
Copy link
Contributor

Hi @Aethedor , There was a problem with record size that has been fixed in the latest development, I can test with MbedTLS server with the maximum response size (by default 16384) and it's working now (before it wasn't) but I can't really know for sure if this solves your problem as well, can you try with the latest development branch and redeploy so I can check? Thanks.

@irwir
Copy link
Contributor

irwir commented Jan 23, 2024

In my case the current development branch resolved the error with 25 KB image.

@tom-cosgrove-arm
Copy link
Contributor

@irwir Thanks for confirming it's fixed. @klook and @waleed-elmelegy-arm thanks for your work on Record Size Limit that fixed this. Closing this issue as fixed.

@Aethedor
Copy link
Author

With the current development release, the issue is no longer there.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug component-tls13 size-s Estimated task size: small (~2d)
Projects
Status: [3.6] TLS 1.3 misc for LTS
Development

No branches or pull requests

7 participants