OpenSSL Security Advisory (14 December 2021)
openssl.org
openssl.org
> OpenSSL 3.0.0 SSL/TLS clients are affected by this issue. > OpenSSL 1.1.1 and 1.0.2 are not affected by this issue.
I haven't encountered any stable distro running a higher version than 1.1.1. Arch, Debian Unstable, Alpine latest, and Ubuntu 20.10 are all on 1.1.1.
But this really isn't that unusual for a library of that complexity and size. All of these vulns are pretty boring, none is likely to cause any bigger harm.
https://github.com/openssl/openssl/commit/c1c1bb7c5e2baa109b...
i = X509_verify_cert(ctx);
/* We treat an error in the same way as a failure to verify */
if (i < 0)
i = 0;
The ssl verify returns
Verify a certificate chain
* Return codes:
* 1: Verify success
* 0: Verify failure or error
* -1: Retry required
*/
If your argument is that the return codes should be consistent across different layers of OpenSSL then I'm not going to disagree, but this is a large old codebase and this isn't some unique case.
int vr = X509_verify_cert(ctx);
if vr <= 0 {
i = 0;
} else {
i = 1;
}
And finally, i isn't a wonderful name for this variable. rv or ret or status are all better options.
Reasonable people disagree about this. John Ousterhout is in the "use meaningful variable names" camp, and Rob Pike is in the "use short variable names when the meaning is obvious" camp.
The reason I think "i" is bad here is that "i" usually (almost always?) means "index". You could return "i" if you were searching for the index of something, but that's not what this function is doing.
A lot of bugs have this pattern of layers of incongruity. "i" is usually an index, but here it's the return status value. These two sections of the library use different return status value semantics. You can kind of think about it like steering a ship through icebergs of inconsistency--eventually you're gonna hit one.
There are things you can do, and they all exist on the continuum of "easy/ineffective - hard/super effective". This function could use enum/#define. As a sibling comment said, if OpenSSL used typedef'd enums for its status codes the compiler would complain, and then you can use switch/case (maybe even wrapped in a conversion function) to convert between different universes of status codes. You could completely separate these sections of the library, which are currently very intermingled, and communicate between them using specific interfaces.
As an example for that last one, this code doesn't care about various error conditions when it comes to cert verification, it just wants to know if the cert checks out or not. So when X509_verify_cert returns more than 2 possible values, it's leaking irrelevant information. The X509 section could have a different function that returned a boolean, specifically for uses that don't care about the details of certificate verification failure.
And actually, it looks like every single use of X509_verify_cert only checks for > 0. Nothing cares about 0 vs. < 0. Was it ever important for something external to know if there were internal errors vs. the certificate failed? Is that even a good idea at all?
These are the kinds of things you look into and think, "what is going on here". Evidence starts mounting that like, this thing isn't built very well (or this gem [1], the product of a code formatter, making one worry about potential other goto fail problems)
[1]: https://github.com/openssl/openssl/blob/1c0eede9827b0962f1d7...
The biggest lie in software is that new software is better than old software. Anyone who's used software for long enough knows this is ridiculous. You introduce some new software which has some new features indented to fix an old flaw. But then it turns out there's new flaws in the new software that become more of an issue than the old flaws.
Most tools in the world don't change significantly over time. Instead of changing the tool, people learn how to use the tool to avoid problems. But that doesn't work in a world where nobody learns how to use the tool right, or is constantly picking up new tools and never learning lessons.
Not sure I’d call him “crappy.” Not to his face, anyway …
FTFY
Further, snark could be made about how this will have near zero security impact yet there’s a far more serious bug (log4j) which has already had significant cost and follows endless patterns of Java developers embedding logic flow in untrusted data. “Just don’t use Java lol” etc
Edit: Hahaha, hey David. Had no idea that was you.