Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Subsume host-sources when union merge #205

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

Subsume host-sources when union merge #205

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
6e9cbc3
Subsume host-sources when union merge
Aug 15, 2018
bf959f3
Merge branch 'master' into wildcard-elimination
shekyan Dec 19, 2018
e37e5bf
Merge branch 'master' into wildcard-elimination
shekyan Apr 23, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/main/java/com/shapesecurity/salvation/data/Policy.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -183,7 +183,7 @@ private void optimise() {
if (directive instanceof SourceListDirective) {
SourceListDirective sourceListDirective = (SourceListDirective) directive;
Optional<SourceExpression> star =
sourceListDirective.values().filter(x -> x instanceof HostSource && ((HostSource) x).isWildcard())
sourceListDirective.values().filter(x -> x instanceof HostSource && ((HostSource) x).isTLDWildcard())
.findAny();
if (star.isPresent()) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should change this to also do the subsumption test.

Set<SourceExpression> newSources = sourceListDirective.values()
Expand Down
43 changes: 30 additions & 13 deletions src/main/java/com/shapesecurity/salvation/directiveValues/HostSource.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,14 +1,5 @@
package com.shapesecurity.salvation.directiveValues;

import com.shapesecurity.salvation.Constants;
import com.shapesecurity.salvation.data.GUID;
import com.shapesecurity.salvation.data.Origin;
import com.shapesecurity.salvation.data.SchemeHostPortTriple;
import com.shapesecurity.salvation.data.URI;
import com.shapesecurity.salvation.interfaces.MatchesSource;

import javax.annotation.Nonnull;
import javax.annotation.Nullable;
import java.io.UnsupportedEncodingException;
import java.net.URLDecoder;
import java.util.ArrayList;
Expand All @@ -17,6 +8,16 @@
import java.util.Objects;
import java.util.regex.Matcher;

import javax.annotation.Nonnull;
import javax.annotation.Nullable;

import com.shapesecurity.salvation.Constants;
import com.shapesecurity.salvation.data.GUID;
import com.shapesecurity.salvation.data.Origin;
import com.shapesecurity.salvation.data.SchemeHostPortTriple;
import com.shapesecurity.salvation.data.URI;
import com.shapesecurity.salvation.interfaces.MatchesSource;


public class HostSource implements SourceExpression, AncestorSource, MatchesSource {
public static final HostSource WILDCARD = new HostSource(null, "*", Constants.EMPTY_PORT, null);
Expand Down Expand Up @@ -46,7 +47,7 @@ public boolean equals(@Nullable Object other) {
return false;
}
HostSource otherPrime = (HostSource) other;
if (this.isWildcard() && otherPrime.isWildcard()) {
if (this.isTLDWildcard() && otherPrime.isTLDWildcard()) {
return true;
}

Expand Down Expand Up @@ -75,10 +76,26 @@ public int hashCode() {
return h;
}

public boolean isWildcard() {
public boolean isTLDWildcard() {
return this.host.equals("*") && this.scheme == null && this.port == Constants.EMPTY_PORT && this.path == null;
}

public boolean isSubdomainWildcard() {
return this.host.startsWith("*.");
}

public boolean subsumes(@Nonnull HostSource other) {
if (this.isTLDWildcard()) return true;
if (this.isSubdomainWildcard() && !other.isTLDWildcard() && !other.isSubdomainWildcard()) {
return Objects.equals(this.scheme != null ? this.scheme.toLowerCase() : null,
other.scheme != null ? other.scheme.toLowerCase() : null) &&
other.host.toLowerCase().endsWith(this.host.substring(1).toLowerCase()) &&
this.port == other.port &&
Objects.equals(this.path, other.path);
}
return this.equals(other);
}

public static boolean hostMatches(@Nonnull String hostA, @Nonnull String hostB) {
if (hostA.startsWith("*")) {
String remaining = hostA.substring(1);
Expand Down Expand Up @@ -121,12 +138,12 @@ public boolean matchesSource(@Nonnull Origin origin, @Nonnull GUID resource) {
public boolean matchesSource(@Nonnull Origin origin, @Nonnull URI resource) {
if (origin instanceof GUID) {
// wildcard matches a network scheme
return this.isWildcard() && resource.isNetworkScheme();
return this.isTLDWildcard() && resource.isNetworkScheme();
} else if (!(origin instanceof SchemeHostPortTriple)) {
return false;
}
SchemeHostPortTriple shpOrigin = (SchemeHostPortTriple) origin;
if (this.isWildcard()) {
if (this.isTLDWildcard()) {
return resource.isNetworkScheme() || shpOrigin.scheme.matches(resource.scheme);
}
boolean schemeMatches;
Expand Down
12 changes: 5 additions & 7 deletions src/main/java/com/shapesecurity/salvation/directives/Directive.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -130,11 +130,9 @@ private static <T> Set<T> union(@Nonnull Set<T> a, @Nonnull Set<T> b) {

set.remove(None.INSTANCE);

Optional<T> star = set.stream().filter(x -> x instanceof HostSource && ((HostSource) x).isWildcard()).findAny();
if (star.isPresent()) {
set.removeIf(y -> y instanceof HostSource);
set.add(star.get());
}
set.stream().filter(x -> x instanceof HostSource).collect(Collectors.toList()).forEach(x -> {
set.removeIf(y -> y != x && y instanceof HostSource && ((HostSource) x).subsumes((HostSource) y));
});

return set;
}
Expand All @@ -159,14 +157,14 @@ private static <T> Set<T> intersect(@Nonnull Set<T> a, @Nonnull Set<T> b) {
return set;
}

Optional<T> star = b.stream().filter(x -> x instanceof HostSource && ((HostSource) x).isWildcard()).findAny();
Optional<T> star = b.stream().filter(x -> x instanceof HostSource && ((HostSource) x).isTLDWildcard()).findAny();
if (star.isPresent()) {
set.addAll(a);
return set;
}

for (T x : a) {
if (x instanceof HostSource && ((HostSource) x).isWildcard()) {
if (x instanceof HostSource && ((HostSource) x).isTLDWildcard()) {
set.clear();
set.addAll(b);
return set;
Expand Down
54 changes: 54 additions & 0 deletions src/test/java/com/shapesecurity/salvation/PolicyMergeTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -709,6 +709,60 @@ public void testUnionNone() {
assertEquals("script-src *", p.show());
}

@Test
public void testUnionTLDWildcard() {
Policy p = parse("default-src *");
Policy q = parse("default-src http://b.c.atest.com");
p.union(q);
assertEquals("default-src *", p.show());

p = parse("default-src *");
q = parse("default-src http://*.c.atest.com");
p.union(q);
assertEquals("default-src *", p.show());
}

@Test
public void testUnionSubdomainWildcard() {
Policy p = parse("default-src http://*.atest.com");
Policy q = parse("default-src http://b.c.atest.com");
p.union(q);
assertEquals("default-src http://*.atest.com", p.show());

p = parse("default-src http://*.b.atest.com");
q = parse("default-src http://x.b.atest.com");
p.union(q);
assertEquals("default-src http://*.b.atest.com", p.show());

p = parse("default-src https://*.b.atest.com:8443");
q = parse("default-src https://x.b.atest.com:8443");
p.union(q);
assertEquals("default-src https://*.b.atest.com:8443", p.show());

p = parse("default-src https://*.b.atest.com:8443/a");
q = parse("default-src https://x.b.atest.com:8443/a");
p.union(q);
assertEquals("default-src https://*.b.atest.com:8443/a", p.show());
}

@Test
public void testUnionFails() {
Policy p = parse("default-src https://*.atest.com");
Policy q = parse("default-src http://b.c.atest.com");
p.union(q);
assertEquals("default-src https://*.atest.com http://b.c.atest.com", p.show());

p = parse("default-src http://*.atest.com:80");
q = parse("default-src http://b.c.atest.com:8080");
p.union(q);
assertEquals("default-src http://*.atest.com http://b.c.atest.com:8080", p.show());

p = parse("default-src http://*.atest.com/a");
q = parse("default-src http://b.c.atest.com/a/b");
p.union(q);
assertEquals("default-src http://*.atest.com/a http://b.c.atest.com/a/b", p.show());
}

@Test
public void testMergeCommutativity() {
String[] policies = new String[] {
Expand Down