Hacker News Reader

Comment by tptacek | Hacker News Reader

OpenSSL is extensively tested. These bugs are just hard to find.

I mean, branch coverage could have fingered this hole in the unit tests, couldn't it? The problem is X509_V_FLAG_X509_STRICT was added in 2004 without any corresponding tests. The project still has decades of debt from past poor practices to pay down.

I don't see how it could. If they had a test that X509_V_FLAG_X509_STRICT carried out the intended additional checks, and a test that non-CA certificates could not issue other certificates, and a test that valid certificate chains were accepted, wouldn't that cover all the branches involved without ever detecting the particular problematic combination involved in this bug?

I dunno, at least the new release slightly uh, changes?, the coverage.

  ~/openssl-1.1.1i % gcov crypto/x509/x509_vfy.c -bm
  File 'crypto/x509/x509_vfy.c'
  Lines executed:63.20% of 1625
  Branches executed:73.70% of 1194
  Taken at least once:53.85% of 1194
  Calls executed:58.47% of 496
  Creating 'x509_vfy.c.gcov'

  ~/openssl-1.1.1k % gcov crypto/x509/x509_vfy.c -bm
  File 'crypto/x509/x509_vfy.c'
  Lines executed:63.21% of 1628
  Branches executed:73.75% of 1196
  Taken at least once:53.93% of 1196
  Calls executed:58.47% of 496
  Creating 'x509_vfy.c.gcov'

There are still hundreds of never-taken branches in this file alone.

  ~/openssl-1.1.1k % grep -c never\ executed x509_vfy.c.gcov
  520

I 100% buy that OpenSSL hasn't dug itself out of the hole it was in back in 2012. But building a new TLS library puts you in the same hole with respect to exhaustive unit test coverage, right? You're better off starting from a place where you already have a fairly extensive (if incomplete) unit test infrastructure than one where you're starting from scratch.

This is a weakly held opinion. I'm mostly motivated by the knee-jerk response OpenSSL gets on threads like these; I think those responses are based on a reputation that doesn't capture the level of work that has gone into the project in the years since 2012. That doesn't mean it's the apogee of what we can do with a TLS library.

I agree that it is in much better shape today and any competing project starting from scratch would be at a disadvantage with respect to the size of their test suite. However, I also like the BoringSSL approach of forking the project and deleting all the dumb features that nobody needs. They deleted the option flag involved in this advisory, years ago. By deleting it they avoided having to add tests for it.

Ultimately I agree with you about what the right approach is, but the route there is convoluted. BoringSSL is able to be simpler by defining pretty tight criteria for what it cares about and everything else either doesn't work or isn't guaranteed to exist at all.

I happen to think the safe way forward is to agree that OK then, we can't do all these other weird things, too bad. But this is an extremely unpopular position because lots of people have at least one weird thing they very much want to do.

Ideally at least the integration tests are interchangeable between implementations of the same thing.

. . . the easy ones have been found and fixed. What remains are the hard ones.

Is it really extensively tested?

Should one even write these kinds of libraries in C?

Oh great, here we go again... I mean, which OS is available in something other than C?

Frankly, C has gotten us quite far in the systems engineering space. No matter what language, bugs will surface. Starting over on the hype-train isn’t always the best idea!

In an ideal world, if you could get the same calibre of talent patrolling the Rust port of OpenSSL and everyone using it, of course you'd want the Rust version.

We live in a fallen world. For a lot of software, the Rust-Go/legacy-C decision is trivial; you'd always take the Rust or Go. But OpenSSL is a tricky case and you can see why in today's announcement: there's a crasher memory corruption problem (but you can crash Rust programs too! today's bug is like a bad "unwrap"). But the first bug is an X.509 logic problem and I don't believe any practical language forecloses on those.

Meanwhile: OpenSSL gets a lot of attention and has a lot of built-up institutional knowledge that other libraries don't. I probably trust it more than I trust any of the other C-language TLS libraries.

In rust, you'd just make a ValidCert type and then it can never be invalid.

The problem seems deeper. From my overview of the hardware/ software interface deep down bellow our infrastructure we have built on top, modern CPUs, assembly, systems programming languages and compilers make it extra hard to do cryptography right. As I understand, most of the stuff is written with "in our experience it is constant time" attitude but I don't think it is guaranteed. CPUs can internally reorder stuff (basically the whole Spectre/ Meltdown and other timing related attacks use this) and we cannot be 100% sure basically about anything. Introduce compilers in the stack and some new optimizations might make previously constant time functions variable time under some circumstances.

Of course OpenSSL is a huge project and mixes many concerns, hard-to-use-right APIs and support for many platforms. This is basically the opposite of what cryptographers and security minded engineers strive for. On the other hand, I am not a cryptographer myself, I am not a C/ Rust/ Assembly or whatever master and I am very far from really understanding modern CPUs though I probably understand a lot more than the average programmer or sysadmin.

I just have the feeling with security, that it is a game of whack-a-mole rather than systematic engineering at times and that doesn't seem to be a sustainable approach.

"But the first bug is an X.509 logic problem and I don't believe any practical language forecloses on those."

I'm likely wrong (this isn't my field), but isn't this particular logic bug avoided by exhaustive enums?

I understand this bug took the form of

(1) a var was assigned an enum type, encoding a logic state ("which X.509 error")

(2) the var gets passed around to different functions, which handle the enum values case-by-case

(3) an update adds a new logic state at (1) (explicit ECC params), but doesn't update the case handling at (2) to handle the new value. Plus, since the error types overwrite each other into the same field, the new error type can clobber an older error type so that they both go unhandled.

If you use strict, exhaustive enums, this type of bug is excluded (isn't it?) The type of the enum will propagate to every place it's read, and the compiler will flag the non-exhaustive case handling in the stale code.

I don't know about that. Look at the diff, it looks like a counting bug. Rust programs also use counting logic, including in places where you could in theory model with exhaustive enums, but nobody does.

Crashing is fine. Continuing with unknown behavior is bad. With rust you might find some obscure bug which lets you crash the software and then docker/k8s/systemd will restart it automatically. The worst you could do is spam the bug and dos the server. But at least no user data is compromised.

There are some issues with cryptographic code in high level languages, so I'm not sure what is better. Even C is considered too high level some times.

Reply on news.ycombinator.com