openwisp / openwisp/netjsonconfig
wireguard_auto_client hardcodes IPv4 family/mask, and the ipv6 schema branch rejects template variables
Nobody has claimed this yet.
- Dominant language
- Python
- Stars
- 388
- Forks
- 87
- Avg merge
- 1h 26m
- Merged PRs (30d)
- 4
Description
Description
OpenWrt.wireguard_auto_client() hardcodes the client address as IPv4:
# netjsonconfig/backends/openwrt/openwrt.py
if data["client"]["ip_address"]:
config["interfaces"][0]["addresses"] = [
{
"proto": "static",
"family": "ipv4",
"address": data["client"]["ip_address"],
"mask": 32,
},
]
When the WireGuard VPN uses an IPv6 subnet, the generated client address is
still declared family: ipv4 with mask: 32, so the rendered configuration is
wrong for every consumer of this helper. server_ip_network is already passed
in through **kwargs, so the family can be derived rather than assumed.
This is the root cause of openwisp/openwisp-controller#721. A workaround was
proposed controller-side in openwisp/openwisp-controller#1255, and during review
@nemesifier noted the fix really belongs here so every consumer benefits.
Second, related problem
The controller-side workaround cannot actually work, because of an asymmetry in
the WireGuard interface schema. The addresses items use a oneOf with an ipv4
and an ipv6 branch. The ipv6 branch carries "format": "ipv6"; the ipv4 branch
deliberately has no format:
# netjsonconfig/backends/openwrt/schema.py, wireguard_interface
{"title": "ipv4", ... "address": {"type": "string", "minLength": 7, ...}},
{"title": "ipv6", ... "address": {"type": "string", "minLength": 3,
"format": "ipv6", ...}},
BaseBackend.validate() runs Draft4Validator(..., format_checker=format_checker),
so the format is enforced. The WireGuard client address is normally a template
variable that is only resolved at render time, for example
{{ip_address_8841b49ed8344a0c8fb35f8286eccb41}} in OpenWISP. That value passes
the ipv4 branch and fails the ipv6 one.
Steps to reproduce
from netjsonconfig import OpenWrt
base = dict(name="wg0", type="wireguard", private_key="{{pvt_key_x}}", port=51820,
mtu=1420, nohostroute=False, fwmark="", ip6prefix=[], network="")
def check(addr, label):
try:
OpenWrt({"interfaces": [dict(base, addresses=[addr])]}).validate()
print("PASS ", label)
except Exception:
print("FAIL ", label)
check({"proto": "static", "family": "ipv4", "address": "{{ip_address_x}}", "mask": 32}, "ipv4 + template var")
check({"proto": "static", "family": "ipv6", "address": "{{ip_address_x}}", "mask": 128}, "ipv6 + template var")
check({"proto": "static", "family": "ipv6", "address": "fd00::1", "mask": 128}, "ipv6 + literal address")
PASS ipv4 + template var
FAIL ipv6 + template var
PASS ipv6 + literal address
Isolating the failing keyword:
branch 0 (ipv4): enum='ipv6' is not one of ['ipv4']
branch 1 (ipv6): format='{{ip_address_x}}' is not a 'ipv6'
Proposed fix
- Derive
familyandmaskinwireguard_auto_client()from
server_ip_network, defaulting to IPv4 when it is missing or unparsable so
current behavior is preserved. - Drop
"format": "ipv6"from the WireGuard ipv6 address branch, matching the
ipv4 branch, since these addresses are routinely template variables.
I have both changes working with tests and can open a PR.
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start in netjsonconfig/backends/openwrt/openwrt.py at wireguard_auto_client(), then inspect the wireguard_interface address branches in netjsonconfig/backends/openwrt/schema.py. Run the supplied validation examples and compare IPv4, IPv6, literal, and template-variable behavior. Done means IPv6 client settings derive the correct family and mask, and IPv6 template variables validate like IPv4 ones.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- python
- Domain
- backend, networking
- Issue type
- Bug
- Difficulty
- 3/5
- Estimated time
- 1-2 days
- Activity status
- Active
- Clarity
- Clearly specified
- Newbie friendliness
- 72/100