https connection processes locks up in infinite loop
schnoberts1 opened this issue · 2 comments
I was debugging SSL connectivity issues using openssl s_server and connecting my device to it using the BearSSLClient. I went through this process:
- Start openssl s_server.
- Start the device which connected fine.
- Restart the device and the connection process doesn't terminate (in the minutes I was prepared to wait anyway).
I started the server thus:
openssl s_server -port 8883 -key <key> -cert <cert> -verify_depth 1 -CAfile <anchors> --security_debug_verbose -msg -state -debug -trace -tls1_2 -verify_return_error -Verify 1 -rev
On investigation what seems to happen is that without specifying something like -rev
to s_server it accepts the first connection and does the handshake and on the second it accepts the connection and does not do the handshake. When it doesn't do the handshake the device is stuck in an infinite loop trying to read. It's because of this line: https://github.com/arduino-libraries/ArduinoIoTCloud/blame/6ef7ad86afee69987af8ef57fbad5c1e2224edde/src/tls/BearSSLClient.cpp#L350
I believe this code violates the requirement here: https://github.com/esp8266/Arduino/blob/c2f136515a396be1101b261fe7b71e137aef0dce/tools/sdk/include/bearssl/bearssl_ssl.h#L3947, particularly because there's no timeout logic to return -1.
I fixed this in my fork of the repo by returning -1 and everything now works fine. Was this translation of a read error to "read 0 bytes" deliberate?
Why care? Well, getting a connection and then not receiving any data is not an unusual failure mode on a server or a server with a faulty proxy in front of it so it should be fixed.
I'm also puzzled by why you guys aren't using Arduino_BearSSL as a git submodule in this repo rather than effectively forking and having to fix issues in two places. This seems like a common pattern in these repos, is this a policy?
... actually this wasn't the fix. It fixed the error handling but broke normal MQTT connectivity.
@schnoberts1 thanks for the heads up, i will try to get a look at it.
Regarding your question
I'm also puzzled by why you guys aren't using Arduino_BearSSL as a git submodule in this repo rather than effectively forking and having to fix issues in two places. This seems like a common pattern in these repos, is this a policy?
This is not a policy and we are removing the BearSSL copy here and moving to Arduino_BearSSL #465