Slashdot Mirror


OpenSSL Cleanup: Hundreds of Commits In a Week

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.

28 of 379 comments (clear)

  1. Re:I would think by Anonymous Coward · · Score: 5, Informative

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

  2. it's a good effort by tero · · Score: 5, Interesting

    Right now, I think the team is mostly focused on having "something usable" in OpenBSD and I doubt they care too much about anything else outside their scope.

    Having said that - forking OpenSSL to something usable and burning the remains with fire is a great idea, however there is considerable risk that the rush will cause new bugs - even though right now those commits have been mostly pulling out old crap.

    Fixing the beast is going to take a long while and several things will need to happen:
    - Upstream hurry to put more crap into the RFC needs to cease for a while. We don't need more features at the moment, we need stability and security.
    - Funding. The project needs to be funded somehow. I think a model similar to Linux Foundation might work - as long as they find a suitable project leads. But major players need to agree on this - and that's easier said than done (who will even pull them to the table?)
    - Project team. Together with funding, we need a stable project team. Writing good crypto code in C, is bloody hard, so the team needs to be on the ball - all the time. And the modus operandi should be "refuse features, increase quality". Requires a strong Project Lead.
    - Patience.. fixing it is a long process, so you can't go into it hastily. You need to start somewhere (and here I applaud the OpenBSD team), but to get it done, assuming that above is in place - expect 1-3 years of effort.

  3. Re:I would think by badger.foo · · 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, 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.

    --
    -- That grumpy BSD guy - http://bsdly.blogspot.com/
  4. Re:I assume this is an R&D exercise? by Anonymous Coward · · Score: 5, Insightful

    The fact that these 250 commits are mostly coding-style changes was conveniently hidden behind the acronym "KNF". Honi soit qui mal y pense!

  5. This isn't fixing SSL by beezly · · Score: 4, 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.... 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.

  6. Re:Merged back or fork? by badger.foo · · Score: 4, Informative

    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.

    --
    -- That grumpy BSD guy - http://bsdly.blogspot.com/
  7. Re:Quatity is not quality by drolli · · Score: 4, Insightful

    I cant talk for C, but in Java the tools which warn you about potentially dangerous constructs are great (e.g. Sonar). You can easily identify many *suspicous* contructs and change them to something more safe. 250 commits per week with 4 devs on a moderatly sized project do not see much to me, much at the "quality" and not the "quantity" side.

    What annoys me is that - with all due respect - the companies which embed openssl in their products could have done a review of the code for quality. To me it seems that it's a fundamental library.

  8. Re:Merged back or fork? by smash · · Score: 4, Informative

    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.

    --
    I run: Windows, OS X, Linux, FreeBSD. Just because you have a hammer, doesn't mean everything is a nail.
  9. Re:I would think by khchung · · Score: 5, Interesting

    Well, I would think that this is mostly to do with publicity. Once someone calls your software into question in a very public light, you will be more willing to go through your project with a fine toothed comb and clean up all that old cruft you've been meaning to clear out.

    This is not a sign of inherent insecurity, but one of obvious house cleaning.

    And how many bugs and vulnerabilities will they put in with such high volume of commits in such short time?

    - If a change is only "house cleaning" which is unrelated to security, why do it in such a rush?

    - If a change is security related, and obviously needed, then why wasn't it made earlier? Didn't that make a mockery of all the "many eyes" arguments oft touted in favor of Open Source?

    - 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 matter how you look at it, making so many changes in a rush is not a good idea.

    --
    Oliver.
  10. Re:I would think by Richard_at_work · · Score: 5, Interesting

    As the other poster says, OpenSSL isn't an OpenBSD project - what is going on here is a full blown fork of OpenSSL by the OpenBSD team, who are putting their money where their mouths are because when the heartbleed bug came out it was noted that the issue could have been mitigated on OpenBSD if the OpenSSL team had used the system provided memory allocation resources.

    So this is less OpenSSL and much more OpenBSD SSL being created.

  11. The commits are funny into themselves. by strredwolf · · Score: 4, Informative

    A Tumblr site popped up a few days ago called OpenSSL Valhalla Rampage. 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...), stripping out now-stupid abstractions and optimizations of the unoptimizables, and more.

    --

    --
    # Canmephians for a better Linux Kernel
    $Stalag99{"URL"}="http://stalag99.net";
  12. Re:Are they still running it through Coverity ? by Anonymous Coward · · 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

  13. Re:code review time! by St.Creed · · Score: 4, Funny

    The NSA, obviously.

    Next question please.

    --
    Therefore, by the (faulty) logic you're using, you're just a cow with a keyboard - osu-neko (2604)
  14. Re:I would think by serviscope_minor · · 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.

    --
    SJW n. One who posts facts.
  15. Re:Merged back or fork? by serviscope_minor · · Score: 4, 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.

    --
    SJW n. One who posts facts.
  16. Thank You by Anonymous Coward · · Score: 5, Insightful

    just a simple thank you to all the coders out there who donate of their skills and time to produce this and other very important software, for free folks! Thank You for making the world a better place

  17. Re:I would think by InvalidError · · Score: 5, Insightful

    From the looks of it, many of the (potential) bugs in OpenSSL are caused by the use of a custom memory allocation scheme instead of a standard C allocator. Removing the custom memory management in favor of standard memory management alone implies dozens if not hundreds of relatively trivial code changes to all the places where the custom (de-)allocator get used. In the process of tracking down all of these, they come across stuff that does not look right and fix it while they are already in there.

    As for why so many bugs, "so many eyes" only works if you still have tons of people actively participating in the project's development. At a glance, it seems like the OpenBSD guys are saying the OpenSSL project was getting stale. Stale projects do not have anywhere near as many eyes going through their code nor as many people actively looking for potential bugs to fix before they get reported in the wild.

    In short: OpenSSL was long overdue for a make-over.

  18. Re:Coding style fixes are "news" for nerds? by K.+S.+Kyosuke · · Score: 4, Informative

    Most? 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?

    --
    Ezekiel 23:20
  19. Re:I would think by stor · · Score: 5, Insightful

    IMNSHO this is more about the OpenBSD folks doing what they do best.

    I sincerely respect their approach to programming, especially with regards to security: rather than just introduce security layers and mechanisms they program defensively and try to avoid or catch fundamental, exploitable mistakes.

    Hats off to the OpenBSD folks.

    --
    "Yeah well there's a lot of stuff that should be, but isn't"
  20. Re:I would think by Halo1 · · Score: 4, 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...

    --
    Donate free food here
  21. Re:I would think by Kythe · · Score: 5, Insightful

    All of what you say may be correct. But one major bug doesn't prove it. I do recall seeing quite a few life-or-death bugs in closed source projects over the years -- including stuff that most people use. So it's unclear to me whether you have additional evidence to support your statement, or are simply saying something self-serving.

    There's a reason why the plural of anecdote isn't evidence.

    On a related note, there's a reason why most of the most prominent security researchers seem to prefer open access to source code. I have yet to hear any of them change their mind over this bug.

    --

    Kythe
  22. Re:I would think by Antique+Geekmeister · · Score: 5, Insightful

    > Multiple eyes on code, security, these are things that are great about open source, except they aren't. This is a prime example of how bugs get through anyhow, major bugs. So it is now shown beyond a shadow of anyones doubt, open source is NOT superior in these respects.

    Really, no. The horses are still pulling plows, and carts, and carriages, every day. The library is still in use in operating systems world wide.

    This is more visiting the barn that had horses stolen and making sure the locks and doors actually work the way they should before it's trusted at all again.

  23. list of changes by monkey999 · · Score: 5, Informative
    A summary of the changes is here :

    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 also:

    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.

  24. Hey - Thanks OpenSSL Contributors by bokmann · · Score: 5, Insightful

    With all the other tripe on this thread, I thought it necessary to say this loud and clear:

    Hey OpenSSL Contributors - thanks for your hard work on OpenSSL, and thanks for the hard work under this spotlight cleaning this up.

    Any serious software engineer with a career behind them has worked on projects with great source code, bad source code, and everything in between. It sounds like OpenSSL is a typical project with tons of legacy code where dealing with legacy is lower priority than dealing with the future. Subtracting out all the ideological debate and conspiracy theories, please realize there are plenty of 'less noisy' people out there who appreciate everything you're doing. And even more who would appreciate it if they understood the situation.

    Its now time for companies who depend on OpenSSL (and other projects) to realize that Open Source software can lower their development costs, but some of that savings needs to be put back into the process or we will all suffer from "the tragedy of the Commons".

  25. Re:I would think by Enigma2175 · · Score: 5, Insightful

    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.

    But the whole heartbleed issue was caused by someone who was doing more than "half hearted scratching", he was adding an entirely new feature (heartbeats). Does anyone else think that hundreds of commits in a week is a BAD thing? It seems to me like committing that much code would make it so each change doesn't get as much of a review as it would if the changes were committed gradually. Poor review is what caused this problem in the first place, they run the risk of adding another critical vulnerability.

    --

    Enigma

  26. Re:I would think by causality · · Score: 5, Insightful

    disagree: mocking people for making mistakes that they should know better is a way to help that person permanently try harder to avoid those mistakes in the future.

    with failure, comes mockery, especially if you are skilled and it should never have happened.

    mistakes can't go unpunished, even if the person doing the punishing is yourself, you can't tell other people to back off, you deserve it, sit back and take it on the chin and try harder next time otherwise people won't have any reason to try, because the penalty for failure is barely noticeable.

    That's the old-school view, in which one's self-esteem is based on achievement of some kind. Those who achieve little or nothing had low self-esteem and this was a principal incentive to identify one's own weaknesses and overcome them with directed effort. The extreme form is Japanese students throwing themselves off buildings (etc.) because their grades didn't quite measure up, making them nobodies.

    The newer view is that everyone is a special snowflake. No matter what. The extreme form is shown by the public schools that play soccer without keeping score, because scoring implies winners and losers and that might hurt someone's feelings.

    I mostly agree with you in that actions have consequences and you should accept the consequences of your own actions. Otherwise nothing really matters and there is no reason to improve yourself and you turn into one of these "perpetual victims" who never take responsibility for anything while simultaneously wondering why nothing ever changes. But that should be tempered with the fact that some mistakes are much more preventable (less understandable) than others, and as Orlando Battista once said, an error doesn't become a mistake until you refuse to correct it.

    There's no reason to metaphorically crucify someone for an honest mistake, but certainly there is going to be a reaction to it and people aren't going to like it. That's to be expected. It's reasonable to expect someone to accept that and yes, it is an incentive to learn something from the experience and be more careful in the future. If I were a programmer and found that completely unacceptable, I could always choose not to work on such an important project critical to the security of so many.

    As an aside: I think replying to you is much more edifying than being like the cowards who modded you down to -1 without once putting forth their own viewpoint which they clearly think is superior. There's too much of that going on at this site. There is no "-1 Disagree" mod for a reason.

    --
    It is a miracle that curiosity survives formal education. - Einstein
  27. Re:Quatity is not quality by VortexCortex · · Score: 4, Interesting

    I cant talk for C, but in Java

    Haha. Oh man. Java is a VM. Do you check for "dangerous constructs" like Just In Time compiling of data into machine code at Runtime and marking data executable then running it by the Java VM? Because that's how it operates. Even just having that capability makes Java less secure, don't even have to get exploit data marked code and executed, just have to get it in memory then jump to the location of the VM code that does it for me with my registers set right. Do any of your Java code checking tools run against the entire API kitchen sink of that massive attack surface you bring with every Java program called the JRE? Do they prevent users from having tens of old deprecated Java runtimes installed at 100MB a pop, since the upgrade feature just left them there and thus are still able to be targeted explicitly by malicious code? No? Didn't think so.

    Don't get me wrong, I get what you're saying, Java code can be secure, but you have to run tests against the VM and API code you'll be using too. Java based checking tools produce programs that are just as vulnerable as C code, and actually demonstrably more so when you factor in their exploit area of their operating environment. Put it this way: The C tools (valgrind) already told us that the memory manager was doing weird shit -- It was expected weird shit. No dangerous construct warning would have caught heartbleed, it's a range check error coupled with the fact that they were using custom memory management. The mem-check warnings are there, but they have been explicitly ignored. It would be like the check engine light coming on, but you know the oil pressure is fine, just the sensor is bad... so no matter how bright of a big red warning light you install it can't help you anymore, it's meaningless. Actually, it's a bit worse than that, it would be like someone knew your check engine lights were on because of some kludge they added for SPEED, and so they knew they could get away with pouring gasoline in your radiator because you wouldn't notice anything wrong until it overheated and blew up -- AND you asked them about the check engine light a few times over the past two years, but they just shrugged and said, "Don't worry about it, I haven't looked under the hood lately, but here's a bit of electrical tape if the light annoys you."

    I write provably secure code all the time in C, ASM (drivers mostly), even my own languages. CPUs are a finite state machines, and program functions have finite state as well. It's fully possible to write and test code for security that performs as it should for every possible input. For bigger wordsize CPUs, Instead of testing every input, one just needs to test a sufficiently large number of them to exercise all the bit ranges and edge cases. As you've noted, automation is key. If you want to write secure code you have to think like a cracker. My build scripts automatically generate missing unit test and fuzz testing stubs based on the function signatures. Input fuzzing tests are what a security researching hacker or bug exploiting cracker will use first off on any piece of code to test for potential weakness, so if you're not using these tests your code shouldn't touch crypto or security products, it's simply not been tested. Using a bit of doc-comments to add a additional semantics I can auto generate the tests for ranges, and I don't commit code to the shared repos that doesn't have 100% test coverage in my profiler. If OpenSSL was using even just a basic code coverage tool to ensure each branch of #ifdef was compilable they'd have caught this heartbleed bug. I recompiled OpenSSL without the heartbeat option as soon as my news crawler AI caught wind of it.

    Code review, chode review. These dumbasses aren't using basic ESSENTIAL testing methodology you'd use for ANY product even mildly secure: Code Coverage + memory checking is the bare minimum for anything that has to do with "credentials". They apparently also have no fuck

  28. Re:I would think by arth1 · · Score: 4, Insightful

    Yup. I can't believe that there were such dodgy trade-offs made for SPEED (at the expense of code readability and complexity) in openSSL.

    At least a couple of reasons:
    - First of all, OpenSSL was designed with much slower hardware in mind. MUCH slower. And much of is in still in use - embedded devices that last for 15+ years, for example.
    - Then there's the problem that while you can dedicate your PC to SSL, the other end seldom can. A single server may serve hundreds or thousands of requests, and doesn't have enough CPUs to dedicate one to each client. Being frugal with resources is very important when it comes to client/server communications, both on the network side and the server side.

    Certain critical functionality should be written highly optimized in low level languages, with out-of-the-box solutions for cutting Gordian knots and reduce delays.
    A problem is when you get code contributes who think high level and write low level, like in this case. Keeping unerring mental track of what's data, pointers and pointers to pointers and pointers to array elements isn't just a good idea in C - it's a must.
    But doing it correctly does pay off. The often repeated mantra that high level language compilers do a better job than humans isn't true, and doesn't become true through repetition. The compilers can do no better than the person programming them, and for a finite size compiler, the optimizations are generic, not specific. And a good low level programmer can take knowledge into effect that the compiler doesn't have.
    The downside is a higher risk - the programmer has to be truly good, and understand the complete impact of any code change. And the APIs have to be written in stone, so an optimization doesn't break something when an API changes.