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

wireguard: T5707: remove previously deconfigured peer #2431

Merged
merged 1 commit into from
Nov 3, 2023

Conversation

c-po
Copy link
Member

@c-po c-po commented Nov 2, 2023

Change Summary

Changing the public key of a peer (updating the key material) left the old WireGuard peer in place, as the key removal command used the new key.

WireGuard only supports peer removal based on the configured public-key, by deleting the entire interface this is the shortcut instead of parsing out all peers and removing them one by one.

Peer reconfiguration will always come with a short downtime while the WireGuard interface is recreated.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Code style update (formatting, renaming)
  • Refactoring (no functional changes)
  • Migration from an old Vyatta component to vyos-1x, please link to related PR inside obsoleted component
  • Other (please describe):

Related Task(s)

Related PR(s)

Component(s) name

WireGuard

Proposed changes

How to test

Added a new smoketest test_05_wireguard_peer_pubkey_change

Or use:

set interfaces wireguard wg1337 address 172.16.0.1/24
set interfaces wireguard wg1337 port 1337
set interfaces wireguard wg1337 private-key iJi4lb2HhkLx2KSAGOjji2alKkYsJjSPkHkrcpxgEVU=
set interfaces wireguard wg1337 peer VyOS public-key srQ8VF6z/LDjKCzpxBzFpmaNUOeuHYzIfc2dcmoc/h4=
set interfaces wireguard wg1337 peer VyOS allowed-ips 10.205.212.10/32
commit

Now use sudo wg show wg1337 to compare the public keys

interface: wg1337
  public key: UMtmJkFRLBddcaP/QjwMx+GAey7eLgHm55iIqs9oXE0=
  private key: (hidden)
  listening port: 1337

peer: srQ8VF6z/LDjKCzpxBzFpmaNUOeuHYzIfc2dcmoc/h4=
  allowed ips: 10.205.212.10/32

Now change the public key and compare again

set interfaces wireguard wg1337 peer VyOS public-key 8pbMHiQ7NECVP7F65Mb2W8+4ldGG2oaGvDSpSEsOBn8=
commit
interface: wg1337
  public key: UMtmJkFRLBddcaP/QjwMx+GAey7eLgHm55iIqs9oXE0=
  private key: (hidden)
  listening port: 1337

peer: 8pbMHiQ7NECVP7F65Mb2W8+4ldGG2oaGvDSpSEsOBn8=
  allowed ips: 10.205.212.10/32

Smoketest result

[email protected]:~$ /usr/libexec/vyos/tests/smoke/cli/test_interfaces_wireguard.py
test_01_wireguard_peer (__main__.WireGuardInterfaceTest.test_01_wireguard_peer) ... ok
test_02_wireguard_add_remove_peer (__main__.WireGuardInterfaceTest.test_02_wireguard_add_remove_peer) ... ok
test_03_wireguard_same_public_key (__main__.WireGuardInterfaceTest.test_03_wireguard_same_public_key) ... ok
test_04_wireguard_threaded (__main__.WireGuardInterfaceTest.test_04_wireguard_threaded) ... ok
test_05_wireguard_peer_pubkey_change (__main__.WireGuardInterfaceTest.test_05_wireguard_peer_pubkey_change) ... ok

----------------------------------------------------------------------
Ran 5 tests in 20.706s

OK

Checklist:

  • I have read the CONTRIBUTING document
  • I have linked this PR to one or more Phabricator Task(s)
  • I have run the components SMOKETESTS if applicable
  • My commit headlines contain a valid Task id
  • My change requires a change to the documentation
  • I have updated the documentation accordingly

Changing the public key of a peer (updating the key material) left the old
WireGuard peer in place, as the key removal command used the new key.

WireGuard only supports peer removal based on the configured public-key, by
deleting the entire interface this is the shortcut instead of parsing out all
peers and removing them one by one.

Peer reconfiguration will always come with a short downtime while the WireGuard
interface is recreated.
@c-po
Copy link
Member Author

c-po commented Nov 2, 2023

@Mergifyio backport sagitta

@vyosbot vyosbot requested review from a team, dmbaturin, sarthurdev, zdc, jestabro and sever-sever and removed request for a team November 2, 2023 20:15
Copy link
Contributor

mergify bot commented Nov 2, 2023

backport sagitta

✅ Backports have been created

@c-po c-po merged commit d8a7197 into vyos:current Nov 3, 2023
8 checks passed
@c-po c-po deleted the wireguard-t5707 branch November 3, 2023 17:00
@c-po
Copy link
Member Author

c-po commented Nov 3, 2023

@Mergifyio refresh

Copy link
Contributor

mergify bot commented Nov 3, 2023

refresh

✅ Pull request refreshed

dmbaturin added a commit that referenced this pull request Nov 3, 2023
wireguard: T5707: remove previously deconfigured peer (backport #2431)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Development

Successfully merging this pull request may close these issues.

3 participants