Merge pull request #765 from jaredhendrickson13/next_patch

v2.6.3 Fixes

Author: jaredhendrickson13

pfSense-pkg-RESTAPI/files/usr/local/pkg/RESTAPI/Fields/InterfaceField.inc

@@ -219,7 +219,8 @@ class InterfaceField extends StringField {
             foreach ($virtual_ips as $virtual_ip) {
                 # Only include CARP virtual iPs with unique IDs
                 if ($virtual_ip['mode'] === 'carp' and $virtual_ip['uniqid']) {
-                    $choices[$virtual_ip['uniqid']] = $virtual_ip['uniqid'];
+                    $uniqid = '_vip' . $virtual_ip['uniqid'];
+                    $choices[$uniqid] = $uniqid;
                 }
             }
         }

pfSense-pkg-RESTAPI/files/usr/local/pkg/RESTAPI/Models/IPsecPhase1.inc

@@ -94,6 +94,7 @@ class IPsecPhase1 extends Model {
         );
         $this->interface = new InterfaceField(
             required: true,
+            allow_carp_interface: true,
             help_text: 'The interface for the local endpoint of this phase 1 entry. This should be an interface ' .
                 'that is reachable by the remote peer.',
         );

pfSense-pkg-RESTAPI/files/usr/local/pkg/RESTAPI/Models/OpenVPNClientExport.inc

