merge from fortress, new build

VERSION

@@ -7,7 +7,7 @@ FWB_MICRO_VERSION=0
 # build number is like "nano" version number. I am incrementing build
 # number during development cycle
 #
-BUILD_NUM="3565"
+BUILD_NUM="3566"
 
 VERSION="$FWB_MAJOR_VERSION.$FWB_MINOR_VERSION.$FWB_MICRO_VERSION.$BUILD_NUM"
 

VERSION.h

@@ -1,2 +1,2 @@
-#define VERSION      "5.0.0.3565"
+#define VERSION      "5.0.0.3566"
 #define GENERATION   "5.0"

doc/ChangeLog

@@ -1,3 +1,26 @@
+2011-07-20  Vadim Kurland  <vadim@netcitadel.com>
+
+	* ObjectMatcher.cpp (dispatch): removed optimization in
+	dispatch(IPv4*,void*) and dispatch(IPv6*, void*) that assumed
+	address matches a host or a firewall if it is located somewhere in
+	the subtree rooted at the firewall object. This assumption fails
+	if the address is a child of a Variable that belongs to the
+	Variables folder of this firewall. Instead, always calling
+	checkComplexMatchForSingleAddress() which uses
+	Interface::findAllInterfaces() and therefore only matches against
+	addresses that belong to the interfaces. See #2598
+
+	* PolicyCompiler_ipt.cpp (processSingleObjectNegationInRE):
+	consolidated rule processors that deal with single object negation
+	into one class. Also, taking into account Variables.
+
+	* Interface.cpp (findAllInterfaces): added more efficient way to
+	get a list of all interfaces of a firewall. This function assumes
+	interfaces are direct children of the firewall and each interface
+	may have a subinterface (one level deep). This function is faster
+	because it does not scan whole tree rooted at the firewall object
+	which might be large if firewall has lots of rules.
+
 2011-07-19  vadim  <vadim@netcitadel.com>
 
 	* NATCompiler_ipt.cpp (processNext): fixed SF bug 3371301 "Error

packaging/fwbuilder-static-qt.spec

@@ -3,7 +3,7 @@
 
 
 %define name    fwbuilder
-%define version 5.0.0.3565
+%define version 5.0.0.3566
 %define release 1
 
 %if "%_vendor" == "MandrakeSoft"

packaging/fwbuilder.control

@@ -4,6 +4,6 @@ Replaces: fwbuilder (<=4.1.1-1), fwbuilder-common, fwbuilder-bsd, fwbuilder-linu
 Priority: extra
 Section: checkinstall
 Maintainer: vadim@fwbuilder.org
-Version: 5.0.0.3565-1
+Version: 5.0.0.3566-1
 Depends: libqt4-gui (>= 4.3.0), libxml2, libxslt1.1, libsnmp | libsnmp15
 Description: Firewall Builder GUI and policy compilers

packaging/fwbuilder.spec

@@ -1,6 +1,6 @@
 
 %define name    fwbuilder
-%define version 5.0.0.3565
+%define version 5.0.0.3566
 %define release 1
 
 %if "%_vendor" == "MandrakeSoft"

src/iptlib/PolicyCompiler_ipt.cpp

@@ -1083,95 +1083,58 @@ bool PolicyCompiler_ipt::printRuleElements::processNext()
     return true;
 }
 
