Skip to content
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

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

Merged
merged 10 commits into from
Dec 16, 2022
Original file line number Diff line number Diff line change
Expand Up @@ -75,8 +75,7 @@ void fetchURL(BearSSL::WiFiClientSecure *client, const char *host, const uint16_
if (!path) { path = "/"; }

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 @@ -88,7 +87,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
Original file line number Diff line number Diff line change
Expand Up @@ -44,8 +44,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 @@ -81,8 +80,7 @@ int fetchMaxFragmentLength() {
Serial.printf("\nConnecting to https://tls.mbed.org\n");
Serial.printf("MFLN supported: %s\n", mfln ? "yes" : "no");
if (mfln) { client.setBufferSizes(512, 512); }
d-a-v marked this conversation as resolved.
Show resolved Hide resolved
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
Original file line number Diff line number Diff line change
Expand Up @@ -63,8 +63,7 @@ void fetchURL(BearSSL::WiFiClientSecure *client, const char *host, const uint16_
if (!path) { path = "/"; }

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 @@ -76,7 +75,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
Original file line number Diff line number Diff line change
Expand Up @@ -49,8 +49,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 @@ -62,7 +61,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
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ void setup() {
client.print(String("GET ") + url + " HTTP/1.1\r\n" + "Host: " + github_host + "\r\n" + "User-Agent: BuildFailureDetectorESP8266\r\n" + "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
61 changes: 44 additions & 17 deletions libraries/ESP8266WiFi/src/WiFiClientSecureBearSSL.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -250,19 +250,32 @@ 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() {
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))) {
if (!_engineConnected()) {
return false;
}

if (_pollRecvBuffer() > 0) {
return true;
}
return false;

return _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 @@ -284,7 +297,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 @@ -331,10 +344,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 @@ -343,12 +357,20 @@ int WiFiClientSecureCtx::read(uint8_t *buf, size_t size) {
return -1;
}

int avail = available();
bool conn = connected();
if (!avail && conn) {
return 0; // We're still connected, but nothing to read
// 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 = _pollRecvBuffer();

// 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 @@ -363,10 +385,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 @@ -395,7 +418,7 @@ int WiFiClientSecureCtx::read() {
return -1;
}

int WiFiClientSecureCtx::available() {
int WiFiClientSecureCtx::_pollRecvBuffer() {
if (_recvapp_buf) {
return _recvapp_len; // Anything from last call?
}
Expand All @@ -416,8 +439,12 @@ int WiFiClientSecureCtx::available() {
return 0;
}

int WiFiClientSecureCtx::available() {
return _pollRecvBuffer();
}

int WiFiClientSecureCtx::peek() {
if (!ctx_present() || !available()) {
if (!ctx_present() || (0 == _pollRecvBuffer())) {
DEBUG_BSSL("peek: Not connected, none left available\n");
return -1;
}
Expand All @@ -436,7 +463,7 @@ size_t WiFiClientSecureCtx::peekBytes(uint8_t *buffer, size_t length) {
}

_startMillis = millis();
while ((available() < (int) length) && ((millis() - _startMillis) < 5000)) {
while ((_pollRecvBuffer() < (int) length) && ((millis() - _startMillis) < 5000)) {
yield();
}

Expand Down
6 changes: 6 additions & 0 deletions libraries/ESP8266WiFi/src/WiFiClientSecureBearSSL.h
Original file line number Diff line number Diff line change
Expand Up @@ -195,7 +195,13 @@ class WiFiClientSecureCtx : public WiFiClient {
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?

std::shared_ptr<unsigned char> _alloc_iobuf(size_t sz);
void _freeSSL();
int _run_until(unsigned target, bool blocking = true);
Expand Down