Despite not having any kernel developers on-staff, Ars was able to verify at least some of Donenfeld’s claims directly, quickly, and without external assistance.
Extremely important paragraph IMO. Why don’t we see things like this in other programming reporting?
Because reporters earning $40k/yr don’t typically have the same skill set as a developer making $150k? If you know how to use grep well, you’ve got the start of a newer, higher paying career.
The most salient critique in here of the FreeBSD project itself might be this paragraph:
Several FreeBSD community members would only speak off the record. In essence, most seem to agree, you either have a commit bit (enabling you to commit code to FreeBSD’s repositories) or you don’t. It’s hard to find code reviews, and there generally isn’t a fixed process ensuring that vitally important code gets reviewed prior to inclusion. This system thus relies heavily on the ability and collegiality of individual code creators.
I gave up my commit bit a while ago, though I might request that it is reactivated at some point. I’ve recently been submitting some fixes to the run-time linker and some tweaks to Capsicum, which have been reviewed and committed by others.
Finding reviewers in FreeBSD is not that different from other projects:
If a patch is small, it will get reviews.
If a patch is in an area where lots of people are working, it will get reviews but if it’s large then they might not be very thorough.
If the person submitting the patch does the work to find good reviewers and tag them, it will be reviewed.
If the person submitting the patch has a reputation for doing good code reviews, others are more likely to go out of their way to help that person.
Not everyone in the community is great at writing non-confrontational reviews and sometimes reviews can be hostile.
I have some problems with some of the outdated infrastructure in the FreeBSD project (the build system is my personal nemesis) and some members of the community, but this feels like a combination of several independent problems all manifesting at the same time. Most of these problems also exist in other projects and could easily line up in the same way.
This is absolutely a process failure, but unfortunately it’s a failure of process that could easily happen in a lot of other high-profile projects (proprietary and open source). I’d love to see the FreeBSD project consider what process could be in place to address this. Normally, crypto code needs to be reviewed by one of the cryptographers on the project, I’m not sure why that wasn’t the case this time. It would be great to see infrastructure for fuzzing network code in CI (which would have caught the jumbo frame bug). The inclusion of GPL’d code is troubling and is one of the things that a CLA typically addresses (the contributor asserts on a legal document that they have the legal right to contribute the code that they’re submitting).
A lot of this article was a personal attack on Matt Macy and is likely to be in direct violation of the EU’s Right to be Forgotten. It’s a great example of why that kind of legislation is important: once you’ve served a sentence, you don’t deserve to have that fact dragged up to defame your character. Most of the rest is character assassination and innuendo.
The one thing I would take issue with from the project side is Scott’s comment:
So I’ll say that being a member of the core team gives me no special privileges to add onto the code, and my voice is one of many. I’m a very small voice in the leadership. I’m a small voice in the community.
I think he is being modest here. Scott Long is, in my experience, one of the most respected FreeBSD developers. He’s been involved since 2000 and has made big contributions across the kernel, plus some non-code contributions such as being release engineer and leading Netflix’s FreeBSD team. His word is not law in the Linus sense, but if Scott says something and there’s no obvious evidence to contradict it, I expect that most FreeBSD developers would assume that he’s correct because he’s done a huge amount to earn their respect. He may be a small voice, but in the 1 Kings 19 sense of the phrase.
A lot of this article was a personal attack on Matt Macy and is likely to be in direct violation of the EU’s Right to be Forgotten.
I strongly support moving past peoples’ past crimes and failing, and welcoming them back into our communities. I have no reason to believe that he’s retained the dishonesty and racism expressed in his landlording - certainly the issues here seem entirely unrelated.
But this has nothing to do with the right to be forgotten. That’s not about expunging records or preventing reporting of the backgrounds of the subjects of news articles. It’s about EU citizens being able to demand that search engines delist such records and news articles so that the first result for someone’s name isn’t some past incident. I personally don’t know exactly how I feel about these laws. I think that merely targeting search indexes was a way of avoiding the important censorship questions that should be addressed. But I also think that people should be able to move beyond past failings in this seemingly append-only digital world.
This is basically true of all open source projects, isn’t it?
The Linux kernel is sort of an outlier here, but only partially. Last I checked, If you get code past one of Linus’s trusted gatekeepers for a given system, chances are it’s going in to stable. But that’s basically the case for FreeBSD, if you get the code past a person with a commit bit, then chances are it’s going into stable.
In Linux’s case, there are in theory at least 2 people involved(Linus and whatever lieutenant/gatekeeper is responsible for that section) Though the distro’s regularly include patches that live outside of Linus’s “official” tree for various reasons, so even that hurdle isn’t a guarantee. Not to mention all the core stuff that lives outside of the Kernel, which are all separate projects with gosh knows what sorts of quality control. I bet it would be very easy to find at least one where there is essentially a single committer, but I’m too lazy to go looking.
FreeBSD is pretty good about yanking crap code out when it does finally surface as being crap though. Like they did here, when the alarm was raised, WG got kicked out in a giant hurry. The sad part is, now it will take a good long while for WG to land in stable. I hope it makes it for the next stable, but who knows.
No, basically all non-trivial open source projects I’ve been involved with have a code review gate on submissions. If it’s not technically enforced it’s socially enforced. This process has saved me from submitting half-assed code when I’m feeling burned out - it’s a pity that Macy didn’t have that kind of support from the FreeBSD community.
All people with commit access can commit whatever they want, right?
Is what I said about Linux incorrect?
I agree for non-committers, there are PR’s in github land, or patches in FreeBSD land, but you only have to get over the hurdle of the 1 person with commit access. Committers don’t. I agree there are some projects where even committers submit PR’s and don’t push directly to the main branch, but these all seem to be fairly new projects, relative to the OS histories, like FreeBSD which has been around since 1993, using code from the 1960’s.
This is an strange example to cite, because in the PR, one of the package’s maintainers states that they are fine with merging the change. So, there was an approval before the merge.
I bet it would be very easy to find at least one where there is essentially a single committer
The Postfix mail server is an example. Used by about 1/3 of mail servers on the internet, and essentially solo maintained by Wietse Venema for ~22 years now. There isn’t even a public repository!
I was thinking something akin to what is in the FreeBSD base system, i.e. GNU coreutils and friends. But I agree both of these are great examples of well-used common systems with essentially a single committer!
Maybe the olson(?) TZ database? though I’m not sure he is still maintaining it, and I hope I didn’t just get his name wrong :)
FreeBSD is pretty good about yanking crap code out when it does finally surface as being crap though. Like they did here, when the alarm was raised, WG got kicked out in a giant hurry. The sad part is, now it will take a good long while for WG to land in stable. I hope it makes it for the next stable, but who knows.
It’s now being developed as a kernel module in ports. It should be available for 12 and 13, you just won’t be able to use it until you’ve run the pkg bootstrap. As long as you have a non-wireguard network that has a route to your package mirror, there won’t be much difference other than the need to run a single extra command once at first-install time.
I was burned out [..] suffered through years of verbal abuse [..] I jumped at the opportunity to leave the project [..] I just felt a moral obligation to get [the WireGuard port] over the finish line. So you’ll have to forgive me if my final efforts were a bit half-hearted.
So the best option here would have been to tell them that you won’t finish this, that you’re burnt out, that you won’t be able to create something that will be useful for them. (and make your project look bad) Instead of doing the most scammy thing that you could imagine. The whole action does not look good, especially with the accusations it’s like something you’d expect..
Also why didn’t the netgate people catch at least all those printfs and 40KLOC copy-pasted linux kernel code ?!
I’ve been in a similar situation – although I never pushed out such bad code; I just didn’t write any code – and it’s harder than it sounds. On one hand you want to do this, you don’t want to let anyone down, and feel like any moment now you’re going to shake it off and it’ll all be fine. Except that this moment never comes. It’s also hard to say “I can’t do this because (vague feelings)”. I don’t mean this to dismiss them, just that it’s hard to explain these kind of things (including to yourself), especially “in the moment”.
The real WTF here is that a single person underperforming wasn’t caught by either Netgate or FreeBSD. These things happen and are part of normal human behaviour. Especially Netgate really dropped the ball here IMHO by not just communicating with their contractor/employee throughout the process. His line manager really should have spotted that Matt wasn’t doing too well. I wonder how much Netgate’s company culture contributed to this. It doesn’t exactly seem like a very friendly company with heaps of empathy and kindness to spare…
you want to do this, you don’t want to let anyone down, and feel like any moment now you’re going to shake it off and it’ll all be fine.
I can empathize with this. I’ve been there. And pile on a healthy dose of “every time it’s ever felt this way before, I shook it off and it was fine” coupled with “I don’t want to have this conversation with that manager right now, because I won’t get support, just more pressure, and besides, it’s going to be fine” and I think I understand how you can get from something that feels like good intentions to something that looks like “the most scammy thing that you could imagine.”
(I’m not saying I think it was right. Just that I can see in my mind’s eye one very slippery slope that takes you exactly here.)
The real WTF here is that a single person underperforming wasn’t caught by either Netgate or FreeBSD.
Yes. It sounds to me like nobody was looking. Possibly for similarly relate-able reasons, but the level of “nobody was looking” required for this to hit an RC on a project this prominent, that serves as upstream for so many other things, makes me worry a bit about the foundation of a few important infrastructure items.
On one hand you want to do this, you don’t want to let anyone down, and feel like any moment now you’re going to shake it off and it’ll all be fine
Fair enough I think I might know how this feels like. I just think he could have told them about the problems and helped otherwise. Like mentoring, showing them the code and talking about the issues etc. Checking this in and going away in that state feels just wrong. Especially with said accusations, as it looks - from the outside - like they just didn’t care about that. Regarding the missing reviews/tests of such a big patch from one person I’m totally on your side.
Side note: I started maintaining a rust crate after the burnout of the original author. And the more I dig into the problems and want to “get it right”, trying to release the next major version (while having zero previous knowledge), the more I can feel the weight of how many different corners they tried to do right across all the three majors operation systems. Meanwhile the project kinda works and gets better over time as some contributors with more in-depth knowledge appear out of nowhere, contribute 1-2 PRs and vanish, fixing some of the worst heisenbugs. There is no real punchline, just my observation.
This combative response from Netgate raised increased scrutiny from many sources, which uncovered surprising elements of Macy’s own past. He and his wife Nicole had been arrested in 2008 after two years spent attempting to illegally evict tenants from a small San Francisco apartment building the pair had bought.
This article is very interesting, but this part makes me really uneasy… Why is the criminal past of some developer relevant? He obviously did something wrong, and paid his debt to society for it. This article reads “this guy wrote shitty code, no wonder, he was a criminal!”
It sounds like he did a “piss-poor job”, as Bruce Willis would say, and it also seemed that the review process was a little flawed. (But let’s give credit the FreeBSD people, they’re doing a great job for a group of volunteers with little resources) I just don’t see how his previous penal convictions have anything to do with it. Does this guy now has to carry this burden for his entire life?
I think the two sentences you quoted contain the answer to why it’s relevant:
This combative response from Netgate raised increased scrutiny from many sources
Normally, when there’s a contribution to an open source project, (hopefully) there’s some review, issues are found, the parties work together to address them, and things land in a mutual effort to improve the project. This time, there was a contribution. (Serious!) Issues were found. The people who found the issues pitched in to help fix them and bring the contribution up to the standards of the project. The company sponsoring the contribution went nuts and started accusing the reviewers/other contributors of “releasing 0days”, etc.
This combative response to the code review made people dig in and look, wonder if the developer was himself a bad actor, and find past behavior that might be consistent with being a bad actor. It got reported.
I don’t think it got reported as a disproportionately large part of the story, if you look at the whole article. I think it was mainly mentioned here because it had been brought up in public discussions already, and I thought the author of this piece did a good job giving it context.
Culture perhaps? Some parts of the world follow the motto “once a criminal, always a criminal”. Harder to get employment, requirement to disclose, disqualified from voting, etc. I believe Americans have different tiers of this depending on where you live, eg “felon” (?)
I also got the impression that the criminal history was mentioned by the article as supporting evidence for him being bad at things in general. I don’t like that. If you are going to mention the bad parts of someone’s past then you need to look at the motivations & context for those bad parts (and then analyse whether they relate to now); not leave it hanging with a feeling of bad guy does bad things.
It was however nice the article talked about the successes of the author too. I am glad they did that.
So? One bad coder committed some bad code for money to FreeBSD Head. Other people saw how bad that code really was and tried to fix it, though there wasn’t much time to fix it for the next release. Doesn’t it look like a good case of code review? At least we know there are people looking at those committed code.
It looks like a bad practice for a for-profit company to contract a third person to port some code without informing any original developers. In this case the company picked a wrong person.
The point of code review is to prevent bad code from ending up in the tree in the first place. If you have bad code in your master branch and just barely avoided shipping a release with the bad code, that’s not a successful application of code review.
I agree that a lot of the blame ends up on Netgate here. Calling the discussion of the bad code a “zero-day disclosure” is especially egregious.
Not even a tiny bit. It was in a release candidate, and when the code review occurred and found problems, the sponsor of the code accused the developers who reviewed and tried to fix the code of releasing 0-days and worse, while at the same time claiming out of the other side of his mouth that the problems they found weren’t real anyway.
It is an example of code review narrowly preventing something that was grossly unfit for purpose from landing in a final release as opposed to only a release candidate.
I can’t find a way to stretch this so that it looks like a “good case” of anything at all.
I wonder how much such a release candidate is actually tested. The article mentions that there are bug reports regarding the if_wg code in the pfSense project. The same bugs should have occurred in the FreeBSD release candidate.
One of the points made by the author (and also re-iterated on a podcast he co-hosts) is that there is no formal code review process to prevent something like this from happening again. The other co-host on the podcast made the point that FreeBSD developers are for the most part unpaid volunteers who have no way of forcing anyone to do the reviews. I think both are valid points.
Having used OpenBSD’s implementation I think another big point might be that this means it’s part of the base system and will likely end up being better integrated into it. If your system always ships with wg, you are more inclined to make it for example better measurable, debugable, configurable within that base system.
That’s less true for the Linux world where everything is seen in a more stand-alone fashion, so I guess there it’s more about distributions or tool builders embracing the use case.
This was a really strange article for me. Developers are human. A lot of us write bad code sometimes. A lot of us haven’t been performing up to our usual standards in the last year, especially if we had COVID and are still recovering. A few of us have mistakes in our past that we regret. I guarantee you you’ll find similar situations in every software project of sufficient size, probably many times a year. The goal of a software development process is to produce a decent product nonetheless. And it does seem to have actually worked in this case, although maybe not as well as desirable.
The problem being pointed out in this article is that there was no good process for catching mistakes. It’s because we’re all human and make mistakes that we need processes for catching and fixing those mistakes before they make it into production. And the way it worked in this instance was that the bad code got caught just before it made it into an official release–of one of the most important operating systems in the world.
At any reasonable organization, this would by far not be a reasonable or accepted practice.
and to add to that, this was apparently fairly clearly bad code. What if it was an obfuscated vulnerability of some kind? With such a lax review policy, who knows what has snuck in over time?
Interesting article, however there’s one point I dislike. Acting like there are no code reviews or somehow less than in other projects. This is very much untrue. See reviews.freebsd.org.
This is not to say, that the process shouldn’t be improved, but it’s not a process unlike in other big projects. In this particular instance someone with commit bit which in this community means that you should are entrusted to not do what has been done in this case.
Variations of this situation exist for many big projects. Usually the “threat” is that you are removed from the project and that you will get bad reputation from that and won’t be entrusted anymore. I think the latter situation has occurred.
Things like that have happened and will happen and while gain, I am not saying that you cannot improve the process - I am completely sure you can - this certainly isn’t a purely technical problem. From al I can see this wasn’t a malicious action, just someone who messed up in a particular situation and it being very clearly visible and loudly discussed. Acting like other, similar projects are somehow immune to this is unrealistic.
One usually uses some kind of gatekeeping, but acting like gate keepers don’t make mistakes is like saying “systems never fail”.
Yes, this is a clear sign that how reviewing works should be improved and the FreeBSD project certainly has got homework to do, but this looks mostly like a very obvious and good to analyze example.
In other words I hope that people don’t end up that this can’t happen in their project, cause they do code reviews. Reading through the discussions I get the feeling that’s happening here. If that’s the main take-away from this one misses the point and maybe an opportunity to improve their processes as well.
FreeBSD might see an uptick in code review culture now that they have moved from SVN to Git. Perhaps the “commit bit” policy used to make more sense given past challenges.
A commit bit policy helps when you have a lot to do and few people who can do it. Make the barriers for the commit bit a little higher, encourage asking for review when something is off, and good stuff will get through.
Extremely important paragraph IMO. Why don’t we see things like this in other programming reporting?
Because reporters earning $40k/yr don’t typically have the same skill set as a developer making $150k? If you know how to use grep well, you’ve got the start of a newer, higher paying career.
The most salient critique in here of the FreeBSD project itself might be this paragraph:
I gave up my commit bit a while ago, though I might request that it is reactivated at some point. I’ve recently been submitting some fixes to the run-time linker and some tweaks to Capsicum, which have been reviewed and committed by others.
Finding reviewers in FreeBSD is not that different from other projects:
I have some problems with some of the outdated infrastructure in the FreeBSD project (the build system is my personal nemesis) and some members of the community, but this feels like a combination of several independent problems all manifesting at the same time. Most of these problems also exist in other projects and could easily line up in the same way.
This is absolutely a process failure, but unfortunately it’s a failure of process that could easily happen in a lot of other high-profile projects (proprietary and open source). I’d love to see the FreeBSD project consider what process could be in place to address this. Normally, crypto code needs to be reviewed by one of the cryptographers on the project, I’m not sure why that wasn’t the case this time. It would be great to see infrastructure for fuzzing network code in CI (which would have caught the jumbo frame bug). The inclusion of GPL’d code is troubling and is one of the things that a CLA typically addresses (the contributor asserts on a legal document that they have the legal right to contribute the code that they’re submitting).
A lot of this article was a personal attack on Matt Macy and is likely to be in direct violation of the EU’s Right to be Forgotten. It’s a great example of why that kind of legislation is important: once you’ve served a sentence, you don’t deserve to have that fact dragged up to defame your character. Most of the rest is character assassination and innuendo.
The one thing I would take issue with from the project side is Scott’s comment:
I think he is being modest here. Scott Long is, in my experience, one of the most respected FreeBSD developers. He’s been involved since 2000 and has made big contributions across the kernel, plus some non-code contributions such as being release engineer and leading Netflix’s FreeBSD team. His word is not law in the Linus sense, but if Scott says something and there’s no obvious evidence to contradict it, I expect that most FreeBSD developers would assume that he’s correct because he’s done a huge amount to earn their respect. He may be a small voice, but in the 1 Kings 19 sense of the phrase.
I strongly support moving past peoples’ past crimes and failing, and welcoming them back into our communities. I have no reason to believe that he’s retained the dishonesty and racism expressed in his landlording - certainly the issues here seem entirely unrelated.
But this has nothing to do with the right to be forgotten. That’s not about expunging records or preventing reporting of the backgrounds of the subjects of news articles. It’s about EU citizens being able to demand that search engines delist such records and news articles so that the first result for someone’s name isn’t some past incident. I personally don’t know exactly how I feel about these laws. I think that merely targeting search indexes was a way of avoiding the important censorship questions that should be addressed. But I also think that people should be able to move beyond past failings in this seemingly append-only digital world.
This is basically true of all open source projects, isn’t it?
The Linux kernel is sort of an outlier here, but only partially. Last I checked, If you get code past one of Linus’s trusted gatekeepers for a given system, chances are it’s going in to stable. But that’s basically the case for FreeBSD, if you get the code past a person with a commit bit, then chances are it’s going into stable.
In Linux’s case, there are in theory at least 2 people involved(Linus and whatever lieutenant/gatekeeper is responsible for that section) Though the distro’s regularly include patches that live outside of Linus’s “official” tree for various reasons, so even that hurdle isn’t a guarantee. Not to mention all the core stuff that lives outside of the Kernel, which are all separate projects with gosh knows what sorts of quality control. I bet it would be very easy to find at least one where there is essentially a single committer, but I’m too lazy to go looking.
FreeBSD is pretty good about yanking crap code out when it does finally surface as being crap though. Like they did here, when the alarm was raised, WG got kicked out in a giant hurry. The sad part is, now it will take a good long while for WG to land in stable. I hope it makes it for the next stable, but who knows.
No, basically all non-trivial open source projects I’ve been involved with have a code review gate on submissions. If it’s not technically enforced it’s socially enforced. This process has saved me from submitting half-assed code when I’m feeling burned out - it’s a pity that Macy didn’t have that kind of support from the FreeBSD community.
All people with commit access can commit whatever they want, right?
Is what I said about Linux incorrect?
I agree for non-committers, there are PR’s in github land, or patches in FreeBSD land, but you only have to get over the hurdle of the 1 person with commit access. Committers don’t. I agree there are some projects where even committers submit PR’s and don’t push directly to the main branch, but these all seem to be fairly new projects, relative to the OS histories, like FreeBSD which has been around since 1993, using code from the 1960’s.
NixOS is easily non-trivial. Committers can clearly commit whatever they want: https://discourse.nixos.org/t/misuse-of-commit-permissions/10071
Debian’s process for QA appears to all happen after it’s committed into unstable: https://wiki.debian.org/qa.debian.org
This is an strange example to cite, because in the PR, one of the package’s maintainers states that they are fine with merging the change. So, there was an approval before the merge.
The Postfix mail server is an example. Used by about 1/3 of mail servers on the internet, and essentially solo maintained by Wietse Venema for ~22 years now. There isn’t even a public repository!
Vim is another well-known example
I was thinking something akin to what is in the FreeBSD base system, i.e. GNU coreutils and friends. But I agree both of these are great examples of well-used common systems with essentially a single committer!
Maybe the olson(?) TZ database? though I’m not sure he is still maintaining it, and I hope I didn’t just get his name wrong :)
It’s a collaborative effort now, although Paul Eggert is prime mover: https://github.com/eggert/tz
It’s now being developed as a kernel module in ports. It should be available for 12 and 13, you just won’t be able to use it until you’ve run the pkg bootstrap. As long as you have a non-wireguard network that has a route to your package mirror, there won’t be much difference other than the need to run a single extra command once at first-install time.
Awesome!
So the best option here would have been to tell them that you won’t finish this, that you’re burnt out, that you won’t be able to create something that will be useful for them. (and make your project look bad) Instead of doing the most scammy thing that you could imagine. The whole action does not look good, especially with the accusations it’s like something you’d expect..
Also why didn’t the netgate people catch at least all those printfs and 40KLOC copy-pasted linux kernel code ?!
I’ve been in a similar situation – although I never pushed out such bad code; I just didn’t write any code – and it’s harder than it sounds. On one hand you want to do this, you don’t want to let anyone down, and feel like any moment now you’re going to shake it off and it’ll all be fine. Except that this moment never comes. It’s also hard to say “I can’t do this because (vague feelings)”. I don’t mean this to dismiss them, just that it’s hard to explain these kind of things (including to yourself), especially “in the moment”.
The real WTF here is that a single person underperforming wasn’t caught by either Netgate or FreeBSD. These things happen and are part of normal human behaviour. Especially Netgate really dropped the ball here IMHO by not just communicating with their contractor/employee throughout the process. His line manager really should have spotted that Matt wasn’t doing too well. I wonder how much Netgate’s company culture contributed to this. It doesn’t exactly seem like a very friendly company with heaps of empathy and kindness to spare…
I can empathize with this. I’ve been there. And pile on a healthy dose of “every time it’s ever felt this way before, I shook it off and it was fine” coupled with “I don’t want to have this conversation with that manager right now, because I won’t get support, just more pressure, and besides, it’s going to be fine” and I think I understand how you can get from something that feels like good intentions to something that looks like “the most scammy thing that you could imagine.”
(I’m not saying I think it was right. Just that I can see in my mind’s eye one very slippery slope that takes you exactly here.)
Yes. It sounds to me like nobody was looking. Possibly for similarly relate-able reasons, but the level of “nobody was looking” required for this to hit an RC on a project this prominent, that serves as upstream for so many other things, makes me worry a bit about the foundation of a few important infrastructure items.
Fair enough I think I might know how this feels like. I just think he could have told them about the problems and helped otherwise. Like mentoring, showing them the code and talking about the issues etc. Checking this in and going away in that state feels just wrong. Especially with said accusations, as it looks - from the outside - like they just didn’t care about that. Regarding the missing reviews/tests of such a big patch from one person I’m totally on your side.
Side note: I started maintaining a rust crate after the burnout of the original author. And the more I dig into the problems and want to “get it right”, trying to release the next major version (while having zero previous knowledge), the more I can feel the weight of how many different corners they tried to do right across all the three majors operation systems. Meanwhile the project kinda works and gets better over time as some contributors with more in-depth knowledge appear out of nowhere, contribute 1-2 PRs and vanish, fixing some of the worst heisenbugs. There is no real punchline, just my observation.
This article is very interesting, but this part makes me really uneasy… Why is the criminal past of some developer relevant? He obviously did something wrong, and paid his debt to society for it. This article reads “this guy wrote shitty code, no wonder, he was a criminal!”
It sounds like he did a “piss-poor job”, as Bruce Willis would say, and it also seemed that the review process was a little flawed. (But let’s give credit the FreeBSD people, they’re doing a great job for a group of volunteers with little resources) I just don’t see how his previous penal convictions have anything to do with it. Does this guy now has to carry this burden for his entire life?
I think the two sentences you quoted contain the answer to why it’s relevant:
Normally, when there’s a contribution to an open source project, (hopefully) there’s some review, issues are found, the parties work together to address them, and things land in a mutual effort to improve the project. This time, there was a contribution. (Serious!) Issues were found. The people who found the issues pitched in to help fix them and bring the contribution up to the standards of the project. The company sponsoring the contribution went nuts and started accusing the reviewers/other contributors of “releasing 0days”, etc.
This combative response to the code review made people dig in and look, wonder if the developer was himself a bad actor, and find past behavior that might be consistent with being a bad actor. It got reported.
I don’t think it got reported as a disproportionately large part of the story, if you look at the whole article. I think it was mainly mentioned here because it had been brought up in public discussions already, and I thought the author of this piece did a good job giving it context.
Culture perhaps? Some parts of the world follow the motto “once a criminal, always a criminal”. Harder to get employment, requirement to disclose, disqualified from voting, etc. I believe Americans have different tiers of this depending on where you live, eg “felon” (?)
I also got the impression that the criminal history was mentioned by the article as supporting evidence for him being bad at things in general. I don’t like that. If you are going to mention the bad parts of someone’s past then you need to look at the motivations & context for those bad parts (and then analyse whether they relate to now); not leave it hanging with a feeling of bad guy does bad things.
It was however nice the article talked about the successes of the author too. I am glad they did that.
So? One bad coder committed some bad code for money to FreeBSD Head. Other people saw how bad that code really was and tried to fix it, though there wasn’t much time to fix it for the next release. Doesn’t it look like a good case of code review? At least we know there are people looking at those committed code.
It looks like a bad practice for a for-profit company to contract a third person to port some code without informing any original developers. In this case the company picked a wrong person.
The point of code review is to prevent bad code from ending up in the tree in the first place. If you have bad code in your master branch and just barely avoided shipping a release with the bad code, that’s not a successful application of code review.
I agree that a lot of the blame ends up on Netgate here. Calling the discussion of the bad code a “zero-day disclosure” is especially egregious.
The company in question is “Netgate”. Unless there’s been a very stealthy acquisition, the two are not related at all.
Sorry, I misremembered from the article. Fixed.
Not even a tiny bit. It was in a release candidate, and when the code review occurred and found problems, the sponsor of the code accused the developers who reviewed and tried to fix the code of releasing 0-days and worse, while at the same time claiming out of the other side of his mouth that the problems they found weren’t real anyway.
It is an example of code review narrowly preventing something that was grossly unfit for purpose from landing in a final release as opposed to only a release candidate.
I can’t find a way to stretch this so that it looks like a “good case” of anything at all.
I wonder how much such a release candidate is actually tested. The article mentions that there are bug reports regarding the
if_wgcode in the pfSense project. The same bugs should have occurred in the FreeBSD release candidate.One of the points made by the author (and also re-iterated on a podcast he co-hosts) is that there is no formal code review process to prevent something like this from happening again. The other co-host on the podcast made the point that FreeBSD developers are for the most part unpaid volunteers who have no way of forcing anyone to do the reviews. I think both are valid points.
Is there a reason why Kernel support for WireGuard is important? Is it just for performance reasons?
Yes. Avoiding context switches for, what is essentially a virtual ethernet stack, is a giant win, at least in theory.
Is that not reason enough? I’ve done RDC over a slow VPN. It’s miserable.
That’s a big point.
Having used OpenBSD’s implementation I think another big point might be that this means it’s part of the base system and will likely end up being better integrated into it. If your system always ships with wg, you are more inclined to make it for example better measurable, debugable, configurable within that base system.
That’s less true for the Linux world where everything is seen in a more stand-alone fashion, so I guess there it’s more about distributions or tool builders embracing the use case.
This was a really strange article for me. Developers are human. A lot of us write bad code sometimes. A lot of us haven’t been performing up to our usual standards in the last year, especially if we had COVID and are still recovering. A few of us have mistakes in our past that we regret. I guarantee you you’ll find similar situations in every software project of sufficient size, probably many times a year. The goal of a software development process is to produce a decent product nonetheless. And it does seem to have actually worked in this case, although maybe not as well as desirable.
The problem being pointed out in this article is that there was no good process for catching mistakes. It’s because we’re all human and make mistakes that we need processes for catching and fixing those mistakes before they make it into production. And the way it worked in this instance was that the bad code got caught just before it made it into an official release–of one of the most important operating systems in the world.
At any reasonable organization, this would by far not be a reasonable or accepted practice.
and to add to that, this was apparently fairly clearly bad code. What if it was an obfuscated vulnerability of some kind? With such a lax review policy, who knows what has snuck in over time?
Interesting article, however there’s one point I dislike. Acting like there are no code reviews or somehow less than in other projects. This is very much untrue. See reviews.freebsd.org.
This is not to say, that the process shouldn’t be improved, but it’s not a process unlike in other big projects. In this particular instance someone with commit bit which in this community means that you should are entrusted to not do what has been done in this case.
Variations of this situation exist for many big projects. Usually the “threat” is that you are removed from the project and that you will get bad reputation from that and won’t be entrusted anymore. I think the latter situation has occurred.
Things like that have happened and will happen and while gain, I am not saying that you cannot improve the process - I am completely sure you can - this certainly isn’t a purely technical problem. From al I can see this wasn’t a malicious action, just someone who messed up in a particular situation and it being very clearly visible and loudly discussed. Acting like other, similar projects are somehow immune to this is unrealistic.
One usually uses some kind of gatekeeping, but acting like gate keepers don’t make mistakes is like saying “systems never fail”.
Yes, this is a clear sign that how reviewing works should be improved and the FreeBSD project certainly has got homework to do, but this looks mostly like a very obvious and good to analyze example.
In other words I hope that people don’t end up that this can’t happen in their project, cause they do code reviews. Reading through the discussions I get the feeling that’s happening here. If that’s the main take-away from this one misses the point and maybe an opportunity to improve their processes as well.
FreeBSD might see an uptick in code review culture now that they have moved from SVN to Git. Perhaps the “commit bit” policy used to make more sense given past challenges.
A commit bit policy helps when you have a lot to do and few people who can do it. Make the barriers for the commit bit a little higher, encourage asking for review when something is off, and good stuff will get through.