Project

General

Profile

Actions

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 about 3 years ago. Updated over 2 years ago.

Status:
Closed
Priority:
Normal
Assignee:
Viktor Gurov
Category:
WireGuard
Target version:
-
Start date:
02/19/2021
Due date:
% Done:

0%

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 #2

Updated by Jim Pingle about 3 years ago

  • Status changed from New to Pull Request Review
Actions #3

Updated by Jim Pingle about 3 years ago

  • Target version changed from CE-Next to 2.5.1
Actions #4

Updated by Jim Pingle about 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 #5

Updated by Renato Botelho about 3 years ago

  • Status changed from Pull Request Review to Feedback
  • Assignee set to Viktor Gurov

PR has been merged. Thanks!

Actions #6

Updated by Renato Botelho about 3 years ago

Cherry-picked to RELENG_2_5_1

Actions #7

Updated by Jim Pingle about 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 #8

Updated by Jim Pingle about 3 years ago

  • Target version changed from 2.5.1 to Future
Actions #9

Updated by Adam Cooper over 2 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 #10

Updated by Adam Cooper over 2 years ago

This ticket can now be closed as the PR has been merged

Actions #11

Updated by Jim Pingle over 2 years ago

  • Status changed from Feedback to Closed
Actions #12

Updated by Jim Pingle over 2 years ago

  • Project changed from pfSense to pfSense Packages
  • Category changed from WireGuard to WireGuard
  • Target version deleted (Future)
  • Release Notes deleted (Default)
Actions

Also available in: Atom PDF