Making payload fast-fail and demasking work together in peace and harmony

This commit is contained in:
Joakim Erdfelt 2012-08-09 13:17:07 -07:00
parent 6a66501701
commit b37da5a5da
13 changed files with 426 additions and 28 deletions

View File

@ -0,0 +1,25 @@
package org.eclipse.jetty.websocket.io.payload;
import java.nio.ByteBuffer;
import org.eclipse.jetty.websocket.protocol.WebSocketFrame;
/**
* Binary payload validator does nothing, essentially.
*/
public class BinaryValidator implements PayloadProcessor
{
public static final BinaryValidator INSTANCE = new BinaryValidator();
@Override
public void process(ByteBuffer payload)
{
/* all payloads are valid in this case */
}
@Override
public void reset(WebSocketFrame frame)
{
/* do nothing */
}
}

View File

@ -0,0 +1,32 @@
package org.eclipse.jetty.websocket.io.payload;
import java.nio.ByteBuffer;
import org.eclipse.jetty.websocket.protocol.OpCode;
/**
* Validate UTF8 correctness for {@link OpCode#CLOSE} Reason message.
*/
public class CloseReasonValidator extends UTF8Validator implements PayloadProcessor
{
private int statusCodeBytes = 2;
@Override
public void process(ByteBuffer payload)
{
if ((payload == null) || (payload.remaining() <= 2))
{
// no validation needed
return;
}
ByteBuffer copy = payload.slice();
while (statusCodeBytes > 0)
{
copy.get();
statusCodeBytes--;
}
super.process(copy);
}
}

View File

@ -0,0 +1,44 @@
package org.eclipse.jetty.websocket.io.payload;
import java.nio.ByteBuffer;
import org.eclipse.jetty.websocket.protocol.WebSocketFrame;
public class DeMaskProcessor implements PayloadProcessor
{
private boolean isMasked;
private byte mask[];
private int offset;
@Override
public void process(ByteBuffer payload)
{
if (!isMasked)
{
return;
}
int start = payload.position();
int end = payload.limit();
for (int i = start; i < end; i++, offset++)
{
payload.put(i,(byte)(payload.get(i) ^ mask[offset % 4]));
}
}
@Override
public void reset(WebSocketFrame frame)
{
this.isMasked = frame.isMasked();
if (isMasked)
{
this.mask = frame.getMask();
}
else
{
this.mask = null;
}
offset = 0;
}
}

View File

@ -0,0 +1,24 @@
package org.eclipse.jetty.websocket.io.payload;
import java.nio.ByteBuffer;
import org.eclipse.jetty.websocket.api.BadPayloadException;
import org.eclipse.jetty.websocket.protocol.WebSocketFrame;
/**
* Process the payload (for demasking, validating, etc..)
*/
public interface PayloadProcessor
{
/**
* Used to process payloads for in the spec.
*
* @param payload
* the payload to process
* @throws BadPayloadException
* the exception when the payload fails to validate properly
*/
public void process(ByteBuffer payload);
public void reset(WebSocketFrame frame);
}

View File

@ -0,0 +1,93 @@
package org.eclipse.jetty.websocket.io.payload;
import java.io.IOException;
import java.nio.ByteBuffer;
import org.eclipse.jetty.util.BufferUtil;
import org.eclipse.jetty.util.Utf8Appendable;
import org.eclipse.jetty.util.log.Log;
import org.eclipse.jetty.util.log.Logger;
import org.eclipse.jetty.websocket.api.BadPayloadException;
import org.eclipse.jetty.websocket.protocol.WebSocketFrame;
/**
* Used to perform validation of UTF8 payload contents (for fast-fail reasons)
*/
public class UTF8Validator extends Utf8Appendable implements PayloadProcessor
{
private static class EmptyAppender implements Appendable
{
private int length = 0;
@Override
public Appendable append(char c) throws IOException
{
length++;
return this;
}
@Override
public Appendable append(CharSequence csq) throws IOException
{
length += csq.length();
return this;
}
@Override
public Appendable append(CharSequence csq, int start, int end) throws IOException
{
length += (end - start);
return this;
}
public int getLength()
{
return length;
}
}
private static final Logger LOG = Log.getLogger(UTF8Validator.class);
private EmptyAppender buffer;
public UTF8Validator()
{
super(new EmptyAppender());
this.buffer = (EmptyAppender)_appendable;
}
@Override
public int length()
{
return this.buffer.getLength();
}
@Override
public void process(ByteBuffer payload)
{
if (LOG.isDebugEnabled())
{
LOG.debug("Payload: {}",BufferUtil.toDetailString(payload));
}
if ((payload == null) || (payload.remaining() <= 0))
{
return;
}
try
{
append(payload.slice());
}
catch (NotUtf8Exception e)
{
throw new BadPayloadException(e);
}
}
@Override
public void reset(WebSocketFrame frame)
{
/* do nothing */
}
}

