* PIXImporter.cpp (fixServiceObjectUsedForBothSrcAndDstPorts):

see #2265 "ASA 8.3 acl import: access-list commands using two
named objects or object-groups", see #2290 "Access lists that
include mix of service objects and inline service definitions are
not properly imported". To import access-list command that matches
both source and destination tcp/udp ports and uses object-group in
either match I should create a new service group with a collection
of TCP or UDP service objects matching all combinations of source
and destination port ranges defined by the rule. This should work
when one or both matches use object-group in combination with
inline port match.

doc/ChangeLog

@@ -1,5 +1,17 @@
 2011-04-07  vadim  <vadim@netcitadel.com>
 
+	* PIXImporter.cpp (fixServiceObjectUsedForBothSrcAndDstPorts):
+	see #2265 "ASA 8.3 acl import: access-list commands using two
+	named objects or object-groups", see #2290 "Access lists that
+	include mix of service objects and inline service definitions are
+	not properly imported". To import access-list command that matches
+	both source and destination tcp/udp ports and uses object-group in
+	either match I should create a new service group with a collection
+	of TCP or UDP service objects matching all combinations of source
+	and destination port ranges defined by the rule. This should work
+	when one or both matches use object-group in combination with
+	inline port match.
+
 	* PIXImporter.cpp (pushPolicyRule): see #2297 Added warning when
 	importer enounters access-list command that matches tcp or udp
 	ports with "neq" port operators in both source and

src/import/PIXImporter.cpp

@@ -47,6 +47,7 @@
 #include "fwbuilder/RuleElement.h"
 #include "fwbuilder/Library.h"
 #include "fwbuilder/ObjectMirror.h"
+#include "fwbuilder/TCPUDPService.h"
 
 #include "../libgui/platforms.h"
 // TODO: FWBTree needs to be refactored into an independent module
@@ -187,7 +188,6 @@ FWObject* PIXImporter::makeDstObj()
 
 FWObject* PIXImporter::makeSrvObj()
 {
-
     if (protocol=="tcp" || protocol=="udp")
     {
         if (!src_port_spec.empty() &&
@@ -231,6 +231,115 @@ void PIXImporter::fixServiceObjectUsedForSrcPorts()
     }
 }
 
+/*
+ * see #2265 and 2290. If access-list command uses object groups
+ * and/or in-line port matches for both source and destination
+ * ports, we need to create several new TCPService or UDPService
+ * objects to match all combinations of ports. However this is only
+ * necessary when at least one of them (source or destination port match)
+ * uses object-group or named object because configuration with two in-line
+ * port matches is taken care in IOSImporter::createTCPService()
+ * and IOSImporter::createUDPService()
+ */
+void PIXImporter::fixServiceObjectUsedForBothSrcAndDstPorts()
+{
+    if (protocol=="tcp" || protocol=="udp")
+    {
+        // empty port_spec means no corresponding port match (either inline or
+        // named object/object group)
+        if (src_port_spec.empty() || dst_port_spec.empty()) return;
+
+        FWObject *src_port_obj = NULL;
+        FWObject *dst_port_obj = NULL;
+        
+        if (!src_port_spec.empty() &&
+            named_objects_registry.count(src_port_spec.c_str()) > 0)
+            src_port_obj = named_objects_registry[src_port_spec.c_str()];
+
+        if (!dst_port_spec.empty() &&
+            named_objects_registry.count(dst_port_spec.c_str()) > 0)
+            dst_port_obj =  named_objects_registry[dst_port_spec.c_str()];
+
+        // if both src_port_obj and dst_port_obj are NULL, this means
+        // both port operations are in-line port matches that will be
+        // taken are of in the base class functions
+        if (src_port_obj == NULL && dst_port_obj == NULL) return;
+
+        // If only one of the two is NULL, use base class functions to
+        // fill it in from its port_op and port_spec variables
+        if (dst_port_obj == NULL)
+        {
+            src_port_spec = "";
+            src_port_op = "";
+            
+            if (protocol=="tcp") dst_port_obj = createTCPService();
+            else dst_port_obj = createUDPService();
+        }
+
+        if (src_port_obj == NULL)
+        {
+            dst_port_spec = "";
+            dst_port_op = "";
+            
+            if (protocol=="tcp") src_port_obj = createTCPService();
+            else src_port_obj = createUDPService();
+        }
+
+        // now we have service objects or groups of service objects for
+        // both source and destination port match
+
+        string group_name =
+            QString("%1 port match line %2").arg(protocol.c_str())
+            .arg(getCurrentLineNumber()).toStdString();
+
+        newObjectGroupService(group_name);
+
+        mixServiceObjects(src_port_obj, dst_port_obj, current_object_group);
+
+        src_port_spec = "";
+        dst_port_spec = group_name;
+    }
+}
+
+void PIXImporter::mixServiceObjects(FWObject *src_ports,
+                                    FWObject *dst_ports,
+                                    FWObject *service_group)
+{
+    if (Group::cast(src_ports)!=NULL)
+    {
+        for (FWObject::iterator i1=src_ports->begin(); i1!=src_ports->end(); ++i1)
+        {
+            FWObject *o1 = FWReference::getObject(*i1);
+            mixServiceObjects(o1, dst_ports, service_group);
+        }
+        return;
+    }
+
+    if (Group::cast(dst_ports)!=NULL)
+    {
+        for (FWObject::iterator i1=dst_ports->begin(); i1!=dst_ports->end(); ++i1)
+        {
+            FWObject *o1 = FWReference::getObject(*i1);
+            mixServiceObjects(src_ports, o1, service_group);
+        }
+        return;
+    }
+
+    assert(src_ports->getTypeName() == dst_ports->getTypeName());
+
+    ObjectSignature sig(error_tracker);
+    sig.type_name = src_ports->getTypeName().c_str();
+    sig.port_range_inclusive = false;
+
+    sig.src_port_range_start = TCPUDPService::cast(src_ports)->getSrcRangeStart();
+    sig.src_port_range_end = TCPUDPService::cast(src_ports)->getSrcRangeEnd();
+
+    sig.dst_port_range_start = TCPUDPService::cast(dst_ports)->getDstRangeStart();
+    sig.dst_port_range_end = TCPUDPService::cast(dst_ports)->getDstRangeEnd();
+
+    service_group->addRef(commitObject(service_maker->createObject(sig)));
+}
+
 FWObject* PIXImporter::mirrorServiceObjectRecursively(FWObject *obj)
 {
     FWObject *res = NULL;
@@ -437,6 +546,8 @@ void PIXImporter::pushPolicyRule()
     // ports but used to match source ports in the access-list command.
     fixServiceObjectUsedForSrcPorts();
 
+    fixServiceObjectUsedForBothSrcAndDstPorts();
+
     // special exception for rules with "neq" port operator in both
     // source and destination. #2297. We have decided to just issue a
     // warning at this time and let user fix the rule manually. We

src/import/PIXImporter.h

@@ -70,6 +70,9 @@ class PIXImporter : public IOSImporter
 {
     libfwbuilder::FWObject* getMirroredServiceObject(libfwbuilder::FWObject *obj);
     libfwbuilder::FWObject* mirrorServiceObjectRecursively(libfwbuilder::FWObject *obj);
+    void mixServiceObjects(libfwbuilder::FWObject *src_ports,
+                           libfwbuilder::FWObject *dst_ports,
+                           libfwbuilder::FWObject *service_group);
     
 public:
 
@@ -138,6 +141,7 @@ public:
     virtual void addLogging();
 
     void fixServiceObjectUsedForSrcPorts();
+    void fixServiceObjectUsedForBothSrcAndDstPorts();
     
     /*
      * the difference is that in PIX, we get interface label instead

src/import/objectMaker.cpp

@@ -371,14 +371,14 @@ void ObjectSignature::setIcmpCode(const QString &s)
         {
             // could not convert
             icmp_code = -1;
-//            throw ObjectMakerException(
             error_tracker->registerError(
                 QString("ICMP code '%1' is unusable").arg(s));
         }
     }
 }
 
-int ObjectSignature::portFromString(const QString &port_spec, const QString &proto,
+int ObjectSignature::portFromString(const QString &port_spec,
+                                    const QString &proto,
                                     int default_port)
 {
     QString ps = port_spec.trimmed();
@@ -388,7 +388,6 @@ int ObjectSignature::portFromString(const QString &port_spec, const QString &pro
     int port = GetServByName::getPortByName(ps, proto);
     if (port == -1)
     {
-//        throw ObjectMakerException(
         error_tracker->registerError(
             QString("%1 port name '%2' is unknown").arg(proto).arg(ps));
         port = 0;

src/unit_tests/PIXImporterTest/test_data/asa8.3-acl.output

@@ -104,12 +104,34 @@
 243: Object Group (service) test-service-4-mirror
 243: Object Group (service) test-service-1-mirror
 247: filtering rule: access list outside_in, action permit
+247: Object Group (service) tcp port match line 247
 248: filtering rule: access list outside_in, action permit
+248: Object Group (service) tcp port match line 248
 249: filtering rule: access list outside_in, action permit
+249: Object Group (service) tcp port match line 249
 250: filtering rule: access list outside_in, action permit
+250: Object Group (service) tcp port match line 250
 253: filtering rule: access list outside_in, action permit
 253: Error: Rule matches tcp or udp ports using "neq" port operator in both source and destination. This configuration is not supported by import at this time, please fix manually
-270: Interface Vlan1 ruleset inside_in direction 'in'
-271: Interface Vlan1 ruleset inside_out direction 'out'
-272: Interface Vlan2 ruleset outside_in direction 'in'
-273: Interface Vlan2 ruleset outside_out direction 'out'
+264: filtering rule: access list inside_in, action permit
+264: Object Group (service) test-service-1-mirror
+264: Object Group (service) tcp port match line 264
+265: filtering rule: access list inside_in, action permit
+265: Object Group (service) test-service-2-mirror
+265: Object Group (service) tcp port match line 265
+266: filtering rule: access list inside_in, action permit
+266: Object Group (service) test-service-1-mirror
+266: Object Group (service) tcp port match line 266
+267: filtering rule: access list inside_in, action permit
+267: Object Group (service) test-service-3-mirror
+267: Object Group (service) test-service-1-mirror
+267: Object Group (service) tcp port match line 267
+268: filtering rule: access list inside_in, action permit
+268: Object Group (service) test-service-1-mirror
+268: Object Group (service) tcp port match line 268
+269: filtering rule: access list inside_in, action permit
+269: Object Group (service) tcp port match line 269
+276: Interface Vlan1 ruleset inside_in direction 'in'
+277: Interface Vlan1 ruleset inside_out direction 'out'
+278: Interface Vlan2 ruleset outside_in direction 'in'
+279: Interface Vlan2 ruleset outside_out direction 'out'

src/unit_tests/PIXImporterTest/test_data/asa8.3-acl.test

@@ -260,7 +260,13 @@ access-list outside_in permit tcp any neq www any neq www
 
 
 ! tests for access lists using object groups for both source service and destination address
-
+!
+access-list inside_in permit tcp 192.168.1.0 255.255.255.0 object-group test-service-1 host 4.2.2.1 object-group test-service-2
+access-list inside_in permit tcp 192.168.1.0 255.255.255.0 object-group test-service-2 host 4.2.2.1 object-group test-service-1
+access-list inside_in permit tcp 192.168.1.0 255.255.255.0 object-group test-service-1 host 4.2.2.1 object-group test-service-3
+access-list inside_in permit tcp 192.168.1.0 255.255.255.0 object-group test-service-3 host 4.2.2.1 object-group test-service-1
+access-list inside_in permit tcp 192.168.1.0 255.255.255.0 object-group test-service-1 host 4.2.2.1 gt 1024
+access-list inside_in permit tcp 192.168.1.0 255.255.255.0 gt 1024 host 4.2.2.1 object-group test-service-1