[PATCH ustream] ustream-openssl: fix bio memory leak

Eneas U de Queiroz cotequeiroz at gmail.com
Thu Dec 10 14:41:07 EST 2020


Hi Petr

On Thu, Dec 10, 2020 at 12:57 PM Petr Štetiar <ynezz at true.cz> wrote:
> > After tackling BIO_free, my suggestion would be to determine where the
> > method table variable should go, and where to call BIO_meth_new and
> > BIO_meth_free.  I would add it to a defined struct
> > ustream_ssl_ctx--which is now just used with a cast to SSL_CTX--and
>
> IIRC I've tried that approach already(this WIP solution is like 3rd
> iteration), but that struct is opaque.

I meant the ustream_ssl_ctx structure, which is an ustream internal
structure.  For openssl, we're just using a straight cast to the
openssl's SSL_CTX struct, so that's why it is opaque, while for
mbedtls, it is a defined struct.  What I meant was to actually define
a ustream_ssl_ctx structure for openssl, just as ustream-mbedtls does,
with the BIO_methods and the SSL_CTX as members.

> > would create and free the object in __ustream_ssl_context_new and
> > __ustream_ssl_context_free, which would give it a possibly larger
> > lifetime than the ssl_session or the BIO object.
>
> AFAIK that's exactly what I'm doing in my current solution.

You're doing it at the SSL struct.  You can have multiple SSL structs
under the same SSL_CTX struct. In a server, for example, you  will
have one SSL_CTX object, which accepts connections, creating a new SSL
structure for each connection.  You know I'm just madly fighting for
every CPU cycle of performance optimization I can get. ;-)

If you look at it from an organization and tidiness POV, you can argue
that the BIO methods structure should be placed along with the BIO,
which is with the SSL structure.  I'll let you pick your side.

> > We should coordinate efforts.  You're the boss, so tell me what you want me
> > to do, if anything.
>
> I didn't wanted to sound like the boss and I apologize if that was the case,
> sorry.

I apologize for the bad choice of words.  Someone has to take the
lead, and that was a rather ill-fated attempt to make it clear that I
would follow your lead, and had nothing to do with your tone or
anything you had done.

> I've just send out some patches for uclient/ustream-ssl, so I would be
> grateful if you could review and test those changes on your device(s), ideally
> on all three SSL libs and client/server setup. Thanks!

I'll do that over the weekend.  I'm updating openssl to 1.1.1i, which
fixes high severity CVE-2020-1971.  I haven't sent it yet because I
want to test it first, and I'm low on testing resources right now.
I'll probably test openssl tonight, then tackle ustream-ssl.

Cheers,

Eneas



More information about the openwrt-devel mailing list