JUnit test for treat-as-withdraw procedures

add test to verify that parsing UPDATE message with
malformed attribute list result in converting:
- Nlri to withdrawn routes (parser-impl test)
- MP_REACH to MP_UNREACH (extensions/inet test)

JIRA: BGPCEP-359
Change-Id: Ia0ad989c15e1645746942a662e7da1d5a079b9ad
Signed-off-by: Matej Perina <matej.perina@pantheon.tech>

diff --git a/bgp/extensions/inet/src/test/java/org/opendaylight/protocol/bgp/inet/codec/BGPParserTest.java b/bgp/extensions/inet/src/test/java/org/opendaylight/protocol/bgp/inet/codec/BGPParserTest.java
index f4a8a2875a01939bf930e706971098905fd44b26..d4fd5c0ed225d3273379904072c3265a87c05a3c 100644 (file)
--- a/bgp/extensions/inet/src/test/java/org/opendaylight/protocol/bgp/inet/codec/BGPParserTest.java
+++ b/bgp/extensions/inet/src/test/java/org/opendaylight/protocol/bgp/inet/codec/BGPParserTest.java
@@ -10,6 +10,7 @@ package org.opendaylight.protocol.bgp.inet.codec;
 
 import static org.junit.Assert.assertArrayEquals;
 import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNotNull;
 import static org.junit.Assert.assertNull;
 
 import com.google.common.collect.Lists;
@@ -24,6 +25,9 @@ import java.util.List;
 import org.junit.BeforeClass;
 import org.junit.Test;
 import org.opendaylight.protocol.bgp.parser.spi.MessageRegistry;
+import org.opendaylight.protocol.bgp.parser.spi.RevisedErrorHandlingSupport;
+import org.opendaylight.protocol.bgp.parser.spi.pojo.PeerSpecificParserConstraintImpl;
+import org.opendaylight.protocol.bgp.parser.spi.pojo.RevisedErrorHandlingSupportImpl;
 import org.opendaylight.protocol.bgp.parser.spi.pojo.ServiceLoaderBGPExtensionProviderContext;
 import org.opendaylight.protocol.util.ByteArray;
 import org.opendaylight.yang.gen.v1.urn.ietf.params.xml.ns.yang.ietf.inet.types.rev130715.AsNumber;
@@ -45,9 +49,12 @@ import org.opendaylight.yang.gen.v1.urn.opendaylight.params.xml.ns.yang.bgp.mess
 import org.opendaylight.yang.gen.v1.urn.opendaylight.params.xml.ns.yang.bgp.message.rev180329.path.attributes.attributes.OriginatorIdBuilder;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.params.xml.ns.yang.bgp.message.rev180329.path.attributes.attributes.as.path.Segments;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.params.xml.ns.yang.bgp.message.rev180329.path.attributes.attributes.as.path.SegmentsBuilder;
