From 8f5f4b4f0e4148f6c4fdcb1526cfe523095375eb Mon Sep 17 00:00:00 2001 From: Vadim Kurland Date: Mon, 1 Feb 2010 06:39:24 +0000 Subject: [PATCH] fixes #1187 regression in compiler for PIX --- build_num | 2 +- doc/ChangeLog | 11 ++ src/cisco_lib/Helper.cpp | 10 +- src/cisco_lib/NATCompiler_pix.cpp | 153 ++++++++++++++++---- src/cisco_lib/NATCompiler_pix.h | 10 ++ src/cisco_lib/PolicyCompiler_cisco.cpp | 7 - src/cisco_lib/PolicyCompiler_cisco_acls.cpp | 38 ++++- src/cisco_lib/PolicyCompiler_pix.cpp | 14 +- test/pix/cluster-tests.fwb | 2 +- test/pix/objects-for-regression-tests.fwb | 62 +++++--- 10 files changed, 234 insertions(+), 75 deletions(-) diff --git a/build_num b/build_num index afe7d1271..12552a908 100644 --- a/build_num +++ b/build_num @@ -1 +1 @@ -#define BUILD_NUM 2477 +#define BUILD_NUM 2478 diff --git a/doc/ChangeLog b/doc/ChangeLog index 274f3cbcf..81c609cb5 100644 --- a/doc/ChangeLog +++ b/doc/ChangeLog @@ -1,5 +1,16 @@ 2010-01-31 vadim + * PolicyCompiler_cisco_acls.cpp (setInterfaceAndDirectionBySrc::processNext): + fixes #1187: "regression in compiler for PIX". Rules that have + cluster or firewall object in src or dst that expands to a bunch + of addresses that match network zones of different interfaces + should still be assigned to the interface dictated by the + combination of both src and dst. There is no need to add them to + the ACL of inetrface 1 in direction "outbound" if destination + belongs to the network zone of inetrface 2. Rule like that should + only be assigned to interface 2, direction outbound. However + this does not apply to anti-spoofing rules. + * NATCompiler_ipt.cpp (AssignInterface::processNext): fixes #1184 "compiler/GUI crash compiling cluster NAT rule when cluster and members have dynamic interface". It should be possible to have diff --git a/src/cisco_lib/Helper.cpp b/src/cisco_lib/Helper.cpp index cfdaa9ca4..afcb34df4 100644 --- a/src/cisco_lib/Helper.cpp +++ b/src/cisco_lib/Helper.cpp @@ -137,6 +137,7 @@ int Helper::findInterfaceByNetzone(const InetAddr *addr) throw(string) << " " << netzone->getName() << endl; #endif + if (netzone_id != -1) { FWObject *netzone = fw->getRoot()->findInIndex(netzone_id); @@ -156,7 +157,9 @@ int Helper::findInterfaceByNetzone(const InetAddr *addr) throw(string) } else { if (Address::cast(*j)->belongs(*addr)) + { zones[iface->getId()] = netzone; + } } } } @@ -167,7 +170,7 @@ int Helper::findInterfaceByNetzone(const InetAddr *addr) throw(string) * pick the one with smallest dimension */ int res_id = -1; - unsigned long res_dim=LONG_MAX; + unsigned long res_dim = LONG_MAX; for (map::iterator i=zones.begin(); i!=zones.end(); ++i) { int iface_id = (*i).first; @@ -176,8 +179,8 @@ int Helper::findInterfaceByNetzone(const InetAddr *addr) throw(string) if (dim<=res_dim) { - res_id=iface_id; - res_dim=dim; + res_id = iface_id; + res_dim = dim; } } @@ -190,6 +193,7 @@ int Helper::findInterfaceByNetzone(const InetAddr *addr) throw(string) if (res_id == -1) throw(string("Can not find interface with network zone that includes " "address ") + string((addr)?addr->toString():"NULL")); + return res_id; } diff --git a/src/cisco_lib/NATCompiler_pix.cpp b/src/cisco_lib/NATCompiler_pix.cpp index bd2ce9d52..b30ab9f8e 100644 --- a/src/cisco_lib/NATCompiler_pix.cpp +++ b/src/cisco_lib/NATCompiler_pix.cpp @@ -36,6 +36,7 @@ #include "fwbuilder/UDPService.h" #include "fwbuilder/Interface.h" #include "fwbuilder/IPv4.h" +#include "fwbuilder/IPv6.h" #include "fwbuilder/InetAddr.h" #include "fwbuilder/Network.h" #include "fwbuilder/Resources.h" @@ -72,6 +73,88 @@ NATCompiler_pix::NATCompiler_pix(FWObjectDatabase *_db, { } + +/* + * Do not expand interfaces in ODst and TSrc + * + */ +void NATCompiler_pix::_expand_addr_recursive_pix(Rule *rule, + FWObject *re, + FWObject *s, + list &ol) +{ + Interface *rule_iface = Interface::cast(dbcopy->findInIndex(rule->getInterfaceId())); + bool odst_or_tsrc = (re->getTypeName() == RuleElementODst::TYPENAME || + re->getTypeName() == RuleElementTSrc::TYPENAME); + + list addrlist; + + for (FWObject::iterator i1=s->begin(); i1!=s->end(); ++i1) + { + FWObject *o = FWReference::getObject(*i1); + assert(o); + + Address *addr = Address::cast(o); + + // this condition includes Host, Firewall and Interface + if (addr && !addr->hasInetAddress()) + { + addrlist.push_back(o); + continue; + } + + // IPv4, IPv6, Network, NetworkIPv6 + if (addr && addr->hasInetAddress() && MatchesAddressFamily(o)) + { + addrlist.push_back(o); + continue; + } + + if (o->getId() == FWObjectDatabase::ANY_ADDRESS_ID || + MultiAddress::cast(o)!=NULL || + Interface::cast(o) || + physAddress::cast(o)) + { + addrlist.push_back(o); + continue; + } + } + + if (addrlist.empty()) + { + if (RuleElement::cast(s)==NULL) ol.push_back(s); + } + else + { + for (list::iterator i2=addrlist.begin(); + i2!=addrlist.end(); ++i2) + { + Interface *i2itf = Interface::cast(*i2); + if (i2itf) + { + // if this is ODst or TSrc, just use interface + if (odst_or_tsrc) + { + ol.push_back(i2itf); + continue; + } + + _expand_interface(rule, i2itf, ol); + continue; + } + _expand_addr_recursive_pix(rule, re, *i2, ol); + } + } +} + + +void NATCompiler_pix::_expand_addr_recursive(Rule *rule, FWObject *re, + list &ol) +{ + _expand_addr_recursive_pix(rule, re, re, ol); +} + + void NATCompiler_pix::_expand_interface(Rule *rule, Interface *iface, std::list &ol) @@ -83,22 +166,18 @@ void NATCompiler_pix::_expand_interface(Rule *rule, return; } - FWObject *failover_group = iface->getFirstByType(FailoverClusterGroup::TYPENAME); + FailoverClusterGroup *failover_group = FailoverClusterGroup::cast( + iface->getFirstByType(FailoverClusterGroup::TYPENAME)); if (failover_group) { - for (FWObjectTypedChildIterator it = - failover_group->findByType(FWObjectReference::TYPENAME); - it != it.end(); ++it) + Interface *member_iface = + failover_group->getInterfaceForMemberFirewall(fw); + if (member_iface) { - Interface *member_iface = - Interface::cast(FWObjectReference::getObject(*it)); - assert(member_iface); - if (member_iface->isChildOf(fw)) - { - Compiler::_expand_interface(rule, member_iface, ol); - return; - } + Compiler::_expand_interface(rule, member_iface, ol); + return; } + QString err("Failover group of cluster interface '%1' (%2) " "does not include interface for the member '%3'"); abort(rule, @@ -899,6 +978,12 @@ bool NATCompiler_pix::createNATCmd::processNext() NATCompiler_pix *pix_comp=dynamic_cast(compiler); NATRule *rule=getNext(); if (rule==NULL) return false; + bool cluster_member = compiler->fw->getOptionsObject()->getBool("cluster_member"); + Cluster *cluster = NULL; + if (cluster_member) + cluster = Cluster::cast( + compiler->dbcopy->findInIndex(compiler->fw->getInt("parent_cluster_id"))); + if (rule->getRuleType()==NATRule::SNAT) { Address *osrc=compiler->getFirstOSrc(rule); assert(osrc); @@ -923,21 +1008,22 @@ bool NATCompiler_pix::createNATCmd::processNext() natcmd->nat_acl_name = pix_comp->getNATACLname(rule,""); pix_comp->registerACL(natcmd->nat_acl_name); - if (Interface::cast(tsrc)!=NULL || natcmd->t_iface->isDyn()) + if (Interface::cast(tsrc)!=NULL || natcmd->t_iface->isDyn()) + { + natcmd->type = INTERFACE; + } else { - natcmd->type=INTERFACE; - } else { if (Network::cast(tsrc)) { - natcmd->type=NETWORK_ADDRESS; + natcmd->type = NETWORK_ADDRESS; } else { - if (AddressRange::cast(tsrc)) natcmd->type=ADDRESS_RANGE; - else natcmd->type=SINGLE_ADDRESS; + if (AddressRange::cast(tsrc)) natcmd->type = ADDRESS_RANGE; + else natcmd->type = SINGLE_ADDRESS; } } natcmd->ignore_nat = natcmd->ignore_nat_and_print_acl = - natcmd->ignore_global=false; + natcmd->ignore_global = false; natcmd->use_nat_0_0 = rule->getBool("use_nat_0_0"); /* @@ -1061,6 +1147,7 @@ bool NATCompiler_pix::mergeNATCmd::processNext() /* since map nat_commands is sorted by the key, we only have to scan it * until we hit natcmd */ + if (natcmd==nc) break; const InetAddr *a1 = natcmd->t_addr->getAddressPtr(); @@ -1081,11 +1168,11 @@ bool NATCompiler_pix::mergeNATCmd::processNext() for (map::iterator i1=pix_comp->nat_commands.begin(); i1!=pix_comp->nat_commands.end(); ++i1) { - NATCmd *nc=(*i1).second; + NATCmd *nc = (*i1).second; /* since map nat_commands is sorted by the key, we only have to scan it * until we hit natcmd */ - if (natcmd==nc) break; + if (natcmd == nc) break; if (nc->ignore_nat) continue; /* using operator==(const Address &o1,const Address &o2) here */ @@ -1104,14 +1191,14 @@ bool NATCompiler_pix::mergeNATCmd::processNext() * nat rule; in this case we need to find this other rule and also * reassign it to the global pool of the rule #2. */ - natcmd->ignore_nat=true; + natcmd->ignore_nat = true; map::iterator i2; for (i2 = pix_comp->nat_commands.begin(); i2 != pix_comp->nat_commands.end(); ++i2) { NATCmd *nc2 = i2->second; if (natcmd->nat_id == nc2->nat_id) - nc2->nat_id=nc->nat_id; + nc2->nat_id = nc->nat_id; } natcmd->nat_id = nc->nat_id; } @@ -1136,7 +1223,7 @@ bool NATCompiler_pix::mergeNATCmd::processNext() if (nc->ignore_nat) continue; if (nc->use_nat_0_0) continue; - if ( natcmd->nat_id==nc->nat_id && + if ( natcmd->nat_id == nc->nat_id && natcmd->t_addr == nc->t_addr && natcmd->o_iface->getId() == nc->o_iface->getId() ) { @@ -1147,7 +1234,7 @@ bool NATCompiler_pix::mergeNATCmd::processNext() * these nat commands. We merge ACLs by assigning them the same name. */ natcmd->nat_acl_name = nc->nat_acl_name; - nc->ignore_nat_and_print_acl=true; + nc->ignore_nat_and_print_acl = true; } } } @@ -1668,15 +1755,23 @@ void NATCompiler_pix::compile() add( new classifyNATRule("determine NAT rule types")); add( new VerifyRules("verify rules" )); + // ReplaceFirewallObjectsODst, ReplaceFirewallObjectsODst and + // UseFirewallInterfaces assume there is one object in ODst, + // TSrc and TDst rule elements. This should have been assured + // by inspector VerifyRules + add( new ReplaceFirewallObjectsODst("replace fw object in ODst" )); + add( new ReplaceFirewallObjectsTSrc("replace fw object in TSrc" )); + add( new UseFirewallInterfaces( + "replace host objects with firewall's interfaces if the have the same address")); + + // ExpandMultipleAddresses acts on different rule elements + // depending on the rule type. + // Also using overloaded virtual function _expand_interface add( new ExpandMultipleAddresses("expand multiple addresses")); add( new MACFiltering( "check for MAC address filtering")); add( new ExpandAddressRanges("expand address range objects")); add( new checkForUnnumbered("check for unnumbered interfaces")); - add( new ReplaceFirewallObjectsODst("replace fw object in ODst" )); - add( new ReplaceFirewallObjectsTSrc("replace fw object in TSrc" )); - add( new UseFirewallInterfaces( - "replace host objects with firewall's interfaces if the have the same address")); add( new ConvertToAtomic("convert to atomic rules" )); add( new AssignInterface("assign rules to interfaces" )); add( new verifyInterfaces("verify interfaces assignment" )); diff --git a/src/cisco_lib/NATCompiler_pix.h b/src/cisco_lib/NATCompiler_pix.h index 524f16e49..138335fbd 100644 --- a/src/cisco_lib/NATCompiler_pix.h +++ b/src/cisco_lib/NATCompiler_pix.h @@ -101,6 +101,16 @@ namespace fwcompiler { std::string debugPrintRule(libfwbuilder::Rule *r); + void _expand_addr_recursive_pix(libfwbuilder::Rule *rule, + libfwbuilder::FWObject *re, + libfwbuilder::FWObject *s, + std::list &ol); + + + virtual void _expand_addr_recursive(libfwbuilder::Rule *rule, + libfwbuilder::FWObject *s, + std::list &ol); + /** * internal: checks if interface is a child of a cluster and calls * Compiler::_expand_interface() with a pointer to the master member diff --git a/src/cisco_lib/PolicyCompiler_cisco.cpp b/src/cisco_lib/PolicyCompiler_cisco.cpp index 31c157d33..8ff468dcb 100644 --- a/src/cisco_lib/PolicyCompiler_cisco.cpp +++ b/src/cisco_lib/PolicyCompiler_cisco.cpp @@ -41,13 +41,6 @@ #include "fwbuilder/AddressTable.h" #include -#if __GNUC__ > 3 || \ - (__GNUC__ == 3 && (__GNUC_MINOR__ > 2 || (__GNUC_MINOR__ == 2 ) ) ) || \ - _MSC_VER -# include -#else -# include -#endif #include #include #include diff --git a/src/cisco_lib/PolicyCompiler_cisco_acls.cpp b/src/cisco_lib/PolicyCompiler_cisco_acls.cpp index 91e637577..20f68ade8 100644 --- a/src/cisco_lib/PolicyCompiler_cisco_acls.cpp +++ b/src/cisco_lib/PolicyCompiler_cisco_acls.cpp @@ -40,6 +40,7 @@ #include "fwbuilder/Management.h" #include "fwbuilder/Resources.h" #include "fwbuilder/AddressTable.h" +#include "fwbuilder/Cluster.h" #include #if __GNUC__ > 3 || \ @@ -71,15 +72,26 @@ bool PolicyCompiler_cisco::setInterfaceAndDirectionBySrc::processNext() PolicyRule *rule=getNext(); if (rule==NULL) return false; Helper helper(compiler); - RuleElementSrc *srcre = rule->getSrc(); - RuleElementDst *dstre = rule->getDst(); - list intf_id_list; if (rule->getInterfaceId() == -1) { - if (rule->getDirection()==PolicyRule::Both) + bool cluster_member = compiler->fw->getOptionsObject()->getBool("cluster_member"); + Cluster *cluster = NULL; + if (cluster_member) + cluster = Cluster::cast( + compiler->dbcopy->findInIndex(compiler->fw->getInt("parent_cluster_id"))); + + RuleElementSrc *srcre = rule->getSrc(); + RuleElementDst *dstre = rule->getDst(); + Address *srcobj = compiler->getFirstSrc(rule); + + if (rule->getDirection()==PolicyRule::Both && + ! compiler->complexMatch(srcobj, compiler->fw) && + ! compiler->complexMatch(srcobj, cluster)) + { intf_id_list = helper.findInterfaceByNetzoneOrAll( srcre ); + } if (rule->getDirection()==PolicyRule::Inbound) intf_id_list = helper.getAllInterfaceIDs(); @@ -109,6 +121,7 @@ bool PolicyCompiler_cisco::setInterfaceAndDirectionBySrc::processNext() tmp_queue.push_back(rule); return true; } + tmp_queue.push_back(rule); return true; } @@ -124,14 +137,25 @@ bool PolicyCompiler_cisco::setInterfaceAndDirectionByDst::processNext() return true; } - RuleElementDst *dstre = rule->getDst(); - list intf_id_list; if (rule->getInterfaceId() == -1) { - if (rule->getDirection()==PolicyRule::Both) + bool cluster_member = compiler->fw->getOptionsObject()->getBool("cluster_member"); + Cluster *cluster = NULL; + if (cluster_member) + cluster = Cluster::cast( + compiler->dbcopy->findInIndex(compiler->fw->getInt("parent_cluster_id"))); + + RuleElementDst *dstre = rule->getDst(); + Address *dstobj = compiler->getFirstDst(rule); + + if (rule->getDirection()==PolicyRule::Both && + ! compiler->complexMatch(dstobj, compiler->fw) && + ! compiler->complexMatch(dstobj, cluster)) + { intf_id_list = helper.findInterfaceByNetzoneOrAll( dstre ); + } if (rule->getDirection()==PolicyRule::Outbound) intf_id_list = helper.getAllInterfaceIDs(); diff --git a/src/cisco_lib/PolicyCompiler_pix.cpp b/src/cisco_lib/PolicyCompiler_pix.cpp index 5d5cee484..bf88626e7 100644 --- a/src/cisco_lib/PolicyCompiler_pix.cpp +++ b/src/cisco_lib/PolicyCompiler_pix.cpp @@ -47,13 +47,6 @@ #include "fwbuilder/FailoverClusterGroup.h" #include -#if __GNUC__ > 3 || \ - (__GNUC__ == 3 && (__GNUC_MINOR__ > 2 || (__GNUC_MINOR__ == 2 ) ) ) || \ - _MSC_VER -# include -#else -# include -#endif #include #include #include @@ -257,7 +250,12 @@ bool PolicyCompiler_pix::checkVersionAndDynamicInterface::processNext() /* if service is ssh, telnet or icmp then we can use dynamic interface * even in earlier versions */ - if (ICMPService::isA(s)) return true; + if (ICMPService::isA(s)) + { + tmp_queue.push_back(rule); + return true; + } + if (TCPService::isA(s)) { if ( s->getInt("dst_range_start")==22 && diff --git a/test/pix/cluster-tests.fwb b/test/pix/cluster-tests.fwb index d95b25570..f991bf8a6 100644 --- a/test/pix/cluster-tests.fwb +++ b/test/pix/cluster-tests.fwb @@ -282,7 +282,7 @@ - + diff --git a/test/pix/objects-for-regression-tests.fwb b/test/pix/objects-for-regression-tests.fwb index 82e675ff1..3a3b40741 100644 --- a/test/pix/objects-for-regression-tests.fwb +++ b/test/pix/objects-for-regression-tests.fwb @@ -2026,7 +2026,7 @@ - + @@ -2070,7 +2070,7 @@ - + @@ -2092,7 +2092,7 @@ - + @@ -5020,7 +5020,7 @@ no sysopt nodnsalias outbound - + @@ -5346,7 +5346,31 @@ no sysopt nodnsalias outbound - + + + + + + + + + + + + + + + + + + + + + + + + + @@ -5367,7 +5391,7 @@ no sysopt nodnsalias outbound - + @@ -5388,7 +5412,7 @@ no sysopt nodnsalias outbound - + @@ -5409,7 +5433,7 @@ no sysopt nodnsalias outbound - + @@ -5430,7 +5454,7 @@ no sysopt nodnsalias outbound - + @@ -5451,7 +5475,7 @@ no sysopt nodnsalias outbound - + @@ -5472,7 +5496,7 @@ no sysopt nodnsalias outbound - + @@ -5493,7 +5517,7 @@ no sysopt nodnsalias outbound - + @@ -5514,7 +5538,7 @@ no sysopt nodnsalias outbound - + @@ -5535,7 +5559,7 @@ no sysopt nodnsalias outbound - + @@ -5556,7 +5580,7 @@ no sysopt nodnsalias outbound - + @@ -5577,7 +5601,7 @@ no sysopt nodnsalias outbound - + @@ -5598,7 +5622,7 @@ no sysopt nodnsalias outbound - + @@ -5619,7 +5643,7 @@ no sysopt nodnsalias outbound - + @@ -5640,7 +5664,7 @@ no sysopt nodnsalias outbound - +