-
Notifications
You must be signed in to change notification settings - Fork 7.6k
Crash/Panic after SSL Handshake fails #211
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
Comments
I did verify that I believe the best way to handle this might be to (close the socket and) abort if That said - I am not sure if a lot of the other |
oh, it's a really odd edge-case hahaha
It's a good solution. But returning -1 or the MbedTLS error, because higher level code just interprets as error if (ret < 0). I want to release some fixes and heap optimizations for this lib soon and include your fix to this too. |
But the Adruino example does:
So this would interpret "0" as an error, and nonzero as valid. This works if zero is not a valid "socket". I was going to come up with a patch, but there seem to be some inconsistencies in how errors are returned. Some places return 0 for error, others return -1. The published client example codes (above) assumes zero is an error. I don't want to break any "existing" applications - but I'm inclined to think a "clean break" at this point might be good. If you think |
But it's called first here: https://github.com/espressif/arduino-esp32/blob/master/libraries/WiFiClientSecure/src/WiFiClientSecure.cpp#L96 just after that it returns to arduino. |
It's only in one of the 4 cases of "connect". In the first two, no explicit "return" is given (will c++ return the last value on the stack by default)? Even in the one you site, it always returns "1" - even on error. In the last case (below that) it returns "0" on name lookup failure, otherwise the value from the "third" case (one you sited) - which always returns "1". Then there's Seems like all the |
@bkgoodman Could you please check if it is fixed on https://github.com/copercini/arduino-esp32/tree/master/libraries/WiFiClientSecure/src ? |
Getting warmer :-) ... It runs a few iterations and then died - but this time, before it did - it got:
(The following panic was in the same .queue.c location, trying to read after the connection failed to come up, etc. I still seem to not be getting the return code that the connect failed). So clearly a case of something not getting freed correctly, even in the happy case. I am just sitting in a loop doing (comments, printfs and cruft removed for legibility):
|
just for check, please put If it works, I know the problem hahahaha |
where do you see setup and loop? |
void testClient(){
WiFiClientSecure client;
if(!client.connect(server, 443, test_ca_cert, test_client_cert, test_client_key)){
return;//connect fail
}
client.printf("GET https://host.domain.com/cgi-bin/time.cgi?i1=%d&i2=%di3=%d&i4=%d&reason=0x%x HTTP/1.0\r\n",
digitalRead(36),digitalRead(37),0,
2,REG(RTC_CNTL_RESET_STATE_REG)
);
client.print("Host: host.domain.com\r\n");
client.print("Connection: close\r\n\r\n");
while (!client.available()){ }
while (client.available()) {
char c = client.read();
Serial.write(c);
}
} |
That worked for several iterations. - then I got the same error in queue.c:
Which appear to come from one of the client->printfs which come after the client->connect. (HTTP request, GET, etc). Another time (after a reset and may loops) I did see:
...and one other time I saw...
|
you are running the latest esp32-arduino right? |
Not sure what you mean about the newlines. I have it in the first (and subsequent) printfs. Like I said - it works the first several times through the loop. My Apache log sees the request properly - no issues. As for why I new/delete all the time - the idea is that this is going to sit in a 7/24 lights-out facility. It needs to survive WiFi outages - so the idea being that I should be completely tearing down and restarting the network connection every time through. What I'm REALLY trying to do is to get a good watchdog to come in and do a hard reset intermittently, but that is a whole other issue... I am running the very latest:
(With the exception of the WifiSecureClient/src files you wanted me to try in the comment above) I just did a pull and while I haven't seen the queue.c error yet - I still see the UNKNOWN ERROR CODE 04E MbedTLS error: -78 (in the printf)? I will keep trying, though... |
in the first printf in your code there isn't a new line at the end! so you are sending actually |
there could be one more reason. Withe every client.print you are sending a new packet. Which might be an issue with TLS. Try sending this instead of all prints: client.printf("GET /cgi-bin/time.cgi?i1=%d&i2=%di3=%d&i4=%d&reason=0x%x HTTP/1.0\r\nHost: %s\r\nConnection: close\r\n\r\n",
digitalRead(36),digitalRead(37),0,
2,REG(RTC_CNTL_RESET_STATE_REG),
"host.domain.com"
); |
You are running out of heap, so these strange errors appears... try call Every time calling
it start a new ssl connection and you are not ending the previous one, running out of heap. |
@copercini what are you talking about? at the end of the method the client destruct is called. If you are not closing the connection, the server will (Connection: close) and it's a bug. No need for the large fonts please. I have very good vision ;) If you are leaking memory, that is your implementation at fault. the code above is valid. |
@me-no-dev sorry about big font, It was not intentional just my poor github formatting code skills @bkgoodman are you getting a message like: "SSL - The peer notified us that the connection is going to be closed" after the request? |
@copercini - no message like that. In fact, the URL goes to a simple CGI script that dumps the environs - which the ESP then prints - all that is coming back fine. This code (which prints the received page to serial) (and the odd GET line with the host name and newline issues) were just all taken verbatim from the WiFiClientSecure Arduino example code. |
I did take my entire request - and send it in as a single printf - adding in the newlines, etc. After a few iterations, I got:
(To answer your prior question) I"m using WiFiClientSecure because this device is going into an off-site place (a pump-house - monitoring equipment) - and will be using third-party/public wifi to communicate back to the monitoring system. (i.e. it's not "local", or a local network). |
* Add CA certificate in example SHA1 fingerprint is broken now: more info: https://shattered.io * Best error handling When occur an error in WiFiClientSecure library just return the error message and clean the context avoiding crash - fix for #211 Translate MbedTLS error codes in messages for best understanding * Declarate certificates as const mbedtls_pk_parse_key needs a const unsigned char * certificate. In old implementation the certificate was declarated as char * so first it converts to unsigned and after to const. When we convert signed to unsigned it may result in a +1 larger output. Fix issue #223
When WiFiClientSecure receive a malformed or incomplete response it's don't close the connection, using about 45k of heap each connection. After timeout, just the sockets is closed, but not the mbedtls stuff, causing a big memory leak. So after some bad iterations ESP will out of heap. |
Solved! |
* Add CA certificate in example SHA1 fingerprint is broken now: more info: https://shattered.io * Best error handling When occur an error in WiFiClientSecure library just return the error message and clean the context avoiding crash - fix for espressif/arduino-esp32#211 Translate MbedTLS error codes in messages for best understanding * Declarate certificates as const mbedtls_pk_parse_key needs a const unsigned char * certificate. In old implementation the certificate was declarated as char * so first it converts to unsigned and after to const. When we convert signed to unsigned it may result in a +1 larger output. Fix issue espressif/arduino-esp32#223
I am running with a continuous connection sending data to Google Firestore every five minutes. I am getting this same error. You said this was solved (5 years ago). Mine appears to be running out of heap which looks like same description. [E][ssl_client.cpp:33] handle_error(): SSL - The connection indicated an EOF What is the specific solution? |
Slightly off topic, but would be helpful to me;
Where can I find this REG macro? Doesn't seem to be imported by default. What include do I need? Cheers |
* Add CA certificate in example SHA1 fingerprint is broken now: more info: https://shattered.io * Best error handling When occur an error in WiFiClientSecure library just return the error message and clean the context avoiding crash - fix for espressif/arduino-esp32#211 Translate MbedTLS error codes in messages for best understanding * Declarate certificates as const mbedtls_pk_parse_key needs a const unsigned char * certificate. In old implementation the certificate was declarated as char * so first it converts to unsigned and after to const. When we convert signed to unsigned it may result in a +1 larger output. Fix issue espressif/arduino-esp32#223
* Add CA certificate in example SHA1 fingerprint is broken now: more info: https://shattered.io * Best error handling When occur an error in WiFiClientSecure library just return the error message and clean the context avoiding crash - fix for espressif/arduino-esp32#211 Translate MbedTLS error codes in messages for best understanding * Declarate certificates as const mbedtls_pk_parse_key needs a const unsigned char * certificate. In old implementation the certificate was declarated as char * so first it converts to unsigned and after to const. When we convert signed to unsigned it may result in a +1 larger output. Fix issue espressif/arduino-esp32#223
This only happens under a kind of odd edge-case - I specified my own Root CA, Client Cert and Key, and under "normal" conditions (when I access my server) this works fine.
In the case of the crash - this occurred when I inadvertently connected to the
www.howsmyssl.com
server when using my own certs. When I tried this only specifying my Root CA, I got the appropriate error that the certificate verify mismatched/failed, and everything aborted normally. But when I had my client cert and key in as well, I got this guru meditation.Crash, I assume at the line:
DEBUG_PRINT("Protocol is %s \nCiphersuite is %s\n", mbedtls_ssl_get_version(&ssl_client->ssl_ctx), mbedtls_ssl_get_ciphersuite(&ssl_client->ssl_ctx));
So, in other words - I believe once the handshake failed (as it should have) - maybe it should just have aborted, rather than to continue - which might have (as a result of the handshake failure) gotten back bad pointers from the
mbedtls_ssl_get_ciphersuite
and/ormbedtls_ssl_get_version
calls?(I could try a fix - if I new how to build it? First time with Arduino here...)
The text was updated successfully, but these errors were encountered: