From d83a68f3ff6e3833278a38798d06215293656e85 Mon Sep 17 00:00:00 2001 From: SquidDev Date: Sat, 5 Dec 2020 11:32:00 +0000 Subject: [PATCH] Allow $private HTTP rule to block any private IP This is a little magic compared with our previous approach of "list every private IP range", but given then the sheer number we were missing[1][2] this feels more reasonable. Also refactor out some of the logic into separate classes, hopefully to make things a little cleaner. Fixes #594. [1]: https://www.iana.org/assignments/iana-ipv4-special-registry/iana-ipv4-special-registry.xhtml [2]: https://www.iana.org/assignments/iana-ipv6-special-registry/iana-ipv6-special-registry.xhtml --- build.gradle | 5 +- .../dan200/computercraft/ComputerCraft.java | 27 +--- .../apis/http/options/AddressPredicate.java | 149 ++++++++++++++++++ .../core/apis/http/options/AddressRule.java | 144 ++++------------- .../dan200/computercraft/shared/Config.java | 12 +- .../apis/http/options/AddressRuleTest.java | 14 ++ .../test-rom/spec/apis/http_spec.lua | 2 + 7 files changed, 209 insertions(+), 144 deletions(-) create mode 100644 src/main/java/dan200/computercraft/core/apis/http/options/AddressPredicate.java diff --git a/build.gradle b/build.gradle index 4f698a445..e19ddfc25 100644 --- a/build.gradle +++ b/build.gradle @@ -116,8 +116,9 @@ dependencies { shade 'org.squiddev:Cobalt:0.5.1-SNAPSHOT' - testImplementation 'org.junit.jupiter:junit-jupiter-api:5.4.2' - testRuntimeOnly 'org.junit.jupiter:junit-jupiter-engine:5.4.2' + testImplementation 'org.junit.jupiter:junit-jupiter-api:5.7.0' + testImplementation 'org.junit.jupiter:junit-jupiter-params:5.7.0' + testRuntimeOnly 'org.junit.jupiter:junit-jupiter-engine:5.7.0' testImplementation 'org.hamcrest:hamcrest:2.2' deployerJars "org.apache.maven.wagon:wagon-ssh:3.0.0" diff --git a/src/main/java/dan200/computercraft/ComputerCraft.java b/src/main/java/dan200/computercraft/ComputerCraft.java index d510ab65b..2daa28eb0 100644 --- a/src/main/java/dan200/computercraft/ComputerCraft.java +++ b/src/main/java/dan200/computercraft/ComputerCraft.java @@ -22,30 +22,17 @@ import net.minecraftforge.fml.common.Mod; import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; +import java.util.Arrays; import java.util.Collections; import java.util.EnumSet; import java.util.List; -import java.util.Objects; import java.util.concurrent.TimeUnit; -import java.util.stream.Collectors; -import java.util.stream.Stream; @Mod( ComputerCraft.MOD_ID ) public final class ComputerCraft { public static final String MOD_ID = "computercraft"; - // Configuration options - public static final String[] DEFAULT_HTTP_ALLOW = new String[] { "*" }; - public static final String[] DEFAULT_HTTP_DENY = new String[] { - "127.0.0.0/8", - "0.0.0.0/8", - "10.0.0.0/8", - "172.16.0.0/12", - "192.168.0.0/16", - "fd00::/8", - }; - public static int computerSpaceLimit = 1000 * 1000; public static int floppySpaceLimit = 125 * 1000; public static int maximumFilesOpen = 128; @@ -61,14 +48,10 @@ public final class ComputerCraft public static boolean httpEnabled = true; public static boolean httpWebsocketEnabled = true; - public static List httpRules = Collections.unmodifiableList( Stream.concat( - Stream.of( DEFAULT_HTTP_DENY ) - .map( x -> AddressRule.parse( x, null, Action.DENY.toPartial() ) ) - .filter( Objects::nonNull ), - Stream.of( DEFAULT_HTTP_ALLOW ) - .map( x -> AddressRule.parse( x, null, Action.ALLOW.toPartial() ) ) - .filter( Objects::nonNull ) - ).collect( Collectors.toList() ) ); + public static List httpRules = Collections.unmodifiableList( Arrays.asList( + AddressRule.parse( "$private", null, Action.DENY.toPartial() ), + AddressRule.parse( "*", null, Action.ALLOW.toPartial() ) + ) ); public static int httpMaxRequests = 16; public static int httpMaxWebsockets = 4; diff --git a/src/main/java/dan200/computercraft/core/apis/http/options/AddressPredicate.java b/src/main/java/dan200/computercraft/core/apis/http/options/AddressPredicate.java new file mode 100644 index 000000000..b647f73a3 --- /dev/null +++ b/src/main/java/dan200/computercraft/core/apis/http/options/AddressPredicate.java @@ -0,0 +1,149 @@ +/* + * This file is part of ComputerCraft - http://www.computercraft.info + * Copyright Daniel Ratcliffe, 2011-2020. Do not distribute without permission. + * Send enquiries to dratcliffe@gmail.com + */ + +package dan200.computercraft.core.apis.http.options; + +import com.google.common.net.InetAddresses; +import dan200.computercraft.ComputerCraft; + +import java.net.InetAddress; +import java.net.InetSocketAddress; +import java.util.regex.Pattern; + +/** + * A predicate on an address. Matches against a domain and an ip address. + * + * @see AddressRule#apply(Iterable, String, InetSocketAddress) for the actual handling of this rule. + */ +interface AddressPredicate +{ + default boolean matches( String domain ) + { + return false; + } + + default boolean matches( InetAddress socketAddress ) + { + return false; + } + + final class HostRange implements AddressPredicate + { + private final byte[] min; + private final byte[] max; + + HostRange( byte[] min, byte[] max ) + { + this.min = min; + this.max = max; + } + + @Override + public boolean matches( InetAddress address ) + { + byte[] entry = address.getAddress(); + if( entry.length != min.length ) return false; + + for( int i = 0; i < entry.length; i++ ) + { + int value = 0xFF & entry[i]; + if( value < (0xFF & min[i]) || value > (0xFF & max[i]) ) return false; + } + + return true; + } + + public static HostRange parse( String addressStr, String prefixSizeStr ) + { + int prefixSize; + try + { + prefixSize = Integer.parseInt( prefixSizeStr ); + } + catch( NumberFormatException e ) + { + ComputerCraft.log.error( + "Malformed http whitelist/blacklist entry '{}': Cannot extract size of CIDR mask from '{}'.", + addressStr + '/' + prefixSizeStr, prefixSizeStr + ); + return null; + } + + InetAddress address; + try + { + address = InetAddresses.forString( addressStr ); + } + catch( IllegalArgumentException e ) + { + ComputerCraft.log.error( + "Malformed http whitelist/blacklist entry '{}': Cannot extract IP address from '{}'.", + addressStr + '/' + prefixSizeStr, prefixSizeStr + ); + return null; + } + + // Mask the bytes of the IP address. + byte[] minBytes = address.getAddress(), maxBytes = address.getAddress(); + int size = prefixSize; + for( int i = 0; i < minBytes.length; i++ ) + { + if( size <= 0 ) + { + minBytes[i] &= 0; + maxBytes[i] |= 0xFF; + } + else if( size < 8 ) + { + minBytes[i] &= 0xFF << (8 - size); + maxBytes[i] |= ~(0xFF << (8 - size)); + } + + size -= 8; + } + + return new HostRange( minBytes, maxBytes ); + } + } + + final class DomainPattern implements AddressPredicate + { + private final Pattern pattern; + + DomainPattern( Pattern pattern ) + { + this.pattern = pattern; + } + + @Override + public boolean matches( String domain ) + { + return pattern.matcher( domain ).matches(); + } + + @Override + public boolean matches( InetAddress socketAddress ) + { + return pattern.matcher( socketAddress.getHostAddress() ).matches(); + } + } + + + final class PrivatePattern implements AddressPredicate + { + static final PrivatePattern INSTANCE = new PrivatePattern(); + + @Override + public boolean matches( InetAddress socketAddress ) + { + return socketAddress.isAnyLocalAddress() + || socketAddress.isLoopbackAddress() + || socketAddress.isLinkLocalAddress() + || socketAddress.isSiteLocalAddress(); + } + } + +} diff --git a/src/main/java/dan200/computercraft/core/apis/http/options/AddressRule.java b/src/main/java/dan200/computercraft/core/apis/http/options/AddressRule.java index c7498f269..f1884e3f3 100644 --- a/src/main/java/dan200/computercraft/core/apis/http/options/AddressRule.java +++ b/src/main/java/dan200/computercraft/core/apis/http/options/AddressRule.java @@ -6,10 +6,13 @@ package dan200.computercraft.core.apis.http.options; import com.google.common.net.InetAddresses; -import dan200.computercraft.ComputerCraft; +import dan200.computercraft.core.apis.http.options.AddressPredicate.DomainPattern; +import dan200.computercraft.core.apis.http.options.AddressPredicate.HostRange; +import dan200.computercraft.core.apis.http.options.AddressPredicate.PrivatePattern; import javax.annotation.Nonnull; import javax.annotation.Nullable; +import java.net.Inet4Address; import java.net.Inet6Address; import java.net.InetAddress; import java.net.InetSocketAddress; @@ -25,45 +28,13 @@ public final class AddressRule public static final int TIMEOUT = 30_000; public static final int WEBSOCKET_MESSAGE = 128 * 1024; - private static final class HostRange - { - private final byte[] min; - private final byte[] max; - - private HostRange( byte[] min, byte[] max ) - { - this.min = min; - this.max = max; - } - - public boolean contains( InetAddress address ) - { - byte[] entry = address.getAddress(); - if( entry.length != min.length ) return false; - - for( int i = 0; i < entry.length; i++ ) - { - int value = 0xFF & entry[i]; - if( value < (0xFF & min[i]) || value > (0xFF & max[i]) ) return false; - } - - return true; - } - } - - private final HostRange ip; - private final Pattern domainPattern; + private final AddressPredicate predicate; private final Integer port; private final PartialOptions partial; - private AddressRule( - @Nullable HostRange ip, - @Nullable Pattern domainPattern, - @Nullable Integer port, - @Nonnull PartialOptions partial ) + private AddressRule( @Nonnull AddressPredicate predicate, @Nullable Integer port, @Nonnull PartialOptions partial ) { - this.ip = ip; - this.domainPattern = domainPattern; + this.predicate = predicate; this.partial = partial; this.port = port; } @@ -76,103 +47,50 @@ public final class AddressRule { String addressStr = filter.substring( 0, cidr ); String prefixSizeStr = filter.substring( cidr + 1 ); - - int prefixSize; - try - { - prefixSize = Integer.parseInt( prefixSizeStr ); - } - catch( NumberFormatException e ) - { - ComputerCraft.log.error( - "Malformed http whitelist/blacklist entry '{}': Cannot extract size of CIDR mask from '{}'.", - filter, prefixSizeStr - ); - return null; - } - - InetAddress address; - try - { - address = InetAddresses.forString( addressStr ); - } - catch( IllegalArgumentException e ) - { - ComputerCraft.log.error( - "Malformed http whitelist/blacklist entry '{}': Cannot extract IP address from '{}'.", - filter, prefixSizeStr - ); - return null; - } - - // Mask the bytes of the IP address. - byte[] minBytes = address.getAddress(), maxBytes = address.getAddress(); - int size = prefixSize; - for( int i = 0; i < minBytes.length; i++ ) - { - if( size <= 0 ) - { - minBytes[i] &= 0; - maxBytes[i] |= 0xFF; - } - else if( size < 8 ) - { - minBytes[i] &= 0xFF << (8 - size); - maxBytes[i] |= ~(0xFF << (8 - size)); - } - - size -= 8; - } - - return new AddressRule( new HostRange( minBytes, maxBytes ), null, port, partial ); + HostRange range = HostRange.parse( addressStr, prefixSizeStr ); + return range == null ? null : new AddressRule( range, port, partial ); + } + else if( filter.equalsIgnoreCase( "$private" ) ) + { + return new AddressRule( PrivatePattern.INSTANCE, port, partial ); } else { - Pattern pattern = Pattern.compile( "^\\Q" + filter.replaceAll( "\\*", "\\\\E.*\\\\Q" ) + "\\E$" ); - return new AddressRule( null, pattern, port, partial ); + Pattern pattern = Pattern.compile( "^\\Q" + filter.replaceAll( "\\*", "\\\\E.*\\\\Q" ) + "\\E$", Pattern.CASE_INSENSITIVE ); + return new AddressRule( new DomainPattern( pattern ), port, partial ); } } /** * Determine whether the given address matches a series of patterns. * - * @param domain The domain to match - * @param socketAddress The address to check. + * @param domain The domain to match + * @param port The port of the address. + * @param address The address to check. + * @param ipv4Address An ipv4 version of the address, if the original was an ipv6 address. * @return Whether it matches any of these patterns. */ - private boolean matches( String domain, InetSocketAddress socketAddress ) + private boolean matches( String domain, int port, InetAddress address, Inet4Address ipv4Address ) { - InetAddress address = socketAddress.getAddress(); - if( port != null && port != socketAddress.getPort() ) return false; - - if( domainPattern != null ) - { - if( domainPattern.matcher( domain ).matches() ) return true; - if( domainPattern.matcher( address.getHostName() ).matches() ) return true; - } - - // Match the normal address - if( matchesAddress( address ) ) return true; - - // If we're an IPv4 address in disguise then let's check that. - return address instanceof Inet6Address && InetAddresses.is6to4Address( (Inet6Address) address ) - && matchesAddress( InetAddresses.get6to4IPv4Address( (Inet6Address) address ) ); + if( this.port != null && this.port != port ) return false; + return predicate.matches( domain ) + || predicate.matches( address ) + || (ipv4Address != null && predicate.matches( ipv4Address )); } - private boolean matchesAddress( InetAddress address ) - { - if( domainPattern != null && domainPattern.matcher( address.getHostAddress() ).matches() ) return true; - return ip != null && ip.contains( address ); - } - - public static Options apply( Iterable rules, String domain, InetSocketAddress address ) + public static Options apply( Iterable rules, String domain, InetSocketAddress socketAddress ) { PartialOptions options = null; boolean hasMany = false; + int port = socketAddress.getPort(); + InetAddress address = socketAddress.getAddress(); + Inet4Address ipv4Address = address instanceof Inet6Address && InetAddresses.is6to4Address( (Inet6Address) address ) + ? InetAddresses.get6to4IPv4Address( (Inet6Address) address ) : null; + for( AddressRule rule : rules ) { - if( !rule.matches( domain, address ) ) continue; + if( !rule.matches( domain, port, address, ipv4Address ) ) continue; if( options == null ) { diff --git a/src/main/java/dan200/computercraft/shared/Config.java b/src/main/java/dan200/computercraft/shared/Config.java index a39f513f6..6655d6fd5 100644 --- a/src/main/java/dan200/computercraft/shared/Config.java +++ b/src/main/java/dan200/computercraft/shared/Config.java @@ -21,12 +21,12 @@ import net.minecraftforge.fml.ModLoadingContext; import net.minecraftforge.fml.common.Mod; import net.minecraftforge.fml.config.ModConfig; +import java.util.Arrays; import java.util.Collections; import java.util.List; import java.util.Objects; import java.util.concurrent.TimeUnit; import java.util.stream.Collectors; -import java.util.stream.Stream; import static net.minecraftforge.common.ForgeConfigSpec.Builder; import static net.minecraftforge.common.ForgeConfigSpec.ConfigValue; @@ -180,12 +180,10 @@ public final class Config "Each rule is an item with a 'host' to match against, and a series of properties. " + "The host may be a domain name (\"pastebin.com\"),\n" + "wildcard (\"*.pastebin.com\") or CIDR notation (\"127.0.0.0/8\"). If no rules, the domain is blocked." ) - .defineList( "rules", - Stream.concat( - Stream.of( ComputerCraft.DEFAULT_HTTP_DENY ).map( x -> AddressRuleConfig.makeRule( x, Action.DENY ) ), - Stream.of( ComputerCraft.DEFAULT_HTTP_ALLOW ).map( x -> AddressRuleConfig.makeRule( x, Action.ALLOW ) ) - ).collect( Collectors.toList() ), - x -> x instanceof UnmodifiableConfig && AddressRuleConfig.checkRule( (UnmodifiableConfig) x ) ); + .defineList( "rules", Arrays.asList( + AddressRuleConfig.makeRule( "$private", Action.DENY ), + AddressRuleConfig.makeRule( "*", Action.ALLOW ) + ), x -> x instanceof UnmodifiableConfig && AddressRuleConfig.checkRule( (UnmodifiableConfig) x ) ); httpMaxRequests = builder .comment( "The number of http requests a computer can make at one time. Additional requests will be queued, and sent when the running requests have finished. Set to 0 for unlimited." ) diff --git a/src/test/java/dan200/computercraft/core/apis/http/options/AddressRuleTest.java b/src/test/java/dan200/computercraft/core/apis/http/options/AddressRuleTest.java index 690cb37f5..3a8246e7f 100644 --- a/src/test/java/dan200/computercraft/core/apis/http/options/AddressRuleTest.java +++ b/src/test/java/dan200/computercraft/core/apis/http/options/AddressRuleTest.java @@ -6,7 +6,10 @@ package dan200.computercraft.core.apis.http.options; +import dan200.computercraft.ComputerCraft; import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.ValueSource; import java.net.InetSocketAddress; import java.util.Collections; @@ -27,6 +30,17 @@ public class AddressRuleTest assertEquals( apply( rules, "localhost", 8081 ).action, Action.DENY ); } + @ParameterizedTest + @ValueSource( strings = { + "0.0.0.0", "[::]", + "localhost", "lvh.me", "127.0.0.1", "[::1]", + "172.17.0.1", "192.168.1.114", "[0:0:0:0:0:ffff:c0a8:172]", "10.0.0.1" + } ) + public void blocksLocalDomains( String domain ) + { + assertEquals( apply( ComputerCraft.httpRules, domain, 80 ).action, Action.DENY ); + } + private Options apply( Iterable rules, String host, int port ) { return AddressRule.apply( rules, host, new InetSocketAddress( host, port ) ); diff --git a/src/test/resources/test-rom/spec/apis/http_spec.lua b/src/test/resources/test-rom/spec/apis/http_spec.lua index ecaf1d307..4a6a8b803 100644 --- a/src/test/resources/test-rom/spec/apis/http_spec.lua +++ b/src/test/resources/test-rom/spec/apis/http_spec.lua @@ -12,6 +12,8 @@ describe("The http library", function() end) it("rejects local domains", function() + -- Note, this is tested more thoroughly in AddressRuleTest. We've just got this here + -- to ensure the general control flow works. expect({ http.checkURL("http://localhost") }):same({ false, "Domain not permitted" }) expect({ http.checkURL("http://127.0.0.1") }):same({ false, "Domain not permitted" }) end)