Author: markt Date: Thu Jun 18 19:33:54 2015 New Revision: 1686300 URL: http://svn.apache.org/r1686300 Log: Implement optional padding validation
Modified: tomcat/trunk/java/org/apache/coyote/http2/Http2Parser.java tomcat/trunk/java/org/apache/coyote/http2/LocalStrings.properties tomcat/trunk/test/org/apache/coyote/http2/TestHttp2Section_6_1.java Modified: tomcat/trunk/java/org/apache/coyote/http2/Http2Parser.java URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/coyote/http2/Http2Parser.java?rev=1686300&r1=1686299&r2=1686300&view=diff ============================================================================== --- tomcat/trunk/java/org/apache/coyote/http2/Http2Parser.java (original) +++ tomcat/trunk/java/org/apache/coyote/http2/Http2Parser.java Thu Jun 18 19:33:54 2015 @@ -86,7 +86,7 @@ class Http2Parser { try { validateFrame(expected, frameType, streamId, flags, payloadSize); } catch (StreamException se) { - swallow(streamId, payloadSize); + swallow(streamId, payloadSize, false); throw se; } @@ -160,7 +160,7 @@ class Http2Parser { ByteBuffer dest = output.getInputByteBuffer(streamId, dataLength); if (dest == null) { - swallow(streamId, dataLength); + swallow(streamId, dataLength, false); if (endOfStream) { output.receiveEndOfStream(streamId); } @@ -174,7 +174,7 @@ class Http2Parser { } } if (padLength > 0) { - swallow(streamId, padLength); + swallow(streamId, padLength, true); output.swallowedPadding(streamId, padLength); } } @@ -189,7 +189,7 @@ class Http2Parser { try { hpackDecoder.setHeaderEmitter(output.headersStart(streamId)); } catch (StreamException se) { - swallow(streamId, payloadSize); + swallow(streamId, payloadSize, false); throw se; } @@ -224,7 +224,7 @@ class Http2Parser { readHeaderBlock(payloadSize, endOfHeaders); - swallow(streamId, padLength); + swallow(streamId, padLength, true); if (endOfHeaders) { output.headersEnd(streamId); @@ -405,12 +405,18 @@ class Http2Parser { private void readUnknownFrame(int streamId, FrameType frameType, int flags, int payloadSize) throws IOException { - swallow(streamId, payloadSize); + try { + swallow(streamId, payloadSize, false); + } catch (ConnectionException e) { + // Will never happen because swallow() is called with mustBeZero set + // to false + } output.swallowed(streamId, frameType, flags, payloadSize); } - private void swallow(int streamId, int len) throws IOException { + private void swallow(int streamId, int len, boolean mustBeZero) + throws IOException, ConnectionException { if (log.isDebugEnabled()) { log.debug(sm.getString("http2Parser.swallow.debug", connectionId, Integer.toString(streamId), Integer.toString(len))); @@ -423,6 +429,17 @@ class Http2Parser { while (read < len) { int thisTime = Math.min(buffer.length, len - read); input.fill(true, buffer, 0, thisTime); + if (mustBeZero) { + // Validate the padding is zero since receiving non-zero padding + // is a strong indication of either a faulty client or a server + // side bug. + for (int i = 0; i < thisTime; i++) { + if (buffer[i] != 0) { + throw new ConnectionException(sm.getString("http2Parser.nonZeroPadding", + connectionId, Integer.toString(streamId)), Http2Error.PROTOCOL_ERROR); + } + } + } read += thisTime; } } Modified: tomcat/trunk/java/org/apache/coyote/http2/LocalStrings.properties URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/coyote/http2/LocalStrings.properties?rev=1686300&r1=1686299&r2=1686300&view=diff ============================================================================== --- tomcat/trunk/java/org/apache/coyote/http2/LocalStrings.properties (original) +++ tomcat/trunk/java/org/apache/coyote/http2/LocalStrings.properties Thu Jun 18 19:33:54 2015 @@ -38,6 +38,7 @@ hpackhuffman.huffmanEncodedHpackValueDid http2Parser.headers.wrongFrameType=Connection [{0}], headers in progress for stream [{1}] but a frame of type [{2}] was received http2Parser.headers.wrongStream=Connection [{0}], headers in progress for stream [{1}] but a frame for stream [{2}] was received +http2Parser.nonZeroPadding=Connection [{0}], Stream [{1}], Non-zero padding received http2Parser.payloadTooBig=The payload is [{0}] bytes long but the maximum frame size is [{1}] http2Parser.preface.invalid=Invalid connection preface [{0}] presented http2Parser.preface.io=Unable to read connection preface Modified: tomcat/trunk/test/org/apache/coyote/http2/TestHttp2Section_6_1.java URL: http://svn.apache.org/viewvc/tomcat/trunk/test/org/apache/coyote/http2/TestHttp2Section_6_1.java?rev=1686300&r1=1686299&r2=1686300&view=diff ============================================================================== --- tomcat/trunk/test/org/apache/coyote/http2/TestHttp2Section_6_1.java (original) +++ tomcat/trunk/test/org/apache/coyote/http2/TestHttp2Section_6_1.java Thu Jun 18 19:33:54 2015 @@ -72,5 +72,25 @@ public class TestHttp2Section_6_1 extend + "3-EndOfStream\n", trace); } + + @Test + public void testDataFrameWithNonZeroPadding() throws Exception { + http2Connect(); + + byte[] padding = new byte[8]; + padding[4] = 0x01; + + sendSimplePostRequest(3, padding); + parser.readFrame(true); + // May see Window updates depending on timing + while (output.getTrace().contains("WindowSize")) { + output.clearTrace(); + parser.readFrame(true); + } + + String trace = output.getTrace(); + Assert.assertTrue(trace, trace.startsWith("0-Goaway-[3]-[1]-[")); + } + // TODO: Remainder if section 6.1 tests } --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org