7 ms·
Vulnerability #2 is a good example of why OpenSSL is a minefield even for a competent coder. For such a security-critical library it's pretty insane that the AP
by simias 9y ago
Vulnerability #2 is a good example of why OpenSSL is a minefield even for a competent coder. For such a security-critical library it's pretty insane that the API is so unfriendly, bordering on hostile.
> The correct way to do this is to call GENERAL_NAMES_free. This is because sk_GENERAL_NAME_free frees only the containing structure, whereas GENERAL_NAMES_free frees the structure AND its items.
And later:
> Here, the code assumes that a return value that is negative or zero indicates failure, and ‘buf’ is not initialized, and needs not to be freed. But in fact, this is ONLY the case if ASN1_STRING_to_UTF8 returns a negative value. A return value 0 simply means a string of length 0, but memory is nonetheless allocated, so there are memory leaks here as well.
It mirrors my experience working with OpenSSL: you have to quadruple check each function invocation with the docs to make sure you got it right. You're never sure at a glance what's an input or an output parameter, what needs to be freed and how you're supposed to free it. What's the return value in case of error? 0? -1? <0? <=0? And then you have the macro soup with their STACK_OF "abstraction" (in the leakiest sense of the word) that just serves to make the code harder to reason about and is a huge pain when you want to create OpenSSL bindings for an other language.
Let me be clear: I don't blame the OpenSSL devs in any way. It's free, it's open source, they don't get a ton of money for that. Instead I blame all the big corporations who use OpenSSL "as-is", directly or indirectly, and don't invest some money to improve that mess. Maybe after a couple more Heartbleed-like vulnerabilities they'll take it a little more seriously.
- mikeash 9y ago"The correct way to do this is to call GENERAL_NAMES_free. This is because sk_GENERAL_NAME_free frees only the containing structure, whereas GENERAL_NAMES_free frees the structure AND its items." Holy moly. It took me about five attempts just to spot the difference in the two names. Can it even be fixed at this point, or does it need to start fresh?
- dcousens 9y agoThis mirrored my confusion when `AES_cbc_encrypt` modified my IV data when _decrypting_.
- andy_ppp 9y ago> You're never sure at a glance what's an input or an output parameter, what needs to be freed and how you're supposed to free it. Terrifying; would a more functional style of programming help here?
- dom0 9y agoProper API design and documentation would be quite sufficient, breaking and expensive.
- simias 9y agoUsing "const" consistently would be a good start. Better and more thorough docs would be nice as well. If you're willing to break everything a more consistent API in general, especially when it comes to error handling, would make it less easy to shoot yourself in the foot. It's the overall inconsistency of the OpenSSL API that's the main issue IMO. If the coder or code reviewer don't know all the function signatures and return values by heart it's very easy to write code that looks absolutely fine, might seem to run fine but is thoroughly broken. TFA is a good example of that. That's why nowadays I much prefer using the Rust bindings over the raw C library. The error handling and ownership rules are checked by the compiler, it's like putting blanks in your footgun. It's liberating really.
- ianai 9y agoMaybe a wrapper API for OpenVPN/SSL that attempts to Do It Rightunder the hood would be worthwhile?
- dom0 9y ago> you have to quadruple check each function invocation with the docs to make sure you got it right. If the docs contain that information ¯\_(ツ)_/¯
- kbenson 9y agoThere's always a trade-off between how complicated the docs and usage are and how many people will use it. OpenSSL is problematic because it has very complicated docs and usage, but people still used it anyway. This leads to a situation where people use it incorrectly, and for a crypto library, that's a very bad thing. If OpenSSL wasn't the de facto king of OSS crypto libraries for so long (partly because it was the only one for so long), then people might have been able to make more informed choices about what library was better.
- the_why_of_y 9y ago> If OpenSSL wasn't the de facto king of OSS crypto libraries for so long (partly because it was the only one for so long), Netscape released the NSS library in 1998, and as far as I can tell it hasn't even changed its ABI incompatibly since 2000, but perhaps this is too recent for programmers to take notice.
- kbenson 9y agoI have no idea why NSS never got more use, but it didn't, for the most part. I assume there's a reason, but for the most part major adoption seemed to have been limited to Mozilla projects, Sun Enterprise Java, and OpenOffice (possibly because it also originated from a Sun project, and it seems to have enjoyed some popularity at Sun). If I had to make a wild guess from that info, it would be that it's more complex to work with, as odd as that sounds given the current context (although I guess it's possible OpenSSL got more complex over time, and was much simpler a decade ago).
- ibotty 9y agoFor one, NSS is not the kitchen sync that is openssl. With openssl you can do anything with any file you can get from the anywhere. Secondly, when using openssl-based servers it's usually easier to specify certificates and keys. You don't have to import them into a database, you only configure the filesystem path.
- vog 9y agoThe LibreSSL fork tries to do exactly that: Although they still provide the original OpenSSL API for existing applications, they add a much simpler and hassle-free API on top of it ("libtls" [1]), with the goal that all applications will be switched part-by-part to the new interface. [1] https://www.openbsd.org/papers/libtls-fsec-2015/ https://www.openbsd.org/papers/libtls-fsec-2015/
- baby 9y agoDoes it make sense to stop recommending OpenSSL and start recommending LibreSSL now?
- wbl 9y agoYes. Or tell people to use mebd or s2n or the other one that doesn't allocate. Don't use OpenSSL.
- baby 9y agoBearSSL
- vog 9y agoYes, if only because LibreSSL is still a working drop-in replacement (which is nice to have as a first step of adoption), and because in LibreSSL they threw away lots of legacy code. This throwing away of legacy code should not be underestimated. There were quite a lot of recent OpenSSL security issues from which LibreSSL had just a fraction, simply because the vulnerable code is no longer present at all in LibreSSL.
- peterwwillis 9y agoThere's no reason they couldn't use more object-oriented return methods to make it easier to grok what's going on.