MDL-81794 core: Fix the TLS issue on the Redis

The fix covers two places:
- The Redis session.
- The Redis cache store.

The PHP Redis has the option to establish the connection using TLS/SSL instead of using the "tls://" prefix.
This is done to ensure consistency with the Redis cluster connection,
as the code for both single and cluster connections is located in one place.
This commit is contained in:
meirzamoodle 2024-05-21 23:48:17 +07:00
parent f49d120761
commit ba8cb7874f
3 changed files with 27 additions and 16 deletions

View File

@ -234,6 +234,15 @@ class cachestore_redis extends cache_store implements cache_is_key_aware, cache_
// We only need the first record for the single redis.
if (!$clustermode) {
// Handle the case when the server is not a Unix domain socket.
if ($port !== 0) {
// We only need the first record for the single redis.
$serverchunks = explode(':', $trimmedservers[0]);
// Get the last chunk as the port.
$port = array_pop($serverchunks);
// Combine the rest of the chunks back into a string as the server.
$server = implode(':', $serverchunks);
}
break;
}
}
@ -259,8 +268,6 @@ class cachestore_redis extends cache_store implements cache_is_key_aware, cache_
if ($clustermode) {
$redis = new RedisCluster(null, $trimmedservers, 1, 1, true, $password, !empty($opts) ? $opts : null);
} else {
// We only need the first record for the single redis.
list($server, $port) = explode(':', $trimmedservers[0]);
$redis = new Redis();
$redis->connect($server, $port, 1, null, 100, 1, $opts);
if (!empty($password)) {

View File

@ -117,9 +117,6 @@ class redis extends handler implements SessionHandlerInterface {
}
if (isset($CFG->session_redis_encrypt) && $CFG->session_redis_encrypt) {
if (!$this->clustermode) {
$this->host[0] = 'tls://' . $this->host[0];
}
$this->sslopts = $CFG->session_redis_encrypt;
}
@ -228,7 +225,7 @@ class redis extends handler implements SessionHandlerInterface {
$port = 0;
$trimmedservers[] = $server;
} else {
$port = 6379; // No Unix socket so set default port.
$port = $this->port ?? 6379; // No Unix socket so set default port.
if (strpos($server, ':')) { // Check for custom port.
list($server, $port) = explode(':', $server);
}
@ -237,6 +234,12 @@ class redis extends handler implements SessionHandlerInterface {
// We only need the first record for the single redis.
if (!$this->clustermode) {
// Handle the case when the server is not a Unix domain socket.
if ($port !== 0) {
list($server, ) = explode(':', $trimmedservers[0]);
} else {
$server = $trimmedservers[0];
}
break;
}
}
@ -266,9 +269,8 @@ class redis extends handler implements SessionHandlerInterface {
$this->auth, !empty($opts) ? $opts : null);
} else {
$delay = rand(100, 500);
list($server, $port) = explode(':', $trimmedservers[0]);
$this->connection = new \Redis();
$this->connection->connect($server, $this->port ?? $port, 1, null, $delay, 1, $opts);
$this->connection->connect($server, $port, 1, null, $delay, 1, $opts);
if ($this->auth !== '' && !$this->connection->auth($this->auth)) {
throw new $exceptionclass('Unable to authenticate.');
}
@ -296,7 +298,7 @@ class redis extends handler implements SessionHandlerInterface {
}
return true;
} catch (RedisException | RedisClusterException $e) {
$redishost = $this->clustermode ? implode(',', $this->host) : $this->host[0].':'.$this->port ?? $port;
$redishost = $this->clustermode ? implode(',', $this->host) : $server . ':' . $port;
$logstring = "Failed to connect (try {$counter} out of " . self::MAX_RETRIES . ") to Redis ";
$logstring .= "at ". $redishost .", the error returned was: {$e->getMessage()}";
debugging($logstring);

View File

@ -69,13 +69,14 @@ class session_redis_test extends \advanced_testcase {
$this->keyprefix = 'phpunit'.rand(1, 100000);
$CFG->session_redis_host = TEST_SESSION_REDIS_HOST;
if (strpos(TEST_SESSION_REDIS_HOST, ':')) {
list($server, $port) = explode(':', TEST_SESSION_REDIS_HOST);
} else {
$server = TEST_SESSION_REDIS_HOST;
$port = 6379;
}
$CFG->session_redis_host = $server;
$CFG->session_redis_port = $port;
$opts = [];
if (defined('TEST_SESSION_REDIS_ENCRYPT') && TEST_SESSION_REDIS_ENCRYPT) {
@ -344,10 +345,10 @@ class session_redis_test extends \advanced_testcase {
$actual = $e->getMessage();
}
$host = TEST_SESSION_REDIS_HOST;
if ($this->encrypted) {
$host = "tls://$host";
}
// The Redis session test config allows the user to put the port number inside the host. e.g. 127.0.0.1:6380.
// Therefore, to get the host, we need to explode it.
list($host, ) = explode(':', TEST_SESSION_REDIS_HOST);
$expected = "Failed to connect (try 5 out of 5) to Redis at $host:111111";
$this->assertDebuggingCalledCount(5);
$this->assertStringContainsString($expected, $actual);
@ -367,7 +368,8 @@ class session_redis_test extends \advanced_testcase {
$sess = new \core\session\redis();
$prop = new \ReflectionProperty(\core\session\redis::class, 'host');
$this->assertEquals('tls://' . TEST_SESSION_REDIS_HOST, $prop->getValue($sess)[0]);
$prop = new \ReflectionProperty(\core\session\redis::class, 'sslopts');
$this->assertEquals($CFG->session_redis_encrypt, $prop->getValue($sess));
}
}