Eclipse
/
jetty.project
mirror of https://github.com/jetty/jetty.project.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Merge pull request #5951 from eclipse/jetty-10.0.x-4515-WebSocketValidation 

Issue #4515 - ValidationExtension should not downcast CoreSession
This commit is contained in:
Lachlan 2021-02-10 10:56:03 +11:00 committed by GitHub
parent 3321ae71be 8bc84d3119
commit 0fe4e78643
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
10 changed files with 212 additions and 136 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

11
jetty-websocket/websocket-core-common/src/main/java/org/eclipse/jetty/websocket/core/AbstractExtension.java
View File

@ -26,11 +26,11 @@ import org.slf4j.LoggerFactory;
public class AbstractExtension implements Extension
{
private final Logger log;
private CoreSession coreSession;
private ByteBufferPool bufferPool;
private ExtensionConfig config;
private OutgoingFrames nextOutgoing;
private IncomingFrames nextIncoming;
private Configuration configuration;
private DeflaterPool deflaterPool;
private InflaterPool inflaterPool;
@ -165,12 +165,17 @@ public class AbstractExtension implements Extension
@Override
public void setCoreSession(CoreSession coreSession)
{
this.configuration = coreSession;
this.coreSession = coreSession;
}
public CoreSession getCoreSession()
{
return coreSession;
}
protected Configuration getConfiguration()
{
return configuration;
return coreSession;
}
@Override

36
jetty-websocket/websocket-core-common/src/main/java/org/eclipse/jetty/websocket/core/CoreSession.java
View File

@ -77,8 +77,7 @@ public interface CoreSession extends OutgoingFrames, Configuration
Behavior getBehavior();
/**
* TODO
* @return
* @return the WebSocketComponents instance in use for this Connection.
*/
WebSocketComponents getWebSocketComponents();
@ -166,6 +165,21 @@ public interface CoreSession extends OutgoingFrames, Configuration
*/
void demand(long n);
/**
* @return true if an extension has been negotiated which uses the RSV1 bit.
*/
boolean isRsv1Used();
/**
* @return true if an extension has been negotiated which uses the RSV2 bit.
*/
boolean isRsv2Used();
/**
* @return true if an extension has been negotiated which uses the RSV3 bit.
*/
boolean isRsv3Used();
class Empty extends ConfigurationCustomizer implements CoreSession
{
@Override
@ -273,5 +287,23 @@ public interface CoreSession extends OutgoingFrames, Configuration
{
callback.succeeded();
}
@Override
public boolean isRsv1Used()
{
return false;
}
@Override
public boolean isRsv2Used()
{
return false;
}
@Override
public boolean isRsv3Used()
{
return false;
}
}
}

3
jetty-websocket/websocket-core-common/src/main/java/org/eclipse/jetty/websocket/core/Extension.java
View File

@ -81,8 +81,7 @@ public interface Extension extends IncomingFrames, OutgoingFrames
void setNextOutgoingFrames(OutgoingFrames nextOutgoing);
/**
* Set the {@link CoreSession} for this Extension.
* @param coreSession
* @param coreSession the {@link CoreSession} for this Extension.
*/
void setCoreSession(CoreSession coreSession);
}

9
jetty-websocket/websocket-core-common/src/main/java/org/eclipse/jetty/websocket/core/internal/FrameSequence.java
View File

@ -20,7 +20,6 @@ import org.eclipse.jetty.websocket.core.exception.ProtocolException;
public class FrameSequence
{
private final AutoLock lock = new AutoLock();
// TODO should we be able to get a non fin frame then get a close frame without error
private byte state = OpCode.UNDEFINED;
public void check(byte opcode, boolean fin) throws ProtocolException
@ -40,15 +39,15 @@ public class FrameSequence
throw new ProtocolException("CONTINUATION after fin==true");
if (fin)
state = OpCode.UNDEFINED;
return;
break;
case OpCode.CLOSE:
state = OpCode.CLOSE;
return;
break;
case OpCode.PING:
case OpCode.PONG:
return;
break;
case OpCode.TEXT:
case OpCode.BINARY:
@ -57,7 +56,7 @@ public class FrameSequence
throw new ProtocolException("DataFrame before fin==true");
if (!fin)
state = opcode;
return;
break;
}
}
}

18
jetty-websocket/websocket-core-common/src/main/java/org/eclipse/jetty/websocket/core/internal/ValidationExtension.java
View File

