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

security/tinc: Fix switch mode #1733

Merged
merged 6 commits into from
May 12, 2020
Merged

security/tinc: Fix switch mode #1733

merged 6 commits into from
May 12, 2020

Conversation

vnxme
Copy link
Contributor

@vnxme vnxme commented Mar 10, 2020

Background

What I would like to achieve is a dual-stack ethernet bridge using Tinc (layer 2 VPN). Since it is completely possible using Tinc on Linux, it should be achievable in OPNsense.

Current situation summary

About this PR

If I change Tinc mode from router (L3VPN) to switch (L2VPN), it wont start properly because irrelevant subnet variables are included in host configuration files. Before this commit I cannot leave them empty in OPNsense WebGUI. Commenting out these variables and restarting Tinc daemon from console fixes the problem.

Following the official manual, subnet configuration variable seems to be used by Tinc for routing purposes in router mode. Although this field can also be a single MAC address which might be suitable for switch mode purposes, OPNsense does not currently support such an option.

This PR includes the following changes:

  • Remove required flag from subnet fields
  • Add a required constraint to network subnet field for router mode
  • Relax validation and drafting mechanism for Tinc configration (only include subnet if not empty)

What could be further improved (but I have failed to implement):

  • WebGUI: Support MAC value in subnet fields
  • WebGUI: Hide/show subnet field when changing mode
  • WebGUI: Make subnet field required in other hosts configuration when they are assigned to a router-mode network (a multi-level constraint, seems to be impossible in current architecture)

A Host class with empty self._payload['subnet'] is considered invalid (lines 38-39). Thus, we can remove self._payload['subnet'] = None from __init__() and add a check for existance to config_text().
Set network.subnet.required and host.subnet.required to N, add a required constraint for network.subnet if network.mode is router.
In order to support various dual-stack configs (primary IPv4/v6 assigned by VPN/Tinc and any combination of alias IPv4/v6 assigned by Firewall/VIP) we need to trigger configctl:
- Primary IPv4: /usr/local/opnsense/service/configd_ctl.py interface newip $interface
- Primary IPv6: /usr/local/opnsense/service/configd_ctl.py interface newipv6 $interface
Destroying tun/tap interface each time Tinc daemon stops/restarts resolves the issue of losing IPv6 network routes (see #3972).
Copy link
Member

@AdSchellevis AdSchellevis left a comment

Choose a reason for hiding this comment

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

@vnxme sorry for the late reply, finally got around checking the code a bit, I've added some small findings inline.

vnxme added 2 commits May 6, 2020 16:59
The network.mode field is now linked to the network.subnet field.
@vnxme vnxme requested a review from AdSchellevis May 6, 2020 15:07
AdSchellevis added a commit to opnsense/core that referenced this pull request May 12, 2020
…eld (shows validation message on field where the option is set, should be the one it's pointing to).

ref opnsense/plugins#1733
@AdSchellevis AdSchellevis merged commit f2db771 into opnsense:master May 12, 2020
fichtner pushed a commit to opnsense/core that referenced this pull request May 19, 2020
…eld (shows validation message on field where the option is set, should be the one it's pointing to).

ref opnsense/plugins#1733

(cherry picked from commit 2eb1ee5)
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.

2 participants