From 724f1ca8bd36594c1601e39f1092eb5fe099749c Mon Sep 17 00:00:00 2001 From: Maxim Prokhorov Date: Thu, 7 Oct 2021 16:58:42 +0300 Subject: [PATCH 1/6] Don't return `true` with WiFiClientSecureBearSSL::connected() when disconnected Apply the same condition as with normal WiFiClient - we are not connected when it's not possible to both write and read. Implement separate methods for actual connection status and the internal ssl engine status and update methods that were previously using available() for this purpose Update examples to check available() when the intent is to only read the data and not interact with the client in any other way. Also, use connect() as a way to notify errors, no need to check things twice --- .../BearSSL_CertStore/BearSSL_CertStore.ino | 5 +- .../BearSSL_MaxFragmentLength.ino | 6 +-- .../BearSSL_Sessions/BearSSL_Sessions.ino | 5 +- .../BearSSL_Validation/BearSSL_Validation.ino | 5 +- .../examples/HTTPSRequest/HTTPSRequest.ino | 2 +- .../src/WiFiClientSecureBearSSL.cpp | 50 +++++++++++++------ .../ESP8266WiFi/src/WiFiClientSecureBearSSL.h | 4 ++ 7 files changed, 47 insertions(+), 30 deletions(-) diff --git a/libraries/ESP8266WiFi/examples/BearSSL_CertStore/BearSSL_CertStore.ino b/libraries/ESP8266WiFi/examples/BearSSL_CertStore/BearSSL_CertStore.ino index a599a0395a..14a52fc8e4 100644 --- a/libraries/ESP8266WiFi/examples/BearSSL_CertStore/BearSSL_CertStore.ino +++ b/libraries/ESP8266WiFi/examples/BearSSL_CertStore/BearSSL_CertStore.ino @@ -77,8 +77,7 @@ void fetchURL(BearSSL::WiFiClientSecure *client, const char *host, const uint16_ } Serial.printf("Trying: %s:443...", host); - client->connect(host, port); - if (!client->connected()) { + if (!client->connect(host, port)) { Serial.printf("*** Can't connect. ***\n-------\n"); return; } @@ -90,7 +89,7 @@ void fetchURL(BearSSL::WiFiClientSecure *client, const char *host, const uint16_ client->write("\r\nUser-Agent: ESP8266\r\n"); client->write("\r\n"); uint32_t to = millis() + 5000; - if (client->connected()) { + while (client->available()) { do { char tmp[32]; memset(tmp, 0, 32); diff --git a/libraries/ESP8266WiFi/examples/BearSSL_MaxFragmentLength/BearSSL_MaxFragmentLength.ino b/libraries/ESP8266WiFi/examples/BearSSL_MaxFragmentLength/BearSSL_MaxFragmentLength.ino index 708d906eb1..e297b22d6f 100644 --- a/libraries/ESP8266WiFi/examples/BearSSL_MaxFragmentLength/BearSSL_MaxFragmentLength.ino +++ b/libraries/ESP8266WiFi/examples/BearSSL_MaxFragmentLength/BearSSL_MaxFragmentLength.ino @@ -46,8 +46,7 @@ int fetchNoMaxFragmentLength() { BearSSL::WiFiClientSecure client; client.setInsecure(); - client.connect("tls.mbed.org", 443); - if (client.connected()) { + if (client.connect("tls.mbed.org", 443)) { Serial.printf("Memory used: %d\n", ret - ESP.getFreeHeap()); ret -= ESP.getFreeHeap(); fetch(&client); @@ -85,8 +84,7 @@ int fetchMaxFragmentLength() { if (mfln) { client.setBufferSizes(512, 512); } - client.connect("tls.mbed.org", 443); - if (client.connected()) { + if (client.connect("tls.mbed.org", 443)) { Serial.printf("MFLN status: %s\n", client.getMFLNStatus() ? "true" : "false"); Serial.printf("Memory used: %d\n", ret - ESP.getFreeHeap()); ret -= ESP.getFreeHeap(); diff --git a/libraries/ESP8266WiFi/examples/BearSSL_Sessions/BearSSL_Sessions.ino b/libraries/ESP8266WiFi/examples/BearSSL_Sessions/BearSSL_Sessions.ino index fa03c7fa38..c53b85159b 100644 --- a/libraries/ESP8266WiFi/examples/BearSSL_Sessions/BearSSL_Sessions.ino +++ b/libraries/ESP8266WiFi/examples/BearSSL_Sessions/BearSSL_Sessions.ino @@ -58,8 +58,7 @@ void fetchURL(BearSSL::WiFiClientSecure *client, const char *host, const uint16_ } Serial.printf("Trying: %s:443...", host); - client->connect(host, port); - if (!client->connected()) { + if (!client->connect(host, port)) { Serial.printf("*** Can't connect. ***\n-------\n"); return; } @@ -71,7 +70,7 @@ void fetchURL(BearSSL::WiFiClientSecure *client, const char *host, const uint16_ client->write("\r\nUser-Agent: ESP8266\r\n"); client->write("\r\n"); uint32_t to = millis() + 5000; - if (client->connected()) { + while (client->available()) { do { char tmp[32]; memset(tmp, 0, 32); diff --git a/libraries/ESP8266WiFi/examples/BearSSL_Validation/BearSSL_Validation.ino b/libraries/ESP8266WiFi/examples/BearSSL_Validation/BearSSL_Validation.ino index b91570da11..93283f73b1 100644 --- a/libraries/ESP8266WiFi/examples/BearSSL_Validation/BearSSL_Validation.ino +++ b/libraries/ESP8266WiFi/examples/BearSSL_Validation/BearSSL_Validation.ino @@ -47,8 +47,7 @@ void fetchURL(BearSSL::WiFiClientSecure *client, const char *host, const uint16_ ESP.resetFreeContStack(); uint32_t freeStackStart = ESP.getFreeContStack(); Serial.printf("Trying: %s:443...", host); - client->connect(host, port); - if (!client->connected()) { + if (!client->connect(host, port)) { Serial.printf("*** Can't connect. ***\n-------\n"); return; } @@ -60,7 +59,7 @@ void fetchURL(BearSSL::WiFiClientSecure *client, const char *host, const uint16_ client->write("\r\nUser-Agent: ESP8266\r\n"); client->write("\r\n"); uint32_t to = millis() + 5000; - if (client->connected()) { + while (client->available()) { do { char tmp[32]; memset(tmp, 0, 32); diff --git a/libraries/ESP8266WiFi/examples/HTTPSRequest/HTTPSRequest.ino b/libraries/ESP8266WiFi/examples/HTTPSRequest/HTTPSRequest.ino index 9ccf3b6b37..284897e10a 100644 --- a/libraries/ESP8266WiFi/examples/HTTPSRequest/HTTPSRequest.ino +++ b/libraries/ESP8266WiFi/examples/HTTPSRequest/HTTPSRequest.ino @@ -80,7 +80,7 @@ void setup() { "Connection: close\r\n\r\n"); Serial.println("Request sent"); - while (client.connected()) { + while (client.available()) { String line = client.readStringUntil('\n'); if (line == "\r") { Serial.println("Headers received"); diff --git a/libraries/ESP8266WiFi/src/WiFiClientSecureBearSSL.cpp b/libraries/ESP8266WiFi/src/WiFiClientSecureBearSSL.cpp index 6a82ff3b96..876acdfbf7 100644 --- a/libraries/ESP8266WiFi/src/WiFiClientSecureBearSSL.cpp +++ b/libraries/ESP8266WiFi/src/WiFiClientSecureBearSSL.cpp @@ -253,19 +253,24 @@ void WiFiClientSecureCtx::_freeSSL() { } bool WiFiClientSecureCtx::_clientConnected() { - return (_client && _client->state() == ESTABLISHED); + return _client && (_client->state() == ESTABLISHED); +} + +bool WiFiClientSecureCtx::_engineConnected() { + return _clientConnected() && _handshake_done && _eng && (br_ssl_engine_current_state(_eng) != BR_SSL_CLOSED); } uint8_t WiFiClientSecureCtx::connected() { - if (available() || (_clientConnected() && _handshake_done && (br_ssl_engine_current_state(_eng) != BR_SSL_CLOSED))) { - return true; + if (!_clientConnected()) { + return false; } - return false; + + return (available() || _engineConnected()); } int WiFiClientSecureCtx::availableForWrite () { - // code taken from ::_write() - if (!connected() || !_handshake_done) { + // Can't write things when there's no connection or br_ssl engine is closed + if (!_engineConnected()) { return 0; } // Get BearSSL to a state where we can send @@ -287,7 +292,7 @@ int WiFiClientSecureCtx::availableForWrite () { size_t WiFiClientSecureCtx::_write(const uint8_t *buf, size_t size, bool pmem) { size_t sent_bytes = 0; - if (!connected() || !size || !_handshake_done) { + if (!size || !_engineConnected()) { return 0; } @@ -334,10 +339,11 @@ size_t WiFiClientSecureCtx::write_P(PGM_P buf, size_t size) { } size_t WiFiClientSecureCtx::write(Stream& stream) { - if (!connected() || !_handshake_done) { - DEBUG_BSSL("write: Connect/handshake not completed yet\n"); + if (!_engineConnected()) { + DEBUG_BSSL("write: no br_ssl engine to work with\n"); return 0; } + return stream.sendAll(this); } @@ -346,12 +352,19 @@ int WiFiClientSecureCtx::read(uint8_t *buf, size_t size) { return -1; } + // will either check the internal buffer, or try to wait for some data int avail = available(); - bool conn = connected(); - if (!avail && conn) { - return 0; // We're still connected, but nothing to read + + // internal buffer might still be available for some time + bool engine = _engineConnected(); + + // we're still connected, but nothing to read + if (!avail && engine) { + return 0; } - if (!avail && !conn) { + + // or, available failed to assign the internal buffer and we are already disconnected + if (!avail && !engine) { DEBUG_BSSL("read: Not connected, none left available\n"); return -1; } @@ -366,10 +379,11 @@ int WiFiClientSecureCtx::read(uint8_t *buf, size_t size) { return to_copy; } - if (!conn) { + if (!engine) { DEBUG_BSSL("read: Not connected\n"); return -1; } + return 0; // If we're connected, no error but no read. } @@ -398,7 +412,7 @@ int WiFiClientSecureCtx::read() { return -1; } -int WiFiClientSecureCtx::available() { +int WiFiClientSecureCtx::_updateRecvBuffer() { if (_recvapp_buf) { return _recvapp_len; // Anything from last call? } @@ -419,8 +433,12 @@ int WiFiClientSecureCtx::available() { return 0; } +int WiFiClientSecureCtx::available() { + return _updateRecvBuffer(); +} + int WiFiClientSecureCtx::peek() { - if (!ctx_present() || !available()) { + if (!ctx_present() || !_updateRecvBuffer()) { DEBUG_BSSL("peek: Not connected, none left available\n"); return -1; } diff --git a/libraries/ESP8266WiFi/src/WiFiClientSecureBearSSL.h b/libraries/ESP8266WiFi/src/WiFiClientSecureBearSSL.h index 73dc9e7337..8f416441d0 100644 --- a/libraries/ESP8266WiFi/src/WiFiClientSecureBearSSL.h +++ b/libraries/ESP8266WiFi/src/WiFiClientSecureBearSSL.h @@ -185,10 +185,14 @@ class WiFiClientSecureCtx : public WiFiClient { uint32_t _tls_min; uint32_t _tls_max; + // Pending received data buffer, used internally & as available() + int _updateRecvBuffer(); unsigned char *_recvapp_buf; size_t _recvapp_len; bool _clientConnected(); // Is the underlying socket alive? + bool _engineConnected(); // Are both socket and the bearssl engine alive? + std::shared_ptr _alloc_iobuf(size_t sz); void _freeSSL(); int _run_until(unsigned target, bool blocking = true); From 8e8cbd21402fa5b6b336c7635b73bd3e47fc9efe Mon Sep 17 00:00:00 2001 From: Max Prokhorov Date: Fri, 8 Oct 2021 00:21:00 +0300 Subject: [PATCH 2/6] notice about _updateRecvBuffer aka external available --- libraries/ESP8266WiFi/src/WiFiClientSecureBearSSL.cpp | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/libraries/ESP8266WiFi/src/WiFiClientSecureBearSSL.cpp b/libraries/ESP8266WiFi/src/WiFiClientSecureBearSSL.cpp index 876acdfbf7..4bc696d9ce 100644 --- a/libraries/ESP8266WiFi/src/WiFiClientSecureBearSSL.cpp +++ b/libraries/ESP8266WiFi/src/WiFiClientSecureBearSSL.cpp @@ -265,7 +265,7 @@ uint8_t WiFiClientSecureCtx::connected() { return false; } - return (available() || _engineConnected()); + return (_updateRecvBuffer() || _engineConnected()); } int WiFiClientSecureCtx::availableForWrite () { @@ -353,7 +353,8 @@ int WiFiClientSecureCtx::read(uint8_t *buf, size_t size) { } // will either check the internal buffer, or try to wait for some data - int avail = available(); + // *may* attempt to write some pending ::write() data b/c of _run_until + int avail = _updateRecvBuffer(); // internal buffer might still be available for some time bool engine = _engineConnected(); @@ -457,7 +458,7 @@ size_t WiFiClientSecureCtx::peekBytes(uint8_t *buffer, size_t length) { } _startMillis = millis(); - while ((available() < (int) length) && ((millis() - _startMillis) < 5000)) { + while ((_updateRecvBuffer() < (int) length) && ((millis() - _startMillis) < 5000)) { yield(); } From 3eb7ab13524a06df2f7e272d721c5fcc51997d93 Mon Sep 17 00:00:00 2001 From: Maxim Prokhorov Date: Fri, 25 Feb 2022 23:16:52 +0300 Subject: [PATCH 3/6] wip --- .../ESP8266WiFi/src/WiFiClientSecureBearSSL.cpp | 16 +++++++++------- .../ESP8266WiFi/src/WiFiClientSecureBearSSL.h | 6 ++++-- 2 files changed, 13 insertions(+), 9 deletions(-) diff --git a/libraries/ESP8266WiFi/src/WiFiClientSecureBearSSL.cpp b/libraries/ESP8266WiFi/src/WiFiClientSecureBearSSL.cpp index 143a898c32..dfa5ff4cf5 100644 --- a/libraries/ESP8266WiFi/src/WiFiClientSecureBearSSL.cpp +++ b/libraries/ESP8266WiFi/src/WiFiClientSecureBearSSL.cpp @@ -260,11 +260,13 @@ bool WiFiClientSecureCtx::_engineConnected() { } uint8_t WiFiClientSecureCtx::connected() { - if (!_clientConnected()) { + if (!_engineConnected()) { return false; } - return (_updateRecvBuffer() || _engineConnected()); + _pollRecvBuffer(); + + return _engineConnected(); } int WiFiClientSecureCtx::availableForWrite () { @@ -353,7 +355,7 @@ int WiFiClientSecureCtx::read(uint8_t *buf, size_t size) { // will either check the internal buffer, or try to wait for some data // *may* attempt to write some pending ::write() data b/c of _run_until - int avail = _updateRecvBuffer(); + int avail = _pollRecvBuffer(); // internal buffer might still be available for some time bool engine = _engineConnected(); @@ -412,7 +414,7 @@ int WiFiClientSecureCtx::read() { return -1; } -int WiFiClientSecureCtx::_updateRecvBuffer() { +int WiFiClientSecureCtx::_pollRecvBuffer() { if (_recvapp_buf) { return _recvapp_len; // Anything from last call? } @@ -434,11 +436,11 @@ int WiFiClientSecureCtx::_updateRecvBuffer() { } int WiFiClientSecureCtx::available() { - return _updateRecvBuffer(); + return _pollRecvBuffer(); } int WiFiClientSecureCtx::peek() { - if (!ctx_present() || !_updateRecvBuffer()) { + if (!ctx_present() || (0 == _pollRecvBuffer())) { DEBUG_BSSL("peek: Not connected, none left available\n"); return -1; } @@ -457,7 +459,7 @@ size_t WiFiClientSecureCtx::peekBytes(uint8_t *buffer, size_t length) { } _startMillis = millis(); - while ((_updateRecvBuffer() < (int) length) && ((millis() - _startMillis) < 5000)) { + while ((_pollRecvBuffer() < (int) length) && ((millis() - _startMillis) < 5000)) { yield(); } diff --git a/libraries/ESP8266WiFi/src/WiFiClientSecureBearSSL.h b/libraries/ESP8266WiFi/src/WiFiClientSecureBearSSL.h index f43e6a8f7c..e3163664d6 100644 --- a/libraries/ESP8266WiFi/src/WiFiClientSecureBearSSL.h +++ b/libraries/ESP8266WiFi/src/WiFiClientSecureBearSSL.h @@ -192,11 +192,13 @@ class WiFiClientSecureCtx : public WiFiClient { uint32_t _tls_min; uint32_t _tls_max; - // Pending received data buffer, used internally & as available() - int _updateRecvBuffer(); unsigned char *_recvapp_buf; size_t _recvapp_len; + int _pollRecvBuffer(); // If there's a buffer with some pending data, return it's length + // If there's no buffer, poll the engine and store any received data there and return the length + // (which also may change the internal state, e.g. make us disconnected) + bool _clientConnected(); // Is the underlying socket alive? bool _engineConnected(); // Are both socket and the bearssl engine alive? From f65b157a8742dbea41b1f1881f2e9eef984266bd Mon Sep 17 00:00:00 2001 From: Maxim Prokhorov Date: Sat, 2 Jul 2022 23:49:26 +0300 Subject: [PATCH 4/6] restore available() for real --- libraries/ESP8266WiFi/src/WiFiClientSecureBearSSL.cpp | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/libraries/ESP8266WiFi/src/WiFiClientSecureBearSSL.cpp b/libraries/ESP8266WiFi/src/WiFiClientSecureBearSSL.cpp index f298eff38a..a10f03663f 100644 --- a/libraries/ESP8266WiFi/src/WiFiClientSecureBearSSL.cpp +++ b/libraries/ESP8266WiFi/src/WiFiClientSecureBearSSL.cpp @@ -250,7 +250,11 @@ void WiFiClientSecureCtx::_freeSSL() { } bool WiFiClientSecureCtx::_clientConnected() { - return _client && (_client->state() == ESTABLISHED); + if (!_client || (_client->state() == CLOSED)) { + return false; + } + + return _client->state() == ESTABLISHED; } bool WiFiClientSecureCtx::_engineConnected() { @@ -264,7 +268,7 @@ uint8_t WiFiClientSecureCtx::connected() { _pollRecvBuffer(); - return _engineConnected(); + return _engineConnected() || (available() > 0); } int WiFiClientSecureCtx::availableForWrite () { From f5e653451033f0d97ffcdfc8480764a362a24baa Mon Sep 17 00:00:00 2001 From: Maxim Prokhorov Date: Mon, 4 Jul 2022 16:18:15 +0300 Subject: [PATCH 5/6] direct call --- libraries/ESP8266WiFi/src/WiFiClientSecureBearSSL.cpp | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/libraries/ESP8266WiFi/src/WiFiClientSecureBearSSL.cpp b/libraries/ESP8266WiFi/src/WiFiClientSecureBearSSL.cpp index a10f03663f..53b606f8c3 100644 --- a/libraries/ESP8266WiFi/src/WiFiClientSecureBearSSL.cpp +++ b/libraries/ESP8266WiFi/src/WiFiClientSecureBearSSL.cpp @@ -266,9 +266,11 @@ uint8_t WiFiClientSecureCtx::connected() { return false; } - _pollRecvBuffer(); + if (_pollRecvBuffer() > 0) { + return true; + } - return _engineConnected() || (available() > 0); + return _engineConnected(); } int WiFiClientSecureCtx::availableForWrite () { From 54543754e00737691157981bdc484b77d7912b45 Mon Sep 17 00:00:00 2001 From: Maxim Prokhorov Date: Thu, 15 Dec 2022 02:38:39 +0300 Subject: [PATCH 6/6] oops --- .../BearSSL_MaxFragmentLength/BearSSL_MaxFragmentLength.ino | 1 + 1 file changed, 1 insertion(+) diff --git a/libraries/ESP8266WiFi/examples/BearSSL_MaxFragmentLength/BearSSL_MaxFragmentLength.ino b/libraries/ESP8266WiFi/examples/BearSSL_MaxFragmentLength/BearSSL_MaxFragmentLength.ino index b3887f646e..70e7f789e4 100644 --- a/libraries/ESP8266WiFi/examples/BearSSL_MaxFragmentLength/BearSSL_MaxFragmentLength.ino +++ b/libraries/ESP8266WiFi/examples/BearSSL_MaxFragmentLength/BearSSL_MaxFragmentLength.ino @@ -79,6 +79,7 @@ int fetchMaxFragmentLength() { bool mfln = client.probeMaxFragmentLength("tls.mbed.org", 443, 512); Serial.printf("\nConnecting to https://tls.mbed.org\n"); Serial.printf("MFLN supported: %s\n", mfln ? "yes" : "no"); + if (mfln) { client.setBufferSizes(512, 512); } if (client.connect("tls.mbed.org", 443)) { Serial.printf("MFLN status: %s\n", client.getMFLNStatus() ? "true" : "false"); Serial.printf("Memory used: %d\n", ret - ESP.getFreeHeap());