Skip to content

Commit ddda374

Browse files
igrrdevyte
authored andcommitted
WiFiClientSecure: don't trash unread decrypted data when writing (#4024)
* WiFiClientSecure: don't decrypt when testing for 'connected' * WiFiClientSecure: don't trash unread decrypted data when writing When application requests to write data, check if there is any unread decrypted data left. If there is, don't write immediately, but save the data to be written. When all decrypted data has been consumed by the application, send out the saved outgoing data. Fixes #2256.
1 parent e4043e9 commit ddda374

File tree

1 file changed

+108
-2
lines changed

1 file changed

+108
-2
lines changed

libraries/ESP8266WiFi/src/WiFiClientSecure.cpp

Lines changed: 108 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@ extern "C"
2727
#include "osapi.h"
2828
#include "ets_sys.h"
2929
}
30+
#include <list>
3031
#include <errno.h>
3132
#include "debug.h"
3233
#include "ESP8266WiFi.h"
@@ -50,6 +51,26 @@ extern "C"
5051
#define SSL_DEBUG_OPTS 0
5152
#endif
5253

54+
55+
typedef struct BufferItem
56+
{
57+
BufferItem(const uint8_t* data_, size_t size_)
58+
: size(size_), data(new uint8_t[size])
59+
{
60+
if (data.get() != nullptr) {
61+
memcpy(data.get(), data_, size);
62+
} else {
63+
DEBUGV(":wcs alloc %d failed\r\n", size_);
64+
size = 0;
65+
}
66+
}
67+
68+
size_t size;
69+
std::unique_ptr<uint8_t[]> data;
70+
} BufferItem;
71+
72+
typedef std::list<BufferItem> BufferList;
73+
5374
class SSLContext
5475
{
5576
public:
@@ -139,6 +160,10 @@ class SSLContext
139160
_available -= will_copy;
140161
if (_available == 0) {
141162
_read_ptr = nullptr;
163+
/* Send pending outgoing data, if any */
164+
if (_hasWriteBuffers()) {
165+
_writeBuffersSend();
166+
}
142167
}
143168
return will_copy;
144169
}
@@ -155,10 +180,34 @@ class SSLContext
155180
--_available;
156181
if (_available == 0) {
157182
_read_ptr = nullptr;
183+
/* Send pending outgoing data, if any */
184+
if (_hasWriteBuffers()) {
185+
_writeBuffersSend();
186+
}
158187
}
159188
return result;
160189
}
161190

191+
int write(const uint8_t* src, size_t size)
192+
{
193+
if (!_available) {
194+
if (_hasWriteBuffers()) {
195+
int rc = _writeBuffersSend();
196+
if (rc < 0) {
197+
return rc;
198+
}
199+
}
200+
return _write(src, size);
201+
}
202+
/* Some received data is still present in the axtls fragment buffer.
203+
We can't call ssl_write now, as that will overwrite the contents of
204+
the fragment buffer, corrupting the received data.
205+
Save a copy of the outgoing data, and call ssl_write when all
206+
recevied data has been consumed by the application.
207+
*/
208+
return _writeBufferAdd(src, size);
209+
}
210+
162211
int peek()
163212
{
164213
if (!_available) {
@@ -193,6 +242,12 @@ class SSLContext
193242
return cb;
194243
}
195244

245+
// similar to availble, but doesn't return exact size
246+
bool hasData()
247+
{
248+
return _available > 0 || (s_io_ctx && s_io_ctx->getSize() > 0);
249+
}
250+
196251
bool loadObject(int type, Stream& stream, size_t size)
197252
{
198253
std::unique_ptr<uint8_t[]> buf(new uint8_t[size]);
@@ -282,12 +337,63 @@ class SSLContext
282337
return _available;
283338
}
284339

340+
int _write(const uint8_t* src, size_t size)
341+
{
342+
if (!_ssl) {
343+
return 0;
344+
}
345+
346+
int rc = ssl_write(_ssl, src, size);
347+
if (rc >= 0) {
348+
return rc;
349+
}
350+
DEBUGV(":wcs write rc=%d\r\n", rc);
351+
return rc;
352+
}
353+
354+
int _writeBufferAdd(const uint8_t* data, size_t size)
355+
{
356+
if (!_ssl) {
357+
return 0;
358+
}
359+
360+
_writeBuffers.emplace_back(data, size);
361+
if (_writeBuffers.back().data.get() == nullptr) {
362+
_writeBuffers.pop_back();
363+
return 0;
364+
}
365+
return size;
366+
}
367+
368+
int _writeBuffersSend()
369+
{
370+
while (!_writeBuffers.empty()) {
371+
auto& first = _writeBuffers.front();
372+
int rc = _write(first.data.get(), first.size);
373+
_writeBuffers.pop_front();
374+
if (rc < 0) {
375+
if (_hasWriteBuffers()) {
376+
DEBUGV(":wcs _writeBuffersSend dropping unsent data\r\n");
377+
_writeBuffers.clear();
378+
}
379+
return rc;
380+
}
381+
}
382+
return 0;
383+
}
384+
385+
bool _hasWriteBuffers()
386+
{
387+
return !_writeBuffers.empty();
388+
}
389+
285390
static SSL_CTX* _ssl_ctx;
286391
static int _ssl_ctx_refcnt;
287392
SSL* _ssl = nullptr;
288393
int _refcnt = 0;
289394
const uint8_t* _read_ptr = nullptr;
290395
size_t _available = 0;
396+
BufferList _writeBuffers;
291397
bool _allowSelfSignedCerts = false;
292398
static ClientContext* s_io_ctx;
293399
};
@@ -371,7 +477,7 @@ size_t WiFiClientSecure::write(const uint8_t *buf, size_t size)
371477
return 0;
372478
}
373479

374-
int rc = ssl_write(*_ssl, buf, size);
480+
int rc = _ssl->write(buf, size);
375481
if (rc >= 0) {
376482
return rc;
377483
}
@@ -458,7 +564,7 @@ err x N N
458564
uint8_t WiFiClientSecure::connected()
459565
{
460566
if (_ssl) {
461-
if (_ssl->available()) {
567+
if (_ssl->hasData()) {
462568
return true;
463569
}
464570
if (_client && _client->state() == ESTABLISHED && _ssl->connected()) {

0 commit comments

Comments
 (0)