TLS/SSL Support

[DevHeaven]

Hello

I believe this package does not yet support connecting to Sentinel over TLS/SSL ?
Are you considering implementation ?
If it does exist, could you point me to where the configuration is done ?

Much appreciated

[Namoshek]

Based on the sentinel documentation of phpredis/phpredis, I'm not sure this is even supported?!

[DevHeaven]

I'm afraid you are right :'(
Thanks for the quick response!

Now to find a solution, because our production clusters have mandatory TLS

[DevHeaven]

But here I see a TLS commit : phpredis/phpredis#2114

[DevHeaven]

Here there are samples with TLS enabled : https://github.com/phpredis/phpredis?tab=readme-ov-file

[Namoshek]

Interesting find, maybe it is possible. This should mostly affect the following part of the code:

laravel-redis-sentinel/src/Connectors/PhpRedisSentinelConnector.php

Lines 84 to 129 in 3d6fad1

	private function connectToSentinel(array $config): RedisSentinel
	{
	$host = $config['sentinel_host'] ?? '';
	$port = $config['sentinel_port'] ?? 26379;
	$timeout = $config['sentinel_timeout'] ?? 0.2;
	$persistent = $config['sentinel_persistent'] ?? null;
	$retryInterval = $config['sentinel_retry_interval'] ?? 0;
	$readTimeout = $config['sentinel_read_timeout'] ?? 0;
	$username = $config['sentinel_username'] ?? '';
	$password = $config['sentinel_password'] ?? '';
	if (strlen(trim($host)) === 0) {
	throw new ConfigurationException('No host has been specified for the Redis Sentinel connection.');
	}
	$auth = null;
	if (strlen(trim($username)) !== 0 && strlen(trim($password)) !== 0) {
	$auth = [$username, $password];
	} elseif (strlen(trim($password)) !== 0) {
	$auth = $password;
	}
	if (version_compare(phpversion('redis'), '6.0', '>=')) {
	$options = [
	'host' => $host,
	'port' => $port,
	'connectTimeout' => $timeout,
	'persistent' => $persistent,
	'retryInterval' => $retryInterval,
	'readTimeout' => $readTimeout,
	];
	if ($auth !== null) {
	$options['auth'] = $auth;
	}
	return new RedisSentinel($options);
	}
	if ($auth !== null) {
	/** @noinspection PhpMethodParametersCountMismatchInspection */
	return new RedisSentinel($host, $port, $timeout, $persistent, $retryInterval, $readTimeout, $auth);
	}
	return new RedisSentinel($host, $port, $timeout, $persistent, $retryInterval, $readTimeout);
	}

However, the way the phpredis library works, you can only pass the object when its actually being used. So the workaround of L128 needs to be used for this as well, making the code a bit bulky. If you want to give it a try, feel free. Otherwise I'll see when I find the time.

[BramVanDaele]

I'm switching to my work account (same person as DevHeaven ;-)
I'll have a look. Thanks again

[BramVanDaele]

By the statement "workaround L128" , you mean the fact that RedisSentinel doesn't accept more then 6 function parameters ?
Not sure why it's not complaining about the $auth .... or I'm misunderstanding you.

[Namoshek]

Sorry if that wasn't very helpful. I'm not sure if the SSL context works similar to the auth options. With the auth options, the RedisSentinel class complains if you pass them as null. Instead, the parameter needs to be omitted entirely. Which, to be honest, is kinda problematic if you need to pass an argument after it... I guess that needs some fiddling and testing.

[BramVanDaele]

Hmmm, if RedisSentinel indeed requires it not to be null, then it would be impossible to provide any following parameters without the auth option (given we can't rewrite the function declaration ourselves).

One could argue that if you want to secure your connection with TLS , you would not want to use NOAUTH.
So , if TLS is to be used AUTH is mandatory. However, that limits the functionality for cases where this is required.

Could you work out an example of connecting with both AUTH and TLS/SSL Context ?

[BramVanDaele]

Apparantly the latest version 6.1 , released 4 weeks ago includes support for TLS/SSL : phpredis/phpredis@f2bb2cd#diff-16398f4eda044362a02548e382f2ec7dfd1aae291be015c075d2409928ddf295R11