Repository: shapesecurity/salvation
Pull request #179 (2017-04-10 22:09:33)

Support for 'unsafe-hashed-attributes'
- Parse 'unsafe-hashed-attributes'
- warn about nonsensical 'unsafe-hashed-attributes' without hash-source
- answer questions about attributes


Patch:
diff --git a/salvation.iml b/salvation.iml
index 791b5cb..3933a03 100644
--- a/salvation.iml
+++ b/salvation.iml
@@ -1,6 +1,6 @@
 <?xml version="1.0" encoding="UTF-8"?>
 <module org.jetbrains.idea.maven.project.MavenProjectsManager.isMavenModule="true" type="JAVA_MODULE" version="4">
-  <component name="NewModuleRootManager" LANGUAGE_LEVEL="JDK_1_8" inherit-compiler-output="false">
+  <component name="NewModuleRootManager" LANGUAGE_LEVEL="JDK_1_8">
     <output url="file://$MODULE_DIR$/target/classes" />
     <output-test url="file://$MODULE_DIR$/target/test-classes" />
     <content url="file://$MODULE_DIR$">
diff --git a/src/main/java/com/shapesecurity/salvation/Parser.java b/src/main/java/com/shapesecurity/salvation/Parser.java
index 43307ce..d2cc6f8 100644
--- a/src/main/java/com/shapesecurity/salvation/Parser.java
+++ b/src/main/java/com/shapesecurity/salvation/Parser.java
@@ -51,7 +51,8 @@ public class Parser {
     private static final String explanation = "Ensure that this pattern is only used for backwards compatibility with older CSP implementations and is not an oversight.";
     private static final String unsafeInlineWarningMessage = "The \"'unsafe-inline'\" keyword-source has no effect in source lists that contain hash-source or nonce-source in CSP2 and later. " + explanation;
     private static final String strictDynamicWarningMessage = "The host-source and scheme-source expressions, as well as the \"'unsafe-inline'\" and \"'self'\" keyword-sources have no effect in source lists that contain \"'strict-dynamic'\" in CSP3 and later. " + explanation;
-    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};
+    private static final String unsafeHashedWithoutHashWarningMessage = "The \"'unsafe-hashed-attributes'\" keyword-source has no effect in source lists that do not contain hash-source 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, SEEN_UNSAFE_HASHED_ATTR};
     @Nonnull protected final Token[] tokens;
     @Nonnull private final Origin origin;
     protected int index = 0;
@@ -386,12 +387,17 @@ public class Parser {
                     seenStates.add(SeenStates.SEEN_STRICT_DYNAMIC);
                 } else if (se instanceof HostSource || se instanceof SchemeSource) {
                     seenStates.add(SeenStates.SEEN_HOST_OR_SCHEME_SOURCE);
+                } else if (se == KeywordSource.UnsafeHashedAttributes) {
+                    seenStates.add(SeenStates.SEEN_UNSAFE_HASHED_ATTR);
                 }
                 sourceExpressions.add(se);
             } catch (DirectiveValueParseException e) {
                 parseException = true;
             }
         }
+        if (seenStates.contains(SeenStates.SEEN_UNSAFE_HASHED_ATTR) && !seenStates.contains(SeenStates.SEEN_HASH)) {
+            this.warn(this.tokens[0], unsafeHashedWithoutHashWarningMessage);
+        }
         if (parseException) {
             throw INVALID_SOURCE_LIST;
         }
