From 41621193ec60e2032471d4dc0aea9673ac48dfad Mon Sep 17 00:00:00 2001 From: md_5 Date: Sun, 15 May 2016 14:52:01 +1000 Subject: [PATCH 1/4] #1862: Support 1.7 pings --- .../net/md_5/bungee/protocol/Protocol.java | 28 +++++++++++++------ .../bungee/connection/InitialHandler.java | 12 ++++---- 2 files changed, 26 insertions(+), 14 deletions(-) diff --git a/protocol/src/main/java/net/md_5/bungee/protocol/Protocol.java b/protocol/src/main/java/net/md_5/bungee/protocol/Protocol.java index 2378754e97..e1db3eaebe 100644 --- a/protocol/src/main/java/net/md_5/bungee/protocol/Protocol.java +++ b/protocol/src/main/java/net/md_5/bungee/protocol/Protocol.java @@ -1,6 +1,7 @@ package net.md_5.bungee.protocol; import com.google.common.base.Preconditions; +import com.google.common.collect.Iterables; import gnu.trove.map.TIntObjectMap; import gnu.trove.map.TObjectIntMap; import gnu.trove.map.hash.TIntObjectHashMap; @@ -217,8 +218,8 @@ public enum Protocol /*========================================================================*/ public static final int MAX_PACKET_ID = 0xFF; /*========================================================================*/ - public final DirectionData TO_SERVER = new DirectionData( ProtocolConstants.Direction.TO_SERVER ); - public final DirectionData TO_CLIENT = new DirectionData( ProtocolConstants.Direction.TO_CLIENT ); + public final DirectionData TO_SERVER = new DirectionData(this, ProtocolConstants.Direction.TO_SERVER ); + public final DirectionData TO_CLIENT = new DirectionData(this, ProtocolConstants.Direction.TO_CLIENT ); @RequiredArgsConstructor private static class ProtocolData { @@ -242,6 +243,7 @@ private static ProtocolMapping map(int protocol, int id) { public static class DirectionData { + private final Protocol protocolPhase; private final TIntObjectMap protocols = new TIntObjectHashMap<>(); { for ( int protocol : ProtocolConstants.SUPPORTED_VERSION_IDS ) @@ -263,12 +265,22 @@ public static class DirectionData @Getter private final ProtocolConstants.Direction direction; - public final DefinedPacket createPacket(int id, int protocol) + private ProtocolData getProtocolData(int version) { - ProtocolData protocolData = protocols.get( protocol ); + ProtocolData protocol = protocols.get( version ); + if ( protocol == null && ( protocolPhase == Protocol.HANDSHAKE || protocolPhase == Protocol.STATUS ) ) + { + protocol = Iterables.getFirst( protocols.valueCollection(), null ); + } + return protocol; + } + + public final DefinedPacket createPacket(int id, int version) + { + ProtocolData protocolData = getProtocolData( version ); if (protocolData == null) { - throw new BadPacketException( "Unsupported protocol" ); + throw new BadPacketException( "Unsupported protocol version" ); } if ( id > MAX_PACKET_ID ) { @@ -319,13 +331,13 @@ protected final void registerPacket(Class packetClass, } } - final int getId(Class packet, int protocol) + final int getId(Class packet, int version) { - ProtocolData protocolData = protocols.get( protocol ); + ProtocolData protocolData = getProtocolData( version ); if (protocolData == null) { - throw new BadPacketException( "Unsupported protocol" ); + throw new BadPacketException( "Unsupported protocol version" ); } Preconditions.checkArgument( protocolData.packetMap.containsKey( packet ), "Cannot get ID for packet " + packet ); diff --git a/proxy/src/main/java/net/md_5/bungee/connection/InitialHandler.java b/proxy/src/main/java/net/md_5/bungee/connection/InitialHandler.java index d714836c36..dab6ef2d93 100644 --- a/proxy/src/main/java/net/md_5/bungee/connection/InitialHandler.java +++ b/proxy/src/main/java/net/md_5/bungee/connection/InitialHandler.java @@ -293,6 +293,12 @@ public void handle(Handshake handshake) throws Exception thisState = State.USERNAME; ch.setProtocol( Protocol.LOGIN ); + if ( !ProtocolConstants.SUPPORTED_VERSION_IDS.contains( handshake.getProtocolVersion() ) ) + { + disconnect( bungee.getTranslation( "outdated_server" ) ); + return; + } + if ( bungee.getConnectionThrottle() != null && bungee.getConnectionThrottle().throttle( getAddress().getAddress() ) ) { disconnect( bungee.getTranslation( "join_throttle_kick", TimeUnit.MILLISECONDS.toSeconds( bungee.getConfig().getThrottle() ) ) ); @@ -309,12 +315,6 @@ public void handle(LoginRequest loginRequest) throws Exception Preconditions.checkState( thisState == State.USERNAME, "Not expecting USERNAME" ); this.loginRequest = loginRequest; - if ( !ProtocolConstants.SUPPORTED_VERSION_IDS.contains( handshake.getProtocolVersion() ) ) - { - disconnect( bungee.getTranslation( "outdated_server" ) ); - return; - } - if ( getName().contains( "." ) ) { disconnect( bungee.getTranslation( "name_invalid" ) ); From d14b96d55e485eb6c31e3bb5d8ca3c2d3560520a Mon Sep 17 00:00:00 2001 From: Zartec Date: Fri, 1 Apr 2016 22:33:07 +0200 Subject: [PATCH 2/4] Added separate exception for packet overflows to limit log output. Attacking a server with a hacked client causes the log to print a huge amount of stacktraces. This will limit the log output to the error message. --- .../md_5/bungee/protocol/DefinedPacket.java | 21 ++++++++++++++----- .../protocol/OverflowPacketException.java | 10 +++++++++ .../net/md_5/bungee/netty/HandlerBoss.java | 7 +++++++ 3 files changed, 33 insertions(+), 5 deletions(-) create mode 100644 protocol/src/main/java/net/md_5/bungee/protocol/OverflowPacketException.java diff --git a/protocol/src/main/java/net/md_5/bungee/protocol/DefinedPacket.java b/protocol/src/main/java/net/md_5/bungee/protocol/DefinedPacket.java index 2fefc266f2..9c086869a1 100644 --- a/protocol/src/main/java/net/md_5/bungee/protocol/DefinedPacket.java +++ b/protocol/src/main/java/net/md_5/bungee/protocol/DefinedPacket.java @@ -1,7 +1,6 @@ package net.md_5.bungee.protocol; import com.google.common.base.Charsets; -import com.google.common.base.Preconditions; import io.netty.buffer.ByteBuf; import java.util.ArrayList; import java.util.List; @@ -15,7 +14,10 @@ public abstract class DefinedPacket public static void writeString(String s, ByteBuf buf) { - Preconditions.checkArgument( s.length() <= Short.MAX_VALUE, "Cannot send string longer than Short.MAX_VALUE (got %s characters)", s.length() ); + if ( s.length() > Short.MAX_VALUE ) + { + throw new OverflowPacketException( String.format( "Cannot send string longer than Short.MAX_VALUE (got %s characters)", s.length() ) ); + } byte[] b = s.getBytes( Charsets.UTF_8 ); writeVarInt( b.length, buf ); @@ -25,7 +27,10 @@ public static void writeString(String s, ByteBuf buf) public static String readString(ByteBuf buf) { int len = readVarInt( buf ); - Preconditions.checkArgument( len <= Short.MAX_VALUE, "Cannot receive string longer than Short.MAX_VALUE (got %s characters)", len ); + if ( len > Short.MAX_VALUE ) + { + throw new OverflowPacketException( String.format( "Cannot receive string longer than Short.MAX_VALUE (got %s characters)", len ) ); + } byte[] b = new byte[ len ]; buf.readBytes( b ); @@ -35,7 +40,10 @@ public static String readString(ByteBuf buf) public static void writeArray(byte[] b, ByteBuf buf) { - Preconditions.checkArgument( b.length <= Short.MAX_VALUE, "Cannot send byte array longer than Short.MAX_VALUE (got %s bytes)", b.length ); + if ( b.length > Short.MAX_VALUE ) + { + throw new OverflowPacketException( String.format( "Cannot send byte array longer than Short.MAX_VALUE (got %s bytes)", b.length ) ); + } writeVarInt( b.length, buf ); buf.writeBytes( b ); } @@ -48,7 +56,10 @@ public static byte[] readArray(ByteBuf buf) public static byte[] readArray(ByteBuf buf, int limit) { int len = readVarInt( buf ); - Preconditions.checkArgument( len <= limit, "Cannot receive byte array longer than %s (got %s bytes)", limit, len ); + if ( len > limit ) + { + throw new OverflowPacketException( String.format( "Cannot receive byte array longer than %s (got %s bytes)", limit, len ) ); + } byte[] ret = new byte[ len ]; buf.readBytes( ret ); return ret; diff --git a/protocol/src/main/java/net/md_5/bungee/protocol/OverflowPacketException.java b/protocol/src/main/java/net/md_5/bungee/protocol/OverflowPacketException.java new file mode 100644 index 0000000000..237955ab52 --- /dev/null +++ b/protocol/src/main/java/net/md_5/bungee/protocol/OverflowPacketException.java @@ -0,0 +1,10 @@ +package net.md_5.bungee.protocol; + +public class OverflowPacketException extends RuntimeException +{ + + public OverflowPacketException(String message) + { + super( message ); + } +} diff --git a/proxy/src/main/java/net/md_5/bungee/netty/HandlerBoss.java b/proxy/src/main/java/net/md_5/bungee/netty/HandlerBoss.java index 8b9712bf1d..94a164e3f6 100644 --- a/proxy/src/main/java/net/md_5/bungee/netty/HandlerBoss.java +++ b/proxy/src/main/java/net/md_5/bungee/netty/HandlerBoss.java @@ -13,6 +13,7 @@ import net.md_5.bungee.connection.InitialHandler; import net.md_5.bungee.connection.PingHandler; import net.md_5.bungee.protocol.BadPacketException; +import net.md_5.bungee.protocol.OverflowPacketException; /** * This class is a primitive wrapper for {@link PacketHandler} instances tied to @@ -104,6 +105,12 @@ public void exceptionCaught(ChannelHandlerContext ctx, Throwable cause) throws E { handler, cause.getCause().getMessage() } ); + } else if ( cause instanceof DecoderException && cause.getCause() instanceof OverflowPacketException ) + { + ProxyServer.getInstance().getLogger().log( Level.WARNING, "{0} - overflow in packet detected! {1}", new Object[] + { + handler, cause.getCause().getMessage() + } ); } else if ( cause instanceof IOException ) { ProxyServer.getInstance().getLogger().log( Level.WARNING, "{0} - IOException: {1}", new Object[] From d9a8311b8e40a0d390a993e409933f8ee737c12e Mon Sep 17 00:00:00 2001 From: PunKeel Date: Sat, 14 May 2016 23:37:12 +0200 Subject: [PATCH 3/4] Use expireAfterWrite to perform throttle --- .../java/net/md_5/bungee/ConnectionThrottle.java | 13 +++++-------- .../src/test/java/net/md_5/bungee/ThrottleTest.java | 4 ++-- 2 files changed, 7 insertions(+), 10 deletions(-) diff --git a/proxy/src/main/java/net/md_5/bungee/ConnectionThrottle.java b/proxy/src/main/java/net/md_5/bungee/ConnectionThrottle.java index a2690b7715..207defada4 100644 --- a/proxy/src/main/java/net/md_5/bungee/ConnectionThrottle.java +++ b/proxy/src/main/java/net/md_5/bungee/ConnectionThrottle.java @@ -8,25 +8,22 @@ public class ConnectionThrottle { - private final int throttleTime; - private final Cache throttle; + private final Cache throttle; public ConnectionThrottle(int throttleTime) { - this.throttleTime = throttleTime; this.throttle = CacheBuilder.newBuilder() .concurrencyLevel( Runtime.getRuntime().availableProcessors() ) .initialCapacity( 100 ) - .expireAfterAccess( throttleTime, TimeUnit.MILLISECONDS ) + .expireAfterWrite( throttleTime, TimeUnit.MILLISECONDS ) .build(); } public boolean throttle(InetAddress address) { - Long value = throttle.getIfPresent( address ); - long currentTime = System.currentTimeMillis(); + boolean isThrottled = throttle.getIfPresent( address ); + throttle.put( address, true ); - throttle.put( address, currentTime ); - return value != null && currentTime - value < throttleTime; + return isThrottled; } } diff --git a/proxy/src/test/java/net/md_5/bungee/ThrottleTest.java b/proxy/src/test/java/net/md_5/bungee/ThrottleTest.java index cedd38ce3e..5f501f7d88 100644 --- a/proxy/src/test/java/net/md_5/bungee/ThrottleTest.java +++ b/proxy/src/test/java/net/md_5/bungee/ThrottleTest.java @@ -11,7 +11,7 @@ public class ThrottleTest @Test public void testThrottle() throws InterruptedException, UnknownHostException { - ConnectionThrottle throttle = new ConnectionThrottle( 5 ); + ConnectionThrottle throttle = new ConnectionThrottle( 10 ); InetAddress address; try @@ -25,7 +25,7 @@ public void testThrottle() throws InterruptedException, UnknownHostException Assert.assertFalse( "Address should not be throttled", throttle.throttle( address ) ); Assert.assertTrue( "Address should be throttled", throttle.throttle( address ) ); - Thread.sleep( 15 ); + Thread.sleep( 50 ); Assert.assertFalse( "Address should not be throttled", throttle.throttle( address ) ); } } From 2df29701edc984c97c6e860aa0ad8e47654fd6d4 Mon Sep 17 00:00:00 2001 From: md_5 Date: Sun, 15 May 2016 16:01:58 +1000 Subject: [PATCH 4/4] #1866: Correct throttle --- proxy/src/main/java/net/md_5/bungee/ConnectionThrottle.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/proxy/src/main/java/net/md_5/bungee/ConnectionThrottle.java b/proxy/src/main/java/net/md_5/bungee/ConnectionThrottle.java index 207defada4..4c9190e21f 100644 --- a/proxy/src/main/java/net/md_5/bungee/ConnectionThrottle.java +++ b/proxy/src/main/java/net/md_5/bungee/ConnectionThrottle.java @@ -21,7 +21,7 @@ public ConnectionThrottle(int throttleTime) public boolean throttle(InetAddress address) { - boolean isThrottled = throttle.getIfPresent( address ); + boolean isThrottled = throttle.getIfPresent( address ) != null; throttle.put( address, true ); return isThrottled;