Page MenuHomeVyOS Platform
Create Task
Maniphest T4774

Disallow duplicate pubkey on peers of a wireguard interface
Closed, WontfixPublicBUG
Actions

Description

It is possible to use a specific public key on multiple peers on the same interface:

set interfaces wireguard wg100 peer main  pubkey 'QApDr57t1SSvzg6Gn5KGwSlVXqxp7wHmgKlBc6ww0mg='
set interfaces wireguard wg100 peer ghost pubkey 'QApDr57t1SSvzg6Gn5KGwSlVXqxp7wHmgKlBc6ww0mg='

If you do so, this will disrupt the interface status and statistics reporting as well as potentially prevent one or more of the connected peers from communicating with the server (based on what is covered in "allowed ips" definition of peers), so lets report the statistics:

show interfaces wireguard wg100

the main and ghost peers may show similar statistics including for example "transfer" and "status" while it is inaccurate (outputs are prepared for illustration only, not actual output):

peer: main
  public key: QApDr57t1SSvzg6Gn5KGwSlVXqxp7wHmgKlBc6ww0mg=
  latest handshake: 0:33:12
  status: active
  endpoint: 4.3.2.1:54321
  allowed ips: 192.168.1.5/32
  transfer: 1 MB received, 14 MB sent


peer: ghost
  public key: QApDr57t1SSvzg6Gn5KGwSlVXqxp7wHmgKlBc6ww0mg=
  latest handshake: 9:26:24
  status: active
  endpoint: 1.2.4.4:12345
  allowed ips: 192.168.1.6/32
  transfer: 1 MB received, 14 MB sent

So it appears that some functions (including processing of access lists and reporting statistics) are based on looking up the pubkeys as the primary key. So it is desired that the commit would prevent or warn existence of duplicate pubkeys on the peers of each wireguard interface.

NOTE: The reported behavior has been observed in since v1.3

Details

Difficulty level
Unknown (require assessment)
Version
VyOS 1.4-rolling-202103251004
Why the issue appeared?
Will be filled on close
Is it a breaking change?
Unspecified (possibly destroys the router)
Issue type
Bug (incorrect behavior)

Event Timeline

Alfa80 created this task.Oct 26 2022, 7:50 PM
Alfa80 created this object in space S1 VyOS Public.
pasik subscribed.Oct 26 2022, 8:51 PM
Alfa80 added projects: VyOS 1.3 Equuleus, VyOS 1.4 Sagitta.Oct 27 2022, 6:52 AM
Viacheslav changed the subtype of this task from "Task" to "Bug".Oct 27 2022, 10:32 AM
sarthurdev changed the task status from Open to In progress.Oct 27 2022, 10:54 PM
sarthurdev claimed this task.
sarthurdev subscribed.

1.4 PR: https://github.com/vyos/vyos-1x/pull/1621

sarthurdev changed the task status from In progress to Backport candidate.Nov 1 2022, 9:18 AM
sarthurdev moved this task from Need Triage to Finished on the VyOS 1.4 Sagitta board.
trae32566 subscribed.EditedNov 22 2022, 5:14 AM

This breaks a perfectly valid use case which I utilize regularly: using IPv4 + IPv6 peers with the same public key. Why would I want to create multiple keys for the exact same devices going over IPv4 and IPv6? If you want to include a warning, fine, but don't limit functionality based on someone's interpretation of how something will be used. I understand where this came from, but any time you limit functionality, you limit your users. As Donald Knuth once said:

Unix was not designed to stop you from doing stupid things, because that would also stop you from doing clever things.

(yes, I'm aware it's linux, but still)

Alfa80 added a comment.EditedNov 22 2022, 10:24 AM

@trae32566 My apologies for the inconveniences. You are right. The criteria for triggering this action shall be narrowed down further.
It would be necessary to issue the warning if and only if such colliding peers also specify the exact same remote endpoint addresses (with empty endpoints also being accounted as to be the same).
In other words, we need to identify incoming peers and apply the rule only to them, not the outgoing ones which already have specific remote endpoint addresses statically defined.

NOTE: See this slack thread for further discussions made about the issue
trae32566 added a subscriber: c-po.EditedFeb 13 2023, 6:44 AM

@sdev @c-po @Alfa80 Can we look at rolling this back until it works fully, or fixing it so that it allows the above scenario please? Newer versions of 1.4 are unusable for me for a few months due to this. Let me know if I should file a separate bug.

c-po added a comment.Feb 13 2023, 6:50 AM

Please note the WireGuard crypto Key routing concept: https://www.wireguard.com/#cryptokey-routing

Keys should not be re-used

trae32566 added a comment.Feb 13 2023, 6:52 AM
In T4774#142529, @c-po wrote:

Please note the WireGuard crypto Key routing concept: https://www.wireguard.com/#cryptokey-routing

Keys should not be re-used

So should I create a separate wireguard interface for IPv4 and IPv6 then? How else can I set a separate private key for IPv4 and IPv6?

c-po added a comment.Feb 14 2023, 7:16 AM

You can either run both address-families through one tunnel

vyos@vyos# show interfaces wireguard
 wireguard wg10 {
     address xxx.xxx.151.214/31
     address 2001:xxx:xxx:xx::100:1/112
     mtu 1500
     peer foo {
         address yy.yy.yy.yy
         allowed-ips 0.0.0.0/0
         allowed-ips ::/0
         port 10010
         pubkey <hidden>
     }
     port 10010
 }

or create one tunnel per address family

syncer edited projects, added VyOS 1.3 Equuleus (1.3.5); removed VyOS 1.3 Equuleus.Aug 27 2023, 12:09 AM
syncer edited projects, added VyOS 1.3 Equuleus (1.3.6); removed VyOS 1.3 Equuleus (1.3.5).Dec 17 2023, 11:36 PM
Viacheslav closed this task as Wontfix.Jan 20 2024, 11:13 AM