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

Add additional tunnel encapsulation choices: PPTP, L2TP, EoIP, SSTP #17960

Closed
jmcguir opened this issue Nov 7, 2024 · 4 comments · Fixed by #18097
Closed

Add additional tunnel encapsulation choices: PPTP, L2TP, EoIP, SSTP #17960

jmcguir opened this issue Nov 7, 2024 · 4 comments · Fixed by #18097
Assignees
Labels
complexity: low Requires minimal effort to implement status: accepted This issue has been accepted for implementation type: feature Introduction of new functionality to the application

Comments

@jmcguir
Copy link
Contributor

jmcguir commented Nov 7, 2024

NetBox version

v4.1.4

Feature type

Change to existing functionality

Triage priority

I volunteer to perform this work (if approved)

Proposed functionality

I'd like to add PPTP, L2TP, and EoIP as Tunnel Encapsulation Choices. PPTP, and L2TP are standards based tunnel protocols while EoIP is a MikroTik proprietary protocol and SSTP is a Microsoft proprietary protocol.

PPTP: https://datatracker.ietf.org/doc/html/rfc2637
L2TP: https://datatracker.ietf.org/doc/html/rfc2661
EoIP: https://help.mikrotik.com/docs/spaces/ROS/pages/24805521/EoIP
SSTP: https://learn.microsoft.com/en-us/openspecs/windows_protocols/ms-sstp/c50ed240-56f3-4309-8e0c-1644898f0ea8

As far as I can tell the only change here would be to add these fourto the choices.py here: https://github.com/netbox-community/netbox/blob/develop/netbox/vpn/choices.py

Use case

This lets users model more accurately their VPN tunnels. Right now the choices don't have an other so you have to use an incorrect Encapsulation choice.

Database changes

No response

External dependencies

No response

@jmcguir jmcguir added status: needs triage This issue is awaiting triage by a maintainer type: feature Introduction of new functionality to the application labels Nov 7, 2024
@chbally
Copy link

chbally commented Nov 7, 2024

Please also add WireGuard:
WireGuard: https://datatracker.ietf.org/doc/html/rfc8922#name-wireguard
and if possible OpenVPN:
OpenVPN: https://datatracker.ietf.org/doc/html/rfc8922#name-openvpn

I think even if is kind of IP in IP lots of people would appreciate these two protokolls in Netbox:
#14683

@jmcguir jmcguir changed the title Add additional tunnel encapsulation choices: PPTP, L2TP, EoIP Add additional tunnel encapsulation choices: PPTP, L2TP, EoIP, SSTP Nov 12, 2024
@jmcguir
Copy link
Contributor Author

jmcguir commented Nov 12, 2024

If approved I'd be happy to add WireGuard and OpenVPN to a PR I'm willing to write.

@bctiemann bctiemann added complexity: low Requires minimal effort to implement status: accepted This issue has been accepted for implementation labels Nov 25, 2024 — with Linear
@bctiemann bctiemann removed the status: needs triage This issue is awaiting triage by a maintainer label Nov 25, 2024
jmcguir pushed a commit to jmcguir/netbox that referenced this issue Nov 25, 2024
@jeremystretch
Copy link
Member

Let's not turn this into a wishlist for every potential tunneling protocol.

L2TP is a standard protocol with its own IP protocol number (115); I see no issue adding this.

Likewise, PPTP is a standard protocol which runs atop GRE using TCP/1723. This also makes sense to add.

EoIP appears to be a vendor-proprietary implementation of GRE. From the documentation provided, I see no justification for declaring this as a discrete tunnel type.

As far as I'm aware, SSTP is employed for client access and does not support site-to-site VPN tunneling. If this is the case, it does not make sense IMO to add it as a site-to-site tunnel encapsulation type.

Wireguard is a relatively newer VPN technology that utilizes multiple UDP ports beginning at 51820. As noted here we need to be careful not to imply that this change implements support for the configuration of Wireguard VPNs, but I have no issue with adding it as an encapsulation option.

Likewise, OpenVPN is a complete VPN solution; we can add it as a tunnel encapsulation option provided the scope of this change does not seek to implement support for specific configuration options.

To summarize, I believe this PR should be limited to adding the following tunnel encapsulation choices:

  • L2TP
  • PPTP
  • Wireguard
  • OpenVPN

@jmcguir
Copy link
Contributor Author

jmcguir commented Dec 6, 2024

Thanks for your input Jeremy. I'll revise my PR with this feedback.

jeremystretch pushed a commit that referenced this issue Dec 9, 2024
* fix #17960

* updated post feedback

---------

Co-authored-by: Joel L. McGuire <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
complexity: low Requires minimal effort to implement status: accepted This issue has been accepted for implementation type: feature Introduction of new functionality to the application
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants