Skip to content

Commit

Permalink
MDL-79121 lib: Normalize proxybypass value
Browse files Browse the repository at this point in the history 
This changes prevent the admin to define an incorrect proxybypass value.

Dropped out formats in MDL-74289 are now fixed automatically.
e.g.: "192.168." becomes "192.168.0.0/16" and ".domain.tld" becomes "*.domain.tld".
  • Loading branch information
@jboulen
jboulen committed Dec 16, 2024
1 parent aa55f38 commit 0e7d392
Show file tree
Hide file tree
Showing 4 changed files with 250 additions and 3 deletions.
11 changes: 9 additions & 2 deletions admin/settings/server.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -181,8 +181,15 @@
new lang_string('configproxyuser', 'admin'), ''));
$temp->add(new admin_setting_configpasswordunmask('proxypassword', new lang_string('proxypassword', 'admin'),
new lang_string('configproxypassword', 'admin'), ''));
$temp->add(new admin_setting_configtext('proxybypass', new lang_string('proxybypass', 'admin'),
new lang_string('configproxybypass', 'admin'), 'localhost, 127.0.0.1'));

$setting = new admin_setting_configtext('proxybypass', new lang_string('proxybypass', 'admin'),
new lang_string('configproxybypass', 'admin'), 'localhost,127.0.0.1');
$setting->set_updatedcallback(function() {
// Normalize $CFG->proxybypass value.
set_config('proxybypass', \core\ip_utils::normalize_internet_address_list(get_config('core', 'proxybypass')));
});
$temp->add($setting);

