Skip to content

Check for VPN Interfaces#3983

Closed
QDaniel wants to merge 3 commits intoopnsense:masterfrom
QDaniel:master
Closed

Check for VPN Interfaces#3983
QDaniel wants to merge 3 commits intoopnsense:masterfrom
QDaniel:master

Conversation

@QDaniel
Copy link

@QDaniel QDaniel commented Mar 16, 2020

Wireguard and TINC Interfaces was defunct

Wireguard and TINC Interfaces was defunct
@AdSchellevis
Copy link
Member

The OpenVPN hook is just a leftover from the legacy system, which we rather don't extend unless there's a generic change which functions for all same type interfaces.

Do you experience issues when you add a gateway manually? As far as I understood so far, you should be able to add manual gateways for these types.

@QDaniel
Copy link
Author

QDaniel commented Mar 16, 2020

With the commit 93bbe1e

The dynamic wireguard and tinc Gateways are broken.

@AdSchellevis
Copy link
Member

I understand that both commits lead to the same result, but I'm not convinced that we need to re-add something similar now (there might be side affects).

What does your setup look like? Is the gateway address unknown?

@QDaniel
Copy link
Author

QDaniel commented Mar 16, 2020

I don't have find a model for the Gateways to add a setting to Override the defunct check

@AdSchellevis
Copy link
Member

there isn't. Maybe if we go one step back, we could determine what's needed here. Hence the question about your setup.

@QDaniel
Copy link
Author

QDaniel commented Mar 16, 2020

Yes, tinc and WG are dynamic Interfaces without a Gateway ip

@AdSchellevis
Copy link
Member

In most situations you know one end of the tunnel, which is likely what most people use. In some cases the address to use is collected dynamically. Sending traffic to an interface without a destination usually isn't needed.

You could always ask @mimugmail what he configures for Wireguard.

@mimugmail
Copy link
Member

@AdSchellevis
Copy link
Member

I don't mind if we do need to reevaluate the choice only support valid addresses in gateways, but I really need more context about when and why we would like to allow this. An extra checkbox might also be an option in this case, or should we do so for all tunnel types.

@QDaniel
Copy link
Author

QDaniel commented Mar 16, 2020

https://forum.opnsense.org/index.php?topic=15105.msg71616#msg71616

I will check it out and test it. Then we need this only for TINC.
Yes a checkbox for no defunct check where the best.

Yes, we have 4 WG VPNs and 1 TINC .
All wg interfaces are allowed 0.0.0.0/0 so we must set routing manual.
With Static Routes and dynamic Gateways.
TINC is in bridge mode , the routes must be only set to the interface.

AdSchellevis added a commit that referenced this pull request Mar 18, 2020
…npoint it on specific drivers. for #3983

Some tunnel interface types, such as Wireguard and Tinc do support sending traffic to the interface without an intermediate host. Since we don't want to add different static checks (and would like to get rid of the ones there for legacy reasons), it's probably better to add an option here.
@AdSchellevis
Copy link
Member

@QDaniel @mimugmail maybe we better add an option on the interface, like dba70c0, so we can support routes without intermediate gateways, but don't try to hardcode every tunnel type in the world.

Comment on lines +284 to +288
if(!self::IsVPNInterface($gw_arr)){
$gw_arr['disabled'] = true;
$gw_arr['defunct'] = true;
unset($gw_arr['gateway']);
}

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In some use cases you may want a VPN interface to be your default gateway, so the IsVPNInterface() check here should probably be removed to maintain functional consistency.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@drivera73 this PR won't be included, we're currently opting for this dba70c0

@QDaniel
Copy link
Author

QDaniel commented Mar 19, 2020

dba70c0

This is good!

fichtner pushed a commit that referenced this pull request Apr 20, 2020
…npoint it on specific drivers. for #3983

Some tunnel interface types, such as Wireguard and Tinc do support sending traffic to the interface without an intermediate host. Since we don't want to add different static checks (and would like to get rid of the ones there for legacy reasons), it's probably better to add an option here.

(cherry picked from commit dba70c0)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

4 participants