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

T6183: interfaces openvpn: suppport specifying IP protocol version #3975

Merged
merged 1 commit into from
Aug 20, 2024

Conversation

lucasec
Copy link
Contributor

@lucasec lucasec commented Aug 13, 2024

Change Summary

Currently, VyOS does not specify the IP protocol version when telling OpenVPN which transport protocol to use. E.g., if you configure a UDP server, it specifies protocol udp. OpenVPN also supports specifying the IP protocol version, e.g. protocol udp4 or protocol udp6. This adds a new ip-version option to the OpenVPN interface to allow specifying an explicit IP protocol version.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Code style update (formatting, renaming)
  • Refactoring (no functional changes)
  • Migration from an old Vyatta component to vyos-1x, please link to related PR inside obsoleted component
  • Other (please describe):

Related Task(s)

https://vyos.dev/T6183

Related PR(s)

Component(s) name

interfaces openvpn

Proposed changes

In general, for client and site-to-site mode, specifying an explicit IP protocol version is rarely necessary. OpenVPN will tell the kernel to open a socket to the remote host, and the kernel will figure out what type of socket to open based on the remote host (e.g. does it resolve to an IPv4 address or an IPv6 address). Using ip-version ipv4 or ip-version ipv6 lets users mandate a particular protocol, which could be useful if you are using a DNS hostname and want to ignore responses of a certain address family.

For server mode, particularly if you don't specify an explicit local-host (letting OpenVPN bind to all interfaces), the kernel tends to choose an IPv4 socket. By setting ip-version ipv6 or ip-version ipv6-dual-stack, you can force the server to listen on IPv6.

The additional ipv6-dual-stack option is only supported in server mode (I don't believe it makes sense in any other mode), and is to account for the following Linux behavior:

When binding an IPv6 socket to the unspecified address, Linux will also send IPv4 traffic to the socket, by way of translating the IPv4 source address into an IPv6 address with the format ::ffff:<ipv4 address>. This assumes the sysctl net.ipv6.bindv6only is disable, which it is on VyOS. By specifying bind ipv6only in the OpenVPN configuration, this "dual-stack" behavior is turned off by OpenVPN setting the IPV6_V6ONLY socket option. I decided to set this by default when ip-version ipv6 is used, and offer an additional keyword ip-version ipv6-dual-stack for users that explicitly want the dual-stack behavior.

How to test

Example dual-stack server configuration:

edit interfaces openvpn vtun0
set ip-version 'ipv6-dual-stack'
set local-port '1194'
set mode 'server'
set persistent-tunnel
set protocol 'udp'
set server subnet '10.23.59.0/24'
set server topology 'subnet'
set tls ca-certificate '<ca name>'
set tls certificate '<cert name>'

Smoketest result

DEBUG - vyos@vyos:~$ /usr/bin/vyos-smoketest
DEBUG - /usr/bin/vyos-smoketest
DEBUG - Running Testcase: /usr/libexec/vyos/tests/smoke/cli/test_interfaces_openvpn.py
DEBUG - test_openvpn_client_interfaces (__main__.TestInterfacesOpenVPN.test_openvpn_client_interfaces) ... ok
DEBUG - test_openvpn_client_ip_version (__main__.TestInterfacesOpenVPN.test_openvpn_client_ip_version) ... ok
DEBUG - test_openvpn_client_verify (__main__.TestInterfacesOpenVPN.test_openvpn_client_verify) ... ok
DEBUG - test_openvpn_options (__main__.TestInterfacesOpenVPN.test_openvpn_options) ... ok
DEBUG - test_openvpn_server_ip_version (__main__.TestInterfacesOpenVPN.test_openvpn_server_ip_version) ... ok
DEBUG - test_openvpn_server_server_bridge (__main__.TestInterfacesOpenVPN.test_openvpn_server_server_bridge) ... ok
DEBUG - test_openvpn_server_subnet_topology (__main__.TestInterfacesOpenVPN.test_openvpn_server_subnet_topology) ... ok
DEBUG - test_openvpn_server_verify (__main__.TestInterfacesOpenVPN.test_openvpn_server_verify) ... ok
DEBUG - test_openvpn_site2site_interfaces_tun (__main__.TestInterfacesOpenVPN.test_openvpn_site2site_interfaces_tun) ... ok
DEBUG - test_openvpn_site2site_ip_version (__main__.TestInterfacesOpenVPN.test_openvpn_site2site_ip_version) ... ok
DEBUG - test_openvpn_site2site_verify (__main__.TestInterfacesOpenVPN.test_openvpn_site2site_verify) ... ok
DEBUG -
DEBUG - ----------------------------------------------------------------------
DEBUG - Ran 11 tests in 76.658s
DEBUG -
DEBUG - OK

Checklist:

  • I have read the CONTRIBUTING document
  • I have linked this PR to one or more Phabricator Task(s)
  • I have run the components SMOKETESTS if applicable
  • My commit headlines contain a valid Task id
  • My change requires a change to the documentation
  • I have updated the documentation accordingly

@lucasec lucasec requested a review from a team as a code owner August 13, 2024 07:03
Copy link

github-actions bot commented Aug 13, 2024

👍
No issues in PR Title / Commit Title

Copy link

github-actions bot commented Aug 13, 2024

✅ No issues found in unused-imports check.. Please refer the workflow run

Copy link
Member

@dmbaturin dmbaturin left a comment

Choose a reason for hiding this comment

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

It's a good idea but I think the UI has some room for improvement.

interface-definitions/interfaces_openvpn.xml.in Outdated Show resolved Hide resolved
src/conf_mode/interfaces_openvpn.py Outdated Show resolved Hide resolved
src/conf_mode/interfaces_openvpn.py Outdated Show resolved Hide resolved
@lucasec
Copy link
Contributor Author

lucasec commented Aug 14, 2024

I pushed a new revision following this solution I proposed inline: #3975 (comment)

Hopefully this makes it a little easier to follow the distinctions between the various options. I would still recommend reading the linked thread in full.

Copy link
Member

@dmbaturin dmbaturin left a comment

Choose a reason for hiding this comment

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

I understand the point about dual-stack now. I think dual-stack is a good, descriptive name.

@c-po c-po requested a review from sever-sever August 20, 2024 05:23
@c-po c-po merged commit 2277371 into vyos:current Aug 20, 2024
13 of 14 checks passed
@lucasec lucasec deleted the t6183 branch August 24, 2024 06:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Development

Successfully merging this pull request may close these issues.

4 participants