Skip to content
New issue

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

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

Already on GitHub? # to your account

SSLClient with W5500 not working (works well with WiFi and TinyGSM) #85

Closed
SunandMittal opened this issue Jul 7, 2024 · 5 comments · Fixed by #91
Closed

SSLClient with W5500 not working (works well with WiFi and TinyGSM) #85

SunandMittal opened this issue Jul 7, 2024 · 5 comments · Fixed by #91
Labels
help wanted Extra attention is needed question Further information is requested

Comments

@SunandMittal
Copy link

We are working on SSLclient library for TLS/SSL layer in Arduino/ESP32. It works very well over WiFi and TinyGSM for both MQTT and HTTP.

But it is failing when W5500 ethernet is used as base client. I am aware of increasing W5500 buffer size (by reducing maximum clients). We tried various combinations but none of them worked. The initial handshake of HTTP/MQTT fails. Same thing works without SSL layer.

Has anyone faced this issue earlier? Any workaround or pointer to look to.
It seems that some data-buffers are insufficient to complete SSL handshake calculations?

Best regards,
Sunand Mittal

@RobertByrnes
Copy link
Collaborator

RobertByrnes commented Jul 8, 2024

Hi @SunandMittal I have just add an issue template to this repo so we can get all the required information to help solve issues just like this one! Please use the template located here as the basis for adding a comment here. Once I have the extra information I will try to assist you. Cheers.

@RobertByrnes RobertByrnes added help wanted Extra attention is needed lacking information labels Jul 8, 2024
@SunandMittal
Copy link
Author

The problem is now resolved after adjusting return values in base Client of W5500. Closing this request.

@RobertByrnes
Copy link
Collaborator

Hey @SunandMittal Great news. Any chance you could post a short description of what you did and I'll to a READMe.md as a note. You are the second person to mention W5500.

@SunandMittal
Copy link
Author

Within perform_ssl_handshake of library, following workaround has been put:

W5500 Client return -1 when there is no data, while mdebtls_ssl_handshake expects MBEDTLS_ERR_SSL_WANT_READ/WRITE.
So, following line is added to accept -1 as ok condition, which a counter (to avoid infinite loop).
I could not find how other clients (tinyGsm or WiFi) return MBEDTLS_ERR_SSL_WANT_READ/WRITE..


  while ((ret = mbedtls_ssl_handshake(&ssl_client->ssl_ctx)) != 0) {
    loopCnt++;
    if (ret != MBEDTLS_ERR_SSL_WANT_READ && ret != MBEDTLS_ERR_SSL_WANT_WRITE) {

     if  ( (loopCnt<200) && (ret == -1))
     {
      // this is not error
     }
     else
     {
       break;
     }
    }

Similar change is done in data read and send function.

Basic functionality worked with this for now. Doing more trials/testing.

@RobertByrnes RobertByrnes reopened this Jul 10, 2024
@RobertByrnes
Copy link
Collaborator

RobertByrnes commented Jul 10, 2024

Hey @SunandMittal Have re-opened this as they may be a case for a permanent workaround here. Just leaving some notes:

Implication of the Workaround
The OP added a workaround in the perform_ssl_handshake function to address the W5500 client's behavior of returning -1 when there is no data. The change involves treating -1 as a non-error condition under certain circumstances, specifically if loopCnt is less than 200. This adjustment is necessary because the mbedtls_ssl_handshake function expects MBEDTLS_ERR_SSL_WANT_READ or MBEDTLS_ERR_SSL_WANT_WRITE to indicate it should be called again.

What are the integer equivalents of MBEDTLS_ERR_SSL_WANT_READ and MBEDTLS_ERR_SSL_WANT_WRITE?

The integer equivalents of the mbedtls error codes for SSL operations are as follows:
MBEDTLS_ERR_SSL_WANT_READ: -0x6900
MBEDTLS_ERR_SSL_WANT_WRITE: -0x6880

The human-readable integer equivalents for the mbedtls error codes are:
MBEDTLS_ERR_SSL_WANT_READ: -26880
MBEDTLS_ERR_SSL_WANT_WRITE: -26752

Detailed Implication

  • Loop Handling: The loop continues up to 200 iterations when ret is -1, avoiding an immediate break from the handshake process.
  • Non-Blocking Condition: The workaround ensures that -1, which indicates no data from the W5500 client, doesn't prematurely terminate the handshake.
  • Consistency with Other Clients: Unlike W5500, other clients like TinyGSM or WiFi return MBEDTLS_ERR_SSL_WANT_READ or MBEDTLS_ERR_SSL_WANT_WRITE, which are correctly handled by mbedtls_ssl_handshake.

Conclusion
This workaround bridges the gap between the expected behavior of mbedtls_ssl_handshake and the actual behavior of the W5500 client, ensuring a smoother SSL handshake process by accommodating the -1 return value. This method effectively prevents infinite loops and handles the unique characteristics of the W5500 Ethernet module.

Possible code change

while ((ret = mbedtls_ssl_handshake(&ssl_client->ssl_ctx)) != 0) {
    loopCnt++;
#if defined(_W5500_H_) || defined(W5500_WORKAROUND)
    if (ret == -1 && loopCnt < 200) {
        continue; // Treat -1 as a non-error for up to 200 iterations
    }
#endif
    if (ret != MBEDTLS_ERR_SSL_WANT_READ && ret != MBEDTLS_ERR_SSL_WANT_WRITE) {
        break; // Break on any other error
    }
}

Possible README.md update

W5500 Workaround for SSL Handshake

When using the W5500 Ethernet module with SSLClient, the perform_ssl_handshake function may fail due to the W5500 client returning -1 when there is no data. To address this, apply the following workaround:

Depending on your include order, the workaround may be automatically applied. If not, add the following line before the SSLClient include:

#define W5500_WORKAROUND
#include <SSLClient.h>

@RobertByrnes RobertByrnes added question Further information is requested and removed lacking information labels Jul 17, 2024
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
help wanted Extra attention is needed question Further information is requested
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants