Forgot your password?
typodupeerror
Encryption Open Source Privacy Software

OpenSSL Cleanup: Hundreds of Commits In a Week 379

Posted by timothy
from the the-good-kind-of-competition dept.
New submitter CrAlt (3208) 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.
This discussion has been archived. No new comments can be posted.

OpenSSL Cleanup: Hundreds of Commits In a Week

Comments Filter:
  • Re:I would think (Score:5, Informative)

    by Anonymous Coward on Sunday April 20, 2014 @06:51AM (#46798619)

    Note that OpenSSL isn't part of the OpenBSD project. It's a separate project.

  • by Anonymous Coward on Sunday April 20, 2014 @07:00AM (#46798657)

    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)

    by badger.foo (447981) <peter@bsdly.net> on Sunday April 20, 2014 @07:01AM (#46798661) Homepage
    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, but also weeding out a lot of fairly obvious bugs, unsafe practices such as trying to work around the mythical slow malloc, feeding your private key to the randomness engine, use after free, and so on.

    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.
  • by beezly (197427) <beezly@nosPaM.beezly.org.uk> on Sunday April 20, 2014 @07:03AM (#46798671) Homepage

    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.

  • by badger.foo (447981) <peter@bsdly.net> on Sunday April 20, 2014 @07:06AM (#46798679) Homepage
    The work by the OpenBSD developers happens in the OpenBSD tree. Whether or not the OpenSSL project chooses to merge back the changes into their tree is yet to be seen. Given the activity level in the OpenSSL tree lately I find it more likely that the primary source of a maintained open source SSL library shifts to the OpenBSD project. To the extent that portability goo is needed it will likely be introduced after the developers consider the code base stable enough.
  • by Anonymous Coward on Sunday April 20, 2014 @07:21AM (#46798723)

    ...but throw accusations and insults about monkeys and too hard focus on security - while still being happy someone else is doing the good work.

  • by carlhaagen (1021273) on Sunday April 20, 2014 @07:23AM (#46798731)
    It seems you're not familiar with the process of software development. You just don't issue one single commit containing a billion changes. It's a step by step process, for several reasons, most importantly the mechanic and iterative process of searching for bugs.
  • by smash (1351) on Sunday April 20, 2014 @07:33AM (#46798751) Homepage Journal
    Not necessarily. They are ripping out a lot of crap, much of which is portability done badly. The priority, it appears to is get back to a minimalist, secure code base, and then re-port it (to selected, actually used architectures, not big-endian x86 for example - which was some of the code removed) as time permits.
  • by strredwolf (532) on Sunday April 20, 2014 @07:40AM (#46798777) Homepage Journal

    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.

  • by Anonymous Coward on Sunday April 20, 2014 @07:48AM (#46798787)

    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.

  • by Anonymous Coward on Sunday April 20, 2014 @07:55AM (#46798793)

    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)

    by serviscope_minor (664417) on Sunday April 20, 2014 @08:02AM (#46798815) Journal

    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.

  • by serviscope_minor (664417) on Sunday April 20, 2014 @08:08AM (#46798825) Journal

    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.

  • by K. S. Kyosuke (729550) on Sunday April 20, 2014 @08:16AM (#46798853)
    Most? [estpak.ee] You've linked one big commit fixing a lot of style problems. Other than that, I see missing checks being added and strange code being fixed all over the place in a lot of commits. Aren't you twisting the reality just a little bit?
  • Re:I would think (Score:5, Informative)

    by Halo1 (136547) <jonas.maebe@nOsPam.elis.ugent.be> on Sunday April 20, 2014 @08:47AM (#46798911) Homepage

    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)

    by monkey999 (3597657) on Sunday April 20, 2014 @09:17AM (#46799001) Journal
    A summary of the changes is here [undeadly.org] :

    Changes so far to OpenSSL 1.0.1g since the 11th include:

    • Splitting up libcrypto and libssl build directories
    • Fixing a use-after-free bug
    • Removal of ancient MacOS, Netware, OS/2, VMS and Windows build junk
    • Removal of “bugs” directory, benchmarks, INSTALL files, and shared library goo for lame platforms
    • Removal of most (all?) backend engines, some of which didn’t even have appropriate licensing
    • Ripping out some windows-specific cruft
    • Removal of various wrappers for things like sockets, snprintf, opendir, etc. to actually expose real return values
    • KNF of most C files
    • Removal of weak entropy additions
    • Removal of all heartbeat functionality which resulted in Heartbleed

    See [twitter.com] also [freshbsd.org]:

    Do not feed RSA private key information to the random subsystem as entropy. It might be fed to a pluggable random subsystem.... What were they thinking?!

    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.

  • by Megol (3135005) on Sunday April 20, 2014 @09:45AM (#46799091)

    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.

I wish you humans would leave me alone.

Working...