Aurich Lawson (after KC Green)
At first glance, Matthew Macy seemed a perfectly reasonable choice to port WireGuard to the FreeBSD kernel. WireGuard is an encrypted end-to-end tunneling protocol, part of what most people call “VPN”. FreeBSD is a Unix-like operating system that powers everything from Cisco and Juniper routers to the Netflix network stack, and Macy had a lot of experience on his development team, including working on various network drivers.
So when Jim Thompson, the CEO of Netgate, which makes routers with FreeBSD, decided it was time for FreeBSD to enjoy the same level of internal support from WireGuard that Linux offers, he offered a contract to Macy. Macy would port WireGuard to the FreeBSD kernel, where Netgate could then use it in the company’s popular pfSense router distribution. The contract was offered without deadlines or milestones; Macy should just get the job done on his own schedule.
With Macy’s level of experience – with kernel coding and network stacks in particular – the project looked like hell. But things went wrong almost immediately. WireGuard’s founding developer Jason Donenfeld didn’t hear about the project until it appeared on a FreeBSD mailing list, and Macy didn’t seem interested in Donenfeld’s help when offered. After about nine months of part-time development, Macy compromised its size – largely unreviewed and inadequately tested – directly into the HEAD section of the FreeBSD code repository, where it was scheduled for incorporation into FreeBSD 13.0-RELEASE.
This unexpected commitment increased the stakes for Donenfeld, whose project would be judged on the quality of any production launch under the name WireGuard. Donenfeld identified several problems with Macy’s code, but instead of opposing the launch of the door, Donenfeld decided to fix the problems. He collaborated with FreeBSD developer Kyle Evans and Matt Dunwoodie, an OpenBSD developer who worked on WireGuard for that operating system. The three replaced almost all of Macy’s code in a crazy week-long run.
This went very wrong with Netgate, which sponsored Macy’s work. Netgate has already taken Macy’s beta code from a candidate for the release of FreeBSD 13 and put it into production in version 2.5.0 of pfSense. The forklift upgrade carried out by Donenfeld and colleagues – along with Donenfeld’s clear characterization of Macy’s code – presented the company with a serious public relations problem.
Netgate’s public response included accusations of “irrational prejudice against mmacy and Netgate” and irresponsible disclosure of “a series of zero-day exploits” – despite Netgate’s almost simultaneous declaration that there were no real vulnerabilities.
This combative response from Netgate increased scrutiny from many sources, which revealed surprising elements of Macy’s own past. He and his wife Nicole were arrested in 2008, after two years trying to illegally evict tenants from a small apartment building bought in San Francisco.
The Macys ‘attempts to force their tenants out included sawing floor support beams to make the building unsuitable for human habitation, sawing holes directly in the floor of tenants’ apartments and forging extremely threatening e-mails that appeared to be from the tenants themselves. The couple fled to Italy to avoid prosecution, but ended up being extradited back to the United States – where he pleaded guilty to a reduced set of crimes and served four years and four months each.
Macy’s history as a landlord, unsurprisingly, pursued him professionally – which contributed to his own lack of attention to the condemned WireGuard port.
“I didn’t even want to do this job,” Macy ended up telling us. “I was exhausted, I spent many months with the post-COVID syndrome … I suffered for years of verbal abuse from non-executors and semi-executors on the project, whose only problem with me is that they are not not criminals. I seized the opportunity to leave the project in December … I just felt a moral obligation to obtain [the WireGuard port] over the finish line. So you’re going to have to forgive me if my final efforts were a little timid. “
This admission answers why an experienced and qualified developer can produce inferior code – but it raises much bigger questions about the process and procedure within the FreeBSD central committee itself.
How did so much below-average code become a large open source operating system? Where was the code review that should have stopped it? And why did the core FreeBSD team and Netgate seem more focused on the fact that the code was being overlooked than its actual quality?
Quality of the Code
The first question is whether Macy’s code really had any significant problems. Donenfeld said yes, and he identified a number of important problems:
- Sleep to mitigate race conditions
- Validation functions that simply return true
- Catastrophic cryptographic vulnerabilities
- Parts of the wg protocol not implemented
- Kernel panics
- Security bypasses
- Printf instructions deep in the cryptographic code
- “Spectacular” buffer overflows
- Linux Labyrinths → FreeBSD ifdefs
But Netgate argued that Donenfeld exaggerated his negative assessment. The original Macy code, they argued, just wasn’t that bad.
Despite having no kernel developers on the team, Ars was able to verify at least some of Donenfeld’s claims directly, quickly and without outside 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
In order to confirm or deny the claim for an empty validation function – one that always “returns true” instead of actually validating the data passed to it – we look for instances of return true or return (true) at Macy’s if_wg code, as verified on FreeBSD 13.0-HEAD.
root@banshee:~/macy-freebsd-wg/sys/dev/if_wg# grep -ir 'return.*true' . | wc -l
21This is a small enough number of returns for easy manual auditing, so we use grep to find the same data, but with three lines of code 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 an empty validation function, in module/module.c:
wg_allowedip_valid(const struct wg_allowedip *wip)
{
return (true);
}It is probably worth mentioning that this empty validation function is not buried at the bottom of a large body of code—module.c as written, it has only 863 lines of code in total.
We have not tried to pursue the use of this function anymore, but it appears that the intention is to verify that the origin and / or destination of a package belongs to WireGuard allowed-ips list, which determines which packets can be routed through a given WireGuard tunnel.