$temp->add(new admin_setting_configcheckbox('proxylogunsafe', new lang_string('proxylogunsafe', 'admin'),
new lang_string('configproxylogunsafe_help', 'admin'), 0));
$temp->add(new admin_setting_configcheckbox('proxyfixunsafe', new lang_string('proxyfixunsafe', 'admin'),
Expand Down
2 changes: 1 addition & 1 deletion lang/en/admin.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -341,7 +341,7 @@
$string['configprofileroles'] = 'Roles that are listed in user profiles and on the participants page.';
$string['configprofilesforenrolledusersonly'] = 'To prevent misuse by spammers, profile descriptions of users who are not yet enrolled in any course are hidden. New users must enrol in at least one course before they can add a profile description.';
$string['configprotectusernames'] = 'If enabled, the forgotten password form will not display any hints allowing account usernames or email addresses to be guessed.';
$string['configproxybypass'] = 'Comma separated list of (partial) hostnames or IPs that should bypass proxy (e.g., 192.168., .mydomain.com)';
$string['configproxybypass'] = 'List of (partial) hostnames or IP addresses that should bypass the proxy. Separate each item by commas, with no spaces. For example: 192.168.0.0/16,*.mydomain.com.';
$string['configproxyhost'] = 'If this <b>server</b> needs to use a proxy computer (eg a firewall) to access the Internet, then provide the proxy hostname here. Otherwise leave it blank.';
$string['configproxypassword'] = 'Password needed to access internet through proxy if required, empty if none (PHP cURL extension required).';
$string['configproxyport'] = 'If this server needs to use a proxy computer, then provide the proxy port here.';
Expand Down
89 changes: 89 additions & 0 deletions lib/classes/ip_utils.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -263,4 +263,93 @@ public static function get_ip_address(string $hostname): ?string {

return null;
}

/**
* Normalize internet address.
*
* Accepted input formats are :
* - a valid range or full ip address (e.g.: 192.168.0.0/16, fe80::ffff, 127.0.0.1 or fe80:fe80:fe80:fe80:fe80:fe80:fe80:fe80)
* - a valid domain name or pattern (e.g.: www.moodle.com or *.moodle.org)
*
* Convert forbidden syntaxes since MDL-74289 to allowed values. For examples:
* - 192.168. => 192.168.0.0/16
* - .domain.tld => *.domain.tld
*
* @param string $address The input string to normalize.
*
* @return string If $address is not normalizable, an empty string is returned.
*/
public static function normalize_internet_address(string $address): string {
$address = str_replace([" ", "\n", "\r", "\t", "\v", "\x00"], '', strtolower($address));

// Replace previous allowed "192.168." format to CIDR format (192.168.0.0/16).
if (str_ends_with($address, '.') && preg_match('/^[0-9\.]+$/', $address) === 1) {
$count = substr_count($address, '.');

// Remove final dot.
$address = substr($address, 0, -1);

// Fill address with missing ".0".
$address .= str_repeat('.0', 4 - $count);

// Add subnet mask.
$address .= '/' . ($count * 8);
}

if (ip_utils::is_ip_address($address) ||
ip_utils::is_ipv4_range($address) || ip_utils::is_ipv6_range($address)) {

// Keep full or range ip addresses.
return $address;
}

// Replace previous allowed ".domain.tld" format to "*.domain.tld" format.
if (str_starts_with($address, '.')) {
$address = '*'.$address;
}

// Usually the trailing dot (null label) is omitted, but is valid if supplied. We'll just remove it and validate as normal.
$address = rtrim($address, '.');

if (ip_utils::is_domain_name($address) || ip_utils::is_domain_matching_pattern($address)) {
// Keep valid or pattern domain name.
return $address;
}

// Return empty string for invalid values.
return '';
}

/**
* Normalize a list of internet addresses.
*
* This function will:
* - normalize internet addresses {@see normalize_internet_address()}
* - remove invalid values
* - remove duplicate values
*
* @param string $addresslist A string representing a list of internet addresses separated by a common value.
*
* @return string
*/
public static function normalize_internet_address_list(string $addresslist, string $separator = ','): string {
$addresses = [];
foreach (explode($separator, $addresslist) as $value) {
$address = self::normalize_internet_address($value);

if (empty($address)) {
// Ignore invalid input.
continue;
}

if (in_array($address, $addresses, true)) {
// Ignore duplicate value.
continue;
}

$addresses[] = $address;
}

return implode($separator, $addresses);
}
}
151 changes: 151 additions & 0 deletions lib/tests/ip_utils_test.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -400,4 +400,155 @@ public function test_is_ip_in_subnet_list($expected, $ip, $list, $delim): void {
$this->assertEquals($expected, \core\ip_utils::is_ip_in_subnet_list($ip, $list, $delim));
}

/**
* Data provider for test_normalize_internet_address.
*
* @return array
*/
public function normalize_internet_address_provider(): array {
return [
'Strip all white spaces on IP address' => [
' 192.168.5.5 ',
'192.168.5.5',
],
'Strip all white spaces on domain name' => [
' www.moodle.org ',
'www.moodle.org',
],
'Preserve IPv4 address' => [
'127.0.0.1',
'127.0.0.1',
],
'Preserve IPv4 address range' => [
'192.168.0.0/16',
'192.168.0.0/16',
],
'Preserve IPv6 address' => [
'fe80:fe80:fe80:fe80:fe80:fe80:fe80:fe80',
'fe80:fe80:fe80:fe80:fe80:fe80:fe80:fe80',
],
'Preserve IPv6 address range' => [
'fe80::ffff',
'fe80::ffff',
],
'Preserve valid domain' => [
'localhost',
'localhost',
],
'Preserve valid FQDN' => [
'www.moodle.org',
'www.moodle.org',
],
'Preserve valid FQDN with trailing dot' => [
'www.moodle.com.',
'www.moodle.com',
],
'Preserve valid domain with wildcard' => [
'*.moodledev.io',
'*.moodledev.io',
],
'Convert previous allowed "127." format to CIDR format (127.0.0.0/8)' => [
'127.',
'127.0.0.0/8',
],
'Convert previous allowed "169.8." format to CIDR format (169.8.0.0/16)' => [
'169.8.',
'169.8.0.0/16',
],
'Convert previous allowed "192.168.10." format to CIDR format (192.168.10.0/24)' => [
'192.168.10.',
'192.168.10.0/24',
],
'Convert previous allowed ".moodle.org" subdomain format to new format (*.moodle.org)' => [
'.moodle.org',
'*.moodle.org',
],
'Ignore invalid IPv4' => [
'327.0.0.1',
'',
],
'Ignore invalid IPv4 range' => [
'192.168',
'',
],
'Ignore invalid IPv6' => [
'fe80::ddddd',
'',
],
'Ignore invalid IPv6 range' => [
'fe80:',
'',
],
'Ignore invalid domain' => [
'-example.com',
'',
],
];
}

/**
* Test if input address value is correctly normalized.
*
* @covers ::normalize_internet_address
*
* @dataProvider normalize_internet_address_provider
*
* @param string $input Raw input value.
* @param string $expected Expected value after normalization.
*/
public function test_normalize_internet_address(string $input, string $expected): void {
$this->assertEquals($expected, \core\ip_utils::normalize_internet_address($input));
}

/**
* Data provider for test_normalize_internet_address_list.
*
* @return array
*/
public function normalize_internet_address_list_provider(): array {
return [
'Strip all white spaces' => [
' 192.168.5.5, 127.0.0.1, www.moodle.org ',
'192.168.5.5,127.0.0.1,www.moodle.org',
],
'Trim input' => [
' 192.168.5.5,127.0.0.1,www.moodle.org ',
'192.168.5.5,127.0.0.1,www.moodle.org',
],
'Preserve valid full and partial IP' => [
'127.0.0.1,192.168.0.0/16,fe80:fe80:fe80:fe80:fe80:fe80:fe80:fe80,fe80::ffff',
'127.0.0.1,192.168.0.0/16,fe80:fe80:fe80:fe80:fe80:fe80:fe80:fe80,fe80::ffff',
],
'Convert previous allowed format to new allowed format' => [
'127.,169.8.,192.168.10.,.moodle.org',
'127.0.0.0/8,169.8.0.0/16,192.168.10.0/24,*.moodle.org',
],
'Preserve valid domain and pattern domain' => [
'localhost,www.moodle.org,.moodle.com,*.moodledev.io',
'localhost,www.moodle.org,*.moodle.com,*.moodledev.io',
],
'Remove all invalid IP and domains' => [
'327.0.0.1,192.168,fe80::ddddd,fe80:,-example.com',
'',
],
'Remove duplicate values' => [
'.moodle.org,*.moodle.org,*.moodle.org,.moodle.org',
'*.moodle.org',
],
];
}

/**
* Test if input address list is correctly normalized.
*
* @covers ::normalize_internet_address_list
*
* @dataProvider normalize_internet_address_list_provider
*
* @param string $input Raw input value.
* @param string $expected Expected value after normalization.
*/
public function test_normalize_internet_address_list(string $input, string $expected): void {
$this->assertEquals($expected, \core\ip_utils::normalize_internet_address_list($input));
}
}

0 comments on commit 0e7d392

Please sign in to comment.