@@ -257,7 +257,7 @@ class OpenVPNClientExport extends Model {
         return openvpn_client_export_config(
             srvid: $this->server->value,
             usrid: $this->username->value ? $this->username->get_related_model()->id : null,
-            crtid: $this->certref->value ? $this->certref->get_related_model()->id : null,
+            crtid: $this->locate_crtid(),
             useaddr: $this->useaddr_hostname->value ?? $this->useaddr->value,
             verifyservercn: $this->verifyservercn->value,
             blockoutsidedns: $this->blockoutsidedns->value,
@@ -291,7 +291,7 @@ class OpenVPNClientExport extends Model {
         return openvpn_client_export_installer(
             srvid: $this->server->value,
             usrid: $this->username->value ? $this->username->get_related_model()->id : null,
-            crtid: $this->certref->value ? $this->certref->get_related_model()->id : null,
+            crtid: $this->locate_crtid(),
             useaddr: $this->useaddr_hostnam->value ?? $this->useaddr->value,
             verifyservercn: $this->verifyservercn->value,
             blockoutsidedns: $this->blockoutsidedns->value,
@@ -319,7 +319,7 @@ class OpenVPNClientExport extends Model {
         return viscosity_openvpn_client_config_exporter(
             srvid: $this->server->value,
             usrid: $this->username->value ? $this->username->get_related_model()->id : null,
-            crtid: $this->certref->value ? $this->certref->get_related_model()->id : null,
+            crtid: $this->locate_crtid(),
             useaddr: $this->useaddr_hostname->value ?? $this->useaddr->value,
             verifyservercn: $this->verifyservercn->value,
             blockoutsidedns: $this->blockoutsidedns->value,
@@ -336,6 +336,42 @@ class OpenVPNClientExport extends Model {
         );
     }
 
+    /**
+     * Obtains the crtid value pfSense expects for a given certref given the current OpenVPN server and username.
+     * This is necessary because pfSense's openvpn_client_export_validate_config function sometimes expects
+     * crtid to be the index of the 'cert' config path, and sometimes expects it to be the index of the
+     * 'system/user/{$usrid}/cert' config path.
+     * @returns int|null The crtid value pfSense expects for a given certref, or null if no certref is set.
+     */
+    public function locate_crtid(): int|null {
+        # If no certref is set, return null
+        if (!$this->certref->value) {
+            return null;
+        }
+
+        # Load related models
+        $ovpnsrv = $this->server->get_related_model();
+        $user = $this->username->get_related_model();
+        $cert = $this->certref->get_related_model();
+
+        # If a username was provided, the server is using server_tls_user and the local database, pfSense expects
+        # the crtid to be the index of the 'system/user/{$usrid}/cert' config path not the 'cert' config path.
+        if (
+            $user and
+            $ovpnsrv->mode->value === 'server_tls_user' and
+            $ovpnsrv->authmode->value === ['Local Database']
+        ) {
+            foreach ($user->cert->value as $idx => $user_cert) {
+                if ($user_cert === $cert->refid->value) {
+                    return $idx;
+                }
+            }
+        }
+
+        # Otherwise, just return the index of the certificate directly
+        return $cert->id;
+    }
+
     /**
      * Obtains the proxy configuration array expected by pfSense functions.
      * @return array the proxy configuration array expected by pfSense functions

pfSense-pkg-RESTAPI/files/usr/local/pkg/RESTAPI/Models/RESTAPISettings.inc

@@ -235,7 +235,7 @@ class RESTAPISettings extends Model {
 
         # Format choices so they include the class's verbose name as the choice's verbose name
         foreach (get_classes_from_namespace(namespace: '\\RESTAPI\\Models\\') as $model_class) {
-            $model = new $model_class();
+            $model = new $model_class(skip_init: true);
             foreach ($model->get_fields() as $field) {
                 if ($model->$field->sensitive) {
                     $choices[$model->get_class_shortname() . ':' . $field] =

pfSense-pkg-RESTAPI/files/usr/local/pkg/RESTAPI/Tests/APIFieldsInterfaceFieldTestCase.inc

@@ -5,6 +5,7 @@ namespace RESTAPI\Tests;
 use RESTAPI\Core\TestCase;
 use RESTAPI\Fields\InterfaceField;
 use RESTAPI\Models\RESTAPISettings;
+use RESTAPI\Models\VirtualIP;
 
 class APIFieldsInterfaceFieldTestCase extends TestCase {
     # TODO: Needs Tests to ensure CARP interfaces and interface groups become available choices when the
@@ -78,6 +79,30 @@ class APIFieldsInterfaceFieldTestCase extends TestCase {
         $this->assert_is_true(in_array('openvpn', $test_field->choices));
     }
 
+    /**
+     * Checks that the InterfaceField's `allow_carp_interface` parameter allows CARP interfaces to become choices.
+     */
+    public function test_get_interface_choices_with_allow_carp_interface(): void {
+        # First, create a carp virtual IP to test with
+        $vip = new VirtualIP(
+            interface: 'lan',
+            mode: 'carp',
+            subnet: '127.0.0.99',
+            subnet_bits: 32,
+            vhid: 99,
+            password: 'test',
+        );
+        $vip->create();
+
+        # Ensure the virtual IP is now a valid choice when `allow_carp_interface` is enabled
+        $test_field = new InterfaceField(allow_carp_interface: true);
+        $test_field->set_choices_from_callable();
+        $this->assert_is_true(in_array("_vip{$vip->uniqid->value}", $test_field->choices));
+
+        # Cleanup
+        $vip->delete();
+    }
+
     /**
      * Checks that the InterfaceField's to_internal() method correctly translates representation values to their
      * internal values.

pfSense-pkg-RESTAPI/files/usr/local/pkg/RESTAPI/Tests/APIModelsOpenVPNClientExportTestCase.inc

@@ -441,4 +441,38 @@ class APIModelsOpenVPNClientExportTestCase extends TestCase {
         $this->assert_is_true(!ctype_print($export->binary_data->value));
         $this->assert_is_false(file_exists("/tmp/{$export->filename->value}"));
     }
+
+    /**
+     * Ensures we can exporta config for a server using the 'server_tls_user' mode, a user cert, and local database
+     * auth. This is a regression test for issue #756. This is a necessary test distinction as pfSense will change the
+     * meaning of the crtid passed into certain pfSense functions based on the server mode and authmode.
+     */
+    public function test_create_with_server_tls_user_and_local_database(): void {
+        # First, update the OpenVPNServer to use the 'server_tls_user' mode and local database auth
+        $this->ovpns->mode->value = 'server_tls_user';
+        $this->ovpns->authmode->value = ['Local Database'];
+        $this->ovpns->update();
+
+        # Setup the export
+        $export = new OpenVPNClientExport(
+            id: $this->ovpnce->id,
+            type: 'confinline',
+            username: $this->user->name->value,
+            certref: $this->user_cert->refid->value,
+        );
+
+        # Ensure the certref's object ID does NOT match the determined crtid. These should not match because
+        # pfSense now refers to the crtid as the index of the cert in the system/user config, NOT the cert config
+        # like usual.
+        $this->assert_not_equals($this->user_cert->refid->get_related_model()->id, $export->locate_crtid());
+
+        # Ensure we can complete the export as intended and that the embedded cert is correct
+        $export->create();
+        $this->assert_is_not_empty($export->binary_data->value);
+        $this->assert_str_contains($export->binary_data->value, $this->user_cert->crt->value);
+
+        # Reset the server mode and authmode for other tests
+        $this->ovpns->mode->value = 'server_tls';
+        $this->ovpns->update();
+    }
 }

pfSense-pkg-RESTAPI/files/usr/local/pkg/RESTAPI/Tests/APIModelsVirtualIPTestCase.inc

@@ -283,4 +283,47 @@ class APIModelsVirtualIPTestCase extends TestCase {
         # Clean up the VIP we created
         $vip->delete();
     }
+
+    /**
+     * Ensures we can create an IP alias virtual IP nested under a parent CARP virtual IP.
+     */
+    public function test_nested_vip_under_carp_vip(): void {
+        # Create a CARP virtual IP to test with
+        $carp_vip = new VirtualIP(
+            interface: 'lan',
+            mode: 'carp',
+            subnet: '127.0.0.105',
+            subnet_bits: 32,
+            vhid: 105,
+            password: 'test',
+            async: false,
+        );
+        $carp_vip->create();
+
+        # Create a new IP alias virtual IP that uses the CARP virtual IP as its interface
+        $child_vip = new VirtualIP(
+            interface: "_vip{$carp_vip->uniqid->value}",
+            mode: 'ipalias',
+            subnet: '127.0.0.106',
+            subnet_bits: 32,
+            async: false,
+        );
+        $child_vip->create(apply: true);
+
+        # Ensure the parent interface shows both virtual IPs sharing the same vhid in ifconfig
+        $iface = $carp_vip->interface->get_related_model()->if->value;
+        $ifconfig = new Command("/sbin/ifconfig $iface");
+        $this->assert_str_contains(
+            $ifconfig->output,
+            'inet 127.0.0.105 netmask 0xffffffff broadcast 127.0.0.105 vhid 105',
+        );
+        $this->assert_str_contains(
+            $ifconfig->output,
+            'inet 127.0.0.106 netmask 0xffffffff broadcast 127.0.0.106 vhid 105',
+        );
+
+        # Clean up the VIPs we created
+        $child_vip->delete();
+        $carp_vip->delete(apply: true);
+    }
 }