Ticket #136.

Fix associated nat rules.
Now both the filter rules and the nat ones contain a associated-rule-id tag which helps link the items together.
The API to use for this is in itemid.inc.

All the issues should be solved now.

6 files changed, 88 insertions, 92 deletions

diff --git a/conf.default/config.xml b/conf.default/config.xml
index da74d68..a8b6056 100644
--- a/conf.default/config.xml
+++ b/conf.default/config.xml
@@ -441,7 +441,7 @@
 			<target></target>
 			<local-port></local-port>
 			<descr></descr>
-			<associated-filter-rule-id></associated-filter-rule-id>
+			<associated-rule-id></associated-rule-id>
 		</rule>
 		-->
 		<!--
@@ -491,7 +491,7 @@
 			<destination>
 				<any/>
 			</destination>
-			<associated-nat-rule-id></associated-nat-rule-id>
+			<associated-rule-id></associated-rule-id>
 		</rule>
 		<!-- rule syntax:
 		<rule>
@@ -793,4 +793,4 @@
 	<widgets>
 		<sequence>system_information-container:col1:show,captive_portal_status-container:col1:close,carp_status-container:col1:close,cpu_graphs-container:col1:close,gateways-container:col1:close,gmirror_status-container:col1:close,installed_packages-container:col1:close,interface_statistics-container:col1:close,interfaces-container:col2:show,ipsec-container:col2:close,load_balancer_status-container:col2:close,log-container:col2:close,picture-container:col2:close,rss-container:col2:close,services_status-container:col2:close,traffic_graphs-container:col2:close</sequence>
 	</widgets>
-</pfsense>
\ No newline at end of file
+</pfsense>
diff --git a/etc/inc/filter.inc b/etc/inc/filter.inc
index 62d42b0..a9cc9ac 100644
--- a/etc/inc/filter.inc
+++ b/etc/inc/filter.inc
@@ -1087,7 +1087,7 @@ function filter_nat_rules_generate() {
 				$natrules .= "# Unresolvable alias {$rule['target']}\n";
 				continue;		/* unresolvable alias */
 			}
-			if($rule['associated-filter-rule-id'] == "pass")
+			if($rule['associated-rule-id'] == "pass")
 				$rdrpass = "pass";
 			else
 				$rdrpass = "";	
