Home / Technology / Buffer overruns, license violations and bad code: FreeBSD 13s frequent calls

Buffer overruns, license violations and bad code: FreeBSD 13s frequent calls

For the most part, FreeBSD's core development team does not seem to see the need to update the review and approval procedures.
Enlarge / For the most part, FreeBSD’s core development team does not seem to see the need to update the review and approval procedures.

Aurich Lawson (after KC Green)

At first glance, Matthew Macy seemed like a perfectly reasonable choice to port WireGuard into the FreeBSD core. WireGuard is an encrypted point-to-point tunnel protocol, part of what most people think of as a “VPN”. FreeBSD is a Unix-like operating system that runs everything from Cisco and Juniper routers to the Netflix network stack, and Macy had extensive experience on its dev team, including working with multiple network drivers.

So when Jim Thompson, CEO of Netgate, which makes FreeBSD-powered routers, decided it was time for FreeBSD to enjoy the same level of core WireGuard support as Linux, he reached out to offer Macy a contract. . Macy wanted to port WireGuard into the FreeBSD core, where Netgate could then use it in the company’s popular pfSense route distribution. The contract was offered without deadlines or milestones; Macy’s was simply getting the job done according to her own schedule.

With Macy’s experience – especially with core coding and network stacks – the project looked like a slam dunk. But things went wrong almost immediately. WireGuard founder developer Jason Donenfeld did not hear about the project until it appeared on a FreeBSD mailing list, and Macy did not appear interested in Donenfeld’s help when it was offered. After about nine months of part-time development, Macy committed its port – largely unexplored and insufficiently tested – directly into the HEAD portion of FreeBSD’s code repository, where it was scheduled to be incorporated into FreeBSD 13.0-RELEASE.

This unexpected commitment increased the efforts of Donenfeld, whose project would eventually be judged on the quality of any production release under the WireGuard name. Donenfeld identified many problems with Macy’s code, but instead of opposing the port’s release, Donenfeld decided to solve the problems. He collaborated with FreeBSD developer Kyle Evans and Matt Dunwoodie, an OpenBSD developer who had worked on WireGuard for that operating system. The three replaced almost all of Macy’s code in an angry week-long sprint.

This went very badly with Netgate, which sponsored Macy’s work. Netgate had already taken Macy’s beta code from a FreeBSD 13 release candidate and placed it in production in pfSense’s 2.5.0 release. The forklift upgrade carried out by Donenfeld and partners – together with Donenfeld’s sharp characterization of Macy’s code – gave the company a serious PR problem.

Netgate’s public response included allegations of “irrational bias against mmacy and Netgate” and irresponsible disclosure of “a series of zero-day exploits” – despite Netgate’s almost simultaneous assertion that there were no actual vulnerabilities.

This conflicting response from Netgate gave increased control from many sources, which revealed surprising elements from Macy’s own past. He and his wife Nicole were arrested in 2008 after two years of trying to outlaw tenants from a small San Francisco apartment the couple had bought.

Macy’s attempts to force tenants out included sawing floor support beams to make the building unsuitable for human habitation, sawing holes directly through the floors of tenants’ apartments and forging extremely threatening emails that appear to be from the tenants themselves. The couple fled to Italy to avoid prosecution, but were eventually extradited back to the United States – where they pleaded guilty to a reduced set of crimes and served four years and four months each.

Macy’s history as a landlord did not, surprisingly, surprise him professionally – which contributed to his own lack of attention to the convicted WireGuard gate.

“I did not even want to do this work,” Macy finally told us. “I was burned out, spent many months with post-COVID syndrome … I had suffered through years of verbal abuse from non-perpetrators and semi-non-perpetrators in the project, if one big thing on me is that they are not I have jumped over the possibility of leaving the project in December … I just felt a moral obligation to get [the WireGuard port] over the finish line. So you have to forgive me if my final effort was a little half-hearted. “

This admission answers why such an experienced, qualified developer can produce poorer code – but it raises much bigger questions about process and procedure in the FreeBSD core committee itself.

How did so much sub-par code make it so far into a large open source operating system? Where was the code evaluation that should have stopped it? And why did both the FreeBSD core team and Netgate seem more focused on the fact that the code was compromised than the actual quality?

Code quality

The first problem is whether Macy’s code actually had significant problems. Donenfeld said it did, and he identified a number of major issues:

  • Sleep to reduce running conditions
  • Validation features that simply come true
  • Catastrophic cryptographic vulnerabilities
  • Parts of the wg protocol were not implemented
  • Core panics
  • Bypassing security
  • Printf statements deep inside the crypto code
  • “Spectacular” buffer overflows
  • Mazes of Linux → FreeBSD ifdefs

But Netgate claimed that Donenfeld had gone overboard with his negative assessment. The original Macy’s code, they claimed, was simply not that bad.

Despite the fact that we did not have any core developers on staff, Ars was able to confirm at least some of Donenfeld’s claims directly, quickly and without external help. For example, finding a validation function that simply returned true – and printf statements buried deep in cryptographic loops – required nothing more complicated than grep.

Empty validation function

To confirm or deny the claim of an empty validation function – one that always “returns true” instead of actually validating the data sent to it, we searched for instances of return true or return (true) and Macy’s if_wg code, which checked into FreeBSD 13.0-HEAD.

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

This is a small enough number of returns for easy hand revision, so we used it grep to find the same data, but with three lines of code that come immediately before and after each return true:

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

Among valid uses for return true, we discovered an 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 feature is not buried at the bottom of a wildly growing code –module.c as written are only 863 total lines of code.

We tried not to chase the use of this feature anymore, but it seems to be meant to check if a package’s source and / or destination belongs to WireGuards allowed-ips list, which determines which packets can be sent down a given WireGuard tunnel.

Source link