You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@qpid.apache.org by ro...@apache.org on 2017/03/06 15:57:31 UTC
[1/2] qpid-proton-j git commit: PROTON-1426: check the initial bytes
to ensure the expected SASL layer header is given
Repository: qpid-proton-j
Updated Branches:
refs/heads/master c9a0206f5 -> c8d67f7bf
PROTON-1426: check the initial bytes to ensure the expected SASL layer header is given
Project: http://git-wip-us.apache.org/repos/asf/qpid-proton-j/repo
Commit: http://git-wip-us.apache.org/repos/asf/qpid-proton-j/commit/3ea9f23d
Tree: http://git-wip-us.apache.org/repos/asf/qpid-proton-j/tree/3ea9f23d
Diff: http://git-wip-us.apache.org/repos/asf/qpid-proton-j/diff/3ea9f23d
Branch: refs/heads/master
Commit: 3ea9f23d049c88fd5e9cf8b3a04d424bf220beb8
Parents: c9a0206
Author: Robert Gemmell <ro...@apache.org>
Authored: Mon Mar 6 15:54:44 2017 +0000
Committer: Robert Gemmell <ro...@apache.org>
Committed: Mon Mar 6 15:54:44 2017 +0000
----------------------------------------------------------------------
.../proton/engine/impl/SaslFrameParser.java | 154 +++++++++++++++++--
.../proton/engine/impl/SaslFrameParserTest.java | 65 +++++++-
2 files changed, 207 insertions(+), 12 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/qpid-proton-j/blob/3ea9f23d/proton-j/src/main/java/org/apache/qpid/proton/engine/impl/SaslFrameParser.java
----------------------------------------------------------------------
diff --git a/proton-j/src/main/java/org/apache/qpid/proton/engine/impl/SaslFrameParser.java b/proton-j/src/main/java/org/apache/qpid/proton/engine/impl/SaslFrameParser.java
index 8becc72..37754ba 100644
--- a/proton-j/src/main/java/org/apache/qpid/proton/engine/impl/SaslFrameParser.java
+++ b/proton-j/src/main/java/org/apache/qpid/proton/engine/impl/SaslFrameParser.java
@@ -21,6 +21,8 @@
package org.apache.qpid.proton.engine.impl;
+import static org.apache.qpid.proton.engine.impl.AmqpHeader.SASL_HEADER;
+
import java.nio.ByteBuffer;
import org.apache.qpid.proton.amqp.Binary;
@@ -35,6 +37,14 @@ class SaslFrameParser
enum State
{
+ HEADER0,
+ HEADER1,
+ HEADER2,
+ HEADER3,
+ HEADER4,
+ HEADER5,
+ HEADER6,
+ HEADER7,
SIZE_0,
SIZE_1,
SIZE_2,
@@ -45,12 +55,11 @@ class SaslFrameParser
ERROR
}
- private State _state = State.SIZE_0;
+ private State _state = State.HEADER0;
private int _size;
private ByteBuffer _buffer;
- private int _ignore = 8;
private final ByteBufferDecoder _decoder;
@@ -70,19 +79,144 @@ class SaslFrameParser
State state = _state;
ByteBuffer oldIn = null;
- // Note that we simply skip over the header rather than parsing it.
- if(_ignore != 0)
- {
- int bytesToEat = Math.min(_ignore, input.remaining());
- input.position(input.position() + bytesToEat);
- _ignore -= bytesToEat;
- }
-
while(input.hasRemaining() && state != State.ERROR && !_sasl.isDone())
{
switch(state)
{
+ case HEADER0:
+ if(input.hasRemaining())
+ {
+ byte c = input.get();
+ if(c != SASL_HEADER[0])
+ {
+ frameParsingError = new TransportException("AMQP SASL header mismatch value %x, expecting %x. In state: %s", c, SASL_HEADER[0], state);
+ state = State.ERROR;
+ break;
+ }
+ state = State.HEADER1;
+ }
+ else
+ {
+ break;
+ }
+ case HEADER1:
+ if(input.hasRemaining())
+ {
+ byte c = input.get();
+ if(c != SASL_HEADER[1])
+ {
+ frameParsingError = new TransportException("AMQP SASL header mismatch value %x, expecting %x. In state: %s", c, SASL_HEADER[1], state);
+ state = State.ERROR;
+ break;
+ }
+ state = State.HEADER2;
+ }
+ else
+ {
+ break;
+ }
+ case HEADER2:
+ if(input.hasRemaining())
+ {
+ byte c = input.get();
+ if(c != SASL_HEADER[2])
+ {
+ frameParsingError = new TransportException("AMQP SASL header mismatch value %x, expecting %x. In state: %s", c, SASL_HEADER[2], state);
+ state = State.ERROR;
+ break;
+ }
+ state = State.HEADER3;
+ }
+ else
+ {
+ break;
+ }
+ case HEADER3:
+ if(input.hasRemaining())
+ {
+ byte c = input.get();
+ if(c != SASL_HEADER[3])
+ {
+ frameParsingError = new TransportException("AMQP SASL header mismatch value %x, expecting %x. In state: %s", c, SASL_HEADER[3], state);
+ state = State.ERROR;
+ break;
+ }
+ state = State.HEADER4;
+ }
+ else
+ {
+ break;
+ }
+ case HEADER4:
+ if(input.hasRemaining())
+ {
+ byte c = input.get();
+ if(c != SASL_HEADER[4])
+ {
+ frameParsingError = new TransportException("AMQP SASL header mismatch value %x, expecting %x. In state: %s", c, SASL_HEADER[4], state);
+ state = State.ERROR;
+ break;
+ }
+ state = State.HEADER5;
+ }
+ else
+ {
+ break;
+ }
+ case HEADER5:
+ if(input.hasRemaining())
+ {
+ byte c = input.get();
+ if(c != SASL_HEADER[5])
+ {
+ frameParsingError = new TransportException("AMQP SASL header mismatch value %x, expecting %x. In state: %s", c, SASL_HEADER[5], state);
+ state = State.ERROR;
+ break;
+ }
+ state = State.HEADER6;
+ }
+ else
+ {
+ break;
+ }
+ case HEADER6:
+ if(input.hasRemaining())
+ {
+ byte c = input.get();
+ if(c != SASL_HEADER[6])
+ {
+ frameParsingError = new TransportException("AMQP SASL header mismatch value %x, expecting %x. In state: %s", c, SASL_HEADER[6], state);
+ state = State.ERROR;
+ break;
+ }
+ state = State.HEADER7;
+ }
+ else
+ {
+ break;
+ }
+ case HEADER7:
+ if(input.hasRemaining())
+ {
+ byte c = input.get();
+ if(c != SASL_HEADER[7])
+ {
+ frameParsingError = new TransportException("AMQP SASL header mismatch value %x, expecting %x. In state: %s", c, SASL_HEADER[7], state);
+ state = State.ERROR;
+ break;
+ }
+ state = State.SIZE_0;
+ }
+ else
+ {
+ break;
+ }
case SIZE_0:
+ if(!input.hasRemaining())
+ {
+ break;
+ }
+
if(input.remaining() >= 4)
{
size = input.getInt();
http://git-wip-us.apache.org/repos/asf/qpid-proton-j/blob/3ea9f23d/proton-j/src/test/java/org/apache/qpid/proton/engine/impl/SaslFrameParserTest.java
----------------------------------------------------------------------
diff --git a/proton-j/src/test/java/org/apache/qpid/proton/engine/impl/SaslFrameParserTest.java b/proton-j/src/test/java/org/apache/qpid/proton/engine/impl/SaslFrameParserTest.java
index 31dd200..7ca6f74 100644
--- a/proton-j/src/test/java/org/apache/qpid/proton/engine/impl/SaslFrameParserTest.java
+++ b/proton-j/src/test/java/org/apache/qpid/proton/engine/impl/SaslFrameParserTest.java
@@ -20,9 +20,10 @@ package org.apache.qpid.proton.engine.impl;
import static org.mockito.Mockito.*;
import static org.junit.Assert.*;
-import static org.junit.matchers.JUnitMatchers.containsString;
+import static org.hamcrest.CoreMatchers.containsString;
import java.nio.ByteBuffer;
+import java.util.Arrays;
import org.apache.qpid.proton.amqp.Binary;
import org.apache.qpid.proton.amqp.Symbol;
@@ -39,7 +40,6 @@ import org.apache.qpid.proton.engine.TransportException;
import org.junit.Test;
/**
- * TODO test case where header is malformed
* TODO test case where input provides frame and half etc
*/
public class SaslFrameParserTest
@@ -131,6 +131,67 @@ public class SaslFrameParserTest
}
}
+ /*
+ * Test that if the first 8 bytes don't match the AMQP SASL header, it causes an error.
+ */
+ @Test
+ public void testInputOfInvalidHeader() {
+ for (int invalidIndex = 0; invalidIndex < 8; invalidIndex++) {
+ doInputOfInvalidHeaderTestImpl(invalidIndex);
+ }
+ }
+
+ private void doInputOfInvalidHeaderTestImpl(int invalidIndex) {
+ SaslFrameHandler mockSaslFrameHandler = mock(SaslFrameHandler.class);
+ ByteBufferDecoder mockDecoder = mock(ByteBufferDecoder.class);
+
+ SaslFrameParser saslFrameParser = new SaslFrameParser(mockSaslFrameHandler, mockDecoder);
+
+ byte[] header = Arrays.copyOf(AmqpHeader.SASL_HEADER, AmqpHeader.SASL_HEADER.length);
+ header[invalidIndex] = 'X';
+
+ try {
+ saslFrameParser.input(ByteBuffer.wrap(header));
+ fail("expected exception");
+ } catch (TransportException e) {
+ assertThat(e.getMessage(), containsString("AMQP SASL header mismatch"));
+ assertThat(e.getMessage(), containsString("In state: HEADER" + invalidIndex));
+ }
+
+ // Check that further interaction throws TransportException.
+ try {
+ saslFrameParser.input(ByteBuffer.wrap(new byte[0]));
+ fail("expected exception");
+ } catch (TransportException e) {
+ // Expected
+ }
+ }
+
+ /*
+ * Test that if the first 8 bytes, fed in one at a time, don't match the AMQP SASL header, it causes an error.
+ */
+ @Test
+ public void testInputOfValidHeaderInSegments()
+ {
+ sendAmqpSaslHeaderInSegments();
+
+ // Try feeding in an actual frame now to check we get past the header parsing ok
+ when(_mockSaslFrameHandler.isDone()).thenReturn(false);
+
+ _frameParser.input(_saslFrameBytes);
+
+ verify(_mockSaslFrameHandler).handle(isA(SaslInit.class), (Binary)isNull());
+ }
+
+ private void sendAmqpSaslHeaderInSegments()
+ {
+ for (int headerIndex = 0; headerIndex < 8; headerIndex++)
+ {
+ byte headerPart = AmqpHeader.SASL_HEADER[headerIndex];
+ _frameParser.input(ByteBuffer.wrap(new byte[] { headerPart }));
+ }
+ }
+
private void sendAmqpSaslHeader(SaslFrameParser saslFrameParser)
{
saslFrameParser.input(ByteBuffer.wrap(AmqpHeader.SASL_HEADER));
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@qpid.apache.org
For additional commands, e-mail: commits-help@qpid.apache.org
[2/2] qpid-proton-j git commit: PROTON-1427: enforce the SASL frame
size limit of 512bytes
Posted by ro...@apache.org.
PROTON-1427: enforce the SASL frame size limit of 512bytes
Also add some tests of sasl frame doff limit handling
Project: http://git-wip-us.apache.org/repos/asf/qpid-proton-j/repo
Commit: http://git-wip-us.apache.org/repos/asf/qpid-proton-j/commit/c8d67f7b
Tree: http://git-wip-us.apache.org/repos/asf/qpid-proton-j/tree/c8d67f7b
Diff: http://git-wip-us.apache.org/repos/asf/qpid-proton-j/diff/c8d67f7b
Branch: refs/heads/master
Commit: c8d67f7bfda81323b26f43f5157601257147c0de
Parents: 3ea9f23
Author: Robert Gemmell <ro...@apache.org>
Authored: Mon Mar 6 15:55:05 2017 +0000
Committer: Robert Gemmell <ro...@apache.org>
Committed: Mon Mar 6 15:55:05 2017 +0000
----------------------------------------------------------------------
.../qpid/proton/engine/impl/FrameParser.java | 2 +-
.../proton/engine/impl/SaslFrameParser.java | 15 +++-
.../proton/engine/impl/SaslFrameParserTest.java | 81 ++++++++++++++++++++
3 files changed, 93 insertions(+), 5 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/qpid-proton-j/blob/c8d67f7b/proton-j/src/main/java/org/apache/qpid/proton/engine/impl/FrameParser.java
----------------------------------------------------------------------
diff --git a/proton-j/src/main/java/org/apache/qpid/proton/engine/impl/FrameParser.java b/proton-j/src/main/java/org/apache/qpid/proton/engine/impl/FrameParser.java
index 6aede84..51ad06e 100644
--- a/proton-j/src/main/java/org/apache/qpid/proton/engine/impl/FrameParser.java
+++ b/proton-j/src/main/java/org/apache/qpid/proton/engine/impl/FrameParser.java
@@ -340,7 +340,7 @@ class FrameParser implements TransportInput
}
else if(dataOffset > size)
{
- frameParsingError = new TransportException("specified frame data offset %d larger than the frame size %d", dataOffset, _size);
+ frameParsingError = new TransportException("specified frame data offset %d larger than the frame size %d", dataOffset, size);
state = State.ERROR;
break;
}
http://git-wip-us.apache.org/repos/asf/qpid-proton-j/blob/c8d67f7b/proton-j/src/main/java/org/apache/qpid/proton/engine/impl/SaslFrameParser.java
----------------------------------------------------------------------
diff --git a/proton-j/src/main/java/org/apache/qpid/proton/engine/impl/SaslFrameParser.java b/proton-j/src/main/java/org/apache/qpid/proton/engine/impl/SaslFrameParser.java
index 37754ba..dbf81cb 100644
--- a/proton-j/src/main/java/org/apache/qpid/proton/engine/impl/SaslFrameParser.java
+++ b/proton-j/src/main/java/org/apache/qpid/proton/engine/impl/SaslFrameParser.java
@@ -253,9 +253,16 @@ class SaslFrameParser
case PRE_PARSE:
if(size < 8)
{
- frameParsingError = new TransportException("specified frame size %d smaller than minimum frame header "
- + "size %d",
- _size, 8);
+ frameParsingError = new TransportException(
+ "specified frame size %d smaller than minimum SASL frame header size 8", size);
+ state = State.ERROR;
+ break;
+ }
+
+ if (size > 512)
+ {
+ frameParsingError = new TransportException(
+ "specified frame size %d larger than maximum SASL frame size 512", size);
state = State.ERROR;
break;
}
@@ -300,7 +307,7 @@ class SaslFrameParser
}
else if(dataOffset > size)
{
- frameParsingError = new TransportException("specified frame data offset %d larger than the frame size %d", dataOffset, _size);
+ frameParsingError = new TransportException("specified frame data offset %d larger than the frame size %d", dataOffset, size);
state = State.ERROR;
break;
}
http://git-wip-us.apache.org/repos/asf/qpid-proton-j/blob/c8d67f7b/proton-j/src/test/java/org/apache/qpid/proton/engine/impl/SaslFrameParserTest.java
----------------------------------------------------------------------
diff --git a/proton-j/src/test/java/org/apache/qpid/proton/engine/impl/SaslFrameParserTest.java b/proton-j/src/test/java/org/apache/qpid/proton/engine/impl/SaslFrameParserTest.java
index 7ca6f74..e67177f 100644
--- a/proton-j/src/test/java/org/apache/qpid/proton/engine/impl/SaslFrameParserTest.java
+++ b/proton-j/src/test/java/org/apache/qpid/proton/engine/impl/SaslFrameParserTest.java
@@ -77,6 +77,87 @@ public class SaslFrameParserTest
verify(_mockSaslFrameHandler).handle(isA(SaslInit.class), (Binary)isNull());
}
+ /*
+ * Test that SASL frames indicating they are over 512 bytes (the minimum max frame size, and the
+ * largest usable during SASL before max frame size is later determined by the Open frames) causes an error.
+ */
+ @Test
+ public void testInputOfFrameWithInvalidSizeExceedingMinMaxFrameSize()
+ {
+ sendAmqpSaslHeader(_frameParserWithMockDecoder);
+
+ // http://docs.oasis-open.org/amqp/core/v1.0/os/amqp-core-security-v1.0-os.html#doc-idp43536
+ // Description: '513byte sized' SASL frame header
+ byte[] oversizedSaslFrameHeader = new byte[] { (byte) 0x00, 0x00, 0x02, 0x01, 0x02, 0x01, 0x00, 0x00 };
+
+ try {
+ _frameParserWithMockDecoder.input(ByteBuffer.wrap(oversizedSaslFrameHeader));
+ fail("expected exception");
+ } catch (TransportException e) {
+ assertThat(e.getMessage(), containsString("frame size 513 larger than maximum"));
+ }
+ }
+
+ /*
+ * Test that SASL frames indicating they are under 8 bytes (the minimum size of the frame header) causes an error.
+ */
+ @Test
+ public void testInputOfFrameWithInvalidSizeBelowMinimumPossible()
+ {
+ sendAmqpSaslHeader(_frameParserWithMockDecoder);
+
+ // http://docs.oasis-open.org/amqp/core/v1.0/os/amqp-core-security-v1.0-os.html#doc-idp43536
+ // Description: '7byte sized' SASL frame header
+ byte[] undersizedSaslFrameHeader = new byte[] { (byte) 0x00, 0x00, 0x00, 0x07, 0x02, 0x01, 0x00, 0x00 };
+
+ try {
+ _frameParserWithMockDecoder.input(ByteBuffer.wrap(undersizedSaslFrameHeader));
+ fail("expected exception");
+ } catch (TransportException e) {
+ assertThat(e.getMessage(), containsString("frame size 7 smaller than minimum"));
+ }
+ }
+
+ /*
+ * Test that SASL frames indicating a doff under 8 bytes (the minimum size of the frame header) causes an error.
+ */
+ @Test
+ public void testInputOfFrameWithInvalidDoffBelowMinimumPossible()
+ {
+ sendAmqpSaslHeader(_frameParserWithMockDecoder);
+
+ // http://docs.oasis-open.org/amqp/core/v1.0/os/amqp-core-security-v1.0-os.html#doc-idp43536
+ // Description: '8byte sized' SASL frame header with invalid doff of 1[*4 = 4bytes]
+ byte[] underMinDoffSaslFrameHeader = new byte[] { (byte) 0x00, 0x00, 0x00, 0x08, 0x01, 0x01, 0x00, 0x00 };
+
+ try {
+ _frameParserWithMockDecoder.input(ByteBuffer.wrap(underMinDoffSaslFrameHeader));
+ fail("expected exception");
+ } catch (TransportException e) {
+ assertThat(e.getMessage(), containsString("data offset 4 smaller than minimum"));
+ }
+ }
+
+ /*
+ * Test that SASL frames indicating a doff larger than the frame size causes an error.
+ */
+ @Test
+ public void testInputOfFrameWithInvalidDoffAboveMaximumPossible()
+ {
+ sendAmqpSaslHeader(_frameParserWithMockDecoder);
+
+ // http://docs.oasis-open.org/amqp/core/v1.0/os/amqp-core-security-v1.0-os.html#doc-idp43536
+ // Description: '8byte sized' SASL frame header with invalid doff of 3[*4 = 12bytes]
+ byte[] overFrameSizeDoffSaslFrameHeader = new byte[] { (byte) 0x00, 0x00, 0x00, 0x08, 0x03, 0x01, 0x00, 0x00 };
+
+ try {
+ _frameParserWithMockDecoder.input(ByteBuffer.wrap(overFrameSizeDoffSaslFrameHeader));
+ fail("expected exception");
+ } catch (TransportException e) {
+ assertThat(e.getMessage(), containsString("data offset 12 larger than the frame size 8"));
+ }
+ }
+
@Test
public void testInputOfInvalidFrame_causesErrorAndRefusesFurtherInput()
{
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@qpid.apache.org
For additional commands, e-mail: commits-help@qpid.apache.org