[WireGuard] Wireguard on OpenWRT/LEDE (here: Luci)

[Jason A. Donenfeld]

 Hey Dan,

This looks beautiful! Thanks for writing it. I'm not a LEDE dev, but I
do have a small bit of preliminary feedback, below.

Thanks for writing this. I'm thrilled!

Jason


1. The first screenshot should be cropped before the start of the
"Peers" section creeps in. Otherwise it looks like there are two
sections for peers. This confused me a bit at first.

2.
- listen_port.placeholder = "500"
+ listen_port.placeholder = "51820"

3. "addresses.placeholder = "fd00:13:37::1/64"" -- might be nice to
give a v4 example too.

4.
- "cryptography for post-quantum resistance")
+ "cryptography for post-quantum resistance.")

5. "Names are resolved prior to establishing the connection for the
first time." It might be more accurate to say that they're resolved at
the time the interface is configured/brought up.

6.
- endpoint.placeholder = "vpn.example.com:500"
+ endpoint.placeholder = "vpn.example.com:51820"

7.
- persistent_keepalive.datatype = "range(0, 3600)"
+ persistent_keepalive.datatype = "range(0, 65535)"

8. This one is more important than it may seem at first, but please
keep the terminology straight and explicit:
- translate("Keep Alive"),
+ translate("Persistent Keep Alive"),

9. ${iface} should be quoted -- "${iface}"

10. 1>/dev/null 2>/dev/null can be shortened to the more idiomatic
>/dev/null 2>&1

11. The existing logic for adding a device is to add it if it doesn't
exist, and otherwise to flush the addresses. Is it a good idea to
flush the routes too? Or simply delete and re-add? Or is a simple
flush of the addresses standard LEDE behavior? I'll defer to Baptiste
(CC'd) on this.

12. mkdir -p is non fatal if the path already exists, so you don't
need to do the -d check before.

13. Touching the configuration file and then recursively chmodding is
not the right way to do things. It opens you up to other types of
security vulnerabilities. Instead, use umask.

Putting 12 and 13 together, we have:

- [ -d "${wg_dir}" ] || mkdir -p "${wg_dir}"
- [ -f "${wg_cfg}" ] || touch "${wg_cfg}"
- chmod -R 0600 "${wg_dir}"
+ umask 077
+ mkdir -p "${wg_dir}"

14.
- allowed_ips=$(echo $allowed_ips | tr ' ' ',')
+ allowed_ips="$(echo "$allowed_ips" | tr ' ' ',')"

You might also want to replace all instances of ,, with , after this
transformation until there aren't any ,, left  just to maintain
sanity.

15. On tear-down, why not just unconditionally delete?