Slashdot Mirror


OpenBSD Team Cleaning Up OpenSSL

First time accepted submitter Iarwain Ben-adar (2393286) writes "The OpenBSD has started a cleanup of their in-tree OpenSSL library. Improvements include removing "exploit mitigation countermeasures", fixing bugs, removal of questionable entropy additions, and many more. If you support the effort of these guys who are responsible for the venerable OpenSSH library, consider a donation to the OpenBSD Foundation. Maybe someday we'll see a 'portable' version of this new OpenSSL fork. Or not."

12 of 304 comments (clear)

  1. Re:Okay, Go! by Rigel47 · · Score: 5, Informative

    It's not a shameless plug. Theo has been openly critical of the OpenSSL team's development practices. By forking in-house he's essentially saying that they will put their proverbial money where their mouth is by doing their own development.

  2. "Please Put OpenSSL Out of Its Misery" by Anonymous Coward · · Score: 5, Informative

    We also have a comment from the FreeBSD developer Poul-Henning Kamp.

    He starts by saying "The OpenSSL software package is around 300,000 lines of code, which means there are probably around 299 bugs still there, now that the Heartbleed bug — which allowed pretty much anybody to retrieve internal state to which they should normally not have access — has been fixed." After that he notes that we need to ensure that the compiler correctly translates the high-level language to machine instructions. Later Kamp rants a bit about the craziness of CAs in general — would you trust "TÜRKTRUST BLG LETM VE BLM GÜVENL HZMETLER A.."? Then he lists some bullet points about things that are wrong in OpenSSL:

    - The code is a mess
    - Documentation is misleading
    - The defaults are deceptive
    - No central architectural authority
    - 6,740 goto statements
    - Inline assembly code
    - Multiple different coding styles
    - Obscure use of macro preprocessors
    - Inconsistent naming conventions
    - Far too many selections and options
    - Unexplained dead code
    - Misleading and incoherent comments

    "And it's nobody's fault. No one was ever truly in charge of OpenSSL, it just sort of became the default landfill for prototypes of cryptographic inventions, and since it had everything cryptographic under the sun (somewhere , if you could find out how to use it), it also became the default source of cryptographic functionality. [...] We need a well-designed API, as simple as possible to make it hard for people to use it incorrectly. And we need multiple independent quality implementations of that API, so that if one turns out to be crap, people can switch to a better one in a matter of hours."

    1. Re:"Please Put OpenSSL Out of Its Misery" by Anonymous Coward · · Score: 5, Informative

      I agree that the OpenSSL code base is very bad. (I was doing some work based on modifying the library recently and I had to hold my nose.) However, I take objection with some of this:

      - 6,740 goto statements

      Otherwise known as "the only sane way to simulate exceptions in C". Seriously. Read up on how "goto" is used in low-level code bases such as OS kernels, instead of citing some vague memory of a 1960s paper without understanding its criticisms.

      - Inline assembly code

      Otherwise known as "making the thing go fast". Yes, I want the bignum library, or hashing algorithms, to use assembly. Things like SIMD make these tasks really effing fast and that is a good thing...

  3. Re:I just donated by Anonymous Coward · · Score: 5, Funny

    I know. Pay your bill, your card is being refused by everybody.

  4. Re:What about a re-implementation... by Lunix+Nutcase · · Score: 5, Insightful

    So if C is so bad why should we trust the languages that are implemented in it? You do realize that most of these "safe" languages are written in C, right?

  5. "Ancient." "Cruft." by jabberw0k · · Score: 5, Insightful

    I read that as discarding stuff for Windows 98, 2000, and other ancient platforms that have fallen almost entirely from use, and certainly outside the pool of what's tested: A good thing.

  6. While it may sound like that... by Anonymous Coward · · Score: 5, Insightful

    The first step to cleaning up the code is getting it into a state where you're not leaving crap in place because 'It's for something I don't understand.'

    That's what got us in the current OpenSSL mess in the first place.

    Additionally, once the core code is cleaned up you can always follow the changelogs and merge the legacy stuff back in (assuming they're using git, or another VCS with a good git check(in/out) module.)

    Honestly anyone still running any of those OSes is probably running a 0.9 series library and thus wasn't vulnerable to this bug to begin with. Who knows how many of those alternate paths even still worked anymore.

  7. Re:de Raadt by bluefoxlucid · · Score: 5, Interesting

    He is technically incapable of evaluating what's actually happening, and likes to go off-list when he's angry and wrong.

    The freelist is not an "exploit mitigation countermeasure", but rather standard allocation caching behavior that many high-rate allocation applications and algorithms implement--for example, ring buffers are common as all hell. The comment even says that it's done because performance on allocators is slow.

    Further, the only bug in Heartbleed was a READ OVERFLOW BUG caused by lack of input validation. It would actually read that a user said "This heartbeat is 65 thousand bytes long", allocate 65 thousand bytes plus room for instrumentation data, put instrumentation data in place, and then copy 65 thousand bytes from a request that was 1 byte long. While there are mitigation techniques, most allocators--anything that uses brk() to allocate the heap for allocations smaller than say 128KB (glibc's pmalloc and freebsd's kmalloc both use brk() until you ask for something bigger than 128KB, then use mmap())--don't do that. That's how this flaw worked: It would just read 64KB, most likely from the brk() area, and send it back to you.

    Read overflows don't kill canaries, so you wouldn't detect it except for with an unmapped page--a phenomena that doesn't happen with individual allocations smaller than 128KB in an allocator that uses brk(), like the default allocator on Linux and FreeBSD. Write overflows would kill canaries, but they actually allocated enough space to copy the too-large read into. And the code is, of course, correct for invalid input.

    Theo made a lot of noise about how all these other broken things were responsible for heartbleed, when the reality is one failed validation carries 100% of the weight for Heartbleed. If you perfectly cleaned up OpenSSL except for that single bug, slapped it on Linux with the default allocator, and ran it, it would still have the vulnerability. And it only behaves strange when being exploited--and any test would have sent back a big packet, raising questions.

    There was never really any hope that this was going to be caught before it was in the wild and "possibly had leaked your SSL keys'. It may have happened sooner, maybe, maybe not; but it still would have been a post-apocalyptic shit storm. And all those technical mitigations Theo is prattling on about would have helped if OpenSSL were cleaned up... AND if those technical mitigations were in Linux, not just OpenBSD.

  8. Re:Thanks! by TechyImmigrant · · Score: 5, Interesting

    No. We all love to hate on OpenSSL because it's a pile of poo.

    There are vested interests who make a living because they have write permissions to OpenSSL and they can charge companies to do it and the barrier to entry to others is really high because it's a undocumented, over complex pile of source.

    --
    I should use this sig to advertise my book ISBN-13 : 978-1501515132.
  9. Re:de Raadt by EvanED · · Score: 5, Informative

    The freelist is not an "exploit mitigation countermeasure",...

    He was being somewhat sarcastic, because OpenBSD's allocator is in contrast to

    Read overflows don't kill canaries, so you wouldn't detect it except for with an unmapped page--a phenomena that doesn't happen with individual allocations smaller than 128KB in an allocator that uses brk(), like the default allocator on Linux and FreeBSD

    and does try to separate allocations specifically to mitigate Heartbleed-style vulnerabilities.

    In other words, the OpenBSD allocatior does have exploit mitigation, and the OpenSSL freelist acts as a countermeasure to those mitigation capabilities whether it was intended or not.

    The comment even says that it's done because performance on allocators is slow.

    It says it's slow on "some platforms", yet they disabled it on all and then didn't test the alternative.

    But of course everyone knows it's way better to quickly implement a dramatically awful security vulnerability than to do things slowly and correctly.

  10. Re:Backport\Upstream? Seems unlikely by serviscope_minor · · Score: 5, Insightful

    they are choosing the greater of two evils.

    No.

    Eventually supporting too many screwy and ancient systems starts to cause just so many problems that it is really, really hard to write solid, well tested, clear code. The heartbleed bug was exactly a result of this. Because of supporting so many screwy platforms, they couldn't even rely on having malloc() work well. That means they had their own malloc implementation working from internal memory pools. Had they not, they would have benefited from the modern mmap() based implementations and you'd have got a segfault rather than a dump of the process memory.

    Supporting especially really old systems means having reimplementations of things which ought to be outside the scope of OpenSSL. Then you have to decide whether to always use the reimplementation or switch on demand between the custom one and the system one and whether or not to have some sort of quirk/bug correction.

    This sort of stuff starts to add up and lead to a maintainance nightmare.

    What OpenBSD are doing: throwing out all the accumulated crud and keeping the good parts is a fine way to proceed. It will almost certainly be portable to the other BSDs, OSX and Linux since they provide similar low level facilities. I doubt a port back to Windows would be hard because modern windows provides enough sane facilities that it's generally not too bad for heavily algorithmic code like this.

    Basically there's no way for them to get started except to first rationalise the code base and then audit it.

    --
    SJW n. One who posts facts.
  11. Re:de Raadt by bmajik · · Score: 5, Insightful

    Actually, it is you who are wrong.

    Theo's point from the beginning is that a custom allocator was used here, which removed any beneficial effects of both good platform allocators AND "evil" allocator tools.

    His response was a specific circumstance of the poor software engineering practices behind openSSL.

    Furthermore, at some point, openSSL became behaviorally dependant on its own allocator -- that is, when you tried to use a system allocator, it broke -- because it wasn't handing you back unmodified memory contents you had just freed.

    This dependency was known and documented. And not fixed.

    IMO, using a custom allocator is a bit like doing your own crypto. "Normal people" shouldn't do it.

    If you look at what open SSL is

    1) crypto software
    2) that is on by default
    3) that listens to the public internet
    4) that accepts data under the control of attackers ... you should already be squarely in the land of "doing every possible software engineering best practice possible". This is software that needs to be written differently than "normal" software; held to a higher standard, and correct for correctness sake.

    I would say that, "taking a hard dependence on my own custom allocator" and not investigating _why_ the platform allocator can no longer be used to give correct behavior is a _worst practice_. And its especially damning given how critical and predisposed to exploitability something like openSSL is.

    Yet that is what the openSSL team did. And they knew it. And they didn't care. And it caught up with them.

    The point of Theo's remarks is not to say "using a system allocator would have prevented bad code from being exploitable". The point is "having an engineering culture that ran tests using a system allocator and a debugging allocator would have prevented this bad code from staying around as long as it did"

    Let people swap the "fast" allocator back in at runtime, if you must. But make damn sure the code is correct enough to pass on "correctness checking" allocators.

    --
    My opinions are my own, and do not necessarily represent those of my employer.