Fixed bug that sometimes prevented port forwards ports from updating, added ability to set firewall rule destination to this firewall

Author: jaredhendrickson13

pfSense-pkg-API/files/etc/inc/api/framework/APITools.inc

@@ -348,7 +348,7 @@ function get_pfsense_if_id($interface) {
 // Check if input is valid for rule source and destination
 function is_valid_rule_addr($addr, $direction) {
     // Variables
-    $addr_types = array("any", "pppoe", "l2tp");   // Array of special src/dst types
+    $addr_types = array("any", "pppoe", "l2tp", "(self)");   // Array of special src/dst types
     $ret_val = array("valid" => true, "data" => array());
     // Check if our source values are valid
     if (is_string($addr)) {

pfSense-pkg-API/files/etc/inc/api/models/APIFirewallNATPortForwardUpdate.inc

@@ -21,6 +21,7 @@ class APIFirewallNATPortForwardUpdate extends APIModel {
     private $nat_reflection;
     private $updated_by_msg;
     private $port_required;
+    private $port_protocol;
 
     # Create our method constructor
     public function __construct() {
@@ -30,6 +31,7 @@ class APIFirewallNATPortForwardUpdate extends APIModel {
         $this->protocols = ["tcp", "udp", "tcp/udp", "icmp", "esp", "ah", "gre", "ipv6", "igmp", "pim", "ospf"];
         $this->nat_reflection = ["enable", "disable", "purenat"];
         $this->port_required = false;
+        $this->port_protocol = false;
     }
 
     public function action() {
@@ -48,6 +50,11 @@ class APIFirewallNATPortForwardUpdate extends APIModel {
             if (array_key_exists($this->initial_data["id"], $this->config["nat"]["rule"])) {
                 $this->id = $this->initial_data["id"];
                 $this->validated_data = $this->config["nat"]["rule"][$this->id];
+
+                # Check if current protocol is a port based protocol
+                if (in_array($this->validated_data["protocol"], ["tcp", "udp", "tcp/udp"])) {
+                    $this->port_protocol = true;
+                }
             } else {
                 $this->errors[] = APIResponse\get(4016);
             }
@@ -74,11 +81,15 @@ class APIFirewallNATPortForwardUpdate extends APIModel {
         if (isset($this->initial_data['protocol'])) {
             # Require protocol to be a known/supported protocol
             if (in_array($this->initial_data['protocol'], $this->protocols)) {
-                # Only require ports if updating to port protocol from non-port protocol
+                # Check if we are updating to a port based protocol
                 if (in_array($this->initial_data["protocol"], ["tcp", "udp", "tcp/udp"])) {
+                    $this->port_protocol = true;
+                    # Only require ports if updating to port protocol from non-port protocol
                     if (!in_array($this->validated_data["protocol"], ["tcp", "udp", "tcp/udp"])) {
                         $this->port_required = true;
                     }
+                } else {
+                    $this->port_protocol = false;
                 }
                 $this->validated_data["protocol"] = $this->initial_data['protocol'];
             } else {
@@ -103,7 +114,7 @@ class APIFirewallNATPortForwardUpdate extends APIModel {
 
     private function __validate_local_port() {
         # Only require a local port if the protocol requires a port
-        if ($this->port_required) {
+        if ($this->port_required or (isset($this->initial_data['local-port']) and $this->port_protocol)) {
             # Require client to pass in a local port to forward to the target
             if (isset($this->initial_data['local-port'])) {
                 # Require the port to be a valid TCP/UDP port or range
@@ -159,7 +170,7 @@ class APIFirewallNATPortForwardUpdate extends APIModel {
 
     private function __validate_srcport() {
         # Only require a source port value if our protocol requires ports
-        if ($this->port_required) {
+        if ($this->port_required or (isset($this->initial_data['srcport']) and $this->port_protocol)) {
             $this->initial_data['srcport'] = str_replace("-", ":", $this->initial_data['srcport']);
             # Require port to be a valid port or range, or be any
             if (!is_port_or_range($this->initial_data['srcport']) and $this->initial_data['srcport'] !== "any") {
@@ -174,7 +185,7 @@ class APIFirewallNATPortForwardUpdate extends APIModel {
 
     private function __validate_dstport() {
         # Only require a destination port value if our protocol requires ports
-        if ($this->port_required) {
+        if ($this->port_required or (isset($this->initial_data['dstport']) and $this->port_protocol)) {
             $this->initial_data['dstport'] = str_replace("-", ":", $this->initial_data['dstport']);
             # Require port to be a valid port or range, or be any
             if (!is_port_or_range($this->initial_data['dstport']) and $this->initial_data['dstport'] !== "any") {

pfSense-pkg-API/files/pkg-deinstall.in

@@ -12,3 +12,4 @@ fi
 
 # Restore overriden files to their original state
 /bin/mv /etc/inc/system.inc.original /etc/inc/system.inc
+echo "Restoring file overrides to their original state... done."

pfSense-pkg-API/files/pkg-install.in

@@ -5,24 +5,24 @@ fi
 
 # Make this package known to pfSense
 /usr/local/bin/php -f /etc/rc.packages %%PORTNAME%% ${2}
-echo "Creating backups of files to override...done."
+echo "Creating backups of files to override... done."
 
 # Backup original files before overriding
 /bin/cp /etc/inc/system.inc /etc/inc/system.inc.original
 
 # Check the local systems version of pfSense before assigning file overrides
 PFSENSE_VERSION=$(/bin/cat /etc/version)
-echo "Checking pfSense version...done."
+echo "Checking pfSense version... done."
 
 # Use the corresponding pfSense version's file overrides if they exist. Otherwise print warning and use default.
 if [ -d "/etc/inc/api/framework/overrides/${PFSENSE_VERSION}" ]
 then
     /bin/cp "/etc/inc/api/framework/overrides/${PFSENSE_VERSION}/system.inc" "/etc/inc/system.inc"
-    echo "Installing file overrides for ${PFSENSE_VERSION}...done."
+    echo "Installing file overrides for ${PFSENSE_VERSION}... done."
 else
     echo "WARNING: No overrides exist for ${PFSENSE_VERSION}, it may be unsupported. Using default overrides."
     /bin/cp "/etc/inc/api/framework/overrides/default/system.inc" "/etc/inc/system.inc"
-    echo "Installing default file overrides...done."
+    echo "Installing default file overrides... done."
 fi
 
 # Setup the pfsense-api command line tool

pfSense-pkg-API/files/usr/local/share/pfSense-pkg-API/manage.php

@@ -37,9 +37,9 @@ function build_endpoints() {
 
         # Print success output if file now exists, otherwise output error and exit on non-zero code
         if (!is_null($endpoint_obj->url) and is_file("/usr/local/www".$endpoint_obj->url."/index.php")) {
-            echo "Builing ".$endpoint_class." endpoint at URL \"".$endpoint_obj->url."\"... done.".PHP_EOL;
+            echo "Building ".$endpoint_class." endpoint at URL \"".$endpoint_obj->url."\"... done.".PHP_EOL;
         } else {
-            echo "Builing ".$endpoint_class." endpoint at URL \"".$endpoint_obj->url."\"... failed.".PHP_EOL;
+            echo "Building ".$endpoint_class." endpoint at URL \"".$endpoint_obj->url."\"... failed.".PHP_EOL;
             exit(1);
         }
     }

tests/test_api_v1_firewall_rule.py

@@ -40,7 +40,7 @@ class APIUnitTestFirewallRule(unit_test_framework.APIUnitTest):
             "protocol": "tcp/udp",
             "src": "172.16.77.125",
             "srcport": "8080-8081",
-            "dst": "127.0.0.1",
+            "dst": "(self)",
             "dstport": "2222-4444",
             "descr": "Updated Unit test",
             "gateway": "WAN_DHCP",