pantheon-systems / wp-redis

WordPress Object Cache using Redis.
https://wordpress.org/plugins/wp-redis/
GNU General Public License v2.0
228 stars 68 forks source link

Support Environment Configuration for Enabling TLS When WP-Predis is used. #322

Open timnolte opened 3 years ago

timnolte commented 3 years ago

Our hope has been to standardize on this plugin regardless of whether we are hosting client sites with Pantheon or AWS hosting. One challenge is that we want to be able to turn on TLS when using with AWS but the only way to do this is with the HumanMade WP-Predis add-on. However, there is no facility in WP Redis to enable TLS via a global constant like the other configuration items (CACHE_...). We are considering the Redis Object Cache plugin since it does support both of those however it doesn't support running on Pantheon out-of-the-box based on environment configuration, since it is using different configuration constants. I may submit a PR for this feature unless I'm missing it when looking at the code and I can accomplish this.

danielbachhuber commented 3 years ago

However, there is no facility in WP Redis to enable TLS via a global constant like the other configuration items (CACHE_...). [...] I may submit a PR for this feature unless I'm missing it when looking at the code and I can accomplish this.

I'm definitely open to looking at a pull request if you need some code changes!

According to the phpredis docs, it may be as easy as prefixing the host with tls://:

image

Let me know what your research yields!

timnolte commented 3 years ago

Ooh, I totally missed that it could be as simple adding tls to the address. I'll see if that will work.

timnolte commented 3 years ago

@danielbachhuber so by setting CACH_HOST and appending tls:// it appears that your plugin is ignoring any protocol in the host and always connecting via TCP:

[03-Aug-2021 20:13:15 UTC] PHP Warning: WP Redis: php_network_getaddresses: getaddrinfo failed: Try again [tcp://tls://redis:6379] in /var/www/html/public/wp-content/plugins/wp-redis/object-cache.php on line 1389

danielbachhuber commented 3 years ago

@timnolte That's odd. It seems to work locally for me with this in my wp-config.php:

$redis_server = [
    'host' => 'tls://127.0.0.1',
    'port' => 6379,
];

Can you share your configuration?

Also, you could try directly entering your values into $redis->connect() and seeing if you can get to a combination that works:

https://github.com/pantheon-systems/wp-redis/blob/7dae4488e68ac71a40d7208581651e1ad758ac1b/object-cache.php#L1200-L1213

timnolte commented 3 years ago

So further digging seems to point to the fact that this is an issue between trying to use PhpRedis vs Predis. I missed your note that called out the PhpRedis documentation specifically and thought you were referring to PRedis. So I think that tcp://tls:// issue was related to the Predis library and non WP Redis. Regardless simply adding the tls:// protocol results in an error about needing a tls wrapper:

PHP Warning:  file_exists(): Unable to find the wrapper "tls" - did you forget to enable it when you configured PHP? in .../services/wordpress/public/wp-content/plugins/wp-redis/object-cache.php on line 1174

I did realize that I can't test this on my local Docker since the Redis Docker image doesn't support TLS since no SSL/TLS tunnel has been setup with it. But testing in our development environment with AWS ElasticCache Redis didn't work resulting in that wrapper error.

timnolte commented 3 years ago

@danielbachhuber I also found this related issue for PhpRedis that seemed to indicate that PhpRedis doesn't actually support TLS out-of -the-box: https://github.com/phpredis/phpredis/issues/1706

timnolte commented 3 years ago

Switching to Predis might work but the addon library doesn't support an setting that via a constant configuration variable or looking for tls:// in the host setting. https://github.com/humanmade/wp-redis-predis-client/blob/master/functions.php#L52

danielbachhuber commented 3 years ago

So further digging seems to point to the fact that this is an issue between trying to use PhpRedis vs Predis. I missed your note that called out the PhpRedis documentation specifically and thought you were referring to PRedis.

That makes sense then.

Regardless simply adding the tls:// protocol results in an error about needing a tls wrapper:

This seems fixable with something like #323

But testing in our development environment with AWS ElasticCache Redis didn't work resulting in that wrapper error.

Besides the wrapper error, what else didn't work about it? Can you share more of the diagnostic details you saw?

I also found this related issue for PhpRedis that seemed to indicate that PhpRedis doesn't actually support TLS out-of -the-box: phpredis/phpredis#1706

I don't fully understand all of the details in that thread but it seems that issue is a bit more specific to AWS?

timnolte commented 3 years ago

@danielbachhuber OK, so I just looked ate that PR and I'm wonde ring if that might be our issue then. With that check failing there was no connection being made to Redis at all. In our case we are using an AWS ElasticCache Redis instance. I will see about possibly getting TLS turned back in with our instance and test that code change

danielbachhuber commented 3 years ago

@timnolte Were you able to track this issue down?

timnolte commented 3 years ago

@danielbachhuber I haven't had a chance to coordinate with our SREa to have TLS turned back on for the Redis instance.