Repository: shapesecurity/salvation (java)

Issue:
Refactor host-part matching to strictly follow the spec
https://w3c.github.io/webappsec-csp/#match-hosts

Pull request description:
fixes #168: Refactor host-part matching to strictly follow the spec
fixes #168
- implements spec algorithm for host-part matching
- fixes a bug in optimise()
- tests

Patch:
diff --git a/src/main/java/com/shapesecurity/salvation/Constants.java b/src/main/java/com/shapesecurity/salvation/Constants.java
index 051a468..a710be3 100644
--- a/src/main/java/com/shapesecurity/salvation/Constants.java
+++ b/src/main/java/com/shapesecurity/salvation/Constants.java
@@ -33,4 +33,8 @@ import java.util.regex.Pattern;
             + ")?" + queryFragmentPart + "$");
     public static final Pattern relativeReportUriPattern =
         Pattern.compile("^(?<path>" + pathPart + ")" + queryFragmentPart + "$");
+    public static final Pattern IPv4address = Pattern.compile("^(?:(?:25[0-5]|2[0-4][0-9]|[01]?[0-9][0-9]?)\\.){3}(?:25[0-5]|2[0-4][0-9]|[01]?[0-9][0-9]?)$");
+    public static final Pattern IPV6loopback = Pattern.compile("^[0:]+:1$");
+    public static final String IPv6address = "(?:(?:(?:[0-9A-Fa-f]{1,4}:){6}|::(?:[0-9A-Fa-f]{1,4}:){5}|(?:[0-9A-Fa-f]{1,4})?::(?:[0-9A-Fa-f]{1,4}:){4}|(?:(?:[0-9A-Fa-f]{1,4}:){0,1}[0-9A-Fa-f]{1,4})?::(?:[0-9A-Fa-f]{1,4}:){3}|(?:(?:[0-9A-Fa-f]{1,4}:){0,2}[0-9A-Fa-f]{1,4})?::(?:[0-9A-Fa-f]{1,4}:){2}|(?:(?:[0-9A-Fa-f]{1,4}:){0,3}[0-9A-Fa-f]{1,4})?::[0-9A-Fa-f]{1,4}:|(?:(?:[0-9A-Fa-f]{1,4}:){0,4}[0-9A-Fa-f]{1,4})?::)(?:[0-9A-Fa-f]{1,4}:[0-9A-Fa-f]{1,4}|(?:(?:25[0-5]|2[0-4][0-9]|[01]?[0-9][0-9]?)\\.){3}(?:25[0-5]|2[0-4][0-9]|[01]?[0-9][0-9]?))|(?:(?:[0-9A-Fa-f]{1,4}:){0,5}[0-9A-Fa-f]{1,4})?::[0-9A-Fa-f]{1,4}|(?:(?:[0-9A-Fa-f]{1,4}:){0,6}[0-9A-Fa-f]{1,4})?::)";
+    public static final Pattern IPv6addressWithOptionalBracket = Pattern.compile("^(?:\\[" + IPv6address + "\\]|" + IPv6address + ")$");
 }
