Skip to content

Commit

Permalink
Support for 'unsafe-hashed-attributes' (#179)
Browse files Browse the repository at this point in the history
- Parse 'unsafe-hashed-attributes'
- warn about nonsensical 'unsafe-hashed-attributes' without hash-source
- answer questions about attributes
  • Loading branch information
shekyan authored Apr 11, 2017
1 parent ae5cbaa commit 97fa105
Show file tree
Hide file tree
Showing 7 changed files with 142 additions and 2 deletions.
2 changes: 1 addition & 1 deletion salvation.iml
Original file line number Diff line number Diff line change
@@ -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$">
Expand Down
10 changes: 9 additions & 1 deletion src/main/java/com/shapesecurity/salvation/Parser.java
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -386,12 +387,17 @@ private void enforceMissingDirectiveValue(@Nonnull Token directiveNameToken) thr
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;
}
Expand Down Expand Up @@ -431,6 +437,8 @@ private void enforceMissingDirectiveValue(@Nonnull Token directiveNameToken) thr
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-")) {
Expand Down
32 changes: 32 additions & 0 deletions src/main/java/com/shapesecurity/salvation/data/Policy.java
Original file line number Diff line number Diff line change
Expand Up @@ -406,6 +406,15 @@ public <V extends DirectiveValue, T extends Directive<V>> T getDirectiveByType(@
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())
Expand Down Expand Up @@ -451,6 +460,14 @@ private boolean defaultsHaveUnsafeInline() {
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) {
Expand Down Expand Up @@ -567,6 +584,16 @@ public boolean allowsScriptWithHash(@Nonnull HashAlgorithm algorithm, @Nonnull B
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) &&
Expand All @@ -575,6 +602,11 @@ public boolean allowsUnsafeInlineScript() {

}

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) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
28 changes: 28 additions & 0 deletions src/test/java/com/shapesecurity/salvation/LocationTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
43 changes: 43 additions & 0 deletions src/test/java/com/shapesecurity/salvation/ParserTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -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());
}
}
28 changes: 28 additions & 0 deletions src/test/java/com/shapesecurity/salvation/PolicyQueryingTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down

0 comments on commit 97fa105

Please sign in to comment.