Native-kernel wireguard support for 21.1 feasible? FreeBSD 13 may have it

Started by TheLinuxGuy, January 19, 2021, 06:34:10 AM

Previous topic - Next topic
kind regards
chemlud
____
"The price of reliability is the pursuit of the utmost simplicity."
C.A.R. Hoare

felix eichhorns premium katzenfutter mit der extraportion energie

A router is not a switch - A router is not a switch - A router is not a switch - A rou....


Oh, that was a classic Jim moment for sure. What wild discussions we had regarding licensing and availability of source code (pfsense-tools) back then. Does anyone remember the "ESF" license?

https://www.pfsense.org/ESF_License_Agreement_v1.2.pdf

On positive note Jason is still dashing forward. https://lists.zx2c4.com/pipermail/wireguard/2021-March/006518.html

For those not-so-deep inside the sheepfencing Netgate story: Gonzopancho is Jim Thompson

https://www.linkedin.com/in/jimthompson7

...and btw doktornotor is... special... :-D

https://github.com/doktornotor/pfsense-still-closedsource
kind regards
chemlud
____
"The price of reliability is the pursuit of the utmost simplicity."
C.A.R. Hoare

felix eichhorns premium katzenfutter mit der extraportion energie

A router is not a switch - A router is not a switch - A router is not a switch - A rou....

Quote from: chemlud on March 17, 2021, 08:50:39 PM
For those not-so-deep inside the sheepfencing Netgate story: Gonzopancho is Jim Thompson

https://www.linkedin.com/in/jimthompson7

...and btw doktornotor is... special... :-D

https://github.com/doktornotor/pfsense-still-closedsource

Running a business, you have to deal with special customers. Back then, reading that, I decided for myself that I should probably stay away from being "on the customer end" with them.

Great to see that the WG developer is working on a speedy implementation! Looking forward to replace my Linux box at some point and run WG on OPNsense.

We are making progress... https://github.com/freebsd/freebsd-ports/commit/e2b5b355f64b

As far as I understood the latest code it supports the native wg commands so that we can start testing this soon with the existing plugin.


Cheers,
Franco

Good news Franco,
let us know when it's time and let's test the experimental Wireguard kernel module.
Then we will see if everything will be good.
Greetings from Germany
OPNsense 22.7.9*WG-kmod*OpenSSL*OpenVPN* AdGuardHome*i7-7700*32GB*256SSD*ix0-1, igb0-4, em0*OpenVPN+Wireguard WG0, WG1*NetGear ProSafe XS508*AP Netgear WAX610*alles echtes Blech* Sorry, my English is translated via app*

Quote from: Mondmann on March 20, 2021, 05:10:02 PM
let us know when it's time and let's test the experimental Wireguard kernel module.
Then we will see if everything will be good.

2nd to this. Thanks Franco.
--
Stay safe.

Still a bit to manage around the wireguard-tools, but the wireguard-kmod package is already where it should be. Then last step is a development version of the plugins that can handle the kernel module. Maybe we can start public testing when 21.1.5 arrives.


Cheers,
Franco

Ars put out a well balanced investigation and summary of this whole mess:
https://arstechnica.com/gadgets/2021/03/buffer-overruns-license-violations-and-bad-code-freebsd-13s-close-call/
Deciso DEC750
People who think they know everything are a great annoyance to those of us who do. (Isaac Asimov)

From ars article

Quote
Empty validation function

In order to confirm or deny the claim of an empty validation function—one which always "returns true" rather than actually validating the data passed to it—we searched for instances of return true or return (true) in Macy's if_wg code, as checked into FreeBSD 13.0-HEAD.

root@banshee:~/macy-freebsd-wg/sys/dev/if_wg# grep -ir 'return.*true' . | wc -l
21

This is a small enough number of returns to easily hand-audit, so we then used grep to find the same data but with three lines of code coming immediately before and after each return true:

root@banshee:~/macy-freebsd-wg/sys/dev/if_wg# grep -ir -A3 -B3 'return.*true' .

Among the valid uses of return true, we discovered one empty validation function, in module/module.c:

wg_allowedip_valid(const struct wg_allowedip *wip)
{

return (true);
}

It's probably worth mentioning that this empty validation function is not buried at the bottom of a sprawling mass of code—module.c as written is only 863 total lines of code.

We did not attempt to chase down the use of this function any further, but it appears to be intended to check whether a packet's source and/or destination belongs to WireGuard's allowed-ips list, which determines what packets may be routed down a given WireGuard tunnel.

and

QuotePrintf in crypto loops

