| Message ID | 20191205105805.29869-1-fe@dev.tdt.de |
|---|---|
| State | Superseded |
| Delegated to: | John Crispin |
| Headers | show |
| Series | [OpenWrt-Devel,v2] wireguard: fix interface remove for lonely peers | expand |
On 05/12/2019 11:58, Florian Eckert wrote: > +++ b/package/network/services/wireguard/Makefile > @@ -93,6 +93,8 @@ define Package/wireguard-tools/install > $(INSTALL_BIN) ./files/wireguard_watchdog $(1)/usr/bin/ > $(INSTALL_DIR) $(1)/lib/netifd/proto/ > $(INSTALL_BIN) ./files/wireguard.sh $(1)/lib/netifd/proto/ > + $(INSTALL_DIR) $(1)/etc/init.d/ > + $(INSTALL_BIN) ./files/wireguard.init $(1)/etc/init.d/wireguard > endef this code is not in the tree ?!
Hi, I think this behavior is not really acceptable. Programs, init scripts, hotplug events etc. should not automatically modify (and commit) uci configurations, especially not such vital ones like the network config. The main problem I see is that you do not know what state the config is in at any point in time, whether there are other (intentionally!) uncommitted user changes etc. Wouldn't it be better to modify the code deleting the wireguard interface to delete the peer sections as well? Or to remodel the wireguard configuration model to cope with orphaned peer sections? ~ Jo
Thanks for feedback > I think this behavior is not really acceptable. Programs, init scripts, > hotplug events etc. should not automatically modify (and commit) uci > configurations, especially not such vital ones like the network config. The new wireguard init script is only executed if the network configuration has changes. This is triggered by the procd ubus service. > The main problem I see is that you do not know what state the config is > in at any point in time, whether there are other (intentionally!) > uncommitted user changes etc. Yes, I agree, but I couldn't think of any other solution for this problem without touching the current configuration handling as you mentioned below > Wouldn't it be better to modify the code deleting the wireguard > interface to delete the peer sections as well? Or to remodel the > wireguard configuration model to cope with orphaned peer sections? I already thought of that. I could imagine a list item that references the peers in the interface section. If this wireguard interface section is deleted then these list element sections should also be deleted. For example config interface 'wg option proto 'wireguard option private_key '8OGwbqPFAJjjMgdF1kwkNYQ+uXCUYMWMyJjreDsruMk4=' list peers <name1> list peers <name2> config wireguard_test <name> option endpoint_host 'test.de option public_key 'Cs46OJr5d3NoDRYf/g2l0RINYLvaypQCMtchlJhQgSQ='. list allowed_ips '0.0.0.0 Deleting an interface section in LuCI is generic. So I don't know if we should do this and make an exception for wireguard. Best regards Flo
Hi Florian, > Deleting an interface section in LuCI is generic. So I don't know if we should > do this and make an exception for wireguard. proper removal of wg peer sections is in LuCI master and openwrt-19.07 now. ~ Jo
diff --git a/package/network/services/wireguard/Makefile b/package/network/services/wireguard/Makefile index ea34b7550b..d78fcfface 100644 --- a/package/network/services/wireguard/Makefile +++ b/package/network/services/wireguard/Makefile @@ -93,6 +93,8 @@ define Package/wireguard-tools/install $(INSTALL_BIN) ./files/wireguard_watchdog $(1)/usr/bin/ $(INSTALL_DIR) $(1)/lib/netifd/proto/ $(INSTALL_BIN) ./files/wireguard.sh $(1)/lib/netifd/proto/ + $(INSTALL_DIR) $(1)/etc/init.d/ + $(INSTALL_BIN) ./files/wireguard.init $(1)/etc/init.d/wireguard endef define KernelPackage/wireguard diff --git a/package/network/services/wireguard/files/wireguard.init b/package/network/services/wireguard/files/wireguard.init new file mode 100644 index 0000000000..781d0839bc --- /dev/null +++ b/package/network/services/wireguard/files/wireguard.init @@ -0,0 +1,31 @@ +#!/bin/sh /etc/rc.common + +START=80 +USE_PROCD=1 + +service_triggers() { + procd_add_reload_trigger "network" +} + +reload_service() { + # delete old peers of related wireguard interface + wireguard_check_peers +} + +wireguard_check_peers() { + local iface peer + + # get all wireguard peers + for peer in $(uci show network | grep =wireguard_); do + # extract peer section type + peer="${peer##*=}" + # extract interface name + iface="${peer#*_}" + + # delete peer if iface is not present anymore + if ! uci -q show "network.${iface}" 1>/dev/null 2>/dev/null; then + uci -q delete "network.@${peer}[-1]" + uci commit network + fi + done +}
When we delete a Wireguard interface, the associated peer sections are not deleted. They remain in the network configuration. This commit adds an init script, that triggers when the network configuration file is changed. For each change event, each Wireguard peer section is checked to see if the corresponding wireguard interface section still exists. If this is not the case, all wireguard peer sections for that interface are deleted. Signed-off-by: Florian Eckert <fe@dev.tdt.de> --- v2: * update commit description package/network/services/wireguard/Makefile | 2 ++ .../services/wireguard/files/wireguard.init | 31 +++++++++++++++++++ 2 files changed, 33 insertions(+) create mode 100644 package/network/services/wireguard/files/wireguard.init