Re: Re-importing WireGuard driver and utilities

On 10/15/22 11:23 AM, Alexander V. Chernikov wrote:
> 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 don't have any strong opinions either way.  We'd likely have to keep the
ioctl approach under suitable COMPAT_FREEBSD<n> options if we adopted
netlink going forward.

-- 
John Baldwin