3 ms·

Yeah, sorry, I'm mostly lamenting that there's not a more objective overview. This whole situation is pretty tiring at this point, so it's unlikely that one wil

Yeah, sorry, I'm mostly lamenting that there's not a more objective overview. This whole situation is pretty tiring at this point, so it's unlikely that one will surface except as a post-mortem down the road.

busterarm 6y ago
I think that unfortunately a lot of people can't separate "this code is bad" from "this person is bad". We all have times when we don't ship our best work. Life happens. It's also worth noting to the readers that Donenfeld's criticism of bad code is completely dispassionate and that he isn't above criticising himself. He seems to be genuinely among the nicest people in the community. This seems unfortunate for mmacy and unfortunately exacerbated by the behavior of Netgate.
- loeg 6y ago
  > It's also worth noting to the readers that Donenfeld's criticism of bad code is completely dispassionate and that he isn't above criticising himself. Er, what? Donenfeld's hyperbole and wild characterizations are part of what fanned the flames and made this a tech-press mess instead of some quiet collaboration and bug reports.[0] > The first step was assessing the current state of the code the previous developer had dumped into the tree. It was not pretty. I imagined strange Internet voices jeering, “this is what gives C a bad name!” There were random sleeps added to “fix” race conditions, validation functions that just returned true, catastrophic cryptographic vulnerabilities, whole parts of the protocol unimplemented, kernel panics, security bypasses, overflows, random printf statements deep in crypto code, the most spectacular buffer overflows, and the whole litany of awful things that go wrong when people aren’t careful when they write C. While some details are based in reality, the paragraph goes well beyond the realm of truth. It makes totally unnecessarily jabs at Macy as "the previous developer." He is (broadly) a competent C/kernel developer. Yes, he did an inadequate job here. No, some Greek chorus isn't jeering about the C code just because stylistically it differs from how Donenfeld would write it. To my knowledge: * There was only a single "validation function that returned true," and it involved validating an ip address internal to a validated and decoded message from a wg peer. The message is already cryptographically verified; only peers that are part of the same mesh could spoof IPs outside of their configured range. (Donenfeld described this as validation functions, plural.) * Donenfeld's only ever found a single real buffer overflow. It's the one where Jumbo frames can cause heap overflow. His other buffer overflow claims are not realistic due to other constraints on the inputs. Mostly they seem to reflect stylistic preferences about using mallocarray(a, n) instead of malloc(a * n). So the claims of "spectacular" buffer overflow(s), plural, feels disingenuous. (I don't know what "spectacular" is supposed to mean in a cold technical critique, either.) Maybe this is "dispassionate," but it seems unnecessarily careless with the facts when writing technical criticism. To be clear, Netgate's press response to this was totally inappropriate and also just a dumb move. The public narrative would be more in their favor if they had been totally silent instead of posting the angry screed they did. Ars takes Donenfeld's hyperbole and runs with it, fact-checking only the easily verified claims. And there is some truthiness to it! Unfortunately, it's the rest of the communication that leaves something wanting. Anyway, I love wireguard and what Donenfeld has accomplished. I just wish the guy would be a bit more considerate and less colorful when writing sensitive emails. [0]: https://lists.zx2c4.com/pipermail/wireguard/2021-March/006494.html https://lists.zx2c4.com/pipermail/wireguard/2021-March/00649...
  - wbl 6y ago
    Mallocarray exists for a very good reason. If you don't check for overflow you can allocate a much smaller buffer than needed. Mallocarray handles this case for you.
    - loeg 6y ago
      I understand what mallocarray does and why it is preferable. In these cases, the multiplied values happen to be constrained such that overflow is not possible. Given that precondition, the two patterns are functionally equivalent. For what it's worth, I tend to advocate for using mallocarray and would use mallocarray in the same places Donenfeld does here. But unless overflow can actually happen, it's stylistic rather than "bug."
  - kaliszad 6y ago
    If a security product/ supposedly trusted projects code loses some company's secrets or hurts somebody, nobody cares whether it was a single bad day, a mishap if you will or something that could have been forseen. The data is now in bad hands, peoples lives are disturbed. Everything else is basically just talk until _proven_ otherwise. FreeBSD really dodged the ball at the last minute here. Would you employ somebody for work on a security product that obviously behaved as a maniac, basically robbed his own family of savings and landed together with his wife in prison for years? Is this person that good that there was literally nobody else to ask? It seems, Donenfeld at al. did a good job in 1-2 weeks porting code to FreeBSD, it may be better quality than the Macy's quasi-original developed over months of work. (Some of the code seems to have been rather similar to a differently licensed code elsewhere.) * Ok, so Donenfeld maybe is right, maybe he just dropped an extra s in an _email_. * The buffer overflow was quite spectacular. A network professional in a security product should handle jumbo frames. Maybe there are other less obvious and maybe less spectacular overflows elsewhere. This one just hit Jim Salters eye (grep). Btw. how would you feel about somebody basically doing your trademark (Wireguard in this case) a bad reputation? I could understand it, if Donenfeld took it personally. It seems though, he didn't. Macy on the other hand wouldn't admit to the poor quality of the software he wrote until pressured with clear evidence and even then he couldn't fully swallow his ego. Ars Technica/ Jim Salter did some great journalism here. It goes way beyond the quality of the average article even at Ars and that is a very decent bar. Yes, Wireguard is great, Donenfeld and friends have done a tremendous job over the years.