@@ -431,6 +437,8 @@ public class Parser {
             case "'unsafe-redirect'":
                 this.warn(token, "'unsafe-redirect' has been removed from CSP as of version 2.0.");
                 return KeywordSource.UnsafeRedirect;
+            case "'unsafe-hashed-attributes'":
+                return KeywordSource.UnsafeHashedAttributes;
             default:
                 checkForUnquotedKeyword(token);
                 if (token.value.startsWith("'nonce-")) {
diff --git a/src/main/java/com/shapesecurity/salvation/data/Policy.java b/src/main/java/com/shapesecurity/salvation/data/Policy.java
index ad39601..dc7a9a2 100644
--- a/src/main/java/com/shapesecurity/salvation/data/Policy.java
+++ b/src/main/java/com/shapesecurity/salvation/data/Policy.java
@@ -406,6 +406,15 @@ public class Policy implements Show {
         return sb.toString();
     }
 
+    private boolean defaultsAllowAttributeWithHash(@Nonnull HashAlgorithm algorithm, @Nonnull Base64Value hashValue) {
+        if (!this.defaultsHaveUnsafeHashedAttributes())
+            return false;
+        DefaultSrcDirective defaultSrcDirective = this.getDirectiveByType(DefaultSrcDirective.class);
+        if (defaultSrcDirective == null) {
+            return false;
+        }
+        return defaultSrcDirective.matchesHash(algorithm, hashValue);
+    }
 
     private boolean defaultsAllowHash(@Nonnull HashAlgorithm algorithm, @Nonnull Base64Value hashValue) {
         if (this.defaultsHaveUnsafeInline() && !this.defaultsHaveNonceSource() && !this.defaultsHaveHashSource() && !this.defaultsHaveStrictDynamic())
@@ -451,6 +460,14 @@ public class Policy implements Show {
         return defaultSrcDirective.values().anyMatch(x -> x == KeywordSource.UnsafeInline);
     }
 
+    private boolean defaultsHaveUnsafeHashedAttributes() {
+        DefaultSrcDirective defaultSrcDirective = this.getDirectiveByType(DefaultSrcDirective.class);
+        if (defaultSrcDirective == null) {
+            return false;
+        }
+        return defaultSrcDirective.values().anyMatch(x -> x == KeywordSource.UnsafeHashedAttributes);
+    }
+
     private boolean defaultsHaveNonceSource() {
         DefaultSrcDirective defaultSrcDirective = this.getDirectiveByType(DefaultSrcDirective.class);
         if (defaultSrcDirective == null) {
@@ -567,6 +584,16 @@ public class Policy implements Show {
         return scriptSrcDirective.matchesHash(algorithm, hashValue);
     }
 
+    public boolean allowsAttributeWithHash(@Nonnull HashAlgorithm algorithm, @Nonnull Base64Value hashValue) {
+        if (!this.haveUnsafeHashedAttributes())
+            return false;
+        ScriptSrcDirective scriptSrcDirective = this.getDirectiveByType(ScriptSrcDirective.class);
+        if (scriptSrcDirective == null) {
+            return this.defaultsAllowAttributeWithHash(algorithm, hashValue);
+        }
+        return scriptSrcDirective.matchesHash(algorithm, hashValue);
+    }
+
     public boolean allowsUnsafeInlineScript() {
         return containsSourceExpression(ScriptSrcDirective.class, x -> x == KeywordSource.UnsafeInline) &&
                 !containsSourceExpression(ScriptSrcDirective.class, x -> x instanceof NonceSource) &&
@@ -575,6 +602,11 @@ public class Policy implements Show {
 
     }
 
+    public boolean haveUnsafeHashedAttributes() {
+        return containsSourceExpression(ScriptSrcDirective.class, x -> x == KeywordSource.UnsafeHashedAttributes) ||
+                defaultsHaveUnsafeHashedAttributes();
+    }
+
     public <T extends SourceListDirective> boolean containsSourceExpression(Class<T> type, @Nonnull Predicate<SourceExpression> predicate) {
         T d = this.getDirectiveByType(type);
         if (d == null) {
diff --git a/src/main/java/com/shapesecurity/salvation/directiveValues/KeywordSource.java b/src/main/java/com/shapesecurity/salvation/directiveValues/KeywordSource.java
index be53c58..a33b0cf 100644
--- a/src/main/java/com/shapesecurity/salvation/directiveValues/KeywordSource.java
+++ b/src/main/java/com/shapesecurity/salvation/directiveValues/KeywordSource.java
@@ -14,6 +14,7 @@ public class KeywordSource implements SourceExpression, AncestorSource, MatchesS
     @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 public static final KeywordSource UnsafeHashedAttributes = new KeywordSource("unsafe-hashed-attributes");
     @Nonnull private final String value;
 
     private KeywordSource(@Nonnull String value) {


Test patch:
diff --git a/src/test/java/com/shapesecurity/salvation/LocationTest.java b/src/test/java/com/shapesecurity/salvation/LocationTest.java
index 6e1847e..33fd5d4 100644
--- a/src/test/java/com/shapesecurity/salvation/LocationTest.java
+++ b/src/test/java/com/shapesecurity/salvation/LocationTest.java
@@ -479,6 +479,34 @@ public class LocationTest extends CSPTest {
             notice.toString());
     }
 
+    @Test public void testUnsafeHashedAttributesWarnings() {
+        ArrayList<Notice> notices = new ArrayList<>();
+        ParserWithLocation
+                .parse("script-src 'unsafe-hashed-attributes'", URI.parse("https://origin"), notices);
+        assertEquals(1, notices.size());
+        Notice notice = notices.get(0);
+        assertEquals(
+                "1:1: The \"'unsafe-hashed-attributes'\" keyword-source has no effect in source lists that do not contain hash-source in CSP3 and later.",
+                notice.show());
+        assertEquals(
+                "Warning: The \"'unsafe-hashed-attributes'\" keyword-source has no effect in source lists that do not contain hash-source in CSP3 and later.",
+                notice.toString());
+
+        notices.clear();
+        ParserWithLocation
+                .parse("script-src self 'unsafe-redirect' 'unsafe-hashed-attributes'", URI.parse("https://origin"), notices);
+        assertEquals(3, notices.size());
+        notice = notices.get(2);
+        // TODO implement location tracking, 1:1: below is not very presize here
+        assertEquals(
+                "1:1: The \"'unsafe-hashed-attributes'\" keyword-source has no effect in source lists that do not contain hash-source in CSP3 and later.",
+                notice.show());
+        assertEquals(
+                "Warning: The \"'unsafe-hashed-attributes'\" keyword-source has no effect in source lists that do not contain hash-source in CSP3 and later.",
+                notice.toString());
+
+    }
+
     @Test public void testNoticeHelpers() {
         ArrayList<Notice> notices = new ArrayList<>();
         ParserWithLocation.parse(
diff --git a/src/test/java/com/shapesecurity/salvation/ParserTest.java b/src/test/java/com/shapesecurity/salvation/ParserTest.java
index a829634..4063bfd 100644
--- a/src/test/java/com/shapesecurity/salvation/ParserTest.java
+++ b/src/test/java/com/shapesecurity/salvation/ParserTest.java
@@ -1002,4 +1002,47 @@ public class ParserTest extends CSPTest {
         assertEquals(1, p.getDirectives().size());
         assertEquals(0, notices.size());
     }
+
+    @Test public void testUnsafeHashedAttributes() {
+        Policy p;
+        ArrayList<Notice> notices = new ArrayList<>();
+        p = parseWithNotices("default-src 'unsafe-hashed-attributes' 'sha512-vSsar3708Jvp9Szi2NWZZ02Bqp1qRCFpbcTZPdBhnWgs5WtNZKnvCXdhztmeD2cmW192CF5bDufKRpayrW/isg=='", notices);
+        assertEquals(1, p.getDirectives().size());
+        assertEquals(0, notices.size());
+
+        notices.clear();
+        p = parseWithNotices("default-src 'unsafe-hashed-attributes'", notices);
+        assertEquals(1, p.getDirectives().size());
+        assertEquals(1, notices.size());
+        assertEquals("The \"'unsafe-hashed-attributes'\" keyword-source has no effect in source lists that do not contain hash-source in CSP3 and later.", notices.get(0).message);
+
+        notices.clear();
+        p = parseWithNotices("default-src 'unsafe-hashed-attributes' 'unsafe-hashed-attributes' 'unsafe-hashed-attributes'", notices);
+        assertEquals(1, p.getDirectives().size());
+        assertEquals(1, notices.size());
+        assertEquals("The \"'unsafe-hashed-attributes'\" keyword-source has no effect in source lists that do not contain hash-source in CSP3 and later.", notices.get(0).message);
+
+        notices.clear();
+        p = parseWithNotices("default-src 'unsafe-hashed-attributes' 'unsafe-hashed-attributes' 'unsafe-hashed-attributes' 'nonce-123'", notices);
+        assertEquals(1, p.getDirectives().size());
+        assertEquals(2, notices.size());
+        assertEquals("Invalid base64-value (should be multiple of 4 bytes: 3).", notices.get(0).message);
+        assertEquals("The \"'unsafe-hashed-attributes'\" keyword-source has no effect in source lists that do not contain hash-source in CSP3 and later.", notices.get(1).message);
+
+        notices.clear();
+        p = parseWithNotices("default-src 'unsafe-hashed-attributes' 'unsafe-hashed-attributes' 'unsafe-hashed-attributes' 'sha512-vSsar3708Jvp9Szi2NWZZ02Bqp1qRCFpbcTZPdBhnWgs5WtNZKnvCXdhztmeD2cmW192CF5bDufKRpayrW/isg=='", notices);
+        assertEquals(1, p.getDirectives().size());
+        assertEquals(0, notices.size());
+
+        notices.clear();
+        p = parseWithNotices("default-src 'sha512-vSsar3708Jvp9Szi2NWZZ02Bqp1qRCFpbcTZPdBhnWgs5WtNZKnvCXdhztmeD2cmW192CF5bDufKRpayrW/isg=='", notices);
+        assertEquals(1, p.getDirectives().size());
+        assertEquals(0, notices.size());
+
+        // while grammar allows this, I am open to throw warnings about directives that don't make sense with 'usnafe-hashed-attributes'
+        notices.clear();
+        p = parseWithNotices("img-src 'unsafe-hashed-attributes' 'sha512-vSsar3708Jvp9Szi2NWZZ02Bqp1qRCFpbcTZPdBhnWgs5WtNZKnvCXdhztmeD2cmW192CF5bDufKRpayrW/isg=='", notices);
+        assertEquals(1, p.getDirectives().size());
+        assertEquals(0, notices.size());
+    }
 }
diff --git a/src/test/java/com/shapesecurity/salvation/PolicyQueryingTest.java b/src/test/java/com/shapesecurity/salvation/PolicyQueryingTest.java
index f1bdfb6..fd502fc 100644
--- a/src/test/java/com/shapesecurity/salvation/PolicyQueryingTest.java
+++ b/src/test/java/com/shapesecurity/salvation/PolicyQueryingTest.java
@@ -340,6 +340,34 @@ public class PolicyQueryingTest extends CSPTest {
         assertFalse("unsafe inline style is not allowed", p.allowsUnsafeInlineStyle());
     }
 
+    @Test public void testAllowsAttributeWithHash() {
+        Policy p;
+
+        p = parse(
+                "script-src 'unsafe-hashed-attributes' 'sha512-vSsar3708Jvp9Szi2NWZZ02Bqp1qRCFpbcTZPdBhnWgs5WtNZKnvCXdhztmeD2cmW192CF5bDufKRpayrW/isg=='");
+        assertTrue("attribute with hash is allowed", p.allowsAttributeWithHash(HashSource.HashAlgorithm.SHA512, new Base64Value(
+                "vSsar3708Jvp9Szi2NWZZ02Bqp1qRCFpbcTZPdBhnWgs5WtNZKnvCXdhztmeD2cmW192CF5bDufKRpayrW/isg==")));
+        assertFalse("script hash is not allowed",
+                p.allowsAttributeWithHash(HashSource.HashAlgorithm.SHA512, new Base64Value("cGl6ZGE=")));
+
+        p = parse(
+                "script-src 'sha512-vSsar3708Jvp9Szi2NWZZ02Bqp1qRCFpbcTZPdBhnWgs5WtNZKnvCXdhztmeD2cmW192CF5bDufKRpayrW/isg=='");
+        assertFalse("attribute with hash is not allowed", p.allowsAttributeWithHash(HashSource.HashAlgorithm.SHA512, new Base64Value(
+                "vSsar3708Jvp9Szi2NWZZ02Bqp1qRCFpbcTZPdBhnWgs5WtNZKnvCXdhztmeD2cmW192CF5bDufKRpayrW/isg==")));
+
+        p = parse(
+                "default-src 'unsafe-hashed-attributes' 'sha512-vSsar3708Jvp9Szi2NWZZ02Bqp1qRCFpbcTZPdBhnWgs5WtNZKnvCXdhztmeD2cmW192CF5bDufKRpayrW/isg=='");
+        assertTrue("attribute with hash is allowed", p.allowsAttributeWithHash(HashSource.HashAlgorithm.SHA512, new Base64Value(
+                "vSsar3708Jvp9Szi2NWZZ02Bqp1qRCFpbcTZPdBhnWgs5WtNZKnvCXdhztmeD2cmW192CF5bDufKRpayrW/isg==")));
+        assertFalse("script hash is not allowed",
+                p.allowsAttributeWithHash(HashSource.HashAlgorithm.SHA512, new Base64Value("cGl6ZGE=")));
+        
+        p = parse(
+                "default-src 'sha512-vSsar3708Jvp9Szi2NWZZ02Bqp1qRCFpbcTZPdBhnWgs5WtNZKnvCXdhztmeD2cmW192CF5bDufKRpayrW/isg=='");
+        assertFalse("attribute with hash is not allowed", p.allowsAttributeWithHash(HashSource.HashAlgorithm.SHA512, new Base64Value(
+                "vSsar3708Jvp9Szi2NWZZ02Bqp1qRCFpbcTZPdBhnWgs5WtNZKnvCXdhztmeD2cmW192CF5bDufKRpayrW/isg==")));
+    }
+
     @Test public void testAllowsConnect() {
         Policy p;