+import org.opendaylight.yang.gen.v1.urn.opendaylight.params.xml.ns.yang.bgp.message.rev180329.update.message.WithdrawnRoutes;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.params.xml.ns.yang.bgp.multiprotocol.rev180329.Attributes1;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.params.xml.ns.yang.bgp.multiprotocol.rev180329.Attributes1Builder;
+import org.opendaylight.yang.gen.v1.urn.opendaylight.params.xml.ns.yang.bgp.multiprotocol.rev180329.Attributes2;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.params.xml.ns.yang.bgp.multiprotocol.rev180329.update.attributes.MpReachNlriBuilder;
+import org.opendaylight.yang.gen.v1.urn.opendaylight.params.xml.ns.yang.bgp.multiprotocol.rev180329.update.attributes.MpUnreachNlri;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.params.xml.ns.yang.bgp.multiprotocol.rev180329.update.attributes.mp.reach.nlri.AdvertizedRoutesBuilder;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.params.xml.ns.yang.bgp.types.rev180329.BgpOrigin;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.params.xml.ns.yang.bgp.types.rev180329.ClusterIdentifier;
@@ -63,27 +70,32 @@ public class BGPParserTest {
 
     private static MessageRegistry messageRegistry;
 
-    private static byte[] input;
+    private static List<byte[]> input;
+
+    private static int MESSAGE_COUNT = 2;
 
     @BeforeClass
     public static void setUp() throws Exception {
         messageRegistry = ServiceLoaderBGPExtensionProviderContext.getSingletonInstance().getMessageRegistry();
-
-        final String name = "/up2.bin";
-        try (InputStream is = BGPParserTest.class.getResourceAsStream(name)) {
-            if (is == null) {
-                throw new IOException("Failed to get resource " + name);
-            }
-            final ByteArrayOutputStream bis = new ByteArrayOutputStream();
-            final byte[] data = new byte[MAX_SIZE];
-            int position;
-            while ((position = is.read(data, 0, data.length)) != -1) {
-                bis.write(data, 0, position);
+        input = new ArrayList<>(MESSAGE_COUNT);
+        for (int i = 1; i <= MESSAGE_COUNT; i++) {
+
+            final String name = "/up" + i + ".bin";
+            try (InputStream is = BGPParserTest.class.getResourceAsStream(name)) {
+                if (is == null) {
+                    throw new IOException("Failed to get resource " + name);
+                }
+                final ByteArrayOutputStream bis = new ByteArrayOutputStream();
+                final byte[] data = new byte[MAX_SIZE];
+                int position;
+                while ((position = is.read(data, 0, data.length)) != -1) {
+                    bis.write(data, 0, position);
+                }
+                bis.flush();
+
+                input.add(bis.toByteArray());
+                is.close();
             }
-            bis.flush();
-
-            input = bis.toByteArray();
-            is.close();
         }
     }
 
@@ -140,7 +152,7 @@ public class BGPParserTest {
      */
     @Test
     public void testIPv6Nlri() throws Exception {
-        final Update message = (Update) messageRegistry.parseMessage(Unpooled.wrappedBuffer(input), null);
+        final Update message = (Update) messageRegistry.parseMessage(Unpooled.wrappedBuffer(input.get(1)), null);
 
         // check fields
         assertNull(message.getWithdrawnRoutes());
@@ -210,7 +222,56 @@ public class BGPParserTest {
 
         final ByteBuf buffer = Unpooled.buffer();
         messageRegistry.serializeMessage(message, buffer);
-        assertArrayEquals(input, ByteArray.readAllBytes(buffer));
+        assertArrayEquals(input.get(1), ByteArray.readAllBytes(buffer));
     }
 
+    /*
+     * Tests withdrawn routes with malformed attribute.
+     *
+     * ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff <- marker
+     * 00 36 <- length (54) - including header
+     * 02 <- message type
+     * 00 00 <- withdrawn routes length
+     * 00 1b <- total path attribute length (27)
+     * 40 <- attribute flags
+     * 01 <- attribute type code (origin)
+     * 01 <- WRONG attribute length
+     * 00 <- Origin value (IGP)
+     * 40 <- attribute flags
+     * 03 <- attribute type code (Next Hop)
+     * 04 <- attribute length
+     * 0a 00 00 02 <- value (10.0.0.2)
+     * 40 <- attribute flags
+     * 0e <- attribute type code (MP_REACH)
+     * 0d <- attribute length
+     * 00 01 <- AFI (Ipv4)
+     * 01 <- SAFI (Unicast)
+     * 04 <- next hop length
+     * ff ff ff ff <- next hop
+     * 00 <- reserved
+     * 18 <- length
+     * 0a 00 01 <- prefix (10.0.1.0)
+     * //NLRI
+     * 18 <- length
+     * 0a 00 02 <- prefix (10.0.2.0)
+     */
+    @Test
+    public void testParseUpdateMessageWithMalformedAttributes() throws Exception {
+        final PeerSpecificParserConstraintImpl constraint = new PeerSpecificParserConstraintImpl();
+        constraint.addPeerConstraint(RevisedErrorHandlingSupport.class,
+                RevisedErrorHandlingSupportImpl.forExternalPeer());
+        final Update message = (Update) messageRegistry.parseMessage(Unpooled.wrappedBuffer(input.get(0)), constraint);
+        assertNotNull(message);
+        assertNull(message.getNlri());
+        final List<WithdrawnRoutes> withdrawnRoutes = message.getWithdrawnRoutes();
+        assertNotNull(withdrawnRoutes);
+        assertEquals(1, withdrawnRoutes.size());
+        final Attributes attributes = message.getAttributes();
+        assertNotNull(attributes);
+        assertNull(attributes.augmentation(Attributes1.class));
+        final Attributes2 attributes2 = attributes.augmentation(Attributes2.class);
+        assertNotNull(attributes2);
+        final MpUnreachNlri mpUnreachNlri = attributes2.getMpUnreachNlri();
+        assertNotNull(mpUnreachNlri);
+    }
 }

diff --git a/bgp/extensions/inet/src/test/resources/up1.bin b/bgp/extensions/inet/src/test/resources/up1.bin
new file mode 100644 (file)
index 0000000..2bd5702
Binary files /dev/null and b/bgp/extensions/inet/src/test/resources/up1.bin differ

diff --git a/bgp/parser-impl/src/test/java/org/opendaylight/protocol/bgp/parser/impl/BGPParserTest.java b/bgp/parser-impl/src/test/java/org/opendaylight/protocol/bgp/parser/impl/BGPParserTest.java
index d4f663e73a778615a551437baad458d1517c8119..b68665e29c687081512533029caf41843730fe73 100644 (file)
--- a/bgp/parser-impl/src/test/java/org/opendaylight/protocol/bgp/parser/impl/BGPParserTest.java
+++ b/bgp/parser-impl/src/test/java/org/opendaylight/protocol/bgp/parser/impl/BGPParserTest.java
@@ -26,12 +26,16 @@ import java.util.Optional;
 import org.junit.BeforeClass;
 import org.junit.Test;
 import org.mockito.Mockito;
+import org.opendaylight.protocol.bgp.parser.BGPDocumentedException;
 import org.opendaylight.protocol.bgp.parser.impl.message.BGPUpdateMessageParser;
 import org.opendaylight.protocol.bgp.parser.impl.message.update.CommunityUtil;
 import org.opendaylight.protocol.bgp.parser.spi.MessageUtil;
 import org.opendaylight.protocol.bgp.parser.spi.MultiPathSupport;
 import org.opendaylight.protocol.bgp.parser.spi.NlriRegistry;
 import org.opendaylight.protocol.bgp.parser.spi.PeerSpecificParserConstraint;
+import org.opendaylight.protocol.bgp.parser.spi.RevisedErrorHandlingSupport;
+import org.opendaylight.protocol.bgp.parser.spi.pojo.PeerSpecificParserConstraintImpl;
+import org.opendaylight.protocol.bgp.parser.spi.pojo.RevisedErrorHandlingSupportImpl;
 import org.opendaylight.protocol.bgp.parser.spi.pojo.ServiceLoaderBGPExtensionProviderContext;
 import org.opendaylight.protocol.bgp.util.HexDumpBGPFileParser;
 import org.opendaylight.protocol.util.ByteArray;
@@ -75,7 +79,7 @@ import org.opendaylight.yang.gen.v1.urn.opendaylight.params.xml.ns.yang.bgp.type
 public class BGPParserTest {
     private static final List<byte[]> INPUT_BYTES = new ArrayList<>();
 
-    private static final int COUNTER = 7;
+    private static final int COUNTER = 8;
 
     private static final int MAX_SIZE = 300;
 
@@ -87,7 +91,7 @@ public class BGPParserTest {
 
     private static List<byte[]> updatesWithMultiplePath;
 
-    private static PeerSpecificParserConstraint constraint;
+    private static PeerSpecificParserConstraint mpConstraint;
 
     private static MultiPathSupport mpSupport;
 
@@ -115,10 +119,10 @@ public class BGPParserTest {
         }
         updatesWithMultiplePath = HexDumpBGPFileParser.parseMessages(BGPParserTest.class.getResourceAsStream(
             MULTIPATH_HEX_FILE));
-        constraint = mock(PeerSpecificParserConstraint.class);
+        mpConstraint = mock(PeerSpecificParserConstraint.class);
         mpSupport = mock(MultiPathSupport.class);
-        Mockito.doReturn(Optional.empty()).when(constraint).getPeerConstraint(Mockito.any());
-        Mockito.doReturn(Optional.of(mpSupport)).when(constraint).getPeerConstraint(MultiPathSupport.class);
+        Mockito.doReturn(Optional.empty()).when(mpConstraint).getPeerConstraint(Mockito.any());
+        Mockito.doReturn(Optional.of(mpSupport)).when(mpConstraint).getPeerConstraint(MultiPathSupport.class);
         Mockito.doReturn(true).when(mpSupport).isTableTypeSupported(Mockito.any());
     }
 
@@ -628,7 +632,7 @@ public class BGPParserTest {
         final int messageLength = ByteArray.bytesToInt(ByteArray.subByte(updatesWithMultiplePath.get(0),
             MessageUtil.MARKER_LENGTH, LENGTH_FIELD_LENGTH));
         final Update message = BGPParserTest.updateParser.parseMessageBody(Unpooled.copiedBuffer(body), messageLength,
-            constraint);
+            mpConstraint);
 
         // check fields
 
@@ -709,6 +713,7 @@ public class BGPParserTest {
      * 00 00 00 02 <- path-id (2)
      * 1e ac 10 00 04 <- route (172.16.0.4)
      * 00 00 <- total path attribute length
+     *
      */
     @Test
     public void testUpdateMessageWithdrawAddPath() throws Exception {
@@ -716,7 +721,7 @@ public class BGPParserTest {
         final int messageLength = ByteArray.bytesToInt(ByteArray.subByte(updatesWithMultiplePath.get(1),
             MessageUtil.MARKER_LENGTH, LENGTH_FIELD_LENGTH));
         final Update message = BGPParserTest.updateParser.parseMessageBody(Unpooled.copiedBuffer(body), messageLength,
-            constraint);
+            mpConstraint);
 
         // attributes
         final List<WithdrawnRoutes> withdrawnRoutes = Lists.newArrayList();
@@ -734,4 +739,50 @@ public class BGPParserTest {
         BGPParserTest.updateParser.serializeMessage(message, buffer);
         assertArrayEquals(updatesWithMultiplePath.get(1), ByteArray.readAllBytes(buffer));
     }
+
+    /*
+     * Tests withdrawn routes with malformed attribute.
+     *
+     * ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff <- marker
+     * 00 35 <- length (53) - including header
+     * 02 <- message type
+     * 00 00 <- withdrawn routes length
+     * 00 1a <- total path attribute length (26)
+     * 40 <- attribute flags
+     * 01 <- attribute type code (origin)
+     * 02 <- WRONG attribute length
+     * 00 <- Origin value (IGP)
+     * 40 <- attribute flags
+     * 03 <- attribute type code (Next Hop)
+     * 04 <- attribute length
+     * 0a 00 00 02 <- value (10.0.0.2)
+     * 40 <- attribute flags
+     * 0e <- attribute type code (MP_REACH)
+     * 00 01 <- AFI (Ipv4)
+     * 01 <- SAFI (Unicast)
+     * 04 <- next hop length
+     * ff ff ff ff <- next hop
+     * 00 <- reserved
+     * 18 <- length
+     * 0a 00 01 <- prefix (10.0.1.0)
+     * //NLRI
+     * 18 <- length
+     * 0a 00 02 <- prefix (10.0.2.0)
+     */
+    @Test
+    public void testUpdateMessageWithMalformedAttribute() throws BGPDocumentedException {
+        final byte[] body = ByteArray.cutBytes(INPUT_BYTES.get(7), MessageUtil.COMMON_HEADER_LENGTH);
+        final int messageLength = ByteArray.bytesToInt(ByteArray.subByte(INPUT_BYTES.get(6), MessageUtil.MARKER_LENGTH,
+                LENGTH_FIELD_LENGTH));
+        final PeerSpecificParserConstraintImpl constraint = new PeerSpecificParserConstraintImpl();
+        constraint.addPeerConstraint(RevisedErrorHandlingSupport.class,
+                RevisedErrorHandlingSupportImpl.forExternalPeer());
+        final Update message = BGPParserTest.updateParser.parseMessageBody(Unpooled.copiedBuffer(body), messageLength,
+                constraint);
+        assertNotNull(message);
+        assertNull(message.getNlri());
+        final List<WithdrawnRoutes> withdrawnRoutes = message.getWithdrawnRoutes();
+        assertNotNull(withdrawnRoutes);
+        assertEquals(1, withdrawnRoutes.size());
+    }
 }

diff --git a/bgp/parser-impl/src/test/resources/up8.bin b/bgp/parser-impl/src/test/resources/up8.bin
new file mode 100644 (file)
index 0000000..962bab1
Binary files /dev/null and b/bgp/parser-impl/src/test/resources/up8.bin differ

diff --git a/bgp/parser-spi/src/main/java/org/opendaylight/protocol/bgp/parser/spi/pojo/SimpleAttributeRegistry.java b/bgp/parser-spi/src/main/java/org/opendaylight/protocol/bgp/parser/spi/pojo/SimpleAttributeRegistry.java
index 1e87b046a6d479e213ed46f180eaf9c74d874f9e..7c2eed994902984538012abe909925f5f6b84d76 100644 (file)
--- a/bgp/parser-spi/src/main/java/org/opendaylight/protocol/bgp/parser/spi/pojo/SimpleAttributeRegistry.java
+++ b/bgp/parser-spi/src/main/java/org/opendaylight/protocol/bgp/parser/spi/pojo/SimpleAttributeRegistry.java
@@ -92,7 +92,8 @@ final class SimpleAttributeRegistry implements AttributeRegistry {
     }
 
     private void addAttribute(final ByteBuf buffer, final RevisedErrorHandling errorHandling,
-            final Map<Integer, RawAttribute> attributes) throws BGPDocumentedException {
+                              final Map<Integer, RawAttribute> attributes)
+            throws BGPDocumentedException, BGPTreatAsWithdrawException {
         final BitArray flags = BitArray.valueOf(buffer.readByte());
         final int type = buffer.readUnsignedByte();
         final int len = flags.get(EXTENDED_LENGTH_BIT) ? buffer.readUnsignedShort() : buffer.readUnsignedByte();
@@ -105,6 +106,12 @@ final class SimpleAttributeRegistry implements AttributeRegistry {
             return;
         }
 
+        final int readable = buffer.readableBytes();
+        if (readable < len) {
+            throw errorHandling.reportError(BGPError.MALFORMED_ATTR_LIST,
+                "Attribute {} length {} cannot be satisfied, only {} bytes are left", type, len, readable);
+        }
+
         if (parser == null) {
             processUnrecognized(flags, type, buffer, len);
         } else {
@@ -133,8 +140,15 @@ final class SimpleAttributeRegistry implements AttributeRegistry {
             throws BGPDocumentedException, BGPParsingException {
         final RevisedErrorHandling errorHandling = RevisedErrorHandling.from(constraint);
         final Map<Integer, RawAttribute> attributes = new TreeMap<>();
+        BGPTreatAsWithdrawException withdrawCause = null;
         while (buffer.isReadable()) {
-            addAttribute(buffer, errorHandling, attributes);
+            try {
+                addAttribute(buffer, errorHandling, attributes);
+            } catch (BGPTreatAsWithdrawException e) {
+                LOG.info("Failed to completely parse attributes list.");
+                withdrawCause = e;
+                break;
+            }
         }
 
         /*
@@ -145,7 +159,6 @@ final class SimpleAttributeRegistry implements AttributeRegistry {
         // all attributes before we can decide whether we can discard attributes, or whether we need to terminate
         // the session.
         final AttributesBuilder builder = new AttributesBuilder();
-        BGPTreatAsWithdrawException withdrawCause = null;
         for (final Entry<Integer, RawAttribute> entry : attributes.entrySet()) {
             LOG.debug("Parsing attribute type {}", entry.getKey());