OpenSSL Cleanup: Hundreds of Commits In a Week 379
A new submitter writes with this news snipped from BSD news stalwart undeadly.org: "After the news of heartbleed broke early last week, the OpenBSD team dove in and started axing it up into shape. Leading this effort are Ted Unangst (tedu@) and Miod Vallat (miod@), who are head-to-head on a pure commit count basis with both having around 50 commits in this part of the tree in the week since Ted's first commit in this area. They are followed closely by Joel Sing (jsing@) who is systematically going through every nook and cranny and applying some basic KNF. Next in line are Theo de Raadt (deraadt@) and Bob Beck (beck@) who've been both doing a lot of cleanup, ripping out weird layers of abstraction for standard system or library calls. ... All combined, there've been over 250 commits cleaning up OpenSSL. In one week.'"
You can check out the stats, in progress.
Re:I would think (Score:5, Informative)
Note that OpenSSL isn't part of the OpenBSD project. It's a separate project.
Coding style fixes are "news" for nerds? (Score:0, Informative)
This must be one of the most ridiculous Slashdot stories ever!
Most of these commits are coding style "fixes": Example [estpak.ee].
Just again proof that these OpenBSD guys are nothing but a bunch of bigmouths. Incidently, the bug in OpenSSH ca. 10 years ago was the only time a machine of mine goot rooted.
Re:I would think (Score:5, Informative)
It would look like it's been a while since anybody did much of anything besides half hearted scratching in very limited parts of the code. This is a very much needed effort which is likely to end up much like OpenSSH, maintained mainly as part of OpenBSD, but available to any takers. We should expect to see a lot more activity before the code base is declared stable, but by now it's clear that the burden of main source maintainership moved to a more responsive and responsible team.
This isn't fixing SSL (Score:5, Informative)
The article doesn't make it completely clear that this doesn't have much to do with the fixing problems in OpenSSL.
Commits to the true OpenSSL source can be seen through the web interface at https://github.com/openssl/ope... [github.com]. What the article is talking about is tidying up the version that is built in to OpenBSD. Not that that isn't worthwhile work, but it's unlikely to fix many hidden problems in OpenSSL itself, unless the OpenBSD devs find something and hand it back to the upstream.
Re:Merged back or fork? (Score:5, Informative)
And Linus still does nothing... (Score:0, Informative)
...but throw accusations and insults about monkeys and too hard focus on security - while still being happy someone else is doing the good work.
Re:Quatity is not quality (Score:4, Informative)
Re:Merged back or fork? (Score:5, Informative)
The commits are funny into themselves. (Score:5, Informative)
A Tumblr site popped up a few days ago called OpenSSL Valhalla Rampage [opensslrampage.org]. The blogger there is going through all the commits and posting the juicy funny comments there. This includes killing... and rekilling... VMS support (which reminds me of Maxim 37: there is no such thing as overkill... [ovalkwiki.com]), stripping out now-stupid abstractions and optimizations of the unoptimizables, and more.
Re:Quatity is not quality (Score:2, Informative)
A new start would introduce far more issues, so a major cleanup can be preferable.
Some of this code is 18 years old. There are whole swathes of code that is depreciated. Cleanup and standardisation of layout helps. Removal of things like the VAX, Windows, obsolete compilers.... Already with the KNF and cruft being removed there are things being seen and some commits being made to remove scary things.
Seriously, read some of those commits and you will see that this /will/ help wrangle it into a far more secure state.
Re:Are they still running it through Coverity ? (Score:5, Informative)
You don't have to wonder why. A quick search shows that they've already blogged about why Coverity didn't detect heartbleed.
http://security.coverity.com/blog/2014/Apr/on-detecting-heartbleed-with-static-analysis.html
Re:I would think (Score:5, Informative)
And how many bugs and vulnerabilities will they put in with such high volume of commits in such short time?
You mean how many will they remove? They're largely replacing nasty portability hacks and unsafe constructs with safe, clean code. The chances are they'll be removing more bugs than they are adding.
Secondly, this is phase 1: massive cleanup. Once they've done that they are then planning on auditing the code.
f a change is only "house cleaning" which is unrelated to security, why do it in such a rush? -
it is security related: they can't audit the code (very overdue) until it's clean.
If a change is security related, and obviously needed, then why wasn't it made earlier?
Good question. Seurity people have been omplaining about the state of OpenSSL for years. But they've always had other day-jobs. I guess now there is the incentive.
Didn't that make a mockery of all the "many eyes" arguments oft touted in favor of Open Source?
Nope. Once the bug was noticed it was fixed very quickly: i.e. it was a shallow bug. If you think than phrase means OSS is bug free, you have misunderstood it.
- If a change is security related and non-obvious, then won't doing it in such a rush probably introduce new bugs/vulnerability into the code?
No, the code is too unclean for a lot of stuff to be obvious. They can't audit it until it is clean. There is a chance they will introduce some problems, but given the nature of the changes and the people involved it's likely they'll fix more than they introduce.
No matter how you look at it, making so many changes in a rush is not a good idea.
Seems like a fine idea to me.
Re:Merged back or fork? (Score:5, Informative)
Well they seem to be ripping out a lot of things related to portability, so my guess is that this new effort is a dead end that the rest of us will never see.
No: OpenBSD is a straightforward, clean, modern unix.
They are ripping out all the stuff for portability to ancient unix and even long obsolete non-unit platforms.
Much software compiles cleanly on OpenBSD, FreeBSD and Linux. If they do it well---and every interation I've had with OpenBSD code indicates that they will---it will e very easy to port it to Linux (and other modern operating systems).
I expect what will happen is they will get it working on OpenBSD with enough API compatibility to compile the ports tree. Once it begins to stabilise, I expect people will maintain branches with patches to allow portability to other operating systems.
Historical portaility causes hidoeus rot. I know: i've had it happen to me. There are old systems out there so perverse, they poison almost every part of your code. I think a semi-clean break like this (keep the good core, mercilessly rip out historically accumulated evil) is necessary.
Re:Coding style fixes are "news" for nerds? (Score:5, Informative)
Re:I would think (Score:5, Informative)
This is actually the OpenBSD developers diving in because the upstream (OpenSSL) was unresponsive. If you look at the actual commits, you will see removal of dead code such as VMS-specific hacks
That code is not dead, there are actually still people using OpenSSL on OpenVMS and actively providing patches for it: https://www.mail-archive.com/o... [mail-archive.com]
list of changes (Score:5, Informative)
See [twitter.com] also [freshbsd.org]:
So far as all the "won't this introduce more bugs than it fixes" comments go, this is a recurring argument I have at work. I am of the "clean as you go", "refactor now" school.
Everyone else says "If it works don't fix it"(IIWDFI), "don't rock the boat" etc.
Heartbleed is what happens when the IIWDFI attitude wins. Bugs lurk under layers of cruft, simple changes become nightmares of wading through a lava flow of wrappers around hacks around bodges.
Whenever anyone says IIWDFI, remind them that testing can only find a small proportion of possible bugs, so if you can't see whether it has bugs or not by reading the code, then no matter how many test cases it passes, it DOESN'T WORK.
Re:The commits are funny into themselves. (Score:4, Informative)
I'm wondering what is supposed to be mysterious about that code. The "/* increment */" comment seems to apply to the code inside the loop, not what is being done to the i variable, so I don't think that's it. Is it because the loop goes from 7 down to 0 instead of the other way around? I remember reading a programming book back in the 80's that advocated doing that for better speed since the assembly code generated to compare to 0 was faster than comparing to some other integer (which seems to no longer be the case, and I suspect could even cause cache misses for a bigger loop, although I don't know enough about how CPUs fill the cache to know for sure).
Comparing to zero is faster in most architectures and still is a valid optimization. There shouldn't be any problems with cache misses either, if the architecture does stream detection it should do it for reversed streams too and if it doesn't (only detecting actual misses) doing it in reverse isn't a problem.