Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
timnolte opened this issue Jul 22, 2021 · 11 comments
Open

Comments

@timnolte
Copy link
Contributor

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
Copy link
Contributor

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
Copy link
Contributor Author

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

@timnolte
Copy link
Contributor Author

timnolte commented Aug 3, 2021

@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
Copy link
Contributor

@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:

wp-redis/object-cache.php

Lines 1200 to 1213 in 7dae448

public function prepare_client_connection( $client_parameters ) {
$redis = new Redis;
$redis->connect(
$client_parameters['host'],
$client_parameters['port'],
// $client_parameters['timeout'] is sent in milliseconds,
// connect() takes seconds, so divide by 1000
$client_parameters['timeout'] / 1000,
null,
$client_parameters['retry_interval']
);
return $redis;

@timnolte
Copy link
Contributor Author

timnolte commented Aug 4, 2021

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
Copy link
Contributor Author

timnolte commented Aug 4, 2021

@danielbachhuber 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

@timnolte
Copy link
Contributor Author

timnolte commented Aug 4, 2021

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
Copy link
Contributor

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
Copy link
Contributor Author

timnolte commented Aug 4, 2021

@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
Copy link
Contributor

@timnolte Were you able to track this issue down?

@timnolte
Copy link
Contributor Author

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants