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-tools: add ippeer option #3810

Closed
wants to merge 1 commit into from

Conversation

champtar
Copy link
Member

This allow to set IPv4 peer address for point to point tunnel
This simplify a lot bird ospf usage / configuration

~# cat /etc/config/network
config interface 'test'
	option proto 'wireguard'
	option private_key '<key>'
	list addresses '1.2.3.4'
	option ippeer '5.6.7.8'
	option nohostroute '1'

~# ip a show test
9: test: <POINTOPOINT,NOARP,UP,LOWER_UP> mtu 1420 qdisc noqueue state UNKNOWN qlen 1000
    link/[65534]
    inet 1.2.3.4 peer 5.6.7.8/32 brd 255.255.255.255 scope global test
       valid_lft forever preferred_lft forever

ippeer is not really a good fit with addresses but I don't have a better idea for now.

@champtar
Copy link
Member Author

@zx2c4

@@ -108,6 +108,7 @@ proto_wireguard_setup() {
config_get private_key "${config}" "private_key"
config_get listen_port "${config}" "listen_port"
config_get addresses "${config}" "addresses"
config_get ippeer "${config}" "ippeer"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

peerip could be named endpoint_ip, and is not at all similar to ippeer
ippeer is a generic option that could be available for any interface type

ip link add name ipip-test type ipip local 9.10.11.12 remote 13.14.15.16
ip addr add dev ipip-test local 1.2.3.4 peer 5.6.7.8
ip addr show ipip-test
15: ipip-test@NONE: <POINTOPOINT,NOARP> mtu 1480 qdisc noop state DOWN group default qlen 1000
    link/ipip 9.10.11.12 peer 13.14.15.16
    inet 1.2.3.4 peer 5.6.7.8/32 scope global ipip-test
       valid_lft forever preferred_lft forever

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@adschm adschm added the core packages pull request/issue for core (in-tree) packages label Jan 24, 2021
@zx2c4
Copy link
Contributor

zx2c4 commented Jan 24, 2021

I'm curious to learn how this simplifies bird and ospf. How does it differ from setting the IP address to a /32 and adding one single interface route of a /32?

@champtar
Copy link
Member Author

champtar commented Jan 24, 2021

# ip link add name wg-test type wireguard
# ip addr add dev wg-test local 1.2.3.4
# ip link set wg-test up
# birdc show ospf interface
Interface wg-test (1.2.3.4/32)
	Type: ptp
	Area: 0.0.0.0 (0)
	State: PtP (stub)
	Priority: 1
	Cost: 40
	Hello timer: 10
	Wait timer: 50
	Dead timer: 50
	Retransmit timer: 5
# ip route add 5.6.7.8/32 dev wg-test
# ip route get 5.6.7.8
5.6.7.8 dev wg-test src 1.2.3.4 uid 0 
    cache 
# birdc show ospf interface
Interface wg-test (1.2.3.4/32)
	Type: ptp
	Area: 0.0.0.0 (0)
	State: PtP (stub)
	Priority: 1
	Cost: 40
	Hello timer: 10
	Wait timer: 50
	Dead timer: 50
	Retransmit timer: 5

Without the peer bird stays in stub mode, so it doesn't advertise

With the peer on the interface

# ip addr del 1.2.3.4/32 dev wg-test
# ip addr add dev wg-test local 1.2.3.4 peer 5.6.7.8
# ip r get 5.6.7.8
5.6.7.8 dev wg-test src 1.2.3.4 uid 0 
    cache 
# birdc show ospf interface
Interface wg-test (peer 5.6.7.8)
	Type: ptp
	Area: 0.0.0.0 (0)
	State: PtP
	Priority: 1
	Cost: 40
	Hello timer: 10
	Wait timer: 50
	Dead timer: 50
	Retransmit timer: 5

See the state is now just PtP

@champtar
Copy link
Member Author

champtar commented Jan 24, 2021

Extract from bird conf

	area 0.0.0.0 {
		stub no;
		interface "wg*" {
			cost 40;
			transmit delay 5;
			dead count 5;
			authentication cryptographic;
			password "<thepassword>";
		};
	};

@zx2c4
Copy link
Contributor

zx2c4 commented Jan 24, 2021

What I'm wondering about is:

# ip link add name wg-test type wireguard
# ip link set wg-test up
# ip addr add 1.2.3.4/32 dev wg-test
# ip route add 5.6.7.8/32 dev wg-test

@champtar
Copy link
Member Author

Sorry I did a bunch of edits, you should have your response in previous messages

It doesn't change the routing, and bird/ospf still uses multicast

# timeout 30s tcpdump -i wg-test -nnev
dropped privs to tcpdump
tcpdump: listening on wg-test, link-type RAW (Raw IP), capture size 262144 bytes
15:52:12.149201 ip: (tos 0xc0, ttl 1, id 42808, offset 0, flags [none], proto OSPF (89), length 80)
    1.2.3.4 > 224.0.0.5: OSPFv2, Hello, length 44
	Router-ID 54.36.54.117, Backbone Area, Authentication Type: MD5 (2)
	Key-ID: 1, Auth-Length: 16, Crypto Sequence Number: 0x600d883d
	Options [External]
	  Hello Timer 10s, Dead Timer 50s, Mask 0.0.0.0, Priority 1

But this is how bird/ospf works with point to point /32 links, if there is no peer set it's a stub

@zx2c4
Copy link
Contributor

zx2c4 commented Jan 24, 2021

Huh, interesting. Is this something that should be fixed in bird? Or is the peer address an important semantic worth supporting in OpenWRT because it has special significance beyond this particular bug?

@champtar
Copy link
Member Author

I have no idea if this is some legacy behavior or important semantic. I've migrated from openvpn p2p tunnel (with /32 & peer) to wireguard last summer and was forced to use /31 and I really prefer /32, it makes the setup way cleaner/easier IMO.
I'll look a bit at bird code and send an email to bird mailing list with you in CC.

@champtar
Copy link
Member Author

@champtar
Copy link
Member Author

Answer: this is a bird limitation but pretty hard to fix (http://trubka.network.cz/pipermail/bird-users/2021-January/015167.html)

@champtar
Copy link
Member Author

@zx2c4 can we merge this ? Between rewriting bird and a 3 lines patch, I prefer the 3 lines patch :)

@champtar
Copy link
Member Author

@zx2c4 friendly ping

@zx2c4
Copy link
Contributor

zx2c4 commented Feb 19, 2021

Isn't this a small patch for bird too? Why a "rewrite"?

@champtar
Copy link
Member Author

champtar commented Feb 19, 2021

BIRD OSPF implementation is strongly coupled with address slots instead of real interfaces.

This makes me think one will need to change a good part of the logic of BIRD OSPF implementation, not just add one or 2 if.
With my skill level in C, my free time and no one offering to do the bird changes, please pretty please accept this feature addition.

@champtar
Copy link
Member Author

@zx2c4 ?

@champtar
Copy link
Member Author

@zx2c4 silence is hard to parse ;)

This allow to set IPv4 peer address for point to point tunnel
This simplify a lot bird ospf usage / configuration

~# cat /etc/config/network
config interface 'test'
	option proto 'wireguard'
	option private_key '<key>'
	list addresses '1.2.3.4'
	option ippeer '5.6.7.8'
	option nohostroute '1'

~# ip a show test
9: test: <POINTOPOINT,NOARP,UP,LOWER_UP> mtu 1420 qdisc noqueue state UNKNOWN qlen 1000
    link/[65534]
    inet 1.2.3.4 peer 5.6.7.8/32 brd 255.255.255.255 scope global test
       valid_lft forever preferred_lft forever

Signed-off-by: Etienne Champetier <champetier.etienne@gmail.com>
@champtar
Copy link
Member Author

Rebased on latest changes
@zx2c4 could you ack/nack please

@zx2c4
Copy link
Contributor

zx2c4 commented Feb 27, 2021

With my skill level in C, my free time and no one offering to do the bird changes, please pretty please accept this feature addition.

This isn't a good rationale for adding a non standard knob that will harder to claw back later when we risk breaking compatibility. I'll feel comfortable acking this if I'm more certain that there's no way for bird to do it, or that there's some important semantic place for ippeer. But if the reason amounts to, "because shell scripting is easier than C", I don't want to start taking shortcuts like that.

Basically, either me or you or somebody else needs to find the time to really look deeply into this.

@tohojo mentioned an interest in adding various forms of WireGuard support to bird at some point. Maybe he has a better idea of the architecture involved.

@champtar
Copy link
Member Author

After double checking, ippeer is not present in NetworkManager either, so improving bird make sense.
I'll let @tohojo comment and close this.

@tohojo
Copy link
Contributor

tohojo commented Mar 1, 2021

I don't really know anything about the OSPF implementation in Bird apart from the reply you linked above, sorry.

As for adding Wireguard support to bird itself, my idea for that was to teach Bird about the wireguard interface type so it could both learn peers from it, and also update allowedips when it installs new routes. Which would make it possible to run Bird on a multi-peer wireguard link instead of using p2p links. That seems rather orthogonal to this, though...

@champtar
Copy link
Member Author

Thanks @tohojo and @zx2c4, I'll close this for now

@champtar champtar closed this Mar 23, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
core packages pull request/issue for core (in-tree) packages
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants