Repository: shapesecurity/salvation (java)

Issue:
support "'unsafe-dynamic'" keyword
https://w3c.github.io/webappsec-csp/#strict-dynamic-usage

Pull request:
Fixes #128: Support `strict-dynamic` keyword-source
This patch introduces basic support for
`strict-dynamic` keyword.

Patch:
diff --git a/src/main/java/com/shapesecurity/salvation/Constants.java b/src/main/java/com/shapesecurity/salvation/Constants.java
index 4ea2ab3..371c04e 100644
--- a/src/main/java/com/shapesecurity/salvation/Constants.java
+++ b/src/main/java/com/shapesecurity/salvation/Constants.java
@@ -14,7 +14,7 @@ import java.util.regex.Pattern;
     public static final Pattern requireSriForEnumeratedTokenPattern = Pattern
             .compile("^(?:script|style)$", Pattern.CASE_INSENSITIVE);
     public static final Pattern mediaTypePattern = Pattern.compile("^(?<type>[^/]+)/(?<subtype>[^/]+)$");
-    public static final Pattern unquotedKeywordPattern = Pattern.compile("^(?:self|unsafe-inline|unsafe-eval|unsafe-redirect|none)$");
+    public static final Pattern unquotedKeywordPattern = Pattern.compile("^(?:self|unsafe-inline|unsafe-eval|unsafe-redirect|none|strict-dynamic)$");
     // port-part constants
     public static final int WILDCARD_PORT = -200;
     public static final int EMPTY_PORT = -1;
