diff --git a/pfSense-pkg-RESTAPI/files/usr/local/pkg/RESTAPI/Fields/InterfaceField.inc b/pfSense-pkg-RESTAPI/files/usr/local/pkg/RESTAPI/Fields/InterfaceField.inc index 6f815cb8..c37421f0 100644 --- a/pfSense-pkg-RESTAPI/files/usr/local/pkg/RESTAPI/Fields/InterfaceField.inc +++ b/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; } } } diff --git a/pfSense-pkg-RESTAPI/files/usr/local/pkg/RESTAPI/Models/IPsecPhase1.inc b/pfSense-pkg-RESTAPI/files/usr/local/pkg/RESTAPI/Models/IPsecPhase1.inc index a92bb3ce..c9375f8f 100644 --- a/pfSense-pkg-RESTAPI/files/usr/local/pkg/RESTAPI/Models/IPsecPhase1.inc +++ b/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.', ); diff --git a/pfSense-pkg-RESTAPI/files/usr/local/pkg/RESTAPI/Models/OpenVPNClientExport.inc b/pfSense-pkg-RESTAPI/files/usr/local/pkg/RESTAPI/Models/OpenVPNClientExport.inc index 60506b8e..68f0a50c 100644 --- a/pfSense-pkg-RESTAPI/files/usr/local/pkg/RESTAPI/Models/OpenVPNClientExport.inc +++ b/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 diff --git a/pfSense-pkg-RESTAPI/files/usr/local/pkg/RESTAPI/Models/RESTAPISettings.inc b/pfSense-pkg-RESTAPI/files/usr/local/pkg/RESTAPI/Models/RESTAPISettings.inc index 20a1dc75..73a40c28 100644 --- a/pfSense-pkg-RESTAPI/files/usr/local/pkg/RESTAPI/Models/RESTAPISettings.inc +++ b/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] = diff --git a/pfSense-pkg-RESTAPI/files/usr/local/pkg/RESTAPI/Tests/APIFieldsInterfaceFieldTestCase.inc b/pfSense-pkg-RESTAPI/files/usr/local/pkg/RESTAPI/Tests/APIFieldsInterfaceFieldTestCase.inc index 2c9a6db0..da745932 100644 --- a/pfSense-pkg-RESTAPI/files/usr/local/pkg/RESTAPI/Tests/APIFieldsInterfaceFieldTestCase.inc +++ b/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. diff --git a/pfSense-pkg-RESTAPI/files/usr/local/pkg/RESTAPI/Tests/APIModelsOpenVPNClientExportTestCase.inc b/pfSense-pkg-RESTAPI/files/usr/local/pkg/RESTAPI/Tests/APIModelsOpenVPNClientExportTestCase.inc index 5ec1858a..0fa17e61 100644 --- a/pfSense-pkg-RESTAPI/files/usr/local/pkg/RESTAPI/Tests/APIModelsOpenVPNClientExportTestCase.inc +++ b/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(); + } } diff --git a/pfSense-pkg-RESTAPI/files/usr/local/pkg/RESTAPI/Tests/APIModelsVirtualIPTestCase.inc b/pfSense-pkg-RESTAPI/files/usr/local/pkg/RESTAPI/Tests/APIModelsVirtualIPTestCase.inc index 1d594a41..350a665d 100644 --- a/pfSense-pkg-RESTAPI/files/usr/local/pkg/RESTAPI/Tests/APIModelsVirtualIPTestCase.inc +++ b/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); + } }