Some pfSense 2.5.0 users reported strange hexadecimal output spamming the root console of their router. This matches Donenfeld's description of printf statements deep in crypto code, and we were able to easily discover the source in much the same way we found the empty validation function above.

kind regards
chemlud
____
"The price of reliability is the pursuit of the utmost simplicity."
C.A.R. Hoare

felix eichhorns premium katzenfutter mit der extraportion energie

A router is not a switch - A router is not a switch - A router is not a switch - A rou....

Adding some more contents, hopefully from the closest to FreeBSD it can be:
https://lists.freebsd.org/pipermail/freebsd-hackers/2021-March/057127.html

Quote


Dear FreeBSD Community,

In light of the recent commentary on FreeBSD's development practices,
members of the Core team would like to issue the following statement.

Code quality is an essential FreeBSD value: From the 1980s when work on
BSD became the de facto standard TCP/IP stack, to our more recent work
around performance scalability on multicore, attention to detail is
critical. The recent concerns regarding the WireGuard patches remind us
that our development processes must always continue to mature. While the
project has historically, and aggressively, led the way in adopting new
development methodologies - from public version control to being early
adopters of static analysis tools such as Coverity - these events have
brought to light a real gap that needs to be addressed.

The high stability and quality of FreeBSD is a testimony to the
experience of our developers. As in any open source project, we rely on
developers to exercise good judgement in seeking review and committing
new features, and to follow the guidelines laid out in the Committer's
Guide. We make heavy use of public code review, and FreeBSD developers
spend a significant amount of time improving each others' contributions.

We were excited to provide a kernel WireGuard implementation in FreeBSD
13.0. Before the if_wg(4) rewrite was committed, several FreeBSD
developers proactively worked on fixing bugs and writing tests and
documentation for the original implementation. In other words, we had
spent time during the release's Q/A period looking for problems, and
that unfortunately culminated in if_wg(4) being removed from 13.0 during
the release cycle. As FreeBSD developers, it is incumbent on each of us
to support each other's work by providing code review and helping test
and fix the code. This incident highlights the need to do this work more
proactively, and to maintain a robust, multi-layered development process
that can catch problems as they fall through the cracks.

Over the next month the FreeBSD Core Team will lead a discussion on
appropriate pre-commit testing, static analysis, code review, and
integration policies to avoid a repeat of this situation and to continue
improving FreeBSD's code quality. We know there will be challenges in
key areas, such as third-party device drivers, and components of the
system where fewer developers have sufficient expertise. The FreeBSD
Foundation has full-time staff members participating in significant code
review today, and is committed to supporting the needs identified by the
Core team and the developer community for this effort.

We look forward to input from the community on our proposals for updated
policies as we move forward, maintaining high code quality as a core
value for FreeBSD.

Thanks,
-Mark, with core@ hat on


Well, again, this is not a code quality issue... it's a management issue:

QuoteBefore the if_wg(4) rewrite was committed, several FreeBSD
developers proactively worked on fixing bugs and writing tests and
documentation for the original implementation. In other words, we had
spent time during the release's Q/A period looking for problems, and
that unfortunately culminated in if_wg(4) being removed from 13.0 during
the release cycle.

The conclusion is really weird. Either they didn't know what they were looking for or they didn't care. Mind you some of these proactive developers were also sponsored by Netgate which then gets to decide to pull WireGuard from the kernel forever after it got fixed if we can trust Scott Long who is a Netgate employee and a FreeBSD core member whose duties do not overlap according to Netgate interview in the Ars article...  ;)

Pulling it from 13.0 release is ok, pulling it out of the development tree for 14.0 or a potential backport of 13.1 is insane if you think about all the work that was done to get it there in the first place.


Cheers,
Franco

Quote from: franco on March 29, 2021, 10:34:06 AM
Pulling it from 13.0 release is ok, pulling it out of the development tree for 14.0 or a potential backport of 13.1 is insane if you think about all the work that was done to get it there in the first place.
It's in ports now, which is a perfectly sane approach in my book:
https://svnweb.freebsd.org/ports/head/net/wireguard-kmod/
Deciso DEC750
People who think they know everything are a great annoyance to those of us who do. (Isaac Asimov)

The big question to me: Is pfsense 2.5 safe to be run with this trash code on board? I still have one running, which has to be updated soon... :-/

The image is still the old one published in February. Anyone seen efforts to remove the trash code from daily snapshots of 2.5.1?
kind regards
chemlud
____
"The price of reliability is the pursuit of the utmost simplicity."
C.A.R. Hoare

felix eichhorns premium katzenfutter mit der extraportion energie

A router is not a switch - A router is not a switch - A router is not a switch - A rou....