PAN-2388 better error on bad header

ethereum/p2p/src/main/java/tech/pegasys/pantheon/ethereum/p2p/rlpx/framing/Framer.java

@@ -162,28 +162,28 @@ public synchronized MessageData deframe(final ByteBuf buf) throws FramingExcepti
    * <p>This method expects a buffer containing the exact number of bytes a well-formed header
    * consists of (32 bytes at this time). Returns the frame size as extracted from the header.
    *
-   * @param tainedHeader The header.
+   * @param encryptedHeader The header as seen on the wire.
    * @throws FramingException If header parsing or decryption failed.
    * @return The frame size as extracted from the header.
    */
-  private int processHeader(final ByteBuf tainedHeader) throws FramingException {
-    if (tainedHeader.readableBytes() != LENGTH_FULL_HEADER) {
+  private int processHeader(final ByteBuf encryptedHeader) throws FramingException {
+    if (encryptedHeader.readableBytes() != LENGTH_FULL_HEADER) {
       throw error(
-          "Expected %s bytes in header, got %s", LENGTH_FULL_HEADER, tainedHeader.readableBytes());
+          "Expected %s bytes in header, got %s", LENGTH_FULL_HEADER, encryptedHeader.readableBytes());
     }
 
     // Decrypt the header.
     final byte[] hCipher = new byte[LENGTH_HEADER_DATA];
     final byte[] hMac = new byte[LENGTH_MAC];
-    tainedHeader.readBytes(hCipher).readBytes(hMac);
+    encryptedHeader.readBytes(hCipher).readBytes(hMac);
 
     // Header MAC validation.
     byte[] expectedMac = new byte[16];
     macEncryptor.processBlock(secrets.getIngressMac(), 0, expectedMac, 0);
     expectedMac = secrets.updateIngress(xor(expectedMac, hCipher)).getIngressMac();
     expectedMac = Arrays.copyOf(expectedMac, LENGTH_MAC);
 
-    validateMac(expectedMac, hMac);
+    validateMac(hMac, expectedMac);
 
     // Perform the header decryption.
     decryptor.processBytes(hCipher, 0, hCipher.length, hCipher, 0);
@@ -199,6 +199,11 @@ private int processHeader(final ByteBuf tainedHeader) throws FramingException {
     // Discard the header data (RLP): being set to fixed value 0xc28080 (list of two null
     // elements) by other clients.
     final int headerDataLength = RLP.calculateSize(BytesValue.wrapBuffer(h.nioBuffer()));
+    if (h.readableBytes() < headerDataLength) {
+      throw error(
+          "Expected at least %d readable bytes while processing header, remaining: %s",
+          headerDataLength, h.readableBytes());
+    }
     h.skipBytes(headerDataLength);
 
     // Discard padding in header (= zero-fill to 16-byte boundary).

ethereum/p2p/src/test/java/tech/pegasys/pantheon/ethereum/p2p/rlpx/framing/FramerTest.java

@@ -107,6 +107,23 @@ public void deframeOne() throws IOException {
         .doesNotThrowAnyException();
   }
 
+  @Test
+  public void deframeExcessDataThrowsException() throws IOException {
+    final JsonNode td = MAPPER.readTree(FramerTest.class.getResource("/peer2.json"));
+    final HandshakeSecrets secrets = secretsFrom(td, false);
+    final Framer framer = new Framer(secrets);
+
+    // frame has 19 byte protocol header
+    final ByteBuf badFrame =
+        Unpooled.wrappedBuffer(
+            decodeHexDump(
+                "1457498af1e2ccd62d3ed4e013b1c78ab1a2790b7edbf77cf5fe903ae15beb024d264ee0942c5ac7c0f8fd1098f36a360cfcabdb958e9c6145b1ff6d41f937aa"));
+
+    assertThatThrownBy(() -> framer.deframe(badFrame))
+        .isInstanceOf(FramingException.class)
+        .hasMessage("Expected at least 19 readable bytes while processing header, remaining: 13");
+  }
+
   @Test
   public void deframeManyNoFragmentation() throws IOException {
     // Load test data.