diff --git a/etc/inc/itemid.inc b/etc/inc/itemid.inc
index f8904df..dde3762 100644
--- a/etc/inc/itemid.inc
+++ b/etc/inc/itemid.inc
@@ -45,10 +45,13 @@ function delete_id($id, &$array){
 	// Index to delete
 	$delete_index = NULL;
 
+	if (!is_array($array))
+		return false;
+
 	// 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 ){
+		if(isset($item['associated-rule-id']) && $item['associated-rule-id']==$id ){
 			$delete_index = $key;
 			break;
 		}
@@ -66,54 +69,38 @@ function delete_id($id, &$array){
 
 /****f* itemid/get_id
  * NAME
- *   get_id - Get an item with ['id'] = $id from $array by reference
+ *   get_id - Get an item id with ['associated-rule-id'] = $id from $array
  * INPUTS
- *   $id       - int: The ID to get
+ *   $id       - string: The ID to get
  *   $array    - array to get the item from
  * RESULT
- *   mixed   - The item, NULL if not found
+ *   mixed   - The id, NULL if not found
  ******/
-function &get_id($id, &$array) {
+function get_id($id, &$array) {
 	// Use $foo = &get_id('id', array('id'=>'value'));
-	// Index to delete
-	$get_index = NULL;
+
+	if (!is_array($array))
+		return false;
 
 	// 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 (isset($item['associated-rule-id']) && $item['associated-rule-id']==$id)
+			return $key;
 	}
 
-	// If we found the item, unset it
-	if( $get_index!==NULL)
-		return $array[$get_index];
-	else
-		return false;
+	return false;
 }
 
-/****f* itemid/get_next_id
+/****f* itemid/get_unique_id
  * NAME
- *   get_next_id - find the next available id from an item list
- * INPUTS
- *   $array      - array of items to get the id for
+ *   get_unique_id - get a unique identifier
  * RESULT
- *   integer     - the next available id
+ *   string     - unique id
  ******/
-function get_next_id($array){
-	// Default value
-	$next_id = 1;
-
-	// Search for IDs
-	foreach ($array as $item){
-		// If this item has an ID, and it's higher or equal to the current "next ID", use that + 1 as the next ID
-		if(isset($item['id']) && $item['id']>=$next_id ){
-			$next_id = $item['id'] + 1;
-		}
-	}
-	return $next_id;
+function get_unique_id(){
+
+	return uniqid("nat_", true);
 }
 
-?>
\ No newline at end of file
+?>
diff --git a/usr/local/www/firewall_nat.php b/usr/local/www/firewall_nat.php
index f7ddc8b..696248f 100755
--- a/usr/local/www/firewall_nat.php
+++ b/usr/local/www/firewall_nat.php
@@ -88,8 +88,8 @@ if (isset($_POST['del_x'])) {
 	    foreach ($_POST['rule'] as $rulei) {
 		$target = $rule['target'];
 			// Check for filter rule associations
-			if (isset($a_nat[$rulei]['associated-filter-rule-id'])){
-				delete_id($a_nat[$rulei]['associated-filter-rule-id'], $config['filter']['rule']);
+			if (isset($a_nat[$rulei]['associated-rule-id'])){
+				delete_id($a_nat[$rulei]['associated-rule-id'], $config['filter']['rule']);
 				
 				mark_subsystem_dirty('filter');
 			}
@@ -230,10 +230,10 @@ echo "<script type=\"text/javascript\" language=\"javascript\" src=\"/javascript
                 <tr valign="top" id="fr<?=$nnats;?>">
                   <td class="listt"><input type="checkbox" id="frc<?=$nnats;?>" name="rule[]" value="<?=$i;?>" onClick="fr_bgcolor('<?=$nnats;?>')" style="margin: 0; padding: 0; width: 15px; height: 15px;"></td>
                   <td class="listt" align="center">
-					<?php if(isset($natent['associated-filter-rule-id']) && $natent['associated-filter-rule-id']>0): ?>
-					<img src="./themes/<?= $g['theme']; ?>/images/icons/icon_chain.png" width="17" height="17" title="Firewall rule ID <?=htmlspecialchars($natent['associated-filter-rule-id']); ?> is managed with this rule" border="0">
+					<?php if(!empty($natent['associated-rule-id'])): ?>
+					<img src="./themes/<?= $g['theme']; ?>/images/icons/icon_chain.png" width="17" height="17" title="Firewall rule ID <?=htmlspecialchars($nnatid); ?> is managed with this rule" border="0">
 					<?php endif; ?>
-					<?php if($natent['associated-filter-rule-id'] == "pass"): ?>
+					<?php if($natent['associated-rule-id'] == "pass"): ?>
 					<img src="./themes/<?= $g['theme']; ?>/images/icons/icon_pass.gif" title="All traffic matching this NAT entry is passed" border="0">
 					<?php endif; ?>
 				  </td>
diff --git a/usr/local/www/firewall_nat_edit.php b/usr/local/www/firewall_nat_edit.php
index af5d6d0..9be1e4b 100755
--- a/usr/local/www/firewall_nat_edit.php
+++ b/usr/local/www/firewall_nat_edit.php
@@ -68,7 +68,7 @@ if (isset($id) && $a_nat[$id]) {
 	$pconfig['localbeginport'] = $a_nat[$id]['local-port'];
 	$pconfig['descr'] = $a_nat[$id]['descr'];
 	$pconfig['interface'] = $a_nat[$id]['interface'];
-	$pconfig['associated-filter-rule-id'] = $a_nat[$id]['associated-filter-rule-id'];
+	$pconfig['associated-rule-id'] = $a_nat[$id]['associated-rule-id'];
 	$pconfig['nosync'] = isset($a_nat[$id]['nosync']);
 	if (!$pconfig['interface'])
 		$pconfig['interface'] = "wan";
@@ -189,10 +189,10 @@ if ($_POST) {
 		$natent['local-port'] = $_POST['localbeginport'];
 		$natent['interface'] = $_POST['interface'];
 		$natent['descr'] = $_POST['descr'];
-		$natent['associated-filter-rule-id'] = $_POST['associated-filter-rule-id'];
+		$natent['associated-rule-id'] = $_POST['associated-rule-id'];
 		
 		if($_POST['filter-rule-association'] == "pass")
-			$natent['associated-filter-rule-id'] = "pass";
+			$natent['associated-rule-id'] = "pass";
 
 		if($_POST['nosync'] == "yes")
 			$natent['nosync'] = true;
@@ -200,20 +200,20 @@ if ($_POST) {
 			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 ) {
+		if ($a_nat[$id]>0 && empty($natent['associated-rule-id'])) {
 			// Delete the previous rule
-			delete_id($a_nat[$id]['associated-filter-rule-id'], $config['filter']['rule']);
+			delete_id($a_nat[$id]['associated-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 )
+		if (!empty($natent['associated-rule-id']))
 			$need_filter_rule = true;
 		// Create a rule or if we want to create a new one
-		if( $natent['associated-filter-rule-id']=='new' ) {
+		if( $natent['associated-rule-id']=='new' ) {
 			$need_filter_rule = true;
-			unset( $natent['associated-filter-rule-id'] );
+			unset( $natent['associated-rule-id'] );
 			$_POST['filter-rule-association']='add-associated';
 		}
 		// If creating a new rule, where we want to add the filter rule, associated or not
@@ -232,21 +232,23 @@ if ($_POST) {
 				$id = count($a_nat);
 		}
 
-		if ($need_filter_rule) {
+		if ($need_filter_rule == true) {
 
 			/* auto-generate a matching firewall rule */
 			$filterent = array();
-
+			unset($filterentid);
 			// 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
+			if (!empty($natent['associated-rule-id'])) {
+				$filterentid = get_id($natent['associated-rule-id'], $config['filter']['rule']);
+				if ($filterentid == false) {
+					$filterent['source']['any'] = "";
+					$filterent['associated-rule-id'] = $natent['associated-rule-id'];
+				} else
+					$filterent =& $config['filter']['rule'][$filterentid];
+			} 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'];
@@ -260,17 +262,15 @@ if ($_POST) {
 			else
 				$filterent['destination']['port'] = $dstpfrom . "-" . $dstpto;
 
-			$filterent['descr'] = "NAT " . $_POST['descr'];
 			/*
 			 * Our firewall filter description may be no longer than
 			 * 63 characters, so don't let it be.
 			 */
-			$filterent['descr'] = substr("NAT " . $_POST['descr'], 0, 59);
+			$filterent['descr'] = substr("NAT " . $_POST['descr'], 0, 62);
 
 			// 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']);
-
+				$filterent['associated-rule-id'] = $natent['associated-rule-id'] = get_unique_id();
 				$config['filter']['rule'][] = $filterent;
 			}
 
@@ -457,32 +457,33 @@ include("fbegin.inc"); ?>
 				<tr>
 					<td width="22%" valign="top" class="vncell">Filter rule association</td>
 					<td width="78%" class="vtable">
-						<select name="associated-filter-rule-id">
+						<select name="associated-rule-id">
 							<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 && ( 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
+							<option value="pass" <?php if($pconfig['associated-rule-id'] == "pass") echo " SELECTED"; ?>>Pass</option>
+							<?php 
+							if (is_array($config['filter']['rule'])) {
+							      foreach ($config['filter']['rule'] as $filter_rule) {
+								if (isset($filter_rule['associated-rule-id'])) {
+									echo "<option value=\"{$filter_rule['associated-rule-id']}\"";
+									if ($filter_rule['associated-rule-id']==$pconfig['associated-rule-id'])
+										echo " SELECTED";
+									echo ">". htmlspecialchars('Rule ' . $filter_rule['descr']) . "</option>\n";
+									
+								}
+							      }
+							}
+							if (isset($pconfig['associated-rule-id']))
+								echo "<option value=\"new\">Create new associated filter rule</option>\n";
+						echo "</select>\n";
+						if(isset($pconfig['associated-rule-id']) && is_array($config['filter']['rule'])) {
 							foreach( $config['filter']['rule'] as $index => $filter_rule ) {
-								if( $filter_rule['id']==$pconfig['associated-filter-rule-id'] ) {
-									?>
-									<a href="firewall_rules_edit.php?id=<?=$filter_rule['id'];?>">View the filter rule</a>
-									<?php
+								if( $filter_rule['assocaited-rule-id']==$pconfig['associated-rule-id'] ) {
+									echo "<a href=\"firewall_rules_edit.php?id={$filter_rule[$index]}\">View the filter rule</a>";
 									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 d799841..034a683 100755
--- a/usr/local/www/firewall_rules_edit.php
+++ b/usr/local/www/firewall_rules_edit.php
@@ -157,7 +157,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'];
+	$pconfig['associated-rule-id'] = $a_filter[$id]['associated-rule-id'];
 
 } else {
 	/* defaults */
@@ -246,10 +246,10 @@ if ($_POST) {
 
 	/* input validation */
 	$reqdfields = explode(" ", "type proto src");
-	if ( isset($a_filter[$id]['associated-nat-rule-id'])===false )
+	if ( isset($a_filter[$id]['associated-rule-id'])===false )
 		$redqfields[] = "dst";
 	$reqdfieldsn = explode(",", "Type,Protocol,Source");
-	if ( isset($a_filter[$id]['associated-nat-rule-id'])===false )
+	if ( isset($a_filter[$id]['associated-rule-id'])===false )
 		$reqdfieldsn[] = "Destination";
 
 	if($_POST['statetype'] == "modulate state" or $_POST['statetype'] == "synproxy state") {
@@ -263,7 +263,7 @@ if ($_POST) {
 		$reqdfields[] = "srcmask";
 		$reqdfieldsn[] = "Source bit count";
 	}
-	if ( isset($a_filter[$id]['associated-nat-rule-id'])===false &&
+	if ( isset($a_filter[$id]['associated-rule-id'])===false &&
 	(!(is_specialnet($_POST['dsttype']) || ($_POST['dsttype'] == "single"))) ) {
 		$reqdfields[] = "dstmask";
 		$reqdfieldsn[] = "Destination bit count";
@@ -463,9 +463,9 @@ if ($_POST) {
 		}
 
 		// If we have an associated nat rule, make sure the destination doesn't change
-		if( isset($a_filter[$id]['associated-nat-rule-id']) ) {
+		if( isset($a_filter[$id]['associated-rule-id']) ) {
 			$filterent['destination'] = $a_filter[$id]['destination'];
-			$filterent['associated-nat-rule-id'] = $a_filter[$id]['associated-nat-rule-id'];
+			$filterent['associated-rule-id'] = $a_filter[$id]['associated-rule-id'];
 		}
 
 		if (isset($id) && $a_filter[$id])
@@ -760,11 +760,19 @@ include("head.inc");
 			<td width="22%" valign="top" class="vncellreq">Destination</td>
 			<td width="78%" class="vtable">
 				<?php $dst_disabled=false; ?>
-				<?php if( isset($pconfig['associated-nat-rule-id']) ): ?>
+				<?php if( isset($pconfig['associated-rule-id']) ): ?>
 					<span class="red"><strong>NOTE: </strong></span> This is 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 />
+                                        <?php
+							if (is_array($config['nat']['rule'])) {
+                                                        	foreach( $config['nat']['rule'] as $index => $nat_rule ) {
+                                                                	if( $nat_rule['assocaited-rule-id']==$pconfig['associated-rule-id'])
+                                                                        	echo "<a href=\"firewall_nat_edit.php?id={$nat_rule[$index]}\">View the NAT rule</a>\n";
+                                                                        break;
+                                                        	}
+							}
+					?>
 					<br />
 					<?php $dst_disabled=true; ?>
 					<script type="text/javascript">