micropython / micropython-esp32

Old port of MicroPython to the ESP32 -- new port is at https://github.com/micropython/micropython
MIT License
673 stars 216 forks source link

Ssl possible memory leak on server side #137

Closed LupascuAndrei closed 7 years ago

LupascuAndrei commented 7 years ago

https://github.com/micropython/micropython-esp32/blob/esp32/extmod/modussl_mbedtls.c#L231-L236

Shouldn't pkey and cert also be freed there? They are always initialized during ssl_wrap_socket (and used when the ssl wrap socket is called with server_mode=true ) https://github.com/micropython/micropython-esp32/blob/esp32/extmod/modussl_mbedtls.c#L117

Esp32 crashes once every 50-100 ssl handshakes when run as ssl server mode; i think that clearing the pkey & cert fixed it for me

    mbedtls_x509_crt_free(&self->cert);
    mbedtls_ssl_free(&self->ssl);
    mbedtls_ssl_config_free(&self->conf);
    mbedtls_ctr_drbg_free(&self->ctr_drbg);
    mbedtls_entropy_free(&self->entropy);
    mbedtls_pk_free(&self->pkey);                      // this
    mbedtls_x509_crt_free(&self->cert);                // also this

Also, regarding the same file, why is null_entropy_func used? https://github.com/micropython/micropython-esp32/blob/esp32/extmod/modussl_mbedtls.c#L74

It seems to be working fine for me with:

ret = mbedtls_ctr_drbg_seed(&o->ctr_drbg, /*null_entropy_func*/mbedtls_entropy_func, &o->entropy, seed, sizeof(seed));
dpgeorge commented 7 years ago

Thanks for the report, it does look like a bug with the freeing of resources. I fixed it upstream at the patch should make its way here soon (I managed to get 1000+ successful connections in server mode with the patch).

Also, regarding the same file, why is null_entropy_func used?

See #122 for a discussion about that.

dpgeorge commented 7 years ago

Resource freeing is now merged, see 0893b273b9471acb02510820f61753501dc04219