MDL-61143 core_files: Update unit tests for curl_security_helper
authorCameron Ball <cameron@moodle.com>
Wed, 3 Jan 2018 08:10:37 +0000 (16:10 +0800)
committerMr. Jenkins (CiBoT) <cibot@moodle.org>
Tue, 9 Jan 2018 08:54:32 +0000 (16:54 +0800)
Previously some of the unit tests were passing "by accident" becuase
we had the security helper letting through domains where the DNS lookup
failed. That behaviour has changed and now such domains are blocked.

Additionally tests for domains with multiple A records and weird Unicode
stuff have been added.

This patch also mocks the DNS resolution in the test, rather than actually
resolving the domain.

lib/tests/curl_security_helper_test.php

index 83b1cc4..f2691b7 100644 (file)
@@ -38,15 +38,29 @@ class core_curl_security_helper_testcase extends advanced_testcase {
     /**
      * Test for \core\files\curl_security_helper::url_is_blocked().
      *
+     * @param array $dns a mapping between hosts and IPs to be used instead of a real DNS lookup. The values must be arrays.
      * @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) {
+    public function test_curl_security_helper_url_is_blocked($dns, $url, $blockedhosts, $allowedports, $expected) {
         $this->resetAfterTest(true);
-        $helper = new \core\files\curl_security_helper();
+        $helper = $this->getMockBuilder('\core\files\curl_security_helper')
+                        ->setMethods(['get_host_list_by_name'])
+                        ->getMock();
+
+        // Override the get host list method to return hard coded values based on a mapping provided by $dns.
+        $helper->method('get_host_list_by_name')
+               ->will(
+                   $this->returnCallback(
+                       function($host) use ($dns) {
+                           return isset($dns[$host]) ? $dns[$host] : [];
+                       }
+                   )
+               );
+
         set_config('curlsecurityblockedhosts', $blockedhosts);
         set_config('curlsecurityallowedport', $allowedports);
         $this->assertEquals($expected, $helper->url_is_blocked($url));
@@ -58,67 +72,82 @@ class core_curl_security_helper_testcase extends advanced_testcase {
      * @return array
      */
     public function curl_security_url_data_provider() {
+        $simpledns = ['localhost' => ['127.0.0.1']];
+        $multiplerecorddns = [
+            'sub.example.com' => ['1.2.3.4', '5.6.7.8']
+        ];
         // 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).
+            [$simpledns, "http://localhost/x.png", "", "", false],       // IP=127.0.0.1, Port=80 (port inferred from http).
+            [$simpledns, "http://localhost:80/x.png", "", "", false],    // IP=127.0.0.1, Port=80 (specific port overrides http scheme).
+            [$simpledns, "https://localhost/x.png", "", "", false],      // IP=127.0.0.1, Port=443 (port inferred from https).
+            [$simpledns, "http://localhost:443/x.png", "", "", false],   // IP=127.0.0.1, Port=443 (specific port overrides http scheme).
+            [$simpledns, "localhost/x.png", "", "", false],              // IP=127.0.0.1, Port=80 (port inferred from http fallback).
+            [$simpledns, "localhost:443/x.png", "", "", false],          // IP=127.0.0.1, Port=443 (port hard specified, despite http fallback).
+            [$simpledns, "http://127.0.0.1/x.png", "", "", false],       // IP=127.0.0.1, Port=80 (port inferred from http).
+            [$simpledns, "127.0.0.1/x.png", "", "", false],              // IP=127.0.0.1, Port=80 (port inferred from http fallback).
+            [$simpledns, "http://localhost:8080/x.png", "", "", false],  // IP=127.0.0.1, Port=8080 (port hard specified).
+            [$simpledns, "http://192.168.1.10/x.png", "", "", false],    // IP=192.168.1.10, Port=80 (port inferred from http).
+            [$simpledns, "https://192.168.1.10/x.png", "", "", false],   // IP=192.168.1.10, Port=443 (port inferred from https).
+            [$simpledns, "http://sub.example.com/x.png", "", "", false], // IP=::1, Port = 80 (port inferred from http).
+            [$simpledns, "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],
+            [$simpledns, "http://localhost/x.png", "localhost", "", true],
+            [$simpledns, "localhost/x.png", "localhost", "", true],
+            [$simpledns, "localhost:0/x.png", "localhost", "", true],
+            [$simpledns, "ftp://localhost/x.png", "localhost", "", true],
+            [$simpledns, "http://sub.example.com/x.png", "localhost", "", false],
+            [$simpledns, "http://example.com/x.png", "example.com", "", true],
+            [$simpledns, "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],
+            [$simpledns, "http://sub.example.com/x.png", "*.com", "", true],
+            [$simpledns, "http://example.com/x.png", "*.example.com", "", false],
+            [$simpledns, "http://sub.example.com/x.png", "*.example.com", "", true],
+            [$simpledns, "http://sub.example.com/x.png", "*.sub.example.com", "", false],
+            [$simpledns, "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],
+            [$simpledns, "http://localhost/x.png", "127.0.0.1", "", true],
+            [$simpledns, "http://127.0.0.1/x.png", "127.0.0.1", "", true],
 
             // 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],
+            [$simpledns, "http://localhost/x.png", "127.0.0.0/24", "", true],
+            [$simpledns, "http://127.0.0.1/x.png", "127.0.0.0/24", "", true],
 
             // 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],
+            [$simpledns, "http://localhost/x.png", "127.0.0.0-30", "", true],
+            [$simpledns, "http://127.0.0.1/x.png", "127.0.0.0-30", "", true],
 
             // 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],
+            [$simpledns, "http://localhost/x.png", "", "80\n443", false],
+            [$simpledns, "http://localhost:80/x.png", "", "80\n443", false],
+            [$simpledns, "https://localhost/x.png", "", "80\n443", false],
+            [$simpledns, "http://localhost:443/x.png", "", "80\n443", false],
+            [$simpledns, "http://sub.example.com:8080/x.png", "", "80\n443", true],
+            [$simpledns, "http://sub.example.com:-80/x.png", "", "80\n443", true],
+            [$simpledns, "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],
+            [$simpledns, "http://localhost/x.png", "127.0.0.1", "80\n443", true],
+            [$simpledns, "http://127.0.0.1/x.png", "127.0.0.1", "80\n443", true],
+
+            // Test using multiple A records.
+            // Multiple record DNS gives two IPs for the same host, we want to make
+            // sure that if we blacklist one of those (doesn't matter which one)
+            // the request is blocked.
+            [$multiplerecorddns, "http://sub.example.com", '1.2.3.4', "", true],
+            [$multiplerecorddns, "http://sub.example.com", '5.6.7.8', "", true],
+
+            // Test when DNS resolution fails.
+            [[], "http://example.com", "127.0.0.1", "", true],
+
+            // Test some freaky deaky Unicode domains. Should be blocked always.
+            [$simpledns, "http://169。254。169。254/", "127.0.0.1", "", true],
+            [$simpledns, "http://169。254。169。254/", "1.2.3.4", "", true],
+            [$simpledns, "http://169。254。169。254/", "127.0.0.1", "80\n443", true]
 
             // Note on testing URLs using IPv6 notation:
             // At present, the curl_security_helper class doesn't support IPv6 url notation.