git: 74ae3f3e33b8 - main - if_wg: import latest fixup work from the wireguard-freebsd project

Scott Long scottl at samsco.org
Mon Mar 15 14:56:59 UTC 2021


Here is the response I sent to you and Donenfeld in private.  I won’t include
my direct conversation with you from Slack/IRC, but I made my concerns
and objections pretty clear.  This commit is quite disappointing.  See below:

The good:
- I’m happy that the community is taking a strong interest in wireguard
in FreeBSD.  More hands make light work, and all that, and it’s why
we participate in open source.

- I was excited that some of the issues that people have complained
about were getting addressed, like packet queueing during re-keying.
I know that vnet/jail was a sore spot, and while it’s not something that
Netgate depends on, I knew that it’s important to the community.

- I was happy to hear that there’s going to be a new wireguard-tools
with “official” FreebSD support.  Again, growing ecosystem, community
interest, collaborative work.

- I was happy to learn about the test code being ported to FreeBSD.
We’re ramping up major resources into testing, and I want to get my
hands on anything and everything that will improve quality and drive
better development.

The bad:
- I want to be pragmatic about code APIs.  Maybe iflib isn’t ready for
pseudo interfaces yet, and fixing it is non-trivial and out of scope.
Going back to ifnet puts it back in line with openbsd and likely does
fix the vnet problems.  However, it’s a radical change to the driver, and
not something that I can support as a last-minute action for FreeBSD
13.0 or for pfSense.  Therefore, I’m advising against its immediate
inclusion.  The final choice is not mine to make for FreeBSD, but
that’s my recommendation.  For pfSense, I’ll be discussing this with
the rest of the engineering staff on Monday.

- The LKML wouldn’t accept this kind of submission, they’d insist that
it be broken down into consumable pieces, and that bug fixes be
considered and provided that don’t rely on massive re-writes.  I’ve
been dealing with linux for 20+ years and BSD for almost 30 years,
and I’ve got to say that despite my distaste for how the LKML is run,
they get results.  Does fixing a segfault or packet drop/reorder
require the removal of iflib?

The Ugly:
- An accusation was made, tonight, to me, that the code Netgate 
sponsored was not reviewed and was shoved into the tree at the last
minute.  This grossly ignores the actual history to the point of
weakening my tolerance for this entire discussion.  It shows a pretty
irrational bias against mmacy and Netgate and a gross ignorance of
the history and provenience of the work.

- The Netgate copyright was unilaterally removed.  Yes, it was re-
instated a few minutes ago, and with good reason.  Please don’t
do that again.

- The removal of the ASM crypto bits really confuses me.  An
accusation was made, again tonight, that Netgate merely paid to
benchmark the code, that we contributed nothing to it, and that it’s
bad code that can’t be audited.  What I see is that the meat of the
implementation is copyrighted by Jason and others.  There’s a
modest but non-trivial amount of glue code that has (or had) the
Netgate copyright. I’m not going to touch the audit nonsense.
I need data here, and I’m not seeing it.

There seems to be a lot of bad blood, poor understanding, and
misinformation going on.  I need all of this bullshit to stop.  This
is 5000-ish lines of a nice way to get a point-to-point VPN going
without the hassle of IPSec or the performance problems of
OpenVPN.  It’s consuming way more time and energy than it’s
worth to me, and I’d much rather spend time and money to
implement OpenVPN DCO and work on profiling and optimizing
IPSec.  Wireguard is important to Netgate because we recognize
that it’s a feature that customers want, we’re committed to making
security accessible, and we strongly believe in open source and
FreeBSD. It’s not perfect code, not by a long shot, but I expect
better collaboration and communication than what I’m seeing here.

Scott