diff --git a/src/main/java/com/shapesecurity/salvation/data/Policy.java b/src/main/java/com/shapesecurity/salvation/data/Policy.java
index 6da18d1..6450c98 100644
--- a/src/main/java/com/shapesecurity/salvation/data/Policy.java
+++ b/src/main/java/com/shapesecurity/salvation/data/Policy.java
@@ -178,7 +178,7 @@ public class Policy implements Show {
                         .filter(x -> !(x instanceof HostSource))
                         // * remove network-schemes in source list that contains *
                         .filter(x -> !(x instanceof SchemeSource) || !((SchemeSource) x).matchesNetworkScheme())
-                        // remove 'unsafe-inline' if source list contains hash or nonce
+                        // * remove 'unsafe-inline' if source list contains hash or nonce
                         .filter(x -> !((x == KeywordSource.UnsafeInline) &&
                             (sourceListDirective.containsNonceSource() || sourceListDirective.containsHashSource())))
                         .collect(Collectors.toCollection(LinkedHashSet::new));
@@ -188,7 +188,7 @@ public class Policy implements Show {
                     this.directives.put(entry.getKey(), sourceListDirective.bind(dv -> {
                         // * replace host-sources that are equivalent to origin with 'self' keyword-source
                         if (dv instanceof HostSource &&
-                            this.origin instanceof SchemeHostPortTriple &&
+                            this.origin instanceof SchemeHostPortTriple && !((HostSource) dv).hasPath() &&
                             ((HostSource) dv).matchesOnlyOrigin((SchemeHostPortTriple) this.origin)) {
                             return Collections.singleton(KeywordSource.Self);
                         }
diff --git a/src/main/java/com/shapesecurity/salvation/directiveValues/HostSource.java b/src/main/java/com/shapesecurity/salvation/directiveValues/HostSource.java
index 48cde48..02f1dd8 100644
--- a/src/main/java/com/shapesecurity/salvation/directiveValues/HostSource.java
+++ b/src/main/java/com/shapesecurity/salvation/directiveValues/HostSource.java
@@ -15,6 +15,7 @@ import java.util.ArrayList;
 import java.util.Iterator;
 import java.util.List;
 import java.util.Objects;
+import java.util.regex.Matcher;
 
 
 public class HostSource implements SourceExpression, AncestorSource, MatchesSource {
@@ -25,6 +26,10 @@ public class HostSource implements SourceExpression, AncestorSource, MatchesSour
     private final int port;
     @Nullable private final String path;
 
+    public boolean hasPath() {
+        return (this.path != null) && !this.path.isEmpty();
+    }
+
     public HostSource(@Nullable String scheme, @Nonnull String host, int port, @Nullable String path) {
         this.scheme = scheme;
         this.host = host;
@@ -82,9 +87,7 @@ public class HostSource implements SourceExpression, AncestorSource, MatchesSour
         } else {
             schemeMatches = SchemeHostPortTriple.matchesSecureScheme(this.scheme, resource.scheme);
         }
-        boolean hostMatches = this.host.equals("*") || (this.host.startsWith("*.") ?
-            resource.host.toLowerCase().endsWith(this.host.substring(1).toLowerCase()) :
-            this.host.equalsIgnoreCase(resource.host));
+        boolean hostMatches = hostMatches(this.host, resource.host);
         boolean uriUsesDefaultPort = resource.port == Constants.EMPTY_PORT
             || SchemeHostPortTriple.defaultPortForProtocol(resource.scheme) == resource.port;
         boolean thisUsesDefaultPort = this.scheme != null && (this.port == Constants.EMPTY_PORT
@@ -98,6 +101,30 @@ public class HostSource implements SourceExpression, AncestorSource, MatchesSour
         return schemeMatches && hostMatches && portMatches && pathMatches;
     }
 
+    public static boolean hostMatches(@Nonnull String hostA, @Nonnull String hostB) {
+        if (hostA.startsWith("*")) {
+            String remaining = hostA.substring(1);
+            if (hostB.toLowerCase().endsWith(remaining.toLowerCase())) {
+                return true;
+            } else {
+                return false;
+            }
+        }
+
+        if (!hostA.equalsIgnoreCase(hostB)) {
+            return false;
+        }
+
+        Matcher IPv4Matcher = Constants.IPv4address.matcher(hostA);
+        Matcher IPv6Matcher = Constants.IPv6addressWithOptionalBracket.matcher(hostA);
+        Matcher IPv6LoopbackMatcher = Constants.IPV6loopback.matcher(hostA);
+        if ((IPv4Matcher.find() && !hostA.equals("127.0.0.1")) || IPv6Matcher.find() || IPv6LoopbackMatcher.find()) {
+            return false;
+        }
+        return true;
+
+    }
+
     @Override public boolean matchesSource(@Nonnull Origin origin, @Nonnull GUID resource) {
         String originScheme = null;
         if (origin instanceof GUID) {


Test patch:
diff --git a/src/test/java/com/shapesecurity/salvation/PolicyQueryingTest.java b/src/test/java/com/shapesecurity/salvation/PolicyQueryingTest.java
index aedddd4..f3fc103 100644
--- a/src/test/java/com/shapesecurity/salvation/PolicyQueryingTest.java
+++ b/src/test/java/com/shapesecurity/salvation/PolicyQueryingTest.java
@@ -383,6 +383,67 @@ public class PolicyQueryingTest extends CSPTest {
         assertTrue("frame ancestor is allowed", p.allowsFrameAncestor(URI.parse("http://example.com")));
     }
 
+    @Test public void testHosts() {
+        Policy p;
+
+        p = Parser.parse("script-src http://*.example.com/a", "http://example.com");
+        assertTrue(p.allowsScriptFromSource(URI.parse("http://a.example.com/a")));
+        assertTrue(p.allowsScriptFromSource(URI.parse("http://A.example.com/a")));
+        assertTrue(p.allowsScriptFromSource(URI.parse("http://a.EXAMPLE.COM/a")));
+        assertTrue(p.allowsScriptFromSource(URI.parse("http://a.b.example.com/a")));
+        assertFalse(p.allowsScriptFromSource(URI.parse("http://example.com/a")));
+        assertFalse(p.allowsScriptFromSource(URI.parse("http://example.com.org/a")));
+
+        p = Parser.parse("script-src http://*.EXAMPLE.COM/a", "http://example.com");
+        assertTrue(p.allowsScriptFromSource(URI.parse("http://a.example.com/a")));
+        assertTrue(p.allowsScriptFromSource(URI.parse("http://A.example.com/a")));
+        assertTrue(p.allowsScriptFromSource(URI.parse("http://a.EXAMPLE.COM/a")));
+        assertTrue(p.allowsScriptFromSource(URI.parse("http://a.b.example.com/a")));
+        assertFalse(p.allowsScriptFromSource(URI.parse("http://example.com/a")));
+        assertFalse(p.allowsScriptFromSource(URI.parse("http://example.com.org/a")));
+
+        p = Parser.parse("script-src http://example.com/a", "http://example.com");
+        assertTrue(p.allowsScriptFromSource(URI.parse("http://example.com/a")));
+        assertTrue(p.allowsScriptFromSource(URI.parse("http://EXAMPLE.COM/a")));
+        assertFalse(p.allowsScriptFromSource(URI.parse("http://a.example.com/a")));
+        assertFalse(p.allowsScriptFromSource(URI.parse("http://A.example.com/a")));
+        assertFalse(p.allowsScriptFromSource(URI.parse("http://a.EXAMPLE.COM/a")));
+        assertFalse(p.allowsScriptFromSource(URI.parse("http://example.com.org/a")));
+
+        p = Parser.parse("script-src http://EXAMPLE.COM/a", "http://example.com");
+        assertTrue(p.allowsScriptFromSource(URI.parse("http://example.com/a")));
+        assertTrue(p.allowsScriptFromSource(URI.parse("http://EXAMPLE.COM/a")));
+        assertFalse(p.allowsScriptFromSource(URI.parse("http://a.example.com/a")));
+        assertFalse(p.allowsScriptFromSource(URI.parse("http://A.example.com/a")));
+        assertFalse(p.allowsScriptFromSource(URI.parse("http://a.EXAMPLE.COM/a")));
+        assertFalse(p.allowsScriptFromSource(URI.parse("http://example.com.org/a")));
+
+        p = Parser.parse("script-src http://127.0.0.1/a", "http://example.com");
+        assertTrue(p.allowsScriptFromSource(URI.parse("http://127.0.0.1/a")));
+        assertFalse(p.allowsScriptFromSource(URI.parse("http://127.0.0.1.com/a")));
+        assertFalse(p.allowsScriptFromSource(URI.parse("http://127.0.0.2/a")));
+        assertFalse(p.allowsScriptFromSource(URI.parse("http://127.0.0.1.1/a")));
+        assertFalse(p.allowsScriptFromSource(URI.parse("http://10.10.0.1/a")));
+
+        p = Parser.parse("script-src http://192.168.1.1/a", "http://example.com");
+        assertFalse(p.allowsScriptFromSource(URI.parse("http://192.168.1.1/a")));
+        assertFalse(p.allowsScriptFromSource(URI.parse("http://192.168.0.1/a")));
+        assertFalse(p.allowsScriptFromSource(URI.parse("http://127.0.0.1/a")));
+
+        // TODO: we can't parse below URLs now
+//        p = Parser.parse("script-src http://0:0:0:0:0:0:0:1/a", "http://example.com");
+//        assertFalse(p.allowsScriptFromSource(URI.parse("http://0:0:0:0:0:0:0:1/a")));
+
+//        p = Parser.parse("script-src http://::1/a", "http://example.com");
+//        assertFalse(p.allowsScriptFromSource(URI.parse("http://::1/a")));
+
+//        p = Parser.parse("script-src http://2001:db8:85a3:8d3:1319:8a2e:370:7348/a", "http://example.com");
+//        assertFalse(p.allowsScriptFromSource(URI.parse("http://2001:db8:85a3:8d3:1319:8a2e:370:7348/a")));
+
+//        p = Parser.parse("script-src http://[2001:db8:85a3:8d3:1319:8a2e:370:7348]:443/a", "http://example.com");
+//        assertFalse(p.allowsScriptFromSource(URI.parse("http://[2001:db8:85a3:8d3:1319:8a2e:370:7348]:443/a")));
+    }
+
     @Test public void testPaths() {
         Policy p;
 
@@ -391,6 +452,7 @@ public class PolicyQueryingTest extends CSPTest {
         assertFalse(p.allowsScriptFromSource(URI.parse("http://example.com/")));
         assertTrue(p.allowsScriptFromSource(URI.parse("http://example.com/a")));
         assertFalse(p.allowsScriptFromSource(URI.parse("http://example.com////a")));
+
         assertFalse(p.allowsScriptFromSource(URI.parse("http://example.com/A")));
         assertFalse(p.allowsScriptFromSource(URI.parse("http://example.com/a/")));
         assertFalse(p.allowsScriptFromSource(URI.parse("http://example.com/a/b")));
@@ -807,6 +869,7 @@ public class PolicyQueryingTest extends CSPTest {
         assertTrue(p.allowsScriptFromSource(URI.parse("http://a.b.example.com/c/d")));
         assertTrue(p.allowsScriptFromSource(URI.parse("http://a.b.example.com")));
         assertTrue(p.allowsScriptFromSource(URI.parse("http://www.example.com")));
+        assertTrue(p.allowsScriptFromSource(URI.parse("http://www.EXAMPLE.com")));
         assertFalse(p.allowsScriptFromSource(URI.parse("http://example.com")));
         assertFalse(p.allowsScriptFromSource(URI.parse("http://com")));
         assertFalse(p.allowsScriptFromSource(URI.parse("ws://example.com/PATH")));
diff --git a/src/test/java/com/shapesecurity/salvation/UtilsTest.java b/src/test/java/com/shapesecurity/salvation/UtilsTest.java
index e123d44..13a2f3b 100644
--- a/src/test/java/com/shapesecurity/salvation/UtilsTest.java
+++ b/src/test/java/com/shapesecurity/salvation/UtilsTest.java
@@ -5,6 +5,7 @@ import com.shapesecurity.salvation.directiveValues.HostSource;
 import org.junit.Test;
 
 import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertTrue;
 
 public class UtilsTest extends CSPTest {
@@ -28,4 +29,35 @@ public class UtilsTest extends CSPTest {
         assertEquals("[, a, b]", HostSource.splitBySpec("/a/b", '/').toString());
         assertEquals("[, a, b, ]", HostSource.splitBySpec("/a/b/", '/').toString());
     }
+
+    @Test public void testHostMatch() {
+        assertTrue(HostSource.hostMatches("*.example.com", "a.example.com"));
+        assertTrue(HostSource.hostMatches("*.example.com", "A.example.com"));
+        assertTrue(HostSource.hostMatches("*.example.com", "A.EXAMPLE.COM"));
+        assertTrue(HostSource.hostMatches("*.EXAMPLE.COM", "a.EXAMPLE.com"));
+        assertTrue(HostSource.hostMatches("*.example.com", "a.b.example.com"));
+        assertFalse(HostSource.hostMatches("*.example.com", "example.com"));
+        assertFalse(HostSource.hostMatches("*.example.com", "example.com.org"));
+
+        assertFalse(HostSource.hostMatches("example.com", "a.example.com"));
+        assertFalse(HostSource.hostMatches("example.com", "a.b.example.com"));
+        assertTrue(HostSource.hostMatches("example.com", "example.com"));
+        assertTrue(HostSource.hostMatches("EXAMPLE.COM", "example.com"));
+        assertFalse(HostSource.hostMatches("example.com", "example.com.org"));
+
+        assertTrue(HostSource.hostMatches("127.0.0.1", "127.0.0.1"));
+        assertFalse(HostSource.hostMatches("127.0.0.2", "127.0.0.2"));
+        assertFalse(HostSource.hostMatches("127.0.0.1.1", "127.0.1.1"));
+        assertFalse(HostSource.hostMatches("1.127.0.0.1.1", "1.127.0.1.1"));
+        assertFalse(HostSource.hostMatches("192.168.1.1", "192.168.1.1"));
+
+
+        assertFalse(HostSource.hostMatches("0:0:0:0:0:0:0:1", "0:0:0:0:0:0:0:1"));
+        assertFalse(HostSource.hostMatches("::1", "::1"));
+        assertFalse(HostSource.hostMatches("2001:0db8:85a3:0000:0000:8a2e:0370:7334", "2001:0db8:85a3:0000:0000:8a2e:0370:7334"));
+        assertFalse(HostSource.hostMatches("2001:db8:85a3::8a2e:370:7334", "2001:db8:85a3::8a2e:370:7334"));
+        assertFalse(HostSource.hostMatches("[2001:db8:85a3:8d3:1319:8a2e:370:7348]", "[2001:db8:85a3:8d3:1319:8a2e:370:7348]"));
+        assertFalse(HostSource.hostMatches("fe80::9a01:a7ff:fe8f:3c5d", "fe80::9a01:a7ff:fe8f:3c5d"));
+
+    }
 }