-bool PolicyCompiler_ipt::singleSrcNegation::processNext()
+void PolicyCompiler_ipt::SingleRENegation::processSingleObjectNegationInRE(
+    FWObject *obj, RuleElement *rel)
 {
     PolicyCompiler_ipt *ipt_comp=dynamic_cast<PolicyCompiler_ipt*>(compiler);
-    PolicyRule *rule = getNext(); if (rule==NULL) return false;
-    RuleElementSrc *srcrel = rule->getSrc();
 
-/*   ! A  B  C  ACTION  */
-    if (srcrel->getNeg() && srcrel->size()==1)
+    // We call singleSrcNegation before we replace AddressTable
+    // objects with MultiAddressRunTime objects
+    if (AddressTable::cast(obj) && AddressTable::cast(obj)->isRunTime() &&
+        ipt_comp->using_ipset)
     {
-        // We call singleSrcNegation before we replace AddressTable
-        // objects with MultiAddressRunTime objects
-        FWObject *o = FWReference::getObject(srcrel->front());
-        if (AddressTable::cast(o) && AddressTable::cast(o)->isRunTime() &&
-            ipt_comp->using_ipset)
-        {
-            srcrel->setNeg(false);
-            srcrel->setBool("single_object_negation", true);
-        }
-
-        Address *src = compiler->getFirstSrc(rule);  
-        // note: src can be NULL if object in this rule element is a group
-        // or MultiAddress
-        if (src!=NULL && src->countInetAddresses(true)==1 &&
-            !compiler->complexMatch(src, compiler->fw)) 
-        {
-            srcrel->setNeg(false);
-            srcrel->setBool("single_object_negation", true);
-        }
+        rel->setNeg(false);
+        rel->setBool("single_object_negation", true);
+        return;
     }
 
-    tmp_queue.push_back(rule);
-    return true;
-}
-
-bool PolicyCompiler_ipt::singleDstNegation::processNext()
-{
-    PolicyCompiler_ipt *ipt_comp=dynamic_cast<PolicyCompiler_ipt*>(compiler);
-    PolicyRule *rule = getNext(); if (rule==NULL) return false;
-    RuleElementDst *dstrel = rule->getDst();
-
-/*   A  ! B  C  ACTION  */
-    if (dstrel->getNeg() && dstrel->size()==1)
+    Address *src = Address::cast(obj);
+    // note: src can be NULL if object in this rule element is a group
+    // or MultiAddress
+    if (src!=NULL && src->countInetAddresses(true)==1 &&
+        !compiler->complexMatch(src, compiler->fw)) 
     {
-        // We call singleSrcNegation before we replace AddressTable
-        // objects with MultiAddressRunTime objects
-        FWObject *o = FWReference::getObject(dstrel->front());
-        if (AddressTable::cast(o) && AddressTable::cast(o)->isRunTime() &&
-            ipt_comp->using_ipset)
-        {
-            dstrel->setNeg(false);
-            dstrel->setBool("single_object_negation", true);
-        }
-
-        Address *dst = compiler->getFirstDst(rule);  
-        if (dst!=NULL && dst->countInetAddresses(true)==1 &&
-            !compiler->complexMatch(dst, compiler->fw)) 
-        {
-            dstrel->setNeg(false);
-            dstrel->setBool("single_object_negation", true);
-        }
+        rel->setNeg(false);
+        rel->setBool("single_object_negation", true);
+        return;
     }
 
-    tmp_queue.push_back(rule);
-    return true;
-}
-
-bool PolicyCompiler_ipt::singleSrvNegation::processNext()
-{
-    PolicyRule *rule=getNext(); if (rule==NULL) return false;
-
-    RuleElementSrv *srvrel=rule->getSrv();
-    Service *srv=compiler->getFirstSrv(rule); // need to make sure it is not a group
+    Service *srv = Service::cast(obj);
     // see comment in compile() where this rule processor is used for why
     // only some services can be processed here.
     if (TagService::isA(srv) || UserService::isA(srv))
     {
 /*   A  B  ! C  ACTION  */
-        if (srvrel->getNeg() && srvrel->size()==1 && srv!=NULL ) 
-        {
-            srvrel->setNeg(false);
-            srvrel->setBool("single_object_negation", true);
-        }
+        rel->setNeg(false);
+        rel->setBool("single_object_negation", true);
+    }
+}
+
+bool PolicyCompiler_ipt::SingleRENegation::processNext()
+{
+    PolicyRule *rule = getNext(); if (rule==NULL) return false;
+    RuleElement *rel = RuleElement::cast(rule->getFirstByType(type_name));
+
+/*   ! A  B  C  ACTION  */
+    if (rel->getNeg() && rel->size()==1)
+    {
+        processSingleObjectNegationInRE(FWReference::getObject(rel->front()), rel);
     }
 
     tmp_queue.push_back(rule);
     return true;
 }
 
-
 bool PolicyCompiler_ipt::SrcNegation::processNext()
 {
     PolicyCompiler_ipt *ipt_comp=dynamic_cast<PolicyCompiler_ipt*>(compiler);
@@ -4308,7 +4271,7 @@ void PolicyCompiler_ipt::compile()
  * Further correction: we CAN use single object negatiob with some types
  * of service objects, such as e.g. TagService or UserService
  */
-    add( new singleSrvNegation("negation in Srv if it holds 1 object"));
+    add( new SingleSrvNegation("negation in Srv if it holds 1 object"));
     add( new splitRuleIfSrvAnyActionReject(
              "split rule if action is reject and srv is any" ) );
     add( new SrvNegation( false, "process negation in Srv") );
@@ -4326,8 +4289,8 @@ void PolicyCompiler_ipt::compile()
              "split if action on reject is TCP reset 2"));
 
 
