Re: "wireguard" implementation improperly merged and needs revert

["Jason A. Donenfeld" <Jason%zx2c4.com@localhost>]

Hi Taylor,

Thanks for your response. It seems like we both have the same goal --
a high quality WireGuard implementation in NetBSD -- but I think there
are a few subtleties I haven't conveyed right. I'll try to address
what you wrote and clarify what's up.

On Sun, Aug 23, 2020 at 1:03 AM Taylor R Campbell <riastradh%netbsd.org@localhost> wrote:
> Sorry about not reaching out.  The history is that the code has been
> kicking around the NetBSD world since Ozaki-san first wrote it in
> 2018, without getting merged into src.  It felt a shame to let it
> wallow in that state indefinitely, and it seemed to be in pretty good
> shape when I reviewed it this year, with a few small issues I saw, so
> I dusted it off and merged it.

So that's the thing. Ozaki-san visibly didn't finish the
implementation. Writing a WireGuard implementation is a big
undertaking. You don't simply take somebody's in-progress code, dust
it off, fix a few kernel API particulars, and ship it. That's not a
WireGuard implementation; that's a slow motion disaster in the making.
I realize this might be a kind of strange idea to consider: most
kernel programming is easily evaluable if you're a kernel programmer,
and most network programming is easily evaluable if you're a network
programmer, and WireGuard involves both kernel code and network code,
and you are both an accomplished kernel programmer and an accomplished
network programmer. So this should be a "straightforward" task, right?
It turns out that this isn't the case at all. There's a decent amount
of unusual domain specific things that go into building a WireGuard
implementation, that simply takes time and patience to get right.
Sending packets that are decipherable by WireGuard implementations is
only half the picture in building an actual implementation. As I
mentioned in my earlier email, Matt spent over a year perfecting his
implementation for OpenBSD, and we did continual code reviews,
knowledge transfers, and he even showed up at my house at some point
to read code together. Now, I'm not insisting on anything as wild and
intense as that, but I mention it to illustrate that this isn't just
something you casually dust off and merge.

> I would be happy to hear specific criticism of the code and its
> implementation flaws and violations, and/or pointers to documentation
> of the certain set of behaviours and security criteria that you expect
> implementations to adhere to.  Also happy to help answer questions
> about and navigate the NetBSD network stack if you want to review it
> yourself.
>
> As far as I know, Ozaki-san wrote the code following the WireGuard
> protocol paper.  There are a few XXX comments in the code that should
> be addressed, and there are some issues I know of that I have a small
> TODO list for but didn't seem critical enough to block committing the
> initial work:
>
> [ ] self-tests for crypto
> [ ] fix libssh dependency
> [ ] dtrace probes
> [ ] lockless radix tree lookups for peers
> [ ] take advantage of sys/crypto/chacha &c.
> [ ] modularize
> [ ] split sliding window out
> [ ] rename wgconfig(8) -> wg(8) and make interface compatible

There's a *lot* more than that missing. What you committed is missing
whole components of WireGuard. It's not a complete implementation. And
in many places it's just plain incorrect. (I should note though -- the
code itself seems very nicely written and clean, and I have no doubt
that Ozaki-san is a brilliant programmer.) It's not productive or
reasonable to play whack-a-mole with that codebase; it's not at the
place where that makes sense.

This isn't about some random bug here or there, or a TODO list of
little errors and optimizations, or something that one would normally
say, "well, the bones are there, so let's throw this in the tree, and
we'll clean it up as a gradual evolutionary thing." Rather, the
foundation is incomplete, and its current form does not belong in a
tree. And more generally, I don't feel comfortable distributing
something that you call "wireguard" when it simply is not WireGuard,
and does not come with the same levels of scrutiny and completeness
that WireGuard implementations deliver.

> For now, users have to go out of their way to enable the experimental
> wg(4) interface, and I didn't have any specific timeline planned for
> enabling it in GENERIC kernels -- wasn't likely to have been before
> September 1st anyway and I'm happy to commit to holding off on that
> until we've had a chance to discuss further in September.  Does that
> work?

In light of the above, I don't think that works. The right course of
action here is to revert the code, and then after we can evaluate
which direction we want to go in -- whether it's doing a full teardown
of Ozaki-san's code, or whether it's doing a simple port of the
OpenBSD code. That's something I would very much look forward to
talking to you about in September. So, can you move ahead with the
revert, and we can then evaluate our options for building an actual
WireGuard implementation? As I mentioned in my first email, my full
efforts and motivations are available in helping to make WireGuard a
reality on NetBSD; let's just do this right.

Thanks,
Jason