From 08ce16350788b38bf335b4f76d393102c7f838ee Mon Sep 17 00:00:00 2001 From: Marek Gradzki Date: Tue, 10 Jan 2017 10:58:48 +0100 Subject: Fixing NPE in TCP/UDP L4 rules Change-Id: Iae90f081c0add7ad9f6dd22229df683c6d395e78 Signed-off-by: Tomas Cechvala Signed-off-by: Marek Gradzki --- .../util/protocol/ProtoPreBindRuleProducer.java | 64 ++++++++++++++++------ .../protocol/ProtoPreBindRuleProducerTest.java | 28 ++++++++++ .../test/resources/rules/tcp-rule-no-flags.json | 31 +++++++++++ 3 files changed, 106 insertions(+), 17 deletions(-) create mode 100644 acl/acl-impl/src/test/resources/rules/tcp-rule-no-flags.json 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 f5de7e393..bcbd02176 100644 --- 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 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 24de2c999..eb177f10b 100644 --- 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 index 000000000..31cc854df --- /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 -- cgit 1.2.3-korg