"wireguard" implementation improperly merged and needs revert

[Jason A. Donenfeld]

Hi Taylor/Ozaki-san/NetBSD developers,

I'm very excited that NetBSD is enthusiastic about WireGuard. Having
more deployment of WireGuard is always great to hear. And NetBSD is
really a terrific project. I've been enthusiastic about using it for
rump kernels for many years, and adding WireGuard capability there
sounds great. WireGuard is not just a protocol; the project also
comprises a set of implementations that meet a certain set of
behaviors and security criteria. I would be very happy to have a
proper WireGuard implementation inside of NetBSD.

However, I think you've gravely jumped the gun attempting to add
"wireguard" support to NetBSD. It's with great sadness that I ask you
to revert those commits from yesterday that added it, until we can
actually work on this together to make a real WireGuard
implementation.

I've had no communication with Taylor/Ozaki-san or other NetBSD
developers about this, and can't find any record of anyone reaching
out to me, which is a bummer. But now that we're in touch, I'd like to
offer full support of these efforts, from myself and from the project
in general. In about a week (~Sept 1), I'll be back at my desk full
time, and will have tons of energy to throw at this, in order to get
this code up to snuff.

In its current form, there are implementation flaws and violations
that I do not consider acceptable, and deploying this kind of thing is
highly irresponsible and harmful to your users. Rather than playing
never ending whack-a-mole misery with this -- which is not a path I'm
willing to go down here -- I'd like to re-examine how this is built
from the ground-up and do some serious code study.

Some background: recently, as I'm sure you're aware, WireGuard shipped
for OpenBSD. This was a year+ effort, with Matt and I working closely
together to get a high quality WireGuard implementation. And actually,
we wrote that to be as reusable by other BSDs as possible. It seems
like it might be possible to inherit a lot of that code, which has
already been refined and debugged, for NetBSD. This is, for example,
what we're currently working on for the upcoming FreeBSD
implementation.

On the other hand, if Ozaki-san is wedded to his codebase, and
believes there are distinct and important advantages to its structure,
I have no objection to working with him on that as a starting point.
In other words, I have no desire to impose anything unwanted on NetBSD
engineering trajectory in this, but if you're going to land a
WireGuard implementation, it needs to be done right, and I gladly
volunteer my time and energy into helping to make that happen.

So, it's a bit of a shame that this is my "hello, " email to the
NetBSD community -- I would have liked to meet you all more jovally --
but I feel very strongly about not ruining WireGuard. And what you
committed yesterday simply is not a WireGuard implementation. Could
you please move ahead with reverting that, and starting on Sept 1,
I'll make myself available to work with you on actually getting things
rolling properly? I can be available as much as is needed on IRC or
video chat or whatever other form works best for working together. I
would really appreciate that, and it would get us off on the right
foot, instead of the current rocky road we're staring down.

Thanks,
Jason

Re: "wireguard" implementation improperly merged and needs revert

[Taylor R Campbell]

[followups to tech-net to reduce cross-posting noise]

Hi, Jason!

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.

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 
[ ] modularize
[ ] split sliding window out
[ ] rename wgconfig(8) -> wg(8) and make interface compatible

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?

Thanks,
-Riastradh

Re: "wireguard" implementation improperly merged and needs revert

[Jason Thorpe]

> On Aug 22, 2020, at 11:35 AM, Jason A. Donenfeld  wrote:
> 
> it needs to be done right,

Can you be specific about what is wrong?

-- thorpej

Re: wait(2) and SIGCHLD

[Rhialto]

On Sat 15 Aug 2020 at 19:57:26 -0400, Terry Moore wrote:
> David Holland wrote:
> >> I would say so, especially since that would mean the child's parent is
> > > no longer the process that forked it (which could break other use
>  >> cases).
> >
> > That depends on how you implement detaching, but I suppose ultimately
> > it's important for getppid() to revert to 1 at the point the parent
> > exits (neither before, nor after, nor never) so some kind of linkage
> > needs to remain.
> >
> > Bah.
> >
> > I guess it's time to invent yet another different interface to
> > fork-and-really-detach.
> 
> No time to experiment today, but from the descriptions it sounds as if a
> double fork would work,
> with the child exiting immediately after forking the grandchild? Kind of
> unpleasant, but nothing
> new needed?

My first thought was that daemon(3) does something like that already
(the idea sounds familiar to me), but it does just a single fork(2) and
a setsid(2).

-Olaf.
-- 
Olaf 'Rhialto' Seibert -- rhialto at falu dot nl
___  Anyone who is capable of getting themselves made President should on
\X/  no account be allowed to do the job.   --Douglas Adams, "THGTTG"


signature.asc
Description: PGP signature