From f6d9efefaac9ab4b658830b384adebbdf26e4e67 Mon Sep 17 00:00:00 2001 From: Jake Dallimore Date: Tue, 8 Nov 2016 14:40:38 +0800 Subject: [PATCH] MDL-48498 core_files: curl_security_helper_base and implementation Base class and core implementation providing a means to check URLs against the curl security admin settings entries. --- lib/classes/files/curl_security_helper.php | 254 ++++++++++++++++ .../files/curl_security_helper_base.php | 63 ++++ lib/phpunit/classes/util.php | 23 ++ lib/tests/curl_security_helper_test.php | 271 ++++++++++++++++++ 4 files changed, 611 insertions(+) create mode 100644 lib/classes/files/curl_security_helper.php create mode 100644 lib/classes/files/curl_security_helper_base.php create mode 100644 lib/tests/curl_security_helper_test.php diff --git a/lib/classes/files/curl_security_helper.php b/lib/classes/files/curl_security_helper.php new file mode 100644 index 00000000000..6c0639f8f81 --- /dev/null +++ b/lib/classes/files/curl_security_helper.php @@ -0,0 +1,254 @@ +. + +/** + * Contains a class providing functions used to check the host/port black/whitelists for curl. + * + * @package core + * @copyright 2016 Jake Dallimore + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + * @author Jake Dallimore + */ + +namespace core\files; +use core\ip_utils; + +defined('MOODLE_INTERNAL') || exit(); + +/** + * Host and port checking for curl. + * + * This class provides a means to check URL/host/port against the system-level cURL security entries. + * It does not provide a means to add URLs, hosts or ports to the black/white lists; this is configured manually + * via the site admin section of Moodle (See: 'Site admin' > 'Security' > 'HTTP Security'). + * + * This class is currently used by the 'curl' wrapper class in lib/filelib.php. + * Depends on: + * core\ip_utils (several functions) + * moodlelib (clean_param) + * + * @package core + * @copyright 2016 Jake Dallimore + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + * @author Jake Dallimore + */ +class curl_security_helper extends curl_security_helper_base { + /** + * @var array of supported transport schemes and their respective default ports. + */ + protected $transportschemes = [ + 'http' => 80, + 'https' => 443 + ]; + + /** + * Checks whether the given URL is blacklisted by checking its address and port number against the black/white lists. + * The behaviour of this function can be classified as strict, as it returns true for URLs which are invalid or + * could not be parsed, as well as those valid URLs which were found in the blacklist. + * + * @param string $urlstring the URL to check. + * @return bool true if the URL is blacklisted or invalid and false if the URL is not blacklisted. + */ + public function url_is_blocked($urlstring) { + // If no config data is present, then all hosts/ports are allowed. + if (!$this->is_enabled()) { + return false; + } + + // Try to parse the URL to get the 'host' and 'port' components. + try { + $url = new \moodle_url($urlstring); + $parsed['scheme'] = $url->get_scheme(); + $parsed['host'] = $url->get_host(); + $parsed['port'] = $url->get_port(); + } catch (\moodle_exception $e) { + // Moodle exception is thrown if the $urlstring is invalid. Treat as blocked. + return true; + } + + // The port will be empty unless explicitly set in the $url (uncommon), so try to infer it from the supported schemes. + if (!$parsed['port'] && $parsed['scheme'] && isset($this->transportschemes[$parsed['scheme']])) { + $parsed['port'] = $this->transportschemes[$parsed['scheme']]; + } + + if ($parsed['port'] && $parsed['host']) { + // Check the host and port against the blacklist/whitelist entries. + return $this->host_is_blocked($parsed['host']) || $this->port_is_blocked($parsed['port']); + } + return true; + } + + /** + * Returns a string message describing a blocked URL. E.g. 'This URL is blocked'. + * + * @return string the string error. + */ + public function get_blocked_url_string() { + return get_string('curlsecurityurlblocked', 'admin'); + } + + /** + * Checks whether the host portion of a url is blocked. + * The host portion may be a FQDN, IPv4 address or a IPv6 address wrapped in square brackets, as per standard URL notation. + * E.g. + * images.example.com + * 127.0.0.1 + * [0.0.0.0.0.0.0.1] + * The method logic is as follows: + * 1. Check the host component against the list of IPv4/IPv6 addresses and ranges. + * - This will perform a DNS forward lookup if required. + * 2. Check the host component against the list of domain names and wildcard domain names. + * - This will perform a DNS reverse lookup if required. + * + * @param string $host the host component of the URL to check against the blacklist. + * @return bool true if the host is both valid and blocked, false otherwise. + */ + protected function host_is_blocked($host) { + if (!$this->is_enabled() || empty($host) || !is_string($host)) { + return false; + } + + // Fix for square brackets in the 'host' portion of the URL (only occurs if an IPv6 address is specified). + $host = str_replace(array('[', ']'), '', $host); // RFC3986, section 3.2.2. + $blacklistedhosts = $this->get_blacklisted_hosts_by_category(); + + if (ip_utils::is_ip_address($host)) { + if ($this->address_explicitly_blocked($host)) { + return true; + } + + // Only perform a reverse lookup if there is a point to it (i.e. we have rules to check against). + if ($blacklistedhosts['domain'] || $blacklistedhosts['domainwildcard']) { + $hostname = gethostbyaddr($host); // DNS reverse lookup - supports both IPv4 and IPv6 address formats. + if ($hostname !== $host && $this->host_explicitly_blocked($hostname)) { + return true; + } + } + } else if (ip_utils::is_domain_name($host)) { + if ($this->host_explicitly_blocked($host)) { + return true; + } + + // Only perform a forward lookup if there are IP rules to check against. + if ($blacklistedhosts['ipv4'] || $blacklistedhosts['ipv6']) { + $hostip = gethostbyname($host); // DNS forward lookup - only returns IPv4 addresses! + if ($hostip !== $host && $this->address_explicitly_blocked($hostip)) { + return true; + } + } + } + return false; + } + + /** + * Checks whether the given port is blocked, as determined by its absence on the ports whitelist. + * Ports are assumed to be blocked unless found in the whitelist. + * + * @param integer|string $port the port to check against the ports whitelist. + * @return bool true if the port is blocked, false otherwise. + */ + protected function port_is_blocked($port) { + $portnum = intval($port); + // Intentionally block port 0 and below and check the int cast was valid. + if (empty($port) || (string)$portnum !== (string)$port || $port < 0) { + return true; + } + $allowedports = $this->get_whitelisted_ports(); + return !empty($allowedports) && !in_array($portnum, $allowedports); + } + + /** + * Convenience method to check whether we have any entries in the host blacklist or ports whitelist admin settings. + * If no entries are found at all, the assumption is that the blacklist is disabled entirely. + * + * @return bool true if one or more entries exist, false otherwise. + */ + public function is_enabled() { + return (!empty($this->get_whitelisted_ports()) || !empty($this->get_blacklisted_hosts())); + } + + /** + * Checks whether the input address is blocked by at any of the IPv4 or IPv6 address rules. + * + * @param string $addr the ip address to check. + * @return bool true if the address is covered by an entry in the blacklist, false otherwise. + */ + protected function address_explicitly_blocked($addr) { + $blockedhosts = $this->get_blacklisted_hosts_by_category(); + $iphostsblocked = array_merge($blockedhosts['ipv4'], $blockedhosts['ipv6']); + return address_in_subnet($addr, implode(',', $iphostsblocked)); + } + + /** + * Checks whether the input hostname is blocked by any of the domain/wildcard rules. + * + * @param string $host the hostname to check + * @return bool true if the host is covered by an entry in the blacklist, false otherwise. + */ + protected function host_explicitly_blocked($host) { + $blockedhosts = $this->get_blacklisted_hosts_by_category(); + $domainhostsblocked = array_merge($blockedhosts['domain'], $blockedhosts['domainwildcard']); + return ip_utils::is_domain_in_allowed_list($host, $domainhostsblocked); + } + + /** + * Helper to get all entries from the admin setting, as an array, sorted by classification. + * Classifications include 'ipv4', 'ipv6', 'domain', 'domainwildcard'. + * + * @return array of host/domain/ip entries from the 'curlsecurityblockedhosts' config. + */ + protected function get_blacklisted_hosts_by_category() { + // For each of the admin setting entries, check and place in the correct section of the config array. + $config = ['ipv6' => [], 'ipv4' => [], 'domain' => [], 'domainwildcard' => []]; + $entries = $this->get_blacklisted_hosts(); + foreach ($entries as $entry) { + if (ip_utils::is_ipv6_address($entry) || ip_utils::is_ipv6_range($entry)) { + $config['ipv6'][] = $entry; + } else if (ip_utils::is_ipv4_address($entry) || ip_utils::is_ipv4_range($entry)) { + $config['ipv4'][] = $entry; + } else if (ip_utils::is_domain_name($entry)) { + $config['domain'][] = $entry; + } else if (ip_utils::is_domain_matching_pattern($entry)) { + $config['domainwildcard'][] = $entry; + } + } + return $config; + } + + /** + * Helper that returns the whitelisted ports, as defined in the 'curlsecurityallowedport' setting. + * + * @return array the array of whitelisted ports. + */ + protected function get_whitelisted_ports() { + global $CFG; + return array_filter(explode("\n", $CFG->curlsecurityallowedport), function($entry) { + return !empty($entry); + }); + } + + /** + * Helper that returns the blacklisted hosts, as defined in the 'curlsecurityblockedhosts' setting. + * + * @return array the array of blacklisted host entries. + */ + protected function get_blacklisted_hosts() { + global $CFG; + return array_filter(array_map('trim', explode("\n", $CFG->curlsecurityblockedhosts)), function($entry) { + return !empty($entry); + }); + } +} diff --git a/lib/classes/files/curl_security_helper_base.php b/lib/classes/files/curl_security_helper_base.php new file mode 100644 index 00000000000..e7f440ccdc8 --- /dev/null +++ b/lib/classes/files/curl_security_helper_base.php @@ -0,0 +1,63 @@ +. + +/** + * Contains an abstract base class definition for curl security helpers. + * + * @package core + * @copyright 2016 Jake Dallimore + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + * @author Jake Dallimore + */ + +namespace core\files; + +defined('MOODLE_INTERNAL') || exit(); + +/** + * Security helper for the curl class. + * + * This class is intended as a base class for all curl security helpers. A curl security helper should provide a means to check + * a URL to determine whether curl should be allowed to request its content. It must also be able to return a simple string to + * explain that the URL is blocked, e.g. 'This URL is blocked'. + * + * Curl security helpers are currently used by the 'curl' wrapper class in lib/filelib.php. + * + * This class depends on: + * - nothing. + * + * @package core + * @copyright 2016 Jake Dallimore + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + * @author Jake Dallimore + */ +abstract class curl_security_helper_base { + + /** + * Check whether the input url should be blocked or not. + * + * @param string $url the url to check. + * @return bool true if the url is deemed to be blocked, false otherwise. + */ + abstract public function url_is_blocked($url); + + /** + * Returns a string, explaining that a URL is blocked. + * + * @return string the lang string indicating that the url has been blocked. + */ + abstract public function get_blocked_url_string(); +} \ No newline at end of file diff --git a/lib/phpunit/classes/util.php b/lib/phpunit/classes/util.php index a3d2b34cef9..6152374cb85 100644 --- a/lib/phpunit/classes/util.php +++ b/lib/phpunit/classes/util.php @@ -839,4 +839,27 @@ class phpunit_util extends testing_util { } } } + + /** + * Helper function to call a protected/private method of an object using reflection. + * + * Example 1. Calling a protected object method: + * $result = call_internal_method($myobject, 'method_name', [$param1, $param2], '\my\namespace\myobjectclassname'); + * + * Example 2. Calling a protected static method: + * $result = call_internal_method(null, 'method_name', [$param1, $param2], '\my\namespace\myclassname'); + * + * @param object|null $object the object on which to call the method, or null if calling a static method. + * @param string $methodname the name of the protected/private method. + * @param array $params the array of function params to pass to the method. + * @param string $classname the fully namespaced name of the class the object was created from (base in the case of mocks), + * or the name of the static class when calling a static method. + * @return mixed the respective return value of the method. + */ + public static function call_internal_method($object, $methodname, array $params = array(), $classname) { + $reflection = new \ReflectionClass($classname); + $method = $reflection->getMethod($methodname); + $method->setAccessible(true); + return $method->invokeArgs($object, $params); + } } diff --git a/lib/tests/curl_security_helper_test.php b/lib/tests/curl_security_helper_test.php new file mode 100644 index 00000000000..83b1cc47e65 --- /dev/null +++ b/lib/tests/curl_security_helper_test.php @@ -0,0 +1,271 @@ +. + +/** + * Unit tests for /lib/classes/curl/curl_security_helper.php. + * + * @package core + * @copyright 2016 Jake Dallimore + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + */ + +defined('MOODLE_INTERNAL') || die(); + +/** + * cURL security test suite. + * + * Note: The curl_security_helper class performs forward and reverse DNS look-ups in some cases. This class will not attempt to test + * this functionality as look-ups can vary from machine to machine. Instead, human testing with known inputs/outputs is recommended. + * + * @package core + * @copyright 2016 Jake Dallimore + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + */ +class core_curl_security_helper_testcase extends advanced_testcase { + /** + * Test for \core\files\curl_security_helper::url_is_blocked(). + * + * @param string $url the url to validate. + * @param string $blockedhosts the list of blocked hosts. + * @param string $allowedports the list of allowed ports. + * @param bool $expected the expected result. + * @dataProvider curl_security_url_data_provider + */ + public function test_curl_security_helper_url_is_blocked($url, $blockedhosts, $allowedports, $expected) { + $this->resetAfterTest(true); + $helper = new \core\files\curl_security_helper(); + set_config('curlsecurityblockedhosts', $blockedhosts); + set_config('curlsecurityallowedport', $allowedports); + $this->assertEquals($expected, $helper->url_is_blocked($url)); + } + + /** + * Data provider for test_curl_security_helper_url_is_blocked(). + * + * @return array + */ + public function curl_security_url_data_provider() { + // Format: url, blocked hosts, allowed ports, expected result. + return [ + // Base set without the blacklist enabled - no checking takes place. + ["http://localhost/x.png", "", "", false], // IP=127.0.0.1, Port=80 (port inferred from http). + ["http://localhost:80/x.png", "", "", false], // IP=127.0.0.1, Port=80 (specific port overrides http scheme). + ["https://localhost/x.png", "", "", false], // IP=127.0.0.1, Port=443 (port inferred from https). + ["http://localhost:443/x.png", "", "", false], // IP=127.0.0.1, Port=443 (specific port overrides http scheme). + ["localhost/x.png", "", "", false], // IP=127.0.0.1, Port=80 (port inferred from http fallback). + ["localhost:443/x.png", "", "", false], // IP=127.0.0.1, Port=443 (port hard specified, despite http fallback). + ["http://127.0.0.1/x.png", "", "", false], // IP=127.0.0.1, Port=80 (port inferred from http). + ["127.0.0.1/x.png", "", "", false], // IP=127.0.0.1, Port=80 (port inferred from http fallback). + ["http://localhost:8080/x.png", "", "", false], // IP=127.0.0.1, Port=8080 (port hard specified). + ["http://192.168.1.10/x.png", "", "", false], // IP=192.168.1.10, Port=80 (port inferred from http). + ["https://192.168.1.10/x.png", "", "", false], // IP=192.168.1.10, Port=443 (port inferred from https). + ["http://sub.example.com/x.png", "", "", false], // IP=::1, Port = 80 (port inferred from http). + ["http://s-1.d-1.com/x.png", "", "", false], // IP=::1, Port = 80 (port inferred from http). + + // Test set using domain name filters but with all ports allowed (empty). + ["http://localhost/x.png", "localhost", "", true], + ["localhost/x.png", "localhost", "", true], + ["localhost:0/x.png", "localhost", "", true], + ["ftp://localhost/x.png", "localhost", "", true], + ["http://sub.example.com/x.png", "localhost", "", false], + ["http://example.com/x.png", "example.com", "", true], + ["http://sub.example.com/x.png", "example.com", "", false], + + // Test set using wildcard domain name filters but with all ports allowed (empty). + ["http://sub.example.com/x.png", "*.com", "", true], + ["http://example.com/x.png", "*.example.com", "", false], + ["http://sub.example.com/x.png", "*.example.com", "", true], + ["http://sub.example.com/x.png", "*.sub.example.com", "", false], + ["http://sub.example.com/x.png", "*.example", "", false], + + // Test set using IP address filters but with all ports allowed (empty). + ["http://localhost/x.png", "127.0.0.1", "", true], + ["http://127.0.0.1/x.png", "127.0.0.1", "", true], + ["http://sub.example.com", "127.0.0.1", "", false], + + // Test set using CIDR IP range filters but with all ports allowed (empty). + ["http://localhost/x.png", "127.0.0.0/24", "", true], + ["http://127.0.0.1/x.png", "127.0.0.0/24", "", true], + ["http://sub.example.com", "127.0.0.0/24", "", false], + + // Test set using last-group range filters but with all ports allowed (empty). + ["http://localhost/x.png", "127.0.0.0-30", "", true], + ["http://127.0.0.1/x.png", "127.0.0.0-30", "", true], + ["http://sub.example.com", "127.0.0.0/24", "", false], + + // Test set using port filters but with all hosts allowed (empty). + ["http://localhost/x.png", "", "80\n443", false], + ["http://localhost:80/x.png", "", "80\n443", false], + ["https://localhost/x.png", "", "80\n443", false], + ["http://localhost:443/x.png", "", "80\n443", false], + ["http://sub.example.com:8080/x.png", "", "80\n443", true], + ["http://sub.example.com:-80/x.png", "", "80\n443", true], + ["http://sub.example.com:aaa/x.png", "", "80\n443", true], + + // Test set using port filters and hosts filters. + ["http://localhost/x.png", "127.0.0.1", "80\n443", true], + ["http://127.0.0.1/x.png", "127.0.0.1", "80\n443", true], + ["http://sub.example.com", "127.0.0.1", "80\n443", false], + + // Note on testing URLs using IPv6 notation: + // At present, the curl_security_helper class doesn't support IPv6 url notation. + // E.g. http://[ad34::dddd]:port/resource + // This is because it uses clean_param(x, PARAM_URL) as part of parsing, which won't validate urls having IPv6 notation. + // The underlying IPv6 address and range support is in place, however, so if clean_param is changed in future, + // please add the following test sets. + // 1. ["http://[::1]/x.png", "", "", false] + // 2. ["http://[::1]/x.png", "::1", "", true] + // 3. ["http://[::1]/x.png", "::1/64", "", true] + // 4. ["http://[fe80::dddd]/x.png", "fe80::cccc-eeee", "", true] + // 5. ["http://[fe80::dddd]/x.png", "fe80::dddd/128", "", true]. + ]; + } + + /** + * Test for \core\files\curl_security_helper->is_enabled(). + * + * @param string $blockedhosts the list of blocked hosts. + * @param string $allowedports the list of allowed ports. + * @param bool $expected the expected result. + * @dataProvider curl_security_settings_data_provider + */ + public function test_curl_security_helper_is_enabled($blockedhosts, $allowedports, $expected) { + $this->resetAfterTest(true); + $helper = new \core\files\curl_security_helper(); + set_config('curlsecurityblockedhosts', $blockedhosts); + set_config('curlsecurityallowedport', $allowedports); + $this->assertEquals($expected, $helper->is_enabled()); + } + + /** + * Data provider for test_curl_security_helper_is_enabled(). + * + * @return array + */ + public function curl_security_settings_data_provider() { + // Format: blocked hosts, allowed ports, expected result. + return [ + ["", "", false], + ["127.0.0.1", "", true], + ["localhost", "", true], + ["127.0.0.0/24\n192.0.0.0/24", "", true], + ["", "80\n443", true], + ]; + } + + /** + * Test for \core\files\curl_security_helper::host_is_blocked(). + * + * @param string $host the host to validate. + * @param string $blockedhosts the list of blocked hosts. + * @param bool $expected the expected result. + * @dataProvider curl_security_host_data_provider + */ + public function test_curl_security_helper_host_is_blocked($host, $blockedhosts, $expected) { + $this->resetAfterTest(true); + $helper = new \core\files\curl_security_helper(); + set_config('curlsecurityblockedhosts', $blockedhosts); + $this->assertEquals($expected, phpunit_util::call_internal_method($helper, 'host_is_blocked', [$host], + '\core\files\curl_security_helper')); + } + + /** + * Data provider for test_curl_security_helper_host_is_blocked(). + * + * @return array + */ + public function curl_security_host_data_provider() { + return [ + // IPv4 hosts. + ["127.0.0.1", "127.0.0.1", true], + ["127.0.0.1", "127.0.0.0/24", true], + ["127.0.0.1", "127.0.0.0-40", true], + ["", "127.0.0.0/24", false], + + // IPv6 hosts. + // Note: ["::", "::", true], - should match but 'address_in_subnet()' has trouble with fully collapsed IPv6 addresses. + ["::1", "::1", true], + ["::1", "::0-cccc", true], + ["::1", "::0/64", true], + ["FE80:0000:0000:0000:0000:0000:0000:0000", "fe80::/128", true], + ["fe80::eeee", "fe80::ddde/64", true], + ["fe80::dddd", "fe80::cccc-eeee", true], + ["fe80::dddd", "fe80::ddde-eeee", false], + + // Domain name hosts. + ["example.com", "example.com", true], + ["sub.example.com", "example.com", false], + ["example.com", "*.com", true], + ["example.com", "*.example.com", false], + ["sub.example.com", "*.example.com", true], + ["sub.sub.example.com", "*.example.com", true], + ["sub.example.com", "*example.com", false], + ["sub.example.com", "*.example", false], + + // International domain name hosts. + ["xn--nw2a.xn--j6w193g", "xn--nw2a.xn--j6w193g", true], // The domain 見.香港 is ace-encoded to xn--nw2a.xn--j6w193g. + ]; + } + + /** + * Test for \core\files\curl_security_helper->port_is_blocked(). + * + * @param int|string $port the port to validate. + * @param string $allowedports the list of allowed ports. + * @param bool $expected the expected result. + * @dataProvider curl_security_port_data_provider + */ + public function test_curl_security_helper_port_is_blocked($port, $allowedports, $expected) { + $this->resetAfterTest(true); + $helper = new \core\files\curl_security_helper(); + set_config('curlsecurityallowedport', $allowedports); + $this->assertEquals($expected, phpunit_util::call_internal_method($helper, 'port_is_blocked', [$port], + '\core\files\curl_security_helper')); + } + + /** + * Data provider for test_curl_security_helper_port_is_blocked(). + * + * @return array + */ + public function curl_security_port_data_provider() { + return [ + ["", "80\n443", true], + [" ", "80\n443", true], + ["-1", "80\n443", true], + [-1, "80\n443", true], + ["n", "80\n443", true], + [0, "80\n443", true], + ["0", "80\n443", true], + [8080, "80\n443", true], + ["8080", "80\n443", true], + ["80", "80\n443", false], + [80, "80\n443", false], + [443, "80\n443", false], + [0, "", true], // Port 0 and below are always invalid, even when the admin hasn't set whitelist entries. + [-1, "", true], // Port 0 and below are always invalid, even when the admin hasn't set whitelist entries. + [null, "", true], // Non-string, non-int values are invalid. + ]; + } + + /** + * Test for \core\files\curl_security_helper::get_blocked_url_string(). + */ + public function test_curl_security_helper_get_blocked_url_string() { + $helper = new \core\files\curl_security_helper(); + $this->assertEquals(get_string('curlsecurityurlblocked', 'admin'), $helper->get_blocked_url_string()); + } +}