diff --git a/src/main/java/com/shapesecurity/salvation/Parser.java b/src/main/java/com/shapesecurity/salvation/Parser.java
index 674c74c..3b9bf1f 100644
--- a/src/main/java/com/shapesecurity/salvation/Parser.java
+++ b/src/main/java/com/shapesecurity/salvation/Parser.java
@@ -50,7 +50,8 @@ public class Parser {
         new DirectiveParseException("Non-empty directive-value list");
 
     private static final String unsafeInlineWarningMessage = "The 'unsafe-inline' keyword-source has no effect in source lists that contain hash-source or nonce-source.";
-    private enum SeenStates {SEEN_HASH, SEEN_NONE, SEEN_NONCE, SEEN_UNSAFE_INLINE};
+    private static final String strictDynamicWarningMessage = "The 'strict-dynamic' keyword-source makes host-source and scheme-source expressions, as well as the \"'unsafe-inline'\" and \"'self' keyword-sources unnecessary in CSP3 and later.";
+    private enum SeenStates {SEEN_HASH, SEEN_HOST_OR_SCHEME_SOURCE, SEEN_NONE, SEEN_NONCE, SEEN_SELF, SEEN_STRICT_DYNAMIC, SEEN_UNSAFE_EVAL, SEEN_UNSAFE_INLINE};
     @Nonnull protected final Token[] tokens;
     @Nonnull private final Origin origin;
     protected int index = 0;
@@ -367,12 +368,20 @@ public class Parser {
                 SourceExpression se = this.parseSourceExpression(seenStates, !sourceExpressions.isEmpty());
                 if (se == None.INSTANCE) {
                     seenStates.add(SeenStates.SEEN_NONE);
+                } else if (se == KeywordSource.UnsafeEval) {
+                    seenStates.add(SeenStates.SEEN_UNSAFE_EVAL);
+                } else if (se == KeywordSource.Self) {
+                    seenStates.add(SeenStates.SEEN_SELF);
                 } else if (se == KeywordSource.UnsafeInline) {
                     seenStates.add(SeenStates.SEEN_UNSAFE_INLINE);
                 } else if (se instanceof HashSource) {
                     seenStates.add(SeenStates.SEEN_HASH);
                 } else if (se instanceof NonceSource) {
                     seenStates.add(SeenStates.SEEN_NONCE);
+                } else if (se == KeywordSource.StrictDynamic) {
+                    seenStates.add(SeenStates.SEEN_STRICT_DYNAMIC);
+                } else if (se instanceof HostSource || se instanceof SchemeSource) {
+                    seenStates.add(SeenStates.SEEN_HOST_OR_SCHEME_SOURCE);
                 }
                 sourceExpressions.add(se);
             } catch (DirectiveValueParseException e) {
@@ -396,11 +405,22 @@ public class Parser {
             case "'none'":
                 return None.INSTANCE;
             case "'self'":
+                if (seenStates.contains(SeenStates.SEEN_STRICT_DYNAMIC)) {
+                    this.warn(token, strictDynamicWarningMessage);
+                }
                 return KeywordSource.Self;
+            case "'strict-dynamic'":
+                if (seenStates.contains(SeenStates.SEEN_UNSAFE_INLINE) || seenStates.contains(SeenStates.SEEN_HOST_OR_SCHEME_SOURCE) || seenStates.contains(SeenStates.SEEN_SELF))  {
+                    this.warn(token, strictDynamicWarningMessage);
+                }
+                return KeywordSource.StrictDynamic;
             case "'unsafe-inline'":
                 if (seenStates.contains(SeenStates.SEEN_HASH) || seenStates.contains(SeenStates.SEEN_NONCE))  {
                     this.warn(token, unsafeInlineWarningMessage);
                 }
+                if (seenStates.contains(SeenStates.SEEN_STRICT_DYNAMIC)) {
+                    this.warn(token, strictDynamicWarningMessage);
+                }
                 return KeywordSource.UnsafeInline;
             case "'unsafe-eval'":
                 return KeywordSource.UnsafeEval;
@@ -460,6 +480,9 @@ public class Parser {
                     }
                     return hashSource;
                 } else if (token.value.matches("^" + Constants.schemePart + ":$")) {
+                    if (seenStates.contains(SeenStates.SEEN_STRICT_DYNAMIC)) {
+                        this.warn(token, strictDynamicWarningMessage);
+                    }
                     return new SchemeSource(token.value.substring(0, token.value.length() - 1));
                 } else {
                     Matcher matcher = Constants.hostSourcePattern.matcher(token.value);
@@ -474,6 +497,9 @@ public class Parser {
                         } else {
                             port = portString.equals(":*") ? Constants.WILDCARD_PORT : Integer.parseInt(portString.substring(1));
                         }
+                        if (seenStates.contains(SeenStates.SEEN_STRICT_DYNAMIC)) {
+                            this.warn(token, strictDynamicWarningMessage);
+                        }
                         String host = matcher.group("host");
                         String path = matcher.group("path");
                         return new HostSource(scheme, host, port, path);
diff --git a/src/main/java/com/shapesecurity/salvation/directiveValues/KeywordSource.java b/src/main/java/com/shapesecurity/salvation/directiveValues/KeywordSource.java
index 2eb3f3e..6f02b50 100644
--- a/src/main/java/com/shapesecurity/salvation/directiveValues/KeywordSource.java
+++ b/src/main/java/com/shapesecurity/salvation/directiveValues/KeywordSource.java
@@ -13,6 +13,7 @@ public class KeywordSource implements SourceExpression, AncestorSource, MatchesS
     @Nonnull public static final KeywordSource UnsafeInline = new KeywordSource("unsafe-inline");
     @Nonnull public static final KeywordSource UnsafeEval = new KeywordSource("unsafe-eval");
     @Nonnull public static final KeywordSource UnsafeRedirect = new KeywordSource("unsafe-redirect");
+    @Nonnull public static final KeywordSource StrictDynamic = new KeywordSource("strict-dynamic");
     @Nonnull private final String value;
 
     private KeywordSource(@Nonnull String value) {


Test patch:
diff --git a/src/test/java/com/shapesecurity/salvation/ParserTest.java b/src/test/java/com/shapesecurity/salvation/ParserTest.java
index dc3ca23..4544294 100644
--- a/src/test/java/com/shapesecurity/salvation/ParserTest.java
+++ b/src/test/java/com/shapesecurity/salvation/ParserTest.java
@@ -574,6 +574,8 @@ public class ParserTest extends CSPTest {
             parse("img-src example.com 'unsafe-eval'").getDirectiveByType(ImgSrcDirective.class).show());
         assertEquals("directive-name, directive-value", "img-src example.com 'unsafe-redirect'",
             parse("img-src example.com 'unsafe-redirect'").getDirectiveByType(ImgSrcDirective.class).show());
+        assertEquals("directive-name, directive-value", "img-src 'nonce-123' 'strict-dynamic'",
+            parse("img-src 'nonce-123' 'strict-dynamic'").getDirectiveByType(ImgSrcDirective.class).show());
     }
 
     @Test public void testDirectNameSpacing() {
@@ -800,8 +802,64 @@ public class ParserTest extends CSPTest {
         assertEquals("The require-sri-for directive should contain only \"script\", \"style\" tokens.", notices.get(0).message);
         assertEquals("The require-sri-for directive contains duplicate token: \"script\".", notices.get(1).message);
         assertEquals("require-sri-for script bla", p.show());
+    }
 
+    @Test public void testStrictDynamic() {
+        Policy p;
+        ArrayList<Notice> notices = new ArrayList<>();
+        p = parseWithNotices("script-src a 'strict-dynamic'", notices);
+        assertEquals(1, p.getDirectives().size());
+        assertEquals(1, notices.size());
+        assertTrue(notices.get(0).isWarning());
+        assertEquals("The 'strict-dynamic' keyword-source makes host-source and scheme-source expressions, as well as the \"'unsafe-inline'\" and \"'self' keyword-sources unnecessary in CSP3 and later.", notices.get(0).message);
+        assertEquals("script-src a 'strict-dynamic'", p.show());
 
+        notices.clear();
+        p = parseWithNotices("script-src 'strict-dynamic' a", notices);
+        assertEquals(1, p.getDirectives().size());
+        assertEquals(1, notices.size());
+        assertTrue(notices.get(0).isWarning());
+        assertEquals("The 'strict-dynamic' keyword-source makes host-source and scheme-source expressions, as well as the \"'unsafe-inline'\" and \"'self' keyword-sources unnecessary in CSP3 and later.", notices.get(0).message);
+        assertEquals("script-src 'strict-dynamic' a", p.show());
+
+        notices.clear();
+        p = parseWithNotices("script-src 'strict-dynamic' https:", notices);
+        assertEquals(1, p.getDirectives().size());
+        assertEquals(1, notices.size());
+        assertTrue(notices.get(0).isWarning());
+        assertEquals("The 'strict-dynamic' keyword-source makes host-source and scheme-source expressions, as well as the \"'unsafe-inline'\" and \"'self' keyword-sources unnecessary in CSP3 and later.", notices.get(0).message);
+        assertEquals("script-src 'strict-dynamic' https:", p.show());
+
+        notices.clear();
+        p = parseWithNotices("script-src 'strict-dynamic' 'self'", notices);
+        assertEquals(1, p.getDirectives().size());
+        assertEquals(1, notices.size());
+        assertTrue(notices.get(0).isWarning());
+        assertEquals("The 'strict-dynamic' keyword-source makes host-source and scheme-source expressions, as well as the \"'unsafe-inline'\" and \"'self' keyword-sources unnecessary in CSP3 and later.", notices.get(0).message);
+        assertEquals("script-src 'strict-dynamic' 'self'", p.show());
+
+        notices.clear();
+        p = parseWithNotices("script-src 'strict-dynamic' 'unsafe-inline'", notices);
+        assertEquals(1, p.getDirectives().size());
+        assertEquals(1, notices.size());
+        assertTrue(notices.get(0).isWarning());
+        assertEquals("The 'strict-dynamic' keyword-source makes host-source and scheme-source expressions, as well as the \"'unsafe-inline'\" and \"'self' keyword-sources unnecessary in CSP3 and later.", notices.get(0).message);
+        assertEquals("script-src 'strict-dynamic' 'unsafe-inline'", p.show());
+
+        notices.clear();
+        p = parseWithNotices("script-src 'strict-dynamic' 'unsafe-inline' 'self' a https:", notices);
+        assertEquals(1, p.getDirectives().size());
+        assertEquals(4, notices.size());
+        assertTrue(notices.get(0).isWarning());
+        assertEquals("The 'strict-dynamic' keyword-source makes host-source and scheme-source expressions, as well as the \"'unsafe-inline'\" and \"'self' keyword-sources unnecessary in CSP3 and later" +
+                ".", notices.get(0).message);
+        assertEquals("script-src 'strict-dynamic' 'unsafe-inline' 'self' a https:", p.show());
+
+        notices.clear();
+        p = parseWithNotices("script-src 'strict-dynamic' 'unsafe-eval'", notices);
+        assertEquals(1, p.getDirectives().size());
+        assertEquals(0, notices.size());
+        assertEquals("script-src 'strict-dynamic' 'unsafe-eval'", p.show());
     }
 
     @Test public void testParseMulti() {
diff --git a/src/test/java/com/shapesecurity/salvation/PolicyMergeTest.java b/src/test/java/com/shapesecurity/salvation/PolicyMergeTest.java
index 839420f..47bcce6 100644
--- a/src/test/java/com/shapesecurity/salvation/PolicyMergeTest.java
+++ b/src/test/java/com/shapesecurity/salvation/PolicyMergeTest.java
@@ -78,6 +78,11 @@ public class PolicyMergeTest extends CSPTest {
         p2 = parse("default-src a");
         p1.union(p2);
         assertEquals("default-src b a; script-src a", p1.show());
+
+        p1 = parse("default-src 'strict-dynamic' 'nonce-1234' b; script-src a");
+        p2 = parse("default-src a");
+        p1.union(p2);
+        assertEquals("default-src 'strict-dynamic' 'nonce-1234' b a; script-src a", p1.show());
     }
 
     @Test public void testIntersect() {
@@ -138,6 +143,11 @@ public class PolicyMergeTest extends CSPTest {
         p1.intersect(p2);
         assertEquals("default-src 'self'; script-src a; style-src", p1.show());
 
+        p1 = parse("default-src 'self' 'strict-dynamic'; script-src a");
+        p2 = parse("default-src *; script-src *; style-src *:80");
+        p1.intersect(p2);
+        assertEquals("default-src 'self' 'strict-dynamic'; script-src a; style-src", p1.show());
+
         p1 = ParserWithLocation.parse("script-src a", URI.parse("https://origin"));
         p2 = parse("script-src b; report-uri /x");
         try {