Re: Re-importing WireGuard driver and utilities

On 13 Oct 2022, at 18:55, John Baldwin <jhb@FreeBSD.org> wrote:
> 
> Over the past several months, I have spent some time reviewing the
> WireGuard driver including its interactions with the rest of the
> kernel and its use of crypto in the kernel.  This work was sponsored
> by the FreeBSD Foundation and had a few goals:
> 
> 1) Review the driver generally and how it interacted with other parts
>   of the kernel.
> 
> 2) Review the use of crypto in the driver and evaluate the feasibility
>   of using existing kernel crypto code where possible.  Specifically
>   included in this goal was aiming to make use of accelerated
>   software crypto via the ossl(4) driver for datapath crypto.
> 
> 3) Work with upstream to fix any issues encoutered and evaluate the
>   potential for reintegrating into the source tree.
> 
> In terms of the driver in general, I found a few minor nits and
> developed fixes for those that were accepted upstream.  I did make one
> non-trivial change (also accepted upstream) to fix a CPU scheduling
> inefficiency that dated back to the originally-imported driver.
> Initially, each input or output packet resulted in scheduling
> encryption or decryption tasks on every CPU in the system.  I modified
> this part of the driver to follow the Linux driver and instead
> schedule a single task on a single CPU (chosen via round-robin) for
> each packet.  This improved throughput in my (simple) tests over epair
> interfaces far more than switching to use ossl(4) for the datapath
> crypto.  Another user also reported that this change alone reduced the
> power overhead of FreeBSD VMs using WireGuard in ESXi to be nearly on
> par with Linux due to avoiding wasted CPU cycles on tasks that did no
> actual work.
> 
> For the crypto used in the driver, I have extended existing crypto
> APIs in the kernel to support the algorithms used by WireGuard that
> weren't already present (and these changes are already in main).  In
> general I have not imported any new crypto code into the kernel, but
> have added wrappers (often with APIs compatible with Linux) for
> existing (but previously unused) routines from libsodium.
> 
> That said, in my review of the driver I also found that the existing
> crypto implementations in the WireGuard driver were fine from a
> correctness standpoint.  Still, I prefer to minimize the number of
> copies of crypto algorithm implementations when possible to minimize
> future maintenance.
> 
> One bit of crypto is still used directly by the WireGuard driver which
> is the use of the Blake2 hashes.  In particular, the construction of
> the Blake2 hashes requires the length of the desired digest as an
> input into initializing the authentication state (similar to how
> CBC-MAC used in AES-CCM encodes L in block 0).  Here FreeBSD's
> in-kernel Blake2 implementation is somewhat broken as it hardcodes
> only a single digest length.  While OCF itself supports a notion of
> variable digest lengths via the csp_mlen field, the software
> auth_xform interface does not pass this length down to the Init() or
> SetKey() routines, so the auth_xform wrappers of Blake2 are not able
> to support variable digest lengths.  This could be fixed in the future
> by altering the auth_xform interface.  However, WireGuard's use of
> Blake2 is not a good fit for OCF.  Instead, adding an explicit
> "library" API around the existing Blake2 implementation is probably
> the most straightforward path if we want to eliminate the last of
> WireGuard's internal crypto.
> 
> On the third question, I feel that the current driver is stable and
> suitable for bring back into the source tree.
> 
> One question compared to the previous driver import is how to manage
> the configuration of WireGuard tunnels.  The previous driver added new
> commands to ifconfig(8) that largely mapped one to one with commands
> passed to the upstream wg(8) tool.  Upstream WireGuard has since
> relicensed their upstream wg(8) tool under a dual MIT/GPL license
> permitting direct use on FreeBSD.  Using the existing tooling means
> that existing recipes for configuring WireGuard on Linux using wg(8)
> also apply directly on FreeBSD.  It also provides a more seamless
> transition path for existing users of the WireGuard driver and utility
> in ports vs adding FreeBSD-specific extensions to ifconfig(8).
> 
> Given all that, Kyle Evans, Ed Maste, and myself would like to move
> forward with importing the current WireGuard driver and wg(8) utility
> into FreeBSD.  From upstream's perspective, the driver should simply
> move into the tree and be maintained directly in the tree (e.g. as
> sys/dev/wg).  The wg(8) tool, on the other hand, will be continued to
> be maintained by upstream, and so would be imported as normal
> third-party software into contrib/.
I love the separation boundary (provided that upstream is happy with it). Having wg(8) as a separate utility, that’s maintained by the software author is also great.
Do you folks have any opinion on the interface used to control the driver? Currently, wg(8) uses netlink on Linux, ioctl+nvlist on FreeBSD and some custom ioctls on OpenBSD.
Netlink support recently landed to main & there are plans to merge it to 13-S so it’s available in 13.2.
Any thoughts on switching our interface to netlink? I’m happy to write the interface & tests if there are no objections.

> 
> I have uploaded the main driver for review (along with some followup
> local cleanup changes) to https://reviews.freebsd.org/D36909.  The
> cleanups are included in the phabricator stack off of that review.
> 
> -- 
> John Baldwin
>