-    add( new singleSrcNegation("negation in Src if it holds single object"));
-    add( new singleDstNegation("negation in Dst if it holds single object"));
+    add( new SingleSrcNegation("negation in Src if it holds single object"));
+    add( new SingleDstNegation("negation in Dst if it holds single object"));
 
     add( new splitIfSrcNegAndFw("split rule if src has negation and fw"));
     add( new splitIfDstNegAndFw("split rule if dst has negation and fw"));

src/iptlib/PolicyCompiler_ipt.h

@@ -288,19 +288,40 @@ protected:
          * but takes into account AddressTable objects if we compile
          * with support for ipset module
 	 */
-        DECLARE_POLICY_RULE_PROCESSOR(singleSrcNegation);
+        class SingleRENegation : public PolicyRuleProcessor
+        {
+            std::string type_name;
+            void processSingleObjectNegationInRE(libfwbuilder::FWObject *obj,
+                                                 libfwbuilder::RuleElement *re);
+            public:
+                SingleRENegation(const std::string &name,
+                     const std::string &tn) : PolicyRuleProcessor(name)
+            {
+                type_name = tn;
+            }
+            virtual bool processNext();
+        };
 
-	/**
-	 * processes rules with negation in Dst if it holds only one object
-	 */
-        DECLARE_POLICY_RULE_PROCESSOR(singleDstNegation);
-
-	/**
-	 * processes rules with negation in Srv if it holds only one object
-	 */
-        DECLARE_POLICY_RULE_PROCESSOR(singleSrvNegation);
+        class SingleSrcNegation : public SingleRENegation
+        {
+            public:
+            SingleSrcNegation(const std::string &name) :
+                SingleRENegation(name, libfwbuilder::RuleElementSrc::TYPENAME) {}
+        };
 
+        class SingleDstNegation : public SingleRENegation
+        {
+            public:
+            SingleDstNegation(const std::string &name) :
+                SingleRENegation(name, libfwbuilder::RuleElementDst::TYPENAME) {}
+        };
 
+        class SingleSrvNegation : public SingleRENegation
+        {
+            public:
+            SingleSrvNegation(const std::string &name) :
+                SingleRENegation(name, libfwbuilder::RuleElementSrv::TYPENAME) {}
+        };
 
 	/**
 	 * processes rules with negation in Src

src/libfwbuilder/src/fwbuilder/Host.h

@@ -105,7 +105,6 @@ class Host : public Address
      */
     static FWObject* getParentHost(FWObject *obj);
 
-    
     protected:
     
     Management *mgmt;

src/libfwbuilder/src/fwbuilder/Interface.cpp

@@ -516,3 +516,19 @@ void Interface::replaceReferenceInternal(int old_id, int new_id, int &counter)
     }
 }
 
+/*
+ * finds all interfaces of the host (or firewall, since class Firewall
+ * inherits Host) without scanning whole tree rooted at this. This is
+ * more efficient than calling getByTypeDeep() when firewall has lots
+ * of rules.
+ */
+void Interface::findAllInterfaces(FWObject *obj, list<FWObject*> &interfaces)
+{
+    for (FWObjectTypedChildIterator it = obj->findByType(Interface::TYPENAME);
+         it != it.end(); ++it)
+    {
+        interfaces.push_back(*it);
+        findAllInterfaces(*it, interfaces);
+    }
+}
+

src/libfwbuilder/src/fwbuilder/Interface.h

@@ -212,6 +212,14 @@ public:
     
         virtual bool isPrimaryObject() const { return false; }
 
+        /**
+         * finds all interfaces of the host (or firewall, since class
+         * Firewall inherits Host) without scanning whole tree rooted
+         * at this. This is more efficient than calling
+         * getByTypeDeep() when firewall has lots of rules.
+         */
+        static void findAllInterfaces(FWObject *obj, std::list<FWObject*> &interfaces);
+    
     };
 
 }

src/libfwbuilder/src/fwbuilder/ObjectMatcher.cpp

@@ -182,9 +182,33 @@ bool ObjectMatcher::checkComplexMatchForSingleAddress(const InetAddr *obj1_addr,
           (recognize_multicasts && obj1_addr->isMulticast()) )
     ) return true;
 
-
     string addr_type = (ipv6) ? IPv6::TYPENAME : IPv4::TYPENAME;
