Repository: shapesecurity/salvation
Pull request #146 (2016-07-04 07:48:55)

fixes GH-144: add support for `require-sri-for` directive
- supports `require-sri-for`
- tests
- refactored sandbox and referrer directive value handling to use generic
RFC7230Token class.

shekyan: Addressed comments.

Patch:
diff --git a/src/main/java/com/shapesecurity/salvation/Constants.java b/src/main/java/com/shapesecurity/salvation/Constants.java
index e6339c2..4ea2ab3 100644
--- a/src/main/java/com/shapesecurity/salvation/Constants.java
+++ b/src/main/java/com/shapesecurity/salvation/Constants.java
@@ -11,6 +11,8 @@ import java.util.regex.Pattern;
     public static final Pattern sandboxEnumeratedTokenPattern = Pattern.compile(
         "^allow-(?:forms|modals|pointer-lock" + "|popups|popups-to-escape-sandbox|same-origin"
             + "|scripts|top-navigation)$");
+    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)$");
     // port-part constants
diff --git a/src/main/java/com/shapesecurity/salvation/Parser.java b/src/main/java/com/shapesecurity/salvation/Parser.java
index 5d20bc5..674c74c 100644
--- a/src/main/java/com/shapesecurity/salvation/Parser.java
+++ b/src/main/java/com/shapesecurity/salvation/Parser.java
@@ -31,13 +31,17 @@ public class Parser {
     private static final DirectiveValueParseException INVALID_ANCESTOR_SOURCE =
         new DirectiveValueParseException("Invalid ancestor-source");
     private static final DirectiveParseException INVALID_REFERRER_TOKEN =
-        new DirectiveParseException("Invalid referrer-token");
+        new DirectiveParseException("Invalid referrer token");
     private static final DirectiveParseException INVALID_REPORT_TO_TOKEN =
         new DirectiveParseException("Invalid report-to token");
+    private static final DirectiveParseException INVALID_REQUIRE_SRI_FOR_TOKEN_LIST =
+            new DirectiveParseException("Invalid require-sri-for token list");
+    private static final DirectiveValueParseException INVALID_REQUIRE_SRI_FOR_TOKEN =
+            new DirectiveValueParseException("Invalid require-sri-for token");
     private static final DirectiveParseException INVALID_SANDBOX_TOKEN_LIST =
-        new DirectiveParseException("Invalid sandbox-token list");
+        new DirectiveParseException("Invalid sandbox token list");
     private static final DirectiveValueParseException INVALID_SANDBOX_TOKEN =
-        new DirectiveValueParseException("Invalid sandbox-token");
+        new DirectiveValueParseException("Invalid sandbox token");
     private static final DirectiveParseException INVALID_URI_REFERENCE_LIST =
         new DirectiveParseException("Invalid uri-reference list");
     private static final DirectiveValueParseException INVALID_URI_REFERENCE =
@@ -265,6 +269,9 @@ public class Parser {
                     }
                     result = new ReportUriDirective(uriList);
                     break;
+                case RequireSriFor:
+                    result = new RequireSriForDirective(this.parseRequireSriForTokenList(token));
+                    break;
                 case Sandbox:
                     result = new SandboxDirective(this.parseSandboxTokenList());
                     break;
@@ -544,12 +551,12 @@ public class Parser {
         }
     }
 
