Opened 7 years ago
Last modified 13 months ago
#28625 assigned enhancement
Enhancement: Add constants to support SSL connections for mysqli
Reported by: |
|
Owned by: | |
---|---|---|---|
Milestone: | Future Release | Priority: | normal |
Severity: | normal | Version: | 4.0 |
Component: | Database | Keywords: | has-patch needs-refresh needs-unit-tests |
Focuses: | Cc: |
Description
In order to support SSL'ed MySQL connections with the mysqli_*
functions introduced in WordPress 3.9 / PHP 5.5 mysqli_ssl_set()
must be called to set the path to the SSL certs and keys to be used by the MySQL client before mysqli_real_connect()
is called.
We should add the following optional constants to allow for users to configure secure connections to the database.
MYSQL_SSL_KEY
MYSQL_SSL_CERT
MYSQL_SSL_CA
MYSQL_SSL_CA_PATH
MYSQL_SSL_CIPHER
In addition this should only be set if the feature flag MYSQLI_CLIENT_SSL
is set for the existing MYSQL_CLIENT_FLAGS
constant.
Attachments (3)
Change History (20)
#1
@
7 years ago
- Keywords has-patch added
- Summary changed from Enhancement: Add constants for support SSL connections for mysqli to Enhancement: Add constants to support SSL connections for mysqli
#2
@
7 years ago
I agree this should be included. But, the patch shouldn't silently drop the certificate file if the file does not exist. This would allow the connection to be opened without SSL; assuming the MySQL server allows that.
It would be better to pass the parameter as given, and allow the mysqli_real_connect call to fail. This seems to be the intended idea, as the PHP docs for mysqli_ssl_set indicate it always returns TRUE (http://www.php.net/manual/en/mysqli.ssl-set.php).
#3
@
7 years ago
Your proposed method works for me. But just for good measure I think we should add a comment in the if ( $call_set )
block, maybe something along the lines of...
// SSL added here! :-)
#7
@
6 years ago
Keep in mind that solution based on constants will force SSL for every new connection so there could be problem when creating new wpdb for database which doesn't support SSL. I think SSL should be set per wpdb object.
#8
@
5 years ago
@pento, any thoughts on this?
How do other software projects handle configuration for this? I presume they do not use constants, and thus can use an array (in the case of PHP projects).
Replying to szaqal21:
Keep in mind that solution based on constants will force SSL for every new connection so there could be problem when creating new wpdb for database which doesn't support SSL. I think SSL should be set per wpdb object.
I agree with the concern here.
#9
follow-up:
↓ 10
@
5 years ago
- Keywords needs-refresh needs-unit-tests added; dev-feedback 4.1-early removed
- Milestone changed from Future Release to 4.5
Other projects generally use PDO, and do their DB config as arrays, instead of constants.
mysqli_ssl_set()
was added in PHP 5.3 - we probably should add a function_exists()
check, even though it'd really only be a problem if someone has defined WP_USE_EXT_MYSQL
as false
in PHP 5.2. (And maybe fail to connect in this case, so that we're not silently using a non-SSL connection.)
For the concern about new wpdb instances, perhaps we need to add an $options
parameter to wpdb::__construct()
? $options['ssl']
would be an array of the SSL options (if your global config doesn't do SSL, or you want different SSL options), or false
to prevent an SSL connection. Undefined uses the global config.
For the actual patch itself, I'm not wild about adding a pile of constants, but that's kind of the best option that keeps with the WordPress-y way. I can't decide if the foreach
/ call_user_func_array
combination is too clever, or just the right amount of clever. It'd need to be reworked to work with $options
, anyway. Also, it needs unit tests.
With all that said, I find myself being in favour of adding this functionality to Core, instead of recommending a drop-in. It's useful, it increases security, and I don't think it's possible to render your site unrecoverably broken (worst case: remove the constants from wp-config.php
) with it.
#10
in reply to:
↑ 9
@
5 years ago
Replying to pento:
With all that said, I find myself being in favour of adding this functionality to Core, instead of recommending a drop-in. It's useful, it increases security, and I don't think it's possible to render your site unrecoverably broken (worst case: remove the constants from
wp-config.php
) with it.
100% agree with the final sentence. In this day and age there is no excuse not to use secure communications. Hope this does make it to Core.
This ticket was mentioned in Slack in #core by jorbin. View the logs.
5 years ago
#14
@
4 years ago
I agree with most of what pento suggests as well but in the meantime I have published the plugin I've been using to set SSL flags for connections as a stop gap.
Initial proposed patch