Fixing NPE in TCP/UDP L4 rules

Change-Id: Iae90f081c0add7ad9f6dd22229df683c6d395e78
Signed-off-by: Tomas Cechvala <[email protected]>
Signed-off-by: Marek Gradzki <[email protected]>

diff --git a/acl/acl-impl/src/main/java/io/fd/hc2vpp/acl/util/protocol/ProtoPreBindRuleProducer.java b/acl/acl-impl/src/main/java/io/fd/hc2vpp/acl/util/protocol/ProtoPreBindRuleProducer.java
index f5de7e3..bcbd021 100644 (file)
--- a/acl/acl-impl/src/main/java/io/fd/hc2vpp/acl/util/protocol/ProtoPreBindRuleProducer.java
+++ b/acl/acl-impl/src/main/java/io/fd/hc2vpp/acl/util/protocol/ProtoPreBindRuleProducer.java
@@ -24,6 +24,7 @@ import io.fd.vpp.jvpp.acl.types.AclRule;
 import java.util.Optional;
 import java.util.Set;
 import javax.annotation.Nonnull;
+import javax.annotation.Nullable;
 import org.opendaylight.yang.gen.v1.urn.ietf.params.xml.ns.yang.ietf.inet.types.rev130715.PortNumber;
 import org.opendaylight.yang.gen.v1.urn.ietf.params.xml.ns.yang.ietf.packet.fields.rev160708.acl.transport.header.fields.DestinationPortRange;
 import org.opendaylight.yang.gen.v1.urn.ietf.params.xml.ns.yang.ietf.packet.fields.rev160708.acl.transport.header.fields.SourcePortRange;
@@ -52,6 +53,7 @@ public interface ProtoPreBindRuleProducer {
     int TCP_INDEX = 6;
     int UDP_INDEX = 17;
     int ICMPV6_INDEX = 58;
+    short MAX_PORT_NUMBER = (short)65535;
 
     Set<ProtocolPair> PROTOCOL_PAIRS = ImmutableSet.of(pair(Icmp.class, ICMP_INDEX), pair(Tcp.class, TCP_INDEX),
             pair(Udp.class, UDP_INDEX), pair(IcmpV6.class, ICMPV6_INDEX));
@@ -123,22 +125,56 @@ public interface ProtoPreBindRuleProducer {
         return rule;
     }
 
+    static void bindSourcePortRange(@Nonnull final AclRule rule, @Nullable final SourcePortRange sourcePortRange) {
+        // allow all ports by default:
+        rule.srcportOrIcmptypeFirst = 0;
+        rule.srcportOrIcmptypeLast = MAX_PORT_NUMBER;
+
+        if(sourcePortRange != null) {
+            // lower port is mandatory
+            rule.srcportOrIcmptypeFirst = portNumber(sourcePortRange.getLowerPort());
+
+            if (sourcePortRange.getUpperPort() != null) {
+                rule.srcportOrIcmptypeLast = portNumber(sourcePortRange.getUpperPort());
+            } else {
+                // if upper port is missing, set lower port value as end of checked range:
+                rule.srcportOrIcmptypeLast = rule.srcportOrIcmptypeFirst;
+            }
+        }
+    }
+
+    static void bindDestinationPortRange(@Nonnull final AclRule rule, @Nullable final DestinationPortRange destinationPortRange) {
+        // allow all ports by default:
+        rule.dstportOrIcmpcodeFirst = 0;
+        rule.dstportOrIcmpcodeLast = MAX_PORT_NUMBER;
+
+        if(destinationPortRange != null) {
+            // lower port is mandatory
+            rule.dstportOrIcmpcodeFirst = portNumber(destinationPortRange.getLowerPort());
+
+            if (destinationPortRange.getUpperPort() != null) {
+                rule.dstportOrIcmpcodeLast = portNumber(destinationPortRange.getUpperPort());
+            } else {
+                // if upper port is missing, set lower port value as end of checked range:
+                rule.dstportOrIcmpcodeLast = rule.dstportOrIcmpcodeFirst;
+            }
+        }
+    }
 
     static AclRule bindTcpNodes(AclRule rule, VppAce ace) {
         final VppAceNodes vppAceNodes = ace.getVppAceNodes();
         checkArgument(vppAceNodes.getIpProtocol() instanceof Tcp);
 
         final TcpNodes tcp = Tcp.class.cast(vppAceNodes.getIpProtocol()).getTcpNodes();
-        final SourcePortRange sourcePortRange = tcp.getSourcePortRange();
-        final DestinationPortRange destinationPortRange = tcp.getDestinationPortRange();
-
-        rule.srcportOrIcmptypeFirst = portNumber(sourcePortRange.getLowerPort());
-        rule.srcportOrIcmptypeLast = portNumber(sourcePortRange.getUpperPort());
-        rule.dstportOrIcmpcodeFirst = portNumber(destinationPortRange.getLowerPort());
-        rule.dstportOrIcmpcodeLast = portNumber(destinationPortRange.getUpperPort());
-        rule.tcpFlagsMask = tcp.getTcpFlagsMask().byteValue();
-        rule.tcpFlagsValue = tcp.getTcpFlagsValue().byteValue();
+        bindSourcePortRange(rule, tcp.getSourcePortRange());
+        bindDestinationPortRange(rule, tcp.getDestinationPortRange());
 
+        if(tcp.getTcpFlagsMask() != null) {
+            rule.tcpFlagsMask = tcp.getTcpFlagsMask().byteValue();
+        }
+        if(tcp.getTcpFlagsValue() != null) {
+            rule.tcpFlagsValue = tcp.getTcpFlagsValue().byteValue();
+        }
         return rule;
     }
 
@@ -147,14 +183,8 @@ public interface ProtoPreBindRuleProducer {
         checkArgument(vppAceNodes.getIpProtocol() instanceof Udp);
 
         final UdpNodes udp = Udp.class.cast(vppAceNodes.getIpProtocol()).getUdpNodes();
-        final SourcePortRange sourcePortRange = udp.getSourcePortRange();
-        final DestinationPortRange destinationPortRange = udp.getDestinationPortRange();
-
-        rule.srcportOrIcmptypeFirst = portNumber(sourcePortRange.getLowerPort());
-        rule.srcportOrIcmptypeLast = portNumber(sourcePortRange.getUpperPort());
-        rule.dstportOrIcmpcodeFirst = portNumber(destinationPortRange.getLowerPort());
-        rule.dstportOrIcmpcodeLast = portNumber(destinationPortRange.getUpperPort());
-
+        bindSourcePortRange(rule, udp.getSourcePortRange());
+        bindDestinationPortRange(rule, udp.getDestinationPortRange());
         return rule;
     }
 

diff --git a/acl/acl-impl/src/test/java/io/fd/hc2vpp/acl/util/protocol/ProtoPreBindRuleProducerTest.java b/acl/acl-impl/src/test/java/io/fd/hc2vpp/acl/util/protocol/ProtoPreBindRuleProducerTest.java
index 24de2c9..eb177f1 100644 (file)
--- a/acl/acl-impl/src/test/java/io/fd/hc2vpp/acl/util/protocol/ProtoPreBindRuleProducerTest.java
+++ b/acl/acl-impl/src/test/java/io/fd/hc2vpp/acl/util/protocol/ProtoPreBindRuleProducerTest.java
@@ -76,6 +76,34 @@ public class ProtoPreBindRuleProducerTest implements ProtoPreBindRuleProducer, A
         assertEquals(7, tcpRule.tcpFlagsValue);
     }
 