@ -17,11 +17,11 @@ import java.util.Map;
import org.eclipse.jetty.util.Callback;
import org.eclipse.jetty.websocket.core.AbstractExtension;
import org.eclipse.jetty.websocket.core.CoreSession;
import org.eclipse.jetty.websocket.core.ExtensionConfig;
import org.eclipse.jetty.websocket.core.Frame;
import org.eclipse.jetty.websocket.core.WebSocketComponents;
import org.eclipse.jetty.websocket.core.exception.ProtocolException;
import org.eclipse.jetty.websocket.core.internal.util.FrameValidation;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
@ -33,7 +33,6 @@ public class ValidationExtension extends AbstractExtension
{
private static final Logger LOG = LoggerFactory.getLogger(ValidationExtension.class);
private WebSocketCoreSession coreSession;
private FrameSequence incomingSequence = null;
private FrameSequence outgoingSequence = null;
private boolean incomingFrameValidation = false;
@ -49,17 +48,6 @@ public class ValidationExtension extends AbstractExtension
return "@validation";
}
@Override
public void setCoreSession(CoreSession coreSession)
{
super.setCoreSession(coreSession);
// TODO: change validation to use static methods instead of down casting CoreSession.
if (!(coreSession instanceof WebSocketCoreSession))
throw new IllegalArgumentException("ValidationExtension needs a CoreSession Configuration");
this.coreSession = (WebSocketCoreSession)coreSession;
}
@Override
public void onFrame(Frame frame, Callback callback)
{
@ -69,7 +57,7 @@ public class ValidationExtension extends AbstractExtension
incomingSequence.check(frame.getOpCode(), frame.isFin());
if (incomingFrameValidation)
coreSession.assertValidIncoming(frame);
FrameValidation.assertValidIncoming(frame, getCoreSession());
if (incomingUtf8Validation != null)
validateUTF8(frame, incomingUtf8Validation, continuedInOpCode);
@ -92,7 +80,7 @@ public class ValidationExtension extends AbstractExtension
outgoingSequence.check(frame.getOpCode(), frame.isFin());
if (outgoingFrameValidation)
coreSession.assertValidOutgoing(frame);
FrameValidation.assertValidOutgoing(frame, getCoreSession());
if (outgoingUtf8Validation != null)
validateUTF8(frame, outgoingUtf8Validation, continuedOutOpCode);

125
jetty-websocket/websocket-core-common/src/main/java/org/eclipse/jetty/websocket/core/internal/WebSocketCoreSession.java
View File

@ -43,11 +43,10 @@ import org.eclipse.jetty.websocket.core.OutgoingFrames;
import org.eclipse.jetty.websocket.core.WebSocketComponents;
import org.eclipse.jetty.websocket.core.WebSocketConstants;
import org.eclipse.jetty.websocket.core.exception.CloseException;
import org.eclipse.jetty.websocket.core.exception.MessageTooLargeException;
import org.eclipse.jetty.websocket.core.exception.ProtocolException;
import org.eclipse.jetty.websocket.core.exception.WebSocketTimeoutException;
import org.eclipse.jetty.websocket.core.exception.WebSocketWriteTimeoutException;
import org.eclipse.jetty.websocket.core.internal.Parser.ParsedFrame;
import org.eclipse.jetty.websocket.core.internal.util.FrameValidation;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
@ -109,106 +108,6 @@ public class WebSocketCoreSession implements IncomingFrames, CoreSession, Dumpab
return demanding;
}
public void assertValidIncoming(Frame frame)
{
assertValidFrame(frame);
// Validate frame size.
if (maxFrameSize > 0 && frame.getPayloadLength() > maxFrameSize)
throw new MessageTooLargeException("Cannot handle payload lengths larger than " + maxFrameSize);
// Assert Incoming Frame Behavior Required by RFC-6455 / Section 5.1
switch (behavior)
{
case SERVER:
if (!frame.isMasked())
throw new ProtocolException("Client MUST mask all frames (RFC-6455: Section 5.1)");
break;
case CLIENT:
if (frame.isMasked())
throw new ProtocolException("Server MUST NOT mask any frames (RFC-6455: Section 5.1)");
break;
default:
throw new IllegalStateException(behavior.toString());
}
/*
* RFC 6455 Section 5.5.1
* close frame payload is specially formatted which is checked in CloseStatus
*/
if (frame.getOpCode() == OpCode.CLOSE)
{
if (!(frame instanceof ParsedFrame)) // already check in parser
CloseStatus.getCloseStatus(frame); // return ignored as get used to validate there is a closeStatus
}
}
public void assertValidOutgoing(Frame frame) throws CloseException
{
assertValidFrame(frame);
// Validate frame size (allowed to be over max frame size if autoFragment is true).
if (!autoFragment && maxFrameSize > 0 && frame.getPayloadLength() > maxFrameSize)
throw new MessageTooLargeException("Cannot handle payload lengths larger than " + maxFrameSize);
/*
* RFC 6455 Section 5.5.1
* close frame payload is specially formatted which is checked in CloseStatus
*/
if (frame.getOpCode() == OpCode.CLOSE)
{
if (!(frame instanceof ParsedFrame)) // already check in parser
{
CloseStatus closeStatus = CloseStatus.getCloseStatus(frame);
if (!CloseStatus.isTransmittableStatusCode(closeStatus.getCode()) && (closeStatus.getCode() != CloseStatus.NO_CODE))
{
throw new ProtocolException("Frame has non-transmittable status code");
}
}
}
}
public void assertValidFrame(Frame frame)
{
if (!OpCode.isKnown(frame.getOpCode()))
throw new ProtocolException("Unknown opcode: " + frame.getOpCode());
int payloadLength = frame.getPayloadLength();
if (frame.isControlFrame())
{
if (!frame.isFin())
throw new ProtocolException("Fragmented Control Frame [" + OpCode.name(frame.getOpCode()) + "]");
if (payloadLength > Frame.MAX_CONTROL_PAYLOAD)
throw new ProtocolException("Invalid control frame payload length, [" + payloadLength + "] cannot exceed [" + Frame.MAX_CONTROL_PAYLOAD + "]");
if (frame.isRsv1())
throw new ProtocolException("Cannot have RSV1==true on Control frames");
if (frame.isRsv2())
throw new ProtocolException("Cannot have RSV2==true on Control frames");
if (frame.isRsv3())
throw new ProtocolException("Cannot have RSV3==true on Control frames");
}
else
{
/*
* RFC 6455 Section 5.2
*
* MUST be 0 unless an extension is negotiated that defines meanings for non-zero values. If a nonzero value is received and none of the negotiated
* extensions defines the meaning of such a nonzero value, the receiving endpoint MUST _Fail the WebSocket Connection_.
*/
ExtensionStack extensionStack = negotiated.getExtensions();
if (frame.isRsv1() && !extensionStack.isRsv1Used())
throw new ProtocolException("RSV1 not allowed to be set");
if (frame.isRsv2() && !extensionStack.isRsv2Used())
throw new ProtocolException("RSV2 not allowed to be set");
if (frame.isRsv3() && !extensionStack.isRsv3Used())
throw new ProtocolException("RSV3 not allowed to be set");
}
}
public ExtensionStack getExtensionStack()
{
return negotiated.getExtensions();
@ -501,6 +400,24 @@ public class WebSocketCoreSession implements IncomingFrames, CoreSession, Dumpab
connection.demand(n);
}
@Override
public boolean isRsv1Used()
{
return getExtensionStack().isRsv1Used();
}
@Override
public boolean isRsv2Used()
{
return getExtensionStack().isRsv2Used();
}
@Override
public boolean isRsv3Used()
{
return getExtensionStack().isRsv3Used();
}
public WebSocketConnection getConnection()
{
return connection;
@ -519,7 +436,7 @@ public class WebSocketCoreSession implements IncomingFrames, CoreSession, Dumpab
try
{
assertValidIncoming(frame);
FrameValidation.assertValidIncoming(frame, this);
}
catch (Throwable t)
{
@ -546,7 +463,7 @@ public class WebSocketCoreSession implements IncomingFrames, CoreSession, Dumpab
try
{
assertValidOutgoing(frame);
FrameValidation.assertValidOutgoing(frame, this);
}
catch (Throwable t)
{

133
jetty-websocket/websocket-core-common/src/main/java/org/eclipse/jetty/websocket/core/internal/util/FrameValidation.java Normal file
View File

@ -0,0 +1,133 @@
//
// ========================================================================
// Copyright (c) 1995-2021 Mort Bay Consulting Pty Ltd and others.
//
// This program and the accompanying materials are made available under the
// terms of the Eclipse Public License v. 2.0 which is available at
// https://www.eclipse.org/legal/epl-2.0, or the Apache License, Version 2.0
// which is available at https://www.apache.org/licenses/LICENSE-2.0.
//
// SPDX-License-Identifier: EPL-2.0 OR Apache-2.0
// ========================================================================
//
package org.eclipse.jetty.websocket.core.internal.util;
import org.eclipse.jetty.websocket.core.Behavior;
import org.eclipse.jetty.websocket.core.CloseStatus;
import org.eclipse.jetty.websocket.core.CoreSession;
import org.eclipse.jetty.websocket.core.Frame;
import org.eclipse.jetty.websocket.core.OpCode;
import org.eclipse.jetty.websocket.core.exception.CloseException;
import org.eclipse.jetty.websocket.core.exception.MessageTooLargeException;
import org.eclipse.jetty.websocket.core.exception.ProtocolException;
import org.eclipse.jetty.websocket.core.internal.Parser;
/**
* Some static utility methods for validating a {@link Frame} based on the state of its {@link CoreSession}.
*/
public class FrameValidation
{
public static void assertValidIncoming(Frame frame, CoreSession coreSession)
{
assertValidFrame(frame, coreSession);
// Validate frame size.
long maxFrameSize = coreSession.getMaxFrameSize();
if (maxFrameSize > 0 && frame.getPayloadLength() > maxFrameSize)
throw new MessageTooLargeException("Cannot handle payload lengths larger than " + maxFrameSize);
// Assert Incoming Frame Behavior Required by RFC-6455 / Section 5.1
Behavior behavior = coreSession.getBehavior();
switch (behavior)
{
case SERVER:
if (!frame.isMasked())
throw new ProtocolException("Client MUST mask all frames (RFC-6455: Section 5.1)");
break;
case CLIENT:
if (frame.isMasked())
throw new ProtocolException("Server MUST NOT mask any frames (RFC-6455: Section 5.1)");
break;
default:
throw new IllegalStateException(behavior.toString());
}
/*
* RFC 6455 Section 5.5.1
* close frame payload is specially formatted which is checked in CloseStatus
*/
if (frame.getOpCode() == OpCode.CLOSE)
{
if (!(frame instanceof Parser.ParsedFrame)) // already check in parser
CloseStatus.getCloseStatus(frame); // return ignored as get used to validate there is a closeStatus
}
}
public static void assertValidOutgoing(Frame frame, CoreSession coreSession) throws CloseException
{
assertValidFrame(frame, coreSession);
// Validate frame size (allowed to be over max frame size if autoFragment is true).
boolean autoFragment = coreSession.isAutoFragment();
long maxFrameSize = coreSession.getMaxFrameSize();
if (!autoFragment && maxFrameSize > 0 && frame.getPayloadLength() > maxFrameSize)
throw new MessageTooLargeException("Cannot handle payload lengths larger than " + maxFrameSize);
/*
* RFC 6455 Section 5.5.1
* close frame payload is specially formatted which is checked in CloseStatus
*/
if (frame.getOpCode() == OpCode.CLOSE)
{
if (!(frame instanceof Parser.ParsedFrame)) // already check in parser
{
CloseStatus closeStatus = CloseStatus.getCloseStatus(frame);
if (!CloseStatus.isTransmittableStatusCode(closeStatus.getCode()) && (closeStatus.getCode() != CloseStatus.NO_CODE))
{
throw new ProtocolException("Frame has non-transmittable status code");
}
}
}
}
public static void assertValidFrame(Frame frame, CoreSession coreSession)
{
if (!OpCode.isKnown(frame.getOpCode()))
throw new ProtocolException("Unknown opcode: " + frame.getOpCode());
int payloadLength = frame.getPayloadLength();
if (frame.isControlFrame())
{
if (!frame.isFin())
throw new ProtocolException("Fragmented Control Frame [" + OpCode.name(frame.getOpCode()) + "]");
if (payloadLength > Frame.MAX_CONTROL_PAYLOAD)
throw new ProtocolException("Invalid control frame payload length, [" + payloadLength + "] cannot exceed [" + Frame.MAX_CONTROL_PAYLOAD + "]");
if (frame.isRsv1())
throw new ProtocolException("Cannot have RSV1==true on Control frames");
if (frame.isRsv2())
throw new ProtocolException("Cannot have RSV2==true on Control frames");
if (frame.isRsv3())
throw new ProtocolException("Cannot have RSV3==true on Control frames");
}
else
{
/*
* RFC 6455 Section 5.2
*
* MUST be 0 unless an extension is negotiated that defines meanings for non-zero values. If a nonzero value is received and none of the negotiated
* extensions defines the meaning of such a nonzero value, the receiving endpoint MUST _Fail the WebSocket Connection_.
*/
if (frame.isRsv1() && !coreSession.isRsv1Used())
throw new ProtocolException("RSV1 not allowed to be set");
if (frame.isRsv2() && !coreSession.isRsv2Used())
throw new ProtocolException("RSV2 not allowed to be set");
if (frame.isRsv3() && !coreSession.isRsv3Used())
throw new ProtocolException("RSV3 not allowed to be set");
}
}
}

3
jetty-websocket/websocket-core-tests/src/test/java/org/eclipse/jetty/websocket/core/GeneratorFrameFlagsTest.java
View File

@ -23,6 +23,7 @@ import org.eclipse.jetty.websocket.core.internal.ExtensionStack;
import org.eclipse.jetty.websocket.core.internal.Generator;
import org.eclipse.jetty.websocket.core.internal.Negotiated;
import org.eclipse.jetty.websocket.core.internal.WebSocketCoreSession;
import org.eclipse.jetty.websocket.core.internal.util.FrameValidation;
import org.junit.jupiter.params.ParameterizedTest;
import org.junit.jupiter.params.provider.Arguments;
import org.junit.jupiter.params.provider.MethodSource;
@ -70,6 +71,6 @@ public class GeneratorFrameFlagsTest
ByteBuffer buffer = BufferUtil.allocate(100);
new Generator().generateWholeFrame(invalidFrame, buffer);
assertThrows(ProtocolException.class, () -> coreSession.assertValidOutgoing(invalidFrame));
assertThrows(ProtocolException.class, () -> FrameValidation.assertValidOutgoing(invalidFrame, coreSession));
}
}

7
jetty-websocket/websocket-core-tests/src/test/java/org/eclipse/jetty/websocket/core/GeneratorTest.java
View File

@ -29,6 +29,7 @@ import org.eclipse.jetty.websocket.core.internal.ExtensionStack;
import org.eclipse.jetty.websocket.core.internal.Generator;
import org.eclipse.jetty.websocket.core.internal.Negotiated;
import org.eclipse.jetty.websocket.core.internal.WebSocketCoreSession;
import org.eclipse.jetty.websocket.core.internal.util.FrameValidation;
import org.hamcrest.Matchers;
import org.junit.jupiter.api.Test;
import org.slf4j.Logger;
@ -399,7 +400,7 @@ public class GeneratorTest
BufferUtil.flipToFlush(bb, 0);
closeFrame.setPayload(bb);
assertThrows(ProtocolException.class, () -> coreSession.assertValidOutgoing(closeFrame));
assertThrows(ProtocolException.class, () -> FrameValidation.assertValidOutgoing(closeFrame, coreSession));
}
/**
@ -640,7 +641,7 @@ public class GeneratorTest
Frame pingFrame = new Frame(OpCode.PING);
pingFrame.setPayload(ByteBuffer.wrap(bytes));
assertThrows(WebSocketException.class, () -> coreSession.assertValidOutgoing(pingFrame));
assertThrows(WebSocketException.class, () -> FrameValidation.assertValidOutgoing(pingFrame, coreSession));
}
/**
@ -654,7 +655,7 @@ public class GeneratorTest
Frame pongFrame = new Frame(OpCode.PONG);
pongFrame.setPayload(ByteBuffer.wrap(bytes));
assertThrows(WebSocketException.class, () -> coreSession.assertValidOutgoing(pongFrame));
assertThrows(WebSocketException.class, () -> FrameValidation.assertValidOutgoing(pongFrame, coreSession));
}
/**

3
jetty-websocket/websocket-core-tests/src/test/java/org/eclipse/jetty/websocket/core/ParserCapture.java
View File

@ -22,6 +22,7 @@ import org.eclipse.jetty.websocket.core.internal.ExtensionStack;
import org.eclipse.jetty.websocket.core.internal.Negotiated;
import org.eclipse.jetty.websocket.core.internal.Parser;
import org.eclipse.jetty.websocket.core.internal.WebSocketCoreSession;
import org.eclipse.jetty.websocket.core.internal.util.FrameValidation;
import static org.hamcrest.MatcherAssert.assertThat;
import static org.hamcrest.Matchers.is;
@ -65,7 +66,7 @@ public class ParserCapture
if (frame == null)
break;
coreSession.assertValidIncoming(frame);
FrameValidation.assertValidIncoming(frame, coreSession);
if (!onFrame(frame))
break;