Enforce outbound parameter/capability length

Capability and Optional Parameter length fields are only single-byte
entities, limiting the size of capabilities to 255 bytes without
overflow.

Rather than emitting an invalid message, catch this error before
writeout so the operator may make adjustments to ensure this limit
is not exceeded.

JIRA: BGPCEP-867
Change-Id: Idecfcfa1e991aadbdf35cd221f9312dea0f6d0c0
Signed-off-by: Robert Varga <robert.varga@pantheon.tech>

diff --git a/bgp/parser-impl/src/main/java/org/opendaylight/protocol/bgp/parser/impl/message/BGPOpenMessageParser.java b/bgp/parser-impl/src/main/java/org/opendaylight/protocol/bgp/parser/impl/message/BGPOpenMessageParser.java
index 6155c25d8f38d4121de0796d56ae661a98f2b78b..ef29013f5b55c2ba70f5495c098f4b791b9da522 100644 (file)
--- a/bgp/parser-impl/src/main/java/org/opendaylight/protocol/bgp/parser/impl/message/BGPOpenMessageParser.java
+++ b/bgp/parser-impl/src/main/java/org/opendaylight/protocol/bgp/parser/impl/message/BGPOpenMessageParser.java
@@ -7,9 +7,9 @@
  */
 package org.opendaylight.protocol.bgp.parser.impl.message;
 
+import static com.google.common.base.Preconditions.checkArgument;
 import static java.util.Objects.requireNonNull;
 
-import com.google.common.base.Preconditions;
 import io.netty.buffer.ByteBuf;
 import io.netty.buffer.ByteBufUtil;
 import io.netty.buffer.Unpooled;
@@ -70,7 +70,7 @@ public final class BGPOpenMessageParser implements MessageParser, MessageSeriali
      */
     @Override
     public void serializeMessage(final Notification msg, final ByteBuf bytes) {
-        Preconditions.checkArgument(msg instanceof Open, "Message needs to be of type Open");
+        checkArgument(msg instanceof Open, "Message needs to be of type Open, not %s", msg);
         final Open open = (Open) msg;
         final ByteBuf msgBody = Unpooled.buffer();
 
@@ -86,12 +86,22 @@ public final class BGPOpenMessageParser implements MessageParser, MessageSeriali
         msgBody.writeBytes(Ipv4Util.bytesForAddress(open.getBgpIdentifier()));
 
         final ByteBuf paramsBuffer = Unpooled.buffer();
-        if (open.getBgpParameters() != null) {
-            for (final BgpParameters param : open.getBgpParameters()) {
+        final List<BgpParameters> params = open.getBgpParameters();
+        if (params != null) {
+            for (final BgpParameters param : params) {
                 this.reg.serializeParameter(param, paramsBuffer);
             }
         }
-        msgBody.writeByte(paramsBuffer.writerIndex());
+        final int paramsLength = paramsBuffer.writerIndex();
+        if (paramsLength > 255) {
+            LOG.error("OPEN message message optional parameter length {} exceeds maximum length supported by BGP. "
+                + "Adjust advertized capabilities toward the peer to fit into limit of 255 bytes by trimming {}",
+                paramsLength, params);
+            throw new IllegalArgumentException(String.format(
+                "Cannot encode OPEN message because optional parameter length %s exceeds length field size.",
+                paramsLength));
+        }
+        msgBody.writeByte(paramsLength);
         msgBody.writeBytes(paramsBuffer);
 
         MessageUtil.formatMessage(TYPE, msgBody, bytes);
@@ -108,7 +118,7 @@ public final class BGPOpenMessageParser implements MessageParser, MessageSeriali
     @Override
     public Open parseMessageBody(final ByteBuf body, final int messageLength,
             final PeerSpecificParserConstraint constraint) throws BGPDocumentedException {
-        Preconditions.checkArgument(body != null, "Buffer cannot be null.");
+        checkArgument(body != null, "Buffer cannot be null.");
 
         if (body.readableBytes() < MIN_MSG_LENGTH) {
             throw BGPDocumentedException.badMessageLength("Open message too small.", messageLength);
@@ -142,8 +152,7 @@ public final class BGPOpenMessageParser implements MessageParser, MessageSeriali
     }
 
     private void fillParams(final ByteBuf buffer, final List<BgpParameters> params) throws BGPDocumentedException {
-        Preconditions.checkArgument(buffer != null && buffer.isReadable(),
-                "Buffer cannot be null or empty.");
+        checkArgument(buffer != null && buffer.isReadable(), "Buffer cannot be null or empty.");
         if (LOG.isTraceEnabled()) {
             LOG.trace("Started parsing of BGP parameter: {}", ByteBufUtil.hexDump(buffer));
         }

diff --git a/bgp/parser-spi/src/main/java/org/opendaylight/protocol/bgp/parser/spi/ParameterUtil.java b/bgp/parser-spi/src/main/java/org/opendaylight/protocol/bgp/parser/spi/ParameterUtil.java
index dc9fb34112df50d515bec8fe0354f2df5df62f70..82fee6d4db5790154bf4d1113765f83ffb59e217 100644 (file)
--- a/bgp/parser-spi/src/main/java/org/opendaylight/protocol/bgp/parser/spi/ParameterUtil.java
+++ b/bgp/parser-spi/src/main/java/org/opendaylight/protocol/bgp/parser/spi/ParameterUtil.java
@@ -8,6 +8,7 @@
 package org.opendaylight.protocol.bgp.parser.spi;
 
 import io.netty.buffer.ByteBuf;
+import io.netty.buffer.ByteBufUtil;
 
 /**
  * Utility class which is intended for formatting parameter.
@@ -22,10 +23,18 @@ public final class ParameterUtil {
      * @param type of the parameter
      * @param value parameter value
      * @param buffer ByteBuf where the parameter will be copied with its header
+     * @throws IllegalArgumentException if value length exceeds 255 bytes
      */
     public static void formatParameter(final int type, final ByteBuf value, final ByteBuf buffer) {
+        final int valueLength = value.writerIndex();
+        if (valueLength > 255) {
+            throw new IllegalArgumentException(String.format(
+                "Cannot encode parameter %s because value length %s exceeds parameter length field size (value %s)",
+                type, valueLength, ByteBufUtil.hexDump(value)));
+        }
+
         buffer.writeByte(type);
-        buffer.writeByte(value.writerIndex());
+        buffer.writeByte(valueLength);
         buffer.writeBytes(value);
     }
 }