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 examples not working any longer #888

Closed
lmm-git opened this issue Mar 15, 2022 · 10 comments
Closed

WireGuard examples not working any longer #888

lmm-git opened this issue Mar 15, 2022 · 10 comments

Comments

@lmm-git
Copy link

lmm-git commented Mar 15, 2022

Hi,

probably due to #882 the examples for WireGuard interfaces are no longer working.

If I try to:

from pyroute2 import IPDB, WireGuard

def main():
    IFNAME = 'wg1test'
    # Create a WireGuard interface
    with IPDB() as ip:
        wg1 = ip.create(kind='wireguard', ifname=IFNAME)
        wg1.add_ip('10.0.0.1/24')
        wg1.up()
        wg1.commit()
    # Create WireGuard object
    wg = WireGuard()
    # Add a WireGuard configuration + first peer
    peer = {'public_key': 'TGFHcm9zc2VCaWNoZV9DJ2VzdExhUGx1c0JlbGxlPDM=',
            'endpoint_addr': '8.8.8.8',
            'endpoint_port': 8888,
            'persistent_keepalive': 15,
            'allowed_ips': ['10.0.0.0/24', '8.8.8.8/32']}
    wg.set(IFNAME, private_key='RCdhcHJlc0JpY2hlLEplU2VyYWlzTGFQbHVzQm9ubmU=',
           fwmark=0x1337, listen_port=2525, peer=peer)

if __name__ == '__main__':
    main()

With version 0.6.8 wg outputs

interface: wg1test
  public key: FZ8nQlgAg7JIViZTAarlvMb9/e89jLdZ+mc3LBqAOmI=
  private key: (hidden)
  listening port: 2525
  fwmark: 0x1337

peer: TGFHcm9zc2VCaWNoZV9DJ2VzdExhUGx1c0JlbGxlPDM=
  allowed ips: 10.0.0.0/24, 8.8.8.8/32
  persistent keepalive: every 15 seconds

With version 0.6.7 it outputs correctly

interface: wg1test
  public key: FZ8nQlgAg7JIViZTAarlvMb9/e89jLdZ+mc3LBqAOmI=
  private key: (hidden)
  listening port: 2525
  fwmark: 0x1337

peer: TGFHcm9zc2VCaWNoZV9DJ2VzdExhUGx1c0JlbGxlPDM=
  endpoint: 8.8.8.8:8888
  allowed ips: 10.0.0.0/24, 8.8.8.8/32
  transfer: 0 B received, 148 B sent
  persistent keepalive: every 15 seconds

There is no error message or something. In another software, we get with version 0.6.8 the following messages:

unsupported event ignored: <class 'pr2modules.netlink.nlmsg'>

as well as

exception <(22, 'Invalid argument')> in source localhost

/cc @inemajo

@felix-gohla
Copy link

felix-gohla commented Mar 15, 2022

It seems that self['addr4'] now is a byte array instead of an integer. Maybe down the pipeline there is an issue with handling that byte array?

Edit: this was probably not the issue. Setting addr6 to 8s in fields restores the correct behaviour, but I think, 8s does not make sense...

@svinota
Copy link
Owner

svinota commented Mar 15, 2022

Yep, thanks. To be fixed asap.

@svinota svinota added the bug label Mar 15, 2022
@svinota
Copy link
Owner

svinota commented Mar 16, 2022

Nope, it makes sense. Pretty much. This particular NLA size is fixed and now differs from what the kernel expects, and that's the cause. Now let's see how can we fix that keeping compatibility as much as we can.

@inemajo
Copy link
Contributor

inemajo commented Mar 16, 2022

Wireguard in kernel check family and size of data

if (attrs[WGPEER_A_ENDPOINT]) {
		struct sockaddr *addr = nla_data(attrs[WGPEER_A_ENDPOINT]);
		size_t len = nla_len(attrs[WGPEER_A_ENDPOINT]);

		if ((len == sizeof(struct sockaddr_in) &&
		     addr->sa_family == AF_INET) ||
		    (len == sizeof(struct sockaddr_in6) &&
		     addr->sa_family == AF_INET6)) {
			struct endpoint endpoint = { { { 0 } } };

			memcpy(&endpoint.addr, addr, len);
			wg_socket_set_peer_endpoint(peer, &endpoint);
		}
	}

https://github.com/torvalds/linux/blob/master/drivers/net/wireguard/netlink.c

And struct sockaddr_in has got char sin_zero[8];, that's why setting addr6 to 8s in fields restores the correct behaviour I think.

We may put addr6 to s and set it's size, same for scope_id

        class parse_endpoint(nla):
            fields = (
                ('family', 'H'),
                ('port', '>H'),
                ('addr4', '4s'),
                ('addr6', 's'),
                ('scope_id', 's'),
            )

            def decode(self):
                nla.decode(self)
                if self['family'] == AF_INET:
                    self['addr'] = inet_ntop(AF_INET, self['addr4'])
                else:
                    self['addr'] = inet_ntop(AF_INET6, self['addr6'])
                del self['addr4']
                del self['addr6']

            def encode(self):
                if self['addr'].find(":") > -1:
                    self['family'] = AF_INET6
                    self['addr4'] = b'\x00\x00\x00\x00'
                    self['addr6'] = inet_pton(AF_INET6, self['addr'])
                    self['scope_id'] = b'\x00\x00\x00\x00'
                else:
                    self['family'] = AF_INET
                    self['addr4'] = inet_pton(AF_INET, self['addr'])
                    self['addr6'] = b'\x00\x00\x00\x00\x00\x00\x00\x00'  # sin_zero[8]
                    self['scope_id'] = b''
                self['port'] = int(self['port'])
                nla.encode(self)

I can't test now but it should work on ip4&6.
May be a good idea to create a nla_sockaddr for theses prupose ?

@svinota
Copy link
Owner

svinota commented Mar 16, 2022

Yep, just found that code in the kernel. Fixing the library.

@svinota
Copy link
Owner

svinota commented Mar 16, 2022

Fixed, tonight comes the update.

svinota added a commit that referenced this issue Mar 16, 2022
@svinota
Copy link
Owner

svinota commented Mar 16, 2022

Should work. If not — pls let me know. Tests will follow.

svinota added a commit that referenced this issue Mar 16, 2022
@svinota
Copy link
Owner

svinota commented Mar 16, 2022

Tests added.

@lmm-git
Copy link
Author

lmm-git commented Mar 17, 2022

Seems to work again in our use case 👍

@svinota
Copy link
Owner

svinota commented Mar 17, 2022

Great.

The next release will be tagged on Saturday.

@svinota svinota closed this as completed Apr 17, 2022
@svinota svinota added the fixed label Apr 17, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants