Repository: shapesecurity/salvation
Pull request #162 (2016-12-02 18:58:07)

fixes GH-161: Check if a policy is using strict-dynamic
introduces hasStrictDynamic, as well as modifying behaviour of
`unsafe-inline`, hash-source, nonce-source querying functions to
take into account presence of strict-dynamic keyword in the expression.
strict-dynamic invalidates unsafe-inline for the scripts and has no
effect for the styles.


Patch:
diff --git a/src/main/java/com/shapesecurity/salvation/data/Policy.java b/src/main/java/com/shapesecurity/salvation/data/Policy.java
index 0580e8d..2661d83 100644
--- a/src/main/java/com/shapesecurity/salvation/data/Policy.java
+++ b/src/main/java/com/shapesecurity/salvation/data/Policy.java
@@ -345,7 +345,7 @@ public class Policy implements Show {
 
 
     private boolean defaultsAllowHash(@Nonnull HashAlgorithm algorithm, @Nonnull Base64Value hashValue) {
-        if (this.defaultsAllowUnsafeInline())
+        if (this.defaultsHaveUnsafeInline() && !this.defaultsHaveStrictDynamic())
             return true;
         DefaultSrcDirective defaultSrcDirective = this.getDirectiveByType(DefaultSrcDirective.class);
         if (defaultSrcDirective == null) {
@@ -355,7 +355,7 @@ public class Policy implements Show {
     }
 
     private boolean defaultsAllowNonce(@Nonnull String nonce) {
-        if (this.defaultsAllowUnsafeInline())
+        if (this.defaultsHaveUnsafeInline() && !this.defaultsHaveStrictDynamic())
             return true;
         DefaultSrcDirective defaultSrcDirective = this.getDirectiveByType(DefaultSrcDirective.class);
         if (defaultSrcDirective == null) {
@@ -368,8 +368,6 @@ public class Policy implements Show {
         return this.defaultsAllowNonce(nonce.value);
     }
 
-
-    // 7.4.1
     private boolean defaultsAllowSource(@Nonnull URI source) {
         DefaultSrcDirective defaultSrcDirective = this.getDirectiveByType(DefaultSrcDirective.class);
         if (defaultSrcDirective == null) {
@@ -386,7 +384,7 @@ public class Policy implements Show {
         return defaultSrcDirective.matchesSource(this.origin, source);
     }
 
-    private boolean defaultsAllowUnsafeInline() {
+    private boolean defaultsHaveUnsafeInline() {
         DefaultSrcDirective defaultSrcDirective = this.getDirectiveByType(DefaultSrcDirective.class);
         if (defaultSrcDirective == null) {
             return false;
@@ -394,6 +392,21 @@ public class Policy implements Show {
         return defaultSrcDirective.values().anyMatch(x -> x == KeywordSource.UnsafeInline);
     }
 
+    private boolean defaultsHaveStrictDynamic() {
+        DefaultSrcDirective defaultSrcDirective = this.getDirectiveByType(DefaultSrcDirective.class);
+        if (defaultSrcDirective == null) {
+            return false;
+        }
+        return defaultSrcDirective.values().anyMatch(x -> x == KeywordSource.StrictDynamic);
+    }
+
+    public boolean hasStrictDynamic() {
+        ScriptSrcDirective scriptSrcDirective = this.getDirectiveByType(ScriptSrcDirective.class);
+        if (scriptSrcDirective == null) {
+            return this.defaultsHaveStrictDynamic();
+        }
+        return scriptSrcDirective.values().anyMatch(x -> x == KeywordSource.StrictDynamic);
+    }
 
     public boolean allowsImgFromSource(@Nonnull URI source) {
         ImgSrcDirective imgSrcDirective = this.getDirectiveByType(ImgSrcDirective.class);
@@ -480,27 +493,34 @@ public class Policy implements Show {
     }
 
     public boolean allowsUnsafeInlineScript() {
+        return this.hasUnsafeInlineScript() && !this.hasStrictDynamic();
+    }
+
+    public boolean hasUnsafeInlineScript() {
         ScriptSrcDirective scriptSrcDirective = this.getDirectiveByType(ScriptSrcDirective.class);
         if (scriptSrcDirective == null) {
-            return this.defaultsAllowUnsafeInline();
+            return this.defaultsHaveUnsafeInline();
         }
         return scriptSrcDirective.values().anyMatch(x -> x == KeywordSource.UnsafeInline);
     }
 
-    public boolean allowsUnsafeInlineStyle() {
+    public boolean hasUnsafeInlineStyle() {
         StyleSrcDirective styleSrcDirective = this.getDirectiveByType(StyleSrcDirective.class);
         if (styleSrcDirective == null) {
-            return this.defaultsAllowUnsafeInline();
+            return this.defaultsHaveUnsafeInline();
         }
         return styleSrcDirective.values().anyMatch(x -> x == KeywordSource.UnsafeInline);
     }
 
+    public boolean allowsUnsafeInlineStyle() {
+        return this.hasUnsafeInlineStyle();
+    }
+
     public boolean allowsPlugin(@Nonnull MediaType mediaType) {
         PluginTypesDirective pluginTypesDirective = this.getDirectiveByType(PluginTypesDirective.class);
         if (pluginTypesDirective == null) {
             return false;
         }
-
         return pluginTypesDirective.matchesMediaType(mediaType);
     }
 
@@ -519,7 +539,7 @@ public class Policy implements Show {
     }
 
     public boolean allowsStyleWithNonce(@Nonnull String nonce) {
-        if (this.allowsUnsafeInlineScript())
+        if (this.allowsUnsafeInlineStyle())
             return true;
         StyleSrcDirective styleSrcDirective = this.getDirectiveByType(StyleSrcDirective.class);
         if (styleSrcDirective == null) {


Test patch:
diff --git a/src/test/java/com/shapesecurity/salvation/PolicyQueryingTest.java b/src/test/java/com/shapesecurity/salvation/PolicyQueryingTest.java
index 425d520..15517f6 100644
--- a/src/test/java/com/shapesecurity/salvation/PolicyQueryingTest.java
+++ b/src/test/java/com/shapesecurity/salvation/PolicyQueryingTest.java
@@ -385,6 +385,74 @@ public class PolicyQueryingTest extends CSPTest {
 
     }
 
+    @Test public void testStrictDynamic() {
+        Policy p;
+
+        // NOTE, this behaviour will likely change, but for now presence of hash does not invalidate unsafe-inline
+        p = Parser.parse("default-src 'unsafe-inline' 'strict-dynamic'", "http://example.com");
+        assertTrue(p.hasStrictDynamic());
+        assertTrue(p.hasUnsafeInlineScript());
+        assertFalse(p.allowsUnsafeInlineScript());
+        assertTrue(p.hasUnsafeInlineStyle());
+        assertTrue(p.allowsUnsafeInlineStyle());
+        assertFalse(p.allowsScriptWithNonce("123"));
+        assertTrue(p.allowsStyleWithNonce("123"));
+        assertFalse(p.allowsScriptWithHash(HashSource.HashAlgorithm.SHA512, new Base64Value(
+                "vSsar3708Jvp9Szi2NWZZ02Bqp1qRCFpbcTZPdBhnWgs5WtNZKnvCXdhztmeD2cmW192CF5bDufKRpayrW/isg==")));
+        assertTrue(p.allowsStyleWithHash(HashSource.HashAlgorithm.SHA512, new Base64Value(
+                "vSsar3708Jvp9Szi2NWZZ02Bqp1qRCFpbcTZPdBhnWgs5WtNZKnvCXdhztmeD2cmW192CF5bDufKRpayrW/isg==")));
+
+        // NOTE, this behaviour will likely change, but for now presence of hash does not invalidate unsafe-inline
+        p = Parser.parse("default-src 'unsafe-inline' 'strict-dynamic' 'nonce-123' 'sha512-vSsar3708Jvp9Szi2NWZZ02Bqp1qRCFpbcTZPdBhnWgs5WtNZKnvCXdhztmeD2cmW192CF5bDufKRpayrW/isg=='", "http://example.com");
+        assertTrue(p.hasStrictDynamic());
+        assertTrue(p.hasUnsafeInlineScript());
+        assertFalse(p.allowsUnsafeInlineScript());
+        assertTrue(p.allowsScriptWithNonce("123"));
+        assertFalse(p.allowsScriptWithNonce("345"));
+        assertTrue(p.allowsScriptWithHash(HashSource.HashAlgorithm.SHA512, new Base64Value(
+                "vSsar3708Jvp9Szi2NWZZ02Bqp1qRCFpbcTZPdBhnWgs5WtNZKnvCXdhztmeD2cmW192CF5bDufKRpayrW/isg==")));
+        assertFalse(p.allowsScriptWithHash(HashSource.HashAlgorithm.SHA512, new Base64Value("cGl6ZGE=")));
+
+        assertTrue(p.allowsUnsafeInlineStyle());
+        assertTrue(p.hasUnsafeInlineStyle());
+        assertTrue(p.allowsStyleWithNonce("123"));
+        assertTrue(p.allowsStyleWithNonce("345"));
+        assertTrue(p.allowsStyleWithHash(HashSource.HashAlgorithm.SHA512, new Base64Value(
+                "vSsar3708Jvp9Szi2NWZZ02Bqp1qRCFpbcTZPdBhnWgs5WtNZKnvCXdhztmeD2cmW192CF5bDufKRpayrW/isg==")));
+        assertTrue(p.allowsStyleWithHash(HashSource.HashAlgorithm.SHA512, new Base64Value("cGl6ZGE=")));
+
+        p = Parser.parse("default-src 'unsafe-inline' 'strict-dynamic' 'nonce-123' 'sha512-vSsar3708Jvp9Szi2NWZZ02Bqp1qRCFpbcTZPdBhnWgs5WtNZKnvCXdhztmeD2cmW192CF5bDufKRpayrW/isg=='; script-src;", "http://example.com");
+        assertFalse(p.hasStrictDynamic());
+        assertFalse(p.hasUnsafeInlineScript());
+        assertFalse(p.allowsUnsafeInlineScript());
+        assertTrue(p.hasUnsafeInlineStyle());
+        assertTrue(p.allowsUnsafeInlineStyle());
+
+        assertFalse(p.allowsScriptWithNonce("123"));
+        assertFalse(p.allowsScriptWithHash(HashSource.HashAlgorithm.SHA512, new Base64Value(
+                "vSsar3708Jvp9Szi2NWZZ02Bqp1qRCFpbcTZPdBhnWgs5WtNZKnvCXdhztmeD2cmW192CF5bDufKRpayrW/isg==")));
+
+        assertTrue(p.allowsStyleWithNonce("123"));
+        assertTrue(p.allowsStyleWithHash(HashSource.HashAlgorithm.SHA512, new Base64Value(
+                "vSsar3708Jvp9Szi2NWZZ02Bqp1qRCFpbcTZPdBhnWgs5WtNZKnvCXdhztmeD2cmW192CF5bDufKRpayrW/isg==")));
+
+        p = Parser.parse("default-src 'unsafe-inline' 'strict-dynamic' 'nonce-123' 'sha512-vSsar3708Jvp9Szi2NWZZ02Bqp1qRCFpbcTZPdBhnWgs5WtNZKnvCXdhztmeD2cmW192CF5bDufKRpayrW/isg=='; style-src;", "http://example.com");
+        assertTrue(p.hasStrictDynamic());
+        assertTrue(p.hasUnsafeInlineScript());
+        assertFalse(p.allowsUnsafeInlineScript());
+        assertFalse(p.hasUnsafeInlineStyle());
+        assertFalse(p.allowsUnsafeInlineStyle());
+
+        assertTrue(p.allowsScriptWithNonce("123"));
+        assertFalse(p.allowsScriptWithNonce("345"));
+        assertTrue(p.allowsScriptWithHash(HashSource.HashAlgorithm.SHA512, new Base64Value(
+                "vSsar3708Jvp9Szi2NWZZ02Bqp1qRCFpbcTZPdBhnWgs5WtNZKnvCXdhztmeD2cmW192CF5bDufKRpayrW/isg==")));
+
+        assertFalse(p.allowsStyleWithNonce("123"));
+        assertFalse(p.allowsStyleWithHash(HashSource.HashAlgorithm.SHA512, new Base64Value(
+                "vSsar3708Jvp9Szi2NWZZ02Bqp1qRCFpbcTZPdBhnWgs5WtNZKnvCXdhztmeD2cmW192CF5bDufKRpayrW/isg==")));
+    }
+
     @Test public void testWildcards() {
         Policy p;