phplist3 icon indicating copy to clipboard operation
phplist3 copied to clipboard

Specify MYSQLI_CLIENT_SSL flag when attempting SSL/TLS secured MySQL connections

Open jjthiessen opened this issue 4 years ago • 8 comments

Description

MYSQLI_CLIENT_SSL_DONT_VERIFY_SERVER_CERT does not seem to imply MYSQLI_CLIENT_SSL. This PR adds the MYSQLI_CLIENT_SSL flag when SSL/TLS secured MySQL connections are desired.

Related Issue

Please see https://github.com/phpList/phplist3/issues/833

jjthiessen avatar Nov 29 '21 23:11 jjthiessen

Thanks for that. It will take a while to review this, but sounds like an interesting issue, and potentially good to resolve.

michield avatar Nov 30 '21 21:11 michield

I agree with @jjthiessen that the php documentation looks to be incorrect, or at least does not match my own tests

Like MYSQLI_CLIENT_SSL, but disables validation of the provided SSL certificate. This is only for installations using MySQL Native Driver and MySQL 5.6 or later.

I tried running his test program on my desktop, which does not have certificates, and the only test that worked was using just the MYSQLI_CLIENT_SSL_DONT_VERIFY_SERVER_CERT flag.

But running the same test on my shared hosting account failed with

Undefined constant "MYSQLI_CLIENT_SSL_DONT_VERIFY_SERVER_CERT"

Maybe a more flexible solution is to allow people to specify their own value for the flags parameter in config.php, e.g.

define('MYSQLI_CONNECTION_FLAGS', MYSQLI_CLIENT_SSL | MYSQLI_CLIENT_SSL_DONT_VERIFY_SERVER_CERT | MYSQLI_CLIENT_COMPRESS);

with a default value of 0.

bramley avatar Dec 01 '21 21:12 bramley

https://bugs.php.net/bug.php?id=68344#1446123152 suggests that from 5.6.16 onward, specifying only MYSQLI_CLIENT_SSL should work for connecting to MySQL servers with SSL enforced (without doing any CN or SAN checks). It sounds like MYSQLI_CLIENT_SSL_DONT_VERIFY_SERVER_CERT is only needed if you use mysqli_ssl_set (which would be needed if we wanted to support client certificate authentication), and then only if you don't want full certificate verification.

The way the connection setup is written presently, I think that simply replacing the MYSQLI_CLIENT_SSL_DONT_VERIFY_SERVER_CERT flag with the MYSQLI_CLIENT_SSL flag might actually be the easiest right thing to do.

For PHP versions 5.6 through 5.6.15, I don't know if there was a way to use SSL for MySQL without certificate verification; for other versions, I don't know if you can use SSL for MySQL with certificate verification without calling mysqli_ssl_set.

MYSQLI_CLIENT_SSL_DONT_VERIFY_SERVER_CERT seems to be defined for me in PHP 7.4.24 (and was presumably introduced in PHP 5.6.16). Out of curiosity, @bramley, what version are you using on your shared hosting server?

jjthiessen avatar Dec 01 '21 21:12 jjthiessen

I also think that exposing the mysqli_real_connect flags parameter directly in/through config.php would be a reasonable way to go. For compatibility reasons, it might be good to bit-wise or values together based on other existing configuration parameters (if the corresponding flags are defined). Something like this:

mysql_real_connect(..., (defined('MYSQLI_CONNECTION_FLAGS') ? MYSQLI_CONNECTION_FLAGS : 0)
    | (!empty($database_connection_ssl) && defined('MYSQLI_CLIENT_SSL') ? MYSQLI_CLIENT_SSL : 0)
    | (!empty($database_connection_ssl) && defined('MYSQLI_CLIENT_SSL_DONT_VERIFY_SERVER_CERT') ? MYSQLI_CLIENT_SSL_DONT_VERIFY_SERVER_CERT : 0)
    | ...
);

An additional consideration with exposing flags directly is that for Docker deployments, people will either have to precompute the relevant bits, or something like the following should be added to the stock config.php:

define('MYSQLI_CONNECTION_FLAGS', array_reduce(explode(' ', getenv('DB_CONNECTION_FLAGS', '')), function ($settings, $flag) {
    return $settings | (defined($flag) ? constant($flag) : 0);
}, 0));

jjthiessen avatar Dec 01 '21 22:12 jjthiessen

