search

sociomantic-tsunami / libdrizzle-redux

The next generation of Libdrizzle with a simplified API and support for more features of the protocol
Other
13 stars 12 forks source link

issue 287: SSL support #345

Closed smanolache closed 4 years ago

smanolache commented 4 years ago

There are three classes of change

  1. SSL_connect, SSL_read, and SSL_write use SSL_get_error in order to monitor the appropriate events on the I/O event loop.

  2. The error you've noticed, namely that the BIO was not set, happened because SSL_set_fd was called only when the connection succeeded in drizzle_connect. I've added it when the connection succeeds in drizzle_state_connecting.

  3. I've removed SSL_new from drizzle_set_ssl. The reason is that drizzle_set_ssl is called only once, at setup, when we configure the certificates, while a SSL object is associated to a live network connection. Thus, the SSL_CTX is built only once, by drizzle_set_ssl, while the SSL object is built whenever the connect system call succeeds and is freed in drizzle_close.

Besides the normal operation, I've tested two scenarios:

  1. I continuously pass SQL requests over the same connection while in another console I launch iptables commands to drop packets. The application detects the errors, calls drizzle_close and tries to reestablish the connection. SSL is correctly reestablished once I stop dropping the packets.

  2. I continuously pass SQL requests over the same connection. Then I shutdown the MySQL server. The application reacts to it by calling drizzle_close. I can see the Encrypted Alert that my client sends to the server before closing the socket. SSL is correctly reestablished once I restart the server.

I am not sure that I handle the shutdowns (SSL_shutdown) correctly though.

bokchan commented 4 years ago

Hi @smanolache Thanks a lot for contributing to the project. I apologize for not reacting sooner, but this is a side project and has been a high priority for the maintainers for quite a while. It seems like the failing CI is related to jobs using outdated versions of ubuntu and centos. I will remove those jobs in a new PR.

Keep up the good work, best @bokchan

bokchan commented 4 years ago

@smanolache The failing CI pipeline should be fixed if you rebase on the latest v6.x.x

codecov-commenter commented 4 years ago

Codecov Report

Merging #345 into v6.x.x will decrease coverage by 1.24%. The diff coverage is 3.53%.

smanolache commented 4 years ago

Rebased. All tests pass.

bokchan commented 4 years ago

LGTM