Re: Re-importing WireGuard driver and utilities

Gordon Bergling <[email protected]> Tue, 25 Oct 2022 02:58:03 +0200
Newsgroups	gmane.os.freebsd.architechture
Message-ID	<Y1c0m/[email protected]>
Hi John,

On Thu, Oct 13, 2022 at 10:55:15AM -0700, John Baldwin 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 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.

Thanks for the heads up and from my side a big +1. I use WireGuard personally
for a few VPN things in my home lab and have it in base is more then welcome.

--Gordon
signature.asc (application/pgp-signature, 618 B)