-    @Nonnull private ReferrerValue parseReferrerToken(@Nonnull Token directiveNameToken) throws DirectiveParseException {
+    @Nonnull private RFC7230Token parseReferrerToken(@Nonnull Token directiveNameToken) throws DirectiveParseException {
         if (this.hasNext(DirectiveValueToken.class)) {
             Token token = this.advance();
             Matcher matcher = Constants.referrerTokenPattern.matcher(Tokeniser.trimRHSWS(token.value));
             if (matcher.find()) {
-                return new ReferrerValue(token.value);
+                return new RFC7230Token(token.value);
             }
             this.error(token, "Expecting referrer directive value but found \"" + token.value + "\".");
         } else {
@@ -573,8 +580,47 @@ public class Parser {
         throw INVALID_REPORT_TO_TOKEN;
     }
 
-    @Nonnull private Set<SandboxValue> parseSandboxTokenList() throws DirectiveParseException {
-        Set<SandboxValue> sandboxTokens = new LinkedHashSet<>();
+    @Nonnull private Set<RFC7230Token> parseRequireSriForTokenList(@Nonnull Token directiveNameToken) throws DirectiveParseException {
+        Set<RFC7230Token> requireSriForTokens = new LinkedHashSet<>();
+        boolean parseException = false;
+        while (this.hasNext(SubDirectiveValueToken.class)) {
+            try {
+                RFC7230Token rsfToken = this.parseRequireSriForToken();
+                if (!requireSriForTokens.add(rsfToken)) {
+                    this.warn(directiveNameToken, "The require-sri-for directive contains duplicate token: \"" + rsfToken.show() + "\".");
+                }
+            } catch (DirectiveValueParseException e) {
+                parseException = true;
+            }
+        }
+        if (parseException) {
+            throw INVALID_REQUIRE_SRI_FOR_TOKEN_LIST;
+        }
+        if (requireSriForTokens.isEmpty()) {
+            this.warn(directiveNameToken, "Empty require-sri-for directive has no effect.");
+        }
+        return requireSriForTokens;
+    }
+
+    @Nonnull private RFC7230Token parseRequireSriForToken() throws DirectiveValueParseException {
+        Token token = this.advance();
+        Matcher matcher = Constants.requireSriForEnumeratedTokenPattern.matcher(token.value);
+        if (matcher.find()) {
+            return new RFC7230Token(token.value.toLowerCase());
+        } else {
+            this.warn(token, "The require-sri-for directive should contain only \"script\", \"style\" tokens.");
+            matcher = Constants.rfc7230TokenPattern.matcher(token.value);
+            if (matcher.find()) {
+                return new RFC7230Token(token.value);
+            }
+        }
+
+        this.error(token, "Expecting RFC 7230 token but found \"" + token.value + "\".");
+        throw INVALID_REQUIRE_SRI_FOR_TOKEN;
+    }
+
+    @Nonnull private Set<RFC7230Token> parseSandboxTokenList() throws DirectiveParseException {
+        Set<RFC7230Token> sandboxTokens = new LinkedHashSet<>();
         boolean parseException = false;
         while (this.hasNext(SubDirectiveValueToken.class)) {
             try {
@@ -589,18 +635,18 @@ public class Parser {
         return sandboxTokens;
     }
 
-    @Nonnull private SandboxValue parseSandboxToken() throws DirectiveValueParseException {
+    @Nonnull private RFC7230Token parseSandboxToken() throws DirectiveValueParseException {
         Token token = this.advance();
         Matcher matcher = Constants.sandboxEnumeratedTokenPattern.matcher(token.value);
         if (matcher.find()) {
-            return new SandboxValue(token.value);
+            return new RFC7230Token(token.value);
         } else {
             this.warn(token, "The sandbox directive should contain only allow-forms, allow-modals, "
                 + "allow-pointer-lock, allow-popups, allow-popups-to-escape-sandbox, "
                 + "allow-same-origin, allow-scripts, or allow-top-navigation.");
             matcher = Constants.rfc7230TokenPattern.matcher(token.value);
             if (matcher.find()) {
-                return new SandboxValue(token.value);
+                return new RFC7230Token(token.value);
             }
         }
 
diff --git a/src/main/java/com/shapesecurity/salvation/Tokeniser.java b/src/main/java/com/shapesecurity/salvation/Tokeniser.java
index 71a688a..1de9a3d 100644
--- a/src/main/java/com/shapesecurity/salvation/Tokeniser.java
+++ b/src/main/java/com/shapesecurity/salvation/Tokeniser.java
@@ -138,6 +138,8 @@ public class Tokeniser {
                     case FrameAncestors:
                 // sandbox directive value
                     case Sandbox:
+                // require-sri-for directive value
+                    case RequireSriFor:
                 // uri-reference list
                     case ReportUri:
                         newTokens.addAll(splitByWSP(t));
diff --git a/src/main/java/com/shapesecurity/salvation/directiveValues/SandboxValue.java b/src/main/java/com/shapesecurity/salvation/directiveValues/RFC7230Token.java
similarity index 75%
rename from src/main/java/com/shapesecurity/salvation/directiveValues/SandboxValue.java
rename to src/main/java/com/shapesecurity/salvation/directiveValues/RFC7230Token.java
index 996479a..77344a1 100644
--- a/src/main/java/com/shapesecurity/salvation/directiveValues/SandboxValue.java
+++ b/src/main/java/com/shapesecurity/salvation/directiveValues/RFC7230Token.java
@@ -5,10 +5,10 @@ import com.shapesecurity.salvation.directives.DirectiveValue;
 import javax.annotation.Nonnull;
 import javax.annotation.Nullable;
 
-public class SandboxValue implements DirectiveValue {
+public class RFC7230Token implements DirectiveValue {
     @Nonnull private final String value;
 
-    public SandboxValue(@Nonnull String value) {
+    public RFC7230Token(@Nonnull String value) {
         this.value = value;
     }
 
@@ -21,6 +21,6 @@ public class SandboxValue implements DirectiveValue {
     }
 
     @Override public boolean equals(@Nullable Object other) {
-        return other instanceof SandboxValue && ((SandboxValue) other).value.equals(this.value);
+        return other instanceof RFC7230Token && ((RFC7230Token) other).value.equals(this.value);
     }
 }
diff --git a/src/main/java/com/shapesecurity/salvation/directiveValues/ReferrerValue.java b/src/main/java/com/shapesecurity/salvation/directiveValues/ReferrerValue.java
deleted file mode 100644
index 4dbf727..0000000
--- a/src/main/java/com/shapesecurity/salvation/directiveValues/ReferrerValue.java
+++ /dev/null
@@ -1,26 +0,0 @@
-package com.shapesecurity.salvation.directiveValues;
-
-import com.shapesecurity.salvation.directives.DirectiveValue;
-
-import javax.annotation.Nonnull;
-import javax.annotation.Nullable;
-
-public class ReferrerValue implements DirectiveValue {
-    @Nonnull private final String value;
-
-    public ReferrerValue(@Nonnull String value) {
-        this.value = value;
-    }
-
-    @Nonnull @Override public String show() {
-        return this.value;
-    }
-
-    @Override public int hashCode() {
-        return this.value.hashCode();
-    }
-
-    @Override public boolean equals(@Nullable Object other) {
-        return other instanceof ReferrerValue && ((ReferrerValue) other).value.equals(this.value);
-    }
-}
diff --git a/src/main/java/com/shapesecurity/salvation/directives/Directive.java b/src/main/java/com/shapesecurity/salvation/directives/Directive.java
index 25e814d..65ca733 100644
--- a/src/main/java/com/shapesecurity/salvation/directives/Directive.java
+++ b/src/main/java/com/shapesecurity/salvation/directives/Directive.java
@@ -30,7 +30,7 @@ public abstract class Directive<Value extends DirectiveValue> implements Show {
     }
 
     static {
-        // NOTE: new direvctive types must be registered here
+        // NOTE: new directive types must be registered here
         register(ScriptSrcDirective.class);
         register(StyleSrcDirective.class);
         register(ImgSrcDirective.class);
@@ -45,6 +45,7 @@ public abstract class Directive<Value extends DirectiveValue> implements Show {
         register(FrameSrcDirective.class);
         register(ReportUriDirective.class);
         register(ReportToDirective.class);
+        register(RequireSriForDirective.class);
         register(FrameAncestorsDirective.class);
         register(SandboxDirective.class);
         register(PluginTypesDirective.class);
diff --git a/src/main/java/com/shapesecurity/salvation/directives/ReferrerDirective.java b/src/main/java/com/shapesecurity/salvation/directives/ReferrerDirective.java
index 3fb3435..5b8fdc5 100644
--- a/src/main/java/com/shapesecurity/salvation/directives/ReferrerDirective.java
+++ b/src/main/java/com/shapesecurity/salvation/directives/ReferrerDirective.java
@@ -1,22 +1,22 @@
 package com.shapesecurity.salvation.directives;
 
-import com.shapesecurity.salvation.directiveValues.ReferrerValue;
+import com.shapesecurity.salvation.directiveValues.RFC7230Token;
 
 import javax.annotation.Nonnull;
 import java.util.Collections;
 import java.util.Set;
 
-public class ReferrerDirective extends Directive<ReferrerValue> {
+public class ReferrerDirective extends Directive<RFC7230Token> {
     @Nonnull private static final String NAME = "referrer";
 
-    public ReferrerDirective(@Nonnull Set<ReferrerValue> values) {
+    public ReferrerDirective(@Nonnull Set<RFC7230Token> values) {
         super(ReferrerDirective.NAME, values);
     }
-    public ReferrerDirective(@Nonnull ReferrerValue v) {
+    public ReferrerDirective(@Nonnull RFC7230Token v) {
         this(Collections.singleton(v));
     }
 
-    @Nonnull @Override public Directive<ReferrerValue> construct(Set<ReferrerValue> newValues) {
+    @Nonnull @Override public Directive<RFC7230Token> construct(Set<RFC7230Token> newValues) {
         return new ReferrerDirective(newValues);
     }
 }
diff --git a/src/main/java/com/shapesecurity/salvation/directives/RequireSriForDirective.java b/src/main/java/com/shapesecurity/salvation/directives/RequireSriForDirective.java
new file mode 100644
index 0000000..19d327a
--- /dev/null
+++ b/src/main/java/com/shapesecurity/salvation/directives/RequireSriForDirective.java
@@ -0,0 +1,18 @@
+package com.shapesecurity.salvation.directives;
+
+import com.shapesecurity.salvation.directiveValues.RFC7230Token;
+
+import javax.annotation.Nonnull;
+import java.util.Set;
+
+public class RequireSriForDirective extends Directive<RFC7230Token> {
+    @Nonnull private static final String NAME = "require-sri-for";
+
+    public RequireSriForDirective(@Nonnull Set<RFC7230Token> values) {
+        super(RequireSriForDirective.NAME, values);
+    }
+
+    @Nonnull @Override public Directive<RFC7230Token> construct(Set<RFC7230Token> newValues) {
+        return new RequireSriForDirective(newValues);
+    }
+}
diff --git a/src/main/java/com/shapesecurity/salvation/directives/SandboxDirective.java b/src/main/java/com/shapesecurity/salvation/directives/SandboxDirective.java
index 18406c7..911a893 100644
--- a/src/main/java/com/shapesecurity/salvation/directives/SandboxDirective.java
+++ b/src/main/java/com/shapesecurity/salvation/directives/SandboxDirective.java
@@ -1,18 +1,18 @@
 package com.shapesecurity.salvation.directives;
 
-import com.shapesecurity.salvation.directiveValues.SandboxValue;
+import com.shapesecurity.salvation.directiveValues.RFC7230Token;
 
 import javax.annotation.Nonnull;
 import java.util.Set;
 
-public class SandboxDirective extends Directive<SandboxValue> {
+public class SandboxDirective extends Directive<RFC7230Token> {
     @Nonnull private static final String NAME = "sandbox";
 
-    public SandboxDirective(@Nonnull Set<SandboxValue> values) {
+    public SandboxDirective(@Nonnull Set<RFC7230Token> values) {
         super(SandboxDirective.NAME, values);
     }
 
-    @Nonnull @Override public Directive<SandboxValue> construct(Set<SandboxValue> newValues) {
+    @Nonnull @Override public Directive<RFC7230Token> construct(Set<RFC7230Token> newValues) {
         return new SandboxDirective(newValues);
     }
 }
diff --git a/src/main/java/com/shapesecurity/salvation/tokens/DirectiveNameToken.java b/src/main/java/com/shapesecurity/salvation/tokens/DirectiveNameToken.java
index 56ed876..391944b 100644
--- a/src/main/java/com/shapesecurity/salvation/tokens/DirectiveNameToken.java
+++ b/src/main/java/com/shapesecurity/salvation/tokens/DirectiveNameToken.java
@@ -34,6 +34,7 @@ public class DirectiveNameToken extends Token {
         Referrer, // in draft at http://www.w3.org/TR/2014/WD-referrer-policy-20140807/#referrer-policy-delivery as of 2015-08-27
         ReportTo, // CSP3; in draft at http://w3c.github.io/webappsec-csp/#directive-report-to
         ReportUri, // CSP3 deprecates it
+        RequireSriFor, // defined in https://w3c.github.io/webappsec-subresource-integrity/#opt-in-require-sri-for
         Sandbox,
         ScriptSrc,
         StyleSrc,
@@ -78,6 +79,8 @@ public class DirectiveNameToken extends Token {
                     return Referrer;
                 case "report-to":
                     return ReportTo;
+                case "require-sri-for":
+                    return RequireSriFor;
                 case "sandbox":
                     return Sandbox;
                 case "script-src":


Test patch:
diff --git a/src/test/java/com/shapesecurity/salvation/LocationTest.java b/src/test/java/com/shapesecurity/salvation/LocationTest.java
index 0435279..6555677 100644
--- a/src/test/java/com/shapesecurity/salvation/LocationTest.java
+++ b/src/test/java/com/shapesecurity/salvation/LocationTest.java
@@ -306,6 +306,54 @@ public class LocationTest extends CSPTest {
         assertEquals(27, notices.get(1).endLocation.offset);
     }
 
+    @Test public void testWarningLocationRequireSriFor() {
+        ArrayList<Notice> notices = new ArrayList<>();
+
+        ParserWithLocation.parse("require-sri-for", "https://origin", notices);
+        assertEquals(1, notices.size());
+        assertNotNull(notices.get(0).startLocation);
+        assertEquals(1, notices.get(0).startLocation.line);
+        assertEquals(1, notices.get(0).startLocation.column);
+        assertEquals(0, notices.get(0).startLocation.offset);
+        assertNotNull(notices.get(0).endLocation);
+        assertEquals(1, notices.get(0).endLocation.line);
+        assertEquals(16, notices.get(0).endLocation.column);
+        assertEquals(15, notices.get(0).endLocation.offset);
+
+        notices.clear();
+        ParserWithLocation.parse("require-sri-for script aaa", "https://origin", notices);
+        assertEquals(1, notices.size());
+        assertNotNull(notices.get(0).startLocation);
+        assertEquals(1, notices.get(0).startLocation.line);
+        assertEquals(24, notices.get(0).startLocation.column);
+        assertEquals(23, notices.get(0).startLocation.offset);
+        assertNotNull(notices.get(0).endLocation);
+        assertEquals(1, notices.get(0).endLocation.line);
+        assertEquals(27, notices.get(0).endLocation.column);
+        assertEquals(26, notices.get(0).endLocation.offset);
+
+        notices.clear();
+        ParserWithLocation.parse("require-sri-for     абц ert", "https://origin", notices);
+        assertEquals(2, notices.size());
+        assertNotNull(notices.get(0).startLocation);
+        assertEquals(1, notices.get(0).startLocation.line);
+        assertEquals(1, notices.get(0).startLocation.column);
+        assertEquals(0, notices.get(0).startLocation.offset);
+        assertNotNull(notices.get(0).endLocation);
+        assertEquals(1, notices.get(0).endLocation.line);
+        assertEquals(16, notices.get(0).endLocation.column);
+        assertEquals(15, notices.get(0).endLocation.offset);
+        assertNotNull(notices.get(1).startLocation);
+        assertEquals(1, notices.get(1).startLocation.line);
+        assertEquals(21, notices.get(1).startLocation.column);
+        assertEquals(20, notices.get(1).startLocation.offset);
+        assertNotNull(notices.get(1).endLocation);
+        assertEquals(1, notices.get(1).endLocation.line);
+        assertEquals(28, notices.get(1).endLocation.column);
+        assertEquals(27, notices.get(1).endLocation.offset);
+    }
+
+
     @Test public void testWarningLocationReports() {
         ArrayList<Notice> notices = new ArrayList<>();
 
diff --git a/src/test/java/com/shapesecurity/salvation/ParserTest.java b/src/test/java/com/shapesecurity/salvation/ParserTest.java
index f654929..dc3ca23 100644
--- a/src/test/java/com/shapesecurity/salvation/ParserTest.java
+++ b/src/test/java/com/shapesecurity/salvation/ParserTest.java
@@ -744,6 +744,64 @@ public class ParserTest extends CSPTest {
         assertEquals(0, p.getDirectives().size());
         assertEquals(2, notices.size());
         assertEquals("The block-all-mixed-content directive must not contain any value.", notices.get(1).message);
+
+        notices.clear();
+        p = parseWithNotices("require-sri-for", notices);
+        assertEquals(1, p.getDirectives().size());
+        assertEquals(1, notices.size());
+        assertEquals("Empty require-sri-for directive has no effect.", notices.get(0).message);
+
+        notices.clear();
+        p = parseWithNotices("require-sri-for script", notices);
+        assertEquals(1, p.getDirectives().size());
+        assertEquals(0, notices.size());
+        assertEquals("require-sri-for script", p.show());
+
+        notices.clear();
+        p = parseWithNotices("require-sri-for style", notices);
+        assertEquals(1, p.getDirectives().size());
+        assertEquals(0, notices.size());
+        assertEquals("require-sri-for style", p.show());
+
+        notices.clear();
+        p = parseWithNotices("require-sri-for script    style", notices);
+        assertEquals(1, p.getDirectives().size());
+        assertEquals(0, notices.size());
+        assertEquals("require-sri-for script style", p.show());
+
+        notices.clear();
+        p = parseWithNotices("require-sri-for Script    StYlE", notices);
+        assertEquals(1, p.getDirectives().size());
+        assertEquals(0, notices.size());
+        assertEquals("require-sri-for script style", p.show());
+
+        notices.clear();
+        p = parseWithNotices("require-sri-for script script", notices);
+        assertEquals(1, p.getDirectives().size());
+        assertEquals(1, notices.size());
+        assertTrue(notices.get(0).isWarning());
+        assertEquals("The require-sri-for directive contains duplicate token: \"script\".", notices.get(0).message);
+        assertEquals("require-sri-for script", p.show());
+
+        notices.clear();
+        p = parseWithNotices("require-sri-for Script sCrIpT", notices);
+        assertEquals(1, p.getDirectives().size());
+        assertEquals(1, notices.size());
+        assertTrue(notices.get(0).isWarning());
+        assertEquals("The require-sri-for directive contains duplicate token: \"script\".", notices.get(0).message);
+        assertEquals("require-sri-for script", p.show());
+
+        notices.clear();
+        p = parseWithNotices("require-sri-for script bla script", notices);
+        assertEquals(1, p.getDirectives().size());
+        assertEquals(2, notices.size());
+        assertTrue(notices.get(0).isWarning());
+        assertTrue(notices.get(1).isWarning());
+        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 testParseMulti() {