-    list<FWObject*> all_addresses = obj2->getByTypeDeep(addr_type);
+
+    list<FWObject*> all_addresses;
+
+    if (Host::cast(obj2))
+    {
+        /*
+         * note that compilers add copies of rules to the same firewall object
+         * (temp_ruleset object) which means the tree rooted at the firewall
+         * can be large, so searching for all interfaces using getByTypeDeep()
+         * may cause scanning very large tree.
+         */
+        list<FWObject*> all_interfaces;
+        Interface::findAllInterfaces(obj2, all_interfaces);
+
+        for (list<FWObject*>::iterator it = all_interfaces.begin();
+             it != all_interfaces.end(); ++it)
+        {
+            list<FWObject*> intf_addresses = (*it)->getByTypeDeep(addr_type);
+            all_addresses.splice(all_addresses.end(), intf_addresses);
+        }
+
+    } else
+    {
+        all_addresses = obj2->getByTypeDeep(addr_type);
+    }
+
     for (list<FWObject*>::iterator it = all_addresses.begin();
          it != all_addresses.end(); ++it)
     {
@@ -198,7 +222,7 @@ bool ObjectMatcher::checkComplexMatchForSingleAddress(const InetAddr *obj1_addr,
         } else
         {
             if (matchRHS(obj1_addr, rhs_addr) == 0) return true;
-    }
+        }
     }
     return false;
 }
@@ -302,20 +326,12 @@ void* ObjectMatcher::dispatch(NetworkIPv6 *obj1, void *_obj2)
 void* ObjectMatcher::dispatch(IPv4 *obj1, void *_obj2)
 {
     FWObject *obj2 = (FWObject*)(_obj2);
-    FWObject *p=obj1;
-    while ( (p=p->getParent())!=NULL)
-        if (p->getId()==obj2->getId()) return obj1;
-
     return checkComplexMatchForSingleAddress(obj1, obj2) ? obj1 : NULL;
 }
 
 void* ObjectMatcher::dispatch(IPv6 *obj1, void *_obj2)
 {
     FWObject *obj2 = (FWObject*)(_obj2);
-    FWObject *p=obj1;
-    while ( (p=p->getParent())!=NULL)
-        if (p->getId()==obj2->getId()) return obj1;
-
     return checkComplexMatchForSingleAddress(obj1, obj2) ? obj1 : NULL;
 }
 
@@ -433,9 +449,13 @@ void* ObjectMatcher::dispatch(Host *obj1, void *_obj2)
  *  match only if all interfaces of obj1 match obj2
  */
     bool res = true;
-    list<FWObject*> l = obj1->getByTypeDeep(Interface::TYPENAME);
-    for (list<FWObject*>::iterator it = l.begin(); it!=l.end(); ++it)
+    list<FWObject*> all_interfaces;
+    Interface::findAllInterfaces(obj1, all_interfaces);
+    for (list<FWObject*>::iterator it = all_interfaces.begin();
+         it != all_interfaces.end(); ++it)
+    {
         res &= checkComplexMatchForSingleAddress(Interface::cast(*it), obj2);
+    }
     return res ? obj1 : NULL;
 }
 
@@ -459,11 +479,7 @@ void* ObjectMatcher::dispatch(Firewall *obj1, void *_obj2)
 /*
  *  match only if all interfaces of obj1 match obj2
  */
-    bool res = true;
-    list<FWObject*> l = obj1->getByTypeDeep(Interface::TYPENAME);
-    for (list<FWObject*>::iterator it = l.begin(); it!=l.end(); ++it)
-        res &= checkComplexMatchForSingleAddress(Interface::cast(*it), obj2);    
-    return res ? obj1 : NULL;
+    return dispatch(static_cast<Host*>(obj1), obj2);
 }
 
 void* ObjectMatcher::dispatch(Cluster *obj1, void *_obj2)
@@ -480,10 +496,6 @@ void* ObjectMatcher::dispatch(Cluster *obj1, void *_obj2)
 /*
  *  match only if all interfaces of obj1 match obj2
  */
-    bool res = true;
-    list<FWObject*> l = obj1->getByTypeDeep(Interface::TYPENAME);
-    for (list<FWObject*>::iterator it = l.begin(); it!=l.end(); ++it)
-        res &= checkComplexMatchForSingleAddress(Interface::cast(*it), obj2);
-    return res ? obj1 : NULL;
+    return dispatch(static_cast<Host*>(obj1), obj2);
 }
 

src/libfwbuilder/src/fwbuilder/ObjectMatcher.h

@@ -37,7 +37,8 @@
 namespace libfwbuilder
 {
 
-    class ObjectMatcher : public Dispatch {
+    class ObjectMatcher : public Dispatch
+    {
 
 public:
         typedef enum {EXACT, PARTIAL} address_range_match;

src/libfwbuilder/src/fwcompiler/Compiler.cpp

@@ -72,6 +72,7 @@ using namespace std;
 int Compiler::prolog() 
 {
     temp = new Group();
+    temp->setName("Temp Group");
     fw->add(temp, false);
     return 0;
 }