Merge branch 'MDL-54704-db-ssl' of https://github.com/catalyst/moodle

This commit is contained in:
Andrew Nicols 2023-07-03 09:36:03 +08:00
commit d0214f0796
No known key found for this signature in database
GPG Key ID: 6D1E3157C8CFBF14
5 changed files with 152 additions and 3 deletions

View File

@ -82,6 +82,19 @@ $CFG->dboptions = array(
// has additional configuration according to its environment,
// which the administrator can specify to alter and
// override any connection options.
// 'ssl' => '', // A connection mode string from the list below.
// Not supported by all drivers.
// prefer Use SSL if available - postgres default Postgres only
// disable Force non secure connection Postgres only
// require Force SSL Postgres and MySQL
// verify-full Force SSL and verify root CA Postgres and MySQL
// All mode names are adopted from Postgres
// and other databases align where possible:
// Postgres: https://www.postgresql.org/docs/current/libpq-connect.html#LIBPQ-CONNECT-SSLMODE
// MySql: https://www.php.net/manual/en/mysqli.real-connect.php
// It is worth noting that for MySQL require and verify-full are the same - in both cases
// verification will take place if you specify hostname as a name,
// and it will be omitted if you put an IP address.
// 'fetchbuffersize' => 100000, // On PostgreSQL, this option sets a limit
// on the number of rows that are fetched into
// memory when doing a large recordset query

View File

@ -41,6 +41,12 @@ class mysqli_native_moodle_database extends moodle_database {
can_use_readonly as read_slave_can_use_readonly;
}
/** @var array $sslmodes */
private static $sslmodes = [
'require',
'verify-full'
];
/** @var mysqli $mysqli */
protected $mysqli = null;
/** @var bool is compressed row format supported cache */
@ -533,6 +539,8 @@ class mysqli_native_moodle_database extends moodle_database {
* @param mixed $prefix string means moodle db prefix, false used for external databases where prefix not used
* @param array $dboptions driver specific options
* @return bool success
* @throws moodle_exception
* @throws dml_connection_exception if error
*/
public function raw_connect(string $dbhost, string $dbuser, string $dbpass, string $dbname, $prefix, array $dboptions=null): bool {
$driverstatus = $this->driver_installed();
@ -577,6 +585,16 @@ class mysqli_native_moodle_database extends moodle_database {
if ($this->dboptions['clientcompress'] ?? false) {
$flags |= MYSQLI_CLIENT_COMPRESS;
}
if (isset($this->dboptions['ssl'])) {
$sslmode = $this->dboptions['ssl'];
if (!in_array($sslmode, self::$sslmodes, true)) {
throw new moodle_exception("Invalid 'dboptions''ssl' value '$sslmode'");
}
$flags |= MYSQLI_CLIENT_SSL;
if ($sslmode === 'verify-full') {
$flags |= MYSQLI_CLIENT_SSL_VERIFY_SERVER_CERT;
}
}
$conn = null;
$dberr = null;
@ -587,7 +605,7 @@ class mysqli_native_moodle_database extends moodle_database {
$dberr = "$e";
}
if (!$conn) {
$dberr = $dberr ?: $this->mysqli->connect_error;
$dberr = $dberr ?: "{$this->mysqli->connect_error} ({$this->mysqli->connect_errno})";
$this->mysqli = null;
throw new dml_connection_exception($dberr);
}

View File

@ -44,6 +44,14 @@ class pgsql_native_moodle_database extends moodle_database {
query_end as read_slave_query_end;
}
/** @var array $sslmodes */
private static $sslmodes = [
'disable',
'prefer',
'require',
'verify-full'
];
/** @var array $serverinfo cache */
private $serverinfo = [];
@ -130,6 +138,7 @@ class pgsql_native_moodle_database extends moodle_database {
* @param mixed $prefix string means moodle db prefix, false used for external databases where prefix not used
* @param array $dboptions driver specific options
* @return bool true
* @throws moodle_exception
* @throws dml_connection_exception if error
*/
public function raw_connect(string $dbhost, string $dbuser, string $dbpass, string $dbname, $prefix, array $dboptions=null): bool {
@ -187,6 +196,14 @@ class pgsql_native_moodle_database extends moodle_database {
$connection .= " options='" . implode(' ', $options) . "'";
}
if (isset($this->dboptions['ssl'])) {
$sslmode = $this->dboptions['ssl'];
if (!in_array($sslmode, self::$sslmodes, true)) {
throw new moodle_exception('validateerrorlist', 'admin', '', "'dboptions''ssl': $sslmode");
}
$connection .= " sslmode=$sslmode";
}
ob_start();
// It seems that pg_connect() handles some errors differently.
// For example, name resolution error will raise an exception, and non-existing

View File

@ -48,10 +48,11 @@ class mysqli_native_moodle_database_test extends \advanced_testcase {
* SSL connection helper.
*
* @param bool|null $compress
* @param string|null $ssl
* @return mysqli
* @throws moodle_exception
*/
public function new_connection(?bool $compress = false): mysqli {
public function new_connection(?bool $compress = false, ?string $ssl = null): mysqli {
global $DB;
// Open new connection.
@ -61,6 +62,7 @@ class mysqli_native_moodle_database_test extends \advanced_testcase {
}
$cfg->dboptions['clientcompress'] = $compress;
$cfg->dboptions['ssl'] = $ssl;
// Get a separate disposable db connection handle with guaranteed 'readonly' config.
$db2 = moodle_database::get_driver_instance($cfg->dbtype, $cfg->dblibrary);
@ -106,4 +108,34 @@ class mysqli_native_moodle_database_test extends \advanced_testcase {
$this->assertLessThan($sent, $sentc);
}
/**
* Test SSL connection.
*
* Well as much as we can, mysqli does not reliably report connect errors.
* @return void
*/
public function test_ssl_connection(): void {
try {
$mysqli = $this->new_connection(false, 'require');
// Either connect ...
$this->assertNotNull($mysqli);
} catch (moodle_exception $e) {
// ... or fail.
// Unfortunately we cannot be sure with the error string.
$this->markTestIncomplete('SSL not supported?');
}
try {
$mysqli = $this->new_connection(false, 'verify-full');
// Either connect ...
$this->assertNotNull($mysqli);
} catch (moodle_exception $e) {
// ... or fail with invalid cert.
// Same as above, but we cannot really expect properly signed cert, so ignore.
}
$this->expectException(moodle_exception::class);
$this->new_connection(false, 'invalid-mode');
}
}

View File

@ -23,6 +23,12 @@
* @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
*/
namespace core;
use stdClass, ReflectionClass;
use moodle_database, pgsql_native_moodle_database;
use xmldb_table;
use moodle_exception;
/**
* Test specific features of the Postgres dml.
@ -31,8 +37,9 @@
* @category test
* @copyright 2020 Ruslan Kabalin
* @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
* @covers \pgsql_native_moodle_database
*/
class pgsql_native_moodle_database_test extends advanced_testcase {
class pgsql_native_moodle_database_test extends \advanced_testcase {
/**
* Setup before class.
@ -358,4 +365,66 @@ class pgsql_native_moodle_database_test extends advanced_testcase {
$oneint = array_column($stored, 'oneint');
$this->assertEquals([-1], $oneint);
}
/**
* SSL connection helper.
*
* @param mixed $ssl
* @return resource|PgSql\Connection
* @throws moodle_exception
*/
public function new_connection($ssl) {
global $DB;
// Open new connection.
$cfg = $DB->export_dbconfig();
if (!isset($cfg->dboptions)) {
$cfg->dboptions = [];
}
$cfg->dboptions['ssl'] = $ssl;
// Get a separate disposable db connection handle with guaranteed 'readonly' config.
$db2 = moodle_database::get_driver_instance($cfg->dbtype, $cfg->dblibrary);
$db2->raw_connect($cfg->dbhost, $cfg->dbuser, $cfg->dbpass, $cfg->dbname, $cfg->prefix, $cfg->dboptions);
$reflector = new ReflectionClass($db2);
$rp = $reflector->getProperty('pgsql');
$rp->setAccessible(true);
return $rp->getValue($db2);
}
/**
* Test SSL connection.
*
* @return void
* @covers ::raw_connect
*/
public function test_ssl_connection(): void {
$pgconnerr = 'pg_connect(): Unable to connect to PostgreSQL server:';
try {
$pgsql = $this->new_connection('require');
// Either connect ...
$this->assertNotNull($pgsql);
} catch (moodle_exception $e) {
// ... or fail with SSL not supported.
$this->assertStringContainsString($pgconnerr, $e->debuginfo);
$this->assertStringContainsString('server does not support SSL', $e->debuginfo);
$this->markTestIncomplete('SSL not supported.');
}
try {
$pgsql = $this->new_connection('verify-full');
// Either connect ...
$this->assertNotNull($pgsql);
} catch (moodle_exception $e) {
// ... or fail with invalid cert.
$this->assertStringContainsString($pgconnerr, $e->debuginfo);
$this->assertStringContainsString('change sslmode to disable server certificate verification', $e->debuginfo);
}
$this->expectException(moodle_exception::class);
$this->new_connection('invalid-mode');
}
}