> On Mar 14, 2021, at 10:52 PM, Kyle Evans <kevans at FreeBSD.org> wrote:
> 
> The branch main has been updated by kevans:
> 
> URL: https://cgit.FreeBSD.org/src/commit/?id=74ae3f3e33b810248da19004c58b3581cd367843
> 
> commit 74ae3f3e33b810248da19004c58b3581cd367843
> Author:     Kyle Evans <kevans at FreeBSD.org>
> AuthorDate: 2021-03-15 02:25:40 +0000
> Commit:     Kyle Evans <kevans at FreeBSD.org>
> CommitDate: 2021-03-15 04:52:04 +0000
> 
>    if_wg: import latest fixup work from the wireguard-freebsd project
> 
>    This is the culmination of about a week of work from three developers to
>    fix a number of functional and security issues.  This patch consists of
>    work done by the following folks:
> 
>    - Jason A. Donenfeld <Jason at zx2c4.com>
>    - Matt Dunwoodie <ncon at noconroy.net>
>    - Kyle Evans <kevans at FreeBSD.org>
> 
>    Notable changes include:
>    - Packets are now correctly staged for processing once the handshake has
>      completed, resulting in less packet loss in the interim.
>    - Various race conditions have been resolved, particularly w.r.t. socket
>      and packet lifetime (panics)
>    - Various tests have been added to assure correct functionality and
>      tooling conformance
>    - Many security issues have been addressed
>    - if_wg now maintains jail-friendly semantics: sockets are created in
>      the interface's home vnet so that it can act as the sole network
>      connection for a jail
>    - if_wg no longer fails to remove peer allowed-ips of 0.0.0.0/0
>    - if_wg now exports via ioctl a format that is future proof and
>      complete.  It is additionally supported by the upstream
>      wireguard-tools (which we plan to merge in to base soon)
>    - if_wg now conforms to the WireGuard protocol and is more closely
>      aligned with security auditing guidelines
> 
>    Note that the driver has been rebased away from using iflib.  iflib
>    poses a number of challenges for a cloned device trying to operate in a
>    vnet that are non-trivial to solve and adds complexity to the
>    implementation for little gain.
> 
>    The crypto implementation that was previously added to the tree was a
>    super complex integration of what previously appeared in an old out of
>    tree Linux module, which has been reduced to crypto.c containing simple
>    boring reference implementations.  This is part of a near-to-mid term
>    goal to work with FreeBSD kernel crypto folks and take advantage of or
>    improve accelerated crypto already offered elsewhere.
> 
>    There's additional test suite effort underway out-of-tree taking
>    advantage of the aforementioned jail-friendly semantics to test a number
>    of real-world topologies, based on netns.sh.
> 
>    Also note that this is still a work in progress; work going further will
>    be much smaller in nature.
> 
>    MFC after:      1 month (maybe)
> ---
> etc/mtree/BSD.include.dist                         |    2 +
> include/Makefile                                   |    9 +-
> sbin/ifconfig/ifwg.c                               |  395 +-
> share/man/man4/wg.4                                |   26 +-
> sys/dev/if_wg/crypto.c                             | 1705 ++++
> sys/dev/if_wg/crypto.h                             |  114 +
> sys/dev/if_wg/if_wg.c                              | 3454 ++++++++
> sys/dev/if_wg/if_wg.h                              |   36 +
> sys/dev/if_wg/include/crypto/blake2s.h             |   56 -
> sys/dev/if_wg/include/crypto/curve25519.h          |   74 -
> sys/dev/if_wg/include/crypto/zinc.h                |   15 -
> sys/dev/if_wg/include/sys/if_wg_session.h          |   89 -
> sys/dev/if_wg/include/sys/if_wg_session_vars.h     |  319 -
> sys/dev/if_wg/include/sys/simd-x86_64.h            |   74 -
> sys/dev/if_wg/include/sys/support.h                |  342 -
> sys/dev/if_wg/include/sys/wg_module.h              |  121 -
> sys/dev/if_wg/include/sys/wg_noise.h               |  286 -
> sys/dev/if_wg/include/zinc/blake2s.h               |   50 -
> sys/dev/if_wg/include/zinc/chacha20.h              |   68 -
> sys/dev/if_wg/include/zinc/chacha20poly1305.h      |   48 -
> sys/dev/if_wg/include/zinc/curve25519.h            |   28 -
> sys/dev/if_wg/include/zinc/poly1305.h              |   29 -
> sys/dev/if_wg/module/blake2s.c                     |  256 -
> sys/dev/if_wg/module/blake2s.h                     |   58 -
> sys/dev/if_wg/module/chacha20-x86_64.S             | 2834 -------
> .../crypto/zinc/chacha20/chacha20-arm-glue.c       |   98 -
> .../module/crypto/zinc/chacha20/chacha20-arm.pl    | 1227 ---
> .../module/crypto/zinc/chacha20/chacha20-arm64.pl  | 1163 ---
> .../crypto/zinc/chacha20/chacha20-mips-glue.c      |   27 -
> .../module/crypto/zinc/chacha20/chacha20-mips.S    |  424 -
> .../crypto/zinc/chacha20/chacha20-x86_64-glue.c    |  132 -
> .../module/crypto/zinc/chacha20/chacha20-x86_64.pl | 4106 ----------
> .../if_wg/module/crypto/zinc/chacha20/chacha20.c   |  238 -
> .../if_wg/module/crypto/zinc/chacha20poly1305.c    |  196 -
> .../crypto/zinc/poly1305/poly1305-arm-glue.c       |  140 -
> .../module/crypto/zinc/poly1305/poly1305-arm.pl    | 1276 ---
> .../module/crypto/zinc/poly1305/poly1305-arm64.pl  |  974 ---
> .../module/crypto/zinc/poly1305/poly1305-donna32.c |  205 -
> .../module/crypto/zinc/poly1305/poly1305-donna64.c |  182 -
> .../crypto/zinc/poly1305/poly1305-mips-glue.c      |   37 -
> .../module/crypto/zinc/poly1305/poly1305-mips.S    |  407 -
> .../module/crypto/zinc/poly1305/poly1305-mips64.pl |  467 --
> .../crypto/zinc/poly1305/poly1305-x86_64-glue.c    |  171 -
> .../module/crypto/zinc/poly1305/poly1305-x86_64.pl | 4266 ----------
> .../if_wg/module/crypto/zinc/poly1305/poly1305.c   |  163 -
> .../if_wg/module/crypto/zinc/selftest/blake2s.c    | 2090 -----
> .../if_wg/module/crypto/zinc/selftest/chacha20.c   | 2703 -------
> .../module/crypto/zinc/selftest/chacha20poly1305.c | 8443 --------------------
> .../if_wg/module/crypto/zinc/selftest/curve25519.c | 1315 ---
> .../if_wg/module/crypto/zinc/selftest/poly1305.c   | 1110 ---
> sys/dev/if_wg/module/crypto/zinc/selftest/run.h    |   43 -
> sys/dev/if_wg/module/curve25519.c                  |  867 --
> sys/dev/if_wg/module/if_wg_session.c               | 1984 -----
> sys/dev/if_wg/module/module.c                      |  954 ---
> sys/dev/if_wg/module/poly1305-x86_64.S             | 3021 -------
> sys/dev/if_wg/support.h                            |   56 +
> sys/dev/if_wg/{module => }/wg_cookie.c             |  105 +-
> sys/dev/if_wg/{include/sys => }/wg_cookie.h        |   81 +-
> sys/dev/if_wg/{module => }/wg_noise.c              |  409 +-
> sys/dev/if_wg/wg_noise.h                           |  191 +
> sys/kern/kern_jail.c                               |    1 +
> sys/kern/uipc_socket.c                             |   11 +
> sys/kern/uipc_syscalls.c                           |    4 +-
> sys/modules/if_wg/Makefile                         |   30 +-
> sys/net/if_types.h                                 |    1 +
> sys/netinet6/nd6.c                                 |    4 +-
> sys/sys/priv.h                                     |    1 +
> sys/sys/socketvar.h                                |    1 +
> tests/sys/netinet/Makefile                         |   10 +-
> tests/sys/netinet/if_wg_test.sh                    |  188 +
> 70 files changed, 6333 insertions(+), 43677 deletions(-)
> 
> diff --git a/etc/mtree/BSD.include.dist b/etc/mtree/BSD.include.dist
> index e7784cbb0a47..0f85798815d5 100644
> --- a/etc/mtree/BSD.include.dist
> +++ b/etc/mtree/BSD.include.dist
> @@ -64,6 +64,8 @@
>         ..
>         iicbus
>         ..
> +        if_wg
> +        ..
>         io
>         ..
>         mfi
> diff --git a/include/Makefile b/include/Makefile
> index 3a34ddb8aa18..31e207f6b199 100644
> --- a/include/Makefile
> +++ b/include/Makefile
> @@ -44,7 +44,7 @@ LDIRS=	bsm cam geom net net80211 netgraph netinet netinet6 \
> LSUBDIRS=	cam/ata cam/mmc cam/nvme cam/scsi \
> 	dev/acpica dev/agp dev/an dev/ciss dev/filemon dev/firewire \
> 	dev/hwpmc dev/hyperv \
> -	dev/ic dev/iicbus dev/io dev/mfi dev/mmc dev/nvme \
> +	dev/ic dev/iicbus dev/if_wg dev/io dev/mfi dev/mmc dev/nvme \
> 	dev/ofw dev/pbio dev/pci ${_dev_powermac_nvram} dev/ppbus dev/pwm \
> 	dev/smbus dev/speaker dev/tcp_log dev/veriexec dev/vkbd \
> 	fs/devfs fs/fdescfs fs/msdosfs fs/nfs fs/nullfs \
> @@ -170,6 +170,10 @@ NVPAIRDIR=	${INCLUDEDIR}/sys
> MLX5=		mlx5io.h
> MLX5DIR=	${INCLUDEDIR}/dev/mlx5
> 
> +.PATH: ${SRCTOP}/sys/dev/if_wg
> +WG=		if_wg.h
> +WGDIR=		${INCLUDEDIR}/dev/if_wg
> +
> INCSGROUPS=	INCS \
> 		ACPICA \
> 		AGP \
> @@ -182,7 +186,8 @@ INCSGROUPS=	INCS \
> 		PCI \
> 		RPC \
> 		TEKEN \
> -		VERIEXEC
> +		VERIEXEC \
> +		WG
> 
> .if ${MK_IPFILTER} != "no"
> INCSGROUPS+=	IPFILTER
> diff --git a/sbin/ifconfig/ifwg.c b/sbin/ifconfig/ifwg.c
> index 86bacc59f50d..a102f392cf80 100644
> --- a/sbin/ifconfig/ifwg.c
> +++ b/sbin/ifconfig/ifwg.c
> @@ -46,6 +46,8 @@ __FBSDID("$FreeBSD$");
> #include <netinet/in.h>
> #include <arpa/inet.h>
> 
> +#include <dev/if_wg/if_wg.h>
> +
> #include <assert.h>
> #include <ctype.h>
> #include <err.h>
> @@ -65,40 +67,60 @@ __FBSDID("$FreeBSD$");
> 
> #include "ifconfig.h"
> 
> -typedef enum {
> -	WGC_GET = 0x5,
> -	WGC_SET = 0x6,
> -} wg_cmd_t;
> +static void wgfinish(int s, void *arg);
> +
> +static bool wgfinish_registered;
> 
> -static nvlist_t *nvl_params;
> -static bool do_peer;
> static int allowed_ips_count;
> static int allowed_ips_max;
> -struct allowedip {
> -	struct sockaddr_storage a_addr;
> -	struct sockaddr_storage a_mask;
> -};
> -struct allowedip *allowed_ips;
> +static nvlist_t **allowed_ips, *nvl_peer;
> 
> #define	ALLOWEDIPS_START 16
> -#define	WG_KEY_LEN 32
> -#define	WG_KEY_LEN_BASE64 ((((WG_KEY_LEN) + 2) / 3) * 4 + 1)
> -#define	WG_KEY_LEN_HEX (WG_KEY_LEN * 2 + 1)
> +#define	WG_KEY_SIZE_BASE64 ((((WG_KEY_SIZE) + 2) / 3) * 4 + 1)
> +#define	WG_KEY_SIZE_HEX (WG_KEY_SIZE * 2 + 1)
> #define	WG_MAX_STRLEN 64
> 
> +struct allowedip {
> +	union {
> +		struct in_addr ip4;
> +		struct in6_addr ip6;
> +	};
> +};
> +
> +static void
> +register_wgfinish(void)
> +{
> +
> +	if (wgfinish_registered)
> +		return;
> +	callback_register(wgfinish, NULL);
> +	wgfinish_registered = true;
> +}
> +
> +static nvlist_t *
> +nvl_device(void)
> +{
> +	static nvlist_t *_nvl_device;
> +
> +	if (_nvl_device == NULL)
> +		_nvl_device = nvlist_create(0);
> +	register_wgfinish();
> +	return (_nvl_device);
> +}
> +
> static bool
> -key_from_base64(uint8_t key[static WG_KEY_LEN], const char *base64)
> +key_from_base64(uint8_t key[static WG_KEY_SIZE], const char *base64)
> {
> 
> -	if (strlen(base64) != WG_KEY_LEN_BASE64 - 1) {
> -		warnx("bad key len - need %d got %zu\n", WG_KEY_LEN_BASE64 - 1, strlen(base64));
> +	if (strlen(base64) != WG_KEY_SIZE_BASE64 - 1) {
> +		warnx("bad key len - need %d got %zu\n", WG_KEY_SIZE_BASE64 - 1, strlen(base64));
> 		return false;
> 	}
> -	if (base64[WG_KEY_LEN_BASE64 - 2] != '=') {
> -		warnx("bad key terminator, expected '=' got '%c'", base64[WG_KEY_LEN_BASE64 - 2]);
> +	if (base64[WG_KEY_SIZE_BASE64 - 2] != '=') {
> +		warnx("bad key terminator, expected '=' got '%c'", base64[WG_KEY_SIZE_BASE64 - 2]);
> 		return false;
> 	}
> -	return (b64_pton(base64, key, WG_KEY_LEN));
> +	return (b64_pton(base64, key, WG_KEY_SIZE));
> }
> 
> static void
> @@ -128,7 +150,7 @@ parse_endpoint(const char *endpoint_)
> 	err = getaddrinfo(endpoint, port, &hints, &res);
> 	if (err)
> 		errx(1, "%s", gai_strerror(err));
> -	nvlist_add_binary(nvl_params, "endpoint", res->ai_addr, res->ai_addrlen);
> +	nvlist_add_binary(nvl_peer, "endpoint", res->ai_addr, res->ai_addrlen);
> 	freeaddrinfo(res);
> 	free(base);
> }
> @@ -227,12 +249,14 @@ in6_mask2len(struct in6_addr *mask, u_char *lim0)
> }
> 
> static bool
> -parse_ip(struct allowedip *aip, const char *value)
> +parse_ip(struct allowedip *aip, uint16_t *family, const char *value)
> {
> 	struct addrinfo hints, *res;
> 	int err;
> +	bool ret;
> 
> -	bzero(&aip->a_addr, sizeof(aip->a_addr));
> +	ret = true;
> +	bzero(aip, sizeof(*aip));
> 	bzero(&hints, sizeof(hints));
> 	hints.ai_family = AF_UNSPEC;
> 	hints.ai_flags = AI_NUMERICHOST;
> @@ -240,10 +264,21 @@ parse_ip(struct allowedip *aip, const char *value)
> 	if (err)
> 		errx(1, "%s", gai_strerror(err));
> 
> -	memcpy(&aip->a_addr, res->ai_addr, res->ai_addrlen);
> +	*family = res->ai_family;
> +	if (res->ai_family == AF_INET) {
> +		struct sockaddr_in *sin = (struct sockaddr_in *)res->ai_addr;
> +
> +		aip->ip4 = sin->sin_addr;
> +	} else if (res->ai_family == AF_INET6) {
> +		struct sockaddr_in6 *sin6 = (struct sockaddr_in6 *)res->ai_addr;
> +
> +		aip->ip6 = sin6->sin6_addr;
> +	} else {
> +		ret = false;
> +	}
> 
> 	freeaddrinfo(res);
> -	return (true);
> +	return (ret);
> }
> 
> static void
> @@ -271,61 +306,84 @@ sa_ntop(const struct sockaddr *sa, char *buf, int *port)
> }
> 
> static void
> -dump_peer(const nvlist_t *nvl_peer)
> +dump_peer(const nvlist_t *nvl_peer_cfg)
> {
> 	const void *key;
> -	const struct allowedip *aips;
> 	const struct sockaddr *endpoint;
> 	char outbuf[WG_MAX_STRLEN];
> 	char addr_buf[INET6_ADDRSTRLEN];
> -	size_t size;
> -	int count, port;
> +	size_t aip_count, size;
> +	int port;
> 	uint16_t persistent_keepalive;
> +	const nvlist_t * const *nvl_aips;
> 
> 	printf("[Peer]\n");
> -	if (nvlist_exists_binary(nvl_peer, "public-key")) {
> -		key = nvlist_get_binary(nvl_peer, "public-key", &size);
> +	if (nvlist_exists_binary(nvl_peer_cfg, "public-key")) {
> +		key = nvlist_get_binary(nvl_peer_cfg, "public-key", &size);
> 		b64_ntop((const uint8_t *)key, size, outbuf, WG_MAX_STRLEN);
> 		printf("PublicKey = %s\n", outbuf);
> 	}
> -	if (nvlist_exists_binary(nvl_peer, "endpoint")) {
> -		endpoint = nvlist_get_binary(nvl_peer, "endpoint", &size);
> +	if (nvlist_exists_binary(nvl_peer_cfg, "preshared-key")) {
> +		key = nvlist_get_binary(nvl_peer_cfg, "preshared-key", &size);
> +		b64_ntop((const uint8_t *)key, size, outbuf, WG_MAX_STRLEN);
> +		printf("PresharedKey = %s\n", outbuf);
> +	}
> +	if (nvlist_exists_binary(nvl_peer_cfg, "endpoint")) {
> +		endpoint = nvlist_get_binary(nvl_peer_cfg, "endpoint", &size);
> 		sa_ntop(endpoint, addr_buf, &port);
> 		printf("Endpoint = %s:%d\n", addr_buf, ntohs(port));
> 	}
> -	if (nvlist_exists_number(nvl_peer, "persistent-keepalive-interval")) {
> -		persistent_keepalive = nvlist_get_number(nvl_peer,
> +	if (nvlist_exists_number(nvl_peer_cfg,
> +	    "persistent-keepalive-interval")) {
> +		persistent_keepalive = nvlist_get_number(nvl_peer_cfg,
> 		    "persistent-keepalive-interval");
> 		printf("PersistentKeepalive = %d\n", persistent_keepalive);
> 	}
> -	if (!nvlist_exists_binary(nvl_peer, "allowed-ips"))
> +	if (!nvlist_exists_nvlist_array(nvl_peer_cfg, "allowed-ips"))
> 		return;
> -	aips = nvlist_get_binary(nvl_peer, "allowed-ips", &size);
> -	if (size == 0 || size % sizeof(struct allowedip) != 0) {
> -		errx(1, "size %zu not integer multiple of allowedip", size);
> -	}
> +
> +	nvl_aips = nvlist_get_nvlist_array(nvl_peer_cfg, "allowed-ips", &aip_count);
> +	if (nvl_aips == NULL || aip_count == 0)
> +		return;
> +
> 	printf("AllowedIPs = ");
> -	count = size / sizeof(struct allowedip);
> -	for (int i = 0; i < count; i++) {
> -		int mask;
> +	for (size_t i = 0; i < aip_count; i++) {
> +		uint8_t cidr;
> +		struct sockaddr_storage ss;
> 		sa_family_t family;
> -		void *bitmask;
> -		struct sockaddr *sa;
> -
> -		sa = __DECONST(void *, &aips[i].a_addr);
> -		bitmask = __DECONST(void *,
> -		    ((const struct sockaddr *)&(&aips[i])->a_mask)->sa_data);
> -		family = aips[i].a_addr.ss_family;
> -		getnameinfo(sa, sa->sa_len, addr_buf, INET6_ADDRSTRLEN, NULL,
> -		    0, NI_NUMERICHOST);
> -		if (family == AF_INET)
> -			mask = in_mask2len(bitmask);
> -		else if (family == AF_INET6)
> -			mask = in6_mask2len(bitmask, NULL);
> -		else
> -			errx(1, "bad family in peer %d\n", family);
> -		printf("%s/%d", addr_buf, mask);
> -		if (i < count -1)
> +
> +		if (!nvlist_exists_number(nvl_aips[i], "cidr"))
> +			continue;
> +		cidr = nvlist_get_number(nvl_aips[i], "cidr");
> +		if (nvlist_exists_binary(nvl_aips[i], "ipv4")) {
> +			struct sockaddr_in *sin = (struct sockaddr_in *)&ss;
> +			const struct in_addr *ip4;
> +
> +			ip4 = nvlist_get_binary(nvl_aips[i], "ipv4", &size);
> +			if (ip4 == NULL || cidr > 32)
> +				continue;
> +			sin->sin_len = sizeof(*sin);
> +			sin->sin_family = AF_INET;
> +			sin->sin_addr = *ip4;
> +		} else if (nvlist_exists_binary(nvl_aips[i], "ipv6")) {
> +			struct sockaddr_in6 *sin6 = (struct sockaddr_in6 *)&ss;
> +			const struct in6_addr *ip6;
> +
> +			ip6 = nvlist_get_binary(nvl_aips[i], "ipv6", &size);
> +			if (ip6 == NULL || cidr > 128)
> +				continue;
> +			sin6->sin6_len = sizeof(*sin6);
> +			sin6->sin6_family = AF_INET6;
> +			sin6->sin6_addr = *ip6;
> +		} else {
> +			continue;
> +		}
> +
> +		family = ss.ss_family;
> +		getnameinfo((struct sockaddr *)&ss, ss.ss_len, addr_buf,
> +		    INET6_ADDRSTRLEN, NULL, 0, NI_NUMERICHOST);
> +		printf("%s/%d", addr_buf, cidr);
> +		if (i < aip_count - 1)
> 			printf(", ");
> 	}
> 	printf("\n");
> @@ -334,36 +392,34 @@ dump_peer(const nvlist_t *nvl_peer)
> static int
> get_nvl_out_size(int sock, u_long op, size_t *size)
> {
> -	struct ifdrv ifd;
> +	struct wg_data_io wgd;
> 	int err;
> 
> -	memset(&ifd, 0, sizeof(ifd));
> +	memset(&wgd, 0, sizeof(wgd));
> 
> -	strlcpy(ifd.ifd_name, name, sizeof(ifd.ifd_name));
> -	ifd.ifd_cmd = op;
> -	ifd.ifd_len = 0;
> -	ifd.ifd_data = NULL;
> +	strlcpy(wgd.wgd_name, name, sizeof(wgd.wgd_name));
> +	wgd.wgd_size = 0;
> +	wgd.wgd_data = NULL;
> 
> -	err = ioctl(sock, SIOCGDRVSPEC, &ifd);
> +	err = ioctl(sock, op, &wgd);
> 	if (err)
> 		return (err);
> -	*size = ifd.ifd_len;
> +	*size = wgd.wgd_size;
> 	return (0);
> }
> 
> static int
> do_cmd(int sock, u_long op, void *arg, size_t argsize, int set)
> {
> -	struct ifdrv ifd;
> +	struct wg_data_io wgd;
> 
> -	memset(&ifd, 0, sizeof(ifd));
> +	memset(&wgd, 0, sizeof(wgd));
> 
> -	strlcpy(ifd.ifd_name, name, sizeof(ifd.ifd_name));
> -	ifd.ifd_cmd = op;
> -	ifd.ifd_len = argsize;
> -	ifd.ifd_data = arg;
> +	strlcpy(wgd.wgd_name, name, sizeof(wgd.wgd_name));
> +	wgd.wgd_size = argsize;
> +	wgd.wgd_data = arg;
> 
> -	return (ioctl(sock, set ? SIOCSDRVSPEC : SIOCGDRVSPEC, &ifd));
> +	return (ioctl(sock, op, &wgd));
> }
> 
> static
> @@ -371,62 +427,83 @@ DECL_CMD_FUNC(peerlist, val, d)
> {
> 	size_t size, peercount;
> 	void *packed;
> -	const nvlist_t *nvl, *nvl_peer;
> +	const nvlist_t *nvl;
> 	const nvlist_t *const *nvl_peerlist;
> 
> -	if (get_nvl_out_size(s, WGC_GET, &size))
> +	if (get_nvl_out_size(s, SIOCGWG, &size))
> 		errx(1, "can't get peer list size");
> 	if ((packed = malloc(size)) == NULL)
> 		errx(1, "malloc failed for peer list");
> -	if (do_cmd(s, WGC_GET, packed, size, 0))
> +	if (do_cmd(s, SIOCGWG, packed, size, 0))
> 		errx(1, "failed to obtain peer list");
> 
> 	nvl = nvlist_unpack(packed, size, 0);
> -	if (!nvlist_exists_nvlist_array(nvl, "peer-list"))
> +	if (!nvlist_exists_nvlist_array(nvl, "peers"))
> 		return;
> -	nvl_peerlist = nvlist_get_nvlist_array(nvl, "peer-list", &peercount);
> +	nvl_peerlist = nvlist_get_nvlist_array(nvl, "peers", &peercount);
> 
> 	for (int i = 0; i < peercount; i++, nvl_peerlist++) {
> -		nvl_peer = *nvl_peerlist;
> -		dump_peer(nvl_peer);
> +		dump_peer(*nvl_peerlist);
> 	}
> }
> 
> static void
> -peerfinish(int s, void *arg)
> +wgfinish(int s, void *arg)
> {
> -	nvlist_t *nvl, **nvl_array;
> 	void *packed;
> 	size_t size;
> +	static nvlist_t *nvl_dev;
> +
> +	nvl_dev = nvl_device();
> +	if (nvl_peer != NULL) {
> +		if (!nvlist_exists_binary(nvl_peer, "public-key"))
> +			errx(1, "must specify a public-key for adding peer");
> +		if (allowed_ips_count != 0) {
> +			nvlist_add_nvlist_array(nvl_peer, "allowed-ips",
> +			    (const nvlist_t * const *)allowed_ips,
> +			    allowed_ips_count);
> +			for (size_t i = 0; i < allowed_ips_count; i++) {
> +				nvlist_destroy(allowed_ips[i]);
> +			}
> +
> +			free(allowed_ips);
> +		}
> +
> +		nvlist_add_nvlist_array(nvl_dev, "peers",
> +		    (const nvlist_t * const *)&nvl_peer, 1);
> +	}
> +
> +	packed = nvlist_pack(nvl_dev, &size);
> 
> -	if ((nvl = nvlist_create(0)) == NULL)
> -		errx(1, "failed to allocate nvlist");
> -	if ((nvl_array = calloc(sizeof(void *), 1)) == NULL)
> -		errx(1, "failed to allocate nvl_array");
> -	if (!nvlist_exists_binary(nvl_params, "public-key"))
> -		errx(1, "must specify a public-key for adding peer");
> -	if (allowed_ips_count == 0)
> -		errx(1, "must specify at least one range of allowed-ips to add a peer");
> -
> -	nvl_array[0] = nvl_params;
> -	nvlist_add_nvlist_array(nvl, "peer-list", (const nvlist_t * const *)nvl_array, 1);
> -	packed = nvlist_pack(nvl, &size);
> -
> -	if (do_cmd(s, WGC_SET, packed, size, true))
> -		errx(1, "failed to install peer");
> +	if (do_cmd(s, SIOCSWG, packed, size, true))
> +		errx(1, "failed to configure");
> }
> 
> static
> DECL_CMD_FUNC(peerstart, val, d)
> {
> -	do_peer = true;
> -	callback_register(peerfinish, NULL);
> -	allowed_ips = malloc(ALLOWEDIPS_START * sizeof(struct allowedip));
> +
> +	if (nvl_peer != NULL)
> +		errx(1, "cannot both add and remove a peer");
> +	register_wgfinish();
> +	nvl_peer = nvlist_create(0);
> +	allowed_ips = calloc(ALLOWEDIPS_START, sizeof(*allowed_ips));
> 	allowed_ips_max = ALLOWEDIPS_START;
> 	if (allowed_ips == NULL)
> 		errx(1, "failed to allocate array for allowedips");
> }
> 
> +static
> +DECL_CMD_FUNC(peerdel, val, d)
> +{
> +
> +	if (nvl_peer != NULL)
> +		errx(1, "cannot both add and remove a peer");
> +	register_wgfinish();
> +	nvl_peer = nvlist_create(0);
> +	nvlist_add_bool(nvl_peer, "remove", true);
> +}
> +
> static
> DECL_CMD_FUNC(setwglistenport, val, d)
> {
> @@ -454,39 +531,53 @@ DECL_CMD_FUNC(setwglistenport, val, d)
> 		errx(1, "unknown family");
> 	}
> 	ul = ntohs((u_short)ul);
> -	nvlist_add_number(nvl_params, "listen-port", ul);
> +	nvlist_add_number(nvl_device(), "listen-port", ul);
> }
> 
> static
> DECL_CMD_FUNC(setwgprivkey, val, d)
> {
> -	uint8_t key[WG_KEY_LEN];
> +	uint8_t key[WG_KEY_SIZE];
> 
> 	if (!key_from_base64(key, val))
> 		errx(1, "invalid key %s", val);
> -	nvlist_add_binary(nvl_params, "private-key", key, WG_KEY_LEN);
> +	nvlist_add_binary(nvl_device(), "private-key", key, WG_KEY_SIZE);
> }
> 
> static
> DECL_CMD_FUNC(setwgpubkey, val, d)
> {
> -	uint8_t key[WG_KEY_LEN];
> +	uint8_t key[WG_KEY_SIZE];
> 
> -	if (!do_peer)
> +	if (nvl_peer == NULL)
> 		errx(1, "setting public key only valid when adding peer");
> 
> 	if (!key_from_base64(key, val))
> 		errx(1, "invalid key %s", val);
> -	nvlist_add_binary(nvl_params, "public-key", key, WG_KEY_LEN);
> +	nvlist_add_binary(nvl_peer, "public-key", key, WG_KEY_SIZE);
> }
> 
> +static
> +DECL_CMD_FUNC(setwgpresharedkey, val, d)
> +{
> +	uint8_t key[WG_KEY_SIZE];
> +
> +	if (nvl_peer == NULL)
> +		errx(1, "setting preshared-key only valid when adding peer");
> +
> +	if (!key_from_base64(key, val))
> +		errx(1, "invalid key %s", val);
> +	nvlist_add_binary(nvl_peer, "preshared-key", key, WG_KEY_SIZE);
> +}
> +
> +
> static
> DECL_CMD_FUNC(setwgpersistentkeepalive, val, d)
> {
> 	unsigned long persistent_keepalive;
> 	char *endp;
> 
> -	if (!do_peer)
> +	if (nvl_peer == NULL)
> 		errx(1, "setting persistent keepalive only valid when adding peer");
> 
> 	errno = 0;
> @@ -496,7 +587,7 @@ DECL_CMD_FUNC(setwgpersistentkeepalive, val, d)
> 	if (persistent_keepalive > USHRT_MAX)
> 		errx(1, "persistent-keepalive '%lu' too large",
> 		    persistent_keepalive);
> -	nvlist_add_number(nvl_params, "persistent-keepalive-interval",
> +	nvlist_add_number(nvl_peer, "persistent-keepalive-interval",
> 	    persistent_keepalive);
> }
> 
> @@ -506,45 +597,57 @@ DECL_CMD_FUNC(setallowedips, val, d)
> 	char *base, *allowedip, *mask;
> 	u_long ul;
> 	char *endp;
> -	struct allowedip *aip;
> +	struct allowedip aip;
> +	nvlist_t *nvl_aip;
> +	uint16_t family;
> 
> -	if (!do_peer)
> +	if (nvl_peer == NULL)
> 		errx(1, "setting allowed ip only valid when adding peer");
> 	if (allowed_ips_count == allowed_ips_max) {
> -		/* XXX grow array */
> +		allowed_ips_max *= 2;
> +		allowed_ips = reallocarray(allowed_ips, allowed_ips_max,
> +		    sizeof(*allowed_ips));
> +		if (allowed_ips == NULL)
> +			errx(1, "failed to grow allowed ip array");
> 	}
> -	aip = &allowed_ips[allowed_ips_count];
> +
> +	allowed_ips[allowed_ips_count] = nvl_aip = nvlist_create(0);
> +	if (nvl_aip == NULL)
> +		errx(1, "failed to create new allowedip nvlist");
> +
> 	base = allowedip = strdup(val);
> 	mask = index(allowedip, '/');
> 	if (mask == NULL)
> 		errx(1, "mask separator not found in allowedip %s", val);
> 	*mask = '\0';
> 	mask++;
> -	parse_ip(aip, allowedip);
> +
> +	parse_ip(&aip, &family, allowedip);
> 	ul = strtoul(mask, &endp, 0);
> 	if (*endp != '\0')
> 		errx(1, "invalid value for allowedip mask");
> -	bzero(&aip->a_mask, sizeof(aip->a_mask));
> -	if (aip->a_addr.ss_family == AF_INET)
> -		in_len2mask((struct in_addr *)&((struct sockaddr *)&aip->a_mask)->sa_data, ul);
> -	else if (aip->a_addr.ss_family == AF_INET6)
> -		in6_prefixlen2mask((struct in6_addr *)&((struct sockaddr *)&aip->a_mask)->sa_data, ul);
> -	else
> -		errx(1, "invalid address family %d\n", aip->a_addr.ss_family);
> +
> +	nvlist_add_number(nvl_aip, "cidr", ul);
> +	if (family == AF_INET) {
> +		nvlist_add_binary(nvl_aip, "ipv4", &aip.ip4, sizeof(aip.ip4));
> +	} else if (family == AF_INET6) {
> +		nvlist_add_binary(nvl_aip, "ipv6", &aip.ip6, sizeof(aip.ip6));
> +	} else {
> +		/* Shouldn't happen */
> +		nvlist_destroy(nvl_aip);
> +		goto out;
> +	}
> +
> 	allowed_ips_count++;
> -	if (allowed_ips_count > 1)
> -		nvlist_free_binary(nvl_params, "allowed-ips");
> -	nvlist_add_binary(nvl_params, "allowed-ips", allowed_ips,
> -					  allowed_ips_count*sizeof(*aip));
> 
> -	dump_peer(nvl_params);
> +out:
> 	free(base);
> }
> 
> static
> DECL_CMD_FUNC(setendpoint, val, d)
> {
> -	if (!do_peer)
> +	if (nvl_peer == NULL)
> 		errx(1, "setting endpoint only valid when adding peer");
> 	parse_endpoint(val);
> }
> @@ -555,15 +658,15 @@ wireguard_status(int s)
> 	size_t size;
> 	void *packed;
> 	nvlist_t *nvl;
> -	char buf[WG_KEY_LEN_BASE64];
> +	char buf[WG_KEY_SIZE_BASE64];
> 	const void *key;
> 	uint16_t listen_port;
> 
> -	if (get_nvl_out_size(s, WGC_GET, &size))
> +	if (get_nvl_out_size(s, SIOCGWG, &size))
> 		return;
> 	if ((packed = malloc(size)) == NULL)
> 		return;
> -	if (do_cmd(s, WGC_GET, packed, size, 0))
> +	if (do_cmd(s, SIOCGWG, packed, size, 0))
> 		return;
> 	nvl = nvlist_unpack(packed, size, 0);
> 	if (nvlist_exists_number(nvl, "listen-port")) {
> @@ -583,10 +686,14 @@ wireguard_status(int s)
> }
> 
> static struct cmd wireguard_cmds[] = {
> -    DEF_CLONE_CMD_ARG("listen-port",  setwglistenport),
> -    DEF_CLONE_CMD_ARG("private-key",  setwgprivkey),
> +    DEF_CMD_ARG("listen-port",  setwglistenport),
> +    DEF_CMD_ARG("private-key",  setwgprivkey),
> +    /* XXX peer-list is deprecated. */
>     DEF_CMD("peer-list",  0, peerlist),
> +    DEF_CMD("peers",  0, peerlist),
>     DEF_CMD("peer",  0, peerstart),
> +    DEF_CMD("-peer",  0, peerdel),
> +    DEF_CMD_ARG("preshared-key",  setwgpresharedkey),
>     DEF_CMD_ARG("public-key",  setwgpubkey),
>     DEF_CMD_ARG("persistent-keepalive",  setwgpersistentkeepalive),
>     DEF_CMD_ARG("allowed-ips",  setallowedips),
> @@ -602,27 +709,10 @@ static struct afswtch af_wireguard = {
> static void
> wg_create(int s, struct ifreq *ifr)
> {
> -	struct iovec iov;
> -	void *packed;
> -	size_t size;
> 
> 	setproctitle("ifconfig %s create ...\n", name);
> -	if (!nvlist_exists_number(nvl_params, "listen-port"))
> -		goto legacy;
> -	if (!nvlist_exists_binary(nvl_params, "private-key"))
> -		goto legacy;
> -
> -	packed = nvlist_pack(nvl_params, &size);
> -	if (packed == NULL)
> -		errx(1, "failed to setup create request");
> -	iov.iov_len = size;
> -	iov.iov_base = packed;
> -	ifr->ifr_data = (caddr_t)&iov;
> -	if (ioctl(s, SIOCIFCREATE2, ifr) < 0)
> -		err(1, "SIOCIFCREATE2");
> -	return;
> -legacy:
> -	ifr->ifr_data == NULL;
> +
> +	ifr->ifr_data = NULL;
> 	if (ioctl(s, SIOCIFCREATE, ifr) < 0)
> 		err(1, "SIOCIFCREATE");
> }
> @@ -632,7 +722,6 @@ wireguard_ctor(void)
> {
> 	int i;
> 
> -	nvl_params = nvlist_create(0);
> 	for (i = 0; i < nitems(wireguard_cmds);  i++)
> 		cmd_register(&wireguard_cmds[i]);
> 	af_register(&af_wireguard);
> diff --git a/share/man/man4/wg.4 b/share/man/man4/wg.4
> index 335d3e70b64a..29215bd438ff 100644
> --- a/share/man/man4/wg.4
> +++ b/share/man/man4/wg.4
> @@ -23,7 +23,7 @@
> .\"
> .\" $FreeBSD$
> .\"
> -.Dd March 9, 2021
> +.Dd March 12, 2021
> .Dt WG 4
> .Os
> .Sh NAME
> @@ -68,7 +68,7 @@ interface.
> The private key of the
> .Nm
> interface.
> -.It Cm pre-shared-key
> +.It Cm preshared-key
> Defines a pre-shared key for the
> .Nm
> interface.
> @@ -76,9 +76,9 @@ interface.
> A list of allowed IP addresses.
> .It Cm endpoint
> The IP address of the WiredGuard to connect to.
> -.It Cm peer-list
> +.It Cm peers
> A list of peering IP addresses to connect to.
> -.It Cm persistent-keepalive
> +.It Cm persistent-keepalive-interval
> Interval, in seconds, at which to send persistent keepalive packets.
> .El
> .Pp
> @@ -188,6 +188,11 @@ Connect to a specific endpoint using its public-key and set the allowed IP addre
> .Bd -literal -offset indent
> # ifconfig wg0 peer public-key '7lWtsDdqaGB3EY9WNxRN3hVaHMtu1zXw71+bOjNOVUw=' endpoint 10.0.1.100:54321 allowed-ips 192.168.2.100/32
> .Ed
> +.Pp
> +Remove a peer
> +.Bd -literal -offset indent
> +# ifconfig wg0 -peer public-key '7lWtsDdqaGB3EY9WNxRN3hVaHMtu1zXw71+bOjNOVUw='
> +.Ed
> .Sh DIAGNOSTICS
> The
> .Nm
> @@ -240,14 +245,11 @@ device driver first appeared in
> .Sh AUTHORS
> The
> .Nm
> -device driver was originally written for
> -.Ox
> -by
> -.An Matt Dunwoodie Aq Mt ncon at nconroy.net
> -and ported to
> -.Fx
> -by
> -.An Matt Macy Aq Mt mmacy at FreeBSD.org .
> +device driver written by
> +.An Jason A. Donenfeld Aq Mt Jason at zx2c4.com ,
> +.An Matt Dunwoodie Aq Mt ncon at nconroy.net ,
> +and
> +.An Kyle Evans Aq Mt kevans at FreeBSD.org .
> .Pp
> This manual page was written by
> .An Gordon Bergling Aq Mt gbe at FreeBSD.org
> diff --git a/sys/dev/if_wg/crypto.c b/sys/dev/if_wg/crypto.c
> new file mode 100644
> index 000000000000..f28585429272
> --- /dev/null
> +++ b/sys/dev/if_wg/crypto.c
> @@ -0,0 +1,1705 @@
> +/*
> + * Copyright (C) 2015-2021 Jason A. Donenfeld <Jason at zx2c4.com>. All Rights Reserved.
> + *
> + * Permission to use, copy, modify, and distribute this software for any
> + * purpose with or without fee is hereby granted, provided that the above
> + * copyright notice and this permission notice appear in all copies.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES
> + * WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF
> + * MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR
> + * ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES
> + * WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN
> + * ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF
> + * OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
> + */
> +
> +#include <sys/types.h>
> +#include <sys/endian.h>
> +#include <sys/systm.h>
> +
> +#include "crypto.h"
> +
> +#ifndef ARRAY_SIZE
> +#define ARRAY_SIZE(x) (sizeof(x) / sizeof((x)[0]))
> +#endif
> +#ifndef noinline
> +#define noinline __attribute__((noinline))
> +#endif
> +#ifndef __aligned
> +#define __aligned(x) __attribute__((aligned(x)))
> +#endif
> +#ifndef DIV_ROUND_UP
> +#define DIV_ROUND_UP(n,d) (((n) + (d) - 1) / (d))
> +#endif
> +
> +#define le32_to_cpup(a) le32toh(*(a))
> +#define le64_to_cpup(a) le64toh(*(a))
> +#define cpu_to_le32(a) htole32(a)
> +#define cpu_to_le64(a) htole64(a)
> +
> +static inline uint32_t get_unaligned_le32(const uint8_t *a)
> +{
> +	uint32_t l;
> +	__builtin_memcpy(&l, a, sizeof(l));
> +	return le32_to_cpup(&l);
> +}
> +static inline uint64_t get_unaligned_le64(const uint8_t *a)
> +{
> +	uint64_t l;
> +	__builtin_memcpy(&l, a, sizeof(l));
> +	return le64_to_cpup(&l);
> +}
> +static inline void put_unaligned_le32(uint32_t s, uint8_t *d)
> +{
> +	uint32_t l = cpu_to_le32(s);
> +	__builtin_memcpy(d, &l, sizeof(l));
> +}
> +static inline void cpu_to_le32_array(uint32_t *buf, unsigned int words)
> +{
> +        while (words--) {
> +		*buf = cpu_to_le32(*buf);
> +		++buf;
> +	}
> +}
> +static inline void le32_to_cpu_array(uint32_t *buf, unsigned int words)
> +{
> +        while (words--) {
> +		*buf = le32_to_cpup(buf);
> +		++buf;
> +        }
> +}
> +
> +static inline uint32_t rol32(uint32_t word, unsigned int shift)
> +{
> +        return (word << (shift & 31)) | (word >> ((-shift) & 31));
> +}
> +static inline uint32_t ror32(uint32_t word, unsigned int shift)
> +{
> +	return (word >> (shift & 31)) | (word << ((-shift) & 31));
> +}
> +
> +static void xor_cpy(uint8_t *dst, const uint8_t *src1, const uint8_t *src2,
> +		    size_t len)
> +{
> +	size_t i;
> +
> +	for (i = 0; i < len; ++i)
> +		dst[i] = src1[i] ^ src2[i];
> +}
> +
> +#define QUARTER_ROUND(x, a, b, c, d) ( \
> +	x[a] += x[b], \
> *** 50620 LINES SKIPPED ***



More information about the dev-commits-src-main mailing list