Add patch from lietu (Janne Enberg). Ticket #136

1) Multiple NAT rules can be assigned the same filter rule
-> Fixed, added assigned-nat-rule-id to filter rules to keep track of the assignment

2) when removing the link (i.e. switching to "pass" or "none", the linked rule isn't deleted (should it be? probably yes)
-> Fixed, when a NAT rule's association is removed, the filter rule is deleted. Added a "create new associated filter rule" option to the
dropdown if there is none selected.

3) The destination IP and port of linked rules can be edited in firewall_rules_edit.php and shouldn't be. Source should be editable but not
destination, since that should strictly be tied to the NAT rule.
-> Fixed, you cannot edit the destination for the filter rules that are linked to NAT rules, this has been disabled both by JavaScript and
PHP.

4) If you edit the source in a linked firewall rule, it gets overwritten when you edit the NAT rule. The NAT rule should never touch the
firewall rule source after the rule exists.
-> Fixed, previously the old rule was deleted and a new one created, now it only updates the old rule and doesn't touch the source.

Also added crosslinking from the NAT rule to the filter rule and back, so you can jump to edit the filter rule from the NAT rule and
vice-versa.

5 files changed, 150 insertions, 46 deletions

diff --git a/conf.default/config.xml b/conf.default/config.xml
index 32b4d6f..da74d68 100644
--- a/conf.default/config.xml
+++ b/conf.default/config.xml
@@ -491,6 +491,7 @@
 			<destination>
 				<any/>
 			</destination>
+			<associated-nat-rule-id></associated-nat-rule-id>
 		</rule>
 		<!-- rule syntax:
 		<rule>
diff --git a/etc/inc/itemid.inc b/etc/inc/itemid.inc
index c4747f3..f8904df 100644
--- a/etc/inc/itemid.inc
+++ b/etc/inc/itemid.inc
@@ -64,6 +64,36 @@ function delete_id($id, &$array){
 
 }
 
+/****f* itemid/get_id
+ * NAME
+ *   get_id - Get an item with ['id'] = $id from $array by reference
+ * INPUTS
+ *   $id       - int: The ID to get
+ *   $array    - array to get the item from
+ * RESULT
+ *   mixed   - The item, NULL if not found
+ ******/
+function &get_id($id, &$array) {
+	// Use $foo = &get_id('id', array('id'=>'value'));
+	// Index to delete
+	$get_index = NULL;
+
+	// Search for the item in the array
+	foreach ($array as $key => $item){
+		// If this item is the one we want to delete
+		if(isset($item['id']) && $item['id']==$id ){
+			$get_index = $key;
+			break;
+		}
+	}
+
+	// If we found the item, unset it
+	if( $get_index!==NULL)
+		return $array[$get_index];
+	else
+		return false;
+}
+
 /****f* itemid/get_next_id
  * NAME
  *   get_next_id - find the next available id from an item list
diff --git a/usr/local/www/firewall_nat_edit.php b/usr/local/www/firewall_nat_edit.php
index a1e0c9c..4f5045c 100755
--- a/usr/local/www/firewall_nat_edit.php
+++ b/usr/local/www/firewall_nat_edit.php
@@ -199,27 +199,57 @@ if ($_POST) {
 		else
 			unset($natent['nosync']);
 
+		// If we used to have an associated filter rule, but no-longer should have one
+		if( $a_nat[$id]>0 && ($natent['associated-filter-rule-id']>0)===false ) {
+			// Delete the previous rule
+			delete_id($a_nat[$id]['associated-filter-rule-id'], $config['filter']['rule']);
+			mark_subsystem_dirty('filter');
+		}
+
 		$need_filter_rule = false;
 		// Updating a rule with a filter rule associated
 		if( $natent['associated-filter-rule-id']>0 )
 			$need_filter_rule = true;
+		// Create a rule or if we want to create a new one
+		if( $natent['associated-filter-rule-id']=='new' ) {
+			$need_filter_rule = true;
+			unset( $natent['associated-filter-rule-id'] );
+			$_POST['filter-rule-association']='add-associated';
+		}
 		// If creating a new rule, where we want to add the filter rule, associated or not
-		else if( isset($_POST['filter-rule-association']) && 
-			($_POST['filter-rule-association']=='add-associated' || 
+		else if( isset($_POST['filter-rule-association']) &&
+			($_POST['filter-rule-association']=='add-associated' ||
 			$_POST['filter-rule-association']=='add-unassociated') )
 			$need_filter_rule = true;
 
-		if ($need_filter_rule) {
+		// Determine NAT entry ID now, we need it for the firewall rule
+		if (isset($id) && $a_nat[$id])
+			$a_nat[$id] = $natent;
+		else {
+			if (is_numeric($after))
+				$id = $after + 1;
+			else
+				$id = count($a_nat);
+		}
 
-			// If we had a previous rule associated with this NAT rule, delete that
-			if( $natent['associated-filter-rule-id'] > 0 )
-				delete_id($natent['associated-filter-rule-id'], $config['filter']['rule']);
+		if ($need_filter_rule) {
 
 			/* auto-generate a matching firewall rule */
 			$filterent = array();