+    @Test
+    public void testTcpRuleNoFlags(@InjectTestData(resourcePath = "/rules/tcp-rule-no-flags.json") AccessLists acls) {
+        final AclRule tcpRule = createPreBindRule(extractAce(acls));
+        assertEquals(6, tcpRule.proto);
+        assertEquals(123, tcpRule.srcportOrIcmptypeFirst);
+        assertEquals(123, tcpRule.srcportOrIcmptypeLast);
+        assertEquals((short)65000, tcpRule.dstportOrIcmpcodeFirst);
+        assertEquals((short)65000, tcpRule.dstportOrIcmpcodeLast);
+        assertEquals(0, tcpRule.tcpFlagsMask);
+        assertEquals(0, tcpRule.tcpFlagsValue);
+    }
+
+    @Test
+    public void testSourcePortRangeNotGiven() {
+        AclRule rule = new AclRule();
+        ProtoPreBindRuleProducer.bindSourcePortRange(rule, null);
+        assertEquals(0, rule.srcportOrIcmptypeFirst);
+        assertEquals(MAX_PORT_NUMBER, rule.srcportOrIcmptypeLast);
+    }
+
+    @Test
+    public void testDestinationPortRangeNotGiven() {
+        AclRule rule = new AclRule();
+        ProtoPreBindRuleProducer.bindDestinationPortRange(rule, null);
+        assertEquals(0, rule.dstportOrIcmpcodeFirst);
+        assertEquals(MAX_PORT_NUMBER, rule.dstportOrIcmpcodeLast);
+    }
+
     @Test
     public void testUdpRule(@InjectTestData(resourcePath = "/rules/udp-rule.json") AccessLists acls) {
         final AclRule udpRule = createPreBindRule(extractAce(acls));

diff --git a/acl/acl-impl/src/test/resources/rules/tcp-rule-no-flags.json b/acl/acl-impl/src/test/resources/rules/tcp-rule-no-flags.json
new file mode 100644 (file)
index 0000000..31cc854
--- /dev/null
+++ b/acl/acl-impl/src/test/resources/rules/tcp-rule-no-flags.json
@@ -0,0 +1,31 @@
+{
+  "access-lists": {
+    "acl": [
+      {
+        "acl-name": "standard-acl",
+        "acl-type": "vpp-acl:vpp-acl",
+        "access-list-entries": {
+          "ace": [
+            {
+              "rule-name": "tcp-no-flags-rule",
+              "matches": {
+                "vpp-ace-nodes": {
+                  "destination-ipv4-network": "192.168.2.1/32",
+                  "source-ipv4-network": "192.168.2.2/32",
+                  "tcp-nodes": {
+                    "source-port-range": {
+                      "lower-port": "123"
+                    },
+                    "destination-port-range": {
+                      "lower-port": "65000"
+                    }
+                  }
+                }
+              }
+            }
+          ]
+        }
+      }
+    ]
+  }
+}
\ No newline at end of file