6 ms·
This patch is setting OPENSSL_NO_HEARTBEAT, but openssl actually only disables heartbeats when you set OPENSSL_NO_HEARTBEATS (note trailing S). Someone should
by ctz 13y ago
This patch is setting OPENSSL_NO_HEARTBEAT, but openssl actually only disables heartbeats when you set OPENSSL_NO_HEARTBEATS (note trailing S).
Someone should make a thing which does sentiment analysis on commit messages, and flags vitriolic or angry commits as possibly containing more typos than usual.
- 0x0 13y agoThat's almost too funny. Edit: Does that last "ok" line mean that two other committers reviewed and OK'ed this patch as well? Three openbsd committers approved a one-line change that didn't fix a single thing? (And people wonder how the OpenSSL bug could live on for two years =) )
- emaste 13y agoThe typo was fixed a little while later: http://www.openbsd.org/cgi-bin/cvsweb/src/lib/libssl/ssl/Makefile.diff?r1=1.29;r2=1.30;f=h http://www.openbsd.org/cgi-bin/cvsweb/src/lib/libssl/ssl/Mak...
- opendais 13y agoIt is but we should have a dose of humility in mocking them since many, many people [including everyone on this site that is a dev] make stupid mistakes from time to time. Such is the nature of the profession. None of us commit perfect code the first time, every time.
- 0x0 13y agoExactly. I'll just leave this here. http://article.gmane.org/gmane.os.openbsd.misc/211963 http://article.gmane.org/gmane.os.openbsd.misc/211963
- IgorPartola 13y agoWha-what? That's pretty crazy. At the very least, if OpenSSL goes through this trouble, why doesn't it just implement its own malloc and friends that are both fast and secure? In fact, shouldn't it include something like scurb_and_free() so that sensitive bits would actually be erased when not needed. That seems like a much better solution than the approach described in this link.
- dgl 13y agoIt gets worse, see http://www.tedunangst.com/flak/post/analysis-of-openssl-freelist-reuse http://www.tedunangst.com/flak/post/analysis-of-openssl-free... Essentially the code mistakenly relies on the freelist being LIFO and not being scrubbed.
- TwoBit 13y agoWhoever said that OpenSSL was written by monkeys continues to be proven right.
- deleted 13y ago[deleted]
- davidw 13y agoThat's a fairly rude thing to say. Would you say that to their faces? This is a pretty large site, and a lot of people stop by here, possibly including some of the "monkeys" . By all means, call the code what you want, but separate that from the people.
- pyvpx 13y agosomething tells me a similar snafu made by a CS student would get "fairly rude" comments in reply by any competent instructor or advisor.
- bowyakka 13y agoyou say that and yet I have had to teach people coming from several fairly top institutions that "no your lecturer was wrong, it is inappropriate to directly memcpy() from the network into a struct"
- SimHacker 13y agoWell Ted Nugent (not to mention Ann Coulter) got away with calling Obama a "monkey," and he's the highly respected darling of the GOP, whose endorsement is sought after and appreciated by not only Republican senators and congressmen, but Mitt Romney himself. (I'm not endorsing anyone's behavior here, just pointing out how low the right hand side of modern American politics has degenerated, to provide some perspective.)
- Peaker 13y agoMany projects that care about performance and memory fragmentation use per-purpose free lists, and not malloc/free. The performance is significantly better, and there is a much smaller danger of running out of space due to fragmentation issues.
- pdabbadabba 13y agoGood advice for for the author of that rage-filled patch as well.
- CHY872 13y agoWell, he has good reason to be enraged. The bug is a horrendous one and on his operating system was due to the developers deliberately the operating system provided security protections. Meanwhile his slightly buggy one liner failed in such a way that it did nothing, before being fixed 3 hours later.
- kzrdude 13y agoAbsolutely. What matters is not to always be right, but how you correct errors.
- jeffdavis 13y ago"What matters is not to always be right, but how you correct errors." In my opinion, heartbleed is a counterexample to your point. It's a case where making the mistake at all caused a lot of damage, no matter how quickly they patched it.
- nileshtrivedi 13y agoWell, it can be argued that this instance of mistake happened BECAUSE they didn't fix their review process the last time a silly bug managed to get in.
- midas007 13y agoSo there's enough blame and rage to go around. Any leader with a spine has to first contemplate "how did I fuck up?"
- pessimizer 13y agoWhat if your error was running someone over with your car?
- annnnd 13y agoYes and no. The major mistake is not a missing "S". The major mistake is not performing a test which would show that the change was made (or in this case that it wasn't made).
- peterwwillis 13y agoTest our changes before we commit? What blasphemy is this?
- Angostura 13y agoAt the very least the organization should look at itself very carefully, find out how this this happened.
- ChrisAntaki 13y agoHindsight is 20/20.
- sigzero 13y agoThey caught it. It is in a newer commit now. Still kind of "funny".
- jholloway7 13y agoCongratulations, you have hereby been granted commit privileges to libssl. Have fun!
- nly 13y agoSo, basically, the patch wasn't tested. This whole thing is getting more and more embarrassing.
- ChrisAntaki 13y agoIt just goes to show, paying attention has a cost, energy. When energy is expended on a fiery commit message, the message can become a distraction. There is an energetic cost to all actions, and strategies. That being said, I fully endorse this commit. There seem to be good intentions behind it, and it was fixed soon after.
- midas007 13y agoI bet big dev shops would actually pay for this to know whom needs a vacation, etc.
- josteink 13y agoThis patch is setting OPENSSL_NO_HEARTBEAT, but openssl actually only disables heartbeats when you set OPENSSL_NO_HEARTBEATS (note trailing S). Having negative ifdefs in the first place is a pretty bad practice. This clearly shows why. If all your IFDEFs are "positive" they have to be declared somewhere and then it's easy to "deactivate" the things you no longer want, by simply commenting a line out. It reduces the possibility for error, and gives you a better picture of how many IFDEF conditions you are dealing with around your code.
- deleted 13y ago[deleted]
- Peaker 13y agoA security bug may well require enabling a feature, rather than disabling one, and then you end up with exactly the same problem. I personally detest #ifdef's, and would rather have multiple .c files that I can choose from to include in my build. Code infected with #ifdef'itis is unreadable, difficult to test, difficult to maintain, ...
- SimHacker 13y agoThat will just drive people to make sarcastic commit messages.