+
+			// If a rule already exists, load it
+			if( $natent['associated-filter-rule-id'] > 0 )
+				$filterent = &get_id($natent['associated-filter-rule-id'], $config['filter']['rule']);
+			else
+				// Create the default source entry for new filter entries
+				$filterent['source']['any'] = "";
+
+			// Update associated nat rule ID
+			$filterent['associated-nat-rule-id'] = $id;
+
+			// Update interface, protocol and destination
 			$filterent['interface'] = $_POST['interface'];
 			$filterent['protocol'] = $_POST['proto'];
-			$filterent['source']['any'] = "";
 			$filterent['destination']['address'] = $_POST['localip'];
 
 			$dstpfrom = $_POST['localbeginport'];
@@ -237,19 +267,17 @@ if ($_POST) {
 			 */
 			$filterent['descr'] = substr("NAT " . $_POST['descr'], 0, 59);
 
-			// If we had a previous rule association, update this rule with that ID so we don't lose association
-			if ($natent['associated-filter-rule-id'] > 0)
-				$filterent['id'] = $natent['associated-filter-rule-id']; 
-			// If we wanted this rule to be associated, make sure the NAT entry is updated with the same ID
-			else if($_POST['filter-rule-association']=='add-associated')
+			// If this is a new rule, create an ID and add the rule
+			if( $_POST['filter-rule-association']=='add-associated' ) {
 				$natent['associated-filter-rule-id'] = $filterent['id'] = get_next_id($config['filter']['rule']);
 
-			$config['filter']['rule'][] = $filterent;
+				$config['filter']['rule'][] = $filterent;
+			}
 
 			mark_subsystem_dirty('filter');
 		}
 
-		// Update NAT entry after creating/updating the firewall rule, so we have it's rule ID if one was created
+		// Update the NAT entry now
 		if (isset($id) && $a_nat[$id])
 			$a_nat[$id] = $natent;
 		else {
@@ -433,13 +461,28 @@ include("fbegin.inc"); ?>
 							<option value="">None</option>
 							<option value="pass" <?php if($pconfig['associated-filter-rule-id'] == "pass") echo " SELECTED"; ?>>Pass</option>
 							<?php foreach ($config['filter']['rule'] as $filter_rule): ?>
-								<?php if (isset($filter_rule['id']) && $filter_rule['id']>0): ?>
+								<?php if (isset($filter_rule['id']) && $filter_rule['id']>0 && ( isset($filter_rule['associated-nat-rule-id'])===false || $filter_rule['id']==$pconfig['associated-filter-rule-id'])): ?>
 									<option value="<?php echo $filter_rule['id']; ?>"<?php if($filter_rule['id']==$pconfig['associated-filter-rule-id']) echo " SELECTED"; ?>>
 									<?php echo htmlspecialchars('Rule ' . $filter_rule['id'] . ' - ' . $filter_rule['descr']); ?>
 									</option>
 								<?php endif; ?>
 							<?php endforeach; ?>
+							<?php if ( ($pconfig['associated-filter-rule-id']>0)===false ): ?>
+								<option value="new">Create new associated filter rule</option>
+							<?php endif; ?>
 						</select>
+						<?php if($pconfig['associated-filter-rule-id']>0): ?>
+							<?php
+							foreach( $config['filter']['rule'] as $index => $filter_rule ) {
+								if( $filter_rule['id']==$pconfig['associated-filter-rule-id'] ) {
+									?>
+									<a href="firewall_rules_edit.php?id=<?=$index;?>">View the filter rule</a>
+									<?php
+									break;
+								}
+							}
+							?>
+						<?php endif; ?>
 					</td>
 				</tr>
 				<?php endif; ?>
diff --git a/usr/local/www/firewall_rules_edit.php b/usr/local/www/firewall_rules_edit.php
index 0871e82..a0ce08a 100755
--- a/usr/local/www/firewall_rules_edit.php
+++ b/usr/local/www/firewall_rules_edit.php
@@ -156,6 +156,7 @@ if (isset($id) && $a_filter[$id]) {
 
 	//schedule support
 	$pconfig['sched'] = $a_filter[$id]['sched'];
+	$pconfig['associated-nat-rule-id'] = $a_filter[$id]['associated-nat-rule-id'];
 
 } else {
 	/* defaults */
@@ -243,8 +244,12 @@ if ($_POST) {
 	}
 
 	/* input validation */
-	$reqdfields = explode(" ", "type proto src dst");
-	$reqdfieldsn = explode(",", "Type,Protocol,Source,Destination");
+	$reqdfields = explode(" ", "type proto src");
+	if ( isset($a_filter[$id]['associated-nat-rule-id'])===false )
+		$redqfields[] = "dst";
+	$reqdfieldsn = explode(",", "Type,Protocol,Source");
+	if ( isset($a_filter[$id]['associated-nat-rule-id'])===false )
+		$reqdfieldsn[] = "Destination";
 
 	if($_POST['statetype'] == "modulate state" or $_POST['statetype'] == "synproxy state") {
 		if( $_POST['proto'] != "tcp" )
@@ -257,7 +262,8 @@ if ($_POST) {
 		$reqdfields[] = "srcmask";
 		$reqdfieldsn[] = "Source bit count";
 	}
-	if (!(is_specialnet($_POST['dsttype']) || ($_POST['dsttype'] == "single"))) {
+	if ( isset($a_filter[$id]['associated-nat-rule-id'])===false &&
+	(!(is_specialnet($_POST['dsttype']) || ($_POST['dsttype'] == "single"))) ) {
 		$reqdfields[] = "dstmask";
 		$reqdfieldsn[] = "Destination bit count";
 	}
@@ -454,6 +460,12 @@ if ($_POST) {
 			$filterent['sched'] = $_POST['sched'];
 		}
 
+		// If we have an associated nat rule, make sure the destination doesn't change
+		if( isset($a_filter[$id]['associated-nat-rule-id']) ) {
+			$filterent['destination'] = $a_filter[$id]['destination'];
+			$filterent['associated-nat-rule-id'] = $a_filter[$id]['associated-nat-rule-id'];
+		}
+
 		if (isset($id) && $a_filter[$id])
 			$a_filter[$id] = $filterent;
 		else {
@@ -746,7 +758,19 @@ include("head.inc");
 		<tr>
 			<td width="22%" valign="top" class="vncellreq">Destination</td>
 			<td width="78%" class="vtable">
-				<input name="dstnot" type="checkbox" id="dstnot" value="yes" <?php if ($pconfig['dstnot']) echo "checked"; ?>>
+				<?php $dst_disabled=false; ?>
+				<?php if( isset($pconfig['associated-nat-rule-id']) ): ?>
+					<span class="red"><strong>NOTE: </strong></span> This is an associated to a NAT rule.<br />
+					You cannot edit the destination of associated filter rules.<br />
+					<br />
+					<a href="firewall_nat_edit.php?id=<?=$pconfig['associated-nat-rule-id'];?>">View the NAT rule</a><br />
+					<br />
+					<?php $dst_disabled=true; ?>
+					<script type="text/javascript">
+					dstenabled = 0;
+					</script>
+				<?php endif; ?>
+				<input<?php echo ($dst_disabled===true?' DISABLED':''); ?> name="dstnot" type="checkbox" id="dstnot" value="yes" <?php if ($pconfig['dstnot']) echo "checked"; ?>>
 				<strong>not</strong>
 					<br />
 				Use this option to invert the sense of the match.
@@ -756,7 +780,7 @@ include("head.inc");
 					<tr>
 						<td>Type:&nbsp;&nbsp;</td>
 						<td>
-							<select name="dsttype" class="formselect" onChange="typesel_change()">
+							<select<?php echo ($dst_disabled===true?' DISABLED':''); ?> name="dsttype" class="formselect" onChange="typesel_change()">
 <?php
 								$sel = is_specialnet($pconfig['dst']); ?>
 								<option value="any" <?php if ($pconfig['dst'] == "any") { echo "selected"; } ?>>any</option>
@@ -786,9 +810,9 @@ include("head.inc");
 					<tr>
 						<td>Address:&nbsp;&nbsp;</td>
 						<td>
-							<input name="dst" type="text" class="formfldalias" id="dst" size="20" value="<?php if (!is_specialnet($pconfig['dst'])) echo htmlspecialchars($pconfig['dst']);?>">
+							<input<?php echo ($dst_disabled===true?' DISABLED':''); ?> name="dst" type="text" class="formfldalias" id="dst" size="20" value="<?php if (!is_specialnet($pconfig['dst'])) echo htmlspecialchars($pconfig['dst']);?>">
 							/
-							<select name="dstmask" class="formselect" id="dstmask">
+							<select<?php echo ($dst_disabled===true?' DISABLED':''); ?> name="dstmask" class="formselect" id="dstmask">
 <?php
 							for ($i = 31; $i > 0; $i--): ?>
 								<option value="<?=$i;?>" <?php if ($i == $pconfig['dstmask']) echo "selected"; ?>><?=$i;?></option>
@@ -806,27 +830,27 @@ include("head.inc");
 					<tr>
 						<td>from:&nbsp;&nbsp;</td>
 						<td>
-							<select name="dstbeginport" class="formselect" onchange="dst_rep_change();ext_change()">
+							<select<?php echo ($dst_disabled===true?' DISABLED':''); ?> name="dstbeginport" class="formselect" onchange="dst_rep_change();ext_change()">
 								<option value="">(other)</option>
 								<option value="any" <?php $bfound = 0; if ($pconfig['dstbeginport'] == "any") { echo "selected"; $bfound = 1; } ?>>any</option>
 <?php 							foreach ($wkports as $wkport => $wkportdesc): ?>
 									<option value="<?=$wkport;?>" <?php if ($wkport == $pconfig['dstbeginport']) { echo "selected"; $bfound = 1; }?>><?=htmlspecialchars($wkportdesc);?></option>
 <?php 							endforeach; ?>
 							</select>
-							<input autocomplete='off' class="formfldalias" name="dstbeginport_cust" id="dstbeginport_cust" type="text" size="5" value="<?php if (!$bfound && $pconfig['dstbeginport']) echo $pconfig['dstbeginport']; ?>">
+							<input<?php echo ($dst_disabled===true?' DISABLED':''); ?> autocomplete='off' class="formfldalias" name="dstbeginport_cust" id="dstbeginport_cust" type="text" size="5" value="<?php if (!$bfound && $pconfig['dstbeginport']) echo $pconfig['dstbeginport']; ?>">
 						</td>
 					</tr>
 					<tr>
 						<td>to:</td>
 						<td>
-							<select name="dstendport" class="formselect" onchange="ext_change()">
+							<select<?php echo ($dst_disabled===true?' DISABLED':''); ?> name="dstendport" class="formselect" onchange="ext_change()">
 								<option value="">(other)</option>
 								<option value="any" <?php $bfound = 0; if ($pconfig['dstendport'] == "any") { echo "selected"; $bfound = 1; } ?>>any</option>
 <?php							foreach ($wkports as $wkport => $wkportdesc): ?>
 									<option value="<?=$wkport;?>" <?php if ($wkport == $pconfig['dstendport']) { echo "selected"; $bfound = 1; } ?>><?=htmlspecialchars($wkportdesc);?></option>
 <?php 							endforeach; ?>
 							</select>
-							<input autocomplete='off' class="formfldalias" name="dstendport_cust" id="dstendport_cust" type="text" size="5" value="<?php if (!$bfound && $pconfig['dstendport']) echo $pconfig['dstendport']; ?>">
+							<input<?php echo ($dst_disabled===true?' DISABLED':''); ?> autocomplete='off' class="formfldalias" name="dstendport_cust" id="dstendport_cust" type="text" size="5" value="<?php if (!$bfound && $pconfig['dstendport']) echo $pconfig['dstendport']; ?>">
 						</td>
 					</tr>
 				</table>
diff --git a/usr/local/www/javascript/firewall_rules_edit/firewall_rules_edit.js b/usr/local/www/javascript/firewall_rules_edit/firewall_rules_edit.js
index 088ebef..75bcb62 100644
--- a/usr/local/www/javascript/firewall_rules_edit/firewall_rules_edit.js
+++ b/usr/local/www/javascript/firewall_rules_edit/firewall_rules_edit.js
@@ -1,5 +1,6 @@
 <!--
 var portsenabled = 1;
+var dstenabled = 1;
 
 function ext_change() {
 	if ((document.iform.srcbeginport.selectedIndex == 0) && portsenabled) {
@@ -14,13 +15,13 @@ function ext_change() {
 		document.iform.srcendport_cust.value = "";
 		document.iform.srcendport_cust.disabled = 1;
 	}
-	if ((document.iform.dstbeginport.selectedIndex == 0) && portsenabled) {
+	if ((document.iform.dstbeginport.selectedIndex == 0) && portsenabled && dstenabled) {
 		document.iform.dstbeginport_cust.disabled = 0;
 	} else {
 		document.iform.dstbeginport_cust.value = "";
 		document.iform.dstbeginport_cust.disabled = 1;
 	}
-	if ((document.iform.dstendport.selectedIndex == 0) && portsenabled) {
+	if ((document.iform.dstendport.selectedIndex == 0) && portsenabled && dstenabled) {
 		document.iform.dstendport_cust.disabled = 0;
 	} else {
 		document.iform.dstendport_cust.value = "";
@@ -35,8 +36,10 @@ function ext_change() {
 	} else {
 		document.iform.srcbeginport.disabled = 0;
 		document.iform.srcendport.disabled = 0;
-		document.iform.dstbeginport.disabled = 0;
-		document.iform.dstendport.disabled = 0;
+		if( dstenabled ) {
+			document.iform.dstbeginport.disabled = 0;
+			document.iform.dstendport.disabled = 0;
+		}
 	}
 }
 
@@ -63,22 +66,25 @@ function typesel_change() {
 			document.iform.srcmask.disabled = 1;
 			break;
 	}
-	switch (document.iform.dsttype.selectedIndex) {
-		case 1:	/* single */
-			document.iform.dst.disabled = 0;
-			document.iform.dstmask.value = "";
-			document.iform.dstmask.disabled = 1;
-			break;
-		case 2:	/* network */
-			document.iform.dst.disabled = 0;
-			document.iform.dstmask.disabled = 0;
-			break;
-		default:
-			document.iform.dst.value = "";
-			document.iform.dst.disabled = 1;
-			document.iform.dstmask.value = "";
-			document.iform.dstmask.disabled = 1;
-			break;
+	if( dstenabled )
+	{
+		switch (document.iform.dsttype.selectedIndex) {
+			case 1:	/* single */
+				document.iform.dst.disabled = 0;
+				document.iform.dstmask.value = "";
+				document.iform.dstmask.disabled = 1;
+				break;
+			case 2:	/* network */
+				document.iform.dst.disabled = 0;
+				document.iform.dstmask.disabled = 0;
+				break;
+			default:
+				document.iform.dst.value = "";
+				document.iform.dst.disabled = 1;
+				document.iform.dstmask.value = "";
+				document.iform.dstmask.disabled = 1;
+				break;
+		}
 	}
 }