@michield, you wouldn't happen to remember the reason for the opposite change in https://github.com/phpList/phplist3/commit/a3bc7189b8b3d048af3a5c685bcc53358af42046, would you?

I've only looked at GitHub/the git log. Perhaps there are more details in Mantis?

jjthiessen avatar Dec 07 '21 05:12 jjthiessen

Interesting. I cannot remember that at all :-)

This search returns some issues in mantis: https://www.google.com/search?q=MYSQLI_CLIENT_SSL_DONT_VERIFY_SERVER_CERT+site%3Amantis.phplist.org&sxsrf=AOaemvKhBl8jI9qlpavDjzwOAyY17TUGIg%3A1638910846556&source=hp&ei=fsuvYYq5H5-IjLsPq5yZqAQ&iflsig=ALs-wAMAAAAAYa_ZjhRZAvfWLIpvYXv3IiF38rnZHBxS&ved=0ahUKEwiK8MzWytL0AhUfBGMBHStOBkUQ4dUDCAk&uact=5&oq=MYSQLI_CLIENT_SSL_DONT_VERIFY_SERVER_CERT+site%3Amantis.phplist.org&gs_lcp=Cgdnd3Mtd2l6EAM6BwgjEOoCECc6BQgAEIAEOgYIABAKEB46CAgAEAUQChAeOgYIABANEB46CAgAEA0QBRAeOgcIIRAKEKABUNMIWKMvYJMwaAFwAHgAgAG5AYgBqxWSAQUxMC4xNZgBAKABAqABAbABCg&sclient=gws-wiz

michield avatar Dec 07 '21 21:12 michield

Looks like it was #84 where this was done

michield avatar Dec 07 '21 21:12 michield

My guess is that when you were testing the $secure switch, there may have been an issue with your mysqld SSL configuration (e.g., it wasn't configured/enabled, something was wrong with the provided cert/key pair, etc), and that switching from MYSQLI_CLIENT_SSL to MYSQLI_CLIENT_SSL_DONT_VERIFY_SERVER_CERT made things work because the client stopped trying to use SSL. This is purely conjecture.

As far as I know, using MYSQLI_CLIENT_SSL_DONT_VERIFY_SERVER_CERT by itself never caused SSL to be enabled for connections (and, moreover, I'm pretty sure that flag is only useful when also using mysqli_ssl_set to setup/configure the connection).

There have been a few ideas/thoughts tossed around here (some in passing):

  1. My original thought of changing MYSQLI_CLIENT_SSL_DONT_VERIFY_SERVER_CERT to be MYSQLI_CLIENT_SSL | MYSQLI_CLIENT_SSL_DONT_VERIFY_SERVER_CERT
  2. Switching from using MYSQLI_CLIENT_SSL_DONT_VERIFY_SERVER_CERT back to using MYSQLI_CLIENT_SSL
  3. Add support for client SSL/TLS authentication by adding additional configuration options and using mysqli_ssl_set (in which case MYSQLI_CLIENT_SSL_DONT_VERIFY_SERVER_CERT would be useful if you didn't want to validate the certificates used)
  4. Add a configuration option for specifying MySQLi connection flags manually/directly, likely ORing this together with flags implied by currently available options (there might be additional considerations here for specifying flags by name in environments where configuration is provided via environment variables instead of being hardcoded in PHP files)

I now think that approach 1. is misguided, as I'm fairly certain that the MYSQLI_CLIENT_SSL_DONT_VERIFY_SERVER_CERT is useless if you aren't also calling mysqli_ssl_set to configure SSL/TLS for the connection.

I think that 2. is the simplest approach to fixing this issue. This is what I'm presently using in production.

Approach 3. sounds great to me, but I've never used mTLS/client SSL/TLS authentication for/with MySQL, so I'm probably not the right person to submit a PR for this.

Option 4. adds additional flexibility (with the question of which flag(s) the existing $secure option should imply), and is not terribly involved, but requires some design decisions that I'd want feedback on from a maintainer.

For what I need, 2. suffices, so I've amended this PR to follow that approach.

jjthiessen avatar Dec 17 '21 05:12 jjthiessen

This pull request has been mentioned on phpList Discuss. There might be relevant details there:

https://discuss.phplist.org/t/is-php-8-x-x-supported-by-phplist/8371/16

phpListDockerBot avatar Oct 15 '22 06:10 phpListDockerBot

Due to #901 Github closed this PR. Please re-open it against the main branch.

michield avatar Oct 23 '22 19:10 michield