View File

@ -25,6 +25,11 @@ import org.eclipse.jetty.websocket.api.ProtocolException;
import org.eclipse.jetty.websocket.api.WebSocketException;
import org.eclipse.jetty.websocket.api.WebSocketPolicy;
import org.eclipse.jetty.websocket.io.IncomingFrames;
import org.eclipse.jetty.websocket.io.payload.BinaryValidator;
import org.eclipse.jetty.websocket.io.payload.CloseReasonValidator;
import org.eclipse.jetty.websocket.io.payload.DeMaskProcessor;
import org.eclipse.jetty.websocket.io.payload.PayloadProcessor;
import org.eclipse.jetty.websocket.io.payload.UTF8Validator;
/**
* Parsing of a frames in WebSocket land.
@ -54,6 +59,8 @@ public class Parser
// payload specific
private ByteBuffer payload;
private int payloadLength;
private PayloadProcessor maskProcessor = new DeMaskProcessor();
private PayloadProcessor strictnessProcessor;
/** Is there an extension using RSV1 */
private boolean rsv1InUse = false;
@ -277,6 +284,19 @@ public class Parser
boolean isContinuation = false;
switch (opcode)
{
case OpCode.TEXT:
strictnessProcessor = new UTF8Validator();
break;
case OpCode.CLOSE:
strictnessProcessor = new CloseReasonValidator();
break;
default:
strictnessProcessor = BinaryValidator.INSTANCE;
break;
}
if (OpCode.isControlFrame(opcode))
{
// control frame validation
@ -359,6 +379,7 @@ public class Parser
return true;
}
maskProcessor.reset(frame);
state = State.PAYLOAD;
}
@ -385,6 +406,7 @@ public class Parser
return true;
}
maskProcessor.reset(frame);
state = State.PAYLOAD;
}
}
@ -404,6 +426,7 @@ public class Parser
return true;
}
maskProcessor.reset(frame);
state = State.PAYLOAD;
}
else
@ -427,6 +450,7 @@ public class Parser
return true;
}
maskProcessor.reset(frame);
state = State.PAYLOAD;
}
break;
@ -476,27 +500,29 @@ public class Parser
BufferUtil.clearToFill(payload);
}
BufferUtil.put(buffer,payload);
// Create a small window of the incoming buffer to work with.
// this should only show the payload itself, and not any more
// bytes that could belong to the start of the next frame.
ByteBuffer window = buffer.slice();
int bytesExpected = payloadLength - payload.position();
int bytesAvailable = buffer.remaining();
int windowBytes = Math.min(bytesAvailable,bytesExpected);
window.limit(window.position() + windowBytes);
if (LOG.isDebugEnabled())
{
LOG.debug("Window: {}",BufferUtil.toDetailString(window));
}
maskProcessor.process(window);
strictnessProcessor.process(window);
int len = BufferUtil.put(window,payload);
buffer.position(buffer.position() + len); // update incoming buffer position
if (payload.position() >= payloadLength)
{
BufferUtil.flipToFlush(payload,0);
LOG.debug("PreMask: {}",BufferUtil.toDetailString(payload));
// demask (if needed)
if (frame.isMasked())
{
byte mask[] = frame.getMask();
int offset;
int start = payload.position();
int end = payload.limit();
for (int i = start; i < end; i++)
{
offset = (i - start);
payload.put(i,(byte)(payload.get(i) ^ mask[offset % 4]));
}
}
frame.setPayload(payload);
this.payload = null;
return true;

View File

@ -15,7 +15,7 @@
//========================================================================
package org.eclipse.jetty.websocket;
import static org.hamcrest.Matchers.is;
import static org.hamcrest.Matchers.*;
import java.nio.ByteBuffer;
@ -44,7 +44,7 @@ public class ByteBufferAssert
public static void assertEquals(String message, String expectedString, ByteBuffer actualBuffer)
{
String actualString = BufferUtil.toString(actualBuffer);
Assert.assertThat(message,expectedString,is(actualString));
Assert.assertThat(message,actualString,is(expectedString));
}
public static void assertSize(String message, int expectedSize, ByteBuffer buffer)

View File

@ -0,0 +1,39 @@
package org.eclipse.jetty.websocket.io.payload;
import java.nio.ByteBuffer;
import org.eclipse.jetty.util.BufferUtil;
import org.eclipse.jetty.util.TypeUtil;
import org.eclipse.jetty.util.log.Log;
import org.eclipse.jetty.util.log.Logger;
import org.eclipse.jetty.websocket.ByteBufferAssert;
import org.eclipse.jetty.websocket.protocol.UnitGenerator;
import org.eclipse.jetty.websocket.protocol.WebSocketFrame;
import org.junit.Test;
public class DeMaskProcessorTest
{
private static final Logger LOG = Log.getLogger(DeMaskProcessorTest.class);
@Test
public void testDeMaskText()
{
String message = "0123456789ABCDEF0123456789ABCDEF0123456789ABCDEF0123456789ABCDEF";
WebSocketFrame frame = WebSocketFrame.text(message);
frame.setMask(TypeUtil.fromHexString("11223344"));
// frame.setMask(TypeUtil.fromHexString("00000000"));
ByteBuffer buf = new UnitGenerator().generate(frame);
LOG.debug("Buf: {}",BufferUtil.toDetailString(buf));
ByteBuffer payload = buf.slice();
payload.position(6); // where payload starts
LOG.debug("Payload: {}",BufferUtil.toDetailString(payload));
DeMaskProcessor demask = new DeMaskProcessor();
demask.reset(frame);
demask.process(payload);
ByteBufferAssert.assertEquals("DeMasked Text Payload",message,payload);
}
}

View File

@ -0,0 +1,38 @@
package org.eclipse.jetty.websocket.io.payload;
import java.nio.ByteBuffer;
import org.eclipse.jetty.util.TypeUtil;
import org.eclipse.jetty.websocket.api.BadPayloadException;
import org.junit.Assert;
import org.junit.Test;
public class UTF8ValidatorTest
{
private ByteBuffer asByteBuffer(String hexStr)
{
byte buf[] = TypeUtil.fromHexString(hexStr);
return ByteBuffer.wrap(buf);
}
@Test
public void testCase6_4_3()
{
ByteBuffer part1 = asByteBuffer("cebae1bdb9cf83cebcceb5"); // good
ByteBuffer part2 = asByteBuffer("f4908080"); // INVALID
ByteBuffer part3 = asByteBuffer("656469746564"); // good
UTF8Validator validator = new UTF8Validator();
validator.process(part1); // good
try
{
validator.process(part2); // bad
Assert.fail("Expected a " + BadPayloadException.class);
}
catch (BadPayloadException e)
{
// expected path
}
validator.process(part3); // good
}
}

View File

@ -15,13 +15,18 @@
//========================================================================
package org.eclipse.jetty.websocket.protocol;
import static org.hamcrest.Matchers.is;
import static org.hamcrest.Matchers.*;
import java.nio.ByteBuffer;
import java.util.ArrayList;
import java.util.List;
import org.eclipse.jetty.util.BufferUtil;
import org.eclipse.jetty.util.StringUtil;
import org.eclipse.jetty.util.TypeUtil;
import org.eclipse.jetty.util.log.Log;
import org.eclipse.jetty.util.log.Logger;
import org.eclipse.jetty.websocket.api.BadPayloadException;
import org.eclipse.jetty.websocket.api.StatusCode;
import org.eclipse.jetty.websocket.api.WebSocketBehavior;
import org.eclipse.jetty.websocket.api.WebSocketPolicy;
@ -30,6 +35,8 @@ import org.junit.Test;
public class ParserTest
{
private static final Logger LOG = Log.getLogger(ParserTest.class);
/**
* Similar to the server side 5.15 testcase. A normal 2 fragment text text message, followed by another continuation.
*/
@ -163,6 +170,53 @@ public class ParserTest
capture.assertHasFrame(OpCode.CLOSE,1);
}
/**
* Similar to the server side 6.4.3 testcase.
*/
@Test
public void testParseCase6_4_3()
{
ByteBuffer payload = ByteBuffer.allocate(64);
BufferUtil.clearToFill(payload);
payload.put(TypeUtil.fromHexString("cebae1bdb9cf83cebcceb5")); // good
payload.put(TypeUtil.fromHexString("f4908080")); // INVALID
payload.put(TypeUtil.fromHexString("656469746564")); // good
BufferUtil.flipToFlush(payload,0);
WebSocketFrame text = new WebSocketFrame();
text.setMask(TypeUtil.fromHexString("11223344"));
text.setPayload(payload);
text.setOpCode(OpCode.TEXT);
ByteBuffer buf = new UnitGenerator().generate(text);
ByteBuffer part1 = ByteBuffer.allocate(17); // header + good
ByteBuffer part2 = ByteBuffer.allocate(4); // invalid
ByteBuffer part3 = ByteBuffer.allocate(10); // the rest (all good utf)
BufferUtil.put(buf,part1);
BufferUtil.put(buf,part2);
BufferUtil.put(buf,part3);
BufferUtil.flipToFlush(part1,0);
BufferUtil.flipToFlush(part2,0);
BufferUtil.flipToFlush(part3,0);
LOG.debug("Part1: {}",BufferUtil.toDetailString(part1));
LOG.debug("Part2: {}",BufferUtil.toDetailString(part2));
LOG.debug("Part3: {}",BufferUtil.toDetailString(part3));
UnitParser parser = new UnitParser();
IncomingFramesCapture capture = new IncomingFramesCapture();
parser.setIncomingFramesHandler(capture);
parser.parse(part1);
capture.assertErrorCount(0);
parser.parse(part2);
capture.assertErrorCount(1);
capture.assertHasErrors(BadPayloadException.class,1);
}
@Test
public void testParseNothing()
{

View File

@ -1,3 +1,5 @@
org.eclipse.jetty.util.log.class=org.eclipse.jetty.util.log.StdErrLog
org.eclipse.jetty.websocket.LEVEL=WARN
# org.eclipse.jetty.websocket.protocol.Parser.LEVEL=DEBUG
# org.eclipse.jetty.websocket.protocol.Parser.LEVEL=DEBUG
# org.eclipse.jetty.websocket.protocol.LEVEL=DEBUG
# org.eclipse.jetty.websocket.io.payload.LEVEL=DEBUG

View File

@ -22,7 +22,9 @@ import java.util.concurrent.TimeUnit;
import org.eclipse.jetty.toolchain.test.AdvancedRunner;
import org.eclipse.jetty.toolchain.test.annotation.Slow;
import org.eclipse.jetty.util.BufferUtil;
import org.eclipse.jetty.util.StringUtil;
import org.eclipse.jetty.util.TypeUtil;
import org.eclipse.jetty.websocket.api.StatusCode;
import org.eclipse.jetty.websocket.protocol.CloseInfo;
import org.eclipse.jetty.websocket.protocol.OpCode;
@ -351,10 +353,15 @@ public class TestABCase6 extends AbstractABCase
@Slow
public void testCase6_4_3() throws Exception
{
byte invalid[] = Hex.asByteArray("CEBAE1BDB9CF83CEBCCEB5F49080808080656469746564");
ByteBuffer payload = ByteBuffer.allocate(64);
BufferUtil.clearToFill(payload);
payload.put(TypeUtil.fromHexString("cebae1bdb9cf83cebcceb5")); // good
payload.put(TypeUtil.fromHexString("f4908080")); // INVALID
payload.put(TypeUtil.fromHexString("656469746564")); // good
BufferUtil.flipToFlush(payload,0);
List<WebSocketFrame> send = new ArrayList<>();
send.add(new WebSocketFrame(OpCode.TEXT).setPayload(invalid));
send.add(new WebSocketFrame(OpCode.TEXT).setPayload(payload));
send.add(new CloseInfo(StatusCode.NORMAL).asFrame());
List<WebSocketFrame> expect = new ArrayList<>();
@ -366,14 +373,27 @@ public class TestABCase6 extends AbstractABCase
fuzzer.connect();
ByteBuffer net = fuzzer.asNetworkBuffer(send);
fuzzer.send(net,6);
fuzzer.send(net,11);
int splits[] =
{ 17, 21, net.limit() };
ByteBuffer part1 = net.slice(); // Header + good UTF
part1.limit(splits[0]);
ByteBuffer part2 = net.slice(); // invalid UTF
part2.position(splits[0]);
part2.limit(splits[1]);
ByteBuffer part3 = net.slice(); // good UTF
part3.position(splits[1]);
part3.limit(splits[2]);
fuzzer.send(part1); // the header + good utf
TimeUnit.SECONDS.sleep(1);
fuzzer.send(net,4);
TimeUnit.SECONDS.sleep(1);
fuzzer.send(net,100); // the rest
fuzzer.send(part2); // the bad UTF
fuzzer.expect(expect);
TimeUnit.SECONDS.sleep(1);
fuzzer.send(part3); // the rest (shouldn't work)
}
finally
{

View File

@ -17,6 +17,7 @@ org.eclipse.jetty.websocket.server.helper.RFCSocket.LEVEL=OFF
# org.eclipse.jetty.websocket.extensions.LEVEL=DEBUG
# org.eclipse.jetty.websocket.protocol.Generator.LEVEL=INFO
# org.eclipse.jetty.websocket.protocol.Parser.LEVEL=DEBUG
# org.eclipse.jetty.websocket.io.payload.LEVEL=DEBUG
# org.eclipse.jetty.websocket.server.ab.LEVEL=DEBUG
# org.eclipse.jetty.websocket.server.ab.Fuzzer.LEVEL=DEBUG
# org.eclipse.jetty.websocket.server.blockhead.LEVEL=DEBUG