8000 Don't return `true` with `WiFiClientSecureBearSSL::connected()` when really disconnected by mcspr · Pull Request #8330 · esp8266/Arduino · GitHub
[go: up one dir, main page]
Skip to content

Don't return true with WiFiClientSecureBearSSL::connected() when really disconnected #8330

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.

Sign up for GitHub

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

Merged
merged 10 commits into from
Dec 16, 2022
Merged

Don't return true with WiFiClientSecureBearSSL::connected() when really disconnected #8330

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
724f1ca
Don't return `true` with WiFiClientSecureBearSSL::connected() when di…
mcspr Oct 7, 2021
8e8cbd2
notice about _updateRecvBuffer aka external available
mcspr Oct 7, 2021
934ab80
Merge remote-tracking branch 'origin/master' into secure-connected-me…
mcspr Feb 21, 2022
3eb7ab1
wip
mcspr Feb 25, 2022
0be8943
Merge remote-tracking branch 'origin/master' into secure-connected-me…
mcspr Jul 2, 2022
f65b157
restore available() for real
mcspr Jul 2, 2022
c541597
Merge branch 'master' into secure-connected-means-read-and-write
d-a-v Dec 14, 2022
f5e6534
direct call
mcspr Jul 4, 2022
5454375
oops
mcspr Dec 14, 2022
3e5500d
Merge branch 'master' into secure-connected-means-read-and-write
d-a-v Dec 16, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Next Next commit
Don't return true with WiFiClientSecureBearSSL::connected() when di… 
…sconnected

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
  • Loading branch information
@mcspr
mcspr committed Oct 7, 2021
commit 724f1ca8bd36594c1601e39f1092eb5fe099749c
5 changes: 2 additions & 3 deletions libraries/ESP8266WiFi/examples/BearSSL_CertStore/BearSSL_CertStore.ino
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand All @@ -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);
Expand Down
6 changes: 2 additions & 4 deletions libraries/ESP8266WiFi/examples/BearSSL_MaxFragmentLength/BearSSL_MaxFragmentLength.ino
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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();
Expand Down
5 changes: 2 additions & 3 deletions libraries/ESP8266WiFi/examples/BearSSL_Sessions/BearSSL_Sessions.ino
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand All @@ -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);
Expand Down
5 changes: 2 additions & 3 deletions libraries/ESP8266WiFi/examples/BearSSL_Validation/BearSSL_Validation.ino
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand All @@ -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);
Expand Down
2 changes: 1 addition & 1 deletion libraries/ESP8266WiFi/examples/HTTPSRequest/HTTPSRequest.ino
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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");
Expand Down
50 changes: 34 additions & 16 deletions libraries/ESP8266WiFi/src/WiFiClientSecureBearSSL.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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;
}

Expand Down Expand Up @@ -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);
}

Expand All @@ -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;
}
Expand All @@ -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.
}

Expand Down Expand Up @@ -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?
}
Expand All @@ -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;
}
Expand Down
4 changes: 4 additions & 0 deletions libraries/ESP8266WiFi/src/WiFiClientSecureBearSSL.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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<unsigned char> _alloc_iobuf(size_t sz);
void _freeSSL();
int _run_until(unsigned target, bool blocking = true);
Expand Down
0