busterarm 6y ago

I think that unfortunately a lot of people can't separate "this code is bad" from "this person is bad". We all have times when we don't ship our best work. Life happens. It's also worth noting to the readers that Donenfeld's criticism of bad code is completely dispassionate and that he isn't above criticising himself. He seems to be genuinely among the nicest people in the community. This seems unfortunate for mmacy and unfortunately exacerbated by the behavior of Netgate.

loeg 6y ago
> It's also worth noting to the readers that Donenfeld's criticism of bad code is completely dispassionate and that he isn't above criticising himself. Er, what? Donenfeld's hyperbole and wild characterizations are part of what fanned the flames and made this a tech-press mess instead of some quiet collaboration and bug reports.[0] > The first step was assessing the current state of the code the previous developer had dumped into the tree. It was not pretty. I imagined strange Internet voices jeering, “this is what gives C a bad name!” There were random sleeps added to “fix” race conditions, validation functions that just returned true, catastrophic cryptographic vulnerabilities, whole parts of the protocol unimplemented, kernel panics, security bypasses, overflows, random printf statements deep in crypto code, the most spectacular buffer overflows, and the whole litany of awful things that go wrong when people aren’t careful when they write C. While some details are based in reality, the paragraph goes well beyond the realm of truth. It makes totally unnecessarily jabs at Macy as "the previous developer." He is (broadly) a competent C/kernel developer. Yes, he did an inadequate job here. No, some Greek chorus isn't jeering about the C code just because stylistically it differs from how Donenfeld would write it. To my knowledge: * There was only a single "validation function that returned true," and it involved validating an ip address internal to a validated and decoded message from a wg peer. The message is already cryptographically verified; only peers that are part of the same mesh could spoof IPs outside of their configured range. (Donenfeld described this as validation functions, plural.) * Donenfeld's only ever found a single real buffer overflow. It's the one where Jumbo frames can cause heap overflow. His other buffer overflow claims are not realistic due to other constraints on the inputs. Mostly they seem to reflect stylistic preferences about using mallocarray(a, n) instead of malloc(a * n). So the claims of "spectacular" buffer overflow(s), plural, feels disingenuous. (I don't know what "spectacular" is supposed to mean in a cold technical critique, either.) Maybe this is "dispassionate," but it seems unnecessarily careless with the facts when writing technical criticism. To be clear, Netgate's press response to this was totally inappropriate and also just a dumb move. The public narrative would be more in their favor if they had been totally silent instead of posting the angry screed they did. Ars takes Donenfeld's hyperbole and runs with it, fact-checking only the easily verified claims. And there is some truthiness to it! Unfortunately, it's the rest of the communication that leaves something wanting. Anyway, I love wireguard and what Donenfeld has accomplished. I just wish the guy would be a bit more considerate and less colorful when writing sensitive emails. [0]: https://lists.zx2c4.com/pipermail/wireguard/2021-March/006494.html https://lists.zx2c4.com/pipermail/wireguard/2021-March/00649...
- wbl 6y ago
  Mallocarray exists for a very good reason. If you don't check for overflow you can allocate a much smaller buffer than needed. Mallocarray handles this case for you.
  - loeg 6y ago
    I understand what mallocarray does and why it is preferable. In these cases, the multiplied values happen to be constrained such that overflow is not possible. Given that precondition, the two patterns are functionally equivalent. For what it's worth, I tend to advocate for using mallocarray and would use mallocarray in the same places Donenfeld does here. But unless overflow can actually happen, it's stylistic rather than "bug."
- kaliszad 6y ago
  If a security product/ supposedly trusted projects code loses some company's secrets or hurts somebody, nobody cares whether it was a single bad day, a mishap if you will or something that could have been forseen. The data is now in bad hands, peoples lives are disturbed. Everything else is basically just talk until _proven_ otherwise. FreeBSD really dodged the ball at the last minute here. Would you employ somebody for work on a security product that obviously behaved as a maniac, basically robbed his own family of savings and landed together with his wife in prison for years? Is this person that good that there was literally nobody else to ask? It seems, Donenfeld at al. did a good job in 1-2 weeks porting code to FreeBSD, it may be better quality than the Macy's quasi-original developed over months of work. (Some of the code seems to have been rather similar to a differently licensed code elsewhere.) * Ok, so Donenfeld maybe is right, maybe he just dropped an extra s in an _email_. * The buffer overflow was quite spectacular. A network professional in a security product should handle jumbo frames. Maybe there are other less obvious and maybe less spectacular overflows elsewhere. This one just hit Jim Salters eye (grep). Btw. how would you feel about somebody basically doing your trademark (Wireguard in this case) a bad reputation? I could understand it, if Donenfeld took it personally. It seems though, he didn't. Macy on the other hand wouldn't admit to the poor quality of the software he wrote until pressured with clear evidence and even then he couldn't fully swallow his ego. Ars Technica/ Jim Salter did some great journalism here. It goes way beyond the quality of the average article even at Ars and that is a very decent bar. Yes, Wireguard is great, Donenfeld and friends have done a tremendous job over the years.