jaredhendrickson13 / pfsense-api

The missing REST API package for pfSense
https://pfrest.org/
Apache License 2.0
676 stars 104 forks source link

When creating an OpenVPN server in v2 of the API, there is no option to generate a TLS Key, as there was in v1 #570

Open Coffee-Processing-Unit opened 1 week ago

Coffee-Processing-Unit commented 1 week ago

Expected behavior

As there was in v1 of the API when creating an OpenVPN server, there will be a way to specify the TLS key will be autogenerated. In v2, the TLS key can either not exist or imported from outside.

Root Cause

In the v1 code, the TLS would have been auto-generated (with pfsense's openvpn_create_key function) based on some conditions:

private function __validate_tls() {
        # Check for our optional `TLS` payload value
        if (isset($this->initial_data["tls"])) {
            # Auto generate TLS key if empty
            if ($this->initial_data["tls"] == "" or $this->initial_data["tls"] == "true") {
                $this->validated_data["tls"] = base64_encode(openvpn_create_key());
            } else {
                $this->validated_data["tls"] = base64_encode($this->initial_data["tls"]);
            }
        }
    }

In v2, this functionality does not exist. I suggest adding the same functionality to the "validate_tls" function in the case the tls field is empty ("") and not null. Also, add to this field's definition the allow_empty: true property, for this to be possible. For example:

public function validate_tls(string $tls): string {
    # this if clause is the code addition
        if ($tls == "") {
            $auto_generated_tls = base64_encode(openvpn_create_key())
            return $auto_generated_tls
        }

        # Ensure this TLS key begins with the OpenVPN key prefix and suffix
        if (!str_contains($tls, self::STATIC_KEY_PREFIX) and !str_contains($tls, self::STATIC_KEY_SUFFIX)) {
            throw new ValidationError(
                message: 'Field `tls` must be a valid OpenVPN TLS key.',
                response_id: 'OPENVPN_SERVER_TLS_INVALID_KEY',
            );
        }

        return $tls;
    }
);

Affected Endpoints: /api/v2/vpn/openvpn/server

jaredhendrickson13 commented 4 days ago

I'll have these adjustments out in v2.2.0 but I've had to rework them a bit. The main issue here is the tls field acts as both a method of assigning a TLS key, as well as sort of boolean that enables TLS for the server. This implicit behavior is somewhat confusing and goes against RESTful principals. Adding an empty string into the mix just furthers the problem.

Instead, I've added a new use_tls boolean field that is populated based on whether a tls value is stored in the config. I've put the related TLS fields behind this field so they will only apply when use_tls is true. This better matches the behavior in the webConfigurator, allows the tls field to use a default value safely, and should hopefully clear up potentially confusion.

v2.2.0 should be out later this week. A new pre-release build should be going out tonight if you want to try it out.

Coffee-Processing-Unit commented 4 days ago

Thank you. To ensure I understand you correctly - if use_tls will be false, then will the TLS be auto-generated?

jaredhendrickson13 commented 3 days ago

Not quite, use_tls will match the behavior of this setting:

Screenshot 2024-09-30 at 8 47 21 AM

If use_tls is false, the related TLS settings will become unavailable like they do in the UI. If this is set to true and there is not a tis value assigned (or null is provided) a new TLS key will be generated.