Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closes #77 #78

Closed
wants to merge 1 commit into from
Closed

Closes #77 #78

wants to merge 1 commit into from

Conversation

SkyHyve
Copy link

@SkyHyve SkyHyve commented Jul 25, 2023

removed network checks for tunnel_network and tunnel_network6 as they should be allowed to specify non-network addresses to have static client ip addresses. This is in line with how the pfsense GUI works and even in line with the examples provided by the pfsense GUI.

… should be allowed to specify non-network addresses to have static client ip addresses
@TTPBruce
Copy link

TTPBruce commented Aug 11, 2023

I added checks for IPv4/IPv6 address before I saw this PR #85

@opoplawski
Copy link
Contributor

I'm pretty sure we don't want to remove all checks. pfSense uses the following validation check:

function openvpn_validate_tunnel_network($value, $ipproto) {
        $value = trim($value);
        if (!empty($value)) {
                if (is_alias($value) && (alias_get_type($value) == 'network')) {
                        $net = alias_to_subnets_recursive($value);
                        if ((!is_subnetv4($net[0]) && ($ipproto == 'ipv4')) ||
                            (!is_subnetv6($net[0]) && ($ipproto == 'ipv6')) ||
                            (count($net) > 1)) {
                                return false;
                        }
                        return true;
                } else {
                        if ((!is_subnetv4($value) && ($ipproto == 'ipv4')) ||
                            (!is_subnetv6($value) && ($ipproto == 'ipv6'))) {
                                return false;
                        }
                }
        }
        return true;
}

I'm going to close this and look closer at #85. Thanks for the PR though.

@opoplawski opoplawski closed this Dec 19, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants