From 30ee6d2f122c658e6b393ec11301d285dee0527d Mon Sep 17 00:00:00 2001 From: Vadim Kurland Date: Thu, 7 Apr 2011 13:50:01 -0700 Subject: [PATCH] * PIXImporterNat.cpp (buildSNATRule): see #2319 "Imported nat rules with multi-line access-lists have only the first entry" --- VERSION | 2 +- VERSION.h | 2 +- doc/ChangeLog | 3 + packaging/fwbuilder-static-qt.spec | 2 +- packaging/fwbuilder.control | 2 +- packaging/fwbuilder.spec | 2 +- src/import/PIXImporterNat.cpp | 195 ++--- .../PIXImporterTest/test_data/pix7-nat.fwb | 688 ++++++++++++------ .../PIXImporterTest/test_data/pix7-nat.output | 61 +- .../PIXImporterTest/test_data/pix7-nat.test | 13 + 10 files changed, 621 insertions(+), 349 deletions(-) diff --git a/VERSION b/VERSION index 064fc4512..aa8046b12 100644 --- a/VERSION +++ b/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="3519" +BUILD_NUM="3520" VERSION="$FWB_MAJOR_VERSION.$FWB_MINOR_VERSION.$FWB_MICRO_VERSION.$BUILD_NUM" diff --git a/VERSION.h b/VERSION.h index bfefe7f54..deabaafa8 100644 --- a/VERSION.h +++ b/VERSION.h @@ -1,2 +1,2 @@ -#define VERSION "4.2.0.3519" +#define VERSION "4.2.0.3520" #define GENERATION "4.2" diff --git a/doc/ChangeLog b/doc/ChangeLog index 262583153..21965eb23 100644 --- a/doc/ChangeLog +++ b/doc/ChangeLog @@ -1,5 +1,8 @@ 2011-04-07 vadim + * PIXImporterNat.cpp (buildSNATRule): see #2319 "Imported nat + rules with multi-line access-lists have only the first entry" + * PIXImporterRun.cpp (run): see #2167 Implemented import of "names" and "name" commands in PIX/ASA configs. diff --git a/packaging/fwbuilder-static-qt.spec b/packaging/fwbuilder-static-qt.spec index 73f6fe52c..2eaaed1ff 100644 --- a/packaging/fwbuilder-static-qt.spec +++ b/packaging/fwbuilder-static-qt.spec @@ -3,7 +3,7 @@ %define name fwbuilder -%define version 4.2.0.3519 +%define version 4.2.0.3520 %define release 1 %if "%_vendor" == "MandrakeSoft" diff --git a/packaging/fwbuilder.control b/packaging/fwbuilder.control index 36e965c93..124ba5546 100644 --- a/packaging/fwbuilder.control +++ b/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: 4.2.0.3519-1 +Version: 4.2.0.3520-1 Depends: libqt4-gui (>= 4.3.0), libxml2, libxslt1.1, libsnmp | libsnmp15 Description: Firewall Builder GUI and policy compilers diff --git a/packaging/fwbuilder.spec b/packaging/fwbuilder.spec index 41a4ba42b..1c7c9da39 100644 --- a/packaging/fwbuilder.spec +++ b/packaging/fwbuilder.spec @@ -1,6 +1,6 @@ %define name fwbuilder -%define version 4.2.0.3519 +%define version 4.2.0.3520 %define release 1 %if "%_vendor" == "MandrakeSoft" diff --git a/src/import/PIXImporterNat.cpp b/src/import/PIXImporterNat.cpp index 630a95a21..0b69d7669 100644 --- a/src/import/PIXImporterNat.cpp +++ b/src/import/PIXImporterNat.cpp @@ -162,45 +162,6 @@ void PIXImporter::buildDNATRule() if (s) tdst->addRef( s ); } - if ( ! real_addr_acl.empty()) - { - UnidirectionalRuleSet *rs = all_rulesets[real_addr_acl]; - if (rs) - { - PolicyRule *policy_rule = PolicyRule::cast( - rs->ruleset->getFirstByType(PolicyRule::TYPENAME)); - - if (policy_rule) - { - - RuleElement* osrc = rule->getOSrc(); - RuleElement* osrv = rule->getOSrv(); - RuleElement* tdst = rule->getTDst(); - - /* copy objects from a policy rule into - * rule elements of a nat rule - * - * Src --> TDst - * Dst --> OSrc - * Srv --> OSrv - */ - RuleElement *re = policy_rule->getSrc(); - for (FWObject::iterator it=re->begin(); it!=re->end(); ++it) - tdst->addRef(FWReference::getObject(*it)); - - re = policy_rule->getDst(); - for (FWObject::iterator it=re->begin(); it!=re->end(); ++it) - osrc->addRef(FWReference::getObject(*it)); - - re = policy_rule->getSrv(); - for (FWObject::iterator it=re->begin(); it!=re->end(); ++it) - osrv->addRef(FWReference::getObject(*it)); - } - - rs->to_be_deleted = true; - } - } - if ( ! mapped_port_spec.empty()) { src_port_spec = ""; @@ -235,9 +196,62 @@ void PIXImporter::buildDNATRule() assert(itf_o_re!=NULL); itf_o_re->addRef(pre_intf); - // add it to the current ruleset - current_ruleset->ruleset->add(rule); - addStandardImportComment(rule, QString::fromUtf8(rule_comment.c_str())); + if ( ! real_addr_acl.empty()) + { + UnidirectionalRuleSet *rs = all_rulesets[real_addr_acl]; + if (rs) + { + for(FWObject::iterator rs_it=rs->ruleset->begin(); + rs_it!=rs->ruleset->end(); ++rs_it) + { + PolicyRule *policy_rule = PolicyRule::cast(*rs_it); + + if (policy_rule) + { + FWObjectDatabase *dbroot = getFirewallObject()->getRoot(); + NATRule *nat_rule = NATRule::cast( + dbroot->create(NATRule::TYPENAME)); + nat_rule->duplicate(rule); + + RuleElement* osrc = nat_rule->getOSrc(); + RuleElement* osrv = nat_rule->getOSrv(); + RuleElement* tdst = nat_rule->getTDst(); + + /* copy objects from a policy rule into + * rule elements of a nat rule + * + * Src --> TDst + * Dst --> OSrc + * Srv --> OSrv + */ + RuleElement *re = policy_rule->getSrc(); + FWObject::iterator it; + for (it=re->begin(); it!=re->end(); ++it) + tdst->addRef(FWReference::getObject(*it)); + + re = policy_rule->getDst(); + for (it=re->begin(); it!=re->end(); ++it) + osrc->addRef(FWReference::getObject(*it)); + + re = policy_rule->getSrv(); + for (it=re->begin(); it!=re->end(); ++it) + osrv->addRef(FWReference::getObject(*it)); + + current_ruleset->ruleset->add(nat_rule); + addStandardImportComment( + nat_rule, QString::fromUtf8(rule_comment.c_str())); + } + } + + rs->to_be_deleted = true; + } + } else + { + // add it to the current ruleset + current_ruleset->ruleset->add(rule); + addStandardImportComment(rule, QString::fromUtf8(rule_comment.c_str())); + } + } /* @@ -289,44 +303,6 @@ void PIXImporter::buildSNATRule() if (s) osrc->addRef( s ); } - if ( ! nat_acl.empty()) - { - UnidirectionalRuleSet *rs = all_rulesets[nat_acl]; - if (rs) - { - PolicyRule *policy_rule = PolicyRule::cast( - rs->ruleset->getFirstByType(PolicyRule::TYPENAME)); - - if (policy_rule) - { - RuleElement* osrc = rule->getOSrc(); - RuleElement* odst = rule->getODst(); - RuleElement* osrv = rule->getOSrv(); - - /* copy objects from a policy rule into "original" - * rule elements of a nat rule - * - * Src --> OSrc - * Dst --> ODst - * Srv --> OSrv - */ - RuleElement *re = policy_rule->getSrc(); - for (FWObject::iterator it=re->begin(); it!=re->end(); ++it) - osrc->addRef(FWReference::getObject(*it)); - - re = policy_rule->getDst(); - for (FWObject::iterator it=re->begin(); it!=re->end(); ++it) - odst->addRef(FWReference::getObject(*it)); - - re = policy_rule->getSrv(); - for (FWObject::iterator it=re->begin(); it!=re->end(); ++it) - osrv->addRef(FWReference::getObject(*it)); - } - - rs->to_be_deleted = true; - } - } - ObjectSignature sig(error_tracker); FWObject *addr = NULL; @@ -361,9 +337,62 @@ void PIXImporter::buildSNATRule() assert(itf_o_re!=NULL); itf_o_re->addRef(post_intf); - // add it to the current ruleset - current_ruleset->ruleset->add(rule); - addStandardImportComment(rule, QString::fromUtf8(rule_comment.c_str())); + if ( ! nat_acl.empty()) + { + UnidirectionalRuleSet *rs = all_rulesets[nat_acl]; + if (rs) + { + for(FWObject::iterator rs_it=rs->ruleset->begin(); + rs_it!=rs->ruleset->end(); ++rs_it) + { + PolicyRule *policy_rule = PolicyRule::cast(*rs_it); + + if (policy_rule) + { + FWObjectDatabase *dbroot = getFirewallObject()->getRoot(); + NATRule *nat_rule = NATRule::cast( + dbroot->create(NATRule::TYPENAME)); + nat_rule->duplicate(rule); + + RuleElement* osrc = nat_rule->getOSrc(); + RuleElement* odst = nat_rule->getODst(); + RuleElement* osrv = nat_rule->getOSrv(); + + /* copy objects from a policy rule into "original" + * rule elements of a nat rule + * + * Src --> OSrc + * Dst --> ODst + * Srv --> OSrv + */ + RuleElement *re = policy_rule->getSrc(); + FWObject::iterator it; + for (it=re->begin(); it!=re->end(); ++it) + osrc->addRef(FWReference::getObject(*it)); + + re = policy_rule->getDst(); + for (it=re->begin(); it!=re->end(); ++it) + odst->addRef(FWReference::getObject(*it)); + + re = policy_rule->getSrv(); + for (it=re->begin(); it!=re->end(); ++it) + osrv->addRef(FWReference::getObject(*it)); + + current_ruleset->ruleset->add(nat_rule); + addStandardImportComment( + nat_rule, QString::fromUtf8(rule_comment.c_str())); + } + } + + rs->to_be_deleted = true; + } + } else + { + // add it to the current ruleset + current_ruleset->ruleset->add(rule); + addStandardImportComment(rule, + QString::fromUtf8(rule_comment.c_str())); + } } } diff --git a/src/unit_tests/PIXImporterTest/test_data/pix7-nat.fwb b/src/unit_tests/PIXImporterTest/test_data/pix7-nat.fwb index 277221fdc..afe6999ea 100644 --- a/src/unit_tests/PIXImporterTest/test_data/pix7-nat.fwb +++ b/src/unit_tests/PIXImporterTest/test_data/pix7-nat.fwb @@ -1,6 +1,6 @@ - + @@ -442,92 +442,96 @@ - - - - - - - - - - - - + + + + + + + + + + + + + - - - - + + + + - + - + - - - - + + + + - - - - - - - - - + + + + + + + + + + + - - + + + - - - - - + + + + + - - - - - - + + + + + + - - + + - - - + + + - - - - + + + + - + - + - + @@ -536,14 +540,14 @@ - + - + - + @@ -563,14 +567,14 @@ - + - + - + @@ -581,7 +585,7 @@ - + @@ -590,14 +594,14 @@ - + - + - + @@ -608,7 +612,7 @@ - + @@ -617,14 +621,14 @@ - + - + - + @@ -635,7 +639,7 @@ - + @@ -644,16 +648,16 @@ - + - + - + - + @@ -671,16 +675,16 @@ - + - + - + - + @@ -689,7 +693,7 @@ - + @@ -698,16 +702,16 @@ - + - + - + - + @@ -716,7 +720,7 @@ - + @@ -725,16 +729,16 @@ - + - + - + - + @@ -743,7 +747,7 @@ - + @@ -752,25 +756,25 @@ - + - + - + - + - + - + @@ -779,41 +783,176 @@ - + - + - + - + - + - + - + - + - + - + - + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + @@ -827,25 +966,25 @@ - + - + - + - + - + @@ -860,41 +999,41 @@ - + - + - + - + - + - + - + - + - + - + @@ -902,35 +1041,62 @@ - + + + + + + + + + + + + + + + + + + + + + + + + + + + + - + - + - + - + - + - - + + @@ -939,26 +1105,26 @@ - + - + - + - + - + - - + + @@ -967,25 +1133,25 @@ - + - + - + - + - + - + @@ -997,22 +1163,22 @@ - + - + - + - + - + @@ -1024,28 +1190,82 @@ - + - + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + - - + + - + - + - + - + @@ -1054,18 +1274,18 @@ - + - + - + - + - + @@ -1074,18 +1294,18 @@ - + - + - + - + - + @@ -1094,18 +1314,18 @@ - + - + - + - + @@ -1115,18 +1335,18 @@ - + - + - + - + @@ -1136,18 +1356,18 @@ - + - + - + - + @@ -1157,18 +1377,18 @@ - + - + - + - + @@ -1177,7 +1397,7 @@ - + @@ -1185,10 +1405,10 @@ - + - + @@ -1198,18 +1418,18 @@ - + - + - + - + - + @@ -1219,18 +1439,18 @@ - + - + - + - + - + @@ -1240,18 +1460,18 @@ - + - + - + - + - + @@ -1261,18 +1481,18 @@ - + - + - + - + @@ -1281,7 +1501,7 @@ - + @@ -1289,10 +1509,10 @@ - + - + @@ -1302,18 +1522,18 @@ - + - + - + - + @@ -1323,18 +1543,18 @@ - + - + - + - + @@ -1344,18 +1564,18 @@ - + - + - + - + @@ -1365,18 +1585,18 @@ - + - + - + - + @@ -1388,28 +1608,28 @@ - + - + - - + + - - + + - - + + @@ -1430,7 +1650,7 @@ - - + + diff --git a/src/unit_tests/PIXImporterTest/test_data/pix7-nat.output b/src/unit_tests/PIXImporterTest/test_data/pix7-nat.output index 0d35fe67a..85a72ab2d 100644 --- a/src/unit_tests/PIXImporterTest/test_data/pix7-nat.output +++ b/src/unit_tests/PIXImporterTest/test_data/pix7-nat.output @@ -46,31 +46,38 @@ Warning: interface Ethernet6 was not imported because it is in "shutdown" mode 104: filtering rule: access list id12251X6282.0, action permit 106: filtering rule: access list id12594X2458.0, action permit 109: filtering rule: access list WEB, action permit -111: filtering rule: access list NET1, action permit -138: Global address pool: number 1, interface outside, address range interface-interface, netmask 255.255.255.255 -139: Source translation rule ("nat" command) -141: Global address pool: number 2, interface outside, address range 192.0.2.10-192.0.2.10, netmask 255.255.255.255 -142: Global address pool: number 2, interface outside, address range 192.0.2.11-192.0.2.15, netmask 255.255.255.255 -143: Global address pool: number 2, interface outside, address range 192.0.2.128-192.0.2.128, netmask 255.255.255.240 -144: Global address pool: number 2, interface dmz20, address range 10.0.0.128-10.0.0.128, netmask 255.255.255.240 -146: Source translation rule ("nat" command) -147: Source translation rule ("nat" command) -149: Source translation rule ("nat" command) -151: Destination translation rule ("static" command) -152: Destination translation rule ("static" command) -153: Destination translation rule ("static" command) -154: Destination translation rule ("static" command) -155: Destination translation rule ("static" command) -157: Destination translation rule ("static" command) -158: Destination translation rule ("static" command) -159: Destination translation rule ("static" command) +111: filtering rule: access list WEB2, action permit +112: filtering rule: access list WEB2, action permit +114: filtering rule: access list NET1, action permit +142: Global address pool: number 1, interface outside, address range interface-interface, netmask 255.255.255.255 +143: Source translation rule ("nat" command) +145: Global address pool: number 2, interface outside, address range 192.0.2.10-192.0.2.10, netmask 255.255.255.255 +146: Global address pool: number 2, interface outside, address range 192.0.2.11-192.0.2.15, netmask 255.255.255.255 +147: Global address pool: number 2, interface outside, address range 192.0.2.128-192.0.2.128, netmask 255.255.255.240 +148: Global address pool: number 2, interface dmz20, address range 10.0.0.128-10.0.0.128, netmask 255.255.255.240 +149: Global address pool: number 3, interface outside, address range 192.0.2.20-192.0.2.20, netmask 255.255.255.255 +150: Global address pool: number 3, interface outside, address range 192.0.2.30-192.0.2.31, netmask 255.255.255.255 +152: Source translation rule ("nat" command) +153: Source translation rule ("nat" command) +155: Source translation rule ("nat" command) +156: Source translation rule ("nat" command) +159: Source translation rule ("nat" command) 161: Destination translation rule ("static" command) -164: Interface Ethernet0.101 ruleset outside_in direction 'in' -165: Interface Ethernet1 ruleset inside_in direction 'in' -166: Interface Ethernet1 ruleset inside_out direction 'out' -207: Interface Ethernet1 ruleset ssh_commands_inside direction 'in' -207: filtering rule: access list ssh_commands_inside, action permit -208: Interface Ethernet1 ruleset ssh_commands_inside direction 'in' -208: filtering rule: access list ssh_commands_inside, action permit -209: Interface Ethernet0.101 ruleset ssh_commands_outside direction 'in' -209: filtering rule: access list ssh_commands_outside, action permit +162: Destination translation rule ("static" command) +163: Destination translation rule ("static" command) +164: Destination translation rule ("static" command) +165: Destination translation rule ("static" command) +167: Destination translation rule ("static" command) +168: Destination translation rule ("static" command) +169: Destination translation rule ("static" command) +171: Destination translation rule ("static" command) +174: Destination translation rule ("static" command) +177: Interface Ethernet0.101 ruleset outside_in direction 'in' +178: Interface Ethernet1 ruleset inside_in direction 'in' +179: Interface Ethernet1 ruleset inside_out direction 'out' +220: Interface Ethernet1 ruleset ssh_commands_inside direction 'in' +220: filtering rule: access list ssh_commands_inside, action permit +221: Interface Ethernet1 ruleset ssh_commands_inside direction 'in' +221: filtering rule: access list ssh_commands_inside, action permit +222: Interface Ethernet0.101 ruleset ssh_commands_outside direction 'in' +222: filtering rule: access list ssh_commands_outside, action permit diff --git a/src/unit_tests/PIXImporterTest/test_data/pix7-nat.test b/src/unit_tests/PIXImporterTest/test_data/pix7-nat.test index 08ed468cd..5f320e04c 100644 --- a/src/unit_tests/PIXImporterTest/test_data/pix7-nat.test +++ b/src/unit_tests/PIXImporterTest/test_data/pix7-nat.test @@ -108,8 +108,12 @@ access-list id12594X2458.0 permit tcp host 10.1.1.43 eq www any ! example from cisco docs, see also nat command below access-list WEB permit tcp 10.1.1.0 255.255.255.0 4.2.2.1 255.255.255.255 eq 80 +access-list WEB2 permit tcp 192.168.2.0 255.255.255.0 4.2.2.1 255.255.255.255 eq 80 +access-list WEB2 permit tcp 192.168.3.0 255.255.255.0 4.2.2.1 255.255.255.255 eq 80 + access-list NET1 permit ip host 10.1.1.20 host 4.2.2.1 + pager lines 24 logging enable logging emblem @@ -142,11 +146,17 @@ global (outside) 2 192.0.2.10 global (outside) 2 192.0.2.11-192.0.2.15 global (outside) 2 192.0.2.128 netmask 255.255.255.240 global (dmz20) 2 10.0.0.128 netmask 255.255.255.240 +global (outside) 3 192.0.2.20 +global (outside) 3 192.0.2.30-192.0.2.31 nat (inside) 2 10.1.1.1 255.255.255.255 nat (inside) 2 10.1.1.32 255.255.255.240 nat (inside) 1 access-list WEB +nat (inside) 1 access-list WEB2 + +! multiple address blocks in pool 3 and multiple lines in access list WEB2 +nat (inside) 3 access-list WEB2 static (inside,dmz20) 10.0.0.16 10.1.1.16 netmask 255.255.255.240 static (inside,dmz20) 10.0.0.100 10.1.1.100 netmask 255.255.255.255 @@ -160,6 +170,9 @@ static (inside,outside) interface access-list id12594X2458.0 0 0 static (inside,outside) 192.0.2.15 access-list NET1 +! acl WEB2 has multiple lines. Does this even make sense ? +static (inside,outside) 192.0.2.15 access-list WEB2 + access-group outside_in in interface outside access-group inside_in in interface inside