Drop X509 context after successful server verification to save heap space

[sislakd]

After completing handshake in BSSL, server is already verified and X509 context is no longer needed. Depending on verification method it save more or less heap space.

#6005

[earlephilhower]

LGTM, thanks! We're in a release freeze, so this will get in right after.

[sislakd]

Added the change for reporting not connected if TLS session is broken and there is no more buffered decrypted data. TLS can be broken if message authentication (MAC) cannot be verified. BearSSL enters BR_SSL_CLOSED state when processing invalid encrypted application data fragment. In such situation the current implementation get stuck forever unless user has own timeout mechanism build on top of WiFiClientSecureBearSSL. This change introduce fail fast via connected() returning false. Further it imply return -1 from read methods indicating broken channel upon which user should perform reconnect if needed.

[sislakd]

It seems that I cannot create another PR for the change in the same file unless previous change is merged. Hopefully, this second change can be included in this open PR.

[earlephilhower]

Yes, GH works on PRs per-branch, so you'd need to make a new branch off master for each separate PR.

It's fine the way you've done it, we'll push them all at the same time. We'll adjust the title to reflect the two (or more) different updates.

[mnunespt]

Added the change for reporting not connected if TLS session is broken and there is no more buffered decrypted data. TLS can be broken if message authentication (MAC) cannot be verified. BearSSL enters BR_SSL_CLOSED state when processing invalid encrypted application data fragment. In such situation the current implementation get stuck forever unless user has own timeout mechanism build on top of WiFiClientSecureBearSSL. This change introduce fail fast via connected() returning false. Further it imply return -1 from read methods indicating broken channel upon which user should perform reconnect if needed.

Thank you very much!

[mnunespt]

Hi,
after more tests i've found that I have the same problem, even with this 2.5.2 version

I have this setup: Internet Router <=> WIFI AP <=> NodeMCU 12E
If i disconnect the cable from InternetRouter to WifiAP, the connection on WifiClientSecure hangs, with no time out.

The ESP8266 hangs in the last line of this debug output:
:ur 1 :del :ref 1 :wr 229 0 :wrc 229 229 0 :ack 229 :rn 536 :rch 536, 536 :rch 1072, 536 :rch 1608, 536 :rd 5, 2144, 0 :rdi 536, 5 :rd 93, 2144, 5 :rdi 531, 93 :rd 5, 2144, 98 :rdi 438, 5 :rd 2041, 2144, 103 :rdi 433, 433 :c 433, 536, 2144 :rdi 536, 536 :c 536, 536, 1608 :rdi 536, 536 :c 536, 536, 1072 :rdi 536, 536 :c0 536, 536

or

:wrc 45 45 0

i'm posting HTTPS and receiving data while the connection is broken.
It stops always in the :c0 536, 536 and some times on :wrc 45 45 0

have no clue... :(

Thanks for any help you can provide.

[mnunespt]

Hi,
I've managed to solve my problem adding some code to the ClientContext.h file
I added a counter variable to the _write_some_from_cb() function to check for the loop and timeout after a few tries.
Here the code I added. For me is enought to restart the ESP, but we can just send a :wustmo timeout instead.

//line 545
    void _write_some_from_cb()
    {
		rcont++;
		DEBUGV("retry: %d\n",rcont);
		if (rcont > 30)
		{
			DEBUGV(" >>>>  Connection hang - will restart!!!");
			ESP.restart();
			rcont=0; 
		}
        if (_send_waiting) {
            _send_waiting = false;
            esp_schedule();
        }
    }

need to declare the global rcont as int

//line 671
private:   
    tcp_pcb* _pcb;
    pbuf* _rx_buf;
    size_t _rx_buf_offset;
    discard_cb_t _discard_cb;
    void* _discard_cb_arg;
    DataSource* _datasource = nullptr;
    size_t _written = 0;
    uint32_t _timeout_ms = 5000;
    uint32_t _op_start_time = 0;
    bool _send_waiting = false;
    uint8_t _connect_pending = 0;
    int8_t _refcnt;
    ClientContext* _next;
    bool _sync;
    int rcont = 0;
};

[d-a-v]

@mnunespt This hack is dangerous. ClientContext is used by WiFiClient, and your restart impacts long transfers (even those unrelated with SSL) because rcont is never reset.
There were timeout fixes released in 2.5.2. If they are not sufficient, please open a new issue with a MCVE reproducing your issue.

[mnunespt]

Yes you are right.
I forget to past the code here rcont is restarted.

rcont is reinitiated in the function:

err_t _acked(tcp_pcb* pcb, uint16_t len)
{
(void) pcb;
(void) len;
DEBUGV(":ack %d\r\n", len);
_write_some_from_cb();
rcont=0;
return ERR_OK;
}

It's not a elegant solution, but in my case, my specific sketch, it works very well.