Bug #11465
closedInput validation does not prevent multiple conflicting WireGuard peers on a single tunnel from attempting to act as default route
0%
Description
WireGuard uses Allowed IPs for internal routing to decide where to send traffic to a peer. When a peer has Allowed IPs set to 0.0.0.0/0 and/or ::/0 that means all traffic gets associated with that peer, and no other peers can be added.
We need input validation to check and prevent this from being configured, both when adding a new peer and when editing existing peers.
Similarly, if there are multiple peers, neither of them can have a blank Allowed IPs list as the result is ambiguous. With a single peer, WireGuard assumes 0.0.0.0/0,::/0, but once another peer is present that becomes invalid.
Updated by Viktor Gurov over 3 years ago
Updated by Jim Pingle over 3 years ago
- Status changed from New to Pull Request Review
Updated by Jim Pingle over 3 years ago
- Target version changed from CE-Next to 2.5.1
Updated by Jim Pingle over 3 years ago
When testing, attempt these configurations in order without removing anything unless noted otherwise:
- Create a tunnel with one peer that has Allowed IPs empty -- should succeed
- Create a tunnel with the first peer Allowed IPs set to "10.0.0.0/24" or equivalent non-default network
- Add a peer to this tunnel with Allowed IPs set to "0.0.0.0/0" -- should succeed (first IPv4 default for this tunnel)
- Add a peer to this tunnel with Allowed IPs set to "::/0" -- should succeed (first IPv6 default for this tunnel)
- Add a peer to this tunnel with Allowed IPs empty -- should fail (additional attempt at IPv4 and IPv6 default)
- Add a peer to this tunnel with Allowed IPs set to "0.0.0.0/0" -- should fail
- Add a peer to this tunnel with Allowed IPs set to "::/0" -- should fail
- Create a tunnel with the first peer Allowed IPs set to "10.0.0.0/24" or equivalent non-default network -- should succeed
- Add a peer to this tunnel with Allowed IPs empty -- should succeed (IPv4 and IPv6 default for this tunnel)
- Add a peer to this tunnel with Allowed IPs empty -- should fail
- Add a peer to this tunnel with Allowed IPs set to "::/0" -- should fail
- Add a peer to this tunnel with Allowed IPs set to "0.0.0.0/0" -- should fail
- Create a tunnel with the first peer Allowed IPs empty -- should succeed
- Add a peer to this tunnel with Allowed IPs set to "::/0" -- should fail
- Add a peer to this tunnel with Allowed IPs set to "0.0.0.0/0" -- should fail
- Add a peer to this tunnel with Allowed IPs set to "10.0.1.0/24" -- should succeed
- Add a peer to this tunnel with Allowed IPs set to "fc07:1::0/64" -- should succeed
- Create a tunnel with the first peer Allowed IPs set to "10.5.0.0/24" or equivalent non-default network -- should succeed
- Add a peer to this tunnel with Allowed IPs set to "10.5.1.0/24" -- should succeed
- Add a peer to this tunnel with Allowed IPs set to "10.5.2.0/24" -- should succeed
Updated by Renato Botelho over 3 years ago
- Status changed from Pull Request Review to Feedback
- Assignee set to Viktor Gurov
PR has been merged. Thanks!
Updated by Jim Pingle over 3 years ago
- Tracker changed from Todo to Bug
- Subject changed from Add validation to prevent multiple WireGuard peers when Allowed IPs is 0.0.0.0/0, ::/0, or blank. to Input validation does not prevent multiple conflicting WireGuard peers on a single tunnel from attempting to act as default route
Updating subject for release notes.
Updated by Jim Pingle over 3 years ago
- Target version changed from 2.5.1 to Future
Updated by Adam Cooper almost 3 years ago
Submitted PR 19 (https://github.com/theonemcdonald/pfSense-pkg-WireGuard/pull/149).
Few queries on the PR regarding the plugin taking no allowed IPs as the assumed default route, validation doesn't currently check / validate this as I don't believe this assertion stands for the plugin anymore.
Updated by Adam Cooper almost 3 years ago
This ticket can now be closed as the PR has been merged
Updated by Jim Pingle almost 3 years ago
- Status changed from Feedback to Closed
Updated by Jim Pingle almost 3 years ago
- Project changed from pfSense to pfSense Packages
- Category changed from WireGuard to WireGuard
- Target version deleted (
Future) - Release Notes deleted (
Default)