Bug #11465: Input validation does not prevent multiple conflicting WireGuard peers on a single tunnel from attempting to act as default route - pfSense Packages - pfSense bugtracker

Actions

Copy link

Bug #11465

closed

Input validation does not prevent multiple conflicting WireGuard peers on a single tunnel from attempting to act as default route

Added by Jim Pingle almost 4 years ago. Updated about 3 years ago.

Status:

Closed

Priority:

Normal

Assignee:

Viktor Gurov

Category:

WireGuard

Target version:

Start date:

02/19/2021

Due date:

% Done:

Estimated time:

Plus Target Version:

Affected Version:

Affected Plus Version:

Affected Architecture:

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.

Actions

Copy link

Updated by Viktor Gurov almost 4 years ago

https://gitlab.netgate.com/pfSense/pfSense/-/merge_requests/149

Actions

Copy link

Updated by Jim Pingle almost 4 years ago

Status changed from New to Pull Request Review

Actions

Copy link

Updated by Jim Pingle over 3 years ago

Target version changed from CE-Next to 2.5.1

Actions

Copy link

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

Actions

Copy link

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!

Actions

Copy link

Updated by Renato Botelho over 3 years ago

Cherry-picked to RELENG_2_5_1

Actions

Copy link

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.

Actions

Copy link

Updated by Jim Pingle over 3 years ago

Target version changed from 2.5.1 to Future

Actions

Copy link

Updated by Adam Cooper about 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.

Actions

Copy link

#10